From 21b2fc1f0129420a8da521cd1e7f33f19beffbc1 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 22 Nov 2019 10:27:38 +0000 Subject: [PATCH] Improve HttpHandlerConnection completion Before this commit the connector waited for a completed response (via ServerHttpResponse#setComplete or ServerHttpResponse#writeWith) or an error signal in handling, but it didn't deal explicitly with the case where both can occur. This commit explicitly waits for the completion of handling (success or error) before passing the response downstream. If an error occurs after response completion, it is wrapped in a dedicated exception that also provides access to the completed response. Close gh-24051 --- .../reactive/MockClientHttpResponse.java | 6 +++ .../reactive/server/HttpHandlerConnector.java | 47 +++++++++++++++++-- .../reactive/test/MockClientHttpResponse.java | 8 +++- 3 files changed, 55 insertions(+), 6 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 9385899bff..739d8ba935 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 @@ -148,4 +148,10 @@ public class MockClientHttpResponse implements ClientHttpResponse { return (charset != null ? charset : StandardCharsets.UTF_8); } + + @Override + public String toString() { + HttpStatus code = HttpStatus.resolve(this.status); + return (code != null ? code.name() + "(" + this.status + ")" : "Status (" + this.status + ")") + this.headers; + } } diff --git a/spring-test/src/main/java/org/springframework/test/web/reactive/server/HttpHandlerConnector.java b/spring-test/src/main/java/org/springframework/test/web/reactive/server/HttpHandlerConnector.java index 792e3c5307..c5841c41ec 100644 --- a/spring-test/src/main/java/org/springframework/test/web/reactive/server/HttpHandlerConnector.java +++ b/spring-test/src/main/java/org/springframework/test/web/reactive/server/HttpHandlerConnector.java @@ -83,7 +83,9 @@ public class HttpHandlerConnector implements ClientHttpConnector { private Mono doConnect( HttpMethod httpMethod, URI uri, Function> requestCallback) { - MonoProcessor result = MonoProcessor.create(); + MonoProcessor requestWriteCompletion = MonoProcessor.create(); + MonoProcessor handlerCompletion = MonoProcessor.create(); + ClientHttpResponse[] savedResponse = new ClientHttpResponse[1]; MockClientHttpRequest mockClientRequest = new MockClientHttpRequest(httpMethod, uri); MockServerHttpResponse mockServerResponse = new MockServerHttpResponse(); @@ -92,20 +94,26 @@ public class HttpHandlerConnector implements ClientHttpConnector { log("Invoking HttpHandler for ", httpMethod, uri); ServerHttpRequest mockServerRequest = adaptRequest(mockClientRequest, requestBody); ServerHttpResponse responseToUse = prepareResponse(mockServerResponse, mockServerRequest); - this.handler.handle(mockServerRequest, responseToUse).subscribe(aVoid -> {}, result::onError); + this.handler.handle(mockServerRequest, responseToUse).subscribe(handlerCompletion); return Mono.empty(); }); mockServerResponse.setWriteHandler(responseBody -> Mono.fromRunnable(() -> { log("Creating client response for ", httpMethod, uri); - result.onNext(adaptResponse(mockServerResponse, responseBody)); + savedResponse[0] = adaptResponse(mockServerResponse, responseBody); })); log("Writing client request for ", httpMethod, uri); - requestCallback.apply(mockClientRequest).subscribe(aVoid -> {}, result::onError); + requestCallback.apply(mockClientRequest).subscribe(requestWriteCompletion); - return result; + return Mono.when(requestWriteCompletion, handlerCompletion) + .onErrorMap(ex -> { + ClientHttpResponse response = savedResponse[0]; + return response != null ? new FailureAfterResponseCompletedException(response, ex) : ex; + }) + .then(Mono.fromCallable(() -> savedResponse[0] != null ? + savedResponse[0] : adaptResponse(mockServerResponse, Flux.empty()))); } private void log(String message, HttpMethod httpMethod, URI uri) { @@ -135,4 +143,33 @@ public class HttpHandlerConnector implements ClientHttpConnector { return clientResponse; } + + /** + * Indicates that an error occurred after the server response was completed, + * via {@link ServerHttpResponse#writeWith} or {@link ServerHttpResponse#setComplete()}, + * and can no longer be changed. This exception wraps the error and also + * provides {@link #getCompletedResponse() access} to the response. + *

What happens on an actual running server depends on when the server + * commits the response and the error may or may not change the response. + * Therefore in tests without a server the exception is wrapped and allowed + * to propagate so the application is alerted. + * @since 5.2.2 + */ + @SuppressWarnings("serial") + public static final class FailureAfterResponseCompletedException extends RuntimeException { + + private final ClientHttpResponse completedResponse; + + + private FailureAfterResponseCompletedException(ClientHttpResponse response, Throwable cause) { + super("Error occurred after response was completed: " + response, cause); + this.completedResponse = response; + } + + + public ClientHttpResponse getCompletedResponse() { + return this.completedResponse; + } + } + } 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 f7b772a74c..e757bfad7c 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 @@ -64,7 +64,7 @@ public class MockClientHttpResponse implements ClientHttpResponse { } public MockClientHttpResponse(int status) { - Assert.isTrue(status >= 100 && status < 600, "Status must be between 1xx and 5xx"); + Assert.isTrue(status > 99 && status < 1000, "Status must be between 100 and 999"); this.status = status; } @@ -148,4 +148,10 @@ public class MockClientHttpResponse implements ClientHttpResponse { return (charset != null ? charset : StandardCharsets.UTF_8); } + + @Override + public String toString() { + HttpStatus code = HttpStatus.resolve(this.status); + return (code != null ? code.name() + "(" + this.status + ")" : "Status (" + this.status + ")") + this.headers; + } }