Disallow DELETE when index does not support delete
Since change Ic72ed4f12 it is implicitly supported to send DELETE
requests to the index REST servlets for accounts and groups, however
the account and group indexes don't support deleting entries.
Add support to reject DELETE requests. Calling DELETE in an endpoint
that does not support it now results in 405 Method Not Allowed.
Add tests to confirm that we get the expected response when sending
DELETE to the account and group endpoints.
Change-Id: I22f57b43a5ad1487cb4e11417832ad325b067ddb
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java
index 6c67074..fca807f 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java
@@ -16,6 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static javax.servlet.http.HttpServletResponse.SC_CONFLICT;
+import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
@@ -37,6 +38,7 @@
private static final Logger logger = LoggerFactory.getLogger(AbstractIndexRestApiServlet.class);
private final Map<T, AtomicInteger> idLocks = new HashMap<>();
private final String type;
+ private final boolean allowDelete;
enum Operation {
INDEX,
@@ -47,8 +49,13 @@
abstract void index(T id, Operation operation) throws IOException, OrmException;
- AbstractIndexRestApiServlet(String type) {
+ AbstractIndexRestApiServlet(String type, boolean allowDelete) {
this.type = type;
+ this.allowDelete = allowDelete;
+ }
+
+ AbstractIndexRestApiServlet(String type) {
+ this(type, false);
}
@Override
@@ -60,7 +67,11 @@
@Override
protected void doDelete(HttpServletRequest req, HttpServletResponse rsp)
throws IOException, ServletException {
- process(req, rsp, Operation.DELETE);
+ if (!allowDelete) {
+ sendError(rsp, SC_METHOD_NOT_ALLOWED, String.format("cannot delete %s from index", type));
+ } else {
+ process(req, rsp, Operation.DELETE);
+ }
}
private void process(HttpServletRequest req, HttpServletResponse rsp, Operation operation) {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java
index 0154bce..f8a3c42 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexChangeRestApiServlet.java
@@ -35,7 +35,7 @@
@Inject
IndexChangeRestApiServlet(ChangeIndexer indexer, SchemaFactory<ReviewDb> schemaFactory) {
- super("change");
+ super("change", true);
this.indexer = indexer;
this.schemaFactory = schemaFactory;
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java
index cc80fbb..9893c0a 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexAccountRestApiServletTest.java
@@ -15,6 +15,7 @@
package com.ericsson.gerrit.plugins.highavailability.forwarder.rest;
import static javax.servlet.http.HttpServletResponse.SC_CONFLICT;
+import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.times;
@@ -66,6 +67,12 @@
}
@Test
+ public void cannotDeleteAccount() throws Exception {
+ servlet.doDelete(req, rsp);
+ verify(rsp).sendError(SC_METHOD_NOT_ALLOWED, "cannot delete account from index");
+ }
+
+ @Test
public void indexerThrowsIOExceptionTryingToIndexAccount() throws Exception {
doThrow(new IOException("io-error")).when(indexer).index(id);
servlet.doPost(req, rsp);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java
index f317d4a..0994b9b 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/IndexGroupRestApiServletTest.java
@@ -15,6 +15,7 @@
package com.ericsson.gerrit.plugins.highavailability.forwarder.rest;
import static javax.servlet.http.HttpServletResponse.SC_CONFLICT;
+import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.times;
@@ -66,6 +67,12 @@
}
@Test
+ public void cannotDeleteGroup() throws Exception {
+ servlet.doDelete(req, rsp);
+ verify(rsp).sendError(SC_METHOD_NOT_ALLOWED, "cannot delete group from index");
+ }
+
+ @Test
public void indexerThrowsIOExceptionTryingToIndexGroup() throws Exception {
doThrow(new IOException("io-error")).when(indexer).index(uuid);
servlet.doPost(req, rsp);
