Merge changes I4f278748,I0f26b6df,If6b772c5,Ia8658605,Id3acdad1, ...
* changes:
NoSuchCheckerException: Remove unused constructors
Checker: Remove unused builder(CheckerUuid) method
Test that checker with invalid query is treated as not required for combined check state
PostCheck: Move into api package
CheckerRef: Add private constructor
Test that checker refs are only protected in the All-Projects project
Support 'is:inprogress' operator in pending checks query
Support 'is:<state>' operator in pending checks query
Rename ListPendingChecks to QueryPendingChecks
Change List Pending Checks REST endpoint to accept a query as input
diff --git a/java/com/google/gerrit/plugins/checks/Check.java b/java/com/google/gerrit/plugins/checks/Check.java
index b7af0f8..bd46191 100644
--- a/java/com/google/gerrit/plugins/checks/Check.java
+++ b/java/com/google/gerrit/plugins/checks/Check.java
@@ -16,11 +16,21 @@
import com.google.auto.value.AutoValue;
import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
import java.sql.Timestamp;
import java.util.Optional;
@AutoValue
public abstract class Check {
+ public static Check newBackfilledCheck(Project.NameKey project, PatchSet ps, Checker checker) {
+ return Check.builder(CheckKey.create(project, ps.getId(), checker.getUuid()))
+ .setState(CheckState.NOT_STARTED)
+ .setCreated(ps.getCreatedOn())
+ .setUpdated(ps.getCreatedOn())
+ .build();
+ }
+
/** The key of the Check. */
public abstract CheckKey key();
diff --git a/java/com/google/gerrit/plugins/checks/Checker.java b/java/com/google/gerrit/plugins/checks/Checker.java
index aa43340..56fa64b 100644
--- a/java/com/google/gerrit/plugins/checks/Checker.java
+++ b/java/com/google/gerrit/plugins/checks/Checker.java
@@ -141,8 +141,8 @@
return new AutoValue_Checker.Builder();
}
- public static Builder builder(CheckerUuid uuid) {
- return builder().setUuid(uuid);
+ public boolean isDisabled() {
+ return CheckerStatus.DISABLED == getStatus();
}
public boolean isCheckerRelevant(ChangeData cd, ChangeQueryBuilder changeQueryBuilder)
diff --git a/java/com/google/gerrit/plugins/checks/CheckerRef.java b/java/com/google/gerrit/plugins/checks/CheckerRef.java
index aa0e2bf..6313ec9 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerRef.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerRef.java
@@ -40,4 +40,6 @@
public static boolean isRefsCheckers(String ref) {
return ref.startsWith(REFS_CHECKERS);
}
+
+ private CheckerRef() {}
}
diff --git a/java/com/google/gerrit/plugins/checks/NoSuchCheckerException.java b/java/com/google/gerrit/plugins/checks/NoSuchCheckerException.java
index dcdadcc..df5d8cd 100644
--- a/java/com/google/gerrit/plugins/checks/NoSuchCheckerException.java
+++ b/java/com/google/gerrit/plugins/checks/NoSuchCheckerException.java
@@ -21,14 +21,6 @@
public static final String MESSAGE = "Checker Not Found: ";
public NoSuchCheckerException(CheckerUuid uuid) {
- this(uuid.get());
- }
-
- public NoSuchCheckerException(String uuid) {
- this(uuid, null);
- }
-
- public NoSuchCheckerException(String uuid, Throwable why) {
- super(MESSAGE + uuid, why);
+ super(MESSAGE + uuid.get());
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/ApiModule.java b/java/com/google/gerrit/plugins/checks/api/ApiModule.java
index be4519c..ad1e7e3 100644
--- a/java/com/google/gerrit/plugins/checks/api/ApiModule.java
+++ b/java/com/google/gerrit/plugins/checks/api/ApiModule.java
@@ -22,7 +22,6 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
-import com.google.gerrit.plugins.checks.PostCheck;
import com.google.inject.AbstractModule;
// TODO(gerrit-team): This should move into HttpModule, but a core bug prevents the bindings from
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckerUuidHandler.java b/java/com/google/gerrit/plugins/checks/api/CheckerUuidHandler.java
deleted file mode 100644
index 5e563ff..0000000
--- a/java/com/google/gerrit/plugins/checks/api/CheckerUuidHandler.java
+++ /dev/null
@@ -1,55 +0,0 @@
-// 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.api;
-
-import static com.google.gerrit.util.cli.Localizable.localizable;
-
-import com.google.gerrit.plugins.checks.CheckerUuid;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-import org.kohsuke.args4j.CmdLineException;
-import org.kohsuke.args4j.CmdLineParser;
-import org.kohsuke.args4j.OptionDef;
-import org.kohsuke.args4j.spi.OptionHandler;
-import org.kohsuke.args4j.spi.Parameters;
-import org.kohsuke.args4j.spi.Setter;
-
-public class CheckerUuidHandler extends OptionHandler<CheckerUuid> {
- @Inject
- public CheckerUuidHandler(
- @Assisted CmdLineParser parser,
- @Assisted OptionDef option,
- @Assisted Setter<CheckerUuid> setter) {
- super(parser, option, setter);
- }
-
- @Override
- public int parseArguments(Parameters params) throws CmdLineException {
- String token = params.getParameter(0);
-
- if (!CheckerUuid.isUuid(token)) {
- throw new CmdLineException(owner, localizable("Invalid checker UUID: %s"), token);
- }
-
- CheckerUuid checkerUuid = CheckerUuid.parse(token);
- setter.addValue(checkerUuid);
- return 1;
- }
-
- @Override
- public String getDefaultMetaVariable() {
- return "UUID";
- }
-}
diff --git a/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java b/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
index cbc454f..206e7b0 100644
--- a/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
@@ -21,7 +21,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.ListChecksOption;
-import com.google.gerrit.plugins.checks.PostCheck;
import com.google.gerrit.server.change.RevisionResource;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
diff --git a/java/com/google/gerrit/plugins/checks/api/ListPendingChecks.java b/java/com/google/gerrit/plugins/checks/api/ListPendingChecks.java
deleted file mode 100644
index 18e2f8c..0000000
--- a/java/com/google/gerrit/plugins/checks/api/ListPendingChecks.java
+++ /dev/null
@@ -1,162 +0,0 @@
-// 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.api;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.plugins.checks.Check;
-import com.google.gerrit.plugins.checks.CheckKey;
-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.Checks;
-import com.google.gerrit.plugins.checks.Checks.GetCheckOptions;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor;
-import com.google.gerrit.server.update.RetryHelper;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Optional;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.kohsuke.args4j.Option;
-
-public class ListPendingChecks implements RestReadView<TopLevelResource> {
- private final Checkers checkers;
- private final Checks checks;
- private final RetryHelper retryHelper;
- private final Provider<ChangeQueryBuilder> queryBuilderProvider;
- private final Provider<ChangeQueryProcessor> changeQueryProcessorProvider;
-
- private CheckerUuid checkerUuid;
- private List<CheckState> states = new ArrayList<>(CheckState.values().length);
-
- @Option(
- name = "--checker",
- metaVar = "UUID",
- usage = "checker UUID formatted as '<scheme>:<id>'",
- handler = CheckerUuidHandler.class)
- public void setChecker(CheckerUuid checkerUuid) {
- this.checkerUuid = checkerUuid;
- }
-
- @Option(name = "--state", metaVar = "STATE", usage = "check state")
- public void addState(CheckState state) {
- this.states.add(state);
- }
-
- @Inject
- public ListPendingChecks(
- Checkers checkers,
- Checks checks,
- RetryHelper retryHelper,
- Provider<ChangeQueryBuilder> queryBuilderProvider,
- Provider<ChangeQueryProcessor> changeQueryProcessorProvider) {
- this.checkers = checkers;
- this.checks = checks;
- this.retryHelper = retryHelper;
- this.queryBuilderProvider = queryBuilderProvider;
- this.changeQueryProcessorProvider = changeQueryProcessorProvider;
- }
-
- @Override
- public List<PendingChecksInfo> apply(TopLevelResource resource)
- throws RestApiException, IOException, ConfigInvalidException, OrmException {
- if (states.isEmpty()) {
- // If no state was specified, assume NOT_STARTED by default.
- states.add(CheckState.NOT_STARTED);
- }
-
- if (checkerUuid == null) {
- throw new BadRequestException("checker UUID is required");
- }
-
- Checker checker =
- checkers
- .getChecker(checkerUuid)
- .orElseThrow(
- () ->
- new UnprocessableEntityException(
- String.format("checker %s not found", checkerUuid)));
-
- if (checker.getStatus() == CheckerStatus.DISABLED) {
- return ImmutableList.of();
- }
-
- // The query system can only match against the current patch set; ignore non-current patch sets
- // for now.
- List<ChangeData> changes =
- checker.queryMatchingChanges(
- retryHelper, queryBuilderProvider.get(), changeQueryProcessorProvider);
- List<PendingChecksInfo> pendingChecks = new ArrayList<>(changes.size());
- for (ChangeData cd : changes) {
- getPostFilteredPendingChecks(cd.project(), cd.currentPatchSet().getId())
- .ifPresent(pendingChecks::add);
- }
- return pendingChecks;
- }
-
- private Optional<PendingChecksInfo> getPostFilteredPendingChecks(
- Project.NameKey repositoryName, PatchSet.Id patchSetId) throws OrmException, IOException {
- CheckState checkState = getCheckState(repositoryName, patchSetId);
- if (!states.contains(checkState)) {
- return Optional.empty();
- }
- return Optional.of(
- createPendingChecksInfo(repositoryName, patchSetId, checkerUuid, checkState));
- }
-
- private CheckState getCheckState(Project.NameKey project, PatchSet.Id patchSetId)
- throws OrmException, IOException {
- Optional<Check> check =
- checks.getCheck(
- CheckKey.create(project, patchSetId, checkerUuid), GetCheckOptions.defaults());
-
- // Backfill if check is not present.
- // Backfilling is only done for relevant checkers (checkers where the repository and the query
- // matches the change). Since the change was found by executing the query of the checker we know
- // that the checker is relevant for this patch set and hence backfilling should be done.
- return check.map(Check::state).orElse(CheckState.NOT_STARTED);
- }
-
- private static PendingChecksInfo createPendingChecksInfo(
- Project.NameKey repositoryName,
- PatchSet.Id patchSetId,
- CheckerUuid checkerUuid,
- CheckState checkState) {
- PendingChecksInfo pendingChecksInfo = new PendingChecksInfo();
-
- pendingChecksInfo.patchSet = new CheckablePatchSetInfo();
- pendingChecksInfo.patchSet.repository = repositoryName.get();
- pendingChecksInfo.patchSet.changeNumber = patchSetId.getParentKey().get();
- pendingChecksInfo.patchSet.patchSetId = patchSetId.get();
-
- pendingChecksInfo.pendingChecks =
- ImmutableMap.of(checkerUuid.get(), new PendingCheckInfo(checkState));
-
- return pendingChecksInfo;
- }
-}
diff --git a/java/com/google/gerrit/plugins/checks/api/PendingChecks.java b/java/com/google/gerrit/plugins/checks/api/PendingChecks.java
index 361472b..458f4ec 100644
--- a/java/com/google/gerrit/plugins/checks/api/PendingChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/PendingChecks.java
@@ -14,41 +14,35 @@
package com.google.gerrit.plugins.checks.api;
-import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.plugins.checks.CheckerUuid;
import java.util.List;
public interface PendingChecks {
- /**
- * Lists the pending checks for the specified checker.
- *
- * @param checkerUuid the UUID of the checker for which pending checks should be listed
- * @param checkStates the states that should be considered as pending, if not specified {@link
- * CheckState#NOT_STARTED} is assumed.
- * @return the pending checks
- */
- List<PendingChecksInfo> list(CheckerUuid checkerUuid, CheckState... checkStates)
- throws RestApiException;
+ QueryRequest query();
- /**
- * Lists the pending checks for the specified checker.
- *
- * @param checkerUuidString the UUID of the checker for which pending checks should be listed
- * @param checkStates the states that should be considered as pending, if not specified {@link
- * CheckState#NOT_STARTED} is assumed.
- * @return the pending checks
- */
- default List<PendingChecksInfo> list(String checkerUuidString, CheckState... checkStates)
- throws RestApiException {
- return list(
- CheckerUuid.tryParse(checkerUuidString)
- .orElseThrow(
- () ->
- new BadRequestException(
- String.format("invalid checker UUID: %s", checkerUuidString))),
- checkStates);
+ default QueryRequest query(String query) {
+ return query().withQuery(query);
+ }
+
+ abstract class QueryRequest {
+ private String query;
+
+ public abstract List<PendingChecksInfo> get() throws RestApiException;
+
+ public QueryRequest withQuery(String query) {
+ this.query = query;
+ return this;
+ }
+
+ public String getQuery() {
+ return query;
+ }
+
+ @Override
+ public String toString() {
+ return query;
+ }
}
/**
@@ -57,7 +51,7 @@
*/
class NotImplemented implements PendingChecks {
@Override
- public List<PendingChecksInfo> list(CheckerUuid checkerUuid, CheckState... checkStates) {
+ public QueryRequest query() {
throw new NotImplementedException();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/PendingChecksCollection.java b/java/com/google/gerrit/plugins/checks/api/PendingChecksCollection.java
index a8363d1..03c710a 100644
--- a/java/com/google/gerrit/plugins/checks/api/PendingChecksCollection.java
+++ b/java/com/google/gerrit/plugins/checks/api/PendingChecksCollection.java
@@ -28,18 +28,18 @@
public class PendingChecksCollection
implements ChildCollection<TopLevelResource, PendingCheckResource> {
private final DynamicMap<RestView<PendingCheckResource>> views;
- private final ListPendingChecks listPendingChecks;
+ private final QueryPendingChecks queryPendingChecks;
@Inject
public PendingChecksCollection(
- DynamicMap<RestView<PendingCheckResource>> views, ListPendingChecks listPendingChecks) {
+ DynamicMap<RestView<PendingCheckResource>> views, QueryPendingChecks queryPendingChecks) {
this.views = views;
- this.listPendingChecks = listPendingChecks;
+ this.queryPendingChecks = queryPendingChecks;
}
@Override
public RestView<TopLevelResource> list() throws RestApiException {
- return listPendingChecks;
+ return queryPendingChecks;
}
@Override
diff --git a/java/com/google/gerrit/plugins/checks/api/PendingChecksImpl.java b/java/com/google/gerrit/plugins/checks/api/PendingChecksImpl.java
index ca4cf18..4d70071 100644
--- a/java/com/google/gerrit/plugins/checks/api/PendingChecksImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/PendingChecksImpl.java
@@ -18,32 +18,37 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.List;
-import java.util.stream.Stream;
@Singleton
public class PendingChecksImpl implements PendingChecks {
- private final Provider<ListPendingChecks> listPendingChecksProvider;
+ private final Provider<QueryPendingChecks> queryPendingChecksProvider;
@Inject
- PendingChecksImpl(Provider<ListPendingChecks> listPendingChecksProvider) {
- this.listPendingChecksProvider = listPendingChecksProvider;
+ PendingChecksImpl(Provider<QueryPendingChecks> queryPendingChecksProvider) {
+ this.queryPendingChecksProvider = queryPendingChecksProvider;
}
@Override
- public List<PendingChecksInfo> list(CheckerUuid checkerUuid, CheckState... checkStates)
- throws RestApiException {
+ public QueryRequest query() {
+ return new QueryRequest() {
+ @Override
+ public List<PendingChecksInfo> get() throws RestApiException {
+ return PendingChecksImpl.this.query(this);
+ }
+ };
+ }
+
+ private List<PendingChecksInfo> query(QueryRequest queryRequest) throws RestApiException {
try {
- ListPendingChecks listPendingChecks = listPendingChecksProvider.get();
- listPendingChecks.setChecker(checkerUuid);
- Stream.of(checkStates).forEach(listPendingChecks::addState);
- return listPendingChecks.apply(TopLevelResource.INSTANCE);
+ QueryPendingChecks queryPendingChecks = queryPendingChecksProvider.get();
+ queryPendingChecks.setQuery(queryRequest.getQuery());
+ return queryPendingChecks.apply(TopLevelResource.INSTANCE);
} catch (Exception e) {
- throw asRestApiException("Cannot list pending checks", e);
+ throw asRestApiException("Cannot query pending checks", e);
}
}
}
diff --git a/java/com/google/gerrit/plugins/checks/PostCheck.java b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
similarity index 87%
rename from java/com/google/gerrit/plugins/checks/PostCheck.java
rename to java/com/google/gerrit/plugins/checks/api/PostCheck.java
index 40cff00..1d8f1e8 100644
--- a/java/com/google/gerrit/plugins/checks/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
@@ -12,16 +12,23 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.plugins.checks;
+package com.google.gerrit.plugins.checks.api;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.plugins.checks.AdministrateCheckersPermission;
+import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.CheckJson;
+import com.google.gerrit.plugins.checks.CheckKey;
+import com.google.gerrit.plugins.checks.CheckUpdate;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+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.api.CheckInfo;
-import com.google.gerrit.plugins.checks.api.CheckInput;
-import com.google.gerrit.plugins.checks.api.CheckResource;
+import com.google.gerrit.plugins.checks.ChecksUpdate;
+import com.google.gerrit.plugins.checks.UrlValidator;
import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.permissions.PermissionBackend;
diff --git a/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
new file mode 100644
index 0000000..ef5d324
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
@@ -0,0 +1,237 @@
+// 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.api;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.index.query.AndPredicate;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.CheckKey;
+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.Checks;
+import com.google.gerrit.plugins.checks.Checks.GetCheckOptions;
+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.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.ChangeQueryProcessor;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.kohsuke.args4j.Option;
+
+public class QueryPendingChecks implements RestReadView<TopLevelResource> {
+ private final CheckQueryBuilder checkQueryBuilder;
+ private final Checkers checkers;
+ private final Checks checks;
+ private final RetryHelper retryHelper;
+ private final Provider<ChangeQueryBuilder> queryBuilderProvider;
+ private final Provider<ChangeQueryProcessor> changeQueryProcessorProvider;
+
+ private String queryString;
+
+ @Option(
+ name = "--query",
+ aliases = {"-q"},
+ metaVar = "QUERY",
+ usage = "check query")
+ public void setQuery(String queryString) {
+ this.queryString = queryString;
+ }
+
+ @Inject
+ public QueryPendingChecks(
+ CheckQueryBuilder checkQueryBuilder,
+ Checkers checkers,
+ Checks checks,
+ RetryHelper retryHelper,
+ Provider<ChangeQueryBuilder> queryBuilderProvider,
+ Provider<ChangeQueryProcessor> changeQueryProcessorProvider) {
+ this.checkQueryBuilder = checkQueryBuilder;
+ this.checkers = checkers;
+ this.checks = checks;
+ this.retryHelper = retryHelper;
+ this.queryBuilderProvider = queryBuilderProvider;
+ this.changeQueryProcessorProvider = changeQueryProcessorProvider;
+ }
+
+ @Override
+ public List<PendingChecksInfo> apply(TopLevelResource resource)
+ throws RestApiException, IOException, ConfigInvalidException, OrmException {
+ if (queryString == null) {
+ throw new BadRequestException("query is required");
+ }
+
+ Predicate<Check> query = validateQuery(parseQuery(queryString));
+ if (!hasStatePredicate(query)) {
+ query = Predicate.and(new CheckStatePredicate(CheckState.NOT_STARTED), query);
+ }
+
+ Optional<Checker> checker = checkers.getChecker(getCheckerUuidFromQuery(query));
+ if (!checker.isPresent() || checker.get().isDisabled()) {
+ return ImmutableList.of();
+ }
+
+ // The query system can only match against the current patch set; ignore non-current patch sets
+ // for now.
+ List<ChangeData> changes =
+ checker
+ .get()
+ .queryMatchingChanges(
+ retryHelper, queryBuilderProvider.get(), changeQueryProcessorProvider);
+ CheckerUuid checkerUuid = checker.get().getUuid();
+ List<PendingChecksInfo> pendingChecks = new ArrayList<>(changes.size());
+ for (ChangeData cd : changes) {
+ PatchSet patchSet = cd.currentPatchSet();
+ CheckKey checkKey = CheckKey.create(cd.project(), patchSet.getId(), checkerUuid);
+
+ // Backfill if check is not present.
+ // Backfilling is only done for relevant checkers (checkers where the repository and the query
+ // matches the change). Since the change was found by executing the query of the checker we
+ // know that the checker is relevant for this patch set and hence backfilling should be done.
+ Check check =
+ checks
+ .getCheck(checkKey, GetCheckOptions.defaults())
+ .orElseGet(() -> Check.newBackfilledCheck(cd.project(), patchSet, checker.get()));
+
+ if (query.asMatchable().match(check)) {
+ pendingChecks.add(createPendingChecksInfo(cd.project(), patchSet, checkerUuid, check));
+ }
+ }
+ return pendingChecks;
+ }
+
+ private Predicate<Check> parseQuery(String query) throws BadRequestException {
+ try {
+ return checkQueryBuilder.parse(query.trim());
+ } catch (QueryParseException e) {
+ throw new BadRequestException(e.getMessage());
+ }
+ }
+
+ private static Predicate<Check> validateQuery(Predicate<Check> predicate)
+ throws BadRequestException {
+ if (countCheckerPredicates(predicate) != 1)
+ throw new BadRequestException(
+ String.format(
+ "query must contain exactly 1 '%s' operator", CheckQueryBuilder.FIELD_CHECKER));
+
+ // 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
+ // AndPredicate, OrPredicate and NotPredicate).
+ if (!predicate.getChildren().stream().anyMatch(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));
+ }
+ // ... 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));
+ }
+ return predicate;
+ }
+
+ private static boolean hasStatePredicate(Predicate<Check> predicate) {
+ if (predicate instanceof CheckStatePredicate) {
+ return true;
+ }
+ if (predicate.getChildCount() == 0) {
+ return false;
+ }
+ return predicate.getChildren().stream().anyMatch(QueryPendingChecks::hasStatePredicate);
+ }
+
+ /**
+ * Counts the number of {@link CheckerPredicate}s in the given predicate.
+ *
+ * <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
+ * @return the number of checker predicates in the given predicate
+ */
+ private static int countCheckerPredicates(Predicate<Check> predicate) {
+ if (predicate instanceof CheckerPredicate) {
+ return 1;
+ }
+ if (predicate.getChildCount() == 0) {
+ return 0;
+ }
+ return predicate.getChildren().stream()
+ .mapToInt(QueryPendingChecks::countCheckerPredicates)
+ .sum();
+ }
+
+ private static 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.
+
+ if (predicate instanceof CheckerPredicate) {
+ return ((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)));
+ }
+
+ private static PendingChecksInfo createPendingChecksInfo(
+ Project.NameKey repositoryName, PatchSet patchSet, CheckerUuid checkerUuid, Check check) {
+ PendingChecksInfo pendingChecksInfo = new PendingChecksInfo();
+
+ pendingChecksInfo.patchSet = new CheckablePatchSetInfo();
+ pendingChecksInfo.patchSet.repository = repositoryName.get();
+ pendingChecksInfo.patchSet.changeNumber = patchSet.getId().getParentKey().get();
+ pendingChecksInfo.patchSet.patchSetId = patchSet.getPatchSetId();
+
+ pendingChecksInfo.pendingChecks =
+ ImmutableMap.of(checkerUuid.get(), new PendingCheckInfo(check.state()));
+
+ return pendingChecksInfo;
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
index 2b24640..2a97201 100644
--- a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
@@ -17,7 +17,6 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.plugins.checks.PostCheck;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckBackfiller.java b/java/com/google/gerrit/plugins/checks/db/CheckBackfiller.java
index dfd3892..2b1a26a 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckBackfiller.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckBackfiller.java
@@ -16,11 +16,9 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.plugins.checks.Check;
-import com.google.gerrit.plugins.checks.CheckKey;
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.CheckState;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.AnonymousUser;
@@ -69,7 +67,7 @@
for (Checker checker : candidates) {
if (checker.isCheckerRelevant(cd, queryBuilder)) {
// Add synthetic check at the creation time of the patch set.
- result.add(newBackfilledCheck(cd, ps, checker));
+ result.add(Check.newBackfilledCheck(cd.project(), ps, checker));
}
}
return result.build();
@@ -95,15 +93,7 @@
|| !checker.get().isCheckerRelevant(cd, newQueryBuilder())) {
return Optional.empty();
}
- return Optional.of(newBackfilledCheck(cd, cd.patchSet(psId), checker.get()));
- }
-
- private Check newBackfilledCheck(ChangeData cd, PatchSet ps, Checker checker) {
- return Check.builder(CheckKey.create(cd.project(), ps.getId(), checker.getUuid()))
- .setState(CheckState.NOT_STARTED)
- .setCreated(ps.getCreatedOn())
- .setUpdated(ps.getCreatedOn())
- .build();
+ return Optional.of(Check.newBackfilledCheck(cd.project(), cd.patchSet(psId), checker.get()));
}
private ChangeQueryBuilder newQueryBuilder() {
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckPredicate.java b/java/com/google/gerrit/plugins/checks/index/CheckPredicate.java
new file mode 100644
index 0000000..562df5f
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/index/CheckPredicate.java
@@ -0,0 +1,30 @@
+// 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 com.google.gerrit.index.query.Matchable;
+import com.google.gerrit.index.query.OperatorPredicate;
+import com.google.gerrit.plugins.checks.Check;
+
+public abstract class CheckPredicate extends OperatorPredicate<Check> implements Matchable<Check> {
+ protected CheckPredicate(String name, String value) {
+ super(name, value);
+ }
+
+ @Override
+ public int getCost() {
+ return 0;
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java b/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
new file mode 100644
index 0000000..e8fe87e
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
@@ -0,0 +1,63 @@
+// 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.stream.Collectors.toList;
+
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryBuilder;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.inject.Inject;
+import java.util.Arrays;
+
+public class CheckQueryBuilder extends QueryBuilder<Check> {
+ public static final String FIELD_CHECKER = "checker";
+ public static final String FIELD_STATE = "state";
+
+ private static final QueryBuilder.Definition<Check, CheckQueryBuilder> mydef =
+ new QueryBuilder.Definition<>(CheckQueryBuilder.class);
+
+ @Inject
+ CheckQueryBuilder() {
+ super(mydef);
+ }
+
+ @Operator
+ public Predicate<Check> checker(String checkerUuid) throws QueryParseException {
+ return CheckerPredicate.parse(checkerUuid);
+ }
+
+ @Operator
+ public Predicate<Check> is(String value) throws QueryParseException {
+ if ("inprogress".equalsIgnoreCase(value)) {
+ return Predicate.or(
+ Arrays.stream(CheckState.values())
+ .filter(CheckState::isInProgress)
+ .map(CheckStatePredicate::new)
+ .collect(toList()));
+ }
+
+ return CheckStatePredicate.tryParse(value)
+ .orElseThrow(
+ () -> new QueryParseException(String.format("unsupported operator: is:%s", value)));
+ }
+
+ @Operator
+ public Predicate<Check> state(String state) throws QueryParseException {
+ return CheckStatePredicate.parse(state);
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckStatePredicate.java b/java/com/google/gerrit/plugins/checks/index/CheckStatePredicate.java
new file mode 100644
index 0000000..326c02a
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/index/CheckStatePredicate.java
@@ -0,0 +1,48 @@
+// 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.common.base.Enums;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.gwtorm.server.OrmException;
+import java.util.Optional;
+
+public class CheckStatePredicate extends CheckPredicate {
+ public static CheckStatePredicate parse(String value) throws QueryParseException {
+ return tryParse(value)
+ .orElseThrow(
+ () -> new QueryParseException(String.format("invalid check state: %s", value)));
+ }
+
+ public static Optional<CheckStatePredicate> tryParse(String value) {
+ return Enums.getIfPresent(CheckState.class, value).toJavaUtil().map(CheckStatePredicate::new);
+ }
+
+ private final CheckState checkState;
+
+ public CheckStatePredicate(CheckState checkState) {
+ super(CheckQueryBuilder.FIELD_STATE, checkState.name());
+ this.checkState = requireNonNull(checkState, "checkState");
+ }
+
+ @Override
+ public boolean match(Check check) throws OrmException {
+ return checkState.equals(check.state());
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckerPredicate.java b/java/com/google/gerrit/plugins/checks/index/CheckerPredicate.java
new file mode 100644
index 0000000..0050ce9
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/index/CheckerPredicate.java
@@ -0,0 +1,47 @@
+// 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.index.query.QueryParseException;
+import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+import com.google.gwtorm.server.OrmException;
+
+public class CheckerPredicate extends CheckPredicate {
+ public static CheckerPredicate parse(String value) throws QueryParseException {
+ return new CheckerPredicate(
+ CheckerUuid.tryParse(value)
+ .orElseThrow(
+ () -> new QueryParseException(String.format("invalid checker UUID: %s", value))));
+ }
+
+ private final CheckerUuid checkerUuid;
+
+ public CheckerPredicate(CheckerUuid checkerUuid) {
+ super(CheckQueryBuilder.FIELD_CHECKER, checkerUuid.toString());
+ this.checkerUuid = requireNonNull(checkerUuid, "checkerUuid");
+ }
+
+ @Override
+ public boolean match(Check check) throws OrmException {
+ return checkerUuid.equals(check.key().checkerUuid());
+ }
+
+ public CheckerUuid getCheckerUuid() {
+ return checkerUuid;
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
index af4ad01..442fc43 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
@@ -22,12 +22,15 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.SkipProjectClone;
import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
+import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.update.BatchUpdate;
@@ -68,6 +71,23 @@
}
@Test
+ public void canCreateCheckerLikeRef() throws Exception {
+ grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.CREATE);
+ grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+
+ String checkerRef = CheckerUuid.parse("test:my-checker").toRefName();
+
+ // checker ref can be created in any project except All-Projects
+ TestRepository<InMemoryRepository> testRepo = cloneProject(project);
+ PushOneCommit.Result r = pushFactory.create(admin.getIdent(), testRepo).to(checkerRef);
+ r.assertOkStatus();
+
+ try (Repository repo = repoManager.openRepository(project)) {
+ assertThat(repo.exactRef(checkerRef)).isNotNull();
+ }
+ }
+
+ @Test
public void cannotDeleteCheckerRef() throws Exception {
grant(allProjects, CheckerRef.REFS_CHECKERS + "*", Permission.DELETE, true, REGISTERED_USERS);
@@ -86,6 +106,26 @@
}
@Test
+ public void canDeleteCheckerLikeRef() throws Exception {
+ grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.DELETE, true, REGISTERED_USERS);
+
+ String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
+
+ allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ createBranch(new Branch.NameKey(project, checkerRef));
+
+ // checker ref can be deleted in any project except All-Projects
+ TestRepository<InMemoryRepository> testRepo = cloneProject(project);
+ PushResult r = deleteRef(testRepo, checkerRef);
+ RemoteRefUpdate refUpdate = r.getRemoteUpdate(checkerRef);
+ assertThat(refUpdate.getStatus()).isEqualTo(RemoteRefUpdate.Status.OK);
+
+ try (Repository repo = repoManager.openRepository(project)) {
+ assertThat(repo.exactRef(checkerRef)).isNull();
+ }
+ }
+
+ @Test
public void updateCheckerRefsByPushIsDisabled() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
String checkerRef = checkerUuid.toRefName();
@@ -101,11 +141,27 @@
}
@Test
+ public void updateCheckerLikeRefByPush() throws Exception {
+ String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
+
+ allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ createBranch(new Branch.NameKey(project, checkerRef));
+
+ TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
+ fetch(repo, checkerRef + ":checkerRef");
+ repo.reset("checkerRef");
+
+ grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ PushOneCommit.Result r = pushFactory.create(admin.getIdent(), repo).to(checkerRef);
+ r.assertOkStatus();
+ }
+
+ @Test
public void submitToCheckerRefsIsDisabled() throws Exception {
CheckerUuid checkerUuid =
checkerOperations.newChecker().status(CheckerStatus.DISABLED).create();
String checkerRef = checkerUuid.toRefName();
- String changeId = createChangeWithoutCommitValidation(checkerRef);
+ String changeId = createChangeWithoutCommitValidation(allProjects, checkerRef);
grantLabel(
"Code-Review",
@@ -126,6 +182,34 @@
}
@Test
+ public void submitToCheckerLikeRef() throws Exception {
+ String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
+
+ allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ createBranch(new Branch.NameKey(project, checkerRef));
+
+ String changeId = createChangeWithoutCommitValidation(project, checkerRef);
+
+ grantLabel(
+ "Code-Review",
+ -2,
+ 2,
+ project,
+ CheckerRef.REFS_CHECKERS + "*",
+ false,
+ adminGroupUuid(),
+ false);
+ approve(changeId);
+
+ grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.SUBMIT);
+
+ // submitting to a checker ref should work in any project except All-Projects
+ gApi.changes().id(changeId).current().submit();
+
+ assertThat(gApi.changes().id(changeId).get().status).isEqualTo(ChangeStatus.MERGED);
+ }
+
+ @Test
public void createChangeForCheckerRefsByPushIsDisabled() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
String checkerRef = checkerUuid.toRefName();
@@ -142,6 +226,24 @@
}
@Test
+ public void createChangeForCheckerLikeRefByPush() throws Exception {
+ String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
+
+ allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ createBranch(new Branch.NameKey(project, checkerRef));
+
+ TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
+ fetch(repo, checkerRef + ":checkerRef");
+ repo.reset("checkerRef");
+
+ // creating a change on a checker ref by push should work in any project except All-Projects
+ grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ PushOneCommit.Result r =
+ pushFactory.create(admin.getIdent(), repo).to("refs/for/" + checkerRef);
+ r.assertOkStatus();
+ }
+
+ @Test
public void createChangeForCheckerRefsViaApiIsDisabled() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
String checkerRef = checkerUuid.toRefName();
@@ -162,8 +264,30 @@
gApi.changes().create(input);
}
- private String createChangeWithoutCommitValidation(String targetRef) throws Exception {
- try (Repository git = repoManager.openRepository(allProjects);
+ @Test
+ public void createChangeForCheckerLikeRefViaApi() throws Exception {
+ String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
+
+ allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ createBranch(new Branch.NameKey(project, checkerRef));
+
+ TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
+ fetch(repo, checkerRef + ":checkerRef");
+ repo.reset("checkerRef");
+ RevCommit head = getHead(repo.getRepository(), "HEAD");
+
+ // creating a change on a checker ref via API should work in any project except All-Projects
+ ChangeInput input = new ChangeInput();
+ input.project = project.get();
+ input.branch = checkerRef;
+ input.baseCommit = head.name();
+ input.subject = "A change.";
+ assertThat(gApi.changes().create(input).get()).isNotNull();
+ }
+
+ private String createChangeWithoutCommitValidation(Project.NameKey project, String targetRef)
+ throws Exception {
+ try (Repository git = repoManager.openRepository(project);
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
RevWalk rw = new RevWalk(reader)) {
@@ -182,8 +306,7 @@
ins.setValidate(false);
ins.setMessage(String.format("Uploaded patch set %s.", ins.getPatchSetId().get()));
try (BatchUpdate bu =
- updateFactory.create(
- allProjects, identifiedUserFactory.create(admin.id), TimeUtil.nowTs())) {
+ updateFactory.create(project, identifiedUserFactory.create(admin.id), TimeUtil.nowTs())) {
bu.setRepository(git, rw, oi);
bu.insertChange(ins);
bu.execute();
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListPendingChecksIT.java
deleted file mode 100644
index 41ddec3..0000000
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListPendingChecksIT.java
+++ /dev/null
@@ -1,402 +0,0 @@
-// 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.acceptance.api;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.plugins.checks.testing.PendingChecksInfoSubject.assertThat;
-import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static org.hamcrest.CoreMatchers.instanceOf;
-
-import com.google.common.collect.Iterables;
-import com.google.gerrit.acceptance.RestResponse;
-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.RestApiException;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.plugins.checks.CheckKey;
-import com.google.gerrit.plugins.checks.CheckerUuid;
-import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
-import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerTestData;
-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.reviewdb.client.PatchSet;
-import com.google.gerrit.server.project.testing.Util;
-import com.google.gerrit.testing.TestTimeUtil;
-import com.google.inject.Inject;
-import java.sql.Timestamp;
-import java.time.Instant;
-import java.util.List;
-import java.util.concurrent.TimeUnit;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-
-public class ListPendingChecksIT extends AbstractCheckersTest {
- @Inject private RequestScopeOperations requestScopeOperations;
-
- private PatchSet.Id patchSetId;
-
- @Before
- public void setUp() throws Exception {
- TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
- TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
-
- patchSetId = createChange().getPatchSetId();
- }
-
- @After
- public void resetTime() {
- TestTimeUtil.useSystemTime();
- }
-
- @Test
- public void specifyingCheckerUuidIsRequired() throws Exception {
- // The extension API doesn't allow to not specify a checker UUID. Call the endpoint over REST to
- // test this.
- RestResponse response = adminRestSession.get("/plugins/checks/checks.pending/");
- response.assertBadRequest();
- assertThat(response.getEntityContent()).isEqualTo("checker UUID is required");
- }
-
- @Test
- public void cannotListPendingChecksForInvalidCheckerUuid() throws Exception {
- exception.expect(BadRequestException.class);
- exception.expectMessage("invalid checker UUID: " + CheckerTestData.INVALID_UUID);
- pendingChecksApi.list(CheckerTestData.INVALID_UUID);
- }
-
- @Test
- public void cannotListPendingChecksForNonExistingChecker() throws Exception {
- exception.expect(UnprocessableEntityException.class);
- exception.expectMessage("checker non:existing not found");
- pendingChecksApi.list("non:existing");
- }
-
- @Test
- public void listPendingChecksNotStartedStateAssumed() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
-
- // Create a check with state "NOT_STARTED" that we expect to be returned.
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- // Create a check with state "FAILED" that we expect to be ignored.
- PatchSet.Id patchSetId2 = createChange().getPatchSetId();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
- .setState(CheckState.FAILED)
- .upsert();
-
- // Create a check with state "NOT_STARTED" for other checker that we expect to be ignored.
- CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid2))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- List<PendingChecksInfo> pendingChecksList = pendingChecksApi.list(checkerUuid);
- assertThat(pendingChecksList).hasSize(1);
- PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasRepository(project);
- assertThat(pendingChecks).hasPatchSet(patchSetId);
- assertThat(pendingChecks)
- .hasPendingChecksMapThat()
- .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
- }
-
- @Test
- public void listPendingChecksForSpecifiedState() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
-
- // Create a check with state "FAILED" that we expect to be returned.
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.FAILED)
- .upsert();
-
- // Create a check with state "NOT_STARTED" that we expect to be ignored.
- PatchSet.Id patchSetId2 = createChange().getPatchSetId();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- // Create a check with state "FAILED" for other checker that we expect to be ignored.
- CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid2))
- .setState(CheckState.FAILED)
- .upsert();
-
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.FAILED);
- assertThat(pendingChecksList).hasSize(1);
- PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasRepository(project);
- assertThat(pendingChecks).hasPatchSet(patchSetId);
- assertThat(pendingChecks)
- .hasPendingChecksMapThat()
- .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.FAILED));
- }
-
- @Test
- public void listPendingChecksForMultipleSpecifiedStates() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
-
- // Create a check with state "NOT_STARTED" that we expect to be returned.
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(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))
- .setState(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))
- .setState(CheckState.SUCCESSFUL)
- .upsert();
-
- // Create a check with state "NOT_STARTED" for other checker that we expect to be ignored.
- CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid2))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED, CheckState.SCHEDULED);
- assertThat(pendingChecksList).hasSize(2);
-
- // 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 pendingChecksChange2 = pendingChecksList.get(0);
- assertThat(pendingChecksChange2).hasRepository(project);
- assertThat(pendingChecksChange2).hasPatchSet(patchSetId2);
- assertThat(pendingChecksChange2)
- .hasPendingChecksMapThat()
- .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.SCHEDULED));
-
- PendingChecksInfo pendingChecksChange1 = pendingChecksList.get(1);
- assertThat(pendingChecksChange1).hasRepository(project);
- assertThat(pendingChecksChange1).hasPatchSet(patchSetId);
- assertThat(pendingChecksChange1)
- .hasPendingChecksMapThat()
- .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
- }
-
- @Test
- public void backfillForApplyingChecker() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- List<PendingChecksInfo> pendingChecksList = pendingChecksApi.list(checkerUuid);
- assertThat(pendingChecksList).hasSize(1);
- PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasRepository(project);
- assertThat(pendingChecks).hasPatchSet(patchSetId);
- assertThat(pendingChecks)
- .hasPendingChecksMapThat()
- .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
- }
-
- @Test
- public void noBackfillForCheckerThatDoesNotApplyToTheProject() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(allProjects).create();
- assertThat(pendingChecksApi.list(checkerUuid)).isEmpty();
- }
-
- @Test
- public void noBackfillForCheckerThatDoesNotApplyToTheChange() throws Exception {
- CheckerUuid checkerUuid =
- checkerOperations.newChecker().repository(project).query("message:not-matching").create();
- assertThat(pendingChecksApi.list(checkerUuid)).isEmpty();
- }
-
- @Test
- public void listPendingChecksForDisabledChecker() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).isNotEmpty();
-
- checkerOperations.checker(checkerUuid).forUpdate().disable().update();
- pendingChecksList = pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).isEmpty();
- }
-
- @Test
- public void listPendingChecksFiltersOutChecksForClosedChangesIfQueryDoesntSpecifyStatus()
- throws Exception {
- CheckerUuid checkerUuid =
- checkerOperations.newChecker().repository(project).clearQuery().create();
-
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- PatchSet.Id patchSetId2 = createChange().getPatchSetId();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).hasSize(2);
-
- gApi.changes().id(patchSetId2.getParentKey().toString()).abandon();
-
- pendingChecksList = pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).hasSize(1);
- }
-
- @Test
- public void listPendingChecksReturnsChecksForClosedChangesIfQuerySpecifiesStatus()
- throws Exception {
- CheckerUuid checkerUuid =
- checkerOperations.newChecker().repository(project).query("is:open OR is:closed").create();
-
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- PatchSet.Id patchSetId2 = createChange().getPatchSetId();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).hasSize(2);
-
- gApi.changes().id(patchSetId2.getParentKey().toString()).abandon();
-
- pendingChecksList = pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).hasSize(2);
- }
-
- @Test
- public void listPendingChecksForInvalidCheckerFails() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
-
- exception.expect(RestApiException.class);
- exception.expectMessage("Cannot list pending checks");
- exception.expectCause(instanceOf(ConfigInvalidException.class));
- pendingChecksApi.list(checkerUuid);
- }
-
- @Test
- public void listPendingChecksForCheckerWithInvalidQueryFails() throws Exception {
- CheckerUuid checkerUuid =
- checkerOperations
- .newChecker()
- .repository(project)
- .query(CheckerTestData.INVALID_QUERY)
- .create();
-
- exception.expect(RestApiException.class);
- exception.expectMessage("Cannot list pending checks");
- exception.expectCause(instanceOf(ConfigInvalidException.class));
- pendingChecksApi.list(checkerUuid);
- }
-
- @Test
- public void listPendingChecksWithoutAdministrateCheckersCapabilityWorks() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- requestScopeOperations.setApiUser(user.getId());
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).hasSize(1);
- PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasRepository(project);
- assertThat(pendingChecks).hasPatchSet(patchSetId);
- assertThat(pendingChecks)
- .hasPendingChecksMapThat()
- .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
- }
-
- @Test
- public void pendingChecksDontIncludeChecksForNonVisibleChanges() throws Exception {
- // restrict project visibility so that it is only visible to administrators
- try (ProjectConfigUpdate u = updateProject(project)) {
- Util.allow(u.getConfig(), Permission.READ, adminGroupUuid(), "refs/*");
- Util.block(u.getConfig(), Permission.READ, REGISTERED_USERS, "refs/*");
- u.save();
- }
-
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- // Check is returned for admin user.
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).isNotEmpty();
-
- // Check is not returned for non-admin user.
- requestScopeOperations.setApiUser(user.getId());
- pendingChecksList = pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).isEmpty();
- }
-
- @Test
- public void pendingChecksDontIncludeChecksForPrivateChangesOfOtherUsers() throws Exception {
- // make change private so that it is only visible to the admin user
- gApi.changes().id(patchSetId.getParentKey().get()).setPrivate(true);
-
- CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkOperations
- .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
- .setState(CheckState.NOT_STARTED)
- .upsert();
-
- // Check is returned for admin user.
- List<PendingChecksInfo> pendingChecksList =
- pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).isNotEmpty();
-
- // Check is not returned for non-admin user.
- requestScopeOperations.setApiUser(user.getId());
- pendingChecksList = pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
- assertThat(pendingChecksList).isEmpty();
- }
-}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
new file mode 100644
index 0000000..c15b5d1
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
@@ -0,0 +1,582 @@
+// 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.acceptance.api;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.plugins.checks.testing.PendingChecksInfoSubject.assertThat;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static org.hamcrest.CoreMatchers.instanceOf;
+
+import com.google.common.collect.Iterables;
+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.RestApiException;
+import com.google.gerrit.plugins.checks.CheckKey;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
+import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerTestData;
+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.reviewdb.client.PatchSet;
+import com.google.gerrit.server.project.testing.Util;
+import com.google.gerrit.testing.TestTimeUtil;
+import com.google.inject.Inject;
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.util.List;
+import java.util.StringJoiner;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class QueryPendingChecksIT extends AbstractCheckersTest {
+ @Inject private RequestScopeOperations requestScopeOperations;
+
+ private PatchSet.Id patchSetId;
+
+ @Before
+ public void setUp() throws Exception {
+ TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
+
+ patchSetId = createChange().getPatchSetId();
+ }
+
+ @After
+ public void resetTime() {
+ TestTimeUtil.useSystemTime();
+ }
+
+ @Test
+ public void specifyingQueryIsRequired() throws Exception {
+ assertInvalidQuery(null, "query is required");
+ }
+
+ @Test
+ public void queryCannotBeEmpty() throws Exception {
+ assertInvalidQuery("", "query is empty");
+ }
+
+ @Test
+ public void queryCannotBeEmptyAfterTrim() throws Exception {
+ assertInvalidQuery(" ", "query is empty");
+ }
+
+ @Test
+ public void specifyingCheckerIsRequired() throws Exception {
+ assertInvalidQuery("state:NOT_STARTED", "query must contain exactly 1 'checker' operator");
+ }
+
+ @Test
+ public void cannotQueryPendingChecksForInvalidCheckerUuid() throws Exception {
+ assertInvalidQuery(
+ "checker:" + CheckerTestData.INVALID_UUID,
+ "invalid checker UUID: " + CheckerTestData.INVALID_UUID);
+ }
+
+ @Test
+ public void cannotSpecifyingMultipleCheckers() 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";
+ assertInvalidQuery(
+ String.format("checker:\"%s\" checker:\"%s\"", checkerUuid1, checkerUuid2),
+ expectedMessage);
+ assertInvalidQuery(
+ String.format("checker:\"%s\" OR checker:\"%s\"", checkerUuid1, checkerUuid2),
+ expectedMessage);
+ assertInvalidQuery(
+ String.format(
+ "checker:\"%s\" (state:NOT_STARTED checker:\"%s\")", checkerUuid1, checkerUuid2),
+ expectedMessage);
+ }
+
+ @Test
+ public void canSpecifyCheckersAsRootPredicate() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ assertThat(queryPendingChecks(String.format("checker:\"%s\"", checkerUuid))).hasSize(1);
+ }
+
+ @Test
+ public void canSpecifyCheckersInAndCondition() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ assertThat(
+ queryPendingChecks(String.format("checker:\"%s\" AND state:NOT_STARTED", checkerUuid)))
+ .hasSize(1);
+ }
+
+ @Test
+ public void andConditionAtRootCanContainAnyCombinationOfOtherPredicates() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ assertThat(
+ queryPendingChecks(
+ String.format(
+ "checker:\"%s\" AND (state:NOT_STARTED OR state:RUNNING)", checkerUuid)))
+ .hasSize(1);
+ assertThat(
+ queryPendingChecks(
+ String.format("checker:\"%s\" AND NOT state:NOT_STARTED)", checkerUuid)))
+ .isEmpty();
+ assertThat(
+ queryPendingChecks(
+ String.format(
+ "checker:\"%s\" AND (NOT state:FAILED AND NOT (state:RUNNING OR state:SUCCESSFUL))",
+ checkerUuid)))
+ .hasSize(1);
+ }
+
+ @Test
+ public void cannotSpecifyCheckersInOrCondition() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ String expectedMessage =
+ "query must be 'checker:<checker-uuid>' or 'checker:<checker-uuid> AND <other-operators>'";
+ assertInvalidQuery(
+ String.format("checker:\"%s\" OR state:NOT_STARTED", checkerUuid), expectedMessage);
+ assertInvalidQuery(
+ String.format("state:NOT_STARTED OR checker:\"%s\"", checkerUuid), expectedMessage);
+ }
+
+ @Test
+ public void cannotSpecifyCheckersInNotCondition() 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>'");
+ }
+
+ @Test
+ public void queryPendingChecksForNonExistingChecker() throws Exception {
+ assertThat(pendingChecksApi.query("checker:\"non:existing\"").get()).isEmpty();
+ }
+
+ @Test
+ public void queryPendingChecksNotStartedStateAssumed() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ // Create a check with state "NOT_STARTED" that we expect to be returned.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ // Create a check with state "FAILED" that we expect to be ignored.
+ PatchSet.Id patchSetId2 = createChange().getPatchSetId();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
+ .setState(CheckState.FAILED)
+ .upsert();
+
+ // Create a check with state "NOT_STARTED" for other checker that we expect to be ignored.
+ CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid2))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecksList = queryPendingChecks(checkerUuid);
+ assertThat(pendingChecksList).hasSize(1);
+ PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
+ assertThat(pendingChecks).hasRepository(project);
+ assertThat(pendingChecks).hasPatchSet(patchSetId);
+ assertThat(pendingChecks)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
+ }
+
+ @Test
+ public void queryPendingChecksForSpecifiedState() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ // Create a check with state "FAILED" that we expect to be returned.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.FAILED)
+ .upsert();
+
+ // Create a check with state "NOT_STARTED" that we expect to be ignored.
+ PatchSet.Id patchSetId2 = createChange().getPatchSetId();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ // Create a check with state "FAILED" for other checker that we expect to be ignored.
+ CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid2))
+ .setState(CheckState.FAILED)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecksList = queryPendingChecks(checkerUuid, CheckState.FAILED);
+ assertThat(pendingChecksList).hasSize(1);
+ PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
+ assertThat(pendingChecks).hasRepository(project);
+ assertThat(pendingChecks).hasPatchSet(patchSetId);
+ assertThat(pendingChecks)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.FAILED));
+ }
+
+ @Test
+ public void queryPendingChecksForSpecifiedStateByIsOperator() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ // Create the check once so that in the for-loop we can always update an existing check, rather
+ // than needing to check if the check already exists and then depending on this either create or
+ // update it.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ for (CheckState checkState : CheckState.values()) {
+ checkOperations
+ .check(CheckKey.create(project, patchSetId, checkerUuid))
+ .forUpdate()
+ .setState(checkState)
+ .upsert();
+
+ assertThat(queryPendingChecks(String.format("checker:\"%s\" is:%s", checkerUuid, checkState)))
+ .hasSize(1);
+ }
+ }
+
+ @Test
+ public void queryPendingChecksByIsInprogressOperator() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ // Create the check once so that in the for-loop we can always update an existing check, rather
+ // than needing to check if the check already exists and then depending on this either create or
+ // update it.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ for (CheckState checkState : CheckState.values()) {
+ checkOperations
+ .check(CheckKey.create(project, patchSetId, checkerUuid))
+ .forUpdate()
+ .setState(checkState)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecks =
+ queryPendingChecks(String.format("checker:\"%s\" is:inprogress", checkerUuid));
+ if (checkState.isInProgress()) {
+ assertThat(pendingChecks).hasSize(1);
+ } else {
+ assertThat(pendingChecks).isEmpty();
+ }
+ }
+ }
+
+ @Test
+ public void invalidStateInIsOperatorIsRejected() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ assertInvalidQuery(
+ String.format("checker:%s is:foo", checkerUuid), "unsupported operator: is:foo");
+ }
+
+ @Test
+ public void queryPendingChecksForMultipleSpecifiedStates() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ // Create a check with state "NOT_STARTED" that we expect to be returned.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(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))
+ .setState(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))
+ .setState(CheckState.SUCCESSFUL)
+ .upsert();
+
+ // Create a check with state "NOT_STARTED" for other checker that we expect to be ignored.
+ CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid2))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks(checkerUuid, CheckState.NOT_STARTED, CheckState.SCHEDULED);
+ assertThat(pendingChecksList).hasSize(2);
+
+ // 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 pendingChecksChange2 = pendingChecksList.get(0);
+ assertThat(pendingChecksChange2).hasRepository(project);
+ assertThat(pendingChecksChange2).hasPatchSet(patchSetId2);
+ assertThat(pendingChecksChange2)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.SCHEDULED));
+
+ PendingChecksInfo pendingChecksChange1 = pendingChecksList.get(1);
+ assertThat(pendingChecksChange1).hasRepository(project);
+ assertThat(pendingChecksChange1).hasPatchSet(patchSetId);
+ assertThat(pendingChecksChange1)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
+ }
+
+ @Test
+ public void queryPendingChecksForSpecifiedStateDifferentCases() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:NOT_STARTED")).hasSize(1);
+ assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:not_started")).hasSize(1);
+ assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:NoT_StArTeD")).hasSize(1);
+ }
+
+ @Test
+ public void backfillForApplyingChecker() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ List<PendingChecksInfo> pendingChecksList = queryPendingChecks(checkerUuid);
+ assertThat(pendingChecksList).hasSize(1);
+ PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
+ assertThat(pendingChecks).hasRepository(project);
+ assertThat(pendingChecks).hasPatchSet(patchSetId);
+ assertThat(pendingChecks)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
+ }
+
+ @Test
+ public void noBackfillForCheckerThatDoesNotApplyToTheProject() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(allProjects).create();
+ assertThat(queryPendingChecks(checkerUuid)).isEmpty();
+ }
+
+ @Test
+ public void noBackfillForCheckerThatDoesNotApplyToTheChange() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).query("message:not-matching").create();
+ assertThat(queryPendingChecks(checkerUuid)).isEmpty();
+ }
+
+ @Test
+ public void queryPendingChecksForDisabledChecker() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).isNotEmpty();
+
+ checkerOperations.checker(checkerUuid).forUpdate().disable().update();
+ pendingChecksList = queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).isEmpty();
+ }
+
+ @Test
+ public void queryPendingChecksFiltersOutChecksForClosedChangesIfQueryDoesntSpecifyStatus()
+ throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).clearQuery().create();
+
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ PatchSet.Id patchSetId2 = createChange().getPatchSetId();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).hasSize(2);
+
+ gApi.changes().id(patchSetId2.getParentKey().toString()).abandon();
+
+ pendingChecksList = queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).hasSize(1);
+ }
+
+ @Test
+ public void queryPendingChecksReturnsChecksForClosedChangesIfQuerySpecifiesStatus()
+ throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).query("is:open OR is:closed").create();
+
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ PatchSet.Id patchSetId2 = createChange().getPatchSetId();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId2, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).hasSize(2);
+
+ gApi.changes().id(patchSetId2.getParentKey().toString()).abandon();
+
+ pendingChecksList = queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).hasSize(2);
+ }
+
+ @Test
+ public void queryPendingChecksForInvalidCheckerFails() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+
+ exception.expect(RestApiException.class);
+ exception.expectMessage("Cannot query pending checks");
+ exception.expectCause(instanceOf(ConfigInvalidException.class));
+ queryPendingChecks(checkerUuid);
+ }
+
+ @Test
+ public void queryPendingChecksForCheckerWithInvalidQueryFails() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations
+ .newChecker()
+ .repository(project)
+ .query(CheckerTestData.INVALID_QUERY)
+ .create();
+
+ exception.expect(RestApiException.class);
+ exception.expectMessage("Cannot query pending checks");
+ exception.expectCause(instanceOf(ConfigInvalidException.class));
+ queryPendingChecks(checkerUuid);
+ }
+
+ @Test
+ public void queryPendingChecksWithoutAdministrateCheckersCapabilityWorks() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ requestScopeOperations.setApiUser(user.getId());
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).hasSize(1);
+ PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
+ assertThat(pendingChecks).hasRepository(project);
+ assertThat(pendingChecks).hasPatchSet(patchSetId);
+ assertThat(pendingChecks)
+ .hasPendingChecksMapThat()
+ .containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.NOT_STARTED));
+ }
+
+ @Test
+ public void pendingChecksDontIncludeChecksForNonVisibleChanges() throws Exception {
+ // restrict project visibility so that it is only visible to administrators
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ Util.allow(u.getConfig(), Permission.READ, adminGroupUuid(), "refs/*");
+ Util.block(u.getConfig(), Permission.READ, REGISTERED_USERS, "refs/*");
+ u.save();
+ }
+
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ // Check is returned for admin user.
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).isNotEmpty();
+
+ // Check is not returned for non-admin user.
+ requestScopeOperations.setApiUser(user.getId());
+ pendingChecksList = queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).isEmpty();
+ }
+
+ @Test
+ public void pendingChecksDontIncludeChecksForPrivateChangesOfOtherUsers() throws Exception {
+ // make change private so that it is only visible to the admin user
+ gApi.changes().id(patchSetId.getParentKey().get()).setPrivate(true);
+
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ // Check is returned for admin user.
+ List<PendingChecksInfo> pendingChecksList =
+ queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).isNotEmpty();
+
+ // Check is not returned for non-admin user.
+ requestScopeOperations.setApiUser(user.getId());
+ pendingChecksList = queryPendingChecks(checkerUuid, CheckState.NOT_STARTED);
+ assertThat(pendingChecksList).isEmpty();
+ }
+
+ private void assertInvalidQuery(String query, String expectedMessage) throws RestApiException {
+ try {
+ pendingChecksApi.query(query).get();
+ assert_().fail("expected BadRequestException");
+ } catch (BadRequestException e) {
+ assertThat(e).hasMessageThat().isEqualTo(expectedMessage);
+ }
+ }
+
+ private List<PendingChecksInfo> queryPendingChecks(String queryString) throws RestApiException {
+ return pendingChecksApi.query(queryString).get();
+ }
+
+ private List<PendingChecksInfo> queryPendingChecks(
+ CheckerUuid checkerUuid, CheckState... checkStates) throws RestApiException {
+ return pendingChecksApi.query(buildQueryString(checkerUuid, checkStates)).get();
+ }
+
+ private String buildQueryString(CheckerUuid checkerUuid, CheckState... checkStates) {
+ StringBuilder queryString = new StringBuilder();
+ queryString.append(String.format("checker:%s", checkerUuid));
+
+ 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/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java
index 0eb82ae..4941b1f 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java
@@ -20,6 +20,7 @@
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.Checks;
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
+import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerTestData;
import com.google.gerrit.plugins.checks.acceptance.testsuite.TestCheckerCreation;
import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckState;
@@ -69,6 +70,16 @@
}
@Test
+ public void returnsWarningForFailedCheckWhoseCheckerHasInvalidQuery() throws Exception {
+ CheckerUuid checkerUuid = newRequiredChecker().query(CheckerTestData.INVALID_QUERY).create();
+ setCheckState(checkerUuid, CheckState.FAILED);
+
+ CombinedCheckState combinedCheckState = checks.getCombinedCheckState(project, patchSetId);
+
+ assertThat(combinedCheckState).isEqualTo(CombinedCheckState.WARNING);
+ }
+
+ @Test
public void returnsFailedWhenAnyRequiredCheckerFailed() throws Exception {
CheckerUuid checkerUuid = newRequiredChecker().create();
setCheckSuccessful(checkerUuid);
diff --git a/src/main/resources/Documentation/rest-api-pending-checks.md b/src/main/resources/Documentation/rest-api-pending-checks.md
index 37d7bc5..8924c95 100644
--- a/src/main/resources/Documentation/rest-api-pending-checks.md
+++ b/src/main/resources/Documentation/rest-api-pending-checks.md
@@ -8,37 +8,43 @@
## <a id="pending-checks-endpoints"> Pending Checks Endpoints
-### <a id="get-checker"> List Pending Checks
+### <a id="query-pending-checks"> Query Pending Checks
_'GET /plugins/@PLUGIN@/checks.pending/'_
-Lists pending checks for a checker.
+Queries pending checks for a checker.
Checks are pending if they are in a non-final state and the external
checker system intends to post further updates on them.
-By default this REST endpoint only returns checks that are in state
-`NOT_STARTED` but callers may specify the states that they are
-interested in (see [state](#state-param) request parameter).
-
Request parameters:
-* <a id="checker-param"> `checker`: the UUID of the checker for which
- pending checks should be listed (required)
-* <a id="state-param"> `state`: state that should be considered as
- pending (optional, by default the state `NOT_STARTED` is assumed,
- this option may be specified multiple times to request checks
- matching any of several states)
+* <a id="query-param"> `query`: Query that should be used to match
+ pending checks (required). The query operators which can be used in
+ this query are described in the [Query Operators](#query-operators)
+ section below.
+
+Limitations for the input query:
+
+* Must contain exactly one [checker](#checker-operator) operator.
+* The `checker` operator must either be the only operator in the query
+ ('checker:<CHECKER_UUID>'), or appear at the root level as part of an
+ `AND` expression (e.g. 'checker:<CHECKER_UUID> state:<state>',
+ 'checker:<CHECKER_UUID> (state:<state> OR state:<state>)').
+
+If no [state](#state-operator) is used in the input query this REST
+endpoint by default only returns checks that are in state
+`NOT_STARTED`.
This REST endpoint only returns pending checks for current patch sets.
-Note that all users are allowed to list pending checks but the result
+Note that all users are allowed to query pending checks but the result
includes only checks on changes that are visible to the calling user.
This means pending checks for non-visible changes are filtered out.
#### Request by checker
```
- GET /plugins/@PLUGIN@/checks.pending/?checker=test:my-checker&state=NOT_STARTED&state=SCHEDULED HTTP/1.0
+ GET /plugins/@PLUGIN@/checks.pending/?query=checker=test:my-checker+(state:NOT_STARTED+OR+state:SCHEDULED) HTTP/1.0
```
As response a list of [PendingChecksInfo](#pending-checks-info)
@@ -106,3 +112,18 @@
| `patch_set` | The patch set for checks are pending as [CheckablePatchSetInfo](#checkable-patch-set-info) entity.
| `pending_checks` | The checks that are pending for the patch set as [checker UUID](./rest-api-checkers.md#checker-id) to [PendingCheckInfo](#pending-check-info) entity.
+## <a id="query-operators"> Query Operators
+
+The following query operators are supported in the input
+[query](#query-param) for the
+[Query Pending Checks](#query-pending-checks) REST endpoint.
+
+* <a id="checker-operator"></a> `checker:'CHECKER_UUID'`:
+ Matches checks of the checker with the UUID 'CHECKER_UUID'.
+* <a id="is-operator"></a> `is:'STATE'`:
+ Matches checks with the state 'STATE'.
+* <a id="is-inprogress-operator"></a> `is:inprogress`:
+ Matches checks with non-final states (`NOT_STARTED`, `SCHEDULED` and
+ `RUNNING`).
+* <a id="state-operator"></a> `state:'STATE'`:
+ Matches checks with the state 'STATE'.
