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 https://github.com/spring-projects/spring-boot/issues/29909 ?

Edit: see repro below.

Comment From: 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.

Comment From: 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

Comment From: wilkinsona

Thanks for the sample, @jonenst. Unfortunately, I'm not sure that it's possible to fix this without regressing the fix for https://github.com/spring-projects/spring-boot/issues/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.

Comment From: 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

Comment From: 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.

https://github.com/spring-projects/spring-framework/issues/28299 will hopefully result in a definitive answer and some recommendations from the Framework team.

Comment From: wilkinsona

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 https://github.com/spring-projects/spring-boot/issues/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.

We've discussed this today and we are in agreement. Closing.

Comment From: jonenst

Hi, just to be sure that I understood correctly: when using raw injection, in order to support spybean on two mutually injected beans, you are dropping support for spybean on self injecting beans ? Thanks

Comment From: wilkinsona

Not exactly. We want to tolerate using SpyBean when multiple beans create a cycle that does not require raw injection to be enabled. Beyond that, we do not want to make any changes to support SpyBean with raw injection enabled. It does not appear to be possible to support all scenarios and flip-flopping from one to the other as issues are raised isn’t a viable option, particularly given the recommendations against using raw injection in the first place.

Comment From: jonenst

Thank you for taking the time to clarify this