Mis-proxying of Mockito mock and poor diagnostics for type mismatch on proxy injection [SPR-14478]

[spring-projects-issues]

Andy Wilkinson opened SPR-14478 and commented

There's a small sample application in the referenced Spring Boot issue that reproduces the problem.

There's a Mockito-created mock bean in the context. Due to @EnableAsync this bean is proxied using a JDK proxy (proxyTarget class is false). An attempt is then made to autowire the mocked bean into another class. This fails because the JDK proxy is for a Mockito interface. This in itself could perhaps be considered a problem as the proxy has the wrong form.

The proxy is considered to be a viable candidate for autowiring due to predictBeanType on AbstractAutowireCapableBeanFactory returning the type of the underlying bean, ignoring the proxying that'll be performed by AsyncAnnotationBeanPostProcessor. Ultimately this results in a BeanInstantiationException that doesn't shed much light on what's actually gone wrong:

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]
	... 41 common frames omitted
Caused by: java.lang.IllegalArgumentException: argument type mismatch
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[na:1.8.0_60]
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) ~[na:1.8.0_60]
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[na:1.8.0_60]
	at java.lang.reflect.Constructor.newInstance(Constructor.java:422) ~[na:1.8.0_60]
	at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:147) ~[spring-beans-4.3.1.RELEASE.jar:4.3.1.RELEASE]
	... 43 common frames omitted

There are three areas where things are going wrong, each probably a knock-on effect of the previous one:

1. The proxy creation goes wrong and creates a proxy for a Mockito interface rather than anything that was actually implemented by the underlying bean
2. The type prediction is wrong and returns a type of which the proxied bean is not an instance
3. The diagnostics for the resulting IllegalArgumentException don't reveal anything about the arguments that were passed to the constructor and their types so you can't immediately see why it might have failed.

---

Affects: 4.3.1

Reference URL: spring-projects/spring-boot#6405

Issue Links:

• An ObjectFactory variant with lenient not-unique handling [SPR-13943] #18515 An ObjectFactory variant with lenient not-unique handling
• Auto-proxy creation should not consider GroovyObject as a user-specified interface [SPR-11715] #16337 Auto-proxy creation should not consider GroovyObject as a user-specified interface
• Improve core container exception meta-data [SPR-13968] #18540 Improve core container exception meta-data
• ObjectFactory lacks method for getting bean with specified constructor arguments [SPR-13956] #18529 ObjectFactory lacks method for getting bean with specified constructor arguments
• Consider target-class proxy mode by default [SPR-14515] #19084 Consider target-class proxy mode by default
• Upgrade to Mockito 2.2 [SPR-14880] #19446 Upgrade to Mockito 2.2
• Non-helpful NoSuchBeanDefinitionException rather than BeanNotOfRequiredTypeException due to creation order [SPR-14504] #19073 Non-helpful NoSuchBeanDefinitionException rather than BeanNotOfRequiredTypeException due to creation order

Referenced from: commits 0e3f0bd, 503d65d

0 votes, 6 watchers

[spring-projects-issues]

Stéphane Nicoll commented

For what is worth, I believe the same issue applied if @Async is swapped with @Cacheable. Not sure if the root cause is the same though.

[spring-projects-issues]

Phil Webb commented

I've encountered this before without Mockito in the mix, it's a particularly difficult issue to track down. I wonder if Spring Framework 5 should make proxyTarget=true the default.

[spring-projects-issues]

Juergen Hoeller commented

Our getBean(Class) variants detect that kind of scenario and throw a reasonable exception... but unfortunately our injection point matching algorithm doesn't do the same. We'll fix that for 4.3.2 and 4.2.8 - and indeed, let's consider target-class as default for Spring 5 - but as a separate issue, keeping this issue to the diagnostics story.

That said, we might be able to exclude Mockito interfaces in our AOP proxy type decision in ProxyProcessorSupport.evaluateProxyInterfaces, like we do for Spring's own callback interfaces and the GroovyObject interface already. I'll consider that within the scope of this issue still.

[spring-projects-issues]

Juergen Hoeller commented

I've got both steps implemented now:

• Injection point resolution is passing the expected type to getBean calls, leading to the usual "Bean named 'xxx' is expected to be of type [MyBeanClass] but was actually of type [com.sun.proxy.$Proxy1]" in case of a proxy mismatch, at the expense of an additional assignability check (which is worth it from my perspective).

• We can easily deal with Mockito mock classes as general CGLIB proxies since that's what they are, even if coming from a Mockito-embedded CGLIB variant. We simply do not consider the CGLIB Factory interface (also in any relocated form) as a reasonable proxy interface, ignoring it just like we do with GroovyObject.

Both of those are candidates for 4.3.2 but no further back, not least of it all since this builds on some refinements that we only really implemented for 4.3 before.