Tidy up Groups classes and cache naming and visibility Reducing classes constructors visibility and methods plus a global clean-up of the cache naming. Cache does not store anymore only organisation data but the entire group ownership of the user. Cache duration default is aligned with the Gerrit groups cache (1h). Change-Id: Iff18ae535bc579f74e6c3ab573dcbbbd5764a2d9
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceModule.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceModule.java index 8822bd9..21926c4 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceModule.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceModule.java
@@ -22,7 +22,7 @@ import com.google.inject.assistedinject.FactoryModuleBuilder; import com.googlesource.gerrit.plugins.github.group.GitHubGroupBackend; import com.googlesource.gerrit.plugins.github.group.GitHubGroupMembership; -import com.googlesource.gerrit.plugins.github.group.GitHubOrganisationsCache; +import com.googlesource.gerrit.plugins.github.group.GitHubGroupsCache; import com.googlesource.gerrit.plugins.github.group.GitHubOrganisationGroup; import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin; import com.googlesource.gerrit.plugins.github.oauth.IdentifiedUserGitHubLoginProvider; @@ -34,7 +34,7 @@ bind(new TypeLiteral<UserScopedProvider<GitHubLogin>>() {}).to( IdentifiedUserGitHubLoginProvider.class); - install(GitHubOrganisationsCache.module()); + install(GitHubGroupsCache.module()); DynamicSet.bind(binder(), TopMenu.class).to(GitHubTopMenu.class); DynamicSet.bind(binder(), GroupBackend.class).to(GitHubGroupBackend.class);
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroup.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroup.java index 4e1279e..4f5947c 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroup.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroup.java
@@ -14,7 +14,7 @@ package com.googlesource.gerrit.plugins.github.group; - +import lombok.Getter; import com.google.gerrit.common.data.GroupDescription.Basic; import com.google.gerrit.reviewdb.client.AccountGroup.UUID; @@ -22,11 +22,15 @@ public abstract class GitHubGroup implements Basic { public static final String UUID_PREFIX = "github:"; public static final String NAME_PREFIX = "github/"; - protected final UUID uuid; + + @Getter + protected final UUID groupUUID; + + @Getter protected final String url; - public GitHubGroup(UUID uuid, String url) { - this.uuid = uuid; + GitHubGroup(UUID groupUUID, String url) { + this.groupUUID = groupUUID; this.url = url; } @@ -34,15 +38,4 @@ public String getEmailAddress() { return ""; } - - @Override - public UUID getGroupUUID() { - return uuid; - } - - @Override - public String getUrl() { - return url; - } - }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupBackend.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupBackend.java index 4c22c91..b6fd5cf 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupBackend.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupBackend.java
@@ -43,13 +43,13 @@ private static final Logger log = LoggerFactory .getLogger(GitHubGroupBackend.class); private final GitHubGroupMembership.Factory ghMembershipProvider; - private final GitHubOrganisationsCache ghOrganisationCache; + private final GitHubGroupsCache ghOrganisationCache; @Inject - public GitHubGroupBackend( + GitHubGroupBackend( UserScopedProvider<GitHubLogin> ghLogin, GitHubGroupMembership.Factory ghMembershipProvider, - GitHubOrganisationsCache ghOrganisationCache) { + GitHubGroupsCache ghOrganisationCache) { this.ghMembershipProvider = ghMembershipProvider; this.ghOrganisationCache = ghOrganisationCache; } @@ -91,14 +91,14 @@ Builder<GroupReference> orgGroups = new ImmutableSet.Builder<GroupReference>(); - for (String ghOrg : ghOrgs) { - if (ghOrg.toLowerCase().startsWith(orgNamePrefixLowercase)) { + for (String organizationName : ghOrgs) { + if (organizationName.toLowerCase().startsWith(orgNamePrefixLowercase)) { GroupReference teamGroupRef = - GitHubOrganisationGroup.groupReference(ghOrg); + GitHubOrganisationGroup.groupReference(organizationName); if ((orgNamePrefixLowercase.length() > 0 && orgNamePrefix .endsWith("/")) || teamNameLowercase.length() > 0) { - for (String teamName : ghOrganisationCache.getTeamsForCurrentUser(ghOrg)) { + for (String teamName : ghOrganisationCache.getTeamsForCurrentUser(organizationName)) { if (teamName.toLowerCase().startsWith(teamNameLowercase)) { orgGroups.add(GitHubTeamGroup.groupReference(teamGroupRef, teamName));
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupMembership.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupMembership.java index c66c8e0..b93c138 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupMembership.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupMembership.java
@@ -30,11 +30,11 @@ } @Inject - public GitHubGroupMembership(GitHubOrganisationsCache ghOrganisationCache, + GitHubGroupMembership(GitHubGroupsCache ghOrganisationCache, @Assisted String username) { this.groups = new ImmutableSet.Builder<UUID>().addAll( - ghOrganisationCache.getAllGroupsForUser(username)).build(); + ghOrganisationCache.getGroupsForUser(username)).build(); } @Override
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubOrganisationsCache.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupsCache.java similarity index 73% rename from github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubOrganisationsCache.java rename to github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupsCache.java index 671ddc3..8cd82e4 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubOrganisationsCache.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupsCache.java
@@ -44,14 +44,14 @@ import com.googlesource.gerrit.plugins.github.oauth.UserScopedProvider; @Singleton -public class GitHubOrganisationsCache { +public class GitHubGroupsCache { private static final Logger log = LoggerFactory - .getLogger(GitHubOrganisationsCache.class); - private static final String ORGS_CACHE_NAME = "org-teams"; - protected static final long GROUPS_CACHE_TTL_MINS = 15; + .getLogger(GitHubGroupsCache.class); + private static final String ORGS_CACHE_NAME = "groups"; + protected static final long GROUPS_CACHE_TTL_MINS = 60; public static class OrganisationLoader extends - CacheLoader<String, Multimap<String,String>> { + CacheLoader<String, Multimap<String, String>> { private static final Logger log = LoggerFactory .getLogger(OrganisationLoader.class); private final UserScopedProvider<GitHubLogin> ghLoginProvider; @@ -88,26 +88,26 @@ return new CacheModule() { @Override protected void configure() { - cache(ORGS_CACHE_NAME, String.class, new TypeLiteral<Multimap<String,String>>() {}) - .expireAfterWrite(GROUPS_CACHE_TTL_MINS, MINUTES).loader( - OrganisationLoader.class); - bind(GitHubOrganisationsCache.class); + cache(ORGS_CACHE_NAME, String.class, + new TypeLiteral<Multimap<String, String>>() {}).expireAfterWrite( + GROUPS_CACHE_TTL_MINS, MINUTES).loader(OrganisationLoader.class); + bind(GitHubGroupsCache.class); } }; } - private final LoadingCache<String, Multimap<String,String>> orgTeamsByUsername; + private final LoadingCache<String, Multimap<String, String>> orgTeamsByUsername; private final Provider<IdentifiedUser> userProvider; @Inject - public GitHubOrganisationsCache( - @Named(ORGS_CACHE_NAME) LoadingCache<String, Multimap<String,String>> byUsername, + GitHubGroupsCache( + @Named(ORGS_CACHE_NAME) LoadingCache<String, Multimap<String, String>> byUsername, Provider<IdentifiedUser> userProvider) { this.orgTeamsByUsername = byUsername; this.userProvider = userProvider; } - public Set<String> getOrganizationsForUser(String username) { + Set<String> getOrganizationsForUser(String username) { try { return orgTeamsByUsername.get(username).keySet(); } catch (ExecutionException e) { @@ -116,35 +116,35 @@ } } - public Set<String> getOrganizationsForCurrentUser() throws ExecutionException { + Set<String> getOrganizationsForCurrentUser() throws ExecutionException { return orgTeamsByUsername.get(userProvider.get().getUserName()).keySet(); } - public Set<String> getTeamsForUser(String organizationName, String username) { + Set<String> getTeamsForUser(String organizationName, String username) { try { return new ImmutableSet.Builder<String>().addAll( orgTeamsByUsername.get(username).get(organizationName)).build(); } catch (ExecutionException e) { - log.warn("Cannot get Teams membership for organisation '" + organizationName - + "' and user '" + username + "'", e); + log.warn("Cannot get Teams membership for organisation '" + + organizationName + "' and user '" + username + "'", e); return Collections.emptySet(); } } - public Set<String> getTeamsForCurrentUser(String organizationName) { + Set<String> getTeamsForCurrentUser(String organizationName) { return getTeamsForUser(organizationName, userProvider.get().getUserName()); } - public Set<UUID> getAllGroupsForUser(String username) { + public Set<UUID> getGroupsForUser(String username) { ImmutableSet.Builder<UUID> groupsBuilder = new ImmutableSet.Builder<>(); - for (String org : getOrganizationsForUser(username)) { - groupsBuilder.add(GitHubOrganisationGroup.uuid(org)); + for (String org : getOrganizationsForUser(username)) { + groupsBuilder.add(GitHubOrganisationGroup.uuid(org)); - for (String team : getTeamsForUser(org, username)) { - groupsBuilder.add(GitHubTeamGroup.uuid( - GitHubOrganisationGroup.uuid(org), team)); - } + for (String team : getTeamsForUser(org, username)) { + groupsBuilder.add(GitHubTeamGroup.uuid( + GitHubOrganisationGroup.uuid(org), team)); } - return groupsBuilder.build(); + } + return groupsBuilder.build(); } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubOrganisationGroup.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubOrganisationGroup.java index 01a0f89..7dca217 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubOrganisationGroup.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubOrganisationGroup.java
@@ -31,6 +31,7 @@ } private final String orgName; + @Inject GitHubOrganisationGroup(@Assisted("orgName") String orgName, @Assisted("orgUrl") @Nullable String orgUrl) {
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubTeamGroup.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubTeamGroup.java index 345d2b3..cd53736 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubTeamGroup.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubTeamGroup.java
@@ -18,6 +18,7 @@ import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup.UUID; +import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; public class GitHubTeamGroup extends GitHubGroup { @@ -29,9 +30,10 @@ private final GitHubOrganisationGroup orgGroup; private final String teamName; - public GitHubTeamGroup(@Assisted GitHubOrganisationGroup orgGroup, + @Inject + GitHubTeamGroup(@Assisted GitHubOrganisationGroup orgGroup, @Assisted String teamName, @Nullable String teamUrl) { - super(uuid(orgGroup.uuid, teamName), teamUrl); + super(uuid(orgGroup.getGroupUUID(), teamName), teamUrl); this.orgGroup = orgGroup; this.teamName = teamName; }