Polish rollback rule support

Author: sbrannen

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -22,21 +22,21 @@
 import org.springframework.util.Assert;
 
 /**
- * Rule determining whether or not a given exception (and any subclasses)
- * should cause a rollback.
+ * Rule determining whether or not a given exception should cause a rollback.
  *
  * <p>Multiple such rules can be applied to determine whether a transaction
  * should commit or rollback after an exception has been thrown.
  *
  * @author Rod Johnson
+ * @author Sam Brannen
  * @since 09.04.2003
  * @see NoRollbackRuleAttribute
  */
 @SuppressWarnings("serial")
 public class RollbackRuleAttribute implements Serializable{
 
 	/**
-	 * The {@link RollbackRuleAttribute rollback rule} for
+	 * The {@linkplain RollbackRuleAttribute rollback rule} for
 	 * {@link RuntimeException RuntimeExceptions}.
 	 */
 	public static final RollbackRuleAttribute ROLLBACK_ON_RUNTIME_EXCEPTIONS =
@@ -48,71 +48,81 @@ public class RollbackRuleAttribute implements Serializable{
 	 * This way does multiple string comparisons, but how often do we decide
 	 * whether to roll back a transaction following an exception?
 	 */
-	private final String exceptionName;
+	private final String exceptionPattern;
 
 
 	/**
-	 * Create a new instance of the {@code RollbackRuleAttribute} class.
+	 * 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 {@link Exception} class, its subclasses, and its nested classes.
-	 * @param clazz throwable class; must be {@link Throwable} or a subclass
+	 * the supplied exception type, its subclasses, and its nested classes.
+	 * @param exceptionType exception type; must be {@link Throwable} or a subclass
 	 * of {@code Throwable}
-	 * @throws IllegalArgumentException if the supplied {@code clazz} is
+	 * @throws IllegalArgumentException if the supplied {@code exceptionType} is
 	 * not a {@code Throwable} type or is {@code null}
 	 */
-	public RollbackRuleAttribute(Class<?> clazz) {
-		Assert.notNull(clazz, "'clazz' cannot be null");
-		if (!Throwable.class.isAssignableFrom(clazz)) {
+	public RollbackRuleAttribute(Class<?> exceptionType) {
+		Assert.notNull(exceptionType, "'exceptionType' cannot be null");
+		if (!Throwable.class.isAssignableFrom(exceptionType)) {
 			throw new IllegalArgumentException(
-					"Cannot construct rollback rule from [" + clazz.getName() + "]: it's not a Throwable");
+					"Cannot construct rollback rule from [" + exceptionType.getName() + "]: it's not a Throwable");
 		}
-		this.exceptionName = clazz.getName();
+		this.exceptionPattern = exceptionType.getName();
 	}
 
 	/**
 	 * Create a new instance of the {@code RollbackRuleAttribute} class
-	 * for the given {@code exceptionName}.
+	 * for the given {@code exceptionPattern}.
 	 * <p>This can be a substring, with no wildcard support at present. A value
 	 * of "ServletException" would match
 	 * {@code javax.servlet.ServletException} and subclasses, for example.
 	 * <p><b>NB:</b> Consider carefully how specific the pattern is, and
 	 * whether to include package information (which is not mandatory). For
 	 * example, "Exception" will match nearly anything, and will probably hide
 	 * other rules. "java.lang.Exception" would be correct if "Exception" was
-	 * meant to define a rule for all checked exceptions. With more unusual
+	 * meant to define a rule for all checked exceptions. With more unique
 	 * exception names such as "BaseBusinessException" there's no need to use a
 	 * fully package-qualified name.
-	 * @param exceptionName the exception name pattern; can also be a fully
+	 * @param exceptionPattern the exception name pattern; can also be a fully
 	 * package-qualified class name
-	 * @throws IllegalArgumentException if the supplied
-	 * {@code exceptionName} is {@code null} or empty
+	 * @throws IllegalArgumentException if the supplied {@code exceptionPattern}
+	 * is {@code null} or empty
 	 */
-	public RollbackRuleAttribute(String exceptionName) {
-		Assert.hasText(exceptionName, "'exceptionName' cannot be null or empty");
-		this.exceptionName = exceptionName;
+	public RollbackRuleAttribute(String exceptionPattern) {
+		Assert.hasText(exceptionPattern, "'exceptionPattern' cannot be null or empty");
+		this.exceptionPattern = exceptionPattern;
 	}
 
 
 	/**
-	 * Return the pattern for the exception name.
+	 * Get the configured exception name pattern that this rule uses for matching.
+	 * @see #getDepth(Throwable)
 	 */
 	public String getExceptionName() {
-		return this.exceptionName;
+		return this.exceptionPattern;
 	}
 
 	/**
-	 * Return the depth of the superclass matching.
-	 * <p>{@code 0} means {@code ex} matches exactly. Returns
-	 * {@code -1} if there is no match. Otherwise, returns depth with the
-	 * lowest depth winning.
+	 * Return the depth of the superclass matching, with the following semantics.
+	 * <ul>
+	 * <li>{@code -1} means this rule does not match the supplied {@code exception}.</li>
+	 * <li>{@code 0} means this rule matches the supplied {@code exception} exactly.</li>
+	 * <li>Any other positive value means this rule matches the supplied {@code exception}
+	 * within the superclass hierarchy, where the value is the number of levels in the
+	 * class hierarchy between the supplied {@code exception} and the exception against
+	 * which this rule matches directly.</li>
+	 * </ul>
+	 * <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}).
 	 */
-	public int getDepth(Throwable ex) {
-		return getDepth(ex.getClass(), 0);
+	public int getDepth(Throwable exception) {
+		return getDepth(exception.getClass(), 0);
 	}
 
 
 	private int getDepth(Class<?> exceptionClass, int depth) {
-		if (exceptionClass.getName().contains(this.exceptionName)) {
+		if (exceptionClass.getName().contains(this.exceptionPattern)) {
 			// Found it!
 			return depth;
 		}
@@ -133,17 +143,17 @@ public boolean equals(@Nullable Object other) {
 			return false;
 		}
 		RollbackRuleAttribute rhs = (RollbackRuleAttribute) other;
-		return this.exceptionName.equals(rhs.exceptionName);
+		return this.exceptionPattern.equals(rhs.exceptionPattern);
 	}
 
 	@Override
 	public int hashCode() {
-		return this.exceptionName.hashCode();
+		return this.exceptionPattern.hashCode();
 	}
 
 	@Override
 	public String toString() {
-		return "RollbackRuleAttribute with pattern [" + this.exceptionName + "]";
+		return "RollbackRuleAttribute with pattern [" + this.exceptionPattern + "]";
 	}
 
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -25,7 +25,13 @@
  */
 @SuppressWarnings("serial")
 class MyRuntimeException extends NestedRuntimeException {
+
+	public MyRuntimeException() {
+		super("");
+	}
+
 	public MyRuntimeException(String msg) {
 		super(msg);
 	}
+
 }

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

@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.beans.FatalBeanException;
@@ -36,65 +37,105 @@
  */
 class RollbackRuleAttributeTests {
 
-	@Test
-	void constructorArgumentMustBeThrowableClassWithNonThrowableType() {
-		assertThatIllegalArgumentException().isThrownBy(() -> new RollbackRuleAttribute(Object.class));
-	}
+	@Nested
+	class ExceptionPatternTests {
 
-	@Test
-	void constructorArgumentMustBeThrowableClassWithNullThrowableType() {
-		assertThatIllegalArgumentException().isThrownBy(() -> new RollbackRuleAttribute((Class<?>) null));
-	}
+		@Test
+		void constructorPreconditions() {
+			assertThatIllegalArgumentException().isThrownBy(() -> new RollbackRuleAttribute((String) null));
+		}
 
-	@Test
-	void constructorArgumentMustBeStringWithNull() {
-		assertThatIllegalArgumentException().isThrownBy(() -> new RollbackRuleAttribute((String) null));
-	}
+		@Test
+		void notFound() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(IOException.class.getName());
+			assertThat(rr.getDepth(new MyRuntimeException())).isEqualTo(-1);
+		}
 
-	@Test
-	void notFound() {
-		RollbackRuleAttribute rr = new RollbackRuleAttribute(IOException.class);
-		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 foundImmediatelyWithString() {
-		RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class.getName());
-		assertThat(rr.getDepth(new Exception())).isEqualTo(0);
-	}
+		@Test
+		void foundImmediatelyWhenDirectMatch() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class.getName());
+			assertThat(rr.getDepth(new Exception())).isEqualTo(0);
+		}
 
-	@Test
-	void foundImmediatelyWithClass() {
-		RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class);
-		assertThat(rr.getDepth(new Exception())).isEqualTo(0);
-	}
+		@Test
+		void foundImmediatelyWhenExceptionThrownIsNestedTypeOfRegisteredException() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(EnclosingException.class.getName());
+			assertThat(rr.getDepth(new EnclosingException.NestedException())).isEqualTo(0);
+		}
 
-	@Test
-	void foundInSuperclassHierarchy() {
-		RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class);
-		// Exception -> RuntimeException -> NestedRuntimeException -> MyRuntimeException
-		assertThat(rr.getDepth(new MyRuntimeException(""))).isEqualTo(3);
-	}
+		@Test
+		void foundImmediatelyWhenNameOfExceptionThrownStartsWithNameOfRegisteredException() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(MyException.class.getName());
+			assertThat(rr.getDepth(new MyException2())).isEqualTo(0);
+		}
 
-	@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);
-	}
+		@Test
+		void foundInSuperclassHierarchy() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class.getName());
+			// Exception -> RuntimeException -> NestedRuntimeException -> MyRuntimeException
+			assertThat(rr.getDepth(new MyRuntimeException())).isEqualTo(3);
+		}
 
