From 7cd94ce0b02bb4ecd7826283d3928e3a66e39301 Mon Sep 17 00:00:00 2001 From: Tobse <1190109+TobseF@users.noreply.github.com> Date: Wed, 27 Mar 2019 18:59:08 +0100 Subject: 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. --- .../java/com/vaadin/v7/client/widgets/Grid.java | 8 +++-- .../components/grid/GridRebindDataSourceV7.java | 42 ++++++++++++++++++++++ .../grid/GridRebindDataSourceV7Test.java | 23 ++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridRebindDataSourceV7Test.java 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 631f3bdb65..1d2f88f570 100755 --- 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 extends ResizeComposite implements HasSelectionHandlers, * on initialization, but not after that. */ private DataSource dataSource; + private Registration changeHandler; /** * Currently available row range in DataSource. @@ -7023,12 +7024,13 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, 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 index 0000000000..28aec9c596 --- /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 index 0000000000..817cc88032 --- /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(); + } +} -- cgit v1.2.3