Add utility for updating group name notes in bulk
This is intended to be used in combination with GroupRebuilder to batch
many rebuilds together, hence the combined test, but it doesn't actually
need to live in the same class.
Change-Id: I8c2673774f2bf29e726d1cd5cf87837cb46954b5
diff --git a/java/com/google/gerrit/extensions/common/testing/BUILD b/java/com/google/gerrit/extensions/common/testing/BUILD
index 160c14a..54d44c6 100644
--- a/java/com/google/gerrit/extensions/common/testing/BUILD
+++ b/java/com/google/gerrit/extensions/common/testing/BUILD
@@ -8,5 +8,6 @@
"//java/com/google/gerrit/extensions/client/testing:client-test-util",
"//java/com/google/gerrit/truth",
"//lib:truth",
+ "//lib/jgit/org.eclipse.jgit:jgit",
],
)
diff --git a/java/com/google/gerrit/extensions/common/testing/GitPersonSubject.java b/java/com/google/gerrit/extensions/common/testing/GitPersonSubject.java
index 7495a52..cdbef34 100644
--- a/java/com/google/gerrit/extensions/common/testing/GitPersonSubject.java
+++ b/java/com/google/gerrit/extensions/common/testing/GitPersonSubject.java
@@ -24,6 +24,8 @@
import com.google.common.truth.Truth;
import com.google.gerrit.extensions.common.GitPerson;
import java.sql.Timestamp;
+import java.util.Date;
+import org.eclipse.jgit.lib.PersonIdent;
public class GitPersonSubject extends Subject<GitPersonSubject, GitPerson> {
@@ -65,4 +67,14 @@
date().isEqualTo(other.date);
tz().isEqualTo(other.tz);
}
+
+ public void matches(PersonIdent ident) {
+ isNotNull();
+ name().isEqualTo(ident.getName());
+ email().isEqualTo(ident.getEmailAddress());
+ Truth.assertThat(new Date(actual().date.getTime()))
+ .named("rounded date")
+ .isEqualTo(ident.getWhen());
+ tz().isEqualTo(ident.getTimeZoneOffset());
+ }
}
diff --git a/java/com/google/gerrit/server/group/db/GroupNameNotes.java b/java/com/google/gerrit/server/group/db/GroupNameNotes.java
index cd8effd..c57e69e 100644
--- a/java/com/google/gerrit/server/group/db/GroupNameNotes.java
+++ b/java/com/google/gerrit/server/group/db/GroupNameNotes.java
@@ -15,9 +15,12 @@
package com.google.gerrit.server.group.db;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.hash.Hashing;
import com.google.gerrit.common.Nullable;
@@ -27,20 +30,25 @@
import com.google.gerrit.server.git.VersionedMetaData;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
// TODO(aliceks): Add Javadoc descriptions.
public class GroupNameNotes extends VersionedMetaData {
@@ -48,6 +56,63 @@
private static final String UUID_PARAM = "uuid";
private static final String NAME_PARAM = "name";
+ @VisibleForTesting
+ static final String UNIQUE_REF_ERROR = "GroupReference collection must contain unique references";
+
+ public static void updateGroupNames(
+ Repository allUsersRepo,
+ ObjectInserter inserter,
+ BatchRefUpdate bru,
+ Collection<GroupReference> groupReferences,
+ PersonIdent ident)
+ throws IOException {
+ // Not strictly necessary for iteration; throws IAE if it encounters duplicates, which is nice.
+ ImmutableBiMap<AccountGroup.UUID, String> biMap = toBiMap(groupReferences);
+
+ try (ObjectReader reader = inserter.newReader();
+ RevWalk rw = new RevWalk(reader)) {
+ // Always start from an empty map, discarding old notes.
+ NoteMap noteMap = NoteMap.newEmptyMap();
+ Ref ref = allUsersRepo.exactRef(RefNames.REFS_GROUPNAMES);
+ RevCommit oldCommit = ref != null ? rw.parseCommit(ref.getObjectId()) : null;
+
+ for (Map.Entry<AccountGroup.UUID, String> e : biMap.entrySet()) {
+ AccountGroup.NameKey nameKey = new AccountGroup.NameKey(e.getValue());
+ ObjectId noteKey = getNoteKey(nameKey);
+ noteMap.set(noteKey, getAsNoteData(e.getKey(), nameKey), inserter);
+ }
+
+ ObjectId newTreeId = noteMap.writeTree(inserter);
+ if (oldCommit != null && newTreeId.equals(oldCommit.getTree())) {
+ return;
+ }
+ CommitBuilder cb = new CommitBuilder();
+ if (oldCommit != null) {
+ cb.addParentId(oldCommit);
+ }
+ cb.setTreeId(newTreeId);
+ cb.setAuthor(ident);
+ cb.setCommitter(ident);
+ int n = groupReferences.size();
+ cb.setMessage("Store " + n + " group name" + (n != 1 ? "s" : ""));
+ ObjectId newId = inserter.insert(cb).copy();
+
+ ObjectId oldId = oldCommit != null ? oldCommit.copy() : ObjectId.zeroId();
+ bru.addCommand(new ReceiveCommand(oldId, newId, RefNames.REFS_GROUPNAMES));
+ }
+ }
+
+ private static ImmutableBiMap<AccountGroup.UUID, String> toBiMap(
+ Collection<GroupReference> groupReferences) {
+ try {
+ return groupReferences
+ .stream()
+ .collect(toImmutableBiMap(gr -> gr.getUUID(), gr -> gr.getName()));
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException(UNIQUE_REF_ERROR, e);
+ }
+ }
+
private final AccountGroup.UUID groupUuid;
private final Optional<AccountGroup.NameKey> oldGroupName;
private final Optional<AccountGroup.NameKey> newGroupName;
@@ -197,7 +262,8 @@
// Use the same approach as ExternalId.Key.sha1().
@SuppressWarnings("deprecation")
- private static ObjectId getNoteKey(AccountGroup.NameKey groupName) {
+ @VisibleForTesting
+ static ObjectId getNoteKey(AccountGroup.NameKey groupName) {
return ObjectId.fromRaw(Hashing.sha1().hashString(groupName.get(), UTF_8).asBytes());
}
@@ -208,7 +274,8 @@
return config.toText();
}
- private static GroupReference getGroupReference(ObjectReader reader, ObjectId noteDataBlobId)
+ @VisibleForTesting
+ public static GroupReference getGroupReference(ObjectReader reader, ObjectId noteDataBlobId)
throws IOException, ConfigInvalidException {
byte[] noteData = reader.open(noteDataBlobId, OBJ_BLOB).getCachedBytes();
return getFromNoteData(noteData);
diff --git a/java/com/google/gerrit/server/group/db/GroupRebuilder.java b/java/com/google/gerrit/server/group/db/GroupRebuilder.java
index 89af751..7f8d8a9 100644
--- a/java/com/google/gerrit/server/group/db/GroupRebuilder.java
+++ b/java/com/google/gerrit/server/group/db/GroupRebuilder.java
@@ -25,6 +25,7 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Sets;
+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;
@@ -56,6 +57,7 @@
import java.util.function.Function;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
@@ -112,7 +114,7 @@
this.getGroupNameFunc = getGroupNameFunc;
}
- public void rebuild(Repository allUsersRepo, GroupBundle bundle)
+ public void rebuild(Repository allUsersRepo, GroupBundle bundle, @Nullable BatchRefUpdate bru)
throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
GroupConfig groupConfig = GroupConfig.loadForGroup(allUsersRepo, bundle.uuid());
AccountGroup group = bundle.group();
@@ -136,7 +138,7 @@
Map<Key, Collection<Event>> events = toEvents(bundle).asMap();
PersonIdent nowServerIdent = getServerIdent(events);
- MetaDataUpdate md = metaDataUpdateFactory.create(allUsers, allUsersRepo, null);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers, allUsersRepo, bru);
// Creation is done by the server (unlike later audit events).
PersonIdent created = new PersonIdent(nowServerIdent, group.getCreatedOn());
diff --git a/java/com/google/gerrit/server/group/db/testing/BUILD b/java/com/google/gerrit/server/group/db/testing/BUILD
new file mode 100644
index 0000000..1a5b598
--- /dev/null
+++ b/java/com/google/gerrit/server/group/db/testing/BUILD
@@ -0,0 +1,15 @@
+package(default_visibility = ["//visibility:public"])
+
+java_library(
+ name = "testing",
+ testonly = 1,
+ srcs = glob(["*.java"]),
+ deps = [
+ "//java/com/google/gerrit/common:server",
+ "//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/reviewdb:server",
+ "//java/com/google/gerrit/server",
+ "//lib:guava",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+ ],
+)
diff --git a/java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java b/java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java
new file mode 100644
index 0000000..7378b15
--- /dev/null
+++ b/java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.group.db.testing;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.gerrit.server.group.db.GroupNameNotes.getGroupReference;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Streams;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.extensions.common.CommitInfo;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.git.CommitUtil;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.Note;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevSort;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/** Test utilities for low-level NoteDb groups. */
+public class GroupTestUtil {
+ public static ImmutableMap<String, String> readNameToUuidMap(Repository repo) throws Exception {
+ ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
+ try (RevWalk rw = new RevWalk(repo)) {
+ Ref ref = repo.exactRef(RefNames.REFS_GROUPNAMES);
+ if (ref != null) {
+ NoteMap noteMap = NoteMap.read(rw.getObjectReader(), rw.parseCommit(ref.getObjectId()));
+ for (Note note : noteMap) {
+ GroupReference gr = getGroupReference(rw.getObjectReader(), note.getData());
+ result.put(gr.getName(), gr.getUUID().get());
+ }
+ }
+ }
+ return result.build();
+ }
+
+ // TODO(dborowitz): Move somewhere even more common.
+ public static ImmutableList<CommitInfo> log(Repository repo, String refName) throws Exception {
+ try (RevWalk rw = new RevWalk(repo)) {
+ Ref ref = repo.exactRef(refName);
+ if (ref != null) {
+ rw.sort(RevSort.REVERSE);
+ rw.markStart(rw.parseCommit(ref.getObjectId()));
+ return Streams.stream(rw).map(CommitUtil::toCommitInfo).collect(toImmutableList());
+ }
+ }
+ return ImmutableList.of();
+ }
+
+ private GroupTestUtil() {}
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java
index dbbd07e..879b9f7 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java
@@ -168,7 +168,7 @@
private InternalGroup rebuild(InternalGroup group) throws Exception {
try (Repository repo = repoManager.openRepository(allUsers)) {
- rebuilder.rebuild(repo, GroupBundle.fromReviewDb(db, group.getId()));
+ rebuilder.rebuild(repo, GroupBundle.fromReviewDb(db, group.getId()), null);
GroupConfig groupConfig = GroupConfig.loadForGroup(repo, group.getGroupUUID());
Optional<InternalGroup> result = groupConfig.getLoadedGroup();
assertThat(result).isPresent();
diff --git a/javatests/com/google/gerrit/server/group/db/BUILD b/javatests/com/google/gerrit/server/group/db/BUILD
new file mode 100644
index 0000000..65e09f6
--- /dev/null
+++ b/javatests/com/google/gerrit/server/group/db/BUILD
@@ -0,0 +1,19 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+
+junit_tests(
+ name = "db_tests",
+ size = "small",
+ srcs = glob(["*.java"]),
+ deps = [
+ "//java/com/google/gerrit/common:server",
+ "//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/extensions/common/testing:common-test-util",
+ "//java/com/google/gerrit/reviewdb:server",
+ "//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/group/db/testing",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:truth",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/jgit/org.eclipse.jgit.junit:junit",
+ ],
+)
diff --git a/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java
new file mode 100644
index 0000000..53bf866
--- /dev/null
+++ b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java
@@ -0,0 +1,213 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.group.db;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.extensions.common.testing.CommitInfoSubject.assertThat;
+import static com.google.gerrit.reviewdb.client.RefNames.REFS_GROUPNAMES;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.extensions.common.CommitInfo;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.config.AllUsersNameProvider;
+import com.google.gerrit.server.group.db.testing.GroupTestUtil;
+import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gerrit.testing.GerritBaseTests;
+import com.google.gerrit.testing.TestTimeUtil;
+import java.util.Arrays;
+import java.util.TimeZone;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class GroupNameNotesTest extends GerritBaseTests {
+ private static final String SERVER_NAME = "Gerrit Server";
+ private static final String SERVER_EMAIL = "noreply@gerritcodereview.com";
+ private static final TimeZone TZ = TimeZone.getTimeZone("America/Los_Angeles");
+
+ private AtomicInteger idCounter;
+ private Repository repo;
+
+ @Before
+ public void setUp() {
+ TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ idCounter = new AtomicInteger();
+ repo = new InMemoryRepository(new DfsRepositoryDescription(AllUsersNameProvider.DEFAULT));
+ }
+
+ @After
+ public void tearDown() {
+ TestTimeUtil.useSystemTime();
+ }
+
+ @Test
+ public void updateGroupNames() throws Exception {
+ GroupReference g1 = newGroup("a");
+ GroupReference g2 = newGroup("b");
+
+ PersonIdent ident = newPersonIdent();
+ updateGroupNames(ident, g1, g2);
+
+ ImmutableList<CommitInfo> log = log();
+ assertThat(log).hasSize(1);
+ assertThat(log.get(0)).parents().isEmpty();
+ assertThat(log.get(0)).message().isEqualTo("Store 2 group names");
+ assertThat(log.get(0)).author().matches(ident);
+ assertThat(log.get(0)).committer().matches(ident);
+
+ assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("a", "a-1", "b", "b-2");
+
+ // Updating the same set of names is a no-op.
+ String commit = log.get(0).commit;
+ updateGroupNames(newPersonIdent(), g1, g2);
+ log = log();
+ assertThat(log).hasSize(1);
+ assertThat(log.get(0)).commit().isEqualTo(commit);
+ }
+
+ @Test
+ public void updateGroupNamesOverwritesExistingNotes() throws Exception {
+ GroupReference g1 = newGroup("a");
+ GroupReference g2 = newGroup("b");
+
+ TestRepository<?> tr = new TestRepository<>(repo);
+ ObjectId k1 = getNoteKey(g1);
+ ObjectId k2 = getNoteKey(g2);
+ ObjectId k3 = GroupNameNotes.getNoteKey(new AccountGroup.NameKey("c"));
+ PersonIdent ident = newPersonIdent();
+ ObjectId origCommitId =
+ tr.branch(REFS_GROUPNAMES)
+ .commit()
+ .message("Prepopulate group name")
+ .author(ident)
+ .committer(ident)
+ .add(k1.name(), "[group]\n\tuuid = a-1\n\tname = a\nanotherKey = foo\n")
+ .add(k2.name(), "[group]\n\tuuid = a-1\n\tname = b\n")
+ .add(k3.name(), "[group]\n\tuuid = c-3\n\tname = c\n")
+ .create()
+ .copy();
+
+ ident = newPersonIdent();
+ updateGroupNames(ident, g1, g2);
+
+ assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("a", "a-1", "b", "b-2");
+
+ ImmutableList<CommitInfo> log = log();
+ assertThat(log).hasSize(2);
+ assertThat(log.get(0)).commit().isEqualTo(origCommitId.name());
+
+ assertThat(log.get(1)).message().isEqualTo("Store 2 group names");
+ assertThat(log.get(1)).author().matches(ident);
+ assertThat(log.get(1)).committer().matches(ident);
+
+ try (RevWalk rw = new RevWalk(repo)) {
+ ObjectReader reader = rw.getObjectReader();
+ NoteMap noteMap =
+ NoteMap.read(reader, rw.parseCommit(ObjectId.fromString(log.get(1).commit)));
+ String note = new String(reader.open(noteMap.get(k1), OBJ_BLOB).getCachedBytes(), UTF_8);
+ // Old note content was overwritten.
+ assertThat(note).isEqualTo("[group]\n\tuuid = a-1\n\tname = a\n");
+ }
+ }
+
+ @Test
+ public void updateGroupNamesWithEmptyCollectionClearsAllNotes() throws Exception {
+ GroupReference g1 = newGroup("a");
+ GroupReference g2 = newGroup("b");
+
+ PersonIdent ident = newPersonIdent();
+ updateGroupNames(ident, g1, g2);
+
+ assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("a", "a-1", "b", "b-2");
+
+ updateGroupNames(ident);
+
+ assertThat(GroupTestUtil.readNameToUuidMap(repo)).isEmpty();
+
+ ImmutableList<CommitInfo> log = log();
+ assertThat(log).hasSize(2);
+ assertThat(log.get(1)).message().isEqualTo("Store 0 group names");
+ }
+
+ @Test
+ public void updateGroupNamesRejectsNonOneToOneGroupReferences() throws Exception {
+ assertIllegalArgument(
+ new GroupReference(new AccountGroup.UUID("uuid1"), "name1"),
+ new GroupReference(new AccountGroup.UUID("uuid1"), "name2"));
+ assertIllegalArgument(
+ new GroupReference(new AccountGroup.UUID("uuid1"), "name1"),
+ new GroupReference(new AccountGroup.UUID("uuid2"), "name1"));
+ assertIllegalArgument(
+ new GroupReference(new AccountGroup.UUID("uuid1"), "name1"),
+ new GroupReference(new AccountGroup.UUID("uuid1"), "name1"));
+ }
+
+ private GroupReference newGroup(String name) {
+ int id = idCounter.incrementAndGet();
+ return new GroupReference(new AccountGroup.UUID(name + "-" + id), name);
+ }
+
+ private static PersonIdent newPersonIdent() {
+ return new PersonIdent(SERVER_NAME, SERVER_EMAIL, TimeUtil.nowTs(), TZ);
+ }
+
+ private static ObjectId getNoteKey(GroupReference g) {
+ return GroupNameNotes.getNoteKey(new AccountGroup.NameKey(g.getName()));
+ }
+
+ private void updateGroupNames(PersonIdent ident, GroupReference... groupRefs) throws Exception {
+ try (ObjectInserter inserter = repo.newObjectInserter()) {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ GroupNameNotes.updateGroupNames(repo, inserter, bru, Arrays.asList(groupRefs), ident);
+ inserter.flush();
+ RefUpdateUtil.executeChecked(bru, repo);
+ }
+ }
+
+ private void assertIllegalArgument(GroupReference... groupRefs) throws Exception {
+ try (ObjectInserter inserter = repo.newObjectInserter()) {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ PersonIdent ident = newPersonIdent();
+ try {
+ GroupNameNotes.updateGroupNames(repo, inserter, bru, Arrays.asList(groupRefs), ident);
+ assert_().fail("Expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ assertThat(e).hasMessageThat().isEqualTo(GroupNameNotes.UNIQUE_REF_ERROR);
+ }
+ }
+ }
+
+ private ImmutableList<CommitInfo> log() throws Exception {
+ return GroupTestUtil.log(repo, REFS_GROUPNAMES);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java b/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java
index 863d195..4e9fc7f 100644
--- a/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java
+++ b/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java
@@ -17,11 +17,12 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.common.testing.CommitInfoSubject.assertThat;
+import static com.google.gerrit.reviewdb.client.RefNames.REFS_GROUPNAMES;
import static com.google.gerrit.server.group.db.GroupBundle.builder;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Streams;
import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -33,9 +34,10 @@
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.CommitUtil;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.group.InternalGroup;
+import com.google.gerrit.server.group.db.testing.GroupTestUtil;
+import com.google.gerrit.server.update.RefUpdateUtil;
import com.google.gerrit.testing.GerritBaseTests;
import com.google.gerrit.testing.TestTimeUtil;
import java.sql.Timestamp;
@@ -45,11 +47,10 @@
import java.util.stream.IntStream;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevSort;
-import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -70,7 +71,7 @@
repo = new InMemoryRepository(new DfsRepositoryDescription(AllUsersNameProvider.DEFAULT));
rebuilder =
new GroupRebuilder(
- () -> new PersonIdent(SERVER_NAME, SERVER_EMAIL, TimeUtil.nowTs(), TZ),
+ GroupRebuilderTest::newPersonIdent,
new AllUsersName(AllUsersNameProvider.DEFAULT),
(project, repo, batch) ->
new MetaDataUpdate(GitReferenceUpdated.DISABLED, project, repo, batch),
@@ -94,12 +95,13 @@
AccountGroup g = newGroup("a");
GroupBundle b = builder().group(g).build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
assertThat(log).hasSize(1);
assertCommit(log.get(0), "Create group", SERVER_NAME, SERVER_EMAIL);
+ assertThat(logGroupNames()).isEmpty();
}
@Test
@@ -110,7 +112,7 @@
g.setVisibleToAll(true);
GroupBundle b = builder().group(g).build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -128,7 +130,7 @@
.byId(byId(g, "x"), byId(g, "y"))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -157,7 +159,7 @@
.memberAudit(addMember(g, 1, 8, t2), addAndRemoveMember(g, 2, 8, t1, 9, t3))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -183,7 +185,7 @@
addMember(g, 2, 8, TimeUtil.nowTs()))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -207,7 +209,7 @@
.memberAudit(addMember(g, 1, 8, TimeUtil.nowTs()))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -232,7 +234,7 @@
.byIdAudit(addById(g, "x", 8, t2), addAndRemoveById(g, "y", 8, t1, 9, t3))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -253,7 +255,7 @@
.byIdAudit(addById(g, "x", 8, TimeUtil.nowTs()))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -283,7 +285,7 @@
addAndRemoveById(g, "z", user, ts, user, ts))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -330,7 +332,7 @@
addById(g, "x", user1, ts), addById(g, "y", user2, ts), addById(g, "z", user1, ts))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -365,7 +367,7 @@
.byIdAudit(addById(g, "x", 8, future))
.build();
- rebuilder.rebuild(repo, b);
+ rebuilder.rebuild(repo, b, null);
assertThat(reload(g)).isEqualTo(b.toInternalGroup());
ImmutableList<CommitInfo> log = log(g);
@@ -380,6 +382,40 @@
assertThat(TimeUtil.nowTs()).isLessThan(future);
}
+ @Test
+ public void combineWithBatchGroupNameNotes() throws Exception {
+ AccountGroup g1 = newGroup("a");
+ AccountGroup g2 = newGroup("b");
+
+ GroupBundle b1 = builder().group(g1).build();
+ GroupBundle b2 = builder().group(g2).build();
+
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+
+ rebuilder.rebuild(repo, b1, bru);
+ rebuilder.rebuild(repo, b2, bru);
+ try (ObjectInserter inserter = repo.newObjectInserter()) {
+ ImmutableList<GroupReference> refs =
+ ImmutableList.of(GroupReference.forGroup(g1), GroupReference.forGroup(g2));
+ GroupNameNotes.updateGroupNames(repo, inserter, bru, refs, newPersonIdent());
+ inserter.flush();
+ }
+
+ assertThat(log(g1)).isEmpty();
+ assertThat(log(g2)).isEmpty();
+ assertThat(logGroupNames()).isEmpty();
+
+ RefUpdateUtil.executeChecked(bru, repo);
+
+ assertThat(log(g1)).hasSize(1);
+ assertThat(log(g2)).hasSize(1);
+ assertThat(logGroupNames()).hasSize(1);
+ assertThat(reload(g1)).isEqualTo(b1.toInternalGroup());
+ assertThat(reload(g2)).isEqualTo(b2.toInternalGroup());
+
+ assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("a", "a-1", "b", "b-2");
+ }
+
private InternalGroup reload(AccountGroup g) throws Exception {
return removeRefState(GroupConfig.loadForGroup(repo, g.getGroupUUID()).getLoadedGroup().get());
}
@@ -444,17 +480,11 @@
}
private ImmutableList<CommitInfo> log(AccountGroup g) throws Exception {
- ImmutableList<CommitInfo> result = ImmutableList.of();
- try (RevWalk rw = new RevWalk(repo)) {
- Ref ref = repo.exactRef(RefNames.refsGroups(g.getGroupUUID()));
- if (ref != null) {
- rw.sort(RevSort.REVERSE);
- rw.setRetainBody(true);
- rw.markStart(rw.parseCommit(ref.getObjectId()));
- result = Streams.stream(rw).map(CommitUtil::toCommitInfo).collect(toImmutableList());
- }
- }
- return result;
+ return GroupTestUtil.log(repo, RefNames.refsGroups(g.getGroupUUID()));
+ }
+
+ private ImmutableList<CommitInfo> logGroupNames() throws Exception {
+ return GroupTestUtil.log(repo, REFS_GROUPNAMES);
}
private static void assertServerCommit(CommitInfo commitInfo, String expectedMessage) {
@@ -477,4 +507,8 @@
private static InternalGroup removeRefState(InternalGroup group) throws Exception {
return group.toBuilder().setRefState(null).build();
}
+
+ private static PersonIdent newPersonIdent() {
+ return new PersonIdent(SERVER_NAME, SERVER_EMAIL, TimeUtil.nowTs(), TZ);
+ }
}
