shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stanton Sievers" <siever...@gmail.com>
Subject Re: Review Request: A patch that enables API for updating person data
Date Fri, 20 Jan 2012 16:35:55 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/#review4491
-----------------------------------------------------------


Small nits on whitespace.

I do have some bigger questions regarding the implementation and maybe these are more for
the spec group.  What's to stop one person from updating another person?  The rest API says
that the User-Id defaults to @me, but doesn't say you can't list another user.  Shouldn't
we only allow @me to be updated, which would be equivalent to the viewer id in the security
token?

You mentioned implementing osapi.people.update() as a gadget API, but I neither see it in
this review (is it there by virtue of the REST API being implemented?) nor do I see any tests
for it.  I  also don't see osapi.people.update() in the social gadget spec and if the intent
is for it to be there, you should try to get the spec updated as well.

- Stanton


On 2012-01-19 09:30:25, Evgeny Bogdanov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/
> -----------------------------------------------------------
> 
> (Updated 2012-01-19 09:30:25)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> A patch that enables API for updating person data
> PUT people/12/@self
> osapi.people.update()
> 
> 
> for this JIRA reports
> https://issues.apache.org/jira/browse/SHINDIG-1674
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java
1231912 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java
1231912 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java
1231912 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java
1231912 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java
1231912 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java
1231912 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java
1231912 
> 
> Diff: https://reviews.apache.org/r/3544/diff
> 
> 
> Testing
> -------
> 
> tests are in the patch
> 
> 
> Thanks,
> 
> Evgeny
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message