diff --git a/dataset_download/source/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServlet.java b/dataset_download/source/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServlet.java index 69ddd24ce1a5e9c7bbc20b491432261c798193bc..c988c546489c8ec4db0f9a618619876099f6a578 100644 --- a/dataset_download/source/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServlet.java +++ b/dataset_download/source/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServlet.java @@ -25,6 +25,8 @@ import java.io.IOException; import java.io.PrintWriter; import java.util.Arrays; import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; import javax.servlet.ServletConfig; import javax.servlet.ServletContext; @@ -53,8 +55,6 @@ import ch.systemsx.cisd.lims.base.LocatorType; */ public class DatasetDownloadServlet extends HttpServlet { - static final String DATA_SET_ROOT_DIR_KEY = "data-set-root-dir"; - static final String DATA_SET_KEY = "data-set"; static final String DATASET_CODE_KEY = "dataSetCode"; @@ -115,7 +115,28 @@ public class DatasetDownloadServlet extends HttpServlet { try { - obtainDataSetFromServer(request); + String pathInfo = request.getPathInfo(); + if (pathInfo == null) + { + throw new UserFailureException("Path not specified in URL."); + } + if (pathInfo.startsWith("/")) + { + pathInfo = pathInfo.substring(1); + } + int indexOfFirstSeparator = pathInfo.indexOf('/'); + String dataSetCode; + if (indexOfFirstSeparator < 0) + { + dataSetCode = pathInfo; + pathInfo = ""; + } else + { + dataSetCode = pathInfo.substring(0, indexOfFirstSeparator); + pathInfo = pathInfo.substring(indexOfFirstSeparator + 1); + } + + obtainDataSetFromServer(dataSetCode, request); HttpSession session = request.getSession(false); if (session == null) @@ -123,13 +144,13 @@ public class DatasetDownloadServlet extends HttpServlet printSessionExpired(response); } else { - ExternalData dataSet = (ExternalData) session.getAttribute(DATA_SET_KEY); - File rootDir = (File) session.getAttribute(DATA_SET_ROOT_DIR_KEY); - String pathInfo = request.getPathInfo(); - if (pathInfo != null && pathInfo.startsWith("/")) + + ExternalData dataSet = tryToGetDataSet(session, dataSetCode); + if (dataSet == null) { - pathInfo = pathInfo.substring(1); + throw new UserFailureException("Unknown data set '" + dataSetCode + "'."); } + File rootDir = createDataSetRootDirectory(dataSet); String requestURI = request.getRequestURI(); RenderingContext context = new RenderingContext(rootDir, requestURI, pathInfo); renderPage(response, dataSet, context); @@ -137,22 +158,34 @@ public class DatasetDownloadServlet extends HttpServlet } catch (Exception e) { - if (e instanceof UserFailureException == false) - { - operationLog.error("Request " + request.getRequestURL() + "?" - + request.getQueryString() + " caused an exception: ", e); - } else if (operationLog.isInfoEnabled()) + printError(request, response, e); + } + } + + private void printError(final HttpServletRequest request, final HttpServletResponse response, + Exception exception) throws IOException + { + if (exception instanceof UserFailureException == false) + { + StringBuffer url = request.getRequestURL(); + String queryString = request.getQueryString(); + if (StringUtils.isNotBlank(queryString)) { - operationLog.info("User failure: " + e.getMessage()); + url.append("?").append(queryString); } - PrintWriter writer = response.getWriter(); - writer.println("<html><body><h1>Error</h1>"); - String message = e.getMessage(); - writer.println(StringUtils.isBlank(message) ? e.toString() : message); - writer.println("</body></html>"); - writer.flush(); - writer.close(); + operationLog.error("Request " + url + " caused an exception: ", exception); + } else if (operationLog.isInfoEnabled()) + { + operationLog.info("User failure: " + exception.getMessage()); } + String message = exception.getMessage(); + String errorText = StringUtils.isBlank(message) ? exception.toString() : message; + PrintWriter writer = response.getWriter(); + writer.println("<html><body><h1>Error</h1>"); + writer.println(errorText); + writer.println("</body></html>"); + writer.flush(); + writer.close(); } private void renderPage(HttpServletResponse response, ExternalData dataSet, @@ -200,7 +233,8 @@ public class DatasetDownloadServlet extends HttpServlet String name = child.getName(); File rootDir = renderingContext.getRootDir(); String relativePath = FileUtilities.getRelativeFile(rootDir, child); - String normalizedRelativePath = relativePath.replace('\\', '/'); + String normalizedRelativePath = + relativePath.replace('\\', '/'); if (child.isDirectory()) { directoryRenderer.printDirectory(name, normalizedRelativePath); @@ -251,21 +285,13 @@ public class DatasetDownloadServlet extends HttpServlet writer.close(); } - private void obtainDataSetFromServer(final HttpServletRequest request) + private void obtainDataSetFromServer(String dataSetCode, final HttpServletRequest request) { - final String dataSetCode = request.getParameter(DATASET_CODE_KEY); final String sessionID = request.getParameter(SESSION_ID_KEY); - if (dataSetCode != null && sessionID != null) + if (sessionID != null) { IDataSetService dataSetService = applicationContext.getDataSetService(); ExternalData dataSet = dataSetService.getDataSet(sessionID, dataSetCode); - File dataSetRootDirectory = new File(createDataSetPath(dataSet)); - if (dataSetRootDirectory.exists() == false) - { - throw new UserFailureException("Data set '" + dataSetCode - + "' not found in store at '" + dataSetRootDirectory.getAbsolutePath() - + "'."); - } if (operationLog.isInfoEnabled()) { operationLog.info("Data set '" + dataSetCode + "' obtained from openBIS server."); @@ -273,19 +299,49 @@ public class DatasetDownloadServlet extends HttpServlet HttpSession session = request.getSession(true); ConfigParameters configParameters = applicationContext.getConfigParameters(); session.setMaxInactiveInterval(configParameters.getSessionTimeout()); - session.setAttribute(DATA_SET_KEY, dataSet); - session.setAttribute(DATA_SET_ROOT_DIR_KEY, dataSetRootDirectory); + putDataSetToMap(session, dataSetCode, dataSet); } } - private String createDataSetPath(ExternalData dataSet) + private File createDataSetRootDirectory(ExternalData dataSet) { - String location = dataSet.getLocation(); + String path = dataSet.getLocation(); LocatorType locatorType = dataSet.getLocatorType(); if (locatorType.getCode().equals(LocatorType.DEFAULT_LOCATOR_TYPE_CODE)) { - return applicationContext.getConfigParameters().getStorePath() + "/" + location; + path = applicationContext.getConfigParameters().getStorePath() + "/" + path; + } + File dataSetRootDirectory = new File(path); + if (dataSetRootDirectory.exists() == false) + { + throw new UserFailureException("Data set '" + dataSet.getCode() + + "' not found in store at '" + dataSetRootDirectory.getAbsolutePath() + + "'."); + } + return dataSetRootDirectory; + } + + private void putDataSetToMap(HttpSession session, String dataSetCode, ExternalData dataSet) + { + getDataSets(session).put(dataSetCode, dataSet); + } + + private ExternalData tryToGetDataSet(HttpSession session, String dataSetCode) + { + return getDataSets(session).get(dataSetCode); + } + + @SuppressWarnings("unchecked") + private Map<String, ExternalData> getDataSets(HttpSession session) + { + Map<String, ExternalData> map = + (Map<String, ExternalData>) session.getAttribute(DATA_SET_KEY); + if (map == null) + { + map = new HashMap<String, ExternalData>(); + session.setAttribute(DATA_SET_KEY, map); } - return location; + return map; } + } diff --git a/dataset_download/sourceTest/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServletTest.java b/dataset_download/sourceTest/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServletTest.java index a6f0068797ad72641b90d24c40d504fcf9777e81..4df19739e8f658d70c79b2de672638570a8ddd9b 100644 --- a/dataset_download/sourceTest/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServletTest.java +++ b/dataset_download/sourceTest/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServletTest.java @@ -23,6 +23,8 @@ import java.io.File; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.HashMap; +import java.util.Map; import java.util.Properties; import javax.servlet.ServletOutputStream; @@ -52,6 +54,9 @@ import ch.systemsx.cisd.lims.base.LocatorType; */ public class DatasetDownloadServletTest { + private static final String EXPIRATION_MESSAGE = + "<html><body>Download session expired.</body></html>"; + private static final String LOGGER_NAME = "OPERATION.ch.systemsx.cisd.openbis.datasetdownload.DatasetDownloadServlet"; @@ -123,8 +128,8 @@ public class DatasetDownloadServletTest { final StringWriter writer = new StringWriter(); final ExternalData externalData = createExternalData(); - prepareForObtainingDataSetFromServer(externalData, true); - prepareForGettingDataSetFromSession(externalData, null); + prepareForObtainingDataSetFromServer(externalData); + prepareForGettingDataSetFromSession(externalData, ""); prepareForCreatingHTML(writer); DatasetDownloadServlet servlet = createServlet(); @@ -132,9 +137,9 @@ public class DatasetDownloadServletTest assertEquals("<html><body>" + OSUtilities.LINE_SEPARATOR + "<h1>Data Set 1234-1</h1>" + OSUtilities.LINE_SEPARATOR + "<table border=\'0\' cellpadding=\'5\' cellspacing=\'0\'>" - + OSUtilities.LINE_SEPARATOR + "<tr><td><a href='download/sub'>sub</td><td></td></tr>" + + OSUtilities.LINE_SEPARATOR + "<tr><td><a href='download/1234-1/sub'>sub</td><td></td></tr>" + OSUtilities.LINE_SEPARATOR - + "<tr><td><a href='download/readme.txt'>readme.txt</td><td>12 Bytes</td></tr>" + + "<tr><td><a href='download/1234-1/readme.txt'>readme.txt</td><td>12 Bytes</td></tr>" + OSUtilities.LINE_SEPARATOR + "</table></body></html>" + OSUtilities.LINE_SEPARATOR, writer.toString()); assertEquals(LOG_INFO + "Data set '1234-1' obtained from openBIS server." @@ -151,7 +156,8 @@ public class DatasetDownloadServletTest final StringWriter writer = new StringWriter(); final ExternalData externalData = createExternalData(); externalData.setLocatorType(new LocatorType("unknown")); - prepareForObtainingDataSetFromServer(externalData, false); + prepareForObtainingDataSetFromServer(externalData); + prepareForGettingDataSetFromSession(externalData, "blabla"); context.checking(new Expectations() { { @@ -173,6 +179,45 @@ public class DatasetDownloadServletTest context.assertIsSatisfied(); } + + @Test + public void testDoGetButUnknownDataSetCode() throws Exception + { + final StringWriter writer = new StringWriter(); + final ExternalData externalData = createExternalData(); + prepareForObtainingDataSetFromServer(externalData); + context.checking(new Expectations() + { + { + one(request).getSession(false); + will(returnValue(httpSession)); + + one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_KEY); + Map<String, ExternalData> map = new HashMap<String, ExternalData>(); + will(returnValue(map)); + + one(request).getPathInfo(); + String codeAndPath = externalData.getCode(); + will(returnValue(codeAndPath)); + + one(response).getWriter(); + will(returnValue(new PrintWriter(writer))); + } + }); + + DatasetDownloadServlet servlet = createServlet(); + servlet.doGet(request, response); + assertEquals("<html><body><h1>Error</h1>" + OSUtilities.LINE_SEPARATOR + + "Unknown data set '1234-1'." + OSUtilities.LINE_SEPARATOR + "</body></html>" + + OSUtilities.LINE_SEPARATOR, writer.toString()); + String logContent = logRecorder.getLogContent(); + assertEquals(LOG_INFO + "Data set '1234-1' obtained from openBIS server." + + OSUtilities.LINE_SEPARATOR + LOG_INFO + + "User failure: Unknown data set '1234-1'.", logContent); + + context.assertIsSatisfied(); + } + @Test public void testDoGetSubFolder() throws Exception { @@ -188,7 +233,7 @@ public class DatasetDownloadServletTest + OSUtilities.LINE_SEPARATOR + "Folder: sub" + OSUtilities.LINE_SEPARATOR + "<table border=\'0\' cellpadding=\'5\' cellspacing=\'0\'>" - + OSUtilities.LINE_SEPARATOR + "<tr><td><a href='download/'>..</td><td></td></tr>" + + OSUtilities.LINE_SEPARATOR + "<tr><td><a href='download/1234-1/'>..</td><td></td></tr>" + OSUtilities.LINE_SEPARATOR + "</table></body></html>" + OSUtilities.LINE_SEPARATOR, writer.toString()); assertEquals(LOG_INFO + "For data set '1234-1' show directory <wd>/data-set-123/sub", @@ -274,6 +319,9 @@ public class DatasetDownloadServletTest context.checking(new Expectations() { { + one(request).getPathInfo(); + will(returnValue(EXAMPLE_DATA_SET_CODE)); + one(request).getSession(false); will(returnValue(null)); @@ -284,12 +332,67 @@ public class DatasetDownloadServletTest DatasetDownloadServlet servlet = createServlet(); servlet.doGet(request, response); - assertEquals("<html><body>Download session expired.</body></html>", writer.toString()); + assertEquals(EXPIRATION_MESSAGE, writer.toString()); assertEquals("", getNormalizedLogContent()); context.assertIsSatisfied(); } + @Test + public void testDoGetForNullPathInfo() throws Exception + { + final StringWriter writer = new StringWriter(); + context.checking(new Expectations() + { + { + one(request).getPathInfo(); + will(returnValue(null)); + + one(response).getWriter(); + will(returnValue(new PrintWriter(writer))); + } + }); + + DatasetDownloadServlet servlet = createServlet(); + servlet.doGet(request, response); + assertEquals("<html><body><h1>Error</h1>" + OSUtilities.LINE_SEPARATOR + + "Path not specified in URL." + OSUtilities.LINE_SEPARATOR + "</body></html>" + + OSUtilities.LINE_SEPARATOR, writer.toString()); + assertEquals(LOG_INFO + "User failure: Path not specified in URL.", + getNormalizedLogContent()); + + context.assertIsSatisfied(); + } + + @Test + public void testDoGetForPathInfoStartingWithSeparator() throws Exception + { + final StringWriter writer = new StringWriter(); + final ExternalData externalData = createExternalData(); + prepareForObtainingDataSetFromServer(externalData); + context.checking(new Expectations() + { + { + one(request).getPathInfo(); + will(returnValue("/" + EXAMPLE_DATA_SET_CODE)); + + one(request).getSession(false); + will(returnValue(null)); + + one(response).getWriter(); + will(returnValue(new PrintWriter(writer))); + } + }); + + DatasetDownloadServlet servlet = createServlet(); + servlet.doGet(request, response); + assertEquals(EXPIRATION_MESSAGE, writer.toString()); + assertEquals(LOG_INFO + "Data set '1234-1' obtained from openBIS server.", + getNormalizedLogContent()); + + context.assertIsSatisfied(); + } + private void prepareForGettingDataSetFromSession(final ExternalData externalData, final String path) { @@ -300,16 +403,16 @@ public class DatasetDownloadServletTest will(returnValue(httpSession)); one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_KEY); - will(returnValue(externalData)); - - one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_ROOT_DIR_KEY); - will(returnValue(EXAMPLE_DATA_SET_FOLDER)); + Map<String, ExternalData> map = new HashMap<String, ExternalData>(); + map.put(externalData.getCode(), externalData); + will(returnValue(map)); one(request).getPathInfo(); - will(returnValue(path)); + String codeAndPath = externalData.getCode() + "/" + path; + will(returnValue(codeAndPath)); - one(request).getRequestURI(); - will(returnValue("download/" + (path == null ? "" : path))); + allowing(request).getRequestURI(); + will(returnValue("download/" + codeAndPath)); } }); @@ -320,41 +423,32 @@ public class DatasetDownloadServletTest context.checking(new Expectations() { { - one(request).getParameter(DatasetDownloadServlet.DATASET_CODE_KEY); - will(returnValue(EXAMPLE_DATA_SET_CODE)); - one(request).getParameter(DatasetDownloadServlet.SESSION_ID_KEY); will(returnValue(null)); } }); } - private void prepareForObtainingDataSetFromServer(final ExternalData externalData, - final boolean fileExists) + private void prepareForObtainingDataSetFromServer(final ExternalData externalData) { context.checking(new Expectations() { { - one(request).getParameter(DatasetDownloadServlet.DATASET_CODE_KEY); - will(returnValue(EXAMPLE_DATA_SET_CODE)); - one(request).getParameter(DatasetDownloadServlet.SESSION_ID_KEY); will(returnValue(EXAMPLE_SESSION_ID)); one(dataSetService).getDataSet(EXAMPLE_SESSION_ID, EXAMPLE_DATA_SET_CODE); will(returnValue(externalData)); - if (fileExists) - { - one(request).getSession(true); - will(returnValue(httpSession)); - - one(httpSession).setMaxInactiveInterval(120); - one(httpSession).setAttribute(DatasetDownloadServlet.DATA_SET_KEY, - externalData); - one(httpSession).setAttribute(DatasetDownloadServlet.DATA_SET_ROOT_DIR_KEY, - EXAMPLE_DATA_SET_FOLDER); - } + one(request).getSession(true); + will(returnValue(httpSession)); + + one(httpSession).setMaxInactiveInterval(120); + one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_KEY); + will(returnValue(null)); + + one(httpSession).setAttribute(DatasetDownloadServlet.DATA_SET_KEY, + new HashMap<String, ExternalData>()); } }); }