Alon Bar-Lev has uploaded a new change for review. Change subject: utils: fileservlet: support non caching mode ......................................................................
utils: fileservlet: support non caching mode Change-Id: If6943435f275d430853553e9e8da21c1de037397 Signed-off-by: Alon Bar-Lev <[email protected]> --- M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/FileServlet.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java M backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/FileServletTest.java M backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java 4 files changed, 122 insertions(+), 11 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/95/23195/1 diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/FileServlet.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/FileServlet.java index d36b1c7..806f0ec 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/FileServlet.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/FileServlet.java @@ -57,6 +57,7 @@ private static final Logger log = Logger.getLogger(FileServlet.class); // The names of the parameters: + private static final String CACHE = "cache"; private static final String TYPE = "type"; private static final String FILE = "file"; @@ -64,6 +65,7 @@ private static final String INDEX = "index.html"; // The values of the parameters: + protected boolean cache; protected String type; protected File base; @@ -71,6 +73,14 @@ public void init(ServletConfig config) throws ServletException { // Let the parent do its work: super.init(config); + + final String cacheString = config.getInitParameter(CACHE); + if (cacheString == null) { + cache = true; + } + else { + cache = Boolean.parseBoolean(cacheString); + } // Get the content type of the file (it can be null, in which case the // global system MIME types map will be used to figure it out): @@ -103,7 +113,7 @@ file = checkForIndex(request, response, file, request.getPathInfo()); // Send the content of the file: // type is the default MIME type of the Servlet. - ServletUtils.sendFile(request, response, file, type); + ServletUtils.sendFile(request, response, file, type, cache); } protected File checkForIndex(HttpServletRequest request, HttpServletResponse response, File file, String path) throws IOException { diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java index 6e0903b..f3ceb26 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ServletUtils.java @@ -72,6 +72,10 @@ * @throws IOException */ public static void sendFile(final HttpServletRequest request, final HttpServletResponse response, final File file, final String defaultType) throws IOException { + sendFile(request, response, file, defaultType, true); + } + + public static void sendFile(final HttpServletRequest request, final HttpServletResponse response, final File file, final String defaultType, boolean cache) throws IOException { // Make sure the file exits and is readable and send a 404 error // response if it doesn't: if (!canReadFile(file)) { @@ -79,19 +83,26 @@ response.sendError(HttpServletResponse.SC_NOT_FOUND); } else { - String eTag = getETag(file); + boolean send = true; - // Always include ETag on response - response.setHeader("ETag", eTag); + if (cache) { + String eTag = getETag(file); - String IfNoneMatch = request.getHeader("If-None-Match"); - if ("*".equals(IfNoneMatch)) { - response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED); + // Always include ETag on response + response.setHeader("ETag", eTag); + + String IfNoneMatch = request.getHeader("If-None-Match"); + if ("*".equals(IfNoneMatch)) { + response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED); + send = false; + } + else if (eTag.equals(IfNoneMatch)) { + response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); + send = false; + } } - else if (eTag.equals(IfNoneMatch)) { - response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); - } - else { + + if (send) { // Send metadata String mime = defaultType; if (mime == null) { diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/FileServletTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/FileServletTest.java index 2a476ca..2def429 100644 --- a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/FileServletTest.java +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/FileServletTest.java @@ -7,8 +7,10 @@ import static org.junit.Assert.fail; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -96,6 +98,48 @@ ServletOutputStream responseOut = mock(ServletOutputStream.class); when(mockResponse.getOutputStream()).thenReturn(responseOut); testServlet.doGet(mockRequest, mockResponse); + //Make sure cache is enabled + verify(mockResponse).setHeader(eq("ETag"), anyString()); + //Make sure something is written to the output stream (assuming it is the file). + verify(responseOut).write((byte[])anyObject(), eq(0), anyInt()); + } + + /** + * Test method for {@link org.ovirt.engine.core.FileServlet#doGet(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)}. + * @throws IOException + * @throws ServletException + */ + @Test + public void testDoGet2() throws ServletException, IOException { + when(mockConfig.getInitParameter("cache")).thenReturn("true"); + when(mockConfig.getInitParameter("file")).thenReturn(file.getParent()); + testServlet.init(mockConfig); + when(mockRequest.getPathInfo()).thenReturn(file.getName()); + ServletOutputStream responseOut = mock(ServletOutputStream.class); + when(mockResponse.getOutputStream()).thenReturn(responseOut); + testServlet.doGet(mockRequest, mockResponse); + //Make sure cache is enabled + verify(mockResponse).setHeader(eq("ETag"), anyString()); + //Make sure something is written to the output stream (assuming it is the file). + verify(responseOut).write((byte[])anyObject(), eq(0), anyInt()); + } + + /** + * Test method for {@link org.ovirt.engine.core.FileServlet#doGet(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)}. + * @throws IOException + * @throws ServletException + */ + @Test + public void testDoGet3() throws ServletException, IOException { + when(mockConfig.getInitParameter("cache")).thenReturn("false"); + when(mockConfig.getInitParameter("file")).thenReturn(file.getParent()); + testServlet.init(mockConfig); + when(mockRequest.getPathInfo()).thenReturn(file.getName()); + ServletOutputStream responseOut = mock(ServletOutputStream.class); + when(mockResponse.getOutputStream()).thenReturn(responseOut); + testServlet.doGet(mockRequest, mockResponse); + //Make sure cache is disabled + verify(mockResponse, never()).setHeader(eq("ETag"), anyString()); //Make sure something is written to the output stream (assuming it is the file). verify(responseOut).write((byte[])anyObject(), eq(0), anyInt()); } diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java index 2f3792d..1dde817 100644 --- a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/servlet/ServletUtilsTest.java @@ -7,8 +7,10 @@ import static org.junit.Assert.fail; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -208,6 +210,50 @@ verify(responseOut).write((byte[]) anyObject(), eq(0), anyInt()); } + /** + * Test method for {@link org.ovirt.engine.core.utils.servlet.ServletUtils#sendFile(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse, java.io.File, java.lang.String, boolean)}. + * @throws IOException + */ + @Test + public void testSendFile_PNGNoMime_Cache() throws IOException { + HttpServletRequest mockRequest = mock(HttpServletRequest.class); + HttpServletResponse mockResponse = mock(HttpServletResponse.class); + ServletOutputStream responseOut = mock(ServletOutputStream.class); + when(mockResponse.getOutputStream()).thenReturn(responseOut); + File file = createTempPng(); + ServletUtils.sendFile(mockRequest, mockResponse, file, null, true); + //Check that we have eTag + verify(mockResponse).setHeader("ETag", ServletUtils.getETag(file)); + //Check that the mime type was set to the one passed in, instead of the one associated with the file + verify(mockResponse).setContentType("image/png"); + //Check the file length is set right. + verify(mockResponse).setContentLength((int) file.length()); + //Make sure the stream is written to. + verify(responseOut).write((byte[]) anyObject(), eq(0), anyInt()); + } + + /** + * Test method for {@link org.ovirt.engine.core.utils.servlet.ServletUtils#sendFile(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse, java.io.File, java.lang.String, boolean)}. + * @throws IOException + */ + @Test + public void testSendFile_PNGNoMime_NoCache() throws IOException { + HttpServletRequest mockRequest = mock(HttpServletRequest.class); + HttpServletResponse mockResponse = mock(HttpServletResponse.class); + ServletOutputStream responseOut = mock(ServletOutputStream.class); + when(mockResponse.getOutputStream()).thenReturn(responseOut); + File file = createTempPng(); + ServletUtils.sendFile(mockRequest, mockResponse, file, null, false); + //Check that we have eTag + verify(mockResponse, never()).setHeader(eq("ETag"), anyString()); + //Check that the mime type was set to the one passed in, instead of the one associated with the file + verify(mockResponse).setContentType("image/png"); + //Check the file length is set right. + verify(mockResponse).setContentLength((int) file.length()); + //Make sure the stream is written to. + verify(responseOut).write((byte[]) anyObject(), eq(0), anyInt()); + } + private File createTempPng() { try { File file = File.createTempFile("favicon", ".png"); -- To view, visit http://gerrit.ovirt.org/23195 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If6943435f275d430853553e9e8da21c1de037397 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
