You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Evgeny Bogdanov <ev...@gmail.com> on 2012/01/19 10:30:25 UTC
Review Request: A patch that enables API for updating person data
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/
-----------------------------------------------------------
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
Re: Review Request: A patch that enables API for updating person data
Posted by Dan Dumont <dd...@us.ibm.com>.
> 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.
>
> Evgeny Bogdanov wrote:
> 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 :)
>
> Stanton Sievers wrote:
> I had some offline conversations about #1 and I think you are right that it depends on the container. For instance, to enable administration oriented use cases, where you have an admin with elevated privileges, you would need to allow one person to update another in the server code. The PersonServiceDb would have to handle the rights management.
>
> Re #2. If you don't mind writing up the tests I think they would be useful. It's reassuring (at least to me :) to have explicit tests for code like that as a place where we can point and say "yes, osapi.people.update()" works, and say it with a greater level of confidence. This doesn't seem to be the case with the osapi.people.get, et al, however... I can't find explicit feature code tests for it. Maybe this is a larger question for the whole dev list.
>
> Evgeny Bogdanov wrote:
> well, it seems as osapi.people.get, etc. are tested.
> 1) in peopleTest.xml file
> 2) in peoplesuite.js (suite.xml)
>
> I don't know if they run automatically with mvn, but at least they can be open as a gadget that will run tests
> I will add those tests
I think I'd like to see the default shindig impl only allow users to edit their own data with this api. I'd also like to see a place where container implementations can override this behavior easily. Where would they do that today, inside the entiyManager?
- Dan
-----------------------------------------------------------
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
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Evgeny Bogdanov <ev...@epfl.ch>.
> 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.
>
> Evgeny Bogdanov wrote:
> 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 :)
>
> Stanton Sievers wrote:
> I had some offline conversations about #1 and I think you are right that it depends on the container. For instance, to enable administration oriented use cases, where you have an admin with elevated privileges, you would need to allow one person to update another in the server code. The PersonServiceDb would have to handle the rights management.
>
> Re #2. If you don't mind writing up the tests I think they would be useful. It's reassuring (at least to me :) to have explicit tests for code like that as a place where we can point and say "yes, osapi.people.update()" works, and say it with a greater level of confidence. This doesn't seem to be the case with the osapi.people.get, et al, however... I can't find explicit feature code tests for it. Maybe this is a larger question for the whole dev list.
well, it seems as osapi.people.get, etc. are tested.
1) in peopleTest.xml file
2) in peoplesuite.js (suite.xml)
I don't know if they run automatically with mvn, but at least they can be open as a gadget that will run tests
I will add those tests
- 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
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Ryan Baxter <rb...@gmail.com>.
> 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.
>
> Evgeny Bogdanov wrote:
> 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 :)
>
> Stanton Sievers wrote:
> I had some offline conversations about #1 and I think you are right that it depends on the container. For instance, to enable administration oriented use cases, where you have an admin with elevated privileges, you would need to allow one person to update another in the server code. The PersonServiceDb would have to handle the rights management.
>
> Re #2. If you don't mind writing up the tests I think they would be useful. It's reassuring (at least to me :) to have explicit tests for code like that as a place where we can point and say "yes, osapi.people.update()" works, and say it with a greater level of confidence. This doesn't seem to be the case with the osapi.people.get, et al, however... I can't find explicit feature code tests for it. Maybe this is a larger question for the whole dev list.
>
> Evgeny Bogdanov wrote:
> well, it seems as osapi.people.get, etc. are tested.
> 1) in peopleTest.xml file
> 2) in peoplesuite.js (suite.xml)
>
> I don't know if they run automatically with mvn, but at least they can be open as a gadget that will run tests
> I will add those tests
>
> Dan Dumont wrote:
> I think I'd like to see the default shindig impl only allow users to edit their own data with this api. I'd also like to see a place where container implementations can override this behavior easily. Where would they do that today, inside the entiyManager?
RE: Tests
I believe at least the peopleTest.xml is run during the MVN build, not sure about peoplesuite.js.
+1 on updating the tests.
- Ryan
-----------------------------------------------------------
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
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Stanton Sievers <si...@gmail.com>.
> 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.
>
> Evgeny Bogdanov wrote:
> 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 :)
I had some offline conversations about #1 and I think you are right that it depends on the container. For instance, to enable administration oriented use cases, where you have an admin with elevated privileges, you would need to allow one person to update another in the server code. The PersonServiceDb would have to handle the rights management.
Re #2. If you don't mind writing up the tests I think they would be useful. It's reassuring (at least to me :) to have explicit tests for code like that as a place where we can point and say "yes, osapi.people.update()" works, and say it with a greater level of confidence. This doesn't seem to be the case with the osapi.people.get, et al, however... I can't find explicit feature code tests for it. Maybe this is a larger question for the whole dev list.
- Stanton
-----------------------------------------------------------
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
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Evgeny Bogdanov <ev...@epfl.ch>.
> 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
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/#review4769
-----------------------------------------------------------
See inline comments.
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java
<https://reviews.apache.org/r/3544/#comment10491>
Is the intent here to punish someone who is trying to update someone else's data? :)
Scenario:
Viewer = john.doe
POST to rpc endpoint
[
{
"method":"people.update",
"id":"people.update",
"params":{
"person":{
"displayName":"Foo Bar"
},
"userId":"jane.doe"
}
}
]
POST to rpc endpoint
[
{
"method":"people.get",
"id":"people.get",
"params":{
"fields":[
"name",
"displayName"
],
"userId":"@viewer",
"groupId":"@self"
}
}
]
Result: John Doe's display name is now "Foo Bar"!
My suggestion is that you create a protected method in PersonServiceDb to check if a person is allowed to update the given person record. By default this method would return true iff the viewer and person id are the same. If the method returned false, we would want to return a 403 or the like. Someone could then override this to provide the admin functionality we've discussed previously.
- Stanton
On 2012-02-02 10:53:21, Evgeny Bogdanov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/
> -----------------------------------------------------------
>
> (Updated 2012-02-02 10:53:21)
>
>
> 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 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1239531
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1239531
>
> Diff: https://reviews.apache.org/r/3544/diff
>
>
> Testing
> -------
>
> tests are in the patch
>
>
> Thanks,
>
> Evgeny
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Paul Lindner <li...@inuus.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/#review4864
-----------------------------------------------------------
Ship it!
lgtm -- modulo trailing whitespace.
- Paul
On 2012-02-07 10:42:50, Evgeny Bogdanov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/
> -----------------------------------------------------------
>
> (Updated 2012-02-07 10:42:50)
>
>
> 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 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
>
> Diff: https://reviews.apache.org/r/3544/diff
>
>
> Testing
> -------
>
> tests are in the patch
>
>
> Thanks,
>
> Evgeny
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/#review4862
-----------------------------------------------------------
Ship it!
Fix the whitespace and you are good to go!
- Ryan
On 2012-02-07 10:42:50, Evgeny Bogdanov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/
> -----------------------------------------------------------
>
> (Updated 2012-02-07 10:42:50)
>
>
> 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 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
>
> Diff: https://reviews.apache.org/r/3544/diff
>
>
> Testing
> -------
>
> tests are in the patch
>
>
> Thanks,
>
> Evgeny
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/#review4898
-----------------------------------------------------------
Regarding whitespace, you should not have any trailing whitespace or whitespace on empty lines. This will show up in red in the diffs on Review Board.
You can look here (https://cwiki.apache.org/SHINDIGxSITE/java-style.html) and (https://cwiki.apache.org/confluence/display/SHINDIGxSITE/Javascript+Style) for style guidelines. If you use Eclipse, you can find resources that can be imported to configure the Eclipse formatters in Shindig trunk /etc/eclipse.
I know Dan removes whitespace this way:
"FYI:
In eclipse, Ctrl+F
Find: [ \t]+$
Replace with:
Check off "Regular expressions" and then press "Replace All" :)"
- Stanton
On 2012-02-08 10:27:44, Evgeny Bogdanov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/
> -----------------------------------------------------------
>
> (Updated 2012-02-08 10:27:44)
>
>
> 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 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
>
> Diff: https://reviews.apache.org/r/3544/diff
>
>
> Testing
> -------
>
> tests are in the patch
>
>
> Thanks,
>
> Evgeny
>
>
Re: Review Request: A patch that enables API for updating person
data
Posted by Evgeny Bogdanov <ev...@epfl.ch>.
All done now.
Best
Evgeny
On 08.02.12 22:52, Stanton Sievers wrote:
> Beat me to it! I just remembered I forgot that step.
>
> Evgeny, can you also close the review if you haven't done so already.
>
>
> Thanks,
> -Stanton
>
>
>
> From: Henry Saputra<he...@gmail.com>
> To: dev@shindig.apache.org,
> Date: 02/08/2012 16:49
> Subject: Re: Review Request: A patch that enables API for updating
> person data
>
>
>
> Evgeny,
>
> Could you upload the final patch to JIRA for legal purpose?
>
> - Henry
>
> On Wed, Feb 8, 2012 at 5:26 AM, Stanton Sievers<si...@gmail.com>
> wrote:
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/3544/#review4900
>> -----------------------------------------------------------
>>
>> Ship it!
>>
>>
>> Committed r1241890
>>
>> Thanks!
>>
>> - Stanton
>>
>>
>> On 2012-02-08 12:49:18, Evgeny Bogdanov wrote:
>>> -----------------------------------------------------------
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/3544/
>>> -----------------------------------------------------------
>>>
>>> (Updated 2012-02-08 12:49:18)
>>>
>>>
>>> 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/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java
> 1241411
>>>
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java
> 1241411
>>> Diff: https://reviews.apache.org/r/3544/diff
>>>
>>>
>>> Testing
>>> -------
>>>
>>> tests are in the patch
>>>
>>>
>>> Thanks,
>>>
>>> Evgeny
>>>
>>>
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Stanton Sievers <ss...@us.ibm.com>.
Beat me to it! I just remembered I forgot that step.
Evgeny, can you also close the review if you haven't done so already.
Thanks,
-Stanton
From: Henry Saputra <he...@gmail.com>
To: dev@shindig.apache.org,
Date: 02/08/2012 16:49
Subject: Re: Review Request: A patch that enables API for updating
person data
Evgeny,
Could you upload the final patch to JIRA for legal purpose?
- Henry
On Wed, Feb 8, 2012 at 5:26 AM, Stanton Sievers <si...@gmail.com>
wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/#review4900
> -----------------------------------------------------------
>
> Ship it!
>
>
> Committed r1241890
>
> Thanks!
>
> - Stanton
>
>
> On 2012-02-08 12:49:18, Evgeny Bogdanov wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/3544/
>> -----------------------------------------------------------
>>
>> (Updated 2012-02-08 12:49:18)
>>
>>
>> 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/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java
1241411
>>
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java
1241411
>>
>> Diff: https://reviews.apache.org/r/3544/diff
>>
>>
>> Testing
>> -------
>>
>> tests are in the patch
>>
>>
>> Thanks,
>>
>> Evgeny
>>
>>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Henry Saputra <he...@gmail.com>.
Evgeny,
Could you upload the final patch to JIRA for legal purpose?
- Henry
On Wed, Feb 8, 2012 at 5:26 AM, Stanton Sievers <si...@gmail.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/#review4900
> -----------------------------------------------------------
>
> Ship it!
>
>
> Committed r1241890
>
> Thanks!
>
> - Stanton
>
>
> On 2012-02-08 12:49:18, Evgeny Bogdanov wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/3544/
>> -----------------------------------------------------------
>>
>> (Updated 2012-02-08 12:49:18)
>>
>>
>> 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/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
>> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java 1241411
>>
>> Diff: https://reviews.apache.org/r/3544/diff
>>
>>
>> Testing
>> -------
>>
>> tests are in the patch
>>
>>
>> Thanks,
>>
>> Evgeny
>>
>>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/#review4900
-----------------------------------------------------------
Ship it!
Committed r1241890
Thanks!
- Stanton
On 2012-02-08 12:49:18, Evgeny Bogdanov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/
> -----------------------------------------------------------
>
> (Updated 2012-02-08 12:49:18)
>
>
> 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/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java 1241411
>
> Diff: https://reviews.apache.org/r/3544/diff
>
>
> Testing
> -------
>
> tests are in the patch
>
>
> Thanks,
>
> Evgeny
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Evgeny Bogdanov <ev...@epfl.ch>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/
-----------------------------------------------------------
(Updated 2012-02-08 12:49:18.692350)
Review request for shindig.
Changes
-------
All trailing whitespaces are removed.
Please ship it whoever has committing access to 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 (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java 1241411
Diff: https://reviews.apache.org/r/3544/diff
Testing
-------
tests are in the patch
Thanks,
Evgeny
Re: Review Request: A patch that enables API for updating person data
Posted by Evgeny Bogdanov <ev...@epfl.ch>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/
-----------------------------------------------------------
(Updated 2012-02-08 10:27:44.989959)
Review request for shindig.
Changes
-------
Whitespaces in the end of the line with text are removed.
What about empty lines? Should there be a whitespace aligned with the beginning of the previous line with text?
I checked other shindig code, there are both variants present?
What's the rule?
How is doing shipping ? :)
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 (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
Diff: https://reviews.apache.org/r/3544/diff
Testing
-------
tests are in the patch
Thanks,
Evgeny
Re: Review Request: A patch that enables API for updating person data
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/#review4858
-----------------------------------------------------------
Ship it!
Looks like some whitespace is still popping up, but other than that this looks good to me. Thanks!
- Stanton
On 2012-02-07 10:42:50, Evgeny Bogdanov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3544/
> -----------------------------------------------------------
>
> (Updated 2012-02-07 10:42:50)
>
>
> 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 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
>
> Diff: https://reviews.apache.org/r/3544/diff
>
>
> Testing
> -------
>
> tests are in the patch
>
>
> Thanks,
>
> Evgeny
>
>
Re: Review Request: A patch that enables API for updating person data
Posted by Evgeny Bogdanov <ev...@epfl.ch>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/
-----------------------------------------------------------
(Updated 2012-02-07 10:42:50.013242)
Review request for shindig.
Changes
-------
2Stanton: Bad behavior should be punished! :)
Attached the patch that has implementation of your suggestion + 3 tests for it (client side, personservicedb and jsonservicedb).
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 (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1241411
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1241411
Diff: https://reviews.apache.org/r/3544/diff
Testing
-------
tests are in the patch
Thanks,
Evgeny
Re: Review Request: A patch that enables API for updating person data
Posted by Evgeny Bogdanov <ev...@epfl.ch>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3544/
-----------------------------------------------------------
(Updated 2012-02-02 10:53:21.630237)
Review request for shindig.
Changes
-------
New patch is applied.
As discussed, by default only the viewer can update his own info.
The containers can override this behavior in PersonServiceDb.java
For example,
String uid = id.getUserId(token);
instead of
String uid = token.getViewerId();
Test for osapi.people.update is added as well
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 (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDbTest.java 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/resources/endtoend/osapi/personTest.xml 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/PersonHandler.java 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/PersonService.java 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/service/PersonHandlerTest.java 1239531
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1239531
Diff: https://reviews.apache.org/r/3544/diff
Testing
-------
tests are in the patch
Thanks,
Evgeny