You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Andreas Kohn <an...@gmail.com> on 2013/12/03 12:05:27 UTC
Review Request 15960: Fix JsonDbOpensocialService#createMessage()
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15960/
-----------------------------------------------------------
Review request for shindig.
Bugs: SHINDIG-1960
https://issues.apache.org/jira/browse/SHINDIG-1960
Repository: shindig
Description
-------
Fix #createMessage():
* assign a random message id if it wasn't done from the outside (no checks for uniqueness are done if one is given)
* properly access the json data
Note: this contains the functional fixes from https://reviews.apache.org/r/15420/ / SHINDIG-1951 without the method signature changes.
Diffs
-----
trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1547331
trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1547331
Diff: https://reviews.apache.org/r/15960/diff/
Testing
-------
Unit test exercises both changed parts.
Thanks,
Andreas Kohn
Re: Review Request 15960: Fix JsonDbOpensocialService#createMessage()
Posted by Andreas Kohn <an...@gmail.com>.
> On Dec. 4, 2013, 1:39 p.m., Stanton Sievers wrote:
> > trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java, line 626
> > <https://reviews.apache.org/r/15960/diff/1/?file=393003#file393003line626>
> >
> > Is it possible for getJSONObject(recipient) to be null?
Interesting question.
For the canonicaldb.json/JsonDbOpensocialService there's no method for creating new persons, so I could argue that that "creation of new persons" is responsible for setting up all data for that person, including any and all related structures such as message collections.
The same in a way is also true for the message collection part: the service doesn't support creating them.
Spec-wise (2.5.1): creating a message is not actually taking an arbitrary collection, but rather only "@outbox". The message itself can however contain an arbitrary number of collections it is "contained in", and collections are user specific. I've not found anything that would describe what should happen to created messages (should the sender be able to find it back by doing a GET on the @outbox collection?), or how failures in sending them are to be handled.
As such I'd rather avoid too much change here, mixing "better implementation of the standard" with "making things at least work with the current structure".
- Andreas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15960/#review29724
-----------------------------------------------------------
On Dec. 4, 2013, 3:53 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15960/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2013, 3:53 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1960
> https://issues.apache.org/jira/browse/SHINDIG-1960
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Fix #createMessage():
> * assign a random message id if it wasn't done from the outside (no checks for uniqueness are done if one is given)
> * properly access the json data
>
> Note: this contains the functional fixes from https://reviews.apache.org/r/15420/ / SHINDIG-1951 without the method signature changes.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1547796
> trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1547796
>
> Diff: https://reviews.apache.org/r/15960/diff/
>
>
> Testing
> -------
>
> Unit test exercises both changed parts.
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15960: Fix JsonDbOpensocialService#createMessage()
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15960/#review29724
-----------------------------------------------------------
trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java
<https://reviews.apache.org/r/15960/#comment57184>
Is it possible for getJSONObject(recipient) to be null?
trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java
<https://reviews.apache.org/r/15960/#comment57183>
"messages" should be a CONSTANT
trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java
<https://reviews.apache.org/r/15960/#comment57182>
You should do this conversion once outside of the recipients loop.
- Stanton Sievers
On Dec. 3, 2013, 11:05 a.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15960/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2013, 11:05 a.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1960
> https://issues.apache.org/jira/browse/SHINDIG-1960
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Fix #createMessage():
> * assign a random message id if it wasn't done from the outside (no checks for uniqueness are done if one is given)
> * properly access the json data
>
> Note: this contains the functional fixes from https://reviews.apache.org/r/15420/ / SHINDIG-1951 without the method signature changes.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1547331
> trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1547331
>
> Diff: https://reviews.apache.org/r/15960/diff/
>
>
> Testing
> -------
>
> Unit test exercises both changed parts.
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15960: Fix JsonDbOpensocialService#createMessage()
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15960/#review29735
-----------------------------------------------------------
Ship it!
Ship It!
- Stanton Sievers
On Dec. 4, 2013, 2:53 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15960/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2013, 2:53 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1960
> https://issues.apache.org/jira/browse/SHINDIG-1960
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Fix #createMessage():
> * assign a random message id if it wasn't done from the outside (no checks for uniqueness are done if one is given)
> * properly access the json data
>
> Note: this contains the functional fixes from https://reviews.apache.org/r/15420/ / SHINDIG-1951 without the method signature changes.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1547796
> trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1547796
>
> Diff: https://reviews.apache.org/r/15960/diff/
>
>
> Testing
> -------
>
> Unit test exercises both changed parts.
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15960: Fix JsonDbOpensocialService#createMessage()
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15960/#review29939
-----------------------------------------------------------
Ship it!
Ship It!
- Ryan Baxter
On Dec. 4, 2013, 2:53 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15960/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2013, 2:53 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1960
> https://issues.apache.org/jira/browse/SHINDIG-1960
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Fix #createMessage():
> * assign a random message id if it wasn't done from the outside (no checks for uniqueness are done if one is given)
> * properly access the json data
>
> Note: this contains the functional fixes from https://reviews.apache.org/r/15420/ / SHINDIG-1951 without the method signature changes.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1547796
> trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1547796
>
> Diff: https://reviews.apache.org/r/15960/diff/
>
>
> Testing
> -------
>
> Unit test exercises both changed parts.
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15960: Fix JsonDbOpensocialService#createMessage()
Posted by Andreas Kohn <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15960/
-----------------------------------------------------------
(Updated Dec. 4, 2013, 3:53 p.m.)
Review request for shindig.
Changes
-------
* add a constant for 'messages'
* pull common code out of the loop
* update javadoc for MESSAGE_TABLE to match comment in canonicaldb.json
Bugs: SHINDIG-1960
https://issues.apache.org/jira/browse/SHINDIG-1960
Repository: shindig
Description
-------
Fix #createMessage():
* assign a random message id if it wasn't done from the outside (no checks for uniqueness are done if one is given)
* properly access the json data
Note: this contains the functional fixes from https://reviews.apache.org/r/15420/ / SHINDIG-1951 without the method signature changes.
Diffs (updated)
-----
trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java 1547796
trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1547796
Diff: https://reviews.apache.org/r/15960/diff/
Testing
-------
Unit test exercises both changed parts.
Thanks,
Andreas Kohn