From 44d3fa4a90cb09db4767c2e4a12c378eaba24f66 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 15 Dec 2015 10:34:00 +0000 Subject: [PATCH] Inherit Environment from main context when creating a Binder The DefaultBinderFactory makes use of SpringApplicationBuilder to build a context containing a binder. It has access to the Environment of the surrounding context, but doesn't use it, and doesn't use it (before this change). Users will be surprised if (for instance) they change the default rabbitmq hostname and the change doesn't propagate to the binder. --- .../stream/binder/DefaultBinderFactory.java | 37 +++++++++++-------- .../BinderFactoryConfigurationTests.java | 12 +++++- .../stream/binder/stub1/StubBinder1.java | 10 +++++ .../stub1/StubBinder1Configuration.java | 4 ++ 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java index 5fd1e2282..05bc9a379 100644 --- a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java +++ b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/DefaultBinderFactory.java @@ -26,7 +26,9 @@ import org.springframework.boot.Banner.Mode; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; +import org.springframework.core.env.StandardEnvironment; import org.springframework.util.StringUtils; /** @@ -59,7 +61,7 @@ public class DefaultBinderFactory implements BinderFactory, DisposableBean @Override public void destroy() throws Exception { - for (Map.Entry> entry : binderInstanceCache.entrySet()) { + for (Map.Entry> entry : this.binderInstanceCache.entrySet()) { BinderInstanceHolder binderInstanceHolder = entry.getValue(); binderInstanceHolder.getBinderContext().close(); } @@ -70,28 +72,28 @@ public class DefaultBinderFactory implements BinderFactory, DisposableBean String configurationName; // Fall back to a default if no argument is provided if (StringUtils.isEmpty(name)) { - if (binderConfigurations.size() == 0) { + if (this.binderConfigurations.size() == 0) { throw new IllegalStateException("A default binder has been requested, but there there is no binder available"); } - else if (binderConfigurations.size() == 1) { - configurationName = binderConfigurations.keySet().iterator().next(); + else if (this.binderConfigurations.size() == 1) { + configurationName = this.binderConfigurations.keySet().iterator().next(); } else { - if (StringUtils.hasText(defaultBinder)) { - configurationName = defaultBinder; + if (StringUtils.hasText(this.defaultBinder)) { + configurationName = this.defaultBinder; } else { throw new IllegalStateException( "A default binder has been requested, but there is more than one binder available: " - + StringUtils.collectionToCommaDelimitedString(binderConfigurations.keySet()) + ", and" + + StringUtils.collectionToCommaDelimitedString(this.binderConfigurations.keySet()) + ", and" + " no default binder has been set."); } } } else { configurationName = name; } - if (!binderInstanceCache.containsKey(configurationName)) { - BinderConfiguration binderConfiguration = binderConfigurations.get(configurationName); + if (!this.binderInstanceCache.containsKey(configurationName)) { + BinderConfiguration binderConfiguration = this.binderConfigurations.get(configurationName); if (binderConfiguration == null) { throw new IllegalStateException("Unknown binder configuration: " + configurationName); } @@ -102,7 +104,7 @@ public class DefaultBinderFactory implements BinderFactory, DisposableBean args.add(String.format("--%s=%s",property.getKey(),property.getValue())); } // Initialize the domain with a unique name based on the bootstrapping context setting - String defaultDomain = environment != null ? environment.getProperty("spring.jmx.default-domain") : null; + String defaultDomain = this.environment != null ? this.environment.getProperty("spring.jmx.default-domain") : null; if (defaultDomain == null) { defaultDomain = ""; } @@ -116,13 +118,18 @@ public class DefaultBinderFactory implements BinderFactory, DisposableBean .sources(binderConfiguration.getBinderType().getConfigurationClasses()) .bannerMode(Mode.OFF) .web(false); + if (this.environment instanceof ConfigurableEnvironment) { + StandardEnvironment environment = new StandardEnvironment(); + environment.merge((ConfigurableEnvironment) this.environment); + springApplicationBuilder.environment(environment); + } ConfigurableApplicationContext binderProducingContext = springApplicationBuilder.run(args.toArray(new String[args.size()])); @SuppressWarnings("unchecked") - Binder binder = (Binder) binderProducingContext.getBean(Binder.class); - binderInstanceCache.put(configurationName, new BinderInstanceHolder<>(binder, binderProducingContext)); + Binder binder = binderProducingContext.getBean(Binder.class); + this.binderInstanceCache.put(configurationName, new BinderInstanceHolder<>(binder, binderProducingContext)); } - return binderInstanceCache.get(configurationName).getBinderInstance(); + return this.binderInstanceCache.get(configurationName).getBinderInstance(); } /** @@ -142,11 +149,11 @@ public class DefaultBinderFactory implements BinderFactory, DisposableBean } public Binder getBinderInstance() { - return binderInstance; + return this.binderInstance; } public ConfigurableApplicationContext getBinderContext() { - return binderContext; + return this.binderContext; } } } diff --git a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/BinderFactoryConfigurationTests.java b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/BinderFactoryConfigurationTests.java index 73e4fcf57..5f497595e 100644 --- a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/BinderFactoryConfigurationTests.java +++ b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/BinderFactoryConfigurationTests.java @@ -33,7 +33,6 @@ import java.net.URL; import java.net.URLClassLoader; import org.junit.Test; - import org.springframework.beans.factory.BeanCreationException; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.builder.SpringApplicationBuilder; @@ -89,6 +88,17 @@ public class BinderFactoryConfigurationTests { assertThat(defaultBinder, is(binder1)); } + @Test + public void loadBinderTypeRegistryWithOneBinderAndSharedEnvironment() throws Exception { + ConfigurableApplicationContext context = createBinderTestContext( + new String[] {"binder1"}, "binder1.name=foo"); + + BinderFactory binderFactory = context.getBean(BinderFactory.class); + + Binder binder1 = binderFactory.getBinder("binder1"); + assertThat(((StubBinder1)binder1).getName(), is(equalTo("foo"))); + } + @Test public void loadBinderTypeRegistryWithTwoBinders() throws Exception { diff --git a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1.java b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1.java index 687ffdebb..833fe9666 100644 --- a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1.java +++ b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1.java @@ -25,6 +25,16 @@ import org.springframework.cloud.stream.binder.Binder; */ public class StubBinder1 implements Binder { + private String name; + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + @Override public void bindConsumer(String name, Object inboundBindTarget, Properties properties) { diff --git a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1Configuration.java b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1Configuration.java index cda49299f..b6a625ae0 100644 --- a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1Configuration.java +++ b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binder/stub1/StubBinder1Configuration.java @@ -16,6 +16,8 @@ package org.springframework.cloud.stream.binder.stub1; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.stream.binder.Binder; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -24,9 +26,11 @@ import org.springframework.context.annotation.Configuration; * @author Marius Bogoevici */ @Configuration +@EnableConfigurationProperties public class StubBinder1Configuration { @Bean + @ConfigurationProperties("binder1") public Binder binder() { return new StubBinder1(); }