Fix Grid not updating selected item immediately (#10569)

3 files changed, 31 insertions, 17 deletions

diff --git a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
index 2819543521..43dcab0271 100644
--- a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
+++ b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
@@ -91,8 +91,7 @@ public class DataCommunicator<T> extends AbstractExtension {
      * that are not in the given collection will be cleaned up and
      * {@link DataGenerator#destroyData(Object)} will be called for them.
      */
-    protected class ActiveDataHandler
-            implements DataGenerator<T> {
+    protected class ActiveDataHandler implements DataGenerator<T> {
 
         /**
          * Set of key strings for currently active data objects
@@ -754,16 +753,14 @@ public class DataCommunicator<T> extends AbstractExtension {
     private void attachDataProviderListener() {
         dataProviderUpdateRegistration = getDataProvider()
                 .addDataProviderListener(event -> {
-                    getUI().access(() -> {
-                        if (event instanceof DataRefreshEvent) {
-                            T item = ((DataRefreshEvent<T>) event).getItem();
-                            getKeyMapper().refresh(item);
-                            generators.forEach(g -> g.refreshData(item));
-                            refresh(item);
-                        } else {
-                            hardReset();
-                        }
-                    });
+                    if (event instanceof DataRefreshEvent) {
+                        T item = ((DataRefreshEvent<T>) event).getItem();
+                        getKeyMapper().refresh(item);
+                        generators.forEach(g -> g.refreshData(item));
+                        getUI().access(() -> refresh(item));
+                    } else {
+                        getUI().access(this::hardReset);
+                    }
                 });
     }
 
diff --git a/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java b/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java
index 9f1591407c..faec5084f7 100644
--- a/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java
+++ b/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java
@@ -3,16 +3,17 @@ package com.vaadin.tests.data;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 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.tests.components.AbstractTestUIWithLog;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Grid;
 
-public class DataProviderRefresh extends AbstractTestUI {
+public class DataProviderRefresh extends AbstractTestUIWithLog {
 
     public static class Bean implements Serializable {
 
@@ -96,9 +97,10 @@ public class DataProviderRefresh extends AbstractTestUI {
         }
     }
 
+    private Grid<Bean> grid = new Grid<>();
+
     @Override
     protected void setup(VaadinRequest request) {
-        Grid<Bean> grid = new Grid<>();
         List<Bean> arrayList = new ArrayList<>();
         Bean foo = new Bean("Foo", 10);
         arrayList.add(foo);
@@ -108,9 +110,19 @@ public class DataProviderRefresh extends AbstractTestUI {
         grid.setDataProvider(dataProvider);
         grid.addColumn(Object::toString).setCaption("toString");
         addComponent(grid);
-        addComponent(new Button("Replace item",
-                event -> dataProvider.refreshItem(new Bean("Bar", 10))));
+        addComponent(new Button("Replace item", event -> {
+            dataProvider.refreshItem(new Bean("Bar", 10));
+            logSelectedItem(grid.asSingleSelect().getValue());
+        }));
         addComponent(new Button("Select old", event -> grid.select(foo)));
+        grid.asSingleSelect().addValueChangeListener(e -> {
+            logSelectedItem(e.getValue());
+        });
     }
 
+    private void logSelectedItem(Bean bean) {
+        Optional.ofNullable(bean)
+                .map(b -> "Currently selected: " + b.toString())
+                .ifPresent(this::log);
+    }
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java b/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java
index bbb31535a8..03e566a741 100644
--- a/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java
@@ -2,8 +2,11 @@ package com.vaadin.tests.data;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
+import static org.hamcrest.CoreMatchers.*;
+
 import org.junit.Test;
 
 import com.vaadin.testbench.elements.ButtonElement;
@@ -21,12 +24,14 @@ public class DataProviderRefreshTest extends SingleBrowserTest {
         // Select item before replace
         $(ButtonElement.class).caption("Select old").first().click();
         assertTrue("Row should be selected", grid.getRow(0).isSelected());
+        assertThat(getLogRow(0), containsString("{ Foo, 10 }"));
 
         $(ButtonElement.class).caption("Replace item").first().click();
         assertTrue("Row should still be selected after item replace",
                 grid.getRow(0).isSelected());
         assertEquals("Grid content was not updated.", "{ Bar, 10 }",
                 grid.getCell(0, 0).getText());
+        assertThat(getLogRow(0), containsString("{ Bar, 10 }"));
 
         // Deselect row
         grid.getCell(0, 0).click();