commit 57ad75ca0ab5d6e9759822a3f9e369a5dfc3ae37
author Changcheng Xiao <xchangcheng@google.com> Tue Mar 19 21:52:39 2019 +0100
committer Changcheng Xiao <xchangcheng@google.com> Wed Mar 20 09:11:49 2019 +0100
tree 88d3dd7c579f41b328ac6f994467d380c003b35b
parent 74b7e82b258bb6bb056622121afb5aceb27785fb

NoteDbCheckersUpdate: fix to not include disabled checker on creation

This commit fixes the "NoteDbCheckerUpdate" to avoid
adding a checker to "CheckersByRepositoryNotes" if
it's disabled on creation.

Change-Id: I7fdb7c872d501317229df42eb02355219cc4b87d

java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java

diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java
index c856c54..6da5a2a 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java
@@ -162,8 +162,13 @@
 
       CheckersByRepositoryNotes checkersByRepositoryNotes =
           CheckersByRepositoryNotes.load(allProjectsName, allProjectsRepo);
-      checkersByRepositoryNotes.insert(
-          checkerCreation.getCheckerUuid(), checkerCreation.getRepository());
+      if (!checkerUpdate.getStatus().isPresent()
+          || checkerUpdate.getStatus().get() == CheckerStatus.ENABLED) {
+        // Only inserts to the notes if the status is not set or set as "ENABLED". Does not insert
+        // if the checker is DISABLED.
+        checkersByRepositoryNotes.insert(
+            checkerCreation.getCheckerUuid(), checkerCreation.getRepository());
+      }
 
       commit(allProjectsRepo, checkerConfig, checkersByRepositoryNotes);
 

javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java

diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java
index 9e47902..d351813 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java
@@ -19,6 +19,7 @@
 import com.google.gerrit.plugins.checks.Checker;
 import com.google.gerrit.plugins.checks.CheckerUuid;
 import com.google.gerrit.plugins.checks.Checkers;
+import com.google.gerrit.plugins.checks.api.CheckerStatus;
 import com.google.gerrit.reviewdb.client.Project;
 import java.util.stream.Stream;
 import org.junit.Test;
@@ -45,19 +46,19 @@
 
   @Test
   public void checkersOfOmitsDisabledCheckers() throws Exception {
+    // Creates a disabled checker.
+    checkerOperations.newChecker().repository(project).status(CheckerStatus.DISABLED).create();
+    // Creates an enabled checker and then disabled it by an update.
     CheckerUuid checkerUuid1 = checkerOperations.newChecker().repository(project).create();
-    CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
     checkerOperations.checker(checkerUuid1).forUpdate().disable().update();
+    // Creates an enabled checker.
+    CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
 
     assertThat(getCheckerUuidsOf(project)).containsExactly(checkerUuid2);
   }
 
   private Stream<CheckerUuid> getCheckerUuidsOf(Project.NameKey projectName) throws Exception {
-    return plugin
-        .getSysInjector()
-        .getInstance(Checkers.class)
-        .checkersOf(projectName)
-        .stream()
+    return plugin.getSysInjector().getInstance(Checkers.class).checkersOf(projectName).stream()
         .map(Checker::getUuid);
   }
 }