From 65231e21400ef7bdf50267792cfd4369e26de956 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Mon, 1 Oct 2018 11:57:58 +0100 Subject: [PATCH] Refactor GetRowsCommand for Jackson migration --- .../refine/commands/row/GetRowsCommand.java | 141 ++++++++++++------ .../commands/row/GetRowsCommandTest.java | 116 ++++++++++++++ 2 files changed, 213 insertions(+), 44 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/tests/commands/row/GetRowsCommandTest.java diff --git a/main/src/com/google/refine/commands/row/GetRowsCommand.java b/main/src/com/google/refine/commands/row/GetRowsCommand.java index add2f4314..60d3bf366 100644 --- a/main/src/com/google/refine/commands/row/GetRowsCommand.java +++ b/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 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 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); - jsonWriter.key("limit"); jsonWriter.value(limit); - jsonWriter.key("pool"); pool.write(jsonWriter, options); - - jsonWriter.endObject(); - + JsonResult result = new JsonResult(engine.getMode(), + rwv.results, rwv.total, + engine.getMode() == Mode.RowBased ? project.rows.size() : project.recordModel.getRecordCount(), + start, limit, pool); + 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; - final Properties options; + public List results; 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.options = options; + this.results = new ArrayList<>(); } @Override @@ -238,29 +306,14 @@ public class GetRowsCommand extends Command { } public boolean internalVisit(Project project, int rowIndex, Row row) { - try { - options.put("rowIndex", rowIndex); - row.write(writer, options); - } catch (JSONException e) { - } + results.add(new WrappedRow(row, rowIndex, null)); 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); - - options.put("rowIndex", r); - - row.write(writer, options); - - } catch (JSONException e) { - } - - options.remove("recordIndex"); + Row row = project.rows.get(r); + results.add(new WrappedRow(row, r, r == record.fromRowIndex ? record.recordIndex : null)); } return false; } diff --git a/main/tests/server/src/com/google/refine/tests/commands/row/GetRowsCommandTest.java b/main/tests/server/src/com/google/refine/tests/commands/row/GetRowsCommandTest.java new file mode 100644 index 000000000..d35315c7d --- /dev/null +++ b/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()); + } +}