From a76ed28fe7c1f71bb0e1b9bd94d08ff3d25b15bc Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 16 Jun 2020 12:13:40 +0200 Subject: [PATCH] DATACMNS-1745 - Refactor CustomAnnotationTransactionAttributeSource to use computeTransactionAttribute template method. We now no longer duplicate Spring Framework code as AbstractFallbackTransactionAttributeSource exposes computeTransactionAttribute which we can use to override to determine TransactionAttribute from the repository interface. Renamed CustomAnnotationTransactionAttributeSource to RepositoryInformationPreferringAnnotationTransactionAttributeSource to reflect the nature of our customization. --- ...sactionalRepositoryProxyPostProcessor.java | 391 ++---------------- ...nTransactionAttributeSourceUnitTests.java} | 8 +- ...RepositoryProxyPostProcessorUnitTests.java | 9 +- 3 files changed, 41 insertions(+), 367 deletions(-) rename src/test/java/org/springframework/data/repository/core/support/{CustomAnnotationTransactionAttributeSourceUnitTests.java => RepositoryInformationPreferringAnnotationTransactionAttributeSourceUnitTests.java} (81%) diff --git a/src/main/java/org/springframework/data/repository/core/support/TransactionalRepositoryProxyPostProcessor.java b/src/main/java/org/springframework/data/repository/core/support/TransactionalRepositoryProxyPostProcessor.java index a5e6a5861..95fb67cb9 100644 --- a/src/main/java/org/springframework/data/repository/core/support/TransactionalRepositoryProxyPostProcessor.java +++ b/src/main/java/org/springframework/data/repository/core/support/TransactionalRepositoryProxyPostProcessor.java @@ -16,17 +16,9 @@ package org.springframework.data.repository.core.support; import java.io.Serializable; -import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; + import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.ListableBeanFactory; @@ -34,18 +26,12 @@ import org.springframework.core.BridgeMethodResolver; import org.springframework.dao.support.PersistenceExceptionTranslationInterceptor; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.util.ProxyUtils; -import org.springframework.transaction.annotation.Ejb3TransactionAnnotationParser; -import org.springframework.transaction.annotation.JtaTransactionAnnotationParser; -import org.springframework.transaction.annotation.SpringTransactionAnnotationParser; -import org.springframework.transaction.annotation.TransactionAnnotationParser; -import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.interceptor.DefaultTransactionAttribute; +import org.springframework.lang.Nullable; +import org.springframework.transaction.annotation.AnnotationTransactionAttributeSource; import org.springframework.transaction.interceptor.TransactionAttribute; -import org.springframework.transaction.interceptor.TransactionAttributeSource; import org.springframework.transaction.interceptor.TransactionInterceptor; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; -import org.springframework.util.ObjectUtils; /** * {@link RepositoryProxyPostProcessor} to add transactional behaviour to repository proxies. Adds a @@ -54,6 +40,7 @@ import org.springframework.util.ObjectUtils; * * @author Oliver Gierke * @author Christoph Strobl + * @author Mark Paluch */ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostProcessor { @@ -86,12 +73,11 @@ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostPr */ public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) { - CustomAnnotationTransactionAttributeSource transactionAttributeSource = new CustomAnnotationTransactionAttributeSource(); - transactionAttributeSource.setRepositoryInformation(repositoryInformation); - transactionAttributeSource.setEnableDefaultTransactions(enableDefaultTransactions); + RepositoryInformationPreferringAnnotationTransactionAttributeSource transactionAttributeSource = new RepositoryInformationPreferringAnnotationTransactionAttributeSource( + enableDefaultTransactions, repositoryInformation); - @SuppressWarnings("null") // TODO: Remove - TransactionInterceptor transactionInterceptor = new TransactionInterceptor(null, transactionAttributeSource); + TransactionInterceptor transactionInterceptor = new TransactionInterceptor(); + transactionInterceptor.setTransactionAttributeSource(transactionAttributeSource); transactionInterceptor.setTransactionManagerBeanName(transactionManagerName); transactionInterceptor.setBeanFactory(beanFactory); transactionInterceptor.afterPropertiesSet(); @@ -99,303 +85,55 @@ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostPr factory.addAdvice(transactionInterceptor); } - // The section below contains copies of two core Spring classes that slightly modify the algorithm transaction - // configuration is discovered. The original Spring implementation favours the implementation class' transaction - // configuration over one declared at an interface. As we need to provide the capability to override transaction - // configuration of the implementation at the interface level we pretty much invert this logic to inspect the - // originally invoked method first before digging down into the implementation class. - // - // Unfortunately the Spring classes do not allow modifying this algorithm easily. That's why we have to copy the two - // classes 1:1. Only modifications done are inside - // AbstractFallbackTransactionAttributeSource#computeTransactionAttribute(Method, Class). - /** - * Implementation of the {@link org.springframework.transaction.interceptor.TransactionAttributeSource} interface for - * working with transaction metadata in JDK 1.5+ annotation format. + * Custom implementation of {@link AnnotationTransactionAttributeSource} that that slightly modify the algorithm + * transaction configuration is discovered. *

- * This class reads Spring's JDK 1.5+ {@link Transactional} annotation and exposes corresponding transaction - * attributes to Spring's transaction infrastructure. Also supports JTA 1.2's {@link javax.transaction.Transactional} - * and EJB3's {@link javax.ejb.TransactionAttribute} annotation (if present). This class may also serve as base class - * for a custom TransactionAttributeSource, or get customized through {@link TransactionAnnotationParser} strategies. + * The original Spring implementation favours the implementation class' transaction configuration over one declared at + * an interface. As we need to provide the capability to override transaction configuration of the implementation at + * the interface level we pretty much invert this logic to inspect the originally invoked method first before digging + * down into the implementation class. * - * @author Colin Sampaleanu - * @author Juergen Hoeller - * @since 1.2 - * @see Transactional - * @see TransactionAnnotationParser - * @see SpringTransactionAnnotationParser - * @see Ejb3TransactionAnnotationParser - * @see org.springframework.transaction.interceptor.TransactionInterceptor#setTransactionAttributeSource - * @see org.springframework.transaction.interceptor.TransactionProxyFactoryBean#setTransactionAttributeSource + * @author Oliver Drotbohm + * @author Mark Paluch */ - @SuppressWarnings({ "serial", "null" }) - static class CustomAnnotationTransactionAttributeSource extends AbstractFallbackTransactionAttributeSource - implements Serializable { - - private static final boolean jta12Present = ClassUtils.isPresent("javax.transaction.Transactional", - CustomAnnotationTransactionAttributeSource.class.getClassLoader()); + static class RepositoryInformationPreferringAnnotationTransactionAttributeSource + extends AnnotationTransactionAttributeSource implements Serializable { - private static final boolean ejb3Present = ClassUtils.isPresent("javax.ejb.TransactionAttribute", - CustomAnnotationTransactionAttributeSource.class.getClassLoader()); + private final boolean enableDefaultTransactions; - private final boolean publicMethodsOnly; - - private final Set annotationParsers; + private final @Nullable RepositoryInformation repositoryInformation; /** * Create a default CustomAnnotationTransactionAttributeSource, supporting public methods that carry the * {@code Transactional} annotation or the EJB3 {@link javax.ejb.TransactionAttribute} annotation. */ - public CustomAnnotationTransactionAttributeSource() { - this(true); + public RepositoryInformationPreferringAnnotationTransactionAttributeSource() { + this(true, null); } /** - * Create a custom CustomAnnotationTransactionAttributeSource, supporting public methods that carry the + * Create a default CustomAnnotationTransactionAttributeSource, supporting public methods that carry the * {@code Transactional} annotation or the EJB3 {@link javax.ejb.TransactionAttribute} annotation. - * - * @param publicMethodsOnly whether to support public methods that carry the {@code Transactional} annotation only - * (typically for use with proxy-based AOP), or protected/private methods as well (typically used with - * AspectJ class weaving) - */ - public CustomAnnotationTransactionAttributeSource(boolean publicMethodsOnly) { - this.publicMethodsOnly = publicMethodsOnly; - this.annotationParsers = new LinkedHashSet<>(2); - this.annotationParsers.add(new SpringTransactionAnnotationParser()); - if (jta12Present) { - this.annotationParsers.add(new JtaTransactionAnnotationParser()); - } - if (ejb3Present) { - this.annotationParsers.add(new Ejb3TransactionAnnotationParser()); - } - } - - /** - * Create a custom CustomAnnotationTransactionAttributeSource. - * - * @param annotationParser the TransactionAnnotationParser to use - */ - public CustomAnnotationTransactionAttributeSource(TransactionAnnotationParser annotationParser) { - this.publicMethodsOnly = true; - Assert.notNull(annotationParser, "TransactionAnnotationParser must not be null"); - this.annotationParsers = Collections.singleton(annotationParser); - } - - /** - * Create a custom CustomAnnotationTransactionAttributeSource. - * - * @param annotationParsers the TransactionAnnotationParsers to use - */ - public CustomAnnotationTransactionAttributeSource(TransactionAnnotationParser... annotationParsers) { - this.publicMethodsOnly = true; - Assert.notEmpty(annotationParsers, "At least one TransactionAnnotationParser needs to be specified"); - Set parsers = new LinkedHashSet<>(annotationParsers.length); - Collections.addAll(parsers, annotationParsers); - this.annotationParsers = parsers; - } - - /** - * Create a custom CustomAnnotationTransactionAttributeSource. - * - * @param annotationParsers the TransactionAnnotationParsers to use - */ - public CustomAnnotationTransactionAttributeSource(Set annotationParsers) { - this.publicMethodsOnly = true; - Assert.notEmpty(annotationParsers, "At least one TransactionAnnotationParser needs to be specified"); - this.annotationParsers = annotationParsers; - } - - @Override - protected TransactionAttribute findTransactionAttribute(Method method) { - return determineTransactionAttribute(method); - } - - @Override - protected TransactionAttribute findTransactionAttribute(Class clazz) { - return determineTransactionAttribute(clazz); - } - - /** - * Determine the transaction attribute for the given method or class. - *

- * This implementation delegates to configured {@link TransactionAnnotationParser TransactionAnnotationParsers} for - * parsing known annotations into Spring's metadata attribute class. Returns {@code null} if it's not transactional. - *

- * Can be overridden to support custom annotations that carry transaction metadata. - * - * @param ae the annotated method or class - * @return TransactionAttribute the configured transaction attribute, or {@code null} if none was found - */ - protected TransactionAttribute determineTransactionAttribute(AnnotatedElement ae) { - for (TransactionAnnotationParser annotationParser : this.annotationParsers) { - TransactionAttribute attr = annotationParser.parseTransactionAnnotation(ae); - if (attr != null) { - return attr; - } - } - return null; - } - - /** - * By default, only public methods can be made transactional. - */ - @Override - protected boolean allowPublicMethodsOnly() { - return this.publicMethodsOnly; - } - - /* - * (non-Javadoc) - * @see java.lang.Object#equals(java.lang.Object) - */ - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (!(other instanceof CustomAnnotationTransactionAttributeSource)) { - return false; - } - CustomAnnotationTransactionAttributeSource otherTas = (CustomAnnotationTransactionAttributeSource) other; - return (this.annotationParsers.equals(otherTas.annotationParsers) - && this.publicMethodsOnly == otherTas.publicMethodsOnly); - } - - /* - * (non-Javadoc) - * @see java.lang.Object#hashCode() - */ - @Override - public int hashCode() { - return this.annotationParsers.hashCode(); - } - } - - /** - * Abstract implementation of {@link TransactionAttributeSource} that caches attributes for methods and implements a - * fallback policy: 1. specific target method; 2. target class; 3. declaring method; 4. declaring class/interface. - *

- * Defaults to using the target class's transaction attribute if none is associated with the target method. Any - * transaction attribute associated with the target method completely overrides a class transaction attribute. If none - * found on the target class, the interface that the invoked method has been called through (in case of a JDK proxy) - * will be checked. - *

- * This implementation caches attributes by method after they are first used. If it is ever desirable to allow dynamic - * changing of transaction attributes (which is very unlikely), caching could be made configurable. Caching is - * desirable because of the cost of evaluating rollback rules. - * - * @author Rod Johnson - * @author Juergen Hoeller - * @since 1.1 - */ - @SuppressWarnings({ "null", "unused" }) - abstract static class AbstractFallbackTransactionAttributeSource implements TransactionAttributeSource { - - /** - * Canonical value held in cache to indicate no transaction attribute was found for this method, and we don't need - * to look again. - */ - private final static TransactionAttribute NULL_TRANSACTION_ATTRIBUTE = new DefaultTransactionAttribute(); - - /** - * Logger available to subclasses. - *

- * As this base class is not marked Serializable, the logger will be recreated after serialization - provided that - * the concrete subclass is Serializable. - */ - protected final Logger logger = LoggerFactory.getLogger(getClass()); - - /** - * Cache of TransactionAttributes, keyed by DefaultCacheKey (Method + target Class). - *

- * As this base class is not marked Serializable, the cache will be recreated after serialization - provided that - * the concrete subclass is Serializable. */ - final Map attributeCache = new ConcurrentHashMap<>(); - - private RepositoryInformation repositoryInformation; - private boolean enableDefaultTransactions = true; - - /** - * @param repositoryInformation the repositoryInformation to set - */ - public void setRepositoryInformation(RepositoryInformation repositoryInformation) { - this.repositoryInformation = repositoryInformation; - } - - /** - * @param enableDefaultTransactions the enableDefaultTransactions to set - */ - public void setEnableDefaultTransactions(boolean enableDefaultTransactions) { + public RepositoryInformationPreferringAnnotationTransactionAttributeSource(boolean enableDefaultTransactions, + @Nullable RepositoryInformation repositoryInformation) { + super(true); this.enableDefaultTransactions = enableDefaultTransactions; + this.repositoryInformation = repositoryInformation; } - /** - * Determine the transaction attribute for this method invocation. - *

- * Defaults to the class's transaction attribute if no method attribute is found. - * - * @param method the method for the current invocation (never null) - * @param targetClass the target class for this invocation (may be null) - * @return TransactionAttribute for this method, or null if the method is not transactional - */ - public TransactionAttribute getTransactionAttribute(Method method, Class targetClass) { - // First, see if we have a cached value. - Object cacheKey = getCacheKey(method, targetClass); - Object cached = this.attributeCache.get(cacheKey); - if (cached != null) { - // Value will either be canonical value indicating there is no transaction attribute, - // or an actual transaction attribute. - if (cached == NULL_TRANSACTION_ATTRIBUTE) { - return null; - } else { - return (TransactionAttribute) cached; - } - } else { - // We need to work it out. - TransactionAttribute txAtt = computeTransactionAttribute(method, targetClass); - // Put it in the cache. - if (txAtt == null) { - this.attributeCache.put(cacheKey, NULL_TRANSACTION_ATTRIBUTE); - } else { - if (logger.isDebugEnabled()) { - logger.debug("Adding transactional method '" + method.getName() + "' with attribute: " + txAtt); - } - this.attributeCache.put(cacheKey, txAtt); - } - return txAtt; - } - } - - /** - * Determine a cache key for the given method and target class. - *

- * Must not produce same key for overloaded methods. Must produce same key for different instances of the same - * method. - * - * @param method the method (never null) - * @param targetClass the target class (may be null) - * @return the cache key (never null) - */ - protected Object getCacheKey(Method method, Class targetClass) { - return new DefaultCacheKey(method, targetClass); - } + @Override + protected TransactionAttribute computeTransactionAttribute(Method method, @Nullable Class targetClass) { - /** - * Same signature as {@link #getTransactionAttribute}, but doesn't cache the result. - * {@link #getTransactionAttribute} is effectively a caching decorator for this method. - * - * @see #getTransactionAttribute - */ - private TransactionAttribute computeTransactionAttribute(Method method, Class targetClass) { // Don't allow no-public methods as required. if (allowPublicMethodsOnly() && !Modifier.isPublic(method.getModifiers())) { return null; } // Ignore CGLIB subclasses - introspect the actual user class. - Class userClass = ProxyUtils.getUserClass(targetClass); + Class userClass = targetClass != null ? ProxyUtils.getUserClass(targetClass) : targetClass; // The method may be on an interface, but we need attributes from the target class. // If the target class is null, the method will be unchanged. Method specificMethod = ClassUtils.getMostSpecificMethod(method, userClass); @@ -432,7 +170,7 @@ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostPr return txAtt; } - if (!enableDefaultTransactions) { + if (!enableDefaultTransactions || repositoryInformation == null) { return null; } @@ -457,71 +195,6 @@ class TransactionalRepositoryProxyPostProcessor implements RepositoryProxyPostPr return null; // End: Implementation class check block } - - /** - * Subclasses need to implement this to return the transaction attribute for the given method, if any. - * - * @param method the method to retrieve the attribute for - * @return all transaction attribute associated with this method (or null if none) - */ - protected abstract TransactionAttribute findTransactionAttribute(Method method); - - /** - * Subclasses need to implement this to return the transaction attribute for the given class, if any. - * - * @param clazz the class to retrieve the attribute for - * @return all transaction attribute associated with this class (or null if none) - */ - protected abstract TransactionAttribute findTransactionAttribute(Class clazz); - - /** - * Should only public methods be allowed to have transactional semantics? - *

- * The default implementation returns false. - */ - protected boolean allowPublicMethodsOnly() { - return false; - } - - /** - * Default cache key for the TransactionAttribute cache. - */ - private static class DefaultCacheKey { - - private final Method method; - - private final Class targetClass; - - public DefaultCacheKey(Method method, Class targetClass) { - this.method = method; - this.targetClass = targetClass; - } - - /* - * (non-Javadoc) - * @see java.lang.Object#equals(java.lang.Object) - */ - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (!(other instanceof DefaultCacheKey)) { - return false; - } - DefaultCacheKey otherKey = (DefaultCacheKey) other; - return this.method.equals(otherKey.method) - && ObjectUtils.nullSafeEquals(this.targetClass, otherKey.targetClass); - } - - /* - * (non-Javadoc) - * @see java.lang.Object#hashCode() - */ - @Override - public int hashCode() { - return this.method.hashCode() * 29 + (this.targetClass != null ? this.targetClass.hashCode() : 0); - } - } } + } diff --git a/src/test/java/org/springframework/data/repository/core/support/CustomAnnotationTransactionAttributeSourceUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/RepositoryInformationPreferringAnnotationTransactionAttributeSourceUnitTests.java similarity index 81% rename from src/test/java/org/springframework/data/repository/core/support/CustomAnnotationTransactionAttributeSourceUnitTests.java rename to src/test/java/org/springframework/data/repository/core/support/RepositoryInformationPreferringAnnotationTransactionAttributeSourceUnitTests.java index 7a102ff72..2ae4600f8 100755 --- a/src/test/java/org/springframework/data/repository/core/support/CustomAnnotationTransactionAttributeSourceUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/RepositoryInformationPreferringAnnotationTransactionAttributeSourceUnitTests.java @@ -18,21 +18,21 @@ package org.springframework.data.repository.core.support; import static org.assertj.core.api.Assertions.*; import org.junit.jupiter.api.Test; -import org.springframework.data.repository.core.support.TransactionalRepositoryProxyPostProcessor.CustomAnnotationTransactionAttributeSource; +import org.springframework.data.repository.core.support.TransactionalRepositoryProxyPostProcessor.RepositoryInformationPreferringAnnotationTransactionAttributeSource; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.interceptor.TransactionAttribute; /** - * Unit tests for {@link CustomAnnotationTransactionAttributeSource}. + * Unit tests for {@link RepositoryInformationPreferringAnnotationTransactionAttributeSource}. * * @author Oliver Gierke */ -class CustomAnnotationTransactionAttributeSourceUnitTests { +class RepositoryInformationPreferringAnnotationTransactionAttributeSourceUnitTests { @Test void usesCustomTransactionConfigurationOnInterface() throws SecurityException, NoSuchMethodException { - CustomAnnotationTransactionAttributeSource source = new TransactionalRepositoryProxyPostProcessor.CustomAnnotationTransactionAttributeSource(); + RepositoryInformationPreferringAnnotationTransactionAttributeSource source = new RepositoryInformationPreferringAnnotationTransactionAttributeSource(); TransactionAttribute attribute = source.getTransactionAttribute(Bar.class.getMethod("bar", Object.class), FooImpl.class); diff --git a/src/test/java/org/springframework/data/repository/core/support/TransactionRepositoryProxyPostProcessorUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/TransactionRepositoryProxyPostProcessorUnitTests.java index a416a5112..28211f30c 100755 --- a/src/test/java/org/springframework/data/repository/core/support/TransactionRepositoryProxyPostProcessorUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/TransactionRepositoryProxyPostProcessorUnitTests.java @@ -31,7 +31,7 @@ import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.RepositoryInformation; -import org.springframework.data.repository.core.support.TransactionalRepositoryProxyPostProcessor.CustomAnnotationTransactionAttributeSource; +import org.springframework.data.repository.core.support.TransactionalRepositoryProxyPostProcessor.RepositoryInformationPreferringAnnotationTransactionAttributeSource; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.interceptor.TransactionAttribute; import org.springframework.transaction.interceptor.TransactionAttributeSource; @@ -41,6 +41,7 @@ import org.springframework.transaction.interceptor.TransactionInterceptor; * Unit test for {@link TransactionalRepositoryProxyPostProcessor}. * * @author Oliver Gierke + * @author Mark Paluch */ @ExtendWith(MockitoExtension.class) class TransactionRepositoryProxyPostProcessorUnitTests { @@ -86,7 +87,7 @@ class TransactionRepositoryProxyPostProcessorUnitTests { Method method = SampleRepository.class.getMethod("methodWithJtaOneDotTwoAtTransactional"); - TransactionAttributeSource attributeSource = new CustomAnnotationTransactionAttributeSource(); + TransactionAttributeSource attributeSource = new RepositoryInformationPreferringAnnotationTransactionAttributeSource(); TransactionAttribute attribute = attributeSource.getTransactionAttribute(method, SampleRepository.class); assertThat(attribute).isNotNull(); @@ -99,8 +100,8 @@ class TransactionRepositoryProxyPostProcessorUnitTests { when(repositoryInformation.getTargetClassMethod(repositorySaveMethod)).thenReturn(implementationClassMethod); - CustomAnnotationTransactionAttributeSource attributeSource = new CustomAnnotationTransactionAttributeSource(); - attributeSource.setRepositoryInformation(repositoryInformation); + RepositoryInformationPreferringAnnotationTransactionAttributeSource attributeSource = new RepositoryInformationPreferringAnnotationTransactionAttributeSource( + true, repositoryInformation); TransactionAttribute attribute = attributeSource.getTransactionAttribute(repositorySaveMethod, SampleImplementation.class);