From 960079e5f5680624f6e05b768929fb53710f38de Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 30 Jul 2019 22:26:46 +0200 Subject: [PATCH] Retain non-null HttpStatus return value in Client(Http)Response See gh-23366 --- .../reactive/MockClientHttpResponse.java | 7 +++-- .../http/client/ClientHttpResponse.java | 9 ++++--- .../client/reactive/ClientHttpResponse.java | 11 ++++---- .../reactive/ClientHttpResponseDecorator.java | 2 +- .../reactive/JettyClientHttpResponse.java | 2 +- .../reactive/ReactorClientHttpResponse.java | 2 +- .../reactive/test/MockClientHttpResponse.java | 7 +++-- .../function/client/ClientResponse.java | 11 ++++---- .../client/DefaultClientResponseBuilder.java | 2 +- .../DefaultClientResponseBuilderTests.java | 27 ++++++++----------- 10 files changed, 36 insertions(+), 44 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 eea0ebb07c..528dd2b2b6 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 @@ -71,7 +71,7 @@ public class MockClientHttpResponse implements ClientHttpResponse { @Override public HttpStatus getStatusCode() { - return HttpStatus.resolve(this.status); + return HttpStatus.valueOf(this.status); } @Override @@ -81,10 +81,9 @@ public class MockClientHttpResponse implements ClientHttpResponse { @Override public HttpHeaders getHeaders() { - String headerName = HttpHeaders.SET_COOKIE; - if (!getCookies().isEmpty() && this.headers.get(headerName) == null) { + if (!getCookies().isEmpty() && this.headers.get(HttpHeaders.SET_COOKIE) == null) { getCookies().values().stream().flatMap(Collection::stream) - .forEach(cookie -> getHeaders().add(headerName, cookie.toString())); + .forEach(cookie -> getHeaders().add(HttpHeaders.SET_COOKIE, cookie.toString())); } return this.headers; } diff --git a/spring-web/src/main/java/org/springframework/http/client/ClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/ClientHttpResponse.java index 03da70d13d..b3b08c2350 100644 --- a/spring-web/src/main/java/org/springframework/http/client/ClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/ClientHttpResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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. @@ -35,10 +35,11 @@ import org.springframework.http.HttpStatus; public interface ClientHttpResponse extends HttpInputMessage, Closeable { /** - * Return the HTTP status code of the response. - * @return the HTTP status as an HttpStatus enum value + * Return the HTTP status code as an {@link HttpStatus} enum value. + * @return the HTTP status as an HttpStatus enum value (never {@code null}) * @throws IOException in case of I/O errors * @throws IllegalArgumentException in case of an unknown HTTP status code + * @since #getRawStatusCode() * @see HttpStatus#valueOf(int) */ HttpStatus getStatusCode() throws IOException; @@ -46,7 +47,7 @@ public interface ClientHttpResponse extends HttpInputMessage, Closeable { /** * 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 + * @return the HTTP status as an integer value * @throws IOException in case of I/O errors * @since 3.1.1 * @see #getStatusCode() 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 559ff2ac12..43e711635b 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 @@ -19,7 +19,6 @@ package org.springframework.http.client.reactive; import org.springframework.http.HttpStatus; import org.springframework.http.ReactiveHttpInputMessage; import org.springframework.http.ResponseCookie; -import org.springframework.lang.Nullable; import org.springframework.util.MultiValueMap; /** @@ -32,18 +31,18 @@ import org.springframework.util.MultiValueMap; public interface ClientHttpResponse extends ReactiveHttpInputMessage { /** - * Return the HTTP status code of the response. - * @return the HTTP status as an HttpStatus enum value + * Return the HTTP status code as an {@link HttpStatus} enum value. + * @return the HTTP status as an HttpStatus enum value (never {@code null}) * @throws IllegalArgumentException in case of an unknown HTTP status code - * @see HttpStatus#resolve(int) + * @since #getRawStatusCode() + * @see HttpStatus#valueOf(int) */ - @Nullable 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 + * @return the HTTP status as an integer value * @since 5.0.6 * @see #getStatusCode() * @see HttpStatus#resolve(int) 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 90dd926079..233ea50476 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 @@ -48,7 +48,7 @@ public class ClientHttpResponseDecorator implements ClientHttpResponse { } - // ServerHttpResponse delegation methods... + // ClientHttpResponse delegation methods... @Override public HttpStatus getStatusCode() { diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java index 51b6c0ce42..60d94517da 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java @@ -53,7 +53,7 @@ class JettyClientHttpResponse implements ClientHttpResponse { @Override public HttpStatus getStatusCode() { - return HttpStatus.resolve(getRawStatusCode()); + return HttpStatus.valueOf(getRawStatusCode()); } @Override 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 c1e29443aa..38b9b58f36 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 @@ -87,7 +87,7 @@ class ReactorClientHttpResponse implements ClientHttpResponse { @Override public HttpStatus getStatusCode() { - return HttpStatus.resolve(getRawStatusCode()); + return HttpStatus.valueOf(getRawStatusCode()); } @Override 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 4ca93984de..f7b772a74c 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 @@ -71,7 +71,7 @@ public class MockClientHttpResponse implements ClientHttpResponse { @Override public HttpStatus getStatusCode() { - return HttpStatus.resolve(this.status); + return HttpStatus.valueOf(this.status); } @Override @@ -81,10 +81,9 @@ public class MockClientHttpResponse implements ClientHttpResponse { @Override public HttpHeaders getHeaders() { - String headerName = HttpHeaders.SET_COOKIE; - if (!getCookies().isEmpty() && this.headers.get(headerName) == null) { + if (!getCookies().isEmpty() && this.headers.get(HttpHeaders.SET_COOKIE) == null) { getCookies().values().stream().flatMap(Collection::stream) - .forEach(cookie -> getHeaders().add(headerName, cookie.toString())); + .forEach(cookie -> getHeaders().add(HttpHeaders.SET_COOKIE, cookie.toString())); } return this.headers; } 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 5190debac9..1adb667114 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 @@ -35,7 +35,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.http.client.reactive.ClientHttpResponse; import org.springframework.http.codec.HttpMessageReader; import org.springframework.http.codec.HttpMessageWriter; -import org.springframework.lang.Nullable; import org.springframework.util.MultiValueMap; import org.springframework.web.reactive.function.BodyExtractor; @@ -60,17 +59,17 @@ import org.springframework.web.reactive.function.BodyExtractor; public interface ClientResponse { /** - * Return the status code of this response. - * @return the status as an HttpStatus enum value + * Return the HTTP status code as an {@link HttpStatus} enum value. + * @return the HTTP status as an HttpStatus enum value (never {@code null}) * @throws IllegalArgumentException in case of an unknown HTTP status code - * @see HttpStatus#resolve(int) + * @since #getRawStatusCode() + * @see HttpStatus#valueOf(int) */ - @Nullable HttpStatus statusCode(); /** * Return the (potentially non-standard) status code of this response. - * @return the status as an integer + * @return the HTTP status as an integer value * @since 5.1 * @see #statusCode() * @see HttpStatus#resolve(int) 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 5fd19969eb..e24b4c08f6 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 @@ -164,7 +164,7 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { @Override public HttpStatus getStatusCode() { - return HttpStatus.resolve(this.statusCode); + return HttpStatus.valueOf(this.statusCode); } @Override diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java index 0a1edc1ee6..999b052af9 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java @@ -18,7 +18,6 @@ package org.springframework.web.reactive.function.client; import java.nio.charset.StandardCharsets; -import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Flux; import reactor.test.StepVerifier; @@ -32,19 +31,15 @@ import org.springframework.http.ResponseCookie; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; /** * @author Arjen Poutsma */ public class DefaultClientResponseBuilderTests { - private DataBufferFactory dataBufferFactory; + private final DataBufferFactory dataBufferFactory = new DefaultDataBufferFactory(); - @Before - public void createBufferFactory() { - this.dataBufferFactory = new DefaultDataBufferFactory(); - } @Test public void normal() { @@ -104,17 +99,17 @@ public class DefaultClientResponseBuilderTests { @Test public void fromCustomStatus() { - - ClientResponse other = ClientResponse.create(499, ExchangeStrategies.withDefaults()) - .build(); - - ClientResponse result = ClientResponse.from(other) - .build(); - + ClientResponse other = ClientResponse.create(499, ExchangeStrategies.withDefaults()).build(); + ClientResponse result = ClientResponse.from(other).build(); assertEquals(499, result.rawStatusCode()); - assertNull(result.statusCode()); + try { + result.statusCode(); + fail("Expected IllegalArgumentException"); + } + catch (IllegalArgumentException ex) { + // expected + } } - }