From a8f56b57cbe57bb77f2ffa477077158e0fe7b25f Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Mon, 20 Apr 2020 12:29:50 -0700 Subject: [PATCH] Consider legacy environment names in isAncestorOf Update the `isAncestorOf` method of SpringConfigurationPropertySources so that legacy names are considered for the system environment. Prior to this commit, binding a property such as `my.camelCase.prop` would detect `MY_CAMELCASE_PROP` but not `MY_CAMEL_CASE_PROP` in the system environment. Fixes gh-14479 Co-authored-by: Phillip Webb --- .../source/DefaultPropertyMapper.java | 7 +++++- .../properties/source/PropertyMapper.java | 11 +++++++- .../SpringConfigurationPropertySource.java | 12 ++++++++- ...ngIterableConfigurationPropertySource.java | 4 +-- .../SystemEnvironmentPropertyMapper.java | 25 +++++++++++++------ ...rableConfigurationPropertySourceTests.java | 21 +++++++++++++++- .../SystemEnvironmentPropertyMapperTests.java | 8 ++++++ .../properties/source/TestPropertyMapper.java | 5 ++++ 8 files changed, 80 insertions(+), 13 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java index b9c97d6ff8..5c36ff0e9f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -76,6 +76,11 @@ final class DefaultPropertyMapper implements PropertyMapper { return NO_MAPPINGS; } + @Override + public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) { + return name.isAncestorOf(candidate); + } + private static class LastMapping { private final T from; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java index 9032d00584..8fbe234333 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -55,4 +55,13 @@ interface PropertyMapper { */ PropertyMapping[] map(String propertySourceName); + /** + * Returns {@code true} if {@code name} is an ancestor (immediate or nested parent) of + * the given candidate when considering mapping rules. + * @param name the source name + * @param candidate the candidate to check + * @return {@code true} if the candidate is an ancestor of the name + */ + boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate); + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java index f83af8c7d2..299d66724f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -249,6 +249,16 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource { } } + @Override + public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) { + return isAncestorOf(this.first, name, candidate) || isAncestorOf(this.second, name, candidate); + } + + private boolean isAncestorOf(PropertyMapper mapper, ConfigurationPropertyName name, + ConfigurationPropertyName candidate) { + return mapper != null && mapper.isAncestorOf(name, candidate); + } + private PropertyMapping[] merge(PropertyMapping[] first, PropertyMapping[] second) { if (ObjectUtils.isEmpty(second)) { return first; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java index 6411fd26c4..923a4997cb 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -86,7 +86,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope @Override public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) { - return ConfigurationPropertyState.search(this, name::isAncestorOf); + return ConfigurationPropertyState.search(this, (candidate) -> getMapper().isAncestorOf(name, candidate)); } private List getConfigurationPropertyNames() { 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 1f58211eb4..58b63c3525 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -39,7 +39,7 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper { @Override public PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName) { String name = convertName(configurationPropertyName); - String legacyName = convertLegacyName(configurationPropertyName); + String legacyName = convertLegacyName(configurationPropertyName, '_', true); if (name.equals(legacyName)) { return new PropertyMapping[] { new PropertyMapping(name, configurationPropertyName) }; } @@ -56,6 +56,15 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper { return new PropertyMapping[] { new PropertyMapping(propertySourceName, name) }; } + @Override + public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) { + return name.isAncestorOf(candidate) || isLegacyAncestorOf(name, candidate); + } + + private boolean isLegacyAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) { + return ConfigurationPropertyName.of(convertLegacyName(name, '.', false)).isAncestorOf(candidate); + } + private ConfigurationPropertyName convertName(String propertySourceName) { try { return ConfigurationPropertyName.adapt(propertySourceName, '_', this::processElementValue); @@ -80,19 +89,21 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper { return result.toString(); } - private String convertLegacyName(ConfigurationPropertyName name) { + private String convertLegacyName(ConfigurationPropertyName name, char joinChar, boolean uppercase) { StringBuilder result = new StringBuilder(); for (int i = 0; i < name.getNumberOfElements(); i++) { if (result.length() > 0) { - result.append("_"); + result.append(joinChar); } - result.append(convertLegacyNameElement(name.getElement(i, Form.ORIGINAL))); + String element = name.getElement(i, Form.ORIGINAL); + result.append(convertLegacyNameElement(element, joinChar, uppercase)); } return result.toString(); } - private Object convertLegacyNameElement(String element) { - return element.replace('-', '_').toUpperCase(Locale.ENGLISH); + private Object convertLegacyNameElement(String element, char joinChar, boolean uppercase) { + String converted = element.replace('-', joinChar); + return !uppercase ? converted : converted.toUpperCase(Locale.ENGLISH); } private CharSequence processElementValue(CharSequence value) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java index e0137cd332..fa3670d975 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -31,6 +31,8 @@ import org.springframework.boot.origin.OriginLookup; import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.PropertySource; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.env.SystemEnvironmentPropertySource; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -154,6 +156,23 @@ class SpringIterableConfigurationPropertySourceTests { .isEqualTo(ConfigurationPropertyState.ABSENT); } + @Test + void containsDescendantOfWhenSystemEnvironmentPropertySourceShouldLegacyProperty() { + Map source = new LinkedHashMap<>(); + source.put("FOO_BAR_BAZ_BONG", "bing"); + source.put("FOO_ALPHABRAVO_GAMMA", "delta"); + SystemEnvironmentPropertySource propertySource = new SystemEnvironmentPropertySource( + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, source); + SpringIterableConfigurationPropertySource adapter = new SpringIterableConfigurationPropertySource( + propertySource, SystemEnvironmentPropertyMapper.INSTANCE); + assertThat(adapter.containsDescendantOf(ConfigurationPropertyName.of("foo.bar-baz"))) + .isEqualTo(ConfigurationPropertyState.PRESENT); + assertThat(adapter.containsDescendantOf(ConfigurationPropertyName.of("foo.alpha-bravo"))) + .isEqualTo(ConfigurationPropertyState.PRESENT); + assertThat(adapter.containsDescendantOf(ConfigurationPropertyName.of("foo.blah"))) + .isEqualTo(ConfigurationPropertyState.ABSENT); + } + @Test void simpleMapPropertySourceKeyDataChangeInvalidatesCache() { // gh-13344 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 64f03abaac..95e3826ea8 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 @@ -75,4 +75,12 @@ class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapperTests { assertThat(applicable).isFalse(); } + @Test + void isAncestorOfConsidersLegacyNames() { + ConfigurationPropertyName name = ConfigurationPropertyName.of("my.spring-boot"); + assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.spring-boot.property"))).isTrue(); + assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.springboot.property"))).isTrue(); + assertThat(getMapper().isAncestorOf(name, ConfigurationPropertyName.of("my.boot.property"))).isFalse(); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java index b55a375868..adac69356e 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java @@ -55,4 +55,9 @@ class TestPropertyMapper implements PropertyMapper { .toArray(new PropertyMapping[0]); } + @Override + public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) { + return name.isAncestorOf(candidate); + } + }