Report deprecated Map properties

Closes gh-27854
pull/34029/head
Moritz Halbritter 2 years ago
parent b7ae9fbfdf
commit 9975ec8be3

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2020 the original author or authors. * Copyright 2012-2023 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.
@ -23,12 +23,11 @@ import java.util.Map;
import java.util.function.Function; import java.util.function.Function;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.springframework.boot.configurationmetadata.ConfigurationMetadataProperty;
/** /**
* Provides a properties migration report. * Provides a properties migration report.
* *
* @author Stephane Nicoll * @author Stephane Nicoll
* @author Moritz Halbritter
*/ */
class PropertiesMigrationReport { class PropertiesMigrationReport {
@ -87,8 +86,7 @@ class PropertiesMigrationReport {
report.append(String.format("Property source '%s':%n", name)); report.append(String.format("Property source '%s':%n", name));
properties.sort(PropertyMigration.COMPARATOR); properties.sort(PropertyMigration.COMPARATOR);
properties.forEach((property) -> { properties.forEach((property) -> {
ConfigurationMetadataProperty metadata = property.getMetadata(); report.append(String.format("\tKey: %s%n", property.getProperty().getName()));
report.append(String.format("\tKey: %s%n", metadata.getId()));
if (property.getLineNumber() != null) { if (property.getLineNumber() != null) {
report.append(String.format("\t\tLine: %d%n", property.getLineNumber())); report.append(String.format("\t\tLine: %d%n", property.getLineNumber()));
} }

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2022 the original author or authors. * Copyright 2012-2023 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.
@ -29,6 +29,7 @@ import org.springframework.boot.context.properties.source.ConfigurationProperty;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
import org.springframework.boot.context.properties.source.ConfigurationPropertySource; import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
import org.springframework.boot.context.properties.source.ConfigurationPropertySources; import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.boot.context.properties.source.IterableConfigurationPropertySource;
import org.springframework.boot.env.OriginTrackedMapPropertySource; import org.springframework.boot.env.OriginTrackedMapPropertySource;
import org.springframework.boot.origin.OriginTrackedValue; import org.springframework.boot.origin.OriginTrackedValue;
import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.ConfigurableEnvironment;
@ -41,6 +42,7 @@ import org.springframework.util.StringUtils;
* Report on {@link PropertyMigration properties migration}. * Report on {@link PropertyMigration properties migration}.
* *
* @author Stephane Nicoll * @author Stephane Nicoll
* @author Moritz Halbritter
*/ */
class PropertiesMigrationReporter { class PropertiesMigrationReporter {
@ -88,24 +90,40 @@ class PropertiesMigrationReporter {
for (PropertyMigration candidate : renamed) { for (PropertyMigration candidate : renamed) {
OriginTrackedValue value = OriginTrackedValue.of(candidate.getProperty().getValue(), OriginTrackedValue value = OriginTrackedValue.of(candidate.getProperty().getValue(),
candidate.getProperty().getOrigin()); candidate.getProperty().getOrigin());
content.put(candidate.getMetadata().getDeprecation().getReplacement(), value); content.put(candidate.getNewPropertyName(), value);
} }
return new OriginTrackedMapPropertySource(target, content); return new OriginTrackedMapPropertySource(target, content);
} }
private boolean isMapType(ConfigurationMetadataProperty property) {
String type = property.getType();
return type != null && type.startsWith(Map.class.getName());
}
private Map<String, List<PropertyMigration>> getMatchingProperties( private Map<String, List<PropertyMigration>> getMatchingProperties(
Predicate<ConfigurationMetadataProperty> filter) { Predicate<ConfigurationMetadataProperty> filter) {
MultiValueMap<String, PropertyMigration> result = new LinkedMultiValueMap<>(); MultiValueMap<String, PropertyMigration> result = new LinkedMultiValueMap<>();
List<ConfigurationMetadataProperty> candidates = this.allProperties.values().stream().filter(filter) List<ConfigurationMetadataProperty> candidates = this.allProperties.values().stream().filter(filter)
.collect(Collectors.toList()); .collect(Collectors.toList());
getPropertySourcesAsMap().forEach((name, source) -> candidates.forEach((metadata) -> { getPropertySourcesAsMap().forEach((propertySourceName, propertySource) -> candidates.forEach((metadata) -> {
ConfigurationPropertyName metadataName = ConfigurationPropertyName.isValid(metadata.getId()) ConfigurationPropertyName metadataName = ConfigurationPropertyName.isValid(metadata.getId())
? ConfigurationPropertyName.of(metadata.getId()) ? ConfigurationPropertyName.of(metadata.getId())
: ConfigurationPropertyName.adapt(metadata.getId(), '.'); : ConfigurationPropertyName.adapt(metadata.getId(), '.');
ConfigurationProperty configurationProperty = source.getConfigurationProperty(metadataName); // Direct match
if (configurationProperty != null) { ConfigurationProperty match = propertySource.getConfigurationProperty(metadataName);
result.add(name, if (match != null) {
new PropertyMigration(configurationProperty, metadata, determineReplacementMetadata(metadata))); result.add(propertySourceName,
new PropertyMigration(match, metadata, determineReplacementMetadata(metadata), false));
}
// Prefix match for maps
if (isMapType(metadata) && propertySource instanceof IterableConfigurationPropertySource) {
IterableConfigurationPropertySource iterableSource = (IterableConfigurationPropertySource) propertySource;
iterableSource.stream().filter(metadataName::isAncestorOf).map(propertySource::getConfigurationProperty)
.forEach((property) -> {
ConfigurationMetadataProperty replacement = determineReplacementMetadata(metadata);
result.add(propertySourceName,
new PropertyMigration(property, metadata, replacement, true));
});
} }
})); }));
return result; return result;
@ -125,8 +143,12 @@ class PropertiesMigrationReporter {
private ConfigurationMetadataProperty detectMapValueReplacement(String fullId) { private ConfigurationMetadataProperty detectMapValueReplacement(String fullId) {
int lastDot = fullId.lastIndexOf('.'); int lastDot = fullId.lastIndexOf('.');
if (lastDot != -1) { if (lastDot == -1) {
return this.allProperties.get(fullId.substring(0, lastDot)); return null;
}
ConfigurationMetadataProperty metadata = this.allProperties.get(fullId.substring(0, lastDot));
if (metadata != null && isMapType(metadata)) {
return metadata;
} }
return null; return null;
} }

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2020 the original author or authors. * Copyright 2012-2023 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.
@ -23,14 +23,17 @@ import java.util.Map;
import org.springframework.boot.configurationmetadata.ConfigurationMetadataProperty; import org.springframework.boot.configurationmetadata.ConfigurationMetadataProperty;
import org.springframework.boot.configurationmetadata.Deprecation; import org.springframework.boot.configurationmetadata.Deprecation;
import org.springframework.boot.context.properties.source.ConfigurationProperty; import org.springframework.boot.context.properties.source.ConfigurationProperty;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
import org.springframework.boot.origin.Origin; import org.springframework.boot.origin.Origin;
import org.springframework.boot.origin.TextResourceOrigin; import org.springframework.boot.origin.TextResourceOrigin;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
/** /**
* Description of a property migration. * Description of a property migration.
* *
* @author Stephane Nicoll * @author Stephane Nicoll
* @author Moritz Halbritter
*/ */
class PropertyMigration { class PropertyMigration {
@ -45,14 +48,20 @@ class PropertyMigration {
private final ConfigurationMetadataProperty replacementMetadata; private final ConfigurationMetadataProperty replacementMetadata;
/**
* Whether this migration happened from a map type.
*/
private final boolean mapMigration;
private final boolean compatibleType; private final boolean compatibleType;
PropertyMigration(ConfigurationProperty property, ConfigurationMetadataProperty metadata, PropertyMigration(ConfigurationProperty property, ConfigurationMetadataProperty metadata,
ConfigurationMetadataProperty replacementMetadata) { ConfigurationMetadataProperty replacementMetadata, boolean mapMigration) {
this.property = property; this.property = property;
this.lineNumber = determineLineNumber(property); this.lineNumber = determineLineNumber(property);
this.metadata = metadata; this.metadata = metadata;
this.replacementMetadata = replacementMetadata; this.replacementMetadata = replacementMetadata;
this.mapMigration = mapMigration;
this.compatibleType = determineCompatibleType(metadata, replacementMetadata); this.compatibleType = determineCompatibleType(metadata, replacementMetadata);
} }
@ -69,9 +78,9 @@ class PropertyMigration {
private static boolean determineCompatibleType(ConfigurationMetadataProperty metadata, private static boolean determineCompatibleType(ConfigurationMetadataProperty metadata,
ConfigurationMetadataProperty replacementMetadata) { ConfigurationMetadataProperty replacementMetadata) {
String currentType = metadata.getType(); String currentType = determineType(metadata);
String replacementType = determineReplacementType(replacementMetadata); String replacementType = determineType(replacementMetadata);
if (replacementType == null || currentType == null) { if (currentType == null || replacementType == null) {
return false; return false;
} }
if (replacementType.equals(currentType)) { if (replacementType.equals(currentType)) {
@ -84,14 +93,15 @@ class PropertyMigration {
return false; return false;
} }
private static String determineReplacementType(ConfigurationMetadataProperty replacementMetadata) { private static String determineType(ConfigurationMetadataProperty metadata) {
if (replacementMetadata == null || replacementMetadata.getType() == null) { if (metadata == null || metadata.getType() == null) {
return null; return null;
} }
String candidate = replacementMetadata.getType(); String candidate = metadata.getType();
if (candidate.startsWith(Map.class.getName())) { if (candidate.startsWith(Map.class.getName())) {
int lastComma = candidate.lastIndexOf(','); int lastComma = candidate.lastIndexOf(',');
if (lastComma != -1) { if (lastComma != -1) {
// Use Map value type
return candidate.substring(lastComma + 1, candidate.length() - 1).trim(); return candidate.substring(lastComma + 1, candidate.length() - 1).trim();
} }
} }
@ -114,9 +124,16 @@ class PropertyMigration {
return this.compatibleType; return this.compatibleType;
} }
String getNewPropertyName() {
if (this.mapMigration) {
return getNewMapPropertyName(this.property, this.metadata, this.replacementMetadata).toString();
}
return this.metadata.getDeprecation().getReplacement();
}
String determineReason() { String determineReason() {
if (this.compatibleType) { if (this.compatibleType) {
return "Replacement: " + this.metadata.getDeprecation().getReplacement(); return "Replacement: " + getNewPropertyName();
} }
Deprecation deprecation = this.metadata.getDeprecation(); Deprecation deprecation = this.metadata.getDeprecation();
if (StringUtils.hasText(deprecation.getShortReason())) { if (StringUtils.hasText(deprecation.getShortReason())) {
@ -132,4 +149,14 @@ class PropertyMigration {
return "Reason: none"; return "Reason: none";
} }
private static ConfigurationPropertyName getNewMapPropertyName(ConfigurationProperty property,
ConfigurationMetadataProperty metadata, ConfigurationMetadataProperty replacement) {
ConfigurationPropertyName oldName = property.getName();
ConfigurationPropertyName oldPrefix = ConfigurationPropertyName.of(metadata.getId());
Assert.state(oldPrefix.isAncestorOf(oldName),
String.format("'%s' is not an ancestor of '%s'", oldPrefix, oldName));
ConfigurationPropertyName newPrefix = ConfigurationPropertyName.of(replacement.getId());
return newPrefix.append(oldName.subName(oldPrefix.getNumberOfElements()));
}
} }

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2022 the original author or authors. * Copyright 2012-2023 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.
@ -46,10 +46,11 @@ import static org.assertj.core.api.Assertions.assertThat;
* Tests for {@link PropertiesMigrationReporter}. * Tests for {@link PropertiesMigrationReporter}.
* *
* @author Stephane Nicoll * @author Stephane Nicoll
* @author Moritz Halbritter
*/ */
class PropertiesMigrationReporterTests { class PropertiesMigrationReporterTests {
private ConfigurableEnvironment environment = new MockEnvironment(); private final ConfigurableEnvironment environment = new MockEnvironment();
@Test @Test
void reportIsNullWithNoMatchingKeys() { void reportIsNullWithNoMatchingKeys() {
@ -67,7 +68,11 @@ class PropertiesMigrationReporterTests {
assertThat(propertySources).hasSize(3); assertThat(propertySources).hasSize(3);
createAnalyzer(loadRepository("metadata/sample-metadata.json")).getReport(); createAnalyzer(loadRepository("metadata/sample-metadata.json")).getReport();
assertThat(mapToNames(propertySources)).containsExactly("one", "migrate-two", "two", "mockProperties"); assertThat(mapToNames(propertySources)).containsExactly("one", "migrate-two", "two", "mockProperties");
assertMappedProperty(propertySources.get("migrate-two"), "test.two", "another", getOrigin(two, "wrong.two")); PropertySource<?> migrateTwo = propertySources.get("migrate-two");
assertThat(migrateTwo).isNotNull();
assertMappedProperty(migrateTwo, "test.two", "another", getOrigin(two, "wrong.two"));
assertMappedProperty(migrateTwo, "custom.the-map-replacement.key", "value",
getOrigin(two, "custom.map-with-replacement.key"));
} }
@Test @Test
@ -76,8 +81,9 @@ class PropertiesMigrationReporterTests {
this.environment.getPropertySources().addFirst(loadPropertySource("ignore", "config/config-error.properties")); this.environment.getPropertySources().addFirst(loadPropertySource("ignore", "config/config-error.properties"));
String report = createWarningReport(loadRepository("metadata/sample-metadata.json")); String report = createWarningReport(loadRepository("metadata/sample-metadata.json"));
assertThat(report).isNotNull(); assertThat(report).isNotNull();
assertThat(report).containsSubsequence("Property source 'test'", "wrong.four.test", "Line: 5", "test.four.test", assertThat(report).containsSubsequence("Property source 'test'", "Key: custom.map-with-replacement.key",
"wrong.two", "Line: 2", "test.two"); "Line: 8", "Replacement: custom.the-map-replacement.key", "wrong.four.test", "Line: 5",
"test.four.test", "wrong.two", "Line: 2", "test.two");
assertThat(report).doesNotContain("wrong.one"); assertThat(report).doesNotContain("wrong.one");
} }
@ -119,6 +125,7 @@ class PropertiesMigrationReporterTests {
"test.time-to-live-ms", "test.time-to-live", "test.ttl", "test.mapped.ttl"); "test.time-to-live-ms", "test.time-to-live", "test.ttl", "test.mapped.ttl");
assertThat(mapToNames(propertySources)).containsExactly("migrate-test", "test", "mockProperties"); assertThat(mapToNames(propertySources)).containsExactly("migrate-test", "test", "mockProperties");
PropertySource<?> propertySource = propertySources.get("migrate-test"); PropertySource<?> propertySource = propertySources.get("migrate-test");
assertThat(propertySource).isNotNull();
assertMappedProperty(propertySource, "test.cache", 50, null); assertMappedProperty(propertySource, "test.cache", 50, null);
assertMappedProperty(propertySource, "test.time-to-live", 1234L, null); assertMappedProperty(propertySource, "test.time-to-live", 1234L, null);
assertMappedProperty(propertySource, "test.mapped.ttl", 5678L, null); assertMappedProperty(propertySource, "test.mapped.ttl", 5678L, null);
@ -151,7 +158,47 @@ class PropertiesMigrationReporterTests {
.addFirst(new MapPropertySource("first", Collections.singletonMap("invalid.property-name", "value"))); .addFirst(new MapPropertySource("first", Collections.singletonMap("invalid.property-name", "value")));
String report = createWarningReport(loadRepository("metadata/sample-metadata-invalid-name.json")); String report = createWarningReport(loadRepository("metadata/sample-metadata-invalid-name.json"));
assertThat(report).isNotNull(); assertThat(report).isNotNull();
assertThat(report).contains("Key: invalid.PropertyName").contains("Replacement: valid.property-name"); assertThat(report).contains("Key: invalid.propertyname").contains("Replacement: valid.property-name");
}
@Test
void mapPropertiesDeprecatedNoReplacement() throws IOException {
this.environment.getPropertySources().addFirst(
new MapPropertySource("first", Collections.singletonMap("custom.map-no-replacement.key", "value")));
String report = createErrorReport(loadRepository("metadata/sample-metadata.json"));
assertThat(report).isNotNull();
assertThat(report).contains("Key: custom.map-no-replacement.key")
.contains("Reason: This is no longer supported.");
}
@Test
void mapPropertiesDeprecatedWithReplacement() throws IOException {
this.environment.getPropertySources().addFirst(
new MapPropertySource("first", Collections.singletonMap("custom.map-with-replacement.key", "value")));
String report = createWarningReport(loadRepository("metadata/sample-metadata.json"));
assertThat(report).isNotNull();
assertThat(report).contains("Key: custom.map-with-replacement.key")
.contains("Replacement: custom.the-map-replacement.key");
}
@Test
void mapPropertiesDeprecatedWithReplacementRelaxedBindingUnderscore() {
this.environment.getPropertySources().addFirst(
new MapPropertySource("first", Collections.singletonMap("custom.map_with_replacement.key", "value")));
String report = createWarningReport(loadRepository("metadata/sample-metadata.json"));
assertThat(report).isNotNull();
assertThat(report).contains("Key: custom.mapwithreplacement.key")
.contains("Replacement: custom.the-map-replacement.key");
}
@Test
void mapPropertiesDeprecatedWithReplacementRelaxedBindingCamelCase() {
this.environment.getPropertySources().addFirst(
new MapPropertySource("first", Collections.singletonMap("custom.MapWithReplacement.key", "value")));
String report = createWarningReport(loadRepository("metadata/sample-metadata.json"));
assertThat(report).isNotNull();
assertThat(report).contains("Key: custom.mapwithreplacement.key")
.contains("Replacement: custom.the-map-replacement.key");
} }
private List<String> mapToNames(PropertySources sources) { private List<String> mapToNames(PropertySources sources) {

@ -3,3 +3,6 @@ wrong.two=another
wrong.four.test=value wrong.four.test=value
custom.map-with-replacement.key=value

@ -36,6 +36,24 @@
"replacement": "test.four.test", "replacement": "test.four.test",
"level": "error" "level": "error"
} }
},
{
"name": "custom.map-no-replacement",
"type": "java.util.Map<java.lang.String,java.lang.String>",
"deprecation": {
"reason": "This is no longer supported."
}
},
{
"name": "custom.map-with-replacement",
"type": "java.util.Map<java.lang.String,java.lang.String>",
"deprecation": {
"replacement": "custom.the-map-replacement"
}
},
{
"name": "custom.the-map-replacement",
"type": "java.util.Map<java.lang.String,java.lang.String>"
} }
] ]
} }
Loading…
Cancel
Save