You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2021/04/30 18:44:36 UTC
[GitHub] [cxf] reta opened a new pull request #782: CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
reta opened a new pull request #782:
URL: https://github.com/apache/cxf/pull/782
--
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:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #782: [WIP] CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #782:
URL: https://github.com/apache/cxf/pull/782#issuecomment-830654594
@andymc12 as always, need your expertise, mind please taking a look? 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:
users@infra.apache.org
[GitHub] [cxf] dblevins commented on pull request #782: CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
Posted by GitBox <gi...@apache.org>.
dblevins commented on pull request #782:
URL: https://github.com/apache/cxf/pull/782#issuecomment-830684310
@reta You're totally right. I read too quickly and missed that `available()` is only getting acted upon if the response is a definitive "yes" and there's pretty comprehensive fallback logic in the `isEmpty` method. I think you have it covered pretty well.
My +1
--
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:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #782: CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #782:
URL: https://github.com/apache/cxf/pull/782#issuecomment-830677003
> I wonder if we wrapped the input stream instead of returning null and had the wrapper count the bytes read. We could check the count on an exception and use that to determine our blocking read returned no payload.
Thought about that as well but dealing with exceptions and counting the bytes would be more complicated comparing to the simple check that stream is empty.
--
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:
users@infra.apache.org
[GitHub] [cxf] reta merged pull request #782: CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
Posted by GitBox <gi...@apache.org>.
reta merged pull request #782:
URL: https://github.com/apache/cxf/pull/782
--
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:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #782: [WIP] CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #782:
URL: https://github.com/apache/cxf/pull/782#issuecomment-830654481
Build is green: https://ci-builds.apache.org/job/CXF/job/CXF-JDK8-PR-Manual/14/
--
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:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #782: CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #782:
URL: https://github.com/apache/cxf/pull/782#issuecomment-830676670
@dblevins thanks a lot for looking, that is correct, we do trust `inputStream.available()` only if it returns non-zero value
```
// if available is 0 it does not mean it is empty
if (is.available() > 0) {
return false;
}
```
Otherwise the assumption is that stream is not empty (and we proceed further with mark / reset of pushback stream). What do you think?
--
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:
users@infra.apache.org
[GitHub] [cxf] dblevins commented on pull request #782: CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest
Posted by GitBox <gi...@apache.org>.
dblevins commented on pull request #782:
URL: https://github.com/apache/cxf/pull/782#issuecomment-830674441
I like this better than my too-brittle hack :)
- https://github.com/dblevins/cxf/commit/32103901420a7811f1b5af0c37e9bf797fac8b81
The one concern I'd have is that `inputStream.available()` is a guess and could potentially return zero when there is a valid payload coming. That might happen in scenarios where the server-side could just be taking a while streaming back the payload.
I wonder if we wrapped the input stream instead of returning null and had the wrapper count the bytes read. We could check the count on an exception and use that to determine our blocking read returned no payload.
It'd be sort of a hybrid of the two approaches. Thoughts?
--
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:
users@infra.apache.org