You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by re...@apache.org on 2020/05/01 17:54:56 UTC

[cxf] branch 3.2.x-fixes updated (7091d4d -> 563a7a3)

This is an automated email from the ASF dual-hosted git repository.

reta pushed a change to branch 3.2.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git.


    from 7091d4d  Recording .gitmergeinfo Changes
     new a5f5092  CXF-8271: The async JAX-RS client never completes the response in case of an exception during the interceptor chain processing. (#665)
     new 563a7a3  Recording .gitmergeinfo Changes

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .gitmergeinfo                                      |   1 +
 .../apache/cxf/jaxrs/client/AbstractClient.java    |   1 +
 .../client/spec/ClientResponseFilterTest.java      |  88 ++++++++++++++++-
 .../org/apache/cxf/systest/jaxrs/BookServer20.java |  53 +++++-----
 .../systest/jaxrs/JAXRS20ClientServerBookTest.java |  91 ++++++++++++++++++
 .../cxf/systest/jaxrs/JAXRSAsyncClientTest.java    | 107 ++++++++++++++++++++-
 6 files changed, 316 insertions(+), 25 deletions(-)


[cxf] 01/02: CXF-8271: The async JAX-RS client never completes the response in case of an exception during the interceptor chain processing. (#665)

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

reta pushed a commit to branch 3.2.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit a5f50926f0b20c62fc017a6bdd100db3e2acd4d1
Author: Andriy Redko <dr...@gmail.com>
AuthorDate: Thu Apr 30 21:58:15 2020 -0400

    CXF-8271: The async JAX-RS client never completes the response in case of an exception during the interceptor chain processing. (#665)
    
    (cherry picked from commit 568b9674b079050cbafde5ce6448734720b49ce9)
    (cherry picked from commit 61087a77d06dc551f81f47f68db2a59a502e629b)
    
    # Conflicts:
    #	systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java
    #	systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java
---
 .../apache/cxf/jaxrs/client/AbstractClient.java    |   1 +
 .../client/spec/ClientResponseFilterTest.java      |  88 ++++++++++++++++-
 .../org/apache/cxf/systest/jaxrs/BookServer20.java |  53 +++++-----
 .../systest/jaxrs/JAXRS20ClientServerBookTest.java |  91 ++++++++++++++++++
 .../cxf/systest/jaxrs/JAXRSAsyncClientTest.java    | 107 ++++++++++++++++++++-
 5 files changed, 315 insertions(+), 25 deletions(-)

diff --git a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
index cb74181..91391f4 100644
--- a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
+++ b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
@@ -962,6 +962,7 @@ public abstract class AbstractClient implements Client {
         List<Interceptor<? extends Message>> i3 = cfg.getConduitSelector().getEndpoint().getInInterceptors();
         PhaseInterceptorChain chain = new PhaseChainCache().get(pm.getInPhases(), i1, i2, i3);
         chain.add(new ClientResponseFilterInterceptor());
+        chain.setFaultObserver(setupInFaultObserver(cfg));
         return chain;
     }
 
diff --git a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/spec/ClientResponseFilterTest.java b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/spec/ClientResponseFilterTest.java
index 9056212..70f5017 100644
--- a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/spec/ClientResponseFilterTest.java
+++ b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/spec/ClientResponseFilterTest.java
@@ -19,6 +19,8 @@
 package org.apache.cxf.jaxrs.client.spec;
 
 import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 
 import javax.annotation.Priority;
 import javax.ws.rs.DELETE;
@@ -42,6 +44,9 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
@@ -107,8 +112,7 @@ public class ClientResponseFilterTest {
              .target(ADDRESS)
              .request()
              .get()) {
-            assertEquals(200, response.getStatus());
-            assertEquals("hello rabbit", response.readEntity(String.class));
+            fail("Should raise ResponseProcessingException");
         }
     }
     
@@ -165,4 +169,84 @@ public class ClientResponseFilterTest {
             fail("Should be handled by ResponseProcessingException block");
         }
     }
+    
+    @Test
+    public void testAsyncClientResponseFilter() throws Exception {
+        try (Response response = ClientBuilder.newClient()
+             .register(AddHeaderClientResponseFilter.class)
+             .target(ADDRESS)
+             .request()
+             .async()
+             .get()
+             .get(10, TimeUnit.SECONDS)) {
+            assertEquals(200, response.getStatus());
+            assertEquals("true", response.getHeaderString("X-Done"));
+        }
+    }
+    
+    @Test
+    public void testExceptionWhenMultipleAsyncClientResponseFilters() {
+        try (Response response = ClientBuilder.newClient()
+             .register(AddHeaderClientResponseFilter.class)
+             .register(FaultyClientResponseFilter.class)
+             .target(ADDRESS)
+             .request()
+             .async()
+             .put(null)
+             .get(10, TimeUnit.SECONDS)) {
+            fail("Should not be invoked");
+        } catch (ExecutionException ex) {
+            assertThat(ex.getCause(), is(instanceOf(ResponseProcessingException.class)));
+        } catch (Throwable ex) {
+            fail("Should be handled by ResponseProcessingException block");
+        }
+    }
+    
+    @Test
+    public void testExceptionInAsyncClientResponseFilter() throws Exception {
+        try (Response response = ClientBuilder.newClient()
+             .register(FaultyClientResponseFilter.class)
+             .target(ADDRESS)
+             .request()
+             .async()
+             .get()
+             .get(10, TimeUnit.SECONDS)) {
+            fail("Should raise ResponseProcessingException");
+        } catch (ExecutionException ex) {
+            assertThat(ex.getCause(), is(instanceOf(ResponseProcessingException.class)));
+        } catch (Throwable ex) {
+            fail("Should be handled by ResponseProcessingException block");
+        }
+    }
+    
+    @Test
+    public void testExceptionInAsyncClientResponseFilterWhenNotFound() throws Exception {
+        try (Response response = ClientBuilder.newClient()
+             .register(FaultyClientResponseFilter.class)
+             .target(ADDRESS)
+             .request()
+             .async()
+             .put(null)
+             .get(10, TimeUnit.SECONDS)) {
+            fail("Should not be invoked");
+        } catch (ExecutionException ex) {
+            assertThat(ex.getCause(), is(instanceOf(ResponseProcessingException.class)));
+        } catch (Throwable ex) {
+            fail("Should be handled by ResponseProcessingException block");
+        }
+    }
+    
+    @Test
+    public void testAsyncClientResponseFilterWhenNotFound() throws Exception {
+        try (Response response = ClientBuilder.newClient()
+             .register(AddHeaderClientResponseFilter.class)
+             .target(ADDRESS)
+             .request()
+             .async()
+             .put(null)
+             .get(10, TimeUnit.SECONDS)) {
+            assertEquals(404, response.getStatus());
+            assertEquals("true", response.getHeaderString("X-Done"));
+        }
+    }
 }
