From ce7d384d2c0504899c19e829404a011e3b91bcfa Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 5 Dec 2023 14:08:14 -0800 Subject: [PATCH] Add MissingParametersFailureAnalyzer Add a new failure analyzer that provides hints whenever parameter names cannot be discovered. Closes gh-38603 --- .../analyzer/BindFailureAnalyzer.java | 25 ++-- .../MissingParameterNamesFailureAnalyzer.java | 116 +++++++++++++++++ .../MissingParametersFailureAnalyzer.java | 83 ++++++++++++ .../main/resources/META-INF/spring.factories | 1 + .../analyzer/BindFailureAnalyzerTests.java | 13 ++ ...ingParameterNamesFailureAnalyzerTests.java | 123 ++++++++++++++++++ 6 files changed, 353 insertions(+), 8 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzer.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParametersFailureAnalyzer.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzerTests.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java index 2ee3fa5ae6b..e5b5efe4361 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java @@ -49,15 +49,20 @@ class BindFailureAnalyzer extends AbstractFailureAnalyzer { || rootCause instanceof UnboundConfigurationPropertiesException) { return null; } - return analyzeGenericBindException(cause); + return analyzeGenericBindException(rootFailure, cause); } - private FailureAnalysis analyzeGenericBindException(BindException cause) { + private FailureAnalysis analyzeGenericBindException(Throwable rootFailure, BindException cause) { + FailureAnalysis missingParametersAnalysis = MissingParameterNamesFailureAnalyzer + .analyzeForMissingParameters(rootFailure); StringBuilder description = new StringBuilder(String.format("%s:%n", cause.getMessage())); ConfigurationProperty property = cause.getProperty(); buildDescription(description, property); description.append(String.format("%n Reason: %s", getMessage(cause))); - return getFailureAnalysis(description, cause); + if (missingParametersAnalysis != null) { + MissingParameterNamesFailureAnalyzer.appendPossibility(description); + } + return getFailureAnalysis(description.toString(), cause, missingParametersAnalysis); } private void buildDescription(StringBuilder description, ConfigurationProperty property) { @@ -98,14 +103,18 @@ class BindFailureAnalyzer extends AbstractFailureAnalyzer { return ex.getClass().getName() + (StringUtils.hasText(message) ? ": " + message : ""); } - private FailureAnalysis getFailureAnalysis(Object description, BindException cause) { - StringBuilder message = new StringBuilder("Update your application's configuration"); + private FailureAnalysis getFailureAnalysis(String description, BindException cause, + FailureAnalysis missingParametersAnalysis) { + StringBuilder action = new StringBuilder("Update your application's configuration"); Collection validValues = findValidValues(cause); if (!validValues.isEmpty()) { - message.append(String.format(". The following values are valid:%n")); - validValues.forEach((value) -> message.append(String.format("%n %s", value))); + action.append(String.format(". The following values are valid:%n")); + validValues.forEach((value) -> action.append(String.format("%n %s", value))); + } + if (missingParametersAnalysis != null) { + action.append(String.format("%n%n%s", missingParametersAnalysis.getAction())); } - return new FailureAnalysis(description.toString(), message.toString(), cause); + return new FailureAnalysis(description, action.toString(), cause); } private Collection findValidValues(BindException ex) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzer.java new file mode 100644 index 00000000000..442a2cabbb7 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzer.java @@ -0,0 +1,116 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://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.HashSet; +import java.util.Set; + +import org.springframework.boot.diagnostics.FailureAnalysis; +import org.springframework.boot.diagnostics.FailureAnalyzer; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.util.StringUtils; + +/** + * {@link FailureAnalyzer} for exceptions caused by missing parameter names. This analyzer + * is ordered last, if other analyzers wish to also report parameter actions they can use + * the {@link #analyzeForMissingParameters(Throwable)} static method. + * + * @author Phillip Webb + */ +@Order(Ordered.LOWEST_PRECEDENCE) +class MissingParameterNamesFailureAnalyzer implements FailureAnalyzer { + + private static final String USE_PARAMETERS_MESSAGE = "Ensure that the compiler uses the '-parameters' flag"; + + static final String POSSIBILITY = "This may be due to missing parameter name information"; + + static String ACTION = """ + Ensure that your compiler is configured to use the '-parameters' flag. + You may need to update both your build tool settings as well as your IDE. + (See https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-6.x#parameter-name-retention) + """; + + @Override + public FailureAnalysis analyze(Throwable failure) { + return analyzeForMissingParameters(failure); + } + + /** + * Analyze the given failure for missing parameter name exceptions. + * @param failure the failure to analyze + * @return a failure analysis or {@code null} + */ + static FailureAnalysis analyzeForMissingParameters(Throwable failure) { + return analyzeForMissingParameters(failure, failure, new HashSet<>()); + } + + private static FailureAnalysis analyzeForMissingParameters(Throwable rootFailure, Throwable cause, + Set seen) { + if (cause != null && seen.add(cause)) { + if (isSpringParametersException(cause)) { + return getAnalysis(rootFailure, cause); + } + FailureAnalysis analysis = analyzeForMissingParameters(rootFailure, cause.getCause(), seen); + if (analysis != null) { + return analysis; + } + for (Throwable suppressed : cause.getSuppressed()) { + analysis = analyzeForMissingParameters(rootFailure, suppressed, seen); + if (analysis != null) { + return analysis; + } + } + } + return null; + } + + private static boolean isSpringParametersException(Throwable failure) { + String message = failure.getMessage(); + return message != null && message.contains(USE_PARAMETERS_MESSAGE) && isSpringException(failure); + } + + private static boolean isSpringException(Throwable failure) { + StackTraceElement[] elements = failure.getStackTrace(); + return elements.length > 0 && isSpringClass(elements[0].getClassName()); + } + + private static boolean isSpringClass(String className) { + return className != null && className.startsWith("org.springframework."); + } + + private static FailureAnalysis getAnalysis(Throwable rootFailure, Throwable cause) { + StringBuilder description = new StringBuilder(String.format("%s:%n", cause.getMessage())); + if (rootFailure != cause) { + description.append(String.format("%n Resulting Failure: %s", getExceptionTypeAndMessage(rootFailure))); + } + return new FailureAnalysis(description.toString(), ACTION, rootFailure); + } + + private static String getExceptionTypeAndMessage(Throwable ex) { + String message = ex.getMessage(); + return ex.getClass().getName() + (StringUtils.hasText(message) ? ": " + message : ""); + } + + public static void appendPossibility(StringBuilder description) { + if (!description.toString().endsWith(System.lineSeparator())) { + description.append("%n".formatted()); + } + description.append("%n%s".formatted(POSSIBILITY)); + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParametersFailureAnalyzer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParametersFailureAnalyzer.java new file mode 100644 index 00000000000..350e30aafa2 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/MissingParametersFailureAnalyzer.java @@ -0,0 +1,83 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://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.HashSet; +import java.util.Set; + +import org.springframework.boot.diagnostics.FailureAnalysis; +import org.springframework.boot.diagnostics.FailureAnalyzer; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; + +/** + * @author Phillip Webb + */ +@Order(Ordered.HIGHEST_PRECEDENCE) +class MissingParametersFailureAnalyzer implements FailureAnalyzer { + + private static final String USE_PARAMETERS_MESSAGE = "Ensure that the compiler uses the '-parameters' flag"; + + @Override + public FailureAnalysis analyze(Throwable failure) { + return analyze(failure, failure, new HashSet<>()); + } + + private FailureAnalysis analyze(Throwable rootFailure, Throwable cause, Set seen) { + if (cause == null || !seen.add(cause)) { + return null; + } + if (isSpringParametersException(cause)) { + return getAnalysis(rootFailure, cause); + } + FailureAnalysis analysis = analyze(rootFailure, cause.getCause(), seen); + if (analysis != null) { + return analysis; + } + for (Throwable suppressed : cause.getSuppressed()) { + analysis = analyze(rootFailure, suppressed, seen); + if (analysis != null) { + return analysis; + } + } + return null; + } + + private boolean isSpringParametersException(Throwable failure) { + String message = failure.getMessage(); + return message != null && message.contains(USE_PARAMETERS_MESSAGE) && isSpringException(failure); + } + + private boolean isSpringException(Throwable failure) { + StackTraceElement[] elements = failure.getStackTrace(); + return elements.length > 0 && isSpringClass(elements[0].getClassName()); + } + + private boolean isSpringClass(String className) { + return className != null && className.startsWith("org.springframework."); + } + + private FailureAnalysis getAnalysis(Throwable rootFailure, Throwable cause) { + String description = ""; + if (rootFailure != cause) { + + } + String action = ""; + return new FailureAnalysis(description, action, rootFailure); + } + +} diff --git a/spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories b/spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories index e6775b7491f..f6cea1c9e68 100644 --- a/spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories +++ b/spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories @@ -70,6 +70,7 @@ org.springframework.boot.diagnostics.analyzer.BeanNotOfRequiredTypeFailureAnalyz org.springframework.boot.diagnostics.analyzer.BindFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.BindValidationFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.UnboundConfigurationPropertyFailureAnalyzer,\ +org.springframework.boot.diagnostics.analyzer.MissingParameterNamesFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.MutuallyExclusiveConfigurationPropertiesFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.NoSuchMethodFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.NoUniqueBeanDefinitionFailureAnalyzer,\ diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java index 5d5f09d0323..5d03732ff9e 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java @@ -99,6 +99,19 @@ class BindFailureAnalyzerTests { + "org.springframework.boot.logging.LogLevel>]")); } + @Test + void bindExceptionWithSupressedMissingParametersException() { + BeanCreationException failure = createFailure(GenericFailureConfiguration.class, "test.foo.value=alpha"); + failure.addSuppressed(new IllegalStateException( + "Missing parameter names. Ensure that the compiler uses the '-parameters' flag")); + FailureAnalysis analysis = new BindFailureAnalyzer().analyze(failure); + assertThat(analysis.getDescription()) + .contains(failure("test.foo.value", "alpha", "\"test.foo.value\" from property source \"test\"", + "failed to convert java.lang.String to int")) + .contains(MissingParameterNamesFailureAnalyzer.POSSIBILITY); + assertThat(analysis.getAction()).contains(MissingParameterNamesFailureAnalyzer.ACTION); + } + private static String failure(String property, String value, String origin, String reason) { return String.format("Property: %s%n Value: \"%s\"%n Origin: %s%n Reason: %s", property, value, origin, reason); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzerTests.java new file mode 100644 index 00000000000..9251d201d8d --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/MissingParameterNamesFailureAnalyzerTests.java @@ -0,0 +1,123 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://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.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.boot.diagnostics.FailureAnalysis; +import org.springframework.core.MethodParameter; +import org.springframework.web.context.request.NativeWebRequest; +import org.springframework.web.method.annotation.AbstractNamedValueMethodArgumentResolver; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link MissingParameterNamesFailureAnalyzer}. + * + * @author Phillip Webb + */ +class MissingParameterNamesFailureAnalyzerTests { + + @Test + void analyzeWhenMissingParametersExceptionReturnsFailure() throws Exception { + MissingParameterNamesFailureAnalyzer analyzer = new MissingParameterNamesFailureAnalyzer(); + FailureAnalysis analysis = analyzer.analyze(getSpringFrameworkMissingParameterException()); + assertThat(analysis.getDescription()) + .isEqualTo(String.format("Name for argument of type [java.lang.String] not specified, and parameter name " + + "information not available via reflection. Ensure that the compiler uses the '-parameters' flag.:%n")); + assertThat(analysis.getAction()).isEqualTo(MissingParameterNamesFailureAnalyzer.ACTION); + } + + @Test + void analyzeForMissingParametersWhenMissingParametersExceptionReturnsFailure() throws Exception { + FailureAnalysis analysis = MissingParameterNamesFailureAnalyzer + .analyzeForMissingParameters(getSpringFrameworkMissingParameterException()); + assertThat(analysis.getDescription()) + .isEqualTo(String.format("Name for argument of type [java.lang.String] not specified, and parameter name " + + "information not available via reflection. Ensure that the compiler uses the '-parameters' flag.:%n")); + assertThat(analysis.getAction()).isEqualTo(MissingParameterNamesFailureAnalyzer.ACTION); + } + + @Test + void analyzeForMissingParametersWhenInCauseReturnsFailure() throws Exception { + RuntimeException exception = new RuntimeException("Badness", getSpringFrameworkMissingParameterException()); + FailureAnalysis analysis = MissingParameterNamesFailureAnalyzer.analyzeForMissingParameters(exception); + assertThat(analysis.getDescription()) + .isEqualTo(String.format("Name for argument of type [java.lang.String] not specified, and parameter name " + + "information not available via reflection. Ensure that the compiler uses the '-parameters' flag.:%n%n" + + " Resulting Failure: java.lang.RuntimeException: Badness")); + assertThat(analysis.getAction()).isEqualTo(MissingParameterNamesFailureAnalyzer.ACTION); + } + + @Test + void analyzeForMissingParametersWhenInSuppressedReturnsFailure() throws Exception { + RuntimeException exception = new RuntimeException("Badness"); + exception.addSuppressed(getSpringFrameworkMissingParameterException()); + FailureAnalysis analysis = MissingParameterNamesFailureAnalyzer.analyzeForMissingParameters(exception); + assertThat(analysis.getDescription()) + .isEqualTo(String.format("Name for argument of type [java.lang.String] not specified, and parameter name " + + "information not available via reflection. Ensure that the compiler uses the '-parameters' flag.:%n%n" + + " Resulting Failure: java.lang.RuntimeException: Badness")); + assertThat(analysis.getAction()).isEqualTo(MissingParameterNamesFailureAnalyzer.ACTION); + } + + @Test + void analyzeForMissingParametersWhenNotPresentReturnsNull() { + RuntimeException exception = new RuntimeException("Badness"); + FailureAnalysis analysis = MissingParameterNamesFailureAnalyzer.analyzeForMissingParameters(exception); + assertThat(analysis).isNull(); + } + + private RuntimeException getSpringFrameworkMissingParameterException() throws Exception { + MockResolver resolver = new MockResolver(); + Method method = getClass().getDeclaredMethod("example", String.class); + MethodParameter parameter = new MethodParameter(method, 0); + try { + resolver.resolveArgument(parameter, null, null, null); + } + catch (RuntimeException ex) { + return ex; + } + throw new AssertionError("Did not throw"); + } + + void example(String name) { + } + + static class MockResolver extends AbstractNamedValueMethodArgumentResolver { + + @Override + public boolean supportsParameter(MethodParameter parameter) { + return true; + } + + @Override + protected NamedValueInfo createNamedValueInfo(MethodParameter parameter) { + return new NamedValueInfo("", false, null); + } + + @Override + protected Object resolveName(String name, MethodParameter parameter, NativeWebRequest request) + throws Exception { + return null; + } + + } + +}