ParsingUtilities.evaluateJsonStringToObject is now unit tested

CommandTests and CancelProcessesCommandTests are slightly formatted to match code standards.
Command and CancelProcessesCommand are slightly tweaked to pass under all unit tests.

git-svn-id: http://google-refine.googlecode.com/svn/trunk@753 7d457c2a-affb-35e4-300a-418c747d4874

src/main/java/com/metaweb/gridworks/commands/Command.java

@@ -51,14 +51,19 @@ public abstract class Command {
      *
      * @param request
      * @return
-     * @throws Exception
+     * @throws JSONException
      */
     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);
+        try{
+            return (json == null) ? null : ParsingUtilities.evaluateJsonStringToObject(json);
+        } catch (JSONException e){
+            logger.debug( json + " could not be parsed to JSON");
+            return null;
+        }
     }
 
     /**

src/main/java/com/metaweb/gridworks/commands/util/CancelProcessesCommand.java

@@ -10,15 +10,17 @@ import com.metaweb.gridworks.commands.Command;
 import com.metaweb.gridworks.model.Project;
 
 public class CancelProcessesCommand extends Command {
-    
+
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
-        
+        if( request == null ) throw new IllegalArgumentException("parameter 'request' should not be null");
+        if( response == null ) throw new IllegalArgumentException("parameter 'request' should not be null");
+
         try {
             Project project = getProject(request);
             project.getProcessManager().cancelAll();
-            
+
             response.setCharacterEncoding("UTF-8");
             response.setHeader("Content-Type", "application/json");
             response.getWriter().write("{ \"code\" : \"ok\" }");

src/main/java/com/metaweb/gridworks/util/ParsingUtilities.java

@@ -22,21 +22,21 @@ import org.json.JSONTokener;
 public class ParsingUtilities {
 
     static final public SimpleDateFormat s_sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'");
-    
+
     static public Properties parseUrlParameters(HttpServletRequest request) {
         Properties options = new Properties();
-        
+
         String query = request.getQueryString();
         if (query != null) {
             if (query.startsWith("?")) {
                 query = query.substring(1);
             }
-    
+
             parseParameters(options,query);
         }
         return options;
     }
-    
+
     static public Properties parseParameters(Properties p, String str) {
         if (str != null) {
             String[] pairs = str.split("&");
@@ -49,11 +49,11 @@ public class ParsingUtilities {
         }
         return p;
     }
-    
+
     static public Properties parseParameters(String str) {
         return (str == null) ? null : parseParameters(new Properties(),str);
     }
-    
+
     static public String inputStreamToString(InputStream is) throws IOException {
         Reader reader = new InputStreamReader(is, "UTF-8");
         try {
@@ -65,18 +65,19 @@ public class ParsingUtilities {
 
     static public String readerToString(Reader reader) throws IOException {
         StringBuffer sb = new StringBuffer();
-        
+
         char[] chars = new char[8192];
         int c;
-        
+
         while ((c = reader.read(chars)) > 0) {
             sb.insert(sb.length(), chars, 0, c);
         }
-        
+
         return sb.toString();
     }
 
     static public JSONObject evaluateJsonStringToObject(String s) throws JSONException {
+        if( s == null ) throw new IllegalArgumentException("parameter 's' should not be null");
         JSONTokener t = new JSONTokener(s);
         Object o = t.nextValue();
         if (o instanceof JSONObject) {
@@ -85,7 +86,7 @@ public class ParsingUtilities {
             throw new JSONException(s + " couldn't be parsed as JSON object");
         }
     }
-    
+
     static public JSONArray evaluateJsonStringToArray(String s) throws JSONException {
         JSONTokener t = new JSONTokener(s);
         Object o = t.nextValue();
@@ -95,7 +96,7 @@ public class ParsingUtilities {
             throw new JSONException(s + " couldn't be parsed as JSON array");
         }
     }
-    
+
     private static final URLCodec codec = new URLCodec();
     static public String encode(String s) {
         try {
@@ -113,11 +114,11 @@ public class ParsingUtilities {
             return s; // should not happen
         }
     }
-    
+
     static public String dateToString(Date d) {
         return s_sdf.format(d);
     }
-    
+
     static public Date stringToDate(String s) {
         try {
             return s_sdf.parse(s);

tests/java/src/com/metaweb/gridworks/tests/commands/CommandTests.java

@@ -99,14 +99,13 @@ public class CommandTests {
     }
 
     @Test
-    public void getEngineConfigThrowsWithEmptyOrBadParameterValue() {
+    public void getEngineConfigReturnsNullWithEmptyOrBadParameterValue() {
         when(request.getParameter("engine")).thenReturn("sdfasdfas");
 
         try {
-            Assert.assertNull(SUT.wrapGetEngineConfig(request));
+            Assert.assertNull( SUT.wrapGetEngineConfig(request) );
+        } catch (JSONException e) {
             Assert.fail();
-        } catch (Exception e) {
-            // expected
         }
 
         verify(request, times(1)).getParameter("engine");

tests/java/src/com/metaweb/gridworks/tests/commands/util/CancelProcessesCommandTests.java

@@ -77,8 +77,10 @@ public class CancelProcessesCommandTests {
         try {
             SUT.doPost(null, null);
             Assert.fail(); // should have thrown exception by this point
+        } catch (IllegalArgumentException e){
+            //expected
         } catch (ServletException e) {
-            // expected
+            Assert.fail();
         } catch (Exception e) {
             Assert.fail();
         }
@@ -87,8 +89,10 @@ public class CancelProcessesCommandTests {
         try {
             SUT.doPost(null, response);
             Assert.fail(); // should have thrown exception by this point
+        } catch (IllegalArgumentException e){
+            //expected
         } catch (ServletException e) {
-            // expected
+            Assert.fail();
         } catch (Exception e) {
             Assert.fail();
         }
@@ -97,8 +101,10 @@ public class CancelProcessesCommandTests {
         try {
             SUT.doPost(request, null);
             Assert.fail(); // should have thrown exception by this point
-        } catch (ServletException e) {
+        } catch (IllegalArgumentException e){
             // expected
+        } catch (ServletException e) {
+            Assert.fail();
         } catch (Exception e) {
             Assert.fail();
         }
@@ -135,7 +141,8 @@ public class CancelProcessesCommandTests {
 
         verify(processMan, times(1)).cancelAll();
         verify(response, times(1)).setCharacterEncoding("UTF-8");
-        verify(response, times(1)).setHeader("Content-Type", "application/json");
+        verify(response, times(1))
+                .setHeader("Content-Type", "application/json");
         verify(proj, times(1)).getProcessManager();
         try {
             verify(response, times(1)).getWriter();
@@ -171,33 +178,32 @@ public class CancelProcessesCommandTests {
          String ERROR_MESSAGE = "hello world";
 
         // mock dependencies
-        when(request.getParameter("project")).thenReturn(PROJECT_ID);
+            when(request.getParameter("project")).thenReturn(PROJECT_ID);
-        when(projMan.getProject(anyLong())).thenReturn(proj);
+            when(projMan.getProject(anyLong())).thenReturn(proj);
-        when(proj.getProcessManager()).thenReturn(processMan);
+            when(proj.getProcessManager()).thenReturn(processMan);
-        try {
+            try {
-            when(response.getWriter())
+                when(response.getWriter()).thenThrow(new IllegalStateException(ERROR_MESSAGE))
-              .thenThrow(new IllegalStateException(ERROR_MESSAGE))
+                .thenReturn(pw);
-              .thenReturn(pw);
+            } catch (IOException e) {
-        } catch (IOException e) {
+                Assert.fail();
-            Assert.fail();
+            }
-        }
 
-        // run
+            // run
-        try {
+            try {
-            SUT.doPost(request, response);
+                SUT.doPost(request, response);
-        } catch (ServletException e) {
+            } catch (ServletException e) {
-            Assert.fail();
+                Assert.fail();
-        } catch (IOException e) {
+            } catch (IOException e) {
-            Assert.fail();
+                Assert.fail();
-        }
+            }
 
-        verify(request, times(1)).getParameter("project");
+            verify(request, times(1)).getParameter("project");
-        verify(projMan, times(1)).getProject(PROJECT_ID_LONG);
+            verify(projMan, times(1)).getProject(PROJECT_ID_LONG);
 
-        verify(processMan, times(1)).cancelAll();
+            verify(processMan, times(1)).cancelAll();
-        verify(response, times(3)).setCharacterEncoding("UTF-8");
+            verify(response, times(3)).setCharacterEncoding("UTF-8");
-        //omitted other verifications for brevity.
+            //omitted other verifications for brevity.
-        //assumption is that expecting response.setCharacterEncoding times(3)
+            //assumption is that expecting response.setCharacterEncoding times(3)
-        //implies it has Command.respondException has been called as expected
+            //implies it has Command.respondException has been called as expected
      }
 }

tests/java/src/com/metaweb/gridworks/tests/util/ParsingUtilitiesTests.java

@@ -0,0 +1,48 @@
+package com.metaweb.gridworks.tests.util;
+
+import org.json.JSONException;
+import org.json.JSONObject;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.metaweb.gridworks.util.ParsingUtilities;
+
+public class ParsingUtilitiesTests {
+    final static protected Logger logger = LoggerFactory.getLogger("ParsingUtilitiesTests");
+
+    //--------------evaluateJsonStringToObject tests-----------------------
+    @Test
+    public void evaluateJsonStringToObjectRegressionTest(){
+        try {
+            JSONObject o = ParsingUtilities.evaluateJsonStringToObject("{\"foo\":\"bar\"}");
+            Assert.assertNotNull(o);
+            Assert.assertEquals("bar", o.getString("foo"));
+        } catch (JSONException e) {
+            Assert.fail();
+        }
+    }
+
+    @Test
+    public void evaluateJsonStringToObjectWithNullParameters(){
+        try {
+            Assert.assertNull(ParsingUtilities.evaluateJsonStringToObject(null));
+            Assert.fail();
+        } catch (IllegalArgumentException e){
+            //expected
+        } catch (JSONException e) {
+            Assert.fail();
+        }
+    }
+
+    @Test
+    public void evaluateJsonStringToObjectWithMalformedParameters(){
+        try {
+            ParsingUtilities.evaluateJsonStringToObject("malformed");
+            Assert.fail();
+        } catch (JSONException e) {
+            //expected
+        }
+    }
+}