You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by David Blevins <da...@gmail.com> on 2021/04/13 02:07:35 UTC
@Encoded TCK issue
Hi All,
I'm investigating a Jakarta EE TCK failure:
- com/sun/ts/tests/jaxrs/ee/rs/beanparam/form/plain/JAXRSClient#formFieldParamEntityWithEncodedTest_from_standalone
This test posts a form and is checking to ensure the @Encoded annotation is being respected. The code to respect that annotation is definitely implemented, however before that code is called we've already parsed, decoded and cached the form into the Message so the test fails.
- https://github.com/apache/cxf/blob/master/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java#L1033-L1054
Essentially, due to this caching the "decode" boolean is ignored after the first field of the bean is processed. When you have a bean like the following where there are two fields, one results in 'decode=true' and the second in 'decode=false', the second field's wishes will not happen as the first field caused the form to be decoded and cached. The second field will then get a decoded value despite the @Encoded annotation.
public class FormBeanParamEntity {
@DefaultValue(Constants.DEFAULT_VALUE)
@FormParam(Constants.PARAM_ENTITY_WITH_CONSTRUCTOR)
public ParamEntityWithConstructor paramEntityWithConstructor;
@Encoded
@DefaultValue(Constants.DEFAULT_VALUE)
@FormParam(Constants.PARAM_ENTITY_WITH_FROMSTRING)
public ParamEntityWithFromString paramEntityWithFromString;
So essentially @Encoded will only work as the first parameter, field, etc.
Investigating fixes at the moment and will submit a PR. Open to thoughts and preferences. This is one of about 33 failures I'm hunting down.
-David
Re: @Encoded TCK issue
Posted by Andriy Redko <dr...@gmail.com>.
Hi Andy, David,
Thanks a lot guys for helping out with the TCK, @Andy you have a number of PRs
opened to fix TCK tests, may be you could spend some time to revive them? I would
be more than happy to help out.
Thanks a lot.
Best Regards,
Andriy Redko
AM> Hi David,
AM> I had to do some digging - but yes, we addressed that issue in our Open
AM> Liberty fork of CXF and I must not have contributed that fix back to the
AM> main CXF fork (apologies for that).
AM> Here are the changes I made in the OL fork:
AM> https://github.com/OpenLiberty/open-liberty/pull/2504 - basically deferring
AM> the decoding until after the map has been populated. If you want, I can
AM> try to push this change back to the main CXF fork.
AM> Thanks for pointing this out,
AM> Andy
AM> On Mon, Apr 12, 2021 at 9:12 PM David Blevins <da...@gmail.com>
AM> wrote:
>> Did you run into this in Open Liberty?
>> --
>> David Blevins
>> http://twitter.com/dblevins
>> http://www.tomitribe.com
>> Begin forwarded message:
>> *From: *David Blevins <da...@gmail.com>
>> *Subject: **@Encoded TCK issue*
>> *Date: *April 12, 2021 at 7:07:35 PM PDT
>> *To: *dev@cxf.apache.org
>> Hi All,
>> I'm investigating a Jakarta EE TCK failure:
>> -
>> com/sun/ts/tests/jaxrs/ee/rs/beanparam/form/plain/JAXRSClient#formFieldParamEntityWithEncodedTest_from_standalone
>> This test posts a form and is checking to ensure the @Encoded annotation
>> is being respected. The code to respect that annotation is definitely
>> implemented, however before that code is called we've already parsed,
>> decoded and cached the form into the Message so the test fails.
>> -
>> https://github.com/apache/cxf/blob/master/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java#L1033-L1054
>> Essentially, due to this caching the "decode" boolean is ignored after the
>> first field of the bean is processed. When you have a bean like the
>> following where there are two fields, one results in 'decode=true' and the
>> second in 'decode=false', the second field's wishes will not happen as the
>> first field caused the form to be decoded and cached. The second field
>> will then get a decoded value despite the @Encoded annotation.
>> public class FormBeanParamEntity {
>> @DefaultValue(Constants.DEFAULT_VALUE)
>> @FormParam(Constants.PARAM_ENTITY_WITH_CONSTRUCTOR)
>> public ParamEntityWithConstructor paramEntityWithConstructor;
>> @Encoded
>> @DefaultValue(Constants.DEFAULT_VALUE)
>> @FormParam(Constants.PARAM_ENTITY_WITH_FROMSTRING)
>> public ParamEntityWithFromString paramEntityWithFromString;
>> So essentially @Encoded will only work as the first parameter, field, etc.
>> Investigating fixes at the moment and will submit a PR. Open to thoughts
>> and preferences. This is one of about 33 failures I'm hunting down.
>> -David
Re: @Encoded TCK issue
Posted by Andriy Redko <dr...@gmail.com>.
That is the one, right. We use it on Jenkins "as-is".
If you need any assistance, please let us know, thanks!
Best Regards,
Andriy Redko
>> On Apr 14, 2021, at 1:13 PM, David Blevins <da...@gmail.com> wrote:
>> Is there any information on how to setup the TCK to run against plain CXF? If so I can maybe create an https://tck.work/cxf/ project and for a non-TomEE view and get some pure-CXF-only results.
DB> Think I found it. Going to pick through this and see if I can't make something go:
DB> - https://github.com/apache/cxf/blob/master/tck/Jenkinsfile
DB> -David
Re: @Encoded TCK issue
Posted by David Blevins <da...@gmail.com>.
> On Apr 14, 2021, at 1:13 PM, David Blevins <da...@gmail.com> wrote:
>
> Is there any information on how to setup the TCK to run against plain CXF? If so I can maybe create an https://tck.work/cxf/ project and for a non-TomEE view and get some pure-CXF-only results.
Think I found it. Going to pick through this and see if I can't make something go:
- https://github.com/apache/cxf/blob/master/tck/Jenkinsfile
-David
Re: @Encoded TCK issue
Posted by David Blevins <da...@gmail.com>.
List of the 30-ish failures we're seeing here (the ones that start with "jaxrs"):
- https://issues.apache.org/jira/browse/TOMEE-3140
Here's a list that shows the actual run data. If you click on the list on the right, it'll show the javatest log.
- https://tck.work/tomee/tests?path=com.sun.ts.tests.jaxrs&build=1618360886800&status=FAILED&display=50
Here's a count of all exceptions thrown in by those failing tests:
- https://tck.work/tomee/issues?path=com.sun.ts.tests.jaxrs&issue=&build=1618360886800&status=FAILED&display=50
Is there any information on how to setup the TCK to run against plain CXF? If so I can maybe create an https://tck.work/cxf/ project and for a non-TomEE view and get some pure-CXF-only results.
-David
> On Apr 14, 2021, at 4:36 AM, Andriy Redko <dr...@gmail.com> wrote:
>
> Thanks a lot, we have this TCK build on Jenkins [1] which we run regularly,
> we have 70 failing tests (+2 failures which we know about).
>
> Thanks!
>
> [1] https://ci-builds.apache.org/job/CXF/job/CXF-JAXRS-TCK/
>
> Best Regards,
> Andriy Redko
>
> AM> Sounds good. I'll try to have a PR tomorrow. The fix is easy enough, but
> AM> I want to make sure that we have a good test case for it too.
>
> AM> Andriy and I (mostly Andriy) were looking at Jakarta TCK issues about a
> AM> year ago. He has some results posted at
> AM> https://issues.apache.org/jira/browse/CXF-7996 - in the most recent report,
> AM> there were ~80 failures, but it's possible that some of them have already
> AM> been fixed since then. It might help if you post your failures in that
> AM> issue to keep better track of them.
>
> AM> Thanks,
>
> AM> Andy
>
> AM> On Tue, Apr 13, 2021 at 8:44 PM David Blevins <da...@gmail.com>
> AM> wrote:
>
>>>> On Apr 13, 2021, at 9:10 AM, Andy McCright <j....@gmail.com>
>>> wrote:
>>>>
>>>> Hi David,
>>>>
>>>> I had to do some digging - but yes, we addressed that issue in our Open
>>> Liberty fork of CXF and I must not have contributed that fix back to the
>>> main CXF fork (apologies for that).
>>>>
>>>> Here are the changes I made in the OL fork:
>>> https://github.com/OpenLiberty/open-liberty/pull/2504 - basically
>>> deferring the decoding until after the map has been populated. If you
>>> want, I can try to push this change back to the main CXF fork.
>
>>> Thanks for the update, Andy!
>
>>> Your patch sounds much better than mine. I was able to get the test to
>>> pass by effectively keeping two cached copies of the parsed form -- one
>>> decoded and one encoded.
>
>>> -
>>> https://github.com/apache/tomee/commit/5c47f4482ea80601e17ead56e6f0a72e494a5b7e#diff-472b947a3abe2e03b6959ef6ad4e1137ff3012dee9de1a4c15acf57919ab525bL1027-L1035
>
>>> Works, but feels a little too hacky. If you want to submit a PR for
>>> yours, that's great. I'll post a list of the remaining 30-ish failures
>>> that we working through in a separate thread.
>
>
>>> -David
>
Re: @Encoded TCK issue
Posted by Andriy Redko <dr...@gmail.com>.
Thanks a lot, we have this TCK build on Jenkins [1] which we run regularly,
we have 70 failing tests (+2 failures which we know about).
Thanks!
[1] https://ci-builds.apache.org/job/CXF/job/CXF-JAXRS-TCK/
Best Regards,
Andriy Redko
AM> Sounds good. I'll try to have a PR tomorrow. The fix is easy enough, but
AM> I want to make sure that we have a good test case for it too.
AM> Andriy and I (mostly Andriy) were looking at Jakarta TCK issues about a
AM> year ago. He has some results posted at
AM> https://issues.apache.org/jira/browse/CXF-7996 - in the most recent report,
AM> there were ~80 failures, but it's possible that some of them have already
AM> been fixed since then. It might help if you post your failures in that
AM> issue to keep better track of them.
AM> Thanks,
AM> Andy
AM> On Tue, Apr 13, 2021 at 8:44 PM David Blevins <da...@gmail.com>
AM> wrote:
>> > On Apr 13, 2021, at 9:10 AM, Andy McCright <j....@gmail.com>
>> wrote:
>> >
>> > Hi David,
>> >
>> > I had to do some digging - but yes, we addressed that issue in our Open
>> Liberty fork of CXF and I must not have contributed that fix back to the
>> main CXF fork (apologies for that).
>> >
>> > Here are the changes I made in the OL fork:
>> https://github.com/OpenLiberty/open-liberty/pull/2504 - basically
>> deferring the decoding until after the map has been populated. If you
>> want, I can try to push this change back to the main CXF fork.
>> Thanks for the update, Andy!
>> Your patch sounds much better than mine. I was able to get the test to
>> pass by effectively keeping two cached copies of the parsed form -- one
>> decoded and one encoded.
>> -
>> https://github.com/apache/tomee/commit/5c47f4482ea80601e17ead56e6f0a72e494a5b7e#diff-472b947a3abe2e03b6959ef6ad4e1137ff3012dee9de1a4c15acf57919ab525bL1027-L1035
>> Works, but feels a little too hacky. If you want to submit a PR for
>> yours, that's great. I'll post a list of the remaining 30-ish failures
>> that we working through in a separate thread.
>> -David
Re: @Encoded TCK issue
Posted by Andy McCright <j....@gmail.com>.
Sounds good. I'll try to have a PR tomorrow. The fix is easy enough, but
I want to make sure that we have a good test case for it too.
Andriy and I (mostly Andriy) were looking at Jakarta TCK issues about a
year ago. He has some results posted at
https://issues.apache.org/jira/browse/CXF-7996 - in the most recent report,
there were ~80 failures, but it's possible that some of them have already
been fixed since then. It might help if you post your failures in that
issue to keep better track of them.
Thanks,
Andy
On Tue, Apr 13, 2021 at 8:44 PM David Blevins <da...@gmail.com>
wrote:
> > On Apr 13, 2021, at 9:10 AM, Andy McCright <j....@gmail.com>
> wrote:
> >
> > Hi David,
> >
> > I had to do some digging - but yes, we addressed that issue in our Open
> Liberty fork of CXF and I must not have contributed that fix back to the
> main CXF fork (apologies for that).
> >
> > Here are the changes I made in the OL fork:
> https://github.com/OpenLiberty/open-liberty/pull/2504 - basically
> deferring the decoding until after the map has been populated. If you
> want, I can try to push this change back to the main CXF fork.
>
> Thanks for the update, Andy!
>
> Your patch sounds much better than mine. I was able to get the test to
> pass by effectively keeping two cached copies of the parsed form -- one
> decoded and one encoded.
>
> -
> https://github.com/apache/tomee/commit/5c47f4482ea80601e17ead56e6f0a72e494a5b7e#diff-472b947a3abe2e03b6959ef6ad4e1137ff3012dee9de1a4c15acf57919ab525bL1027-L1035
>
> Works, but feels a little too hacky. If you want to submit a PR for
> yours, that's great. I'll post a list of the remaining 30-ish failures
> that we working through in a separate thread.
>
>
> -David
>
>
Re: @Encoded TCK issue
Posted by David Blevins <da...@gmail.com>.
> On Apr 13, 2021, at 9:10 AM, Andy McCright <j....@gmail.com> wrote:
>
> Hi David,
>
> I had to do some digging - but yes, we addressed that issue in our Open Liberty fork of CXF and I must not have contributed that fix back to the main CXF fork (apologies for that).
>
> Here are the changes I made in the OL fork: https://github.com/OpenLiberty/open-liberty/pull/2504 - basically deferring the decoding until after the map has been populated. If you want, I can try to push this change back to the main CXF fork.
Thanks for the update, Andy!
Your patch sounds much better than mine. I was able to get the test to pass by effectively keeping two cached copies of the parsed form -- one decoded and one encoded.
- https://github.com/apache/tomee/commit/5c47f4482ea80601e17ead56e6f0a72e494a5b7e#diff-472b947a3abe2e03b6959ef6ad4e1137ff3012dee9de1a4c15acf57919ab525bL1027-L1035
Works, but feels a little too hacky. If you want to submit a PR for yours, that's great. I'll post a list of the remaining 30-ish failures that we working through in a separate thread.
-David
Re: @Encoded TCK issue
Posted by Andy McCright <j....@gmail.com>.
Hi David,
I had to do some digging - but yes, we addressed that issue in our Open
Liberty fork of CXF and I must not have contributed that fix back to the
main CXF fork (apologies for that).
Here are the changes I made in the OL fork:
https://github.com/OpenLiberty/open-liberty/pull/2504 - basically deferring
the decoding until after the map has been populated. If you want, I can
try to push this change back to the main CXF fork.
Thanks for pointing this out,
Andy
On Mon, Apr 12, 2021 at 9:12 PM David Blevins <da...@gmail.com>
wrote:
> Did you run into this in Open Liberty?
>
>
> --
> David Blevins
> http://twitter.com/dblevins
> http://www.tomitribe.com
>
> Begin forwarded message:
>
> *From: *David Blevins <da...@gmail.com>
> *Subject: **@Encoded TCK issue*
> *Date: *April 12, 2021 at 7:07:35 PM PDT
> *To: *dev@cxf.apache.org
>
> Hi All,
>
> I'm investigating a Jakarta EE TCK failure:
>
> -
> com/sun/ts/tests/jaxrs/ee/rs/beanparam/form/plain/JAXRSClient#formFieldParamEntityWithEncodedTest_from_standalone
>
> This test posts a form and is checking to ensure the @Encoded annotation
> is being respected. The code to respect that annotation is definitely
> implemented, however before that code is called we've already parsed,
> decoded and cached the form into the Message so the test fails.
>
> -
> https://github.com/apache/cxf/blob/master/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java#L1033-L1054
>
> Essentially, due to this caching the "decode" boolean is ignored after the
> first field of the bean is processed. When you have a bean like the
> following where there are two fields, one results in 'decode=true' and the
> second in 'decode=false', the second field's wishes will not happen as the
> first field caused the form to be decoded and cached. The second field
> will then get a decoded value despite the @Encoded annotation.
>
> public class FormBeanParamEntity {
> @DefaultValue(Constants.DEFAULT_VALUE)
> @FormParam(Constants.PARAM_ENTITY_WITH_CONSTRUCTOR)
> public ParamEntityWithConstructor paramEntityWithConstructor;
>
> @Encoded
> @DefaultValue(Constants.DEFAULT_VALUE)
> @FormParam(Constants.PARAM_ENTITY_WITH_FROMSTRING)
> public ParamEntityWithFromString paramEntityWithFromString;
>
>
> So essentially @Encoded will only work as the first parameter, field, etc.
>
> Investigating fixes at the moment and will submit a PR. Open to thoughts
> and preferences. This is one of about 33 failures I'm hunting down.
>
>
> -David
>
>
>