> 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 > >
