From 763b6c1711d281f4b5f8555a25e886796325a6b5 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 13 Dec 2019 12:01:23 -0500 Subject: [PATCH] Add each PropertySource from PropertySourceLocators individually (#652) * Add each PropertySource from PropertySourceLocators individually instead of as a CompositePropertySource. Fixes #611 --- .../PropertySourceBootstrapConfiguration.java | 94 +++++++++++---- .../config/PropertySourceLocator.java | 15 +++ .../config/BootstrapConfigurationTests.java | 114 ++++++++++++++++-- .../refresh/ContextRefresherTests.java | 5 +- .../src/test/resources/bootstrap.properties | 2 +- 5 files changed, 194 insertions(+), 36 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java index 355f068a..01a52250 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java @@ -17,8 +17,10 @@ package org.springframework.cloud.bootstrap.config; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -45,6 +47,7 @@ import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.Environment; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; @@ -52,6 +55,9 @@ import org.springframework.core.env.StandardEnvironment; import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; +import static org.springframework.cloud.bootstrap.config.PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME; +import static org.springframework.core.env.StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME; + /** * @author Dave Syer * @@ -87,27 +93,31 @@ public class PropertySourceBootstrapConfiguration implements @Override public void initialize(ConfigurableApplicationContext applicationContext) { - CompositePropertySource composite = new CompositePropertySource( - BOOTSTRAP_PROPERTY_SOURCE_NAME); + List> composite = new ArrayList<>(); AnnotationAwareOrderComparator.sort(this.propertySourceLocators); boolean empty = true; ConfigurableEnvironment environment = applicationContext.getEnvironment(); for (PropertySourceLocator locator : this.propertySourceLocators) { - PropertySource source = null; - source = locator.locate(environment); + Collection> source = locator.locateCollection(environment); if (source == null) { continue; } - logger.info("Located property source: " + source); - composite.addPropertySource(source); + List sourceList = new ArrayList<>(); + for (PropertySource p : source) { + sourceList.add(new BootstrapPropertySource(p)); + } + logger.info("Located property source: " + sourceList); + composite.addAll(sourceList); empty = false; } if (!empty) { MutablePropertySources propertySources = environment.getPropertySources(); String logConfig = environment.resolvePlaceholders("${logging.config:}"); LogFile logFile = LogFile.get(environment); - if (propertySources.contains(BOOTSTRAP_PROPERTY_SOURCE_NAME)) { - propertySources.remove(BOOTSTRAP_PROPERTY_SOURCE_NAME); + for (PropertySource p : environment.getPropertySources()) { + if (p.getName().startsWith(BOOTSTRAP_PROPERTY_SOURCE_NAME)) { + propertySources.remove(p.getName()); + } } insertPropertySources(propertySources, composite); reinitializeLoggingSystem(environment, logConfig, logFile); @@ -154,36 +164,49 @@ public class PropertySourceBootstrapConfiguration implements } private void insertPropertySources(MutablePropertySources propertySources, - CompositePropertySource composite) { + List> composite) { MutablePropertySources incoming = new MutablePropertySources(); - incoming.addFirst(composite); + List> reversedComposite = new ArrayList(composite); + // Reverse the list so that when we call addFirst below we are maintaining the + // same order of PropertySournces + // Wherever we call addLast we can use the order in the List since the first item + // will end up before the rest + Collections.reverse(reversedComposite); + for (PropertySource p : reversedComposite) { + incoming.addFirst(p); + } PropertySourceBootstrapProperties remoteProperties = new PropertySourceBootstrapProperties(); Binder.get(environment(incoming)).bind("spring.cloud.config", Bindable.ofInstance(remoteProperties)); if (!remoteProperties.isAllowOverride() || (!remoteProperties.isOverrideNone() && remoteProperties.isOverrideSystemProperties())) { - propertySources.addFirst(composite); + for (PropertySource p : reversedComposite) { + propertySources.addFirst(p); + } return; } if (remoteProperties.isOverrideNone()) { - propertySources.addLast(composite); + for (PropertySource p : composite) { + propertySources.addLast(p); + } return; } - if (propertySources - .contains(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) { + if (propertySources.contains(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) { if (!remoteProperties.isOverrideSystemProperties()) { - propertySources.addAfter( - StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, - composite); + for (PropertySource p : composite) { + propertySources.addAfter(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, p); + } } else { - propertySources.addBefore( - StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, - composite); + for (PropertySource p : reversedComposite) { + propertySources.addBefore(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, p); + } } } else { - propertySources.addLast(composite); + for (PropertySource p : composite) { + propertySources.addLast(p); + } } } @@ -241,3 +264,32 @@ public class PropertySourceBootstrapConfiguration implements } } + +class BootstrapPropertySource extends EnumerablePropertySource { + + private PropertySource p; + + BootstrapPropertySource(PropertySource p) { + super(BOOTSTRAP_PROPERTY_SOURCE_NAME + "-" + p.getName(), p.getSource()); + this.p = p; + } + + @Override + public Object getProperty(String name) { + return p.getProperty(name); + } + + @Override + public String[] getPropertyNames() { + Set names = new LinkedHashSet<>(); + if (!(this.p instanceof EnumerablePropertySource)) { + throw new IllegalStateException( + "Failed to enumerate property names due to non-enumerable property source: " + + p); + } + names.addAll(Arrays.asList(((EnumerablePropertySource) p).getPropertyNames())); + + return StringUtils.toStringArray(names); + } + +} diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java index 537b9916..24332e1e 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java @@ -16,6 +16,11 @@ package org.springframework.cloud.bootstrap.config; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.Environment; import org.springframework.core.env.PropertySource; @@ -36,4 +41,14 @@ public interface PropertySourceLocator { */ PropertySource locate(Environment environment); + default Collection> locateCollection(Environment environment) { + PropertySource propertySource = locate(environment); + if (CompositePropertySource.class.isInstance(propertySource)) { + return ((CompositePropertySource) propertySource).getPropertySources(); + } + else { + return (List) Arrays.asList(propertySource); + } + } + } 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 8cec3546..97a385a7 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 @@ -18,6 +18,7 @@ package org.springframework.cloud.bootstrap.config; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.junit.After; @@ -29,10 +30,13 @@ import org.springframework.boot.WebApplicationType; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.context.properties.bind.Bindable; +import org.springframework.boot.context.properties.bind.Binder; import org.springframework.cloud.bootstrap.TestHigherPriorityBootstrapConfiguration; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; import org.springframework.core.env.MapPropertySource; @@ -63,6 +67,8 @@ public class BootstrapConfigurationTests { // Used to test system properties override System.clearProperty("bootstrap.foo"); PropertySourceConfiguration.MAP.clear(); + CompositePropertySourceConfiguration.MAP1.clear(); + CompositePropertySourceConfiguration.MAP2.clear(); if (this.context != null) { this.context.close(); } @@ -82,8 +88,8 @@ public class BootstrapConfigurationTests { then(this.context.getEnvironment().getProperty("info.name")) .isEqualTo("externalPropertiesInfoName"); then(this.context.getEnvironment().getPropertySources().contains( - PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME)) - .isTrue(); + PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-testBootstrap")).isTrue(); } @Test @@ -101,8 +107,8 @@ public class BootstrapConfigurationTests { }) .run(); then(this.context.getEnvironment().getPropertySources().contains( - PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME)) - .isTrue(); + PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-testBootstrap")).isTrue(); } /** @@ -122,8 +128,8 @@ public class BootstrapConfigurationTests { .sources(BareConfiguration.class).run(); then(this.context.getEnvironment().getProperty("bootstrap.foo")).isEqualTo("bar"); then(this.context.getEnvironment().getPropertySources().contains( - PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME)) - .isTrue(); + PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-testBootstrap")).isTrue(); } @Test @@ -262,7 +268,8 @@ public class BootstrapConfigurationTests { MutablePropertySources sources = this.context.getEnvironment() .getPropertySources(); PropertySource bootstrap = sources - .get(PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME); + .get(PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-testBootstrap"); then(bootstrap).isNotNull(); then(sources.precedenceOf(bootstrap)).isEqualTo(0); } @@ -280,6 +287,17 @@ public class BootstrapConfigurationTests { then(this.context.getEnvironment().getProperty("custom.foo")).isEqualTo("bar"); } + @Test + public void listOverride() { + this.context = new SpringApplicationBuilder().sources(BareConfiguration.class) + .child(BareConfiguration.class).web(WebApplicationType.NONE).run(); + ListProperties listProperties = new ListProperties(); + Binder.get(this.context.getEnvironment()).bind("list", + Bindable.ofInstance(listProperties)); + then(listProperties.getFoo().size()).isEqualTo(1); + then(listProperties.getFoo().get(0)).isEqualTo("hello world"); + } + @Test public void bootstrapContextSharedBySiblings() { TestHigherPriorityBootstrapConfiguration.count.set(0); @@ -318,12 +336,12 @@ public class BootstrapConfigurationTests { then(this.context.getParent().getEnvironment()) .isNotSameAs(this.context.getEnvironment()); then(this.context.getEnvironment().getPropertySources().contains( - PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME)) - .isTrue(); + PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-testBootstrap")).isTrue(); then(((ConfigurableEnvironment) this.context.getParent().getEnvironment()) .getPropertySources().contains( - PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME)) - .isTrue(); + PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-testBootstrap")).isTrue(); } @Test @@ -347,8 +365,8 @@ public class BootstrapConfigurationTests { .isTrue(); then(((ConfigurableEnvironment) this.context.getParent().getEnvironment()) .getPropertySources().contains( - PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME)) - .isTrue(); + PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-testBootstrap")).isTrue(); then(this.context.getEnvironment().getProperty("bootstrap.foo")).isEqualTo("bar"); // The "bootstrap" property source is not shared now, but it has the same // properties in it because they are pulled from the PropertySourceConfiguration @@ -427,4 +445,74 @@ public class BootstrapConfigurationTests { } + @Configuration + @ConfigurationProperties("compositeexpected") + // This is added to bootstrap context as a source in bootstrap.properties + protected static class CompositePropertySourceConfiguration + implements PropertySourceLocator { + + public static Map MAP1 = new HashMap(); + + public static Map MAP2 = new HashMap(); + + public CompositePropertySourceConfiguration() { + MAP1.put("list.foo[0]", "hello"); + MAP1.put("list.food[1]", "world"); + MAP2.put("list.foo[0]", "hello world"); + } + + private String name; + + private boolean fail = false; + + @Override + public PropertySource locate(Environment environment) { + if (this.name != null) { + then(this.name) + .isEqualTo(environment.getProperty("spring.application.name")); + } + if (this.fail) { + throw new RuntimeException("Planned"); + } + CompositePropertySource compositePropertySource = new CompositePropertySource( + "listTestBootstrap"); + compositePropertySource.addFirstPropertySource( + new MapPropertySource("testBootstrap1", MAP1)); + compositePropertySource.addFirstPropertySource( + new MapPropertySource("testBootstrap2", MAP2)); + return compositePropertySource; + } + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + + public boolean isFail() { + return this.fail; + } + + public void setFail(boolean fail) { + this.fail = fail; + } + + } + + protected static class ListProperties { + + private List foo; + + public List getFoo() { + return foo; + } + + public void setFoo(List foo) { + this.foo = foo; + } + + } + } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java index 9b74838d..83382797 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java @@ -31,6 +31,7 @@ import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.test.util.TestPropertyValues; import org.springframework.boot.test.util.TestPropertyValues.Type; import org.springframework.cloud.bootstrap.TestBootstrapConfiguration; +import org.springframework.cloud.bootstrap.config.PropertySourceBootstrapConfiguration; import org.springframework.cloud.bootstrap.config.PropertySourceLocator; import org.springframework.cloud.context.scope.refresh.RefreshScope; import org.springframework.context.ConfigurableApplicationContext; @@ -90,7 +91,9 @@ public class ContextRefresherTests { .applyTo(context.getEnvironment(), Type.MAP, "defaultProperties"); refresher.refresh(); names = names(context.getEnvironment().getPropertySources()); - then(names).first().isEqualTo("bootstrapProperties"); + then(names).first().isEqualTo( + PropertySourceBootstrapConfiguration.BOOTSTRAP_PROPERTY_SOURCE_NAME + + "-refreshTest"); } } diff --git a/spring-cloud-context/src/test/resources/bootstrap.properties b/spring-cloud-context/src/test/resources/bootstrap.properties index dd6946b7..3bac76f8 100644 --- a/spring-cloud-context/src/test/resources/bootstrap.properties +++ b/spring-cloud-context/src/test/resources/bootstrap.properties @@ -1,2 +1,2 @@ -spring.main.sources:org.springframework.cloud.bootstrap.config.BootstrapConfigurationTests.PropertySourceConfiguration +spring.main.sources:org.springframework.cloud.bootstrap.config.BootstrapConfigurationTests.PropertySourceConfiguration,org.springframework.cloud.bootstrap.config.BootstrapConfigurationTests.CompositePropertySourceConfiguration info.name:child