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/11/11 10:43:40 UTC
Review Request 15402: Call the correct MessageService method when no message
ids are specified in a DELETE request on the MessageHandler.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/
-----------------------------------------------------------
Review request for shindig.
Bugs: SHINDIG-1943
https://issues.apache.org/jira/browse/SHINDIG-1943
Repository: shindig
Description
-------
Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
Diffs
-----
trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
Diff: https://reviews.apache.org/r/15402/diff/
Testing
-------
Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
Thanks,
Andreas Kohn
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/#review28732
-----------------------------------------------------------
Ship it!
Andreas, I agree with your interpretation of how DELETE should work given how Message-Collection-ID is used for GET and PUT. Because the spec is not clear, I think following the same pattern as GET and PUT is reasonable.
- Stanton Sievers
On Nov. 11, 2013, 2:36 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15402/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 2:36 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1946
> https://issues.apache.org/jira/browse/SHINDIG-1946
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
>
> Diff: https://reviews.apache.org/r/15402/diff/
>
>
> Testing
> -------
>
> Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Andreas Kohn <an...@gmail.com>.
> On Nov. 12, 2013, 12:17 a.m., Ryan Baxter wrote:
> > I don't see in the spec where it says that passing an empty message collection should delete all messages.
I'm not sure I'm following what you're saying here.
The current code does not allow to be called without a message-collection-id (even though OS 2.5.1 says "optional", but the spec is horribly unclear what that would actually mean), and the patch isn't about 'empty message collection should delete all messages', but rather 'if no message-id is specified, then delete the collection with the name'.
Again, the spec is unclear, and I think this should also be addressed on their side (currently working out how to send patches over there ...), but, my current reading is that:
1. DELETE <api>/messages/<user>/@self/<collection>/<message> would delete that specific message in the collection
2. DELETE <api>/messages/<user>/@self/<collection>/<multiple message ids> is no longer available in 2.5.1 (but shindig still can handle multiple message ids)
3. DELETE <api>/messages/<user>/@self/<collection> would delete the collection (no message id provided).
The patch purely adds the support for the third case in the MessageHandler, putting the burden on the MessageService to implement this optional support for deleting message collections if it so intends.
- Andreas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/#review28712
-----------------------------------------------------------
On Nov. 11, 2013, 2:36 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15402/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 2:36 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1946
> https://issues.apache.org/jira/browse/SHINDIG-1946
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
>
> Diff: https://reviews.apache.org/r/15402/diff/
>
>
> Testing
> -------
>
> Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Ryan Baxter <rb...@gmail.com>.
> On Nov. 12, 2013, 12:17 a.m., Ryan Baxter wrote:
> > I don't see in the spec where it says that passing an empty message collection should delete all messages.
>
> Andreas Kohn wrote:
> I'm not sure I'm following what you're saying here.
>
> The current code does not allow to be called without a message-collection-id (even though OS 2.5.1 says "optional", but the spec is horribly unclear what that would actually mean), and the patch isn't about 'empty message collection should delete all messages', but rather 'if no message-id is specified, then delete the collection with the name'.
>
> Again, the spec is unclear, and I think this should also be addressed on their side (currently working out how to send patches over there ...), but, my current reading is that:
> 1. DELETE <api>/messages/<user>/@self/<collection>/<message> would delete that specific message in the collection
> 2. DELETE <api>/messages/<user>/@self/<collection>/<multiple message ids> is no longer available in 2.5.1 (but shindig still can handle multiple message ids)
> 3. DELETE <api>/messages/<user>/@self/<collection> would delete the collection (no message id provided).
>
> The patch purely adds the support for the third case in the MessageHandler, putting the burden on the MessageService to implement this optional support for deleting message collections if it so intends.
Ah OK makes more sense now. Thanks!
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/#review28712
-----------------------------------------------------------
On Nov. 11, 2013, 2:36 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15402/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 2:36 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1946
> https://issues.apache.org/jira/browse/SHINDIG-1946
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
>
> Diff: https://reviews.apache.org/r/15402/diff/
>
>
> Testing
> -------
>
> Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/#review28712
-----------------------------------------------------------
I don't see in the spec where it says that passing an empty message collection should delete all messages.
- Ryan Baxter
On Nov. 11, 2013, 2:36 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15402/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 2:36 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1946
> https://issues.apache.org/jira/browse/SHINDIG-1946
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
>
> Diff: https://reviews.apache.org/r/15402/diff/
>
>
> Testing
> -------
>
> Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/#review28737
-----------------------------------------------------------
Ship it!
Ship It!
- Ryan Baxter
On Nov. 11, 2013, 2:36 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15402/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 2:36 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1946
> https://issues.apache.org/jira/browse/SHINDIG-1946
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
>
> Diff: https://reviews.apache.org/r/15402/diff/
>
>
> Testing
> -------
>
> Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/#review29107
-----------------------------------------------------------
Ship it!
Committed revision 1543403. Please close this review.
- Stanton Sievers
On Nov. 11, 2013, 2:36 p.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15402/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 2:36 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1946
> https://issues.apache.org/jira/browse/SHINDIG-1946
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
>
> Diff: https://reviews.apache.org/r/15402/diff/
>
>
> Testing
> -------
>
> Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
>
>
> Thanks,
>
> Andreas Kohn
>
>
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Andreas Kohn <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/
-----------------------------------------------------------
(Updated Nov. 11, 2013, 2:36 p.m.)
Review request for shindig.
Changes
-------
Indeed, should have been https://issues.apache.org/jira/browse/SHINDIG-1946
Bugs: SHINDIG-1946
https://issues.apache.org/jira/browse/SHINDIG-1946
Repository: shindig
Description
-------
Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
Diffs
-----
trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
Diff: https://reviews.apache.org/r/15402/diff/
Testing
-------
Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
Thanks,
Andreas Kohn
Re: Review Request 15402: Call the correct MessageService method when no
message ids are specified in a DELETE request on the MessageHandler.
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15402/#review28651
-----------------------------------------------------------
I think this is pointing to the wrong JIRA. Can you double-check please?
- Stanton Sievers
On Nov. 11, 2013, 9:43 a.m., Andreas Kohn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15402/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 9:43 a.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1943
> https://issues.apache.org/jira/browse/SHINDIG-1943
>
>
> Repository: shindig
>
>
> Description
> -------
>
> Call MessageService#deleteMessageCollection() from MessageHandler#delete() when no message ids where specified.
>
>
> Diffs
> -----
>
> trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/MessageHandler.java 1540645
>
> Diff: https://reviews.apache.org/r/15402/diff/
>
>
> Testing
> -------
>
> Call method via REST, find that in the default sample application exception about "unsupported" appears, rather than "no message ids specified."
>
>
> Thanks,
>
> Andreas Kohn
>
>