added support for incomplete cache of groups to improve isMember and getGroup when possible

projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java

@@ -774,7 +774,7 @@ public class GMSClient implements TransferListener
             throw new IllegalArgumentException("userID and role are required.");
         }
         
-        List<Group> cachedGroups = getCachedGroups(userID, role);
+        List<Group> cachedGroups = getCachedGroups(userID, role, true);
         if (cachedGroups != null)
         {
             return cachedGroups;
@@ -881,18 +881,10 @@ public class GMSClient implements TransferListener
             throw new IllegalArgumentException("userID and role are required.");
         }
         
-        List<Group> cachedGroups = getCachedGroups(userID, role);
-        if (cachedGroups != null)
-        {
-            int index = cachedGroups.indexOf(new Group(groupName));
-            if (index != -1)
-            {
-                return cachedGroups.get(index);
-            }
-            else
+        Group cachedGroup = getCachedGroup(userID, groupName, role);
+        if (cachedGroup != null)
         {
-                return null;
-            }
+            return cachedGroup;
         }
         
         String idType = AuthenticationUtil.getPrincipalType(userID);
@@ -951,9 +943,9 @@ public class GMSClient implements TransferListener
             }
             if (groups.size() == 1)
             {
-                // don't cache these results as it is not a complete
-                // list of memberships--it only applies to one group.
-                return groups.get(0);
+                Group ret = groups.get(0);
+                addCachedGroup(userID, ret, role);
+                return ret;
             }
             throw new IllegalStateException(
                     "Duplicate membership for " + id + " in group " + groupName);
@@ -1049,15 +1041,13 @@ public class GMSClient implements TransferListener
     {
         AccessControlContext acContext = AccessController.getContext();
         Subject subject = Subject.getSubject(acContext);
-        
         if (subject != null)
         {
-            log.debug("Clearing cache");
-            subject.getPrivateCredentials().clear();
+            subject.getPrivateCredentials().remove(new GroupMemberships());
         }
     }
 
-    protected List<Group> getCachedGroups(Principal userID, Role role)
+    protected GroupMemberships getGroupCache(Principal userID)
     {
         AccessControlContext acContext = AccessController.getContext();
         Subject subject = Subject.getSubject(acContext);
@@ -1065,43 +1055,81 @@ public class GMSClient implements TransferListener
         // only consult cache if the userID is of the calling subject
         if (userIsSubject(userID, subject))
         {
-            Set groupCredentialSet = subject.getPrivateCredentials(GroupMemberships.class);
-            if ((groupCredentialSet != null) && 
-                (groupCredentialSet.size() == 1))
+            Set<GroupMemberships> gset = subject.getPrivateCredentials(GroupMemberships.class);
+            if (gset == null || gset.isEmpty())
             {
-                Iterator i = groupCredentialSet.iterator();
-                GroupMemberships groupMemberships = ((GroupMemberships) i.next());
-                return groupMemberships.memberships.get(role);
+                GroupMemberships mems = new GroupMemberships();
+                subject.getPrivateCredentials().add(mems);
+                return mems;
             }
+            GroupMemberships mems = gset.iterator().next();
+            return mems;
         }
-        return null;
+        return null; // no cache
     }
     
