Affects: 5.2.2
Currently we are migrating from from Spring 4.3.23 to 5.2.2(3). During the process we found out that some of our methods, which should be running without transaction, started to fail because of missing context (related to our application) to start transaction.
In example below (only for idea, not really tested) only one method will create transaction in 4.3 (expected) but both methods will create transactions in 5.2 (wrong):
@Transactional(propagation=Propagation.REQUIRED)
interface TransactionalInterface {
void transactionalMethod();
}
interface NormalInterface {
void normalMethod();
}
class TargetClass implements TransactionalInterface, NormalInterface {
@Override
public void transactionalMethod() {
System.out.println("Method starting transaction");
}
@Override
public void normalMethod() {
System.out.println("Method shouldn't start transaction but does");
}
}
After some debugging I found out that behavior changed after commit https://github.com/spring-projects/spring-framework/commit/42d6d7ec4e2ddabc9c50dab6d6ac572ef46d2a6a. Removing check for existing annotations on passed element in AnnotationTransactionAttributeSource#determineTransactionAttribute
caused that all @Transactional
annotations from target class hierarchy are merged (as far as I understand it) and returned already for target class level (second lookup in AbstractFallbackTransactionAttributeSource#computeTransactionAttribute
). By disregarding next two lookups for annotations on original method or its class/interface it basically "propagate" @Transactional
to all implemented interfaces. And that's wrong behavior from my point of view.
Comment From: jhoeller
While this is technically a regression in 5.0, the behavior for annotated interfaces was never well-defined to begin with. In 5.0, we opted to exhaustively find @Transactional
in particular for CGLIB proxies which implement interfaces with annotation declarations since those were silently ignored before. As a side effect, class-level annotations on an interface also apply to the implementing class now just like they were declared on that class itself. Given that CGLIB proxies are the common choice in Spring Boot now, this seems like the better tradeoff. Also, with the previous lookup arrangement, method-level transaction declarations might have been ignored which is arguably worse than over-applying class-level transactions. Last but not least, this has been the behavior for two generations of the framework now, so a lot of code might rely on those new over-applying semantics already. I'm afraid there is not much we can do about this now.
In practice, I advise against class-level @Transactional
declarations in any scenario. For clarity and readability, I rather recommend local @Transactional
declarations for every affected method. If the class-level semantics turn out to remain a source of trouble, I'd rather deprecate those completely than change its semantics again.