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 2021/11/06 18:18:25 UTC

[cxf] branch 3.4.x-fixes updated: CXF-8601: [Regression] jaxrs.ee.rs.core.response tests (#866)

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

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


The following commit(s) were added to refs/heads/3.4.x-fixes by this push:
     new 0871c0b  CXF-8601: [Regression] jaxrs.ee.rs.core.response tests (#866)
0871c0b is described below

commit 0871c0b26b3deca33fd4208097253b6aec3370f6
Author: Andriy Redko <dr...@gmail.com>
AuthorDate: Sat Nov 6 12:45:59 2021 -0400

    CXF-8601: [Regression] jaxrs.ee.rs.core.response tests (#866)
    
    (cherry picked from commit 9e47ff2e4348fdf0cd40993965a67b21d3c7867b)
---
 .../org/apache/cxf/jaxrs/impl/ResponseImpl.java    | 20 +++++--
 .../org/apache/cxf/jaxrs/utils/JAXRSUtils.java     | 67 +++++++++++++++++++++-
 .../apache/cxf/jaxrs/impl/ResponseImplTest.java    | 39 +++++++++++++
 .../apache/cxf/jaxrs/client/AbstractClient.java    |  2 +-
 4 files changed, 122 insertions(+), 6 deletions(-)

diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
index fc711f8..f157c2a 100644
--- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
@@ -66,7 +66,7 @@ import org.apache.cxf.message.Message;
 import org.apache.cxf.message.MessageUtils;
 
 public final class ResponseImpl extends Response {
-
+    public static final String RESPONSE_STREAM_AUTO_CLOSE = "response.stream.auto.close";
     private static final Pattern LINK_DELIMITER = Pattern.compile(",\\s*(?=\\<|$)");
 
     private StatusType status;
@@ -453,6 +453,13 @@ public final class ResponseImpl extends Response {
             .createMessageBodyReaderInterceptor(cls, t, anns, mediaType, outMessage, entityStreamAvailable, null);
 
         if (readers != null) {
+            // By default, the response entity was never closed automatically which is not compliant
+            // with JAX-RS specification and TCK. The auto-close behavior is controlled by 
+            // ResponseImpl#RESPONSE_STREAM_AUTO_CLOSE property which is set to "false" by default. 
+            // The autoCloseHint alters the default value of this property to be "true" for all 
+            // non-streaming and non-streaming like responses (to minimize the impact of the change
+            // as much as possible) in order to follow the specification.
+            final boolean autoCloseHint = !JAXRSUtils.isStreamingLikeOutType(cls, t);
             try {
                 if (entityBufferred) {
                     InputStream.class.cast(entity).reset();
@@ -468,7 +475,7 @@ public final class ResponseImpl extends Response {
                                                                   responseMessage);
                 // close the entity after readEntity is called.
                 T tCastLastEntity = castLastEntity();
-                autoClose(cls, false);
+                autoCloseWithHint(cls, autoCloseHint, false);
                 return tCastLastEntity;
             } catch (NoContentException ex) {
                 //when content is empty, return null instead of throw exception to pass TCK
@@ -477,7 +484,7 @@ public final class ResponseImpl extends Response {
                     autoClose(cls, true);
                     reportMessageHandlerProblem("MSG_READER_PROBLEM", cls, mediaType, ex);
                 } else {
-                    autoClose(cls, false);
+                    autoCloseWithHint(cls, autoCloseHint, false);
                     return null;
                 }
             } catch (Exception ex) {
@@ -544,8 +551,13 @@ public final class ResponseImpl extends Response {
     }
 
     protected void autoClose(Class<?> cls, boolean exception) {
+        autoCloseWithHint(cls, false, exception);
+    }
+    
+    protected void autoCloseWithHint(Class<?> cls, boolean autoCloseHint, boolean exception) {
         if (!entityBufferred && !JAXRSUtils.isStreamingOutType(cls)
-            && (exception || MessageUtils.getContextualBoolean(outMessage, "response.stream.auto.close"))) {
+            && (exception || MessageUtils.getContextualBoolean(outMessage, 
+                RESPONSE_STREAM_AUTO_CLOSE, autoCloseHint))) {
             close();
         }
     }
diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
index 302e435..a8e6ea0 100644
--- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
@@ -19,6 +19,7 @@
 
 package org.apache.cxf.jaxrs.utils;
 
+import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -81,6 +82,7 @@ import javax.ws.rs.ext.ReaderInterceptorContext;
 import javax.ws.rs.ext.WriterInterceptor;
 import javax.ws.rs.ext.WriterInterceptorContext;
 import javax.xml.namespace.QName;
+import javax.xml.transform.Source;
 
 import org.apache.cxf.common.i18n.BundleUtils;
 import org.apache.cxf.common.logging.LogUtils;
@@ -101,7 +103,10 @@ import org.apache.cxf.jaxrs.ext.MessageContext;
 import org.apache.cxf.jaxrs.ext.MessageContextImpl;
 import org.apache.cxf.jaxrs.ext.ProtocolHeaders;
 import org.apache.cxf.jaxrs.ext.ProtocolHeadersImpl;
+import org.apache.cxf.jaxrs.ext.multipart.Attachment;
+import org.apache.cxf.jaxrs.ext.multipart.InputStreamDataSource;
 import org.apache.cxf.jaxrs.ext.multipart.MultipartBody;
+import org.apache.cxf.jaxrs.ext.xml.XMLSource;
 import org.apache.cxf.jaxrs.impl.AsyncResponseImpl;
 import org.apache.cxf.jaxrs.impl.ContainerRequestContextImpl;
 import org.apache.cxf.jaxrs.impl.ContainerResponseContextImpl;
@@ -169,12 +174,72 @@ public final class JAXRSUtils {
     private static final Annotation[] EMPTY_ANNOTATIONS = new Annotation[0];
     private static final Set<Class<?>> STREAMING_OUT_TYPES = new HashSet<>(
         Arrays.asList(InputStream.class, Reader.class, StreamingOutput.class));
+    private static final Set<Class<?>> STREAMING_LIKE_OUT_TYPES = new HashSet<>(
+        Arrays.asList(XMLSource.class, InputStreamDataSource.class, 
+            MultipartBody.class, Attachment.class));
+    private static final Set<String> REACTIVE_OUT_TYPES = new HashSet<>(
+        Arrays.asList(
+            // Reactive Streams
+            "org.reactivestreams.Publisher",
+            // Project Reactor
+            "reactor.core.publisher.Mono",
+            "reactor.core.publisher.Flux",
+            // RxJava
+            "rx.Observable",
+            // RxJava2
+            "io.reactivex.Flowable",
+            "io.reactivex.Maybe",
+            "io.reactivex.Observable",
+            "io.reactivex.Single",
+            // RxJava3
+            "io.reactivex.rxjava3.core.Flowable",
+            "io.reactivex.rxjava3.core.Maybe",
+            "io.reactivex.rxjava3.core.Observable",
+            "io.reactivex.rxjava3.core.Single",
+            // JDK
+            "java.util.concurrent.CompletableFuture",
+            "java.util.concurrent.CompletionStage"
+        ));
 
     private JAXRSUtils() {
     }
 
+    /**
+     * Consider additional types as stream-like and returns if the class and corresponding type
+     * refer to one of those. 
+     * @param cls class to check
+     * @param type type to check
+     * @return "true" is the class and corresponding type could be considered streaming-like, 
+     * "false" otherwise.
+     */
+    public static boolean isStreamingLikeOutType(Class<?> cls, Type type) {
+        if (cls != null && (isStreamingOutType(cls) 
+                || STREAMING_LIKE_OUT_TYPES.contains(cls) 
+                || REACTIVE_OUT_TYPES.contains(cls.getName()))) {
+            return true;
+        }
+
+        if (type instanceof ParameterizedType) {
+            final ParameterizedType parameterizedType = (ParameterizedType)type;
+            for (Type arg: parameterizedType.getActualTypeArguments()) {
+                if (isStreamingLikeOutType(null, arg)) {
+                    return true;
+                }
+            }
+        }
+        
+        if (type instanceof Class) {
+            final Class<?> typeCls = (Class<?>)type;
+            return isStreamingOutType(typeCls) || STREAMING_LIKE_OUT_TYPES.contains(typeCls);
+        }
+        
+        return false;
+    }
+    
     public static boolean isStreamingOutType(Class<?> type) {
-        return STREAMING_OUT_TYPES.contains(type);
+        return STREAMING_OUT_TYPES.contains(type) 
+            || Closeable.class.isAssignableFrom(type)
+            || Source.class.isAssignableFrom(type);
     }
 
     public static List<PathSegment> getPathSegments(String thePath, boolean decode) {
diff --git a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/ResponseImplTest.java b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/ResponseImplTest.java
index 95e8b41..00a3d9d 100644
--- a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/ResponseImplTest.java
+++ b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/ResponseImplTest.java
@@ -21,6 +21,8 @@ package org.apache.cxf.jaxrs.impl;
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
+import java.io.Reader;
+import java.lang.annotation.Annotation;
 import java.net.URI;
 import java.util.List;
 import java.util.Locale;
@@ -71,6 +73,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 
@@ -598,6 +601,42 @@ public class ResponseImplTest {
         }
     }
 
+    @Test
+    public void testReadEntityAndGetEntityAfter() {
+        final String str = "ouch";
+
+        final ResponseImpl response = new ResponseImpl(500, str);
+        final Message outMessage = createMessage();
+        outMessage.put(Message.REQUEST_URI, "http://localhost");
+        response.setOutMessage(outMessage);
+
+        final MultivaluedMap<String, Object> headers = new MetadataMap<>();
+        headers.putSingle("Content-Type", "text/xml");
+        response.addMetadata(headers);
+
+        assertEquals(str, response.readEntity(String.class));
+        assertThrows(IllegalStateException.class, () -> response.getEntity());
+    }
+    
+    @Test
+    public void testReadEntityWithAnnotations() {
+        final String str = "ouch";
+
+        final ResponseImpl response = new ResponseImpl(500, str);
+        final Message outMessage = createMessage();
+        outMessage.put(Message.REQUEST_URI, "http://localhost");
+        response.setOutMessage(outMessage);
+
+        final MultivaluedMap<String, Object> headers = new MetadataMap<>();
+        headers.putSingle("Content-Type", "text/xml");
+        response.addMetadata(headers);
+
+        final Annotation[] annotations = String.class.getAnnotations();
+        assertEquals(str, response.readEntity(String.class, annotations));
+        assertThrows(IllegalStateException.class, 
+            () -> response.readEntity(Reader.class, annotations));
+    }
+    
     public static class StringBean {
         private String header;
 
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 af9bc5a..1ca60dc 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
@@ -554,7 +554,7 @@ public abstract class AbstractClient implements Client {
 
     protected boolean responseStreamCanBeClosed(Message outMessage, Class<?> cls) {
         return !JAXRSUtils.isStreamingOutType(cls)
-            && MessageUtils.getContextualBoolean(outMessage, "response.stream.auto.close");
+            && MessageUtils.getContextualBoolean(outMessage, ResponseImpl.RESPONSE_STREAM_AUTO_CLOSE);
     }
 
     protected void completeExchange(Exchange exchange, boolean proxy) {