From 59fecfc6e366364bd2c220b5f003ca09a1fa2a75 Mon Sep 17 00:00:00 2001
From: Changcheng Xiao <xchangcheng@google.com>
Date: Mon, 27 Nov 2017 15:21:55 +0100
Subject: [PATCH] GroupNameNotes: add consistency check for
 loadAllGroupReferences

This change checks whether there are duplicate UUIDs when we
load all of the group name notes. Possible inconsistencies are
logged as warnings.

Change-Id: Id68353c9e58710b9f4db967c7ec8ac6582a75e6f
---
 .../gerrit/server/group/db/GroupNameNotes.java  | 15 ++++++++++++---
 .../db/GroupsNoteDbConsistencyChecker.java      | 17 ++++++++++++++++-
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/java/com/google/gerrit/server/group/db/GroupNameNotes.java b/java/com/google/gerrit/server/group/db/GroupNameNotes.java
index 6a9166bd28..aa15decca0 100644
--- a/java/com/google/gerrit/server/group/db/GroupNameNotes.java
+++ b/java/com/google/gerrit/server/group/db/GroupNameNotes.java
@@ -32,9 +32,11 @@ import com.google.gerrit.server.git.VersionedMetaData;
 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;
@@ -178,12 +180,19 @@ public class GroupNameNotes extends VersionedMetaData {
         ObjectReader reader = revWalk.getObjectReader()) {
       RevCommit notesCommit = revWalk.parseCommit(ref.getObjectId());
       NoteMap noteMap = NoteMap.read(reader, notesCommit);
-      ImmutableSet.Builder<GroupReference> groupReferences = ImmutableSet.builder();
+
+      Set<GroupReference> groupReferences = new LinkedHashSet<>();
       for (Note note : noteMap) {
         GroupReference groupReference = getGroupReference(reader, note.getData());
-        groupReferences.add(groupReference);
+        boolean result = groupReferences.add(groupReference);
+        if (!result) {
+          GroupsNoteDbConsistencyChecker.logConsistencyProblemAsWarning(
+              "The UUID of group %s (%s) is duplicate in group name notes",
+              groupReference.getName(), groupReference.getUUID());
+        }
       }
-      return groupReferences.build();
+
+      return ImmutableSet.copyOf(groupReferences);
     }
   }
 
diff --git a/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java b/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
index 204695f55d..899c3a9e0b 100644
--- a/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
+++ b/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.server.group.db;
 
 import static com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo.error;
+import static com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo.warning;
 
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
@@ -41,11 +42,13 @@ import org.eclipse.jgit.notes.Note;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** Check the referential integrity of NoteDb group storage. */
 @Singleton
 public class GroupsNoteDbConsistencyChecker {
-
+  private static final Logger log = LoggerFactory.getLogger(GroupsNoteDbConsistencyChecker.class);
   /**
    * The result of a consistency check. The UUID map is only non-null if no problems were detected.
    */
@@ -206,4 +209,16 @@ public class GroupsNoteDbConsistencyChecker {
 
     return problems;
   }
+
+  public static void logConsistencyProblemAsWarning(String fmt, Object... args) {
+    logConsistencyProblem(warning(fmt, args));
+  }
+
+  public static void logConsistencyProblem(ConsistencyProblemInfo p) {
+    if (p.status == ConsistencyProblemInfo.Status.WARNING) {
+      log.warn(p.message);
+    } else {
+      log.error(p.message);
+    }
+  }
 }