-	@Test
-	void foundNestedExceptionInEnclosingException() {
-		RollbackRuleAttribute rr = new RollbackRuleAttribute(EnclosingException.class);
-		assertThat(rr.getDepth(new EnclosingException.NestedException())).isEqualTo(0);
 	}
 
-	@Test
-	void foundWhenNameOfExceptionThrownStartsWithTheNameOfTheRegisteredExceptionType() {
-		RollbackRuleAttribute rr = new RollbackRuleAttribute(MyException.class);
-		assertThat(rr.getDepth(new MyException2())).isEqualTo(0);
+	@Nested
+	class ExceptionTypeTests {
+
+		@Test
+		void constructorPreconditions() {
+			assertThatIllegalArgumentException().isThrownBy(() -> new RollbackRuleAttribute(Object.class));
+			assertThatIllegalArgumentException().isThrownBy(() -> new RollbackRuleAttribute((Class<?>) null));
+		}
+
+		@Test
+		void notFound() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(IOException.class);
+			assertThat(rr.getDepth(new MyRuntimeException())).isEqualTo(-1);
+		}
+
+		@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);
+		}
+
+		@Test
+		void foundImmediatelyWhenDirectMatch() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class);
+			assertThat(rr.getDepth(new Exception())).isEqualTo(0);
+		}
+
+		@Test
+		void foundImmediatelyWhenExceptionThrownIsNestedTypeOfRegisteredException() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(EnclosingException.class);
+			assertThat(rr.getDepth(new EnclosingException.NestedException())).isEqualTo(0);
+		}
+
+		@Test
+		void foundImmediatelyWhenNameOfExceptionThrownStartsWithNameOfRegisteredException() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(MyException.class);
+			assertThat(rr.getDepth(new MyException2())).isEqualTo(0);
+		}
+
+		@Test
+		void foundInSuperclassHierarchy() {
+			RollbackRuleAttribute rr = new RollbackRuleAttribute(Exception.class);
+			// Exception -> RuntimeException -> NestedRuntimeException -> MyRuntimeException
+			assertThat(rr.getDepth(new MyRuntimeException())).isEqualTo(3);
+		}
+
 	}