Flush all column caches on row removals/changes. Fixes issue 567.
This commit is contained in:
parent
b4f3e750d6
commit
c961bb64de
@ -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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user