Skip to content
Snippets Groups Projects
Commit 5a8cbbbb authored by felmer's avatar felmer
Browse files

LMS-401 bug fixed concerning multiple data sets shown in one session. More tests written. 

SVN: 5916
parent 6e44eafd
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
dataset_download/source/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServlet.java
+ 94
− 38
View file @ 5a8cbbbb
...@@ -25,6 +25,8 @@ import java.io.IOException; ...@@ -25,6 +25,8 @@ import java.io.IOException;
import java.io.PrintWriter; import java.io.PrintWriter;
import java.util.Arrays; import java.util.Arrays;
import java.util.Comparator; import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;
import javax.servlet.ServletConfig; import javax.servlet.ServletConfig;
import javax.servlet.ServletContext; import javax.servlet.ServletContext;
...@@ -53,8 +55,6 @@ import ch.systemsx.cisd.lims.base.LocatorType; ...@@ -53,8 +55,6 @@ import ch.systemsx.cisd.lims.base.LocatorType;
*/ */
public class DatasetDownloadServlet extends HttpServlet 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 DATA_SET_KEY = "data-set";
static final String DATASET_CODE_KEY = "dataSetCode"; static final String DATASET_CODE_KEY = "dataSetCode";
...@@ -115,7 +115,28 @@ public class DatasetDownloadServlet extends HttpServlet ...@@ -115,7 +115,28 @@ public class DatasetDownloadServlet extends HttpServlet
{ {
try 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); HttpSession session = request.getSession(false);
if (session == null) if (session == null)
...@@ -123,13 +144,13 @@ public class DatasetDownloadServlet extends HttpServlet ...@@ -123,13 +144,13 @@ public class DatasetDownloadServlet extends HttpServlet
printSessionExpired(response); printSessionExpired(response);
} else } else
{ {
ExternalData dataSet = (ExternalData) session.getAttribute(DATA_SET_KEY);
File rootDir = (File) session.getAttribute(DATA_SET_ROOT_DIR_KEY); ExternalData dataSet = tryToGetDataSet(session, dataSetCode);
String pathInfo = request.getPathInfo(); if (dataSet == null)
if (pathInfo != null && pathInfo.startsWith("/"))
{ {
pathInfo = pathInfo.substring(1); throw new UserFailureException("Unknown data set '" + dataSetCode + "'.");
} }
File rootDir = createDataSetRootDirectory(dataSet);
String requestURI = request.getRequestURI(); String requestURI = request.getRequestURI();
RenderingContext context = new RenderingContext(rootDir, requestURI, pathInfo); RenderingContext context = new RenderingContext(rootDir, requestURI, pathInfo);
renderPage(response, dataSet, context); renderPage(response, dataSet, context);
...@@ -137,22 +158,34 @@ public class DatasetDownloadServlet extends HttpServlet ...@@ -137,22 +158,34 @@ public class DatasetDownloadServlet extends HttpServlet
} catch (Exception e) } catch (Exception e)
{ {
if (e instanceof UserFailureException == false) printError(request, response, e);
{ }
operationLog.error("Request " + request.getRequestURL() + "?" }
+ request.getQueryString() + " caused an exception: ", e);
} else if (operationLog.isInfoEnabled()) 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(); operationLog.error("Request " + url + " caused an exception: ", exception);
writer.println("<html><body><h1>Error</h1>"); } else if (operationLog.isInfoEnabled())
String message = e.getMessage(); {
writer.println(StringUtils.isBlank(message) ? e.toString() : message); operationLog.info("User failure: " + exception.getMessage());
writer.println("</body></html>");
writer.flush();
writer.close();
} }
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, private void renderPage(HttpServletResponse response, ExternalData dataSet,
...@@ -200,7 +233,8 @@ public class DatasetDownloadServlet extends HttpServlet ...@@ -200,7 +233,8 @@ public class DatasetDownloadServlet extends HttpServlet
String name = child.getName(); String name = child.getName();
File rootDir = renderingContext.getRootDir(); File rootDir = renderingContext.getRootDir();
String relativePath = FileUtilities.getRelativeFile(rootDir, child); String relativePath = FileUtilities.getRelativeFile(rootDir, child);
String normalizedRelativePath = relativePath.replace('\\', '/'); String normalizedRelativePath =
relativePath.replace('\\', '/');
if (child.isDirectory()) if (child.isDirectory())
{ {
directoryRenderer.printDirectory(name, normalizedRelativePath); directoryRenderer.printDirectory(name, normalizedRelativePath);
...@@ -251,21 +285,13 @@ public class DatasetDownloadServlet extends HttpServlet ...@@ -251,21 +285,13 @@ public class DatasetDownloadServlet extends HttpServlet
writer.close(); 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); final String sessionID = request.getParameter(SESSION_ID_KEY);
if (dataSetCode != null && sessionID != null) if (sessionID != null)
{ {
IDataSetService dataSetService = applicationContext.getDataSetService(); IDataSetService dataSetService = applicationContext.getDataSetService();
ExternalData dataSet = dataSetService.getDataSet(sessionID, dataSetCode); 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()) if (operationLog.isInfoEnabled())
{ {
operationLog.info("Data set '" + dataSetCode + "' obtained from openBIS server."); operationLog.info("Data set '" + dataSetCode + "' obtained from openBIS server.");
...@@ -273,19 +299,49 @@ public class DatasetDownloadServlet extends HttpServlet ...@@ -273,19 +299,49 @@ public class DatasetDownloadServlet extends HttpServlet
HttpSession session = request.getSession(true); HttpSession session = request.getSession(true);
ConfigParameters configParameters = applicationContext.getConfigParameters(); ConfigParameters configParameters = applicationContext.getConfigParameters();
session.setMaxInactiveInterval(configParameters.getSessionTimeout()); session.setMaxInactiveInterval(configParameters.getSessionTimeout());
session.setAttribute(DATA_SET_KEY, dataSet); putDataSetToMap(session, dataSetCode, dataSet);
session.setAttribute(DATA_SET_ROOT_DIR_KEY, dataSetRootDirectory);
} }
} }
private String createDataSetPath(ExternalData dataSet) private File createDataSetRootDirectory(ExternalData dataSet)
{ {
String location = dataSet.getLocation(); String path = dataSet.getLocation();
LocatorType locatorType = dataSet.getLocatorType(); LocatorType locatorType = dataSet.getLocatorType();
if (locatorType.getCode().equals(LocatorType.DEFAULT_LOCATOR_TYPE_CODE)) 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;
} }
} }
dataset_download/sourceTest/java/ch/systemsx/cisd/openbis/datasetdownload/DatasetDownloadServletTest.java
+ 127
− 33
View file @ 5a8cbbbb
...@@ -23,6 +23,8 @@ import java.io.File; ...@@ -23,6 +23,8 @@ import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.io.PrintWriter; import java.io.PrintWriter;
import java.io.StringWriter; import java.io.StringWriter;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties; import java.util.Properties;
import javax.servlet.ServletOutputStream; import javax.servlet.ServletOutputStream;
...@@ -52,6 +54,9 @@ import ch.systemsx.cisd.lims.base.LocatorType; ...@@ -52,6 +54,9 @@ import ch.systemsx.cisd.lims.base.LocatorType;
*/ */
public class DatasetDownloadServletTest public class DatasetDownloadServletTest
{ {
private static final String EXPIRATION_MESSAGE =
"<html><body>Download session expired.</body></html>";
private static final String LOGGER_NAME = private static final String LOGGER_NAME =
"OPERATION.ch.systemsx.cisd.openbis.datasetdownload.DatasetDownloadServlet"; "OPERATION.ch.systemsx.cisd.openbis.datasetdownload.DatasetDownloadServlet";
...@@ -123,8 +128,8 @@ public class DatasetDownloadServletTest ...@@ -123,8 +128,8 @@ public class DatasetDownloadServletTest
{ {
final StringWriter writer = new StringWriter(); final StringWriter writer = new StringWriter();
final ExternalData externalData = createExternalData(); final ExternalData externalData = createExternalData();
prepareForObtainingDataSetFromServer(externalData, true); prepareForObtainingDataSetFromServer(externalData);
prepareForGettingDataSetFromSession(externalData, null); prepareForGettingDataSetFromSession(externalData, "");
prepareForCreatingHTML(writer); prepareForCreatingHTML(writer);
DatasetDownloadServlet servlet = createServlet(); DatasetDownloadServlet servlet = createServlet();
...@@ -132,9 +137,9 @@ public class DatasetDownloadServletTest ...@@ -132,9 +137,9 @@ public class DatasetDownloadServletTest
assertEquals("<html><body>" + OSUtilities.LINE_SEPARATOR + "<h1>Data Set 1234-1</h1>" assertEquals("<html><body>" + OSUtilities.LINE_SEPARATOR + "<h1>Data Set 1234-1</h1>"
+ OSUtilities.LINE_SEPARATOR + OSUtilities.LINE_SEPARATOR
+ "<table border=\'0\' cellpadding=\'5\' cellspacing=\'0\'>" + "<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 + 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 + "</table></body></html>"
+ OSUtilities.LINE_SEPARATOR, writer.toString()); + OSUtilities.LINE_SEPARATOR, writer.toString());
assertEquals(LOG_INFO + "Data set '1234-1' obtained from openBIS server." assertEquals(LOG_INFO + "Data set '1234-1' obtained from openBIS server."
...@@ -151,7 +156,8 @@ public class DatasetDownloadServletTest ...@@ -151,7 +156,8 @@ public class DatasetDownloadServletTest
final StringWriter writer = new StringWriter(); final StringWriter writer = new StringWriter();
final ExternalData externalData = createExternalData(); final ExternalData externalData = createExternalData();
externalData.setLocatorType(new LocatorType("unknown")); externalData.setLocatorType(new LocatorType("unknown"));
prepareForObtainingDataSetFromServer(externalData, false); prepareForObtainingDataSetFromServer(externalData);
prepareForGettingDataSetFromSession(externalData, "blabla");
context.checking(new Expectations() context.checking(new Expectations()
{ {
{ {
...@@ -173,6 +179,45 @@ public class DatasetDownloadServletTest ...@@ -173,6 +179,45 @@ public class DatasetDownloadServletTest
context.assertIsSatisfied(); 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 @Test
public void testDoGetSubFolder() throws Exception public void testDoGetSubFolder() throws Exception
{ {
...@@ -188,7 +233,7 @@ public class DatasetDownloadServletTest ...@@ -188,7 +233,7 @@ public class DatasetDownloadServletTest
+ OSUtilities.LINE_SEPARATOR + "Folder: sub" + OSUtilities.LINE_SEPARATOR + "Folder: sub"
+ OSUtilities.LINE_SEPARATOR + OSUtilities.LINE_SEPARATOR
+ "<table border=\'0\' cellpadding=\'5\' cellspacing=\'0\'>" + "<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 + "</table></body></html>"
+ OSUtilities.LINE_SEPARATOR, writer.toString()); + OSUtilities.LINE_SEPARATOR, writer.toString());
assertEquals(LOG_INFO + "For data set '1234-1' show directory <wd>/data-set-123/sub", assertEquals(LOG_INFO + "For data set '1234-1' show directory <wd>/data-set-123/sub",
...@@ -274,6 +319,9 @@ public class DatasetDownloadServletTest ...@@ -274,6 +319,9 @@ public class DatasetDownloadServletTest
context.checking(new Expectations() context.checking(new Expectations()
{ {
{ {
one(request).getPathInfo();
will(returnValue(EXAMPLE_DATA_SET_CODE));
one(request).getSession(false); one(request).getSession(false);
will(returnValue(null)); will(returnValue(null));
...@@ -284,12 +332,67 @@ public class DatasetDownloadServletTest ...@@ -284,12 +332,67 @@ public class DatasetDownloadServletTest
DatasetDownloadServlet servlet = createServlet(); DatasetDownloadServlet servlet = createServlet();
servlet.doGet(request, response); servlet.doGet(request, response);
assertEquals("<html><body>Download session expired.</body></html>", writer.toString()); assertEquals(EXPIRATION_MESSAGE, writer.toString());
assertEquals("", getNormalizedLogContent()); assertEquals("", getNormalizedLogContent());
context.assertIsSatisfied(); 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, private void prepareForGettingDataSetFromSession(final ExternalData externalData,
final String path) final String path)
{ {
...@@ -300,16 +403,16 @@ public class DatasetDownloadServletTest ...@@ -300,16 +403,16 @@ public class DatasetDownloadServletTest
will(returnValue(httpSession)); will(returnValue(httpSession));
one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_KEY); one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_KEY);
will(returnValue(externalData)); Map<String, ExternalData> map = new HashMap<String, ExternalData>();
map.put(externalData.getCode(), externalData);
one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_ROOT_DIR_KEY); will(returnValue(map));
will(returnValue(EXAMPLE_DATA_SET_FOLDER));
one(request).getPathInfo(); one(request).getPathInfo();
will(returnValue(path)); String codeAndPath = externalData.getCode() + "/" + path;
will(returnValue(codeAndPath));
one(request).getRequestURI(); allowing(request).getRequestURI();
will(returnValue("download/" + (path == null ? "" : path))); will(returnValue("download/" + codeAndPath));
} }
}); });
...@@ -320,41 +423,32 @@ public class DatasetDownloadServletTest ...@@ -320,41 +423,32 @@ public class DatasetDownloadServletTest
context.checking(new Expectations() context.checking(new Expectations()
{ {
{ {
one(request).getParameter(DatasetDownloadServlet.DATASET_CODE_KEY);
will(returnValue(EXAMPLE_DATA_SET_CODE));
one(request).getParameter(DatasetDownloadServlet.SESSION_ID_KEY); one(request).getParameter(DatasetDownloadServlet.SESSION_ID_KEY);
will(returnValue(null)); will(returnValue(null));
} }
}); });
} }
private void prepareForObtainingDataSetFromServer(final ExternalData externalData, private void prepareForObtainingDataSetFromServer(final ExternalData externalData)
final boolean fileExists)
{ {
context.checking(new Expectations() context.checking(new Expectations()
{ {
{ {
one(request).getParameter(DatasetDownloadServlet.DATASET_CODE_KEY);
will(returnValue(EXAMPLE_DATA_SET_CODE));
one(request).getParameter(DatasetDownloadServlet.SESSION_ID_KEY); one(request).getParameter(DatasetDownloadServlet.SESSION_ID_KEY);
will(returnValue(EXAMPLE_SESSION_ID)); will(returnValue(EXAMPLE_SESSION_ID));
one(dataSetService).getDataSet(EXAMPLE_SESSION_ID, EXAMPLE_DATA_SET_CODE); one(dataSetService).getDataSet(EXAMPLE_SESSION_ID, EXAMPLE_DATA_SET_CODE);
will(returnValue(externalData)); will(returnValue(externalData));
if (fileExists) one(request).getSession(true);
{ will(returnValue(httpSession));
one(request).getSession(true);
will(returnValue(httpSession)); one(httpSession).setMaxInactiveInterval(120);
one(httpSession).getAttribute(DatasetDownloadServlet.DATA_SET_KEY);
one(httpSession).setMaxInactiveInterval(120); will(returnValue(null));
one(httpSession).setAttribute(DatasetDownloadServlet.DATA_SET_KEY,
externalData); one(httpSession).setAttribute(DatasetDownloadServlet.DATA_SET_KEY,
one(httpSession).setAttribute(DatasetDownloadServlet.DATA_SET_ROOT_DIR_KEY, new HashMap<String, ExternalData>());
EXAMPLE_DATA_SET_FOLDER);
}
} }
}); });
} }
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment