Fix NPE in v7 compatibility Grid during datasource rebind (#11473)

Add DataChangeHandler removal in v7 Grid just as in v8 Grid.
Adding tests to the fix to verify, that NPE is not thrown.

diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
index 631f3bdb6511c8fbd091ba47ed1b0f2756061d40..1d2f88f57062ccad627671504a3fe57505cd5913 100755 (executable)
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java
@@ -4152,6 +4152,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
      * on initialization, but not after that.
      */
     private DataSource<T> dataSource;
+    private Registration changeHandler;
 
     /**
      * Currently available row range in DataSource.
@@ -7023,12 +7024,13 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
 
         selectionModel.reset();
 
-        if (this.dataSource != null) {
-            this.dataSource.addDataChangeHandler((DataChangeHandler) null);
+        if (changeHandler != null) {
+            changeHandler.remove();
+            changeHandler = null;
         }
 
         this.dataSource = dataSource;
-        dataSource.addDataChangeHandler(new DataChangeHandler() {
+               changeHandler = dataSource.addDataChangeHandler(new DataChangeHandler() {
                     @Override
                     public void dataUpdated(int firstIndex, int numberOfItems) {
                         escalator.getBody().refreshRows(firstIndex, numberOfItems);

diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7.java
new file mode 100644 (file)
index 0000000..28aec9c
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7.java
@@ -0,0 +1,42 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.v7.data.util.IndexedContainer;
+import com.vaadin.v7.ui.Grid;
+
+@Widgetset("com.vaadin.v7.Vaadin7WidgetSet")
+public class GridRebindDataSourceV7 extends AbstractTestUI {
+    private Grid grid;
+    private IndexedContainer container = new IndexedContainer();
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        container.addContainerProperty("name", String.class, null);
+        container.addItem("test").getItemProperty("name").setValue("test");
+        grid = new Grid();
+        grid.setContainerDataSource(container);
+        grid.setEditorEnabled(true);
+        addComponent(grid);
+
+        Button button = new Button("Change container",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(Button.ClickEvent event) {
+                        IndexedContainer container = new IndexedContainer();
+                        container.addContainerProperty("age", Integer.class,
+                                null);
+                        container.addItem("first").getItemProperty("age")
+                                .setValue(45);
+                        grid.removeAllColumns();
+                        grid.setContainerDataSource(container);
+                    }
+                });
+        button.setId("changeContainer");
+        addComponent(button);
+    }
+
+}

diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7Test.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7Test.java
new file mode 100644 (file)
index 0000000..817cc88
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7Test.java
@@ -0,0 +1,23 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Test;
+import org.openqa.selenium.By;
+
+public class GridRebindDataSourceV7Test extends MultiBrowserTest {
+
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+
+        setDebug(true);
+        openTestURL();
+        waitForElementPresent(By.className("v-grid"));
+    }
+
+    @Test
+    public void testNoNPE() {
+        findElement(By.id("changeContainer")).click();
+        assertNoErrorNotifications();
+    }
+}