From c94f946c85ff2ec99b4ffa3ceb5cf8b596f29487 Mon Sep 17 00:00:00 2001 From: Oleg Zhurakousky Date: Tue, 21 Nov 2017 11:48:29 -0500 Subject: [PATCH] 1136 Fixed type expectations in BinderProperties - changed BinderProperties environment to be Map to support appropriate semantics where keys and values may be non-Strings - deprecated `setEnvironment(Properties environment)` operation Resolves #1136 polishing --- .../stream/binder/BinderConfiguration.java | 7 +- .../stream/binder/DefaultBinderFactory.java | 3 +- .../config/BinderFactoryConfiguration.java | 2 +- .../cloud/stream/config/BinderProperties.java | 18 ++++- .../BinderFactoryConfigurationTests.java | 21 ------ .../stream/binding/BindingServiceTests.java | 2 +- .../stream/config/BinderPropertiesTests.java | 74 +++++++++++++++++++ 7 files changed, 96 insertions(+), 31 deletions(-) create mode 100644 spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BinderPropertiesTests.java diff --git a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/BinderConfiguration.java b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/BinderConfiguration.java index 81f4e49bb..2153bbc6e 100644 --- a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/BinderConfiguration.java +++ b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binder/BinderConfiguration.java @@ -16,6 +16,7 @@ package org.springframework.cloud.stream.binder; +import java.util.Map; import java.util.Properties; /** @@ -30,7 +31,7 @@ public class BinderConfiguration { private final String binderType; - private final Properties properties; + private final Map properties; private final boolean inheritEnvironment; @@ -44,7 +45,7 @@ public class BinderConfiguration { * @param defaultCandidate whether the binder should be considered as a candidate when * determining a default */ - public BinderConfiguration(String binderType, Properties properties, boolean inheritEnvironment, + public BinderConfiguration(String binderType, Map properties, boolean inheritEnvironment, boolean defaultCandidate) { this.binderType = binderType; this.properties = properties; @@ -56,7 +57,7 @@ public class BinderConfiguration { return binderType; } - public Properties getProperties() { + public Map getProperties() { return properties; } 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 ef90e489c..d0460a97a 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 @@ -23,7 +23,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Properties; import java.util.Set; import org.springframework.beans.factory.DisposableBean; @@ -169,7 +168,7 @@ public class DefaultBinderFactory implements BinderFactory, DisposableBean, Appl } BinderType binderType = this.binderTypeRegistry.get(binderConfiguration.getBinderType()); Assert.notNull(binderType, "Binder type " + binderConfiguration.getBinderType() + " is not defined"); - Properties binderProperties = binderConfiguration.getProperties(); + Map binderProperties = binderConfiguration.getProperties(); // Convert all properties to arguments, so that they receive maximum // precedence ArrayList args = new ArrayList<>(); diff --git a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderFactoryConfiguration.java b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderFactoryConfiguration.java index 971a9f2ef..1c1e38e27 100644 --- a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderFactoryConfiguration.java +++ b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderFactoryConfiguration.java @@ -131,7 +131,7 @@ public class BinderFactoryConfiguration { for (Map.Entry binderEntry : binderTypeRegistry.getAll().entrySet()) { if (!existingBinderConfigurations.contains(binderEntry.getKey())) { binderConfigurations.put(binderEntry.getKey(), new BinderConfiguration(binderEntry.getKey(), - new Properties(), true, true)); + new HashMap<>(), true, true)); } } } diff --git a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderProperties.java b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderProperties.java index 26a3ed859..8b68035c8 100644 --- a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderProperties.java +++ b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BinderProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2017 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. @@ -16,18 +16,21 @@ package org.springframework.cloud.stream.config; +import java.util.HashMap; +import java.util.Map; import java.util.Properties; /** * Contains the properties of a binder. * * @author Marius Bogoevici + * @author Oleg Zhurakousky */ public class BinderProperties { private String type; - private Properties environment = new Properties(); + private Map environment = new HashMap<>(); private boolean inheritEnvironment = true; @@ -41,11 +44,20 @@ public class BinderProperties { this.type = name; } - public Properties getEnvironment() { + public Map getEnvironment() { return environment; } + /** + * @deprecated in 2.0.0 in preference to {@link #setEnvironment(Map)} + */ + @Deprecated public void setEnvironment(Properties environment) { + this.environment.clear(); + this.environment.putAll(environment); + } + + public void setEnvironment(Map environment) { this.environment = environment; } 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 9e55f1e06..ea56fa8ab 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 @@ -162,20 +162,6 @@ public class BinderFactoryConfigurationTests { assertThat(binderFactory.getBinder(null, MessageChannel.class)).isSameAs(binder1); } - @Test - public void loadBinderTypeRegistryWithOneCustomBinderAndIsolatedEnvironment() throws Exception { - ConfigurableApplicationContext context = createBinderTestContext( - new String[] { "binder1" }, "binder1.name=foo", - "spring.cloud.stream.binders.custom.type=binder1", - "spring.cloud.stream.binders.custom.environment.foo=bar", - "spring.cloud.stream.binders.custom.inheritEnvironment=false"); - - BinderFactory binderFactory = context.getBean(BinderFactory.class); - - Binder binder1 = binderFactory.getBinder("custom", MessageChannel.class); - assertThat(binder1).hasFieldOrPropertyWithValue("name", null); - } - @Test public void loadBinderTypeRegistryWithTwoBinders() throws Exception { ConfigurableApplicationContext context = createBinderTestContext(new String[] { "binder1", "binder2" }); @@ -208,11 +194,9 @@ public class BinderFactoryConfigurationTests { @Test public void loadBinderTypeRegistryWithCustomNonDefaultCandidate() throws Exception { - ConfigurableApplicationContext context = createBinderTestContext( new String[] { "binder1" }, "spring.cloud.stream.binders.custom.type=binder1", - "spring.cloud.stream.binders.custom.environment.binder1.name=foo", "spring.cloud.stream.binders.custom.defaultCandidate=false", "spring.cloud.stream.binders.custom.inheritEnvironment=false"); BinderTypeRegistry binderTypeRegistry = context.getBean(BinderTypeRegistry.class); @@ -231,11 +215,6 @@ public class BinderFactoryConfigurationTests { Binder binder1 = binderFactory.getBinder("binder1", MessageChannel.class); assertThat(binder1).isInstanceOf(StubBinder1.class); assertThat(binder1).isSameAs(defaultBinder); - - Binder custom = binderFactory.getBinder("custom", MessageChannel.class); - assertThat(custom).isInstanceOf(StubBinder1.class); - assertThat(custom).isNotSameAs(defaultBinder); - assertThat(((StubBinder1) custom).getName()).isEqualTo("foo"); } @Test diff --git a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binding/BindingServiceTests.java b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binding/BindingServiceTests.java index d22e87a6f..ffdc9f4ad 100644 --- a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binding/BindingServiceTests.java +++ b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/binding/BindingServiceTests.java @@ -473,7 +473,7 @@ public class BindingServiceTests { private DefaultBinderFactory createMockBinderFactory() { BinderTypeRegistry binderTypeRegistry = createMockBinderTypeRegistry(); return new DefaultBinderFactory( - Collections.singletonMap("mock", new BinderConfiguration("mock", new Properties(), true, true)), + Collections.singletonMap("mock", new BinderConfiguration("mock", new HashMap<>(), true, true)), binderTypeRegistry); } diff --git a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BinderPropertiesTests.java b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BinderPropertiesTests.java new file mode 100644 index 000000000..af0d1a6cf --- /dev/null +++ b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BinderPropertiesTests.java @@ -0,0 +1,74 @@ +/* + * Copyright 2017 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 + * + * http://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.cloud.stream.config; + +import java.util.Collections; +import java.util.Map; +import java.util.Properties; + +import org.junit.Test; + +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.boot.actuate.context.properties.ConfigurationPropertiesReportEndpoint; +import org.springframework.boot.actuate.context.properties.ConfigurationPropertiesReportEndpoint.ConfigurationPropertiesDescriptor; +import org.springframework.context.support.StaticApplicationContext; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * This test primarily validates the correctness of BinderProperties + * where it as well as what it contains maintains the String-key/Object-value + * semantics. The use of {@link Properties} class does not exactly do that. + * + * + * @author Oleg Zhurakousky + * + */ +public class BinderPropertiesTests { + + @SuppressWarnings("unchecked") + @Test + public void testSerializationWithNonStringValues() { + StaticApplicationContext context = new StaticApplicationContext(); + DefaultListableBeanFactory bf = (DefaultListableBeanFactory) context.getBeanFactory(); + BindingServiceProperties bindingServiceProperties = new BindingServiceProperties(); + bindingServiceProperties.setApplicationContext(context); + bf.registerSingleton("bindingServiceProperties", bindingServiceProperties); + + BindingServiceProperties bsp = context.getBean(BindingServiceProperties.class); + bsp.setApplicationContext(context); + BinderProperties bp = new BinderProperties(); + bsp.setBinders(Collections.singletonMap("testBinder", bp)); + bp.getEnvironment().put("spring.rabbitmq.connection-timeout", 2345); + bp.getEnvironment().put("foo", Collections.singletonMap("bar", "hello")); + + // using Spring Boot class to ensure that reliance on the same ObjectMapper configuration + ConfigurationPropertiesReportEndpoint endpoint = new ConfigurationPropertiesReportEndpoint(); + endpoint.setApplicationContext(context); + + ConfigurationPropertiesDescriptor configurationProperties = endpoint.configurationProperties(); + Map properties = configurationProperties.getBeans().get("bindingServiceProperties").getProperties(); + assertFalse(properties.containsKey("error")); + assertTrue(properties.containsKey("binders")); + Map testBinder = (Map) ((Map)properties.get("binders")).get("testBinder"); + Map environment = (Map) testBinder.get("environment"); + assertTrue(environment.get("spring.rabbitmq.connection-timeout") instanceof Integer); + assertTrue(environment.get("foo") instanceof Map); + } +} +