diff --git a/main/src/com/google/refine/model/ColumnModel.java b/main/src/com/google/refine/model/ColumnModel.java index d13b1a560..4bf1fb598 100644 --- a/main/src/com/google/refine/model/ColumnModel.java +++ b/main/src/com/google/refine/model/ColumnModel.java @@ -74,6 +74,9 @@ public class ColumnModel implements Jsonizable { return _maxCellIndex; } + /** + * @return the next available cell index + */ synchronized public int allocateNewCellIndex() { return ++_maxCellIndex; } @@ -287,4 +290,13 @@ public class ColumnModel implements Jsonizable { _columnNames.add(column.getName()); } } + + /** + * Clear cached value computations for all columns + */ + public void clearPrecomputes() { + for (Column column : columns) { + column.clearPrecomputes(); + } + } } diff --git a/main/src/com/google/refine/model/changes/MassRowChange.java b/main/src/com/google/refine/model/changes/MassRowChange.java index eee70e46c..c5484d99d 100644 --- a/main/src/com/google/refine/model/changes/MassRowChange.java +++ b/main/src/com/google/refine/model/changes/MassRowChange.java @@ -40,6 +40,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Properties; +import com.google.refine.ProjectManager; import com.google.refine.history.Change; import com.google.refine.model.Project; import com.google.refine.model.Row; @@ -60,6 +61,9 @@ public class MassRowChange implements Change { project.rows.clear(); project.rows.addAll(_newRows); + project.columnModel.clearPrecomputes(); + ProjectManager.singleton.getInterProjectModel().flushJoinsInvolvingProject(project.id); + project.update(); } } diff --git a/main/src/com/google/refine/model/changes/RowRemovalChange.java b/main/src/com/google/refine/model/changes/RowRemovalChange.java index b1f446fb2..fecf21ed0 100644 --- a/main/src/com/google/refine/model/changes/RowRemovalChange.java +++ b/main/src/com/google/refine/model/changes/RowRemovalChange.java @@ -40,6 +40,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Properties; +import com.google.refine.ProjectManager; import com.google.refine.history.Change; import com.google.refine.model.Project; import com.google.refine.model.Row; @@ -70,6 +71,9 @@ public class RowRemovalChange implements Change { offset--; } + project.columnModel.clearPrecomputes(); + ProjectManager.singleton.getInterProjectModel().flushJoinsInvolvingProject(project.id); + project.update(); } } diff --git a/main/tests/server/src/com/google/refine/tests/model/CacheTests.java b/main/tests/server/src/com/google/refine/tests/model/CacheTests.java new file mode 100644 index 000000000..02c6246b8 --- /dev/null +++ b/main/tests/server/src/com/google/refine/tests/model/CacheTests.java @@ -0,0 +1,181 @@ +/* + +Copyright 2010, Google Inc. +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Google Inc. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +*/ + +package com.google.refine.tests.model; + +import static org.mockito.Mockito.mock; + +import java.io.IOException; +import java.io.StringWriter; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Properties; + +import org.json.JSONException; +import org.json.JSONObject; +import org.slf4j.LoggerFactory; +import org.testng.Assert; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.BeforeTest; +import org.testng.annotations.Test; + +import com.google.refine.ProjectManager; +import com.google.refine.ProjectMetadata; +import com.google.refine.browsing.Engine; +import com.google.refine.browsing.RowVisitor; +import com.google.refine.expr.functions.FacetCount; +import com.google.refine.grel.Function; +import com.google.refine.io.FileProjectManager; +import com.google.refine.model.Cell; +import com.google.refine.model.Column; +import com.google.refine.model.ModelException; +import com.google.refine.model.Project; +import com.google.refine.model.Row; +import com.google.refine.operations.EngineDependentOperation; +import com.google.refine.operations.row.RowRemovalOperation; +import com.google.refine.tests.RefineTest; + + +public class CacheTests extends RefineTest { + + //{project=1718051861971, engine= ...} + //engine={ "facets" : ..., "mode":"row-based"}} + //facets = [{"type":"list","name":"row","columnName":"row","expression":"facetCount(value, 'value', 'row') > 1","omitBlank":false,"omitError":false,"selection":[],"selectBlank":false,"selectError":false,"invert":false}] + + // {project=1718051861971, engine= + + // Equivalent to duplicate facet on Column A with true selected + static final String ENGINE_JSON_DUPLICATES = "{\"facets\":[{\"type\":\"list\",\"name\":\"facet A\",\"columnName\":\"Column A\",\"expression\":\"facetCount(value, 'value', 'Column A') > 1\",\"omitBlank\":false,\"omitError\":false,\"selection\":[{\"v\":{\"v\":true,\"l\":\"true\"}}],\"selectBlank\":false,\"selectError\":false,\"invert\":false}],\"mode\":\"row-based\"}}"; + + @Override + @BeforeTest + public void init() { + logger = LoggerFactory.getLogger(this.getClass()); + } + + // dependencies + StringWriter writer; + Project project; + Properties options; + JSONObject engine_config; + Engine engine; + Properties bindings; + + @BeforeMethod + public void SetUp() throws JSONException, IOException, ModelException { + writer = new StringWriter(); + + Path dir = Files.createTempDirectory("openrefine-test-workspace-dir"); + FileProjectManager.initialize(dir.toFile()); + project = new Project(); + ProjectMetadata pm = new ProjectMetadata(); + pm.setName("TNG Test Project"); + ProjectManager.singleton.registerProject(project, pm); + + int index = project.columnModel.allocateNewCellIndex(); + Column column = new Column(index,"Column A"); + project.columnModel.addColumn(index, column, true); + + options = mock(Properties.class); + engine = new Engine(project); + engine_config = new JSONObject(ENGINE_JSON_DUPLICATES); +// engine_config.getJSONArray("facets").getJSONObject(0).getJSONArray("selection").put(new JSONArray()); + engine.initializeFromJSON(engine_config); + engine.setMode(Engine.Mode.RowBased); + + bindings = new Properties(); + bindings.put("project", project); + + } + + @AfterMethod + public void TearDown() { + writer = null; + project = null; + options = null; + } + + /** + * Test for issue 567. Problem doesn't seem to occur when testing + * interactively, but this demonstrates that the facet count cache + * can get stale after row removal operations + * + * @throws Exception + */ + @Test + public void testIssue567() throws Exception { + for (int i = 0; i < 5; i++) { + Row row = new Row(5); + row.setCell(0, new Cell(i < 4 ? "a":"b", null)); + project.rows.add(row); + } + engine.getAllRows().accept(project, new CountingRowVisitor(5)) ; + engine.getAllFilteredRows().accept(project, new CountingRowVisitor(4)); + Function fc = new FacetCount(); + Integer count = (Integer) fc.call(bindings, new Object[] {"a", "value", "Column A"}); + Assert.assertEquals(count.intValue(), 4); + EngineDependentOperation op = new RowRemovalOperation(engine_config); + op.createProcess(project, options).performImmediate(); + engine.getAllRows().accept(project, new CountingRowVisitor(1)) ; + engine.getAllFilteredRows().accept(project, new CountingRowVisitor(0)); + count = (Integer) fc.call(bindings, new Object[] {"a", "value", "Column A"}); + Assert.assertEquals(count.intValue(), 0); + } + + class CountingRowVisitor implements RowVisitor { + + private int count = 0; + private int target; + + private CountingRowVisitor(int targetCount) { + target = targetCount; + } + + @Override + public boolean visit(Project project, int rowIndex, Row row) { + count++; + return false; + } + + @Override + public void start(Project project) { + count = 0; + } + + @Override + public void end(Project project) { + Assert.assertEquals(count, target); + } + } +}