Mailing-List: contact issues-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Date: Mon, 20 Apr 2015 01:49:59 +0000 (UTC)
From: "Neal Hu (JIRA)" <jira@apache.org>
To: issues@cxf.apache.org
Message-ID: <JIRA.12783533.1426841036000.43438.1429494599221@Atlassian.JIRA>
In-Reply-To: <JIRA.12783533.1426841036000@Atlassian.JIRA>
References: <JIRA.12783533.1426841036000@Atlassian.JIRA>
 <JIRA.12783533.1426841036059@arcas>
Subject: [jira] [Comment Edited] (CXF-6307) Wrong select the message body
 reader
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/CXF-6307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14502184#comment-14502184 ] 

Neal Hu edited comment on CXF-6307 at 4/20/15 1:49 AM:
-------------------------------------------------------

Hi Sergey,

We have fixed the problem of wrong select the message body reader/writer
and passed the TCK test. Paste the reader example here and the new class
for your referrence. 


org.apache.cxf.jaxrs.provider.ProviderFactory.createMessageBodyReader
	...
	ProviderInfo<MessageBodyReader<?>> mr =
(ProviderInfo<MessageBodyReader<?>>) chooseMessageReader(bodyType,
parameterType, parameterAnnotations, mediaType, m);
        ProviderInfo<MessageBodyReader<?>> baseMr =
(ProviderInfo<MessageBodyReader<?>>) baseFactory.chooseMessageReader
(bodyType,  parameterType, parameterAnnotations, mediaType, m);
        if (mr != null && baseMr != null) {
            MessageBodyReaderComparator comparator = new
MessageBodyReaderComparator();
            int res = comparator.compare(mr, baseMr);
            candidateProviderInfos.add(res > 0 ? baseMr : mr);
        } else if (mr != null) { candidateProviderInfos.add(mr);
        } else if (baseMr != null) { candidateProviderInfos.add(baseMr);  }
...
 return (MessageBodyReader<T>) candidateProviderInfos.get(0).getProvider();
---------
    private <T> ProviderInfo<?> chooseMessageReader(Class<T> type,
                                                    Type genericType,
                                                    Annotation[] annotations,
                                                    MediaType mediaType,
                                                    Message m,
                                                    List<ProviderInfo<?>> candidateProviderInfos) {

        List<MessageBodyReader<?>> candidates = new LinkedList<MessageBodyReader<?>>();
        ProviderInfo<?> pi = null;
        for (ProviderInfo<MessageBodyReader<?>> ep : messageReaders) {
            if (matchesReaderCriterias(ep, type, genericType, annotations, mediaType, m)) {

                candidateProviderInfos.add(ep);
                boolean isReadable = ep.getProvider().isReadable(type, genericType, annotations, mediaType);

                if (isBaseFactory()) {
                    if (candidates.isEmpty() && isReadable) {
                        pi = ep;
                        candidates.add(ep.getProvider());
                    }
                    continue;
                }
                if (isReadable && candidates.isEmpty()) {
                    handleMapper(candidates, ep, type, m, MessageBodyReader.class, false);
                    if (!candidates.isEmpty())
                        pi = ep;
                }

            }
        }

        return pi;

    }

    private <T> boolean matchesReaderCriterias(ProviderInfo<MessageBodyReader<?>> pi,
                                               Class<T> type,
                                               Type genericType,
                                               Annotation[] annotations,
                                               MediaType mediaType,
                                               Message m) {
        MessageBodyReader<?> ep = pi.getProvider();
        List<MediaType> supportedMediaTypes = JAXRSUtils.getProviderConsumeTypes(ep);

        List<MediaType> availableMimeTypes =
                        JAXRSUtils.intersectMimeTypes(Collections.singletonList(mediaType), supportedMediaTypes, false);

        if (availableMimeTypes.size() == 0) {
            return false;
        }
        if (m.get(ACTIVE_JAXRS_PROVIDER_KEY) != ep || pi.getProvider() != null) {
            injectContextValues(pi, m);
        }
        return true;
    }
---------------
org.apache.cxf.jaxrs.provider.PrimitiveTextProvider
@Produces(MediaType.TEXT_PLAIN)
@Consumes(MediaType.TEXT_PLAIN)
public class PrimitiveTextProvider<T> extends AbstractConfigurableProvider
implements MessageBodyReader<T>, MessageBodyWriter<T> {
...
}
 add org.apache.cxf.jaxrs.provider.StringProvider -  Only for none
