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
