fixed issue #796 Columnize by key/value columns creates empty lines

OpenRefine · Sep 30, 2015 · 7a2a0eb · 7a2a0eb
1 parent 35b01fb
commit 7a2a0eb
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 5 deletions.
diff --git a/main/src/com/google/refine/model/ColumnModel.java b/main/src/com/google/refine/model/ColumnModel.java
@@ -81,6 +81,14 @@ synchronized public int allocateNewCellIndex() {
         return ++_maxCellIndex;
     }
 
+    synchronized public void removeCellIndex(int index) {
+        if (index > _maxCellIndex - 1)
+            return;
+
+        columns.remove(index);
+        _maxCellIndex--;
+    }
+
     synchronized public void setKeyColumnIndex(int keyColumnIndex) {
         // TODO: check validity of new cell index, e.g., it's not in any group
         this._keyColumnIndex = keyColumnIndex;

diff --git a/main/src/com/google/refine/operations/cell/KeyValueColumnizeOperation.java b/main/src/com/google/refine/operations/cell/KeyValueColumnizeOperation.java
@@ -251,6 +251,18 @@ protected HistoryEntry createHistoryEntry(Project project, long historyEntryID)
         allColumns.addAll(newColumns);
         allColumns.addAll(newNoteColumns);
 
+        // clean up the reused column model and row model
+        int smallIndex = Math.min(keyColumnIndex, valueColumnIndex);
+        int bigIndex = Math.max(keyColumnIndex, valueColumnIndex);
+
+        project.columnModel.removeCellIndex(bigIndex);
+        project.columnModel.removeCellIndex(smallIndex);
+
+        for (Row row : newRows) {
+            row.cells.remove(bigIndex);
+            row.cells.remove(smallIndex);
+        }
+
         return new HistoryEntry(
             historyEntryID,
             project, 

diff --git a/main/tests/server/src/com/google/refine/operations/cell/TransposeTests.java b/main/tests/server/src/com/google/refine/operations/cell/TransposeTests.java
@@ -140,13 +140,15 @@ public void keyValueComumnize() throws Exception {
         Assert.assertEquals(project.columnModel.columns.get(3).getName(), "c");
         Assert.assertEquals(project.columnModel.columns.get(4).getName(), "d");
         Assert.assertEquals(project.rows.size(), 3);
-        Assert.assertEquals(project.rows.get(0).cells.size(), 5);
-        Assert.assertEquals(project.rows.get(1).cells.size(), 5);
+
+        // the last 2 cells are not added as expected, the size is 5-2
+        Assert.assertEquals(project.rows.get(0).cells.size(), 5 - 2);
+        Assert.assertEquals(project.rows.get(1).cells.size(), 5 - 1);
         Assert.assertEquals(project.rows.get(2).cells.size(), 5);
 
-        Assert.assertEquals(project.rows.get(0).cells.get(0).value, "data1");
-        Assert.assertEquals(project.rows.get(0).cells.get(1).value, "data2");
-        Assert.assertEquals(project.rows.get(0).cells.get(2).value, "data3");
+        Assert.assertEquals(project.rows.get(0).cells.get(0).value, "1");
+        Assert.assertEquals(project.rows.get(0).cells.get(1).value, "1");
+        Assert.assertEquals(project.rows.get(0).cells.get(2).value, "3");
     }