Merge "Compare incoming change-id to target change"
diff --git a/java/com/google/gerrit/common/data/testing/BUILD b/java/com/google/gerrit/common/data/testing/BUILD
new file mode 100644
index 0000000..83f1c06
--- /dev/null
+++ b/java/com/google/gerrit/common/data/testing/BUILD
@@ -0,0 +1,11 @@
+java_library(
+    name = "common-data-test-util",
+    testonly = 1,
+    srcs = glob(["**/*.java"]),
+    visibility = ["//visibility:public"],
+    deps = [
+        "//java/com/google/gerrit/common:server",
+        "//java/com/google/gerrit/reviewdb:server",
+        "//lib:truth",
+    ],
+)
diff --git a/java/com/google/gerrit/common/data/testing/GroupReferenceSubject.java b/java/com/google/gerrit/common/data/testing/GroupReferenceSubject.java
new file mode 100644
index 0000000..1988d66
--- /dev/null
+++ b/java/com/google/gerrit/common/data/testing/GroupReferenceSubject.java
@@ -0,0 +1,48 @@
+// Copyright (C) 2018 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.common.data.testing;
+
+import static com.google.common.truth.Truth.assertAbout;
+
+import com.google.common.truth.ComparableSubject;
+import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.StringSubject;
+import com.google.common.truth.Subject;
+import com.google.common.truth.Truth;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+
+public class GroupReferenceSubject extends Subject<GroupReferenceSubject, GroupReference> {
+
+  public static GroupReferenceSubject assertThat(GroupReference group) {
+    return assertAbout(GroupReferenceSubject::new).that(group);
+  }
+
+  private GroupReferenceSubject(FailureMetadata metadata, GroupReference group) {
+    super(metadata, group);
+  }
+
+  public ComparableSubject<?, AccountGroup.UUID> groupUuid() {
+    isNotNull();
+    GroupReference group = actual();
+    return Truth.assertThat(group.getUUID()).named("groupUuid");
+  }
+
+  public StringSubject name() {
+    isNotNull();
+    GroupReference group = actual();
+    return Truth.assertThat(group.getName()).named("name");
+  }
+}
diff --git a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
index 4c8918d..88154b0 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
@@ -224,7 +224,9 @@
     detail.setOwnerOf(ownerOf);
     detail.setCanUpload(
         canWriteProjectConfig
-            || (checkReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE)));
+            || (checkReadConfig
+                && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE)
+                && projectState.statePermitsWrite()));
     detail.setConfigVisible(canWriteProjectConfig || checkReadConfig);
     detail.setGroupInfo(buildGroupInfo(local));
     detail.setLabelTypes(projectState.getLabelTypes());
diff --git a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
index 0240e2e..e44d680 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
@@ -110,7 +110,7 @@
   public final T call()
       throws NoSuchProjectException, IOException, ConfigInvalidException, InvalidNameException,
           NoSuchGroupException, OrmException, UpdateParentFailedException,
-          PermissionDeniedException, PermissionBackendException {
+          PermissionDeniedException, PermissionBackendException, ResourceConflictException {
     try {
       contributorAgreements.check(projectName, user);
     } catch (AuthException e) {
@@ -195,7 +195,7 @@
   protected abstract T updateProjectConfig(
       ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
       throws IOException, NoSuchProjectException, ConfigInvalidException, OrmException,
-          PermissionDeniedException, PermissionBackendException;
+          PermissionDeniedException, PermissionBackendException, ResourceConflictException;
 
   private void replace(ProjectConfig config, Set<String> toDelete, AccessSection section)
       throws NoSuchGroupException {
diff --git a/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
index 81957d6..497354d 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
@@ -23,6 +23,7 @@
 import com.google.gerrit.common.errors.PermissionDeniedException;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Change;
@@ -134,7 +135,7 @@
   protected Change.Id updateProjectConfig(
       ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
       throws IOException, OrmException, PermissionDeniedException, PermissionBackendException,
-          ConfigInvalidException {
+          ConfigInvalidException, ResourceConflictException {
     PermissionBackend.ForProject perm = permissionBackend.user(user).project(config.getName());
     if (!check(perm, ProjectPermission.READ_CONFIG)) {
       throw new PermissionDeniedException(RefNames.REFS_CONFIG + " not visible");
@@ -145,6 +146,8 @@
       throw new PermissionDeniedException("cannot create change for " + RefNames.REFS_CONFIG);
     }
 
+    projectCache.checkedGet(config.getName()).checkStatePermitsWrite();
+
     md.setInsertChangeId(true);
     Change.Id changeId = new Change.Id(seq.nextChangeId());
     RevCommit commit =
diff --git a/java/com/google/gerrit/pgm/init/GroupsOnInit.java b/java/com/google/gerrit/pgm/init/GroupsOnInit.java
index 3385244..7fe227d 100644
--- a/java/com/google/gerrit/pgm/init/GroupsOnInit.java
+++ b/java/com/google/gerrit/pgm/init/GroupsOnInit.java
@@ -150,7 +150,7 @@
       File allUsersRepoPath = getPathToAllUsersRepository();
       if (allUsersRepoPath != null) {
         try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
-          return GroupNameNotes.loadAllGroupReferences(allUsersRepo).stream();
+          return GroupNameNotes.loadAllGroups(allUsersRepo).stream();
         }
       }
       return Stream.empty();
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index eb3559a..292fa7a 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1521,6 +1521,10 @@
       reject(cmd, denied.getMessage());
       return;
     }
+    if (!projectState.statePermitsWrite()) {
+      reject(cmd, "project state does not permit write");
+      return;
+    }
 
     if (magicBranch.isPrivate && magicBranch.removePrivate) {
       reject(cmd, "the options 'private' and 'remove-private' are mutually exclusive");
diff --git a/java/com/google/gerrit/server/group/db/GroupNameNotes.java b/java/com/google/gerrit/server/group/db/GroupNameNotes.java
index 775ebd6..87aee8e 100644
--- a/java/com/google/gerrit/server/group/db/GroupNameNotes.java
+++ b/java/com/google/gerrit/server/group/db/GroupNameNotes.java
@@ -21,8 +21,10 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
+import com.google.common.collect.HashMultiset;
 import com.google.common.collect.ImmutableBiMap;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Multiset;
 import com.google.common.hash.Hashing;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GroupReference;
@@ -32,11 +34,9 @@
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import java.io.IOException;
 import java.util.Collection;
-import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.CommitBuilder;
@@ -68,7 +68,7 @@
   @VisibleForTesting
   static final String UNIQUE_REF_ERROR = "GroupReference collection must contain unique references";
 
-  public static GroupNameNotes loadForRename(
+  public static GroupNameNotes forRename(
       Repository repository,
       AccountGroup.UUID groupUuid,
       AccountGroup.NameKey oldName,
@@ -83,7 +83,7 @@
     return groupNameNotes;
   }
 
-  public static GroupNameNotes loadForNewGroup(
+  public static GroupNameNotes forNewGroup(
       Repository repository, AccountGroup.UUID groupUuid, AccountGroup.NameKey groupName)
       throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
     checkNotNull(groupName);
@@ -94,18 +94,19 @@
     return groupNameNotes;
   }
 
-  public static Optional<GroupReference> loadOneGroupReference(
-      Repository allUsersRepo, String groupName) throws IOException, ConfigInvalidException {
-    Ref ref = allUsersRepo.exactRef(RefNames.REFS_GROUPNAMES);
+  public static Optional<GroupReference> loadGroup(
+      Repository repository, AccountGroup.NameKey groupName)
+      throws IOException, ConfigInvalidException {
+    Ref ref = repository.exactRef(RefNames.REFS_GROUPNAMES);
     if (ref == null) {
       return Optional.empty();
     }
 
-    try (RevWalk revWalk = new RevWalk(allUsersRepo);
+    try (RevWalk revWalk = new RevWalk(repository);
         ObjectReader reader = revWalk.getObjectReader()) {
       RevCommit notesCommit = revWalk.parseCommit(ref.getObjectId());
       NoteMap noteMap = NoteMap.read(reader, notesCommit);
-      ObjectId noteDataBlobId = noteMap.get(getNoteKey(new AccountGroup.NameKey(groupName)));
+      ObjectId noteDataBlobId = noteMap.get(getNoteKey(groupName));
       if (noteDataBlobId == null) {
         return Optional.empty();
       }
@@ -113,34 +114,34 @@
     }
   }
 
-  public static ImmutableSet<GroupReference> loadAllGroupReferences(Repository repository)
+  public static ImmutableList<GroupReference> loadAllGroups(Repository repository)
       throws IOException, ConfigInvalidException {
     Ref ref = repository.exactRef(RefNames.REFS_GROUPNAMES);
     if (ref == null) {
-      return ImmutableSet.of();
+      return ImmutableList.of();
     }
     try (RevWalk revWalk = new RevWalk(repository);
         ObjectReader reader = revWalk.getObjectReader()) {
       RevCommit notesCommit = revWalk.parseCommit(ref.getObjectId());
       NoteMap noteMap = NoteMap.read(reader, notesCommit);
 
-      Set<GroupReference> groupReferences = new LinkedHashSet<>();
+      Multiset<GroupReference> groupReferences = HashMultiset.create();
       for (Note note : noteMap) {
         GroupReference groupReference = getGroupReference(reader, note.getData());
-        boolean result = groupReferences.add(groupReference);
-        if (!result) {
+        int numOfOccurrences = groupReferences.add(groupReference, 1);
+        if (numOfOccurrences > 1) {
           GroupsNoteDbConsistencyChecker.logConsistencyProblemAsWarning(
               "The UUID of group %s (%s) is duplicate in group name notes",
               groupReference.getName(), groupReference.getUUID());
         }
       }
 
-      return ImmutableSet.copyOf(groupReferences);
+      return ImmutableList.copyOf(groupReferences);
     }
   }
 
-  public static void updateGroupNames(
-      Repository allUsersRepo,
+  public static void updateAllGroups(
+      Repository repository,
       ObjectInserter inserter,
       BatchRefUpdate bru,
       Collection<GroupReference> groupReferences,
@@ -153,7 +154,7 @@
         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);
+      Ref ref = repository.exactRef(RefNames.REFS_GROUPNAMES);
       RevCommit oldCommit = ref != null ? rw.parseCommit(ref.getObjectId()) : null;
 
       for (Map.Entry<AccountGroup.UUID, String> e : biMap.entrySet()) {
@@ -195,8 +196,8 @@
   }
 
   private final AccountGroup.UUID groupUuid;
-  private final Optional<AccountGroup.NameKey> oldGroupName;
-  private final Optional<AccountGroup.NameKey> newGroupName;
+  private Optional<AccountGroup.NameKey> oldGroupName;
+  private Optional<AccountGroup.NameKey> newGroupName;
 
   private boolean nameConflicting;
 
@@ -278,6 +279,9 @@
     commit.setTreeId(noteMap.writeTree(inserter));
     commit.setMessage(getCommitMessage());
 
+    oldGroupName = Optional.empty();
+    newGroupName = Optional.empty();
+
     return true;
   }
 
@@ -311,8 +315,7 @@
     return config.toText();
   }
 
-  @VisibleForTesting
-  public static GroupReference getGroupReference(ObjectReader reader, ObjectId noteDataBlobId)
+  private 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/Groups.java b/java/com/google/gerrit/server/group/db/Groups.java
index f3232be..d0ea9ca 100644
--- a/java/com/google/gerrit/server/group/db/Groups.java
+++ b/java/com/google/gerrit/server/group/db/Groups.java
@@ -192,7 +192,7 @@
       throws OrmException, IOException, ConfigInvalidException {
     if (groupsMigration.readFromNoteDb()) {
       try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
-        return GroupNameNotes.loadAllGroupReferences(allUsersRepo).stream();
+        return GroupNameNotes.loadAllGroups(allUsersRepo).stream();
       }
     }
 
@@ -298,10 +298,9 @@
         .filter(groupUuid -> !AccountGroup.isInternalGroup(groupUuid));
   }
 
-  private Stream<AccountGroup.UUID> getExternalGroupsFromNoteDb(Repository allUsersRepo)
+  private static Stream<AccountGroup.UUID> getExternalGroupsFromNoteDb(Repository allUsersRepo)
       throws IOException, ConfigInvalidException {
-    ImmutableSet<GroupReference> allInternalGroups =
-        GroupNameNotes.loadAllGroupReferences(allUsersRepo);
+    ImmutableList<GroupReference> allInternalGroups = GroupNameNotes.loadAllGroups(allUsersRepo);
     ImmutableSet.Builder<AccountGroup.UUID> allSubgroups = ImmutableSet.builder();
     for (GroupReference internalGroup : allInternalGroups) {
       Optional<InternalGroup> group = getGroupFromNoteDb(allUsersRepo, internalGroup.getUUID());
diff --git a/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java b/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
index 65ac12b..9f0cb3a 100644
--- a/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
+++ b/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
@@ -218,7 +218,7 @@
       Repository allUsersRepo, InternalGroup group) throws IOException {
     List<ConsistencyCheckInfo.ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, group.getName(), group.getGroupUUID());
+            allUsersRepo, group.getNameKey(), group.getGroupUUID());
     problems.forEach(GroupsNoteDbConsistencyChecker::logConsistencyProblem);
   }
 
@@ -232,10 +232,10 @@
    */
   @VisibleForTesting
   static List<ConsistencyProblemInfo> checkWithGroupNameNotes(
-      Repository allUsersRepo, String groupName, AccountGroup.UUID groupUUID) throws IOException {
+      Repository allUsersRepo, AccountGroup.NameKey groupName, AccountGroup.UUID groupUUID)
+      throws IOException {
     try {
-      Optional<GroupReference> groupRef =
-          GroupNameNotes.loadOneGroupReference(allUsersRepo, groupName);
+      Optional<GroupReference> groupRef = GroupNameNotes.loadGroup(allUsersRepo, groupName);
 
       if (!groupRef.isPresent()) {
         return ImmutableList.of(
@@ -243,7 +243,6 @@
       }
 
       AccountGroup.UUID uuid = groupRef.get().getUUID();
-      String name = groupRef.get().getName();
 
       List<ConsistencyProblemInfo> problems = new ArrayList<>();
       if (!Objects.equals(groupUUID, uuid)) {
@@ -253,9 +252,11 @@
                 groupName, groupUUID, uuid));
       }
 
-      if (!Objects.equals(groupName, name)) {
+      String name = groupName.get();
+      String actualName = groupRef.get().getName();
+      if (!Objects.equals(name, actualName)) {
         problems.add(
-            warning("group note of name '%s' claims to represent name of '%s'", groupName, name));
+            warning("group note of name '%s' claims to represent name of '%s'", name, actualName));
       }
       return problems;
     } catch (ConfigInvalidException e) {
diff --git a/java/com/google/gerrit/server/group/db/GroupsUpdate.java b/java/com/google/gerrit/server/group/db/GroupsUpdate.java
index d93c8bd..fd5c453 100644
--- a/java/com/google/gerrit/server/group/db/GroupsUpdate.java
+++ b/java/com/google/gerrit/server/group/db/GroupsUpdate.java
@@ -482,7 +482,7 @@
     try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
       AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey);
       GroupNameNotes groupNameNotes =
-          GroupNameNotes.loadForNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName);
+          GroupNameNotes.forNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName);
 
       GroupConfig groupConfig = GroupConfig.createForNewGroup(allUsersRepo, groupCreation);
       groupConfig.setGroupUpdate(groupUpdate, this::getAccountNameEmail, this::getGroupName);
@@ -515,7 +515,7 @@
       if (groupUpdate.getName().isPresent()) {
         AccountGroup.NameKey oldName = originalGroup.getNameKey();
         AccountGroup.NameKey newName = groupUpdate.getName().get();
-        groupNameNotes = GroupNameNotes.loadForRename(allUsersRepo, groupUuid, oldName, newName);
+        groupNameNotes = GroupNameNotes.forRename(allUsersRepo, groupUuid, oldName, newName);
       }
 
       commit(allUsersRepo, groupConfig, groupNameNotes);
diff --git a/java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java b/java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java
index 46fd666..9197a01 100644
--- a/java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java
+++ b/java/com/google/gerrit/server/group/db/testing/GroupTestUtil.java
@@ -15,14 +15,10 @@
 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.config.AllUsersName;
 import com.google.gerrit.server.git.CommitUtil;
 import com.google.gerrit.server.git.GitRepositoryManager;
@@ -31,29 +27,12 @@
 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.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)) {
diff --git a/java/com/google/gerrit/server/project/ChangeControl.java b/java/com/google/gerrit/server/project/ChangeControl.java
index 9bc59e2..631d705 100644
--- a/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/java/com/google/gerrit/server/project/ChangeControl.java
@@ -22,6 +22,7 @@
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.LabelFunction;
 import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.Account;
@@ -129,7 +130,7 @@
     if (getChange().isPrivate() && !isPrivateVisible(db, cd)) {
       return false;
     }
-    return refControl.isVisible();
+    return refControl.isVisible() && getProjectControl().getProject().getState().permitsRead();
   }
 
   /** Can this user abandon this change? */
@@ -137,7 +138,7 @@
     return (isOwner() // owner (aka creator) of the change can abandon
             || refControl.isOwner() // branch owner can abandon
             || getProjectControl().isOwner() // project owner can abandon
-            || refControl.canAbandon() // user can abandon a specific ref
+            || refControl.canPerform(Permission.ABANDON) // user can abandon a specific ref
             || getProjectControl().isAdmin())
         && !isPatchSetLocked(db);
   }
@@ -147,7 +148,8 @@
     switch (status) {
       case NEW:
       case ABANDONED:
-        return (isOwner() && refControl.canDeleteOwnChanges()) || getProjectControl().isAdmin();
+        return (isOwner() && refControl.canPerform(Permission.DELETE_OWN_CHANGES))
+            || getProjectControl().isAdmin();
       case MERGED:
       default:
         return false;
@@ -158,13 +160,16 @@
   private boolean canRebase(ReviewDb db) throws OrmException {
     return (isOwner() || refControl.canSubmit(isOwner()) || refControl.canRebase())
         && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)
+        && getProjectControl().getProjectState().statePermitsWrite()
         && !isPatchSetLocked(db);
   }
 
   /** Can this user restore this change? */
   private boolean canRestore(ReviewDb db) throws OrmException {
     // Anyone who can abandon the change can restore it, as long as they can create changes.
-    return canAbandon(db) && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
+    return canAbandon(db)
+        && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)
+        && getProjectControl().getProjectState().statePermitsWrite();
   }
 
   /** The range of permitted values associated with a label permission. */
@@ -174,7 +179,9 @@
 
   /** Can this user add a patch set to this change? */
   private boolean canAddPatchSet(ReviewDb db) throws OrmException {
-    if (!refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE) || isPatchSetLocked(db)) {
+    if (!(refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)
+            && getProjectControl().getProjectState().statePermitsWrite())
+        || isPatchSetLocked(db)) {
       return false;
     }
     if (isOwner()) {
@@ -241,7 +248,8 @@
       return isOwner() // owner (aka creator) of the change can edit topic
           || refControl.isOwner() // branch owner can edit topic
           || getProjectControl().isOwner() // project owner can edit topic
-          || refControl.canEditTopicName() // user can edit topic on a specific ref
+          || refControl.canPerform(
+              Permission.EDIT_TOPIC_NAME) // user can edit topic on a specific ref
           || getProjectControl().isAdmin();
     }
     return refControl.canForceEditTopicName();
@@ -261,7 +269,7 @@
   private boolean canEditAssignee() {
     return isOwner()
         || getProjectControl().isOwner()
-        || refControl.canEditAssignee()
+        || refControl.canPerform(Permission.EDIT_ASSIGNEE)
         || isAssignee();
   }
 
@@ -270,14 +278,15 @@
     return isOwner() // owner (aka creator) of the change can edit hashtags
         || refControl.isOwner() // branch owner can edit hashtags
         || getProjectControl().isOwner() // project owner can edit hashtags
-        || refControl.canEditHashtags() // user can edit hashtag on a specific ref
+        || refControl.canPerform(
+            Permission.EDIT_HASHTAGS) // user can edit hashtag on a specific ref
         || getProjectControl().isAdmin();
   }
 
   private boolean isPrivateVisible(ReviewDb db, ChangeData cd) throws OrmException {
     return isOwner()
         || isReviewer(db, cd)
-        || refControl.canViewPrivateChanges()
+        || refControl.canPerform(Permission.VIEW_PRIVATE_CHANGES)
         || getUser().isInternalUser();
   }
 
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index e3d6078..6a5ecd8 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -259,6 +259,10 @@
     return config.getMaxObjectSizeLimit();
   }
 
