Skip to content

@MockBean doesn't work with @Async annotated services #6405

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

Closed
michael-simons opened this issue Jul 17, 2016 · 23 comments
Closed

@MockBean doesn't work with @Async annotated services #6405

michael-simons opened this issue Jul 17, 2016 · 23 comments
Assignees

Comments

@michael-simons
Copy link
Contributor

michael-simons commented Jul 17, 2016

Hello everybody,

I want to test a MVC @controller with 1.4.0.RC1 using @WebMvcTest. The controller depends on a service which as an @async method:

@RunWith(SpringRunner.class)
@WebMvcTest(TestController.class)
public class TestControllerTest {

    @Autowired
    private MockMvc mvc;

    @MockBean
    private TestService testService;

    @Test
    public void testSomeMethod() {
        // blah...
    }
}

The service:

package com.example;

import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Service;

@Service
public class TestService {

    @Async
        public int doSomethingElse() {
            return 4711;
        }
}

Test fails with

Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'testController' defined in file [/Users/msimons/Downloads/mock_async_proxy_bug/target/classes/com/example/TestController.class]: Bean instantiation via constructor failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.example.TestController]: Illegal arguments for constructor; nested exception is java.lang.IllegalArgumentException: argument type mismatch
    at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:279) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1143) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1046) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:510) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:482) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:306) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:302) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:775) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:861) ~[spring-context-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:541) ~[spring-context-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:759) ~[spring-boot-1.4.0.RC1.jar:1.4.0.RC1]
    at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:369) ~[spring-boot-1.4.0.RC1.jar:1.4.0.RC1]
    at org.springframework.boot.SpringApplication.run(SpringApplication.java:313) ~[spring-boot-1.4.0.RC1.jar:1.4.0.RC1]
    at org.springframework.boot.test.context.SpringBootContextLoader.loadContext(SpringBootContextLoader.java:111) ~[spring-boot-test-1.4.0.RC1.jar:1.4.0.RC1]
    at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:98) ~[spring-test-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:116) ~[spring-test-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    ... 25 common frames omitted
Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.example.TestController]: Illegal arguments for constructor; nested exception is java.lang.IllegalArgumentException: argument type mismatch
    at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:156) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:122) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:271) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    ... 42 common frames omitted
Caused by: java.lang.IllegalArgumentException: argument type mismatch
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[na:1.8.0_92]
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) ~[na:1.8.0_92]
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[na:1.8.0_92]
    at java.lang.reflect.Constructor.newInstance(Constructor.java:423) ~[na:1.8.0_92]
    at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:147) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
    ... 44 common frames omitted

My first guess that this is related to #5837 or similar. Throwing @EnableAsync at the test didn't help either, using a separate context configuration would defeat the purpose of WebMvcTest i guess

Example project is attached mock_async_proxy_bug.tar.gz
Actual use case was writing a test for this controller in preparation for 1.4.

Thanks,
Michael.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 17, 2016
@wilkinsona wilkinsona self-assigned this Jul 18, 2016
@wilkinsona
Copy link
Member

wilkinsona commented Jul 18, 2016

It looks like the @Async support has created the wrong sort of proxy. It's trying to inject a JDK proxy which has been created because the mocked bean implements a single interface, org.mockito.cglib.proxy.Factory.

A workaround is to force the creation of CGLib proxies:

@EnableAsync(proxyTargetClass=true)

@wilkinsona
Copy link
Member

The same problem occurs if the mocked bean is created manually so @MockBean isn't making things worse than they normally would be.

@michael-simons
Copy link
Contributor Author

So that means I basically have two options: Change the behavior of the application by changing the annotation on the application class or adding an additional configuration with "test" profile or something like that? Just adding @EnableAsync(proxyTargetClass=true) won't do…

I understand what's happening and the workaround would work for me I guess but I guess that behavior would come as a surprise to many.

@michael-simons
Copy link
Contributor Author

Hm… I tried to implement the workaround in my project and not only in the demo: @EnableCaching certainly causes the same error…

@snicoll
Copy link
Member

snicoll commented Jul 18, 2016

Well, that's exactly the same root cause...

@wilkinsona
Copy link
Member

but I guess that behavior would come as a surprise to many.

