Refactor GetRowsCommand for Jackson migration

main/src/com/google/refine/commands/row/GetRowsCommand.java

@@ -35,6 +35,8 @@ package com.google.refine.commands.row;
 
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Properties;
 
 import javax.servlet.ServletException;
@@ -45,6 +47,12 @@ import org.json.JSONException;
 import org.json.JSONObject;
 import org.json.JSONWriter;
 
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonInclude.Include;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonUnwrapped;
+
+import com.google.refine.Jsonizable;
 import com.google.refine.browsing.Engine;
 import com.google.refine.browsing.Engine.Mode;
 import com.google.refine.browsing.FilteredRecords;
@@ -65,6 +73,82 @@ import com.google.refine.util.Pool;
 
 public class GetRowsCommand extends Command {
     
+    protected static class WrappedRow implements Jsonizable {
+        @JsonUnwrapped
+        protected final Row row;
+        @JsonProperty("rowIndex")
+        protected final int rowIndex;
+        @JsonProperty("recordIndex")
+        @JsonInclude(Include.NON_NULL)
+        protected final Integer recordIndex;
+        
+        protected WrappedRow(Row rowOrRecord, int rowIndex, Integer recordIndex) {
+            this.row = rowOrRecord;
+            this.rowIndex = rowIndex;
+            this.recordIndex = recordIndex;
+        }
+
+        @Override
+        public void write(JSONWriter writer, Properties options)
+                throws JSONException {
+            options.put("rowIndex", rowIndex);
+            if(recordIndex != null) {
+                options.put("recordIndex", recordIndex);
+            }
+            row.write(writer, options);  
+            if(recordIndex != null) {
+                options.remove("recordIndex");
+            }
+        }
+        
+    }
+    
+    protected static class JsonResult implements Jsonizable {
+        @JsonProperty("mode")
+        protected final Mode mode;
+        @JsonProperty("rows")
+        protected final List<WrappedRow> rows;
+        @JsonProperty("filtered")
+        protected final int filtered;
+        @JsonProperty("total")
+        protected final int totalCount;
+        @JsonProperty("start")
+        protected final int start;
+        @JsonProperty("limit")
+        protected final int limit;
+        @JsonProperty("pool")
+        protected final Pool pool;
+        
+        protected JsonResult(Mode mode, List<WrappedRow> rows, int filtered,
+                int totalCount, int start, int limit, Pool pool) {
+            this.mode = mode;
+            this.rows = rows;
+            this.filtered = filtered;
+            this.totalCount = totalCount;
+            this.start = start;
+            this.limit = limit;
+            this.pool = pool;
+        }
+
+        @Override
+        public void write(JSONWriter jsonWriter, Properties options)
+                throws JSONException {
+            jsonWriter.object();
+            jsonWriter.key("mode"); jsonWriter.value(mode == Mode.RowBased ? "row-based" : "record-based");
+            jsonWriter.key("rows"); jsonWriter.array();
+            for(WrappedRow row : rows) {
+                row.write(jsonWriter, options);
+            }
+            jsonWriter.endArray();
+            jsonWriter.key("filtered"); jsonWriter.value(filtered);
+            jsonWriter.key("total"); jsonWriter.value(totalCount);
+            jsonWriter.key("start"); jsonWriter.value(start);
+            jsonWriter.key("limit"); jsonWriter.value(limit);
+            jsonWriter.key("pool"); pool.write(jsonWriter, options);
+            jsonWriter.endObject();
+        }
+    }
+    
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response)
             throws ServletException, IOException {
@@ -117,9 +201,8 @@ public class GetRowsCommand extends Command {
             }
             
             JSONWriter jsonWriter = new JSONWriter(writer);
-            jsonWriter.object();
             
-            RowWritingVisitor rwv = new RowWritingVisitor(start, limit, jsonWriter, options);
+            RowWritingVisitor rwv = new RowWritingVisitor(start, limit);
             
             SortingConfig sortingConfig = null;
             try{
@@ -131,7 +214,7 @@ public class GetRowsCommand extends Command {
                 }
             } catch (JSONException e) {
             }
-
+            
             if (engine.getMode() == Mode.RowBased) {
                 FilteredRows filteredRows = engine.getAllFilteredRows();
                 RowVisitor visitor = rwv;
@@ -144,13 +227,7 @@ public class GetRowsCommand extends Command {
                         visitor = srv;
                     }
                 }
-                
-                jsonWriter.key("mode"); jsonWriter.value("row-based");
-                jsonWriter.key("rows"); jsonWriter.array();
                 filteredRows.accept(project, visitor);
-                jsonWriter.endArray();
-                jsonWriter.key("filtered"); jsonWriter.value(rwv.total);
-                jsonWriter.key("total"); jsonWriter.value(project.rows.size());
             } else {
                 FilteredRecords filteredRecords = engine.getFilteredRecords();
                 RecordVisitor visitor = rwv;
@@ -163,22 +240,15 @@ public class GetRowsCommand extends Command {
                         visitor = srv;
                     }
                 }
