You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wink.apache.org by "Bryant Luk (JIRA)" <ji...@apache.org> on 2009/07/07 18:12:14 UTC
[jira] Created: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
MessageBodyReader.isReadable methods should do straight ==
----------------------------------------------------------
Key: WINK-53
URL: https://issues.apache.org/jira/browse/WINK-53
Project: Wink
Issue Type: Bug
Components: Common
Affects Versions: 0.1
Reporter: Bryant Luk
Assignee: Bryant Luk
Priority: Minor
In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
For instance in InputStreamProvider:
public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
MediaType mediaType) {
- return type != null && InputStream.class.isAssignableFrom(type);
+ return type != null && InputStream.class == type;
}
If the method parameter were:
@GET
public void getMethod(FileInputStream fis) {
}
we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryant Luk updated WINK-53:
---------------------------
Attachment: WINK-53.patch
The only one that I didn't know what to do for is:
org.apache.wink.common.internal.providers.entity.csv.CsvDeserializerProvider
I think the code for that is correct as is.
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryant Luk resolved WINK-53.
----------------------------
Resolution: Fixed
Fix Version/s: 0.1
If there's an issue with the fix, please reopen.
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12730435#action_12730435 ]
Hudson commented on WINK-53:
----------------------------
Integrated in Wink-Trunk-JDK15 #26 (See [http://hudson.zones.apache.org/hudson/job/Wink-Trunk-JDK15/26/])
Update reverse isAssignableFrom for readers
See []
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Nadav Fischer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nadav Fischer reopened WINK-53:
-------------------------------
You're completely right that the previous implementation was incorrect, but after re-thinking this, I think that we are missing some functionality by using the == (thanks go to Eli Baram for pointing this out)
Consider this scenario:
Given the following class hierarchy, where B inherits from A, and C and D inherit from B:
{noformat}
A
|-B
|-C
|-D
{noformat}
and a Provider for class B (let's call it BProvider :-)), if we simply use == for the isReadable, then BProvider will not be picked up for an entity of type A (assuming the media type is not the issue), even though it shouldn't be a problem.
I think that what we want to do is reverse the order of classes in the isAssignableFrom invocation in isReadable so that C and D entities won't be picked up, but A will be, like so:
instead of this
{code}
return type != null && InputStream.class.isAssignableFrom(type);
{code}
do this
{code}
return type != null && type.isAssignableFrom(InputStream.class);
{code}
What do you think?
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728934#action_12728934 ]
Hudson commented on WINK-53:
----------------------------
Integrated in Wink-Trunk-JDK15 #13 (See [http://hudson.zones.apache.org/hudson/job/Wink-Trunk-JDK15/13/])
Use == for isReadable instead of isAssignable
See []
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryant Luk resolved WINK-53.
----------------------------
Resolution: Fixed
I think I've fixed this for the ones I touched originally and added it for String as well. I think this should be "good enough" as Object will be filled by something. If not, please re-open.
Thanks Eli and Nadav for catching this.
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12729282#action_12729282 ]
Bryant Luk commented on WINK-53:
--------------------------------
(If this reposts, sorry. JIRA didn't think I was logged in so I lost my original comment).
In essence, I think I understand what you're saying. Honestly, I didn't think about moving up the hierarchy. However, beyond the Source built-in providers, what type are you expecting to support above (i.e. is it only Object)? I would hope that no one is expecting an Object entity parameter to be useful since every JAX-RS implementation could implement this very differently and who knows what gets injected (i.e. String, Source, etc.)
I think we should be careful about supporting things up the hierarchy and/or supporting Object entity parameter injection. This can "wildly" change behavior depending on providers moving around, media types being added, etc.
Taking the Source provider as an example, I would also be harder pressed to change the StreamSource that is returned for a plain Source entity parameter to a a different Source type after we do a release unless there's a strong reason to do so. Just throwing this out there (which I'm fairly certain everyone already knows), but some developers expect certain behavior/performance characteristics after a release to stay constant even though the runtime should have the flexibility to do things differently.
My recommendation to them is of course, to write their own application provided Provider (which is one of the things I like about JAX-RS in that we supposedly always get out of your way) but not everyone will be "happy" about that. :)
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryant Luk closed WINK-53.
--------------------------
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12730362#action_12730362 ]
Bryant Luk commented on WINK-53:
--------------------------------
I'll redo this fix, just wanted to let people know what my initial thoughts were. For certain providers where we have some ambiguity (Source), I'll try to clear the ambiguity up so we return something consistent.
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (WINK-53) MessageBodyReader.isReadable methods
should do straight ==
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-53?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryant Luk closed WINK-53.
--------------------------
Please reopen again if this is still an issue.
> MessageBodyReader.isReadable methods should do straight ==
> ----------------------------------------------------------
>
> Key: WINK-53
> URL: https://issues.apache.org/jira/browse/WINK-53
> Project: Wink
> Issue Type: Bug
> Components: Common
> Affects Versions: 0.1
> Reporter: Bryant Luk
> Assignee: Bryant Luk
> Priority: Minor
> Fix For: 0.1
>
> Attachments: WINK-53.patch
>
>
> In some cases the built in MessageBodyReaders providers's isReadable method does an isAssignableFrom. This should just be a straight == instead.
> For instance in InputStreamProvider:
> public boolean isReadable(Class<?> type, Type genericType, Annotation[] ann
> MediaType mediaType) {
> - return type != null && InputStream.class.isAssignableFrom(type);
> + return type != null && InputStream.class == type;
> }
> If the method parameter were:
> @GET
> public void getMethod(FileInputStream fis) {
> }
> we get a 500 back (because I think the injection throws) instead of a 415. I can fix this up.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.