From bff76d756b2d23a4f1903ad8274e8257fc76c58f Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 23 Oct 2024 18:58:48 +0100 Subject: [PATCH] Refactor implementation of retrieve in RestClient Closes gh-33777 --- .../web/client/DefaultRestClient.java | 149 ++++++++---------- .../web/client/RestClient.java | 13 +- 2 files changed, 73 insertions(+), 89 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java b/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java index 2273e74e61c..0572b986c9d 100644 --- a/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java +++ b/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java @@ -206,8 +206,8 @@ final class DefaultRestClient implements RestClient { @Nullable @SuppressWarnings({"rawtypes", "unchecked"}) - private T readWithMessageConverters(ClientHttpResponse clientResponse, Runnable callback, Type bodyType, - Class bodyClass, @Nullable Observation observation) { + private T readWithMessageConverters( + ClientHttpResponse clientResponse, Runnable callback, Type bodyType, Class bodyClass) { MediaType contentType = getContentType(clientResponse); @@ -257,21 +257,8 @@ final class DefaultRestClient implements RestClient { else { cause = exc; } - RestClientException restClientException = new RestClientException("Error while extracting response for type [" + + throw new RestClientException("Error while extracting response for type [" + ResolvableType.forType(bodyType) + "] and content type [" + contentType + "]", cause); - if (observation != null) { - observation.error(restClientException); - } - throw restClientException; - } - catch (RestClientException restClientException) { - if (observation != null) { - observation.error(restClientException); - } - throw restClientException; - } - finally { - clientResponse.close(); } } @@ -536,14 +523,16 @@ final class DefaultRestClient implements RestClient { @Override public ResponseSpec retrieve() { - return exchangeInternal(DefaultResponseSpec::new, false); + return new DefaultResponseSpec(this); } @Override + @Nullable public T exchange(ExchangeFunction exchangeFunction, boolean close) { return exchangeInternal(exchangeFunction, close); } + @Nullable private T exchangeInternal(ExchangeFunction exchangeFunction, boolean close) { Assert.notNull(exchangeFunction, "ExchangeFunction must not be null"); @@ -578,39 +567,31 @@ final class DefaultRestClient implements RestClient { } clientResponse = clientRequest.execute(); observationContext.setResponse(clientResponse); - ConvertibleClientHttpResponse convertibleWrapper = new DefaultConvertibleClientHttpResponse(clientResponse, observation, observationScope); + ConvertibleClientHttpResponse convertibleWrapper = new DefaultConvertibleClientHttpResponse(clientResponse); return exchangeFunction.exchange(clientRequest, convertibleWrapper); } catch (IOException ex) { ResourceAccessException resourceAccessException = createResourceAccessException(uri, this.httpMethod, ex); - if (observationScope != null) { - observationScope.close(); - } if (observation != null) { observation.error(resourceAccessException); - observation.stop(); } throw resourceAccessException; } catch (Throwable error) { - if (observationScope != null) { - observationScope.close(); - } if (observation != null) { observation.error(error); - observation.stop(); } throw error; } finally { + if (observationScope != null) { + observationScope.close(); + } + if (observation != null) { + observation.stop(); + } if (close && clientResponse != null) { clientResponse.close(); - if (observationScope != null) { - observationScope.close(); - } - if (observation != null) { - observation.stop(); - } } } } @@ -719,17 +700,14 @@ final class DefaultRestClient implements RestClient { private class DefaultResponseSpec implements ResponseSpec { - private final HttpRequest clientRequest; - - private final ClientHttpResponse clientResponse; + private final RequestHeadersSpec requestHeadersSpec; private final List statusHandlers = new ArrayList<>(1); private final int defaultStatusHandlerCount; - DefaultResponseSpec(HttpRequest clientRequest, ClientHttpResponse clientResponse) { - this.clientRequest = clientRequest; - this.clientResponse = clientResponse; + DefaultResponseSpec(RequestHeadersSpec requestHeadersSpec) { + this.requestHeadersSpec = requestHeadersSpec; this.statusHandlers.addAll(DefaultRestClient.this.defaultStatusHandlers); this.statusHandlers.add(StatusHandler.defaultHandler(DefaultRestClient.this.messageConverters)); this.defaultStatusHandlerCount = this.statusHandlers.size(); @@ -761,7 +739,7 @@ final class DefaultRestClient implements RestClient { @Override @Nullable public T body(Class bodyType) { - return readBody(bodyType, bodyType); + return executeAndExtract((request, response) -> readBody(request, response, bodyType, bodyType)); } @Override @@ -769,7 +747,7 @@ final class DefaultRestClient implements RestClient { public T body(ParameterizedTypeReference bodyType) { Type type = bodyType.getType(); Class bodyClass = bodyClass(type); - return readBody(type, bodyClass); + return executeAndExtract((request, response) -> readBody(request, response, type, bodyClass)); } @Override @@ -785,50 +763,64 @@ final class DefaultRestClient implements RestClient { } private ResponseEntity toEntityInternal(Type bodyType, Class bodyClass) { - T body = readBody(bodyType, bodyClass); - try { - return ResponseEntity.status(this.clientResponse.getStatusCode()) - .headers(this.clientResponse.getHeaders()) - .body(body); - } - catch (IOException ex) { - throw new ResourceAccessException("Could not retrieve response status code: " + ex.getMessage(), ex); - } + ResponseEntity entity = executeAndExtract((request, response) -> { + T body = readBody(request, response, bodyType, bodyClass); + try { + return ResponseEntity.status(response.getStatusCode()) + .headers(response.getHeaders()) + .body(body); + } + catch (IOException ex) { + throw new ResourceAccessException( + "Could not retrieve response status code: " + ex.getMessage(), ex); + } + }); + Assert.state(entity != null, "No ResponseEntity"); + return entity; } @Override public ResponseEntity toBodilessEntity() { - try (this.clientResponse) { - applyStatusHandlers(); - return ResponseEntity.status(this.clientResponse.getStatusCode()) - .headers(this.clientResponse.getHeaders()) - .build(); - } - catch (UncheckedIOException ex) { - throw new ResourceAccessException("Could not retrieve response status code: " + ex.getMessage(), ex.getCause()); - } - catch (IOException ex) { - throw new ResourceAccessException("Could not retrieve response status code: " + ex.getMessage(), ex); - } + ResponseEntity entity = executeAndExtract((request, response) -> { + try (response) { + applyStatusHandlers(request, response); + return ResponseEntity.status(response.getStatusCode()) + .headers(response.getHeaders()) + .build(); + } + catch (UncheckedIOException ex) { + throw new ResourceAccessException( + "Could not retrieve response status code: " + ex.getMessage(), ex.getCause()); + } + catch (IOException ex) { + throw new ResourceAccessException( + "Could not retrieve response status code: " + ex.getMessage(), ex); + } + }); + Assert.state(entity != null, "No ResponseEntity"); + return entity; } + @Nullable + public T executeAndExtract(RequestHeadersSpec.ExchangeFunction exchangeFunction) { + return this.requestHeadersSpec.exchange(exchangeFunction); + } @Nullable - private T readBody(Type bodyType, Class bodyClass) { - return DefaultRestClient.this.readWithMessageConverters(this.clientResponse, this::applyStatusHandlers, - bodyType, bodyClass, getCurrentObservation()); + private T readBody(HttpRequest request, ClientHttpResponse response, Type bodyType, Class bodyClass) { + return DefaultRestClient.this.readWithMessageConverters( + response, () -> applyStatusHandlers(request, response), bodyType, bodyClass); } - private void applyStatusHandlers() { + private void applyStatusHandlers(HttpRequest request, ClientHttpResponse response) { try { - ClientHttpResponse response = this.clientResponse; if (response instanceof DefaultConvertibleClientHttpResponse convertibleResponse) { response = convertibleResponse.delegate; } for (StatusHandler handler : this.statusHandlers) { if (handler.test(response)) { - handler.handle(this.clientRequest, response); + handler.handle(request, response); return; } } @@ -838,14 +830,6 @@ final class DefaultRestClient implements RestClient { } } - @Nullable - private Observation getCurrentObservation() { - if (this.clientResponse instanceof DefaultConvertibleClientHttpResponse convertibleResponse) { - return convertibleResponse.observation; - } - return null; - } - } @@ -853,21 +837,14 @@ final class DefaultRestClient implements RestClient { private final ClientHttpResponse delegate; - private final Observation observation; - - private final Observation.Scope observationScope; - - public DefaultConvertibleClientHttpResponse(ClientHttpResponse delegate, Observation observation, Observation.Scope observationScope) { + public DefaultConvertibleClientHttpResponse(ClientHttpResponse delegate) { this.delegate = delegate; - this.observation = observation; - this.observationScope = observationScope; } - @Nullable @Override public T bodyTo(Class bodyType) { - return readWithMessageConverters(this.delegate, () -> {} , bodyType, bodyType, this.observation); + return readWithMessageConverters(this.delegate, () -> {} , bodyType, bodyType); } @Nullable @@ -875,7 +852,7 @@ final class DefaultRestClient implements RestClient { public T bodyTo(ParameterizedTypeReference bodyType) { Type type = bodyType.getType(); Class bodyClass = bodyClass(type); - return readWithMessageConverters(this.delegate, () -> {}, type, bodyClass, this.observation); + return readWithMessageConverters(this.delegate, () -> {}, type, bodyClass); } @Override @@ -901,8 +878,6 @@ final class DefaultRestClient implements RestClient { @Override public void close() { this.delegate.close(); - this.observationScope.close(); - this.observation.stop(); } } diff --git a/spring-web/src/main/java/org/springframework/web/client/RestClient.java b/spring-web/src/main/java/org/springframework/web/client/RestClient.java index 7de2e2eab35..f25e3f0c2e4 100644 --- a/spring-web/src/main/java/org/springframework/web/client/RestClient.java +++ b/spring-web/src/main/java/org/springframework/web/client/RestClient.java @@ -615,8 +615,10 @@ public interface RestClient { S httpRequest(Consumer requestConsumer); /** - * Proceed to declare how to extract the response. For example to extract - * a {@link ResponseEntity} with status, headers, and body: + * Enter the retrieve workflow and use the returned {@link ResponseSpec} + * to select from a number of built-in options to extract the response. + * For example: + * * 
 		 * ResponseEntity<Person> entity = client.get()
 		 *     .uri("/persons/1")
@@ -632,6 +634,10 @@ public interface RestClient {
 		 *     .retrieve()
 		 *     .body(Person.class);
 		 *
+ * Note that this method does not actually execute the request until you + * call one of the returned {@link ResponseSpec}. Use the + * {@link #exchange(ExchangeFunction)} variants if you need to separate + * request execution from response extraction. *

By default, 4xx response code result in a * {@link HttpClientErrorException} and 5xx response codes in a * {@link HttpServerErrorException}. To customize error handling, use @@ -664,6 +670,7 @@ public interface RestClient { * @param the type the response will be transformed to * @return the value returned from the exchange function */ + @Nullable default T exchange(ExchangeFunction exchangeFunction) { return exchange(exchangeFunction, true); } @@ -695,6 +702,7 @@ public interface RestClient { * @param the type the response will be transformed to * @return the value returned from the exchange function */ + @Nullable T exchange(ExchangeFunction exchangeFunction, boolean close); @@ -712,6 +720,7 @@ public interface RestClient { * @return the exchanged type * @throws IOException in case of I/O errors */ + @Nullable T exchange(HttpRequest clientRequest, ConvertibleClientHttpResponse clientResponse) throws IOException; }