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

java/com/google/gerrit/extensions/common/testing/BUILD

@@ -8,5 +8,6 @@ java_library(
         "//java/com/google/gerrit/extensions/client/testing:client-test-util",
         "//java/com/google/gerrit/truth",
         "//lib:truth",
+        "//lib/jgit/org.eclipse.jgit:jgit",
     ],
 )

java/com/google/gerrit/extensions/common/testing/GitPersonSubject.java

@@ -24,6 +24,8 @@ import com.google.common.truth.Subject;
 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 @@ public class GitPersonSubject extends Subject<GitPersonSubject, GitPerson> {
     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());
+  }
 }

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.reviewdb.client.RefNames;
 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 @@ public class GroupNameNotes extends VersionedMetaData {
   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 @@ public class GroupNameNotes extends VersionedMetaData {
 
   // 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 @@ public class GroupNameNotes extends VersionedMetaData {
     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);

java/com/google/gerrit/server/group/db/GroupRebuilder.java

@@ -25,6 +25,7 @@ import com.google.common.collect.Iterables;
 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.Consumer;
 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 @@ public class GroupRebuilder {
     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 @@ public class GroupRebuilder {
     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());

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",
+    ],
+)

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() {}
+}

javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java

@@ -168,7 +168,7 @@ public class GroupRebuilderIT extends AbstractDaemonTest {
 
   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();

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",
+    ],
+)

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

javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java

@@ -17,11 +17,12 @@ package com.google.gerrit.server.group.db;
 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.reviewdb.client.RefNames;
 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.concurrent.atomic.AtomicInteger;
 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 @@ public class GroupRebuilderTest extends GerritBaseTests {
     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 @@ public class GroupRebuilderTest extends GerritBaseTests {
     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 @@ public class GroupRebuilderTest extends GerritBaseTests {
     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 @@ public class GroupRebuilderTest extends GerritBaseTests {
             .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 @@ public class GroupRebuilderTest extends GerritBaseTests {
             .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 @@ public class GroupRebuilderTest extends GerritBaseTests {
                 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 @@ public class GroupRebuilderTest extends GerritBaseTests {
             .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 @@ public class GroupRebuilderTest extends GerritBaseTests {
             .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 @@ public class GroupRebuilderTest extends GerritBaseTests {
             .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 @@ public class GroupRebuilderTest extends GerritBaseTests {
                 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 @@ public class GroupRebuilderTest extends GerritBaseTests {
                 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 @@ public class GroupRebuilderTest extends GerritBaseTests {
             .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 @@ public class GroupRebuilderTest extends GerritBaseTests {
     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 @@ public class GroupRebuilderTest extends GerritBaseTests {
   }
 
   private ImmutableList<CommitInfo> log(AccountGroup g) throws Exception {
-    ImmutableList<CommitInfo> result = ImmutableList.of();
+    return GroupTestUtil.log(repo, RefNames.refsGroups(g.getGroupUUID()));
-    try (RevWalk rw = new RevWalk(repo)) {
+  }
-      Ref ref = repo.exactRef(RefNames.refsGroups(g.getGroupUUID()));
+
-      if (ref != null) {
+  private ImmutableList<CommitInfo> logGroupNames() throws Exception {
-        rw.sort(RevSort.REVERSE);
+    return GroupTestUtil.log(repo, REFS_GROUPNAMES);
-        rw.setRetainBody(true);
-        rw.markStart(rw.parseCommit(ref.getObjectId()));
-        result = Streams.stream(rw).map(CommitUtil::toCommitInfo).collect(toImmutableList());
-      }
-    }
-    return result;
   }
 
   private static void assertServerCommit(CommitInfo commitInfo, String expectedMessage) {
@@ -477,4 +507,8 @@ public class GroupRebuilderTest extends GerritBaseTests {
   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);
+  }
 }