You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Henning Schmiedehausen (JIRA)" <ji...@apache.org> on 2008/12/03 22:19:44 UTC
[jira] Created: (SHINDIG-728) The Shindig social-api should not use
Guice annotations to find its implementation classes.
The Shindig social-api should not use Guice annotations to find its implementation classes.
-------------------------------------------------------------------------------------------
Key: SHINDIG-728
URL: https://issues.apache.org/jira/browse/SHINDIG-728
Project: Shindig
Issue Type: Bug
Components: RESTful API (Java)
Reporter: Henning Schmiedehausen
Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by Adam Winer <aw...@google.com>.
On Thu, Dec 4, 2008 at 8:10 AM, Ian Boston (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653339#action_12653339 ]
>
> Ian Boston commented on SHINDIG-728:
> ------------------------------------
>
>
> Not being Guice expert, I would have thought a
>
> Object o = injector.getInstance(clazz);
> if ( o != null ) {
> return o.getClass();
> }
> return clazz;
>
> would be simpler.
It is. The use of getProvider and Key in the patch is unnecessary.
>
> Also I am not 100% certain that the interface check is safe, its probably possible to map an concrete class to an abstract class, which is going to give a class that cant in created.
The interface check is wrong; it's perfectly legit to use Guice to bind
ConcreteClass.class to SubclassOfConcreteClass.class.
>
> But anyway, I am no Guice expert and from your use of getProvider() I guess you are.
>
>> The Shindig social-api should not use Guice annotations to find its implementation classes.
>> -------------------------------------------------------------------------------------------
>>
>> Key: SHINDIG-728
>> URL: https://issues.apache.org/jira/browse/SHINDIG-728
>> Project: Shindig
>> Issue Type: Bug
>> Components: RESTful API (Java)
>> Reporter: Henning Schmiedehausen
>> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>>
>>
>> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>
[jira] Updated: (SHINDIG-728) The Shindig social-api should not use
Guice annotations to find its implementation classes.
Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henning Schmiedehausen updated SHINDIG-728:
-------------------------------------------
Attachment: 0001-Add-testcase-for-InterfaceClassMapper.patch
This is a test case testing the behaviour of the InterfaceClassMapper. It fails with the current, @ImplementedBy based mapper.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Attachments: 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653987#action_12653987 ]
Henning Schmiedehausen commented on SHINDIG-728:
------------------------------------------------
Yes, that looks very nice. Thanks for tracking this down.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Assignee: Ian Boston
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653405#action_12653405 ]
Henning Schmiedehausen commented on SHINDIG-728:
------------------------------------------------
I fully agree. I am not a Guice maven either (but I have one sitting right across the hallway and I can wave at him if I need something. ;-) )
In the second case, it will blow up. Given the fact that this mapper is used to map transfer objects onto concrete classes, I feel that the performance gain by not going to Guice for non-interface types outweights the risk of blowing up. If it really happens, we look at the use case and reengineer.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Ian Boston (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653339#action_12653339 ]
Ian Boston commented on SHINDIG-728:
------------------------------------
Not being Guice expert, I would have thought a
Object o = injector.getInstance(clazz);
if ( o != null ) {
return o.getClass();
}
return clazz;
would be simpler.
Also I am not 100% certain that the interface check is safe, its probably possible to map an concrete class to an abstract class, which is going to give a class that cant in created.
But anyway, I am no Guice expert and from your use of getProvider() I guess you are.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (SHINDIG-728) The Shindig social-api should not use
Guice annotations to find its implementation classes.
Posted by "Ian Boston (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ian Boston closed SHINDIG-728.
------------------------------
Resolution: Fixed
Removed method and reconfigured to use GuiceBeanProvider
All tests pass.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Assignee: Ian Boston
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Ian Boston (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653646#action_12653646 ]
Ian Boston commented on SHINDIG-728:
------------------------------------
On waking up this morning, I thought, 'Why do we need to resolve the interface at all, we have Guice and we can create beans from that directly as in the net-sf json lib serializer (which btw is not in use at the moment)?'
Looking deeper at the converter code, the above method is irrelevant and can be deleted all together, I will submit a fix.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-728) The Shindig social-api should not use
Guice annotations to find its implementation classes.
Posted by "Ian Boston (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ian Boston updated SHINDIG-728:
-------------------------------
Comment: was deleted
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Assignee: Ian Boston
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-728) The Shindig social-api should not use
Guice annotations to find its implementation classes.
Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henning Schmiedehausen updated SHINDIG-728:
-------------------------------------------
Attachment: 0001-A-sane-InterfaceClassMapper.patch
This is a better implementation of the InterfaceClassMapper which uses Guice to ask what is the current implementation of an interface.
Due to a Guice limitation (you can only get an instance from a Provider, not the class it provides), this actually instantiates an object every time it needs to find out the class mapping. As XStream caches this internally, this shouldn't be a problem; however if the class generation has some internal side effects (which it should not have as long as these are simple beans), this means that one bean more is instantiated than expected. Caveat Emptor.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653570#action_12653570 ]
Kevin Brown commented on SHINDIG-728:
-------------------------------------
"In the second case, it will blow up. Given the fact that this mapper is used to map transfer objects onto concrete classes, I feel that the performance gain by not going to Guice for non-interface types outweights the risk of blowing up. If it really happens, we look at the use case and reengineer."
Isn't this cached? see also your previous post reasoning about why constructing the object is acceptable.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Ian Boston (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653651#action_12653651 ]
Ian Boston commented on SHINDIG-728:
------------------------------------
Thank you for the patches, but in this instance the solution was elsewhere, so I did not apply them.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Assignee: Ian Boston
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Ian Boston (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653653#action_12653653 ]
Ian Boston commented on SHINDIG-728:
------------------------------------
I am traveling until 1st January 2009, and will check emails occasionally.
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Assignee: Ian Boston
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (SHINDIG-728) The Shindig social-api should not
use Guice annotations to find its implementation classes.
Posted by "Ian Boston (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ian Boston reassigned SHINDIG-728:
----------------------------------
Assignee: Ian Boston
> The Shindig social-api should not use Guice annotations to find its implementation classes.
> -------------------------------------------------------------------------------------------
>
> Key: SHINDIG-728
> URL: https://issues.apache.org/jira/browse/SHINDIG-728
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java)
> Reporter: Henning Schmiedehausen
> Assignee: Ian Boston
> Attachments: 0001-A-sane-InterfaceClassMapper.patch, 0001-Add-testcase-for-InterfaceClassMapper.patch
>
>
> Currently, the social-api uses some clever annotation magic to find its implementation classes inside the XStream marshalling/unmarshalling code. This code actually does not work with custom object implementations.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.