From 53c12ca2df2331f70e73c3a538b5460c9a65092a Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Sat, 21 Apr 2018 22:11:54 +0200 Subject: [PATCH] Fix FillDown and BlankDown operations on records. Closes #472. --- .../operations/cell/BlankDownOperation.java | 11 ++- .../operations/cell/FillDownOperation.java | 13 ++- .../tests/operations/cell/BlankDownTests.java | 64 +++++++++++++++ .../tests/operations/cell/FillDownTests.java | 82 +++++++++++++++++++ 4 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/tests/operations/cell/BlankDownTests.java create mode 100644 main/tests/server/src/com/google/refine/tests/operations/cell/FillDownTests.java diff --git a/main/src/com/google/refine/operations/cell/BlankDownOperation.java b/main/src/com/google/refine/operations/cell/BlankDownOperation.java index 3828cd9cb..ce17ecf31 100644 --- a/main/src/com/google/refine/operations/cell/BlankDownOperation.java +++ b/main/src/com/google/refine/operations/cell/BlankDownOperation.java @@ -40,6 +40,7 @@ import org.json.JSONException; import org.json.JSONObject; import org.json.JSONWriter; +import com.google.refine.browsing.Engine.Mode; import com.google.refine.browsing.RowVisitor; import com.google.refine.expr.ExpressionUtils; import com.google.refine.model.AbstractOperation; @@ -97,15 +98,18 @@ public class BlankDownOperation extends EngineDependentMassCellOperation { @Override protected RowVisitor createRowVisitor(Project project, List cellChanges, long historyEntryID) throws Exception { Column column = project.columnModel.getColumnByName(_columnName); + Mode engineMode = createEngine(project).getMode(); return new RowVisitor() { int cellIndex; List cellChanges; Cell previousCell; + Mode engineMode; - public RowVisitor init(int cellIndex, List cellChanges) { + public RowVisitor init(int cellIndex, List cellChanges, Mode engineMode) { this.cellIndex = cellIndex; this.cellChanges = cellChanges; + this.engineMode = engineMode; return this; } @@ -121,6 +125,9 @@ public class BlankDownOperation extends EngineDependentMassCellOperation { @Override public boolean visit(Project project, int rowIndex, Row row) { + if (engineMode.equals(Mode.RecordBased) && ExpressionUtils.isNonBlankData(row.getCellValue(0))) { + previousCell = null; + } Object value = row.getCellValue(cellIndex); if (ExpressionUtils.isNonBlankData(value)) { Cell cell = row.getCell(cellIndex); @@ -134,6 +141,6 @@ public class BlankDownOperation extends EngineDependentMassCellOperation { } return false; } - }.init(column.getCellIndex(), cellChanges); + }.init(column.getCellIndex(), cellChanges, engineMode); } } diff --git a/main/src/com/google/refine/operations/cell/FillDownOperation.java b/main/src/com/google/refine/operations/cell/FillDownOperation.java index b6b7bd1b5..24d5f2260 100644 --- a/main/src/com/google/refine/operations/cell/FillDownOperation.java +++ b/main/src/com/google/refine/operations/cell/FillDownOperation.java @@ -40,6 +40,8 @@ import org.json.JSONException; import org.json.JSONObject; import org.json.JSONWriter; +import com.google.refine.browsing.Engine; +import com.google.refine.browsing.Engine.Mode; import com.google.refine.browsing.RowVisitor; import com.google.refine.expr.ExpressionUtils; import com.google.refine.model.AbstractOperation; @@ -97,15 +99,19 @@ public class FillDownOperation extends EngineDependentMassCellOperation { @Override protected RowVisitor createRowVisitor(Project project, List cellChanges, long historyEntryID) throws Exception { Column column = project.columnModel.getColumnByName(_columnName); + Engine engine = createEngine(project); + Mode engineMode = engine.getMode(); return new RowVisitor() { int cellIndex; List cellChanges; Cell previousCell; + Mode engineMode; - public RowVisitor init(int cellIndex, List cellChanges) { + public RowVisitor init(int cellIndex, List cellChanges, Mode engineMode) { this.cellIndex = cellIndex; this.cellChanges = cellChanges; + this.engineMode = engineMode; return this; } @@ -122,6 +128,9 @@ public class FillDownOperation extends EngineDependentMassCellOperation { @Override public boolean visit(Project project, int rowIndex, Row row) { Object value = row.getCellValue(cellIndex); + if (engineMode.equals(Mode.RecordBased) && ExpressionUtils.isNonBlankData(row.getCellValue(0))) { + previousCell = null; + } if (ExpressionUtils.isNonBlankData(value)) { previousCell = row.getCell(cellIndex); } else if (previousCell != null) { @@ -130,6 +139,6 @@ public class FillDownOperation extends EngineDependentMassCellOperation { } return false; } - }.init(column.getCellIndex(), cellChanges); + }.init(column.getCellIndex(), cellChanges, engineMode); } } diff --git a/main/tests/server/src/com/google/refine/tests/operations/cell/BlankDownTests.java b/main/tests/server/src/com/google/refine/tests/operations/cell/BlankDownTests.java new file mode 100644 index 000000000..7b4d3b5fa --- /dev/null +++ b/main/tests/server/src/com/google/refine/tests/operations/cell/BlankDownTests.java @@ -0,0 +1,64 @@ +package com.google.refine.tests.operations.cell; + +import java.util.Properties; + +import org.json.JSONObject; +import org.testng.Assert; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.refine.ProjectManager; +import com.google.refine.model.AbstractOperation; +import com.google.refine.model.Project; +import com.google.refine.operations.cell.BlankDownOperation; +import com.google.refine.process.Process; +import com.google.refine.tests.RefineTest; + +public class BlankDownTests extends RefineTest { + + Project project = null; + + @BeforeMethod + public void setUp() { + project = createCSVProject( + "key,first,second\n"+ + "a,b,c\n"+ + ",d,c\n"+ + "e,f,c\n"+ + ",,c\n"); + } + + @AfterMethod + public void tearDown() { + ProjectManager.singleton.deleteProject(project.id); + } + + @Test + public void testBlankDownRecords() throws Exception { + AbstractOperation op = new BlankDownOperation( + new JSONObject("{\"mode\":\"record-based\",\"facets\":[]}"), + "second"); + Process process = op.createProcess(project, new Properties()); + process.performImmediate(); + + Assert.assertEquals("c", project.rows.get(0).cells.get(2).value); + Assert.assertNull(project.rows.get(1).cells.get(2)); + Assert.assertEquals("c", project.rows.get(2).cells.get(2).value); + Assert.assertNull(project.rows.get(3).cells.get(2)); + } + + @Test + public void testBlankDownRows() throws Exception { + AbstractOperation op = new BlankDownOperation( + new JSONObject("{\"mode\":\"row-based\",\"facets\":[]}"), + "second"); + Process process = op.createProcess(project, new Properties()); + process.performImmediate(); + + Assert.assertEquals("c", project.rows.get(0).cells.get(2).value); + Assert.assertNull(project.rows.get(1).cells.get(2)); + Assert.assertNull(project.rows.get(2).cells.get(2)); + Assert.assertNull(project.rows.get(3).cells.get(2)); + } +} diff --git a/main/tests/server/src/com/google/refine/tests/operations/cell/FillDownTests.java b/main/tests/server/src/com/google/refine/tests/operations/cell/FillDownTests.java new file mode 100644 index 000000000..1b6a0154e --- /dev/null +++ b/main/tests/server/src/com/google/refine/tests/operations/cell/FillDownTests.java @@ -0,0 +1,82 @@ +package com.google.refine.tests.operations.cell; + +import java.util.Properties; + +import org.json.JSONObject; +import org.testng.Assert; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.refine.ProjectManager; +import com.google.refine.model.AbstractOperation; +import com.google.refine.model.Project; +import com.google.refine.operations.cell.FillDownOperation; +import com.google.refine.tests.RefineTest; +import com.google.refine.process.Process; + +public class FillDownTests extends RefineTest { + + Project project = null; + + @BeforeMethod + public void setUp() { + project = createCSVProject( + "key,first,second\n"+ + "a,b,c\n"+ + ",d,\n"+ + "e,f,\n"+ + ",,h\n"); + } + + @AfterMethod + public void tearDown() { + ProjectManager.singleton.deleteProject(project.id); + } + + @Test + public void testFillDownRecordKey() throws Exception { + AbstractOperation op = new FillDownOperation( + new JSONObject("{\"mode\":\"record-based\",\"facets\":[]}"), + "key"); + Process process = op.createProcess(project, new Properties()); + process.performImmediate(); + + Assert.assertEquals("a", project.rows.get(0).cells.get(0).value); + Assert.assertEquals("a", project.rows.get(1).cells.get(0).value); + Assert.assertEquals("e", project.rows.get(2).cells.get(0).value); + Assert.assertEquals("e", project.rows.get(3).cells.get(0).value); + } + + // For issue #742 + // https://github.com/OpenRefine/OpenRefine/issues/742 + @Test + public void testFillDownRecords() throws Exception { + AbstractOperation op = new FillDownOperation( + new JSONObject("{\"mode\":\"record-based\",\"facets\":[]}"), + "second"); + Process process = op.createProcess(project, new Properties()); + process.performImmediate(); + + Assert.assertEquals("c", project.rows.get(0).cells.get(2).value); + Assert.assertEquals("c", project.rows.get(1).cells.get(2).value); + Assert.assertNull(project.rows.get(2).cells.get(2)); + Assert.assertEquals("h", project.rows.get(3).cells.get(2).value); + } + + // For issue #742 + // https://github.com/OpenRefine/OpenRefine/issues/742 + @Test + public void testFillDownRows() throws Exception { + AbstractOperation op = new FillDownOperation( + new JSONObject("{\"mode\":\"row-based\",\"facets\":[]}"), + "second"); + Process process = op.createProcess(project, new Properties()); + process.performImmediate(); + + Assert.assertEquals("c", project.rows.get(0).cells.get(2).value); + Assert.assertEquals("c", project.rows.get(1).cells.get(2).value); + Assert.assertEquals("c", project.rows.get(2).cells.get(2).value); + Assert.assertEquals("h", project.rows.get(3).cells.get(2).value); + } +}