diff --git a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReport.java b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReport.java index 1bc79db0aa7..62a6d00d39a 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReport.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReport.java @@ -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"); * 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.stream.Collectors; -import org.springframework.boot.configurationmetadata.ConfigurationMetadataProperty; - /** * Provides a properties migration report. * * @author Stephane Nicoll + * @author Moritz Halbritter */ class PropertiesMigrationReport { @@ -87,8 +86,7 @@ class PropertiesMigrationReport { report.append(String.format("Property source '%s':%n", name)); properties.sort(PropertyMigration.COMPARATOR); properties.forEach((property) -> { - ConfigurationMetadataProperty metadata = property.getMetadata(); - report.append(String.format("\tKey: %s%n", metadata.getId())); + report.append(String.format("\tKey: %s%n", property.getProperty().getName())); if (property.getLineNumber() != null) { report.append(String.format("\t\tLine: %d%n", property.getLineNumber())); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java index 28bd7d1a85c..0c608d1939a 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java @@ -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"); * 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.ConfigurationPropertySource; 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.origin.OriginTrackedValue; import org.springframework.core.env.ConfigurableEnvironment; @@ -41,6 +42,7 @@ import org.springframework.util.StringUtils; * Report on {@link PropertyMigration properties migration}. * * @author Stephane Nicoll + * @author Moritz Halbritter */ class PropertiesMigrationReporter { @@ -88,24 +90,40 @@ class PropertiesMigrationReporter { for (PropertyMigration candidate : renamed) { OriginTrackedValue value = OriginTrackedValue.of(candidate.getProperty().getValue(), candidate.getProperty().getOrigin()); - content.put(candidate.getMetadata().getDeprecation().getReplacement(), value); + content.put(candidate.getNewPropertyName(), value); } return new OriginTrackedMapPropertySource(target, content); } + private boolean isMapType(ConfigurationMetadataProperty property) { + String type = property.getType(); + return type != null && type.startsWith(Map.class.getName()); + } + private Map> getMatchingProperties( Predicate filter) { MultiValueMap result = new LinkedMultiValueMap<>(); List candidates = this.allProperties.values().stream().filter(filter) .collect(Collectors.toList()); - getPropertySourcesAsMap().forEach((name, source) -> candidates.forEach((metadata) -> { + getPropertySourcesAsMap().forEach((propertySourceName, propertySource) -> candidates.forEach((metadata) -> { ConfigurationPropertyName metadataName = ConfigurationPropertyName.isValid(metadata.getId()) ? ConfigurationPropertyName.of(metadata.getId()) : ConfigurationPropertyName.adapt(metadata.getId(), '.'); - ConfigurationProperty configurationProperty = source.getConfigurationProperty(metadataName); - if (configurationProperty != null) { - result.add(name, - new PropertyMigration(configurationProperty, metadata, determineReplacementMetadata(metadata))); + // Direct match + ConfigurationProperty match = propertySource.getConfigurationProperty(metadataName); + if (match != null) { + 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; @@ -125,8 +143,12 @@ class PropertiesMigrationReporter { private ConfigurationMetadataProperty detectMapValueReplacement(String fullId) { int lastDot = fullId.lastIndexOf('.'); - if (lastDot != -1) { - return this.allProperties.get(fullId.substring(0, lastDot)); + if (lastDot == -1) { + return null; + } + ConfigurationMetadataProperty metadata = this.allProperties.get(fullId.substring(0, lastDot)); + if (metadata != null && isMapType(metadata)) { + return metadata; } return null; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertyMigration.java b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertyMigration.java index eb184029291..5be2c95d11e 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertyMigration.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertyMigration.java @@ -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"); * 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.Deprecation; 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.TextResourceOrigin; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** * Description of a property migration. * * @author Stephane Nicoll + * @author Moritz Halbritter */ class PropertyMigration { @@ -45,14 +48,20 @@ class PropertyMigration { private final ConfigurationMetadataProperty replacementMetadata; + /** + * Whether this migration happened from a map type. + */ + private final boolean mapMigration; + private final boolean compatibleType; PropertyMigration(ConfigurationProperty property, ConfigurationMetadataProperty metadata, - ConfigurationMetadataProperty replacementMetadata) { + ConfigurationMetadataProperty replacementMetadata, boolean mapMigration) { this.property = property; this.lineNumber = determineLineNumber(property); this.metadata = metadata; this.replacementMetadata = replacementMetadata; + this.mapMigration = mapMigration; this.compatibleType = determineCompatibleType(metadata, replacementMetadata); } @@ -68,9 +77,9 @@ class PropertyMigration { private static boolean determineCompatibleType(ConfigurationMetadataProperty metadata, ConfigurationMetadataProperty replacementMetadata) { - String currentType = metadata.getType(); - String replacementType = determineReplacementType(replacementMetadata); - if (replacementType == null || currentType == null) { + String currentType = determineType(metadata); + String replacementType = determineType(replacementMetadata); + if (currentType == null || replacementType == null) { return false; } if (replacementType.equals(currentType)) { @@ -83,14 +92,15 @@ class PropertyMigration { return false; } - private static String determineReplacementType(ConfigurationMetadataProperty replacementMetadata) { - if (replacementMetadata == null || replacementMetadata.getType() == null) { + private static String determineType(ConfigurationMetadataProperty metadata) { + if (metadata == null || metadata.getType() == null) { return null; } - String candidate = replacementMetadata.getType(); + String candidate = metadata.getType(); if (candidate.startsWith(Map.class.getName())) { int lastComma = candidate.lastIndexOf(','); if (lastComma != -1) { + // Use Map value type return candidate.substring(lastComma + 1, candidate.length() - 1).trim(); } } @@ -113,9 +123,16 @@ class PropertyMigration { return this.compatibleType; } + String getNewPropertyName() { + if (this.mapMigration) { + return getNewMapPropertyName(this.property, this.metadata, this.replacementMetadata).toString(); + } + return this.metadata.getDeprecation().getReplacement(); + } + String determineReason() { if (this.compatibleType) { - return "Replacement: " + this.metadata.getDeprecation().getReplacement(); + return "Replacement: " + getNewPropertyName(); } Deprecation deprecation = this.metadata.getDeprecation(); if (StringUtils.hasText(deprecation.getShortReason())) { @@ -131,4 +148,14 @@ class PropertyMigration { 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())); + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java index f353eb923d6..44382f77ca2 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java @@ -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"); * you may not use this file except in compliance with the License. @@ -46,6 +46,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link PropertiesMigrationReporter}. * * @author Stephane Nicoll + * @author Moritz Halbritter */ class PropertiesMigrationReporterTests { @@ -67,7 +68,11 @@ class PropertiesMigrationReporterTests { assertThat(propertySources).hasSize(3); createAnalyzer(loadRepository("metadata/sample-metadata.json")).getReport(); 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 @@ -76,8 +81,9 @@ class PropertiesMigrationReporterTests { this.environment.getPropertySources().addFirst(loadPropertySource("ignore", "config/config-error.properties")); String report = createWarningReport(loadRepository("metadata/sample-metadata.json")); assertThat(report).isNotNull(); - assertThat(report).containsSubsequence("Property source 'test'", "wrong.four.test", "Line: 5", "test.four.test", - "wrong.two", "Line: 2", "test.two"); + assertThat(report).containsSubsequence("Property source 'test'", "Key: custom.map-with-replacement.key", + "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"); } @@ -119,6 +125,7 @@ class PropertiesMigrationReporterTests { "test.time-to-live-ms", "test.time-to-live", "test.ttl", "test.mapped.ttl"); assertThat(mapToNames(propertySources)).containsExactly("migrate-test", "test", "mockProperties"); PropertySource propertySource = propertySources.get("migrate-test"); + assertThat(propertySource).isNotNull(); assertMappedProperty(propertySource, "test.cache", 50, null); assertMappedProperty(propertySource, "test.time-to-live", 1234L, null); assertMappedProperty(propertySource, "test.mapped.ttl", 5678L, null); @@ -151,7 +158,47 @@ class PropertiesMigrationReporterTests { .addFirst(new MapPropertySource("first", Collections.singletonMap("invalid.property-name", "value"))); String report = createWarningReport(loadRepository("metadata/sample-metadata-invalid-name.json")); 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 mapToNames(PropertySources sources) { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/config/config-warnings.properties b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/config/config-warnings.properties index e862c7d743d..a91b52da90d 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/config/config-warnings.properties +++ b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/config/config-warnings.properties @@ -3,3 +3,6 @@ wrong.two=another wrong.four.test=value + + +custom.map-with-replacement.key=value diff --git a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/metadata/sample-metadata.json b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/metadata/sample-metadata.json index e7ac0c8d687..4c76e6cbaa8 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/metadata/sample-metadata.json +++ b/spring-boot-project/spring-boot-tools/spring-boot-properties-migrator/src/test/resources/metadata/sample-metadata.json @@ -36,6 +36,24 @@ "replacement": "test.four.test", "level": "error" } + }, + { + "name": "custom.map-no-replacement", + "type": "java.util.Map", + "deprecation": { + "reason": "This is no longer supported." + } + }, + { + "name": "custom.map-with-replacement", + "type": "java.util.Map", + "deprecation": { + "replacement": "custom.the-map-replacement" + } + }, + { + "name": "custom.the-map-replacement", + "type": "java.util.Map" } ] -} \ No newline at end of file +}