-                
-                jsonWriter.key("mode"); jsonWriter.value("record-based");
-                jsonWriter.key("rows"); jsonWriter.array();
                 filteredRecords.accept(project, visitor);
-                jsonWriter.endArray();
-                jsonWriter.key("filtered"); jsonWriter.value(rwv.total);
-                jsonWriter.key("total"); jsonWriter.value(project.recordModel.getRecordCount());
             }
             
             
-            jsonWriter.key("start"); jsonWriter.value(start);
+            JsonResult result = new JsonResult(engine.getMode(),
-            jsonWriter.key("limit"); jsonWriter.value(limit);
+                    rwv.results, rwv.total,
-            jsonWriter.key("pool"); pool.write(jsonWriter, options);
+                    engine.getMode() == Mode.RowBased ? project.rows.size() : project.recordModel.getRecordCount(),
-            
+                            start, limit, pool);
-            jsonWriter.endObject();
+            result.write(jsonWriter, options);
-            
             if (callback != null) {
                 writer.write(")");
             }
@@ -195,16 +265,14 @@ public class GetRowsCommand extends Command {
     static protected class RowWritingVisitor implements RowVisitor, RecordVisitor {
         final int           start;
         final int           limit;
-        final JSONWriter  writer;
+        public List<WrappedRow> results;
-        final Properties  options;
         
         public int total;
         
-        public RowWritingVisitor(int start, int limit, JSONWriter writer, Properties options) {
+        public RowWritingVisitor(int start, int limit) {
             this.start = start;
             this.limit = limit;
-            this.writer = writer;
+            this.results = new ArrayList<>();
-            this.options = options;
         }
         
         @Override
@@ -238,29 +306,14 @@ public class GetRowsCommand extends Command {
         }
         
         public boolean internalVisit(Project project, int rowIndex, Row row) {
-            try {
+            results.add(new WrappedRow(row, rowIndex, null));
-                options.put("rowIndex", rowIndex);
-                row.write(writer, options);
-            } catch (JSONException e) {
-            }
             return false;
         }
         
         protected boolean internalVisit(Project project, Record record) {
-            options.put("recordIndex", record.recordIndex);
-            
             for (int r = record.fromRowIndex; r < record.toRowIndex; r++) {
-                try {
+                Row row = project.rows.get(r);
-                    Row row = project.rows.get(r);
+                results.add(new WrappedRow(row, r, r == record.fromRowIndex ? record.recordIndex : null));
-                    
-                    options.put("rowIndex", r);
-                    
-                    row.write(writer, options);
-                    
-                } catch (JSONException e) {
-                }
-                
-                options.remove("recordIndex");
             }
             return false;
         }

main/tests/server/src/com/google/refine/tests/commands/row/GetRowsCommandTest.java

@@ -0,0 +1,116 @@
+package com.google.refine.tests.commands.row;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.refine.commands.Command;
+import com.google.refine.commands.row.GetRowsCommand;
+import com.google.refine.model.Project;
+import com.google.refine.tests.RefineTest;
+import com.google.refine.tests.util.TestUtils;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
+public class GetRowsCommandTest extends RefineTest {
+    
+    HttpServletRequest request = null;
+    HttpServletResponse response = null;
+    Command command = null;
+    Project project = null;
+    StringWriter writer = null;
+    
+    @BeforeMethod
+    public void setUp() {
+        request = mock(HttpServletRequest.class);
+        response = mock(HttpServletResponse.class);
+        project = createCSVProject("a,b\nc,d\n,f");
+        command = new GetRowsCommand();
+        writer = new StringWriter();
+        when(request.getParameter("project")).thenReturn(String.valueOf(project.id));
+        try {
+            when(response.getWriter()).thenReturn(new PrintWriter(writer));
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+    }
+    
+    @Test
+    public void testJsonOutputRows() throws ServletException, IOException {
+        String rowJson = "{\n" + 
+                "       \"filtered\" : 2,\n" + 
+                "       \"limit\" : 2,\n" + 
+                "       \"mode\" : \"row-based\",\n" + 
+                "       \"pool\" : {\n" + 
+                "         \"recons\" : { }\n" + 
+                "       },\n" + 
+                "       \"rows\" : [ {\n" + 
+                "         \"cells\" : [ {\n" + 
+                "           \"v\" : \"c\"\n" + 
+                "         }, {\n" + 
+                "           \"v\" : \"d\"\n" + 
+                "         } ],\n" + 
+                "         \"flagged\" : false,\n" + 
+                "         \"i\" : 0,\n" + 
+                "         \"starred\" : false\n" + 
+                "       }, {\n" + 
+                "         \"cells\" : [ null, {\n" + 
+                "           \"v\" : \"f\"\n" + 
+                "         } ],\n" + 
+                "         \"flagged\" : false,\n" + 
+                "         \"i\" : 1,\n" + 
+                "         \"starred\" : false\n" + 
+                "       } ],\n" + 
+                "       \"start\" : 0,\n" + 
+                "       \"total\" : 2\n" + 
+                "     }";
+        
+        when(request.getParameter("engine")).thenReturn("{\"mode\":\"row-based\",\"facets\":[]}");
+        command.doPost(request, response);
+        TestUtils.assertEqualAsJson(rowJson, writer.toString());
+    }
+    
+    @Test
+    public void testJsonOutputRecords() throws ServletException, IOException {
+        String recordJson = "{\n" + 
+                "       \"filtered\" : 1,\n" + 
+                "       \"limit\" : 2,\n" + 
+                "       \"mode\" : \"record-based\",\n" + 
+                "       \"pool\" : {\n" + 
+                "         \"recons\" : { }\n" + 
+                "       },\n" + 
+                "       \"rows\" : [ {\n" + 
+                "         \"cells\" : [ {\n" + 
+                "           \"v\" : \"c\"\n" + 
+                "         }, {\n" + 
+                "           \"v\" : \"d\"\n" + 
+                "         } ],\n" + 
+                "         \"flagged\" : false,\n" + 
+                "         \"i\" : 0,\n" + 
+                "         \"j\" : 0,\n" + 
+                "         \"starred\" : false\n" + 
+                "       }, {\n" + 
+                "         \"cells\" : [ null, {\n" + 
+                "           \"v\" : \"f\"\n" + 
+                "         } ],\n" + 
+                "         \"flagged\" : false,\n" + 
+                "         \"i\" : 1,\n" + 
+                "         \"starred\" : false\n" + 
+                "       } ],\n" + 
+                "       \"start\" : 0,\n" + 
+                "       \"total\" : 1\n" + 
+                "     }";
+        
+        when(request.getParameter("engine")).thenReturn("{\"mode\":\"record-based\",\"facets\":[]}");
+        command.doPost(request, response);
+        TestUtils.assertEqualAsJson(recordJson, writer.toString());
+    }
+}