Hi all - I've been working on support for a more flexible converter. I had planned to test this out internally before contributing to Shindig, as I didn't want to make major changes before 0.8. However, there are a few open issues that this may help with so I wanted to send out a patch for discussion.
Patch: https://issues.apache.org/jira/browse/SHINDIG-651 @codereview: http://codereview.appspot.com/7286/show Brief overview: - Annotate bean methods with @Export annotation to make visible for (de)serialization. Example: @Export String public getFoo() {} - Can also annotate class with @ExportAll for simple value classes that are only used with serialization. - Supports exporting public fields as well. I built a JSON converter on top of this, seems that it would be easy to add an XML converter as well. Anyway, would love feedback on: - Whether this is the right general approach - Whether we should wait until after 0.8 - The specific details of the CL Evan On Fri, Oct 10, 2008 at 5:07 PM, Aleksey Perfilov <[EMAIL PROTECTED]> wrote: > > Adapters might be painful, depending on how complex the class that's > imported and how many such classes there are. > > If all the getters with parameters are simply ignored and we only invoke > getFoo(), it will work. If a getter needs some parameters it's not really a > simple accessor that's meant for serialization. Of course I'm not certain > that all classes will be able to be properly de-serialized from resulting > json, but it won't make things worse because right now they would not work > for sure :) > > For example I have a calendar class that has such accessors, plus other > fields that are classes with such accessors, and when I locally modify > shindig to ignore getFoo(xxx) type of getters then it works. > > > On 10/10/08 4:13 PM, "Kevin Brown" <[EMAIL PROTECTED]> wrote: > > > On Fri, Oct 10, 2008 at 4:08 PM, Aleksey Perfilov <[EMAIL PROTECTED]> > wrote: > > > >> > >> I think I misinterpreted what Kevin said. > >> > >> I agree that using annotations is preferable, although name based > matching > >> can be also left in place, in case annotations are not present, as I > >> mentioned, for imported classes. > > > > > > That doesn't actually address the issue. If you import class X and it has > > method getFoo(xxx), it doesn't work. > > > > An adapter is definitely preferrable for that. > > > > > >> > >> > >> > >> On 10/10/08 3:33 PM, "Aleksey Perfilov" <[EMAIL PROTECTED]> wrote: > >> > >>> > >>> True, and of course we do that for our own classes. But in case you end > >> up > >>> using some other class that comes from standard Java or some library, > you > >>> can't annotate that. Perhaps using an adapter in some way might be the > >> only > >>> way then. > >>> > >>> Besides, I actually don't see annotations being taken into account in > >>> BeanJsonConverter code. It just grabs all methods that start with "get" > >> when > >>> converting object to json. Actually, I just noticed that in the Person > >> class > >>> from org.apache.shindig.social.opensocial.model, getGender is not > >> annotated, > >>> neither is getUtcOffset, yet both of them are converted to json. > >>> > >>> Aleksey > >>> > >>> > >>> On 10/10/08 2:36 PM, "Kevin Brown" <[EMAIL PROTECTED]> wrote: > >>> > >>>> I think it would make more sense to use annotations on the beans > instead > >> of > >>>> doing name based matching. That way you're always explicit in what you > >>>> export and don't have problems like this. > >>>> > >>>> On Fri, Oct 10, 2008 at 12:55 PM, Aleksey Perfilov <[EMAIL PROTECTED] > >>> wrote: > >>>> > >>>>> > >>>>> Hi all, > >>>>> > >>>>> I¹ve had problems using BeanJsonConverter on objects that contain > >> getters > >>>>> that have 1 or more arguments. > >>>>> Since convertMethodsToJson() expects not to see any arguments on > >> getters, > >>>>> invoke() will crash on getters that have some. > >>>>> > >>>>> Do you think we should adjust getMatchingMethods() to filter out > >> getters > >>>>> that require parameters? Or just skip those getters in > >>>>> convertMethodsToJson(). > >>>>> I think it is reasonable to assume we don¹t need those for conversion > >>>>> purposes. > >>>>> > >>>>> Thanks, > >>>>> > >>>>> Aleksey > >>>>> > >>>>> > >>> > >> > >> > >
