Allow proxied @SpyBeans to be used with Mockito's inline mock maker

[jacky1193610322]

spring-boot version is 2.1.3.RELEASE
I already addressed the issue in mockito. issue
the mockito 3.4.0 can mock static method, so I upgrade mockito to 3.4.0, but the mock framework changed, now when I use
the @SpyBean and then verify it. it will throw NotAMockException.

@SpringBootTest
@RunWith(SpringJUnit4ClassRunner.class)
@AutoConfigureMockMvc
public class AppGroupClusterActionTest {

  @Autowired
  private MockMvc mockMvc;

  @MockBean
  private AppGroupClusterService appGroupClusterService;

  @SpyBean
  private AppGroupClusterAction appGroupClusterAction;

  @Captor
  private ArgumentCaptor<Object[]> argumentCaptor;

  @Test
  public void testUpdateExecuteAuditSpel() throws Exception {
    ServiceResult mockRes = new ServiceResult();
    mockRes.setSuccess(true);
    doReturn(mockRes).when(appGroupClusterService).update(
        Mockito.any(), Mockito.any(), Mockito.any(),
        Mockito.any(), Mockito.any(), Mockito.any(),
        Mockito.any(), Mockito.any(), Mockito.any());

    mockMvc.perform(
        MockMvcRequestBuilders.post("/platforms/{platform}/appGroupNames/{appGroupName}/clusterNames/{clusterName}/update",
            "platform", "appGroupName", "clusterName").content("{}"))
        .andExpect(status().isOk())
        .andReturn();

//    not work
//    verify(appGroupClusterAction).addAfterAuditResult(argumentCaptor.capture());
    verify((AppGroupClusterAction) AopTestUtils.getTargetObject(appGroupClusterAction)).addAfterAuditResult(argumentCaptor.capture());
    assertArrayEquals(argumentCaptor.getValue(), new Object[]{"appGroupName", "platform", "clusterName", "{}"});

    verify((AppGroupClusterAction) AopTestUtils.getTargetObject(appGroupClusterAction)).addAfterAuditResult(argumentCaptor.capture());
    assertArrayEquals(argumentCaptor.getValue(), new Object[]{"appGroupName", "platform", "clusterName", "{}"});
  }
}

[wilkinsona]

@jacky1193610322 Thanks for the report. We don't officially support Mockito 3.4 yet, but it's likely that we will want to do so in Spring Boot 2.4. To help us to investigate your problem, can you please provide a minimal sample that reproduces it?
You can share it with us by pushing it to a separate repository on GitHub or by zipping it up and attaching it to this issue.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[jacky1193610322]

sorry，I will offer simple code，but because of the rule of company, I can't upload the whole project, I will paste the core code.

[snicoll]

@jacky1193610322 it doesn't have to be your project. Actually, it has to be a minimal sample and that shouldn't expose any company specific information. Please don't paste the core code as the only thing we could do is try to copy/paste that in an actual project we can run.

[jacky1193610322]

I know. but push the code to GitHub is forbid.

[snicoll]

You can share the code in a different form (attaching a zip to this issue) or you can send it to me by email if that's easier. My email is on my github profile.

[jacky1193610322]

ok

[jacky1193610322]

sorry, the mail can't work now, can't upload code, but the example is simple, any aop proxy can reproduction.
Aspect

@Component
@org.aspectj.lang.annotation.Aspect
public class Aspect {
  @Pointcut("execution(* com.example.demo.*.*(..))")
  public void pointcut() {
  }

  @Around("pointcut()")
  public Object around(ProceedingJoinPoint joinPoint) throws Throwable {
    try {
      return joinPoint.proceed();
    } catch (Throwable e) {
      throw e;
    } finally {
      System.out.println("around");
    }
  }
}

http action

@RequestMapping(value = "/demo/", produces = MediaType.APPLICATION_JSON_VALUE)
@RestController
public class Action {
  public void test() {

  }
}

test

@SpringBootTest
class ActionTest {
  @SpyBean
  private Action action;

  @Test
  void name() {
    doNothing().when(action).test();
    Mockito.verify(action);
  }
}

