Polishing in DefaultResponseErrorHandler

See gh-33980

spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java

@@ -38,7 +38,6 @@ import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
-import org.springframework.util.FileCopyUtils;
 import org.springframework.util.ObjectUtils;
 
 /**
@@ -135,8 +134,7 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 	 */
 	@Override
 	public void handleError(ClientHttpResponse response) throws IOException {
-		HttpStatusCode statusCode = response.getStatusCode();
-		handleError(response, statusCode, null, null);
+		handleError(response, response.getStatusCode(), null, null);
 	}
 
 	/**
@@ -159,46 +157,7 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 	 */
 	@Override
 	public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
-		HttpStatusCode statusCode = response.getStatusCode();
-		handleError(response, statusCode, url, method);
-	}
-
-	/**
-	 * Return error message with details from the response body. For example:
-	 * <pre>
-	 * 404 Not Found on GET request for "https://example.com": [{'id': 123, 'message': 'my message'}]
-	 * </pre>
-	 */
-	private String getErrorMessage(int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset,
-			@Nullable URI url, @Nullable HttpMethod method) {
-
-		StringBuilder msg = new StringBuilder(rawStatusCode + " " + statusText);
-		if (method != null) {
-			msg.append(" on ").append(method).append(" request");
-		}
-		if (url != null) {
-			msg.append(" for \"");
-			String urlString = url.toString();
-			int idx = urlString.indexOf('?');
-			if (idx != -1) {
-				msg.append(urlString, 0, idx);
-			}
-			else {
-				msg.append(urlString);
-			}
-			msg.append("\"");
-		}
-		msg.append(": ");
-		if (ObjectUtils.isEmpty(responseBody)) {
-			msg.append("[no body]");
-		}
-		else {
-			charset = (charset != null ? charset : StandardCharsets.UTF_8);
-			String bodyText = new String(responseBody, charset);
-			bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
-			msg.append(bodyText);
-		}
-		return msg.toString();
+		handleError(response, response.getStatusCode(), url, method);
 	}
 
 	/**
@@ -211,7 +170,8 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 	 * @see HttpClientErrorException#create
 	 * @see HttpServerErrorException#create
 	 */
-	protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode,
+	protected void handleError(
+			ClientHttpResponse response, HttpStatusCode statusCode,
 			@Nullable URI url, @Nullable HttpMethod method) throws IOException {
 
 		String statusText = response.getStatusText();
@@ -238,6 +198,68 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 		throw ex;
 	}
 
+	/**
+	 * Read the body of the given response (for inclusion in a status exception).
+	 * @param response the response to inspect
+	 * @return the response body as a byte array,
+	 * or an empty byte array if the body could not be read
+	 * @since 4.3.8
+	 */
+	protected byte[] getResponseBody(ClientHttpResponse response) {
+		return RestClientUtils.getBody(response);
+	}
+
+	/**
+	 * Determine the charset of the response (for inclusion in a status exception).
+	 * @param response the response to inspect
+	 * @return the associated charset, or {@code null} if none
+	 * @since 4.3.8
+	 */
+	@Nullable
+	protected Charset getCharset(ClientHttpResponse response) {
+		MediaType contentType = response.getHeaders().getContentType();
+		return (contentType != null ? contentType.getCharset() : null);
+	}
+
+	/**
+	 * Return an error message with details from the response body. For example:
+	 * <pre>
+	 * 404 Not Found on GET request for "https://example.com": [{'id': 123, 'message': 'my message'}]
+	 * </pre>
+	 */
+	private String getErrorMessage(
+			int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset,
+			@Nullable URI url, @Nullable HttpMethod method) {
+
+		StringBuilder msg = new StringBuilder(rawStatusCode + " " + statusText);
+		if (method != null) {
+			msg.append(" on ").append(method).append(" request");
+		}
+		if (url != null) {
+			msg.append(" for \"");
+			String urlString = url.toString();
+			int idx = urlString.indexOf('?');
+			if (idx != -1) {
+				msg.append(urlString, 0, idx);
+			}
+			else {
+				msg.append(urlString);
+			}
+			msg.append("\"");
+		}
+		msg.append(": ");
+		if (ObjectUtils.isEmpty(responseBody)) {
+			msg.append("[no body]");
+		}
+		else {
+			charset = (charset != null ? charset : StandardCharsets.UTF_8);
+			String bodyText = new String(responseBody, charset);
+			bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
+			msg.append(bodyText);
+		}
+		return msg.toString();
+	}
+
 	/**
 	 * Return a function for decoding the error content. This can be passed to
 	 * {@link RestClientResponseException#setBodyConvertFunction(Function)}.
@@ -265,34 +287,4 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 		};
 	}
 
-	/**
-	 * Read the body of the given response (for inclusion in a status exception).
-	 * @param response the response to inspect
-	 * @return the response body as a byte array,
-	 * or an empty byte array if the body could not be read
-	 * @since 4.3.8
-	 */
-	protected byte[] getResponseBody(ClientHttpResponse response) {
-		try {
-			return FileCopyUtils.copyToByteArray(response.getBody());
-		}
-		catch (IOException ex) {
-			// ignore
-		}
-		return new byte[0];
-	}
-
-	/**
-	 * Determine the charset of the response (for inclusion in a status exception).
-	 * @param response the response to inspect
-	 * @return the associated charset, or {@code null} if none
-	 * @since 4.3.8
-	 */
-	@Nullable
-	protected Charset getCharset(ClientHttpResponse response) {
-		HttpHeaders headers = response.getHeaders();
-		MediaType contentType = headers.getContentType();
-		return (contentType != null ? contentType.getCharset() : null);
-	}
-
 }

