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 12:51:07 UTC
[cxf] branch 3.3.x-fixes updated: CXF-8271: The async JAX-RS client
never completes the response in case of an exception during the interceptor
chain processing. (#665)
This is an automated email from the ASF dual-hosted git repository.
reta pushed a commit to branch 3.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/3.3.x-fixes by this push:
new 61087a7 CXF-8271: The async JAX-RS client never completes the response in case of an exception during the interceptor chain processing. (#665)
61087a7 is described below
commit 61087a77d06dc551f81f47f68db2a59a502e629b
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)
---
.../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 c65945c..511dba4 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 ae87a19..bda7f19 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 845f196..ead9898 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;
@@ -75,6 +77,8 @@ import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -854,6 +858,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
@@ -1017,4 +1089,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 c1c4fa2..af0fe42 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;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -391,6 +397,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);
}
@@ -470,7 +559,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");
+ }
}
}