+  public boolean statePermitsRead() {
+    return getProject().getState().permitsRead();
+  }
+
   public boolean statePermitsWrite() {
     return getProject().getState().permitsWrite();
   }
diff --git a/java/com/google/gerrit/server/project/RefControl.java b/java/com/google/gerrit/server/project/RefControl.java
index c65eb10..f0a9b64 100644
--- a/java/com/google/gerrit/server/project/RefControl.java
+++ b/java/com/google/gerrit/server/project/RefControl.java
@@ -100,9 +100,7 @@
   /** Can this user see this reference exists? */
   boolean isVisible() {
     if (isVisible == null) {
-      isVisible =
-          (getUser().isInternalUser() || canPerform(Permission.READ))
-              && isProjectStatePermittingRead();
+      isVisible = getUser().isInternalUser() || canPerform(Permission.READ);
     }
     return isVisible;
   }
@@ -132,35 +130,6 @@
     return canPerform(Permission.SUBMIT, isChangeOwner);
   }
 
-  /** @return true if this user can abandon a change for this ref */
-  boolean canAbandon() {
-    return canPerform(Permission.ABANDON);
-  }
-
-  /** @return true if this user can view private changes. */
-  boolean canViewPrivateChanges() {
-    return canPerform(Permission.VIEW_PRIVATE_CHANGES);
-  }
-
-  /** @return true if this user can delete their own changes. */
-  boolean canDeleteOwnChanges() {
-    return canPerform(Permission.DELETE_OWN_CHANGES);
-  }
-
-  /** @return true if this user can edit topic names. */
-  boolean canEditTopicName() {
-    return canPerform(Permission.EDIT_TOPIC_NAME);
-  }
-
-  /** @return true if this user can edit hashtag names. */
-  boolean canEditHashtags() {
-    return canPerform(Permission.EDIT_HASHTAGS);
-  }
-
-  boolean canEditAssignee() {
-    return canPerform(Permission.EDIT_ASSIGNEE);
-  }
-
   /** @return true if this user can force edit topic names. */
   boolean canForceEditTopicName() {
     return canForcePerform(Permission.EDIT_TOPIC_NAME);
@@ -189,17 +158,16 @@
     return canPerform(permissionName, false);
   }
 
