Affects: Spring Framework 4.3.0+

When @Primary support was added for spring-test in 677a321, the lookup was added before TransactionManagementConfigurer lookup and not after as it should have been, given that TransactionManagementConfigurer should take higher precedence. In fact maybe it should be the first check even before the beansOfTypeIncludingAncestors lookup as the configurer can construct a TransactionManager that is not a registered bean (a registered bean could be in the context that is not the one that the configurer will return).

Steps to reproduce

Configure a context with a @Primary TransactionManager and a TransactionManagementConfigurer.

Current result

Test uses the @Primary annotated bean.

Expected result

Test should use the bean provided by TransactionManagementConfigurer.

Comment From: sbrannen

Thank you for bringing this to our attention! 👍

There is indeed a discrepancy between the transaction manager lookup algorithm in production code and the algorithm used within the Spring TestContext Framework (TCF).

I introduced tests for the status quo for production code in 144b0e143759509c2b1497b8a44967209b425274 and for the TCF in a5498ba2ad629616030ffd6b5f3ca88954d7b4d7.

In Spring Framework 5.3 we will revise the transaction manager lookup in TestContextTransactionUtils so that a transaction manager configured via the TransactionManagementConfigurer API is favored over a @Primary transaction manager.

Comment From: friscoMad

Thank you for such fast reply. I know that this situation is a fringe case (I have several configurations in my classpath that I can not remove) and probably not a lot of projects are using TransactionManagementConfigurer I did some more test after the report to confirm that in production it does work as it should and I think that the lookup is here and has this priority:

  1. Qualified
  2. TransactionManagementConfigurer
  3. Regular bean search (@Primary or single one)

Comment From: sbrannen

This has been addressed in 613bd3be1db254f713e39dafcf22f52686611cd7.

Feel free to try it out in an upcoming 5.3 M1 snapshot.

Comment From: friscoMad

I will try as soon as posible but while the change does fix my initial report and my use case, it still does not replicate production transaction manager lookup, at least by my understanding looking at production code, the TransactionManagementConfigurer is the top priority unless the annotation is qualified, so by my understanding the code block in lines 187-191 (single transaction manager bean) should be moved after TransactionManagementConfigurer block

Comment From: sbrannen

Reopening to align with production behavior.

Comment From: sbrannen

@friscoMad, thanks for the feedback.

it still does not replicate production transaction manager lookup, at least by my understanding looking at production code, the TransactionManagementConfigurer is the top priority unless the annotation is qualified.

According to the Javadoc for TransactionManagementConfigurer.annotationDrivenTransactionManager():

it is important that the PlatformTransactionManager instance is managed as a Spring bean within the container since most PlatformTransactionManager implementations take advantage of Spring lifecycle callbacks such as InitializingBean and BeanFactoryAware.

Thus, although the scenario you have described is highly discouraged, it is technically possible and supported in production arrangements.

I have therefore further refined the transaction manager lookup in the TestContext framework in 715e8c9ef6ec58fd55b7e11e35586a0806b76cc0.