Merge pull request #19498 from michalszynkiewicz/rest-client-reactive…

…-exception-mapper-once

Rest Client Reactive: don't retrigger client response filters when they fail

extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/error/ResponseExceptionMapperTest.java

@@ -0,0 +1,70 @@
+package io.quarkus.rest.client.reactive.error;
+
+import static io.quarkus.rest.client.reactive.RestClientTestUtil.setUrlForClass;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.Provider;
+
+import org.eclipse.microprofile.rest.client.ext.ResponseExceptionMapper;
+import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
+import org.eclipse.microprofile.rest.client.inject.RestClient;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+public class ResponseExceptionMapperTest {
+    @RegisterExtension
+    static final QuarkusUnitTest TEST = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(Client.class, Resource.class, MyExceptionMapper.class).addAsResource(
+                            new StringAsset(setUrlForClass(Client.class)),
+                            "application.properties"));
+    public static final String ERROR_MESSAGE = "The entity was not found";
+
+    @RestClient
+    Client client;
+
+    @Test
+    void shouldInvokeExceptionMapperOnce() {
+        assertThrows(RuntimeException.class, client::get);
+        assertThat(MyExceptionMapper.executionCount.get()).isEqualTo(1);
+    }
+
+    @Path("/error")
+    public static class Resource {
+        @GET
+        public Response returnError() {
+            return Response.status(404).entity(ERROR_MESSAGE).build();
+        }
+    }
+
+    @Path("/error")
+    @RegisterRestClient
+    public interface Client {
+        @GET
+        String get();
+    }
+
+    @Provider
+    public static class MyExceptionMapper implements ResponseExceptionMapper<Exception> {
+
+        private static final AtomicInteger executionCount = new AtomicInteger();
+
+        @Override
+        public Exception toThrowable(Response response) {
+            executionCount.incrementAndGet();
+            return new RuntimeException("My exception");
+        }
+    }
+
+}

independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/PreResponseFilterHandler.java

@@ -0,0 +1,15 @@
+package org.jboss.resteasy.reactive.client.handlers;
+
+import org.jboss.resteasy.reactive.client.impl.RestClientRequestContext;
+import org.jboss.resteasy.reactive.client.spi.ClientRestHandler;
+
+/**
+ * This Handler is invoked before ClientResponseFilters handler. It changes the abort handler chain
+ * to a one without the ClientResponseFilterRestHandler's so that the filters are not retriggered in case of failure
+ */
+public class PreResponseFilterHandler implements ClientRestHandler {
+    @Override
+    public void handle(RestClientRequestContext requestContext) throws Exception {
+        requestContext.setAbortHandlerChain(requestContext.getAbortHandlerChainWithoutResponseFilters());
+    }
+}

independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/AsyncInvokerImpl.java

@@ -256,7 +256,8 @@ RestClientRequestContext performRequestInternal(String httpMethodName, Entity<?>
         RestClientRequestContext restClientRequestContext = new RestClientRequestContext(restClient, httpClient, httpMethodName,
                 uri, requestSpec.configuration, requestSpec.headers,
                 entity, responseType, registerBodyHandler, properties, handlerChain.createHandlerChain(configuration),
-                handlerChain.createAbortHandlerChain(configuration), requestContext);
+                handlerChain.createAbortHandlerChain(configuration),
+                handlerChain.createAbortHandlerChainWithoutResponseFilters(), requestContext);
         restClientRequestContext.run();
         return restClientRequestContext;
     }

independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/HandlerChain.java

@@ -10,6 +10,7 @@
 import org.jboss.resteasy.reactive.client.handlers.ClientResponseFilterRestHandler;
 import org.jboss.resteasy.reactive.client.handlers.ClientSendRequestHandler;
 import org.jboss.resteasy.reactive.client.handlers.ClientSetResponseEntityRestHandler;
+import org.jboss.resteasy.reactive.client.handlers.PreResponseFilterHandler;
 import org.jboss.resteasy.reactive.client.spi.ClientRestHandler;
 import org.jboss.resteasy.reactive.common.jaxrs.ConfigurationImpl;
 
@@ -53,6 +54,7 @@ ClientRestHandler[] createHandlerChain(ConfigurationImpl configuration) {
         }
         result.add(clientSendHandler);
         result.add(clientSetResponseEntityRestHandler);
+        result.add(new PreResponseFilterHandler());
         for (int i = 0; i < responseFilters.size(); i++) {
             result.add(new ClientResponseFilterRestHandler(responseFilters.get(i)));
         }
