Skip to content

Commit 0e00a49

Browse files
wilkinsonasnicoll
authored andcommitted
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
1 parent 52d7282 commit 0e00a49

12 files changed

+235
-122
lines changed

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/DefinitionsParser.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ private void parseMockBeanAnnotation(MockBean annotation, AnnotatedElement eleme
9292
for (ResolvableType typeToMock : typesToMock) {
9393
MockDefinition definition = new MockDefinition(annotation.name(), typeToMock,
9494
annotation.extraInterfaces(), annotation.answer(),
95-
annotation.serializable(), annotation.reset(),
96-
annotation.proxyTargetAware());
95+
annotation.serializable(), annotation.reset());
9796
addDefinition(element, definition, "mock");
9897
}
9998
}

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockBean.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.junit.runner.RunWith;
2727
import org.mockito.Answers;
2828
import org.mockito.MockSettings;
29-
import org.mockito.Mockito;
3029

3130
import org.springframework.context.ApplicationContext;
3231
import org.springframework.core.annotation.AliasFor;
@@ -140,15 +139,4 @@
140139
*/
141140
MockReset reset() default MockReset.AFTER;
142141

143-
/**
144-
* Indicates that Mockito methods such as {@link Mockito#verify(Object) verify(mock)}
145-
* should use the {@code target} of AOP advised beans, rather than the proxy itself.
146-
* If set to {@code false} you may need to use the result of
147-
* {@link org.springframework.test.util.AopTestUtils#getUltimateTargetObject(Object)
148-
* AopTestUtils.getUltimateTargetObject(...)} when calling Mockito methods.
149-
* @return {@code true} if the target of AOP advised beans is used or {@code false} if
150-
* the proxy is used directly
151-
*/
152-
boolean proxyTargetAware() default true;
153-
154142
}

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockDefinition.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,12 @@ class MockDefinition extends Definition {
5454
}
5555

5656
MockDefinition(ResolvableType typeToMock) {
57-
this(null, typeToMock, null, null, false, null, true);
57+
this(null, typeToMock, null, null, false, null);
5858
}
5959

6060
MockDefinition(String name, ResolvableType typeToMock, Class<?>[] extraInterfaces,
61-
Answers answer, boolean serializable, MockReset reset,
62-
boolean proxyTargetAware) {
63-
super(name, reset, proxyTargetAware);
61+
Answers answer, boolean serializable, MockReset reset) {
62+
super(name, reset, false);
6463
Assert.notNull(typeToMock, "TypeToMock must not be null");
6564
this.typeToMock = typeToMock;
6665
this.extraInterfaces = asClassSet(extraInterfaces);
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright 2012-2016 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.test.mock.mockito;
18+
19+
import java.util.ArrayList;
20+
import java.util.Iterator;
21+
import java.util.List;
22+
23+
/**
24+
* Beans created using Mockito.
25+
*
26+
* @author Andy Wilkinson
27+
*/
28+
public class MockitoBeans implements Iterable<Object> {
29+
30+
private final List<Object> beans = new ArrayList<Object>();
31+
32+
void add(Object bean) {
33+
this.beans.add(bean);
34+
}
35+
36+
@Override
37+
public Iterator<Object> iterator() {
38+
return this.beans.iterator();
39+
}
40+
41+
}

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
9494

9595
private final BeanNameGenerator beanNameGenerator = new DefaultBeanNameGenerator();
9696

97+
private final MockitoBeans mockitoBeans = new MockitoBeans();
98+
9799
private Map<Definition, String> beanNameRegistry = new HashMap<Definition, String>();
98100

99101
private Map<Field, RegisteredField> fieldRegistry = new HashMap<Field, RegisteredField>();
@@ -132,6 +134,7 @@ public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory)
132134

