Implement Checks#areAllRequiredChecksPassing
Hard-coding "passing" state in the "CombinedCheckState"
enum makes it hard to decide whether all required
checker have passed when their combined check state
is "IN_PROGRESS".
This commit adds a new method to "Checks" interface
to calculate the "passing" state based on the
"CheckStateCount", which is also used to create
a combined check state.
This new method can be used by other code, e.g.
ChecksSubmitRule, who cares about whether all required
checkers have passed or not.
Change-Id: If16d42b46c15cf3ac40c45947cfd446d5c96f5ef
diff --git a/java/com/google/gerrit/plugins/checks/Checks.java b/java/com/google/gerrit/plugins/checks/Checks.java
index 40a9c4c..c4527e5 100644
--- a/java/com/google/gerrit/plugins/checks/Checks.java
+++ b/java/com/google/gerrit/plugins/checks/Checks.java
@@ -78,6 +78,18 @@
CombinedCheckState getCombinedCheckState(Project.NameKey projectName, PatchSet.Id patchSetId)
throws IOException, StorageException;
+ /**
+ * Returns whether all required checks have passed.
+ *
+ * @param projectName the name of the project.
+ * @param patchSetId the ID of the patch set
+ * @return true if all required checks have passed.
+ * @throws IOException if failed to check if all required checks have passed.
+ * @throws StorageException if failed to check if all required checks have passed.
+ */
+ boolean areAllRequiredCheckersPassing(Project.NameKey projectName, PatchSet.Id patchSetId)
+ throws IOException, StorageException;
+
@AutoValue
abstract class GetCheckOptions {
public static GetCheckOptions defaults() {
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index 1fbbb4b..bd3dd1e 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -32,6 +32,7 @@
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
+import com.google.gerrit.plugins.checks.api.CombinedCheckState.CheckStateCount;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.Project;
@@ -128,8 +129,24 @@
@Override
public CombinedCheckState getCombinedCheckState(NameKey projectName, Id patchSetId)
throws IOException, StorageException {
+ ImmutableListMultimap<CheckState, Boolean> statesAndRequired =
+ getStatesAndRequiredMap(projectName, patchSetId);
+ return CombinedCheckState.combine(statesAndRequired);
+ }
+
+ @Override
+ public boolean areAllRequiredCheckersPassing(NameKey projectName, Id patchSetId)
+ throws IOException, StorageException {
+ ImmutableListMultimap<CheckState, Boolean> statesAndRequired =
+ getStatesAndRequiredMap(projectName, patchSetId);
+ CheckStateCount checkStateCount = CheckStateCount.create(statesAndRequired);
+ return checkStateCount.failedRequiredCount() == 0
+ && checkStateCount.inProgressRequiredCount() == 0;
+ }
+
+ private ImmutableListMultimap<CheckState, Boolean> getStatesAndRequiredMap(
+ NameKey projectName, Id patchSetId) throws IOException, StorageException {
ChangeData changeData = changeDataFactory.create(projectName, patchSetId.changeId());
- CheckerQuery checkerQuery = checkerQueryProvider.get();
ImmutableMap<String, Checker> allCheckersOfProject =
checkers.checkersOf(projectName).stream()
.collect(ImmutableMap.toImmutableMap(c -> c.getUuid().get(), c -> c));
@@ -157,11 +174,11 @@
boolean isRequired =
checker.getStatus() == CheckerStatus.ENABLED
&& checker.isRequired()
- && checkerQuery.isCheckerRelevant(checker, changeData);
+ && checkerQueryProvider.get().isCheckerRelevant(checker, changeData);
statesAndRequired.put(check.state(), isRequired);
}
- return CombinedCheckState.combine(statesAndRequired.build());
+ return statesAndRequired.build();
}
private ImmutableList<Checker> getCheckersForBackfiller(
