ProjectConfig: Make GroupReference an AutoValue and remove byName map

This commit is part of a larger series that aims at making all state
cached by ProjectCache immutable and serializable. We want cached
entities to be immutable because it minimizes all sorts of threading
issues. This commit is an incremental step towards this goal.

We used to keep an inverse map from name to GroupReference. Since
the size of the map is low - usually below 10 - we remove this
logic and go over all values when required instead.

Change-Id: I76d7d698f35f3ec0abe7a3365f89ebf08e984f81

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -1257,7 +1257,7 @@ public abstract class AbstractDaemonTest {
 
   protected GroupReference groupRef(AccountGroup.UUID groupUuid) {
     GroupDescription.Basic groupDescription = groupBackend.get(groupUuid);
-    return new GroupReference(groupDescription.getGroupUUID(), groupDescription.getName());
+    return GroupReference.create(groupDescription.getGroupUUID(), groupDescription.getName());
   }
 
   protected InternalGroup group(String groupName) {
@@ -1269,7 +1269,7 @@ public abstract class AbstractDaemonTest {
   protected GroupReference groupRef(String groupName) {
     InternalGroup group = groupCache.get(AccountGroup.nameKey(groupName)).orElse(null);
     assertThat(group).isNotNull();
-    return new GroupReference(group.getGroupUUID(), group.getName());
+    return GroupReference.create(group.getGroupUUID(), group.getName());
   }
 
   protected AccountGroup.UUID groupUuid(String groupName) {

java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java

@@ -154,7 +154,7 @@ public class ProjectOperationsImpl implements ProjectOperations {
         Permission permission =
             projectConfig.getAccessSection(p.section(), true).getPermission(p.name(), true);
         if (p.group().isPresent()) {
-          GroupReference group = new GroupReference(p.group().get(), p.group().get().get());
+          GroupReference group = GroupReference.create(p.group().get(), p.group().get().get());
           group = projectConfig.resolve(group);
           permission.removeRule(group);
         } else {
@@ -325,7 +325,7 @@ public class ProjectOperationsImpl implements ProjectOperations {
   }
 
   private static PermissionRule newRule(ProjectConfig project, AccountGroup.UUID groupUUID) {
-    GroupReference group = new GroupReference(groupUUID, groupUUID.get());
+    GroupReference group = GroupReference.create(groupUUID, groupUUID.get());
     group = project.resolve(group);
     return new PermissionRule(group);
   }

java/com/google/gerrit/common/data/GroupReference.java

@@ -16,16 +16,18 @@ package com.google.gerrit.common.data;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.auto.value.AutoValue;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.AccountGroup;
 
 /** Describes a group within a projects {@link AccessSection}s. */
-public class GroupReference implements Comparable<GroupReference> {
+@AutoValue
+public abstract class GroupReference implements Comparable<GroupReference> {
 
   private static final String PREFIX = "group ";
 
   public static GroupReference forGroup(GroupDescription.Basic group) {
-    return new GroupReference(group.getGroupUUID(), group.getName());
+    return GroupReference.create(group.getGroupUUID(), group.getName());
   }
 
   public static boolean isGroupReference(String configValue) {
@@ -40,10 +42,10 @@ public class GroupReference implements Comparable<GroupReference> {
     return configValue.substring(PREFIX.length()).trim();
   }
 
-  protected String uuid;
+  @Nullable
-  protected String name;
+  public abstract AccountGroup.UUID getUUID();
 
-  protected GroupReference() {}
+  public abstract String getName();
 
   /**
    * Create a group reference.
@@ -51,9 +53,8 @@ public class GroupReference implements Comparable<GroupReference> {
    * @param uuid UUID of the group, must not be {@code null}
    * @param name the group name, must not be {@code null}
    */
-  public GroupReference(AccountGroup.UUID uuid, String name) {
+  public static GroupReference create(AccountGroup.UUID uuid, String name) {
-    setUUID(requireNonNull(uuid));
+    return new AutoValue_GroupReference(requireNonNull(uuid), requireNonNull(name));
-    setName(name);
   }
 
   /**
@@ -61,33 +62,12 @@ public class GroupReference implements Comparable<GroupReference> {
    *
    * @param name the group name, must not be {@code null}
    */
-  public GroupReference(String name) {
+  public static GroupReference create(String name) {
-    setUUID(null);
+    return new AutoValue_GroupReference(null, name);
-    setName(name);
-  }
-
-  @Nullable
-  public AccountGroup.UUID getUUID() {
-    return uuid != null ? AccountGroup.uuid(uuid) : null;
-  }
-
-  public void setUUID(@Nullable AccountGroup.UUID newUUID) {
-    uuid = newUUID != null ? newUUID.get() : null;
-  }
-
-  public String getName() {
-    return name;
-  }
-
-  public void setName(String newName) {
-    if (newName == null) {
-      throw new NullPointerException();
-    }
-    this.name = newName;
   }
 
   @Override
-  public int compareTo(GroupReference o) {
+  public final int compareTo(GroupReference o) {
     return uuid(this).compareTo(uuid(o));
   }
 
@@ -100,21 +80,21 @@ public class GroupReference implements Comparable<GroupReference> {
   }
 
   @Override
-  public int hashCode() {
+  public final int hashCode() {
     return uuid(this).hashCode();
   }
 
   @Override
-  public boolean equals(Object o) {
+  public final boolean equals(Object o) {
     return o instanceof GroupReference && compareTo((GroupReference) o) == 0;
   }
 
-  public String toConfigValue() {
-    return PREFIX + name;
-  }
-
   @Override
-  public String toString() {
+  public final String toString() {
     return "Group[" + getName() + " / " + getUUID() + "]";
   }
+
+  public String toConfigValue() {
+    return PREFIX + getName();
+  }
 }

java/com/google/gerrit/common/data/PermissionRule.java

@@ -255,8 +255,7 @@ public class PermissionRule implements Comparable<PermissionRule> {
 
     String groupName = GroupReference.extractGroupName(src);
     if (groupName != null) {
-      GroupReference group = new GroupReference();
+      GroupReference group = GroupReference.create(groupName);
-      group.setName(groupName);
       rule.setGroup(group);
     } else {
       throw new IllegalArgumentException("Rule must include group: " + orig);

java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java

@@ -91,7 +91,7 @@ public class LdapGroupBackend implements GroupBackend {
 
   private static GroupReference groupReference(ParameterizedString p, LdapQuery.Result res)
       throws NamingException {
-    return new GroupReference(
+    return GroupReference.create(
         AccountGroup.uuid(LDAP_UUID + res.getDN()), LDAP_NAME + LdapRealm.apply(p, res));
   }
 

java/com/google/gerrit/server/group/SystemGroupBackend.java

@@ -104,7 +104,7 @@ public class SystemGroupBackend extends AbstractGroupBackend {
       reservedNamesBuilder.add(defaultName);
       String configuredName = cfg.getString("groups", uuid.get(), "name");
       GroupReference ref =
-          new GroupReference(uuid, MoreObjects.firstNonNull(configuredName, defaultName));
+          GroupReference.create(uuid, MoreObjects.firstNonNull(configuredName, defaultName));
       n.put(ref.getName().toLowerCase(Locale.US), ref);
       u.put(ref.getUUID(), ref);
     }

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

@@ -443,7 +443,7 @@ public class GroupNameNotes extends VersionedMetaData {
       throw new ConfigInvalidException(String.format("UUID for group '%s' must be defined", name));
     }
 
-    return new GroupReference(AccountGroup.uuid(uuid), name);
+    return GroupReference.create(AccountGroup.uuid(uuid), name);
   }
 
   private String getCommitMessage() {

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

@@ -125,7 +125,7 @@ class RenameGroupOp extends DefaultQueueOp {
         return;
       }
 
-      ref.setName(newName);
+      config.renameGroup(uuid, newName);
       md.getCommitBuilder().setAuthor(author);
       md.setMessage("Rename group " + oldName + " to " + newName + "\n");
       try {

java/com/google/gerrit/server/project/GroupList.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.project;
 
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.entities.AccountGroup;
 import com.google.gerrit.entities.Project;
@@ -56,7 +57,7 @@ public class GroupList extends TabFile {
       }
       AccountGroup.UUID uuid = AccountGroup.uuid(row.left);
       String name = row.right;
-      GroupReference ref = new GroupReference(uuid, name);
+      GroupReference ref = GroupReference.create(uuid, name);
 
       groupsByUUID.put(uuid, ref);
     }
@@ -64,10 +65,26 @@ public class GroupList extends TabFile {
     return new GroupList(groupsByUUID);
   }
 
+  @Nullable
   public GroupReference byUUID(AccountGroup.UUID uuid) {
     return byUUID.get(uuid);
   }
 
+  @Nullable
+  public GroupReference byName(String name) {
+    return byUUID.entrySet().stream()
+        .map(Map.Entry::getValue)
+        .filter(groupReference -> name.equals(groupReference.getName()))
+        .findAny()
+        .orElse(null);
+  }
+
+  /**
+   * Returns the {@link GroupReference} instance that {@link GroupList} holds on to that has the
+   * same {@link com.google.gerrit.entities.AccountGroup.UUID} as the argument. Will store the
+   * argument internally, if no group with this {@link com.google.gerrit.entities.AccountGroup.UUID}
+   * was stored previously.
+   */
   public GroupReference resolve(GroupReference group) {
     if (group != null) {
       if (group.getUUID() == null || group.getUUID().get() == null) {
@@ -86,6 +103,10 @@ public class GroupList extends TabFile {
     return group;
   }
 
+  public void renameGroup(AccountGroup.UUID uuid, String name) {
+    byUUID.replace(uuid, GroupReference.create(uuid, name));
+  }
+
   public Collection<GroupReference> references() {
     return byUUID.values();
   }

java/com/google/gerrit/server/project/ProjectConfig.java

@@ -250,7 +250,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
   private Set<String> sectionsWithUnknownPermissions;
   private boolean hasLegacyPermissions;
   private Map<String, List<String>> extensionPanelSections;
-  private Map<String, GroupReference> groupsByName;
 
   public static CommentLinkInfoImpl buildCommentLink(Config cfg, String name, boolean allowRaw)
       throws IllegalArgumentException {
@@ -485,13 +484,11 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
   }
 
   public GroupReference resolve(GroupReference group) {
-    GroupReference groupRef = groupList.resolve(group);
+    return groupList.resolve(group);
-    if (groupRef != null
-        && groupRef.getUUID() != null
-        && !groupsByName.containsKey(groupRef.getName())) {
-      groupsByName.put(groupRef.getName(), groupRef);
   }
-    return groupRef;
+
+  public void renameGroup(AccountGroup.UUID uuid, String newName) {
+    groupList.renameGroup(uuid, newName);
   }
 
   /** @return the group reference, if the group is used by at least one rule. */
@@ -504,7 +501,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
    *     at least one rule or plugin value.
    */
   public GroupReference getGroup(String groupName) {
-    return groupsByName.get(groupName);
+    return groupList.byName(groupName);
   }
 
   /** @return set of all groups used by this configuration. */
@@ -541,7 +538,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       GroupDescription.Basic g = groupBackend.get(ref.getUUID());
       if (g != null && !g.getName().equals(ref.getName())) {
         dirty = true;
-        ref.setName(g.getName());
+        groupList.renameGroup(ref.getUUID(), g.getName());
       }
     }
     return dirty;
@@ -570,7 +567,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       baseConfig.load();
     }
     readGroupList();
-    groupsByName = mapGroupReferences();
 
     rulesId = getObjectId("rules.pl");
     Config rc = readConfig(PROJECT_CONFIG, baseConfig);
@@ -628,7 +624,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
   private void loadAccountsSection(Config rc) {
     accountsSection = new AccountsSection();
     accountsSection.setSameGroupVisibility(
-        loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, groupsByName, false));
+        loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, false));
   }
 
   private void loadExtensionPanelSections(Config rc) {
@@ -656,15 +652,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       ContributorAgreement ca = getContributorAgreement(name, true);
       ca.setDescription(rc.getString(CONTRIBUTOR_AGREEMENT, name, KEY_DESCRIPTION));
       ca.setAgreementUrl(rc.getString(CONTRIBUTOR_AGREEMENT, name, KEY_AGREEMENT_URL));
-      ca.setAccepted(
+      ca.setAccepted(loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_ACCEPTED, false));
-          loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_ACCEPTED, groupsByName, false));
       ca.setExcludeProjectsRegexes(
           loadPatterns(rc, CONTRIBUTOR_AGREEMENT, name, KEY_EXCLUDE_PROJECTS));
       ca.setMatchProjectsRegexes(loadPatterns(rc, CONTRIBUTOR_AGREEMENT, name, KEY_MATCH_PROJECTS));
 
       List<PermissionRule> rules =
-          loadPermissionRules(
+          loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY, false);
-              rc, CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY, groupsByName, false);
       if (rules.isEmpty()) {
         ca.setAutoVerify(null);
       } else if (rules.size() > 1) {
@@ -728,10 +722,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       for (String dst : rc.getStringList(NOTIFY, sectionName, KEY_EMAIL)) {
         String groupName = GroupReference.extractGroupName(dst);
         if (groupName != null) {
-          GroupReference ref = groupsByName.get(groupName);
+          GroupReference ref = groupList.byName(groupName);
           if (ref == null) {
-            ref = new GroupReference(groupName);
+            ref = groupList.resolve(GroupReference.create(groupName));
-            groupsByName.put(ref.getName(), ref);
           }
           if (ref.getUUID() != null) {
             n.addEmail(ref);
@@ -779,13 +772,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
           if (isCoreOrPluginPermission(convertedName)) {
             Permission perm = as.getPermission(convertedName, true);
             loadPermissionRules(
-                rc,
+                rc, ACCESS, refName, varName, perm, Permission.hasRange(convertedName));
-                ACCESS,
-                refName,
-                varName,
-                groupsByName,
-                perm,
-                Permission.hasRange(convertedName));
           } else {
             sectionsWithUnknownPermissions.add(as.getName());
           }
@@ -800,8 +787,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
         accessSections.put(AccessSection.GLOBAL_CAPABILITIES, capability);
       }
       Permission perm = capability.getPermission(varName, true);
-      loadPermissionRules(
+      loadPermissionRules(rc, CAPABILITY, null, varName, perm, GlobalCapability.hasRange(varName));
-          rc, CAPABILITY, null, varName, groupsByName, perm, GlobalCapability.hasRange(varName));
     }
   }
 
@@ -844,14 +830,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
   }
 
   private ImmutableList<PermissionRule> loadPermissionRules(
-      Config rc,
+      Config rc, String section, String subsection, String varName, boolean useRange) {
-      String section,
-      String subsection,
-      String varName,
-      Map<String, GroupReference> groupsByName,
-      boolean useRange) {
     Permission perm = new Permission(varName);
-    loadPermissionRules(rc, section, subsection, varName, groupsByName, perm, useRange);
+    loadPermissionRules(rc, section, subsection, varName, perm, useRange);
     return ImmutableList.copyOf(perm.getRules());
   }
 
@@ -860,7 +841,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       String section,
       String subsection,
       String varName,
-      Map<String, GroupReference> groupsByName,
       Permission perm,
       boolean useRange) {
     for (String ruleString : rc.getStringList(section, subsection, varName)) {
@@ -881,14 +861,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
         continue;
       }
 
-      GroupReference ref = groupsByName.get(rule.getGroup().getName());
+      GroupReference ref = groupList.byName(rule.getGroup().getName());
       if (ref == null) {
         // The group wasn't mentioned in the groups table, so there is
         // no valid UUID for it. Pool the reference anyway so at least
         // all rules in the same file share the same GroupReference.
         //
-        ref = rule.getGroup();
+        ref = groupList.resolve(rule.getGroup());
-        groupsByName.put(ref.getName(), ref);
         error(
             new ValidationError(
                 PROJECT_CONFIG, "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME));
@@ -1117,7 +1096,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
         String value = rc.getString(PLUGIN, plugin, name);
         String groupName = GroupReference.extractGroupName(value);
         if (groupName != null) {
-          GroupReference ref = groupsByName.get(groupName);
+          GroupReference ref = groupList.byName(groupName);
           if (ref == null) {
             error(
                 new ValidationError(
@@ -1144,16 +1123,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
     groupList = GroupList.parse(projectName, readUTF8(GroupList.FILE_NAME), this);
   }
 
-  private Map<String, GroupReference> mapGroupReferences() {
-    Collection<GroupReference> references = groupList.references();
-    Map<String, GroupReference> result = new HashMap<>(references.size());
-    for (GroupReference ref : references) {
-      result.put(ref.getName(), ref);
-    }
-
-    return result;
-  }
-
   @Override
   protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
     if (commit.getMessage() == null || "".equals(commit.getMessage())) {
@@ -1558,7 +1527,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
         String value = pluginConfig.getString(PLUGIN, plugin, name);
         String groupName = GroupReference.extractGroupName(value);
         if (groupName != null) {
-          GroupReference ref = groupsByName.get(groupName);
+          GroupReference ref = groupList.byName(groupName);
           if (ref != null && ref.getUUID() != null) {
             keepGroups.add(ref.getUUID());
             pluginConfig.setString(PLUGIN, plugin, name, "group " + ref.getName());

java/com/google/gerrit/server/schema/SchemaCreatorImpl.java

@@ -212,7 +212,7 @@ public class SchemaCreatorImpl implements SchemaCreator {
 
   private GroupReference createGroupReference(String name) {
     AccountGroup.UUID groupUuid = GroupUuid.make(name, serverUser);
-    return new GroupReference(groupUuid, name);
+    return GroupReference.create(groupUuid, name);
   }
 
   private InternalGroupCreation getGroupCreation(Sequences seqs, GroupReference groupReference) {

javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java

@@ -81,7 +81,7 @@ public class AgreementsIT extends AbstractDaemonTest {
     GroupApi groupApi = gApi.groups().id(g.get());
     groupApi.description("CLA test group");
     InternalGroup caGroup = group(AccountGroup.uuid(groupApi.detail().id));
-    GroupReference groupRef = new GroupReference(caGroup.getGroupUUID(), caGroup.getName());
+    GroupReference groupRef = GroupReference.create(caGroup.getGroupUUID(), caGroup.getName());
     PermissionRule rule = new PermissionRule(groupRef);
     rule.setAction(PermissionRule.Action.ALLOW);
     if (autoVerify) {

javatests/com/google/gerrit/common/data/GroupReferenceTest.java

@@ -58,7 +58,7 @@ public class GroupReferenceTest {
   public void create() {
     AccountGroup.UUID uuid = AccountGroup.uuid("uuid");
     String name = "foo";
-    GroupReference groupReference = new GroupReference(uuid, name);
+    GroupReference groupReference = GroupReference.create(uuid, name);
     assertThat(groupReference.getUUID()).isEqualTo(uuid);
     assertThat(groupReference.getName()).isEqualTo(name);
   }
@@ -68,7 +68,7 @@ public class GroupReferenceTest {
     // GroupReferences where the UUID is null are used to represent groups from project.config that
     // cannot be resolved.
     String name = "foo";
-    GroupReference groupReference = new GroupReference(name);
+    GroupReference groupReference = GroupReference.create(name);
     assertThat(groupReference.getUUID()).isNull();
     assertThat(groupReference.getName()).isEqualTo(name);
   }
@@ -76,7 +76,7 @@ public class GroupReferenceTest {
   @Test
   public void cannotCreateWithoutName() {
     assertThrows(
-        NullPointerException.class, () -> new GroupReference(AccountGroup.uuid("uuid"), null));
+        NullPointerException.class, () -> GroupReference.create(AccountGroup.uuid("uuid"), null));
   }
 
   @Test
@@ -97,41 +97,10 @@ public class GroupReferenceTest {
     assertThat(GroupReference.extractGroupName("group foo bar")).isEqualTo("foo bar");
   }
 
-  @Test
-  public void getAndSetUuid() {
-    AccountGroup.UUID uuid = AccountGroup.uuid("uuid-foo");
-    String name = "foo";
-    GroupReference groupReference = new GroupReference(uuid, name);
-    assertThat(groupReference.getUUID()).isEqualTo(uuid);
-
-    AccountGroup.UUID uuid2 = AccountGroup.uuid("uuid-bar");
-    groupReference.setUUID(uuid2);
-    assertThat(groupReference.getUUID()).isEqualTo(uuid2);
-
-    // GroupReferences where the UUID is null are used to represent groups from project.config that
-    // cannot be resolved.
-    groupReference.setUUID(null);
-    assertThat(groupReference.getUUID()).isNull();
-  }
-
-  @Test
-  public void getAndSetName() {
-    AccountGroup.UUID uuid = AccountGroup.uuid("uuid-foo");
-    String name = "foo";
-    GroupReference groupReference = new GroupReference(uuid, name);
-    assertThat(groupReference.getName()).isEqualTo(name);
-
-    String name2 = "bar";
-    groupReference.setName(name2);
-    assertThat(groupReference.getName()).isEqualTo(name2);
-
-    assertThrows(NullPointerException.class, () -> groupReference.setName(null));
-  }
-
   @Test
   public void toConfigValue() {
     String name = "foo";
-    GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-foo"), name);
+    GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-foo"), name);
     assertThat(groupReference.toConfigValue()).isEqualTo("group " + name);
   }
 
@@ -142,9 +111,9 @@ public class GroupReferenceTest {
     String name1 = "foo";
     String name2 = "bar";
 
-    GroupReference groupReference1 = new GroupReference(uuid1, name1);
+    GroupReference groupReference1 = GroupReference.create(uuid1, name1);
-    GroupReference groupReference2 = new GroupReference(uuid1, name2);
+    GroupReference groupReference2 = GroupReference.create(uuid1, name2);
-    GroupReference groupReference3 = new GroupReference(uuid2, name1);
+    GroupReference groupReference3 = GroupReference.create(uuid2, name1);
 
     assertThat(groupReference1.equals(groupReference2)).isTrue();
     assertThat(groupReference1.equals(groupReference3)).isFalse();
@@ -154,10 +123,10 @@ public class GroupReferenceTest {
   @Test
   public void testHashcode() {
     AccountGroup.UUID uuid1 = AccountGroup.uuid("uuid1");
-    assertThat(new GroupReference(uuid1, "foo").hashCode())
+    assertThat(GroupReference.create(uuid1, "foo").hashCode())
-        .isEqualTo(new GroupReference(uuid1, "bar").hashCode());
+        .isEqualTo(GroupReference.create(uuid1, "bar").hashCode());
 
     // Check that the following calls don't fail with an exception.
-    new GroupReference("bar").hashCode();
+    GroupReference.create("bar").hashCode();
   }
 }

javatests/com/google/gerrit/common/data/PermissionRuleTest.java

@@ -28,7 +28,7 @@ public class PermissionRuleTest {
 
   @Before
   public void setup() {
-    this.groupReference = new GroupReference(AccountGroup.uuid("uuid"), "group");
+    this.groupReference = GroupReference.create(AccountGroup.uuid("uuid"), "group");
     this.permissionRule = new PermissionRule(groupReference);
   }
 
@@ -130,7 +130,7 @@ public class PermissionRuleTest {
 
   @Test
   public void setGroup() {
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     assertThat(groupReference2).isNotEqualTo(groupReference);
 
     assertThat(permissionRule.getGroup()).isEqualTo(groupReference);
@@ -141,10 +141,10 @@ public class PermissionRuleTest {
 
   @Test
   public void mergeFromAnyBlock() {
-    GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1");
+    GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1");
     PermissionRule permissionRule1 = new PermissionRule(groupReference1);
 
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     PermissionRule permissionRule2 = new PermissionRule(groupReference2);
 
     permissionRule1.mergeFrom(permissionRule2);
@@ -169,10 +169,10 @@ public class PermissionRuleTest {
 
   @Test
   public void mergeFromAnyDeny() {
-    GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1");
+    GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1");
     PermissionRule permissionRule1 = new PermissionRule(groupReference1);
 
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     PermissionRule permissionRule2 = new PermissionRule(groupReference2);
 
     permissionRule1.mergeFrom(permissionRule2);
@@ -192,10 +192,10 @@ public class PermissionRuleTest {
 
   @Test
   public void mergeFromAnyBatch() {
-    GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1");
+    GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1");
     PermissionRule permissionRule1 = new PermissionRule(groupReference1);
 
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     PermissionRule permissionRule2 = new PermissionRule(groupReference2);
 
     permissionRule1.mergeFrom(permissionRule2);
@@ -215,10 +215,10 @@ public class PermissionRuleTest {
 
   @Test
   public void mergeFromAnyForce() {
-    GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1");
+    GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1");
     PermissionRule permissionRule1 = new PermissionRule(groupReference1);
 
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     PermissionRule permissionRule2 = new PermissionRule(groupReference2);
 
     permissionRule1.mergeFrom(permissionRule2);
@@ -238,11 +238,11 @@ public class PermissionRuleTest {
 
   @Test
   public void mergeFromMergeRange() {
-    GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1");
+    GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1");
     PermissionRule permissionRule1 = new PermissionRule(groupReference1);
     permissionRule1.setRange(-1, 2);
 
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     PermissionRule permissionRule2 = new PermissionRule(groupReference2);
     permissionRule2.setRange(-2, 1);
 
@@ -255,10 +255,10 @@ public class PermissionRuleTest {
 
   @Test
   public void mergeFromGroupNotChanged() {
-    GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1");
+    GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1");
     PermissionRule permissionRule1 = new PermissionRule(groupReference1);
 
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     PermissionRule permissionRule2 = new PermissionRule(groupReference2);
 
     permissionRule1.mergeFrom(permissionRule2);
@@ -347,7 +347,7 @@ public class PermissionRuleTest {
 
   @Test
   public void testEquals() {
-    GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2");
+    GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2");
     PermissionRule permissionRuleOther = new PermissionRule(groupReference2);
     assertThat(permissionRule.equals(permissionRuleOther)).isFalse();
 

javatests/com/google/gerrit/common/data/PermissionTest.java

@@ -154,14 +154,14 @@ public class PermissionTest {
   @Test
   public void setAndGetRules() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
     permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
     assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder();
 
     PermissionRule permissionRule3 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-3"), "group3"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"));
     permission.setRules(ImmutableList.of(permissionRule3));
     assertThat(permission.getRules()).containsExactly(permissionRule3);
   }
@@ -169,10 +169,10 @@ public class PermissionTest {
   @Test
   public void cannotAddPermissionByModifyingListThatWasProvidedToAccessSection() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
-    GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3");
+    GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
 
     List<PermissionRule> rules = new ArrayList<>();
     rules.add(permissionRule1);
@@ -187,14 +187,14 @@ public class PermissionTest {
 
   @Test
   public void getNonExistingRule() {
-    GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-1"), "group1");
+    GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
     assertThat(permission.getRule(groupReference)).isNull();
     assertThat(permission.getRule(groupReference, false)).isNull();
   }
 
   @Test
   public void getRule() {
-    GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-1"), "group1");
+    GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
     PermissionRule permissionRule = new PermissionRule(groupReference);
     permission.setRules(ImmutableList.of(permissionRule));
     assertThat(permission.getRule(groupReference)).isEqualTo(permissionRule);
@@ -202,7 +202,7 @@ public class PermissionTest {
 
   @Test
   public void createMissingRuleOnGet() {
-    GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-1"), "group1");
+    GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
     assertThat(permission.getRule(groupReference)).isNull();
 
     assertThat(permission.getRule(groupReference, true))
@@ -212,11 +212,11 @@ public class PermissionTest {
   @Test
   public void addRule() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
     permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
-    GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3");
+    GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
     assertThat(permission.getRule(groupReference3)).isNull();
 
     PermissionRule permissionRule3 = new PermissionRule(groupReference3);
@@ -230,10 +230,10 @@ public class PermissionTest {
   @Test
   public void removeRule() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
-    GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3");
+    GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
     PermissionRule permissionRule3 = new PermissionRule(groupReference3);
 
     permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3));
@@ -247,10 +247,10 @@ public class PermissionTest {
   @Test
   public void removeRuleByGroupReference() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
-    GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3");
+    GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
     PermissionRule permissionRule3 = new PermissionRule(groupReference3);
 
     permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3));
@@ -264,9 +264,9 @@ public class PermissionTest {
   @Test
   public void clearRules() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
 
     permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
     assertThat(permission.getRules()).isNotEmpty();
@@ -278,11 +278,11 @@ public class PermissionTest {
   @Test
   public void mergePermissions() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
     PermissionRule permissionRule3 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-3"), "group3"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"));
 
     Permission permission1 = new Permission("foo");
     permission1.setRules(ImmutableList.of(permissionRule1, permissionRule2));
@@ -299,9 +299,9 @@ public class PermissionTest {
   @Test
   public void testEquals() {
     PermissionRule permissionRule1 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
     PermissionRule permissionRule2 =
-        new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2"));
+        new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
 
     permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
 

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

@@ -393,8 +393,8 @@ public class GroupNameNotesTest {
 
     ImmutableList<GroupReference> allGroups = GroupNameNotes.loadAllGroups(repo);
 
-    GroupReference group1 = new GroupReference(groupUuid1, groupName1.get());
+    GroupReference group1 = GroupReference.create(groupUuid1, groupName1.get());
-    GroupReference group2 = new GroupReference(groupUuid2, groupName2.get());
+    GroupReference group2 = GroupReference.create(groupUuid2, groupName2.get());
     assertThat(allGroups).containsExactly(group1, group2);
   }
 
@@ -406,8 +406,8 @@ public class GroupNameNotesTest {
 
     ImmutableList<GroupReference> allGroups = GroupNameNotes.loadAllGroups(repo);
 
-    GroupReference group1 = new GroupReference(groupUuid, groupName.get());
+    GroupReference group1 = GroupReference.create(groupUuid, groupName.get());
-    GroupReference group2 = new GroupReference(groupUuid, anotherGroupName.get());
+    GroupReference group2 = GroupReference.create(groupUuid, anotherGroupName.get());
     assertThat(allGroups).containsExactly(group1, group2);
   }
 
@@ -498,14 +498,14 @@ public class GroupNameNotesTest {
   @Test
   public void updateGroupNamesRejectsNonOneToOneGroupReferences() throws Exception {
     assertIllegalArgument(
-        new GroupReference(AccountGroup.uuid("uuid1"), "name1"),
+        GroupReference.create(AccountGroup.uuid("uuid1"), "name1"),
-        new GroupReference(AccountGroup.uuid("uuid1"), "name2"));
+        GroupReference.create(AccountGroup.uuid("uuid1"), "name2"));
     assertIllegalArgument(
-        new GroupReference(AccountGroup.uuid("uuid1"), "name1"),
+        GroupReference.create(AccountGroup.uuid("uuid1"), "name1"),
-        new GroupReference(AccountGroup.uuid("uuid2"), "name1"));
+        GroupReference.create(AccountGroup.uuid("uuid2"), "name1"));
     assertIllegalArgument(
-        new GroupReference(AccountGroup.uuid("uuid1"), "name1"),
+        GroupReference.create(AccountGroup.uuid("uuid1"), "name1"),
-        new GroupReference(AccountGroup.uuid("uuid1"), "name1"));
+        GroupReference.create(AccountGroup.uuid("uuid1"), "name1"));
   }
 
   @Test
@@ -554,7 +554,7 @@ public class GroupNameNotesTest {
 
   private GroupReference newGroup(String name) {
     int id = idCounter.incrementAndGet();
-    return new GroupReference(AccountGroup.uuid(name + "-" + id), name);
+    return GroupReference.create(AccountGroup.uuid(name + "-" + id), name);
   }
 
   private static PersonIdent newPersonIdent() {

javatests/com/google/gerrit/server/project/GroupListTest.java

@@ -63,7 +63,7 @@ public class GroupListTest {
   @Test
   public void put() {
     AccountGroup.UUID uuid = AccountGroup.uuid("abc");
-    GroupReference groupReference = new GroupReference(uuid, "Hutzliputz");
+    GroupReference groupReference = GroupReference.create(uuid, "Hutzliputz");
 
     groupList.put(uuid, groupReference);
 
@@ -78,7 +78,7 @@ public class GroupListTest {
 
     assertEquals(2, result.size());
     AccountGroup.UUID uuid = AccountGroup.uuid("ebe31c01aec2c9ac3b3c03e87a47450829ff4310");
-    GroupReference expected = new GroupReference(uuid, "Administrators");
+    GroupReference expected = GroupReference.create(uuid, "Administrators");
 
     assertTrue(result.contains(expected));
   }

javatests/com/google/gerrit/server/project/ProjectConfigTest.java

@@ -90,8 +90,8 @@ public class ProjectConfigTest {
   @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
   private final GroupReference developers =
-      new GroupReference(AccountGroup.uuid("X"), "Developers");
+      GroupReference.create(AccountGroup.uuid("X"), "Developers");
-  private final GroupReference staff = new GroupReference(AccountGroup.uuid("Y"), "Staff");
+  private final GroupReference staff = GroupReference.create(AccountGroup.uuid("Y"), "Staff");
 
   private SitePaths sitePaths;
   private ProjectConfig.Factory factory;

javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -1861,7 +1861,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
       ProjectConfig config = projectConfigFactory.read(md);
       AccessSection s = config.getAccessSection(ref, true);
       Permission p = s.getPermission(permission, true);
-      PermissionRule rule = new PermissionRule(new GroupReference(groupUUID, groupUUID.get()));
+      PermissionRule rule = new PermissionRule(GroupReference.create(groupUUID, groupUUID.get()));
       rule.setForce(force);
       p.add(rule);
       config.commit(md);

javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java

@@ -102,7 +102,7 @@ public class AllProjectsCreatorTest {
 
   private GroupReference createGroupReference(String name) {
     AccountGroup.UUID groupUuid = GroupUuid.make(name, serverUser);
-    return new GroupReference(groupUuid, name);
+    return GroupReference.create(groupUuid, name);
   }
 
   @Test