@@ -63,7 +65,7 @@ ClientRestHandler[] createHandlerChain(ConfigurationImpl configuration) {
     ClientRestHandler[] createAbortHandlerChain(ConfigurationImpl configuration) {
         List<ClientResponseFilter> responseFilters = configuration.getResponseFilters();
         if (responseFilters.isEmpty()) {
-            return new ClientRestHandler[] { clientErrorHandler };
+            return createAbortHandlerChainWithoutResponseFilters();
         }
         List<ClientRestHandler> result = new ArrayList<>(1 + responseFilters.size());
         for (int i = 0; i < responseFilters.size(); i++) {
@@ -72,4 +74,8 @@ ClientRestHandler[] createAbortHandlerChain(ConfigurationImpl configuration) {
         result.add(clientErrorHandler);
         return result.toArray(EMPTY_REST_HANDLERS);
     }
+
+    ClientRestHandler[] createAbortHandlerChainWithoutResponseFilters() {
+        return new ClientRestHandler[] { clientErrorHandler };
+    }
 }

independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java

@@ -57,6 +57,7 @@ public class RestClientRequestContext extends AbstractResteasyReactiveContext<Re
     // see Javadoc of javax.ws.rs.client.Invocation or javax.ws.rs.client.SyncInvoker
     private final boolean checkSuccessfulFamily;
     private final CompletableFuture<ResponseImpl> result;
+    private final ClientRestHandler[] abortHandlerChainWithoutResponseFilters;
     /**
      * Only initialised if we have request or response filters
      */
@@ -82,6 +83,7 @@ public RestClientRequestContext(ClientImpl restClient,
             Entity<?> entity, GenericType<?> responseType, boolean registerBodyHandler, Map<String, Object> properties,
             ClientRestHandler[] handlerChain,
             ClientRestHandler[] abortHandlerChain,
+            ClientRestHandler[] abortHandlerChainWithoutResponseFilters,
             ThreadSetupAction requestContext) {
         super(handlerChain, abortHandlerChain, requestContext);
         this.restClient = restClient;
@@ -91,6 +93,7 @@ public RestClientRequestContext(ClientImpl restClient,
         this.requestHeaders = requestHeaders;
         this.configuration = configuration;
         this.entity = entity;
+        this.abortHandlerChainWithoutResponseFilters = abortHandlerChainWithoutResponseFilters;
         if (responseType == null) {
             this.responseType = new GenericType<>(String.class);
             this.checkSuccessfulFamily = false;
@@ -108,6 +111,7 @@ public RestClientRequestContext(ClientImpl restClient,
     }
 
     public void abort() {
+        setAbortHandlerChainStarted(true);
         restart(abortHandlerChain);
     }
 
@@ -378,4 +382,8 @@ public boolean isMultipart() {
     public Map<String, Object> getClientFilterProperties() {
         return properties;
     }
+
+    public ClientRestHandler[] getAbortHandlerChainWithoutResponseFilters() {
+        return abortHandlerChainWithoutResponseFilters;
+    }
 }

independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/core/AbstractResteasyReactiveContext.java

@@ -32,6 +32,7 @@ public abstract class AbstractResteasyReactiveContext<T extends AbstractResteasy
     private ThreadSetupAction.ThreadState currentRequestScope;
     private List<CompletionCallback> completionCallbacks;
     private List<ConnectionCallback> connectionCallbacks;
+    private boolean abortHandlerChainStarted;
 
     private boolean closed = false;
 
@@ -158,7 +159,7 @@ public void run() {
                         }
                     }
                 } catch (Throwable t) {
-                    aborted = handlers == abortHandlerChain;
+                    aborted = abortHandlerChainStarted;
                     if (t instanceof PreserveTargetException) {
                         handleException(t.getCause(), true);
                     } else {
@@ -303,19 +304,21 @@ public H[] getHandlers() {
      * a response result and switch to the abort chain
      */
     public void handleException(Throwable t) {
-        if (handlers == abortHandlerChain) {
+        if (abortHandlerChainStarted) {
             handleUnrecoverableError(unwrapException(t));
         } else {
             this.throwable = unwrapException(t);
+            abortHandlerChainStarted = true;
             restart(abortHandlerChain);
         }
     }
 
     public void handleException(Throwable t, boolean keepSameTarget) {
-        if (handlers == abortHandlerChain) {
+        if (abortHandlerChainStarted) {
             handleUnrecoverableError(unwrapException(t));
         } else {
             this.throwable = unwrapException(t);
+            abortHandlerChainStarted = true;
             restart(abortHandlerChain, keepSameTarget);
         }
     }
@@ -383,4 +386,8 @@ public synchronized void registerConnectionCallback(ConnectionCallback callback)
             connectionCallbacks = new ArrayList<>();
         connectionCallbacks.add(callback);
     }
+
+    public void setAbortHandlerChainStarted(boolean value) {
+        abortHandlerChainStarted = value;
+    }
 }

independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java

@@ -183,11 +183,12 @@ public void restart(RuntimeResource target, boolean setLocatorTarget) {
     }
 
     /**
-     * Meant to be used when a error occurred early in processing chain
+     * Meant to be used when an error occurred early in processing chain
      */
     @Override
     public void abortWith(Response response) {
         setResult(response);
+        setAbortHandlerChainStarted(true);
         restart(getAbortHandlerChain());
         // this is a valid action after suspend, in which case we must resume
         if (isSuspended()) {

independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/ContainerRequestContextImpl.java

@@ -181,6 +181,7 @@ public ContainerRequestContextImpl setPreMatch(boolean preMatch) {
     public void abortWith(Response response) {
         assertNotResponse();
         quarkusRestContext.setResult(response);
+        quarkusRestContext.setAbortHandlerChainStarted(true);
         quarkusRestContext.restart(quarkusRestContext.getAbortHandlerChain(), true);
         aborted = true;
         // this is a valid action after suspend, in which case we must resume