shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Evgeny Bogdanov" <evgeny.bogda...@epfl.ch>
Subject Re: Review Request: A patch that enables API for updating person data
Date Mon, 23 Jan 2012 08:56:51 GMT


> On 2012-01-20 16:35:55, Stanton Sievers wrote:
> > 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.

Thanks Stanton for your points.
> 1. What's to stop one person from updating another person?
I also had this question, but then I thought that maybe it depends on container,
because anyways it's done in PersonServiceDb which is container specific normally.
I'll start a discussion on it in the spec mailing list.

> 2. osapi.people.update()
it's true, it is not in the spec, only REST api, should be added I believe.
Once one implements the rest api in shindig, javascript api is automatically available.
I m not sure whether it should be tested or not, since this code was not written by me :)


- Evgeny


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


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