Skip to content

Commit db12d4d

Browse files
snicollsnicoll
committed
Polish "Add support for configuring start timeout for JMS health checks"
See gh-50957
1 parent 5f7bdcc commit db12d4d

5 files changed

Lines changed: 50 additions & 36 deletions

File tree

‎module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfiguration.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public final class JmsHealthContributorAutoConfiguration
4848
extends CompositeHealthContributorConfiguration<JmsHealthIndicator, ConnectionFactory> {
4949

5050
JmsHealthContributorAutoConfiguration(JmsHealthIndicatorProperties properties) {
51-
super((connectionFactory) -> new JmsHealthIndicator(connectionFactory, properties.getTimeout()));
51+
super((connectionFactory) -> new JmsHealthIndicator(connectionFactory, properties.getStartTimeout()));
5252
}
5353

5454
@Bean

‎module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthIndicatorProperties.java‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,30 +20,28 @@
2020

2121
import org.springframework.boot.context.properties.ConfigurationProperties;
2222
import org.springframework.boot.jms.health.JmsHealthIndicator;
23-
import org.springframework.util.Assert;
2423

2524
/**
26-
* External configuration properties for {@link JmsHealthIndicator}.
25+
* Configuration properties for {@link JmsHealthIndicator}.
2726
*
2827
* @author Venkata Naga Sai Srikanth Gollapudi
29-
* @since 4.x
28+
* @author Stephane Nicoll
29+
* @since 4.2.0
3030
*/
3131
@ConfigurationProperties("management.health.jms")
3232
public class JmsHealthIndicatorProperties {
3333

3434
/**
3535
* Timeout to use when starting a connection for the health check.
3636
*/
37-
private Duration timeout = Duration.ofSeconds(5);
37+
private Duration startTimeout = JmsHealthIndicator.DEFAULT_START_TIMEOUT;
3838

39-
public Duration getTimeout() {
40-
return this.timeout;
39+
public Duration getStartTimeout() {
40+
return this.startTimeout;
4141
}
4242

43-
public void setTimeout(Duration timeout) {
44-
Assert.notNull(timeout, "'timeout' must not be null");
45-
Assert.isTrue(timeout.compareTo(Duration.ZERO) > 0, "'timeout' must be greater than 0");
46-
this.timeout = timeout;
43+
public void setStartTimeout(Duration startTimeout) {
44+
this.startTimeout = startTimeout;
4745
}
4846

4947
}

‎module/spring-boot-jms/src/main/java/org/springframework/boot/jms/health/JmsHealthIndicator.java‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,39 @@
4242
*/
4343
public class JmsHealthIndicator extends AbstractHealthIndicator {
4444

45-
private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(5);
45+
/**
46+
* Default timeout to use when starting a connection for the health check.
47+
*/
48+
public static final Duration DEFAULT_START_TIMEOUT = Duration.ofSeconds(5);
4649

4750
private final Log logger = LogFactory.getLog(JmsHealthIndicator.class);
4851

4952
private final ConnectionFactory connectionFactory;
5053

51-
private final Duration timeout;
54+
private final Duration startTimeout;
5255

56+
/**
57+
* Create a new {@link JmsHealthIndicator} instance with a
58+
* {@linkplain #DEFAULT_START_TIMEOUT default} start timeout.
59+
* @param connectionFactory the connection factory to use
60+
*/
5361
public JmsHealthIndicator(ConnectionFactory connectionFactory) {
54-
this(connectionFactory, DEFAULT_TIMEOUT);
62+
this(connectionFactory, DEFAULT_START_TIMEOUT);
5563
}
5664

57-
public JmsHealthIndicator(ConnectionFactory connectionFactory, Duration timeout) {
65+
/**
66+
* Create a new {@link JmsHealthIndicator} instance with the given
67+
* {@code startTimeout}.
68+
* @param connectionFactory the connection factory to use
69+
* @param startTimeout timeout to use when starting a connection for the health check
70+
* @since 4.2.0
71+
*/
72+
public JmsHealthIndicator(ConnectionFactory connectionFactory, Duration startTimeout) {
5873
super("JMS health check failed");
59-
Assert.notNull(timeout, "'timeout' must not be null");
60-
Assert.isTrue(timeout.compareTo(Duration.ZERO) > 0, "'timeout' must be greater than 0");
74+
Assert.notNull(startTimeout, "'startTimeout' must not be null");
75+
Assert.isTrue(startTimeout.compareTo(Duration.ZERO) > 0, "'startTimeout' must be greater than 0");
6176
this.connectionFactory = connectionFactory;
62-
this.timeout = timeout;
77+
this.startTimeout = startTimeout;
6378
}
6479

6580
@Override
@@ -83,10 +98,11 @@ private final class MonitoredConnection {
8398
void start() throws JMSException {
8499
new Thread(() -> {
85100
try {
86-
if (!this.latch.await(JmsHealthIndicator.this.timeout.toNanos(), TimeUnit.NANOSECONDS)) {
101+
Duration startTimeout1 = JmsHealthIndicator.this.startTimeout;
102+
if (!this.latch.await(startTimeout1.toNanos(), TimeUnit.NANOSECONDS)) {
87103
JmsHealthIndicator.this.logger
88104
.warn(LogMessage.format("Connection failed to start within %s and will be closed.",
89-
DurationStyle.SIMPLE.print(JmsHealthIndicator.this.timeout)));
105+
DurationStyle.SIMPLE.print(startTimeout1)));
90106
closeConnection();
91107
}
92108
}

‎module/spring-boot-jms/src/test/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfigurationTests.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,24 +48,24 @@ void runShouldCreateIndicator() {
4848
}
4949

5050
@Test
51-
void runWhenTimeoutIsConfiguredShouldCreateIndicatorWithConfiguredTimeout() {
52-
this.contextRunner.withPropertyValues("management.health.jms.timeout=10ms").run((context) -> {
51+
void runWhenStartTimeoutIsConfiguredShouldCreateIndicatorWithConfiguredStartTimeout() {
52+
this.contextRunner.withPropertyValues("management.health.jms.start-timeout=10ms").run((context) -> {
5353
assertThat(context).hasSingleBean(JmsHealthIndicator.class);
5454
assertThat(context).hasSingleBean(JmsHealthIndicatorProperties.class);
55-
assertThat(context.getBean(JmsHealthIndicatorProperties.class).getTimeout())
55+
assertThat(context.getBean(JmsHealthIndicatorProperties.class).getStartTimeout())
5656
.isEqualTo(Duration.ofMillis(10));
57-
assertThat(context.getBean(JmsHealthIndicator.class)).hasFieldOrPropertyWithValue("timeout",
57+
assertThat(context.getBean(JmsHealthIndicator.class)).hasFieldOrPropertyWithValue("startTimeout",
5858
Duration.ofMillis(10));
5959
});
6060
}
6161

6262
@Test
63-
void runWhenTimeoutIsZeroShouldFail() {
64-
this.contextRunner.withPropertyValues("management.health.jms.timeout=0ms")
63+
void runWhenStartTimeoutIsZeroShouldFail() {
64+
this.contextRunner.withPropertyValues("management.health.jms.start-timeout=0ms")
6565
.run((context) -> assertThat(context).hasFailed()
6666
.getFailure()
6767
.rootCause()
68-
.hasMessage("'timeout' must be greater than 0"));
68+
.hasMessage("'startTimeout' must be greater than 0"));
6969
}
7070

7171
@Test

‎module/spring-boot-jms/src/test/java/org/springframework/boot/jms/health/JmsHealthIndicatorTests.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,25 +47,25 @@ class JmsHealthIndicatorTests {
4747

4848
@Test
4949
@SuppressWarnings("NullAway") // Test null check
50-
void createWhenTimeoutIsNullThrowsException() {
50+
void createWhenStartTimeoutIsNullThrowsException() {
5151
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
5252
assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, null))
53-
.withMessage("'timeout' must not be null");
53+
.withMessage("'startTimeout' must not be null");
5454
}
5555

5656
@Test
57-
void createWhenTimeoutIsZeroThrowsException() {
57+
void createWhenStartTimeoutIsZeroThrowsException() {
5858
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
5959
assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ZERO))
60-
.withMessage("'timeout' must be greater than 0");
60+
.withMessage("'startTimeout' must be greater than 0");
6161
}
6262

6363
@Test
64-
void createWhenTimeoutIsNegativeThrowsException() {
64+
void createWhenStartTimeoutIsNegativeThrowsException() {
6565
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
6666
assertThatIllegalArgumentException()
6767
.isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ofMillis(-1)))
68-
.withMessage("'timeout' must be greater than 0");
68+
.withMessage("'startTimeout' must be greater than 0");
6969
}
7070

7171
@Test
@@ -131,13 +131,13 @@ void whenConnectionStartIsUnresponsiveStatusIsDown() throws JMSException {
131131
}
132132

133133
@Test
134-
void whenConnectionStartIsUnresponsiveUsesConfiguredTimeout() throws JMSException {
134+
void whenConnectionStartIsUnresponsiveUsesConfiguredStartTimeout() throws JMSException {
135135
Health health = healthWhenConnectionStartIsUnresponsive(Duration.ofMillis(10));
136136
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
137137
assertThat((String) health.getDetails().get("error")).contains("Connection closed");
138138
}
139139

140-
private Health healthWhenConnectionStartIsUnresponsive(Duration timeout) throws JMSException {
140+
private Health healthWhenConnectionStartIsUnresponsive(Duration startTimeout) throws JMSException {
141141
ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class);
142142
given(connectionMetaData.getJMSProviderName()).willReturn("JMS test provider");
143143
Connection connection = mock(Connection.class);
@@ -149,7 +149,7 @@ private Health healthWhenConnectionStartIsUnresponsive(Duration timeout) throws
149149
}).given(connection).close();
150150
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
151151
given(connectionFactory.createConnection()).willReturn(connection);
152-
JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, timeout);
152+
JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, startTimeout);
153153
return indicator.health();
154154
}
155155

0 commit comments

Comments
 (0)