mirror of
https://github.com/spring-projects/spring-boot.git
synced 2024-09-03 04:26:12 +08:00
Merge pull request #15394 from hdeadman
* pr/15394: Polish "Avoid NPE when replacement property does not exist" Avoid NPE when replacement property does not exist
This commit is contained in:
commit
06be9053f6
@ -17,7 +17,6 @@
|
||||
package org.springframework.boot.context.properties.migrator;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@ -25,8 +24,6 @@ import java.util.function.Function;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import org.springframework.boot.configurationmetadata.ConfigurationMetadataProperty;
|
||||
import org.springframework.boot.configurationmetadata.Deprecation;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
||||
/**
|
||||
* Provides a properties migration report.
|
||||
@ -51,8 +48,7 @@ class PropertiesMigrationReport {
|
||||
StringBuilder report = new StringBuilder();
|
||||
report.append(String.format("%nThe use of configuration keys that have been "
|
||||
+ "renamed was found in the environment:%n%n"));
|
||||
append(report, content, (metadata) -> "Replacement: "
|
||||
+ metadata.getDeprecation().getReplacement());
|
||||
append(report, content);
|
||||
report.append(String.format("%n"));
|
||||
report.append("Each configuration key has been temporarily mapped to its "
|
||||
+ "replacement for your convenience. To silence this warning, please "
|
||||
@ -75,7 +71,7 @@ class PropertiesMigrationReport {
|
||||
StringBuilder report = new StringBuilder();
|
||||
report.append(String.format("%nThe use of configuration keys that are no longer "
|
||||
+ "supported was found in the environment:%n%n"));
|
||||
append(report, content, this::determineReason);
|
||||
append(report, content);
|
||||
report.append(String.format("%n"));
|
||||
report.append("Please refer to the migration guide or reference guide for "
|
||||
+ "potential alternatives.");
|
||||
@ -83,19 +79,6 @@ class PropertiesMigrationReport {
|
||||
return report.toString();
|
||||
}
|
||||
|
||||
private String determineReason(ConfigurationMetadataProperty metadata) {
|
||||
Deprecation deprecation = metadata.getDeprecation();
|
||||
if (StringUtils.hasText(deprecation.getShortReason())) {
|
||||
return "Reason: " + deprecation.getShortReason();
|
||||
}
|
||||
if (StringUtils.hasText(deprecation.getReplacement())) {
|
||||
return String.format(
|
||||
"Reason: Replacement key '%s' uses an incompatible " + "target type",
|
||||
deprecation.getReplacement());
|
||||
}
|
||||
return "Reason: none";
|
||||
}
|
||||
|
||||
private Map<String, List<PropertyMigration>> getContent(
|
||||
Function<LegacyProperties, List<PropertyMigration>> extractor) {
|
||||
return this.content.entrySet().stream()
|
||||
@ -105,8 +88,7 @@ class PropertiesMigrationReport {
|
||||
}
|
||||
|
||||
private void append(StringBuilder report,
|
||||
Map<String, List<PropertyMigration>> content,
|
||||
Function<ConfigurationMetadataProperty, String> deprecationMessage) {
|
||||
Map<String, List<PropertyMigration>> content) {
|
||||
content.forEach((name, properties) -> {
|
||||
report.append(String.format("Property source '%s':%n", name));
|
||||
properties.sort(PropertyMigration.COMPARATOR);
|
||||
@ -117,8 +99,7 @@ class PropertiesMigrationReport {
|
||||
report.append(
|
||||
String.format("\t\tLine: %d%n", property.getLineNumber()));
|
||||
}
|
||||
report.append(
|
||||
String.format("\t\t%s%n", deprecationMessage.apply(metadata)));
|
||||
report.append(String.format("\t\t%s%n", property.determineReason()));
|
||||
});
|
||||
report.append(String.format("%n"));
|
||||
});
|
||||
@ -127,36 +108,29 @@ class PropertiesMigrationReport {
|
||||
/**
|
||||
* Register a new property source.
|
||||
* @param name the name of the property source
|
||||
* @param renamed the properties that were renamed
|
||||
* @param unsupported the properties that are no longer supported
|
||||
* @param properties the {@link PropertyMigration} instances
|
||||
*/
|
||||
void add(String name, List<PropertyMigration> renamed,
|
||||
List<PropertyMigration> unsupported) {
|
||||
this.content.put(name, new LegacyProperties(renamed, unsupported));
|
||||
void add(String name, List<PropertyMigration> properties) {
|
||||
this.content.put(name, new LegacyProperties(properties));
|
||||
}
|
||||
|
||||
private static class LegacyProperties {
|
||||
|
||||
private final List<PropertyMigration> renamed;
|
||||
private final List<PropertyMigration> properties;
|
||||
|
||||
private final List<PropertyMigration> unsupported;
|
||||
|
||||
LegacyProperties(List<PropertyMigration> renamed,
|
||||
List<PropertyMigration> unsupported) {
|
||||
this.renamed = asNewList(renamed);
|
||||
this.unsupported = asNewList(unsupported);
|
||||
}
|
||||
|
||||
private <T> List<T> asNewList(List<T> source) {
|
||||
return (source != null) ? new ArrayList<>(source) : Collections.emptyList();
|
||||
LegacyProperties(List<PropertyMigration> properties) {
|
||||
this.properties = new ArrayList<>(properties);
|
||||
}
|
||||
|
||||
public List<PropertyMigration> getRenamed() {
|
||||
return this.renamed;
|
||||
return this.properties.stream().filter(PropertyMigration::isCompatibleType)
|
||||
.collect(Collectors.toList());
|
||||
}
|
||||
|
||||
public List<PropertyMigration> getUnsupported() {
|
||||
return this.unsupported;
|
||||
return this.properties.stream()
|
||||
.filter((property) -> !property.isCompatibleType())
|
||||
.collect(Collectors.toList());
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -16,8 +16,6 @@
|
||||
|
||||
package org.springframework.boot.context.properties.migrator;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
@ -83,11 +81,9 @@ class PropertiesMigrationReporter {
|
||||
private PropertySource<?> mapPropertiesWithReplacement(
|
||||
PropertiesMigrationReport report, String name,
|
||||
List<PropertyMigration> properties) {
|
||||
List<PropertyMigration> renamed = new ArrayList<>();
|
||||
List<PropertyMigration> unsupported = new ArrayList<>();
|
||||
properties.forEach((property) -> (isRenamed(property) ? renamed : unsupported)
|
||||
.add(property));
|
||||
report.add(name, renamed, unsupported);
|
||||
report.add(name, properties);
|
||||
List<PropertyMigration> renamed = properties.stream()
|
||||
.filter(PropertyMigration::isCompatibleType).collect(Collectors.toList());
|
||||
if (renamed.isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
@ -102,52 +98,6 @@ class PropertiesMigrationReporter {
|
||||
return new OriginTrackedMapPropertySource(target, content);
|
||||
}
|
||||
|
||||
private boolean isRenamed(PropertyMigration property) {
|
||||
ConfigurationMetadataProperty metadata = property.getMetadata();
|
||||
String replacementId = metadata.getDeprecation().getReplacement();
|
||||
if (StringUtils.hasText(replacementId)) {
|
||||
ConfigurationMetadataProperty replacement = this.allProperties
|
||||
.get(replacementId);
|
||||
if (replacement != null) {
|
||||
return isCompatibleType(metadata.getType(), replacement.getType());
|
||||
}
|
||||
return isCompatibleType(metadata.getType(),
|
||||
detectMapValueReplacementType(replacementId));
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean isCompatibleType(String currentType, String replacementType) {
|
||||
if (replacementType == null || currentType == null) {
|
||||
return false;
|
||||
}
|
||||
if (replacementType.equals(currentType)) {
|
||||
return true;
|
||||
}
|
||||
if (replacementType.equals(Duration.class.getName())
|
||||
&& (currentType.equals(Long.class.getName())
|
||||
|| currentType.equals(Integer.class.getName()))) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private String detectMapValueReplacementType(String fullId) {
|
||||
int lastDot = fullId.lastIndexOf('.');
|
||||
if (lastDot != -1) {
|
||||
ConfigurationMetadataProperty property = this.allProperties
|
||||
.get(fullId.substring(0, lastDot));
|
||||
String type = property.getType();
|
||||
if (type != null && type.startsWith(Map.class.getName())) {
|
||||
int lastComma = type.lastIndexOf(',');
|
||||
if (lastComma != -1) {
|
||||
return type.substring(lastComma + 1, type.length() - 1).trim();
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private Map<String, List<PropertyMigration>> getMatchingProperties(
|
||||
Predicate<ConfigurationMetadataProperty> filter) {
|
||||
MultiValueMap<String, PropertyMigration> result = new LinkedMultiValueMap<>();
|
||||
@ -159,14 +109,36 @@ class PropertiesMigrationReporter {
|
||||
.getConfigurationProperty(
|
||||
ConfigurationPropertyName.of(metadata.getId()));
|
||||
if (configurationProperty != null) {
|
||||
result.add(name,
|
||||
new PropertyMigration(metadata, configurationProperty));
|
||||
result.add(name, new PropertyMigration(configurationProperty,
|
||||
metadata, determineReplacementMetadata(metadata)));
|
||||
}
|
||||
});
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
private ConfigurationMetadataProperty determineReplacementMetadata(
|
||||
ConfigurationMetadataProperty metadata) {
|
||||
String replacementId = metadata.getDeprecation().getReplacement();
|
||||
if (StringUtils.hasText(replacementId)) {
|
||||
ConfigurationMetadataProperty replacement = this.allProperties
|
||||
.get(replacementId);
|
||||
if (replacement != null) {
|
||||
return replacement;
|
||||
}
|
||||
return detectMapValueReplacement(replacementId);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private ConfigurationMetadataProperty detectMapValueReplacement(String fullId) {
|
||||
int lastDot = fullId.lastIndexOf('.');
|
||||
if (lastDot != -1) {
|
||||
return this.allProperties.get(fullId.substring(0, lastDot));
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private Predicate<ConfigurationMetadataProperty> deprecatedFilter() {
|
||||
return (property) -> property.getDeprecation() != null
|
||||
&& property.getDeprecation().getLevel() == Deprecation.Level.ERROR;
|
||||
|
@ -16,12 +16,16 @@
|
||||
|
||||
package org.springframework.boot.context.properties.migrator;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.util.Comparator;
|
||||
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.origin.Origin;
|
||||
import org.springframework.boot.origin.TextResourceOrigin;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
||||
/**
|
||||
* Description of a property migration.
|
||||
@ -33,17 +37,24 @@ class PropertyMigration {
|
||||
public static final Comparator<PropertyMigration> COMPARATOR = Comparator
|
||||
.comparing((property) -> property.getMetadata().getId());
|
||||
|
||||
private final ConfigurationMetadataProperty metadata;
|
||||
|
||||
private final ConfigurationProperty property;
|
||||
|
||||
private final Integer lineNumber;
|
||||
|
||||
PropertyMigration(ConfigurationMetadataProperty metadata,
|
||||
ConfigurationProperty property) {
|
||||
this.metadata = metadata;
|
||||
private final ConfigurationMetadataProperty metadata;
|
||||
|
||||
private final ConfigurationMetadataProperty replacementMetadata;
|
||||
|
||||
private final boolean compatibleType;
|
||||
|
||||
PropertyMigration(ConfigurationProperty property,
|
||||
ConfigurationMetadataProperty metadata,
|
||||
ConfigurationMetadataProperty replacementMetadata) {
|
||||
this.property = property;
|
||||
this.lineNumber = determineLineNumber(property);
|
||||
this.metadata = metadata;
|
||||
this.replacementMetadata = replacementMetadata;
|
||||
this.compatibleType = determineCompatibleType(metadata, replacementMetadata);
|
||||
}
|
||||
|
||||
private static Integer determineLineNumber(ConfigurationProperty property) {
|
||||
@ -57,8 +68,37 @@ class PropertyMigration {
|
||||
return null;
|
||||
}
|
||||
|
||||
public ConfigurationMetadataProperty getMetadata() {
|
||||
return this.metadata;
|
||||
private static boolean determineCompatibleType(ConfigurationMetadataProperty metadata,
|
||||
ConfigurationMetadataProperty replacementMetadata) {
|
||||
String currentType = metadata.getType();
|
||||
String replacementType = determineReplacementType(replacementMetadata);
|
||||
if (replacementType == null || currentType == null) {
|
||||
return false;
|
||||
}
|
||||
if (replacementType.equals(currentType)) {
|
||||
return true;
|
||||
}
|
||||
if (replacementType.equals(Duration.class.getName())
|
||||
&& (currentType.equals(Long.class.getName())
|
||||
|| currentType.equals(Integer.class.getName()))) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private static String determineReplacementType(
|
||||
ConfigurationMetadataProperty replacementMetadata) {
|
||||
if (replacementMetadata == null || replacementMetadata.getType() == null) {
|
||||
return null;
|
||||
}
|
||||
String candidate = replacementMetadata.getType();
|
||||
if (candidate.startsWith(Map.class.getName())) {
|
||||
int lastComma = candidate.lastIndexOf(',');
|
||||
if (lastComma != -1) {
|
||||
return candidate.substring(lastComma + 1, candidate.length() - 1).trim();
|
||||
}
|
||||
}
|
||||
return candidate;
|
||||
}
|
||||
|
||||
public ConfigurationProperty getProperty() {
|
||||
@ -69,4 +109,34 @@ class PropertyMigration {
|
||||
return this.lineNumber;
|
||||
}
|
||||
|
||||
public ConfigurationMetadataProperty getMetadata() {
|
||||
return this.metadata;
|
||||
}
|
||||
|
||||
public boolean isCompatibleType() {
|
||||
return this.compatibleType;
|
||||
}
|
||||
|
||||
public String determineReason() {
|
||||
if (this.compatibleType) {
|
||||
return "Replacement: " + this.metadata.getDeprecation().getReplacement();
|
||||
}
|
||||
Deprecation deprecation = this.metadata.getDeprecation();
|
||||
if (StringUtils.hasText(deprecation.getShortReason())) {
|
||||
return "Reason: " + deprecation.getShortReason();
|
||||
}
|
||||
if (StringUtils.hasText(deprecation.getReplacement())) {
|
||||
if (this.replacementMetadata != null) {
|
||||
return String.format(
|
||||
"Reason: Replacement key '%s' uses an incompatible target type",
|
||||
deprecation.getReplacement());
|
||||
}
|
||||
else {
|
||||
return String.format("Reason: No metadata found for replacement key '%s'",
|
||||
deprecation.getReplacement());
|
||||
}
|
||||
}
|
||||
return "Reason: none";
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -152,6 +152,19 @@ public class PropertiesMigrationReporterTests {
|
||||
+ "'test.inconvertible' uses an incompatible target type");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void invalidReplacementHandled() throws IOException {
|
||||
this.environment.getPropertySources().addFirst(loadPropertySource("first",
|
||||
"config/config-error-invalid-replacement.properties"));
|
||||
String report = createErrorReport(
|
||||
loadRepository("metadata/sample-metadata-invalid-replacement.json"));
|
||||
assertThat(report).isNotNull();
|
||||
assertThat(report).containsSubsequence("Property source 'first'",
|
||||
"deprecated.six.test", "Line: 1", "Reason",
|
||||
"No metadata found for replacement key 'does.not.exist'");
|
||||
assertThat(report).doesNotContain("null");
|
||||
}
|
||||
|
||||
private List<String> mapToNames(PropertySources sources) {
|
||||
List<String> names = new ArrayList<>();
|
||||
for (PropertySource<?> source : sources) {
|
||||
|
@ -0,0 +1 @@
|
||||
deprecated.six.test=abc
|
@ -0,0 +1,12 @@
|
||||
{
|
||||
"properties": [
|
||||
{
|
||||
"name": "deprecated.six.test",
|
||||
"type": "java.lang.String",
|
||||
"deprecation": {
|
||||
"replacement": "does.not.exist",
|
||||
"level": "error"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
Loading…
Reference in New Issue
Block a user