Prevent beans created with @MockBean from being post-processed

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
This commit is contained in:
Andy Wilkinson 2016-08-17 10:19:47 +01:00 committed by Stephane Nicoll
parent 52d7282f5e
commit 0e00a49dcc
12 changed files with 235 additions and 122 deletions

View File

@ -92,8 +92,7 @@ class DefinitionsParser {
for (ResolvableType typeToMock : typesToMock) { for (ResolvableType typeToMock : typesToMock) {
MockDefinition definition = new MockDefinition(annotation.name(), typeToMock, MockDefinition definition = new MockDefinition(annotation.name(), typeToMock,
annotation.extraInterfaces(), annotation.answer(), annotation.extraInterfaces(), annotation.answer(),
annotation.serializable(), annotation.reset(), annotation.serializable(), annotation.reset());
annotation.proxyTargetAware());
addDefinition(element, definition, "mock"); addDefinition(element, definition, "mock");
} }
} }

View File

@ -26,7 +26,6 @@ import java.lang.annotation.Target;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.Answers; import org.mockito.Answers;
import org.mockito.MockSettings; import org.mockito.MockSettings;
import org.mockito.Mockito;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.core.annotation.AliasFor; import org.springframework.core.annotation.AliasFor;
@ -140,15 +139,4 @@ public @interface MockBean {
*/ */
MockReset reset() default MockReset.AFTER; MockReset reset() default MockReset.AFTER;
/**
* Indicates that Mockito methods such as {@link Mockito#verify(Object) verify(mock)}
* should use the {@code target} of AOP advised beans, rather than the proxy itself.
* If set to {@code false} you may need to use the result of
* {@link org.springframework.test.util.AopTestUtils#getUltimateTargetObject(Object)
* AopTestUtils.getUltimateTargetObject(...)} when calling Mockito methods.
* @return {@code true} if the target of AOP advised beans is used or {@code false} if
* the proxy is used directly
*/
boolean proxyTargetAware() default true;
} }

View File

@ -54,13 +54,12 @@ class MockDefinition extends Definition {
} }
MockDefinition(ResolvableType typeToMock) { MockDefinition(ResolvableType typeToMock) {
this(null, typeToMock, null, null, false, null, true); this(null, typeToMock, null, null, false, null);
} }
MockDefinition(String name, ResolvableType typeToMock, Class<?>[] extraInterfaces, MockDefinition(String name, ResolvableType typeToMock, Class<?>[] extraInterfaces,
Answers answer, boolean serializable, MockReset reset, Answers answer, boolean serializable, MockReset reset) {
boolean proxyTargetAware) { super(name, reset, false);
super(name, reset, proxyTargetAware);
Assert.notNull(typeToMock, "TypeToMock must not be null"); Assert.notNull(typeToMock, "TypeToMock must not be null");
this.typeToMock = typeToMock; this.typeToMock = typeToMock;
this.extraInterfaces = asClassSet(extraInterfaces); this.extraInterfaces = asClassSet(extraInterfaces);

View File

@ -0,0 +1,41 @@
/*
* Copyright 2012-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.test.mock.mockito;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
/**
* Beans created using Mockito.
*
* @author Andy Wilkinson
*/
public class MockitoBeans implements Iterable<Object> {
private final List<Object> beans = new ArrayList<Object>();
void add(Object bean) {
this.beans.add(bean);
}
@Override
public Iterator<Object> iterator() {
return this.beans.iterator();
}
}

View File

@ -94,6 +94,8 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
private final BeanNameGenerator beanNameGenerator = new DefaultBeanNameGenerator(); private final BeanNameGenerator beanNameGenerator = new DefaultBeanNameGenerator();
private final MockitoBeans mockitoBeans = new MockitoBeans();
private Map<Definition, String> beanNameRegistry = new HashMap<Definition, String>(); private Map<Definition, String> beanNameRegistry = new HashMap<Definition, String>();
private Map<Field, RegisteredField> fieldRegistry = new HashMap<Field, RegisteredField>(); private Map<Field, RegisteredField> fieldRegistry = new HashMap<Field, RegisteredField>();
@ -132,6 +134,7 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
private void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory, private void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory,
BeanDefinitionRegistry registry) { BeanDefinitionRegistry registry) {
beanFactory.registerSingleton(MockitoBeans.class.getName(), this.mockitoBeans);
DefinitionsParser parser = new DefinitionsParser(this.definitions); DefinitionsParser parser = new DefinitionsParser(this.definitions);
for (Class<?> configurationClass : getConfigurationClasses(beanFactory)) { for (Class<?> configurationClass : getConfigurationClasses(beanFactory)) {
parser.parse(configurationClass); parser.parse(configurationClass);
@ -182,7 +185,13 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
String beanName = getBeanName(beanFactory, registry, definition, beanDefinition); String beanName = getBeanName(beanFactory, registry, definition, beanDefinition);
beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(1, beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(1,
beanName); beanName);
if (registry.containsBeanDefinition(beanName)) {
registry.removeBeanDefinition(beanName);
}
registry.registerBeanDefinition(beanName, beanDefinition); registry.registerBeanDefinition(beanName, beanDefinition);
Object mock = createMock(definition, beanName);
beanFactory.registerSingleton(beanName, mock);
this.mockitoBeans.add(mock);
this.beanNameRegistry.put(definition, beanName); this.beanNameRegistry.put(definition, beanName);
if (field != null) { if (field != null) {
this.fieldRegistry.put(field, new RegisteredField(definition, beanName)); this.fieldRegistry.put(field, new RegisteredField(definition, beanName));

View File

@ -22,6 +22,7 @@ import java.util.Set;
import org.mockito.Mockito; import org.mockito.Mockito;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
@ -70,6 +71,17 @@ public class ResetMocksTestExecutionListener extends AbstractTestExecutionListen
} }
} }
} }
try {
MockitoBeans mockedBeans = beanFactory.getBean(MockitoBeans.class);
for (Object mockedBean : mockedBeans) {
if (reset.equals(MockReset.get(mockedBean))) {
Mockito.reset(mockedBean);
}
}
}
catch (NoSuchBeanDefinitionException ex) {
// Continue
}
if (applicationContext.getParent() != null) { if (applicationContext.getParent() != null) {
resetMocks(applicationContext.getParent(), reset); resetMocks(applicationContext.getParent(), reset);
} }

