Merge "Extend project resetter to groups"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 3488588..fbf3b84 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -334,14 +334,14 @@
// Don't reset all refs so that refs/sequences/changes is not touched and change IDs are
// not reused.
.reset(allProjects, RefNames.REFS_CONFIG)
- // Don't reset group branches since this would make the groups inconsistent between
- // ReviewDb and NoteDb.
// Don't reset refs/sequences/accounts so that account IDs are not reused.
.reset(
allUsers,
RefNames.REFS_CONFIG,
RefNames.REFS_USERS + "*",
RefNames.REFS_EXTERNAL_IDS,
+ RefNames.REFS_GROUPNAMES,
+ RefNames.REFS_GROUPS + "*",
RefNames.REFS_STARRED_CHANGES + "*",
RefNames.REFS_DRAFT_COMMENTS + "*");
}
diff --git a/java/com/google/gerrit/acceptance/ProjectResetter.java b/java/com/google/gerrit/acceptance/ProjectResetter.java
index 86718be..7d17285 100644
--- a/java/com/google/gerrit/acceptance/ProjectResetter.java
+++ b/java/com/google/gerrit/acceptance/ProjectResetter.java
@@ -24,13 +24,17 @@
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.GroupIncludeCache;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.RefState;
import com.google.gerrit.server.index.account.AccountIndexer;
+import com.google.gerrit.server.index.group.GroupIndexer;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RefPatternMatcher;
import com.google.inject.Inject;
@@ -88,6 +92,9 @@
@Nullable private final AccountCreator accountCreator;
@Nullable private final AccountCache accountCache;
@Nullable private final AccountIndexer accountIndexer;
+ @Nullable private final GroupCache groupCache;
+ @Nullable private final GroupIncludeCache groupIncludeCache;
+ @Nullable private final GroupIndexer groupIndexer;
@Nullable private final ProjectCache projectCache;
@Inject
@@ -97,12 +104,18 @@
@Nullable AccountCreator accountCreator,
@Nullable AccountCache accountCache,
@Nullable AccountIndexer accountIndexer,
+ @Nullable GroupCache groupCache,
+ @Nullable GroupIncludeCache groupIncludeCache,
+ @Nullable GroupIndexer groupIndexer,
@Nullable ProjectCache projectCache) {
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.accountCreator = accountCreator;
this.accountCache = accountCache;
this.accountIndexer = accountIndexer;
+ this.groupCache = groupCache;
+ this.groupIncludeCache = groupIncludeCache;
+ this.groupIndexer = groupIndexer;
this.projectCache = projectCache;
}
@@ -113,6 +126,9 @@
accountCreator,
accountCache,
accountIndexer,
+ groupCache,
+ groupIncludeCache,
+ groupIndexer,
projectCache,
input.refsByProject);
}
@@ -139,12 +155,18 @@
@Inject private AllUsersName allUsersName;
@Inject @Nullable private AccountCreator accountCreator;
@Inject @Nullable private AccountCache accountCache;
+ @Inject @Nullable private GroupCache groupCache;
+ @Inject @Nullable private GroupIncludeCache groupIncludeCache;
+ @Inject @Nullable private GroupIndexer groupIndexer;
@Inject @Nullable private AccountIndexer accountIndexer;
@Inject @Nullable private ProjectCache projectCache;
private final Multimap<Project.NameKey, String> refsPatternByProject;
+
+ // State to which to reset to.
private final Multimap<Project.NameKey, RefState> savedRefStatesByProject;
+ // Results of the resetting
private Multimap<Project.NameKey, String> keptRefsByProject;
private Multimap<Project.NameKey, String> restoredRefsByProject;
private Multimap<Project.NameKey, String> deletedRefsByProject;
@@ -155,6 +177,9 @@
@Nullable AccountCreator accountCreator,
@Nullable AccountCache accountCache,
@Nullable AccountIndexer accountIndexer,
+ @Nullable GroupCache groupCache,
+ @Nullable GroupIncludeCache groupIncludeCache,
+ @Nullable GroupIndexer groupIndexer,
@Nullable ProjectCache projectCache,
Multimap<Project.NameKey, String> refPatternByProject)
throws IOException {
@@ -163,6 +188,9 @@
this.accountCreator = accountCreator;
this.accountCache = accountCache;
this.accountIndexer = accountIndexer;
+ this.groupCache = groupCache;
+ this.groupIndexer = groupIndexer;
+ this.groupIncludeCache = groupIncludeCache;
this.projectCache = projectCache;
this.refsPatternByProject = refPatternByProject;
this.savedRefStatesByProject = readRefStates();
@@ -265,8 +293,8 @@
private void evictCachesAndReindex() throws IOException {
evictAndReindexProjects();
evictAndReindexAccounts();
+ evictAndReindexGroups();
- // TODO(ekempin): Evict groups from cache if group refs were modified.
// TODO(ekempin): Reindex changes if starred-changes refs in All-Users were modified.
}
@@ -329,16 +357,47 @@
}
}
+ /** Evict groups that were modified. */
+ private void evictAndReindexGroups() throws IOException {
+ if (groupCache != null || groupIndexer != null) {
+ Set<AccountGroup.UUID> modifiedGroups =
+ new HashSet<>(groupUUIDs(restoredRefsByProject.get(allUsersName)));
+ Set<AccountGroup.UUID> deletedGroups =
+ new HashSet<>(groupUUIDs(deletedRefsByProject.get(allUsersName)));
+
+ // Evict and reindex all modified and deleted groups.
+ for (AccountGroup.UUID uuid : Sets.union(modifiedGroups, deletedGroups)) {
+ evictAndReindexGroup(uuid);
+ }
+ }
+ }
+
private void evictAndReindexAccount(Account.Id accountId) throws IOException {
if (accountCache != null) {
accountCache.evict(accountId);
}
-
+ if (groupIncludeCache != null) {
+ groupIncludeCache.evictGroupsWithMember(accountId);
+ }
if (accountIndexer != null) {
accountIndexer.index(accountId);
}
}
+ private void evictAndReindexGroup(AccountGroup.UUID uuid) throws IOException {
+ if (groupCache != null) {
+ groupCache.evict(uuid);
+ }
+
+ if (groupIncludeCache != null) {
+ groupIncludeCache.evictParentGroupsOf(uuid);
+ }
+
+ if (groupIndexer != null) {
+ groupIndexer.index(uuid);
+ }
+ }
+
private Set<Account.Id> accountIds(Collection<String> refs) {
return refs.stream()
.filter(r -> r.startsWith(REFS_USERS))
@@ -346,4 +405,12 @@
.filter(Objects::nonNull)
.collect(toSet());
}
+
+ private Set<AccountGroup.UUID> groupUUIDs(Collection<String> refs) {
+ return refs.stream()
+ .filter(RefNames::isRefsGroups)
+ .map(r -> AccountGroup.UUID.fromRef(r))
+ .filter(Objects::nonNull)
+ .collect(toSet());
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java
index fce28de..bed2504 100644
--- a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java
+++ b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java
@@ -20,12 +20,16 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.GroupIncludeCache;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.index.account.AccountIndexer;
+import com.google.gerrit.server.index.group.GroupIndexer;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.testing.GerritBaseTests;
import com.google.gerrit.testing.InMemoryRepositoryManager;
@@ -224,7 +228,7 @@
Ref nonMetaConfig = createRef("refs/heads/master");
try (ProjectResetter resetProject =
- builder(null, null, null, projectCache)
+ builder(null, null, null, null, null, null, projectCache)
.build(new ProjectResetter.Config().reset(project).reset(project2))) {
updateRef(nonMetaConfig);
updateRef(repo2, metaConfig);
@@ -244,7 +248,7 @@
EasyMock.replay(projectCache);
try (ProjectResetter resetProject =
- builder(null, null, null, projectCache)
+ builder(null, null, null, null, null, null, projectCache)
.build(new ProjectResetter.Config().reset(project).reset(project2))) {
createRef("refs/heads/master");
createRef(repo2, RefNames.REFS_CONFIG);
@@ -274,7 +278,7 @@
Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(2)));
try (ProjectResetter resetProject =
- builder(null, accountCache, accountIndexer, null)
+ builder(null, accountCache, accountIndexer, null, null, null, null)
.build(new ProjectResetter.Config().reset(project).reset(allUsers))) {
updateRef(nonUserBranch);
updateRef(allUsersRepo, userBranch);
@@ -300,7 +304,7 @@
EasyMock.replay(accountIndexer);
try (ProjectResetter resetProject =
- builder(null, accountCache, accountIndexer, null)
+ builder(null, accountCache, accountIndexer, null, null, null, null)
.build(new ProjectResetter.Config().reset(project).reset(allUsers))) {
// Non-user branch because it's not in All-Users.
createRef(RefNames.refsUsers(new Account.Id(2)));
@@ -339,7 +343,7 @@
Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(3)));
try (ProjectResetter resetProject =
- builder(null, accountCache, accountIndexer, null)
+ builder(null, accountCache, accountIndexer, null, null, null, null)
.build(new ProjectResetter.Config().reset(project).reset(allUsers))) {
updateRef(nonUserBranch);
updateRef(allUsersRepo, externalIds);
@@ -376,7 +380,7 @@
Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(3)));
try (ProjectResetter resetProject =
- builder(null, accountCache, accountIndexer, null)
+ builder(null, accountCache, accountIndexer, null, null, null, null)
.build(new ProjectResetter.Config().reset(project).reset(allUsers))) {
updateRef(nonUserBranch);
createRef(allUsersRepo, RefNames.REFS_EXTERNAL_IDS);
@@ -398,7 +402,7 @@
EasyMock.replay(accountCreator);
try (ProjectResetter resetProject =
- builder(accountCreator, null, null, null)
+ builder(accountCreator, null, null, null, null, null, null)
.build(new ProjectResetter.Config().reset(project).reset(allUsers))) {
createRef(allUsersRepo, RefNames.refsUsers(accountId));
}
@@ -406,6 +410,39 @@
EasyMock.verify(accountCreator);
}
+ @Test
+ public void groupEviction() throws Exception {
+ AccountGroup.UUID uuid1 = new AccountGroup.UUID("abcd1");
+ AccountGroup.UUID uuid2 = new AccountGroup.UUID("abcd2");
+ AccountGroup.UUID uuid3 = new AccountGroup.UUID("abcd3");
+ Project.NameKey allUsers = new Project.NameKey(AllUsersNameProvider.DEFAULT);
+ Repository allUsersRepo = repoManager.createRepository(allUsers);
+
+ GroupCache cache = EasyMock.createNiceMock(GroupCache.class);
+ GroupIndexer indexer = EasyMock.createNiceMock(GroupIndexer.class);
+ GroupIncludeCache includeCache = EasyMock.createNiceMock(GroupIncludeCache.class);
+ cache.evict(uuid2);
+ indexer.index(uuid2);
+ includeCache.evictParentGroupsOf(uuid2);
+ cache.evict(uuid3);
+ indexer.index(uuid3);
+ includeCache.evictParentGroupsOf(uuid3);
+ EasyMock.expectLastCall();
+
+ EasyMock.replay(cache, indexer);
+
+ Ref ref1 = createRef(allUsersRepo, RefNames.refsGroups(uuid1));
+ Ref ref2 = createRef(allUsersRepo, RefNames.refsGroups(uuid2));
+ try (ProjectResetter resetProject =
+ builder(null, null, null, cache, includeCache, indexer, null)
+ .build(new ProjectResetter.Config().reset(project).reset(allUsers))) {
+ updateRef(allUsersRepo, ref2);
+ createRef(allUsersRepo, RefNames.refsGroups(uuid3));
+ }
+
+ EasyMock.verify(cache, indexer);
+ }
+
private Ref createRef(String ref) throws IOException {
return createRef(repo, ref);
}
@@ -474,13 +511,16 @@
}
private ProjectResetter.Builder builder() {
- return builder(null, null, null, null);
+ return builder(null, null, null, null, null, null, null);
}
private ProjectResetter.Builder builder(
@Nullable AccountCreator accountCreator,
@Nullable AccountCache accountCache,
@Nullable AccountIndexer accountIndexer,
+ @Nullable GroupCache groupCache,
+ @Nullable GroupIncludeCache groupIncludeCache,
+ @Nullable GroupIndexer groupIndexer,
@Nullable ProjectCache projectCache) {
return new ProjectResetter.Builder(
repoManager,
@@ -488,6 +528,9 @@
accountCreator,
accountCache,
accountIndexer,
+ groupCache,
+ groupIncludeCache,
+ groupIndexer,
projectCache);
}
}
