From bf7540e2af6dee7ea593923fb94ba7c2e3acebf0 Mon Sep 17 00:00:00 2001 From: yawkat Date: Fri, 18 Nov 2022 16:43:07 +0100 Subject: [PATCH 1/2] Still emit response if return type is Void Before this patch, HttpClient would remove the actual response from the response publisher if the expected body type is Void. I don't see a reason to do this, and it leads to HTTP filters not seeing the response as they should. Fixes #8366 --- .../http/client/netty/DefaultHttpClient.java | 4 -- .../http/client/aop/ClientFilterSpec.groovy | 46 ++++++++++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java b/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java index 8bf5d2456f7..ddda4531902 100644 --- a/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java +++ b/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java @@ -1490,10 +1490,6 @@ private void sendRequestThroughChannel( new FullHttpResponseHandler<>(responsePromise, poolHandle, secure, finalRequest, bodyType, errorType)); poolHandle.notifyRequestPipelineBuilt(); Publisher> publisher = new NettyFuturePublisher<>(responsePromise, true); - if (bodyType != null && bodyType.isVoid()) { - // don't emit response if bodyType is void - publisher = Flux.from(publisher).filter(r -> false); - } publisher.subscribe(new ForwardingSubscriber<>(emitter)); requestWriter.write(channel, secure, emitter); diff --git a/http-client/src/test/groovy/io/micronaut/http/client/aop/ClientFilterSpec.groovy b/http-client/src/test/groovy/io/micronaut/http/client/aop/ClientFilterSpec.groovy index eb57296ac5a..d6efb6145ae 100644 --- a/http-client/src/test/groovy/io/micronaut/http/client/aop/ClientFilterSpec.groovy +++ b/http-client/src/test/groovy/io/micronaut/http/client/aop/ClientFilterSpec.groovy @@ -17,6 +17,7 @@ package io.micronaut.http.client.aop import io.micronaut.context.ApplicationContext import io.micronaut.context.annotation.Requires +import io.micronaut.core.async.annotation.SingleResult import io.micronaut.http.HttpResponse import io.micronaut.http.HttpVersion import io.micronaut.http.MediaType @@ -32,8 +33,9 @@ import io.micronaut.http.filter.ClientFilterChain import io.micronaut.http.filter.HttpClientFilter import io.micronaut.runtime.server.EmbeddedServer import org.reactivestreams.Publisher +import reactor.core.publisher.Flux +import reactor.core.publisher.Mono import spock.lang.AutoCleanup -import spock.lang.Shared import spock.lang.Specification /** @@ -267,4 +269,46 @@ class ClientFilterSpec extends Specification{ throw new RuntimeException("from filter") } } + + void "filter always observes a response"() { + given: + ObservesResponseClient client = context.getBean(ObservesResponseClient) + ObservesResponseFilter filter = context.getBean(ObservesResponseFilter) + + when: + Mono.from(client.monoVoid()).block() == null + then: + filter.observedResponse != null + } + + @Requires(property = 'spec.name', value = "ClientFilterSpec") + @Client('/observes-response') + static interface ObservesResponseClient { + + @Get + @SingleResult + Publisher monoVoid() + } + + @Requires(property = 'spec.name', value = "ClientFilterSpec") + @Filter("/observes-response/**") + static class ObservesResponseFilter implements HttpClientFilter { + HttpResponse observedResponse + + @Override + Publisher> doFilter(MutableHttpRequest request, ClientFilterChain chain) { + return Flux.from(chain.proceed(request)).doOnNext(r -> { + observedResponse = r + }) + } + } + + @Requires(property = 'spec.name', value = "ClientFilterSpec") + @Controller('/observes-response') + static class ObservesResponseController { + @Get + String index() { + return "" + } + } } From e781fb5a860b3879d0a010d4fb2ba17252de97e5 Mon Sep 17 00:00:00 2001 From: yawkat Date: Mon, 21 Nov 2022 13:08:39 +0100 Subject: [PATCH 2/2] fix Publisher HttpClientIntroductionAdvice --- .../main/java/io/micronaut/http/client/HttpClient.java | 8 +++++++- .../client/interceptor/HttpClientIntroductionAdvice.java | 4 ++-- .../io/micronaut/http/client/netty/DefaultHttpClient.java | 7 ++++++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/http-client-core/src/main/java/io/micronaut/http/client/HttpClient.java b/http-client-core/src/main/java/io/micronaut/http/client/HttpClient.java index b54aa145806..fdba6c1d9bb 100644 --- a/http-client-core/src/main/java/io/micronaut/http/client/HttpClient.java +++ b/http-client-core/src/main/java/io/micronaut/http/client/HttpClient.java @@ -152,9 +152,15 @@ default Publisher> exchange(@NonNull HttpRequest reque * @param The error type * @return A {@link Publisher} that emits a result of the given type */ + @SuppressWarnings("unchecked") default Publisher retrieve(@NonNull HttpRequest request, @NonNull Argument bodyType, @NonNull Argument errorType) { // note: this default impl isn't used by us anymore, it's overridden by DefaultHttpClient - return Flux.from(exchange(request, bodyType, errorType)).map(response -> { + Flux> exchange = Flux.from(exchange(request, bodyType, errorType)); + if (bodyType.getType() == void.class) { + // exchange() returns a HttpResponse, we can't map the Void body properly, so just drop it and complete + return (Publisher) exchange.ignoreElements(); + } + return exchange.map(response -> { if (bodyType.getType() == HttpStatus.class) { return (O) response.getStatus(); } else { diff --git a/http-client-core/src/main/java/io/micronaut/http/client/interceptor/HttpClientIntroductionAdvice.java b/http-client-core/src/main/java/io/micronaut/http/client/interceptor/HttpClientIntroductionAdvice.java index e13c3b90578..24ccc2f94c6 100644 --- a/http-client-core/src/main/java/io/micronaut/http/client/interceptor/HttpClientIntroductionAdvice.java +++ b/http-client-core/src/main/java/io/micronaut/http/client/interceptor/HttpClientIntroductionAdvice.java @@ -55,8 +55,8 @@ import io.micronaut.http.annotation.Produces; import io.micronaut.http.client.BlockingHttpClient; import io.micronaut.http.client.HttpClient; -import io.micronaut.http.client.ReactiveClientResultTransformer; import io.micronaut.http.client.HttpClientRegistry; +import io.micronaut.http.client.ReactiveClientResultTransformer; import io.micronaut.http.client.StreamingHttpClient; import io.micronaut.http.client.annotation.Client; import io.micronaut.http.client.bind.ClientArgumentRequestBinder; @@ -426,7 +426,7 @@ private Publisher httpClientResponsePublisher(HttpClient httpClient, MutableHttp Class argumentType = reactiveValueArgument.getType(); if (Void.class == argumentType || returnType.isVoid()) { request.getHeaders().remove(HttpHeaders.ACCEPT); - return httpClient.exchange(request, Argument.VOID, errorType); + return httpClient.retrieve(request, Argument.VOID, errorType); } else { if (HttpResponse.class.isAssignableFrom(argumentType)) { return httpClient.exchange(request, reactiveValueArgument, errorType); diff --git a/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java b/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java index ddda4531902..968bdb773f0 100644 --- a/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java +++ b/http-client/src/main/java/io/micronaut/http/client/netty/DefaultHttpClient.java @@ -755,7 +755,12 @@ public Publisher> exchange(@NonNull @Override public Publisher retrieve(io.micronaut.http.HttpRequest request, Argument bodyType, Argument errorType) { // mostly same as default impl, but with exception customization - return Flux.from(exchange(request, bodyType, errorType)).map(response -> { + Flux> exchange = Flux.from(exchange(request, bodyType, errorType)); + if (bodyType.getType() == void.class) { + // exchange() returns a HttpResponse, we can't map the Void body properly, so just drop it and complete + return (Publisher) exchange.ignoreElements(); + } + return exchange.map(response -> { if (bodyType.getType() == HttpStatus.class) { return (O) response.getStatus(); } else {