From 1e44bdb605d11da77f7577bc09ba1df2fc674d16 Mon Sep 17 00:00:00 2001
From: Patrick Dowler <patrick.dowler@nrc-cnrc.gc.ca>
Date: Wed, 19 Aug 2015 11:29:40 -0700
Subject: [PATCH] added support for incomplete cache of groups to improve
isMember and getGroup when possible
---
.../src/ca/nrc/cadc/ac/client/GMSClient.java | 144 +++++++++++-------
.../ca/nrc/cadc/ac/client/GMSClientTest.java | 31 ++--
2 files changed, 114 insertions(+), 61 deletions(-)
diff --git a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java b/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
index ce1d0058..c1916258 100755
--- a/projects/cadcAccessControl/src/ca/nrc/cadc/ac/client/GMSClient.java
+++ b/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)
+ Group cachedGroup = getCachedGroup(userID, groupName, role);
+ if (cachedGroup != null)
{
- int index = cachedGroups.indexOf(new Group(groupName));
- if (index != -1)
- {
- return cachedGroups.get(index);
- }
- else
- {
- 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; // no cache
+ }
+
+ protected Group getCachedGroup(Principal userID, String groupID, Role role)
+ {
+ 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
+
+ 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)
+ {
+ GroupMemberships mems = getGroupCache(userID);
+ if (mems == null)
+ return; // no cache
+
+ List<Group> groups = mems.memberships.get(role);
+ if (groups == null)
+ {
+ groups = new ArrayList<Group>();
+ mems.complete.put(role, Boolean.FALSE);
+ mems.memberships.put(role, groups);
+ }
+ if (!groups.contains(group))
+ groups.add(group);
+ }
+
protected void setCachedGroups(Principal userID, List<Group> groups, Role role)
{
- AccessControlContext acContext = AccessController.getContext();
- Subject subject = Subject.getSubject(acContext);
+ GroupMemberships mems = getGroupCache(userID);
+ if (mems == null)
+ return; // no cache
- // only save to cache if the userID is of the calling subject
- if (userIsSubject(userID, subject))
+ log.debug("Caching groups for " + userID + ", role " + role);
+ List<Group> cur = mems.memberships.get(role);
+ if (cur == null)
{
- log.debug("Caching groups for " + userID + ", role " + role);
-
- final GroupMemberships groupCredentials;
- Set groupCredentialSet = subject.getPrivateCredentials(GroupMemberships.class);
- if ((groupCredentialSet != null) &&
- (groupCredentialSet.size() == 1))
- {
- Iterator i = groupCredentialSet.iterator();
- groupCredentials = ((GroupMemberships) i.next());
- }
- else
- {
- groupCredentials = new GroupMemberships();
- subject.getPrivateCredentials().add(groupCredentials);
- }
-
- groupCredentials.memberships.put(role, groups);
+ 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
+ }
}
}
diff --git a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/GMSClientTest.java b/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/GMSClientTest.java
index 3025fb37..dd228052 100644
--- a/projects/cadcAccessControl/test/src/ca/nrc/cadc/ac/client/GMSClientTest.java
+++ b/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)
--
GitLab