diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java index 89fee1f2b6..671ac1833f 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java @@ -18,6 +18,7 @@ package org.springframework.boot.context.properties.source; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.Function; @@ -174,6 +175,7 @@ public final class ConfigurationPropertyName * value. * @param elementValue the single element value to append * @return a new {@link ConfigurationPropertyName} + * @throws InvalidConfigurationPropertyNameException if elementValue is not valid */ public ConfigurationPropertyName append(String elementValue) { if (elementValue == null) { @@ -182,9 +184,12 @@ public final class ConfigurationPropertyName process(elementValue, '.', (value, start, end, indexed) -> Assert.isTrue( start == 0, () -> "Element value '" + elementValue + "' must be a single item")); - Assert.isTrue( - isIndexed(elementValue) || ElementValidator.isValidElement(elementValue), - () -> "Element value '" + elementValue + "' is not valid"); + if (!isIndexed(elementValue)) { + List invalidChars = ElementValidator.getInvalidChars(elementValue); + if (!invalidChars.isEmpty()) { + throw new InvalidConfigurationPropertyNameException(invalidChars, elementValue); + } + } int length = this.elements.length; CharSequence[] elements = new CharSequence[length + 1]; System.arraycopy(this.elements, 0, elements, 0, length); @@ -436,14 +441,13 @@ public final class ConfigurationPropertyName * Return a {@link ConfigurationPropertyName} for the specified string. * @param name the source name * @return a {@link ConfigurationPropertyName} instance - * @throws IllegalArgumentException if the name is not valid + * @throws InvalidConfigurationPropertyNameException if the name is not valid */ public static ConfigurationPropertyName of(CharSequence name) { Assert.notNull(name, "Name must not be null"); if (name.length() >= 1 && (name.charAt(0) == '.' || name.charAt(name.length() - 1) == '.')) { - throw new IllegalArgumentException( - "Configuration property name '" + name + "' is not valid"); + throw new InvalidConfigurationPropertyNameException(Collections.singletonList('.'), name); } if (name.length() == 0) { return EMPTY; @@ -451,8 +455,12 @@ public final class ConfigurationPropertyName List elements = new ArrayList(10); process(name, '.', (elementValue, start, end, indexed) -> { if (elementValue.length() > 0) { - Assert.isTrue(indexed || ElementValidator.isValidElement(elementValue), - () -> "Configuration property name '" + name + "' is not valid"); + if (!indexed) { + List invalidChars = ElementValidator.getInvalidChars(elementValue); + if (!invalidChars.isEmpty()) { + throw new InvalidConfigurationPropertyNameException(invalidChars, name); + } + } elements.add(elementValue); } }); @@ -651,12 +659,7 @@ public final class ConfigurationPropertyName } public static boolean isValidElement(CharSequence elementValue) { - for (int i = 0; i < elementValue.length(); i++) { - if (!isValidChar(elementValue.charAt(i), i)) { - return false; - } - } - return true; + return getInvalidChars(elementValue).isEmpty(); } public static boolean isValidChar(char ch, int index) { @@ -668,6 +671,17 @@ public final class ConfigurationPropertyName return isAlpha || isNumeric || ch == '-'; } + private static List getInvalidChars(CharSequence elementValue) { + List chars = new ArrayList<>(); + for (int i = 0; i < elementValue.length(); i++) { + char ch = elementValue.charAt(i); + if (!isValidChar(ch, i)) { + chars.add(ch); + } + } + return chars; + } + } } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/InvalidConfigurationPropertyNameException.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/InvalidConfigurationPropertyNameException.java new file mode 100644 index 0000000000..a30bfa1200 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/InvalidConfigurationPropertyNameException.java @@ -0,0 +1,49 @@ +/* + * Copyright 2012-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.properties.source; + +import java.util.List; + +/** + * Exception thrown when {@link ConfigurationPropertyName} has invalid + * characters. + * + * @author Madhura Bhave + * @since 2.0.0 + */ +public class InvalidConfigurationPropertyNameException extends RuntimeException { + + private final List invalidCharacters; + + private final CharSequence name; + + public InvalidConfigurationPropertyNameException(List invalidCharacters, CharSequence name) { + super("Configuration property name '" + name + "' is not valid"); + this.invalidCharacters = invalidCharacters; + this.name = name; + } + + public List getInvalidCharacters() { + return this.invalidCharacters; + } + + public CharSequence getName() { + return this.name; + } + +} + diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyNameFailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyNameFailureAnalyzer.java new file mode 100644 index 0000000000..62a65c6546 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyNameFailureAnalyzer.java @@ -0,0 +1,57 @@ +/* + * Copyright 2012-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.diagnostics.analyzer; + +import java.util.stream.Collectors; + +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyNameException; +import org.springframework.boot.diagnostics.AbstractFailureAnalyzer; +import org.springframework.boot.diagnostics.FailureAnalysis; + +/** + * An {@link AbstractFailureAnalyzer} that performs analysis of failures + * caused by {@link InvalidConfigurationPropertyNameException}. + * + * @author Madhura Bhave + * @since 2.0.0 + */ +public class InvalidConfigurationPropertyNameFailureAnalyzer extends AbstractFailureAnalyzer { + + @Override + protected FailureAnalysis analyze(Throwable rootFailure, InvalidConfigurationPropertyNameException cause) { + BeanCreationException exception = findCause(rootFailure, BeanCreationException.class); + String description = buildDescription(cause, exception); + String action = String.format("Modify '%s' so that it conforms to the canonical names requirements.", cause.getName()); + return new FailureAnalysis(description, action, cause); + } + + private String buildDescription(InvalidConfigurationPropertyNameException cause, BeanCreationException exception) { + StringBuilder description = new StringBuilder( + String.format("Configuration property name '%s' is not valid:%n", cause.getName())); + String invalid = cause.getInvalidCharacters().stream().map(c -> "'" + c.toString() + "'").collect(Collectors.joining(",")); + description.append(String.format("%n Invalid characters: %s", invalid)); + if (exception != null) { + description.append(String.format("%n Bean: %s", exception.getBeanName())); + } + description.append(String.format("%n Reason: Canonical names should be kebab-case('-' separated), lowercase alpha-numeric characters" + + " and must start with a letter")); + return description.toString(); + } + +} + diff --git a/spring-boot/src/main/resources/META-INF/spring.factories b/spring-boot/src/main/resources/META-INF/spring.factories index 10cb4861d1..5603c58b66 100644 --- a/spring-boot/src/main/resources/META-INF/spring.factories +++ b/spring-boot/src/main/resources/META-INF/spring.factories @@ -46,7 +46,8 @@ org.springframework.boot.diagnostics.analyzer.UnboundConfigurationPropertyFailur org.springframework.boot.diagnostics.analyzer.ConnectorStartFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.NoUniqueBeanDefinitionFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.PortInUseFailureAnalyzer,\ -org.springframework.boot.diagnostics.analyzer.ValidationExceptionFailureAnalyzer +org.springframework.boot.diagnostics.analyzer.ValidationExceptionFailureAnalyzer,\ +org.springframework.boot.diagnostics.analyzer.InvalidConfigurationPropertyNameFailureAnalyzer # FailureAnalysisReporters org.springframework.boot.diagnostics.FailureAnalysisReporter=\ diff --git a/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java index c4bf8525ca..043e9cf99d 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java @@ -51,35 +51,35 @@ public class ConfigurationPropertyNameTests { @Test public void ofNameShouldNotStartWithNumber() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("1foo"); } @Test public void ofNameShouldNotStartWithDash() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("-foo"); } @Test public void ofNameShouldNotStartWithDot() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of(".foo"); } @Test public void ofNameShouldNotEndWithDot() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("foo."); } @Test public void ofNameShouldNotContainUppercase() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("fOo"); } @@ -92,7 +92,7 @@ public class ConfigurationPropertyNameTests { ConfigurationPropertyName.of("foo" + c); fail("Did not throw for invalid char " + c); } - catch (IllegalArgumentException ex) { + catch (InvalidConfigurationPropertyNameException ex) { assertThat(ex.getMessage()).contains("is not valid"); } } @@ -163,21 +163,21 @@ public class ConfigurationPropertyNameTests { @Test public void ofNameWhenMissingCloseBracket() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("[bar"); } @Test public void ofNameWhenMissingOpenBracket() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("bar]"); } @Test public void ofNameWhenMultipleMismatchedBrackets() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("[a[[[b]ar]"); } @@ -192,7 +192,7 @@ public class ConfigurationPropertyNameTests { @Test public void ofNameWithWhitespaceInName() throws Exception { - this.thrown.expect(IllegalArgumentException.class); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); this.thrown.expectMessage("is not valid"); ConfigurationPropertyName.of("foo. bar"); } @@ -420,8 +420,8 @@ public class ConfigurationPropertyNameTests { @Test public void appendWhenElementNameIsNotValidShouldThrowException() throws Exception { - this.thrown.expect(IllegalArgumentException.class); - this.thrown.expectMessage("Element value '1bar' is not valid"); + this.thrown.expect(InvalidConfigurationPropertyNameException.class); + this.thrown.expectMessage("Configuration property name '1bar' is not valid"); ConfigurationPropertyName.of("foo").append("1bar"); } diff --git a/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyNameFailureAnalyzerTests.java b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyNameFailureAnalyzerTests.java new file mode 100644 index 0000000000..e2cd731dc0 --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyNameFailureAnalyzerTests.java @@ -0,0 +1,86 @@ +/* + * Copyright 2012-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.diagnostics.analyzer; + +import org.junit.Test; + +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.diagnostics.FailureAnalysis; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link InvalidConfigurationPropertyNameFailureAnalyzer}. + * + * @author Madhura Bhave + */ +public class InvalidConfigurationPropertyNameFailureAnalyzerTests { + + private InvalidConfigurationPropertyNameFailureAnalyzer analyzer = new InvalidConfigurationPropertyNameFailureAnalyzer(); + + @Test + public void analysisWhenRootCauseIsBeanCreationFailureShouldContainBeanName() throws Exception { + BeanCreationException failure = createFailure(InvalidPrefixConfiguration.class); + FailureAnalysis analysis = this.analyzer.analyze(failure); + assertThat(analysis.getDescription()).contains(String.format( + "%n Invalid characters: %s%n Bean: %s%n Reason: %s", + "'F','P'", "invalidPrefixProperties", "Canonical names should be kebab-case('-' separated), " + + "lowercase alpha-numeric characters and must start with a letter")); + } + + private BeanCreationException createFailure(Class configuration) { + try { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(configuration); + context.refresh(); + context.close(); + return null; + } + catch (BeanCreationException ex) { + return ex; + } + } + + @EnableConfigurationProperties(InvalidPrefixProperties.class) + static class InvalidPrefixConfiguration { + + @Bean(name = "invalidPrefixProperties") + public InvalidPrefixProperties invalidPrefixProperties() { + return new InvalidPrefixProperties(); + } + + } + + @ConfigurationProperties("FooPrefix") + static class InvalidPrefixProperties { + + private String value; + + public String getValue() { + return this.value; + } + + public void setValue(String value) { + this.value = value; + } + } + +}