From 87547f208a248794ad0dc4fec5cf04c12894b19b Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 19 Dec 2016 15:58:25 -0800 Subject: [PATCH 1/4] Add test for @SpyBean for generic bean definition Add an integration test to check that @SpyBean can be used with on a bean definition that returns a generic type. Closes gh-7625 --- spring-boot-dependencies/pom.xml | 2 +- ...orExistingGenericBeanIntegrationTests.java | 72 +++++++++++++++++++ .../SimpleExampleIntegerGenericService.java | 32 +++++++++ .../SimpleExampleStringGenericService.java | 31 ++++++++ 4 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingGenericBeanIntegrationTests.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleIntegerGenericService.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java diff --git a/spring-boot-dependencies/pom.xml b/spring-boot-dependencies/pom.xml index d4837b98e0..9493ea7dc2 100644 --- a/spring-boot-dependencies/pom.xml +++ b/spring-boot-dependencies/pom.xml @@ -2664,4 +2664,4 @@ integration-test - \ No newline at end of file + diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingGenericBeanIntegrationTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingGenericBeanIntegrationTests.java new file mode 100644 index 0000000000..f6becdc74f --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingGenericBeanIntegrationTests.java @@ -0,0 +1,72 @@ +/* + * 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.boot.test.mock.mockito.example.ExampleGenericService; +import org.springframework.boot.test.mock.mockito.example.ExampleGenericServiceCaller; +import org.springframework.boot.test.mock.mockito.example.SimpleExampleIntegerGenericService; +import org.springframework.boot.test.mock.mockito.example.SimpleExampleStringGenericService; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; + +/** + * Test {@link SpyBean} on a test class field can be used to replace existing beans. + * + * @author Phillip Webb + * @see SpyBeanOnTestFieldForExistingBeanCacheIntegrationTests + */ +@RunWith(SpringRunner.class) +public class SpyBeanOnTestFieldForExistingGenericBeanIntegrationTests { + + // gh-7625 + + @SpyBean + private ExampleGenericService exampleService; + + @Autowired + private ExampleGenericServiceCaller caller; + + @Test + public void testSpying() throws Exception { + assertThat(this.caller.sayGreeting()).isEqualTo("I say 123 simple"); + verify(this.exampleService).greeting(); + } + + @Configuration + @Import({ ExampleGenericServiceCaller.class, + SimpleExampleIntegerGenericService.class }) + static class SpyBeanOnTestFieldForExistingBeanConfig { + + @Bean + public ExampleGenericService simpleExampleStringGenericService() { + // In order to trigger issue we need a method signature that returns the + // generic type not the actual implementation class + return new SimpleExampleStringGenericService(); + } + + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleIntegerGenericService.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleIntegerGenericService.java new file mode 100644 index 0000000000..fc2484891c --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleIntegerGenericService.java @@ -0,0 +1,32 @@ +/* + * 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.example; + +/** + * Example generic service implementation for spy tests. + * + * @author Phillip Webb + */ +public class SimpleExampleIntegerGenericService + implements ExampleGenericService { + + @Override + public Integer greeting() { + return 123; + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java new file mode 100644 index 0000000000..fa7a6b5840 --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java @@ -0,0 +1,31 @@ +/* + * 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.example; + +/** + * Example generic service implementation for spy tests. + * + * @author Phillip Webb + */ +public class SimpleExampleStringGenericService implements ExampleGenericService { + + @Override + public String greeting() { + return "simple"; + } + +} From 782417141395765cda94b3bd36c406b86910fd22 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 19 Dec 2016 18:14:51 -0800 Subject: [PATCH 2/4] Allow @SpyBean to work with @Primary beans Update `MockitoPostProcessor` so that `@SpyBean` will automatically pick the `@Primary` bean when multiple candidates exist. Fixes gh-7621 --- .../mock/mockito/MockitoPostProcessor.java | 46 +++++++++--- ...MultipleExistingBeansIntegrationTests.java | 73 +++++++++++++++++++ .../ExampleGenericStringServiceCaller.java | 41 +++++++++++ .../SimpleExampleStringGenericService.java | 12 ++- 4 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansIntegrationTests.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/ExampleGenericStringServiceCaller.java 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 94337fe101..6ff5666169 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 @@ -37,6 +37,8 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.BeanPostProcessor; @@ -250,7 +252,7 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda createSpy(registry, definition, field); } else { - registerSpies(definition, field, existingBeans); + registerSpies(registry, definition, field, existingBeans); } } @@ -307,15 +309,41 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda registerSpy(definition, field, beanName); } - private void registerSpies(SpyDefinition definition, Field field, - String[] existingBeans) { - Assert.state(field == null || existingBeans.length == 1, - "Unable to register spy bean " + definition.getTypeToSpy() - + " expected a single existing bean to replace but found " - + new TreeSet(Arrays.asList(existingBeans))); - for (String beanName : existingBeans) { - registerSpy(definition, field, beanName); + private void registerSpies(BeanDefinitionRegistry registry, SpyDefinition definition, + Field field, String[] existingBeans) { + ResolvableType type = definition.getTypeToSpy(); + try { + if (ObjectUtils.isEmpty(existingBeans)) { + throw new NoSuchBeanDefinitionException(type); + } + if (existingBeans.length > 1) { + existingBeans = new String[] { + determinePrimaryCandidate(registry, existingBeans, type) }; + } + registerSpy(definition, field, existingBeans[0]); + } + catch (RuntimeException ex) { + throw new IllegalStateException( + "Unable to register spy bean " + definition.getTypeToSpy(), ex); + } + } + + private String determinePrimaryCandidate(BeanDefinitionRegistry registry, + String[] candidateBeanNames, ResolvableType type) { + String primaryBeanName = null; + for (String candidateBeanName : candidateBeanNames) { + BeanDefinition beanDefinition = registry.getBeanDefinition(candidateBeanName); + if (beanDefinition.isPrimary()) { + if (primaryBeanName != null) { + throw new NoUniqueBeanDefinitionException(type.resolve(), + candidateBeanNames.length, + "more than one 'primary' bean found among candidates: " + + Arrays.asList(candidateBeanNames)); + } + primaryBeanName = candidateBeanName; + } } + return primaryBeanName; } private void registerSpy(SpyDefinition definition, Field field, String beanName) { diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansIntegrationTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansIntegrationTests.java new file mode 100644 index 0000000000..c4880c533d --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForMultipleExistingBeansIntegrationTests.java @@ -0,0 +1,73 @@ +/* + * 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.mockito.internal.util.MockUtil; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.example.ExampleGenericStringServiceCaller; +import org.springframework.boot.test.mock.mockito.example.SimpleExampleStringGenericService; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Primary; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; + +/** + * Test {@link SpyBean} on a test class field can be used to inject new spy instances. + * + * @author Phillip Webb + */ +@RunWith(SpringRunner.class) +public class SpyBeanOnTestFieldForMultipleExistingBeansIntegrationTests { + + @SpyBean + private SimpleExampleStringGenericService spy; + + @Autowired + private ExampleGenericStringServiceCaller caller; + + @Test + public void testSpying() throws Exception { + assertThat(this.caller.sayGreeting()).isEqualTo("I say two"); + assertThat(new MockUtil().getMockName(this.spy).toString()).isEqualTo("two"); + verify(this.spy).greeting(); + } + + @Configuration + @Import(ExampleGenericStringServiceCaller.class) + static class Config { + + @Bean + public SimpleExampleStringGenericService one() { + return new SimpleExampleStringGenericService("one"); + } + + @Bean + @Primary + public SimpleExampleStringGenericService two() { + return new SimpleExampleStringGenericService("two"); + } + + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/ExampleGenericStringServiceCaller.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/ExampleGenericStringServiceCaller.java new file mode 100644 index 0000000000..08df288575 --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/ExampleGenericStringServiceCaller.java @@ -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.example; + +/** + * Example bean for mocking tests that calls {@link ExampleGenericService}. + * + * @author Phillip Webb + */ +public class ExampleGenericStringServiceCaller { + + private final ExampleGenericService stringService; + + public ExampleGenericStringServiceCaller( + ExampleGenericService stringService) { + this.stringService = stringService; + } + + public ExampleGenericService getStringService() { + return this.stringService; + } + + public String sayGreeting() { + return "I say " + this.stringService.greeting(); + } + +} diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java index fa7a6b5840..ce2e721c60 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/example/SimpleExampleStringGenericService.java @@ -23,9 +23,19 @@ package org.springframework.boot.test.mock.mockito.example; */ public class SimpleExampleStringGenericService implements ExampleGenericService { + private final String greeting; + + public SimpleExampleStringGenericService() { + this("simple"); + } + + public SimpleExampleStringGenericService(String greeting) { + this.greeting = greeting; + } + @Override public String greeting() { - return "simple"; + return this.greeting; } } From f36ed673fd7216026244cd2d0f19600f67ea27bd Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 19 Dec 2016 18:28:39 -0800 Subject: [PATCH 3/4] Make Logback 1.1.8 upgrade back compatible Update the fix committed in a2d46dc16c to fallback to reflection based invocation in case earlier versions of Logback are being used. Closes gh-7638 --- .../logback/DefaultLogbackConfiguration.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java b/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java index 221cfa0727..d6c4921154 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java @@ -16,6 +16,7 @@ package org.springframework.boot.logging.logback; +import java.lang.reflect.Method; import java.nio.charset.Charset; import ch.qos.logback.classic.Level; @@ -35,6 +36,7 @@ import org.springframework.boot.logging.LoggingInitializationContext; import org.springframework.core.env.Environment; import org.springframework.core.env.PropertyResolver; import org.springframework.core.env.PropertySourcesPropertyResolver; +import org.springframework.util.ReflectionUtils; /** * Default logback configuration used by Spring Boot. Uses {@link LogbackConfigurator} to @@ -133,22 +135,36 @@ class DefaultLogbackConfiguration { encoder.setPattern(OptionHelper.substVars(logPattern, config.getContext())); appender.setEncoder(encoder); config.start(encoder); - appender.setFile(logFile); + getRollingPolicy(appender, config, logFile); + getMaxFileSize(appender, config); + config.appender("FILE", appender); + return appender; + } + private void getRollingPolicy(RollingFileAppender appender, + LogbackConfigurator config, String logFile) { FixedWindowRollingPolicy rollingPolicy = new FixedWindowRollingPolicy(); rollingPolicy.setFileNamePattern(logFile + ".%i"); appender.setRollingPolicy(rollingPolicy); rollingPolicy.setParent(appender); config.start(rollingPolicy); + } + private void getMaxFileSize(RollingFileAppender appender, + LogbackConfigurator config) { SizeBasedTriggeringPolicy triggeringPolicy = new SizeBasedTriggeringPolicy(); - triggeringPolicy.setMaxFileSize(FileSize.valueOf("10MB")); + try { + triggeringPolicy.setMaxFileSize(FileSize.valueOf("10MB")); + } + catch (NoSuchMethodError ex) { + // Logback < 1.1.8 used String configuration + Method method = ReflectionUtils.findMethod(SizeBasedTriggeringPolicy.class, + "setMaxFileSize", String.class); + ReflectionUtils.invokeMethod(method, triggeringPolicy, "10MB"); + } appender.setTriggeringPolicy(triggeringPolicy); config.start(triggeringPolicy); - - config.appender("FILE", appender); - return appender; } } From 2ea4d4b1d9d17a6a62f3db6246040eff99caac06 Mon Sep 17 00:00:00 2001 From: Chaouki Dhib Date: Tue, 13 Dec 2016 23:58:28 +0100 Subject: [PATCH 4/4] Broaden LoggingApplicationListener ignores Update `LoggingApplicationListener` to ignore all `-D` prefixed property values. As well as catching the Azure use-case, this update now means Spring Boot application can start when Tomcat is missing `CATALINA_BASE\conf\logging.properties` and sets the value `-Dnop`. Closes gh-7639 --- .../boot/logging/LoggingApplicationListener.java | 7 +------ .../logging/LoggingApplicationListenerTests.java | 12 ++++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java index 01ac287e04..3a6fdbe53e 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java @@ -314,12 +314,7 @@ public class LoggingApplicationListener implements GenericApplicationListener { } private boolean ignoreLogConfig(String logConfig) { - return !StringUtils.hasLength(logConfig) - || isDefaultAzureLoggingConfig(logConfig); - } - - private boolean isDefaultAzureLoggingConfig(String candidate) { - return candidate.startsWith("-Djava.util.logging.config.file="); + return !StringUtils.hasLength(logConfig) || logConfig.startsWith("-D"); } private void initializeFinalLoggingLevels(ConfigurableEnvironment environment, diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java index 6378cbee5d..92536c1f27 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java @@ -162,6 +162,18 @@ public class LoggingApplicationListenerTests { assertThat(new File(tmpDir() + "/spring.log").exists()).isFalse(); } + @Test + public void tomcatNopLoggingConfigDoesNotCauseAFailure() throws Exception { + TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.context, + "logging.config: -Dnop"); + this.initializer.initialize(this.context.getEnvironment(), + this.context.getClassLoader()); + this.logger.info("Hello world"); + String output = this.outputCapture.toString().trim(); + assertThat(output).contains("Hello world").doesNotContain("???"); + assertThat(new File(tmpDir() + "/spring.log").exists()).isFalse(); + } + @Test public void overrideConfigBroken() throws Exception { TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.context,