View File

@ -1,91 +0,0 @@
/*
* Copyright 2012-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.test.mock.mockito;
import java.util.Arrays;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.exceptions.misusing.UnfinishedVerificationException;
import org.springframework.cache.CacheManager;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.cache.annotation.EnableCaching;
import org.springframework.cache.concurrent.ConcurrentMapCacheManager;
import org.springframework.cache.interceptor.CacheResolver;
import org.springframework.cache.interceptor.SimpleCacheResolver;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.stereotype.Service;
import org.springframework.test.context.junit4.SpringRunner;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
/**
* Test {@link MockBean} when mixed with Spring AOP.
*
* @author Phillip Webb
* @see <a href="https://github.com/spring-projects/spring-boot/issues/5837">5837</a>
*/
@RunWith(SpringRunner.class)
public class MockBeanWithAopProxyAndNotProxyTargetAwareTests {
@MockBean(proxyTargetAware = false)
private DateService dateService;
@Test(expected = UnfinishedVerificationException.class)
public void verifyShouldUseProxyTarget() throws Exception {
this.dateService.getDate();
verify(this.dateService, times(1)).getDate();
reset(this.dateService);
}
@Configuration
@EnableCaching(proxyTargetClass = true)
@Import(DateService.class)
static class Config {
@Bean
public CacheResolver cacheResolver(CacheManager cacheManager) {
SimpleCacheResolver resolver = new SimpleCacheResolver();
resolver.setCacheManager(cacheManager);
return resolver;
}
@Bean
public ConcurrentMapCacheManager cacheManager() {
ConcurrentMapCacheManager cacheManager = new ConcurrentMapCacheManager();
cacheManager.setCacheNames(Arrays.asList("test"));
return cacheManager;
}
}
@Service
static class DateService {
@Cacheable(cacheNames = "test")
public Long getDate() {
return System.nanoTime();
}
}
}

View File

