Fixed NPE after columns have been removed #13334

The indexes were never updated for the columns so after columns
was removed and new data was fetched an ArrayOutOfBounds exception
could occur. To fix this we use the id of the column to resolve the
index whenever a cell value is needed to ensure the correct data is
always retrived for the correct column without needing to keep track
of column indexes.

Change-Id: I8dc6fd4bb9f1cf917d7a6beea30a4a0230eea074

2 files changed, 37 insertions, 5 deletions

diff --git a/client/src/com/vaadin/client/ui/grid/GridConnector.java b/client/src/com/vaadin/client/ui/grid/GridConnector.java
index e55a71cb2e..8100cd875a 100644
--- a/client/src/com/vaadin/client/ui/grid/GridConnector.java
+++ b/client/src/com/vaadin/client/ui/grid/GridConnector.java
@@ -51,15 +51,26 @@ public class GridConnector extends AbstractComponentConnector {
      */
     private class CustomGridColumn extends GridColumn<String, String[]> {
 
-        private final int columnIndex;
+        private final String id;
 
-        public CustomGridColumn(int columnIndex) {
-            this.columnIndex = columnIndex;
+        public CustomGridColumn(String id) {
+            this.id = id;
         }
 
         @Override
         public String getValue(String[] obj) {
-            return obj[columnIndex];
+            return obj[resolveCurrentIndexFromState()];
+        }
+
+        private int resolveCurrentIndexFromState() {
+            List<GridColumnState> columns = getState().columns;
+            int numColumns = columns.size();
+            for (int index = 0; index < numColumns; index++) {
+                if (columns.get(index).id.equals(id)) {
+                    return index;
+                }
+            }
+            return -1;
         }
     }
 
@@ -203,7 +214,7 @@ public class GridConnector extends AbstractComponentConnector {
      */
     private void addColumnFromStateChangeEvent(int columnIndex) {
         GridColumnState state = getState().columns.get(columnIndex);
-        CustomGridColumn column = new CustomGridColumn(columnIndex);
+        CustomGridColumn column = new CustomGridColumn(state.id);
         columnIdToColumn.put(state.id, column);
 
         // Adds a column to grid, and registers Grid with the column.
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridBasicFeaturesTest.java b/uitest/src/com/vaadin/tests/components/grid/GridBasicFeaturesTest.java
index 7163fbea75..73bef67c32 100644
--- a/uitest/src/com/vaadin/tests/components/grid/GridBasicFeaturesTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/GridBasicFeaturesTest.java
@@ -15,6 +15,8 @@
  */
 package com.vaadin.tests.components.grid;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsNot.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -167,6 +169,25 @@ public class GridBasicFeaturesTest extends MultiBrowserTest {
     }
 
     @Test
+    public void testDataLoadingAfterRowRemoval() throws Exception {
+        openTestURL();
+
+        // Remove columns 2,3,4
+        selectMenuPath("Component", "Columns", "Column2", "Remove");
+        selectMenuPath("Component", "Columns", "Column3", "Remove");
+        selectMenuPath("Component", "Columns", "Column4", "Remove");
+
+        // Scroll so new data is lazy loaded
+        scrollGridVerticallyTo(1000);
+
+        // Let lazy loading do its job
+        sleep(1000);
+
+        // Check that row is loaded
+        assertThat(getBodyCellByRowAndColumn(11, 1).getText(), not("..."));
+    }
+
+    @Test
     public void testFreezingColumn() throws Exception {
         openTestURL();