Return "Bad Request" status code on failure to parse request entity
Change-Id: I645580bfe7fbb7b23d90bd8ed8cd79172a630df6
diff --git a/src/main/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServlet.java
index cf30402..bb4f0ba 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServlet.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServlet.java
@@ -14,6 +14,7 @@
package com.ericsson.gerrit.plugins.evictcache;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import com.google.common.base.Splitter;
@@ -63,11 +64,23 @@
Context.setForwardedEvent();
evictCache(cache, cacheName, key);
rsp.setStatus(SC_NO_CONTENT);
+ } catch (IOException e) {
+ logger.error("Failed to process eviction request: " + e.getMessage(), e);
+ sendError(rsp, SC_BAD_REQUEST, e.getMessage());
} finally {
Context.unsetForwardedEvent();
}
}
+ private static void sendError(HttpServletResponse rsp, int statusCode,
+ String message) {
+ try {
+ rsp.sendError(statusCode, message);
+ } catch (IOException e) {
+ logger.error("Failed to send error messsage: " + e.getMessage(), e);
+ }
+ }
+
private void evictCache(Cache<?, ?> cache, String cacheName, Object key) {
if (PROJECT_LIST.equals(cacheName)) {
// One key is holding the list of projects
diff --git a/src/test/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServletTest.java
index aea1586..9c85978 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServletTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/evictcache/EvictCacheRestApiServletTest.java
@@ -14,12 +14,16 @@
package com.ericsson.gerrit.plugins.evictcache;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
+import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.registration.DynamicMap;
import org.easymock.EasyMockSupport;
+import org.junit.Before;
import org.junit.Test;
import java.io.BufferedReader;
@@ -41,57 +45,84 @@
@Test
public void evictAccounts() throws IOException, ServletException {
- setUp(Constants.ACCOUNTS);
+ configureMocksFor(Constants.ACCOUNTS);
servlet.doPost(request, response);
verifyAll();
}
@Test
public void evictProjectList() throws IOException, ServletException {
- setUp(Constants.PROJECT_LIST);
+ configureMocksFor(Constants.PROJECT_LIST);
servlet.doPost(request, response);
verifyAll();
}
@Test
public void evictGroups() throws IOException, ServletException {
- setUp(Constants.GROUPS);
+ configureMocksFor(Constants.GROUPS);
servlet.doPost(request, response);
verifyAll();
}
@Test
public void evictGroupsByInclude() throws IOException, ServletException {
- setUp(Constants.GROUPS_BYINCLUDE);
+ configureMocksFor(Constants.GROUPS_BYINCLUDE);
servlet.doPost(request, response);
verifyAll();
}
@Test
public void evictGroupsMembers() throws IOException, ServletException {
- setUp(Constants.GROUPS_MEMBERS);
+ configureMocksFor(Constants.GROUPS_MEMBERS);
servlet.doPost(request, response);
verifyAll();
}
@Test
public void evictDefault() throws IOException, ServletException {
- setUp(Constants.DEFAULT);
+ configureMocksFor(Constants.DEFAULT);
servlet.doPost(request, response);
verifyAll();
}
+ @Test
+ public void badRequest() throws IOException, ServletException {
+ expect(request.getPathInfo()).andReturn("/someCache");
+ String errorMessage = "someError";
+ expect(request.getReader()).andThrow(new IOException(errorMessage));
+ response.sendError(SC_BAD_REQUEST, errorMessage);
+ expectLastCall().once();
+ replayAll();
+ servlet.doPost(request, response);
+ verifyAll();
+ }
+
+ @Test
+ public void errorWhileSendingErrorMessage() throws Exception {
+ expect(request.getPathInfo()).andReturn("/someCache");
+ String errorMessage = "someError";
+ expect(request.getReader()).andThrow(new IOException(errorMessage));
+ response.sendError(SC_BAD_REQUEST, errorMessage);
+ expectLastCall().andThrow(new IOException("someOtherError"));
+ replayAll();
+ servlet.doPost(request, response);
+ verifyAll();
+ }
+
+ @Before
@SuppressWarnings("unchecked")
- private void setUp(String cacheName) throws IOException {
- resetAll();
+ public void setUp() {
cacheMap = createMock(DynamicMap.class);
request = createMock(HttpServletRequest.class);
reader = createMock(BufferedReader.class);
response = createNiceMock(HttpServletResponse.class);
cache = createNiceMock(Cache.class);
-
- expect(cacheMap.get(PLUGIN_NAME, cacheName)).andReturn(cache);
servlet = new EvictCacheRestApiServlet(cacheMap);
+ }
+
+ @SuppressWarnings("unchecked")
+ private void configureMocksFor(String cacheName) throws IOException {
+ expect(cacheMap.get(PLUGIN_NAME, cacheName)).andReturn(cache);
expect(request.getPathInfo()).andReturn("/" + cacheName);
expect(request.getReader()).andReturn(reader);
@@ -103,6 +134,8 @@
} else {
expect(reader.readLine()).andReturn("{}");
}
+ response.setStatus(SC_NO_CONTENT);
+ expectLastCall().once();
replayAll();
}
}