Query all external groups for internal group memberships
When asking for the known groups a user belongs to they may belong to an internal group by way of membership in a non-internal group, such as LDAP. Cache in memory the complete list of any non-internal group UUIDs used as members of an internal group. These must get checked for membership before completing the known group data from the internal backend. Change-Id: I10aef9f185915771deb58f4f180818c08e784ea3
This commit is contained in:
@@ -26,7 +26,9 @@ public interface GroupIncludeCache {
|
||||
/** @return any groups the passed group belongs to. */
|
||||
public Set<AccountGroup.UUID> memberIn(AccountGroup.UUID groupId);
|
||||
|
||||
/** @return set of any UUIDs that are not internal groups. */
|
||||
public Set<AccountGroup.UUID> allExternalMembers();
|
||||
|
||||
public void evictMembersOf(AccountGroup.UUID groupId);
|
||||
public void evictMemberIn(AccountGroup.UUID groupId);
|
||||
}
|
||||
|
||||
|
||||
@@ -44,6 +44,7 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
||||
.getLogger(GroupIncludeCacheImpl.class);
|
||||
private static final String BYINCLUDE_NAME = "groups_byinclude";
|
||||
private static final String MEMBERS_NAME = "groups_members";
|
||||
private static final String EXTERNAL_NAME = "groups_external";
|
||||
|
||||
public static Module module() {
|
||||
return new CacheModule() {
|
||||
@@ -59,6 +60,11 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
||||
new TypeLiteral<Set<AccountGroup.UUID>>() {})
|
||||
.loader(MembersOfLoader.class);
|
||||
|
||||
cache(EXTERNAL_NAME,
|
||||
String.class,
|
||||
new TypeLiteral<Set<AccountGroup.UUID>>() {})
|
||||
.loader(AllExternalLoader.class);
|
||||
|
||||
bind(GroupIncludeCacheImpl.class);
|
||||
bind(GroupIncludeCache.class).to(GroupIncludeCacheImpl.class);
|
||||
}
|
||||
@@ -67,15 +73,19 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
||||
|
||||
private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> membersOf;
|
||||
private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> memberIn;
|
||||
private final LoadingCache<String, Set<AccountGroup.UUID>> external;
|
||||
|
||||
@Inject
|
||||
GroupIncludeCacheImpl(
|
||||
@Named(MEMBERS_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> membersOf,
|
||||
@Named(BYINCLUDE_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> memberIn) {
|
||||
@Named(BYINCLUDE_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> memberIn,
|
||||
@Named(EXTERNAL_NAME) LoadingCache<String, Set<AccountGroup.UUID>> external) {
|
||||
this.membersOf = membersOf;
|
||||
this.memberIn = memberIn;
|
||||
this.external = external;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<AccountGroup.UUID> membersOf(AccountGroup.UUID groupId) {
|
||||
try {
|
||||
return membersOf.get(groupId);
|
||||
@@ -85,6 +95,7 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<AccountGroup.UUID> memberIn(AccountGroup.UUID groupId) {
|
||||
try {
|
||||
return memberIn.get(groupId);
|
||||
@@ -94,15 +105,31 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void evictMembersOf(AccountGroup.UUID groupId) {
|
||||
if (groupId != null) {
|
||||
membersOf.invalidate(groupId);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void evictMemberIn(AccountGroup.UUID groupId) {
|
||||
if (groupId != null) {
|
||||
memberIn.invalidate(groupId);
|
||||
|
||||
if (!AccountGroup.isInternalGroup(groupId)) {
|
||||
external.invalidate(EXTERNAL_NAME);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<AccountGroup.UUID> allExternalMembers() {
|
||||
try {
|
||||
return external.get(EXTERNAL_NAME);
|
||||
} catch (ExecutionException e) {
|
||||
log.warn("Cannot load set of non-internal groups", e);
|
||||
return Collections.emptySet();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -170,4 +197,30 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static class AllExternalLoader extends
|
||||
CacheLoader<String, Set<AccountGroup.UUID>> {
|
||||
private final SchemaFactory<ReviewDb> schema;
|
||||
|
||||
@Inject
|
||||
AllExternalLoader(final SchemaFactory<ReviewDb> sf) {
|
||||
schema = sf;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<AccountGroup.UUID> load(String key) throws Exception {
|
||||
final ReviewDb db = schema.open();
|
||||
try {
|
||||
Set<AccountGroup.UUID> ids = Sets.newHashSet();
|
||||
for (AccountGroupIncludeByUuid agi : db.accountGroupIncludesByUuid().all()) {
|
||||
if (!AccountGroup.isInternalGroup(agi.getIncludeUUID())) {
|
||||
ids.add(agi.getIncludeUUID());
|
||||
}
|
||||
}
|
||||
return ImmutableSet.copyOf(ids);
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -116,9 +116,18 @@ public class IncludingGroupMembership implements GroupMembership {
|
||||
}
|
||||
|
||||
private ImmutableSet<AccountGroup.UUID> computeKnownGroups() {
|
||||
GroupMembership membership = user.getEffectiveGroups();
|
||||
Set<AccountGroup.UUID> direct = user.state().getInternalGroups();
|
||||
Set<AccountGroup.UUID> r = Sets.newHashSet(direct);
|
||||
List<AccountGroup.UUID> q = Lists.newArrayList(r);
|
||||
|
||||
for (AccountGroup.UUID g : membership.intersection(
|
||||
includeCache.allExternalMembers())) {
|
||||
if (r.add(g)) {
|
||||
q.add(g);
|
||||
}
|
||||
}
|
||||
|
||||
while (!q.isEmpty()) {
|
||||
AccountGroup.UUID id = q.remove(q.size() - 1);
|
||||
for (AccountGroup.UUID g : includeCache.memberIn(id)) {
|
||||
|
||||
Reference in New Issue
Block a user