-  boolean canPerform(String permissionName, boolean isChangeOwner) {
-    return doCanPerform(permissionName, isChangeOwner, false);
-  }
-
   ForRef asForRef() {
     return new ForRefImpl();
   }
 
+  private boolean canPerform(String permissionName, boolean isChangeOwner) {
+    return doCanPerform(permissionName, isChangeOwner, false);
+  }
+
   private boolean canUpload() {
-    return projectControl.controlForRef("refs/for/" + refName).canPerform(Permission.PUSH)
-        && isProjectStatePermittingWrite();
+    return projectControl.controlForRef("refs/for/" + refName).canPerform(Permission.PUSH);
   }
 
   /** @return true if this user can submit merge patch sets to this ref */
@@ -246,14 +214,6 @@
     }
   }
 
-  private boolean isProjectStatePermittingWrite() {
-    return getProjectControl().getProject().getState().permitsWrite();
-  }
-
-  private boolean isProjectStatePermittingRead() {
-    return getProjectControl().getProject().getState().permitsRead();
-  }
-
   private boolean canPushWithForce() {
     if (RefNames.REFS_CONFIG.equals(refName) && !projectControl.isOwner()) {
       // Pushing requires being at least project owner, in addition to push.
@@ -530,7 +490,7 @@
     private boolean can(RefPermission perm) throws PermissionBackendException {
       switch (perm) {
         case READ:
-          return isVisible();
+          return isVisible() && getProjectControl().getProjectState().statePermitsRead();
         case CREATE:
           // TODO This isn't an accurate test.
           return canPerform(perm.permissionName().get());
@@ -562,7 +522,7 @@
           return projectControl.controlForRef(MagicBranch.NEW_CHANGE + refName).canSubmit(true);
 
         case READ_PRIVATE_CHANGES:
-          return canViewPrivateChanges();
+          return canPerform(Permission.VIEW_PRIVATE_CHANGES);
 
         case READ_CONFIG:
           return projectControl
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPick.java b/java/com/google/gerrit/server/restapi/change/CherryPick.java
index c1479b7..2de5ba46 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPick.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPick.java
@@ -37,6 +37,7 @@
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.RetryHelper;
 import com.google.gerrit.server.update.RetryingRestModifyView;
@@ -47,16 +48,20 @@
 import com.google.inject.Singleton;
 import java.io.IOException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Singleton
 public class CherryPick
     extends RetryingRestModifyView<RevisionResource, CherryPickInput, ChangeInfo>
     implements UiAction<RevisionResource> {
+  private static final Logger log = LoggerFactory.getLogger(CherryPick.class);
   private final PermissionBackend permissionBackend;
   private final Provider<CurrentUser> user;
   private final CherryPickChange cherryPickChange;
   private final ChangeJson.Factory json;
   private final ContributorAgreementsChecker contributorAgreements;
+  private final ProjectCache projectCache;
 
   @Inject
   CherryPick(
@@ -65,13 +70,15 @@
       RetryHelper retryHelper,
       CherryPickChange cherryPickChange,
       ChangeJson.Factory json,
-      ContributorAgreementsChecker contributorAgreements) {
+      ContributorAgreementsChecker contributorAgreements,
+      ProjectCache projectCache) {
     super(retryHelper);
     this.permissionBackend = permissionBackend;
     this.user = user;
     this.cherryPickChange = cherryPickChange;
     this.json = json;
     this.contributorAgreements = contributorAgreements;
+    this.projectCache = projectCache;
   }
 
   @Override
@@ -94,6 +101,7 @@
         .project(rsrc.getChange().getProject())
         .ref(refName)
         .check(RefPermission.CREATE_CHANGE);
+    projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
 
     try {
       Change.Id cherryPickedChangeId =
@@ -113,12 +121,18 @@
 
   @Override
   public UiAction.Description getDescription(RevisionResource rsrc) {
+    boolean projectStatePermitsWrite = false;
+    try {
+      projectStatePermitsWrite = projectCache.checkedGet(rsrc.getProject()).statePermitsWrite();
+    } catch (IOException e) {
+      log.error("Failed to check if project state permits write: " + rsrc.getProject(), e);
+    }
     return new UiAction.Description()
         .setLabel("Cherry Pick")
         .setTitle("Cherry pick change to a different branch")
         .setVisible(
             and(
-                rsrc.isCurrent(),
+                rsrc.isCurrent() && projectStatePermitsWrite,
                 permissionBackend
                     .user(user)
                     .project(rsrc.getProject())
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java b/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
index 039c3ca6..7c10086 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
@@ -94,6 +94,7 @@
         .project(projectName)
         .ref(refName)
         .check(RefPermission.CREATE_CHANGE);
+    rsrc.getProjectState().checkStatePermitsWrite();
 
     try {
       Change.Id cherryPickedChangeId =
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index 363feb8..9aa1957 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -196,6 +196,7 @@
     Project.NameKey project = rsrc.getNameKey();
     String refName = RefNames.fullName(input.branch);
     permissionBackend.user(user).project(project).ref(refName).check(RefPermission.CREATE_CHANGE);
+    rsrc.getProjectState().checkStatePermitsWrite();
 
     try (Repository git = gitManager.openRepository(project);
         ObjectInserter oi = git.newObjectInserter();
diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java
index 2607f9c..2ad954d 100644
--- a/java/com/google/gerrit/server/restapi/change/Move.java
+++ b/java/com/google/gerrit/server/restapi/change/Move.java
@@ -70,10 +70,14 @@
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Singleton
 public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, ChangeInfo>
     implements UiAction<ChangeResource> {
+  private static final Logger log = LoggerFactory.getLogger(Move.class);
+
   private final PermissionBackend permissionBackend;
   private final Provider<ReviewDb> dbProvider;
   private final ChangeJson.Factory json;
@@ -114,7 +118,8 @@
   @Override
   protected ChangeInfo applyImpl(
       BatchUpdate.Factory updateFactory, ChangeResource rsrc, MoveInput input)
-      throws RestApiException, OrmException, UpdateException, PermissionBackendException {
+      throws RestApiException, OrmException, UpdateException, PermissionBackendException,
+          IOException {
     Change change = rsrc.getChange();
     Project.NameKey project = rsrc.getProject();
     IdentifiedUser caller = rsrc.getUser().asIdentifiedUser();
@@ -136,6 +141,7 @@
     } catch (AuthException denied) {
       throw new AuthException("move not permitted", denied);
     }
+    projectCache.checkedGet(project).checkStatePermitsWrite();
 
     try (BatchUpdate u =
         updateFactory.create(dbProvider.get(), project, caller, TimeUtil.nowTs())) {
@@ -274,12 +280,18 @@
   @Override
   public UiAction.Description getDescription(ChangeResource rsrc) {
     Change change = rsrc.getChange();
+    boolean projectStatePermitsWrite = false;
+    try {
+      projectStatePermitsWrite = projectCache.checkedGet(rsrc.getProject()).statePermitsWrite();
+    } catch (IOException e) {
+      log.error("Failed to check if project state permits write: " + rsrc.getProject(), e);
+    }
     return new UiAction.Description()
         .setLabel("Move Change")
         .setTitle("Move change to a different branch")
         .setVisible(
             and(
-                change.getStatus().isOpen(),
+                change.getStatus().isOpen() && projectStatePermitsWrite,
                 and(
                     permissionBackend
                         .user(rsrc.getUser())
diff --git a/java/com/google/gerrit/server/restapi/change/Revert.java b/java/com/google/gerrit/server/restapi/change/Revert.java
index bdab012..b55ca5e 100644
--- a/java/com/google/gerrit/server/restapi/change/Revert.java
+++ b/java/com/google/gerrit/server/restapi/change/Revert.java
@@ -53,6 +53,7 @@
 import com.google.gerrit.server.project.ContributorAgreementsChecker;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.BatchUpdateOp;
 import com.google.gerrit.server.update.ChangeContext;
@@ -100,6 +101,7 @@
   private final ApprovalsUtil approvalsUtil;
   private final ChangeReverted changeReverted;
   private final ContributorAgreementsChecker contributorAgreements;
+  private final ProjectCache projectCache;
 
   @Inject
   Revert(
@@ -116,7 +118,8 @@
       @GerritPersonIdent PersonIdent serverIdent,
       ApprovalsUtil approvalsUtil,
       ChangeReverted changeReverted,
-      ContributorAgreementsChecker contributorAgreements) {
+      ContributorAgreementsChecker contributorAgreements,
+      ProjectCache projectCache) {
     super(retryHelper);
     this.db = db;
     this.permissionBackend = permissionBackend;
@@ -131,6 +134,7 @@
     this.approvalsUtil = approvalsUtil;
     this.changeReverted = changeReverted;
     this.contributorAgreements = contributorAgreements;
+    this.projectCache = projectCache;
   }
 
   @Override
@@ -145,6 +149,7 @@
 
     contributorAgreements.check(rsrc.getProject(), rsrc.getUser());
     permissionBackend.user(rsrc.getUser()).ref(change.getDest()).check(CREATE_CHANGE);
+    projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
 
     Change.Id revertId =
         revert(updateFactory, rsrc.getNotes(), rsrc.getUser(), Strings.emptyToNull(input.message));
@@ -243,12 +248,18 @@
   @Override
   public UiAction.Description getDescription(ChangeResource rsrc) {
     Change change = rsrc.getChange();
+    boolean projectStatePermitsWrite = false;
+    try {
+      projectStatePermitsWrite = projectCache.checkedGet(rsrc.getProject()).statePermitsWrite();
+    } catch (IOException e) {
+      log.error("Failed to check if project state permits write: " + rsrc.getProject(), e);
+    }
     return new UiAction.Description()
         .setLabel("Revert")
         .setTitle("Revert the change")
         .setVisible(
             and(
-                change.getStatus() == Change.Status.MERGED,
+                change.getStatus() == Change.Status.MERGED && projectStatePermitsWrite,
                 permissionBackend
                     .user(rsrc.getUser())
                     .ref(change.getDest())
diff --git a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
index a06c8c5..44005c0 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
@@ -40,6 +40,7 @@
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectResource;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.UpdateException;
@@ -65,6 +66,7 @@
   private final Provider<ReviewDb> db;
   private final SetAccessUtil setAccess;
   private final ChangeJson.Factory jsonFactory;
+  private final ProjectCache projectCache;
 
   @Inject
   CreateAccessChange(
@@ -75,7 +77,8 @@
       Provider<MetaDataUpdate.User> metaDataUpdateFactory,
       Provider<ReviewDb> db,
       SetAccessUtil accessUtil,
-      ChangeJson.Factory jsonFactory) {
+      ChangeJson.Factory jsonFactory,
+      ProjectCache projectCache) {
     this.permissionBackend = permissionBackend;
     this.seq = seq;
     this.changeInserterFactory = changeInserterFactory;
@@ -84,6 +87,7 @@
     this.db = db;
     this.setAccess = accessUtil;
     this.jsonFactory = jsonFactory;
+    this.projectCache = projectCache;
   }
 
   @Override
@@ -103,6 +107,7 @@
         throw new PermissionDeniedException("cannot create change for " + RefNames.REFS_CONFIG);
       }
     }
+    projectCache.checkedGet(rsrc.getNameKey()).checkStatePermitsWrite();
 
     MetaDataUpdate.User metaDataUpdateUser = metaDataUpdateFactory.get();
     List<AccessSection> removals = setAccess.getAccessSections(input.remove);
diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java
index 1568a4c..520d74c 100644
--- a/java/com/google/gerrit/server/restapi/project/GetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java
@@ -251,8 +251,10 @@
     info.isOwner = toBoolean(canWriteConfig);
     info.canUpload =
         toBoolean(
-            canWriteConfig
-                || (canReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE)));
+            projectState.statePermitsWrite()
+                && (canWriteConfig
+                    || (canReadConfig
+                        && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE))));
     info.canAdd = toBoolean(perm.testOrFalse(CREATE_REF));
     info.configVisible = canReadConfig || canWriteConfig;
 
diff --git a/java/com/google/gerrit/server/schema/SchemaCreator.java b/java/com/google/gerrit/server/schema/SchemaCreator.java
index 2004c98..895e75c 100644
--- a/java/com/google/gerrit/server/schema/SchemaCreator.java
+++ b/java/com/google/gerrit/server/schema/SchemaCreator.java
@@ -240,7 +240,7 @@
 
     AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey);
     GroupNameNotes groupNameNotes =
-        GroupNameNotes.loadForNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName);
+        GroupNameNotes.forNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName);
 
     commit(allUsersRepo, groupConfig, groupNameNotes);
 
diff --git a/java/com/google/gerrit/truth/ListSubject.java b/java/com/google/gerrit/truth/ListSubject.java
index bcd8dcf..cccf51b 100644
--- a/java/com/google/gerrit/truth/ListSubject.java
+++ b/java/com/google/gerrit/truth/ListSubject.java
@@ -30,8 +30,7 @@
   @SuppressWarnings("unchecked")
   public static <S extends Subject<S, E>, E> ListSubject<S, E> assertThat(
       List<E> list, Function<E, S> elementAssertThatFunction) {
-    // The ListSubjectFactory always returns ListSubjects.
-    // -> Casting is appropriate.
+    // The ListSubjectFactory always returns ListSubjects. -> Casting is appropriate.
     return (ListSubject<S, E>)
         assertAbout(new ListSubjectFactory<>(elementAssertThatFunction)).that(list);
   }
@@ -44,11 +43,8 @@
 
   public S element(int index) {
     checkArgument(index >= 0, "index(%s) must be >= 0", index);
-    // The constructor only accepts lists.
-    // -> Casting is appropriate.
-    @SuppressWarnings("unchecked")
-    List<E> list = (List<E>) actual();
     isNotNull();
+    List<E> list = getActualList();
     if (index >= list.size()) {
       fail("has an element at index " + index);
     }
@@ -61,11 +57,23 @@
     return element(0);
   }
 
+  public S lastElement() {
+    isNotNull();
+    isNotEmpty();
+    List<E> list = getActualList();
+    return element(list.size() - 1);
+  }
+
+  @SuppressWarnings("unchecked")
+  private List<E> getActualList() {
+    // The constructor only accepts lists. -> Casting is appropriate.
+    return (List<E>) actual();
+  }
+
   @SuppressWarnings("unchecked")
   @Override
   public ListSubject<S, E> named(String s, Object... objects) {
-    // This object is returned which is of type ListSubject.
-    // -> Casting is appropriate.
+    // This object is returned which is of type ListSubject. -> Casting is appropriate.
     return (ListSubject<S, E>) super.named(s, objects);
   }
 
@@ -81,8 +89,7 @@
     @SuppressWarnings("unchecked")
     @Override
     public ListSubject<S, T> createSubject(FailureMetadata failureMetadata, Iterable<?> objects) {
-      // The constructor of ListSubject only accepts lists.
-      // -> Casting is appropriate.
+      // The constructor of ListSubject only accepts lists. -> Casting is appropriate.
       return new ListSubject<>(failureMetadata, (List<T>) objects, elementAssertThatFunction);
     }
   }
diff --git a/javatests/com/google/gerrit/server/group/db/BUILD b/javatests/com/google/gerrit/server/group/db/BUILD
index 1ba0ce9..9816603 100644
--- a/javatests/com/google/gerrit/server/group/db/BUILD
+++ b/javatests/com/google/gerrit/server/group/db/BUILD
@@ -6,12 +6,14 @@
     srcs = glob(["*.java"]),
     deps = [
         "//java/com/google/gerrit/common:server",
+        "//java/com/google/gerrit/common/data/testing:common-data-test-util",
         "//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",
+        "//java/com/google/gerrit/truth",
         "//lib:gwtorm",
         "//lib:truth",
         "//lib/jgit/org.eclipse.jgit:jgit",
diff --git a/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java
index d4ddbcf..c059da9 100644
--- a/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java
+++ b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java
@@ -18,24 +18,38 @@
 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 com.google.gerrit.truth.OptionalSubject.assertThat;
 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.common.data.testing.GroupReferenceSubject;
 import com.google.gerrit.extensions.common.CommitInfo;
+import com.google.gerrit.extensions.common.testing.CommitInfoSubject;
 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.config.AllUsersNameProvider;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.MetaDataUpdate;
 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 com.google.gerrit.truth.ListSubject;
+import com.google.gerrit.truth.OptionalSubject;
+import com.google.gwtorm.client.KeyUtil;
+import com.google.gwtorm.server.OrmDuplicateKeyException;
+import com.google.gwtorm.server.StandardKeyEncoder;
+import java.io.IOException;
 import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
 import java.util.TimeZone;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
@@ -47,16 +61,28 @@
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
-public class GroupNameNotesTest extends GerritBaseTests {
+public class GroupNameNotesTest {
+  static {
+    KeyUtil.setEncoderImpl(new StandardKeyEncoder());
+  }
+
   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");
 
+  @Rule public ExpectedException expectedException = ExpectedException.none();
+
+  private final AccountGroup.UUID groupUuid = new AccountGroup.UUID("users-XYZ");
+  private final AccountGroup.NameKey groupName = new AccountGroup.NameKey("users");
+
   private AtomicInteger idCounter;
   private Repository repo;
 
@@ -73,12 +99,311 @@
   }
 
   @Test
+  public void newGroupCanBeCreated() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    Optional<GroupReference> groupReference = loadGroup(groupName);
+    assertThatGroup(groupReference).value().groupUuid().isEqualTo(groupUuid);
+    assertThatGroup(groupReference).value().name().isEqualTo(groupName.get());
+  }
+
+  @Test
+  public void uuidOfNewGroupMustNotBeNull() throws Exception {
+    expectedException.expect(NullPointerException.class);
+    GroupNameNotes.forNewGroup(repo, null, groupName);
+  }
+
+  @Test
+  public void nameOfNewGroupMustNotBeNull() throws Exception {
+    expectedException.expect(NullPointerException.class);
+    GroupNameNotes.forNewGroup(repo, groupUuid, null);
+  }
+
+  @Test
+  public void nameOfNewGroupMayBeEmpty() throws Exception {
+    AccountGroup.NameKey emptyName = new AccountGroup.NameKey("");
+    createGroup(groupUuid, emptyName);
+
+    Optional<GroupReference> groupReference = loadGroup(emptyName);
+    assertThatGroup(groupReference).value().name().isEqualTo("");
+  }
+
+  @Test
+  public void newGroupMustNotReuseNameOfAnotherGroup() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.UUID anotherGroupUuid = new AccountGroup.UUID("AnotherGroup");
+    expectedException.expect(OrmDuplicateKeyException.class);
+    expectedException.expectMessage(groupName.get());
+    GroupNameNotes.forNewGroup(repo, anotherGroupUuid, groupName);
+  }
+
+  @Test
+  public void newGroupMayReuseUuidOfAnotherGroup() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    createGroup(groupUuid, anotherName);
+
+    Optional<GroupReference> group1 = loadGroup(groupName);
+    assertThatGroup(group1).value().groupUuid().isEqualTo(groupUuid);
+    Optional<GroupReference> group2 = loadGroup(anotherName);
+    assertThatGroup(group2).value().groupUuid().isEqualTo(groupUuid);
+  }
+
+  @Test
+  public void groupCanBeRenamed() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    renameGroup(groupUuid, groupName, anotherName);
+
+    Optional<GroupReference> groupReference = loadGroup(anotherName);
+    assertThatGroup(groupReference).value().groupUuid().isEqualTo(groupUuid);
+    assertThatGroup(groupReference).value().name().isEqualTo(anotherName.get());
+  }
+
+  @Test
+  public void previousNameOfGroupCannotBeUsedAfterRename() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    renameGroup(groupUuid, groupName, anotherName);
+
+    Optional<GroupReference> group = loadGroup(groupName);
+    assertThatGroup(group).isAbsent();
+  }
+
+  @Test
+  public void groupCannotBeRenamedToNull() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    expectedException.expect(NullPointerException.class);
+    GroupNameNotes.forRename(repo, groupUuid, groupName, null);
+  }
+
+  @Test
+  public void oldNameOfGroupMustBeSpecifiedForRename() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    expectedException.expect(NullPointerException.class);
+    GroupNameNotes.forRename(repo, groupUuid, null, anotherName);
+  }
+
+  @Test
+  public void groupCannotBeRenamedWhenOldNameIsWrong() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherOldName = new AccountGroup.NameKey("contributors");
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    expectedException.expect(ConfigInvalidException.class);
+    expectedException.expectMessage(anotherOldName.get());
+    GroupNameNotes.forRename(repo, groupUuid, anotherOldName, anotherName);
+  }
+
+  @Test
+  public void groupCannotBeRenamedToNameOfAnotherGroup() throws Exception {
+    createGroup(groupUuid, groupName);
+    AccountGroup.UUID anotherGroupUuid = new AccountGroup.UUID("admins-ABC");
+    AccountGroup.NameKey anotherGroupName = new AccountGroup.NameKey("admins");
+    createGroup(anotherGroupUuid, anotherGroupName);
+
+    expectedException.expect(OrmDuplicateKeyException.class);
+    expectedException.expectMessage(anotherGroupName.get());
+    GroupNameNotes.forRename(repo, groupUuid, groupName, anotherGroupName);
+  }
+
+  @Test
+  public void groupCannotBeRenamedWithoutSpecifiedUuid() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    expectedException.expect(NullPointerException.class);
+    GroupNameNotes.forRename(repo, null, groupName, anotherName);
+  }
+
+  @Test
+  public void groupCannotBeRenamedWhenUuidIsWrong() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.UUID anotherGroupUuid = new AccountGroup.UUID("admins-ABC");
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    expectedException.expect(ConfigInvalidException.class);
+    expectedException.expectMessage(groupUuid.get());
+    GroupNameNotes.forRename(repo, anotherGroupUuid, groupName, anotherName);
+  }
+
+  @Test
+  public void firstGroupCreationCreatesARootCommit() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    Ref ref = repo.exactRef(RefNames.REFS_GROUPNAMES);
+    assertThat(ref.getObjectId()).isNotNull();
+
+    try (RevWalk revWalk = new RevWalk(repo)) {
+      RevCommit revCommit = revWalk.parseCommit(ref.getObjectId());
+      assertThat(revCommit.getParentCount()).isEqualTo(0);
+    }
+  }
+
+  @Test
+  public void furtherGroupCreationAppendsACommit() throws Exception {
+    createGroup(groupUuid, groupName);
+    ImmutableList<CommitInfo> commitsAfterCreation = log();
+
+    AccountGroup.UUID anotherGroupUuid = new AccountGroup.UUID("admins-ABC");
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    createGroup(anotherGroupUuid, anotherName);
+
+    ImmutableList<CommitInfo> commitsAfterFurtherGroup = log();
+    assertThatCommits(commitsAfterFurtherGroup).containsAllIn(commitsAfterCreation);
+    assertThatCommits(commitsAfterFurtherGroup).lastElement().isNotIn(commitsAfterCreation);
+  }
+
+  @Test
+  public void groupRenamingAppendsACommit() throws Exception {
+    createGroup(groupUuid, groupName);
+    ImmutableList<CommitInfo> commitsAfterCreation = log();
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    renameGroup(groupUuid, groupName, anotherName);
+
+    ImmutableList<CommitInfo> commitsAfterRename = log();
+    assertThatCommits(commitsAfterRename).containsAllIn(commitsAfterCreation);
+    assertThatCommits(commitsAfterRename).lastElement().isNotIn(commitsAfterCreation);
+  }
+
+  @Test
+  public void newCommitIsNotCreatedForRedundantNameUpdate() throws Exception {
+    createGroup(groupUuid, groupName);
+    ImmutableList<CommitInfo> commitsAfterCreation = log();
+
+    renameGroup(groupUuid, groupName, groupName);
+
+    ImmutableList<CommitInfo> commitsAfterRename = log();
+    assertThatCommits(commitsAfterRename).isEqualTo(commitsAfterCreation);
+  }
+
+  @Test
+  public void newCommitIsNotCreatedWhenCommittingGroupCreationTwice() throws Exception {
+    GroupNameNotes groupNameNotes = GroupNameNotes.forNewGroup(repo, groupUuid, groupName);
+
+    commit(groupNameNotes);
+    ImmutableList<CommitInfo> commitsAfterFirstCommit = log();
+    commit(groupNameNotes);
+    ImmutableList<CommitInfo> commitsAfterSecondCommit = log();
+
+    assertThatCommits(commitsAfterSecondCommit).isEqualTo(commitsAfterFirstCommit);
+  }
+
+  @Test
+  public void newCommitIsNotCreatedWhenCommittingGroupRenamingTwice() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    GroupNameNotes groupNameNotes =
+        GroupNameNotes.forRename(repo, groupUuid, groupName, anotherName);
+
+    commit(groupNameNotes);
+    ImmutableList<CommitInfo> commitsAfterFirstCommit = log();
+    commit(groupNameNotes);
+    ImmutableList<CommitInfo> commitsAfterSecondCommit = log();
+
+    assertThatCommits(commitsAfterSecondCommit).isEqualTo(commitsAfterFirstCommit);
+  }
+
+  @Test
+  public void commitMessageMentionsGroupCreation() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    ImmutableList<CommitInfo> commits = log();
+    assertThatCommits(commits).lastElement().message().contains("Create");
+    assertThatCommits(commits).lastElement().message().contains(groupName.get());
+  }
+
+  @Test
+  public void commitMessageMentionsGroupRenaming() throws Exception {
+    createGroup(groupUuid, groupName);
+
+    AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
+    renameGroup(groupUuid, groupName, anotherName);
+
+    ImmutableList<CommitInfo> commits = log();
+    assertThatCommits(commits).lastElement().message().contains("Rename");
+    assertThatCommits(commits).lastElement().message().contains(groupName.get());
+    assertThatCommits(commits).lastElement().message().contains(anotherName.get());
+  }
+
+  @Test
+  public void nonExistentNotesRefIsEquivalentToNonExistentGroup() throws Exception {
+    Optional<GroupReference> group = loadGroup(groupName);
+
+    assertThatGroup(group).isAbsent();
+  }
+
+  @Test
+  public void nonExistentGroupCannotBeLoaded() throws Exception {
+    createGroup(new AccountGroup.UUID("contributors-MN"), new AccountGroup.NameKey("contributors"));
+    createGroup(groupUuid, groupName);
+
+    Optional<GroupReference> group = loadGroup(new AccountGroup.NameKey("admins"));
+    assertThatGroup(group).isAbsent();
+  }
+
+  @Test
+  public void specificGroupCanBeLoaded() throws Exception {
+    createGroup(new AccountGroup.UUID("contributors-MN"), new AccountGroup.NameKey("contributors"));
+    createGroup(groupUuid, groupName);
+    createGroup(new AccountGroup.UUID("admins-ABC"), new AccountGroup.NameKey("admins"));
+
+    Optional<GroupReference> group = loadGroup(groupName);
+    assertThatGroup(group).value().groupUuid().isEqualTo(groupUuid);
+  }
+
+  @Test
+  public void nonExistentNotesRefIsEquivalentToNotAnyExistingGroups() throws Exception {
+    ImmutableList<GroupReference> allGroups = GroupNameNotes.loadAllGroups(repo);
+
+    assertThat(allGroups).isEmpty();
+  }
+
+  @Test
+  public void allGroupsCanBeLoaded() throws Exception {
+    AccountGroup.UUID groupUuid1 = new AccountGroup.UUID("contributors-MN");
+    AccountGroup.NameKey groupName1 = new AccountGroup.NameKey("contributors");
+    createGroup(groupUuid1, groupName1);
+    AccountGroup.UUID groupUuid2 = new AccountGroup.UUID("admins-ABC");
+    AccountGroup.NameKey groupName2 = new AccountGroup.NameKey("admins");
+    createGroup(groupUuid2, groupName2);
+
+    ImmutableList<GroupReference> allGroups = GroupNameNotes.loadAllGroups(repo);
+
+    GroupReference group1 = new GroupReference(groupUuid1, groupName1.get());
+    GroupReference group2 = new GroupReference(groupUuid2, groupName2.get());
+    assertThat(allGroups).containsExactly(group1, group2);
+  }
+
+  @Test
+  public void loadedGroupsContainGroupsWithDuplicateGroupUuids() throws Exception {
+    createGroup(groupUuid, groupName);
+    AccountGroup.NameKey anotherGroupName = new AccountGroup.NameKey("admins");
+    createGroup(groupUuid, anotherGroupName);
+
+    ImmutableList<GroupReference> allGroups = GroupNameNotes.loadAllGroups(repo);
+
+    GroupReference group1 = new GroupReference(groupUuid, groupName.get());
+    GroupReference group2 = new GroupReference(groupUuid, anotherGroupName.get());
+    assertThat(allGroups).containsExactly(group1, group2);
+  }
+
+  @Test
   public void updateGroupNames() throws Exception {
     GroupReference g1 = newGroup("a");
     GroupReference g2 = newGroup("b");
 
     PersonIdent ident = newPersonIdent();
-    updateGroupNames(ident, g1, g2);
+    updateAllGroups(ident, g1, g2);
 
     ImmutableList<CommitInfo> log = log();
     assertThat(log).hasSize(1);
@@ -87,11 +412,11 @@
     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");
+    assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2);
 
     // Updating the same set of names is a no-op.
     String commit = log.get(0).commit;
-    updateGroupNames(newPersonIdent(), g1, g2);
+    updateAllGroups(newPersonIdent(), g1, g2);
     log = log();
     assertThat(log).hasSize(1);
     assertThat(log.get(0)).commit().isEqualTo(commit);
@@ -120,9 +445,9 @@
             .copy();
 
     ident = newPersonIdent();
-    updateGroupNames(ident, g1, g2);
+    updateAllGroups(ident, g1, g2);
 
-    assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("a", "a-1", "b", "b-2");
+    assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2);
 
     ImmutableList<CommitInfo> log = log();
     assertThat(log).hasSize(2);
