From a62a27e686eaeca85434c73a48e3609e0395a9ec Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 8 Jan 2018 10:57:19 -0800 Subject: [PATCH] Polish InvalidConfigurationPropertyValueException --- ...idConfigurationPropertyValueException.java | 6 +- ...igurationPropertyValueFailureAnalyzer.java | 138 ++++++++++-------- ...tionPropertyValueFailureAnalyzerTests.java | 32 ++-- 3 files changed, 102 insertions(+), 74 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/InvalidConfigurationPropertyValueException.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/InvalidConfigurationPropertyValueException.java index 534b136eab..47e8845a41 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/InvalidConfigurationPropertyValueException.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/InvalidConfigurationPropertyValueException.java @@ -23,8 +23,7 @@ package org.springframework.boot.context.properties.source; * @since 2.0.0 */ @SuppressWarnings("serial") -public class InvalidConfigurationPropertyValueException - extends RuntimeException { +public class InvalidConfigurationPropertyValueException extends RuntimeException { private final String name; @@ -34,8 +33,7 @@ public class InvalidConfigurationPropertyValueException public InvalidConfigurationPropertyValueException(String name, Object value, String reason) { - super(String.format("Property %s with value '%s' is invalid: %s", name, - value, reason)); + super("Property " + name + " with value '" + value + "' is invalid: " + reason); this.name = name; this.value = value; this.reason = reason; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzer.java index 77f83df144..767778c875 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzer.java @@ -16,16 +16,18 @@ package org.springframework.boot.diagnostics.analyzer; -import java.util.ArrayList; -import java.util.LinkedHashMap; +import java.util.Collections; import java.util.List; -import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import org.springframework.boot.context.properties.source.ConfigurationPropertySources; import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException; import org.springframework.boot.diagnostics.AbstractFailureAnalyzer; import org.springframework.boot.diagnostics.FailureAnalysis; import org.springframework.boot.diagnostics.FailureAnalyzer; +import org.springframework.boot.origin.Origin; import org.springframework.boot.origin.OriginLookup; import org.springframework.context.EnvironmentAware; import org.springframework.core.env.ConfigurableEnvironment; @@ -36,7 +38,7 @@ import org.springframework.util.StringUtils; /** * A {@link FailureAnalyzer} that performs analysis of failures caused by a * {@link InvalidConfigurationPropertyValueException}. - + * * @author Stephane Nicoll */ class InvalidConfigurationPropertyValueFailureAnalyzer @@ -53,94 +55,114 @@ class InvalidConfigurationPropertyValueFailureAnalyzer @Override protected FailureAnalysis analyze(Throwable rootFailure, InvalidConfigurationPropertyValueException cause) { - List descriptors = getPropertySourceDescriptors(cause.getName()); + List descriptors = getDescriptors(cause.getName()); if (descriptors.isEmpty()) { return null; } - PropertyValueDescriptor main = descriptors.get(0); - StringBuilder message = new StringBuilder(); - message.append(String.format("The value '%s'", main.value)); - if (main.origin != null) { - message.append(String.format(" from origin '%s'", main.origin)); - } - message.append(String.format(" of configuration property '%s' was invalid. ", - cause.getName())); + StringBuilder description = new StringBuilder(); + appendDetails(description, cause, descriptors); + appendReason(description, cause); + appendAdditionalProperties(description, descriptors); + return new FailureAnalysis(description.toString(), getAction(cause), cause); + } + + private List getDescriptors(String propertyName) { + return getPropertySources() + .filter((source) -> source.containsProperty(propertyName)) + .map((source) -> Descriptor.get(source, propertyName)) + .collect(Collectors.toList()); + } + + private Stream> getPropertySources() { + Iterable> sources = (this.environment == null + ? Collections.emptyList() : this.environment.getPropertySources()); + return StreamSupport.stream(sources.spliterator(), false) + .filter((source) -> !ConfigurationPropertySources + .isAttachedConfigurationPropertySource(source)); + } + + private void appendDetails(StringBuilder message, + InvalidConfigurationPropertyValueException cause, + List descriptors) { + Descriptor mainDescriptor = descriptors.get(0); + message.append("Invalid value '" + mainDescriptor.getValue() + + "' for configuration property '" + cause.getName() + "'"); + mainDescriptor.appendOrigin(message); + message.append("."); + } + + private void appendReason(StringBuilder message, + InvalidConfigurationPropertyValueException cause) { if (StringUtils.hasText(cause.getReason())) { - message.append(String.format( - "Validation failed for the following reason:%n%n")); + message.append(" Validation failed for the following reason\n\n"); message.append(cause.getReason()); } else { - message.append("No reason was provided."); + message.append(" No reason was provided."); } - List others = descriptors.subList(1, descriptors.size()); + } + + private void appendAdditionalProperties(StringBuilder message, + List descriptors) { + List others = descriptors.subList(1, descriptors.size()); if (!others.isEmpty()) { message.append(String.format( "%n%nAdditionally, this property is also set in the following " + "property %s:%n%n", others.size() > 1 ? "sources" : "source")); - for (PropertyValueDescriptor other : others) { - message.append(String.format("\t- %s: %s%n", other.propertySource, - other.value)); + for (Descriptor other : others) { + message.append("\t- In '" + other.getPropertySource() + "'"); + message.append(" with the value '" + other.getValue() + "'"); + other.appendOrigin(message); + message.append(".\n"); } } + } + + private String getAction(InvalidConfigurationPropertyValueException cause) { StringBuilder action = new StringBuilder(); action.append("Review the value of the property"); if (cause.getReason() != null) { action.append(" with the provided reason"); } action.append("."); - return new FailureAnalysis(message.toString(), action.toString(), cause); + return action.toString(); } - private List getPropertySourceDescriptors( - String propertyName) { - List propertySources = new ArrayList<>(); - getPropertySourcesAsMap() - .forEach((sourceName, source) -> { - if (source.containsProperty(propertyName)) { - propertySources.add(describeValueOf(propertyName, source)); - } - }); - return propertySources; - } - - private Map> getPropertySourcesAsMap() { - Map> map = new LinkedHashMap<>(); - if (this.environment != null) { - for (PropertySource source : this.environment.getPropertySources()) { - if (!ConfigurationPropertySources - .isAttachedConfigurationPropertySource(source)) { - map.put(source.getName(), source); - } - } - } - return map; - } - - private PropertyValueDescriptor describeValueOf(String name, - PropertySource source) { - Object value = source.getProperty(name); - String origin = (source instanceof OriginLookup) - ? ((OriginLookup) source).getOrigin(name).toString() : null; - return new PropertyValueDescriptor(source.getName(), value, origin); - } - - private static class PropertyValueDescriptor { + private static final class Descriptor { private final String propertySource; private final Object value; - private final String origin; + private final Origin origin; - PropertyValueDescriptor(String propertySource, - Object value, String origin) { + private Descriptor(String propertySource, Object value, Origin origin) { this.propertySource = propertySource; this.value = value; this.origin = origin; } + public String getPropertySource() { + return this.propertySource; + } + + public Object getValue() { + return this.value; + } + + public void appendOrigin(StringBuilder message) { + if (this.origin != null) { + message.append(" (originating from '" + this.origin + "')"); + } + } + + static Descriptor get(PropertySource source, String propertyName) { + Object value = source.getProperty(propertyName); + Origin origin = OriginLookup.getOrigin(source, propertyName); + return new Descriptor(source.getName(), value, origin); + } + } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzerTests.java index c18358a3f0..4c37b06631 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzerTests.java @@ -53,7 +53,8 @@ public class InvalidConfigurationPropertyValueFailureAnalyzerTests { public void analysisWithKnownProperty() { MapPropertySource source = new MapPropertySource("test", Collections.singletonMap("test.property", "invalid")); - this.environment.getPropertySources().addFirst(new OriginCapablePropertySource(source)); + this.environment.getPropertySources() + .addFirst(OriginCapablePropertySource.get(source)); InvalidConfigurationPropertyValueException failure = new InvalidConfigurationPropertyValueException( "test.property", "invalid", "this is not valid"); FailureAnalysis analysis = performAnalysis(failure); @@ -68,13 +69,13 @@ public class InvalidConfigurationPropertyValueFailureAnalyzerTests { public void analysisWithKnownPropertyAndNoReason() { MapPropertySource source = new MapPropertySource("test", Collections.singletonMap("test.property", "invalid")); - this.environment.getPropertySources().addFirst(new OriginCapablePropertySource(source)); + this.environment.getPropertySources() + .addFirst(OriginCapablePropertySource.get(source)); InvalidConfigurationPropertyValueException failure = new InvalidConfigurationPropertyValueException( "test.property", "invalid", null); FailureAnalysis analysis = performAnalysis(failure); assertCommonParts(failure, analysis); - assertThat(analysis.getDescription()) - .contains("No reason was provided.") + assertThat(analysis.getDescription()).contains("No reason was provided.") .doesNotContain("Additionally, this property is also set"); } @@ -86,17 +87,20 @@ public class InvalidConfigurationPropertyValueFailureAnalyzerTests { Collections.singletonMap("test.property", "valid")); MapPropertySource another = new MapPropertySource("another", Collections.singletonMap("test.property", "test")); - this.environment.getPropertySources().addFirst(new OriginCapablePropertySource(source)); + this.environment.getPropertySources() + .addFirst(OriginCapablePropertySource.get(source)); this.environment.getPropertySources().addLast(additional); - this.environment.getPropertySources().addLast(another); + this.environment.getPropertySources() + .addLast(OriginCapablePropertySource.get(another)); InvalidConfigurationPropertyValueException failure = new InvalidConfigurationPropertyValueException( "test.property", "invalid", "this is not valid"); FailureAnalysis analysis = performAnalysis(failure); assertCommonParts(failure, analysis); assertThat(analysis.getDescription()) - .contains("Additionally, this property is also set in the following " + - "property sources:") - .contains("additional: valid").contains("another: test"); + .contains("Additionally, this property is also set in the following " + + "property sources:") + .contains("In 'additional' with the value 'valid'").contains( + "In 'another' with the value 'test' (originating from 'TestOrigin test.property')"); } @Test @@ -115,7 +119,6 @@ public class InvalidConfigurationPropertyValueFailureAnalyzerTests { assertThat(analysis.getCause()).isSameAs(failure); } - private FailureAnalysis performAnalysis( InvalidConfigurationPropertyValueException failure) { InvalidConfigurationPropertyValueFailureAnalyzer analyzer = new InvalidConfigurationPropertyValueFailureAnalyzer(); @@ -127,8 +130,8 @@ public class InvalidConfigurationPropertyValueFailureAnalyzerTests { return analysis; } - private static class OriginCapablePropertySource - extends EnumerablePropertySource implements OriginLookup { + static class OriginCapablePropertySource extends EnumerablePropertySource + implements OriginLookup { private final EnumerablePropertySource propertySource; @@ -159,6 +162,11 @@ public class InvalidConfigurationPropertyValueFailureAnalyzerTests { }; } + static OriginCapablePropertySource get( + EnumerablePropertySource propertySource) { + return new OriginCapablePropertySource<>(propertySource); + } + } }