133135
private void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory,
134136
BeanDefinitionRegistry registry) {
137+
beanFactory.registerSingleton(MockitoBeans.class.getName(), this.mockitoBeans);
135138
DefinitionsParser parser = new DefinitionsParser(this.definitions);
136139
for (Class<?> configurationClass : getConfigurationClasses(beanFactory)) {
137140
parser.parse(configurationClass);
@@ -182,7 +185,13 @@ private void registerMock(ConfigurableListableBeanFactory beanFactory,
182185
String beanName = getBeanName(beanFactory, registry, definition, beanDefinition);
183186
beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(1,
184187
beanName);
188+
if (registry.containsBeanDefinition(beanName)) {
189+
registry.removeBeanDefinition(beanName);
190+
}
185191
registry.registerBeanDefinition(beanName, beanDefinition);
192+
Object mock = createMock(definition, beanName);
193+
beanFactory.registerSingleton(beanName, mock);
194+
this.mockitoBeans.add(mock);
186195
this.beanNameRegistry.put(definition, beanName);
187196
if (field != null) {
188197
this.fieldRegistry.put(field, new RegisteredField(definition, beanName));

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/ResetMocksTestExecutionListener.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import org.mockito.Mockito;
2424

25+
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
2526
import org.springframework.beans.factory.config.BeanDefinition;
2627
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
2728
import org.springframework.context.ApplicationContext;
@@ -70,6 +71,17 @@ private void resetMocks(ConfigurableApplicationContext applicationContext,
7071
}
7172
}
7273
}
74+
try {
75+
MockitoBeans mockedBeans = beanFactory.getBean(MockitoBeans.class);
76+
for (Object mockedBean : mockedBeans) {
77+
if (reset.equals(MockReset.get(mockedBean))) {
78+
Mockito.reset(mockedBean);
79+
}
80+
}
81+
}
82+
catch (NoSuchBeanDefinitionException ex) {
83+
// Continue
84+
}
7385
if (applicationContext.getParent() != null) {
7486
resetMocks(applicationContext.getParent(), reset);
7587
}

spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyAndNotProxyTargetAwareTests.java

Lines changed: 0 additions & 91 deletions
This file was deleted.

spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyTests.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.test.context.junit4.SpringRunner;
3535

3636
import static org.assertj.core.api.Assertions.assertThat;
37+
import static org.mockito.BDDMockito.given;
3738
import static org.mockito.Mockito.times;
3839
import static org.mockito.Mockito.verify;
3940

@@ -51,11 +52,13 @@ public class MockBeanWithAopProxyTests {
5152

5253
@Test
5354
public void verifyShouldUseProxyTarget() throws Exception {
55+
given(this.dateService.getDate()).willReturn(1L);
5456
Long d1 = this.dateService.getDate();
55-
Thread.sleep(200);
57+
assertThat(d1).isEqualTo(1L);
58+
given(this.dateService.getDate()).willReturn(2L);
5659
Long d2 = this.dateService.getDate();
57-
assertThat(d1).isEqualTo(d2);
58-
verify(this.dateService, times(1)).getDate();
60+
assertThat(d2).isEqualTo(2L);
61+
verify(this.dateService, times(2)).getDate();
5962
}
6063

6164
@Configuration
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright 2012-2016 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.test.mock.mockito;
18+
19+
import org.junit.Test;
20+
import org.junit.runner.RunWith;
21+
22+
import org.springframework.beans.factory.annotation.Autowired;
23+
import org.springframework.context.annotation.Bean;
24+
import org.springframework.context.annotation.Configuration;
25+
import org.springframework.scheduling.annotation.Async;
26+
import org.springframework.scheduling.annotation.EnableAsync;
27+
import org.springframework.test.context.junit4.SpringRunner;
28+
29+
import static org.assertj.core.api.Assertions.assertThat;
30+
import static org.mockito.BDDMockito.given;
31+
32+
/**
33+
* Tests for a mock bean where the mocked interface has an async method.
34+
*
35+
* @author Andy Wilkinson
36+
*/
37+
@RunWith(SpringRunner.class)
38+
public class MockBeanWithAsyncInterfaceMethodIntegrationTests {
39+
40+
@MockBean
41+
private Transformer transformer;
42+
43+
@Autowired
44+
private MyService service;
45+
46+
@Test
47+
public void mockedMethodsAreNotAsync() {
48+
given(this.transformer.transform("foo")).willReturn("bar");
49+
assertThat(this.service.transform("foo")).isEqualTo("bar");
50+
}
51+
52+
private interface Transformer {
53+
54+
@Async
55+
String transform(String input);
56+
57+
}
58+
59+
private static class MyService {
60+
61+
private final Transformer transformer;
62+
63+
MyService(Transformer transformer) {
64+
this.transformer = transformer;
65+
}
66+
67+
public String transform(String input) {
68+
return this.transformer.transform(input);
69+
}
70+
71+
}
72+
73+
@Configuration
74+
@EnableAsync
75+
static class MyConfiguration {
76+
77+
@Bean
78+
public MyService myService(Transformer transformer) {
79+
return new MyService(transformer);
80+
}
81+
}
82+
83+
}

0 commit comments

Comments
 (0)