You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Rodrigo Gallardo <rg...@google.com> on 2008/09/24 02:44:33 UTC
A first stab at Message support
I have started writing a first implementation of Message posting.
As this is my first foray into shindig I would very much like to get
some early reviews on it. I will post the patch for final consideration
after I determine how to include the functionality in the sample server.
http://codereview.appspot.com/5702
Re: A first stab at Message support
Posted by Ropu <ro...@gmail.com>.
Hi Dan
i've just sent the proposal the the spec list
http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/12bd4b524169618a
regards
ropu
On Sun, Sep 28, 2008 at 10:35 PM, Dan Peterson <dp...@google.com> wrote:
> Ropu, can you drop the spec proposal to the spec list?
> -Dan
>
> On Fri, Sep 26, 2008 at 12:57 PM, Ropu <ro...@gmail.com> wrote:
>
> > +1 to add the field in the Messaging REST API spec...
> >
> > is a small change to avoid multiple implementations and move from the
> > standard
> >
> > ropu
> >
> > On Fri, Sep 26, 2008 at 3:21 PM, Rodrigo Gallardo <rgallardo@google.com
> > >wrote:
> >
> > > On Fri, Sep 26, 2008 at 01:28:10PM -0400, Bobby Bissett wrote:
> > > > Hi Rodrigo and all,
> > > >
> > > > I've attached my simple jsonmessage.js file (mostly it just adds a
> > > > toJsonObject method) and related diffs.
> > > > ...
> > > > org.apache.shindig.social.opensocial.model.Message
> > > > - This is a minor change, but it's an API change so it's a bigger
> > > > deal. To match the rest/rpc proposal, I added a list of recipients to
> > > > the class so that they can be pulled out in the message handler.
> > >
> > > I also lean towards this solution. Given how nothing inside shindig is
> > > actualy using that part of the API, it should not be a very painful
> > > change.
> > >
> > > > I see
> > > > that's a todo in your handler; the recipients could be another param
> > > > in the post body, but I don't think that works well in the rest case
> > > > without clashing with the messaging URI template.
> > >
> > > The whole rest code assumes a single argument payload in the HTTP body,
> > > so doing this clashes completely with it all.
> > >
> > > > One other issue is that, when returning a message to the client,
> there
> > > > needs to be some way to include the sender information. Another field
> > > > could be added to Message, or this could be handled in an impl-
> > > > specific way.
> > >
> > > I'd add another field. Even thought getting messages is not part of
> this
> > > spec we should try to get a common way to do it. maybe we should even
> > > propose to add it to the API.
> > >
> >
> >
> >
> > --
> > .-. --- .--. ..-
> > R o p u
> >
>
--
.-. --- .--. ..-
R o p u
Re: A first stab at Message support
Posted by Dan Peterson <dp...@google.com>.
Ropu, can you drop the spec proposal to the spec list?
-Dan
On Fri, Sep 26, 2008 at 12:57 PM, Ropu <ro...@gmail.com> wrote:
> +1 to add the field in the Messaging REST API spec...
>
> is a small change to avoid multiple implementations and move from the
> standard
>
> ropu
>
> On Fri, Sep 26, 2008 at 3:21 PM, Rodrigo Gallardo <rgallardo@google.com
> >wrote:
>
> > On Fri, Sep 26, 2008 at 01:28:10PM -0400, Bobby Bissett wrote:
> > > Hi Rodrigo and all,
> > >
> > > I've attached my simple jsonmessage.js file (mostly it just adds a
> > > toJsonObject method) and related diffs.
> > > ...
> > > org.apache.shindig.social.opensocial.model.Message
> > > - This is a minor change, but it's an API change so it's a bigger
> > > deal. To match the rest/rpc proposal, I added a list of recipients to
> > > the class so that they can be pulled out in the message handler.
> >
> > I also lean towards this solution. Given how nothing inside shindig is
> > actualy using that part of the API, it should not be a very painful
> > change.
> >
> > > I see
> > > that's a todo in your handler; the recipients could be another param
> > > in the post body, but I don't think that works well in the rest case
> > > without clashing with the messaging URI template.
> >
> > The whole rest code assumes a single argument payload in the HTTP body,
> > so doing this clashes completely with it all.
> >
> > > One other issue is that, when returning a message to the client, there
> > > needs to be some way to include the sender information. Another field
> > > could be added to Message, or this could be handled in an impl-
> > > specific way.
> >
> > I'd add another field. Even thought getting messages is not part of this
> > spec we should try to get a common way to do it. maybe we should even
> > propose to add it to the API.
> >
>
>
>
> --
> .-. --- .--. ..-
> R o p u
>
Re: A first stab at Message support
Posted by Ropu <ro...@gmail.com>.
+1 to add the field in the Messaging REST API spec...
is a small change to avoid multiple implementations and move from the
standard
ropu
On Fri, Sep 26, 2008 at 3:21 PM, Rodrigo Gallardo <rg...@google.com>wrote:
> On Fri, Sep 26, 2008 at 01:28:10PM -0400, Bobby Bissett wrote:
> > Hi Rodrigo and all,
> >
> > I've attached my simple jsonmessage.js file (mostly it just adds a
> > toJsonObject method) and related diffs.
> > ...
> > org.apache.shindig.social.opensocial.model.Message
> > - This is a minor change, but it's an API change so it's a bigger
> > deal. To match the rest/rpc proposal, I added a list of recipients to
> > the class so that they can be pulled out in the message handler.
>
> I also lean towards this solution. Given how nothing inside shindig is
> actualy using that part of the API, it should not be a very painful
> change.
>
> > I see
> > that's a todo in your handler; the recipients could be another param
> > in the post body, but I don't think that works well in the rest case
> > without clashing with the messaging URI template.
>
> The whole rest code assumes a single argument payload in the HTTP body,
> so doing this clashes completely with it all.
>
> > One other issue is that, when returning a message to the client, there
> > needs to be some way to include the sender information. Another field
> > could be added to Message, or this could be handled in an impl-
> > specific way.
>
> I'd add another field. Even thought getting messages is not part of this
> spec we should try to get a common way to do it. maybe we should even
> propose to add it to the API.
>
--
.-. --- .--. ..-
R o p u
Re: A first stab at Message support
Posted by Rodrigo Gallardo <rg...@google.com>.
On Fri, Sep 26, 2008 at 01:28:10PM -0400, Bobby Bissett wrote:
> Hi Rodrigo and all,
>
> I've attached my simple jsonmessage.js file (mostly it just adds a
> toJsonObject method) and related diffs.
> ...
> org.apache.shindig.social.opensocial.model.Message
> - This is a minor change, but it's an API change so it's a bigger
> deal. To match the rest/rpc proposal, I added a list of recipients to
> the class so that they can be pulled out in the message handler.
I also lean towards this solution. Given how nothing inside shindig is
actualy using that part of the API, it should not be a very painful
change.
> I see
> that's a todo in your handler; the recipients could be another param
> in the post body, but I don't think that works well in the rest case
> without clashing with the messaging URI template.
The whole rest code assumes a single argument payload in the HTTP body,
so doing this clashes completely with it all.
> One other issue is that, when returning a message to the client, there
> needs to be some way to include the sender information. Another field
> could be added to Message, or this could be handled in an impl-
> specific way.
I'd add another field. Even thought getting messages is not part of this
spec we should try to get a common way to do it. maybe we should even
propose to add it to the API.
Re: A first stab at Message support
Posted by Rodrigo Gallardo <ro...@nul-unu.com>.
On Mon, Sep 29, 2008 at 12:36:02PM -0400, Bobby Bissett wrote:
> Got to it sooner than I expected. https://issues.apache.org/jira/browse/SHINDIG-629
>
> On Sep 29, 2008, at 12:17 PM, Bobby Bissett wrote:
>
>>> Could you attach this to file to the/a Jira and Grant ASF a license
>>> to it, it keeps a track of a) the contributions and b) the grant of
>>> license to ASF.
>>
>> Will do. I'll create a new issue and reference Rodrigo's issue for
>> adding Message support.
My server side code, depending on Bobby's changes, with passing tests but without
(as yet) the sample server's implementation:
http://issues.apache.org/jira/browse/SHINDIG-646
Re: A first stab at Message support
Posted by Bobby Bissett <Ro...@Sun.COM>.
Got to it sooner than I expected. https://issues.apache.org/jira/browse/SHINDIG-629
On Sep 29, 2008, at 12:17 PM, Bobby Bissett wrote:
>> Could you attach this to file to the/a Jira and Grant ASF a license
>> to it, it keeps a track of a) the contributions and b) the grant of
>> license to ASF.
>
> Will do. I'll create a new issue and reference Rodrigo's issue for
> adding Message support.
>
> Cheers,
> Bobby
>
>
Re: A first stab at Message support
Posted by Bobby Bissett <Ro...@Sun.COM>.
> Could you attach this to file to the/a Jira and Grant ASF a license
> to it, it keeps a track of a) the contributions and b) the grant of
> license to ASF.
Will do. I'll create a new issue and reference Rodrigo's issue for
adding Message support.
Cheers,
Bobby
Re: A first stab at Message support
Posted by Ian Boston <ie...@tfd.co.uk>.
Bobby,
Could you attach this to file to the/a Jira and Grant ASF a license
to it, it keeps a track of a) the contributions and b) the grant of
license to ASF.
It also avoids the pain of filtering mail servers.
Thanks
Ian
On 26 Sep 2008, at 13:37, Bobby Bissett wrote:
>> I've attached my simple jsonmessage.js file (mostly it just adds a
>> toJsonObject method) and related diffs. With these changes, I
>> create a message in the gadget with:
>
> For readers whose mail servers, like mine, won't pass through *.js
> files, here's a zip of the javascript file and diff output.
>
> Cheers,
> Bobby
>
> <files.zip>
Re: A first stab at Message support
Posted by Bobby Bissett <Ro...@Sun.COM>.
> I've attached my simple jsonmessage.js file (mostly it just adds a
> toJsonObject method) and related diffs. With these changes, I create
> a message in the gadget with:
For readers whose mail servers, like mine, won't pass through *.js
files, here's a zip of the javascript file and diff output.
Cheers,
Bobby
Re: A first stab at Message support
Posted by Bobby Bissett <Ro...@Sun.COM>.
Hi Rodrigo and all,
I've attached my simple jsonmessage.js file (mostly it just adds a
toJsonObject method) and related diffs. With these changes, I create a
message in the gadget with:
var req = opensocial.newDataRequest();
var message = opensocial.newMessage(msg);
message.setField(opensocial.Message.Field.TYPE,
opensocial.Message.Type.PRIVATE_MESSAGE);
message.setField(opensocial.Message.Field.TITLE, subject);
// not part of api, but in RPC proposal
var recipients = [ tempToId ];
message.setField("recipients", recipients);
and create a JsonRpcRequestItem with a param:
rpc.params.message = message.toJsonObject();
On the server side, I can get the Message object (almost) in my
MessageHandler with:
Message incomingMessage =
reqitem.getTypedParameter("message", Message.class);
...except that I'm running into an error in the bean json converter
due to the list of recipients I've added (following the proposal to
add recipients in an array in the message request).
Here's a description of the diffs.
jsonrpccontainer.js
- Added newMessage() function in order to return the new jsonmessage
object.
feature.xml
- Added jsonmessage.js file so that it's loaded.
message.js
- Changed the text of the Type fields to match Message.java on the
server, otherwise the json could not be converted to a pojo.
org.apache.shindig.social.opensocial.model.Message
- This is a minor change, but it's an API change so it's a bigger
deal. To match the rest/rpc proposal, I added a list of recipients to
the class so that they can be pulled out in the message handler. I see
that's a todo in your handler; the recipients could be another param
in the post body, but I don't think that works well in the rest case
without clashing with the messaging URI template.
org.apache.shindig.social.core.model.MessageImpl
- Added List<String> of recipients to match interface. Just made the
minimum change to compile.
I hope some of this is helpful. That list of recipients is giving me
some grief in the converter, so I'll have to go with my workarounds
for now (e.g. treating the incoming messages as a simple JSONObject)
and can help more with this after you have a chance to look it over.
One other issue is that, when returning a message to the client, there
needs to be some way to include the sender information. Another field
could be added to Message, or this could be handled in an impl-
specific way.
Let me know please if you have any questions or would like me to try
something else. You can see the server-side code as I commit it here
(I hope to wrap up message support today, though with some workarounds):
https://socialsite.dev.java.net/source/browse/socialsite/trunk/src/java/com/sun/socialsite/web/rest/core/
Cheers,
Bobby
Re: A first stab at Message support
Posted by Rodrigo Gallardo <rg...@google.com>.
On Thu, Sep 25, 2008 at 04:47:02PM -0400, Bobby Bissett wrote:
> Hi Rodrigo,
>
> I'm also implementing message support for our project. We had support
> already that wasn't using Opensocial requests, so the work has been
> mainly switching to use a MessageHandler and proper request types. One
> thing that's currently missing in Shindig is the jsonmessage.js object
> for handling requests in the json over rpc case.
>
> I'm working on a basic jsonmessage implementation, mainly to add the
> toJsonObject() support since without it the message over the wire
> wraps the message fields in a 'fields_':{...} element. Would you like
> me to send you what I have once I have it working?
Yes, I would, thanks.
Re: A first stab at Message support
Posted by Bobby Bissett <Ro...@Sun.COM>.
Hi Rodrigo,
I'm also implementing message support for our project. We had support
already that wasn't using Opensocial requests, so the work has been
mainly switching to use a MessageHandler and proper request types. One
thing that's currently missing in Shindig is the jsonmessage.js object
for handling requests in the json over rpc case.
I'm working on a basic jsonmessage implementation, mainly to add the
toJsonObject() support since without it the message over the wire
wraps the message fields in a 'fields_':{...} element. Would you like
me to send you what I have once I have it working?
Cheers,
Bobby
Re: A first stab at Message support
Posted by Bobby Bissett <Ro...@Sun.COM>.
>> Another question. No matter how hard I look, I cannot find in the
>> spec a
>> description on how the post message method is suposed to get the
>> list of
>> recipients. It's not part of the proposed URI template (and I can't
>> see
>> how to fit it there) and it's not part of the post body either, as
>> that's supossed to be just the message itself. Am I missing something
>> obvious here?
>
>
> I also couldnt see anything, it might be something that is missing
> from the Spec, that only becomes apparent on implementation.
> Anyone ?
At least in the RPC proposal (sec 8.4):
http://docs.google.com/View?docid=dhjrqr8t_4cwzqq7gh
...the recipients are included in the message itself and not the URI.
I see that that field isn't in the message.js object, which may be an
omission or may be by design. In our implementation, I'm just setting
it manually as a field on the message.
Cheers,
Bobby
Re: A first stab at Message support
Posted by Ian Boston <ie...@tfd.co.uk>.
On 24 Sep 2008, at 17:19, rodrigo.gallardo@gmail.com wrote:
> On Wed, Sep 24, 2008 at 04:53:35AM -0400, Ian Boston wrote:
>> Looks ok, but needs some javadoc and may need some SPI methods
>> outside
>> the Spec to make it meaningful. Depends on if we think Shindig
>> scope is
>> purely to the spec, or if it should go just far enough to be a
>> running
>> RI of OpenSocial.
>
> Working on the javadoc and the handler's test.
great :)
>
> About the scope, I would say having a reference implementation (I
> assume
> that's what you mean by RI) is way more useful.
yes RI == Reference Implementation
> I will implement some
> reading methods in the sample classes anyways, but I don't feel
> qualified yet to propose a SPI API.
Ok, I think the query mechanism in the other Services is a workable
pattern and if binds well to the REST interface.... but.... I am
almost certain that reading messages to present back to the user is a
container thing, and might be outside scope for use by a gadget
directly, and hence not part of OpenSocial...... however, I can see
use cases that say gadgets need access to their own message feeds...
probably needs to be discussed on the spec list.
>
> Another question. No matter how hard I look, I cannot find in the
> spec a
> description on how the post message method is suposed to get the
> list of
> recipients. It's not part of the proposed URI template (and I can't
> see
> how to fit it there) and it's not part of the post body either, as
> that's supossed to be just the message itself. Am I missing something
> obvious here?
I also couldnt see anything, it might be something that is missing
from the Spec, that only becomes apparent on implementation.
Anyone ?
Ian
Re: A first stab at Message support
Posted by ro...@gmail.com.
On Wed, Sep 24, 2008 at 04:53:35AM -0400, Ian Boston wrote:
> Looks ok, but needs some javadoc and may need some SPI methods outside
> the Spec to make it meaningful. Depends on if we think Shindig scope is
> purely to the spec, or if it should go just far enough to be a running
> RI of OpenSocial.
Working on the javadoc and the handler's test.
About the scope, I would say having a reference implementation (I assume
that's what you mean by RI) is way more useful. I will implement some
reading methods in the sample classes anyways, but I don't feel
qualified yet to propose a SPI API.
Another question. No matter how hard I look, I cannot find in the spec a
description on how the post message method is suposed to get the list of
recipients. It's not part of the proposed URI template (and I can't see
how to fit it there) and it's not part of the post body either, as
that's supossed to be just the message itself. Am I missing something
obvious here?
Re: A first stab at Message support
Posted by Ian Boston <ie...@tfd.co.uk>.
Looks ok, but needs some javadoc and may need some SPI methods outside
the Spec to make it meaningful. Depends on if we think Shindig scope is
purely to the spec, or if it should go just far enough to be a running
RI of OpenSocial.
http://codereview.appspot.com/5702/diff/1/2
File
java/social-api/src/main/java/org/apache/shindig/social/opensocial/
service/MessageHandler.java
(right):
http://codereview.appspot.com/5702/diff/1/2#newcode33
Line 33: public class MessageHandler extends DataRequestHandler {
The impl is fine, but some javadoc would be great especially on the
methods where there are some behaviors that would aid understanding what
the code is doing.
http://codereview.appspot.com/5702/diff/1/3
File
java/social-api/src/main/java/org/apache/shindig/social/opensocial/
spi/MessageService.java
(right):
http://codereview.appspot.com/5702/diff/1/3#newcode34
Line 34: public interface MessageService {
I know that strictly speaking the REST API doesnt have an query
mechanism for messages, but I wonder if it might make sense to have a
query mechanism in the internal SPI?
http://codereview.appspot.com/5702/diff/1/4
File
java/social-api/src/main/java/org/apache/shindig/social/sample/spi/
JsonDbOpensocialService.java
(right):
http://codereview.appspot.com/5702/diff/1/4#newcode442
Line 442: public Future<Void> createMessage(UserId userId, String appId,
Set<UserId> recipients,
This does need an implementation (obviously),
But seeing as there is no query, probably just a check on the parameters
and a test controlled response.
http://codereview.appspot.com/5702/diff/1/5
File
java/social-api/src/test/java/org/apache/shindig/social/opensocial/
service/MessageHandlerTest.java
(right):
http://codereview.appspot.com/5702/diff/1/5#newcode41
Line 41: public void testPostMessage() {
Although setup does provide some coverage, sending a message probably
needs to be here ?
Please review this at http://codereview.appspot.com/5702
Affected files:
A java/social-api/src/main/java/org/apache/shindig/social/
opensocial/service/MessageHandler.java
A java/social-api/src/main/java/org/apache/shindig/social/
opensocial/spi/MessageService.java
M java/social-api/src/main/java/org/apache/shindig/social/sample/
spi/JsonDbOpensocialService.java
A java/social-api/src/test/java/org/apache/shindig/social/
opensocial/service/MessageHandlerTest.java
On 23 Sep 2008, at 20:44, Rodrigo Gallardo wrote:
> I have started writing a first implementation of Message posting.
>
> As this is my first foray into shindig I would very much like to get
> some early reviews on it. I will post the patch for final
> consideration
> after I determine how to include the functionality in the sample
> server.
>
> http://codereview.appspot.com/5702