From b4c2c14d684554ffcd7d101f93aba77f5c6ef583 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 22 Jan 2016 13:04:23 +0000 Subject: [PATCH] Further paring down of leaky proeprty sources in Bootstrap phase The biggest problem addressed here is one where an EnvironmentPostProcessor (reasonably) adds entries to the defaultProperties in the bootstrap context, but then that property source is not merged with the parent, or is merged too late (because it only happens when the application context parent is set). The result would be that things that were activated during bootstrap would be not be activated in the main context, or would be activated too late (early enough for beans to bind to but not for other listeners and post processors to get access to the additional properties). See https://github.com/spring-cloud/spring-cloud-sleuth/issues/126 --- .../BootstrapApplicationListener.java | 41 ++++++++++++++++++- .../config/BootstrapConfigurationTests.java | 4 +- 2 files changed, 41 insertions(+), 4 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 24163a11..8cf84804 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 @@ -108,8 +108,9 @@ public class BootstrapApplicationListener // TODO: is it possible or sensible to share a ResourceLoader? SpringApplicationBuilder builder = new SpringApplicationBuilder() .profiles(environment.getActiveProfiles()).bannerMode(Mode.OFF) - .environment(bootstrapEnvironment).registerShutdownHook(false) - .properties("spring.application.name:" + configName).web(false); + .environment(bootstrapEnvironment) + .properties("spring.application.name:" + configName) + .registerShutdownHook(false).web(false); List> sources = new ArrayList<>(); for (String name : names) { Class cls = ClassUtils.resolveClassName(name, null); @@ -129,9 +130,45 @@ public class BootstrapApplicationListener // It only has properties in it now that we don't want in the parent so remove // it (and it will be added back later) bootstrapProperties.remove(BOOTSTRAP_PROPERTY_SOURCE_NAME); + mergeDefaultProperties(environment.getPropertySources(), bootstrapProperties); return context; } + private void mergeDefaultProperties(MutablePropertySources environment, + MutablePropertySources bootstrap) { + String name = "defaultProperties"; + if (!bootstrap.contains(name)) { + return; + } + PropertySource source = bootstrap.get(name); + if (source instanceof MapPropertySource) { + Map map = ((MapPropertySource) source).getSource(); + // The application name is "bootstrap" (by default) at this point and + // we don't want that to appear in the parent context at all. + map.remove("spring.application.name"); + } + if (!environment.contains(name)) { + environment.addLast(source); + } + else { + PropertySource target = environment.get(name); + if (target instanceof MapPropertySource) { + Map targetMap = ((MapPropertySource) target).getSource(); + if (target == source) { + return; + } + if (source instanceof MapPropertySource) { + Map map = ((MapPropertySource) source).getSource(); + for (String key : map.keySet()) { + if (!target.containsProperty(key)) { + targetMap.put(key, map.get(key)); + } + } + } + } + } + } + private void addAncestorInitializer(SpringApplication application, ConfigurableApplicationContext context) { boolean installed = false; 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 da4956ea..93bcd7a8 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 @@ -212,9 +212,9 @@ public class BootstrapConfigurationTests { .sources(BareConfiguration.class).run(); assertEquals("main", this.context.getEnvironment().getProperty("spring.application.name")); - // The parent is called "application" because spring.application.name is not + // The parent has no name because spring.application.name is not // defined in the bootstrap properties - assertEquals("application", this.context.getParent().getEnvironment() + assertEquals(null, this.context.getParent().getEnvironment() .getProperty("spring.application.name")); }