Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Support type-safe transaction rollback rules

Browse Source 
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
This commit is contained in:
Sam Brannen
2022-03-04 18:28:48 +01:00
parent 9cb4783296
commit c1033dbfb3
4 changed files with 140 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -50,38 +50,42 @@ import org.springframework.transaction.TransactionDefinition;
* 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,7 +211,7 @@ public @interface Transactional {
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}
@@ -215,10 +219,9 @@ public @interface Transactional {
* {@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 @@ public @interface Transactional {
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)

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

@@ -27,21 +27,28 @@ import org.springframework.util.Assert;
* <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,27 +67,36 @@ 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)) {
@@ -88,6 +104,7 @@ public class RollbackRuleAttribute implements Serializable{
"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 class RollbackRuleAttribute implements Serializable{
* {@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 class RollbackRuleAttribute implements Serializable{
public RollbackRuleAttribute(String exceptionPattern) {
Assert.hasText(exceptionPattern, "'exceptionPattern' cannot be null or empty");
this.exceptionPattern = exceptionPattern;
this.exceptionType = null;
}
@@ -129,7 +149,8 @@ public class RollbackRuleAttribute implements Serializable{
* <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 class RollbackRuleAttribute implements Serializable{
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;
}

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

@@ -51,15 +51,6 @@ class RollbackRuleAttributeTests {
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 @@ class RollbackRuleAttributeTests {
@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 @@ class RollbackRuleAttributeTests {
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,12 +106,18 @@ class RollbackRuleAttributeTests {
}
@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();
RollbackRuleAttribute rr = new RollbackRuleAttribute(MyException.class);
assertThat(rr.getDepth(new MyException2())).isEqualTo(-1);
}
@Test
void notFoundWhenExceptionThrownIsNestedTypeOfRegisteredException() {
RollbackRuleAttribute rr = new RollbackRuleAttribute(EnclosingException.class);
assertThat(rr.getDepth(new EnclosingException.NestedException())).isEqualTo(-1);
}
@Test
@@ -117,18 +126,6 @@ class RollbackRuleAttributeTests {
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);
@@ -136,6 +133,15 @@ class RollbackRuleAttributeTests {
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);
}
}

49
src/docs/asciidoc/data-access.adoc
View File

@@ -1042,40 +1042,45 @@ including checked exceptions by specifying _rollback rules_.
[NOTE]
====
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 `Throwable`), with no wildcard support at present. For example, a value of
`"jakarta.servlet.ServletException"` or `"ServletException"` will match
`jakarta.servlet.ServletException` and its subclasses.
thrown, and the rules are based on exception types or exception patterns.
Rollback rules may be configured in XML via the `rollback-for` and `no-rollback-for`
attributes, which allow patterns to be specified as strings. When using
attributes, which allow rules to be defined as patterns. When using
<<transaction-declarative-attransactional-settings,`@Transactional`>>, rollback rules may
be configured via the `rollbackFor`/`noRollbackFor` and
`rollbackForClassName`/`noRollbackForClassName` attributes, which allow patterns to be
specified as `Class` references or strings, respectively. When an exception type is
specified as a class reference its fully qualified name will be used as the pattern.
Consequently, `@Transactional(rollbackFor = example.CustomException.class)` is equivalent
to `@Transactional(rollbackForClassName = "example.CustomException")`.
`rollbackForClassName`/`noRollbackForClassName` attributes, which allow rules to be
defined based on exception types or patterns, respectively.
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 `jakarta.servlet.ServletException.class` will only match thrown exceptions of
type `jakarta.servlet.ServletException` and its subclasses.
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 `Throwable`), with no wildcard support at present. For
example, a value of `"jakarta.servlet.ServletException"` or `"ServletException"` will
match `jakarta.servlet.ServletException` and its subclasses.
[WARNING]
=====
You must carefully consider how specific the pattern is and whether to include package
You must carefully consider how specific a pattern is and whether to include package
information (which isn't mandatory). For example, `"Exception"` will match nearly
anything and will probably hide other rules. `"java.lang.Exception"` would be correct if
`"Exception"` were meant to define a rule for all checked exceptions. With more unique
exception names such as `"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 `com.example.CustomException`, that rule would match against
an exception named `com.example.CustomExceptionV2` (an exception in the same package as
`CustomException` but with an additional suffix) or an exception named
`com.example.CustomException$AnotherException` (an exception declared as a nested class
in `CustomException`).
Furthermore, pattern-based 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 pattern-based rollback rule if the name
of the thrown exception contains the exception pattern configured for the rollback rule.
For example, given a rule configured to match on `"com.example.CustomException"`, that
rule will match against an exception named `com.example.CustomExceptionV2` (an exception
in the same package as `CustomException` but with an additional suffix) or an exception
named `com.example.CustomException$AnotherException` (an exception declared as a nested
class in `CustomException`).
=====
====
@@ -1770,7 +1775,7 @@ properties of the `@Transactional` annotation:
TIP: See <<transaction-declarative-rollback-rules, Rollback rules>> for further details
on rollback rule semantics, patterns, and warnings regarding possible unintentional
matches.
matches for pattern-based rollback rules.
Currently, you cannot have explicit control over the name of a transaction, where 'name'
means the transaction name that appears in a transaction monitor, if applicable