From e9f31f9c34b8ee3ea2ee90361011d3a8e44ce936 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 6 Nov 2017 11:21:16 -0800 Subject: [PATCH] Improve configuration properties back-compatibility Refine `SystemEnvironmentPropertyMapper` to support environment variables that would have worked in Spring Boot 1.5. Specifically, camelCase property bindings now support an additional underscore. The recommended way to map `fooBar` is still `PREFIX_FOOBAR`, however, `PREFIX_FOO_BAR` will now also work. Fixes gh-10873 --- .../SystemEnvironmentPropertyMapper.java | 67 ++++++++++++------- ...ackCompatibiltyBinderIntegrationTests.java | 66 ++++++++++++++++++ .../SystemEnvironmentPropertyMapperTests.java | 3 +- 3 files changed, 110 insertions(+), 26 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BackCompatibiltyBinderIntegrationTests.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java index 0873c2e3d3..5da50654ef 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java @@ -18,7 +18,9 @@ package org.springframework.boot.context.properties.source; import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.function.Function; import java.util.stream.IntStream; @@ -49,6 +51,37 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper { private SystemEnvironmentPropertyMapper() { } + @Override + public List map(PropertySource propertySource, + ConfigurationPropertyName configurationPropertyName) { + Set names = new LinkedHashSet<>(); + names.add(convertName(configurationPropertyName)); + names.add(convertLegacyName(configurationPropertyName)); + List result = new ArrayList<>(); + names.forEach((name) -> result + .add(new PropertyMapping(name, configurationPropertyName))); + if (isListShortcutPossible(configurationPropertyName)) { + result.addAll(mapListShortcut(propertySource, configurationPropertyName)); + } + return result; + } + + private boolean isListShortcutPossible(ConfigurationPropertyName name) { + return (name.isLastElementIndexed() && isNumber(name.getLastElement(Form.UNIFORM)) + && name.getNumberOfElements() >= 1); + } + + private List mapListShortcut(PropertySource propertySource, + ConfigurationPropertyName name) { + String result = convertName(name, name.getNumberOfElements() - 1) + "__"; + if (propertySource.containsProperty(result)) { + int index = Integer.parseInt(name.getLastElement(Form.UNIFORM)); + return Collections.singletonList( + new PropertyMapping(result, name, new ElementExtractor(index))); + } + return Collections.emptyList(); + } + @Override public List map(PropertySource propertySource, String propertySourceName) { @@ -89,19 +122,6 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper { return mappings; } - @Override - public List map(PropertySource propertySource, - ConfigurationPropertyName configurationPropertyName) { - String name = convertName(configurationPropertyName); - List result = Collections - .singletonList(new PropertyMapping(name, configurationPropertyName)); - if (isListShortcutPossible(configurationPropertyName)) { - result = new ArrayList<>(result); - result.addAll(mapListShortcut(propertySource, configurationPropertyName)); - } - return result; - } - private String convertName(ConfigurationPropertyName name) { return convertName(name, name.getNumberOfElements()); } @@ -115,20 +135,17 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper { return result.toString(); } - private boolean isListShortcutPossible(ConfigurationPropertyName name) { - return (name.isLastElementIndexed() && isNumber(name.getLastElement(Form.UNIFORM)) - && name.getNumberOfElements() >= 1); + private String convertLegacyName(ConfigurationPropertyName name) { + StringBuilder result = new StringBuilder(); + for (int i = 0; i < name.getNumberOfElements(); i++) { + result.append(result.length() == 0 ? "" : "_"); + result.append(convertLegacyNameElement(name.getElement(i, Form.ORIGINAL))); + } + return result.toString(); } - private List mapListShortcut(PropertySource propertySource, - ConfigurationPropertyName name) { - String result = convertName(name, name.getNumberOfElements() - 1) + "__"; - if (propertySource.containsProperty(result)) { - int index = Integer.parseInt(name.getLastElement(Form.UNIFORM)); - return Collections.singletonList( - new PropertyMapping(result, name, new ElementExtractor(index))); - } - return Collections.emptyList(); + private Object convertLegacyNameElement(String element) { + return element.replace("-", "_").toUpperCase(); } private CharSequence processElementValue(CharSequence value) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BackCompatibiltyBinderIntegrationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BackCompatibiltyBinderIntegrationTests.java new file mode 100644 index 0000000000..5ee9a8a36e --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BackCompatibiltyBinderIntegrationTests.java @@ -0,0 +1,66 @@ +/* + * Copyright 2012-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.boot.context.properties.bind; + +import java.util.Collections; + +import org.junit.Test; + +import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.env.SystemEnvironmentPropertySource; +import org.springframework.mock.env.MockEnvironment; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests to ensure that the {@link Binder} offers at least some support for + * Boot 1.5 style binding. + * + * @author Phillip Webb + */ +public class BackCompatibiltyBinderIntegrationTests { + + // gh-10873 + + @Test + public void bindWhenBindingCamelCaseToEnvironmentWithExtractUnderscore() + throws Exception { + MockEnvironment environment = new MockEnvironment(); + SystemEnvironmentPropertySource propertySource = new SystemEnvironmentPropertySource( + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, + Collections.singletonMap("FOO_ZK_NODES", "foo")); + environment.getPropertySources().addFirst(propertySource); + ExampleCamelCaseBean result = Binder.get(environment) + .bind("foo", Bindable.of(ExampleCamelCaseBean.class)).get(); + assertThat(result.getZkNodes()).isEqualTo("foo"); + } + + public static class ExampleCamelCaseBean { + + private String zkNodes; + + public String getZkNodes() { + return this.zkNodes; + } + + public void setZkNodes(String zkNodes) { + this.zkNodes = zkNodes; + } + + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java index 8ce18f1992..13a3acca96 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java @@ -65,7 +65,8 @@ public class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapper assertThat(namesFromConfiguration("host[0].name")).containsExactly("HOST_0_NAME"); assertThat(namesFromConfiguration("host.f00.name")) .containsExactly("HOST_F00_NAME"); - assertThat(namesFromConfiguration("foo.the-bar")).containsExactly("FOO_THEBAR"); + assertThat(namesFromConfiguration("foo.the-bar")).containsExactly("FOO_THEBAR", + "FOO_THE_BAR"); } @Test