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;
}
