From a9548f93e4ad581d1be069c89ccad0ac820eaefa Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 27 Apr 2018 18:25:11 +0200 Subject: [PATCH] Support for non-standard HTTP status in reactive ClientHttpResponse Issue: SPR-16748 (cherry picked from commit a683472) --- .../reactive/MockClientHttpResponse.java | 10 ++++- .../client/reactive/ClientHttpResponse.java | 17 +++++++- .../reactive/ClientHttpResponseDecorator.java | 7 +++- .../reactive/ReactorClientHttpResponse.java | 14 ++++--- .../reactive/test/MockClientHttpResponse.java | 10 ++++- .../function/client/ClientRequest.java | 7 +--- .../function/client/ClientResponse.java | 26 +++++------- .../client/DefaultClientResponse.java | 15 ++++--- .../client/DefaultClientResponseBuilder.java | 41 ++++++++----------- .../function/client/ExchangeFunctions.java | 12 +++--- .../function/server/ServerRequest.java | 5 +-- .../function/server/ServerResponse.java | 3 -- 12 files changed, 92 insertions(+), 75 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/http/client/reactive/MockClientHttpResponse.java b/spring-test/src/main/java/org/springframework/mock/http/client/reactive/MockClientHttpResponse.java index d14abc1fb3e..ee0673c33bd 100644 --- a/spring-test/src/main/java/org/springframework/mock/http/client/reactive/MockClientHttpResponse.java +++ b/spring-test/src/main/java/org/springframework/mock/http/client/reactive/MockClientHttpResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,6 +40,7 @@ import org.springframework.util.MultiValueMap; /** * Mock implementation of {@link ClientHttpResponse}. + * * @author Brian Clozel * @author Rossen Stoyanchev * @since 5.0 @@ -68,6 +69,11 @@ public class MockClientHttpResponse implements ClientHttpResponse { return this.status; } + @Override + public int getRawStatusCode() { + return this.status.value(); + } + @Override public HttpHeaders getHeaders() { String headerName = HttpHeaders.SET_COOKIE; @@ -138,4 +144,4 @@ public class MockClientHttpResponse implements ClientHttpResponse { return (charset != null ? charset : StandardCharsets.UTF_8); } -} \ No newline at end of file +} diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponse.java index df478d432ae..a9f2f7c063a 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,10 +31,23 @@ import org.springframework.util.MultiValueMap; public interface ClientHttpResponse extends ReactiveHttpInputMessage { /** - * Return the HTTP status as an {@link HttpStatus} enum value. + * Return the HTTP status code of the response. + * @return the HTTP status as an HttpStatus enum value + * @throws IllegalArgumentException in case of an unknown HTTP status code + * @see HttpStatus#valueOf(int) */ HttpStatus getStatusCode(); + /** + * Return the HTTP status code (potentially non-standard and not + * resolvable through the {@link HttpStatus} enum) as an integer. + * @return the HTTP status as an integer + * @since 5.0.6 + * @see #getStatusCode() + * @see HttpStatus#resolve(int) + */ + int getRawStatusCode(); + /** * Return a read-only map of response cookies received from the server. */ diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponseDecorator.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponseDecorator.java index 199a9495cff..2ca32e3de6d 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponseDecorator.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ClientHttpResponseDecorator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -55,6 +55,11 @@ public class ClientHttpResponseDecorator implements ClientHttpResponse { return this.delegate.getStatusCode(); } + @Override + public int getRawStatusCode() { + return this.delegate.getRawStatusCode(); + } + @Override public HttpHeaders getHeaders() { return this.delegate.getHeaders(); diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java index 54aea4ae730..adbf6b1b5f2 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -62,14 +62,18 @@ class ReactorClientHttpResponse implements ClientHttpResponse { @Override public HttpHeaders getHeaders() { HttpHeaders headers = new HttpHeaders(); - this.response.responseHeaders().entries() - .forEach(e -> headers.add(e.getKey(), e.getValue())); + this.response.responseHeaders().entries().forEach(e -> headers.add(e.getKey(), e.getValue())); return headers; } @Override public HttpStatus getStatusCode() { - return HttpStatus.valueOf(this.response.status().code()); + return HttpStatus.valueOf(getRawStatusCode()); + } + + @Override + public int getRawStatusCode() { + return this.response.status().code(); } @Override @@ -91,7 +95,7 @@ class ReactorClientHttpResponse implements ClientHttpResponse { public String toString() { return "ReactorClientHttpResponse{" + "request=[" + this.response.method().name() + " " + this.response.uri() + "]," + - "status=" + getStatusCode() + '}'; + "status=" + getRawStatusCode() + '}'; } } diff --git a/spring-web/src/test/java/org/springframework/mock/http/client/reactive/test/MockClientHttpResponse.java b/spring-web/src/test/java/org/springframework/mock/http/client/reactive/test/MockClientHttpResponse.java index 88554014e0c..8fc030bb242 100644 --- a/spring-web/src/test/java/org/springframework/mock/http/client/reactive/test/MockClientHttpResponse.java +++ b/spring-web/src/test/java/org/springframework/mock/http/client/reactive/test/MockClientHttpResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,6 +40,7 @@ import org.springframework.util.MultiValueMap; /** * Mock implementation of {@link ClientHttpResponse}. + * * @author Brian Clozel * @author Rossen Stoyanchev * @since 5.0 @@ -68,6 +69,11 @@ public class MockClientHttpResponse implements ClientHttpResponse { return this.status; } + @Override + public int getRawStatusCode() { + return this.status.value(); + } + @Override public HttpHeaders getHeaders() { String headerName = HttpHeaders.SET_COOKIE; @@ -138,4 +144,4 @@ public class MockClientHttpResponse implements ClientHttpResponse { return (charset != null ? charset : StandardCharsets.UTF_8); } -} \ No newline at end of file +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java index 9cc9d0c50a6..b9fb4a78bc5 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientRequest.java @@ -86,7 +86,6 @@ public interface ClientRequest { } } - /** * Return the attributes of this request. */ @@ -222,8 +221,7 @@ public interface ClientRequest { * @param

the type of the {@code Publisher} * @return the built request */ - > Builder body(P publisher, - ParameterizedTypeReference typeReference); + > Builder body(P publisher, ParameterizedTypeReference typeReference); /** * Set the attribute with the given name to the given value. @@ -243,8 +241,7 @@ public interface ClientRequest { Builder attributes(Consumer> attributesConsumer); /** - * Builds the request. - * @return the request + * Build the request. */ ClientRequest build(); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java index e15862e9548..15251067e64 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java @@ -160,13 +160,13 @@ public interface ClientResponse { * @return the created builder */ static Builder from(ClientResponse other) { - Assert.notNull(other, "'other' must not be null"); + Assert.notNull(other, "Other ClientResponse must not be null"); return new DefaultClientResponseBuilder(other); } /** - * Create a response builder with the given status code and using default strategies for reading - * the body. + * Create a response builder with the given status code and using default strategies for + * reading the body. * @param statusCode the status code * @return the created builder */ @@ -181,10 +181,7 @@ public interface ClientResponse { * @return the created builder */ static Builder create(HttpStatus statusCode, ExchangeStrategies strategies) { - Assert.notNull(statusCode, "'statusCode' must not be null"); - Assert.notNull(strategies, "'strategies' must not be null"); - return new DefaultClientResponseBuilder(strategies) - .statusCode(statusCode); + return new DefaultClientResponseBuilder(strategies).statusCode(statusCode); } /** @@ -194,24 +191,20 @@ public interface ClientResponse { * @return the created builder */ static Builder create(HttpStatus statusCode, List> messageReaders) { - Assert.notNull(statusCode, "'statusCode' must not be null"); - Assert.notNull(messageReaders, "'messageReaders' must not be null"); - return create(statusCode, new ExchangeStrategies() { @Override public List> messageReaders() { return messageReaders; } - @Override public List> messageWriters() { // not used in the response return Collections.emptyList(); } }); - } + /** * Represents the headers of the HTTP response. * @see ClientResponse#headers() @@ -243,6 +236,7 @@ public interface ClientResponse { HttpHeaders asHttpHeaders(); } + /** * Defines a builder for a response. */ @@ -295,7 +289,7 @@ public interface ClientResponse { Builder cookies(Consumer> cookiesConsumer); /** - * Sets the body of the response. Calling this methods will + * Set the body of the response. Calling this methods will * {@linkplain org.springframework.core.io.buffer.DataBufferUtils#release(DataBuffer) release} * the existing body of the builder. * @param body the new body. @@ -304,7 +298,7 @@ public interface ClientResponse { Builder body(Flux body); /** - * Sets the body of the response to the UTF-8 encoded bytes of the given string. + * Set the body of the response to the UTF-8 encoded bytes of the given string. * Calling this methods will * {@linkplain org.springframework.core.io.buffer.DataBufferUtils#release(DataBuffer) release} * the existing body of the builder. @@ -314,9 +308,9 @@ public interface ClientResponse { Builder body(String body); /** - * Builds the response. - * @return the response + * Build the response. */ ClientResponse build(); } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java index 3c4bbd3806e..64647ccdb1b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java @@ -61,6 +61,7 @@ class DefaultClientResponse implements ClientResponse { this.headers = new DefaultHeaders(); } + @Override public ExchangeStrategies strategies() { return this.strategies; @@ -88,12 +89,10 @@ class DefaultClientResponse implements ClientResponse { public List> messageReaders() { return strategies.messageReaders(); } - @Override public Optional serverResponse() { return Optional.empty(); } - @Override public Map hints() { return Collections.emptyMap(); @@ -187,8 +186,7 @@ class DefaultClientResponse implements ClientResponse { } @Override - public Mono>> toEntityList( - ParameterizedTypeReference typeReference) { + public Mono>> toEntityList(ParameterizedTypeReference typeReference) { return toEntityListInternal(bodyToFlux(typeReference)); } @@ -220,7 +218,7 @@ class DefaultClientResponse implements ClientResponse { @Override public List header(String headerName) { List headerValues = delegate().get(headerName); - return headerValues != null ? headerValues : Collections.emptyList(); + return (headerValues != null ? headerValues : Collections.emptyList()); } @Override @@ -229,12 +227,13 @@ class DefaultClientResponse implements ClientResponse { } private OptionalLong toOptionalLong(long value) { - return value != -1 ? OptionalLong.of(value) : OptionalLong.empty(); + return (value != -1 ? OptionalLong.of(value) : OptionalLong.empty()); } - } + @SuppressWarnings("serial") - private class ReadCancellationException extends RuntimeException { + private static class ReadCancellationException extends RuntimeException { } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java index c8293f92e5f..39b4e5e7d1b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java @@ -29,7 +29,6 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; import org.springframework.http.client.reactive.ClientHttpResponse; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; @@ -55,7 +54,7 @@ class DefaultClientResponseBuilder implements ClientResponse.Builder { public DefaultClientResponseBuilder(ExchangeStrategies strategies) { - Assert.notNull(strategies, "'strategies' must not be null"); + Assert.notNull(strategies, "ExchangeStrategies must not be null"); this.strategies = strategies; } @@ -66,9 +65,10 @@ class DefaultClientResponseBuilder implements ClientResponse.Builder { cookies(cookies -> cookies.addAll(other.cookies())); } + @Override public DefaultClientResponseBuilder statusCode(HttpStatus statusCode) { - Assert.notNull(statusCode, "'statusCode' must not be null"); + Assert.notNull(statusCode, "HttpStatus must not be null"); this.statusCode = statusCode; return this; } @@ -83,7 +83,7 @@ class DefaultClientResponseBuilder implements ClientResponse.Builder { @Override public ClientResponse.Builder headers(Consumer headersConsumer) { - Assert.notNull(headersConsumer, "'headersConsumer' must not be null"); + Assert.notNull(headersConsumer, "Consumer must not be null"); headersConsumer.accept(this.headers); return this; } @@ -97,16 +97,15 @@ class DefaultClientResponseBuilder implements ClientResponse.Builder { } @Override - public ClientResponse.Builder cookies( - Consumer> cookiesConsumer) { - Assert.notNull(cookiesConsumer, "'cookiesConsumer' must not be null"); + public ClientResponse.Builder cookies(Consumer> cookiesConsumer) { + Assert.notNull(cookiesConsumer, "Consumer must not be null"); cookiesConsumer.accept(this.cookies); return this; } @Override public ClientResponse.Builder body(Flux body) { - Assert.notNull(body, "'body' must not be null"); + Assert.notNull(body, "Body must not be null"); releaseBody(); this.body = body; return this; @@ -114,7 +113,7 @@ class DefaultClientResponseBuilder implements ClientResponse.Builder { @Override public ClientResponse.Builder body(String body) { - Assert.notNull(body, "'body' must not be null"); + Assert.notNull(body, "Body must not be null"); releaseBody(); DataBufferFactory dataBufferFactory = new DefaultDataBufferFactory(); this.body = Flux.just(body). @@ -131,11 +130,12 @@ class DefaultClientResponseBuilder implements ClientResponse.Builder { @Override public ClientResponse build() { - ClientHttpResponse clientHttpResponse = new BuiltClientHttpResponse(this.statusCode, - this.headers, this.cookies, this.body); + ClientHttpResponse clientHttpResponse = new BuiltClientHttpResponse( + this.statusCode, this.headers, this.cookies, this.body); return new DefaultClientResponse(clientHttpResponse, this.strategies); } + private static class BuiltClientHttpResponse implements ClientHttpResponse { private final HttpStatus statusCode; @@ -147,29 +147,24 @@ class DefaultClientResponseBuilder implements ClientResponse.Builder { private final Flux body; public BuiltClientHttpResponse(HttpStatus statusCode, HttpHeaders headers, - MultiValueMap cookies, - Flux body) { + MultiValueMap cookies, Flux body) { this.statusCode = statusCode; this.headers = HttpHeaders.readOnlyHttpHeaders(headers); - this.cookies = unmodifiableCopy(cookies); + this.cookies = CollectionUtils.unmodifiableMultiValueMap(cookies); this.body = body; } - private static @Nullable MultiValueMap unmodifiableCopy(@Nullable MultiValueMap original) { - if (original != null) { - return CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<>(original)); - } - else { - return null; - } - } - @Override public HttpStatus getStatusCode() { return this.statusCode; } + @Override + public int getRawStatusCode() { + return this.statusCode.value(); + } + @Override public HttpHeaders getHeaders() { return this.headers; 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 97662ed8791..24ca1352ea2 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 @@ -55,8 +55,8 @@ public abstract class ExchangeFunctions { * @return the created function */ public static ExchangeFunction create(ClientHttpConnector connector, ExchangeStrategies strategies) { - Assert.notNull(connector, "'connector' must not be null"); - Assert.notNull(strategies, "'strategies' must not be null"); + Assert.notNull(connector, "ClientHttpConnector must not be null"); + Assert.notNull(strategies, "ExchangeStrategies must not be null"); return new DefaultExchangeFunction(connector, strategies); } @@ -74,7 +74,7 @@ public abstract class ExchangeFunctions { @Override public Mono exchange(ClientRequest request) { - Assert.notNull(request, "'request' must not be null"); + Assert.notNull(request, "ClientRequest must not be null"); return this.connector .connect(request.method(), request.url(), clientHttpRequest -> request.writeTo(clientHttpRequest, this.strategies)) @@ -82,9 +82,11 @@ public abstract class ExchangeFunctions { .doOnRequest(n -> logger.debug("Demand signaled")) .doOnCancel(() -> logger.debug("Cancelling request")) .map(response -> { - HttpStatus status = response.getStatusCode(); if (logger.isDebugEnabled()) { - logger.debug("Response received, status: " + status + " " + status.getReasonPhrase()); + int status = response.getRawStatusCode(); + HttpStatus resolvedStatus = HttpStatus.resolve(status); + logger.debug("Response received, status: " + status + + (resolvedStatus != null ? " " + resolvedStatus.getReasonPhrase() : "")); } return new DefaultClientResponse(response, this.strategies); }); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java index 9ee7d8cb2d3..5eea3dff9ca 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java @@ -274,12 +274,11 @@ public interface ServerRequest { * @param messageReaders the message readers * @return the created {@code ServerRequest} */ - static ServerRequest create(ServerWebExchange exchange, - List> messageReaders) { - + static ServerRequest create(ServerWebExchange exchange, List> messageReaders) { return new DefaultServerRequest(exchange, messageReaders); } + /** * Represents the headers of the HTTP request. * @see ServerRequest#headers() diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerResponse.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerResponse.java index 3990342462f..50e6f9c1597 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerResponse.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerResponse.java @@ -322,7 +322,6 @@ public interface ServerResponse { /** * Build the response entity with no body. - * @return the built response */ Mono build(); @@ -330,14 +329,12 @@ public interface ServerResponse { * Build the response entity with no body. * The response will be committed when the given {@code voidPublisher} completes. * @param voidPublisher publisher publisher to indicate when the response should be committed - * @return the built response */ Mono build(Publisher voidPublisher); /** * Build the response entity with a custom writer function. * @param writeFunction the function used to write to the {@link ServerWebExchange} - * @return the built response */ Mono build(BiFunction> writeFunction); }