Merge "Don't support checks for change edits"
diff --git a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
index 397d8b6..78d43e4 100644
--- a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
+++ b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -57,6 +58,10 @@
@Override
public CheckResource parse(RevisionResource parent, IdString id)
throws RestApiException, PermissionBackendException, IOException, StorageException {
+ if (parent.getEdit().isPresent()) {
+ throw new ResourceConflictException("checks are not supported on a change edit");
+ }
+
CheckerUuid checkerUuid =
CheckerUuid.tryParse(id.get())
.orElseThrow(
diff --git a/java/com/google/gerrit/plugins/checks/api/ListChecks.java b/java/com/google/gerrit/plugins/checks/api/ListChecks.java
index 6f1257d..39346e4 100644
--- a/java/com/google/gerrit/plugins/checks/api/ListChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/ListChecks.java
@@ -58,6 +58,10 @@
public ImmutableList<CheckInfo> apply(RevisionResource resource)
throws AuthException, BadRequestException, ResourceConflictException, StorageException,
IOException {
+ if (resource.getEdit().isPresent()) {
+ throw new ResourceConflictException("checks are not supported on a change edit");
+ }
+
ImmutableList.Builder<CheckInfo> result = ImmutableList.builder();
GetCheckOptions getCheckOptions = GetCheckOptions.withBackfilling();
diff --git a/java/com/google/gerrit/plugins/checks/api/PostCheck.java b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
index 5de27ad..18572e9 100644
--- a/java/com/google/gerrit/plugins/checks/api/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
@@ -17,6 +17,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -81,6 +82,10 @@
}
permissionBackend.currentUser().check(permission);
+ if (rsrc.getEdit().isPresent()) {
+ throw new ResourceConflictException("checks are not supported on a change edit");
+ }
+
if (input == null) {
input = new CheckInput();
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index bd3dd1e..02847e8 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -103,6 +103,10 @@
// TODO(gerrit-team): Instead of reading the complete notes map, read just one note.
ChangeData changeData = changeDataFactory.create(repositoryName, psId.changeId());
PatchSet patchSet = changeData.patchSet(psId);
+ if (patchSet == null) {
+ throw new StorageException("patch set not found: " + psId);
+ }
+
CheckNotes checkNotes = checkNotesFactory.create(changeData.change());
checkNotes.load();
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 808027b..a8a8d19 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -354,6 +355,23 @@
assertThat(thrown).hasMessageThat().contains("Authentication required");
}
+ @Test
+ public void createCheckOnChangeEditRejected() throws Exception {
+ int changeId = patchSetId.changeId().get();
+ gApi.changes().id(changeId).edit().modifyCommitMessage("new message");
+ assertThat(gApi.changes().id(changeId).edit().get()).isPresent();
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = CheckState.RUNNING;
+ RestResponse response =
+ adminRestSession.post("/changes/" + changeId + "/revisions/edit/checks~checks", input);
+
+ response.assertConflict();
+ assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
+ }
+
// TODO(gerrit-team) More tests, especially for multiple checkers and PS and how commits behave
private Check getCheck(Project.NameKey project, PatchSet.Id patchSetId, CheckerUuid checkerUuid)
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
index 2fbeeeb..9e2a066 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -16,6 +16,7 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
@@ -27,6 +28,7 @@
import com.google.gerrit.acceptance.rest.util.RestCall;
import com.google.gerrit.acceptance.rest.util.RestCall.Method;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -46,6 +48,7 @@
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.time.Instant;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
import org.junit.After;
import org.junit.Before;
@@ -517,6 +520,22 @@
.contains(String.format("change %d", patchSetId.changeId().get()));
}
+ @Test
+ public void getCheckOnChangeEditRejected() throws Exception {
+ int changeId = patchSetId.changeId().get();
+ gApi.changes().id(changeId).edit().modifyCommitMessage("new message");
+ Optional<EditInfo> editInfo = gApi.changes().id(changeId).edit().get();
+ assertThat(editInfo).isPresent();
+
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ RestResponse response =
+ adminRestSession.get(
+ "/changes/" + changeId + "/revisions/edit/checks~checks/" + checkerUuid.get());
+
+ response.assertConflict();
+ assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
+ }
+
private CheckInfo getCheckInfo(
PatchSet.Id patchSetId, CheckerUuid checkerUuid, ListChecksOption... options)
throws RestApiException {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
index bf12827..9650784 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
@@ -23,6 +23,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
@@ -327,6 +328,18 @@
assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
+ @Test
+ public void listAllOnChangeEditRejected() throws Exception {
+ gApi.changes().id(changeId).edit().modifyCommitMessage("new message");
+ Optional<EditInfo> editInfo = gApi.changes().id(changeId).edit().get();
+ assertThat(editInfo).isPresent();
+
+ RestResponse response =
+ adminRestSession.get("/changes/" + changeId + "/revisions/edit/checks?o=CHECKER");
+ response.assertConflict();
+ assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
+ }
+
private Timestamp getPatchSetCreated(Change.Id changeId) throws RestApiException {
return getOnlyElement(
gApi.changes().id(changeId.get()).get(CURRENT_REVISION).revisions.values())
