From 667e5ca30c4c967a132878800cdbec8bdea6d131 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 21 Sep 2021 12:04:38 -0700 Subject: [PATCH] Polish --- .../boot/actuate/endpoint/Sanitizer.java | 20 +++++ .../health/AbstractHealthIndicator.java | 10 +-- .../boot/actuate/health/Health.java | 17 ++-- .../health/AbstractHealthIndicatorTests.java | 80 ++++++++++--------- ...sticsearchRestClientAutoConfiguration.java | 75 +++++++---------- .../src/docs/asciidoc/actuator/metrics.adoc | 1 - 6 files changed, 102 insertions(+), 101 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java index a14b0b2428..02ce77b809 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/Sanitizer.java @@ -65,18 +65,38 @@ public class Sanitizer { DEFAULT_KEYS_TO_SANITIZE.addAll(URI_USERINFO_KEYS); } + /** + * Create a new {@link Sanitizer} instance with a default set of keys to sanitize. + */ public Sanitizer() { this(DEFAULT_KEYS_TO_SANITIZE.toArray(new String[0])); } + /** + * Create a new {@link Sanitizer} instance with specific keys to sanitize. + * @param keysToSanitize the keys to sanitize + */ public Sanitizer(String... keysToSanitize) { this(Collections.emptyList(), keysToSanitize); } + /** + * Create a new {@link Sanitizer} instance with a default set of keys to sanitize and + * additional sanitizing functions. + * @param sanitizingFunctions the sanitizing functions to apply + * @since 2.6.0 + */ public Sanitizer(Iterable sanitizingFunctions) { this(sanitizingFunctions, DEFAULT_KEYS_TO_SANITIZE.toArray(new String[0])); } + /** + * Create a new {@link Sanitizer} instance with specific keys to sanitize and + * additional sanitizing functions. + * @param sanitizingFunctions the sanitizing functions to apply + * @param keysToSanitize the keys to sanitize + * @since 2.6.0 + */ public Sanitizer(Iterable sanitizingFunctions, String... keysToSanitize) { sanitizingFunctions.forEach(this.sanitizingFunctions::add); this.sanitizingFunctions.add(getDefaultSanitizingFunction()); diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractHealthIndicator.java index dc3f61428c..0e9bcabfc0 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractHealthIndicator.java @@ -84,17 +84,13 @@ public abstract class AbstractHealthIndicator implements HealthIndicator { catch (Exception ex) { builder.down(ex); } - logExceptionIfPresent(builder); + logExceptionIfPresent(builder.getException()); return builder.build(); } - private void logExceptionIfPresent(Builder builder) { - Throwable ex = builder.getException(); + private void logExceptionIfPresent(Throwable ex) { if (ex != null && this.logger.isWarnEnabled()) { - String message = null; - if (ex instanceof Exception) { - message = this.healthCheckFailedMessage.apply((Exception) ex); - } + String message = (ex instanceof Exception) ? this.healthCheckFailedMessage.apply((Exception) ex) : null; this.logger.warn(StringUtils.hasText(message) ? message : DEFAULT_MESSAGE, ex); } } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java index 190f3ebf6c..d0f293e28d 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/Health.java @@ -323,15 +323,6 @@ public final class Health extends HealthComponent { return this; } - /** - * Return the {@link Exception}. - * @return the exception or {@code null} if the builder has no exception - * @since 2.6.0 - */ - public Throwable getException() { - return this.exception; - } - /** * Create a new {@link Health} instance with the previously specified code and * details. @@ -341,6 +332,14 @@ public final class Health extends HealthComponent { return new Health(this); } + /** + * Return the {@link Exception}. + * @return the exception or {@code null} if the builder has no exception + */ + Throwable getException() { + return this.exception; + } + } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractHealthIndicatorTests.java index 23dabfe51d..09950b7e5f 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/AbstractHealthIndicatorTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.health; +import java.util.function.Consumer; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -36,76 +38,78 @@ class AbstractHealthIndicatorTests { @Test void healthCheckWhenUpDoesNotLogHealthCheckFailedMessage(CapturedOutput output) { - Health heath = new AbstractHealthIndicator("Test message") { - @Override - protected void doHealthCheck(Builder builder) { - builder.up(); - } - }.health(); + TestHealthIndicator indicator = new TestHealthIndicator("Test message", Builder::up); + Health heath = indicator.health(); assertThat(heath.getStatus()).isEqualTo(Status.UP); assertThat(output).doesNotContain("Test message"); } @Test void healthCheckWhenDownWithExceptionThrownDoesNotLogHealthCheckFailedMessage(CapturedOutput output) { - Health heath = new AbstractHealthIndicator("Test message") { - @Override - protected void doHealthCheck(Builder builder) { - throw new IllegalStateException("Test exception"); - } - }.health(); + TestHealthIndicator indicator = new TestHealthIndicator("Test message", (builder) -> { + throw new IllegalStateException("Test exception"); + }); + Health heath = indicator.health(); assertThat(heath.getStatus()).isEqualTo(Status.DOWN); assertThat(output).contains("Test message").contains("Test exception"); } @Test void healthCheckWhenDownWithExceptionConfiguredDoesNotLogHealthCheckFailedMessage(CapturedOutput output) { - Health heath = new AbstractHealthIndicator("Test message") { - @Override - protected void doHealthCheck(Builder builder) { - builder.down().withException(new IllegalStateException("Test exception")); - } - }.health(); + Health heath = new TestHealthIndicator("Test message", + (builder) -> builder.down().withException(new IllegalStateException("Test exception"))).health(); assertThat(heath.getStatus()).isEqualTo(Status.DOWN); assertThat(output).contains("Test message").contains("Test exception"); } @Test void healthCheckWhenDownWithExceptionConfiguredDoesNotLogHealthCheckFailedMessageTwice(CapturedOutput output) { - Health heath = new AbstractHealthIndicator("Test message") { - @Override - protected void doHealthCheck(Builder builder) { - IllegalStateException ex = new IllegalStateException("Test exception"); - builder.down().withException(ex); - throw ex; - } - }.health(); + TestHealthIndicator indicator = new TestHealthIndicator("Test message", (builder) -> { + IllegalStateException ex = new IllegalStateException("Test exception"); + builder.down().withException(ex); + throw ex; + }); + Health heath = indicator.health(); assertThat(heath.getStatus()).isEqualTo(Status.DOWN); assertThat(output).contains("Test message").containsOnlyOnce("Test exception"); } @Test void healthCheckWhenDownWithExceptionAndNoFailureMessageLogsDefaultMessage(CapturedOutput output) { - Health heath = new AbstractHealthIndicator() { - @Override - protected void doHealthCheck(Builder builder) { - builder.down().withException(new IllegalStateException("Test exception")); - } - }.health(); + TestHealthIndicator indicator = new TestHealthIndicator( + (builder) -> builder.down().withException(new IllegalStateException("Test exception"))); + Health heath = indicator.health(); assertThat(heath.getStatus()).isEqualTo(Status.DOWN); assertThat(output).contains("Health check failed").contains("Test exception"); } @Test void healthCheckWhenDownWithErrorLogsDefaultMessage(CapturedOutput output) { - Health heath = new AbstractHealthIndicator("Test Message") { - @Override - protected void doHealthCheck(Builder builder) { - builder.down().withException(new Error("Test error")); - } - }.health(); + TestHealthIndicator indicator = new TestHealthIndicator("Test Message", + (builder) -> builder.down().withException(new Error("Test error"))); + Health heath = indicator.health(); assertThat(heath.getStatus()).isEqualTo(Status.DOWN); assertThat(output).contains("Health check failed").contains("Test error"); } + static class TestHealthIndicator extends AbstractHealthIndicator { + + private Consumer action; + + TestHealthIndicator(String message, Consumer action) { + super(message); + this.action = action; + } + + TestHealthIndicator(Consumer action) { + this.action = action; + } + + @Override + protected void doHealthCheck(Builder builder) throws Exception { + this.action.accept(builder); + } + + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/elasticsearch/ReactiveElasticsearchRestClientAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/elasticsearch/ReactiveElasticsearchRestClientAutoConfiguration.java index 64c11abaf1..e605260a84 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/elasticsearch/ReactiveElasticsearchRestClientAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/elasticsearch/ReactiveElasticsearchRestClientAutoConfiguration.java @@ -37,6 +37,7 @@ import org.springframework.data.elasticsearch.client.reactive.ReactiveElasticsea import org.springframework.data.elasticsearch.client.reactive.ReactiveRestClients; import org.springframework.data.elasticsearch.client.reactive.ReactiveRestClients.WebClientConfigurationCallback; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; import org.springframework.util.unit.DataSize; import org.springframework.web.reactive.function.client.ExchangeStrategies; import org.springframework.web.reactive.function.client.WebClient; @@ -120,7 +121,11 @@ public class ReactiveElasticsearchRestClientAutoConfiguration { if (this.deprecatedProperties.isCustomized()) { return this.deprecatedProperties.getEndpoints(); } - return this.uris.stream().map((uri) -> uri.getHost() + ":" + uri.getPort()).collect(Collectors.toList()); + return this.uris.stream().map(this::getEndpoint).collect(Collectors.toList()); + } + + private String getEndpoint(URI uri) { + return uri.getHost() + ":" + uri.getPort(); } private Credentials getCredentials() { @@ -132,13 +137,10 @@ public class ReactiveElasticsearchRestClientAutoConfiguration { if (uriCredentials == null) { return propertyCredentials; } - if (propertyCredentials != null && !uriCredentials.equals(propertyCredentials)) { - throw new IllegalArgumentException( - "Credentials from URI user info do not match those from spring.elasticsearch.username and " - + "spring.elasticsearch.password"); - } + Assert.isTrue(propertyCredentials == null || uriCredentials.equals(propertyCredentials), + "Credentials from URI user info do not match those from spring.elasticsearch.username and " + + "spring.elasticsearch.password"); return uriCredentials; - } private Duration getConnectionTimeout() { @@ -155,8 +157,8 @@ public class ReactiveElasticsearchRestClientAutoConfiguration { if (this.deprecatedProperties.isCustomized()) { return this.deprecatedProperties.isUseSsl(); } - Set schemes = this.uris.stream().map((uri) -> uri.getScheme()).collect(Collectors.toSet()); - Assert.isTrue(schemes.size() == 1, () -> "Configured Elasticsearch URIs have varying schemes"); + Set schemes = this.uris.stream().map(URI::getScheme).collect(Collectors.toSet()); + Assert.isTrue(schemes.size() == 1, "Configured Elasticsearch URIs have varying schemes"); return schemes.iterator().next().equals("https"); } @@ -189,29 +191,28 @@ public class ReactiveElasticsearchRestClientAutoConfiguration { } private static Credentials from(List uris) { - Set userInfos = uris.stream().map(URI::create).map((uri) -> uri.getUserInfo()) + Set userInfos = uris.stream().map(URI::create).map(URI::getUserInfo) .collect(Collectors.toSet()); - Assert.isTrue(userInfos.size() == 1, () -> "Configured Elasticsearch URIs have varying user infos"); + Assert.isTrue(userInfos.size() == 1, "Configured Elasticsearch URIs have varying user infos"); String userInfo = userInfos.iterator().next(); - if (userInfo != null) { - String[] parts = userInfo.split(":"); - return new Credentials(parts[0], (parts.length == 2) ? parts[1] : ""); + if (userInfo == null) { + return null; } - return null; + String[] parts = userInfo.split(":"); + String username = parts[0]; + String password = (parts.length != 2) ? "" : parts[1]; + return new Credentials(username, password); } private static Credentials from(ElasticsearchProperties properties) { - String username = properties.getUsername(); - String password = properties.getPassword(); - if (username == null && password == null) { - return null; - } - return new Credentials(username, password); + return getCredentials(properties.getUsername(), properties.getPassword()); } private static Credentials from(DeprecatedReactiveElasticsearchRestClientProperties properties) { - String username = properties.getUsername(); - String password = properties.getPassword(); + return getCredentials(properties.getUsername(), properties.getPassword()); + } + + private static Credentials getCredentials(String username, String password) { if (username == null && password == null) { return null; } @@ -223,38 +224,20 @@ public class ReactiveElasticsearchRestClientAutoConfiguration { if (this == obj) { return true; } - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { + if (obj == null || getClass() != obj.getClass()) { return false; } Credentials other = (Credentials) obj; - if (this.password == null) { - if (other.password != null) { - return false; - } - } - else if (!this.password.equals(other.password)) { - return false; - } - if (this.username == null) { - if (other.username != null) { - return false; - } - } - else if (!this.username.equals(other.username)) { - return false; - } - return true; + return ObjectUtils.nullSafeEquals(this.username, other.username) + && ObjectUtils.nullSafeEquals(this.password, other.password); } @Override public int hashCode() { final int prime = 31; int result = 1; - result = prime * result + ((this.password == null) ? 0 : this.password.hashCode()); - result = prime * result + ((this.username == null) ? 0 : this.username.hashCode()); + result = prime * result + ObjectUtils.nullSafeHashCode(this.username); + result = prime * result + ObjectUtils.nullSafeHashCode(this.password); return result; } diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc index 170cce8cea..4de7eb2583 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc @@ -1081,7 +1081,6 @@ Long task timers require a separate metric name and can be stacked with a short [[actuator.metrics.supported.redis]] ==== Redis Metrics - Auto-configuration registers a `MicrometerCommandLatencyRecorder` for the auto-configured `LettuceConnectionFactory`. For more details refer to the {lettuce-docs}#command.latency.metrics.micrometer[Micrometer Metrics section] of the Lettuce documentation.