Merge "Adapt to change in Gerrit core"
diff --git a/gr-checks/gr-checkers-list.html b/gr-checks/gr-checkers-list.html
index 36e8f85..6c62e4d 100644
--- a/gr-checks/gr-checkers-list.html
+++ b/gr-checks/gr-checkers-list.html
@@ -121,7 +121,7 @@
</template>
</nav>
- <gr-overlay id="createOverlay" with-backdrop>
+ <gr-overlay id="createOverlay">
<gr-dialog
id="createDialog"
confirm-label="Create"
@@ -138,7 +138,7 @@
</div>
</gr-dialog>
</gr-overlay>
- <gr-overlay id="editOverlay" with-backdrop>
+ <gr-overlay id="editOverlay">
<gr-dialog
id="editDialog"
confirm-label="Save"
diff --git a/gr-checks/gr-checkers-list.js b/gr-checks/gr-checkers-list.js
index 2542832..4914274 100644
--- a/gr-checks/gr-checkers-list.js
+++ b/gr-checks/gr-checkers-list.js
@@ -9,7 +9,15 @@
Polymer({
is: 'gr-checkers-list',
properties: {
- pluginRestApi: Object,
+ /**
+ * Add observer on pluginRestApi to call getCheckers when it's defined
+ * as initially getCheckers was being called before pluginRestApi was
+ * initialised by gr-checks-view
+ */
+ pluginRestApi: {
+ type: Object,
+ observer: '_getCheckers'
+ },
// Checker that will be passed to the editOverlay modal
checker: Object,
_checkers: Array,
@@ -51,10 +59,6 @@
'_showCheckers(_checkers, _filter)',
],
- attached() {
- this._getCheckers();
- },
-
_contains(target, keyword) {
return target.toLowerCase().includes(keyword.toLowerCase().trim());
},
@@ -117,11 +121,9 @@
}
},
- _getCheckers() {
- this.pluginRestApi.fetchJSON({
- method: 'GET',
- url: GET_CHECKERS_URL,
- }).then(checkers => {
+ _getCheckers(pluginRestApi) {
+ if (!pluginRestApi) return;
+ pluginRestApi.get(GET_CHECKERS_URL).then(checkers => {
if (!checkers) { return; }
this._checkers = checkers;
this._startingIndex = 0;
diff --git a/gr-checks/gr-checkers-list_test.html b/gr-checks/gr-checkers-list_test.html
new file mode 100644
index 0000000..6b5e0ad
--- /dev/null
+++ b/gr-checks/gr-checkers-list_test.html
@@ -0,0 +1,244 @@
+<!DOCTYPE html>
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<script src="/bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="/bower_components/web-component-tester/browser.js"></script>
+
+<link rel="import" href="/bower_components/polymer/polymer.html">
+
+<title>gr-checkers-list-view</title>
+<link rel="import" href="gr-checkers-list.html">
+
+<test-fixture id="basic">
+ <template is="dom-template">
+ <gr-checkers-list
+ plugin-rest-api="[[pluginRestApi]]"
+ >
+ </gr-checkers-list>
+</template>
+</test-fixture>
+
+<script>
+ const CHECKERS = [
+ {
+ "uuid":"C:D",
+ "name":"A",
+ "description":"B",
+ "repository":"Backend",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-25 13:08:43.000000000",
+ "updated":"2019-07-25 13:08:43.000000000"
+ },
+ {
+ "uuid":"aa:bb",
+ "name":"n1",
+ "description":"d1",
+ "repository":"All-Users",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-29 13:07:17.000000000",
+ "updated":"2019-07-29 13:07:17.000000000"
+ },
+ {
+ "uuid":"adsf:asdasdas",
+ "name":"ds",
+ "description":"s",
+ "repository":"Scripts",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-29 13:28:09.000000000",
+ "updated":"2019-07-29 13:28:09.000000000"
+ },
+ {
+ "uuid":"ijkl:mnop",
+ "name":"abcd",
+ "description":"efgh",
+ "repository":"All-Projects",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-29 09:33:25.000000000",
+ "updated":"2019-07-29 09:33:25.000000000"
+ },
+ {
+ "uuid":"ngfnf:mhghgnhghn",
+ "name":"nbvfg",
+ "description":"fjhgj",
+ "repository":"All-Users",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-08-06 14:21:34.000000000",
+ "updated":"2019-08-06 14:21:34.000000000"
+ },
+ {
+ "uuid":"sdfsdf--:sdfsdf333",
+ "name":"sdfsdf",
+ "description":"sdfsdfsd",
+ "repository":"Scripts",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-30 13:00:19.000000000",
+ "updated":"2019-07-30 13:00:19.000000000"
+ },
+ {
+ "uuid":"test:checker1",
+ "name":"Unit Tests",
+ "description":"Random description that should be improved at some point",
+ "repository":"Backend",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-22 13:16:52.000000000",
+ "updated":"2019-07-22 14:21:14.000000000"
+ },
+ {
+ "uuid":"test:checker2",
+ "name":"Code Style",
+ "repository":"Backend",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-22 13:26:56.000000000",
+ "updated":"2019-07-22 13:26:56.000000000"
+ },
+ {
+ "uuid":"xddf:sdfsdfsdf",
+ "name":"sdfsdf",
+ "description":"sdfsdf",
+ "repository":"Scripts",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-29 14:11:59.000000000",
+ "updated":"2019-07-29 14:11:59.000000000"
+ },
+ {
+ "uuid":"zxczxc:bnvnbvnbvn",
+ "name":"zxc",
+ "description":"zxc",
+ "repository":"Scripts",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-29 14:00:24.000000000",
+ "updated":"2019-07-29 14:00:24.000000000"
+ },
+ {
+ "uuid":"zxczxc:sdfsdf",
+ "name":"zxc",
+ "description":"zxc",
+ "repository":"Scripts",
+ "status":"ENABLED",
+ "blocking":[
+
+ ],
+ "query":"status:open",
+ "created":"2019-07-29 13:30:47.000000000",
+ "updated":"2019-07-29 13:30:47.000000000"
+ }
+ ];
+</script>
+
+<script>
+
+ suite('gr-checkers-list tests', () => {
+ let element;
+ let sandbox;
+ let fetchJSONSpy, fetchJSONPromise, fetchJSONResolve;
+
+ setup((done) => {
+ sandbox = sinon.sandbox.create();
+
+ fetchJSONSpy = sinon.stub();
+ const fetchJSONPromise = new Promise((resolve, reject) => {
+ fetchJSONResolve = resolve;
+ })
+ fetchJSONSpy.returns(fetchJSONPromise)
+
+ const pluginRestApi = {
+ fetchJSON: fetchJSONSpy,
+ };
+
+ element = fixture('basic', {
+ pluginRestApi
+ });
+ flush(done);
+ });
+
+ teardown(() => { sandbox.restore(); });
+
+ test('renders checker list table headings', () => {
+ const checkersList = element.$$('table')
+ const headings = checkersList.firstElementChild.firstElementChild.
+ children;
+ const expectedHeadings = ["Checker Name", "Repository", "Status",
+ "Required", "Checker Description", "Edit"];
+ for (let i = 0;i < headings.length;i++) {
+ assert(headings[i].innerText === expectedHeadings[i]);
+ }
+ });
+
+ test('create checker button renders', () => {
+ assert(element.querySelector('#createNewContainer'));
+ const button = element.querySelector('#createNewContainer')
+ .querySelector('gr-button');
+ assert(button);
+ assert(button.innerText === 'Create New');
+ });
+
+ suite('with checkers', () => {
+ setup((done) => {
+ fetchJSONResolve(CHECKERS);
+ flush(done);
+ })
+ test('renders correct number of checkers', () => {
+ const checkers = element.$$('table > tbody:nth-child(2)')
+ .querySelectorAll('tr');
+ assert(checkers.length === CHECKERS.length);
+ })
+ test('renders correct checkers', () => {
+ const checkers = element.$$('table > tbody:nth-child(2)')
+ .querySelectorAll('tr');
+ for (let i = 0;i < checkers.length;i++) {
+ const checkerDetails = checkers[i].querySelectorAll('td');
+ assert(CHECKERS[i].name === checkerDetails[0].innerText);
+ assert(CHECKERS[i].repository === checkerDetails[1].innerText);
+ const status = CHECKERS[i].status || "NO";
+ assert(status === checkerDetails[2].innerText);
+ const checkerRequired = (CHECKERS[i].blocking &&
+ CHECKERS[i].blocking.length > 0)
+ ? "YES": "NO";
+ assert(checkerRequired === checkerDetails[3].innerText);
+ const description = CHECKERS[i].description || '';
+ assert(description === checkerDetails[4].innerText);
+ }
+ })
+ });
+
+ });
+</script>
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 7c0647e..f78d143 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,6 @@
private final Checkers checkers;
private final Checks checks;
private final Provider<CheckerQuery> checkerQueryProvider;
-
private String queryString;
@Option(
@@ -91,21 +92,53 @@
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);
+ 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);
@@ -117,13 +150,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 {
@@ -136,28 +169,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;
}
@@ -178,40 +219,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..d0656ac 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,114 @@
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 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 +828,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 +843,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-checks.md b/resources/Documentation/rest-api-checks.md
index 4395644..1c64598 100644
--- a/resources/Documentation/rest-api-checks.md
+++ b/resources/Documentation/rest-api-checks.md
@@ -9,7 +9,7 @@
## <a id="check-endpoints"> Check Endpoints
### <a id="list-checks"> List Checks
-_'GET /changes/[\{change-id\}](../../../Documentation/rest-api-changes.html#change-id)/revisions/[\{revision-id\}](../../../Documentation/rest-api-changes.html#revision-id)/revisions/checks'_
+_'GET /changes/[\{change-id\}](../../../Documentation/rest-api-changes.html#change-id)/revisions/[\{revision-id\}](../../../Documentation/rest-api-changes.html#revision-id)/checks'_
Retrieves all checks for a given revision and change.
@@ -56,7 +56,7 @@
```
### <a id="get-check"> Get Check
-_'GET /changes/[\{change-id\}](../../../Documentation/rest-api-changes.html#change-id)/revisions/[\{revision-id\}](../../../Documentation/rest-api-changes.html#revision-id)/revisions/checks/[\{checker-id\}](./rest-api-checkers.md#checker-id)'_
+_'GET /changes/[\{change-id\}](../../../Documentation/rest-api-changes.html#change-id)/revisions/[\{revision-id\}](../../../Documentation/rest-api-changes.html#revision-id)/checks/[\{checker-id\}](./rest-api-checkers.md#checker-id)'_
Retrieves a check.
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'.
