Skip to content

Commit 7b0aa7d

Browse files
wilkinsonawilkinsona
committed
Merge pull request #35915 from ThomazPassarelli
* gh-35915: Polish "Replace calls to verifyComplete() to avoid indefinite blocking" Replace calls to verifyComplete() to avoid indefinite blocking Closes gh-35915
2 parents cee73ce + d2966e1 commit 7b0aa7d

File tree

25 files changed

+168
-71
lines changed

25 files changed

+168
-71
lines changed

‎buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java

+17-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ void checkArchitecture() throws IOException {
7474
.importPaths(this.classes.getFiles().stream().map(File::toPath).collect(Collectors.toList()));
7575
List<EvaluationResult> violations = Stream.of(allPackagesShouldBeFreeOfTangles(),
7676
allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization(),
77-
allBeanFactoryPostProcessorBeanMethodsShouldBeStaticAndHaveNoParameters())
77+
allBeanFactoryPostProcessorBeanMethodsShouldBeStaticAndHaveNoParameters(),
78+
noClassesShouldCallStepVerifierStepVerifyComplete(),
79+
noClassesShouldConfigureDefaultStepVerifierTimeout())
7880
.map((rule) -> rule.evaluate(javaClasses))
7981
.filter(EvaluationResult::hasViolation)
8082
.collect(Collectors.toList());
@@ -163,6 +165,20 @@ public void check(JavaMethod item, ConditionEvents events) {
163165
};
164166
}
165167

168+
private ArchRule noClassesShouldCallStepVerifierStepVerifyComplete() {
169+
return ArchRuleDefinition.noClasses()
170+
.should()
171+
.callMethod("reactor.test.StepVerifier$Step", "verifyComplete")
172+
.because("it can block indefinitely and expectComplete().verify(Duration) should be used instead");
173+
}
174+
175+
private ArchRule noClassesShouldConfigureDefaultStepVerifierTimeout() {
176+
return ArchRuleDefinition.noClasses()
177+
.should()
178+
.callMethod("reactor.test.StepVerifier", "setDefaultTimeout", "java.time.Duration")
179+
.because("expectComplete().verify(Duration) should be used instead");
180+
}
181+
166182
public void setClasses(FileCollection classes) {
167183
this.classes = classes;
168184
}

‎spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/reactive/ReactiveCloudFoundrySecurityInterceptorTests.java

+12-6
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.boot.actuate.autoconfigure.cloudfoundry.reactive;
1818

19+
import java.time.Duration;
20+
1921
import org.junit.jupiter.api.BeforeEach;
2022
import org.junit.jupiter.api.Test;
2123
import org.junit.jupiter.api.extension.ExtendWith;
@@ -66,7 +68,8 @@ void preHandleWhenRequestIsPreFlightShouldBeOk() {
6668
.build());
6769
StepVerifier.create(this.interceptor.preHandle(request, "/a"))
6870
.consumeNextWith((response) -> assertThat(response.getStatus()).isEqualTo(HttpStatus.OK))
69-
.verifyComplete();
71+
.expectComplete()
72+
.verify(Duration.ofSeconds(30));
7073
}
7174

7275
@Test
@@ -75,7 +78,8 @@ void preHandleWhenTokenIsMissingShouldReturnMissingAuthorization() {
7578
StepVerifier.create(this.interceptor.preHandle(request, "/a"))
7679
.consumeNextWith(
7780
(response) -> assertThat(response.getStatus()).isEqualTo(Reason.MISSING_AUTHORIZATION.getStatus()))
78-
.verifyComplete();
81+
.expectComplete()
82+
.verify(Duration.ofSeconds(30));
7983
}
8084

8185
@Test
@@ -85,7 +89,8 @@ void preHandleWhenTokenIsNotBearerShouldReturnMissingAuthorization() {
8589
StepVerifier.create(this.interceptor.preHandle(request, "/a"))
8690
.consumeNextWith(
8791
(response) -> assertThat(response.getStatus()).isEqualTo(Reason.MISSING_AUTHORIZATION.getStatus()))
88-
.verifyComplete();
92+
.expectComplete()
93+
.verify(Duration.ofSeconds(30));
8994
}
9095

9196
@Test
@@ -121,7 +126,8 @@ void preHandleWhenAccessIsNotAllowedShouldReturnAccessDenied() {
121126
.build());
122127
StepVerifier.create(this.interceptor.preHandle(request, "/a"))
123128
.consumeNextWith((response) -> assertThat(response.getStatus()).isEqualTo(Reason.ACCESS_DENIED.getStatus()))
124-
.verifyComplete();
129+
.expectComplete()
130+
.verify(Duration.ofSeconds(30));
125131
}
126132

