Mockito mock falsely initialized as CGLIB proxy with AspectJ aspect

[andrethiel1und1]

Affects: Framework 6.1.10 (most likely also 6.1.9)

Disclaimer: I have a hard time digging into this issue, because most of the concepts here are new to me. I might use the terminology wrong or be way off with my assessment.

We experienced a change with Framework 6.1.8 where AspectJ related Mockito tests started failing, most likely related to #32970
However, instead of being executed twice there were now 3 calls for one method.
Besides the actual delegate and the cglib proxy (as described in the issue) there was another call from a Mockito mock instance with a cglib proxy - at least I think that is what the debugger indicates: a call from MyService$MockitoMock$<somerandomstuff>$$SpringCGLIB$$0

Since we only manage the spring-boot version directly, we tested the backported fix (I think it is this one: 628b050) using framework version 6.1.10 (spring-boot update to 3.2.7).
The tests still fail because there are still two calls. The CTW cglib proxy call has been fixed, but there is still the Mockito mock related one.
At least that is what it looks like to me.

[jhoeller]

Thanks for raising this, I can imagine a side effect in a Mockito scenario indeed.

Am I understanding correctly that you got an AspectJ aspect configured as a bean and applied with compile-time weaving, and the Spring AOP auto-proxying accidentally picked it up for building a CGLIB proxy - and that latter part got fixed by #32970?

So what remains is an accidental extra call for a Mockito mock that's set up as a Spring bean as well? Is there still a CGLIB proxy being built for that Mockito mock where you do not intend to have a CGLIB proxy as all?

I assume that Mockito mock is of the same type as your regular target bean and therefore matched by auto-proxying as well, and not excluded by #32970 either since the Mockito mock class itself is not compiled by ajc.

What is your test actually trying to assert there? Is it expecting any interactions between the aspect and the Mockito mock at all?

[jhoeller]

Is your Mockito mock effectively extending the class that the aspect actually targets? Is its superclass compiled by ajc and we are not detecting that yet? Any other indication why it is not passing the compiledByAjc(targetClass) check in AspectJExpressionPointcut:286 which is meant to find out about pre-weaved classes that we need to skip for proxying?

[jhoeller]

Trying various Mockito scenarios, your MyService$MockitoMock class name above clearly indicates a subclass generated by Mockito (in case of extra interfaces or extra settings - otherwise Mockito just transforms the existing class), with the original MyService class as its superclass. Assuming this is registered as a bean and that the original MyService class is compile-time weaved, it looks we should simply check for ajc markers in the superclass as well in such a scenario.

I've tried that with a patch in AspectJExpressionPointcut.compiledByAjc where it recurses through the given class hierarchy now - which seems to work fine for me so far. @andrethiel1und1 if this seems to match your scenario, I'll commit the patch so that you could try a 6.1.11 snapshot with it. If there is anything else to consider for your scenario, let me know.

[andrethiel1und1]

Thanks for raising this, I can imagine a side effect in a Mockito scenario indeed.

Am I understanding correctly that you got an AspectJ aspect configured as a bean and applied with compile-time weaving, and the Spring AOP auto-proxying accidentally picked it up for building a CGLIB proxy - and that latter part got fixed by #32970?

The project configuration is all over the place, so I have a hard time evaluating this. I might outline what I find in more detail later.
An a quick initial response, I can outline how the tests work.

We definitely have compile time weaving set up in the project. That much I know.

What is your test actually trying to assert there? Is it expecting any interactions between the aspect and the Mockito mock at all?

There is a LoggingAspect where I set a breakpoint in the log Method to see where it is called from in the stack:

@Aspect
@Slf4j
public class LoggingAspect {

    ...

    private String log(...) {
        return myService.log(le);
    }

    ...

This is a rough outline of a test that fails:

        InOrder inOrder = inOrder(myServiceMock, myAdminServiceMock);
        myAdminService.cancelSomething(getInfo(), SOME_ID);
        inOrder.verify(myServiceMock).log(isCancellation(...));

Here the inOrder.verify() call indirectly expects only one call to log and with 6.1.8 there were 3 calls to log instead of one, now there are 2 calls:

 // I think the Delegate classes are Camunda Steps.

 3.2.6 (6.1.8)
 MyAdminServiceAspectDelegate$$SpringCGLIB$$0 -> cancelSomething
 MyAdminServiceAspectDelegate -> cancelSomething
 MyAdminService$MockitoMock$w7AKM2b0$$SpringCGLIB$$0 -> cancelSomething
 
 3.2.7 (6.1.10)
 MyAdminServiceAspectDelegate -> cancelSomething
 MyAdminService$MockitoMock$xYX7lgEd$$SpringCGLIB$$0 -> cancelSomething

I think up to 6.1.8 there was only one call from MyAdminServiceAspectDelegate, none from a Mockito mock.

So I think what you are writing is basically correct!?

So what remains is an accidental extra call for a Mockito mock that's set up as a Spring bean as well? Is there still a CGLIB proxy being built for that Mockito mock where you do not intend to have a CGLIB proxy as all?

I think this is the case and debugging confirmed it - not sure if I understand the nuances here, though.

I assume that Mockito mock is of the same type as your regular target bean and therefore matched by auto-proxying as well, and not excluded by #32970 either since the Mockito mock class itself is not compiled by ajc.

This is where I have a hard time wrapping my head around the configuration. I'm still looking for the code that defines how the Mockito mock instance is defined. But that seems to make sense. If LTW for example has changed in 6.1.8 and is not covered by the fix, something like that might explain things.

[andrethiel1und1]

Trying various Mockito scenarios, your MyService$MockitoMock class name above clearly indicates a subclass generated by Mockito (in case of extra interfaces or extra settings - otherwise Mockito just transforms the existing class), with the original MyService class as its superclass. Assuming this is registered as a bean and that the original MyService class is compile-time weaved, it looks we should simply check for ajc markers in the superclass as well in such a scenario.

I've tried that with a patch in AspectJExpressionPointcut.compiledByAjc where it recurses through the given class hierarchy now - which seems to work fine for me so far. @andrethiel1und1 if this seems to match your scenario, I'll commit the patch so that you could try a 6.1.11 snapshot with it. If there is anything else to consider for your scenario, let me know.

Purely intuitively speaking I think checking for ajc markers in the superclass is worth a shot and nothing else comes to mind immediately that needs to be taken into consideration. It is worth a shot, I think.