-    protected void setCachedGroups(Principal userID, List<Group> groups, Role role)
+    protected Group getCachedGroup(Principal userID, String groupID, Role role)
     {
-        AccessControlContext acContext = AccessController.getContext();
-        Subject subject = Subject.getSubject(acContext);
+        List<Group> groups = getCachedGroups(userID, role, false);
+        if (groups == null)
+            return null; // no cache
+        for (Group g : groups)
+        {
+            if (g.getID().equals(groupID))
+                return g;
+        }
+        return null;
+    }
+    protected List<Group> getCachedGroups(Principal userID, Role role, boolean complete)
+    {
+        GroupMemberships mems = getGroupCache(userID);
+        if (mems == null)
+            return null; // no cache
 
-        // only save to cache if the userID is of the calling subject
-        if (userIsSubject(userID, subject))
+        Boolean cacheState = mems.complete.get(role);
+        if (!complete || Boolean.TRUE.equals(cacheState))
+            return mems.memberships.get(role);
+        
+        // caller wanted complete and we don't have that
+        return null;
+    }
+
+    protected void addCachedGroup(Principal userID, Group group, Role role)
     {
-            log.debug("Caching groups for " + userID + ", role " + role);
+        GroupMemberships mems = getGroupCache(userID);
+        if (mems == null)
+            return; // no cache
         
-            final GroupMemberships groupCredentials;
-            Set groupCredentialSet = subject.getPrivateCredentials(GroupMemberships.class);
-            if ((groupCredentialSet != null) && 
-                (groupCredentialSet.size() == 1))
+        List<Group> groups = mems.memberships.get(role);
+        if (groups == null)
         {
-                Iterator i = groupCredentialSet.iterator();
-                groupCredentials = ((GroupMemberships) i.next());
+            groups = new ArrayList<Group>();
+            mems.complete.put(role, Boolean.FALSE);
+            mems.memberships.put(role, groups);
         }
-            else
-            {
-                groupCredentials = new GroupMemberships();
-                subject.getPrivateCredentials().add(groupCredentials);
+        if (!groups.contains(group))
+            groups.add(group);
     }
     
-            groupCredentials.memberships.put(role,  groups);
+    protected void setCachedGroups(Principal userID, List<Group> groups, Role role)
+    {
+        GroupMemberships mems = getGroupCache(userID);
+        if (mems == null)
+            return; // no cache
+        
+        log.debug("Caching groups for " + userID + ", role " + role);
+        List<Group> cur = mems.memberships.get(role);
+        if (cur == null)
+        {
+            cur = new ArrayList<Group>();
+            mems.complete.put(role, Boolean.FALSE);
+            mems.memberships.put(role, cur);
+        }
+        for (Group group : groups)
+        {
+            if (!cur.contains(group))
+                cur.add(group);
+            mems.complete.put(role, Boolean.TRUE);
         }
     }
     
@@ -1114,7 +1142,7 @@ public class GMSClient implements TransferListener
         
         for (Principal subjectPrincipal : subject.getPrincipals())
         {
-            if (subjectPrincipal.equals(userID))
+            if (AuthenticationUtil.equals(subjectPrincipal, userID))
             {
                 return true;
             }
@@ -1123,17 +1151,31 @@ public class GMSClient implements TransferListener
     }
 
     /**
-     * Class used to hold list of groups in which
-     * a user is a member.
+     * Class used to hold list of groups in which a user is known to be a member.
      */
-    protected class GroupMemberships
+    protected class GroupMemberships implements Comparable
     {
         Map<Role, List<Group>> memberships = new HashMap<Role, List<Group>>();
+        Map<Role, Boolean> complete = new HashMap<Role, Boolean>();
         
         protected GroupMemberships()
         {
         }
 
+        // only allow one in a set - makes clearCache simple too
+        public boolean equals(Object rhs)
+        {
+            if (rhs != null && rhs instanceof GroupMemberships)
+                return true;
+            return false;
+        }
+
+        public int compareTo(Object t)
+        {
+            if (this.equals(t))
+                return 0;
+            return -1; // wonder if this is sketchy
+        }
     }
 
 }

projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/GMSClientTest.java

