Skip to content

Commit

Permalink
Document that TX rollback rules may result in unintentional matches
Browse files Browse the repository at this point in the history 
Closes gh-28125
  • Loading branch information
@sbrannen
sbrannen committed Mar 4, 2022
1 parent b3e5f86 commit fa3130d
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 87 deletions.
105 changes: 72 additions & 33 deletions spring-tx/src/main/java/org/springframework/transaction/annotation/Transactional.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 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.
Expand Down Expand Up @@ -37,17 +37,57 @@
* <a href="https://docs.spring.io/spring-framework/docs/current/reference/html/data-access.html#transaction">Transaction Management</a>
* section of the reference manual.
*
* <p>This annotation type is generally directly comparable to Spring's
* <p>This annotation is generally directly comparable to Spring's
* {@link org.springframework.transaction.interceptor.RuleBasedTransactionAttribute}
* class, and in fact {@link AnnotationTransactionAttributeSource} will directly
* convert the data to the latter class, so that Spring's transaction support code
* does not have to know about annotations. If no custom rollback rules apply,
* the transaction will roll back on {@link RuntimeException} and {@link Error}
* but not on checked exceptions.
* convert this annotation's attributes to properties in {@code RuleBasedTransactionAttribute},
* so that Spring's transaction support code does not have to know about annotations.
*
* <p>For specific information about the semantics of this annotation's attributes,
* consult the {@link org.springframework.transaction.TransactionDefinition} and
* {@link org.springframework.transaction.interceptor.TransactionAttribute} javadocs.
* <h3>Attribute Semantics</h3>
*
* <p>If no custom rollback rules are configured in this annotation, the transaction
* will roll back on {@link RuntimeException} and {@link Error} but not on checked
* 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
* wildcard support at present. For example, a value of
* {@code "javax.servlet.ServletException"} or {@code "ServletException"} will
* match {@code javax.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
* 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
* {@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}).
*
* <p>For specific information about the semantics of other attributes in this
* annotation, consult the {@link org.springframework.transaction.TransactionDefinition}
* and {@link org.springframework.transaction.interceptor.TransactionAttribute} javadocs.
*
* <h3>Transaction Management</h3>
*
* <p>This annotation commonly works with thread-bound transactions managed by a
* {@link org.springframework.transaction.PlatformTransactionManager}, exposing a
Expand Down Expand Up @@ -167,37 +207,33 @@
boolean readOnly() default false;

/**
* Defines zero (0) or more exception {@link Class classes}, which must be
* Defines zero (0) or more exception {@linkplain Class classes}, which must be
* subclasses of {@link Throwable}, indicating which exception types must cause
* a transaction rollback.
* <p>By default, a transaction will be rolling back on {@link RuntimeException}
* <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 class and its subclasses.
* <p>Similar to {@link org.springframework.transaction.interceptor.RollbackRuleAttribute#RollbackRuleAttribute(Class clazz)}.
* {@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.
* @see #rollbackForClassName
* @see org.springframework.transaction.interceptor.RollbackRuleAttribute#RollbackRuleAttribute(Class)
* @see org.springframework.transaction.interceptor.DefaultTransactionAttribute#rollbackOn(Throwable)
*/
Class<? extends Throwable>[] rollbackFor() default {};

