> On 2012-05-21 20:34:39, BrianLillie wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/LocalId.java, > > line 41 > > <https://reviews.apache.org/r/4268/diff/21/?file=109211#file109211line41> > > > > Any need to validate non-null?
Yes, because a LocalId is defined as [Local-Id = *( ALPHA / DIGIT / "_" / "." / "-" )] in http://opensocial-resources.googlecode.com/svn/spec/2.0.1/Core-Data.xml#Local-Id > On 2012-05-21 20:34:39, BrianLillie wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/GlobalId.java, > > line 44 > > <https://reviews.apache.org/r/4268/diff/21/?file=109209#file109209line44> > > > > Any need to validate non-null? Yes, because a GlobalId is defined as [Global-Id = Domain-Name ":" Local-Id] in http://opensocial-resources.googlecode.com/svn/spec/2.0.1/Core-Data.xml#Global-Id > On 2012-05-21 20:34:39, BrianLillie wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/DomainName.java, > > line 42 > > <https://reviews.apache.org/r/4268/diff/21/?file=109208#file109208line42> > > > > Any need to validate non-null ? Yes, because a DomainName is defined as [Domain-Name = *( ALPHA / DIGIT / "_" / "." / "-" )] in http://opensocial-resources.googlecode.com/svn/spec/2.0.1/Core-Data.xml#Domain-Name > On 2012-05-21 20:34:39, BrianLillie wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/GroupImpl.java, > > line 66 > > <https://reviews.apache.org/r/4268/diff/21/?file=109206#file109206line66> > > > > Should this be the generic type Map, or is a HashMap specifically > > required ? This is part of the implementation reference. We only create HashMaps when converting the canonicaldb.json into Java. Using HashMap should be sufficient however for the sack of being generic, using Map will also work. I will change it over. > On 2012-05-21 20:34:39, BrianLillie wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/GroupImpl.java, > > line 67 > > <https://reviews.apache.org/r/4268/diff/21/?file=109206#file109206line67> > > > > Is the Map always going to contain the "value" object, or is there a > > possibility of it missing the object, resulting in null going to the > > GroupId constructor? Added code to allow null to be returned from get("value"). "" is a perfectly valid GroupId (because it is a valid LocalId). The code now handles a null value to mean "". - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4268/#review8019 ----------------------------------------------------------- On 2012-05-17 14:02:58, Mike May wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4268/ > ----------------------------------------------------------- > > (Updated 2012-05-17 14:02:58) > > > Review request for shindig, Henry Saputra, Ryan Baxter, and Stanton Sievers. > > > Summary > ------- > > A base implementation for OpenSocial Groups. Provides groups to the osapi > javascript namespace with an implementation link for get() in GroupService. > > > This addresses bug SHINDIG-1780. > https://issues.apache.org/jira/browse/SHINDIG-1780 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/SocialHelloWorld.xml > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/content/sampledata/canonicaldb.json > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/ActivityServiceDb.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/AppDataServiceDb.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/main/java/org/apache/shindig/social/opensocial/jpa/spi/PersonServiceDb.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiDatabaseBootstrap.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/java/org/apache/shindig/social/opensocial/jpa/spi/SpiTestUtil.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/samples/src/test/resources/sampledata/canonicaldb.json > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/config/SocialApiGuiceModule.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/GroupImpl.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/Group.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/DomainName.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/GlobalId.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/GroupId.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/LocalId.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/ObjectId.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/SampleModule.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialService.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonPeopleTest.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulXmlPeopleTest.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/DomainNameTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/GlobalIdTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/GroupIdTest.java > 1339181 > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/LocalIdTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/ObjectIdTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/4268/diff > > > Testing > ------- > > Used the common container with a gadget to call osapi.groups.get() > > -- > > Created and updated unit tests > All tests pass > > > Thanks, > > Mike > >