@ -34,6 +34,7 @@ import org.springframework.stereotype.Service;
import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.context.junit4.SpringRunner;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
@ -51,11 +52,13 @@ public class MockBeanWithAopProxyTests {
@Test @Test
public void verifyShouldUseProxyTarget() throws Exception { public void verifyShouldUseProxyTarget() throws Exception {
given(this.dateService.getDate()).willReturn(1L);
Long d1 = this.dateService.getDate(); Long d1 = this.dateService.getDate();
Thread.sleep(200); assertThat(d1).isEqualTo(1L);
given(this.dateService.getDate()).willReturn(2L);
Long d2 = this.dateService.getDate(); Long d2 = this.dateService.getDate();
assertThat(d1).isEqualTo(d2); assertThat(d2).isEqualTo(2L);
verify(this.dateService, times(1)).getDate(); verify(this.dateService, times(2)).getDate();
} }
@Configuration @Configuration

View File

@ -0,0 +1,83 @@
/*
* Copyright 2012-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.test.mock.mockito;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.scheduling.annotation.Async;
import org.springframework.scheduling.annotation.EnableAsync;
import org.springframework.test.context.junit4.SpringRunner;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;
/**
* Tests for a mock bean where the mocked interface has an async method.
*
* @author Andy Wilkinson
*/
@RunWith(SpringRunner.class)
public class MockBeanWithAsyncInterfaceMethodIntegrationTests {
@MockBean
private Transformer transformer;
@Autowired
private MyService service;
@Test
public void mockedMethodsAreNotAsync() {
given(this.transformer.transform("foo")).willReturn("bar");
assertThat(this.service.transform("foo")).isEqualTo("bar");
}
private interface Transformer {
@Async
String transform(String input);
}
private static class MyService {
private final Transformer transformer;
MyService(Transformer transformer) {
this.transformer = transformer;
}
public String transform(String input) {
return this.transformer.transform(input);
}
}
@Configuration
@EnableAsync
static class MyConfiguration {
@Bean
public MyService myService(Transformer transformer) {
return new MyService(transformer);
}
}
}

View File

@ -0,0 +1,64 @@
/*
* Copyright 2012-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.test.mock.mockito;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.context.junit4.SpringRunner;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;
/**
* Tests for a mock bean where the class being mocked uses field injection.
*
* @author Andy Wilkinson
*/
@RunWith(SpringRunner.class)
public class MockBeanWithInjectedFieldIntegrationTests {
@MockBean
private MyService myService;
@Test
public void fieldInjectionIntoMyServiceMockIsNotAttempted() {
given(this.myService.getCount()).willReturn(5);
assertThat(this.myService.getCount()).isEqualTo(5);
}
private static class MyService {
@Autowired
private MyRepository repository;
public int getCount() {
return this.repository.findAll().size();
}
}
private interface MyRepository {
List<Object> findAll();
}
}

View File

@ -43,16 +43,16 @@ public class MockDefinitionTests {
public ExpectedException thrown = ExpectedException.none(); public ExpectedException thrown = ExpectedException.none();
@Test @Test
public void ClassToMockMustNotBeNull() throws Exception { public void classToMockMustNotBeNull() throws Exception {
this.thrown.expect(IllegalArgumentException.class); this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("TypeToMock must not be null"); this.thrown.expectMessage("TypeToMock must not be null");
new MockDefinition(null, null, null, null, false, null, true); new MockDefinition(null, null, null, null, false, null);
} }
@Test @Test
public void createWithDefaults() throws Exception { public void createWithDefaults() throws Exception {
MockDefinition definition = new MockDefinition(null, EXAMPLE_SERVICE_TYPE, null, MockDefinition definition = new MockDefinition(null, EXAMPLE_SERVICE_TYPE, null,
null, false, null, true); null, false, null);
assertThat(definition.getName()).isNull(); assertThat(definition.getName()).isNull();
assertThat(definition.getTypeToMock()).isEqualTo(EXAMPLE_SERVICE_TYPE); assertThat(definition.getTypeToMock()).isEqualTo(EXAMPLE_SERVICE_TYPE);
assertThat(definition.getExtraInterfaces()).isEmpty(); assertThat(definition.getExtraInterfaces()).isEmpty();
@ -65,7 +65,7 @@ public class MockDefinitionTests {
public void createExplicit() throws Exception { public void createExplicit() throws Exception {
MockDefinition definition = new MockDefinition("name", EXAMPLE_SERVICE_TYPE, MockDefinition definition = new MockDefinition("name", EXAMPLE_SERVICE_TYPE,
new Class<?>[] { ExampleExtraInterface.class }, new Class<?>[] { ExampleExtraInterface.class },
Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE, false); Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE);
assertThat(definition.getName()).isEqualTo("name"); assertThat(definition.getName()).isEqualTo("name");
assertThat(definition.getTypeToMock()).isEqualTo(EXAMPLE_SERVICE_TYPE); assertThat(definition.getTypeToMock()).isEqualTo(EXAMPLE_SERVICE_TYPE);
assertThat(definition.getExtraInterfaces()) assertThat(definition.getExtraInterfaces())
@ -80,7 +80,7 @@ public class MockDefinitionTests {
public void createMock() throws Exception { public void createMock() throws Exception {
MockDefinition definition = new MockDefinition("name", EXAMPLE_SERVICE_TYPE, MockDefinition definition = new MockDefinition("name", EXAMPLE_SERVICE_TYPE,
new Class<?>[] { ExampleExtraInterface.class }, new Class<?>[] { ExampleExtraInterface.class },
Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE, true); Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE);
ExampleService mock = definition.createMock(); ExampleService mock = definition.createMock();
MockCreationSettings<?> settings = new MockUtil().getMockSettings(mock); MockCreationSettings<?> settings = new MockUtil().getMockSettings(mock);
assertThat(mock).isInstanceOf(ExampleService.class); assertThat(mock).isInstanceOf(ExampleService.class);

View File

@ -67,17 +67,23 @@ public class ResetMocksTestExecutionListenerTests {
static class Config { static class Config {
@Bean @Bean
public ExampleService before() { public ExampleService before(MockitoBeans mockedBeans) {
return mock(ExampleService.class, MockReset.before()); ExampleService mock = mock(ExampleService.class, MockReset.before());
mockedBeans.add(mock);
return mock;
} }
@Bean @Bean
public ExampleService after() { public ExampleService after(MockitoBeans mockedBeans) {
return mock(ExampleService.class, MockReset.before()); ExampleService mock = mock(ExampleService.class, MockReset.before());
mockedBeans.add(mock);
return mock;
} }
@Bean @Bean
public ExampleService none() { public ExampleService none(MockitoBeans mockedBeans) {
ExampleService mock = mock(ExampleService.class, MockReset.before());
mockedBeans.add(mock);
return mock(ExampleService.class); return mock(ExampleService.class);
} }