From 70d601df84bbe2cbc5b6a1522e741b5d9d60a6b8 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 10 Mar 2017 09:30:59 +0000 Subject: [PATCH 1/2] Register a BeanDefinition manually instead of using @Bean For RefreshScope this stops Spring from moaning about the @Bean method with a WARN log (which is benign but hard to get rid of and confusing for users). See gh-401 --- .../RefreshAutoConfiguration.java | 29 ++++++++++-- .../LifecycleMvcAutoConfigurationTests.java | 1 + .../RefreshAutoConfigurationTests.java | 45 +++++++++++++++++++ .../RefreshScopeLazyIntegrationTests.java | 5 ++- 4 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshAutoConfiguration.java index 9b0c94f3..6ee17ede 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshAutoConfiguration.java @@ -17,6 +17,12 @@ package org.springframework.cloud.autoconfigure; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -29,6 +35,7 @@ import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.stereotype.Component; /** * Autoconfiguration for the refresh scope and associated features to do with changes in @@ -42,10 +49,24 @@ import org.springframework.core.env.ConfigurableEnvironment; @AutoConfigureAfter(WebMvcAutoConfiguration.class) public class RefreshAutoConfiguration { - @Bean - @ConditionalOnMissingBean - public static RefreshScope refreshScope() { - return new RefreshScope(); + @Component + @ConditionalOnMissingBean(RefreshScope.class) + protected static class RefreshScopeConfiguration + implements BeanDefinitionRegistryPostProcessor { + + @Override + public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) + throws BeansException { + } + + @Override + public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) + throws BeansException { + registry.registerBeanDefinition("refreshScope", + BeanDefinitionBuilder.genericBeanDefinition(RefreshScope.class) + .setRole(BeanDefinition.ROLE_INFRASTRUCTURE) + .getBeanDefinition()); + } } @Bean diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java index 78270451..703a4782 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java @@ -1,6 +1,7 @@ package org.springframework.cloud.autoconfigure; import org.junit.Test; + import org.springframework.boot.actuate.endpoint.mvc.EndpointMvcAdapter; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.builder.SpringApplicationBuilder; diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java new file mode 100644 index 00000000..e3f54646 --- /dev/null +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java @@ -0,0 +1,45 @@ +package org.springframework.cloud.autoconfigure; + +import org.junit.Rule; +import org.junit.Test; + +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.test.OutputCapture; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Configuration; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +/** + * @author Dave Syer + */ +public class RefreshAutoConfigurationTests { + + @Rule + public OutputCapture output = new OutputCapture(); + + @Test + public void noWarnings() { + try (ConfigurableApplicationContext context = getApplicationContext( + Config.class)) { + assertTrue(context.containsBean("refreshScope")); + assertThat(output.toString(), not(containsString("WARN"))); + } + } + + private static ConfigurableApplicationContext getApplicationContext( + Class configuration, String... properties) { + return new SpringApplicationBuilder(configuration).web(false) + .properties(properties).run(); + } + + @Configuration + @EnableAutoConfiguration + static class Config { + + } +} diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeLazyIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeLazyIntegrationTests.java index 27747bcf..ffcc7f45 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeLazyIntegrationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeLazyIntegrationTests.java @@ -21,11 +21,13 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; + import org.springframework.aop.framework.Advised; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; +import org.springframework.boot.autoconfigure.test.ImportAutoConfiguration; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.SpringApplicationConfiguration; @@ -36,7 +38,6 @@ import org.springframework.cloud.context.scope.refresh.RefreshScopeLazyIntegrati import org.springframework.context.ApplicationListener; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Import; import org.springframework.jmx.export.annotation.ManagedAttribute; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.test.annotation.DirtiesContext; @@ -198,7 +199,7 @@ public class RefreshScopeLazyIntegrationTests { @Configuration @EnableConfigurationProperties(TestProperties.class) - @Import({ RefreshAutoConfiguration.class, + @ImportAutoConfiguration({ RefreshAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class }) protected static class TestConfiguration { From ba281228f6dab5defe3129c343703c52b32f657f Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 15 Mar 2017 11:27:52 +0000 Subject: [PATCH 2/2] Apply bootstrap initializers to all contexts in a hierarchy The fix for gh-153 was sensible, but has now exposed another issue which is that the bootstrap initializers, whilst they are now only created once, actually only get applied to the parent context in a hierarchy. Most of the time this doesn't matter because the child contexts all benefit from environment changes and additional beans added to the parent. If the children are supposed to actually have different environments, though, there is a problem. So this change locates the existing bootstrap context and applies it to each new context created separately. Unfortunately we have to use reflection to do that for now because there is no way to discover the parent with a public API before the context is refreshed. See gh-190 --- .../BootstrapApplicationListener.java | 46 ++++++++++++++++--- .../bootstrap/TestBootstrapConfiguration.java | 26 +++++++++++ .../config/BootstrapConfigurationTests.java | 36 ++++++++++++++- 3 files changed, 99 insertions(+), 9 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java index 648c5361..b67846d8 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java @@ -16,6 +16,7 @@ package org.springframework.cloud.bootstrap; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -46,6 +47,7 @@ import org.springframework.core.env.StandardEnvironment; import org.springframework.core.env.SystemEnvironmentPropertySource; import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -81,26 +83,56 @@ public class BootstrapApplicationListener if (environment.getPropertySources().contains(BOOTSTRAP_PROPERTY_SOURCE_NAME)) { return; } - for (ApplicationContextInitializer initializer : event.getSpringApplication().getInitializers()) { + ConfigurableApplicationContext context = null; + String configName = environment + .resolvePlaceholders("${spring.cloud.bootstrap.name:bootstrap}"); + for (ApplicationContextInitializer initializer : event.getSpringApplication() + .getInitializers()) { if (initializer instanceof ParentContextApplicationContextInitializer) { - return; + context = findBootstrapContext( + (ParentContextApplicationContextInitializer) initializer, + configName); } } - ConfigurableApplicationContext context = bootstrapServiceContext(environment, - event.getSpringApplication()); + if (context == null) { + context = bootstrapServiceContext(environment, event.getSpringApplication(), + configName); + } apply(context, event.getSpringApplication(), environment); } + private ConfigurableApplicationContext findBootstrapContext( + ParentContextApplicationContextInitializer initializer, String configName) { + Field field = ReflectionUtils + .findField(ParentContextApplicationContextInitializer.class, "parent"); + ReflectionUtils.makeAccessible(field); + ConfigurableApplicationContext parent = safeCast( + ConfigurableApplicationContext.class, + ReflectionUtils.getField(field, initializer)); + if (parent != null && !configName.equals(parent.getId())) { + parent = safeCast(ConfigurableApplicationContext.class, parent.getParent()); + } + return parent; + } + + private T safeCast(Class type, Object object) { + try { + return type.cast(object); + } + catch (ClassCastException e) { + return null; + } + } + private ConfigurableApplicationContext bootstrapServiceContext( - ConfigurableEnvironment environment, final SpringApplication application) { + ConfigurableEnvironment environment, final SpringApplication application, + String configName) { StandardEnvironment bootstrapEnvironment = new StandardEnvironment(); MutablePropertySources bootstrapProperties = bootstrapEnvironment .getPropertySources(); for (PropertySource source : bootstrapProperties) { bootstrapProperties.remove(source.getName()); } - String configName = environment - .resolvePlaceholders("${spring.cloud.bootstrap.name:bootstrap}"); String configLocation = environment .resolvePlaceholders("${spring.cloud.bootstrap.location:}"); Map bootstrapMap = new HashMap<>(); diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/TestBootstrapConfiguration.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/TestBootstrapConfiguration.java index 798db7b8..7265a3a2 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/TestBootstrapConfiguration.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/TestBootstrapConfiguration.java @@ -1,6 +1,14 @@ package org.springframework.cloud.bootstrap; +import java.util.Collections; + +import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; import static org.springframework.cloud.bootstrap.TestHigherPriorityBootstrapConfiguration.firstToBeCreated; @@ -8,10 +16,28 @@ import static org.springframework.cloud.bootstrap.TestHigherPriorityBootstrapCon * @author Spencer Gibb */ @Order(0) +@Configuration public class TestBootstrapConfiguration { public TestBootstrapConfiguration() { firstToBeCreated.compareAndSet(null, TestBootstrapConfiguration.class); } + @Bean + public ApplicationContextInitializer customInitializer() { + return new ApplicationContextInitializer() { + + @Override + public void initialize(ConfigurableApplicationContext applicationContext) { + ConfigurableEnvironment environment = applicationContext.getEnvironment(); + environment.getPropertySources() + .addLast(new MapPropertySource("customProperties", + Collections.singletonMap("custom.foo", + environment.resolvePlaceholders( + "${spring.application.name:bar}")))); + } + + }; + } + } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/config/BootstrapConfigurationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/config/BootstrapConfigurationTests.java index 87ad1f3a..f6f8ce33 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/config/BootstrapConfigurationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/config/BootstrapConfigurationTests.java @@ -25,6 +25,7 @@ import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; + import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -54,6 +55,8 @@ public class BootstrapConfigurationTests { private ConfigurableApplicationContext context; + private ConfigurableApplicationContext sibling; + @Rule public ExpectedException expected = ExpectedException.none(); @@ -68,6 +71,9 @@ public class BootstrapConfigurationTests { if (this.context != null) { this.context.close(); } + if (this.sibling != null) { + this.sibling.close(); + } } @Test @@ -197,7 +203,7 @@ public class BootstrapConfigurationTests { PropertySourceConfiguration.MAP.put("spring.cloud.config.allowOverride", "true"); ConfigurableEnvironment environment = new StandardEnvironment(); environment.getPropertySources().addLast(new MapPropertySource("last", - Collections. singletonMap("bootstrap.foo", "splat"))); + Collections.singletonMap("bootstrap.foo", "splat"))); this.context = new SpringApplicationBuilder().web(false).environment(environment) .sources(BareConfiguration.class).run(); assertEquals("splat", this.context.getEnvironment().getProperty("bootstrap.foo")); @@ -282,6 +288,32 @@ public class BootstrapConfigurationTests { assertNotNull(context.getParent()); assertEquals("bootstrap", context.getParent().getParent().getId()); assertNull(context.getParent().getParent().getParent()); + assertEquals("bar", context.getEnvironment().getProperty("custom.foo")); + } + + @Test + public void bootstrapContextSharedBySiblings() { + TestHigherPriorityBootstrapConfiguration.count.set(0); + PropertySourceConfiguration.MAP.put("bootstrap.foo", "bar"); + SpringApplicationBuilder builder = new SpringApplicationBuilder() + .sources(BareConfiguration.class); + this.sibling = builder.child(BareConfiguration.class) + .properties("spring.application.name=sibling").web(false).run(); + this.context = builder.child(BareConfiguration.class) + .properties("spring.application.name=context").web(false).run(); + assertEquals(1, TestHigherPriorityBootstrapConfiguration.count.get()); + assertNotNull(context.getParent()); + assertEquals("bootstrap", context.getParent().getParent().getId()); + assertNull(context.getParent().getParent().getParent()); + assertEquals("context", context.getEnvironment().getProperty("custom.foo")); + assertEquals("context", + context.getEnvironment().getProperty("spring.application.name")); + assertNotNull(sibling.getParent()); + assertEquals("bootstrap", sibling.getParent().getParent().getId()); + assertNull(sibling.getParent().getParent().getParent()); + assertEquals("sibling", sibling.getEnvironment().getProperty("custom.foo")); + assertEquals("sibling", + sibling.getEnvironment().getProperty("spring.application.name")); } @Test @@ -340,7 +372,7 @@ public class BootstrapConfigurationTests { protected static class PropertySourceConfiguration implements PropertySourceLocator { public static Map MAP = new HashMap( - Collections. singletonMap("bootstrap.foo", "bar")); + Collections.singletonMap("bootstrap.foo", "bar")); private String name;