From ba9325446ce6a7f5863883e9ce344041c7d40168 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 12 Nov 2020 18:36:31 +0000 Subject: [PATCH] Optimize WebClientUtils Use constant Predicate for exception wrapping. Use ResponseEntity constructor instead of builder. See gh-26069 --- .../springframework/http/ResponseEntity.java | 15 ++++++++-- .../function/client/DefaultWebClient.java | 30 +++++-------------- .../function/client/ExchangeFunctions.java | 2 +- .../function/client/WebClientUtils.java | 27 +++++++++-------- 4 files changed, 35 insertions(+), 39 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/ResponseEntity.java b/spring-web/src/main/java/org/springframework/http/ResponseEntity.java index 591b56606f..3cd5b81edc 100644 --- a/spring-web/src/main/java/org/springframework/http/ResponseEntity.java +++ b/spring-web/src/main/java/org/springframework/http/ResponseEntity.java @@ -113,9 +113,18 @@ public class ResponseEntity extends HttpEntity { * @param status the status code */ public ResponseEntity(@Nullable T body, @Nullable MultiValueMap headers, HttpStatus status) { - super(body, headers); - Assert.notNull(status, "HttpStatus must not be null"); - this.status = status; + this(body, headers, (Object) status); + } + + /** + * Create a new {@code HttpEntity} with the given body, headers, and status code. + * @param body the entity body + * @param headers the entity headers + * @param rawStatus the status code value + * @since 5.3.2 + */ + public ResponseEntity(@Nullable T body, @Nullable MultiValueMap headers, int rawStatus) { + this(body, headers, (Object) rawStatus); } /** diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java index 659bf81734..f45841c7f3 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java @@ -543,16 +543,10 @@ class DefaultWebClient implements WebClient { return this.responseMono.flatMap(response -> handleBodyMono(response, response.bodyToMono(elementTypeRef))); } - private Mono handleBodyMono(ClientResponse response, Mono bodyPublisher) { + private Mono handleBodyMono(ClientResponse response, Mono body) { + body = body.onErrorResume(WebClientUtils.WRAP_EXCEPTION_PREDICATE, exceptionWrappingFunction(response)); Mono result = statusHandlers(response); - Mono wrappedExceptions = bodyPublisher.onErrorResume(WebClientUtils::shouldWrapException, - t -> wrapException(t, response)); - if (result != null) { - return result.switchIfEmpty(wrappedExceptions); - } - else { - return wrappedExceptions; - } + return (result != null ? result.switchIfEmpty(body) : body); } @Override @@ -567,16 +561,10 @@ class DefaultWebClient implements WebClient { return this.responseMono.flatMapMany(response -> handleBodyFlux(response, response.bodyToFlux(elementTypeRef))); } - private Publisher handleBodyFlux(ClientResponse response, Flux bodyPublisher) { + private Publisher handleBodyFlux(ClientResponse response, Flux body) { + body = body.onErrorResume(WebClientUtils.WRAP_EXCEPTION_PREDICATE, exceptionWrappingFunction(response)); Mono result = statusHandlers(response); - Flux wrappedExceptions = bodyPublisher.onErrorResume(WebClientUtils::shouldWrapException, - t -> wrapException(t, response)); - if (result != null) { - return result.flux().switchIfEmpty(wrappedExceptions); - } - else { - return wrappedExceptions; - } + return (result != null ? result.flux().switchIfEmpty(body) : body); } @Nullable @@ -608,10 +596,8 @@ class DefaultWebClient implements WebClient { return result.checkpoint(description); } - private Mono wrapException(Throwable throwable, ClientResponse response) { - return response.createException() - .map(responseException -> responseException.initCause(throwable)) - .flatMap(Mono::error); + private Function> exceptionWrappingFunction(ClientResponse response) { + return t -> response.createException().flatMap(ex -> Mono.error(ex.initCause(t))); } @Override diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java index 8566a1bad8..d06f6bda3d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java @@ -104,7 +104,7 @@ public abstract class ExchangeFunctions { .connect(httpMethod, url, httpRequest -> clientRequest.writeTo(httpRequest, this.strategies)) .doOnRequest(n -> logRequest(clientRequest)) .doOnCancel(() -> logger.debug(logPrefix + "Cancel signal (to close connection)")) - .onErrorResume(WebClientUtils::shouldWrapException, t -> wrapException(t, clientRequest)) + .onErrorResume(WebClientUtils.WRAP_EXCEPTION_PREDICATE, t -> wrapException(t, clientRequest)) .map(httpResponse -> { logResponse(httpResponse, logPrefix); return new DefaultClientResponse( diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java index 799ef5bf79..c234e0e578 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java @@ -17,6 +17,7 @@ package org.springframework.web.reactive.function.client; import java.util.List; +import java.util.function.Predicate; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -26,7 +27,8 @@ import org.springframework.core.codec.CodecException; import org.springframework.http.ResponseEntity; /** - * Internal methods shared between {@link DefaultWebClient} and {@link DefaultClientResponse}. + * Internal methods shared between {@link DefaultWebClient} and + * {@link DefaultClientResponse}. * * @author Arjen Poutsma * @since 5.2 @@ -35,6 +37,12 @@ abstract class WebClientUtils { private static final String VALUE_NONE = "\n\t\t\n\t\t\n\uE000\uE001\uE002\n\t\t\t\t\n"; + /** + * Predicate that returns true if an exception should be wrapped. + */ + public final static Predicate WRAP_EXCEPTION_PREDICATE = + t -> !(t instanceof WebClientException) && !(t instanceof CodecException); + /** * Map the given response to a single value {@code ResponseEntity}. @@ -42,9 +50,10 @@ abstract class WebClientUtils { @SuppressWarnings("unchecked") public static Mono> mapToEntity(ClientResponse response, Mono bodyMono) { return ((Mono) bodyMono).defaultIfEmpty(VALUE_NONE).map(body -> - ResponseEntity.status(response.rawStatusCode()) - .headers(response.headers().asHttpHeaders()) - .body(body != VALUE_NONE ? (T) body : null)); + new ResponseEntity<>( + body != VALUE_NONE ? (T) body : null, + response.headers().asHttpHeaders(), + response.rawStatusCode())); } /** @@ -52,15 +61,7 @@ abstract class WebClientUtils { */ public static Mono>> mapToEntityList(ClientResponse response, Publisher body) { return Flux.from(body).collectList().map(list -> - ResponseEntity.status(response.rawStatusCode()) - .headers(response.headers().asHttpHeaders()) - .body(list)); + new ResponseEntity<>(list, response.headers().asHttpHeaders(), response.rawStatusCode())); } - /** - * Indicates whether the given exception should be wrapped. - */ - public static boolean shouldWrapException(Throwable t) { - return !(t instanceof WebClientException) && !(t instanceof CodecException); - } }