New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mocking action declaration may not take effect(failed to match) when spring adds a proxy for the mock object #758

Closed
djj0809 opened this Issue Aug 18, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@djj0809

djj0809 commented Aug 18, 2017

Issue description

Mocking action declaration may not take effect when:

  1. @EnableTransactionManagement is added in the unit test's Spring context(i.e. in other context config classes)
  2. The mocked interface has @Transactional annotations in class level or some methods declaration (A typical scenario is using Spring-data-jpa's custom repository interface, we may add @Transactional to declare related config)

Above settings will cause Spring adding proxy for the spock generated mock object, which cause the problem.

See sample code if necessary.

After some digging, I found Something in org.spockframework.mock.runtime.MockObject line 95 :

  public boolean matches(Object target, IMockInteraction interaction) {
    if (target instanceof Wildcard) return verified || !interaction.isRequired();
    // instance is the spock generated mock object, and target is the spring proxy of the mock object, 
    // so match is false causing is issue
    boolean match = global ? instance.getClass() == target.getClass() : instance == target;
    if (match) {
      checkRequiredInteractionAllowed(interaction);
    }
    return match;
  }

Is there a way that I can get rid of this problem(A PR or anything else)? Thanks for any attention or help!

How to reproduce

here is a piece of sample code:

@ContextConfiguration(classes = [MockConfig, DomainAppCnxt])
// @EnableTransactionManagement added in DomainAppCnxt
class SampleSpec extends Specification {

    @Autowired
    SampleService sampleService
    @Autowired
    // The problem is, @EnableTransactionManagement will add a proxy for those beans with @Transactional declared,
    // and this injected bean is a spring proxy of the mock object (which is also a proxy)
    SampleRepositry sampleRepositry

    def "sample service test"() {
        when:
        sampleService.doSomething()   // simply invoke sampleRepositry.doSomething() in it
        then:
        1 * sampleRepositry.doSomething()  // this declaration can't take effect
        0 * _                     // unit test failed on unexpected invocation of sampleRepositry.doSomething()
    }

    @Configuration
    static class MockConfig {
        def mockFactory = new DetachedMockFactory()

        @Bean
        @Primary
        // @Primary is used for replacing the same type bean registered by DomainAppCnxt
        SampleRepository sampleRepositoryMock() {
            return mockFactory.Mock(SampleRepository)
        }
    }
}
@Transactional(propagation = Propagation.SUPPORTS)
// Spring setups a proxy to handle transaction issues
interface SampleRepositry extends JpaRepository<Long, SampleDomain> {

}
@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Aug 19, 2017

Member

@djj0809 you can unwrap the proxy yourself see (http://amgohan.agileasoft.com/how-to-acess-target-object-behind-a-spring-proxy/).

    public static <T> T getTargetObject(Object proxy) throws Exception {
        if (AopUtils.isJdkDynamicProxy(proxy)) {
            return (T) ((Advised) proxy).getTargetSource().getTarget();
        } else {
            return (T) proxy;
        }
    }
    def "sample service test"() {
        given:
        def sampleRepositryMock = getTargetObject(sampleRepositry)
        when:
        sampleService.doSomething()   // simply invoke sampleRepositry.doSomething() in it
        then:
        1 * sampleRepositryMock.doSomething() 
        0 * _                   
    }

As for a solution in spock, I'm not sure how to go about this, since the mocking code is part of spock-core which has no knowledge of spring. spock-spring on the other hand does not have access to this part of the code.

Member

leonard84 commented Aug 19, 2017

@djj0809 you can unwrap the proxy yourself see (http://amgohan.agileasoft.com/how-to-acess-target-object-behind-a-spring-proxy/).

    public static <T> T getTargetObject(Object proxy) throws Exception {
        if (AopUtils.isJdkDynamicProxy(proxy)) {
            return (T) ((Advised) proxy).getTargetSource().getTarget();
        } else {
            return (T) proxy;
        }
    }
    def "sample service test"() {
        given:
        def sampleRepositryMock = getTargetObject(sampleRepositry)
        when:
        sampleService.doSomething()   // simply invoke sampleRepositry.doSomething() in it
        then:
        1 * sampleRepositryMock.doSomething() 
        0 * _                   
    }

As for a solution in spock, I'm not sure how to go about this, since the mocking code is part of spock-core which has no knowledge of spring. spock-spring on the other hand does not have access to this part of the code.

@djj0809

This comment has been minimized.

Show comment
Hide comment
@djj0809

djj0809 Aug 21, 2017

@leonard84 Thanks for your help, your unwrap solution works fine!
As for a general solution, I suggest one of following ways:

  1. Do this unwrap thing in InteractionBuilder#addEqualTarget when necessary(check the target object is a spring proxy using reflection).
  2. If cannot do this unwrap thing(coupling is disgusting), I think at least spock should complain about that(check in addEqualTarget, a real mock object or not), and add some hints in spock's documentation, and let users handle it.

djj0809 commented Aug 21, 2017

@leonard84 Thanks for your help, your unwrap solution works fine!
As for a general solution, I suggest one of following ways:

  1. Do this unwrap thing in InteractionBuilder#addEqualTarget when necessary(check the target object is a spring proxy using reflection).
  2. If cannot do this unwrap thing(coupling is disgusting), I think at least spock should complain about that(check in addEqualTarget, a real mock object or not), and add some hints in spock's documentation, and let users handle it.
@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Aug 28, 2017

Member
  1. this would produce a coupling to spring in spock-core, so no.
  2. does not work either, as for groovy global mocks the instances are not actually mocks and don't implement ISpockMockObject
  3. would you like to contribute some docs?
  4. we could add a little utility class in spock-spring to unwrap the proxy so that users don't have to copy the code
Member

leonard84 commented Aug 28, 2017

  1. this would produce a coupling to spring in spock-core, so no.
  2. does not work either, as for groovy global mocks the instances are not actually mocks and don't implement ISpockMockObject
  3. would you like to contribute some docs?
  4. we could add a little utility class in spock-spring to unwrap the proxy so that users don't have to copy the code
@Steve973

This comment has been minimized.

Show comment
Hide comment
@Steve973

Steve973 Nov 2, 2017

Leonard, thank you for the information. This helped me, but in a slightly different way than how you described. The mock was proxied by cglib, so instead of using AopUtils.isJdkDynamicProxy(proxy), I used AopUtils.isCglibProxy(proxy). Depending on the situation, others might need to use AopUtils.isAopProxy(proxy).

Thank you for the suggestion!

Steve973 commented Nov 2, 2017

Leonard, thank you for the information. This helped me, but in a slightly different way than how you described. The mock was proxied by cglib, so instead of using AopUtils.isJdkDynamicProxy(proxy), I used AopUtils.isCglibProxy(proxy). Depending on the situation, others might need to use AopUtils.isAopProxy(proxy).

Thank you for the suggestion!

@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Nov 4, 2017

Member

Actually AopUtils.isAopProxy(proxy) is just delegating to both AopUtils.isJdkDynamicProxy(proxy) and AopUtils.isCglibProxy(proxy) so it is the better choice.

Member

leonard84 commented Nov 4, 2017

Actually AopUtils.isAopProxy(proxy) is just delegating to both AopUtils.isJdkDynamicProxy(proxy) and AopUtils.isCglibProxy(proxy) so it is the better choice.

@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Jan 10, 2018

Member

#796 adds @UnwrapAopProxy extension that automatically unwraps spring AopProxies for annotated fields. I'd consider this fixed.

Member

leonard84 commented Jan 10, 2018

#796 adds @UnwrapAopProxy extension that automatically unwraps spring AopProxies for annotated fields. I'd consider this fixed.

@leonard84 leonard84 closed this Jan 10, 2018

@radarsh

This comment has been minimized.

Show comment
Hide comment
@radarsh

radarsh Apr 19, 2018

@leonard84 we're desperate to use this feature. When can we expect it to be released?

radarsh commented Apr 19, 2018

@leonard84 we're desperate to use this feature. When can we expect it to be released?

@leonard84

This comment has been minimized.

Show comment
Hide comment
@leonard84

leonard84 Sep 9, 2018

Member

@radarsh Spock 1.2-RC2 has been released recently

Member

leonard84 commented Sep 9, 2018

@radarsh Spock 1.2-RC2 has been released recently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment