From 773151380e4ce3b2b911cf8076ea19fbff70ade4 Mon Sep 17 00:00:00 2001
From: Qi Cui <jackyq2015@gmail.com>
Date: Wed, 24 Aug 2016 13:56:35 -0400
Subject: [PATCH] fix #1138. column transpose

---
 .../com/google/refine/model/ColumnModel.java  |  8 -----
 .../cell/KeyValueColumnizeOperation.java      | 14 +++-----
 .../operations/cell/TransposeTests.java       | 35 +++++++++++--------
 3 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/main/src/com/google/refine/model/ColumnModel.java b/main/src/com/google/refine/model/ColumnModel.java
index 5f3df65a0..f310d3807 100644
--- a/main/src/com/google/refine/model/ColumnModel.java
+++ b/main/src/com/google/refine/model/ColumnModel.java
@@ -81,14 +81,6 @@ public class ColumnModel implements Jsonizable {
         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
index 16a7c6e14..7ca3380cf 100644
--- a/main/src/com/google/refine/operations/cell/KeyValueColumnizeOperation.java
+++ b/main/src/com/google/refine/operations/cell/KeyValueColumnizeOperation.java
@@ -251,16 +251,10 @@ public class KeyValueColumnizeOperation extends AbstractOperation {
         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);
+        // clean up the empty rows 
+        for (int i = newRows.size() - 1;i>=0;i--) {
+            if (newRows.get(i).isEmpty())
+                newRows.remove(i);
         }
         
         return new HistoryEntry(
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
index 5354a7f45..b4d9cb766 100644
--- a/main/tests/server/src/com/google/refine/operations/cell/TransposeTests.java
+++ b/main/tests/server/src/com/google/refine/operations/cell/TransposeTests.java
@@ -126,13 +126,11 @@ public class TransposeTests extends RefineTest {
         
         HistoryEntry historyEntry = process.performImmediate();
             
-        // Expected output
-        
-//        ID;a;b;c;d
-//        1;1;3;;
-//        2;;4;5;
-//        3;2;5;;3
-        
+        // Expected output from the GUI. 
+        // ID;a;b;c;d
+        // 1;1;3;;
+        // 2;;4;5;
+        // 3;2;5;;3
         Assert.assertEquals(project.columnModel.columns.size(), 5);
         Assert.assertEquals(project.columnModel.columns.get(0).getName(), "ID");
         Assert.assertEquals(project.columnModel.columns.get(1).getName(), "a");
@@ -141,14 +139,23 @@ public class TransposeTests extends RefineTest {
         Assert.assertEquals(project.columnModel.columns.get(4).getName(), "d");
         Assert.assertEquals(project.rows.size(), 3);
         
-        // 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);
-        
+        // The actual row data structure has to leave the columns model untouched for redo/undo purpose.
+        // So we have 2 empty columns(column 1,2) on the row level.
+        // 1;1;3;;
         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");
+        Assert.assertEquals(project.rows.get(0).cells.get(3).value, "1");
+        Assert.assertEquals(project.rows.get(0).cells.get(4).value, "3");
+        
+        // 2;;4;5;
+        Assert.assertEquals(project.rows.get(1).cells.get(0).value, "2");
+        Assert.assertEquals(project.rows.get(1).cells.get(4).value, "4");
+        Assert.assertEquals(project.rows.get(1).cells.get(5).value, "5");
+        
+        // 3;2;5;;3
+        Assert.assertEquals(project.rows.get(2).cells.get(0).value, "3");
+        Assert.assertEquals(project.rows.get(2).cells.get(3).value, "2");
+        Assert.assertEquals(project.rows.get(2).cells.get(4).value, "5");
+        Assert.assertEquals(project.rows.get(2).cells.get(6).value, "3");
     }