Add failure analyzer for invalid config property name

Closes gh-9545

Author: mbhave

spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java

@@ -18,6 +18,7 @@
 
 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 int getNumberOfElements() {
 	 * 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 ConfigurationPropertyName append(String elementValue) {
 		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<Character> 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,23 +441,26 @@ public static boolean isValid(CharSequence name) {
 	 * 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;
 		}
 		List<CharSequence> elements = new ArrayList<CharSequence>(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<Character> invalidChars = ElementValidator.getInvalidChars(elementValue);
+					if (!invalidChars.isEmpty()) {
+						throw new InvalidConfigurationPropertyNameException(invalidChars, name);
+					}
+				}
 				elements.add(elementValue);
 			}
 		});
@@ -651,12 +659,7 @@ public boolean isValid() {
 		}
 
 		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 static boolean isValidChar(char ch, int index) {
 			return isAlpha || isNumeric || ch == '-';
 		}
 
+		private static List<Character> getInvalidChars(CharSequence elementValue) {
+			List<Character> 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;
+		}
+
 	}
 
 }

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<Character> invalidCharacters;
+
+	private final CharSequence name;
+
+	public InvalidConfigurationPropertyNameException(List<Character> invalidCharacters, CharSequence name) {
+		super("Configuration property name '" + name + "' is not valid");
+		this.invalidCharacters = invalidCharacters;
+		this.name = name;
+	}
+
+	public List<Character> getInvalidCharacters() {
+		return this.invalidCharacters;
+	}
+
+	public CharSequence getName() {
+		return this.name;
+	}
+
+}
+

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<InvalidConfigurationPropertyNameException> {
+
+	@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();
+	}
+
+}
+

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=\

spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java

@@ -51,35 +51,35 @@ public void ofNameShouldNotBeNull() throws Exception {
 
 	@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 void ofNameShouldNotContainInvalidChars() throws Exception {
 				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 void ofNameWhenDoubleDotAndAssociative() throws Exception {
 
 	@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 void ofNameWhenNestedBrackets() throws Exception {
 
 	@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 void appendWhenIndexedShouldAppendWithBrackets() throws Exception {
 
 	@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");
 	}
 

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;
+		}
+	}
+
+}