Fix binding of DELETE REST calls from plugins
As reported in the referenced issue the deletion of projects using:
DELETE /projects/foo
didn't work, while the other option, using POST request worked:
POST /projects/foo/delete-project~delete
The processing of the DELETE request actually never reached the
delete-project plugin due to bug in the RestApiServlet which wrongly
delegated the request to a child collection in Gerrit core. Limit that
"fallback" to GET requests only.
Bug: Issue 14127
Change-Id: I9b9632e8f719937e5f7c61466996be79e6f29c14
(cherry picked from commit 899698797ca1952c9adf3f5d098ee22dbbe22481)
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index fe3621e..8160754 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1304,9 +1304,11 @@
// Check if we want to delegate to a child collection. Child collections are bound with
// GET.name so we have to check for this since we haven't found any other views.
- core = views.get(PluginName.GERRIT, "GET." + p.get(0));
- if (core != null) {
- return new ViewData(PluginName.GERRIT, core);
+ if (method.equals("GET")) {
+ core = views.get(PluginName.GERRIT, "GET." + p.get(0));
+ if (core != null) {
+ return new ViewData(PluginName.GERRIT, core);
+ }
}
Map<String, RestView<RestResource>> r = new TreeMap<>();
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
index 22feeb7..b53f182 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
@@ -15,13 +15,21 @@
package com.google.gerrit.acceptance.rest.binding;
import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND;
+import static com.google.gerrit.server.change.RobotCommentResource.ROBOT_COMMENT_KIND;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.rest.util.RestApiCallHelper;
import com.google.gerrit.acceptance.rest.util.RestCall;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Change.Id;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
+import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
@@ -33,11 +41,16 @@
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.change.RobotCommentResource;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
import org.junit.Test;
/**
@@ -54,7 +67,7 @@
private static final String PLUGIN_NAME = "my-plugin";
- private static final ImmutableSet<RestCall> TEST_CALLS =
+ private static final ImmutableSet<RestCall> REVISION_TEST_CALLS =
ImmutableSet.of(
// Calls that have the plugin name as part of the collection name
RestCall.get("/changes/%s/revisions/%s/" + PLUGIN_NAME + "~test-collection/"),
@@ -70,6 +83,9 @@
RestCall.post("/changes/%s/revisions/%s/test-collection/"),
RestCall.post("/changes/%s/revisions/%s/test-collection/1/update"));
+ private static final ImmutableSet<RestCall> ROBOTCOMMENT_TEST_CALLS =
+ ImmutableSet.of(RestCall.delete("/changes/%s/revisions/%s/robotcomments/%s"));
+
/**
* Module for all sys bindings.
*
@@ -89,6 +105,7 @@
postOnCollection(TEST_KIND).to(TestPostOnCollection.class);
post(TEST_KIND, "update").to(TestPost.class);
get(TEST_KIND, "detail").to(TestGet.class);
+ delete(ROBOT_COMMENT_KIND).to(TestDelete.class);
}
});
}
@@ -148,15 +165,69 @@
}
}
+ @Singleton
+ static class TestDelete implements RestModifyView<RobotCommentResource, String> {
+ @Override
+ public Response<?> apply(RobotCommentResource resource, String input) throws Exception {
+ return Response.none();
+ }
+ }
+
@Test
- public void testEndpoints() throws Exception {
+ public void testRevisionEndpoints() throws Exception {
PatchSet.Id patchSetId = createChange().getPatchSetId();
try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, MyPluginSysModule.class, null, null)) {
RestApiCallHelper.execute(
adminRestSession,
- TEST_CALLS.asList(),
+ REVISION_TEST_CALLS.asList(),
String.valueOf(patchSetId.changeId().get()),
String.valueOf(patchSetId.get()));
}
}
+
+ @Test
+ public void testRobotCommentEndpoints() throws Exception {
+ PatchSet.Id patchSetId = createChange().getPatchSetId();
+ String robotCommentUuid = createRobotComment(patchSetId.changeId());
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, MyPluginSysModule.class, null, null)) {
+ RestApiCallHelper.execute(
+ adminRestSession,
+ ROBOTCOMMENT_TEST_CALLS.asList(),
+ String.valueOf(patchSetId.changeId().get()),
+ String.valueOf(patchSetId.get()),
+ robotCommentUuid);
+ }
+ }
+
+ private String createRobotComment(Change.Id changeId) throws Exception {
+ addRobotComment(changeId, createRobotCommentInput(PushOneCommit.FILE_NAME));
+ return Iterables.getOnlyElement(
+ Iterables.getOnlyElement(
+ gApi.changes().id(changeId.get()).current().robotComments().values()))
+ .id;
+ }
+
+ private void addRobotComment(Id changeId, RobotCommentInput robotCommentInput) throws Exception {
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.robotComments =
+ Collections.singletonMap(robotCommentInput.path, ImmutableList.of(robotCommentInput));
+ reviewInput.message = "Test robot comment";
+ reviewInput.tag = ChangeMessagesUtil.AUTOGENERATED_TAG_PREFIX;
+ gApi.changes().id(changeId.get()).current().review(reviewInput);
+ }
+
+ private static RobotCommentInput createRobotCommentInput(
+ String path, FixSuggestionInfo... fixSuggestionInfos) {
+ RobotCommentInput in = new RobotCommentInput();
+ in.robotId = "happyRobot";
+ in.robotRunId = "1";
+ in.message = "nit: trailing whitespace";
+ in.path = path;
+ in.url = "http://www.happy-robot.com";
+ in.properties = new HashMap<>();
+ in.properties.put("key1", "value1");
+ in.properties.put("key2", "value2");
+ in.fixSuggestions = Arrays.asList(fixSuggestionInfos);
+ return in;
+ }
}