Reorganize facet parsing and discontinue initializeFromJSON on Facet

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<Facet> _facets = new LinkedList<Facet>();
-    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) {
+        EngineConfig config = EngineConfig.reconstruct(o);
-            return;
+        initializeFromConfig(config);
-        }
+    }
-
+    
-        if (o.has("facets") && !o.isNull("facets")) {
+    public void initializeFromConfig(EngineConfig config) {
-            JSONArray a = o.getJSONArray("facets");
+        _config = config;
-            int length = a.length();
+        _facets = config.getFacetConfigs().stream()
-
+                .map(c -> c.apply(_project))
-            for (int i = 0; i < length; i++) {
+                .collect(Collectors.toList());
-                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;
-        }
     }
 
     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();
     }
 }

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<FacetConfig> getFacetConfigs() {
+        return _facets;
+    }
+    
     public static EngineConfig reconstruct(JSONObject o) {
         if (o == null) {
-            return null;
+            return new EngineConfig(Collections.emptyList(), Mode.RowBased);
         }
 
         List<FacetConfig> facets = new LinkedList<>();

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;
 }

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;

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) {

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;

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;

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;

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());
+    }
 }

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
+        configureFilter(filter);
-        textfilter = new TextSearchFacet();
-        textsearchfacet = new JSONObject(filter);
-        textfilter.initializeFromJSON(project,textsearchfacet);
-        rowfilter = textfilter.getRowFilter(project);
 
         //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
+        configureFilter(filter);
-        textfilter = new TextSearchFacet();
-        textsearchfacet = new JSONObject(filter);
-        textfilter.initializeFromJSON(project,textsearchfacet);
-        rowfilter = textfilter.getRowFilter(project);
 
         //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
+        configureFilter(filter);
-        textfilter = new TextSearchFacet();
-        textsearchfacet = new JSONObject(filter);
-        textfilter.initializeFromJSON(project,textsearchfacet);
-        rowfilter = textfilter.getRowFilter(project);
 
         //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"
-
         
-
+        configureFilter(sensitiveConfigJson);
-        //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);
 
         //Check each row in the project against the filter
         //Expect to retrieve one row containing "Abc"