Search code examples
aspectjspring-aopspring-testspring-boot-test

Spring AOP + AspectJ: @AfterReturning advice wrongly executed while mocking(before actual execution)


In an integration test, my advice of @AfterReturning is wrongly executed while in the test I mock it to throw TimeoutException, and the arg passed to the aspect is null.

My advice:

    @AfterReturning("execution(* xxxxxx" +
            "OrderKafkaProducerService.sendOrderPaidMessage(..)) && " +
            "args(order)")
    public void orderComplete(CheckoutOrder order) { // order is null when debugging
        metricService.orderPaidKafkaSent();
        log.trace("Counter inc: order paid kafka"); // this line of log is shown in console
        metricService.orderCompleted();
        log.trace("Order complete! {}", order.getId()); // this line is not, because NPE
    }

And my test:

// mocking
doThrow(new ServiceException(new TimeoutException("A timeout occurred"), FAILED_PRODUCING_ORDER_MESSAGE))
    .when(orderKafkaProducerService).sendOrderPaidMessage(any()); // this is where advice is executed, which is wrong

...
// when
(API call with RestAssured, launch a real HTTP call to endpoint; service is called during this process)

// then
verify(orderKafkaProducerService).sendOrderPaidMessage(any(CheckoutOrder.class)); // it should be called
verify(metricService, never()).orderCompleted(); // but we are throwing, not returning, we should not see this advice executed

This test is failing because of NPE(order is null).

In debugging, I find that when I was mocking, I already execute the advice, and at this point, any() has no value yet, is null, so NPE. But I don't think the advice should execute while mocking. How can I avoid that while testing?? This is absurd for me.


Solution

  • Currently Spring test support does not explicitly handle the situation that an injected mock or spy (which is a proxy subclass via Mockito) might actually be an AOP target later on (i.e. proxied and thus subclassed again via CGLIB).

    There are several bug tickets related to this topic for Spring, Spring Boot and Mockito. Nobody has done anything about it yet. I do understand why the Mockito maintainers won't include Spring-specific stuff into their code base, but I do not understand why the Spring people do not improve their testing tools.

    Actually when debugging your failing test and inspecting kafkaService, you will find out the following facts:

    1. kafkaService.getClass() is com.example.template.services.KafkaService$MockitoMock$92961867$$EnhancerBySpringCGLIB$$8fc4fe95
    2. kafkaService.getClass().getSuperclass() is com.example.template.services.KafkaService$MockitoMock$92961867
    3. kafkaService.getClass().getSuperclass().getSuperclass() is class com.example.template.services.KafkaService

    In other words:

    1. kafkaService is a CGLIB Spring AOP proxy.
    2. The AOP proxy wraps a Mockito spy (probably a ByteBuddy proxy).
    3. The Mockito spy wraps the original object.

    Besides, changing the wrapping order to make the Mockito spy the outermost object would not work because CGLIB deliberately makes its overriding methods final, i.e. you cannot extend and override them again. If Mockito was just as restrictive, the hierarchical wrapping would not work at all.

    Anyway, what can you do?

    • Either you use a sophisticated approach like described in this tutorial
    • or you go for the cheap solution to explicitly unwrap an AOP proxy via AopTestUtils.getTargetObject(Object). You can call this method safely because if the passed candidate object is not a Spring proxy (internally easy to identify because it implements the Advised interface which also gives access to the target object), it just returns the passed object again.

    In your case the latter solution would look like this:

    @Test
    void shouldCompleteHappyPath() {
        // fetch spy bean by unwrapping the AOP proxy, if any
        KafkaService kafkaServiceSpy = AopTestUtils.getTargetObject(kafkaService);
        // given mocked
        doNothing().when(kafkaServiceSpy).send(ArgumentMatchers.any());
        // when (real request)
        testClient.create().expectStatus().isOk();
        // then
        verify(kafkaServiceSpy).send(ArgumentMatchers.any());
        verify(metricService).incKafka();
    }
    

    This has the effect that when(kafkaServiceSpy).send(ArgumentMatchers.any()) no longer triggers the aspect advice because kafkaServiceSpy is no longer an AOP proxy. The auto-wired bean kafkaService still is, though, thus AOP gets triggered as expected, but no longer unwantedly while recording the mock interaction.

    Actually, for the verification you could even use kafkaService instead of the spy and only unwrap the spy when recording the interaction you want to verify later:

    @Test
    void shouldCompleteHappyPath() {
        // given mocked
        doNothing()
          .when(
            // fetch spy bean by unwrapping the AOP proxy, if any
            AopTestUtils.<KafkaService>getTargetObject(kafkaService)
          )
          .send(ArgumentMatchers.any());
        // when(real request)
        testClient.create().expectStatus().isOk();
        // then
        verify(kafkaService).send(ArgumentMatchers.any());
        verify(metricService).incKafka();
    }
    

    P.S.: Without your MCVE I would never have been able to debug this and find out what the heck was going on. This proves again that asking questions including an MCVE is the best thing you can do for yourself because it helps you get answers to questions which otherwise probably would remain unanswered.


    Update: After I had mentioned this problem under the similar closed issue Spring Boot #6871, one of the maintainers has by himself created Spring Boot #22281 which specifically addresses your problem here. You might want to watch the new issue in order to find out if/when it can be fixed.