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(); } }