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 <[email protected]>

Author: mbhave

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 @@ private PropertyMapping[] tryMap(String propertySourceName) {
 		return NO_MAPPINGS;
 	}
 
+	@Override
+	public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
+		return name.isAncestorOf(candidate);
+	}
+
 	private static class LastMapping<T> {
 
 		private final T from;

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);
+
 }

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 @@ private PropertyMapping[] map(PropertyMapper mapper, String propertySourceName)
 			}
 		}
 
+		@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;

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 @@ public Iterator<ConfigurationPropertyName> iterator() {
 
 	@Override
 	public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) {
-		return ConfigurationPropertyState.search(this, name::isAncestorOf);
+		return ConfigurationPropertyState.search(this, (candidate) -> getMapper().isAncestorOf(name, candidate));
 	}
 
 	private List<ConfigurationPropertyName> getConfigurationPropertyNames() {

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 @@ public PropertyMapping[] map(String propertySourceName) {
 		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 @@ private String convertName(ConfigurationPropertyName name, int numberOfElements)
 		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) {

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.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 @@ void containsDescendantOfShouldCheckSourceNames() {
 				.isEqualTo(ConfigurationPropertyState.ABSENT);
 	}
 
+	@Test
+	void containsDescendantOfWhenSystemEnvironmentPropertySourceShouldLegacyProperty() {
+		Map<String, Object> 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

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java

@@ -75,4 +75,12 @@ void underscoreWithWhitespaceShouldNotMapToEmptyString() {
 		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();
+	}
+
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java

@@ -55,4 +55,9 @@ public PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName
 				.toArray(new PropertyMapping[0]);
 	}
 
+	@Override
+	public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
+		return name.isAncestorOf(candidate);
+	}
+
 }