Alon Bar-Lev has uploaded a new change for review. Change subject: webadmin: branding: support If-Last-Modified instead of proprietary ......................................................................
webadmin: branding: support If-Last-Modified instead of proprietary Change-Id: I9e9f3c5ab6306300533de47b0e456a8fbaa2a218 Signed-off-by: Alon Bar-Lev <[email protected]> --- M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java 2 files changed, 16 insertions(+), 84 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/00/16000/1 diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java index 117cd5f..2ca306f 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java @@ -46,53 +46,10 @@ public void doGet(final HttpServletRequest request, final HttpServletResponse response) throws IOException, ServletException { - // Get the requested path: - String path = request.getPathInfo(); - // Locate the requested file: - String fullPath = getFullPath(brandingManager.getBrandingRootPath(), path); - if (fullPath != null) { - File file = new File(fullPath); - if (!file.exists() || !file.canRead() || file.isDirectory()) { - log.error("Unable to retrieve file: " + file.getAbsolutePath() //$NON-NLS-1$ - + ", will send a 404 error response."); //$NON-NLS-1$ - response.sendError(HttpServletResponse.SC_NOT_FOUND); - } else { - String etag = generateEtag(file); - if (etag.equals(request.getHeader(GwtDynamicHostPageServlet. - IF_NONE_MATCH_HEADER))) { - response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); - } else { - String mime = ServletUtils.getMimeMap(). - getContentType(file); - response.addHeader(GwtDynamicHostPageServlet.ETAG_HEADER, - etag); - // Send the content of the file: - ServletUtils.sendFile(request, response, file, mime); - } - } - } else { - response.sendError(HttpServletResponse.SC_NOT_FOUND); - } - } - - /** - * Generate an ETAG based on the file length, and last modified date - * of the passed in file. - * @param file The {@code File} object to check. - * @return A {@code String} representing the ETag. - */ - String generateEtag(final File file) { - StringBuilder builder = new StringBuilder(); - // Start of ETag - builder.append("W/\""); //$NON-NLS-1$ - builder.append(file.length()); - // Divider between length and last modified date. - builder.append('-'); //$NON-NLS-1$ - builder.append(file.lastModified()); - // End of ETag - builder.append("\""); //$NON-NLS-1$ - return builder.toString(); + // serve the file + ServletUtils.sendFile(request, response, + getFile(brandingManager.getBrandingRootPath(), request.getPathInfo()), null); } /** @@ -102,13 +59,13 @@ * @param path The path to translate. * @return A full absolute path for the passed in path. */ - String getFullPath(final File brandingRootPath, final String path) { - String result = null; + File getFile(final File brandingRootPath, final String path) { + File result = null; String mergedPath = FileSystems.getDefault().getPath(brandingRootPath.getAbsolutePath(), path == null ? "": path).toString(); if (path != null && ServletUtils.isSane(mergedPath)) { // Return a result relative to the branding root path. - result = mergedPath; + result = new File(mergedPath); } else { log.error("The path \"" + mergedPath + "\" is not sane"); //$NON-NLS-1$ //$NON-NLS-2$ } diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java index 5be734b..03687df 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.java @@ -3,13 +3,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.File; import java.io.IOException; -import java.util.Date; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; @@ -42,7 +40,7 @@ ServletOutputStream mockResponseOutputStream; BrandingServlet testServlet; - String testFileEtag; + File testFile; @Before public void setUp() throws Exception { @@ -52,10 +50,9 @@ when(mockFile.getAbsolutePath()).thenReturn("/abs/test"); //$NON-NLS-1$ when(mockRequest.getPathInfo()).thenReturn("/test/something.txt"); //$NON-NLS-1$ when(mockResponse.getOutputStream()).thenReturn(mockResponseOutputStream); - File testFile = new File(this.getClass().getClassLoader(). + testFile = new File(this.getClass().getClassLoader(). getResource("./org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.class") //$NON-NLS-1$ .getFile()); - testFileEtag = testServlet.generateEtag(testFile); } @Test @@ -79,49 +76,27 @@ when(mockFile.getAbsolutePath()).thenReturn(this.getClass().getClassLoader(). getResource(".").getFile()); //$NON-NLS-1$ testServlet.doGet(mockRequest, mockResponse); - verify(mockResponse).addHeader(GwtDynamicHostPageServlet.ETAG_HEADER, testFileEtag); - } - - @Test - public void testDoGet_ExistingFile_NotModified() throws IOException, ServletException { - when(mockRequest.getPathInfo()) - .thenReturn("/org/ovirt/engine/ui/frontend/server/gwt/BrandingServletTest.class"); //$NON-NLS-1$ - when(mockFile.getAbsolutePath()).thenReturn(this.getClass().getClassLoader(). - getResource(".").getFile()); //$NON-NLS-1$ - when(mockRequest.getHeader(GwtDynamicHostPageServlet.IF_NONE_MATCH_HEADER)).thenReturn(testFileEtag); - testServlet.doGet(mockRequest, mockResponse); - verify(mockResponse).setStatus(HttpServletResponse.SC_NOT_MODIFIED); - } - - @Test - public void testGenerateEtag() { - File mockFile = mock(File.class); - when(mockFile.length()).thenReturn(1234L); - Date lastModifiedDate = new Date(); - when(mockFile.lastModified()).thenReturn(lastModifiedDate.getTime()); - String result = testServlet.generateEtag(mockFile); - assertNotNull("There should be a result", result); //$NON-NLS-1$ - assertEquals("W/\"1234-" + lastModifiedDate.getTime() + "\"", result); //$NON-NLS-1$ //$NON-NLS-2$ + verify(mockResponse).setDateHeader("Last-Modified", testFile.lastModified()); //$NON-NLS-1$ } @Test public void testGetFullPath_nullParameter() { - String fullPath = testServlet.getFullPath(mockFile, null); - assertNull("Path should be null", fullPath); //$NON-NLS-1$ + File file = testServlet.getFile(mockFile, null); + assertNull("Path should be null", file); //$NON-NLS-1$ } @Test public void testGetFullPath_NonSaneParameter() { - String fullPath = testServlet.getFullPath(mockFile, "../something"); //$NON-NLS-1$ - assertNull("Path should be null", fullPath); //$NON-NLS-1$ + File file = testServlet.getFile(mockFile, "../something"); //$NON-NLS-1$ + assertNull("Path should be null", file); //$NON-NLS-1$ } @Test public void testGetFullPath_SaneParameter() { - String fullPath = testServlet.getFullPath(mockFile, "/branding/test"); //$NON-NLS-1$ - assertNotNull("Path should not be null", fullPath); //$NON-NLS-1$ + File file = testServlet.getFile(mockFile, "/branding/test"); //$NON-NLS-1$ + assertNotNull("Path should not be null", file); //$NON-NLS-1$ assertEquals("Path should be '/abs/test/branding/test'", //$NON-NLS-1$ - "/abs/test/branding/test", fullPath); //$NON-NLS-1$ + "/abs/test/branding/test", file.getAbsolutePath()); //$NON-NLS-1$ } } -- To view, visit http://gerrit.ovirt.org/16000 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9e9f3c5ab6306300533de47b0e456a8fbaa2a218 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