text/plain String
 public class StringProvider<T> implements MessageBodyReader<T>,
MessageBodyWriter<T> {
    private static boolean isSupported(Class<?> type) {
        return String.class == type;
    }
    @Override
    public boolean isReadable(Class<?> type, Type genericType, Annotation[]
annotations, MediaType mt) {
        return isSupported(type);
    }
...
}

Best Regards,
                                          
From:	"Sergey Beryozkin (JIRA)" <jira@apache.org>
Date:	2015/04/03 23:39
Subject:	[jira] [Comment Edited] (CXF-6307) Wrong select the message
            body reader


    [
https://issues.apache.org/jira/browse/CXF-6307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14394579#comment-14394579
 ]

Sergey Beryozkin edited comment on CXF-6307 at 4/3/15 3:38 PM:
---------------------------------------------------------------

Just a short summary based on the analysis/conversations I've done so far.

- MBR can depend on the declarative Consumes support or do the checks
dynamically in its isReadable. Both options are valid/compliant. Either
option can be used independently or both options can be combined.
- MBR can provide a valid implementation of "Boolean/etc for text/plain
only" by using either declarative Consumes or dynamic checks. Spec can
not/does not enforce which approach is used to implement a given
requirement
- This test is technically valid
- This test should not be at the CTS level but at a concrete JAX-RS
implementation level (example, at RI level) because it was written with the
assumption that a default MBR uses a declarative approach as opposed to a
dynamic check approach.

Base on the above, but also on the fact that having all of CXF default
providers (many of them with wildcard Consumes as per the spec) will
slowdown a selection process but also be a very sensitive operation in the
short term, my recommendation is to challenge this test.

I'm not opposed in principle to refactoring ProviderFactory and hence I
will keep this issue open and will review how this can be addressed. But
given that we have major releases coming in now, and also because it is
very important that a validity of MBRs doing the dynamic checks is
enforced, IMHO the faster solution to getting behind this test is to
challenge it

Cheers, Sergey


was (Author: sergey_beryozkin):
Just a short summary based on the anslysis/conversations I've done so far.

- MBR can depend on the declarative Consumes support or do the checks
dynamically in its isReadable. Both options are valid/compliant. Either
option can be used independently or both options can be combined.
- MBR can provide a valid implementation of "Boolean/etc for text/plain
only" by using either declarative Consumes or dynamic checks. Spec can
not/does not enforce which approach is used to implement a given
requirement
- This test is technically valid
- This test should not be at the CTS level but at a concrete JAX-RS
implementation level (example, at RI level) because it was written with the
assumption that a default MBR uses a declarative approach as opposed to a
dynamic check approach.

Base on the above, but also on the fact that having all of CXF default
providers (many of them with wildcard Consumes as per the spec) will
slowdown a selection process but also be a very sensitive operation in the
short term, my recommendation is to challenge this test.

I'm not opposed in principle to refactoring ProviderFactory and hence I
will keep this issue open and will review how this can be addressed. But
given that we have major releases coming in now, and also because it is
very important that a validity of MBRs doing the dynamic checks is
enforced, IMHO the faster solution to getting behind this test is to
challenge it

Cheers, Sergey


