Add support for listing pending checks by scheme prefix
We used to support those types of queries:
GET /plugins/checks~checks.pending/?query=checker:<checker-uuid>&state=<state>
GET /plugins/checks~checks.pending/?query=checker:<checker-uuid>
Often users want to know all the checks of a specific scheme, since they
are often related. It is time consuming to query all checks by UUID one
by one.
This change also supports query by scheme, in addition to query by UUID:
GET /plugins/checks~checks.pending/?query=scheme:<scheme_name>&state=<state>
GET /plugins/checks~checks.pending/?query=scheme:<scheme_name>
Those queries return all the relevant checks with that scheme, when
successful (it fails if there are more than 10 different UUID with that
scheme).
Change-Id: I70c74dbca699ffc4966c689cd0a97e17d9dedcf8
diff --git a/java/com/google/gerrit/plugins/checks/CheckerQuery.java b/java/com/google/gerrit/plugins/checks/CheckerQuery.java
index 71f7ff2..accab2a 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerQuery.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerQuery.java
@@ -25,6 +25,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
@@ -45,9 +46,11 @@
import com.google.gerrit.server.update.RetryHelper.ActionType;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;
+import java.util.stream.Collectors;
import org.antlr.runtime.tree.Tree;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -234,6 +237,24 @@
return query;
}
+ public List<List<ChangeData>> queryMatchingChanges(List<Checker> checkers)
+ throws ConfigInvalidException {
+
+ try {
+ List<Predicate<ChangeData>> predicateList = new ArrayList<>();
+ for (Checker checker : checkers) {
+ predicateList.add(
+ createQueryPredicate(checker.getUuid(), checker.getRepository(), checker.getQuery()));
+ }
+ return executeIndexQueryWithRetry(qp -> {}, predicateList);
+ } catch (QueryParseException e) {
+ throw new ConfigInvalidException(
+ String.format(
+ "A checker in scheme %s has an invalid query (%s)",
+ checkers.get(0).getUuid().scheme(), e.getMessage()));
+ }
+ }
+
public List<ChangeData> queryMatchingChanges(Checker checker)
throws ConfigInvalidException, StorageException {
return queryMatchingChanges(
@@ -300,13 +321,21 @@
private List<ChangeData> executeIndexQueryWithRetry(
Consumer<ChangeQueryProcessor> queryProcessorSetup, Predicate<ChangeData> predicate)
throws StorageException, QueryParseException {
+ return executeIndexQueryWithRetry(queryProcessorSetup, ImmutableList.of(predicate)).get(0);
+ }
+
+ private List<List<ChangeData>> executeIndexQueryWithRetry(
+ Consumer<ChangeQueryProcessor> queryProcessorSetup, List<Predicate<ChangeData>> predicateList)
+ throws StorageException, QueryParseException {
try {
return retryHelper.execute(
ActionType.INDEX_QUERY,
() -> {
ChangeQueryProcessor qp = changeQueryProcessorProvider.get();
queryProcessorSetup.accept(qp);
- return qp.query(predicate).entities();
+ return qp.query(predicateList).stream()
+ .map(predicate -> predicate.entities())
+ .collect(Collectors.toList());
},
StorageException.class::isInstance);
} catch (Exception e) {
diff --git a/java/com/google/gerrit/plugins/checks/Checkers.java b/java/com/google/gerrit/plugins/checks/Checkers.java
index 9c00528..d08f1f1 100644
--- a/java/com/google/gerrit/plugins/checks/Checkers.java
+++ b/java/com/google/gerrit/plugins/checks/Checkers.java
@@ -73,6 +73,16 @@
ImmutableList<Checker> listCheckers() throws IOException;
/**
+ * Returns a list with all checkers of the given scheme.
+ *
+ * <p>Checkers with invalid configuration are silently ignored.
+ *
+ * @param scheme the name of the scheme of the relevant checkers
+ * @return all checkers with that scheme, sorted by UUID
+ * @throws IOException if any checker couldn't be retrieved from the storage
+ */
+ ImmutableList<Checker> listCheckers(String scheme) throws IOException;
+ /**
* Returns the checkers that apply to the given repository.
*
* <p>Never returns disabled checkers. Checkers with invalid configuration are silently ignored.
diff --git a/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
index 870cd61..bc87b5e 100644
--- a/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
@@ -14,12 +14,12 @@
package com.google.gerrit.plugins.checks.api;
-import static com.google.common.base.Preconditions.checkState;
-
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -35,9 +35,11 @@
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.Checks;
import com.google.gerrit.plugins.checks.Checks.GetCheckOptions;
+import com.google.gerrit.plugins.checks.index.CheckPredicate;
import com.google.gerrit.plugins.checks.index.CheckQueryBuilder;
import com.google.gerrit.plugins.checks.index.CheckStatePredicate;
import com.google.gerrit.plugins.checks.index.CheckerPredicate;
+import com.google.gerrit.plugins.checks.index.CheckerSchemePredicate;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.query.change.ChangeData;
@@ -55,7 +57,7 @@
private final Checkers checkers;
private final Checks checks;
private final Provider<CheckerQuery> checkerQueryProvider;
-
+ @VisibleForTesting public static final int MAX_ALLOWED_QUERIES = 10;
private String queryString;
@Option(
@@ -92,21 +94,59 @@
throw new BadRequestException("query is required");
}
- Predicate<Check> query = validateQuery(parseQuery(queryString));
- if (!hasStatePredicate(query)) {
- query = Predicate.and(new CheckStatePredicate(CheckState.NOT_STARTED), query);
+ Predicate<Check> predicate = validateQuery(parseQuery(queryString));
+ if (!hasStatePredicate(predicate)) {
+ predicate = Predicate.and(new CheckStatePredicate(CheckState.NOT_STARTED), predicate);
}
+ // this variable is for the lambda expressions when using orElseThrow
+ final Predicate<Check> finalPredicate = predicate;
- Optional<Checker> checker = checkers.getChecker(getCheckerUuidFromQuery(query));
- if (!checker.isPresent() || checker.get().isDisabled()) {
- return Response.ok(ImmutableList.of());
+ if (countPredicates(predicate, CheckerPredicate.class) == 1) {
+ // Checker query
+ Optional<Checker> checker =
+ checkers.getChecker(
+ getCheckerUuidFromQuery(predicate)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format("no checker predicate found: %s", finalPredicate))));
+ if (!checker.isPresent() || checker.get().isDisabled()) {
+ return Response.ok(ImmutableList.of());
+ }
+ List<ChangeData> changes = checkerQueryProvider.get().queryMatchingChanges(checker.get());
+ return Response.ok(getPendingChecksOfChecker(checker.get(), predicate, changes));
}
+ // Scheme query
+ String scheme =
+ getSchemeFromQuery(predicate)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format("no checker scheme predicate found: %s", finalPredicate)));
+ ImmutableList<Checker> checkersOfScheme = checkers.listCheckers(scheme);
+ if (checkersOfScheme.size() > MAX_ALLOWED_QUERIES) {
+ throw new ResourceConflictException(
+ String.format(
+ "Too many checkers exist with that scheme, allowed maximum is %s. Found %s checkers",
+ MAX_ALLOWED_QUERIES, checkersOfScheme.size()));
+ }
+ List<List<ChangeData>> changes =
+ checkerQueryProvider.get().queryMatchingChanges(checkersOfScheme);
+ List<PendingChecksInfo> pendingChecks = new ArrayList<>();
+ for (int i = 0; i < changes.size(); i++) {
+ pendingChecks.addAll(
+ getPendingChecksOfChecker(checkersOfScheme.get(i), predicate, changes.get(i)));
+ }
+ return Response.ok(pendingChecks);
+ }
+
+ private List<PendingChecksInfo> getPendingChecksOfChecker(
+ Checker checker, Predicate<Check> query, List<ChangeData> changes) throws IOException {
// The query system can only match against the current patch set; ignore non-current patch sets
// for now.
- List<ChangeData> changes = checkerQueryProvider.get().queryMatchingChanges(checker.get());
- CheckerUuid checkerUuid = checker.get().getUuid();
- List<PendingChecksInfo> pendingChecks = new ArrayList<>(changes.size());
+ List<PendingChecksInfo> pendingChecks = new ArrayList<>();
+ CheckerUuid checkerUuid = checker.getUuid();
for (ChangeData cd : changes) {
PatchSet patchSet = cd.currentPatchSet();
CheckKey checkKey = CheckKey.create(cd.project(), patchSet.id(), checkerUuid);
@@ -118,13 +158,13 @@
Check check =
checks
.getCheck(checkKey, GetCheckOptions.defaults())
- .orElseGet(() -> Check.newBackfilledCheck(cd.project(), patchSet, checker.get()));
+ .orElseGet(() -> Check.newBackfilledCheck(cd.project(), patchSet, checker));
if (query.asMatchable().match(check)) {
pendingChecks.add(createPendingChecksInfo(cd.project(), patchSet, checkerUuid, check));
}
}
- return Response.ok(pendingChecks);
+ return pendingChecks;
}
private Predicate<Check> parseQuery(String query) throws BadRequestException {
@@ -137,28 +177,36 @@
private static Predicate<Check> validateQuery(Predicate<Check> predicate)
throws BadRequestException {
- if (countCheckerPredicates(predicate) != 1)
+ int numCheckPredicates = countPredicates(predicate, CheckerPredicate.class);
+ int numSchemePredicates = countPredicates(predicate, CheckerSchemePredicate.class);
+ String exceptionMessage =
+ String.format(
+ "query must be '%s:<checker-uuid>' or '%s:<checker-uuid> AND <other-operators>' or '%s:<checker-scheme>' or '%s:<checker-scheme> AND <other-operators>'",
+ CheckQueryBuilder.FIELD_CHECKER,
+ CheckQueryBuilder.FIELD_CHECKER,
+ CheckQueryBuilder.FIELD_SCHEME,
+ CheckQueryBuilder.FIELD_SCHEME);
+ if (numCheckPredicates + numSchemePredicates != 1) {
throw new BadRequestException(
String.format(
- "query must contain exactly 1 '%s' operator", CheckQueryBuilder.FIELD_CHECKER));
+ "query must contain exactly 1 '%s' operator or '%s' operator",
+ CheckQueryBuilder.FIELD_CHECKER, CheckQueryBuilder.FIELD_SCHEME));
+ }
// the root predicate must either be an AndPredicate ....
if (predicate instanceof AndPredicate) {
// if the root predicate is an AndPredicate, any of its direct children must be a
- // CheckerPredicate, the other child predicates can be anything (including any combination of
+ // CheckerPredicate or CheckerSchemePredicate, the other child predicates can be anything
+ // (including any combination of
// AndPredicate, OrPredicate and NotPredicate).
- if (predicate.getChildren().stream().noneMatch(CheckerPredicate.class::isInstance)) {
- throw new BadRequestException(
- String.format(
- "query must be '%s:<checker-uuid>' or '%s:<checker-uuid> AND <other-operators>'",
- CheckQueryBuilder.FIELD_CHECKER, CheckQueryBuilder.FIELD_CHECKER));
+ if (predicate.getChildren().stream().noneMatch(CheckerPredicate.class::isInstance)
+ && predicate.getChildren().stream().noneMatch(CheckerSchemePredicate.class::isInstance)) {
+ throw new BadRequestException(exceptionMessage);
}
- // ... or a CheckerPredicate
- } else if (!(predicate instanceof CheckerPredicate)) {
- throw new BadRequestException(
- String.format(
- "query must be '%s:<checker-uuid>' or '%s:<checker-uuid> AND <other-operators>'",
- CheckQueryBuilder.FIELD_CHECKER, CheckQueryBuilder.FIELD_CHECKER));
+ // ... or a CheckerPredicate / CheckerSchemePredicate
+ } else if (!(predicate instanceof CheckerPredicate
+ || predicate instanceof CheckerSchemePredicate)) {
+ throw new BadRequestException(exceptionMessage);
}
return predicate;
}
@@ -179,40 +227,54 @@
* <p>This method doesn't validate that the checker predicates appear in any particular location.
*
* @param predicate the predicate in which the checker predicates should be counted
+ * @param predicateClass the class of the predicate (schema or checker)
* @return the number of checker predicates in the given predicate
*/
- private static int countCheckerPredicates(Predicate<Check> predicate) {
- if (predicate instanceof CheckerPredicate) {
+ private static int countPredicates(
+ Predicate<Check> predicate, Class<? extends CheckPredicate> predicateClass) {
+ if (predicateClass.isInstance(predicate)) {
return 1;
}
if (predicate.getChildCount() == 0) {
return 0;
}
return predicate.getChildren().stream()
- .mapToInt(QueryPendingChecks::countCheckerPredicates)
+ .mapToInt(p -> QueryPendingChecks.countPredicates(p, predicateClass))
.sum();
}
- private static CheckerUuid getCheckerUuidFromQuery(Predicate<Check> predicate) {
+ private static Optional<CheckerUuid> getCheckerUuidFromQuery(Predicate<Check> predicate) {
// the query validation (see #validateQuery(Predicate<Check>)) ensures that there is exactly 1
- // CheckerPredicate and that it is on the first or second level of the predicate tree.
+ // CheckerPredicate or CheckerSchemePredicate and that it is on the first or second level of the
+ // predicate tree.
if (predicate instanceof CheckerPredicate) {
- return ((CheckerPredicate) predicate).getCheckerUuid();
+ return Optional.of(((CheckerPredicate) predicate).getCheckerUuid());
}
- checkState(predicate.getChildCount() > 0, "no checker predicate found: %s", predicate);
Optional<CheckerPredicate> checkerPredicate =
predicate.getChildren().stream()
.filter(CheckerPredicate.class::isInstance)
.map(p -> (CheckerPredicate) p)
.findAny();
- return checkerPredicate
- .map(CheckerPredicate::getCheckerUuid)
- .orElseThrow(
- () ->
- new IllegalStateException(
- String.format("no checker predicate found: %s", predicate)));
+ return checkerPredicate.map(CheckerPredicate::getCheckerUuid);
+ }
+
+ private static Optional<String> getSchemeFromQuery(Predicate<Check> predicate) {
+ // the query validation (see #validateQuery(Predicate<Check>)) ensures that there is exactly 1
+ // CheckerPredicate or CheckerSchemePredicate and that it is on the first or second level of the
+ // predicate tree.
+
+ if (predicate instanceof CheckerSchemePredicate) {
+ return Optional.of(((CheckerSchemePredicate) predicate).getCheckerScheme());
+ }
+
+ Optional<CheckerSchemePredicate> checkerSchemePredicate =
+ predicate.getChildren().stream()
+ .filter(CheckerSchemePredicate.class::isInstance)
+ .map(p -> (CheckerSchemePredicate) p)
+ .findAny();
+ return checkerSchemePredicate.map(CheckerSchemePredicate::getCheckerScheme);
}
private static PendingChecksInfo createPendingChecksInfo(
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckers.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckers.java
index be8bd8e..717a793 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckers.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckers.java
@@ -62,8 +62,17 @@
@Override
public ImmutableList<Checker> listCheckers() throws IOException {
+ return listCheckers("");
+ }
+
+ @Override
+ public ImmutableList<Checker> listCheckers(String scheme) throws IOException {
+ if (scheme != null && !scheme.isEmpty() && !scheme.endsWith("/")) {
+ scheme = scheme + "/";
+ }
try (Repository allProjectsRepo = repoManager.openRepository(allProjectsName)) {
- return allProjectsRepo.getRefDatabase().getRefsByPrefix(CheckerRef.REFS_CHECKERS).stream()
+ return allProjectsRepo.getRefDatabase().getRefsByPrefix(CheckerRef.REFS_CHECKERS + scheme)
+ .stream()
.flatMap(ref -> Streams.stream(tryLoadChecker(allProjectsRepo, ref)))
.sorted(comparing(Checker::getUuid))
.collect(toImmutableList());
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java b/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
index ff6d1f5..e970e86 100644
--- a/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
+++ b/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
@@ -27,6 +27,7 @@
public class CheckQueryBuilder extends QueryBuilder<Check, CheckQueryBuilder> {
public static final String FIELD_CHECKER = "checker";
public static final String FIELD_STATE = "state";
+ public static final String FIELD_SCHEME = "scheme";
private static final QueryBuilder.Definition<Check, CheckQueryBuilder> mydef =
new QueryBuilder.Definition<>(CheckQueryBuilder.class);
@@ -60,4 +61,9 @@
public Predicate<Check> state(String state) throws QueryParseException {
return CheckStatePredicate.parse(state);
}
+
+ @Operator
+ public Predicate<Check> scheme(String scheme) {
+ return new CheckerSchemePredicate(scheme);
+ }
}
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckerSchemePredicate.java b/java/com/google/gerrit/plugins/checks/index/CheckerSchemePredicate.java
new file mode 100644
index 0000000..d2eea3a
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/index/CheckerSchemePredicate.java
@@ -0,0 +1,38 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.checks.index;
+
+import static java.util.Objects.requireNonNull;
+
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.plugins.checks.Check;
+
+public class CheckerSchemePredicate extends CheckPredicate {
+ private final String checkerScheme;
+
+ public CheckerSchemePredicate(String checkerScheme) {
+ super(CheckQueryBuilder.FIELD_SCHEME, checkerScheme);
+ this.checkerScheme = requireNonNull(checkerScheme, "checkerScheme");
+ }
+
+ @Override
+ public boolean match(Check check) throws StorageException {
+ return checkerScheme.equals(check.key().checkerUuid().scheme());
+ }
+
+ public String getCheckerScheme() {
+ return checkerScheme;
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
index 2727203..d284bf1 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
@@ -35,6 +36,7 @@
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.plugins.checks.api.PendingCheckInfo;
import com.google.gerrit.plugins.checks.api.PendingChecksInfo;
+import com.google.gerrit.plugins.checks.api.QueryPendingChecks;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.gson.reflect.TypeToken;
@@ -85,8 +87,10 @@
}
@Test
- public void specifyingCheckerIsRequired() throws Exception {
- assertInvalidQuery("state:NOT_STARTED", "query must contain exactly 1 'checker' operator");
+ public void specifyingCheckerOrSchemeIsRequired() throws Exception {
+ assertInvalidQuery(
+ "state:NOT_STARTED",
+ "query must contain exactly 1 'checker' operator or 'scheme' operator");
}
@Test
@@ -97,11 +101,11 @@
}
@Test
- public void cannotSpecifyingMultipleCheckers() throws Exception {
+ public void cannotSpecifyMultipleCheckers() throws Exception {
CheckerUuid checkerUuid1 = checkerOperations.newChecker().repository(project).create();
CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
- String expectedMessage = "query must contain exactly 1 'checker' operator";
+ String expectedMessage = "query must contain exactly 1 'checker' operator or 'scheme' operator";
assertInvalidQuery(
String.format("checker:\"%s\" checker:\"%s\"", checkerUuid1, checkerUuid2),
expectedMessage);
@@ -115,6 +119,26 @@
}
@Test
+ public void cannotSpecifyMultipleSchemes() throws Exception {
+ CheckerUuid checkerUuid1 = checkerOperations.newChecker().repository(project).create();
+ CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+
+ String expectedMessage = "query must contain exactly 1 'checker' operator or 'scheme' operator";
+ assertInvalidQuery(
+ String.format("scheme:\"%s\" scheme:\"%s\"", checkerUuid1.scheme(), checkerUuid2.scheme()),
+ expectedMessage);
+ assertInvalidQuery(
+ String.format(
+ "scheme:\"%s\" OR scheme:\"%s\"", checkerUuid1.scheme(), checkerUuid2.scheme()),
+ expectedMessage);
+ assertInvalidQuery(
+ String.format(
+ "scheme:\"%s\" (state:NOT_STARTED scheme:\"%s\")",
+ checkerUuid1.scheme(), checkerUuid2.scheme()),
+ expectedMessage);
+ }
+
+ @Test
public void canSpecifyCheckerAsRootPredicate() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
assertThat(queryPendingChecks(String.format("checker:\"%s\"", checkerUuid))).hasSize(1);
@@ -133,7 +157,7 @@
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
String expectedMessage =
- "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>'";
+ "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>' or 'scheme:<checker-scheme>' or 'scheme:<checker-scheme> AND <other-operators>'";
assertInvalidQuery(
String.format("state:NOT_STARTED AND (checker:\"%s\" OR state:NOT_STARTED)", checkerUuid),
expectedMessage);
@@ -142,6 +166,21 @@
}
@Test
+ public void cannotSpecifySchemeInAndConditionIfNotImmediateChild() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ String expectedMessage =
+ "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>' or 'scheme:<checker-scheme>' or 'scheme:<checker-scheme> AND <other-operators>'";
+ assertInvalidQuery(
+ String.format(
+ "state:NOT_STARTED AND (scheme:\"%s\" OR state:NOT_STARTED)", checkerUuid.scheme()),
+ expectedMessage);
+ assertInvalidQuery(
+ String.format("state:NOT_STARTED AND NOT scheme:\"%s\"", checkerUuid.scheme()),
+ expectedMessage);
+ }
+
+ @Test
public void andConditionAtRootCanContainAnyCombinationOfOtherPredicates() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
@@ -168,7 +207,7 @@
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
String expectedMessage =
- "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>'";
+ "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>' or 'scheme:<checker-scheme>' or 'scheme:<checker-scheme> AND <other-operators>'";
assertInvalidQuery(
String.format("checker:\"%s\" OR state:NOT_STARTED", checkerUuid), expectedMessage);
assertInvalidQuery(
@@ -176,11 +215,31 @@
}
@Test
+ public void cannotSpecifySchemeInOrCondition() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ String expectedMessage =
+ "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>' or 'scheme:<checker-scheme>' or 'scheme:<checker-scheme> AND <other-operators>'";
+ assertInvalidQuery(
+ String.format("scheme:\"%s\" OR state:NOT_STARTED", checkerUuid.scheme()), expectedMessage);
+ assertInvalidQuery(
+ String.format("state:NOT_STARTED OR scheme:\"%s\"", checkerUuid.scheme()), expectedMessage);
+ }
+
+ @Test
public void cannotSpecifyCheckerInNotCondition() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
assertInvalidQuery(
String.format("NOT checker:\"%s\"", checkerUuid),
- "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>'");
+ "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>' or 'scheme:<checker-scheme>' or 'scheme:<checker-scheme> AND <other-operators>'");
+ }
+
+ @Test
+ public void cannotSpecifySchemeInNotCondition() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ assertInvalidQuery(
+ String.format("NOT scheme:\"%s\"", checkerUuid.scheme()),
+ "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>' or 'scheme:<checker-scheme>' or 'scheme:<checker-scheme> AND <other-operators>'");
}
@Test
@@ -646,6 +705,128 @@
assertThat(pendingChecksList).isNotEmpty();
}
+ @Test
+ public void queryPendingChecksWithScheme() throws Exception {
+ // create a check with a scheme that we expect to never be returned.
+ CheckerUuid checkerUuidOtherScheme =
+ checkerOperations
+ .newChecker()
+ .uuid(CheckerUuid.parse("otherscheme:checker-1"))
+ .repository(project)
+ .create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuidOtherScheme))
+ .state(CheckState.NOT_STARTED)
+ .upsert();
+
+ CheckerUuid checkerUuid =
+ checkerOperations
+ .newChecker()
+ .uuid(CheckerUuid.parse("test:checker-1"))
+ .repository(project)
+ .create();
+ // Create a check with state "NOT_STARTED" that we expect to be returned.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .state(CheckState.NOT_STARTED)
+ .upsert();
+
+ // Create a check with state "SCHEDULED" that we expect to be returned.
+ PatchSet.Id patchSetId2 = createChange().getPatchSetId();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
+ .state(CheckState.SCHEDULED)
+ .upsert();
+
+ // Create a check with state "SUCCESSFUL" that we expect to be ignored.
+ PatchSet.Id patchSetId3 = createChange().getPatchSetId();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId3, checkerUuid))
+ .state(CheckState.SUCCESSFUL)
+ .upsert();
+
+ // Create a check with state "SUCCESSFUL" that we expect to be ignored, for patchsetId.
+ // by default, patchSetId2 and patchSetId3 should be NOT_STARTED, and should be returned.
+ CheckerUuid checkerUuid2 =
+ checkerOperations
+ .newChecker()
+ .uuid(CheckerUuid.parse("test:checker-2"))
+ .repository(project)
+ .create();
+ assertThat(checkerUuid2.scheme()).isEqualTo("test");
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid2))
+ .state(CheckState.FAILED)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks("test", CheckState.NOT_STARTED, CheckState.SCHEDULED);
+ assertThat(pendingChecksList).hasSize(4);
+
+ // The sorting of the pendingChecksList matches the sorting in which the matching changes are
+ // returned from the change index, which is by last updated timestamp. Use this knowledge here
+ // to do the assertions although the REST endpoint doesn't document a guaranteed sort order.
+ PendingChecksInfo pendingChecksChange = pendingChecksList.get(0);
+ assertThat(pendingChecksChange).hasRepository(project);
+ assertThat(pendingChecksChange).hasPatchSet(patchSetId2);
+ assertThat(pendingChecksChange)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.SCHEDULED));
+
+ pendingChecksChange = pendingChecksList.get(1);
+ assertThat(pendingChecksChange).hasRepository(project);
+ assertThat(pendingChecksChange).hasPatchSet(patchSetId);
+ assertThat(pendingChecksChange)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
+
+ pendingChecksChange = pendingChecksList.get(2);
+ assertThat(pendingChecksChange).hasRepository(project);
+ assertThat(pendingChecksChange).hasPatchSet(patchSetId3);
+ assertThat(pendingChecksChange)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid2.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
+
+ pendingChecksChange = pendingChecksList.get(3);
+ assertThat(pendingChecksChange).hasRepository(project);
+ assertThat(pendingChecksChange).hasPatchSet(patchSetId2);
+ assertThat(pendingChecksChange)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid2.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
+ }
+
+ @Test
+ public void queryPendingChecksWithSchemeTooManyChecksThrowsError() throws Exception {
+ for (int i = 0; i < QueryPendingChecks.MAX_ALLOWED_QUERIES + 1; i++) {
+ checkerOperations
+ .newChecker()
+ .repository(project)
+ .uuid(CheckerUuid.parse(String.format("test:checker-%d", i)))
+ .create();
+ }
+ assertThrows(
+ ResourceConflictException.class,
+ () -> queryPendingChecks("test", CheckState.NOT_STARTED, CheckState.SCHEDULED));
+ }
+
+ @Test
+ public void queryOnlyExactSchemas() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations
+ .newChecker()
+ .uuid(CheckerUuid.parse("foobar:checker-1"))
+ .repository(project)
+ .create();
+ // Create a check with state "NOT_STARTED" that we expect to be returned.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .state(CheckState.NOT_STARTED)
+ .upsert();
+ assertThat(queryPendingChecks("foo", CheckState.NOT_STARTED, CheckState.SCHEDULED)).isEmpty();
+ assertThat(queryPendingChecks("bar", CheckState.NOT_STARTED, CheckState.RUNNING)).isEmpty();
+ assertThat(queryPendingChecks("foobar", CheckState.NOT_STARTED)).hasSize(1);
+ }
+
private void assertInvalidQuery(String query, String expectedMessage) {
BadRequestException thrown =
assertThrows(BadRequestException.class, () -> pendingChecksApi.query(query).get());
@@ -661,6 +842,11 @@
return pendingChecksApi.query(buildQueryString(checkerUuid, checkStates)).get();
}
+ private List<PendingChecksInfo> queryPendingChecks(String scheme, CheckState... checkStates)
+ throws RestApiException {
+ return pendingChecksApi.query(buildQueryString(scheme, checkStates)).get();
+ }
+
private String buildQueryString(CheckerUuid checkerUuid, CheckState... checkStates) {
StringBuilder queryString = new StringBuilder();
queryString.append(String.format("checker:%s", checkerUuid));
@@ -671,4 +857,15 @@
return queryString.toString();
}
+
+ private String buildQueryString(String scheme, CheckState... checkStates) {
+ StringBuilder queryString = new StringBuilder();
+ queryString.append(String.format("scheme:%s", scheme));
+
+ StringJoiner stateJoiner = new StringJoiner(" OR state:", " (state:", ")");
+ Stream.of(checkStates).map(CheckState::name).forEach(stateJoiner::add);
+ queryString.append(stateJoiner.toString());
+
+ return queryString.toString();
+ }
}
diff --git a/resources/Documentation/rest-api-pending-checks.md b/resources/Documentation/rest-api-pending-checks.md
index 91771e3..2974410 100644
--- a/resources/Documentation/rest-api-pending-checks.md
+++ b/resources/Documentation/rest-api-pending-checks.md
@@ -128,3 +128,5 @@
`RUNNING`).
* <a id="state-operator"></a> `state:'STATE'`:
Matches checks with the state 'STATE'.
+* <a id="scheme-operator"></a> `scheme:'SCHEME'`:
+ Matches checks with the scheme 'SCHEME'.