@@ -154,27 +154,38 @@ public class GMSClientTest
                     public Object run() throws Exception
                     {
 
-                        List<Group> initial = client.getCachedGroups(test1UserID, Role.MEMBER);
+                        List<Group> initial = client.getCachedGroups(test1UserID, Role.MEMBER, true);
                         Assert.assertNull("Cache should be null", initial);
                         
+                        // add single group as isMember might do
+                        Group group0 = new Group("0");
+                        client.addCachedGroup(test1UserID, group0, Role.MEMBER);
+                        List<Group> actual = client.getCachedGroups(test1UserID, Role.MEMBER, true);
+                        Assert.assertNull("Cache should be null", actual);
+                        
+                        Group g = client.getCachedGroup(test1UserID, "0", Role.MEMBER);
+                        Assert.assertNotNull("cached group from incomplete cache", g);
+
+                        // add all groups like getMemberships might do
                         List<Group> expected = new ArrayList<Group>();
                         Group group1 = new Group("1");
                         Group group2 = new Group("2");
+                        expected.add(group0);
                         expected.add(group1);
                         expected.add(group2);
 
                         client.setCachedGroups(test1UserID, expected, Role.MEMBER);
 
-                        List<Group> actual = client.getCachedGroups(test1UserID, Role.MEMBER);
+                        actual = client.getCachedGroups(test1UserID, Role.MEMBER, true);
                         Assert.assertEquals("Wrong cached groups", expected, actual);
                         
                         // check against another role
-                        actual = client.getCachedGroups(test1UserID, Role.OWNER);
+                        actual = client.getCachedGroups(test1UserID, Role.OWNER, true);
                         Assert.assertNull("Cache should be null", actual);
                         
                         // check against another userid
                         final HttpPrincipal anotherUserID = new HttpPrincipal("anotheruser");
-                        actual = client.getCachedGroups(anotherUserID, Role.MEMBER);
+                        actual = client.getCachedGroups(anotherUserID, Role.MEMBER, true);
                         Assert.assertNull("Cache should be null", actual);
 
                         return null;
@@ -192,7 +203,7 @@ public class GMSClientTest
                         public Object run() throws Exception
                         {
 
-                            List<Group> initial = client.getCachedGroups(test2UserID, Role.MEMBER);
+                            List<Group> initial = client.getCachedGroups(test2UserID, Role.MEMBER, true);
                             Assert.assertNull("Cache should be null", initial);
 
                             List<Group> expected = new ArrayList<Group>();
@@ -203,16 +214,16 @@ public class GMSClientTest
 
                             client.setCachedGroups(test2UserID, expected, Role.MEMBER);
 
-                            List<Group> actual = client.getCachedGroups(test2UserID, Role.MEMBER);
+                            List<Group> actual = client.getCachedGroups(test2UserID, Role.MEMBER, true);
                             Assert.assertEquals("Wrong cached groups", expected, actual);
                             
                             // check against another role
-                            actual = client.getCachedGroups(test2UserID, Role.OWNER);
+                            actual = client.getCachedGroups(test2UserID, Role.OWNER, true);
                             Assert.assertNull("Cache should be null", actual);
                             
                             // check against another userid
                             final HttpPrincipal anotherUserID = new HttpPrincipal("anotheruser");
-                            actual = client.getCachedGroups(anotherUserID, Role.MEMBER);
+                            actual = client.getCachedGroups(anotherUserID, Role.MEMBER, true);
                             Assert.assertNull("Cache should be null", actual);
 
                             return null;
@@ -221,7 +232,7 @@ public class GMSClientTest
 
             // do the same without a subject
 
-            List<Group> initial = client.getCachedGroups(test1UserID, Role.MEMBER);
+            List<Group> initial = client.getCachedGroups(test1UserID, Role.MEMBER, true);
             Assert.assertNull("Cache should be null", initial);
 
             List<Group> newgroups = new ArrayList<Group>();
@@ -232,7 +243,7 @@ public class GMSClientTest
 
             client.setCachedGroups(test1UserID, newgroups, Role.MEMBER);
 
-            List<Group> actual = client.getCachedGroups(test1UserID, Role.MEMBER);
+            List<Group> actual = client.getCachedGroups(test1UserID, Role.MEMBER, true);
             Assert.assertNull("Cache should still be null", actual);
         }
         catch (Throwable t)