(Status.NOT_ACCEPTABLE);
antns, MediaType mt) {
mm,
WebApplicationException {
java.lang.Boolean, java.lang.Character, java.lang.Number Only for
text/plain. Corresponding primitive types supported via boxing/unboxing
conversion.
MUST use those in preference to its own pre-packaged providers when either
could handle the same request. More precisely, step 4 in Section 4.2.1 and
step 5 in Section 4.2.2 MUST prefer application-provided over pre-packaged
entity providers.
type of the request, see Section
the isReadable method of
type.
(the media type is the same) on Section 4.2.1 and step 5 in Section 4.2.2
MUST prefer application-provided over pre-packaged entity providers. So
org.apache.cxf.jaxrs.provider.PrimitiveTextProvider should be selected as
its media type MUST be text/plain and the application provider's media type
is */*.(x/y>x/*>*/*)
the application provided provider should not be chosen.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


was (Author: xingjhu@cn.ibm.com):
Hi Sergey,

We have fixed the problem of wrong select the message body reader/writer
and passed the TCK test. Paste the reader example here and the new class
for your referrence. Please note we add some caches for the reader/writer
selection for performence optimization, ProviderInfo.getOldProvider for
integration with CDI/EJB injection. You can ignore these two enhencements.

org.apache.cxf.jaxrs.provider.ProviderFactory.createMessageBodyReader
	...
	ProviderInfo<MessageBodyReader<?>> mr =
(ProviderInfo<MessageBodyReader<?>>) chooseMessageReader(bodyType,
parameterType, parameterAnnotations, mediaType, m);
        ProviderInfo<MessageBodyReader<?>> baseMr =
(ProviderInfo<MessageBodyReader<?>>) baseFactory.chooseMessageReader
(bodyType,  parameterType, parameterAnnotations, mediaType, m);
        if (mr != null && baseMr != null) {
            MessageBodyReaderComparator comparator = new
MessageBodyReaderComparator();
            int res = comparator.compare(mr, baseMr);
            candidateProviderInfos.add(res > 0 ? baseMr : mr);
        } else if (mr != null) { candidateProviderInfos.add(mr);
        } else if (baseMr != null) { candidateProviderInfos.add(baseMr);  }
...
 return (MessageBodyReader<T>) candidateProviderInfos.get(0).getProvider();
---------------
org.apache.cxf.jaxrs.provider.PrimitiveTextProvider
@Produces(MediaType.TEXT_PLAIN)
@Consumes(MediaType.TEXT_PLAIN)
public class PrimitiveTextProvider<T> extends AbstractConfigurableProvider
implements MessageBodyReader<T>, MessageBodyWriter<T> {
...
}
 add org.apache.cxf.jaxrs.provider.StringProvider -  Only for none
text/plain String
 public class StringProvider<T> implements MessageBodyReader<T>,
MessageBodyWriter<T> {
    private static boolean isSupported(Class<?> type) {
        return String.class == type;
    }
    @Override
    public boolean isReadable(Class<?> type, Type genericType, Annotation[]
annotations, MediaType mt) {
        return isSupported(type);
    }
...
}

(See attached file: StringProvider.java)(See attached file:
ProviderFactory.java)(See attached file: PrimitiveTextProvider.java)

Best Regards,
                                          
From:	"Sergey Beryozkin (JIRA)" <jira@apache.org>
Date:	2015/04/03 23:39
Subject:	[jira] [Comment Edited] (CXF-6307) Wrong select the message
            body reader


    [
https://issues.apache.org/jira/browse/CXF-6307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14394579#comment-14394579
 ]

Sergey Beryozkin edited comment on CXF-6307 at 4/3/15 3:38 PM:
---------------------------------------------------------------

Just a short summary based on the analysis/conversations I've done so far.

- MBR can depend on the declarative Consumes support or do the checks
dynamically in its isReadable. Both options are valid/compliant. Either
option can be used independently or both options can be combined.
- MBR can provide a valid implementation of "Boolean/etc for text/plain
only" by using either declarative Consumes or dynamic checks. Spec can
not/does not enforce which approach is used to implement a given
requirement
- This test is technically valid
- This test should not be at the CTS level but at a concrete JAX-RS
implementation level (example, at RI level) because it was written with the
assumption that a default MBR uses a declarative approach as opposed to a
dynamic check approach.

Base on the above, but also on the fact that having all of CXF default
providers (many of them with wildcard Consumes as per the spec) will
slowdown a selection process but also be a very sensitive operation in the
short term, my recommendation is to challenge this test.

I'm not opposed in principle to refactoring ProviderFactory and hence I
will keep this issue open and will review how this can be addressed. But
given that we have major releases coming in now, and also because it is
very important that a validity of MBRs doing the dynamic checks is
enforced, IMHO the faster solution to getting behind this test is to
challenge it

Cheers, Sergey


was (Author: sergey_beryozkin):
Just a short summary based on the anslysis/conversations I've done so far.

- MBR can depend on the declarative Consumes support or do the checks
dynamically in its isReadable. Both options are valid/compliant. Either
option can be used independently or both options can be combined.
- MBR can provide a valid implementation of "Boolean/etc for text/plain
only" by using either declarative Consumes or dynamic checks. Spec can
not/does not enforce which approach is used to implement a given
requirement
- This test is technically valid
- This test should not be at the CTS level but at a concrete JAX-RS
implementation level (example, at RI level) because it was written with the
assumption that a default MBR uses a declarative approach as opposed to a
dynamic check approach.

Base on the above, but also on the fact that having all of CXF default
providers (many of them with wildcard Consumes as per the spec) will
slowdown a selection process but also be a very sensitive operation in the
short term, my recommendation is to challenge this test.

I'm not opposed in principle to refactoring ProviderFactory and hence I
will keep this issue open and will review how this can be addressed. But
given that we have major releases coming in now, and also because it is
very important that a validity of MBRs doing the dynamic checks is
enforced, IMHO the faster solution to getting behind this test is to
challenge it

Cheers, Sergey


(Status.NOT_ACCEPTABLE);
antns, MediaType mt) {
mm,
WebApplicationException {
java.lang.Boolean, java.lang.Character, java.lang.Number Only for
text/plain. Corresponding primitive types supported via boxing/unboxing
conversion.
MUST use those in preference to its own pre-packaged providers when either
could handle the same request. More precisely, step 4 in Section 4.2.1 and
step 5 in Section 4.2.2 MUST prefer application-provided over pre-packaged
entity providers.
type of the request, see Section
the isReadable method of
type.
(the media type is the same) on Section 4.2.1 and step 5 in Section 4.2.2
MUST prefer application-provided over pre-packaged entity providers. So
org.apache.cxf.jaxrs.provider.PrimitiveTextProvider should be selected as
its media type MUST be text/plain and the application provider's media type
is */*.(x/y>x/*>*/*)
the application provided provider should not be chosen.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


> Wrong select the message body reader
> ------------------------------------
>
>                 Key: CXF-6307
>                 URL: https://issues.apache.org/jira/browse/CXF-6307
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 3.0.3
>         Environment: Windows
>            Reporter: Neal Hu
>             Fix For: 3.0.5
>
>
> This is a CTS testcase, the resource class is like this:   
>    @POST
>     @Path("boolean")
>     public Boolean postBoolean(Boolean bool) {
>     	if(bool){    		 
>     		throw new WebApplicationException(Status.NOT_ACCEPTABLE);
>     	}
>         return false;
>     }
> The application provided provider is like this:
> public class MyReader implements MessageBodyReader<Boolean> {
>     @Override
>     public boolean isReadable(Class<?> type, Type type1, Annotation[] antns, MediaType mt) {
>     	return type== Boolean.class;
>     }
>     @Override
>     public Object readFrom(Class<Object > type,
>                              Type type1,
>                              Annotation[] antns,
>                              MediaType mt, MultivaluedMap<String, String> mm,
>                              InputStream in) throws IOException, WebApplicationException {
> 		return Boolean.valueOf("true");        
>     }
> }
> The request
> Content-Type:text/plain
> Accept:text/plain 
> Method:POST
> Body:false
> According to jsr339 section 4.2.4 Standard Entity Providers java.lang.Boolean, java.lang.Character, java.lang.Number Only for text/plain. Corresponding primitive types supported via boxing/unboxing conversion.
> An implementation MUST support application-provided entity providers and MUST use those in preference to its own pre-packaged providers when either could handle the same request. More precisely, step 4 in Section 4.2.1 and step 5 in Section 4.2.2 MUST prefer application-provided over pre-packaged entity providers.
> 4.2.1 Message Body Reader
> 3. Select the set of MessageBodyReader classes that support the media type of the request, see Section
> 4.2.3.
> 4. Iterate through the selected MessageBodyReader classes and, utilizing the isReadable method of
> each, choose a MessageBodyReader provider that supports the desired Java type.
> It says the entity providers should be sorted by media type firstly then(the media type is the same) on Section 4.2.1 and step 5 in Section 4.2.2 MUST prefer application-provided over pre-packaged entity providers. So org.apache.cxf.jaxrs.provider.PrimitiveTextProvider should be selected as its media type MUST be text/plain and the application provider's media type is */*.(x/y>x/*>*/*)
> So at this point the CTS expected response code is 200 instead of 406, the application provided provider should not be chosen.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)