@@ -142,13 +467,13 @@
     GroupReference g2 = newGroup("b");
 
     PersonIdent ident = newPersonIdent();
-    updateGroupNames(ident, g1, g2);
+    updateAllGroups(ident, g1, g2);
 
-    assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("a", "a-1", "b", "b-2");
+    assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2);
 
-    updateGroupNames(ident);
+    updateAllGroups(ident);
 
-    assertThat(GroupTestUtil.readNameToUuidMap(repo)).isEmpty();
+    assertThat(GroupNameNotes.loadAllGroups(repo)).isEmpty();
 
     ImmutableList<CommitInfo> log = log();
     assertThat(log).hasSize(2);
@@ -171,12 +496,46 @@
   @Test
   public void emptyGroupName() throws Exception {
     GroupReference g = newGroup("");
-    updateGroupNames(newPersonIdent(), g);
+    updateAllGroups(newPersonIdent(), g);
 
-    assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("", "-1");
+    assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g);
     assertThat(readNameNote(g)).isEqualTo("[group]\n\tuuid = -1\n\tname = \n");
   }
 
+  private void createGroup(AccountGroup.UUID groupUuid, AccountGroup.NameKey groupName)
+      throws Exception {
+    GroupNameNotes groupNameNotes = GroupNameNotes.forNewGroup(repo, groupUuid, groupName);
+    commit(groupNameNotes);
+  }
+
+  private void renameGroup(
+      AccountGroup.UUID groupUuid, AccountGroup.NameKey oldName, AccountGroup.NameKey newName)
+      throws Exception {
+    GroupNameNotes groupNameNotes = GroupNameNotes.forRename(repo, groupUuid, oldName, newName);
+    commit(groupNameNotes);
+  }
+
+  private Optional<GroupReference> loadGroup(AccountGroup.NameKey groupName) throws Exception {
+    return GroupNameNotes.loadGroup(repo, groupName);
+  }
+
+  private void commit(GroupNameNotes groupNameNotes) throws IOException {
+    try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) {
+      groupNameNotes.commit(metaDataUpdate);
+    }
+  }
+
+  private MetaDataUpdate createMetaDataUpdate() {
+    PersonIdent serverIdent = newPersonIdent();
+
+    MetaDataUpdate metaDataUpdate =
+        new MetaDataUpdate(
+            GitReferenceUpdated.DISABLED, new Project.NameKey("Test Repository"), repo);
+    metaDataUpdate.getCommitBuilder().setCommitter(serverIdent);
+    metaDataUpdate.getCommitBuilder().setAuthor(serverIdent);
+    return metaDataUpdate;
+  }
+
   private GroupReference newGroup(String name) {
     int id = idCounter.incrementAndGet();
     return new GroupReference(new AccountGroup.UUID(name + "-" + id), name);
@@ -190,10 +549,10 @@
     return GroupNameNotes.getNoteKey(new AccountGroup.NameKey(g.getName()));
   }
 
