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 <pwebb@pivotal.io>
pull/21361/head
Madhura Bhave 5 years ago committed by Phillip Webb
parent fb9bf7a0b2
commit a8f56b57cb

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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; return NO_MAPPINGS;
} }
@Override
public boolean isAncestorOf(ConfigurationPropertyName name, ConfigurationPropertyName candidate) {
return name.isAncestorOf(candidate);
}
private static class LastMapping<T> { private static class LastMapping<T> {
private final T from; private final T from;

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -55,4 +55,13 @@ interface PropertyMapper {
*/ */
PropertyMapping[] map(String propertySourceName); 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);
} }

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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) { private PropertyMapping[] merge(PropertyMapping[] first, PropertyMapping[] second) {
if (ObjectUtils.isEmpty(second)) { if (ObjectUtils.isEmpty(second)) {
return first; return first;

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -86,7 +86,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
@Override @Override
public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) { public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) {
return ConfigurationPropertyState.search(this, name::isAncestorOf); return ConfigurationPropertyState.search(this, (candidate) -> getMapper().isAncestorOf(name, candidate));
} }
private List<ConfigurationPropertyName> getConfigurationPropertyNames() { private List<ConfigurationPropertyName> getConfigurationPropertyNames() {

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -39,7 +39,7 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
@Override @Override
public PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName) { public PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName) {
String name = convertName(configurationPropertyName); String name = convertName(configurationPropertyName);
String legacyName = convertLegacyName(configurationPropertyName); String legacyName = convertLegacyName(configurationPropertyName, '_', true);
if (name.equals(legacyName)) { if (name.equals(legacyName)) {
return new PropertyMapping[] { new PropertyMapping(name, configurationPropertyName) }; return new PropertyMapping[] { new PropertyMapping(name, configurationPropertyName) };
} }
@ -56,6 +56,15 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
return new PropertyMapping[] { new PropertyMapping(propertySourceName, name) }; 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) { private ConfigurationPropertyName convertName(String propertySourceName) {
try { try {
return ConfigurationPropertyName.adapt(propertySourceName, '_', this::processElementValue); return ConfigurationPropertyName.adapt(propertySourceName, '_', this::processElementValue);
@ -80,19 +89,21 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
return result.toString(); return result.toString();
} }
private String convertLegacyName(ConfigurationPropertyName name) { private String convertLegacyName(ConfigurationPropertyName name, char joinChar, boolean uppercase) {
StringBuilder result = new StringBuilder(); StringBuilder result = new StringBuilder();
for (int i = 0; i < name.getNumberOfElements(); i++) { for (int i = 0; i < name.getNumberOfElements(); i++) {
if (result.length() > 0) { 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(); return result.toString();
} }
private Object convertLegacyNameElement(String element) { private Object convertLegacyNameElement(String element, char joinChar, boolean uppercase) {
return element.replace('-', '_').toUpperCase(Locale.ENGLISH); String converted = element.replace('-', joinChar);
return !uppercase ? converted : converted.toUpperCase(Locale.ENGLISH);
} }
private CharSequence processElementValue(CharSequence value) { private CharSequence processElementValue(CharSequence value) {

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.EnumerablePropertySource;
import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource; 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.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@ -154,6 +156,23 @@ class SpringIterableConfigurationPropertySourceTests {
.isEqualTo(ConfigurationPropertyState.ABSENT); .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 @Test
void simpleMapPropertySourceKeyDataChangeInvalidatesCache() { void simpleMapPropertySourceKeyDataChangeInvalidatesCache() {
// gh-13344 // gh-13344

@ -75,4 +75,12 @@ class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapperTests {
assertThat(applicable).isFalse(); 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();
}
} }

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

Loading…
Cancel
Save