Support type-safe transaction rollback rules

Prior to this commit, there was no way to configure type-safe rollback
rules for transactions.

Even though a rollback rule could be defined using a Class reference
via the `rollbackFor` and `noRollbackFor` attributes in @transactional,
those Class references got converted to Strings (as the fully qualified
class names of the exception types) in RollbackRuleAttribute which then
applied a pattern-based matching algorithm as if the Class references
had been supplied as Strings/patterns to begin with, thereby losing the
type information.

Pattern-based rollback rules suffer from the following three categories
of unintentional matches.

- identically named exceptions in different packages when the pattern
  does not include the package name -- for example,
  example.client.WebException and example.server.WebException both
  match against a "WebException" pattern.

- similarly named exceptions in the same package when a given exception
  name starts with the name of another exception -- for example,
  example.BusinessException and example.BusinessExceptionWithDetails
  both match against an "example.BusinessException" pattern.

- nested exceptions when an exception type is declared in another
  exception -- for example, example.BusinessException and
  example.BusinessException$NestedException both match against an
  "example.BusinessException" pattern.

This commit prevents the latter two categories of unintentional matches
for rollback rules defined using a Class reference by storing the
exceptionType in RollbackRuleAttribute and using that type in the
implementation of RollbackRuleAttribute.getDepth(Class, int), resulting
in type-safe rollback rules whenever the `rollbackFor` and
`noRollbackFor` attributes in `@Transactional` are used.

Note that the first category of unintentional matches never applied to
rollback rules created from a Class reference since the fully qualified
name of a Class reference always includes the package name.

Closes gh-28098

spring-tx/src/main/java/org/springframework/transaction/annotation/Transactional.java

@@ -50,38 +50,42 @@
  * exceptions.
  *
  * <p>Rollback rules determine if a transaction should be rolled back when a given
