Disallow creation of checks for non-existing checkers
Change-Id: Iffbea430a1707893716e69dbf24fc081ed12d747
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/checks/PostCheck.java b/java/com/google/gerrit/plugins/checks/PostCheck.java
index c5e2c02..9b78e07 100644
--- a/java/com/google/gerrit/plugins/checks/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/PostCheck.java
@@ -17,6 +17,7 @@
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.api.CheckInfo;
import com.google.gerrit.plugins.checks.api.CheckInput;
import com.google.gerrit.plugins.checks.api.CheckResource;
@@ -30,12 +31,14 @@
import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;
+import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class PostCheck
implements RestCollectionModifyView<RevisionResource, CheckResource, CheckInput> {
private final PermissionBackend permissionBackend;
private final AdministrateCheckersPermission permission;
+ private final Checkers checkers;
private final Checks checks;
private final Provider<ChecksUpdate> checksUpdate;
private final CheckJson checkJson;
@@ -44,11 +47,13 @@
PostCheck(
PermissionBackend permissionBackend,
AdministrateCheckersPermission permission,
+ Checkers checkers,
Checks checks,
@UserInitiated Provider<ChecksUpdate> checksUpdate,
CheckJson checkJson) {
this.permissionBackend = permissionBackend;
this.permission = permission;
+ this.checkers = checkers;
this.checks = checks;
this.checksUpdate = checksUpdate;
this.checkJson = checkJson;
@@ -56,7 +61,8 @@
@Override
public CheckInfo apply(RevisionResource rsrc, CheckInput input)
- throws OrmException, IOException, RestApiException, PermissionBackendException {
+ throws OrmException, IOException, RestApiException, PermissionBackendException,
+ ConfigInvalidException {
if (input == null) {
input = new CheckInput();
}
@@ -64,14 +70,20 @@
throw new BadRequestException("checker UUID is required");
}
if (!CheckerUuid.isUuid(input.checkerUuid)) {
- throw new BadRequestException("invalid checker UUID: " + input.checkerUuid);
+ throw new BadRequestException(String.format("invalid checker UUID: %s", input.checkerUuid));
}
permissionBackend.currentUser().check(permission);
- CheckKey key =
- CheckKey.create(
- rsrc.getProject(), rsrc.getPatchSet().getId(), CheckerUuid.parse(input.checkerUuid));
+ CheckerUuid checkerUuid = CheckerUuid.parse(input.checkerUuid);
+ checkers
+ .getChecker(checkerUuid)
+ .orElseThrow(
+ () ->
+ new UnprocessableEntityException(
+ String.format("checker %s not found", checkerUuid)));
+
+ CheckKey key = CheckKey.create(rsrc.getProject(), rsrc.getPatchSet().getId(), checkerUuid);
Optional<Check> check = checks.getCheck(key);
if (!check.isPresent()) {
if (input.state == null) {
diff --git a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
index 259c5ca..df517f4 100644
--- a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
@@ -23,6 +23,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class UpdateCheck implements RestModifyView<CheckResource, CheckInput> {
@@ -35,7 +36,8 @@
@Override
public CheckInfo apply(CheckResource checkResource, CheckInput input)
- throws RestApiException, IOException, OrmException, PermissionBackendException {
+ throws RestApiException, IOException, OrmException, PermissionBackendException,
+ ConfigInvalidException {
if (input == null) {
input = new CheckInput();
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
index d354ce5..b3591a7 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -20,6 +20,7 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+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;
@@ -98,6 +99,17 @@
}
@Test
+ public void cannotCreateCheckForNonExistingChecker() throws Exception {
+ CheckInput input = new CheckInput();
+ input.checkerUuid = "foo:non-existing";
+ input.state = CheckState.RUNNING;
+
+ exception.expect(UnprocessableEntityException.class);
+ exception.expectMessage("checker " + input.checkerUuid + " not found");
+ checksApiFactory.revision(patchSetId).create(input);
+ }
+
+ @Test
public void cannotCreateCheckWithoutAdministrateCheckers() throws Exception {
requestScopeOperations.setApiUser(user.getId());
