From 6931106c5eb1160091be17ef205258079f63dd0d Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 8 Jun 2023 17:28:52 +0200 Subject: [PATCH] Redesign inner Pointcut implementations as standalone classes Avoids exposure of implicit Advisor instance state in Pointcut instance. See gh-30621 --- ...anFactoryJCacheOperationSourceAdvisor.java | 55 +++++++++++++++---- .../JCacheOperationSourcePointcut.java | 4 +- ...eanFactoryCacheOperationSourceAdvisor.java | 20 +++---- .../CacheOperationSourcePointcut.java | 38 ++++++------- ...toryTransactionAttributeSourceAdvisor.java | 16 +----- .../TransactionAttributeSourceAdvisor.java | 12 ++-- .../TransactionAttributeSourcePointcut.java | 38 ++++++------- 7 files changed, 93 insertions(+), 90 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/BeanFactoryJCacheOperationSourceAdvisor.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/BeanFactoryJCacheOperationSourceAdvisor.java index 62ae44f03a68..baf635e4d26a 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/BeanFactoryJCacheOperationSourceAdvisor.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/BeanFactoryJCacheOperationSourceAdvisor.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. @@ -16,30 +16,30 @@ package org.springframework.cache.jcache.interceptor; +import java.io.Serializable; +import java.lang.reflect.Method; + import org.springframework.aop.ClassFilter; import org.springframework.aop.Pointcut; import org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor; +import org.springframework.aop.support.StaticMethodMatcherPointcut; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; /** * Advisor driven by a {@link JCacheOperationSource}, used to include a * cache advice bean for methods that are cacheable. * * @author Stephane Nicoll + * @author Juergen Hoeller * @since 4.1 + * @see #setAdviceBeanName + * @see JCacheInterceptor */ @SuppressWarnings("serial") public class BeanFactoryJCacheOperationSourceAdvisor extends AbstractBeanFactoryPointcutAdvisor { - @Nullable - private JCacheOperationSource cacheOperationSource; - - private final JCacheOperationSourcePointcut pointcut = new JCacheOperationSourcePointcut() { - @Override - protected JCacheOperationSource getCacheOperationSource() { - return cacheOperationSource; - } - }; + private final JCacheOperationSourcePointcut pointcut = new JCacheOperationSourcePointcut(); /** @@ -48,7 +48,7 @@ protected JCacheOperationSource getCacheOperationSource() { * set on the cache interceptor itself. */ public void setCacheOperationSource(JCacheOperationSource cacheOperationSource) { - this.cacheOperationSource = cacheOperationSource; + this.pointcut.setCacheOperationSource(cacheOperationSource); } /** @@ -64,4 +64,37 @@ public Pointcut getPointcut() { return this.pointcut; } + + private static class JCacheOperationSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { + + @Nullable + private JCacheOperationSource cacheOperationSource; + + public void setCacheOperationSource(@Nullable JCacheOperationSource cacheOperationSource) { + this.cacheOperationSource = cacheOperationSource; + } + + @Override + public boolean matches(Method method, Class targetClass) { + return (this.cacheOperationSource == null || + this.cacheOperationSource.getCacheOperation(method, targetClass) != null); + } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof JCacheOperationSourcePointcut otherPc && + ObjectUtils.nullSafeEquals(this.cacheOperationSource, otherPc.cacheOperationSource))); + } + + @Override + public int hashCode() { + return JCacheOperationSourcePointcut.class.hashCode(); + } + + @Override + public String toString() { + return getClass().getName() + ": " + this.cacheOperationSource; + } + } + } diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheOperationSourcePointcut.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheOperationSourcePointcut.java index 1dbb4210894c..b81512728bad 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheOperationSourcePointcut.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/JCacheOperationSourcePointcut.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. @@ -29,7 +29,9 @@ * * @author Stephane Nicoll * @since 4.1 + * @deprecated since 6.0.10, as it is not used by the framework anymore */ +@Deprecated(since = "6.0.10", forRemoval = true) @SuppressWarnings("serial") public abstract class JCacheOperationSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/BeanFactoryCacheOperationSourceAdvisor.java b/spring-context/src/main/java/org/springframework/cache/interceptor/BeanFactoryCacheOperationSourceAdvisor.java index efd0cf7aae45..70844ad1b04a 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/BeanFactoryCacheOperationSourceAdvisor.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/BeanFactoryCacheOperationSourceAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 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,37 +19,31 @@ import org.springframework.aop.ClassFilter; import org.springframework.aop.Pointcut; import org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor; -import org.springframework.lang.Nullable; /** * Advisor driven by a {@link CacheOperationSource}, used to include a * cache advice bean for methods that are cacheable. * * @author Costin Leau + * @author Juergen Hoeller * @since 3.1 + * @see #setAdviceBeanName + * @see CacheInterceptor */ @SuppressWarnings("serial") public class BeanFactoryCacheOperationSourceAdvisor extends AbstractBeanFactoryPointcutAdvisor { - @Nullable - private CacheOperationSource cacheOperationSource; - - private final CacheOperationSourcePointcut pointcut = new CacheOperationSourcePointcut() { - @Override - @Nullable - protected CacheOperationSource getCacheOperationSource() { - return cacheOperationSource; - } - }; + private final CacheOperationSourcePointcut pointcut = new CacheOperationSourcePointcut(); /** * Set the cache operation attribute source which is used to find cache * attributes. This should usually be identical to the source reference * set on the cache interceptor itself. + * @see CacheInterceptor#setCacheOperationSource */ public void setCacheOperationSource(CacheOperationSource cacheOperationSource) { - this.cacheOperationSource = cacheOperationSource; + this.pointcut.setCacheOperationSource(cacheOperationSource); } /** 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 d555d802a234..3911dffe50a1 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 @@ -35,28 +35,31 @@ * @since 3.1 */ @SuppressWarnings("serial") -abstract class CacheOperationSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { +class CacheOperationSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { - protected CacheOperationSourcePointcut() { + @Nullable + private CacheOperationSource cacheOperationSource; + + + public CacheOperationSourcePointcut() { setClassFilter(new CacheOperationSourceClassFilter()); } + public void setCacheOperationSource(@Nullable CacheOperationSource cacheOperationSource) { + this.cacheOperationSource = cacheOperationSource; + } + @Override public boolean matches(Method method, Class targetClass) { - CacheOperationSource cas = getCacheOperationSource(); - return (cas != null && !CollectionUtils.isEmpty(cas.getCacheOperations(method, targetClass))); + return (this.cacheOperationSource == null || + !CollectionUtils.isEmpty(this.cacheOperationSource.getCacheOperations(method, targetClass))); } @Override public boolean equals(@Nullable Object other) { - if (this == other) { - return true; - } - if (!(other instanceof CacheOperationSourcePointcut otherPc)) { - return false; - } - return ObjectUtils.nullSafeEquals(getCacheOperationSource(), otherPc.getCacheOperationSource()); + return (this == other || (other instanceof CacheOperationSourcePointcut otherPc && + ObjectUtils.nullSafeEquals(this.cacheOperationSource, otherPc.cacheOperationSource))); } @Override @@ -66,18 +69,10 @@ public int hashCode() { @Override public String toString() { - return getClass().getName() + ": " + getCacheOperationSource(); + return getClass().getName() + ": " + this.cacheOperationSource; } - /** - * Obtain the underlying {@link CacheOperationSource} (may be {@code null}). - * To be implemented by subclasses. - */ - @Nullable - protected abstract CacheOperationSource getCacheOperationSource(); - - /** * {@link ClassFilter} that delegates to {@link CacheOperationSource#isCandidateClass} * for filtering classes whose methods are not worth searching to begin with. @@ -89,8 +84,7 @@ public boolean matches(Class clazz) { if (CacheManager.class.isAssignableFrom(clazz)) { return false; } - CacheOperationSource cas = getCacheOperationSource(); - return (cas == null || cas.isCandidateClass(clazz)); + return (cacheOperationSource == null || cacheOperationSource.isCandidateClass(clazz)); } } diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/BeanFactoryTransactionAttributeSourceAdvisor.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/BeanFactoryTransactionAttributeSourceAdvisor.java index f52410ed9eaa..75f6b391d53f 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/BeanFactoryTransactionAttributeSourceAdvisor.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/BeanFactoryTransactionAttributeSourceAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 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,7 +19,6 @@ import org.springframework.aop.ClassFilter; import org.springframework.aop.Pointcut; import org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor; -import org.springframework.lang.Nullable; /** * Advisor driven by a {@link TransactionAttributeSource}, used to include @@ -34,16 +33,7 @@ @SuppressWarnings("serial") public class BeanFactoryTransactionAttributeSourceAdvisor extends AbstractBeanFactoryPointcutAdvisor { - @Nullable - private TransactionAttributeSource transactionAttributeSource; - - private final TransactionAttributeSourcePointcut pointcut = new TransactionAttributeSourcePointcut() { - @Override - @Nullable - protected TransactionAttributeSource getTransactionAttributeSource() { - return transactionAttributeSource; - } - }; + private final TransactionAttributeSourcePointcut pointcut = new TransactionAttributeSourcePointcut(); /** @@ -53,7 +43,7 @@ protected TransactionAttributeSource getTransactionAttributeSource() { * @see TransactionInterceptor#setTransactionAttributeSource */ public void setTransactionAttributeSource(TransactionAttributeSource transactionAttributeSource) { - this.transactionAttributeSource = transactionAttributeSource; + this.pointcut.setTransactionAttributeSource(transactionAttributeSource); } /** diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourceAdvisor.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourceAdvisor.java index 5e65816fdba3..bbaca68870bb 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourceAdvisor.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourceAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 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. @@ -43,13 +43,7 @@ public class TransactionAttributeSourceAdvisor extends AbstractPointcutAdvisor { @Nullable private TransactionInterceptor transactionInterceptor; - private final TransactionAttributeSourcePointcut pointcut = new TransactionAttributeSourcePointcut() { - @Override - @Nullable - protected TransactionAttributeSource getTransactionAttributeSource() { - return (transactionInterceptor != null ? transactionInterceptor.getTransactionAttributeSource() : null); - } - }; + private final TransactionAttributeSourcePointcut pointcut = new TransactionAttributeSourcePointcut(); /** @@ -71,7 +65,9 @@ public TransactionAttributeSourceAdvisor(TransactionInterceptor interceptor) { * Set the transaction interceptor to use for this advisor. */ public void setTransactionInterceptor(TransactionInterceptor interceptor) { + Assert.notNull(interceptor, "TransactionInterceptor must not be null"); this.transactionInterceptor = interceptor; + this.pointcut.setTransactionAttributeSource(interceptor.getTransactionAttributeSource()); } /** 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 a7cdc8867d02..0877acaf8a70 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 @@ -34,28 +34,31 @@ * @since 2.5.5 */ @SuppressWarnings("serial") -abstract class TransactionAttributeSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { +class TransactionAttributeSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { - protected TransactionAttributeSourcePointcut() { + @Nullable + private TransactionAttributeSource transactionAttributeSource; + + + public TransactionAttributeSourcePointcut() { setClassFilter(new TransactionAttributeSourceClassFilter()); } + public void setTransactionAttributeSource(@Nullable TransactionAttributeSource transactionAttributeSource) { + this.transactionAttributeSource = transactionAttributeSource; + } + @Override public boolean matches(Method method, Class targetClass) { - TransactionAttributeSource tas = getTransactionAttributeSource(); - return (tas == null || tas.getTransactionAttribute(method, targetClass) != null); + return (this.transactionAttributeSource == null || + this.transactionAttributeSource.getTransactionAttribute(method, targetClass) != null); } @Override public boolean equals(@Nullable Object other) { - if (this == other) { - return true; - } - if (!(other instanceof TransactionAttributeSourcePointcut otherPc)) { - return false; - } - return ObjectUtils.nullSafeEquals(getTransactionAttributeSource(), otherPc.getTransactionAttributeSource()); + return (this == other || (other instanceof TransactionAttributeSourcePointcut otherPc && + ObjectUtils.nullSafeEquals(this.transactionAttributeSource, otherPc.transactionAttributeSource))); } @Override @@ -65,18 +68,10 @@ public int hashCode() { @Override public String toString() { - return getClass().getName() + ": " + getTransactionAttributeSource(); + return getClass().getName() + ": " + this.transactionAttributeSource; } - /** - * Obtain the underlying TransactionAttributeSource (may be {@code null}). - * To be implemented by subclasses. - */ - @Nullable - protected abstract TransactionAttributeSource getTransactionAttributeSource(); - - /** * {@link ClassFilter} that delegates to {@link TransactionAttributeSource#isCandidateClass} * for filtering classes whose methods are not worth searching to begin with. @@ -90,8 +85,7 @@ public boolean matches(Class clazz) { PersistenceExceptionTranslator.class.isAssignableFrom(clazz)) { return false; } - TransactionAttributeSource tas = getTransactionAttributeSource(); - return (tas == null || tas.isCandidateClass(clazz)); + return (transactionAttributeSource == null || transactionAttributeSource.isCandidateClass(clazz)); } }