From db12d4d940c9fbf01c23ab413d3d553203ecd7b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Thu, 16 Jul 2026 11:21:45 +0200 Subject: [PATCH] Polish "Add support for configuring start timeout for JMS health checks" See gh-50957 --- ...JmsHealthContributorAutoConfiguration.java | 2 +- .../health/JmsHealthIndicatorProperties.java | 18 +++++----- .../boot/jms/health/JmsHealthIndicator.java | 34 ++++++++++++++----- ...althContributorAutoConfigurationTests.java | 14 ++++---- .../jms/health/JmsHealthIndicatorTests.java | 18 +++++----- 5 files changed, 50 insertions(+), 36 deletions(-) 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 d3c49b689fb..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 @@ -48,7 +48,7 @@ public final class JmsHealthContributorAutoConfiguration extends CompositeHealthContributorConfiguration { JmsHealthContributorAutoConfiguration(JmsHealthIndicatorProperties properties) { - super((connectionFactory) -> new JmsHealthIndicator(connectionFactory, properties.getTimeout())); + 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 index d8b4f9ca6b5..d0cfcf24b69 100644 --- 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 @@ -20,13 +20,13 @@ import java.time.Duration; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.jms.health.JmsHealthIndicator; -import org.springframework.util.Assert; /** - * External configuration properties for {@link JmsHealthIndicator}. + * Configuration properties for {@link JmsHealthIndicator}. * * @author Venkata Naga Sai Srikanth Gollapudi - * @since 4.x + * @author Stephane Nicoll + * @since 4.2.0 */ @ConfigurationProperties("management.health.jms") public class JmsHealthIndicatorProperties { @@ -34,16 +34,14 @@ public class JmsHealthIndicatorProperties { /** * Timeout to use when starting a connection for the health check. */ - private Duration timeout = Duration.ofSeconds(5); + private Duration startTimeout = JmsHealthIndicator.DEFAULT_START_TIMEOUT; - public Duration getTimeout() { - return this.timeout; + public Duration getStartTimeout() { + return this.startTimeout; } - public void setTimeout(Duration timeout) { - Assert.notNull(timeout, "'timeout' must not be null"); - Assert.isTrue(timeout.compareTo(Duration.ZERO) > 0, "'timeout' must be greater than 0"); - this.timeout = timeout; + 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 625cdf7b68f..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 @@ -42,24 +42,39 @@ import org.springframework.util.Assert; */ public class JmsHealthIndicator extends AbstractHealthIndicator { - private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(5); + /** + * 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 timeout; + 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_TIMEOUT); + this(connectionFactory, DEFAULT_START_TIMEOUT); } - public JmsHealthIndicator(ConnectionFactory connectionFactory, Duration 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(timeout, "'timeout' must not be null"); - Assert.isTrue(timeout.compareTo(Duration.ZERO) > 0, "'timeout' must be greater than 0"); + 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.timeout = timeout; + this.startTimeout = startTimeout; } @Override @@ -83,10 +98,11 @@ public class JmsHealthIndicator extends AbstractHealthIndicator { void start() throws JMSException { new Thread(() -> { try { - if (!this.latch.await(JmsHealthIndicator.this.timeout.toNanos(), TimeUnit.NANOSECONDS)) { + Duration startTimeout1 = JmsHealthIndicator.this.startTimeout; + if (!this.latch.await(startTimeout1.toNanos(), TimeUnit.NANOSECONDS)) { JmsHealthIndicator.this.logger .warn(LogMessage.format("Connection failed to start within %s and will be closed.", - DurationStyle.SIMPLE.print(JmsHealthIndicator.this.timeout))); + 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 30b9ab31444..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 @@ -48,24 +48,24 @@ class JmsHealthContributorAutoConfigurationTests { } @Test - void runWhenTimeoutIsConfiguredShouldCreateIndicatorWithConfiguredTimeout() { - this.contextRunner.withPropertyValues("management.health.jms.timeout=10ms").run((context) -> { + 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).getTimeout()) + assertThat(context.getBean(JmsHealthIndicatorProperties.class).getStartTimeout()) .isEqualTo(Duration.ofMillis(10)); - assertThat(context.getBean(JmsHealthIndicator.class)).hasFieldOrPropertyWithValue("timeout", + assertThat(context.getBean(JmsHealthIndicator.class)).hasFieldOrPropertyWithValue("startTimeout", Duration.ofMillis(10)); }); } @Test - void runWhenTimeoutIsZeroShouldFail() { - this.contextRunner.withPropertyValues("management.health.jms.timeout=0ms") + void runWhenStartTimeoutIsZeroShouldFail() { + this.contextRunner.withPropertyValues("management.health.jms.start-timeout=0ms") .run((context) -> assertThat(context).hasFailed() .getFailure() .rootCause() - .hasMessage("'timeout' must be greater than 0")); + .hasMessage("'startTimeout' must be greater than 0")); } @Test 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 ee9c783b89b..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 @@ -47,25 +47,25 @@ class JmsHealthIndicatorTests { @Test @SuppressWarnings("NullAway") // Test null check - void createWhenTimeoutIsNullThrowsException() { + void createWhenStartTimeoutIsNullThrowsException() { ConnectionFactory connectionFactory = mock(ConnectionFactory.class); assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, null)) - .withMessage("'timeout' must not be null"); + .withMessage("'startTimeout' must not be null"); } @Test - void createWhenTimeoutIsZeroThrowsException() { + void createWhenStartTimeoutIsZeroThrowsException() { ConnectionFactory connectionFactory = mock(ConnectionFactory.class); assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ZERO)) - .withMessage("'timeout' must be greater than 0"); + .withMessage("'startTimeout' must be greater than 0"); } @Test - void createWhenTimeoutIsNegativeThrowsException() { + void createWhenStartTimeoutIsNegativeThrowsException() { ConnectionFactory connectionFactory = mock(ConnectionFactory.class); assertThatIllegalArgumentException() .isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ofMillis(-1))) - .withMessage("'timeout' must be greater than 0"); + .withMessage("'startTimeout' must be greater than 0"); } @Test @@ -131,13 +131,13 @@ class JmsHealthIndicatorTests { } @Test - void whenConnectionStartIsUnresponsiveUsesConfiguredTimeout() throws JMSException { + 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 timeout) throws JMSException { + private Health healthWhenConnectionStartIsUnresponsive(Duration startTimeout) throws JMSException { ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class); given(connectionMetaData.getJMSProviderName()).willReturn("JMS test provider"); Connection connection = mock(Connection.class); @@ -149,7 +149,7 @@ class JmsHealthIndicatorTests { }).given(connection).close(); ConnectionFactory connectionFactory = mock(ConnectionFactory.class); given(connectionFactory.createConnection()).willReturn(connection); - JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, timeout); + JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, startTimeout); return indicator.health(); }