127133
@Test
@@ -135,7 +141,7 @@ void preHandleSuccessfulWithFullAccess() {
135141
StepVerifier.create(this.interceptor.preHandle(exchange, "/a")).consumeNextWith((response) -> {
136142
assertThat(response.getStatus()).isEqualTo(HttpStatus.OK);
137143
assertThat((AccessLevel) exchange.getAttribute("cloudFoundryAccessLevel")).isEqualTo(AccessLevel.FULL);
138-
}).verifyComplete();
144+
}).expectComplete().verify(Duration.ofSeconds(30));
139145
}
140146

141147
@Test
@@ -151,7 +157,7 @@ void preHandleSuccessfulWithRestrictedAccess() {
151157
assertThat(response.getStatus()).isEqualTo(HttpStatus.OK);
152158
assertThat((AccessLevel) exchange.getAttribute("cloudFoundryAccessLevel"))
153159
.isEqualTo(AccessLevel.RESTRICTED);
154-
}).verifyComplete();
160+
}).expectComplete().verify(Duration.ofSeconds(30));
155161
}
156162

157163
private String mockAccessToken() {

‎spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/reactive/ReactiveTokenValidatorTests.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.security.Signature;
2626
import java.security.spec.InvalidKeySpecException;
2727
import java.security.spec.PKCS8EncodedKeySpec;
28+
import java.time.Duration;
2829
import java.util.Collections;
2930
import java.util.Map;
3031
import java.util.concurrent.ConcurrentHashMap;
@@ -121,7 +122,8 @@ void validateTokenWhenKidValidationSucceedsInTheSecondAttempt() throws Exception
121122
String claims = "{\"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
122123
StepVerifier
123124
.create(this.tokenValidator.validate(new Token(getSignedToken(header.getBytes(), claims.getBytes()))))
124-
.verifyComplete();
125+
.expectComplete()
126+
.verify(Duration.ofSeconds(30));
125127
assertThat(this.tokenValidator).hasFieldOrPropertyWithValue("cachedTokenKeys", VALID_KEYS);
126128
fetchTokenKeys.assertWasSubscribed();
127129
}
@@ -135,7 +137,8 @@ void validateTokenWhenCacheIsEmptyShouldFetchTokenKeys() throws Exception {
135137
String claims = "{\"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
136138
StepVerifier
137139
.create(this.tokenValidator.validate(new Token(getSignedToken(header.getBytes(), claims.getBytes()))))
138-
.verifyComplete();
140+
.expectComplete()
141+
.verify(Duration.ofSeconds(30));
139142
assertThat(this.tokenValidator).hasFieldOrPropertyWithValue("cachedTokenKeys", VALID_KEYS);
140143
fetchTokenKeys.assertWasSubscribed();
141144
}
@@ -167,7 +170,8 @@ void validateTokenWhenCacheValidShouldNotFetchTokenKeys() throws Exception {
167170
String claims = "{\"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
168171
StepVerifier
169172
.create(this.tokenValidator.validate(new Token(getSignedToken(header.getBytes(), claims.getBytes()))))
170-
.verifyComplete();
173+
.expectComplete()
174+
.verify(Duration.ofSeconds(30));
171175
fetchTokenKeys.assertWasNotSubscribed();
172176
}
173177

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cassandra/CassandraDriverReactiveHealthIndicatorTests.java

+18-10
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.boot.actuate.cassandra;
1818

19+
import java.time.Duration;
1920
import java.util.ArrayList;
2021
import java.util.Collections;
2122
import java.util.HashMap;
@@ -61,7 +62,8 @@ void healthWithOneHealthyNodeShouldReturnUp() {
6162
Mono<Health> health = healthIndicator.health();
6263
StepVerifier.create(health)
6364
.consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.UP))
64-
.verifyComplete();
65+
.expectComplete()
66+
.verify(Duration.ofSeconds(30));
6567
}
6668

6769
@Test
@@ -71,7 +73,8 @@ void healthWithOneUnhealthyNodeShouldReturnDown() {
7173
Mono<Health> health = healthIndicator.health();
7274
StepVerifier.create(health)
7375
.consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.DOWN))
74-
.verifyComplete();
76+
.expectComplete()
77+
.verify(Duration.ofSeconds(30));
7578
}
7679

7780
@Test
@@ -81,7 +84,8 @@ void healthWithOneUnknownNodeShouldReturnDown() {
8184
Mono<Health> health = healthIndicator.health();
8285
StepVerifier.create(health)
8386
.consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.DOWN))
84-
.verifyComplete();
87+
.expectComplete()
88+
.verify(Duration.ofSeconds(30));
8589
}
8690