- * exception is thrown, and the rules are based on patterns. A pattern can be a
- * fully qualified class name or a substring of a fully qualified class name for
- * an exception type (which must be a subclass of {@code Throwable}), with no
+ * exception is thrown, and the rules are based on types or patterns. Custom
+ * rules may be configured via {@link #rollbackFor}/{@link #noRollbackFor} and
+ * {@link #rollbackForClassName}/{@link #noRollbackForClassName}, which allow
+ * rules to be specified as types or patterns, respectively.
+ *
+ * <p>When a rollback rule is defined with an exception type, that type will be
+ * used to match against the type of a thrown exception and its super types,
+ * providing type safety and avoiding any unintentional matches that may occur
+ * when using a pattern. For example, a value of
+ * {@code jakarta.servlet.ServletException.class} will only match thrown exceptions
+ * of type {@code jakarta.servlet.ServletException} and its subclasses.
+ *
+ * <p>When a rollback rule is defined with an exception pattern, the pattern can
+ * be a fully qualified class name or a substring of a fully qualified class name
+ * for an exception type (which must be a subclass of {@code Throwable}), with no
  * wildcard support at present. For example, a value of
  * {@code "jakarta.servlet.ServletException"} or {@code "ServletException"} will
  * match {@code jakarta.servlet.ServletException} and its subclasses.
  *
- * <p>Rollback rules may be configured via {@link #rollbackFor}/{@link #noRollbackFor}
- * and {@link #rollbackForClassName}/{@link #noRollbackForClassName}, which allow
- * patterns to be specified as {@link Class} references or {@linkplain String
- * strings}, respectively. When an exception type is specified as a class reference
- * its fully qualified name will be used as the pattern. Consequently,
- * {@code @Transactional(rollbackFor = example.CustomException.class)} is equivalent
- * to {@code @Transactional(rollbackForClassName = "example.CustomException")}.
- *
- * <p><strong>WARNING:</strong> You must carefully consider how specific the pattern
+ * <p><strong>WARNING:</strong> You must carefully consider how specific a pattern
  * is and whether to include package information (which isn't mandatory). For example,
  * {@code "Exception"} will match nearly anything and will probably hide other
  * rules. {@code "java.lang.Exception"} would be correct if {@code "Exception"}
  * were meant to define a rule for all checked exceptions. With more unique
  * exception names such as {@code "BaseBusinessException"} there is likely no
  * need to use the fully qualified class name for the exception pattern. Furthermore,
- * rollback rules may result in unintentional matches for similarly named exceptions
- * and nested classes. This is due to the fact that a thrown exception is considered
- * to be a match for a given rollback rule if the name of thrown exception contains
- * the exception pattern configured for the rollback rule. For example, given a
- * rule configured to match on {@code com.example.CustomException}, that rule
- * would match against an exception named
- * {@code com.example.CustomExceptionV2} (an exception in the same package as
+ * rollback rules defined via patterns may result in unintentional matches for
+ * similarly named exceptions and nested classes. This is due to the fact that a
+ * thrown exception is considered to be a match for a given pattern-based rollback
+ * rule if the name of thrown exception contains the exception pattern configured
+ * for the rollback rule. For example, given a rule configured to match against
+ * {@code "com.example.CustomException"}, that rule will match against an exception
+ * named {@code com.example.CustomExceptionV2} (an exception in the same package as
  * {@code CustomException} but with an additional suffix) or an exception named
- * {@code com.example.CustomException$AnotherException}
- * (an exception declared as a nested class in {@code CustomException}).
+ * {@code com.example.CustomException$AnotherException} (an exception declared as
+ * a nested class in {@code CustomException}).
  *
  * <p>For specific information about the semantics of other attributes in this
  * annotation, consult the {@link org.springframework.transaction.TransactionDefinition}
@@ -207,18 +211,17 @@
 	boolean readOnly() default false;
 
 	/**
-	 * Defines zero (0) or more exception {@linkplain Class classes}, which must be
+	 * Defines zero (0) or more exception {@linkplain Class types}, which must be
 	 * subclasses of {@link Throwable}, indicating which exception types must cause
 	 * a transaction rollback.
 	 * <p>By default, a transaction will be rolled back on {@link RuntimeException}
 	 * and {@link Error} but not on checked exceptions (business exceptions). See
 	 * {@link org.springframework.transaction.interceptor.DefaultTransactionAttribute#rollbackOn(Throwable)}
 	 * for a detailed explanation.
 	 * <p>This is the preferred way to construct a rollback rule (in contrast to
-	 * {@link #rollbackForClassName}), matching the exception type, its subclasses,
-	 * and its nested classes. See the {@linkplain Transactional class-level javadocs}
-	 * for further details on rollback rule semantics and warnings regarding possible
-	 * unintentional matches.
+	 * {@link #rollbackForClassName}), matching the exception type and its subclasses
+	 * in a type-safe manner. See the {@linkplain Transactional class-level javadocs}
+	 * for further details on rollback rule semantics.
 	 * @see #rollbackForClassName
 	 * @see org.springframework.transaction.interceptor.RollbackRuleAttribute#RollbackRuleAttribute(Class)
 	 * @see org.springframework.transaction.interceptor.DefaultTransactionAttribute#rollbackOn(Throwable)
@@ -239,14 +242,13 @@
 	String[] rollbackForClassName() default {};
 
 	/**
-	 * Defines zero (0) or more exception {@link Class Classes}, which must be
+	 * Defines zero (0) or more exception {@link Class types}, which must be
 	 * subclasses of {@link Throwable}, indicating which exception types must
 	 * <b>not</b> cause a transaction rollback.
 	 * <p>This is the preferred way to construct a rollback rule (in contrast to
-	 * {@link #noRollbackForClassName}), matching the exception type, its subclasses,
-	 * and its nested classes. See the {@linkplain Transactional class-level javadocs}
-	 * for further details on rollback rule semantics and warnings regarding possible
-	 * unintentional matches.
+	 * {@link #noRollbackForClassName}), matching the exception type and its subclasses
+	 * in a type-safe manner. See the {@linkplain Transactional class-level javadocs}
+	 * for further details on rollback rule semantics.
 	 * @see #noRollbackForClassName
 	 * @see org.springframework.transaction.interceptor.NoRollbackRuleAttribute#NoRollbackRuleAttribute(Class)
 	 * @see org.springframework.transaction.interceptor.DefaultTransactionAttribute#rollbackOn(Throwable)

spring-tx/src/main/java/org/springframework/transaction/interceptor/RollbackRuleAttribute.java

@@ -27,21 +27,28 @@
  * <p>Multiple such rules can be applied to determine whether a transaction
  * should commit or rollback after an exception has been thrown.
  *
- * <p>Each rule is based on an exception pattern which can be a fully qualified
- * class name or a substring of a fully qualified class name for an exception
- * type (which must be a subclass of {@code Throwable}), with no wildcard support
- * at present. For example, a value of {@code "jakarta.servlet.ServletException"}
- * or {@code "ServletException"} would match {@code jakarta.servlet.ServletException}
- * and its subclasses.
+ * <p>Each rule is based on an exception type or exception pattern, supplied via
+ * {@link #RollbackRuleAttribute(Class)} or {@link #RollbackRuleAttribute(String)},
+ * respectively.
  *
- * <p>An exception pattern can be specified as a {@link Class} reference or a
- * {@link String} in {@link #RollbackRuleAttribute(Class)} and
- * {@link #RollbackRuleAttribute(String)}, respectively. When an exception type
- * is specified as a class reference its fully qualified name will be used as the
- * pattern. See the javadocs for
+ * <p>When a rollback rule is defined with an exception type, that type will be
+ * used to match against the type of a thrown exception and its super types,
+ * providing type safety and avoiding any unintentional matches that may occur
+ * when using a pattern. For example, a value of
+ * {@code jakarta.servlet.ServletException.class} will only match thrown exceptions
+ * of type {@code jakarta.servlet.ServletException} and its subclasses.
+ *
+ * <p>When a rollback rule is defined with an exception pattern, the pattern can
+ * be a fully qualified class name or a substring of a fully qualified class name
+ * for an exception type (which must be a subclass of {@code Throwable}), with no
+ * wildcard support at present. For example, a value of
+ * {@code "jakarta.servlet.ServletException"} or {@code "ServletException"} will
+ * match {@code jakarta.servlet.ServletException} and its subclasses.
+ *
+ * <p>See the javadocs for
  * {@link org.springframework.transaction.annotation.Transactional @Transactional}
  * for further details on rollback rule semantics, patterns, and warnings regarding
- * possible unintentional matches.
+ * possible unintentional matches with pattern-based rules.
  *
  * @author Rod Johnson
  * @author Sam Brannen
@@ -60,34 +67,44 @@ public class RollbackRuleAttribute implements Serializable{
 
 
 	/**
-	 * Could hold exception, resolving class name but would always require FQN.
-	 * This way does multiple string comparisons, but how often do we decide
-	 * whether to roll back a transaction following an exception?
+	 * Exception pattern: used when searching for matches in a thrown exception's
+	 * class hierarchy based on names of exceptions, with zero type safety and
+	 * potentially resulting in unintentional matches for similarly named exception
+	 * types and nested exception types.
 	 */
 	private final String exceptionPattern;
 
+	/**
+	 * Exception type: used to ensure type safety when searching for matches in
+	 * a thrown exception's class hierarchy.
+	 * @since 6.0
+	 */
+	@Nullable
+	private final Class<? extends Throwable> exceptionType;
+
 
 	/**
 	 * Create a new instance of the {@code RollbackRuleAttribute} class
 	 * for the given {@code exceptionType}.
 	 * <p>This is the preferred way to construct a rollback rule that matches
-	 * the supplied exception type, its subclasses, and its nested classes.
+	 * the supplied exception type and its subclasses with type safety.
 	 * <p>See the javadocs for
 	 * {@link org.springframework.transaction.annotation.Transactional @Transactional}
-	 * for further details on rollback rule semantics, patterns, and warnings regarding
-	 * possible unintentional matches.
+	 * for further details on rollback rule semantics.
 	 * @param exceptionType exception type; must be {@link Throwable} or a subclass
 	 * of {@code Throwable}
 	 * @throws IllegalArgumentException if the supplied {@code exceptionType} is
 	 * not a {@code Throwable} type or is {@code null}
 	 */
+	@SuppressWarnings("unchecked")
 	public RollbackRuleAttribute(Class<?> exceptionType) {
 		Assert.notNull(exceptionType, "'exceptionType' cannot be null");
 		if (!Throwable.class.isAssignableFrom(exceptionType)) {
 			throw new IllegalArgumentException(
 					"Cannot construct rollback rule from [" + exceptionType.getName() + "]: it's not a Throwable");
 		}
 		this.exceptionPattern = exceptionType.getName();
+		this.exceptionType = (Class<? extends Throwable>) exceptionType;
 	}
 
 	/**
@@ -97,6 +114,8 @@ public RollbackRuleAttribute(Class<?> exceptionType) {
 	 * {@link org.springframework.transaction.annotation.Transactional @Transactional}
 	 * for further details on rollback rule semantics, patterns, and warnings regarding
 	 * possible unintentional matches.
+	 * <p>For improved type safety and to avoid unintentional matches, use
+	 * {@link #RollbackRuleAttribute(Class)} instead.
 	 * @param exceptionPattern the exception name pattern; can also be a fully
 	 * package-qualified class name
 	 * @throws IllegalArgumentException if the supplied {@code exceptionPattern}
@@ -105,6 +124,7 @@ public RollbackRuleAttribute(Class<?> exceptionType) {
 	public RollbackRuleAttribute(String exceptionPattern) {
 		Assert.hasText(exceptionPattern, "'exceptionPattern' cannot be null or empty");
 		this.exceptionPattern = exceptionPattern;
+		this.exceptionType = null;
 	}
 
 
@@ -129,7 +149,8 @@ public String getExceptionName() {
 	 * <p>When comparing roll back rules that match against a given exception, a rule
 	 * with a lower matching depth wins. For example, a direct match ({@code depth == 0})
 	 * wins over a match in the superclass hierarchy ({@code depth > 0}).
-	 * <p>A match against a nested exception type or similarly named exception type
+	 * <p>When constructed with an exception pattern via {@link #RollbackRuleAttribute(String)},
+	 * a match against a nested exception type or similarly named exception type
 	 * will return a depth signifying a match at the corresponding level in the
 	 * class hierarchy as if there had been a direct match.
 	 */
@@ -139,7 +160,13 @@ public int getDepth(Throwable exception) {
 
 
 	private int getDepth(Class<?> exceptionType, int depth) {
-		if (exceptionType.getName().contains(this.exceptionPattern)) {
+		if (this.exceptionType != null) {
+			if (this.exceptionType.equals(exceptionType)) {
+				// Found it!
+				return depth;
+			}
+		}
+		else if (exceptionType.getName().contains(this.exceptionPattern)) {
 			// Found it!
 			return depth;
 		}

spring-tx/src/test/java/org/springframework/transaction/interceptor/RollbackRuleAttributeTests.java

@@ -51,15 +51,6 @@ void notFound() {
 			assertThat(rr.getDepth(new MyRuntimeException())).isEqualTo(-1);
 		}
 
-		@Test
-		void alwaysFoundForThrowable() {
-			RollbackRuleAttribute rr = new RollbackRuleAttribute(Throwable.class.getName());
-			assertThat(rr.getDepth(new MyRuntimeException())).isGreaterThan(0);
-			assertThat(rr.getDepth(new IOException())).isGreaterThan(0);
-			assertThat(rr.getDepth(new FatalBeanException(null, null))).isGreaterThan(0);
-			assertThat(rr.getDepth(new RuntimeException())).isGreaterThan(0);
-		}
-
 		@Test
 		void foundImmediatelyWhenDirectMatch() {
 			RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class.getName());
@@ -74,6 +65,9 @@ void foundImmediatelyWhenExceptionThrownIsNestedTypeOfRegisteredException() {
 
 		@Test
 		void foundImmediatelyWhenNameOfExceptionThrownStartsWithNameOfRegisteredException() {
+			// Precondition for this use case.
+			assertThat(MyException.class.isAssignableFrom(MyException2.class)).isFalse();
+
 			RollbackRuleAttribute rr = new RollbackRuleAttribute(MyException.class.getName());
 			assertThat(rr.getDepth(new MyException2())).isEqualTo(0);
 		}
@@ -85,6 +79,15 @@ void foundInSuperclassHierarchy() {
 			assertThat(rr.getDepth(new MyRuntimeException())).isEqualTo(3);
 		}
 
+		@Test
+		void alwaysFoundForThrowable() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(Throwable.class.getName());
+			assertThat(rr.getDepth(new MyRuntimeException())).isGreaterThan(0);
+			assertThat(rr.getDepth(new IOException())).isGreaterThan(0);
+			assertThat(rr.getDepth(new FatalBeanException(null, null))).isGreaterThan(0);
+			assertThat(rr.getDepth(new RuntimeException())).isGreaterThan(0);
+		}
+
 	}
 
 	@Nested
@@ -103,30 +106,24 @@ void notFound() {
 		}
 
 		@Test
-		void alwaysFoundForThrowable() {
-			RollbackRuleAttribute rr = new RollbackRuleAttribute(Throwable.class);
-			assertThat(rr.getDepth(new MyRuntimeException())).isGreaterThan(0);
-			assertThat(rr.getDepth(new IOException())).isGreaterThan(0);
-			assertThat(rr.getDepth(new FatalBeanException(null, null))).isGreaterThan(0);
-			assertThat(rr.getDepth(new RuntimeException())).isGreaterThan(0);
-		}
+		void notFoundWhenNameOfExceptionThrownStartsWithNameOfRegisteredException() {
+			// Precondition for this use case.
+			assertThat(MyException.class.isAssignableFrom(MyException2.class)).isFalse();
 
-		@Test
-		void foundImmediatelyWhenDirectMatch() {
-			RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class);
-			assertThat(rr.getDepth(new Exception())).isEqualTo(0);
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(MyException.class);
+			assertThat(rr.getDepth(new MyException2())).isEqualTo(-1);
 		}
 
 		@Test
-		void foundImmediatelyWhenExceptionThrownIsNestedTypeOfRegisteredException() {
+		void notFoundWhenExceptionThrownIsNestedTypeOfRegisteredException() {
 			RollbackRuleAttribute rr = new RollbackRuleAttribute(EnclosingException.class);
-			assertThat(rr.getDepth(new EnclosingException.NestedException())).isEqualTo(0);
+			assertThat(rr.getDepth(new EnclosingException.NestedException())).isEqualTo(-1);
 		}
 
 		@Test
-		void foundImmediatelyWhenNameOfExceptionThrownStartsWithNameOfRegisteredException() {
-			RollbackRuleAttribute rr = new RollbackRuleAttribute(MyException.class);
-			assertThat(rr.getDepth(new MyException2())).isEqualTo(0);
+		void foundImmediatelyWhenDirectMatch() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class);
+			assertThat(rr.getDepth(new Exception())).isEqualTo(0);
 		}
 
 		@Test
@@ -136,6 +133,15 @@ void foundInSuperclassHierarchy() {
 			assertThat(rr.getDepth(new MyRuntimeException())).isEqualTo(3);
 		}
 
+		@Test
+		void alwaysFoundForThrowable() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(Throwable.class);
+			assertThat(rr.getDepth(new MyRuntimeException())).isGreaterThan(0);
+			assertThat(rr.getDepth(new IOException())).isGreaterThan(0);
+			assertThat(rr.getDepth(new FatalBeanException(null, null))).isGreaterThan(0);
+			assertThat(rr.getDepth(new RuntimeException())).isGreaterThan(0);
+		}
+
 	}