-  private void updateGroupNames(PersonIdent ident, GroupReference... groupRefs) throws Exception {
+  private void updateAllGroups(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);
+      GroupNameNotes.updateAllGroups(repo, inserter, bru, Arrays.asList(groupRefs), ident);
       inserter.flush();
       RefUpdateUtil.executeChecked(bru, repo);
     }
@@ -204,7 +563,7 @@
       BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
       PersonIdent ident = newPersonIdent();
       try {
-        GroupNameNotes.updateGroupNames(repo, inserter, bru, Arrays.asList(groupRefs), ident);
+        GroupNameNotes.updateAllGroups(repo, inserter, bru, Arrays.asList(groupRefs), ident);
         assert_().fail("Expected IllegalArgumentException");
       } catch (IllegalArgumentException e) {
         assertThat(e).hasMessageThat().isEqualTo(GroupNameNotes.UNIQUE_REF_ERROR);
@@ -225,4 +584,14 @@
       return new String(reader.open(noteMap.get(k), OBJ_BLOB).getCachedBytes(), UTF_8);
     }
   }
+
+  private static OptionalSubject<GroupReferenceSubject, GroupReference> assertThatGroup(
+      Optional<GroupReference> group) {
+    return assertThat(group, GroupReferenceSubject::assertThat);
+  }
+
+  private static ListSubject<CommitInfoSubject, CommitInfo> assertThatCommits(
+      List<CommitInfo> commits) {
+    return ListSubject.assertThat(commits, CommitInfoSubject::assertThat);
+  }
 }
