You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by rb...@gmail.com on 2012/06/11 00:04:15 UTC

Using custom @ ids GroupIds throws IllegalArgumentException (issue 6305079)

Reviewers: daviesd_oclc.org, themistymay_gmail.com, dev_shindig.apache.org,

Description:
If you try to create a GroupId with a non standard @ id an
IllegalArgumentException will be thrown. This is because we default the
type to ObjectId and @ is not a valid character in an ObjectId.

Please review this at http://codereview.appspot.com/6305079/

Affected files:
    
java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/GroupId.java
    
java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/GroupIdTest.java


Index:  
java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/GroupId.java
===================================================================
---  
java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/GroupId.java	 
(revision 1348658)
+++  
java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/GroupId.java	 
(working copy)
@@ -56,7 +56,7 @@
        this.objectId = (ObjectId) objectId;
      // Else it must be a string, store as such
      } else {
-      if(getType(objectId).equals(Type.objectId)) {
+      if(Type.objectId.equals(getType(objectId))) {
          this.objectId = new ObjectId(objectId.toString());
        } else {
          this.objectId = objectId.toString();
@@ -119,6 +119,10 @@
        return Type.friends;
      } else if(type.equals("all")) {
        return Type.all;
+    } else if(objectId instanceof String &&  
((String)objectId).startsWith("@")) {
+    	// Could be a custom @ id, and it certainly is not an object id
+    	// return null we don't know the type
+    	return null;
      } else {
        return Type.objectId;
      }
Index:  
java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/GroupIdTest.java
===================================================================
---  
java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/GroupIdTest.java	 
(revision 1348658)
+++  
java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/GroupIdTest.java	 
(working copy)
@@ -38,6 +38,10 @@
      GroupId group = GroupId.fromJson("superbia");
      assertEquals(GroupId.Type.objectId, group.getType());
      assertEquals("superbia", group.getObjectId().toString());
+
+    GroupId unknown = GroupId.fromJson("@foo");
+    assertNull(unknown.getType());
+    assertEquals("@foo", unknown.getObjectId().toString());
    }

    @Test
@@ -51,6 +55,11 @@

      assertEquals(g1.getType(), g2.getType());
      assertEquals(g1.getObjectId().toString(), g2.getObjectId().toString());
+
+   GroupId g3 =  new GroupId("@foo");
+   assertNull(g3.getType());
+   assertEquals("@foo", g3.getObjectId().toString());
+
    }

    @Test(expected=IllegalArgumentException.class)