Agreed. We need to do something better here, I'm just not sure what yet.

Change the behavior of the application by changing the annotation on the application class

This shouldn't be (much of) a change in behaviour. You'll end up with a different type of proxy (CGLib rather than JDK) but that shouldn't matter to your application.

or adding an additional configuration with "test" profile or something like that?

That may work, although I'm not sure which @EnableAsyc annotation would win.

Just adding @EnableAsync(proxyTargetClass=true) won't do…

Not sure I've understood this. You already have @EnableAsync. A workaround is to add proxyTargetClass=true to the existing annotation.

@EnableCaching certainly causes the same error…

Caching uses proxies too and, in its default configuration, it uses JDK proxies. It has a proxyTargetClasses attribute too.

@michael-simons
Copy link
Contributor Author

Agreed. We need to do something better here, I'm just not sure what yet.

Thanks, Andy! As well as I understand

This shouldn't be (much of) a change in behaviour. You'll end up with a different type of proxy (CGLib rather than JDK) but that shouldn't matter to your application.

it will probably raise some questions…
I wish I could help more but that proxy area is not my usual territory.

That may work, although I'm not sure which @EnableAsyc annotation would win.

If I have @Enable… at application class level, it win's and I cannot overwrite it through separate test configuration.

Not sure I've understood this. You already have @EnableAsync. A workaround is to add proxyTargetClass=true to the existing annotation.

I was hoping I could overwrite without introducing new configuration classes by adding it to the test class itself.

@wilkinsona
Copy link
Member

I was hoping I could overwrite without introducing new configuration classes by adding it to the test class itself.

I wouldn't recommend doing that. It doesn't feel like a good idea to be running your tests with different configuration from normal runtime.

It feels like the best that we can do here is to make it more obvious that you need to configure proxyTargetClass=true. I've opened SPR-14478 to see what can be done at the Spring Framework level.

@philwebb
Copy link
Member

I've generally found proxyTargetClass=true to be a sensible default for most applications. The single interface problem is really common and quite hard to debug. Since CGLib is shipped as part of Spring Framework these days, there aren't many downsides.

@michael-simons
Copy link
Contributor Author

@philwebb What does that mean:

This approach has no negative impact in practice unless one is explicitly expecting one type of proxy vs. another — for example, in tests.

From the @EnableAsync doc…

Anyway, why don't I get CGLIB proxies anyway? The service in the example doesn't implement any interface…

@philwebb
Copy link
Member

@michael-simons The service doesn't directly implement an interface, but when mock(...) is in mix Mockito is adding one. Here's what we think happens:

  • @MockBean on TestService causes a mockito mock to be created for TestService.
  • Mockto uses its own packaged version of CGLib. This actually creates a class that extends the real TestService (overriding methods to support mocking) and also implements a org.mockito.cglib.proxy.Factory interface.
  • Spring sees the @Async annotation and decided to create a proxy to support it.
  • Since TestService now implements Factory, Spring decided to use a JDK proxy.
  • You now have a bean that's an instance of Factory but isn't a TestService.

@snicoll
Copy link
Member

snicoll commented Jul 19, 2016

@philwebb my take on this would be to copy/paste your last comment (with some polishing) in the doc.

@wilkinsona
Copy link
Member

I disagree, for now at least. Users shouldn't have to know any of that. The ideal is that it just works. I doubt we'll get there in Boot 1.4 and Framework 4.3, but there's still plenty of scope for improvement. I'd like to wait and see what SPR-14478 brings us.

@michael-simons
Copy link
Contributor Author

Thanks everybody, for the explanation and workaround and also for the last comment, that is exactly which I would expect as a user, @wilkinsona.

@wilkinsona
Copy link
Member

wilkinsona commented Jul 20, 2016

With the latest Spring Framework 4.3.2 snapshot, the provided example passes even with proxyTargetClass=false. The Mockito interface is now ignored and a CGLib proxy is created.

@michael-simons
Copy link
Contributor Author

@wilkinsona good news! But you mean false, or? true was the workaround …

@wilkinsona
Copy link
Member

wilkinsona commented Jul 20, 2016

@michael-simons Sorry. I meant false. I've updated my comment.

@wilkinsona
Copy link
Member

