[GitHub] [cxf] andymc12 opened a new pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] andymc12 opened a new pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
andymc12 opened a new pull request #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620
 
 
   Should fix:
   allParamsInParamTest_from_standalone
   allParamsOnFieldTest_from_standalone
   formParamInParamTest_from_standalone
   formParamOnFieldTest_from_standalone

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] andymc12 commented on issue #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
andymc12 commented on issue #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620#issuecomment-571310555
 
 
   I see two Jenkins failures in different JMS buckets - I don't think these are related to this change though, as I don't think JAX-RS is even involved in these buckets.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] reta commented on issue #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
In reply to this post by GitBox
reta commented on issue #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620#issuecomment-571411941
 
 
   @andymc12 the JMS test failures should not be related, one of them is flaky for sure, second one became flaky recently, I know why, will address it, thanks.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] reta commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
In reply to this post by GitBox
reta commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620#discussion_r364511829
 
 

 ##########
 File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
 ##########
 @@ -1878,4 +1891,16 @@ public static String logMessageHandlerProblem(String name, Class<?> cls, MediaTy
         return errorMessage;
     }
 
+    // copy the input stream so that it is not inadvertently closed
+    private static InputStream copyAndGetEntityStream(Message m) {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        try {
+            IOUtils.copy(m.getContent(InputStream.class), baos);
+        } catch (IOException e) {
+            throw ExceptionUtils.toInternalServerErrorException(e, null);
+        }
+        final byte[] copiedBytes = baos.toByteArray();
+        m.setContent(InputStream.class, new ByteArrayInputStream(copiedBytes));
 
 Review comment:
   I am wondering if we have to close the source (origin) `InputStream` here, since we replace it (dangling?) with the copy , what do you think @andymc12 ?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] andymc12 commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
In reply to this post by GitBox
andymc12 commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620#discussion_r365002716
 
 

 ##########
 File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
 ##########
 @@ -1878,4 +1891,16 @@ public static String logMessageHandlerProblem(String name, Class<?> cls, MediaTy
         return errorMessage;
     }
 
+    // copy the input stream so that it is not inadvertently closed
+    private static InputStream copyAndGetEntityStream(Message m) {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        try {
+            IOUtils.copy(m.getContent(InputStream.class), baos);
+        } catch (IOException e) {
+            throw ExceptionUtils.toInternalServerErrorException(e, null);
+        }
+        final byte[] copiedBytes = baos.toByteArray();
+        m.setContent(InputStream.class, new ByteArrayInputStream(copiedBytes));
 
 Review comment:
   Good question.  I'm running that suggestion through our automated tests and the TCK - on the surface, I think it should be fine, but just want to be sure.  The change I added looks like this:
   ```
   InputStream origInputStream = m.getContent(InputStream.class);
           try {
               IOUtils.copy(origInputStream, baos);
           } catch (IOException e) {
               throw ExceptionUtils.toInternalServerErrorException(e, null);
           } finally {
               try {
                   origInputStream.close();
               } catch (Throwable t) { /* AutoFFDC */ }
           }
   ```
   
   I'm out of the office tomorrow, so I may not have the results until Monday.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] reta commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
In reply to this post by GitBox
reta commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620#discussion_r365065856
 
 

 ##########
 File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
 ##########
 @@ -1878,4 +1891,16 @@ public static String logMessageHandlerProblem(String name, Class<?> cls, MediaTy
         return errorMessage;
     }
 
+    // copy the input stream so that it is not inadvertently closed
+    private static InputStream copyAndGetEntityStream(Message m) {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        try {
+            IOUtils.copy(m.getContent(InputStream.class), baos);
+        } catch (IOException e) {
+            throw ExceptionUtils.toInternalServerErrorException(e, null);
+        }
+        final byte[] copiedBytes = baos.toByteArray();
+        m.setContent(InputStream.class, new ByteArrayInputStream(copiedBytes));
 
 Review comment:
   :+1: thanks a lot!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] dkulp merged pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
In reply to this post by GitBox
dkulp merged pull request #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] andymc12 commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
In reply to this post by GitBox
andymc12 commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620#discussion_r365845990
 
 

 ##########
 File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
 ##########
 @@ -1878,4 +1891,16 @@ public static String logMessageHandlerProblem(String name, Class<?> cls, MediaTy
         return errorMessage;
     }
 
+    // copy the input stream so that it is not inadvertently closed
+    private static InputStream copyAndGetEntityStream(Message m) {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        try {
+            IOUtils.copy(m.getContent(InputStream.class), baos);
+        } catch (IOException e) {
+            throw ExceptionUtils.toInternalServerErrorException(e, null);
+        }
+        final byte[] copiedBytes = baos.toByteArray();
+        m.setContent(InputStream.class, new ByteArrayInputStream(copiedBytes));
 
 Review comment:
   I see Dan already merged it (thanks Dan!), but just to followup, the change had no ill effects in our automated testing - the normal functional tests and TCK all passed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] reta commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures

GitBox
In reply to this post by GitBox
reta commented on a change in pull request #620: CXF-7996: TCK: Resolves FormParam-related failures
URL: https://github.com/apache/cxf/pull/620#discussion_r366102864
 
 

 ##########
 File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
 ##########
 @@ -1878,4 +1891,16 @@ public static String logMessageHandlerProblem(String name, Class<?> cls, MediaTy
         return errorMessage;
     }
 
+    // copy the input stream so that it is not inadvertently closed
+    private static InputStream copyAndGetEntityStream(Message m) {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        try {
+            IOUtils.copy(m.getContent(InputStream.class), baos);
+        } catch (IOException e) {
+            throw ExceptionUtils.toInternalServerErrorException(e, null);
+        }
+        final byte[] copiedBytes = baos.toByteArray();
+        m.setContent(InputStream.class, new ByteArrayInputStream(copiedBytes));
 
 Review comment:
   Yep, I also rerun TCK (https://issues.apache.org/jira/browse/CXF-7996) and these failure are gone, thanks @andymc12 !

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services