On Fri, Feb 20, 2009 at 9:27 AM, Paul Lindner <[email protected]> wrote:
> Well, one thing we could do is this: > > * If a filter for a single user is supplied, then call getPeople. This > means that getPerson remains filter/sort free > * If no filter, then continue to call getPerson. Seems like a reasonable compromise that keeps the SPI intact. > > > On Feb 20, 2009, at 9:20 AM, Ian Boston wrote: > > Sorry, I didn't know.... it was talked about at length on list, but no one >> looked like there were interested. I will bow out and let those with more >> skin in the game decide. >> >> On 20 Feb 2009, at 17:12, Paul Lindner wrote: >> >> +1 on adding collectionOptions to getPerson() >>> >>> We too have a heavily optimized version of getPerson() that's now not >>> being called. >>> >>> >>> On Feb 20, 2009, at 9:03 AM, Ben Smith wrote: >>> >>> All sounds pretty reasonable. I'm going to have to bow out and let the >>>> committers come to the right decision. There is a patch for either option >>>> (that will probably need updating). >>>> >>>> Let me know what you decide. >>>> Ben >>>> >>>> On 20 Feb 2009, at 16:50, Adam Winer wrote: >>>> >>>> This breaks the interface in a worse way, and leaves the APIs broken >>>>> going >>>>> forward - one method that doesn't get called, and one method that >>>>> *thinks* >>>>> its returning a collection, but really isn't in some undocumented >>>>> cases. I >>>>> know it broke our servers when we upgraded (our implementation of >>>>> getPerson() was subtly different from getPeople(), and intentionally >>>>> so.) >>>>> Better to break APIs in a clear and simple way than do so cryptically. >>>>> We shouldn't be changing the service layer on the 1.0 branch, but we >>>>> can't >>>>> commit to API stability on trunk. >>>>> >>>>> On Fri, Feb 20, 2009 at 8:06 AM, Ben Smith <[email protected]> >>>>> wrote: >>>>> >>>>> That's what I suggested (and supplied a patch for) but Ian was keen >>>>>> not to >>>>>> break the interface for the service layer. >>>>>> >>>>>> >>>>>> On 20 Feb 2009, at 16:02, Adam Winer wrote: >>>>>> >>>>>> Speaking of this change: what got committed seems odd. For scalar >>>>>> >>>>>>> calls, PersonHandler now calls PersonService.getPeople(), and drops >>>>>>> all but the first result. PersonService.getPerson() is never called. >>>>>>> >>>>>>> It seems significantly cleaner to add CollectionOptions to >>>>>>> PersonService.getPerson() (and, perhaps, rename CollectionOptions). >>>>>>> Why did we go this route? Sorry that I didn't follow the discussion >>>>>>> the first time through. >>>>>>> >>>>>>> On Fri, Feb 20, 2009 at 1:39 AM, Ben Smith <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>> Hi Louis, sorry I missed this. >>>>>>>> >>>>>>>> I'll make sure I do this from now on. I thought it was something >>>>>>>> that >>>>>>>> commiters did, sorry for the omission. >>>>>>>> >>>>>>>> On 17 Feb 2009, at 17:40, Louis Ryan wrote: >>>>>>>> >>>>>>>> Ben >>>>>>>> >>>>>>>>> >>>>>>>>> Can you post this patch to codereview.appspot.com. Its become >>>>>>>>> working >>>>>>>>> practice to post changes that are likely to incur feedback there. >>>>>>>>> >>>>>>>>> -Louis >>>>>>>>> >>>>>>>>> On Sat, Feb 14, 2009 at 8:37 AM, Ben Smith (JIRA) <[email protected] >>>>>>>>> > >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> [ >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> https://issues.apache.org/jira/browse/SHINDIG-918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel >>>>>>>>>> ] >>>>>>>>>> >>>>>>>>>> Ben Smith updated SHINDIG-918: >>>>>>>>>> ------------------------------ >>>>>>>>>> >>>>>>>>>> Attachment: SHINDIG-918-improvement.patch >>>>>>>>>> >>>>>>>>>> Has PersonHandler call getPeople for all types of request. >>>>>>>>>> >>>>>>>>>> Change to PersonHandler to allow filtering of requests - 0.9 >>>>>>>>>> >>>>>>>>>>> improvement >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> ------------------------------------------------------------------------ >>>>>>>>>>> >>>>>>>>>>> Key: SHINDIG-918 >>>>>>>>>>> URL: https://issues.apache.org/jira/browse/SHINDIG-918 >>>>>>>>>>> Project: Shindig >>>>>>>>>>> Issue Type: Improvement >>>>>>>>>>> Components: Java >>>>>>>>>>> Affects Versions: trunk >>>>>>>>>>> Reporter: Ben Smith >>>>>>>>>>> Fix For: trunk >>>>>>>>>>> >>>>>>>>>>> Attachments: SHINDIG-918-improvement.patch >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Calls for single people (say, /people/@me/@self) should be >>>>>>>>>>> filterable: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> http://opensocial-resources.googlecode.com/svn/spec/draft/REST-API.xml#standardQueryParameters >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Because PersonHandler calls PersonService.getPerson() for such >>>>>>>>>>> requests, >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> which doesn't accept CollectionOptions, the result can't be >>>>>>>>>> filtered. A >>>>>>>>>> patch in SHINDIG-904 solved this by changing the getPerson() >>>>>>>>>> method >>>>>>>>>> signature but after much discussion on the mailing list it was >>>>>>>>>> decided >>>>>>>>>> that >>>>>>>>>> a better solution would be to change PersonHandler to only call >>>>>>>>>> getPeople, >>>>>>>>>> and convert the RestfulCollection result to a single Person when >>>>>>>>>> calls >>>>>>>>>> for a >>>>>>>>>> single user are made (like, /people/@me/@self). >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Patch to follow. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> This message is automatically generated by JIRA. >>>>>>>>>> - >>>>>>>>>> You can reply to this email to add a comment to the issue online. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>>> >>> >> >
