From 027c5a0e35b81259191cae8236a1939e5e4827de Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 3 Oct 2017 11:31:07 +0100 Subject: [PATCH] Remove usage of Mockito's internals Previously, we relied on Mockito's internals to bypass any Spring AOP proxy during verification of a spy. Thanks to a new API in Mockito, we can replace the use of Mockito's internals with a VerificationStartedListener. This listener changes Mockito's view of the mock to be the ultimate target of the AOP proxy, i.e. to be the actual Mockito-created spy, allowing Mockito's verification of the spy to proceed successfully. This above-described change will mean that we require a very up-to-date version of Mockito so the tests that verify our compatibility with 2.5 have been removed as we will no longer support it when using @MockBean or @SpyBean. Lastly, two tests have been updated to replace their usage of the internal MockUtil class with the equivalent public API calls. Closes gh-10352 --- .../MockitoAopProxyTargetInterceptor.java | 133 ------------------ .../mock/mockito/MockitoPostProcessor.java | 13 -- .../boot/test/mock/mockito/SpyDefinition.java | 21 +++ .../test/mock/mockito/Mockito25Tests.java | 59 -------- ...ngBeansWithOnePrimaryIntegrationTests.java | 5 +- ...estFieldForMultipleExistingBeansTests.java | 9 +- 6 files changed, 30 insertions(+), 210 deletions(-) delete mode 100644 spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java delete mode 100644 spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/Mockito25Tests.java diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java deleted file mode 100644 index f33161ba08..0000000000 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoAopProxyTargetInterceptor.java +++ /dev/null @@ -1,133 +0,0 @@ -/* - * Copyright 2012-2017 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.Collections; -import java.util.List; - -import org.aopalliance.aop.Advice; -import org.aopalliance.intercept.Interceptor; -import org.aopalliance.intercept.MethodInterceptor; -import org.aopalliance.intercept.MethodInvocation; -import org.mockito.internal.matchers.LocalizedMatcher; -import org.mockito.internal.progress.ArgumentMatcherStorage; -import org.mockito.internal.progress.MockingProgress; -import org.mockito.internal.progress.ThreadSafeMockingProgress; -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; - -/** - * 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 = new Verification(); - - MockitoAopProxyTargetInterceptor(Object source, Object target) throws Exception { - this.source = source; - this.target = 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 Object monitor = new Object(); - - private final MockingProgress progress = ThreadSafeMockingProgress - .mockingProgress(); - - public boolean isVerifying() { - synchronized (this.monitor) { - VerificationMode mode = this.progress.pullVerificationMode(); - if (mode != null) { - resetVerificationStarted(mode); - return true; - } - return false; - } - } - - public void replaceVerifyMock(Object source, Object target) { - synchronized (this.monitor) { - VerificationMode mode = this.progress.pullVerificationMode(); - if (mode != null) { - if (mode instanceof MockAwareVerificationMode) { - MockAwareVerificationMode mockAwareMode = (MockAwareVerificationMode) mode; - if (mockAwareMode.getMock() == source) { - mode = new MockAwareVerificationMode(target, mode, - Collections.emptySet()); - } - } - resetVerificationStarted(mode); - } - } - } - - private void resetVerificationStarted(VerificationMode mode) { - ArgumentMatcherStorage storage = this.progress.getArgumentMatcherStorage(); - List matchers = storage.pullLocalizedMatchers(); - this.progress.verificationStarted(mode); - matchers.stream().map(LocalizedMatcher::getMatcher) - .forEach(storage::reportMatcher); - } - - } - -} diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java index 5cafdc7fe2..a84186e7a6 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java @@ -27,7 +27,6 @@ import java.util.Set; import java.util.TreeSet; import org.springframework.aop.scope.ScopedProxyUtils; -import org.springframework.aop.support.AopUtils; import org.springframework.beans.BeansException; import org.springframework.beans.PropertyValues; import org.springframework.beans.factory.BeanClassLoaderAware; @@ -391,9 +390,6 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda 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() && isAopProxy(bean)) { - MockitoAopProxyTargetInterceptor.applyTo(bean); - } ReflectionUtils.setField(field, target, bean); } catch (Throwable ex) { @@ -401,15 +397,6 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda } } - private boolean isAopProxy(Object object) { - try { - return AopUtils.isAopProxy(object); - } - catch (Throwable ex) { - return false; - } - } - @Override public int getOrder() { return Ordered.LOWEST_PRECEDENCE - 10; diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java index 8b37dc5ff6..9af4d6eccd 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java @@ -18,9 +18,12 @@ package org.springframework.boot.test.mock.mockito; import org.mockito.MockSettings; import org.mockito.Mockito; +import org.mockito.listeners.VerificationStartedEvent; +import org.mockito.listeners.VerificationStartedListener; import org.springframework.core.ResolvableType; import org.springframework.core.style.ToStringCreator; +import org.springframework.test.util.AopTestUtils; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -93,7 +96,25 @@ class SpyDefinition extends Definition { } settings.spiedInstance(instance); settings.defaultAnswer(Mockito.CALLS_REAL_METHODS); + if (this.isProxyTargetAware()) { + settings.verificationStartedListeners( + new SpringAopBypassingVerificationStartedListener()); + } return (T) Mockito.mock(instance.getClass(), settings); } + /** + * A {@link VerificationStartedListener} that bypasses any proxy created by Spring AOP + * when the verification of a spy starts. + */ + private static final class SpringAopBypassingVerificationStartedListener + implements VerificationStartedListener { + + @Override + public void onVerificationStarted(VerificationStartedEvent event) { + event.setMock(AopTestUtils.getUltimateTargetObject(event.getMock())); + } + + } + } diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/Mockito25Tests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/Mockito25Tests.java deleted file mode 100644 index adfc655fef..0000000000 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/Mockito25Tests.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright 2012-2017 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.JUnitCore; -import org.junit.runner.Result; -import org.junit.runner.RunWith; -import org.junit.runner.notification.Failure; - -import org.springframework.boot.testsupport.runner.classpath.ClassPathOverrides; -import org.springframework.boot.testsupport.runner.classpath.ModifiedClassPathRunner; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * Tests for compatibility with Mockito 2.5 - * - * @author Andy Wilkinson - * @author Phillip Webb - */ -@RunWith(ModifiedClassPathRunner.class) -@ClassPathOverrides("org.mockito:mockito-core:2.5.4") -public class Mockito25Tests { - - @Test - public void resetMocksTestExecutionListenerTestsWithMockito2() { - runTests(ResetMocksTestExecutionListenerTests.class); - } - - @Test - public void spyBeanWithAopProxyTestsWithMockito2() { - runTests(SpyBeanWithAopProxyTests.class); - } - - private void runTests(Class testClass) { - Result result = new JUnitCore().run(testClass); - for (Failure failure : result.getFailures()) { - System.err.println(failure.getTrace()); - } - assertThat(result.getFailureCount()).isEqualTo(0); - assertThat(result.getRunCount()).isGreaterThan(0); - } - -} diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansWithOnePrimaryIntegrationTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansWithOnePrimaryIntegrationTests.java index 4e9ab72821..f4756514bb 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansWithOnePrimaryIntegrationTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansWithOnePrimaryIntegrationTests.java @@ -18,7 +18,7 @@ package org.springframework.boot.test.mock.mockito; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.internal.util.MockUtil; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.mock.mockito.example.ExampleGenericStringServiceCaller; @@ -50,7 +50,8 @@ public class SpyBeanOnTestFieldForMultipleExistingBeansWithOnePrimaryIntegration @Test public void testSpying() throws Exception { assertThat(this.caller.sayGreeting()).isEqualTo("I say two"); - assertThat(MockUtil.getMockName(this.spy).toString()).isEqualTo("two"); + assertThat(Mockito.mockingDetails(this.spy).getMockCreationSettings() + .getMockName().toString()).isEqualTo("two"); verify(this.spy).greeting(); } diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithNameOnTestFieldForMultipleExistingBeansTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithNameOnTestFieldForMultipleExistingBeansTests.java index 90178f7042..529d906e86 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithNameOnTestFieldForMultipleExistingBeansTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanWithNameOnTestFieldForMultipleExistingBeansTests.java @@ -18,7 +18,8 @@ package org.springframework.boot.test.mock.mockito; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.internal.util.MockUtil; +import org.mockito.MockingDetails; +import org.mockito.Mockito; import org.springframework.boot.test.mock.mockito.example.SimpleExampleStringGenericService; import org.springframework.context.annotation.Bean; @@ -42,8 +43,10 @@ public class SpyBeanWithNameOnTestFieldForMultipleExistingBeansTests { @Test public void testSpying() throws Exception { - assertThat(MockUtil.isSpy(this.spy)).isTrue(); - assertThat(MockUtil.getMockName(this.spy).toString()).isEqualTo("two"); + MockingDetails mockingDetails = Mockito.mockingDetails(this.spy); + assertThat(mockingDetails.isMock()).isTrue(); + assertThat(mockingDetails.getMockCreationSettings().getMockName().toString()) + .isEqualTo("two"); } @Configuration