[wilkinsona]

Looking more closely, I think this is a Mockito bug or at least a limitation. As a result of using @SpyBean a Mockito spy is created for Action. This spy is then proxied by Spring's AOP infrastructure and it's this proxy that is injected into the test class. It's then passed into a call to when at which point Mockito checks that it's a mock by checking that is has a non-null MockHandler. This works when using the standard mock maker but fails when using the inline mock maker. In the working case, SubclassByteBuddyMockMaker retrieves the MockHandler as follows:

if (!(mock instanceof MockAccess)) {
    return null;
}
return ((MockAccess) mock).getMockitoInterceptor().getMockHandler();

mock is an instance of MockAccess and the handler from the interceptor is returned.

In the failing case, InlineByteBuddyMockMaker attempts to retrieve the MockHandler has follows:

MockMethodInterceptor interceptor = mocks.get(mock);
if (interceptor == null) {
    return null;
} else {
    return interceptor.handler;
}

mocks.get(mock) returns null so null is returned.

Given the same input to their getHandler methods, I would expect InlineByteBuddyMockMaker and SubclassByteBuddyMockMaker to return the same output, hence the feeling that this is a Mockito bug or at least a limitation.

The problem can be worked around by removing Spring's proxy before passing the spy into Mockito:

Action actionSpy = AopTestUtils.<Action>getTargetObject(action);
doNothing().when(actionSpy).test();
action.test();
verify(actionSpy).test();

[raphw]

Cross-posting from our issue tracker: TLNR; this is not a Mockito bug, it's Spring relying on an undocumented implementation detail that might just change in any bugfix release.

The reason things currently (!) work with the SubclassByteBuddyMockMaker is that Spring proxies the (internal) MockAccess interface which we implement in the mock to extract the mock state which is stored in a field of the mock instance. We could just as much had read that field directly to avoid the interface; and this wouldn't even be such a bad idea as it less pollutes the reflection API. But at this time, when we call our internal interface method, it gets redirected to the actual mock object by Spring which is why this functions (accidentally).

The reason it does not work with the InlineByteBuddyMockMaker is that we cannot implement interfaces in a class after it has been loaded. The whole premise of the inline mock maker is to keep changes invisible to make use of the JVMs retransformation facilities even after class loading which makes this a requirement. Therefore, we need to identify mocks by the object's identity and store the mock state externally in a weak map (which requires using object identity), which is not the same for the Spring proxy object. We cannot change this and from our perspective nothing is broken.

From Mockito's side things are simple: if one does not provide "our" mock object to "our" API, we do not guarantee to function, not in the past, nor the future. That it does currently work with the subclass mock maker facility is rather accidental and might change in the future. If you implemented the MockAccess interface yourself and returned another object's mock state from it and passed this to Mockito, you'd break Mockito, too. If Spring cannot do anything about this, users need to unproxy the objects explicitly but there is nothing we can do about it. A proxied mock is not a mock from our side, just some object with a reference to a mock and of those there are of course plenty.

[wilkinsona]

It looks like we'll be able to address this with a custom MockResolver that builds on the change that @raphw has prototyped for mockito/mockito#1980.

[raphw]

Great. new Mockito release to Central is underway just now.

[TimvdLippe]

I just realized we actually didn't publish a new minor version to Maven Central. However, I just tried to publish 3.6.0 and Bintray had issues. Hopefully we can figure out soon how to resolve that.

[TimvdLippe]

We have been able to fix our release woes and 3.6.0 has been published to Maven Central: https://repo1.maven.org/maven2/org/mockito/mockito-core/3.6.0/

[snicoll]

Thanks @TimvdLippe. The upgrade is scheduled for 2.4.0-RC1, see #23924

[wilkinsona]

Closed by d9084ea.

[elxnis]

Hi @wilkinsona,

I think org.springframework.test.util.AopTestUtils#getUltimateTargetObject causes issues with scoped beans, similarly as before in #19020. I will try to test with my sample from that issue and raise new case if it is failing again.

Thanks,
Reinis