Let REST API binding tests fail if '405 Method Not Allowed' is returned

For POST requests on a REST collection RestApiServlet returns '405
Method Not Allowed' if the RestCollection exists, but doesn't implement
AcceptsPost.

E.g. this means the 'POST /changes/' call continued to succeed even if
ChangesCollection was no longer implementing AcceptsPost. To detect when
'implements AcceptsPost' was accidentally removed from a RestCollection
treat '405 Method Not Allowed' as error and let the test fail.

However for some tests we do expect '405 Method Not Allowed'. These
tests must explicitly set '405 Method Not Allowed' as expected response
code now:
- PUT /accounts/%s/username:
  Changing the username is not allowed.
- GET /projects/%s/branches/%s/reflog:
  The tests use DfsRepository which does not support getting the reflog.

Some other tests must be adapted so that they no longer return '405
Method Not Allowed':
- POST /plugins/%s/<view>:
  The view name must be prefixed with 'gerrit~'
- POST /changes/%s/hashtags:
  Setting hashtags is only supported with NoteDb.
- PUT /plugins/<new-plugin>:
  Requires plugins.allowRemoteAdmin = true

Change-Id: Id7e778c28def5c59d65485d19dc957019f143be2
Signed-off-by: Edwin Kempin <ekempin@google.com>

javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static org.apache.http.HttpStatus.SC_FORBIDDEN;
 import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
 import static org.apache.http.HttpStatus.SC_NOT_FOUND;
 
 import com.google.auto.value.AutoValue;
@@ -80,8 +81,13 @@ public abstract class AbstractRestApiBindingsTest extends AbstractDaemonTest {
     String msg = String.format("%s %s returned %d: %s", method, uri, status, body);
     if (restCall.expectedResponseCode().isPresent()) {
       assertWithMessage(msg).that(status).isEqualTo(restCall.expectedResponseCode().get());
+      if (restCall.expectedMessage().isPresent()) {
+        assertWithMessage(msg).that(body).contains(restCall.expectedMessage().get());
+      }
     } else {
-      assertWithMessage(msg).that(status).isNotIn(ImmutableList.of(SC_FORBIDDEN, SC_NOT_FOUND));
+      assertWithMessage(msg)
+          .that(status)
+          .isNotIn(ImmutableList.of(SC_FORBIDDEN, SC_NOT_FOUND, SC_METHOD_NOT_ALLOWED));
       assertWithMessage(msg).that(status).isLessThan(SC_INTERNAL_SERVER_ERROR);
     }
   }
