@SpyBean and @Transactional and SelfInjection regression in 2.6.4

[jonenst]

Hi, when using @SpyBean on a transactional (proxied) bean that uses self injection (to have transaction semantics for inner calls), everything works on 2.6.3, but on 2.6.4 the bean in the test is not the mockito mock anymore, so calling mockito mocks method fail.
Maybe a regression from #29909 ?

Edit: see repro below.

[snicoll]

Thanks for the report.

pom.xml if you want to reproduce this problem:

Can you please take the time to move all of that in a project that we can clone and run. We'd have to do that anyway and we'd rather focus on the problem at hand rather than copy bits and pieces to a new project.

[jonenst]

bar.zip

Is this ok ?

$ unzip bar.zip 
$ cd bar
$ mvn test
 :: Spring Boot ::                (v2.6.4)
 START TEST
service foo.AppTest$App$MyService@1d7c9811
service.self foo.AppTest$App$MyService$MockitoMock$1531434114@39342614
FAILURE
$ mvn test -Dspringboot.version=2.6.3
 :: Spring Boot ::                (v2.6.3)
START TEST
service foo.AppTest$App$MyService$MockitoMock$423369319@612ac38b
service.self foo.AppTest$App$MyService$MockitoMock$423369319@14993306
SUCCESS

[wilkinsona]

Thanks for the sample, @jonenst. Unfortunately, I'm not sure that it's possible to fix this without regressing the fix for #29909 or introducing a similar problem.

I've prototyped something in this branch. The approach that I have taken is to restore the old behavior when raw injection is allowed. It fixes the scenario described in this issue, restoring the behaviour where MyService is spied and a different MyService spy is injected into the MyService spy bean. This is covered by SpyBeanOnTestFieldForExistingSelfInjectingProxiedBeanTests. Unfortunately it does not work for the situation where there are two separate beans with a circular dependency when raw injection is enabled. This is covered by SpyBeanOnTestFieldForExistingCircularBeansWithRawInjectionAllowedIntegrationTests. This latter test passes without the changes to MockitoPostProcessor.

The javadoc for setAllowRawInjectionDespiteWrapping contains the following note:

It is generally recommended to not rely on circular references between your beans, in particular with auto-proxying involved.

This is exactly the situation here. There's auto-proxying involved and there's a circular reference as a bean depends upon itself.

There's also a note on the javadoc for setAllowCircularReferences that is relevant to this situation:

It is generally recommended to not rely on circular references between your beans. Refactor your application logic to have the two beans involved delegate to a third bean that encapsulates their common logic.

Given the apparent difficulty in fixing this in a way that will work for everyone and the recommendations made in Framework, I am inclined to decline this issue. I think our priority should be on @SpyBean working when raw injection is disabled. With that priority in mind, I think the changes for #29909 were a step in the right direction.

I'll flag this for a forthcoming team meeting so that we can discuss this in case the rest of the team disagree.

[jonenst]

Hi,
Thanks a lot for your time.

I always thought that self injecting was a legitimate use case (typically for @Transactional proxying), and different from a more general dependency cycle. Isn't that the case ? Is there an official recommendation on this ?

In any case, I'm happy to wait for your decision during your next team meeting. I assume you'll update this issue.
Thanks

[wilkinsona]

I always thought that self injecting was a legitimate use case (typically for @Transactional proxying), and different from a more general dependency cycle. Isn't that the case?

There's no distinction that I am aware of in Framework between a cycle caused by a bean injecting itself and a larger cycle involving multiple beans. I'd take the need to enable raw injection and the note in its javadoc as a strong signal that you're doing something that's not recommended and is best avoided.

spring-projects/spring-framework#28299 will hopefully result in a definitive answer and some recommendations from the Framework team.