GroupList: Log null UUIDs

These shouldn't happen, but we've observed some in production
(possibly involving a custom group backend). Logging might help us
track it down.

Change-Id: Ib7a956774647d1bea4fb49ad55376de0408dcf71

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.pgm.init.api;
 
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.GroupList;
@@ -66,7 +67,9 @@ public class AllProjectsConfig extends VersionedMetaDataOnInit {
   }
 
   private GroupList readGroupList() throws IOException {
-    return GroupList.parse(readUTF8(GroupList.FILE_NAME),
+    return GroupList.parse(
+        new Project.NameKey(project),
+        readUTF8(GroupList.FILE_NAME),
         GroupList.createLoggerSink(GroupList.FILE_NAME, log));
   }
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/VersionedMetaDataOnInit.java

@@ -37,9 +37,9 @@ import java.nio.file.Path;
 
 public abstract class VersionedMetaDataOnInit extends VersionedMetaData {
 
+  protected final String project;
   private final InitFlags flags;
   private final SitePaths site;
-  private final String project;
   private final String ref;
 
   protected VersionedMetaDataOnInit(InitFlags flags, SitePaths site,

gerrit-server/src/main/java/com/google/gerrit/server/git/GroupList.java

@@ -16,7 +16,10 @@ package com.google.gerrit.server.git;
 
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
+import com.google.gerrit.reviewdb.client.Project;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -27,19 +30,29 @@ import java.util.Map;
 import java.util.Set;
 
 public class GroupList extends TabFile {
+  private static final Logger log = LoggerFactory.getLogger(GroupList.class);
+
   public static final String FILE_NAME = "groups";
+
+  private final Project.NameKey project;
   private final Map<AccountGroup.UUID, GroupReference> byUUID;
 
-  private GroupList(Map<AccountGroup.UUID, GroupReference> byUUID) {
+  private GroupList(Project.NameKey project,
+      Map<AccountGroup.UUID, GroupReference> byUUID) {
+    this.project = project;
     this.byUUID = byUUID;
   }
 
-  public static GroupList parse(String text, ValidationError.Sink errors)
+  public static GroupList parse(Project.NameKey project, String text,
-      throws IOException {
+      ValidationError.Sink errors) throws IOException {
     List<Row> rows = parse(text, FILE_NAME, TRIM, TRIM, errors);
     Map<AccountGroup.UUID, GroupReference> groupsByUUID =
         new HashMap<>(rows.size());
     for (Row row : rows) {
+      if (row.left == null) {
+        log.warn("null field in group list for {}:\n{}", project, text);
+        continue;
+      }
       AccountGroup.UUID uuid = new AccountGroup.UUID(row.left);
       String name = row.right;
       GroupReference ref = new GroupReference(uuid, name);
@@ -47,7 +60,7 @@ public class GroupList extends TabFile {
       groupsByUUID.put(uuid, ref);
     }
 
-    return new GroupList(groupsByUUID);
+    return new GroupList(project, groupsByUUID);
   }
 
   public GroupReference byUUID(AccountGroup.UUID uuid) {
@@ -56,6 +69,10 @@ public class GroupList extends TabFile {
 
   public GroupReference resolve(GroupReference group) {
     if (group != null) {
+      if (group.getUUID() == null || group.getUUID().get() == null) {
+        log.warn("attempting to resolve null group in {}: {}", project, group);
+        return group;
+      }
       GroupReference ref = byUUID.get(group.getUUID());
       if (ref != null) {
         return ref;
@@ -73,7 +90,11 @@ public class GroupList extends TabFile {
     return byUUID.keySet();
   }
 
-  public void put(UUID uuid, GroupReference reference) {
+  public void put(AccountGroup.UUID uuid, GroupReference reference) {
+    if (uuid == null || uuid.get() == null) {
+      log.warn("attempting to put null group in {}: {}", project, reference);
+      return;
+    }
     byUUID.put(uuid, reference);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java

@@ -910,7 +910,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
   }
 
   private void readGroupList() throws IOException {
-    groupList = GroupList.parse(readUTF8(GroupList.FILE_NAME), this);
+    groupList = GroupList.parse(
+        projectName, readUTF8(GroupList.FILE_NAME), this);
   }
 
   private Map<String, GroupReference> mapGroupReferences() {

gerrit-server/src/test/java/com/google/gerrit/server/git/GroupListTest.java

@@ -27,6 +27,7 @@ import static org.junit.Assert.assertTrue;
 
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.Project;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -37,7 +38,7 @@ import java.util.Collections;
 import java.util.Set;
 
 public class GroupListTest {
-
+  private static final Project.NameKey PROJECT = new Project.NameKey("project");
   private static final String TEXT =
       "# UUID                                  \tGroup Name\n" + "#\n"
           + "d96b998f8a66ff433af50befb975d0e2bb6e0999\tNon-Interactive Users\n"
@@ -49,7 +50,7 @@ public class GroupListTest {
   public void setup() throws IOException {
     ValidationError.Sink sink = createNiceMock(ValidationError.Sink.class);
     replay(sink);
-    groupList = GroupList.parse(TEXT, sink);
+    groupList = GroupList.parse(PROJECT, TEXT, sink);
   }
 
   @Test
@@ -103,7 +104,7 @@ public class GroupListTest {
     sink.error(anyObject(ValidationError.class));
     expectLastCall().times(2);
     replay(sink);
-    groupList = GroupList.parse(TEXT.replace("\t", "    "), sink);
+    groupList = GroupList.parse(PROJECT, TEXT.replace("\t", "    "), sink);
     verify(sink);
   }