diff --git a/build.xml b/build.xml index a5f6fa897..a7295d50a 100644 --- a/build.xml +++ b/build.xml @@ -95,6 +95,7 @@ + diff --git a/gridworks b/gridworks index 4f09bf2fb..6624e0c7a 100755 --- a/gridworks +++ b/gridworks @@ -526,7 +526,7 @@ server_test() { ant build_tests echo "" - CLASSPATH="$GRIDWORKS_BUILD_DIR/classes${SEP}$GRIDWORKS_WEBAPP/WEB-INF/classes${SEP}$GRIDWORKS_TEST_DIR/java/classes${SEP}$GRIDWORKS_TEST_DIR/java/lib/*${SEP}$GRIDWORKS_LIB_DIR/*${SEP}$GRIDWORKS_WEBAPP/WEB-INF/lib/*" + CLASSPATH="$GRIDWORKS_TEST_DIR/java/classes${SEP}$GRIDWORKS_WEBAPP/WEB-INF/classes${SEP}$GRIDWORKS_BUILD_DIR/classes${SEP}$GRIDWORKS_TEST_DIR/java/lib/*${SEP}$GRIDWORKS_LIB_DIR/*${SEP}$GRIDWORKS_WEBAPP/WEB-INF/lib/*" if [ -z "$1" ]; then cd "$GRIDWORKS_TEST_DIR/java/classes" diff --git a/src/main/java/com/metaweb/gridworks/commands/Command.java b/src/main/java/com/metaweb/gridworks/commands/Command.java index 8fca5b886..8c58d3647 100644 --- a/src/main/java/com/metaweb/gridworks/commands/Command.java +++ b/src/main/java/com/metaweb/gridworks/commands/Command.java @@ -30,62 +30,68 @@ import com.metaweb.gridworks.util.ParsingUtilities; * are AJAX calls. */ public abstract class Command { - + final static protected Logger logger = LoggerFactory.getLogger("command"); - - public void doPost(HttpServletRequest request, HttpServletResponse response) + + public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - + throw new NotImplementedException(); }; - - public void doGet(HttpServletRequest request, HttpServletResponse response) + + public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - + throw new NotImplementedException(); }; - + /** * Utility function to get the browsing engine's configuration as a JSON object * from the "engine" request parameter, most often in the POST body. - * + * * @param request * @return * @throws Exception */ - static protected JSONObject getEngineConfig(HttpServletRequest request) throws Exception { + static protected JSONObject getEngineConfig(HttpServletRequest request) + throws JSONException { + if (request == null) throw new IllegalArgumentException("parameter 'request' should not be null"); + String json = request.getParameter("engine"); return (json == null) ? null : ParsingUtilities.evaluateJsonStringToObject(json); } - + /** - * Utility function to reconstruct the browsing engine from the "engine" request parameter, + * Utility function to reconstruct the browsing engine from the "engine" request parameter, * most often in the POST body. - * + * * @param request * @param project * @return * @throws Exception */ - static protected Engine getEngine(HttpServletRequest request, Project project) throws Exception { + static protected Engine getEngine(HttpServletRequest request, Project project) + throws Exception { + if (request == null) throw new IllegalArgumentException("parameter 'request' should not be null"); + if (project == null) throw new IllegalArgumentException("parameter 'project' should not be null"); + Engine engine = new Engine(project); - String json = request.getParameter("engine"); - if (json != null) { - JSONObject o = ParsingUtilities.evaluateJsonStringToObject(json); + JSONObject o = getEngineConfig(request); + if (o != null) engine.initializeFromJSON(o); - } return engine; } - + /** * Utility method for retrieving the Project object having the ID specified * in the "project" URL parameter. - * + * * @param request * @return * @throws ServletException */ - static protected Project getProject(HttpServletRequest request) throws ServletException { + protected Project getProject(HttpServletRequest request) throws ServletException { + if (request == null) throw new IllegalArgumentException("parameter 'request' should not be null"); try { Project p = ProjectManager.singleton.getProject(Long.parseLong(request.getParameter("project"))); if (p != null) { @@ -94,10 +100,11 @@ public abstract class Command { } catch (Exception e) { // ignore } - throw new ServletException("Missing or bad project URL parameter"); + throw new ServletException("Can't find project: missing or bad URL parameter"); } - + static protected int getIntegerParameter(HttpServletRequest request, String name, int def) { + if (request == null) throw new IllegalArgumentException("parameter 'request' should not be null"); try { return Integer.parseInt(request.getParameter(name)); } catch (Exception e) { @@ -105,8 +112,9 @@ public abstract class Command { } return def; } - + static protected JSONObject getJsonParameter(HttpServletRequest request, String name) { + if (request == null) throw new IllegalArgumentException("parameter 'request' should not be null"); String value = request.getParameter(name); if (value != null) { try { @@ -117,37 +125,37 @@ public abstract class Command { } return null; } - + static protected void performProcessAndRespond( - HttpServletRequest request, + HttpServletRequest request, HttpServletResponse response, Project project, Process process ) throws Exception { response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); - + HistoryEntry historyEntry = project.processManager.queueProcess(process); if (historyEntry != null) { Writer w = response.getWriter(); JSONWriter writer = new JSONWriter(w); Properties options = new Properties(); - + writer.object(); writer.key("code"); writer.value("ok"); writer.key("historyEntry"); historyEntry.write(writer, options); writer.endObject(); - + w.flush(); w.close(); } else { respond(response, "{ \"code\" : \"pending\" }"); } } - - static protected void respond(HttpServletResponse response, String content) + + static protected void respond(HttpServletResponse response, String content) throws IOException, ServletException { - + response.setCharacterEncoding("UTF-8"); response.setStatus(HttpServletResponse.SC_OK); Writer w = response.getWriter(); @@ -159,10 +167,10 @@ public abstract class Command { throw new ServletException("response returned a null writer"); } } - - static protected void respond(HttpServletResponse response, String status, String message) + + static protected void respond(HttpServletResponse response, String status, String message) throws IOException, JSONException { - + Writer w = response.getWriter(); JSONWriter writer = new JSONWriter(w); writer.object(); @@ -172,31 +180,31 @@ public abstract class Command { w.flush(); w.close(); } - - static protected void respondJSON(HttpServletResponse response, Jsonizable o) + + static protected void respondJSON(HttpServletResponse response, Jsonizable o) throws IOException, JSONException { - + respondJSON(response, o, new Properties()); } - + static protected void respondJSON( - HttpServletResponse response, Jsonizable o, Properties options) + HttpServletResponse response, Jsonizable o, Properties options) throws IOException, JSONException { - + response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); - + Writer w = response.getWriter(); JSONWriter writer = new JSONWriter(w); - + o.write(writer, options); w.flush(); w.close(); } - - static protected void respondException(HttpServletResponse response, Exception e) + + static protected void respondException(HttpServletResponse response, Exception e) throws IOException, ServletException { - + logger.warn("Exception caught", e); if (response == null) { @@ -207,15 +215,15 @@ public abstract class Command { JSONObject o = new JSONObject(); o.put("code", "error"); o.put("message", e.getMessage()); - + StringWriter sw = new StringWriter(); PrintWriter pw = new PrintWriter(sw); e.printStackTrace(pw); pw.flush(); sw.flush(); - + o.put("stack", sw.toString()); - + response.setCharacterEncoding("UTF-8"); response.setHeader("Content-Type", "application/json"); respond(response, o.toString()); @@ -223,9 +231,9 @@ public abstract class Command { e.printStackTrace(response.getWriter()); } } - + static protected void redirect(HttpServletResponse response, String url) throws IOException { response.sendRedirect(url); } - + } diff --git a/src/main/java/com/metaweb/gridworks/util/ParsingUtilities.java b/src/main/java/com/metaweb/gridworks/util/ParsingUtilities.java index 5891d4b16..3c60e0db3 100644 --- a/src/main/java/com/metaweb/gridworks/util/ParsingUtilities.java +++ b/src/main/java/com/metaweb/gridworks/util/ParsingUtilities.java @@ -78,14 +78,22 @@ public class ParsingUtilities { static public JSONObject evaluateJsonStringToObject(String s) throws JSONException { JSONTokener t = new JSONTokener(s); - JSONObject o = (JSONObject) t.nextValue(); - return o; + Object o = t.nextValue(); + if (o instanceof JSONObject) { + return (JSONObject) o; + } else { + throw new JSONException(s + " couldn't be parsed as JSON object"); + } } static public JSONArray evaluateJsonStringToArray(String s) throws JSONException { JSONTokener t = new JSONTokener(s); - JSONArray a = (JSONArray) t.nextValue(); - return a; + Object o = t.nextValue(); + if (o instanceof JSONArray) { + return (JSONArray) o; + } else { + throw new JSONException(s + " couldn't be parsed as JSON array"); + } } private static final URLCodec codec = new URLCodec(); diff --git a/tests/java/src/com/metaweb/gridworks/tests/GridworksTests.java b/tests/java/src/com/metaweb/gridworks/tests/GridworksTests.java deleted file mode 100644 index 1d9a1fe9b..000000000 --- a/tests/java/src/com/metaweb/gridworks/tests/GridworksTests.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.metaweb.gridworks.tests; - -import static org.junit.Assert.assertSame; - -import org.junit.Test; - -public class GridworksTests { - - // this is just a placeholder for now - - @Test public void test() { - assertSame("Just a placeholder for now", Integer.parseInt("1"), 1); - } -} \ No newline at end of file diff --git a/tests/java/src/com/metaweb/gridworks/tests/commands/CommandStub.java b/tests/java/src/com/metaweb/gridworks/tests/commands/CommandStub.java new file mode 100644 index 000000000..ef6a93fbe --- /dev/null +++ b/tests/java/src/com/metaweb/gridworks/tests/commands/CommandStub.java @@ -0,0 +1,42 @@ +package com.metaweb.gridworks.tests.commands; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; + +import org.json.JSONException; +import org.json.JSONObject; +import org.junit.Ignore; + +import com.metaweb.gridworks.browsing.Engine; +import com.metaweb.gridworks.commands.Command; +import com.metaweb.gridworks.model.Project; + +/** + * Implementation of abstract class for testing Exposes protected members as public + */ +@Ignore +public class CommandStub extends Command { + + public Project wrapGetProject(HttpServletRequest request) + throws ServletException { + return getProject(request); + } + + public JSONObject wrapGetEngineConfig(HttpServletRequest request) + throws JSONException { + return getEngineConfig(request); + } + + public Engine wrapGetEngine(HttpServletRequest request, Project project) + throws Exception { + return getEngine(request, project); + } + + public int wrapGetIntegerParameter(HttpServletRequest request, String name,int def) { + return getIntegerParameter(request, name, def); + } + + public JSONObject wrapGetJsonParameter(HttpServletRequest request,String name) { + return getJsonParameter(request, name); + } +} diff --git a/tests/java/src/com/metaweb/gridworks/tests/commands/CommandTests.java b/tests/java/src/com/metaweb/gridworks/tests/commands/CommandTests.java new file mode 100644 index 000000000..cf85ef2f6 --- /dev/null +++ b/tests/java/src/com/metaweb/gridworks/tests/commands/CommandTests.java @@ -0,0 +1,315 @@ +package com.metaweb.gridworks.tests.commands; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; + +import org.json.JSONException; +import org.json.JSONObject; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.metaweb.gridworks.ProjectManager; +import com.metaweb.gridworks.browsing.Engine; +import com.metaweb.gridworks.model.Project; + +public class CommandTests { + + final static protected Logger logger = LoggerFactory.getLogger("CommandTests"); + + CommandStub SUT = null; + HttpServletRequest request = null; + ProjectManager projectManager = null; + Project project = null; + + @Before + public void SetUp() { + SUT = new CommandStub(); + request = mock(HttpServletRequest.class); + projectManager = mock(ProjectManager.class); + project = mock(Project.class); + } + + @After + public void TearDown() { + SUT = null; + request = null; + projectManager = null; + project = null; + } + + // -----------------getProject tests------------ + + @Test + public void getProjectThrowsWithNullParameter() { + try { + SUT.wrapGetProject(null); + Assert.fail(); // should throw exception before this + } catch (IllegalArgumentException e) { + // expected + } catch (Exception e) { + Assert.fail(); + } + } + + @Test + public void getProjectThrowsIfResponseHasNoOrBrokenProjectParameter() { + when(request.getParameter("project")).thenReturn(""); // null + try { + SUT.wrapGetProject(request); + } catch (ServletException e) { + // expected + } catch (Exception e) { + Assert.fail(); + } + verify(request, times(1)).getParameter("project"); + } + + // -----------------getEngineConfig tests----------------- + @Test + public void getEngineConfigThrowsWithNullParameter() { + try { + SUT.wrapGetEngineConfig(null); + Assert.fail(); + } catch (IllegalArgumentException e) { + // expected + } catch (Exception e) { + Assert.fail(); + } + } + + @Test + public void getEngineConfigReturnsNullWithNullEngineParameter() { + when(request.getParameter("engine")).thenReturn(null); + try { + Assert.assertNull(SUT.wrapGetEngineConfig(request)); + } catch (JSONException e) { + Assert.fail(); + } catch (Exception e) { + Assert.fail(); + } + } + + @Test + public void getEngineConfigThrowsWithEmptyOrBadParameterValue() { + when(request.getParameter("engine")).thenReturn("sdfasdfas"); + + try { + Assert.assertNull(SUT.wrapGetEngineConfig(request)); + Assert.fail(); + } catch (Exception e) { + // expected + } + + verify(request, times(1)).getParameter("engine"); + } + + @Test + public void getEngineConfigRegressionTest() { + when(request.getParameter("engine")).thenReturn("{\"hello\":\"world\"}"); + JSONObject o = null; + try { + o = SUT.wrapGetEngineConfig(request); + Assert.assertEquals("world", o.getString("hello")); + } catch (JSONException e) { + Assert.fail(); + } catch (Exception e) { + Assert.fail(); + } + verify(request, times(1)).getParameter("engine"); + } + + // -----------------getEngine tests---------------------- + @Test + public void getEngineThrowsOnNullParameter() { + try { + SUT.wrapGetEngine(null, null); + } catch (IllegalArgumentException e) { + // expected + } catch (Exception e) { + Assert.fail(); + } + + try { + SUT.wrapGetEngine(null, project); + } catch (IllegalArgumentException e) { + // expected + } catch (Exception e) { + Assert.fail(); + } + + try { + SUT.wrapGetEngine(request, null); + } catch (IllegalArgumentException e) { + // expected + } catch (Exception e) { + Assert.fail(); + } + } + + @Test + public void getEngineRegressionTest() { + // TODO refactor getEngine to use dependency injection, so a mock Engine + // object can be used. + + Engine engine = null; + when(request.getParameter("engine")).thenReturn("{\"hello\":\"world\"}"); + + try { + engine = SUT.wrapGetEngine(request, project); + Assert.assertNotNull(engine); + } catch (Exception e) { + Assert.fail(); + } + + verify(request, times(1)).getParameter("engine"); + // JSON configuration doesn't have 'facets' key or 'INCLUDE_DEPENDENT' + // key, so there should be no further action + // Engine._facets is protected so can't test that it is of zero length. + } + + // ------------------ + @Test + public void getIntegerParameterWithNullParameters() { + // all null + try { + SUT.wrapGetIntegerParameter(null, null, 0); + Assert.fail(); + } catch (IllegalArgumentException e) { + // expected + } + + // request null + try { + SUT.wrapGetIntegerParameter(null, "name", 0); + Assert.fail(); + } catch (IllegalArgumentException e) { + // expected + } + } + + @Test + public void getIntegerParametersWithIncorrectParameterName() { + + when(request.getParameter(null)).thenReturn(null); + when(request.getParameter("incorrect")).thenReturn(null); + + // name null + try { + int returned = SUT.wrapGetIntegerParameter(request, null, 5); + Assert.assertEquals(5, returned); + } catch (IllegalArgumentException e) { + Assert.fail(); + } + + // name incorrect + try { + int returned = SUT.wrapGetIntegerParameter(request, "incorrect", 5); + Assert.assertEquals(5, returned); + } catch (IllegalArgumentException e) { + Assert.fail(); + } + + verify(request, times(1)).getParameter(null); + verify(request, times(1)).getParameter("incorrect"); + } + + @Test + public void getIntegerParametersRegressionTest() { + when(request.getParameter("positivenumber")).thenReturn("22"); + when(request.getParameter("zeronumber")).thenReturn("0"); + when(request.getParameter("negativenumber")).thenReturn("-40"); + + // positive + try { + int returned = SUT.wrapGetIntegerParameter(request,"positivenumber", 5); + Assert.assertEquals(22, returned); + } catch (IllegalArgumentException e) { + Assert.fail(); + } + + // zero + try { + int returned = SUT.wrapGetIntegerParameter(request, "zeronumber", 5); + Assert.assertEquals(0, returned); + } catch (IllegalArgumentException e) { + Assert.fail(); + } + + // negative + try { + int returned = SUT.wrapGetIntegerParameter(request, + "negativenumber", 5); + Assert.assertEquals(-40, returned); + } catch (IllegalArgumentException e) { + Assert.fail(); + } + + verify(request, times(1)).getParameter("positivenumber"); + verify(request, times(1)).getParameter("zeronumber"); + verify(request, times(1)).getParameter("negativenumber"); + } + + // ---------------------getJsonParameter tests---------------- + @Test + public void getJsonParameterWithNullParameters() { + when(request.getParameter(null)).thenReturn(null); + when(request.getParameter("")).thenReturn(null); + + try { + SUT.wrapGetJsonParameter(null, null); + Assert.fail(); + } catch (IllegalArgumentException e) { + // expected + } + + Assert.assertNull(SUT.wrapGetJsonParameter(request, null)); + + try { + SUT.wrapGetJsonParameter(null, "test"); + } catch (IllegalArgumentException e) { + // expected + } + + Assert.assertNull(SUT.wrapGetJsonParameter(request, "")); + + verify(request, times(1)).getParameter(null); + verify(request, times(1)).getParameter(""); + } + + @Test + public void getJsonParameterRegressionTest() { + when(request.getParameter("test")).thenReturn("{\"foo\":\"bar\"}"); + + JSONObject o = SUT.wrapGetJsonParameter(request, "test"); + Assert.assertNotNull(o); + try { + Assert.assertEquals("bar", o.getString("foo")); + } catch (JSONException e) { + Assert.fail(); + } + + verify(request, times(1)).getParameter("test"); + } + + @Test + public void getJsonParameterWithMalformedJson() { + when(request.getParameter("test")).thenReturn("brokenJSON"); + + try { + Assert.assertNull(SUT.wrapGetJsonParameter(request, "test")); + } catch (Exception e) { + Assert.fail(); + } + + verify(request, times(1)).getParameter("test"); + } +} diff --git a/tests/java/src/com/metaweb/gridworks/tests/commands/util/CancelProcessesCommandTests.java b/tests/java/src/com/metaweb/gridworks/tests/commands/util/CancelProcessesCommandTests.java index d55647e9e..8bfc2593a 100644 --- a/tests/java/src/com/metaweb/gridworks/tests/commands/util/CancelProcessesCommandTests.java +++ b/tests/java/src/com/metaweb/gridworks/tests/commands/util/CancelProcessesCommandTests.java @@ -41,15 +41,17 @@ public class CancelProcessesCommandTests { HttpServletRequest request = null; HttpServletResponse response = null; ProjectManager projMan = null; - Project p = null; + Project proj = null; ProcessManager processMan = null; + PrintWriter pw = null; @Before public void SetUp() { projMan = mock(ProjectManager.class); ProjectManager.singleton = projMan; - p = mock(Project.class); + proj = mock(Project.class); processMan = mock(ProcessManager.class); + pw = mock(PrintWriter.class); request = mock(HttpServletRequest.class); response = mock(HttpServletResponse.class); @@ -62,7 +64,8 @@ public class CancelProcessesCommandTests { projMan = null; ProjectManager.singleton = null; - p = null; + proj = null; + pw = null; request = null; response = null; } @@ -79,7 +82,7 @@ public class CancelProcessesCommandTests { } catch (Exception e) { Assert.fail(); } - + // request is null try { SUT.doPost(null, response); @@ -101,15 +104,16 @@ public class CancelProcessesCommandTests { } } - // runs through a complete working post + /** + * Contract for a complete working post + */ @Test - public void doPost() { + public void doPostRegressionTest() { // mock dependencies when(request.getParameter("project")).thenReturn(PROJECT_ID); - when(projMan.getProject(anyLong())).thenReturn(p); - when(p.getProcessManager()).thenReturn(processMan); - PrintWriter pw = mock(PrintWriter.class); + when(projMan.getProject(anyLong())).thenReturn(proj); + when(proj.getProcessManager()).thenReturn(processMan); try { when(response.getWriter()).thenReturn(pw); } catch (IOException e1) { @@ -128,17 +132,72 @@ public class CancelProcessesCommandTests { // verify verify(request, times(1)).getParameter("project"); verify(projMan, times(1)).getProject(PROJECT_ID_LONG); - verify(p, times(1)).getProcessManager(); + + verify(processMan, times(1)).cancelAll(); + verify(response, times(1)).setCharacterEncoding("UTF-8"); + verify(response, times(1)).setHeader("Content-Type", "application/json"); + verify(proj, times(1)).getProcessManager(); try { verify(response, times(1)).getWriter(); } catch (IOException e) { Assert.fail(); } - - verify(processMan, times(1)).cancelAll(); - verify(response, times(1)).setCharacterEncoding("UTF-8"); - verify(response, times(1)) - .setHeader("Content-Type", "application/json"); verify(pw, times(1)).write("{ \"code\" : \"ok\" }"); } + + @Test + public void doPostThrowsIfCommand_getProjectReturnsNull(){ + // mock dependencies + when(request.getParameter("project")).thenReturn(PROJECT_ID); + when(projMan.getProject(anyLong())) + .thenReturn(null); + + // run + try { + SUT.doPost(request, response); + } catch (ServletException e) { + //expected + } catch (IOException e) { + Assert.fail(); + } + + // verify + verify(request, times(1)).getParameter("project"); + verify(projMan, times(1)).getProject(PROJECT_ID_LONG); + } + + @Test + public void doPostCatchesExceptionFromWriter(){ + String ERROR_MESSAGE = "hello world"; + + // mock dependencies + when(request.getParameter("project")).thenReturn(PROJECT_ID); + when(projMan.getProject(anyLong())).thenReturn(proj); + when(proj.getProcessManager()).thenReturn(processMan); + try { + when(response.getWriter()) + .thenThrow(new IllegalStateException(ERROR_MESSAGE)) + .thenReturn(pw); + } catch (IOException e) { + Assert.fail(); + } + + // run + try { + SUT.doPost(request, response); + } catch (ServletException e) { + Assert.fail(); + } catch (IOException e) { + Assert.fail(); + } + + verify(request, times(1)).getParameter("project"); + verify(projMan, times(1)).getProject(PROJECT_ID_LONG); + + verify(processMan, times(1)).cancelAll(); + verify(response, times(3)).setCharacterEncoding("UTF-8"); + //omitted other verifications for brevity. + //assumption is that expecting response.setCharacterEncoding times(3) + //implies it has Command.respondException has been called as expected + } } diff --git a/tests/java/src/log4j.properties b/tests/java/src/log4j.properties new file mode 100644 index 000000000..7ccee1634 --- /dev/null +++ b/tests/java/src/log4j.properties @@ -0,0 +1,4 @@ +log4j.rootLogger=ERROR, console + +log4j.appender.console=org.apache.log4j.ConsoleAppender +log4j.appender.console.layout=com.metaweb.util.logging.IndentingLayout