spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

Fix transaction aspect's incorrect state after creating new context

Open SammyVimes opened this issue 6 years ago • 3 comments

As aspects are singletons, transactionManagerCache and transactionManager in TransactionAspectSupport are persistent. It's ok if context is not changing, but if new contexts gets created (in tests, for example), new transaction manager should be used.

I faced it using Spring Data with repository's @Transactional set to MANDATORY:

  1. Service's Transactional method is called
  2. Transaction is created in aspect
  3. Transaction manager is put to cache
  4. Transaction is used in TransactionInterceptor of SpringData repository
  5. New context got created for another test
  6. Service's Transactional method is called
  7. Transaction is created in old transaction manager, that is cached inside aspect
  8. TransactionInterceptor in SpringData repository fails to find transaction, because it uses different (new) transaction manager

I fixed it with clearing cache every time aspect's bean is created (and also remove a null check for txManager parameter, because if it's null, it should clear txManager field and also setter's parameter marked Nullable) and created tests for that.

I also had to change gradle settings for integration tests, because they were not processed by aspectj AND that fixed one strange test where aspects were not found in classpath.

To demonstrate the issue, I also created this repository, where the test which is run second always fails.

SammyVimes avatar Sep 17 '19 20:09 SammyVimes

Potential fix for #11019

sbrannen avatar Sep 24 '19 13:09 sbrannen

Assigned to the 5.x backlog for further investigation

sbrannen avatar Sep 24 '19 13:09 sbrannen

By the way, I assume there would be similar cases for all aspects. If the assumption will be confirmed, I would gladly fix other aspects as well.

SammyVimes avatar Oct 08 '19 06:10 SammyVimes