diff --git a/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfiguration.java b/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfiguration.java index 6c3642f48a2..2bbe76bc6c8 100644 --- a/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfiguration.java +++ b/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfiguration.java @@ -24,6 +24,7 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.health.autoconfigure.contributor.CompositeHealthContributorConfiguration; import org.springframework.boot.health.autoconfigure.contributor.ConditionalOnEnabledHealthIndicator; import org.springframework.boot.health.contributor.HealthContributor; @@ -35,17 +36,19 @@ import org.springframework.context.annotation.Bean; * {@link EnableAutoConfiguration Auto-configuration} for {@link JmsHealthIndicator}. * * @author Stephane Nicoll + * @author Venkata Naga Sai Srikanth Gollapudi * @since 4.0.0 */ @AutoConfiguration(after = JmsAutoConfiguration.class) @ConditionalOnClass({ ConnectionFactory.class, JmsHealthIndicator.class }) @ConditionalOnBean(ConnectionFactory.class) @ConditionalOnEnabledHealthIndicator("jms") +@EnableConfigurationProperties(JmsHealthIndicatorProperties.class) public final class JmsHealthContributorAutoConfiguration extends CompositeHealthContributorConfiguration { - JmsHealthContributorAutoConfiguration() { - super(JmsHealthIndicator::new); + JmsHealthContributorAutoConfiguration(JmsHealthIndicatorProperties properties) { + super((connectionFactory) -> new JmsHealthIndicator(connectionFactory, properties.getStartTimeout())); } @Bean diff --git a/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthIndicatorProperties.java b/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthIndicatorProperties.java new file mode 100644 index 00000000000..d0cfcf24b69 --- /dev/null +++ b/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthIndicatorProperties.java @@ -0,0 +1,47 @@ +/* + * Copyright 2012-present 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.jms.autoconfigure.health; + +import java.time.Duration; + +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.jms.health.JmsHealthIndicator; + +/** + * Configuration properties for {@link JmsHealthIndicator}. + * + * @author Venkata Naga Sai Srikanth Gollapudi + * @author Stephane Nicoll + * @since 4.2.0 + */ +@ConfigurationProperties("management.health.jms") +public class JmsHealthIndicatorProperties { + + /** + * Timeout to use when starting a connection for the health check. + */ + private Duration startTimeout = JmsHealthIndicator.DEFAULT_START_TIMEOUT; + + public Duration getStartTimeout() { + return this.startTimeout; + } + + public void setStartTimeout(Duration startTimeout) { + this.startTimeout = startTimeout; + } + +} diff --git a/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/health/JmsHealthIndicator.java b/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/health/JmsHealthIndicator.java index ac3681de69b..1c788f3086b 100644 --- a/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/health/JmsHealthIndicator.java +++ b/module/spring-boot-jms/src/main/java/org/springframework/boot/jms/health/JmsHealthIndicator.java @@ -16,6 +16,7 @@ package org.springframework.boot.jms.health; +import java.time.Duration; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -25,25 +26,55 @@ import jakarta.jms.JMSException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.boot.convert.DurationStyle; import org.springframework.boot.health.contributor.AbstractHealthIndicator; import org.springframework.boot.health.contributor.Health; import org.springframework.boot.health.contributor.HealthIndicator; +import org.springframework.core.log.LogMessage; +import org.springframework.util.Assert; /** * {@link HealthIndicator} for a JMS {@link ConnectionFactory}. * * @author Stephane Nicoll + * @author Venkata Naga Sai Srikanth Gollapudi * @since 4.0.0 */ public class JmsHealthIndicator extends AbstractHealthIndicator { + /** + * Default timeout to use when starting a connection for the health check. + */ + public static final Duration DEFAULT_START_TIMEOUT = Duration.ofSeconds(5); + private final Log logger = LogFactory.getLog(JmsHealthIndicator.class); private final ConnectionFactory connectionFactory; + private final Duration startTimeout; + + /** + * Create a new {@link JmsHealthIndicator} instance with a + * {@linkplain #DEFAULT_START_TIMEOUT default} start timeout. + * @param connectionFactory the connection factory to use + */ public JmsHealthIndicator(ConnectionFactory connectionFactory) { + this(connectionFactory, DEFAULT_START_TIMEOUT); + } + + /** + * Create a new {@link JmsHealthIndicator} instance with the given + * {@code startTimeout}. + * @param connectionFactory the connection factory to use + * @param startTimeout timeout to use when starting a connection for the health check + * @since 4.2.0 + */ + public JmsHealthIndicator(ConnectionFactory connectionFactory, Duration startTimeout) { super("JMS health check failed"); + Assert.notNull(startTimeout, "'startTimeout' must not be null"); + Assert.isTrue(startTimeout.compareTo(Duration.ZERO) > 0, "'startTimeout' must be greater than 0"); this.connectionFactory = connectionFactory; + this.startTimeout = startTimeout; } @Override @@ -67,9 +98,11 @@ public class JmsHealthIndicator extends AbstractHealthIndicator { void start() throws JMSException { new Thread(() -> { try { - if (!this.latch.await(5, TimeUnit.SECONDS)) { + Duration startTimeout1 = JmsHealthIndicator.this.startTimeout; + if (!this.latch.await(startTimeout1.toNanos(), TimeUnit.NANOSECONDS)) { JmsHealthIndicator.this.logger - .warn("Connection failed to start within 5 seconds and will be closed."); + .warn(LogMessage.format("Connection failed to start within %s and will be closed.", + DurationStyle.SIMPLE.print(startTimeout1))); closeConnection(); } } diff --git a/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfigurationTests.java b/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfigurationTests.java index 43795bff614..8b25767f797 100644 --- a/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfigurationTests.java +++ b/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfigurationTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.jms.autoconfigure.health; +import java.time.Duration; + import jakarta.jms.ConnectionFactory; import org.junit.jupiter.api.Test; @@ -31,6 +33,7 @@ import static org.mockito.Mockito.mock; * Tests for {@link JmsHealthContributorAutoConfiguration}. * * @author Phillip Webb + * @author Venkata Naga Sai Srikanth Gollapudi */ class JmsHealthContributorAutoConfigurationTests { @@ -44,6 +47,27 @@ class JmsHealthContributorAutoConfigurationTests { this.contextRunner.run((context) -> assertThat(context).hasSingleBean(JmsHealthIndicator.class)); } + @Test + void runWhenStartTimeoutIsConfiguredShouldCreateIndicatorWithConfiguredStartTimeout() { + this.contextRunner.withPropertyValues("management.health.jms.start-timeout=10ms").run((context) -> { + assertThat(context).hasSingleBean(JmsHealthIndicator.class); + assertThat(context).hasSingleBean(JmsHealthIndicatorProperties.class); + assertThat(context.getBean(JmsHealthIndicatorProperties.class).getStartTimeout()) + .isEqualTo(Duration.ofMillis(10)); + assertThat(context.getBean(JmsHealthIndicator.class)).hasFieldOrPropertyWithValue("startTimeout", + Duration.ofMillis(10)); + }); + } + + @Test + void runWhenStartTimeoutIsZeroShouldFail() { + this.contextRunner.withPropertyValues("management.health.jms.start-timeout=0ms") + .run((context) -> assertThat(context).hasFailed() + .getFailure() + .rootCause() + .hasMessage("'startTimeout' must be greater than 0")); + } + @Test void runWhenDisabledShouldNotCreateIndicator() { this.contextRunner.withPropertyValues("management.health.jms.enabled:false") diff --git a/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/health/JmsHealthIndicatorTests.java b/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/health/JmsHealthIndicatorTests.java index c454945b5df..cf2a8ac069b 100644 --- a/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/health/JmsHealthIndicatorTests.java +++ b/module/spring-boot-jms/src/test/java/org/springframework/boot/jms/health/JmsHealthIndicatorTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.jms.health; +import java.time.Duration; + import jakarta.jms.Connection; import jakarta.jms.ConnectionFactory; import jakarta.jms.ConnectionMetaData; @@ -28,6 +30,7 @@ import org.springframework.boot.health.contributor.Health; import org.springframework.boot.health.contributor.Status; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; import static org.mockito.BDDMockito.willAnswer; @@ -38,9 +41,33 @@ import static org.mockito.Mockito.mock; * Tests for {@link JmsHealthIndicator}. * * @author Stephane Nicoll + * @author Venkata Naga Sai Srikanth Gollapudi */ class JmsHealthIndicatorTests { + @Test + @SuppressWarnings("NullAway") // Test null check + void createWhenStartTimeoutIsNullThrowsException() { + ConnectionFactory connectionFactory = mock(ConnectionFactory.class); + assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, null)) + .withMessage("'startTimeout' must not be null"); + } + + @Test + void createWhenStartTimeoutIsZeroThrowsException() { + ConnectionFactory connectionFactory = mock(ConnectionFactory.class); + assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ZERO)) + .withMessage("'startTimeout' must be greater than 0"); + } + + @Test + void createWhenStartTimeoutIsNegativeThrowsException() { + ConnectionFactory connectionFactory = mock(ConnectionFactory.class); + assertThatIllegalArgumentException() + .isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ofMillis(-1))) + .withMessage("'startTimeout' must be greater than 0"); + } + @Test void jmsBrokerIsUp() throws JMSException { ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class); @@ -98,6 +125,19 @@ class JmsHealthIndicatorTests { @Test void whenConnectionStartIsUnresponsiveStatusIsDown() throws JMSException { + Health health = healthWhenConnectionStartIsUnresponsive(Duration.ofSeconds(5)); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat((String) health.getDetails().get("error")).contains("Connection closed"); + } + + @Test + void whenConnectionStartIsUnresponsiveUsesConfiguredStartTimeout() throws JMSException { + Health health = healthWhenConnectionStartIsUnresponsive(Duration.ofMillis(10)); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat((String) health.getDetails().get("error")).contains("Connection closed"); + } + + private Health healthWhenConnectionStartIsUnresponsive(Duration startTimeout) throws JMSException { ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class); given(connectionMetaData.getJMSProviderName()).willReturn("JMS test provider"); Connection connection = mock(Connection.class); @@ -109,10 +149,8 @@ class JmsHealthIndicatorTests { }).given(connection).close(); ConnectionFactory connectionFactory = mock(ConnectionFactory.class); given(connectionFactory.createConnection()).willReturn(connection); - JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); - Health health = indicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.DOWN); - assertThat((String) health.getDetails().get("error")).contains("Connection closed"); + JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, startTimeout); + return indicator.health(); } private static final class UnresponsiveStartAnswer implements Answer {