spring-web/src/main/java/org/springframework/web/client/ExtractingResponseErrorHandler.java

@@ -32,26 +32,27 @@ import org.springframework.lang.Nullable;
 import org.springframework.util.CollectionUtils;
 
 /**
- * Implementation of {@link ResponseErrorHandler} that uses {@link HttpMessageConverter
- * HttpMessageConverters} to convert HTTP error responses to {@link RestClientException
- * RestClientExceptions}.
+ * Implementation of {@link ResponseErrorHandler} that uses
+ * {@link HttpMessageConverter HttpMessageConverters} to convert HTTP error
+ * responses to {@link RestClientException RestClientExceptions}.
  *
  * <p>To use this error handler, you must specify a
  * {@linkplain #setStatusMapping(Map) status mapping} and/or a
- * {@linkplain #setSeriesMapping(Map) series mapping}. If either of these mappings has a match
- * for the {@linkplain ClientHttpResponse#getStatusCode() status code} of a given
- * {@code ClientHttpResponse}, {@link #hasError(ClientHttpResponse)} will return
- * {@code true}, and {@link #handleError(ClientHttpResponse)} will attempt to use the
- * {@linkplain #setMessageConverters(List) configured message converters} to convert the response
- * into the mapped subclass of {@link RestClientException}. Note that the
- * {@linkplain #setStatusMapping(Map) status mapping} takes precedence over
- * {@linkplain #setSeriesMapping(Map) series mapping}.
+ * {@linkplain #setSeriesMapping(Map) series mapping}. If either of these
+ * mappings has a match for the {@linkplain ClientHttpResponse#getStatusCode()
+ * status code} of a given {@code ClientHttpResponse},
+ * {@link #hasError(ClientHttpResponse)} will return {@code true}, and
+ * {@link #handleError(ClientHttpResponse)} will attempt to use the
+ * {@linkplain #setMessageConverters(List) configured message converters} to
+ * convert the response into the mapped subclass of {@link RestClientException}.
+ * Note that the {@linkplain #setStatusMapping(Map) status mapping} takes
+ * precedence over {@linkplain #setSeriesMapping(Map) series mapping}.
  *
  * <p>If there is no match, this error handler will default to the behavior of
- * {@link DefaultResponseErrorHandler}. Note that you can override this default behavior
- * by specifying a {@linkplain #setSeriesMapping(Map) series mapping} from
- * {@code HttpStatus.Series#CLIENT_ERROR} and/or {@code HttpStatus.Series#SERVER_ERROR}
- * to {@code null}.
+ * {@link DefaultResponseErrorHandler}. Note that you can override this default
+ * behavior by specifying a {@linkplain #setSeriesMapping(Map) series mapping}
+ * from {@code HttpStatus.Series#CLIENT_ERROR} and/or
+ * {@code HttpStatus.Series#SERVER_ERROR} to {@code null}.
  *
  * @author Simon Galperin
  * @author Arjen Poutsma
@@ -126,11 +127,11 @@ public class ExtractingResponseErrorHandler extends DefaultResponseErrorHandler
 	@Override
 	protected boolean hasError(HttpStatusCode statusCode) {
 		if (this.statusMapping.containsKey(statusCode)) {
-			return this.statusMapping.get(statusCode) != null;
+			return (this.statusMapping.get(statusCode) != null);
 		}
 		HttpStatus.Series series = HttpStatus.Series.resolve(statusCode.value());
 		if (this.seriesMapping.containsKey(series)) {
-			return this.seriesMapping.get(series) != null;
+			return (this.seriesMapping.get(series) != null);
 		}
 		else {
 			return super.hasError(statusCode);
@@ -138,15 +139,14 @@ public class ExtractingResponseErrorHandler extends DefaultResponseErrorHandler
 	}
 
 	@Override
-	public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
-		handleError(response, response.getStatusCode(), url, method);
-	}
+	protected void handleError(
+			ClientHttpResponse response, HttpStatusCode statusCode,
+			@Nullable URI url, @Nullable HttpMethod method) throws IOException {
 
-	@Override
-	protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode, @Nullable URI url, @Nullable HttpMethod method) throws IOException {
 		if (this.statusMapping.containsKey(statusCode)) {
 			extract(this.statusMapping.get(statusCode), response);
 		}
+
 		HttpStatus.Series series = HttpStatus.Series.resolve(statusCode.value());
 		if (this.seriesMapping.containsKey(series)) {
 			extract(this.seriesMapping.get(series), response);
@@ -156,8 +156,9 @@ public class ExtractingResponseErrorHandler extends DefaultResponseErrorHandler
 		}
 	}
 
-	private void extract(@Nullable Class<? extends RestClientException> exceptionClass,
-			ClientHttpResponse response) throws IOException {
+	private void extract(
+			@Nullable Class<? extends RestClientException> exceptionClass, ClientHttpResponse response)
+			throws IOException {
 
 		if (exceptionClass == null) {
 			return;
@@ -165,6 +166,7 @@ public class ExtractingResponseErrorHandler extends DefaultResponseErrorHandler
 
 		HttpMessageConverterExtractor<? extends RestClientException> extractor =
 				new HttpMessageConverterExtractor<>(exceptionClass, this.messageConverters);
+
 		RestClientException exception = extractor.extractData(response);
 		if (exception != null) {
 			throw exception;

spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java

@@ -19,7 +19,6 @@ package org.springframework.web.client;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.net.URI;
-import java.nio.charset.StandardCharsets;
 
 import org.junit.jupiter.api.Test;
 
@@ -32,6 +31,7 @@ import org.springframework.http.client.ClientHttpResponse;
 import org.springframework.lang.Nullable;
 import org.springframework.util.StreamUtils;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.catchThrowable;
@@ -72,7 +72,7 @@ class DefaultResponseErrorHandlerTests {
 		given(response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
 		given(response.getStatusText()).willReturn("Not Found");
 		given(response.getHeaders()).willReturn(headers);
-		given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(StandardCharsets.UTF_8)));
+		given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(UTF_8)));
 
 		assertThatExceptionOfType(HttpClientErrorException.class)
 				.isThrownBy(() -> handler.handleError(response))
@@ -90,18 +90,20 @@ class DefaultResponseErrorHandlerTests {
 
 	@Test
 	void handleErrorWithUrlAndQueryParameters() throws Exception {
+		String url = "https://example.com/resource";
 		setupClientHttpResponse(HttpStatus.NOT_FOUND, "Hello World");
 		assertThatExceptionOfType(HttpClientErrorException.class)
-				.isThrownBy(() -> handler.handleError(URI.create("https://example.com/resource?access_token=123"), HttpMethod.GET, response))
-				.withMessage("404 Not Found on GET request for \"https://example.com/resource\": \"Hello World\"");
+				.isThrownBy(() -> handler.handleError(URI.create(url + "?access_token=123"), HttpMethod.GET, response))
+				.withMessage("404 Not Found on GET request for \"" + url + "\": \"Hello World\"");
 	}
 
 	@Test
 	void handleErrorWithUrlAndNoBody() throws Exception {
+		String url = "https://example.com";
 		setupClientHttpResponse(HttpStatus.NOT_FOUND, null);
 		assertThatExceptionOfType(HttpClientErrorException.class)
-				.isThrownBy(() -> handler.handleError(URI.create("https://example.com"), HttpMethod.GET, response))
-				.withMessage("404 Not Found on GET request for \"https://example.com\": [no body]");
+				.isThrownBy(() -> handler.handleError(URI.create(url), HttpMethod.GET, response))
+				.withMessage("404 Not Found on GET request for \"" + url + "\": [no body]");
 	}
 
 	private void setupClientHttpResponse(HttpStatus status, @Nullable String textBody) throws Exception {
@@ -110,7 +112,7 @@ class DefaultResponseErrorHandlerTests {
 		given(response.getStatusText()).willReturn(status.getReasonPhrase());
 		if (textBody != null) {
 			headers.setContentType(MediaType.TEXT_PLAIN);
-			given(response.getBody()).willReturn(new ByteArrayInputStream(textBody.getBytes(StandardCharsets.UTF_8)));
+			given(response.getBody()).willReturn(new ByteArrayInputStream(textBody.getBytes(UTF_8)));
 		}
 		given(response.getHeaders()).willReturn(headers);
 	}
@@ -187,7 +189,7 @@ class DefaultResponseErrorHandlerTests {
 		headers.setContentType(MediaType.TEXT_PLAIN);
 
 		String responseBody = "Hello World";
-		TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
+		TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(UTF_8));
 
 		given(response.getStatusCode()).willReturn(statusCode);
 		given(response.getStatusText()).willReturn(statusText);
@@ -227,7 +229,7 @@ class DefaultResponseErrorHandlerTests {
 		headers.setContentType(MediaType.TEXT_PLAIN);
 
 		String responseBody = "Hello World";
-		TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
+		TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(UTF_8));
 
 		given(response.getStatusCode()).willReturn(statusCode);
 		given(response.getStatusText()).willReturn(statusText);
@@ -250,7 +252,7 @@ class DefaultResponseErrorHandlerTests {
 	public void bodyAvailableAfterHasErrorForUnknownStatusCode() throws Exception {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.TEXT_PLAIN);
-		TestByteArrayInputStream body = new TestByteArrayInputStream("Hello World".getBytes(StandardCharsets.UTF_8));
+		TestByteArrayInputStream body = new TestByteArrayInputStream("Hello World".getBytes(UTF_8));
 
 		given(response.getStatusCode()).willReturn(HttpStatusCode.valueOf(999));
 		given(response.getStatusText()).willReturn("Custom status code");
@@ -259,7 +261,7 @@ class DefaultResponseErrorHandlerTests {
 
 		assertThat(handler.hasError(response)).isFalse();
 		assertThat(body.isClosed()).isFalse();
-		assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.UTF_8)).isEqualTo("Hello World");
+		assertThat(StreamUtils.copyToString(response.getBody(), UTF_8)).isEqualTo("Hello World");
 	}
 
 

spring-web/src/test/java/org/springframework/web/client/ExtractingResponseErrorHandlerTests.java

@@ -19,6 +19,8 @@ package org.springframework.web.client;
 import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -36,8 +38,11 @@ import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
 /**
+ * Unit tests for {@link ExtractingResponseErrorHandler}.
+ *
  * @author Arjen Poutsma
  */
