Correctly use id to identify data when refreshing (#9398)

This patch refactors the internals of Grid single selection model
implementation.

Fixes #9380

2 files changed, 184 insertions, 0 deletions

diff --git a/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java b/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java
new file mode 100644
index 0000000000..4476073b6d
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java
@@ -0,0 +1,118 @@
+package com.vaadin.tests.data;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.Random;
+import java.util.stream.Stream;
+
+import com.vaadin.data.provider.AbstractDataProvider;
+import com.vaadin.data.provider.Query;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Grid;
+
+public class DataProviderRefresh extends AbstractTestUI {
+
+    public static class Bean implements Serializable {
+
+        private String value;
+        private final int id;
+
+        public Bean(String value, int id) {
+            this.value = value;
+            this.id = id;
+        }
+
+        public String getValue() {
+            return value;
+        }
+
+        public int getId() {
+            return id;
+        }
+
+        public void setValue(String value) {
+            this.value = value;
+        }
+
+        @Override
+        public String toString() {
+            return "{ " + value + ", " + id + " }";
+        }
+    }
+
+    /**
+     * A dummy data provider for testing item replacement and stale elements.
+     */
+    public class ReplaceListDataProvider
+            extends AbstractDataProvider<Bean, Void> {
+
+        private final List<Bean> backend;
+
+        public ReplaceListDataProvider(List<Bean> items) {
+            backend = items;
+        }
+
+        @Override
+        public void refreshItem(Bean item) {
+            if (replaceItem(item)) {
+                super.refreshItem(item);
+            }
+        }
+
+        private boolean replaceItem(Bean item) {
+            for (int i = 0; i < backend.size(); ++i) {
+                if (getId(backend.get(i)).equals(getId(item))) {
+                    if (backend.get(i).equals(item)) {
+                        return false;
+                    }
+                    backend.set(i, item);
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        @Override
+        public boolean isInMemory() {
+            return true;
+        }
+
+        @Override
+        public int size(Query<Bean, Void> t) {
+            return backend.size();
+        }
+
+        @Override
+        public Stream<Bean> fetch(Query<Bean, Void> query) {
+            return backend.stream().skip(query.getOffset())
+                    .limit(query.getLimit());
+        }
+
+        @Override
+        public Object getId(Bean item) {
+            return item.getId();
+        }
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Grid<Bean> grid = new Grid<>();
+        ArrayList<Bean> arrayList = new ArrayList<>();
+        Bean foo = new Bean("Foo", 10);
+        arrayList.add(foo);
+        arrayList.add(new Bean("Baz", 11));
+        ReplaceListDataProvider dataProvider = new ReplaceListDataProvider(
+                arrayList);
+        grid.setDataProvider(dataProvider);
+        grid.addColumn(Object::toString).setCaption("toString");
+        addComponent(grid);
+        addComponent(new Button("Replace item",
+                e -> dataProvider.refreshItem(new Bean("Bar", 10))));
+        addComponent(new Button("Select old", e -> grid.select(foo)));
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java b/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java
new file mode 100644
index 0000000000..f32ec543a8
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java
@@ -0,0 +1,66 @@
+package com.vaadin.tests.data;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class DataProviderRefreshTest extends SingleBrowserTest {
+
+    @Test
+    public void select_and_replace() {
+        openTestURL();
+        GridElement grid = $(GridElement.class).first();
+        Assert.assertFalse("Row should not be initially selected",
+                grid.getRow(0).isSelected());
+        // Select item before replace
+        $(ButtonElement.class).caption("Select old").first().click();
+        Assert.assertTrue("Row should be selected",
+                grid.getRow(0).isSelected());
+
+        $(ButtonElement.class).caption("Replace item").first().click();
+        Assert.assertTrue("Row should still be selected after item replace",
+                grid.getRow(0).isSelected());
+        Assert.assertEquals("Grid content was not updated.", "{ Bar, 10 }",
+                grid.getCell(0, 0).getText());
+
+        // Deselect row
+        grid.getCell(0, 0).click();
+        Assert.assertFalse("Row should be deselected after click",
+                grid.getRow(0).isSelected());
+
+        Assert.assertEquals("Second row was affected", "{ Baz, 11 }",
+                grid.getCell(1, 0).getText());
+    }
+
+    @Test
+    public void replace_and_select() {
+        openTestURL();
+        GridElement grid = $(GridElement.class).first();
+        Assert.assertFalse("Row should not be initially selected",
+                grid.getRow(0).isSelected());
+
+        // Replace item before select
+        $(ButtonElement.class).caption("Replace item").first().click();
+        Assert.assertFalse("Row should not be selected after item replace",
+                grid.getRow(0).isSelected());
+        Assert.assertEquals("Grid content was not updated.", "{ Bar, 10 }",
+                grid.getCell(0, 0).getText());
+
+        $(ButtonElement.class).caption("Select old").first().click();
+        Assert.assertTrue("Row should be selected",
+                grid.getRow(0).isSelected());
+        Assert.assertEquals("Grid content should not update.", "{ Bar, 10 }",
+                grid.getCell(0, 0).getText());
+
+        // Deselect row
+        grid.getCell(0, 0).click();
+        Assert.assertFalse("Row should be deselected after click",
+                grid.getRow(0).isSelected());
+
+        Assert.assertEquals("Second row was affected", "{ Baz, 11 }",
+                grid.getCell(1, 0).getText());
+    }
+}