Adam,
There was a long(ish) thread on this over 2 weeks. IMHO its not quite
as simple as that.
The use of the getPeople was specified by the REST API was to
get all the people associated with one ID and then filter that set
down to match a single ID.
getPerson gets the person specified in the call. How would the filter
be applied ? I think returning null depending on the filter would be
almost as confusing.
Before suggesting that we didn't change the SPI, I read the javadoc
for the SPI and considered if a set of a single ID with a filter
matched the SPI specification. IMHO it did, hence the suggestion.
Unfortunately getPeople does not return a structure of the correct
form so it needs to be processed without breaking the Future contract.
------------------------------------
As for the commit: In the past, we have reviewed patches in JIRA,
talked about them and the committed them.
Have we changed the process to have all patches go into code review
before being comitted ?
I dont remember seeing any conclusions about this on list?
If this is the way we are supposed to be working, then what are the
thresholds requiring review. I realize that I haven't been able to do
anything for several months, manly because of the pressures of my
employment, so I might be out of touch.
Ian
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.