@@ -123,6 +129,8 @@ public abstract class AbstractRestApiBindingsTest extends AbstractDaemonTest {
 
     abstract Optional<Integer> expectedResponseCode();
 
+    abstract Optional<String> expectedMessage();
+
     String uri(String... args) {
       String uriFormat = uriFormat();
       int expectedArgNum = StringUtils.countMatches(uriFormat, "%s");
@@ -144,6 +152,8 @@ public abstract class AbstractRestApiBindingsTest extends AbstractDaemonTest {
 
       abstract Builder expectedResponseCode(int expectedResponseCode);
 
+      abstract Builder expectedMessage(String expectedMessage);
+
       abstract RestCall build();
     }
   }

javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java

@@ -14,6 +14,9 @@
 
 package com.google.gerrit.acceptance.rest;
 
+import static com.google.gerrit.acceptance.rest.AbstractRestApiBindingsTest.Method.PUT;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
+
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.extensions.common.ChangeInput;
@@ -40,7 +43,11 @@ public class AccountsRestApiBindingsIT extends AbstractRestApiBindingsTest {
           RestCall.put("/accounts/%s/name"),
           RestCall.delete("/accounts/%s/name"),
           RestCall.get("/accounts/%s/username"),
-          RestCall.put("/accounts/%s/username"),
+          RestCall.builder(PUT, "/accounts/%s/username")
+              // Changing the username is not allowed.
+              .expectedResponseCode(SC_METHOD_NOT_ALLOWED)
+              .expectedMessage("Username cannot be changed.")
+              .build(),
           RestCall.get("/accounts/%s/active"),
           RestCall.put("/accounts/%s/active"),
           RestCall.delete("/accounts/%s/active"),

javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java

@@ -59,7 +59,6 @@ public class ChangesRestApiBindingsIT extends AbstractRestApiBindingsTest {
           RestCall.delete("/changes/%s/topic"),
           RestCall.get("/changes/%s/in"),
           RestCall.get("/changes/%s/hashtags"),
-          RestCall.post("/changes/%s/hashtags"),
           RestCall.get("/changes/%s/comments"),
           RestCall.get("/changes/%s/robotcomments"),
           RestCall.get("/changes/%s/drafts"),
@@ -116,7 +115,8 @@ public class ChangesRestApiBindingsIT extends AbstractRestApiBindingsTest {
    * identifier.
    */
   private static final ImmutableList<RestCall> CHANGE_ENDPOINTS_NOTEDB =
-      ImmutableList.of(RestCall.post("/changes/%s/rebuild.notedb"));
+      ImmutableList.of(
+          RestCall.post("/changes/%s/hashtags"), RestCall.post("/changes/%s/rebuild.notedb"));
 
   /**
    * Reviewer REST endpoints to be tested, each URL contains placeholders for the change identifier

javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java

@@ -42,9 +42,9 @@ public class PluginsRestApiBindingsIT extends AbstractRestApiBindingsTest {
           RestCall.get("/plugins/%s/gerrit~status"),
 
           // POST (and PUT) requests don't require the 'gerrit~' prefix in front of the view name.
-          RestCall.post("/plugins/%s/enable"),
-          RestCall.post("/plugins/%s/disable"),
-          RestCall.post("/plugins/%s/reload"),
+          RestCall.post("/plugins/%s/gerrit~enable"),
+          RestCall.post("/plugins/%s/gerrit~disable"),
+          RestCall.post("/plugins/%s/gerrit~reload"),
 
           // Plugin deletion must be tested last
           RestCall.delete("/plugins/%s"));

javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java

@@ -19,6 +19,7 @@ import static com.google.gerrit.acceptance.GitUtil.pushHead;
 import static com.google.gerrit.acceptance.rest.AbstractRestApiBindingsTest.Method.GET;
 import static com.google.gerrit.reviewdb.client.RefNames.REFS_DASHBOARDS;
 import static com.google.gerrit.server.restapi.project.DashboardsCollection.DEFAULT_DASHBOARD_NAME;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
 import static org.apache.http.HttpStatus.SC_NOT_FOUND;
 
 import com.google.common.collect.ImmutableList;
@@ -93,7 +94,11 @@ public class ProjectsRestApiBindingsIT extends AbstractRestApiBindingsTest {
           RestCall.get("/projects/%s/branches/%s"),
           RestCall.put("/projects/%s/branches/%s"),
           RestCall.get("/projects/%s/branches/%s/mergeable"),
-          RestCall.get("/projects/%s/branches/%s/reflog"),
+          RestCall.builder(GET, "/projects/%s/branches/%s/reflog")
+              // The tests use DfsRepository which does not support getting the reflog.
+              .expectedResponseCode(SC_METHOD_NOT_ALLOWED)
+              .expectedMessage("reflog not supported on")
+              .build(),
           RestCall.builder(GET, "/projects/%s/branches/%s/files")
               // GET /projects/<project>/branches/<branch>/files is not implemented
               .expectedResponseCode(SC_NOT_FOUND)

javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java

@@ -18,6 +18,7 @@ import static com.google.gerrit.acceptance.rest.AbstractRestApiBindingsTest.Meth
 import static org.apache.http.HttpStatus.SC_NOT_FOUND;
 
 import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.GerritConfig;
 import org.junit.Test;
 
 /**
@@ -48,6 +49,7 @@ public class RootCollectionsRestApiBindingsIT extends AbstractRestApiBindingsTes
           RestCall.put("/projects/new-project"));
 
   @Test
+  @GerritConfig(name = "plugins.allowRemoteAdmin", value = "true")
   public void rootEndpoints() throws Exception {
     execute(ROOT_ENDPOINTS);
   }