From 1eb9ef78ef701779015a75cddc811a3dc0cbac26 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Sat, 3 Nov 2018 17:10:23 +0000 Subject: [PATCH] Fix cell index computation in filldown/blankdown --- .../operations/cell/BlankDownOperation.java | 6 ++-- .../operations/cell/FillDownOperation.java | 6 ++-- .../tests/operations/cell/BlankDownTests.java | 33 +++++++++++++++++++ .../tests/operations/cell/FillDownTests.java | 30 +++++++++++++++++ 4 files changed, 71 insertions(+), 4 deletions(-) diff --git a/main/src/com/google/refine/operations/cell/BlankDownOperation.java b/main/src/com/google/refine/operations/cell/BlankDownOperation.java index 6af9743f3..17d2f0b1a 100644 --- a/main/src/com/google/refine/operations/cell/BlankDownOperation.java +++ b/main/src/com/google/refine/operations/cell/BlankDownOperation.java @@ -103,6 +103,7 @@ public class BlankDownOperation extends EngineDependentMassCellOperation { return new RowVisitor() { int cellIndex; + int keyCellIndex; List cellChanges; Cell previousCell; Mode engineMode; @@ -116,7 +117,8 @@ public class BlankDownOperation extends EngineDependentMassCellOperation { @Override public void start(Project project) { - // nothing to do + keyCellIndex = project.columnModel.columns.get( + project.columnModel.getKeyColumnIndex()).getCellIndex(); } @Override @@ -126,7 +128,7 @@ 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))) { + if (engineMode.equals(Mode.RecordBased) && ExpressionUtils.isNonBlankData(row.getCellValue(keyCellIndex))) { previousCell = null; } Object value = row.getCellValue(cellIndex); diff --git a/main/src/com/google/refine/operations/cell/FillDownOperation.java b/main/src/com/google/refine/operations/cell/FillDownOperation.java index 0b58b8e7d..23a42c5a3 100644 --- a/main/src/com/google/refine/operations/cell/FillDownOperation.java +++ b/main/src/com/google/refine/operations/cell/FillDownOperation.java @@ -105,6 +105,7 @@ public class FillDownOperation extends EngineDependentMassCellOperation { return new RowVisitor() { int cellIndex; + int keyCellIndex; List cellChanges; Cell previousCell; Mode engineMode; @@ -118,7 +119,8 @@ public class FillDownOperation extends EngineDependentMassCellOperation { @Override public void start(Project project) { - // nothing to do + keyCellIndex = project.columnModel.columns.get( + project.columnModel.getKeyColumnIndex()).getCellIndex(); } @Override @@ -129,7 +131,7 @@ 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))) { + if (engineMode.equals(Mode.RecordBased) && ExpressionUtils.isNonBlankData(row.getCellValue(keyCellIndex))) { previousCell = null; } if (ExpressionUtils.isNonBlankData(value)) { 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 index e58088305..1e7669b29 100644 --- 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 @@ -1,5 +1,7 @@ package com.google.refine.tests.operations.cell; +import java.util.ArrayList; +import java.util.List; import java.util.Properties; import org.json.JSONException; @@ -13,12 +15,16 @@ import org.testng.annotations.Test; import com.google.refine.ProjectManager; import com.google.refine.browsing.EngineConfig; import com.google.refine.model.AbstractOperation; +import com.google.refine.model.Column; import com.google.refine.model.Project; +import com.google.refine.model.Row; import com.google.refine.operations.OperationRegistry; import com.google.refine.operations.cell.BlankDownOperation; +import com.google.refine.operations.cell.FillDownOperation; import com.google.refine.process.Process; import com.google.refine.tests.RefineTest; import com.google.refine.tests.util.TestUtils; +import com.google.refine.util.Pool; public class BlankDownTests extends RefineTest { @@ -81,4 +87,31 @@ public class BlankDownTests extends RefineTest { Assert.assertNull(project.rows.get(2).cells.get(2)); Assert.assertNull(project.rows.get(3).cells.get(2)); } + + @Test + public void testKeyColumnIndex() throws Exception { + // Shift all column indices + for(Row r : project.rows) { + r.cells.add(0, null); + } + List newColumns = new ArrayList<>(); + for(Column c : project.columnModel.columns) { + newColumns.add(new Column(c.getCellIndex()+1, c.getName())); + } + project.columnModel.columns.clear(); + project.columnModel.columns.addAll(newColumns); + project.columnModel.update(); + + AbstractOperation op = new BlankDownOperation( + EngineConfig.reconstruct(new JSONObject("{\"mode\":\"record-based\",\"facets\":[]}")), + "second"); + Process process = op.createProcess(project, new Properties()); + process.performImmediate(); + //project.saveToOutputStream(System.out, new Pool()); + + Assert.assertEquals("c", project.rows.get(0).cells.get(3).value); + Assert.assertNull(project.rows.get(1).cells.get(3)); + Assert.assertEquals("c", project.rows.get(2).cells.get(3).value); + Assert.assertNull(project.rows.get(3).cells.get(3)); + } } 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 index 815a2687f..eb15e693d 100644 --- 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 @@ -1,5 +1,7 @@ package com.google.refine.tests.operations.cell; +import java.util.ArrayList; +import java.util.List; import java.util.Properties; import org.json.JSONException; @@ -13,7 +15,9 @@ import org.testng.annotations.Test; import com.google.refine.ProjectManager; import com.google.refine.browsing.EngineConfig; import com.google.refine.model.AbstractOperation; +import com.google.refine.model.Column; import com.google.refine.model.Project; +import com.google.refine.model.Row; import com.google.refine.operations.OperationRegistry; import com.google.refine.operations.cell.FillDownOperation; import com.google.refine.tests.RefineTest; @@ -98,4 +102,30 @@ public class FillDownTests extends RefineTest { Assert.assertEquals("c", project.rows.get(2).cells.get(2).value); Assert.assertEquals("h", project.rows.get(3).cells.get(2).value); } + + @Test + public void testKeyColumnIndex() throws Exception { + // Shift all column indices + for(Row r : project.rows) { + r.cells.add(0, null); + } + List newColumns = new ArrayList<>(); + for(Column c : project.columnModel.columns) { + newColumns.add(new Column(c.getCellIndex()+1, c.getName())); + } + project.columnModel.columns.clear(); + project.columnModel.columns.addAll(newColumns); + project.columnModel.update(); + + AbstractOperation op = new FillDownOperation( + EngineConfig.reconstruct(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(3).value); + Assert.assertEquals("c", project.rows.get(1).cells.get(3).value); + Assert.assertNull(project.rows.get(2).cells.get(3)); + Assert.assertEquals("h", project.rows.get(3).cells.get(3).value); + } }