diff --git a/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java b/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java
index 51cf987..048eaba 100644
--- a/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java
+++ b/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java
@@ -536,7 +536,7 @@
     try (ObjectInserter inserter = repo.newObjectInserter()) {
       ImmutableList<GroupReference> refs =
           ImmutableList.of(GroupReference.forGroup(g1), GroupReference.forGroup(g2));
-      GroupNameNotes.updateGroupNames(repo, inserter, bru, refs, newPersonIdent());
+      GroupNameNotes.updateAllGroups(repo, inserter, bru, refs, newPersonIdent());
       inserter.flush();
     }
 
@@ -552,7 +552,9 @@
     assertMigratedCleanly(reload(g1), b1);
     assertMigratedCleanly(reload(g2), b2);
 
-    assertThat(GroupTestUtil.readNameToUuidMap(repo)).containsExactly("a", "a-1", "b", "b-2");
+    GroupReference group1 = GroupReference.forGroup(g1);
+    GroupReference group2 = GroupReference.forGroup(g2);
+    assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(group1, group2);
   }
 
   @Test
diff --git a/javatests/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyCheckerTest.java b/javatests/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyCheckerTest.java
index eff755f..a5b04ee 100644
--- a/javatests/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyCheckerTest.java
+++ b/javatests/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyCheckerTest.java
@@ -30,7 +30,7 @@
   public void groupNamesRefIsMissing() throws Exception {
     List<ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
+            allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
     assertThat(problems)
         .containsExactly(warning("Group with name 'g-1' doesn't exist in the list of all names"));
   }