wilkinsona commented Jul 20, 2016

If I update the example so that TestService implements an interface but it's still injected as TestService then we now get some more helpful diagnostics:

Caused by: org.springframework.beans.factory.BeanNotOfRequiredTypeException: Bean named 'com.example.TestService#0' is expected to be of type [com.example.TestService] but was actually of type [com.sun.proxy.$Proxy73]
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:378) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:202) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    at org.springframework.beans.factory.config.DependencyDescriptor.resolveCandidate(DependencyDescriptor.java:207) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.findAutowireCandidates(DefaultListableBeanFactory.java:1213) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1053) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1018) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:835) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:741) ~[spring-beans-4.3.2.BUILD-SNAPSHOT.jar:4.3.2.BUILD-SNAPSHOT]
    ... 42 common frames omitted

I think we should add a FailureAnalyzer for BeanNotOfRequiredTypeException that recommends proxyTargetClass=true when the actual type is a com.sun.proxy.$Proxy

@wilkinsona
Copy link
Member

I've opened #6434 for the FailureAnalyzer and we have #6318 for the Spring Framework 4.3.2 upgrade which fixes this issue's specific problem.

@wilkinsona wilkinsona removed the status: waiting-for-triage An issue we've not yet triaged label Jul 20, 2016
@WalternativE
Copy link

Hello,

I experience issues which seem to be closely related to this thread in Spring Boot 1.4.0.RELEASE - should I go on or should I open a new issue for this?

Thanks
Gregor

@wilkinsona
Copy link
Member

@WalternativE Thanks for asking. Please open a new issue, ideally with a small sample that reproduces the problem

@michael-simons
Copy link
Contributor Author

Hi @WalternativE Would you please link this? I think just stumbled onto something as well and I wanna check if it's the same.

wilkinsona added a commit to wilkinsona/spring-boot that referenced this issue Aug 17, 2016
Post-processing of mocked beans causes a number of problems:

 - The mock may be proxied for asynchronous processing which can cause
   problems when configuring expectations on a mock (spring-projectsgh-6573)
 - The mock may be proxied so that its return values can be cached or
   so that its methods can be transactional. This causes problems with
   verification of the expected calls to a mock (spring-projectsgh-6573, spring-projectsgh-5837)
 - If the mock is created from a class that uses field injection, the
   container will attempt to inject values into its fields. This causes
   problems if the mock is being created to avoid the use of one of
   those dependencies (spring-projectsgh-6663)
 - Proxying a mocked bean can lead to a JDK proxy being created
   (if proxyTargetClass=false) as the mock implements a Mockito
   interface. This can then cause injection failures as the types don’t
   match (spring-projectsgh-6405, spring-projectsgh-6665)

All of these problems can be avoided if a mocked bean is not
post-processed. Avoiding post-processing prevents proxies from being
created and autowiring from being performed. This commit avoids
post-processing by registering mocked beans as singletons rather than
via a bean definition.
@kcmvp
Copy link

kcmvp commented Aug 25, 2016

I ran into the same issue. after add "proxyTargetClass = true" it works.
it's a bug or not?
thanks

snicoll pushed a commit that referenced this issue Sep 2, 2016
Post-processing of mocked beans causes a number of problems:

 - The mock may be proxied for asynchronous processing which can cause
   problems when configuring expectations on a mock (gh-6573)
 - The mock may be proxied so that its return values can be cached or
   so that its methods can be transactional. This causes problems with
   verification of the expected calls to a mock (gh-6573, gh-5837)
 - If the mock is created from a class that uses field injection, the
   container will attempt to inject values into its fields. This causes
   problems if the mock is being created to avoid the use of one of
   those dependencies (gh-6663)
 - Proxying a mocked bean can lead to a JDK proxy being created
   (if proxyTargetClass=false) as the mock implements a Mockito
   interface. This can then cause injection failures as the types don’t
   match (gh-6405, gh-6665)

All of these problems can be avoided if a mocked bean is not
post-processed. Avoiding post-processing prevents proxies from being
created and autowiring from being performed. This commit avoids
post-processing by registering mocked beans as singletons as well as
via a bean definition. The latter is still used by the context for type
matching purposes.

Closes gh-6573, gh-6663, gh-6664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants