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.

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.










Reply via email to