From eae95f8d17a796519e2a33b41e843ec92269d714 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Wed, 25 Oct 2023 13:52:36 -0500 Subject: [PATCH] Handle null host value in MailHealthIndicator If both the host and port are omitted from the mail properties, the `location` field will be omitted from the health indicator details. Fixes gh-38007 --- .../actuate/mail/MailHealthIndicator.java | 12 ++++- .../mail/MailHealthIndicatorTests.java | 47 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/mail/MailHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/mail/MailHealthIndicator.java index afd67045f7f..99bc8a5eac4 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/mail/MailHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/mail/MailHealthIndicator.java @@ -20,11 +20,13 @@ import org.springframework.boot.actuate.health.AbstractHealthIndicator; import org.springframework.boot.actuate.health.Health.Builder; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.mail.javamail.JavaMailSenderImpl; +import org.springframework.util.StringUtils; /** * {@link HealthIndicator} for configured smtp server(s). * * @author Johannes Edmeier + * @author Scott Frederick * @since 2.0.0 */ public class MailHealthIndicator extends AbstractHealthIndicator { @@ -38,9 +40,15 @@ public class MailHealthIndicator extends AbstractHealthIndicator { @Override protected void doHealthCheck(Builder builder) throws Exception { + String host = this.mailSender.getHost(); int port = this.mailSender.getPort(); - builder.withDetail("location", (port != JavaMailSenderImpl.DEFAULT_PORT) - ? this.mailSender.getHost() + ":" + this.mailSender.getPort() : this.mailSender.getHost()); + StringBuilder location = new StringBuilder((host != null) ? host : ""); + if (port != JavaMailSenderImpl.DEFAULT_PORT) { + location.append(":").append(port); + } + if (StringUtils.hasLength(location)) { + builder.withDetail("location", location.toString()); + } this.mailSender.testConnection(); builder.up(); } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/mail/MailHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/mail/MailHealthIndicatorTests.java index 1fdb6dba44a..5bfbb1d2fbe 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/mail/MailHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/mail/MailHealthIndicatorTests.java @@ -44,6 +44,7 @@ import static org.mockito.Mockito.mock; * * @author Johannes Edmeier * @author Stephane Nicoll + * @author Scott Frederick */ class MailHealthIndicatorTests { @@ -61,6 +62,52 @@ class MailHealthIndicatorTests { this.indicator = new MailHealthIndicator(this.mailSender); } + @Test + void smtpOnDefaultHostAndPortIsUp() { + given(this.mailSender.getHost()).willReturn(null); + given(this.mailSender.getPort()).willReturn(-1); + given(this.mailSender.getProtocol()).willReturn("success"); + Health health = this.indicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + assertThat(health.getDetails()).doesNotContainKey("location"); + } + + @Test + void smtpOnDefaultHostAndPortIsDown() throws MessagingException { + given(this.mailSender.getHost()).willReturn(null); + given(this.mailSender.getPort()).willReturn(-1); + willThrow(new MessagingException("A test exception")).given(this.mailSender).testConnection(); + Health health = this.indicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails()).doesNotContainKey("location"); + Object errorMessage = health.getDetails().get("error"); + assertThat(errorMessage).isNotNull(); + assertThat(errorMessage.toString().contains("A test exception")).isTrue(); + } + + @Test + void smtpOnDefaultHostAndCustomPortIsUp() { + given(this.mailSender.getHost()).willReturn(null); + given(this.mailSender.getPort()).willReturn(1234); + given(this.mailSender.getProtocol()).willReturn("success"); + Health health = this.indicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + assertThat(health.getDetails().get("location")).isEqualTo(":1234"); + } + + @Test + void smtpOnDefaultHostAndCustomPortIsDown() throws MessagingException { + given(this.mailSender.getHost()).willReturn(null); + given(this.mailSender.getPort()).willReturn(1234); + willThrow(new MessagingException("A test exception")).given(this.mailSender).testConnection(); + Health health = this.indicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails().get("location")).isEqualTo(":1234"); + Object errorMessage = health.getDetails().get("error"); + assertThat(errorMessage).isNotNull(); + assertThat(errorMessage.toString().contains("A test exception")).isTrue(); + } + @Test void smtpOnDefaultPortIsUp() { given(this.mailSender.getPort()).willReturn(-1);