From 5baa71f1b3c83aa93c8deb7c52b439c774b9493a Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 18 Feb 2022 13:36:11 +0000 Subject: [PATCH 1/3] Allow beans with circular dependencies to be spied Closes gh-29639 --- .../mock/mockito/MockitoPostProcessor.java | 18 ++++- ...ExistingCircularBeansIntegrationTests.java | 80 +++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingCircularBeansIntegrationTests.java 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 22731ae3e7..c87a9e6a71 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2022 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. @@ -26,6 +26,7 @@ import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; import org.springframework.aop.scope.ScopedProxyUtils; import org.springframework.beans.BeansException; @@ -433,6 +434,8 @@ public class MockitoPostProcessor implements InstantiationAwareBeanPostProcessor private static final String BEAN_NAME = SpyPostProcessor.class.getName(); + private final Map earlySpyReferences = new ConcurrentHashMap<>(16); + private final MockitoPostProcessor mockitoPostProcessor; SpyPostProcessor(MockitoPostProcessor mockitoPostProcessor) { @@ -446,6 +449,10 @@ public class MockitoPostProcessor implements InstantiationAwareBeanPostProcessor @Override public Object getEarlyBeanReference(Object bean, String beanName) throws BeansException { + if (bean instanceof FactoryBean) { + return bean; + } + this.earlySpyReferences.put(getCacheKey(bean, beanName), bean); return this.mockitoPostProcessor.createSpyIfNecessary(bean, beanName); } @@ -454,7 +461,14 @@ public class MockitoPostProcessor implements InstantiationAwareBeanPostProcessor if (bean instanceof FactoryBean) { return bean; } - return this.mockitoPostProcessor.createSpyIfNecessary(bean, beanName); + if (this.earlySpyReferences.remove(getCacheKey(bean, beanName)) != bean) { + return this.mockitoPostProcessor.createSpyIfNecessary(bean, beanName); + } + return bean; + } + + private String getCacheKey(Object bean, String beanName) { + return StringUtils.hasLength(beanName) ? beanName : bean.getClass().getName(); } static void register(BeanDefinitionRegistry registry) { diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingCircularBeansIntegrationTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingCircularBeansIntegrationTests.java new file mode 100644 index 0000000000..f751e19e3f --- /dev/null +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/mock/mockito/SpyBeanOnTestFieldForExistingCircularBeansIntegrationTests.java @@ -0,0 +1,80 @@ +/* + * Copyright 2012-2022 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 + * + * https://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.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.SpyBeanOnTestFieldForExistingCircularBeansIntegrationTests.SpyBeanOnTestFieldForExistingCircularBeansConfig; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; + +import static org.mockito.BDDMockito.then; + +/** + * Test {@link SpyBean @SpyBean} on a test class field can be used to replace existing + * beans with circular dependencies. + * + * @author Andy Wilkinson + */ +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = SpyBeanOnTestFieldForExistingCircularBeansConfig.class) +class SpyBeanOnTestFieldForExistingCircularBeansIntegrationTests { + + @SpyBean + private One one; + + @Autowired + private Two two; + + @Test + void beanWithCircularDependenciesCanBeSpied() { + this.two.callOne(); + then(this.one).should().someMethod(); + } + + @Import({ One.class, Two.class }) + static class SpyBeanOnTestFieldForExistingCircularBeansConfig { + + } + + static class One { + + @Autowired + @SuppressWarnings("unused") + private Two two; + + void someMethod() { + + } + + } + + static class Two { + + @Autowired + private One one; + + void callOne() { + this.one.someMethod(); + } + + } + +} From c8466a14e97ff98d26cc5b2fc4e8d10d50001aef Mon Sep 17 00:00:00 2001 From: gcoppex Date: Thu, 17 Feb 2022 19:14:28 +0100 Subject: [PATCH 2/3] Clarify log message with a profile containing a comma See gh-29896 --- .../org/springframework/boot/SpringApplication.java | 10 ++++++++-- .../boot/SpringApplicationTests.java | 13 +++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index a705c250b7..29c06093fc 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -667,13 +667,19 @@ public class SpringApplication { Log log = getApplicationLog(); if (log.isInfoEnabled()) { String[] activeProfiles = context.getEnvironment().getActiveProfiles(); + for (int i = 0; i < activeProfiles.length; i++) { + activeProfiles[i] = "\"" + activeProfiles[i] + "\""; + } if (ObjectUtils.isEmpty(activeProfiles)) { String[] defaultProfiles = context.getEnvironment().getDefaultProfiles(); - log.info("No active profile set, falling back to default profiles: " + for (int i = 0; i < defaultProfiles.length; i++) { + defaultProfiles[i] = "\"" + defaultProfiles[i] + "\""; + } + log.info("No active profile set, falling back to " + defaultProfiles.length + " default profile(s): " + StringUtils.arrayToCommaDelimitedString(defaultProfiles)); } else { - log.info("The following profiles are active: " + log.info("The following " + activeProfiles.length + " profile(s) are active: " + StringUtils.arrayToCommaDelimitedString(activeProfiles)); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index 25c0c46f65..c03784fe46 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -253,7 +253,7 @@ class SpringApplicationTests { SpringApplication application = new SpringApplication(ExampleConfig.class); application.setWebApplicationType(WebApplicationType.NONE); this.context = application.run(); - assertThat(output).contains("No active profile set, falling back to default profiles: default"); + assertThat(output).contains("No active profile set, falling back to 1 default profile(s): \"default\""); } @Test @@ -261,7 +261,7 @@ class SpringApplicationTests { SpringApplication application = new SpringApplication(ExampleConfig.class); application.setWebApplicationType(WebApplicationType.NONE); this.context = application.run("--spring.profiles.active=myprofiles"); - assertThat(output).contains("The following profiles are active: myprofile"); + assertThat(output).contains("The following 1 profile(s) are active: \"myprofiles\""); } @Test @@ -273,6 +273,15 @@ class SpringApplicationTests { assertThat(output).contains("o.s.b.SpringApplication"); } + @Test + void logsMultipleActiveProfilesWithComma(CapturedOutput output) { + SpringApplication application = new SpringApplication(ExampleConfig.class); + application.setWebApplicationType(WebApplicationType.NONE); + application.setAdditionalProfiles("p1,p2", "p3"); + application.run(); + assertThat(output).contains("The following 2 profile(s) are active: \"p1,p2\",\"p3\""); + } + @Test void setIgnoreBeanInfoPropertyByDefault(CapturedOutput output) { SpringApplication application = new SpringApplication(ExampleConfig.class); From 92cd51e6b01fbf84751f6f6713db85263ce8e2af Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 21 Feb 2022 08:55:49 +0100 Subject: [PATCH 3/3] Polish "Clarify log message with a profile containing a comma" See gh-29896 --- .../boot/SpringApplication.java | 26 +++++++------- .../boot/SpringApplicationTests.java | 36 ++++++++++++------- .../ConfigFileApplicationListenerTests.java | 6 ++-- 3 files changed, 41 insertions(+), 27 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index 29c06093fc..643d724915 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.stream.Collectors; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -666,25 +667,26 @@ public class SpringApplication { protected void logStartupProfileInfo(ConfigurableApplicationContext context) { Log log = getApplicationLog(); if (log.isInfoEnabled()) { - String[] activeProfiles = context.getEnvironment().getActiveProfiles(); - for (int i = 0; i < activeProfiles.length; i++) { - activeProfiles[i] = "\"" + activeProfiles[i] + "\""; - } + List activeProfiles = quoteProfiles(context.getEnvironment().getActiveProfiles()); if (ObjectUtils.isEmpty(activeProfiles)) { - String[] defaultProfiles = context.getEnvironment().getDefaultProfiles(); - for (int i = 0; i < defaultProfiles.length; i++) { - defaultProfiles[i] = "\"" + defaultProfiles[i] + "\""; - } - log.info("No active profile set, falling back to " + defaultProfiles.length + " default profile(s): " - + StringUtils.arrayToCommaDelimitedString(defaultProfiles)); + List defaultProfiles = quoteProfiles(context.getEnvironment().getDefaultProfiles()); + String message = String.format("%s default %s: ", defaultProfiles.size(), + (defaultProfiles.size() <= 1) ? "profile" : "profiles"); + log.info("No active profile set, falling back to " + message + + StringUtils.collectionToDelimitedString(defaultProfiles, ", ")); } else { - log.info("The following " + activeProfiles.length + " profile(s) are active: " - + StringUtils.arrayToCommaDelimitedString(activeProfiles)); + String message = (activeProfiles.size() == 1) ? "1 profile is active: " + : activeProfiles.size() + " profiles are active: "; + log.info("The following " + message + StringUtils.collectionToDelimitedString(activeProfiles, ", ")); } } } + private List quoteProfiles(String[] profiles) { + return Arrays.stream(profiles).map((profile) -> "\"" + profile + "\"").collect(Collectors.toList()); + } + /** * Returns the {@link Log} for the application. By default will be deduced. * @return the application log diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index c03784fe46..6f1cb74228 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -249,37 +249,49 @@ class SpringApplicationTests { } @Test - void logsNoActiveProfiles(CapturedOutput output) { + void logsActiveProfilesWithoutProfileAndSingleDefault(CapturedOutput output) { SpringApplication application = new SpringApplication(ExampleConfig.class); application.setWebApplicationType(WebApplicationType.NONE); this.context = application.run(); - assertThat(output).contains("No active profile set, falling back to 1 default profile(s): \"default\""); + assertThat(output).contains("No active profile set, falling back to 1 default profile: \"default\""); } @Test - void logsActiveProfiles(CapturedOutput output) { + void logsActiveProfilesWithoutProfileAndMultipleDefaults(CapturedOutput output) { + MockEnvironment environment = new MockEnvironment(); + environment.setDefaultProfiles("p0,p1", "default"); SpringApplication application = new SpringApplication(ExampleConfig.class); application.setWebApplicationType(WebApplicationType.NONE); - this.context = application.run("--spring.profiles.active=myprofiles"); - assertThat(output).contains("The following 1 profile(s) are active: \"myprofiles\""); + application.setEnvironment(environment); + this.context = application.run(); + assertThat(output) + .contains("No active profile set, falling back to 2 default profiles: \"p0,p1\", \"default\""); } @Test - void enableBannerInLogViaProperty(CapturedOutput output) { - SpringApplication application = spy(new SpringApplication(ExampleConfig.class)); + void logsActiveProfilesWithSingleProfile(CapturedOutput output) { + SpringApplication application = new SpringApplication(ExampleConfig.class); application.setWebApplicationType(WebApplicationType.NONE); - this.context = application.run("--spring.main.banner-mode=log"); - then(application).should(atLeastOnce()).setBannerMode(Banner.Mode.LOG); - assertThat(output).contains("o.s.b.SpringApplication"); + this.context = application.run("--spring.profiles.active=myprofiles"); + assertThat(output).contains("The following 1 profile is active: \"myprofiles\""); } @Test - void logsMultipleActiveProfilesWithComma(CapturedOutput output) { + void logsActiveProfilesWithMultipleProfiles(CapturedOutput output) { SpringApplication application = new SpringApplication(ExampleConfig.class); application.setWebApplicationType(WebApplicationType.NONE); application.setAdditionalProfiles("p1,p2", "p3"); application.run(); - assertThat(output).contains("The following 2 profile(s) are active: \"p1,p2\",\"p3\""); + assertThat(output).contains("The following 2 profiles are active: \"p1,p2\", \"p3\""); + } + + @Test + void enableBannerInLogViaProperty(CapturedOutput output) { + SpringApplication application = spy(new SpringApplication(ExampleConfig.class)); + application.setWebApplicationType(WebApplicationType.NONE); + this.context = application.run("--spring.main.banner-mode=log"); + then(application).should(atLeastOnce()).setBannerMode(Banner.Mode.LOG); + assertThat(output).contains("o.s.b.SpringApplication"); } @Test diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java index 9040a27782..a5083d86c9 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2022 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. @@ -815,8 +815,8 @@ class ConfigFileApplicationListenerTests { assertThat(environment).has(matchingProfile("morespecific")); assertThat(environment).has(matchingProfile("yetmorespecific")); assertThat(environment).doesNotHave(matchingProfile("missing")); - assertThat(output) - .contains("The following profiles are active: includeprofile,specific,morespecific,yetmorespecific"); + assertThat(output).contains( + "The following 4 profiles are active: \"includeprofile\", \"specific\", \"morespecific\", \"yetmorespecific\""); } @Test