From acb5a90273abf5b7210779ee39cc940992ca04ce Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 25 Oct 2018 15:55:47 -0700 Subject: [PATCH] Log warning when using deprecated EndpointId chars Update `EndpointId` to log a warning when `-` or `.` are used. Closes gh-14840 --- .../boot/actuate/endpoint/EndpointId.java | 29 +++++++++++++++++-- .../actuate/endpoint/EndpointIdTests.java | 14 +++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java index 755888dbd49..c41ec14aefb 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java @@ -16,9 +16,14 @@ package org.springframework.boot.actuate.endpoint; +import java.util.HashSet; import java.util.Locale; +import java.util.Set; import java.util.regex.Pattern; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.util.Assert; /** @@ -31,7 +36,13 @@ import org.springframework.util.Assert; */ public final class EndpointId { - private static final Pattern VALID_CHARS = Pattern.compile("[a-zA-Z0-9\\.\\-]+"); + private static final Log logger = LogFactory.getLog(EndpointId.class); + + private static Set loggedWarnings = new HashSet<>(); + + private static final Pattern VALID_PATTERN = Pattern.compile("[a-zA-Z0-9\\.\\-]+"); + + private static final Pattern WARNING_PATTERN = Pattern.compile("[\\.\\-]+"); private final String value; @@ -41,12 +52,15 @@ public final class EndpointId { private EndpointId(String value) { Assert.hasText(value, "Value must not be empty"); - Assert.isTrue(VALID_CHARS.matcher(value).matches(), + Assert.isTrue(VALID_PATTERN.matcher(value).matches(), "Value must only contain valid chars"); Assert.isTrue(!Character.isDigit(value.charAt(0)), "Value must not start with a number"); Assert.isTrue(!Character.isUpperCase(value.charAt(0)), "Value must not start with an uppercase letter"); + if (WARNING_PATTERN.matcher(value).find()) { + logWarning(value); + } this.value = value; this.lowerCaseValue = value.toLowerCase(Locale.ENGLISH); this.lowerCaseAlphaNumeric = getAlphaNumerics(this.lowerCaseValue); @@ -112,4 +126,15 @@ public final class EndpointId { return new EndpointId(value.replace("-", "")); } + static void resetLoggedWarnings() { + loggedWarnings.clear(); + } + + private static void logWarning(String value) { + if (logger.isWarnEnabled() && loggedWarnings.add(value)) { + logger.warn("Endpoint ID '" + value + + "' contains invalid characters, please migrate to a valid format."); + } + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java index 46a93779ec0..5fad4f02ba5 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java @@ -16,8 +16,11 @@ package org.springframework.boot.actuate.endpoint; +import org.junit.Rule; import org.junit.Test; +import org.springframework.boot.test.rule.OutputCapture; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -28,6 +31,9 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException */ public class EndpointIdTests { + @Rule + public OutputCapture output = new OutputCapture(); + @Test public void ofWhenNullThrowsException() { assertThatIllegalArgumentException().isThrownBy(() -> EndpointId.of(null)) @@ -80,6 +86,14 @@ public class EndpointIdTests { assertThat(endpointId.toString()).isEqualTo("foo-bar"); } + @Test + public void ofWhenContainsDeprecatedCharsLogsWarning() { + EndpointId.resetLoggedWarnings(); + EndpointId.of("foo-bar"); + assertThat(this.output.toString()).contains( + "Endpoint ID 'foo-bar' contains invalid characters, please migrate to a valid format"); + } + @Test public void equalsAndHashCode() { EndpointId one = EndpointId.of("foobar1");