Source code in question:
private int getDepth(Class<?> exceptionClass, int depth) {
if (exceptionClass.getName().contains(this.exceptionName)) {
// Found it!
return depth;
}
// If we've gone as far as we can go and haven't found it...
if (exceptionClass == Throwable.class) {
return -1;
}
return getDepth(exceptionClass.getSuperclass(), depth + 1);
}
test code:
public class CustomException extends Exception {
}
public class CustomExceptionX extends Exception {
}
@Override
@Transactional(rollbackFor = CustomException.class)
public void testTransaction() throws Exception {
taskMapper.softDeleteById(1, "test");
if (1 == 1) {
throw new CustomExceptionX();
}
}
Comment From: sbrannen
This is by design and was originally implemented using contains()
for use in XML configuration files where users often specified the simple name of a custom exception type instead of the fully qualified class name.
You can see examples of this in the reference docs.
<tx:advice id="txAdvice" transaction-manager="txManager">
<tx:attributes>
<tx:method name="get*" read-only="true" rollback-for="NoProductInStockException"/>
<tx:method name="*"/>
</tx:attributes>
</tx:advice>
With your proposal to use equals()
instead of contains()
, configuration like that would no longer work since NoProductInStockException
would only ever be equal to the fully qualified class name if the NoProductInStockException
was declared as a top-level class in the default package -- which is rather unlikely.
Please take note of the Javadoc for RollbackRuleAttribute
as well:
NB: 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 exception names such as "BaseBusinessException" there's no need to use a fully package-qualified name.
Similar documentation exists for the rollbackForClassName
attribute in @Transactional
.
The Javadoc for the RollbackRuleAttribute(Class)
constructor states the following,
Create a new instance of the RollbackRuleAttribute class.
This is the preferred way to construct a rollback rule that matches the supplied Exception class, its subclasses, and its nested classes.
However, that last sentence is not honored in the current implementation, since the type information (supplied via the Class
reference) is not taken into account in the implementation of getDepth(...)
.
Comment From: sbrannen
Correction to my previous statement. The Javadoc for the RollbackRuleAttribute(Class)
constructor is mostly correct.
However, the documentation for rollback rules can be improved to warn that unintentional matches may arise if the name of a thrown exception contains the name of a registered exception type.
I am therefore repurposing this issue to improve the documentation.
Comment From: hduyyg
I think this rollback rule is fallible and needs more precise matching rules。 A lot of people don't really read documents。
Comment From: sbrannen
I think this rollback rule is fallible and needs more precise matching rules。
I agree with you. I've raised #28125 to improve the documentation for 5.3.x
.
And we'll use this issue to improve the behavior in 6.0
.
Specifically:
- If an exception pattern is supplied as a
String
-- for example, in XML configuration or via@Transactional(rollbackForClassName = "example.CustomException")
-- the existingcontains()
logic will continue to be used. - If a concrete exception type is supplied as a
Class
reference -- for example, via@Transactional(rollbackFor = example.CustomException.class)
-- new logic will be implemented which honors the supplied type information, thereby avoiding an unintentional match againstexample.CustomException2
whenexample.CustomException
(without the2)
was supplied as the exception type.