diff --git a/main/src/com/google/refine/browsing/Engine.java b/main/src/com/google/refine/browsing/Engine.java index f8cffce53..3910fa723 100644 --- a/main/src/com/google/refine/browsing/Engine.java +++ b/main/src/com/google/refine/browsing/Engine.java @@ -33,22 +33,18 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package com.google.refine.browsing; +import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Properties; +import java.util.stream.Collectors; -import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; import org.json.JSONWriter; import com.google.refine.Jsonizable; import com.google.refine.browsing.facets.Facet; -import com.google.refine.browsing.facets.ListFacet; -import com.google.refine.browsing.facets.RangeFacet; -import com.google.refine.browsing.facets.ScatterplotFacet; -import com.google.refine.browsing.facets.TextSearchFacet; -import com.google.refine.browsing.facets.TimeRangeFacet; import com.google.refine.browsing.util.ConjunctiveFilteredRecords; import com.google.refine.browsing.util.ConjunctiveFilteredRows; import com.google.refine.browsing.util.FilteredRecordsAsFilteredRows; @@ -71,7 +67,7 @@ public class Engine implements Jsonizable { protected Project _project; protected List _facets = new LinkedList(); - protected Mode _mode = Mode.RowBased; + protected EngineConfig _config = new EngineConfig(Collections.emptyList(), Mode.RowBased); static public String modeToString(Mode mode) { return mode == Mode.RowBased ? MODE_ROW_BASED : MODE_RECORD_BASED; @@ -85,10 +81,10 @@ public class Engine implements Jsonizable { } public Mode getMode() { - return _mode; + return _config.getMode(); } public void setMode(Mode mode) { - _mode = mode; + _config = new EngineConfig(_config.getFacetConfigs(), mode); } public FilteredRows getAllRows() { @@ -117,9 +113,9 @@ public class Engine implements Jsonizable { } public FilteredRows getFilteredRows(Facet except) { - if (_mode == Mode.RecordBased) { + if (_config.getMode().equals(Mode.RecordBased)) { return new FilteredRecordsAsFilteredRows(getFilteredRecords(except)); - } else if (_mode == Mode.RowBased) { + } else if (_config.getMode().equals(Mode.RowBased)) { ConjunctiveFilteredRows cfr = new ConjunctiveFilteredRows(); for (Facet facet : _facets) { if (facet != except) { @@ -157,7 +153,7 @@ public class Engine implements Jsonizable { } public FilteredRecords getFilteredRecords(Facet except) { - if (_mode == Mode.RecordBased) { + if (_config.getMode().equals(Mode.RecordBased)) { ConjunctiveFilteredRecords cfr = new ConjunctiveFilteredRecords(); for (Facet facet : _facets) { if (facet != except) { @@ -173,56 +169,25 @@ public class Engine implements Jsonizable { } public void initializeFromJSON(JSONObject o) throws JSONException { - if (o == null) { - return; - } - - if (o.has("facets") && !o.isNull("facets")) { - JSONArray a = o.getJSONArray("facets"); - int length = a.length(); - - for (int i = 0; i < length; i++) { - JSONObject fo = a.getJSONObject(i); - String type = fo.has("type") ? fo.getString("type") : "list"; - - Facet facet = null; - if ("list".equals(type)) { - facet = new ListFacet(); - } else if ("range".equals(type)) { - facet = new RangeFacet(); - } else if ("timerange".equals(type)) { - facet = new TimeRangeFacet(); - } else if ("scatterplot".equals(type)) { - facet = new ScatterplotFacet(); - } else if ("text".equals(type)) { - facet = new TextSearchFacet(); - } - - if (facet != null) { - facet.initializeFromJSON(_project, fo); - _facets.add(facet); - } - } - } - - // for backward compatibility - if (o.has(INCLUDE_DEPENDENT) && !o.isNull(INCLUDE_DEPENDENT)) { - _mode = o.getBoolean(INCLUDE_DEPENDENT) ? Mode.RecordBased : Mode.RowBased; - } - - if (o.has(MODE) && !o.isNull(MODE)) { - _mode = MODE_ROW_BASED.equals(o.getString(MODE)) ? Mode.RowBased : Mode.RecordBased; - } + EngineConfig config = EngineConfig.reconstruct(o); + initializeFromConfig(config); + } + + public void initializeFromConfig(EngineConfig config) { + _config = config; + _facets = config.getFacetConfigs().stream() + .map(c -> c.apply(_project)) + .collect(Collectors.toList()); } public void computeFacets() throws JSONException { - if (_mode == Mode.RowBased) { + if (_config.getMode().equals(Mode.RowBased)) { for (Facet facet : _facets) { FilteredRows filteredRows = getFilteredRows(facet); facet.computeChoices(_project, filteredRows); } - } else if (_mode == Mode.RecordBased) { + } else if (_config.getMode().equals(Mode.RecordBased)) { for (Facet facet : _facets) { FilteredRecords filteredRecords = getFilteredRecords(facet); @@ -244,7 +209,7 @@ public class Engine implements Jsonizable { facet.write(writer, options); } writer.endArray(); - writer.key(MODE); writer.value(_mode == Mode.RowBased ? MODE_ROW_BASED : MODE_RECORD_BASED); + writer.key(MODE); writer.value(_config.getMode().equals(Mode.RowBased) ? MODE_ROW_BASED : MODE_RECORD_BASED); writer.endObject(); } } diff --git a/main/src/com/google/refine/browsing/EngineConfig.java b/main/src/com/google/refine/browsing/EngineConfig.java index 622ed3729..0ae50313b 100644 --- a/main/src/com/google/refine/browsing/EngineConfig.java +++ b/main/src/com/google/refine/browsing/EngineConfig.java @@ -1,5 +1,6 @@ package com.google.refine.browsing; +import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Properties; @@ -29,9 +30,17 @@ public class EngineConfig implements Jsonizable { _mode = mode; } + public Mode getMode() { + return _mode; + } + + public List getFacetConfigs() { + return _facets; + } + public static EngineConfig reconstruct(JSONObject o) { if (o == null) { - return null; + return new EngineConfig(Collections.emptyList(), Mode.RowBased); } List facets = new LinkedList<>(); diff --git a/main/src/com/google/refine/browsing/facets/Facet.java b/main/src/com/google/refine/browsing/facets/Facet.java index eb48416d9..5720806ca 100644 --- a/main/src/com/google/refine/browsing/facets/Facet.java +++ b/main/src/com/google/refine/browsing/facets/Facet.java @@ -33,9 +33,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package com.google.refine.browsing.facets; -import org.json.JSONException; -import org.json.JSONObject; - import com.google.refine.Jsonizable; import com.google.refine.browsing.FilteredRecords; import com.google.refine.browsing.FilteredRows; @@ -54,6 +51,4 @@ public interface Facet extends Jsonizable { public void computeChoices(Project project, FilteredRows filteredRows); public void computeChoices(Project project, FilteredRecords filteredRecords); - - public void initializeFromJSON(Project project, JSONObject o) throws JSONException; } diff --git a/main/src/com/google/refine/browsing/facets/ListFacet.java b/main/src/com/google/refine/browsing/facets/ListFacet.java index 484b283e8..bec36af53 100644 --- a/main/src/com/google/refine/browsing/facets/ListFacet.java +++ b/main/src/com/google/refine/browsing/facets/ListFacet.java @@ -212,13 +212,6 @@ public class ListFacet implements Facet { } return 2000; } - - @Override - public void initializeFromJSON(Project project, JSONObject o) throws JSONException { - ListFacetConfig config = new ListFacetConfig(); - config.initializeFromJSON(o); - initializeFromConfig(config, project); - } public void initializeFromConfig(ListFacetConfig config, Project project) { _config = config; diff --git a/main/src/com/google/refine/browsing/facets/RangeFacet.java b/main/src/com/google/refine/browsing/facets/RangeFacet.java index b54f7b022..662253697 100644 --- a/main/src/com/google/refine/browsing/facets/RangeFacet.java +++ b/main/src/com/google/refine/browsing/facets/RangeFacet.java @@ -228,13 +228,6 @@ public class RangeFacet implements Facet { } } - @Override - public void initializeFromJSON(Project project, JSONObject o) throws JSONException { - RangeFacetConfig config = new RangeFacetConfig(); - config.initializeFromJSON(o); - initializeFromConfig(config, project); - } - @Override public RowFilter getRowFilter(Project project) { if (_eval != null && _errorMessage == null && _config._selected) { diff --git a/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java b/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java index 52f7cc208..3a6862d59 100644 --- a/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java +++ b/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java @@ -296,13 +296,6 @@ public class ScatterplotFacet implements Facet { writer.endObject(); } - - @Override - public void initializeFromJSON(Project project, JSONObject o) throws JSONException { - ScatterplotFacetConfig config = new ScatterplotFacetConfig(); - config.initializeFromJSON(o); - initializeFromConfig(config, project); - } public void initializeFromConfig(ScatterplotFacetConfig configuration, Project project) { config = configuration; diff --git a/main/src/com/google/refine/browsing/facets/TextSearchFacet.java b/main/src/com/google/refine/browsing/facets/TextSearchFacet.java index f0011aac1..96f97813b 100644 --- a/main/src/com/google/refine/browsing/facets/TextSearchFacet.java +++ b/main/src/com/google/refine/browsing/facets/TextSearchFacet.java @@ -123,13 +123,6 @@ public class TextSearchFacet implements Facet { writer.key("invert"); writer.value(_config._invert); writer.endObject(); } - - @Override - public void initializeFromJSON(Project project, JSONObject o) throws JSONException { - TextSearchFacetConfig config = new TextSearchFacetConfig(); - config.initializeFromJSON(o); - initializeFromConfig(config, project); - } public void initializeFromConfig(TextSearchFacetConfig config, Project project) { _config = config; diff --git a/main/src/com/google/refine/browsing/facets/TimeRangeFacet.java b/main/src/com/google/refine/browsing/facets/TimeRangeFacet.java index 52566aa05..30e1ece0c 100644 --- a/main/src/com/google/refine/browsing/facets/TimeRangeFacet.java +++ b/main/src/com/google/refine/browsing/facets/TimeRangeFacet.java @@ -201,13 +201,6 @@ public class TimeRangeFacet implements Facet { } writer.endObject(); } - - @Override - public void initializeFromJSON(Project project, JSONObject o) throws JSONException { - TimeRangeFacetConfig config = new TimeRangeFacetConfig(); - config.initializeFromJSON(o); - initializeFromConfig(config, project); - } public void initializeFromConfig(TimeRangeFacetConfig config, Project project) { _config = config; diff --git a/main/tests/server/src/com/google/refine/tests/browsing/facets/EngineConfigTests.java b/main/tests/server/src/com/google/refine/tests/browsing/facets/EngineConfigTests.java index 07960a721..d63cd2f76 100644 --- a/main/tests/server/src/com/google/refine/tests/browsing/facets/EngineConfigTests.java +++ b/main/tests/server/src/com/google/refine/tests/browsing/facets/EngineConfigTests.java @@ -1,9 +1,11 @@ package com.google.refine.tests.browsing.facets; import org.json.JSONObject; +import org.testng.Assert; import org.testng.annotations.Test; import com.google.refine.browsing.EngineConfig; +import com.google.refine.browsing.Engine.Mode; import com.google.refine.tests.util.TestUtils; public class EngineConfigTests { @@ -24,9 +26,37 @@ public class EngineConfigTests { + " ]\n" + " }"; + public static String engineConfigRecordModeJson = + "{" + + " \"mode\":\"record-based\"," + + " \"facets\":[]" + + "}"; + + public static String noFacetProvided = "{\"mode\":\"row-based\"}"; + @Test public void serializeEngineConfig() { EngineConfig ec = EngineConfig.reconstruct(new JSONObject(engineConfigJson)); TestUtils.isSerializedTo(ec, engineConfigJson); } + + @Test + public void serializeEngineConfigRecordMode() { + EngineConfig ec = EngineConfig.reconstruct(new JSONObject(engineConfigRecordModeJson)); + TestUtils.isSerializedTo(ec, engineConfigRecordModeJson); + } + + @Test + public void reconstructNullEngineConfig() { + EngineConfig ec = EngineConfig.reconstruct(null); + Assert.assertEquals(ec.getMode(), Mode.RowBased); + Assert.assertTrue(ec.getFacetConfigs().isEmpty()); + } + + @Test + public void reconstructNoFacetsProvided() { + EngineConfig ec = EngineConfig.reconstruct(new JSONObject(noFacetProvided)); + Assert.assertEquals(ec.getMode(), Mode.RowBased); + Assert.assertTrue(ec.getFacetConfigs().isEmpty()); + } } diff --git a/main/tests/server/src/com/google/refine/tests/browsing/facets/TextSearchFacetTests.java b/main/tests/server/src/com/google/refine/tests/browsing/facets/TextSearchFacetTests.java index 24e74104f..870c5f36e 100644 --- a/main/tests/server/src/com/google/refine/tests/browsing/facets/TextSearchFacetTests.java +++ b/main/tests/server/src/com/google/refine/tests/browsing/facets/TextSearchFacetTests.java @@ -55,9 +55,9 @@ import com.google.refine.tests.util.TestUtils; public class TextSearchFacetTests extends RefineTest { // dependencies private Project project; + private TextSearchFacetConfig textfilterconfig; private TextSearchFacet textfilter; private RowFilter rowfilter; - private JSONObject textsearchfacet; private String sensitiveConfigJson = "{\"type\":\"text\"," + "\"name\":\"Value\"," + "\"columnName\":\"Value\"," @@ -89,6 +89,14 @@ public class TextSearchFacetTests extends RefineTest { + "Abc\n"); } + private void configureFilter(String filter) { + //Add the facet to the project and create a row filter + textfilterconfig = new TextSearchFacetConfig(); + textfilterconfig.initializeFromJSON(new JSONObject(filter)); + textfilter = textfilterconfig.apply(project); + rowfilter = textfilter.getRowFilter(project); + } + /** * Test to demonstrate the intended behaviour of the function */ @@ -110,11 +118,7 @@ public class TextSearchFacetTests extends RefineTest { + "\"invert\":false," + "\"query\":\"a\"}"; - //Add the facet to the project and create a row filter - textfilter = new TextSearchFacet(); - textsearchfacet = new JSONObject(filter); - textfilter.initializeFromJSON(project,textsearchfacet); - rowfilter = textfilter.getRowFilter(project); + configureFilter(filter); //Check each row in the project against the filter Assert.assertEquals(rowfilter.filterRow(project, 0, project.rows.get(0)),true); @@ -140,11 +144,7 @@ public class TextSearchFacetTests extends RefineTest { + "\"invert\":true," + "\"query\":\"a\"}"; - //Add the facet to the project and create a row filter - textfilter = new TextSearchFacet(); - textsearchfacet = new JSONObject(filter); - textfilter.initializeFromJSON(project,textsearchfacet); - rowfilter = textfilter.getRowFilter(project); + configureFilter(filter); //Check each row in the project against the filter Assert.assertEquals(rowfilter.filterRow(project, 0, project.rows.get(0)),false); @@ -170,11 +170,7 @@ public class TextSearchFacetTests extends RefineTest { + "\"invert\":false," + "\"query\":\"[bc]\"}"; - //Add the facet to the project and create a row filter - textfilter = new TextSearchFacet(); - textsearchfacet = new JSONObject(filter); - textfilter.initializeFromJSON(project,textsearchfacet); - rowfilter = textfilter.getRowFilter(project); + configureFilter(filter); //Check each row in the project against the filter Assert.assertEquals(rowfilter.filterRow(project, 0, project.rows.get(0)),false); @@ -186,14 +182,8 @@ public class TextSearchFacetTests extends RefineTest { @Test public void testCaseSensitiveFilter() throws Exception { //Apply case-sensitive filter "A" - - - //Add the facet to the project and create a row filter - textfilter = new TextSearchFacet(); - textsearchfacet = new JSONObject(sensitiveConfigJson); - textfilter.initializeFromJSON(project,textsearchfacet); - rowfilter = textfilter.getRowFilter(project); + configureFilter(sensitiveConfigJson); //Check each row in the project against the filter //Expect to retrieve one row containing "Abc"