@@ -40,7 +40,7 @@
     updateGroupNamesRef("g-2", "[group]\n\tuuid = uuid-2\n\tname = g-2\n");
     List<ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
+            allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
     assertThat(problems)
         .containsExactly(warning("Group with name 'g-1' doesn't exist in the list of all names"));
   }
@@ -50,7 +50,7 @@
     updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-1\n\tname = g-1\n");
     List<ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
+            allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
     assertThat(problems).isEmpty();
   }
 
@@ -59,7 +59,7 @@
     updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-2\n\tname = g-1\n");
     List<ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
+            allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
     assertThat(problems)
         .containsExactly(
             warning(
@@ -72,7 +72,7 @@
     updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-1\n\tname = g-2\n");
     List<ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
+            allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
     assertThat(problems)
         .containsExactly(warning("group note of name 'g-1' claims to represent name of 'g-2'"));
   }
@@ -82,7 +82,7 @@
     updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-2\n\tname = g-2\n");
     List<ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
+            allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
     assertThat(problems)
         .containsExactly(
             warning(
@@ -97,7 +97,7 @@
     updateGroupNamesRef("g-1", "[invalid");
     List<ConsistencyProblemInfo> problems =
         GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
-            allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
+            allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
     assertThat(problems)
         .containsExactly(
             warning(