Skip to content

Commit 3691c18

Browse files
committed
Preserve order of onStatus handlers
Closes gh-23880
1 parent 74b7b55 commit 3691c18

File tree

2 files changed

+54
-2
lines changed

2 files changed

+54
-2
lines changed

spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,21 +419,28 @@ private static class DefaultResponseSpec implements ResponseSpec {
419419

420420
private static final IntPredicate STATUS_CODE_ERROR = value -> value >= 400;
421421

422+
private static final StatusHandler DEFAULT_STATUS_HANDLER =
423+
new StatusHandler(STATUS_CODE_ERROR, ClientResponse::createException);
424+
425+
422426
private final Mono<ClientResponse> responseMono;
423427

424428
private final Supplier<HttpRequest> requestSupplier;
425429

426430
private final List<StatusHandler> statusHandlers = new ArrayList<>(1);
427431

432+
428433
DefaultResponseSpec(Mono<ClientResponse> responseMono, Supplier<HttpRequest> requestSupplier) {
429434
this.responseMono = responseMono;
430435
this.requestSupplier = requestSupplier;
431-
this.statusHandlers.add(new StatusHandler(STATUS_CODE_ERROR, ClientResponse::createException));
436+
this.statusHandlers.add(DEFAULT_STATUS_HANDLER);
432437
}
433438

439+
434440
@Override
435441
public ResponseSpec onStatus(Predicate<HttpStatus> statusPredicate,
436442
Function<ClientResponse, Mono<? extends Throwable>> exceptionFunction) {
443+
437444
return onRawStatus(toIntPredicate(statusPredicate), exceptionFunction);
438445
}
439446

@@ -450,7 +457,8 @@ public ResponseSpec onRawStatus(IntPredicate statusCodePredicate,
450457

451458
Assert.notNull(statusCodePredicate, "IntPredicate must not be null");
452459
Assert.notNull(exceptionFunction, "Function must not be null");
453-
this.statusHandlers.add(0, new StatusHandler(statusCodePredicate, exceptionFunction));
460+
int index = this.statusHandlers.size() - 1; // Default handler always last
461+
this.statusHandlers.add(index, new StatusHandler(statusCodePredicate, exceptionFunction));
454462
return this;
455463
}
456464

spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Collections;
2121
import java.util.HashMap;
2222
import java.util.Map;
23+
import java.util.function.Predicate;
2324

2425
import org.junit.jupiter.api.BeforeEach;
2526
import org.junit.jupiter.api.Test;
@@ -33,6 +34,7 @@
3334

3435
import org.springframework.core.NamedThreadLocal;
3536
import org.springframework.http.HttpHeaders;
37+
import org.springframework.http.HttpStatus;
3638
import org.springframework.http.MediaType;
3739

3840
import static org.assertj.core.api.Assertions.assertThat;
@@ -309,6 +311,48 @@ public void shouldApplyFiltersAtSubscription() {
309311
assertThat(request.headers().getFirst("Custom")).isEqualTo("value");
310312
}
311313

314+
@Test // gh-23880
315+
public void onStatusHandlersOrderIsPreserved() {
316+
317+
ClientResponse response = ClientResponse.create(HttpStatus.BAD_REQUEST).build();
318+
given(exchangeFunction.exchange(any())).willReturn(Mono.just(response));
319+
320+
Mono<Void> result = this.builder.build().get()
321+
.uri("/path")
322+
.retrieve()
323+
.onStatus(HttpStatus::is4xxClientError, resp -> Mono.error(new IllegalStateException("1")))
324+
.onStatus(HttpStatus::is4xxClientError, resp -> Mono.error(new IllegalStateException("2")))
325+
.bodyToMono(Void.class);
326+
327+
StepVerifier.create(result).expectErrorMessage("1").verify();
328+
}
329+
330+
@Test // gh-23880
331+
@SuppressWarnings("unchecked")
332+
public void onStatusHandlersDefaultHandlerIsLast() {
333+
334+
ClientResponse response = ClientResponse.create(HttpStatus.BAD_REQUEST).build();
335+
given(exchangeFunction.exchange(any())).willReturn(Mono.just(response));
336+
337+
Predicate<HttpStatus> predicate1 = mock(Predicate.class);
338+
Predicate<HttpStatus> predicate2 = mock(Predicate.class);
339+
340+
given(predicate1.test(HttpStatus.BAD_REQUEST)).willReturn(false);
341+
given(predicate2.test(HttpStatus.BAD_REQUEST)).willReturn(false);
342+
343+
Mono<Void> result = this.builder.build().get()
344+
.uri("/path")
345+
.retrieve()
346+
.onStatus(predicate1, resp -> Mono.error(new IllegalStateException()))
347+
.onStatus(predicate2, resp -> Mono.error(new IllegalStateException()))
348+
.bodyToMono(Void.class);
349+
350+
StepVerifier.create(result).expectError(WebClientResponseException.class).verify();
351+
352+
verify(predicate1).test(HttpStatus.BAD_REQUEST);
353+
verify(predicate2).test(HttpStatus.BAD_REQUEST);
354+
}
355+
312356
private ClientRequest verifyAndGetRequest() {
313357
ClientRequest request = this.captor.getValue();
314358
verify(this.exchangeFunction).exchange(request);

0 commit comments

Comments
 (0)