From edd1e9134f4d5a76ca652ee6df7b5d4d1461cecd Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 18 Sep 2023 17:17:56 +0200 Subject: [PATCH 1/3] Polishing --- .../org/springframework/aop/ClassFilter.java | 12 +-- .../springframework/aop/MethodMatcher.java | 61 ++++++++------- .../aop/aspectj/AspectJPointcutAdvisor.java | 2 +- .../aop/framework/AdvisedSupport.java | 42 +++++----- .../CacheOperationSourcePointcut.java | 2 +- .../TransactionAttributeSourcePointcut.java | 2 +- ...AnnotationTransactionInterceptorTests.java | 76 +++++++++---------- 7 files changed, 101 insertions(+), 96 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java b/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java index fa8e2066c0..de20021fd6 100644 --- a/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java +++ b/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,11 +17,11 @@ package org.springframework.aop; /** - * Filter that restricts matching of a pointcut or introduction to - * a given set of target classes. + * Filter that restricts matching of a pointcut or introduction to a given set + * of target classes. * - *

Can be used as part of a {@link Pointcut} or for the entire - * targeting of an {@link IntroductionAdvisor}. + *

Can be used as part of a {@link Pointcut} or for the entire targeting of + * an {@link IntroductionAdvisor}. * *

Concrete implementations of this interface typically should provide proper * implementations of {@link Object#equals(Object)} and {@link Object#hashCode()} @@ -44,7 +44,7 @@ public interface ClassFilter { /** - * Canonical instance of a ClassFilter that matches all classes. + * Canonical instance of a {@code ClassFilter} that matches all classes. */ ClassFilter TRUE = TrueClassFilter.INSTANCE; diff --git a/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java b/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java index d5f8c2a89a..71f19e97dd 100644 --- a/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java +++ b/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,24 +21,25 @@ import java.lang.reflect.Method; /** * Part of a {@link Pointcut}: Checks whether the target method is eligible for advice. * - *

A MethodMatcher may be evaluated statically or at runtime (dynamically). - * Static matching involves method and (possibly) method attributes. Dynamic matching - * also makes arguments for a particular call available, and any effects of running - * previous advice applying to the joinpoint. + *

A {@code MethodMatcher} may be evaluated statically or at runtime + * (dynamically). Static matching involves a method and (possibly) method attributes. + * Dynamic matching also makes arguments for a particular call available, and any + * effects of running previous advice applying to the joinpoint. * *

If an implementation returns {@code false} from its {@link #isRuntime()} * method, evaluation can be performed statically, and the result will be the same * for all invocations of this method, whatever their arguments. This means that * if the {@link #isRuntime()} method returns {@code false}, the 3-arg - * {@link #matches(java.lang.reflect.Method, Class, Object[])} method will never be invoked. + * {@link #matches(Method, Class, Object[])} method will never be invoked. * *

If an implementation returns {@code true} from its 2-arg - * {@link #matches(java.lang.reflect.Method, Class)} method and its {@link #isRuntime()} method - * returns {@code true}, the 3-arg {@link #matches(java.lang.reflect.Method, Class, Object[])} - * method will be invoked immediately before each potential execution of the related advice, - * to decide whether the advice should run. All previous advice, such as earlier interceptors - * in an interceptor chain, will have run, so any state changes they have produced in - * parameters or ThreadLocal state will be available at the time of evaluation. + * {@link #matches(Method, Class)} method and its {@link #isRuntime()} method + * returns {@code true}, the 3-arg {@link #matches(Method, Class, Object[])} + * method will be invoked immediately before each potential execution of the + * related advice to decide whether the advice should run. All previous advice, + * such as earlier interceptors in an interceptor chain, will have run, so any + * state changes they have produced in parameters or {@code ThreadLocal} state will + * be available at the time of evaluation. * *

Concrete implementations of this interface typically should provide proper * implementations of {@link Object#equals(Object)} and {@link Object#hashCode()} @@ -53,11 +54,10 @@ import java.lang.reflect.Method; public interface MethodMatcher { /** - * Perform static checking whether the given method matches. - *

If this returns {@code false} or if the {@link #isRuntime()} - * method returns {@code false}, no runtime check (i.e. no - * {@link #matches(java.lang.reflect.Method, Class, Object[])} call) - * will be made. + * Perform static checking to determine whether the given method matches. + *

If this method returns {@code false} or if {@link #isRuntime()} + * returns {@code false}, no runtime check (i.e. no + * {@link #matches(Method, Class, Object[])} call) will be made. * @param method the candidate method * @param targetClass the target class * @return whether this method matches statically @@ -65,36 +65,35 @@ public interface MethodMatcher { boolean matches(Method method, Class targetClass); /** - * Is this MethodMatcher dynamic, that is, must a final call be made on the - * {@link #matches(java.lang.reflect.Method, Class, Object[])} method at - * runtime even if the 2-arg matches method returns {@code true}? + * Is this {@code MethodMatcher} dynamic, that is, must a final check be made + * via the {@link #matches(Method, Class, Object[])} method at runtime even + * if {@link #matches(Method, Class)} returns {@code true}? *

Can be invoked when an AOP proxy is created, and need not be invoked - * again before each method invocation, - * @return whether a runtime match via the 3-arg - * {@link #matches(java.lang.reflect.Method, Class, Object[])} method + * again before each method invocation. + * @return whether a runtime match via {@link #matches(Method, Class, Object[])} * is required if static matching passed */ boolean isRuntime(); /** - * Check whether there a runtime (dynamic) match for this method, - * which must have matched statically. - *

This method is invoked only if the 2-arg matches method returns - * {@code true} for the given method and target class, and if the - * {@link #isRuntime()} method returns {@code true}. Invoked - * immediately before potential running of the advice, after any + * Check whether there is a runtime (dynamic) match for this method, which + * must have matched statically. + *

This method is invoked only if {@link #matches(Method, Class)} returns + * {@code true} for the given method and target class, and if + * {@link #isRuntime()} returns {@code true}. + *

Invoked immediately before potential running of the advice, after any * advice earlier in the advice chain has run. * @param method the candidate method * @param targetClass the target class * @param args arguments to the method * @return whether there's a runtime match - * @see MethodMatcher#matches(Method, Class) + * @see #matches(Method, Class) */ boolean matches(Method method, Class targetClass, Object... args); /** - * Canonical instance that matches all methods. + * Canonical instance of a {@code MethodMatcher} that matches all methods. */ MethodMatcher TRUE = TrueMethodMatcher.INSTANCE; diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJPointcutAdvisor.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJPointcutAdvisor.java index 2bfafe1d36..2bb7ab237a 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJPointcutAdvisor.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJPointcutAdvisor.java @@ -25,7 +25,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** - * AspectJPointcutAdvisor that adapts an {@link AbstractAspectJAdvice} + * AspectJ {@link PointcutAdvisor} that adapts an {@link AbstractAspectJAdvice} * to the {@link org.springframework.aop.PointcutAdvisor} interface. * * @author Adrian Colyer diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java b/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java index d88c58aa3b..6020cbbd99 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java @@ -48,7 +48,8 @@ import org.springframework.util.ObjectUtils; /** * Base class for AOP proxy configuration managers. - * These are not themselves AOP proxies, but subclasses of this class are + * + *

These are not themselves AOP proxies, but subclasses of this class are * normally factories from which AOP proxy instances are obtained directly. * *

This class frees subclasses of the housekeeping of Advices @@ -56,7 +57,8 @@ import org.springframework.util.ObjectUtils; * methods, which are provided by subclasses. * *

This class is serializable; subclasses need not be. - * This class is used to hold snapshots of proxies. + * + *

This class is used to hold snapshots of proxies. * * @author Rod Johnson * @author Juergen Hoeller @@ -111,7 +113,7 @@ public class AdvisedSupport extends ProxyConfig implements Advised { } /** - * Create a AdvisedSupport instance with the given parameters. + * Create an {@code AdvisedSupport} instance with the given parameters. * @param interfaces the proxied interfaces */ public AdvisedSupport(Class... interfaces) { @@ -131,7 +133,7 @@ public class AdvisedSupport extends ProxyConfig implements Advised { /** * Set the given object as target. - * Will create a SingletonTargetSource for the object. + *

Will create a SingletonTargetSource for the object. * @see #setTargetSource * @see org.springframework.aop.target.SingletonTargetSource */ @@ -506,9 +508,9 @@ public class AdvisedSupport extends ProxyConfig implements Advised { } /** - * Copy the AOP configuration from the given AdvisedSupport object, - * but allow substitution of a fresh TargetSource and a given interceptor chain. - * @param other the AdvisedSupport object to take proxy configuration from + * Copy the AOP configuration from the given {@link AdvisedSupport} object, + * but allow substitution of a fresh {@link TargetSource} and a given interceptor chain. + * @param other the {@code AdvisedSupport} object to take proxy configuration from * @param targetSource the new TargetSource * @param advisors the Advisors for the chain */ @@ -528,8 +530,8 @@ public class AdvisedSupport extends ProxyConfig implements Advised { } /** - * Build a configuration-only copy of this AdvisedSupport, - * replacing the TargetSource. + * Build a configuration-only copy of this {@link AdvisedSupport}, + * replacing the {@link TargetSource}. */ AdvisedSupport getConfigurationOnlyCopy() { AdvisedSupport copy = new AdvisedSupport(this.advisorChainFactory, this.methodCache); @@ -604,8 +606,7 @@ public class AdvisedSupport extends ProxyConfig implements Advised { @Override public boolean equals(@Nullable Object other) { - return (this == other || (other instanceof MethodCacheKey methodCacheKey && - this.method == methodCacheKey.method)); + return (this == other || (other instanceof MethodCacheKey that && this.method == that.method)); } @Override @@ -630,7 +631,7 @@ public class AdvisedSupport extends ProxyConfig implements Advised { /** - * Stub for an Advisor instance that is just needed for key purposes, + * Stub for an {@link Advisor} instance that is just needed for key purposes, * allowing for efficient equals and hashCode comparisons against the * advice class and the pointcut. * @since 6.0.10 @@ -642,10 +643,11 @@ public class AdvisedSupport extends ProxyConfig implements Advised { private final Class adviceType; @Nullable - private String classFilterKey; + private final String classFilterKey; @Nullable - private String methodMatcherKey; + private final String methodMatcherKey; + public AdvisorKeyEntry(Advisor advisor) { this.adviceType = advisor.getAdvice().getClass(); @@ -654,6 +656,10 @@ public class AdvisedSupport extends ProxyConfig implements Advised { this.classFilterKey = ObjectUtils.identityToString(pointcut.getClassFilter()); this.methodMatcherKey = ObjectUtils.identityToString(pointcut.getMethodMatcher()); } + else { + this.classFilterKey = null; + this.methodMatcherKey = null; + } } @Override @@ -663,10 +669,10 @@ public class AdvisedSupport extends ProxyConfig implements Advised { @Override public boolean equals(Object other) { - return (this == other || (other instanceof AdvisorKeyEntry otherEntry && - this.adviceType == otherEntry.adviceType && - ObjectUtils.nullSafeEquals(this.classFilterKey, otherEntry.classFilterKey) && - ObjectUtils.nullSafeEquals(this.methodMatcherKey, otherEntry.methodMatcherKey))); + return (this == other || (other instanceof AdvisorKeyEntry that && + this.adviceType == that.adviceType && + ObjectUtils.nullSafeEquals(this.classFilterKey, that.classFilterKey) && + ObjectUtils.nullSafeEquals(this.methodMatcherKey, that.methodMatcherKey))); } @Override diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java index 18927949bf..e7988d5e12 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java @@ -27,7 +27,7 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; /** - * A Pointcut that matches if the underlying {@link CacheOperationSource} + * A {@code Pointcut} that matches if the underlying {@link CacheOperationSource} * has an attribute for a given method. * * @author Costin Leau diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java index 99265d6818..9917ab7b2e 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java @@ -27,7 +27,7 @@ import org.springframework.transaction.TransactionManager; import org.springframework.util.ObjectUtils; /** - * Abstract class that implements a Pointcut that matches if the underlying + * Abstract class that implements a {@code Pointcut} that matches if the underlying * {@link TransactionAttributeSource} has an attribute for a given method. * * @author Juergen Hoeller diff --git a/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java b/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java index 2de33308dd..e4a54b8033 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,7 +41,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; * @author Juergen Hoeller * @author Mark Paluch */ -public class AnnotationTransactionInterceptorTests { +class AnnotationTransactionInterceptorTests { private final CallCountingTransactionManager ptm = new CallCountingTransactionManager(); @@ -53,7 +53,7 @@ public class AnnotationTransactionInterceptorTests { @Test - public void classLevelOnly() { + void classLevelOnly() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestClassLevelOnly()); proxyFactory.addAdvice(this.ti); @@ -74,7 +74,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withSingleMethodOverride() { + void withSingleMethodOverride() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithSingleMethodOverride()); proxyFactory.addAdvice(this.ti); @@ -95,7 +95,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withSingleMethodOverrideInverted() { + void withSingleMethodOverrideInverted() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithSingleMethodOverrideInverted()); proxyFactory.addAdvice(this.ti); @@ -116,7 +116,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withMultiMethodOverride() { + void withMultiMethodOverride() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithMultiMethodOverride()); proxyFactory.addAdvice(this.ti); @@ -137,7 +137,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withRollbackOnRuntimeException() { + void withRollbackOnRuntimeException() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithExceptions()); proxyFactory.addAdvice(this.ti); @@ -154,7 +154,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withCommitOnCheckedException() { + void withCommitOnCheckedException() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithExceptions()); proxyFactory.addAdvice(this.ti); @@ -167,7 +167,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withRollbackOnCheckedExceptionAndRollbackRule() { + void withRollbackOnCheckedExceptionAndRollbackRule() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithExceptions()); proxyFactory.addAdvice(this.ti); @@ -180,7 +180,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withMonoSuccess() { + void withMonoSuccess() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithReactive()); proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source)); @@ -192,7 +192,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withMonoFailure() { + void withMonoFailure() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithReactive()); proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source)); @@ -204,7 +204,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withMonoRollback() { + void withMonoRollback() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithReactive()); proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source)); @@ -216,7 +216,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withFluxSuccess() { + void withFluxSuccess() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithReactive()); proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source)); @@ -228,7 +228,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withFluxFailure() { + void withFluxFailure() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithReactive()); proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source)); @@ -240,7 +240,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withFluxRollback() { + void withFluxRollback() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithReactive()); proxyFactory.addAdvice(new TransactionInterceptor(rtm, this.source)); @@ -252,7 +252,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withVavrTrySuccess() { + void withVavrTrySuccess() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithVavrTry()); proxyFactory.addAdvice(this.ti); @@ -264,7 +264,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withVavrTryRuntimeException() { + void withVavrTryRuntimeException() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithVavrTry()); proxyFactory.addAdvice(this.ti); @@ -276,7 +276,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withVavrTryCheckedException() { + void withVavrTryCheckedException() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithVavrTry()); proxyFactory.addAdvice(this.ti); @@ -288,7 +288,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withVavrTryCheckedExceptionAndRollbackRule() { + void withVavrTryCheckedExceptionAndRollbackRule() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithVavrTry()); proxyFactory.addAdvice(this.ti); @@ -300,7 +300,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withInterface() { + void withInterface() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new TestWithInterfaceImpl()); proxyFactory.addInterface(TestWithInterface.class); @@ -325,7 +325,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void crossClassInterfaceMethodLevelOnJdkProxy() { + void crossClassInterfaceMethodLevelOnJdkProxy() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new SomeServiceImpl()); proxyFactory.addInterface(SomeService.class); @@ -344,7 +344,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void crossClassInterfaceOnJdkProxy() { + void crossClassInterfaceOnJdkProxy() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new OtherServiceImpl()); proxyFactory.addInterface(OtherService.class); @@ -357,7 +357,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withInterfaceOnTargetJdkProxy() { + void withInterfaceOnTargetJdkProxy() { ProxyFactory targetFactory = new ProxyFactory(); targetFactory.setTarget(new TestWithInterfaceImpl()); targetFactory.addInterface(TestWithInterface.class); @@ -386,7 +386,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void withInterfaceOnTargetCglibProxy() { + void withInterfaceOnTargetCglibProxy() { ProxyFactory targetFactory = new ProxyFactory(); targetFactory.setTarget(new TestWithInterfaceImpl()); targetFactory.setProxyTargetClass(true); @@ -436,7 +436,7 @@ public class AnnotationTransactionInterceptorTests { @Transactional - public static class TestClassLevelOnly { + static class TestClassLevelOnly { public void doSomething() { assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue(); @@ -451,7 +451,7 @@ public class AnnotationTransactionInterceptorTests { @Transactional - public static class TestWithSingleMethodOverride { + static class TestWithSingleMethodOverride { public void doSomething() { assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue(); @@ -472,7 +472,7 @@ public class AnnotationTransactionInterceptorTests { @Transactional(readOnly = true) - public static class TestWithSingleMethodOverrideInverted { + static class TestWithSingleMethodOverrideInverted { @Transactional public void doSomething() { @@ -493,7 +493,7 @@ public class AnnotationTransactionInterceptorTests { @Transactional - public static class TestWithMultiMethodOverride { + static class TestWithMultiMethodOverride { @Transactional(readOnly = true) public void doSomething() { @@ -515,7 +515,7 @@ public class AnnotationTransactionInterceptorTests { @Transactional - public static class TestWithExceptions { + static class TestWithExceptions { public void doSomethingErroneous() { assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue(); @@ -545,7 +545,7 @@ public class AnnotationTransactionInterceptorTests { } @Transactional - public static class TestWithReactive { + static class TestWithReactive { public Mono monoSuccess() { return Mono.delay(Duration.ofSeconds(10)).then(); @@ -565,7 +565,7 @@ public class AnnotationTransactionInterceptorTests { } @Transactional - public static class TestWithVavrTry { + static class TestWithVavrTry { public Try doSomething() { assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue(); @@ -594,14 +594,14 @@ public class AnnotationTransactionInterceptorTests { } - public interface BaseInterface { + interface BaseInterface { void doSomething(); } @Transactional - public interface TestWithInterface extends BaseInterface { + interface TestWithInterface extends BaseInterface { @Transactional(readOnly = true) void doSomethingElse(); @@ -613,7 +613,7 @@ public class AnnotationTransactionInterceptorTests { } - public static class TestWithInterfaceImpl implements TestWithInterface { + static class TestWithInterfaceImpl implements TestWithInterface { @Override public void doSomething() { @@ -629,7 +629,7 @@ public class AnnotationTransactionInterceptorTests { } - public interface SomeService { + interface SomeService { void foo(); @@ -641,7 +641,7 @@ public class AnnotationTransactionInterceptorTests { } - public static class SomeServiceImpl implements SomeService { + static class SomeServiceImpl implements SomeService { @Override public void bar() { @@ -659,14 +659,14 @@ public class AnnotationTransactionInterceptorTests { } - public interface OtherService { + interface OtherService { void foo(); } @Transactional - public static class OtherServiceImpl implements OtherService { + static class OtherServiceImpl implements OtherService { @Override public void foo() { From 9120f87897d2e3ad8f0577199c2e7d3ee0815413 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 20 Sep 2023 15:53:25 +0200 Subject: [PATCH 2/3] Consolidate AspectJ test fixtures --- .../AbstractAspectJAdvisorFactoryTests.java | 12 ++--- .../AspectJPointcutAdvisorTests.java | 4 +- .../annotation/AspectMetadataTests.java | 4 +- .../annotation/AspectProxyFactoryTests.java | 4 +- .../src/test/java/test/aop/PerThisAspect.java | 37 -------------- .../test/java/test/aop/TwoAdviceAspect.java | 37 -------------- .../testfixture/aspectj}/CommonPointcuts.java | 2 +- .../testfixture/aspectj}/PerTargetAspect.java | 4 +- .../testfixture/aspectj}/PerThisAspect.java | 6 +-- .../testfixture/aspectj}/TwoAdviceAspect.java | 4 +- .../testfixture/mixin}/DefaultLockable.java | 4 +- .../aop/testfixture}/mixin/LockMixin.java | 4 +- .../testfixture}/mixin/LockMixinAdvisor.java | 4 +- .../aop/testfixture/mixin}/Lockable.java | 8 ++- .../testfixture}/mixin/LockedException.java | 6 +-- .../aop/aspectj/DeclareParentsTests.java | 4 +- .../AspectJAutoProxyCreatorTests.java | 29 +---------- .../aop/framework/AbstractAopProxyTests.java | 8 +-- .../aop/framework/CglibProxyTests.java | 2 +- .../aop/framework/ProxyFactoryBeanTests.java | 4 +- .../AdvisorAutoProxyCreatorTests.java | 2 +- .../BeanNameAutoProxyCreatorTests.java | 6 +-- .../java/test/aspect/PerTargetAspect.java | 50 ------------------- .../test/java/test/mixin/DefaultLockable.java | 44 ---------------- .../src/test/java/test/mixin/Lockable.java | 33 ------------ .../aop/aspectj/DeclareParentsTests.xml | 12 ++--- ...AspectJAutoProxyCreatorTests-pertarget.xml | 3 +- .../AspectJAutoProxyCreatorTests-perthis.xml | 2 +- ...JAutoProxyCreatorTests-twoAdviceAspect.xml | 2 +- ...yCreatorTests-twoAdviceAspectPrototype.xml | 2 +- ...yCreatorTests-twoAdviceAspectSingleton.xml | 2 +- .../ProxyFactoryBeanTests-context.xml | 6 +-- ...oProxyCreatorTests-common-interceptors.xml | 2 +- .../BeanNameAutoProxyCreatorTests-context.xml | 3 +- 34 files changed, 62 insertions(+), 294 deletions(-) delete mode 100644 spring-aop/src/test/java/test/aop/PerThisAspect.java delete mode 100644 spring-aop/src/test/java/test/aop/TwoAdviceAspect.java rename {spring-context/src/test/java/test/aspect => spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj}/CommonPointcuts.java (93%) rename spring-aop/src/{test/java/test/aop => testFixtures/java/org/springframework/aop/testfixture/aspectj}/PerTargetAspect.java (91%) rename {spring-context/src/test/java/test/aspect => spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj}/PerThisAspect.java (79%) rename {spring-context/src/test/java/test/aspect => spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj}/TwoAdviceAspect.java (91%) rename spring-aop/src/{test/java/test/aop => testFixtures/java/org/springframework/aop/testfixture/mixin}/DefaultLockable.java (89%) rename {spring-context/src/test/java/test => spring-aop/src/testFixtures/java/org/springframework/aop/testfixture}/mixin/LockMixin.java (94%) rename {spring-context/src/test/java/test => spring-aop/src/testFixtures/java/org/springframework/aop/testfixture}/mixin/LockMixinAdvisor.java (89%) rename spring-aop/src/{test/java/test/aop => testFixtures/java/org/springframework/aop/testfixture/mixin}/Lockable.java (82%) rename {spring-context/src/test/java/test => spring-aop/src/testFixtures/java/org/springframework/aop/testfixture}/mixin/LockedException.java (86%) delete mode 100644 spring-context/src/test/java/test/aspect/PerTargetAspect.java delete mode 100644 spring-context/src/test/java/test/mixin/DefaultLockable.java delete mode 100644 spring-context/src/test/java/test/mixin/Lockable.java diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java index a4030f8d2e..978cd04954 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java @@ -40,10 +40,6 @@ import org.aspectj.lang.annotation.Pointcut; import org.aspectj.lang.reflect.MethodSignature; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import test.aop.DefaultLockable; -import test.aop.Lockable; -import test.aop.PerTargetAspect; -import test.aop.TwoAdviceAspect; import org.springframework.aop.Advisor; import org.springframework.aop.framework.Advised; @@ -51,6 +47,10 @@ import org.springframework.aop.framework.AopConfigException; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.interceptor.ExposeInvocationInterceptor; import org.springframework.aop.support.AopUtils; +import org.springframework.aop.testfixture.aspectj.PerTargetAspect; +import org.springframework.aop.testfixture.aspectj.TwoAdviceAspect; +import org.springframework.aop.testfixture.mixin.DefaultLockable; +import org.springframework.aop.testfixture.mixin.Lockable; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.core.OrderComparator; @@ -459,10 +459,10 @@ abstract class AbstractAspectJAdvisorFactoryTests { assertThat(advisors).as("Two advice methods found").hasSize(2); ITestBean itb = createProxy(target, ITestBean.class, advisors); itb.setName(""); - assertThat(itb.getAge()).isEqualTo(0); + assertThat(itb.age()).isEqualTo(0); int newAge = 32; itb.setAge(newAge); - assertThat(itb.getAge()).isEqualTo(1); + assertThat(itb.age()).isEqualTo(1); } @Test diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java index 806f30587b..15203879aa 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,13 +17,13 @@ package org.springframework.aop.aspectj.annotation; import org.junit.jupiter.api.Test; -import test.aop.PerTargetAspect; import org.springframework.aop.Pointcut; import org.springframework.aop.aspectj.AspectJExpressionPointcut; import org.springframework.aop.aspectj.AspectJExpressionPointcutTests; import org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.ExceptionThrowingAspect; import org.springframework.aop.framework.AopConfigException; +import org.springframework.aop.testfixture.aspectj.PerTargetAspect; import org.springframework.beans.testfixture.beans.TestBean; import static org.assertj.core.api.Assertions.assertThat; diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java index 2554895430..242d72ce99 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,11 +18,11 @@ package org.springframework.aop.aspectj.annotation; import org.aspectj.lang.reflect.PerClauseKind; import org.junit.jupiter.api.Test; -import test.aop.PerTargetAspect; import org.springframework.aop.Pointcut; import org.springframework.aop.aspectj.AspectJExpressionPointcut; import org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.ExceptionThrowingAspect; +import org.springframework.aop.testfixture.aspectj.PerTargetAspect; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java index a00a27bb6d..3fb1b05d81 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,8 +24,8 @@ import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; import org.junit.jupiter.api.Test; -import test.aop.PerThisAspect; +import org.springframework.aop.testfixture.aspectj.PerThisAspect; import org.springframework.core.testfixture.io.SerializationTestUtils; import static org.assertj.core.api.Assertions.assertThat; diff --git a/spring-aop/src/test/java/test/aop/PerThisAspect.java b/spring-aop/src/test/java/test/aop/PerThisAspect.java deleted file mode 100644 index f6b9a3d4a9..0000000000 --- a/spring-aop/src/test/java/test/aop/PerThisAspect.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2002-2005 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package test.aop; - -import org.aspectj.lang.ProceedingJoinPoint; -import org.aspectj.lang.annotation.Around; -import org.aspectj.lang.annotation.Aspect; - -@Aspect("perthis(execution(* getAge()))") -public class PerThisAspect { - - private int invocations = 0; - - public int getInvocations() { - return this.invocations; - } - - @Around("execution(* getAge())") - public int changeAge(ProceedingJoinPoint pjp) throws Throwable { - return invocations++; - } - -} diff --git a/spring-aop/src/test/java/test/aop/TwoAdviceAspect.java b/spring-aop/src/test/java/test/aop/TwoAdviceAspect.java deleted file mode 100644 index f77ad9a517..0000000000 --- a/spring-aop/src/test/java/test/aop/TwoAdviceAspect.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2002-2008 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package test.aop; - -import org.aspectj.lang.ProceedingJoinPoint; -import org.aspectj.lang.annotation.Around; -import org.aspectj.lang.annotation.Aspect; -import org.aspectj.lang.annotation.Before; - -@Aspect -public class TwoAdviceAspect { - private int totalCalls; - - @Around("execution(* getAge())") - public int returnCallCount(ProceedingJoinPoint pjp) throws Exception { - return totalCalls; - } - - @Before("execution(* setAge(int)) && args(newAge)") - public void countSet(int newAge) throws Exception { - ++totalCalls; - } -} diff --git a/spring-context/src/test/java/test/aspect/CommonPointcuts.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/CommonPointcuts.java similarity index 93% rename from spring-context/src/test/java/test/aspect/CommonPointcuts.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/CommonPointcuts.java index c7dfd107ff..8421c8a1d2 100644 --- a/spring-context/src/test/java/test/aspect/CommonPointcuts.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/CommonPointcuts.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.aspect; +package org.springframework.aop.testfixture.aspectj; import org.aspectj.lang.annotation.Pointcut; diff --git a/spring-aop/src/test/java/test/aop/PerTargetAspect.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/PerTargetAspect.java similarity index 91% rename from spring-aop/src/test/java/test/aop/PerTargetAspect.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/PerTargetAspect.java index 4dd5c29dbb..0f9a34dde0 100644 --- a/spring-aop/src/test/java/test/aop/PerTargetAspect.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/PerTargetAspect.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.aop; +package org.springframework.aop.testfixture.aspectj; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; diff --git a/spring-context/src/test/java/test/aspect/PerThisAspect.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/PerThisAspect.java similarity index 79% rename from spring-context/src/test/java/test/aspect/PerThisAspect.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/PerThisAspect.java index e4e8e02b3d..fa7b5133e4 100644 --- a/spring-context/src/test/java/test/aspect/PerThisAspect.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/PerThisAspect.java @@ -14,13 +14,13 @@ * limitations under the License. */ -package test.aspect; +package org.springframework.aop.testfixture.aspectj; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; -@Aspect("perthis(test.aspect.CommonPointcuts.getAgeExecution())") +@Aspect("perthis(org.springframework.aop.testfixture.aspectj.CommonPointcuts.getAgeExecution())") public class PerThisAspect { private int invocations = 0; @@ -29,7 +29,7 @@ public class PerThisAspect { return this.invocations; } - @Around("test.aspect.CommonPointcuts.getAgeExecution()") + @Around("org.springframework.aop.testfixture.aspectj.CommonPointcuts.getAgeExecution()") public int changeAge(ProceedingJoinPoint pjp) { return this.invocations++; } diff --git a/spring-context/src/test/java/test/aspect/TwoAdviceAspect.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/TwoAdviceAspect.java similarity index 91% rename from spring-context/src/test/java/test/aspect/TwoAdviceAspect.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/TwoAdviceAspect.java index 3e544072e4..449bbdc714 100644 --- a/spring-context/src/test/java/test/aspect/TwoAdviceAspect.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/aspectj/TwoAdviceAspect.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.aspect; +package org.springframework.aop.testfixture.aspectj; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; diff --git a/spring-aop/src/test/java/test/aop/DefaultLockable.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/DefaultLockable.java similarity index 89% rename from spring-aop/src/test/java/test/aop/DefaultLockable.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/DefaultLockable.java index 1e9499df80..b495d2dc88 100644 --- a/spring-aop/src/test/java/test/aop/DefaultLockable.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/DefaultLockable.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.aop; +package org.springframework.aop.testfixture.mixin; /** * Simple implementation of Lockable interface for use in mixins. diff --git a/spring-context/src/test/java/test/mixin/LockMixin.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockMixin.java similarity index 94% rename from spring-context/src/test/java/test/mixin/LockMixin.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockMixin.java index 12915b678b..33ffc13f39 100644 --- a/spring-context/src/test/java/test/mixin/LockMixin.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockMixin.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.mixin; +package org.springframework.aop.testfixture.mixin; import org.aopalliance.intercept.MethodInvocation; diff --git a/spring-context/src/test/java/test/mixin/LockMixinAdvisor.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockMixinAdvisor.java similarity index 89% rename from spring-context/src/test/java/test/mixin/LockMixinAdvisor.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockMixinAdvisor.java index 6ec81a798c..21a7255c88 100644 --- a/spring-context/src/test/java/test/mixin/LockMixinAdvisor.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockMixinAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ * limitations under the License. */ -package test.mixin; +package org.springframework.aop.testfixture.mixin; import org.springframework.aop.support.DefaultIntroductionAdvisor; diff --git a/spring-aop/src/test/java/test/aop/Lockable.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/Lockable.java similarity index 82% rename from spring-aop/src/test/java/test/aop/Lockable.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/Lockable.java index 7e9058d1a0..83d0ea05f0 100644 --- a/spring-aop/src/test/java/test/aop/Lockable.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/Lockable.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,14 +14,12 @@ * limitations under the License. */ -package test.aop; - +package org.springframework.aop.testfixture.mixin; /** - * Simple interface to use for mixins + * Simple interface to use for mixins. * * @author Rod Johnson - * */ public interface Lockable { diff --git a/spring-context/src/test/java/test/mixin/LockedException.java b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockedException.java similarity index 86% rename from spring-context/src/test/java/test/mixin/LockedException.java rename to spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockedException.java index 6f8d8537a8..04a8268e8f 100644 --- a/spring-context/src/test/java/test/mixin/LockedException.java +++ b/spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/mixin/LockedException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,12 +14,12 @@ * limitations under the License. */ -package test.mixin; - +package org.springframework.aop.testfixture.mixin; @SuppressWarnings("serial") public class LockedException extends RuntimeException { public LockedException() { } + } diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/DeclareParentsTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/DeclareParentsTests.java index de1c9ae8da..a063eb7fb9 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/DeclareParentsTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/DeclareParentsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,9 +19,9 @@ package org.springframework.aop.aspectj; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import test.mixin.Lockable; import org.springframework.aop.support.AopUtils; +import org.springframework.aop.testfixture.mixin.Lockable; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.context.support.ClassPathXmlApplicationContext; diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java index e87949928e..8a7e0b5011 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java @@ -46,6 +46,7 @@ import org.springframework.aop.framework.ProxyConfig; import org.springframework.aop.support.AbstractPointcutAdvisor; import org.springframework.aop.support.AopUtils; import org.springframework.aop.support.StaticMethodMatcherPointcutAdvisor; +import org.springframework.aop.testfixture.aspectj.PerTargetAspect; import org.springframework.beans.PropertyValue; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.config.MethodInvokingFactoryBean; @@ -62,7 +63,6 @@ import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.DecoratingProxy; import org.springframework.core.NestedRuntimeException; -import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.lang.Nullable; @@ -354,33 +354,6 @@ class AspectJAutoProxyCreatorTests { } -@Aspect("pertarget(execution(* *.getSpouse()))") -class PerTargetAspect implements Ordered { - - public int count; - - private int order = Ordered.LOWEST_PRECEDENCE; - - @Around("execution(int *.getAge())") - public int returnCountAsAge() { - return count++; - } - - @Before("execution(void *.set*(int))") - public void countSetter() { - ++count; - } - - @Override - public int getOrder() { - return this.order; - } - - public void setOrder(int order) { - this.order = order; - } -} - @Aspect class AdviceUsingThisJoinPoint { diff --git a/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java index 193d58451e..9460d8c969 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java @@ -34,10 +34,6 @@ import org.aopalliance.intercept.MethodInvocation; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import test.mixin.LockMixin; -import test.mixin.LockMixinAdvisor; -import test.mixin.Lockable; -import test.mixin.LockedException; import org.springframework.aop.Advisor; import org.springframework.aop.AfterReturningAdvice; @@ -64,6 +60,10 @@ import org.springframework.aop.testfixture.advice.MyThrowsHandler; import org.springframework.aop.testfixture.interceptor.NopInterceptor; import org.springframework.aop.testfixture.interceptor.SerializableNopInterceptor; import org.springframework.aop.testfixture.interceptor.TimestampIntroductionInterceptor; +import org.springframework.aop.testfixture.mixin.LockMixin; +import org.springframework.aop.testfixture.mixin.LockMixinAdvisor; +import org.springframework.aop.testfixture.mixin.Lockable; +import org.springframework.aop.testfixture.mixin.LockedException; import org.springframework.beans.testfixture.beans.IOther; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.beans.testfixture.beans.Person; diff --git a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java index 3fddbf2af9..1c99d3fb0e 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java @@ -21,7 +21,6 @@ import java.io.Serializable; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.junit.jupiter.api.Test; -import test.mixin.LockMixinAdvisor; import org.springframework.aop.ClassFilter; import org.springframework.aop.MethodMatcher; @@ -31,6 +30,7 @@ import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.aop.support.annotation.AnnotationMatchingPointcut; import org.springframework.aop.testfixture.advice.CountingBeforeAdvice; import org.springframework.aop.testfixture.interceptor.NopInterceptor; +import org.springframework.aop.testfixture.mixin.LockMixinAdvisor; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.context.ApplicationContext; diff --git a/spring-context/src/test/java/org/springframework/aop/framework/ProxyFactoryBeanTests.java b/spring-context/src/test/java/org/springframework/aop/framework/ProxyFactoryBeanTests.java index 1ec383a468..9d1c9b8987 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/ProxyFactoryBeanTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/ProxyFactoryBeanTests.java @@ -27,8 +27,6 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import test.mixin.Lockable; -import test.mixin.LockedException; import org.springframework.aop.ClassFilter; import org.springframework.aop.IntroductionAdvisor; @@ -42,6 +40,8 @@ import org.springframework.aop.testfixture.advice.CountingBeforeAdvice; import org.springframework.aop.testfixture.advice.MyThrowsHandler; import org.springframework.aop.testfixture.interceptor.NopInterceptor; import org.springframework.aop.testfixture.interceptor.TimestampIntroductionInterceptor; +import org.springframework.aop.testfixture.mixin.Lockable; +import org.springframework.aop.testfixture.mixin.LockedException; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.FactoryBean; diff --git a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java index de560cbf8e..5c3c66ffe8 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java @@ -19,7 +19,6 @@ package org.springframework.aop.framework.autoproxy; import java.io.IOException; import org.junit.jupiter.api.Test; -import test.mixin.Lockable; import org.springframework.aop.Advisor; import org.springframework.aop.framework.Advised; @@ -32,6 +31,7 @@ import org.springframework.aop.target.PrototypeTargetSource; import org.springframework.aop.target.ThreadLocalTargetSource; import org.springframework.aop.testfixture.advice.CountingBeforeAdvice; import org.springframework.aop.testfixture.interceptor.NopInterceptor; +import org.springframework.aop.testfixture.mixin.Lockable; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.testfixture.beans.CountingTestBean; import org.springframework.beans.testfixture.beans.ITestBean; diff --git a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java index 153080c0ee..e316ac4bae 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,13 +17,13 @@ package org.springframework.aop.framework.autoproxy; import org.junit.jupiter.api.Test; -import test.mixin.Lockable; -import test.mixin.LockedException; import org.springframework.aop.framework.Advised; import org.springframework.aop.support.AopUtils; import org.springframework.aop.testfixture.advice.CountingBeforeAdvice; import org.springframework.aop.testfixture.interceptor.NopInterceptor; +import org.springframework.aop.testfixture.mixin.Lockable; +import org.springframework.aop.testfixture.mixin.LockedException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.testfixture.beans.ITestBean; diff --git a/spring-context/src/test/java/test/aspect/PerTargetAspect.java b/spring-context/src/test/java/test/aspect/PerTargetAspect.java deleted file mode 100644 index cb93dc492d..0000000000 --- a/spring-context/src/test/java/test/aspect/PerTargetAspect.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2002-2018 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package test.aspect; - -import org.aspectj.lang.annotation.Around; -import org.aspectj.lang.annotation.Aspect; -import org.aspectj.lang.annotation.Before; - -import org.springframework.core.Ordered; - -@Aspect("pertarget(execution(* *.getSpouse()))") -public class PerTargetAspect implements Ordered { - - public int count; - - private int order = Ordered.LOWEST_PRECEDENCE; - - @Around("execution(int *.getAge())") - public int returnCountAsAge() { - return count++; - } - - @Before("execution(void *.set*(int))") - public void countSetter() { - ++count; - } - - @Override - public int getOrder() { - return this.order; - } - - public void setOrder(int order) { - this.order = order; - } -} diff --git a/spring-context/src/test/java/test/mixin/DefaultLockable.java b/spring-context/src/test/java/test/mixin/DefaultLockable.java deleted file mode 100644 index 0482eb0961..0000000000 --- a/spring-context/src/test/java/test/mixin/DefaultLockable.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2002-2018 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package test.mixin; - - -/** - * Simple implementation of Lockable interface for use in mixins. - * - * @author Rod Johnson - */ -public class DefaultLockable implements Lockable { - - private boolean locked; - - @Override - public void lock() { - this.locked = true; - } - - @Override - public void unlock() { - this.locked = false; - } - - @Override - public boolean locked() { - return this.locked; - } - -} diff --git a/spring-context/src/test/java/test/mixin/Lockable.java b/spring-context/src/test/java/test/mixin/Lockable.java deleted file mode 100644 index 41abef3bc4..0000000000 --- a/spring-context/src/test/java/test/mixin/Lockable.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2002-2018 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package test.mixin; - - -/** - * Simple interface to use for mixins - * - * @author Rod Johnson - * - */ -public interface Lockable { - - void lock(); - - void unlock(); - - boolean locked(); -} diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/DeclareParentsTests.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/DeclareParentsTests.xml index 96779df333..5212c81601 100644 --- a/spring-context/src/test/resources/org/springframework/aop/aspectj/DeclareParentsTests.xml +++ b/spring-context/src/test/resources/org/springframework/aop/aspectj/DeclareParentsTests.xml @@ -1,16 +1,16 @@ + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xmlns:aop="http://www.springframework.org/schema/aop" + xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans-2.0.xsd + http://www.springframework.org/schema/aop https://www.springframework.org/schema/aop/spring-aop-2.0.xsd"> - + diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-perthis.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-perthis.xml index 231038cafc..836577fcc5 100644 --- a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-perthis.xml +++ b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-perthis.xml @@ -5,7 +5,7 @@ - + diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspect.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspect.xml index c8dcfe9ee9..c3cd370b75 100644 --- a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspect.xml +++ b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspect.xml @@ -5,7 +5,7 @@ - + diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectPrototype.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectPrototype.xml index 5bb345353c..99e1ca8cd2 100644 --- a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectPrototype.xml +++ b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectPrototype.xml @@ -5,7 +5,7 @@ - + diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectSingleton.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectSingleton.xml index 9202f08e82..23fc935618 100644 --- a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectSingleton.xml +++ b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-twoAdviceAspectSingleton.xml @@ -5,7 +5,7 @@ - + diff --git a/spring-context/src/test/resources/org/springframework/aop/framework/ProxyFactoryBeanTests-context.xml b/spring-context/src/test/resources/org/springframework/aop/framework/ProxyFactoryBeanTests-context.xml index a8a77341e5..ed794c1e2c 100644 --- a/spring-context/src/test/resources/org/springframework/aop/framework/ProxyFactoryBeanTests-context.xml +++ b/spring-context/src/test/resources/org/springframework/aop/framework/ProxyFactoryBeanTests-context.xml @@ -135,7 +135,7 @@ --> - + @@ -150,13 +150,13 @@ - + org.springframework.beans.testfixture.beans.ITestBean - test.mixin.Lockable + org.springframework.aop.testfixture.mixin.Lockable false diff --git a/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml b/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml index 7819d2c86d..c526e0c90f 100644 --- a/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml +++ b/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml @@ -31,7 +31,7 @@ - + diff --git a/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests-context.xml b/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests-context.xml index 6327f5ecba..4e148eb3e6 100644 --- a/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests-context.xml +++ b/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests-context.xml @@ -54,8 +54,7 @@ - + From 865fa33927f1ab3923cc7c7bd6b799fde877983f Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 18 Sep 2023 18:22:08 +0200 Subject: [PATCH 3/3] Cache CGLIB proxy classes properly again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The introduction of AdvisedSupport.AdvisorKeyEntry in Spring Framework 6.0.10 resulted in a regression regarding caching of CGLIB generated proxy classes. Specifically, equality checks for the proxy class cache became based partially on identity rather than equivalence. For example, if an ApplicationContext was configured to create a class-based @Transactional proxy, a second attempt to create the ApplicationContext resulted in a duplicate proxy class for the same @Transactional component. On the JVM this went unnoticed; however, when running Spring integration tests within a native image, if a test made use of @⁠DirtiesContext, a second attempt to create the test ApplicationContext resulted in an exception stating, "CGLIB runtime enhancement not supported on native image." This is because Test AOT processing only refreshes a test ApplicationContext once, and the duplicate CGLIB proxy classes are only requested in subsequent refreshes of the same ApplicationContext which means that duplicate proxy classes are not tracked during AOT processing and consequently not included in a native image. This commit addresses this regression as follows. - AdvisedSupport.AdvisorKeyEntry is now based on the toString() representations of the ClassFilter and MethodMatcher in the corresponding Pointcut instead of the filter's and matcher's identities. - Due to the above changes to AdvisorKeyEntry, ClassFilter and MethodMatcher implementations are now required to implement equals(), hashCode(), AND toString(). - Consequently, the following now include proper equals(), hashCode(), and toString() implementations. - CacheOperationSourcePointcut - TransactionAttributeSourcePointcut - PerTargetInstantiationModelPointcut Closes gh-31238 --- .../org/springframework/aop/ClassFilter.java | 13 ++++-- .../springframework/aop/MethodMatcher.java | 13 ++++-- ...ntiationModelAwarePointcutAdvisorImpl.java | 19 ++++++++ .../aop/framework/AdvisedSupport.java | 5 ++- .../CacheOperationSourcePointcut.java | 22 ++++++++++ .../AspectJAutoProxyCreatorTests.java | 44 +++++++++++++++++++ .../config/EnableCachingIntegrationTests.java | 23 +++++++++- .../TransactionAttributeSourcePointcut.java | 26 ++++++++++- .../EnableTransactionManagementTests.java | 21 ++++++++- 9 files changed, 172 insertions(+), 14 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java b/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java index de20021fd6..7926be708e 100644 --- a/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java +++ b/spring-aop/src/main/java/org/springframework/aop/ClassFilter.java @@ -23,12 +23,17 @@ package org.springframework.aop; *

Can be used as part of a {@link Pointcut} or for the entire targeting of * an {@link IntroductionAdvisor}. * - *

Concrete implementations of this interface typically should provide proper - * implementations of {@link Object#equals(Object)} and {@link Object#hashCode()} - * in order to allow the filter to be used in caching scenarios — for - * example, in proxies generated by CGLIB. + *

WARNING: Concrete implementations of this interface must + * provide proper implementations of {@link Object#equals(Object)}, + * {@link Object#hashCode()}, and {@link Object#toString()} in order to allow the + * filter to be used in caching scenarios — for example, in proxies generated + * by CGLIB. As of Spring Framework 6.0.13, the {@code toString()} implementation + * must generate a unique string representation that aligns with the logic used + * to implement {@code equals()}. See concrete implementations of this interface + * within the framework for examples. * * @author Rod Johnson + * @author Sam Brannen * @see Pointcut * @see MethodMatcher */ diff --git a/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java b/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java index 71f19e97dd..9e04831a01 100644 --- a/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java +++ b/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java @@ -41,12 +41,17 @@ import java.lang.reflect.Method; * state changes they have produced in parameters or {@code ThreadLocal} state will * be available at the time of evaluation. * - *

Concrete implementations of this interface typically should provide proper - * implementations of {@link Object#equals(Object)} and {@link Object#hashCode()} - * in order to allow the matcher to be used in caching scenarios — for - * example, in proxies generated by CGLIB. + *

WARNING: Concrete implementations of this interface must + * provide proper implementations of {@link Object#equals(Object)}, + * {@link Object#hashCode()}, and {@link Object#toString()} in order to allow the + * matcher to be used in caching scenarios — for example, in proxies generated + * by CGLIB. As of Spring Framework 6.0.13, the {@code toString()} implementation + * must generate a unique string representation that aligns with the logic used + * to implement {@code equals()}. See concrete implementations of this interface + * within the framework for examples. * * @author Rod Johnson + * @author Sam Brannen * @since 11.11.2003 * @see Pointcut * @see ClassFilter diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java index 74a2e1b3c4..f4d150d64d 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java @@ -32,6 +32,7 @@ import org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactory. import org.springframework.aop.support.DynamicMethodMatcherPointcut; import org.springframework.aop.support.Pointcuts; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; /** * Internal implementation of AspectJPointcutAdvisor. @@ -40,6 +41,7 @@ import org.springframework.lang.Nullable; * * @author Rod Johnson * @author Juergen Hoeller + * @author Sam Brannen * @since 2.0 */ @SuppressWarnings("serial") @@ -297,6 +299,23 @@ final class InstantiationModelAwarePointcutAdvisorImpl private boolean isAspectMaterialized() { return (this.aspectInstanceFactory == null || this.aspectInstanceFactory.isMaterialized()); } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof PerTargetInstantiationModelPointcut that && + ObjectUtils.nullSafeEquals(this.declaredPointcut.getExpression(), that.declaredPointcut.getExpression()))); + } + + @Override + public int hashCode() { + return ObjectUtils.nullSafeHashCode(this.declaredPointcut.getExpression()); + } + + @Override + public String toString() { + return PerTargetInstantiationModelPointcut.class.getName() + ": " + this.declaredPointcut.getExpression(); + } + } } diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java b/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java index 6020cbbd99..41c5b1d21c 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java @@ -62,6 +62,7 @@ import org.springframework.util.ObjectUtils; * * @author Rod Johnson * @author Juergen Hoeller + * @author Sam Brannen * @see org.springframework.aop.framework.AopProxy */ public class AdvisedSupport extends ProxyConfig implements Advised { @@ -653,8 +654,8 @@ public class AdvisedSupport extends ProxyConfig implements Advised { this.adviceType = advisor.getAdvice().getClass(); if (advisor instanceof PointcutAdvisor pointcutAdvisor) { Pointcut pointcut = pointcutAdvisor.getPointcut(); - this.classFilterKey = ObjectUtils.identityToString(pointcut.getClassFilter()); - this.methodMatcherKey = ObjectUtils.identityToString(pointcut.getMethodMatcher()); + this.classFilterKey = pointcut.getClassFilter().toString(); + this.methodMatcherKey = pointcut.getMethodMatcher().toString(); } else { this.classFilterKey = null; diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java index e7988d5e12..e70275aeae 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java @@ -32,6 +32,7 @@ import org.springframework.util.ObjectUtils; * * @author Costin Leau * @author Juergen Hoeller + * @author Sam Brannen * @since 3.1 */ @SuppressWarnings("serial") @@ -86,6 +87,27 @@ class CacheOperationSourcePointcut extends StaticMethodMatcherPointcut implement } return (cacheOperationSource == null || cacheOperationSource.isCandidateClass(clazz)); } + + private CacheOperationSource getCacheOperationSource() { + return cacheOperationSource; + } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof CacheOperationSourceClassFilter that && + ObjectUtils.nullSafeEquals(cacheOperationSource, that.getCacheOperationSource()))); + } + + @Override + public int hashCode() { + return CacheOperationSourceClassFilter.class.hashCode(); + } + + @Override + public String toString() { + return CacheOperationSourceClassFilter.class.getName() + ": " + cacheOperationSource; + } + } } diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java index 8a7e0b5011..69f5f4bd88 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java @@ -53,12 +53,14 @@ import org.springframework.beans.factory.config.MethodInvokingFactoryBean; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.beans.testfixture.beans.TestBean; +import org.springframework.cglib.proxy.Factory; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.context.annotation.Scope; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.DecoratingProxy; @@ -208,6 +210,31 @@ class AspectJAutoProxyCreatorTests { assertThat(adrian1.getAge()).isEqualTo(3); } + @Test // gh-31238 + void cglibProxyClassIsCachedAcrossApplicationContextsForPerTargetAspect() { + Class configClass = PerTargetProxyTargetClassTrueConfig.class; + TestBean testBean1; + TestBean testBean2; + + // Round #1 + try (ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(configClass)) { + testBean1 = context.getBean(TestBean.class); + assertThat(AopUtils.isCglibProxy(testBean1)).as("CGLIB proxy").isTrue(); + assertThat(testBean1.getClass().getInterfaces()) + .containsExactlyInAnyOrder(Factory.class, SpringProxy.class, Advised.class); + } + + // Round #2 + try (ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(configClass)) { + testBean2 = context.getBean(TestBean.class); + assertThat(AopUtils.isCglibProxy(testBean2)).as("CGLIB proxy").isTrue(); + assertThat(testBean2.getClass().getInterfaces()) + .containsExactlyInAnyOrder(Factory.class, SpringProxy.class, Advised.class); + } + + assertThat(testBean1.getClass()).isSameAs(testBean2.getClass()); + } + @Test void twoAdviceAspect() { ClassPathXmlApplicationContext bf = newContext("twoAdviceAspect.xml"); @@ -615,6 +642,23 @@ class ProxyTargetClassFalseConfig extends AbstractProxyTargetClassConfig { class ProxyTargetClassTrueConfig extends AbstractProxyTargetClassConfig { } +@Configuration +@EnableAspectJAutoProxy(proxyTargetClass = true) +class PerTargetProxyTargetClassTrueConfig { + + @Bean + @Scope("prototype") + TestBean testBean() { + return new TestBean("Jane", 34); + } + + @Bean + @Scope("prototype") + PerTargetAspect perTargetAspect() { + return new PerTargetAspect(); + } +} + @FunctionalInterface interface MessageGenerator { String generateMessage(); diff --git a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java index 9d371d23a1..f4e25b70ca 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ import java.util.concurrent.atomic.AtomicLong; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; @@ -45,6 +46,7 @@ import static org.springframework.context.testfixture.cache.CacheTestUtils.asser * Tests that represent real use cases with advanced configuration. * * @author Stephane Nicoll + * @author Sam Brannen */ class EnableCachingIntegrationTests { @@ -83,6 +85,25 @@ class EnableCachingIntegrationTests { assertCacheHit(key, value, cache); } + @Test // gh-31238 + public void cglibProxyClassIsCachedAcrossApplicationContexts() { + ConfigurableApplicationContext ctx; + + // Round #1 + ctx = new AnnotationConfigApplicationContext(FooConfigCglib.class); + FooService service1 = ctx.getBean(FooService.class); + assertThat(AopUtils.isCglibProxy(service1)).as("FooService #1 is not a CGLIB proxy").isTrue(); + ctx.close(); + + // Round #2 + ctx = new AnnotationConfigApplicationContext(FooConfigCglib.class); + FooService service2 = ctx.getBean(FooService.class); + assertThat(AopUtils.isCglibProxy(service2)).as("FooService #2 is not a CGLIB proxy").isTrue(); + ctx.close(); + + assertThat(service1.getClass()).isSameAs(service2.getClass()); + } + @Test void barServiceWithCacheableInterfaceCglib() { this.context = new AnnotationConfigApplicationContext(BarConfigCglib.class); diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java index 9917ab7b2e..10ac08147a 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java @@ -27,14 +27,15 @@ import org.springframework.transaction.TransactionManager; import org.springframework.util.ObjectUtils; /** - * Abstract class that implements a {@code Pointcut} that matches if the underlying + * Internal class that implements a {@code Pointcut} that matches if the underlying * {@link TransactionAttributeSource} has an attribute for a given method. * * @author Juergen Hoeller + * @author Sam Brannen * @since 2.5.5 */ @SuppressWarnings("serial") -class TransactionAttributeSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { +final class TransactionAttributeSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { @Nullable private TransactionAttributeSource transactionAttributeSource; @@ -87,6 +88,27 @@ class TransactionAttributeSourcePointcut extends StaticMethodMatcherPointcut imp } return (transactionAttributeSource == null || transactionAttributeSource.isCandidateClass(clazz)); } + + private TransactionAttributeSource getTransactionAttributeSource() { + return transactionAttributeSource; + } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof TransactionAttributeSourceClassFilter that && + ObjectUtils.nullSafeEquals(transactionAttributeSource, that.getTransactionAttributeSource()))); + } + + @Override + public int hashCode() { + return TransactionAttributeSourceClassFilter.class.hashCode(); + } + + @Override + public String toString() { + return TransactionAttributeSourceClassFilter.class.getName() + ": " + transactionAttributeSource; + } + } } diff --git a/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java b/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java index 8f33660d36..b9d70ee6a6 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,6 +69,25 @@ public class EnableTransactionManagementTests { ctx.close(); } + @Test // gh-31238 + public void cglibProxyClassIsCachedAcrossApplicationContexts() { + ConfigurableApplicationContext ctx; + + // Round #1 + ctx = new AnnotationConfigApplicationContext(EnableTxConfig.class, TxManagerConfig.class); + TransactionalTestBean bean1 = ctx.getBean(TransactionalTestBean.class); + assertThat(AopUtils.isCglibProxy(bean1)).as("testBean #1 is not a CGLIB proxy").isTrue(); + ctx.close(); + + // Round #2 + ctx = new AnnotationConfigApplicationContext(EnableTxConfig.class, TxManagerConfig.class); + TransactionalTestBean bean2 = ctx.getBean(TransactionalTestBean.class); + assertThat(AopUtils.isCglibProxy(bean2)).as("testBean #2 is not a CGLIB proxy").isTrue(); + ctx.close(); + + assertThat(bean1.getClass()).isSameAs(bean2.getClass()); + } + @Test public void transactionProxyIsCreatedWithEnableOnSuperclass() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(