8791
@Test
@@ -91,7 +95,8 @@ void healthWithOneForcedDownNodeShouldReturnDown() {
9195
Mono<Health> health = healthIndicator.health();
9296
StepVerifier.create(health)
9397
.consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.DOWN))
94-
.verifyComplete();
98+
.expectComplete()
99+
.verify(Duration.ofSeconds(30));
95100
}
96101

97102
@Test
@@ -101,7 +106,8 @@ void healthWithOneHealthyNodeAndOneUnhealthyNodeShouldReturnUp() {
101106
Mono<Health> health = healthIndicator.health();
102107
StepVerifier.create(health)
103108
.consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.UP))
104-
.verifyComplete();
109+
.expectComplete()
110+
.verify(Duration.ofSeconds(30));
105111
}
106112

107113
@Test
@@ -111,7 +117,8 @@ void healthWithOneHealthyNodeAndOneUnknownNodeShouldReturnUp() {
111117
Mono<Health> health = healthIndicator.health();
112118
StepVerifier.create(health)
113119
.consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.UP))
114-
.verifyComplete();
120+
.expectComplete()
121+
.verify(Duration.ofSeconds(30));
115122
}
116123

117124
@Test
@@ -121,7 +128,8 @@ void healthWithOneHealthyNodeAndOneForcedDownNodeShouldReturnUp() {
121128
Mono<Health> health = healthIndicator.health();
122129
StepVerifier.create(health)
123130
.consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.UP))
124-
.verifyComplete();
131+
.expectComplete()
132+
.verify(Duration.ofSeconds(30));
125133
}
126134

127135
@Test
@@ -139,7 +147,7 @@ void healthWithNodeVersionShouldAddVersionDetail() {
139147
assertThat(h.getStatus()).isEqualTo(Status.UP);
140148
assertThat(h.getDetails()).containsOnlyKeys("version");
141149
assertThat(h.getDetails().get("version")).isEqualTo(Version.V4_0_0);
142-
}).verifyComplete();
150+
}).expectComplete().verify(Duration.ofSeconds(30));
143151
}
144152

145153
@Test
@@ -150,7 +158,7 @@ void healthWithoutNodeVersionShouldNotAddVersionDetail() {
150158
StepVerifier.create(health).consumeNextWith((h) -> {
151159
assertThat(h.getStatus()).isEqualTo(Status.UP);
152160
assertThat(h.getDetails().get("version")).isNull();
153-
}).verifyComplete();
161+
}).expectComplete().verify(Duration.ofSeconds(30));
154162
}
155163

156164
@Test
@@ -165,7 +173,7 @@ void healthWithCassandraDownShouldReturnDown() {
165173
assertThat(h.getDetails()).containsOnlyKeys("error");
166174
assertThat(h.getDetails().get("error"))
167175
.isEqualTo(DriverTimeoutException.class.getName() + ": Test Exception");
168-
}).verifyComplete();
176+
}).expectComplete().verify(Duration.ofSeconds(30));
169177
}
170178

171179
private CqlSession mockCqlSessionWithNodeState(NodeState... nodeStates) {

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthIndicatorReactiveAdapterTests.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ void delegateReturnsHealth() {
3737
HealthIndicatorReactiveAdapter adapter = new HealthIndicatorReactiveAdapter(delegate);
3838
Health status = Health.up().build();
3939
given(delegate.health()).willReturn(status);
40-
StepVerifier.create(adapter.health()).expectNext(status).verifyComplete();
40+
StepVerifier.create(adapter.health()).expectNext(status).expectComplete().verify(Duration.ofSeconds(30));
4141
}
4242

4343
@Test
@@ -55,7 +55,10 @@ void delegateRunsOnTheElasticScheduler() {
5555
.status(Thread.currentThread().getName().equals(currentThread) ? Status.DOWN : Status.UP)
5656
.build();
5757
HealthIndicatorReactiveAdapter adapter = new HealthIndicatorReactiveAdapter(delegate);
58-
StepVerifier.create(adapter.health()).expectNext(Health.status(Status.UP).build()).verifyComplete();
58+
StepVerifier.create(adapter.health())
59+
.expectNext(Health.status(Status.UP).build())
60+
.expectComplete()
61+
.verify(Duration.ofSeconds(30));
5962
}
6063

6164
}

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/ReactiveHealthIndicatorImplementationTests.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.boot.actuate.health;
1818

