From cdfbf28099b30621a118ebf2ec4988364295b610 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 10 May 2016 22:40:09 -0700 Subject: [PATCH] Allow @MockBean/@SpyBean on Spring AOP proxies Update Mockito support so that AOP Proxies automatically get additional `Advice` that allows them to work with Mockito. Prior to this commit a call to `verify` would fail because exiting AOP advice would confuse Mockito and an `UnfinishedVerificationException` would be thrown. The `MockitoAopProxyTargetInterceptor` works by detecting calls to a mock that have been proceeded by `verify()` and bypassing AOP to directly call the mock. The order that `@SpyBean` creation occurs has also been updated to ensure that that the spy is created before AOP advice is applied. Without this, the creation of a spy would fail because Mockito copies 'state' to the newly created spied instance. Unfortunately, in the case of AOP proxies, 'state' includes cglib interceptor fields. This means that Mockito's own interceptors are clobbered by Spring's AOP interceptors. Fixes gh-5837 --- .../boot/test/mock/mockito/Definition.java | 17 +- .../test/mock/mockito/DefinitionsParser.java | 5 +- .../boot/test/mock/mockito/MockBean.java | 12 ++ .../test/mock/mockito/MockDefinition.java | 7 +- .../boot/test/mock/mockito/MockReset.java | 13 +- .../MockitoAopProxyTargetInterceptor.java | 128 +++++++++++++++ .../mock/mockito/MockitoPostProcessor.java | 146 ++++++++++++++---- .../ResetMocksTestExecutionListener.java | 5 +- .../boot/test/mock/mockito/SpyBean.java | 12 ++ .../boot/test/mock/mockito/SpyDefinition.java | 5 +- ...thAopProxyAndNotProxyTargetAwareTests.java | 91 +++++++++++ .../mockito/MockBeanWithAopProxyTests.java | 92 +++++++++++ .../mock/mockito/MockDefinitionTests.java | 10 +- ...thAopProxyAndNotProxyTargetAwareTests.java | 91 +++++++++++ .../mockito/SpyBeanWithAopProxyTests.java | 92 +++++++++++ .../test/mock/mockito/SpyDefinitionTests.java | 18 ++- 16 files changed, 683 insertions(+), 61 deletions(-) create mode 100644 spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyAndNotProxyTargetAwareTests.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyTests.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyAndNotProxyTargetAwareTests.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyTests.java diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/Definition.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/Definition.java index 916414d46c..e1dbd0b18c 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/Definition.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/Definition.java @@ -32,9 +32,12 @@ abstract class Definition { private final MockReset reset; - Definition(String name, MockReset reset) { + private final boolean proxyTargetAware; + + Definition(String name, MockReset reset, boolean proxyTargetAware) { this.name = name; this.reset = (reset != null ? reset : MockReset.AFTER); + this.proxyTargetAware = proxyTargetAware; } /** @@ -53,11 +56,21 @@ abstract class Definition { return this.reset; } + /** + * Return if AOP advised beans should be proxy target aware. + * @return if proxy target aware + */ + public boolean isProxyTargetAware() { + return this.proxyTargetAware; + } + @Override public int hashCode() { int result = 1; result = MULTIPLIER * result + ObjectUtils.nullSafeHashCode(this.name); result = MULTIPLIER * result + ObjectUtils.nullSafeHashCode(this.reset); + result = MULTIPLIER * result + + ObjectUtils.nullSafeHashCode(this.proxyTargetAware); return result; } @@ -73,6 +86,8 @@ abstract class Definition { boolean result = true; result &= ObjectUtils.nullSafeEquals(this.name, other.name); result &= ObjectUtils.nullSafeEquals(this.reset, other.reset); + result &= ObjectUtils.nullSafeEquals(this.proxyTargetAware, + other.proxyTargetAware); return result; } diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/DefinitionsParser.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/DefinitionsParser.java index 1ad2e17128..07249d0210 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/DefinitionsParser.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/DefinitionsParser.java @@ -92,7 +92,8 @@ class DefinitionsParser { for (Class classToMock : classesToMock) { MockDefinition definition = new MockDefinition(annotation.name(), classToMock, annotation.extraInterfaces(), annotation.answer(), - annotation.serializable(), annotation.reset()); + annotation.serializable(), annotation.reset(), + annotation.proxyTargetAware()); addDefinition(element, definition, "mock"); } } @@ -107,7 +108,7 @@ class DefinitionsParser { } for (Class classToSpy : classesToSpy) { SpyDefinition definition = new SpyDefinition(annotation.name(), classToSpy, - annotation.reset()); + annotation.reset(), annotation.proxyTargetAware()); addDefinition(element, definition, "spy"); } } diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockBean.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockBean.java index 4f76b9f6df..bb19f10d54 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockBean.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockBean.java @@ -26,6 +26,7 @@ import java.lang.annotation.Target; import org.junit.runner.RunWith; import org.mockito.Answers; import org.mockito.MockSettings; +import org.mockito.Mockito; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.AliasFor; @@ -139,4 +140,15 @@ public @interface MockBean { */ 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; + } diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockDefinition.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockDefinition.java index 668167a5a8..e54f5fdb28 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockDefinition.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockDefinition.java @@ -48,12 +48,13 @@ class MockDefinition extends Definition { private final boolean serializable; MockDefinition(Class classToMock) { - this(null, classToMock, null, null, false, null); + this(null, classToMock, null, null, false, null, true); } MockDefinition(String name, Class classToMock, Class[] extraInterfaces, - Answers answer, boolean serializable, MockReset reset) { - super(name, reset); + Answers answer, boolean serializable, MockReset reset, + boolean proxyTargetAware) { + super(name, reset, proxyTargetAware); Assert.notNull(classToMock, "ClassToMock must not be null"); this.classToMock = classToMock; this.extraInterfaces = asClassSet(extraInterfaces); diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockReset.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockReset.java index 0da472c807..f0042116d5 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockReset.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockReset.java @@ -33,6 +33,7 @@ import org.springframework.util.Assert; * the {@code ApplicationContext} using the static methods. * * @author Phillip Webb + * @since 1.4.0 * @see ResetMocksTestExecutionListener */ public enum MockReset { @@ -55,8 +56,8 @@ public enum MockReset { private static final MockUtil util = new MockUtil(); /** - * Create {@link MockSettings settings} to be used with mocks where reset should - * occur before each test method runs. + * Create {@link MockSettings settings} to be used with mocks where reset should occur + * before each test method runs. * @return mock settings */ public static MockSettings before() { @@ -64,8 +65,8 @@ public enum MockReset { } /** - * Create {@link MockSettings settings} to be used with mocks where reset should - * occur after each test method runs. + * Create {@link MockSettings settings} to be used with mocks where reset should occur + * after each test method runs. * @return mock settings */ public static MockSettings after() { @@ -73,8 +74,8 @@ public enum MockReset { } /** - * Create {@link MockSettings settings} to be used with mocks where a specific - * reset should occur. + * Create {@link MockSettings settings} to be used with mocks where a specific reset + * should occur. * @param reset the reset type * @return mock settings */ diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java new file mode 100644 index 0000000000..58c5c2c6f1 --- /dev/null +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java @@ -0,0 +1,128 @@ +/* + * 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.lang.reflect.Field; + +import org.aopalliance.aop.Advice; +import org.aopalliance.intercept.Interceptor; +import org.aopalliance.intercept.MethodInterceptor; +import org.aopalliance.intercept.MethodInvocation; +import org.mockito.internal.InternalMockHandler; +import org.mockito.internal.progress.MockingProgress; +import org.mockito.internal.stubbing.InvocationContainer; +import org.mockito.internal.util.MockUtil; +import org.mockito.internal.verification.MockAwareVerificationMode; +import org.mockito.verification.VerificationMode; + +import org.springframework.aop.Advisor; +import org.springframework.aop.framework.Advised; +import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.util.AopTestUtils; +import org.springframework.util.Assert; +import org.springframework.util.ReflectionUtils; + +/** + * AOP {@link Interceptor} that attempts to make AOP proxy beans work with Mockito. Works + * by bypassing AOP advice when a method is invoked via + * {@code Mockito#verify(Object) verify(mock)}. + * + * @author Phillip Webb + */ +class MockitoAopProxyTargetInterceptor implements MethodInterceptor { + + private final Object source; + + private final Object target; + + private final Verification verification; + + MockitoAopProxyTargetInterceptor(Object source, Object target) throws Exception { + this.source = source; + this.target = target; + this.verification = new Verification(target); + } + + @Override + public Object invoke(MethodInvocation invocation) throws Throwable { + if (this.verification.isVerifying()) { + this.verification.replaceVerifyMock(this.source, this.target); + return AopUtils.invokeJoinpointUsingReflection(this.target, + invocation.getMethod(), invocation.getArguments()); + } + return invocation.proceed(); + } + + @Autowired + public static void applyTo(Object source) { + Assert.state(AopUtils.isAopProxy(source), "Source must be an AOP proxy"); + try { + Advised advised = (Advised) source; + for (Advisor advisor : advised.getAdvisors()) { + if (advisor instanceof MockitoAopProxyTargetInterceptor) { + return; + } + } + Object target = AopTestUtils.getUltimateTargetObject(source); + Advice advice = new MockitoAopProxyTargetInterceptor(source, target); + advised.addAdvice(0, advice); + } + catch (Exception ex) { + throw new IllegalStateException("Unable to apply Mockito AOP support", ex); + } + } + + private static class Verification { + + private final MockingProgress progress; + + Verification(Object target) { + MockUtil mockUtil = new MockUtil(); + InternalMockHandler handler = mockUtil.getMockHandler(target); + InvocationContainer container = handler.getInvocationContainer(); + Field field = ReflectionUtils.findField(container.getClass(), + "mockingProgress"); + ReflectionUtils.makeAccessible(field); + this.progress = (MockingProgress) ReflectionUtils.getField(field, container); + } + + public synchronized boolean isVerifying() { + VerificationMode mode = this.progress.pullVerificationMode(); + if (mode != null) { + this.progress.verificationStarted(mode); + return true; + } + return false; + } + + public synchronized void replaceVerifyMock(Object source, Object target) { + VerificationMode mode = this.progress.pullVerificationMode(); + if (mode != null) { + if (mode instanceof MockAwareVerificationMode) { + MockAwareVerificationMode mockAwareMode = (MockAwareVerificationMode) mode; + if (mockAwareMode.getMock() == source) { + mode = new MockAwareVerificationMode(target, mockAwareMode); + } + } + this.progress.verificationStarted(mode); + } + } + + } + +} diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java index e76208eedf..113da46e45 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.BeansException; import org.springframework.beans.PropertyValues; import org.springframework.beans.factory.BeanClassLoaderAware; @@ -39,6 +40,7 @@ import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.ConstructorArgumentValues; import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder; import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessorAdapter; +import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.beans.factory.support.DefaultBeanNameGenerator; @@ -47,6 +49,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.ConfigurationClassPostProcessor; import org.springframework.core.Conventions; import org.springframework.core.Ordered; +import org.springframework.core.PriorityOrdered; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -67,7 +70,7 @@ import org.springframework.util.StringUtils; */ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAdapter implements BeanClassLoaderAware, BeanFactoryAware, BeanFactoryPostProcessor, - BeanPostProcessor, Ordered { + Ordered { private static final String BEAN_NAME = MockitoPostProcessor.class.getName(); @@ -85,7 +88,7 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda private Map beanNameRegistry = new HashMap(); - private Map fieldRegistry = new HashMap(); + private Map fieldRegistry = new HashMap(); private Map spies = new HashMap(); @@ -168,12 +171,13 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda private void registerMock(ConfigurableListableBeanFactory beanFactory, BeanDefinitionRegistry registry, MockDefinition definition, Field field) { RootBeanDefinition beanDefinition = createBeanDefinition(definition); - String name = getBeanName(beanFactory, registry, definition, beanDefinition); - beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(1, name); - registry.registerBeanDefinition(name, beanDefinition); - this.beanNameRegistry.put(definition, name); + String beanName = getBeanName(beanFactory, registry, definition, beanDefinition); + beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(1, + beanName); + registry.registerBeanDefinition(beanName, beanDefinition); + this.beanNameRegistry.put(definition, beanName); if (field != null) { - this.fieldRegistry.put(field, name); + this.fieldRegistry.put(field, new RegisteredField(definition, beanName)); } } @@ -219,55 +223,53 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda } private void registerSpy(ConfigurableListableBeanFactory beanFactory, - BeanDefinitionRegistry registry, SpyDefinition spyDefinition, Field field) { + BeanDefinitionRegistry registry, SpyDefinition definition, Field field) { String[] existingBeans = beanFactory - .getBeanNamesForType(spyDefinition.getClassToSpy()); + .getBeanNamesForType(definition.getClassToSpy()); if (ObjectUtils.isEmpty(existingBeans)) { - createSpy(registry, spyDefinition, field); + createSpy(registry, definition, field); } else { - registerSpies(spyDefinition, field, existingBeans); + registerSpies(definition, field, existingBeans); } } - private void createSpy(BeanDefinitionRegistry registry, SpyDefinition spyDefinition, + private void createSpy(BeanDefinitionRegistry registry, SpyDefinition definition, Field field) { RootBeanDefinition beanDefinition = new RootBeanDefinition( - spyDefinition.getClassToSpy()); + definition.getClassToSpy()); String beanName = this.beanNameGenerator.generateBeanName(beanDefinition, registry); registry.registerBeanDefinition(beanName, beanDefinition); - registerSpy(spyDefinition, field, beanName); + registerSpy(definition, field, beanName); } - private void registerSpies(SpyDefinition spyDefinition, Field field, + private void registerSpies(SpyDefinition definition, Field field, String[] existingBeans) { if (field != null) { Assert.state(field == null || existingBeans.length == 1, - "Unable to register spy bean " - + spyDefinition.getClassToSpy().getName() + "Unable to register spy bean " + definition.getClassToSpy().getName() + " expected a single existing bean to replace but found " + new TreeSet(Arrays.asList(existingBeans))); } for (String beanName : existingBeans) { - registerSpy(spyDefinition, field, beanName); + registerSpy(definition, field, beanName); } } - private void registerSpy(SpyDefinition spyDefinition, Field field, String beanName) { - this.spies.put(beanName, spyDefinition); - this.beanNameRegistry.put(spyDefinition, beanName); + private void registerSpy(SpyDefinition definition, Field field, String beanName) { + this.spies.put(beanName, definition); + this.beanNameRegistry.put(definition, beanName); if (field != null) { - this.fieldRegistry.put(field, beanName); + this.fieldRegistry.put(field, new RegisteredField(definition, beanName)); } } - @Override - public Object postProcessAfterInitialization(Object bean, String beanName) + protected Object createSpyIfNecessary(Object bean, String beanName) throws BeansException { - SpyDefinition spyDefinition = this.spies.get(beanName); - if (spyDefinition != null) { - bean = spyDefinition.createSpy(beanName, bean); + SpyDefinition definition = this.spies.get(beanName); + if (definition != null) { + bean = definition.createSpy(beanName, bean); } return bean; } @@ -289,9 +291,9 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda } private void postProcessField(Object bean, Field field) { - String beanName = this.fieldRegistry.get(field); - if (StringUtils.hasLength(beanName)) { - inject(field, bean, beanName); + RegisteredField registered = this.fieldRegistry.get(field); + if (registered != null && StringUtils.hasLength(registered.getBeanName())) { + inject(field, bean, registered.getBeanName(), registered.getDefinition()); } } @@ -299,15 +301,19 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda String beanName = this.beanNameRegistry.get(definition); Assert.state(StringUtils.hasLength(beanName), "No bean found for definition " + definition); - inject(field, target, beanName); + inject(field, target, beanName, definition); } - private void inject(Field field, Object target, String beanName) { + private void inject(Field field, Object target, String beanName, + Definition definition) { try { field.setAccessible(true); Assert.state(ReflectionUtils.getField(field, target) == null, "The field " + field + " cannot have an existing value"); Object bean = this.beanFactory.getBean(beanName, field.getType()); + if (definition.isProxyTargetAware() && AopUtils.isAopProxy(bean)) { + MockitoAopProxyTargetInterceptor.applyTo(bean); + } ReflectionUtils.setField(field, target, bean); } catch (Throwable ex) { @@ -351,6 +357,7 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda public static void register(BeanDefinitionRegistry registry, Class postProcessor, Set definitions) { + SpyPostProcessor.register(registry); BeanDefinition definition = getOrAddBeanDefinition(registry, postProcessor); ValueHolder constructorArg = definition.getConstructorArgumentValues() .getIndexedArgumentValue(0, Set.class); @@ -375,4 +382,79 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda return registry.getBeanDefinition(BEAN_NAME); } + /** + * {@link BeanPostProcessor} to handle {@link SpyBean} definitions. Registered as a + * separate processor so that it can ordered above AOP post processors. + */ + static class SpyPostProcessor extends InstantiationAwareBeanPostProcessorAdapter + implements PriorityOrdered { + + private static final String BEAN_NAME = SpyPostProcessor.class.getName(); + + private final MockitoPostProcessor mockitoPostProcessor; + + SpyPostProcessor(MockitoPostProcessor mockitoPostProcessor) { + this.mockitoPostProcessor = mockitoPostProcessor; + } + + @Override + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; + } + + @Override + public Object getEarlyBeanReference(Object bean, String beanName) + throws BeansException { + return createSpyIfNecessary(bean, beanName); + } + + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) + throws BeansException { + return createSpyIfNecessary(bean, beanName); + } + + private Object createSpyIfNecessary(Object bean, String beanName) { + return this.mockitoPostProcessor.createSpyIfNecessary(bean, beanName); + } + + public static void register(BeanDefinitionRegistry registry) { + if (!registry.containsBeanDefinition(BEAN_NAME)) { + RootBeanDefinition definition = new RootBeanDefinition( + SpyPostProcessor.class); + definition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); + ConstructorArgumentValues constructorArguments = definition + .getConstructorArgumentValues(); + constructorArguments.addIndexedArgumentValue(0, + new RuntimeBeanReference(MockitoPostProcessor.BEAN_NAME)); + registry.registerBeanDefinition(BEAN_NAME, definition); + } + } + + } + + /** + * An registered field item. + */ + private static class RegisteredField { + + private final Definition definition; + + private final String beanName; + + RegisteredField(Definition definition, String beanName) { + this.definition = definition; + this.beanName = beanName; + } + + public Definition getDefinition() { + return this.definition; + } + + public String getBeanName() { + return this.beanName; + } + + } + } diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/ResetMocksTestExecutionListener.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/ResetMocksTestExecutionListener.java index 81f8535d77..9d02dc2084 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/ResetMocksTestExecutionListener.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/ResetMocksTestExecutionListener.java @@ -50,7 +50,6 @@ public class ResetMocksTestExecutionListener extends AbstractTestExecutionListen if (applicationContext instanceof ConfigurableApplicationContext) { resetMocks((ConfigurableApplicationContext) applicationContext, reset); } - } private void resetMocks(ConfigurableApplicationContext applicationContext, @@ -59,7 +58,9 @@ public class ResetMocksTestExecutionListener extends AbstractTestExecutionListen String[] names = beanFactory.getBeanDefinitionNames(); for (String name : names) { BeanDefinition definition = beanFactory.getBeanDefinition(name); - if (AbstractBeanDefinition.SCOPE_DEFAULT.equals(definition.getScope())) { + String scope = definition.getScope(); + if (AbstractBeanDefinition.SCOPE_DEFAULT.equals(scope) + || BeanDefinition.SCOPE_SINGLETON.equals(scope)) { Object bean = beanFactory.getBean(name); if (reset.equals(MockReset.get(bean))) { Mockito.reset(bean); diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyBean.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyBean.java index 929bb441e8..69faeb4dd6 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyBean.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyBean.java @@ -24,6 +24,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.AliasFor; @@ -116,4 +117,15 @@ public @interface SpyBean { */ 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; + } diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java index 2409cc6045..46f69b0a61 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java @@ -38,8 +38,9 @@ class SpyDefinition extends Definition { private final Class classToSpy; - SpyDefinition(String name, Class classToSpy, MockReset reset) { - super(name, reset); + SpyDefinition(String name, Class classToSpy, MockReset reset, + boolean proxyTargetAware) { + super(name, reset, proxyTargetAware); Assert.notNull(classToSpy, "ClassToSpy must not be null"); this.classToSpy = classToSpy; diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyAndNotProxyTargetAwareTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyAndNotProxyTargetAwareTests.java new file mode 100644 index 0000000000..878c67df7b --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyAndNotProxyTargetAwareTests.java @@ -0,0 +1,91 @@ +/* + * 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 5837 + */ +@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(); + } + + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyTests.java new file mode 100644 index 0000000000..27c199c374 --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockBeanWithAopProxyTests.java @@ -0,0 +1,92 @@ +/* + * 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.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.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Test {@link MockBean} when mixed with Spring AOP. + * + * @author Phillip Webb + * @see 5837 + */ +@RunWith(SpringRunner.class) +public class MockBeanWithAopProxyTests { + + @MockBean + private DateService dateService; + + @Test + public void verifyShouldUseProxyTarget() throws Exception { + Long d1 = this.dateService.getDate(); + Thread.sleep(200); + Long d2 = this.dateService.getDate(); + assertThat(d1).isEqualTo(d2); + verify(this.dateService, times(1)).getDate(); + } + + @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(); + } + + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockDefinitionTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockDefinitionTests.java index 3dbcd47bbb..950bb4fa92 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockDefinitionTests.java +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/MockDefinitionTests.java @@ -42,13 +42,13 @@ public class MockDefinitionTests { public void ClassToMockMustNotBeNull() throws Exception { this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("ClassToMock must not be null"); - new MockDefinition(null, null, null, null, false, null); + new MockDefinition(null, null, null, null, false, null, true); } @Test public void createWithDefaults() throws Exception { MockDefinition definition = new MockDefinition(null, ExampleService.class, null, - null, false, null); + null, false, null, true); assertThat(definition.getName()).isNull(); assertThat(definition.getClassToMock()).isEqualTo(ExampleService.class); assertThat(definition.getExtraInterfaces()).isEmpty(); @@ -61,7 +61,7 @@ public class MockDefinitionTests { public void createExplicit() throws Exception { MockDefinition definition = new MockDefinition("name", ExampleService.class, new Class[] { ExampleExtraInterface.class }, - Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE); + Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE, false); assertThat(definition.getName()).isEqualTo("name"); assertThat(definition.getClassToMock()).isEqualTo(ExampleService.class); assertThat(definition.getExtraInterfaces()) @@ -69,13 +69,14 @@ public class MockDefinitionTests { assertThat(definition.getAnswer()).isEqualTo(Answers.RETURNS_SMART_NULLS); assertThat(definition.isSerializable()).isTrue(); assertThat(definition.getReset()).isEqualTo(MockReset.BEFORE); + assertThat(definition.isProxyTargetAware()).isFalse(); } @Test public void createMock() throws Exception { MockDefinition definition = new MockDefinition("name", ExampleService.class, new Class[] { ExampleExtraInterface.class }, - Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE); + Answers.RETURNS_SMART_NULLS, true, MockReset.BEFORE, true); ExampleService mock = definition.createMock(); MockCreationSettings settings = new MockUtil().getMockSettings(mock); assertThat(mock).isInstanceOf(ExampleService.class); @@ -85,7 +86,6 @@ public class MockDefinitionTests { .isEqualTo(Answers.RETURNS_SMART_NULLS.get()); assertThat(settings.isSerializable()).isTrue(); assertThat(MockReset.get(mock)).isEqualTo(MockReset.BEFORE); - } } diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyAndNotProxyTargetAwareTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyAndNotProxyTargetAwareTests.java new file mode 100644 index 0000000000..f701eeddc7 --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyAndNotProxyTargetAwareTests.java @@ -0,0 +1,91 @@ +/* + * 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 SpyBean} when mixed with Spring AOP. + * + * @author Phillip Webb + * @see 5837 + */ +@RunWith(SpringRunner.class) +public class SpyBeanWithAopProxyAndNotProxyTargetAwareTests { + + @SpyBean(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(); + } + + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyTests.java new file mode 100644 index 0000000000..49d9572dcd --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithAopProxyTests.java @@ -0,0 +1,92 @@ +/* + * 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.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.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Test {@link SpyBean} when mixed with Spring AOP. + * + * @author Phillip Webb + * @see 5837 + */ +@RunWith(SpringRunner.class) +public class SpyBeanWithAopProxyTests { + + @SpyBean + private DateService dateService; + + @Test + public void verifyShouldUseProxyTarget() throws Exception { + Long d1 = this.dateService.getDate(); + Thread.sleep(200); + Long d2 = this.dateService.getDate(); + assertThat(d1).isEqualTo(d2); + verify(this.dateService, times(1)).getDate(); + } + + @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(); + } + + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyDefinitionTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyDefinitionTests.java index d7265cd9e8..243b24ced6 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyDefinitionTests.java +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyDefinitionTests.java @@ -43,31 +43,33 @@ public class SpyDefinitionTests { public void classToSpyMustNotBeNull() throws Exception { this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("ClassToSpy must not be null"); - new SpyDefinition(null, null, null); + new SpyDefinition(null, null, null, true); } @Test public void createWithDefaults() throws Exception { - SpyDefinition definition = new SpyDefinition(null, RealExampleService.class, - null); + SpyDefinition definition = new SpyDefinition(null, RealExampleService.class, null, + true); assertThat(definition.getName()).isNull(); assertThat(definition.getClassToSpy()).isEqualTo(RealExampleService.class); assertThat(definition.getReset()).isEqualTo(MockReset.AFTER); + assertThat(definition.isProxyTargetAware()).isTrue(); } @Test public void createExplicit() throws Exception { SpyDefinition definition = new SpyDefinition("name", RealExampleService.class, - MockReset.BEFORE); + MockReset.BEFORE, false); assertThat(definition.getName()).isEqualTo("name"); assertThat(definition.getClassToSpy()).isEqualTo(RealExampleService.class); assertThat(definition.getReset()).isEqualTo(MockReset.BEFORE); + assertThat(definition.isProxyTargetAware()).isFalse(); } @Test public void createSpy() throws Exception { SpyDefinition definition = new SpyDefinition("name", RealExampleService.class, - MockReset.BEFORE); + MockReset.BEFORE, true); RealExampleService spy = definition.createSpy(new RealExampleService("hello")); MockCreationSettings settings = new MockUtil().getMockSettings(spy); assertThat(spy).isInstanceOf(ExampleService.class); @@ -80,7 +82,7 @@ public class SpyDefinitionTests { @Test public void createSpyWhenNullInstanceShouldThrowException() throws Exception { SpyDefinition definition = new SpyDefinition("name", RealExampleService.class, - MockReset.BEFORE); + MockReset.BEFORE, true); this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("Instance must not be null"); definition.createSpy(null); @@ -89,7 +91,7 @@ public class SpyDefinitionTests { @Test public void createSpyWhenWrongInstanceShouldThrowException() throws Exception { SpyDefinition definition = new SpyDefinition("name", RealExampleService.class, - MockReset.BEFORE); + MockReset.BEFORE, true); this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("must be an instance of"); definition.createSpy(new ExampleServiceCaller(null)); @@ -98,7 +100,7 @@ public class SpyDefinitionTests { @Test public void createSpyTwice() throws Exception { SpyDefinition definition = new SpyDefinition("name", RealExampleService.class, - MockReset.BEFORE); + MockReset.BEFORE, true); Object instance = new RealExampleService("hello"); instance = definition.createSpy(instance); instance = definition.createSpy(instance);