Redesign inner Pointcut implementations as standalone classes

Avoids exposure of implicit Advisor instance state in Pointcut instance. See gh-30621
spring-projects · Jun 8, 2023 · 6931106 · 6931106
1 parent 045df81
commit 6931106
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 90 deletions.
diff --git a/...org/springframework/cache/jcache/interceptor/BeanFactoryJCacheOperationSourceAdvisor.java b/...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/...main/java/org/springframework/cache/jcache/interceptor/JCacheOperationSourcePointcut.java b/...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/...in/java/org/springframework/cache/interceptor/BeanFactoryCacheOperationSourceAdvisor.java b/...in/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/...ext/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java b/...ext/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/...springframework/transaction/interceptor/BeanFactoryTransactionAttributeSourceAdvisor.java b/...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/...n/java/org/springframework/transaction/interceptor/TransactionAttributeSourceAdvisor.java b/...n/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/.../java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java b/.../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));
 		}
 	}