19+
import java.time.Duration;
20+
1921
import org.junit.jupiter.api.Test;
2022
import org.junit.jupiter.api.extension.ExtendWith;
2123
import reactor.core.publisher.Mono;
@@ -40,7 +42,8 @@ class ReactiveHealthIndicatorImplementationTests {
4042
void healthUp(CapturedOutput output) {
4143
StepVerifier.create(new SimpleReactiveHealthIndicator().health())
4244
.consumeNextWith((health) -> assertThat(health).isEqualTo(Health.up().build()))
43-
.verifyComplete();
45+
.expectComplete()
46+
.verify(Duration.ofSeconds(30));
4447
assertThat(output).doesNotContain("Health check failed for simple");
4548
}
4649

@@ -49,15 +52,17 @@ void healthDownWithCustomErrorMessage(CapturedOutput output) {
4952
StepVerifier.create(new CustomErrorMessageReactiveHealthIndicator().health())
5053
.consumeNextWith(
5154
(health) -> assertThat(health).isEqualTo(Health.down(new UnsupportedOperationException()).build()))
52-
.verifyComplete();
55+
.expectComplete()
56+
.verify(Duration.ofSeconds(30));
5357
assertThat(output).contains("Health check failed for custom");
5458
}
5559

5660
@Test
5761
void healthDownWithCustomErrorMessageFunction(CapturedOutput output) {
5862
StepVerifier.create(new CustomErrorMessageFunctionReactiveHealthIndicator().health())
5963
.consumeNextWith((health) -> assertThat(health).isEqualTo(Health.down(new RuntimeException()).build()))
60-
.verifyComplete();
64+
.expectComplete()
65+
.verify(Duration.ofSeconds(30));
6166
assertThat(output).contains("Health check failed with RuntimeException");
6267
}
6368

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/r2dbc/ConnectionPoolMetricsTests.java

+13-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2021 the original author or authors.
2+
* Copyright 2012-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.boot.actuate.metrics.r2dbc;
1818

19+
import java.time.Duration;
1920
import java.util.Collections;
2021
import java.util.UUID;
2122

@@ -59,7 +60,7 @@ void init() {
5960
@AfterEach
6061
void close() {
6162
if (this.connectionFactory != null) {
62-
StepVerifier.create(this.connectionFactory.close()).verifyComplete();
63+
StepVerifier.create(this.connectionFactory.close()).expectComplete().verify(Duration.ofSeconds(30));
6364
}
6465
}
6566

@@ -72,8 +73,16 @@ void connectionFactoryIsInstrumented() {
7273
Tags.of(testTag, regionTag));
7374
metrics.bindTo(registry);
7475
// acquire two connections
75-
connectionPool.create().as(StepVerifier::create).expectNextCount(1).verifyComplete();
76-
connectionPool.create().as(StepVerifier::create).expectNextCount(1).verifyComplete();
76+
connectionPool.create()
77+
.as(StepVerifier::create)
78+
.expectNextCount(1)
79+
.expectComplete()
80+
.verify(Duration.ofSeconds(30));
81+
connectionPool.create()
82+
.as(StepVerifier::create)
83+
.expectNextCount(1)
84+
.expectComplete()
85+
.verify(Duration.ofSeconds(30));
7786
assertGauge(registry, "r2dbc.pool.acquired", 2);
7887
assertGauge(registry, "r2dbc.pool.allocated", 3);
7988
assertGauge(registry, "r2dbc.pool.idle", 1);

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ void disconnectedExceptionShouldProduceMetrics() {
162162
exchange.getResponse().setRawStatusCode(500);
163163
return exchange.getResponse().setComplete();
164164
});
165-
StepVerifier.create(processing).expectComplete().verify(Duration.ofSeconds(5));
165+
StepVerifier.create(processing).expectComplete().verify(Duration.ofSeconds(30));
166166
assertMetricsContainsTag("uri", "/projects/{project}");
167167
assertMetricsContainsTag("status", "500");
168168
assertMetricsContainsTag("outcome", "UNKNOWN");

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/mongo/MongoReactiveHealthIndicatorTests.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.boot.actuate.mongo;
1818

19+
import java.time.Duration;
20+
1921
import com.mongodb.MongoException;
2022
import org.bson.Document;
2123
import org.junit.jupiter.api.Test;
@@ -50,7 +52,7 @@ void testMongoIsUp() {
5052
assertThat(h.getStatus()).isEqualTo(Status.UP);
5153
assertThat(h.getDetails()).containsOnlyKeys("version");
5254
assertThat(h.getDetails().get("version")).isEqualTo("2.6.4");
53-
}).verifyComplete();
55+
}).expectComplete().verify(Duration.ofSeconds(30));
5456
}
5557

5658
@Test
@@ -65,7 +67,7 @@ void testMongoIsDown() {
6567
assertThat(h.getStatus()).isEqualTo(Status.DOWN);
6668
assertThat(h.getDetails()).containsOnlyKeys("error");
6769
assertThat(h.getDetails().get("error")).isEqualTo(MongoException.class.getName() + ": Connection failed");
68-
}).verifyComplete();
70+
}).expectComplete().verify(Duration.ofSeconds(30));
6971
}
7072

7173
}

0 commit comments

Comments
 (0)