/**
* Defines zero (0) or more exception names (for exceptions which must be a
* Defines zero (0) or more exception name patterns (for exceptions which must be a
* subclass of {@link Throwable}), indicating which exception types must cause
* a transaction rollback.
* <p>This can be a substring of a fully qualified class name, with no wildcard
* support at present. For example, a value of {@code "ServletException"} would
* match {@code javax.servlet.ServletException} and its subclasses.
* <p><b>NB:</b> Consider carefully how specific the 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 unusual
* {@link Exception} names such as {@code "BaseBusinessException"} there is no
* need to use a FQN.
* <p>Similar to {@link org.springframework.transaction.interceptor.RollbackRuleAttribute#RollbackRuleAttribute(String exceptionName)}.
* <p>See the {@linkplain Transactional class-level javadocs} for further details
* on rollback rule semantics, patterns, and warnings regarding possible
* unintentional matches.
* @see #rollbackFor
* @see org.springframework.transaction.interceptor.RollbackRuleAttribute#RollbackRuleAttribute(String)
* @see org.springframework.transaction.interceptor.DefaultTransactionAttribute#rollbackOn(Throwable)
*/
String[] rollbackForClassName() default {};
Expand All @@ -206,23 +242,26 @@
* Defines zero (0) or more exception {@link Class Classes}, 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 class and
* its subclasses.
* <p>Similar to {@link org.springframework.transaction.interceptor.NoRollbackRuleAttribute#NoRollbackRuleAttribute(Class clazz)}.
* <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.
* @see #noRollbackForClassName
* @see org.springframework.transaction.interceptor.NoRollbackRuleAttribute#NoRollbackRuleAttribute(Class)
* @see org.springframework.transaction.interceptor.DefaultTransactionAttribute#rollbackOn(Throwable)
*/
Class<? extends Throwable>[] noRollbackFor() default {};

/**
* Defines zero (0) or more exception names (for exceptions which must be a
* Defines zero (0) or more exception name patterns (for exceptions which must be a
* subclass of {@link Throwable}) indicating which exception types must <b>not</b>
* cause a transaction rollback.
* <p>See the description of {@link #rollbackForClassName} for further
* information on how the specified names are treated.
* <p>Similar to {@link org.springframework.transaction.interceptor.NoRollbackRuleAttribute#NoRollbackRuleAttribute(String exceptionName)}.
* <p>See the {@linkplain Transactional class-level javadocs} for further details
* on rollback rule semantics, patterns, and warnings regarding possible
* unintentional matches.
* @see #noRollbackFor
* @see org.springframework.transaction.interceptor.NoRollbackRuleAttribute#NoRollbackRuleAttribute(String)
* @see org.springframework.transaction.interceptor.DefaultTransactionAttribute#rollbackOn(Throwable)
*/
String[] noRollbackForClassName() default {};
Expand Down
25 changes: 16 additions & 9 deletions ...tx/src/main/java/org/springframework/transaction/interceptor/NoRollbackRuleAttribute.java
View file
@@ -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.
Expand All @@ -21,29 +21,36 @@
* to the {@code RollbackRuleAttribute} superclass.
*
* @author Rod Johnson
* @author Sam Brannen
* @since 09.04.2003
*/
@SuppressWarnings("serial")
public class NoRollbackRuleAttribute extends RollbackRuleAttribute {

/**
* Create a new instance of the {@code NoRollbackRuleAttribute} class
* for the supplied {@link Throwable} class.
* @param clazz the {@code Throwable} class
* for the given {@code exceptionType}.
* @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}
* @see RollbackRuleAttribute#RollbackRuleAttribute(Class)
*/
public NoRollbackRuleAttribute(Class<?> clazz) {
super(clazz);
public NoRollbackRuleAttribute(Class<?> exceptionType) {
super(exceptionType);
}

/**
* Create a new instance of the {@code NoRollbackRuleAttribute} class
* for the supplied {@code exceptionName}.
* @param exceptionName the exception name pattern
* for the supplied {@code exceptionPattern}.
* @param exceptionPattern the exception name pattern; can also be a fully
* package-qualified class name
* @throws IllegalArgumentException if the supplied {@code exceptionPattern}
* is {@code null} or empty
* @see RollbackRuleAttribute#RollbackRuleAttribute(String)
*/
public NoRollbackRuleAttribute(String exceptionName) {
super(exceptionName);
public NoRollbackRuleAttribute(String exceptionPattern) {
super(exceptionPattern);
}

@Override
Expand Down
47 changes: 32 additions & 15 deletions ...g-tx/src/main/java/org/springframework/transaction/interceptor/RollbackRuleAttribute.java
View file
Expand Up @@ -27,6 +27,22 @@
* <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 "javax.servlet.ServletException"}
* or {@code "ServletException"} would match {@code javax.servlet.ServletException}
* and its subclasses.
*
* <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
* {@link org.springframework.transaction.annotation.Transactional @Transactional}
* for further details on rollback rule semantics, patterns, and warnings regarding
* possible unintentional matches.
*
* @author Rod Johnson
* @author Sam Brannen
* @since 09.04.2003
Expand Down Expand Up @@ -56,6 +72,10 @@ public class RollbackRuleAttribute implements Serializable{
* 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.
* <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.
* @param exceptionType exception type; must be {@link Throwable} or a subclass
* of {@code Throwable}
* @throws IllegalArgumentException if the supplied {@code exceptionType} is
Expand All @@ -73,16 +93,10 @@ public RollbackRuleAttribute(Class<?> exceptionType) {
/**
* Create a new instance of the {@code RollbackRuleAttribute} class
* 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 unique
* exception names such as "BaseBusinessException" there's no need to use a
* fully package-qualified name.
* <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.
* @param exceptionPattern the exception name pattern; can also be a fully
* package-qualified class name
* @throws IllegalArgumentException if the supplied {@code exceptionPattern}
Expand All @@ -106,7 +120,7 @@ public String getExceptionName() {
* 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>{@code 0} means this rule matches the supplied {@code exception} directly.</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
Expand All @@ -115,22 +129,25 @@ 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
* will return a depth signifying a match at the corresponding level in the
* class hierarchy as if there had been a direct match.
*/
public int getDepth(Throwable exception) {
return getDepth(exception.getClass(), 0);
}


private int getDepth(Class<?> exceptionClass, int depth) {
if (exceptionClass.getName().contains(this.exceptionPattern)) {
private int getDepth(Class<?> exceptionType, int depth) {
if (exceptionType.getName().contains(this.exceptionPattern)) {
// Found it!
return depth;
}
// If we've gone as far as we can go and haven't found it...
if (exceptionClass == Throwable.class) {
if (exceptionType == Throwable.class) {
return -1;
}
return getDepth(exceptionClass.getSuperclass(), depth + 1);
return getDepth(exceptionType.getSuperclass(), depth + 1);
}


Expand Down

0 comments on commit fa3130d

Please sign in to comment.