+@SuppressWarnings("ALL")
 class ExtractingResponseErrorHandlerTests {
 
 	private ExtractingResponseErrorHandler errorHandler;
@@ -48,13 +53,10 @@ class ExtractingResponseErrorHandlerTests {
 	@BeforeEach
 	void setup() {
 		HttpMessageConverter<Object> converter = new MappingJackson2HttpMessageConverter();
-		this.errorHandler = new ExtractingResponseErrorHandler(
-				Collections.singletonList(converter));
+		this.errorHandler = new ExtractingResponseErrorHandler(List.of(converter));
 
-		this.errorHandler.setStatusMapping(
-				Collections.singletonMap(HttpStatus.I_AM_A_TEAPOT, MyRestClientException.class));
-		this.errorHandler.setSeriesMapping(Collections
-				.singletonMap(HttpStatus.Series.SERVER_ERROR, MyRestClientException.class));
+		this.errorHandler.setStatusMapping(Map.of(HttpStatus.I_AM_A_TEAPOT, MyRestClientException.class));
+		this.errorHandler.setSeriesMapping(Map.of(HttpStatus.Series.SERVER_ERROR, MyRestClientException.class));
 	}
 
 
@@ -72,8 +74,7 @@ class ExtractingResponseErrorHandlerTests {
 
 	@Test
 	void hasErrorOverride() throws Exception {
-		this.errorHandler.setSeriesMapping(Collections
-				.singletonMap(HttpStatus.Series.CLIENT_ERROR, null));
+		this.errorHandler.setSeriesMapping(Collections.singletonMap(HttpStatus.Series.CLIENT_ERROR, null));
 
 		given(this.response.getStatusCode()).willReturn(HttpStatus.I_AM_A_TEAPOT);
 		assertThat(this.errorHandler.hasError(this.response)).isTrue();
@@ -96,9 +97,9 @@ class ExtractingResponseErrorHandlerTests {
 		responseHeaders.setContentLength(body.length);
 		given(this.response.getBody()).willReturn(new ByteArrayInputStream(body));
 
-		assertThatExceptionOfType(MyRestClientException.class).isThrownBy(() ->
-				this.errorHandler.handleError(this.response))
-			.satisfies(ex -> assertThat(ex.getFoo()).isEqualTo("bar"));
+		assertThatExceptionOfType(MyRestClientException.class)
+				.isThrownBy(() -> this.errorHandler.handleError(this.response))
+				.satisfies(ex -> assertThat(ex.getFoo()).isEqualTo("bar"));
 	}
 
 	@Test
@@ -112,9 +113,9 @@ class ExtractingResponseErrorHandlerTests {
 		responseHeaders.setContentLength(body.length);
 		given(this.response.getBody()).willReturn(new ByteArrayInputStream(body));
 
-		assertThatExceptionOfType(MyRestClientException.class).isThrownBy(() ->
-				this.errorHandler.handleError(this.response))
-			.satisfies(ex -> assertThat(ex.getFoo()).isEqualTo("bar"));
+		assertThatExceptionOfType(MyRestClientException.class)
+				.isThrownBy(() -> this.errorHandler.handleError(this.response))
+				.satisfies(ex -> assertThat(ex.getFoo()).isEqualTo("bar"));
 	}
 
 	@Test
@@ -128,18 +129,17 @@ class ExtractingResponseErrorHandlerTests {
 		responseHeaders.setContentLength(body.length);
 		given(this.response.getBody()).willReturn(new ByteArrayInputStream(body));
 
-		assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
-				this.errorHandler.handleError(this.response))
-			.satisfies(ex -> {
-				assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
-				assertThat(ex.getResponseBodyAsByteArray()).isEqualTo(body);
-			});
+		assertThatExceptionOfType(HttpClientErrorException.class)
+				.isThrownBy(() -> this.errorHandler.handleError(this.response))
+				.satisfies(ex -> {
+					assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
+					assertThat(ex.getResponseBodyAsByteArray()).isEqualTo(body);
+				});
 	}
 
 	@Test
 	void handleNoMatchOverride() throws Exception {
-		this.errorHandler.setSeriesMapping(Collections
-				.singletonMap(HttpStatus.Series.CLIENT_ERROR, null));
+		this.errorHandler.setSeriesMapping(Collections.singletonMap(HttpStatus.Series.CLIENT_ERROR, null));
 
 		given(this.response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
 		HttpHeaders responseHeaders = new HttpHeaders();