\ No newline at end of file
diff --git a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookServer20.java b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookServer20.java
index 02b61a6..b8d6fc6 100644
--- a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookServer20.java
+++ b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookServer20.java
@@ -293,31 +293,34 @@ public class BookServer20 extends AbstractBusTestServerBase {
         @Override
         public void filter(ContainerRequestContext requestContext,
                            ContainerResponseContext responseContext) throws IOException {
-            String ct = responseContext.getMediaType().toString();
-            if (requestContext.getProperty("filterexception") != null) {
-                if (!"text/plain".equals(ct)) {
-                    throw new RuntimeException();
+            if (responseContext.getMediaType() != null) {
+                String ct = responseContext.getMediaType().toString();
+                if (requestContext.getProperty("filterexception") != null) {
+                    if (!"text/plain".equals(ct)) {
+                        throw new RuntimeException();
+                    }
+                    responseContext.getHeaders().putSingle("FilterException",
+                                                           requestContext.getProperty("filterexception"));
                 }
-                responseContext.getHeaders().putSingle("FilterException",
-                                                       requestContext.getProperty("filterexception"));
-            }
-            Object entity = responseContext.getEntity();
-            Type entityType = responseContext.getEntityType();
-            if (entity instanceof GenericHandler && InjectionUtils.getActualType(entityType) == Book.class) {
-                ct += ";charset=ISO-8859-1";
-                if ("getGenericBook2".equals(rInfo.getResourceMethod().getName())) {
-                    Annotation[] anns = responseContext.getEntityAnnotations();
-                    if (anns.length == 4 && anns[3].annotationType() == Context.class) {
-                        responseContext.getHeaders().addFirst("Annotations", "OK");
+            
+                Object entity = responseContext.getEntity();
+                Type entityType = responseContext.getEntityType();
+                if (entity instanceof GenericHandler && InjectionUtils.getActualType(entityType) == Book.class) {
+                    ct += ";charset=ISO-8859-1";
+                    if ("getGenericBook2".equals(rInfo.getResourceMethod().getName())) {
+                        Annotation[] anns = responseContext.getEntityAnnotations();
+                        if (anns.length == 4 && anns[3].annotationType() == Context.class) {
+                            responseContext.getHeaders().addFirst("Annotations", "OK");
+                        }
+                    } else {
+                        responseContext.setEntity(new Book("book", 124L));
                     }
                 } else {
-                    responseContext.setEntity(new Book("book", 124L));
+                    ct += ";charset=";
                 }
-            } else {
-                ct += ";charset=";
+                responseContext.getHeaders().putSingle("Content-Type", ct);
+                responseContext.getHeaders().add("Response", "OK");
             }
-            responseContext.getHeaders().putSingle("Content-Type", ct);
-            responseContext.getHeaders().add("Response", "OK");
         }
 
     }
@@ -333,8 +336,14 @@ public class BookServer20 extends AbstractBusTestServerBase {
                 && "addBook2".equals(ri.getResourceMethod().getName())) {
                 return;
             }
-            if (!responseContext.getHeaders().containsKey("Response")) {
-                throw new RuntimeException();
+            
+            if (ri.getResourceMethod() != null
+                && "addBook2".equals(ri.getResourceMethod().getName())) {
+                return;
+            }
+            
+            if (requestContext.getUriInfo().getPath().contains("/notFound")) {
+                return;
             }
 
             if ((!responseContext.getHeaders().containsKey("DynamicResponse")
diff --git a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java
index d2bc099..583f587 100644
--- a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java
+++ b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java
@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 
+import javax.annotation.Priority;
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
@@ -41,6 +42,7 @@ import javax.ws.rs.client.ClientResponseFilter;
 import javax.ws.rs.client.Entity;
 import javax.ws.rs.client.Invocation;
 import javax.ws.rs.client.InvocationCallback;
+import javax.ws.rs.client.ResponseProcessingException;
 import javax.ws.rs.client.WebTarget;
 import javax.ws.rs.core.Feature;
 import javax.ws.rs.core.FeatureContext;
@@ -76,6 +78,8 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+
 public class JAXRS20ClientServerBookTest extends AbstractBusClientServerTestBase {
 
     public static final String PORT = BookServer20.PORT;
@@ -895,6 +899,74 @@ public class JAXRS20ClientServerBookTest extends AbstractBusClientServerTestBase
         assertEquals(entity, response.readEntity(String.class));
     }
 
+    @Test
+    public void testClientResponseFilter() {
+        final String address = "http://localhost:" + PORT + "/bookstore/books/wildcard";
+        try (Response response = ClientBuilder.newClient()
+             .register(AddHeaderClientResponseFilter.class)
+             .target(address)
+             .request("text/plain")
+             .get()) {
+            assertEquals(200, response.getStatus());
+            assertEquals("true", response.getHeaderString("X-Done"));
+        }
+    }
+
+    @Test
+    public void testExceptionWhenMultipleClientResponseFilters() {
+        final String address = "http://localhost:" + PORT + "/bookstore/books/wildcard";
+        try (Response response = ClientBuilder.newClient()
+             .register(AddHeaderClientResponseFilter.class)
+             .register(FaultyClientResponseFilter.class)
+             .target(address)
+             .request("text/plain")
+             .put(null)) {
+            fail("Should not be invoked");
+        } catch (ResponseProcessingException ex) {
+            // Seems to be an issue here, CXF creates the response context only once
+            // for all client response filters, the changes performed upstream the chain
+            // are not visible to the downstream filters. 
+            assertEquals(null, ex.getResponse().getHeaderString("X-Done"));
+        } catch (Throwable ex) {
+            fail("Should be handled by ResponseProcessingException block");
+        }
+    }
+
+    @Test(expected = ResponseProcessingException.class)
+    public void testExceptionInClientResponseFilter() {
+        final String address = "http://localhost:" + PORT + "/bookstore/books/wildcard";
+        try (Response response = ClientBuilder.newClient()
+             .register(FaultyClientResponseFilter.class)
+             .target(address)
+             .request("text/plain")
+             .get()) {
+            fail("Should raise ResponseProcessingException");
+        }
+    }
+
+    @Test(expected = ResponseProcessingException.class)
+    public void testExceptionInClientResponseFilterWhenNotFound() {
+        final String address = "http://localhost:" + PORT + "/bookstore/notFound";
+        try (Response response = ClientBuilder.newClient()
+             .register(FaultyClientResponseFilter.class)
+             .target(address)
+             .request("text/plain")
+             .put(null)) {
+            fail("Should not be invoked");
+        }
+    }
+
+    @Test
+    public void testNotFound() throws Exception {
+        final String address = "http://localhost:" + PORT + "/bookstore/notFound";
+        try (Response response = ClientBuilder.newClient()
+             .target(address)
+             .request("text/plain")
+             .put(null)) {
+            assertThat(response.getStatus(), equalTo(404));
+        }
+    }
+
     private static class ReplaceBodyFilter implements ClientRequestFilter {
 
         @Override
@@ -1058,4 +1130,23 @@ public class JAXRS20ClientServerBookTest extends AbstractBusClientServerTestBase
         }
 
     }
+    
+    
+    @Priority(2)
+    public static class AddHeaderClientResponseFilter implements ClientResponseFilter {
+        @Override
+        public void filter(ClientRequestContext requestContext, ClientResponseContext responseContext) 
+                throws IOException {
+            responseContext.getHeaders().add("X-Done", "true");
+        }
+    }
+
+    @Priority(1)
+    public static class FaultyClientResponseFilter implements ClientResponseFilter {
+        @Override
+        public void filter(ClientRequestContext requestContext, ClientResponseContext responseContext) 
+                throws IOException {
+            throw new IOException("Exception from client response filter");
+        }
+    }
 }
diff --git a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java
index 81d981c..e1598a9 100644
--- a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java
+++ b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSAsyncClientTest.java
@@ -29,7 +29,9 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 
+import javax.annotation.Priority;
 import javax.ws.rs.Consumes;
 import javax.ws.rs.NotFoundException;
 import javax.ws.rs.ProcessingException;
@@ -63,6 +65,10 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+
 public class JAXRSAsyncClientTest extends AbstractBusClientServerTestBase {
     public static final String PORT = BookServerAsyncClient.PORT;
 
@@ -385,6 +391,89 @@ public class JAXRSAsyncClientTest extends AbstractBusClientServerTestBase {
         wc.close();
     }
 
+    @Test
+    public void testClientResponseFilter() throws Exception {
+        final String address = "http://localhost:" + PORT + "/bookstore/books/wildcard";
+        try (Response response = ClientBuilder.newClient()
+             .register(AddHeaderClientResponseFilter.class)
+             .target(address)
+             .request("text/plain")
+             .async()
+             .get()
+             .get()) {
+            assertEquals(200, response.getStatus());
+            assertEquals("true", response.getHeaderString("X-Done"));
+        }
+    }
+
+    @Test
+    public void testExceptionWhenMultipleClientResponseFilters() {
+        final String address = "http://localhost:" + PORT + "/bookstore/books/wildcard";
+        try (Response response = ClientBuilder.newClient()
+             .register(AddHeaderClientResponseFilter.class)
+             .register(FaultyClientResponseFilter.class)
+             .target(address)
+             .request()
+             .async()
+             .put(null)
+             .get(10, TimeUnit.SECONDS)) {
+            fail("Should not be invoked");
+        } catch (ExecutionException ex) {
+            assertThat(ex.getCause(), is(instanceOf(ResponseProcessingException.class)));
+        } catch (Throwable ex) {
+            fail("Should be handled by ResponseProcessingException block");
+        }
+    }
+
+    @Test
+    public void testExceptionInClientResponseFilter() throws Exception {
+        final String address = "http://localhost:" + PORT + "/bookstore/books/wildcard";
+        try (Response response = ClientBuilder.newClient()
+             .register(FaultyClientResponseFilter.class)
+             .target(address)
+             .request("text/plain")
+             .async()
+             .get()
+             .get(10, TimeUnit.SECONDS)) {
+            fail("Should raise ResponseProcessingException");
+        } catch (ExecutionException ex) {
+            assertThat(ex.getCause(), is(instanceOf(ResponseProcessingException.class)));
+        } catch (Throwable ex) {
+            fail("Should be handled by ResponseProcessingException block");
+        }
+    }
+
+    @Test
+    public void testExceptionInClientResponseFilterWhenNotFound() throws Exception {
+        final String address = "http://localhost:" + PORT + "/bookstore/notFound";
+        try (Response response = ClientBuilder.newClient()
+             .register(FaultyClientResponseFilter.class)
+             .target(address)
+             .request("text/plain")
+             .async()
+             .put(null)
+             .get(10, TimeUnit.SECONDS)) {
+            fail("Should not be invoked");
+        } catch (ExecutionException ex) {
+            assertThat(ex.getCause(), is(instanceOf(ResponseProcessingException.class)));
+        } catch (Throwable ex) {
+            fail("Should be handled by ResponseProcessingException block");
+        }
+    }
+    
+    @Test
+    public void testNotFound() throws Exception {
+        final String address = "http://localhost:" + PORT + "/bookstore/notFound";
+        try (Response response = ClientBuilder.newClient()
+             .target(address)
+             .request("text/plain")
+             .async()
+             .put(null)
+             .get(10, TimeUnit.SECONDS)) {
+            assertThat(response.getStatus(), equalTo(404));
+        }
+    }
+    
     private WebClient createWebClient(String address) {
         return WebClient.create(address);
     }
@@ -464,7 +553,23 @@ public class JAXRSAsyncClientTest extends AbstractBusClientServerTestBase {
         public Response getResult() {
             return (Response)result;
         }
+    }
+    
+    @Priority(2)
+    public static class AddHeaderClientResponseFilter implements ClientResponseFilter {
+        @Override
+        public void filter(ClientRequestContext requestContext, ClientResponseContext responseContext) 
+                throws IOException {
+            responseContext.getHeaders().add("X-Done", "true");
+        }
+    }
 
-
+    @Priority(1)
+    public static class FaultyClientResponseFilter implements ClientResponseFilter {
+        @Override
+        public void filter(ClientRequestContext requestContext, ClientResponseContext responseContext) 
+                throws IOException {
+            throw new IOException("Exception from client response filter");
+        }
     }
 }


[cxf] 02/02: Recording .gitmergeinfo Changes

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

reta pushed a commit to branch 3.2.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 563a7a31f9995c5aa971ed79cd644f6883f73430
Author: reta <dr...@gmail.com>
AuthorDate: Fri May 1 13:50:29 2020 -0400

    Recording .gitmergeinfo Changes
---
 .gitmergeinfo | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitmergeinfo b/.gitmergeinfo
index 8a7145a..ee8a8a1 100644
--- a/.gitmergeinfo
+++ b/.gitmergeinfo
@@ -988,6 +988,7 @@ M 5cef96612ccae5cce36e0b7802c1c866aa7226f5
 M 5cfbdfeb59581514f3b2773abdca57728e19fdad
 M 5e3f98774ae7bcd02d13af5d3fb33b01c9def249
 M 5f6c4497f03cbd8a59d0732c9ea329891ef38f49
+M 61087a77d06dc551f81f47f68db2a59a502e629b
 M 6284e1e4d6a0bdcb773e1c3f0a7cce01543b725f
 M 68a248a5ce0d8db6d991222c110a161764aedf9d
 M 6b121380f908030dd9829615deb507c2e0fa2a08