Refactoring DataCommunicator code (#11304)

This refactoring addresses two issues cased by dropping updatedData before it was processed. Issues arise , when visibility has change.

Fixes #11274 and similar issue within RadioButton

There is no reliable way to test automatically NativeSelects in Grid, but adding UI test, at least

4 files changed, 156 insertions, 9 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 bb76d7cdcb..449cbc5711 100644
--- a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
+++ b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
@@ -358,6 +358,15 @@ public class DataCommunicator<T> extends AbstractExtension {
             rpc.reset(getDataProviderSize());
         }
 
+        if (!updatedData.isEmpty()) {
+            JsonArray dataArray = Json.createArray();
+            int i = 0;
+            for (T data : updatedData) {
+                dataArray.set(i++, getDataObject(data));
+            }
+            rpc.updateData(dataArray);
+        }
+
         Range requestedRows = getPushRows();
         boolean triggerReset = false;
         if (!requestedRows.isEmpty()) {
@@ -373,15 +382,6 @@ public class DataCommunicator<T> extends AbstractExtension {
             pushData(offset, rowsToPush);
         }
 
-        if (!updatedData.isEmpty()) {
-            JsonArray dataArray = Json.createArray();
-            int i = 0;
-            for (T data : updatedData) {
-                dataArray.set(i++, getDataObject(data));
-            }
-            rpc.updateData(dataArray);
-        }
-
         setPushRows(Range.withLength(0, 0));
         reset = triggerReset;
         updatedData.clear();
diff --git a/uitest/src/main/java/com/vaadin/tests/components/nativeselect/NativeSelectsInGrid.java b/uitest/src/main/java/com/vaadin/tests/components/nativeselect/NativeSelectsInGrid.java
new file mode 100644
index 0000000000..55efd7d1f7
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/nativeselect/NativeSelectsInGrid.java
@@ -0,0 +1,71 @@
+package com.vaadin.tests.components.nativeselect;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.NativeSelect;
+import com.vaadin.ui.VerticalLayout;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Button;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class NativeSelectsInGrid extends AbstractTestUI {
+    @Override
+    protected void setup(VaadinRequest request) {
+        final VerticalLayout layout = new VerticalLayout(new Label(
+                "Keep opening the editor and selecting an enum value. Eventually the list will not show "
+                        + "all options"),
+                createGrid());
+        NativeSelect<Enum> ns = new NativeSelect<>();
+        ns.setEmptySelectionAllowed(false);
+        ns.setItems(Enum.values());
+
+        NativeSelect<Enum> ns2 = new NativeSelect<>();
+        ns2.setItems(Enum.values());
+        addComponent(ns);
+        addComponent(ns2);
+        addComponent(new Button("Change visibility of NS", e -> {
+            ns2.setVisible(!ns2.isVisible());
+        }));
+        addComponent(layout);
+
+    }
+
+    public Component createGrid() {
+        Grid<Person> grid = new Grid<>();
+        grid.setItems(new Person(Enum.baz), new Person(Enum.foo),
+                new Person(Enum.bizzle));
+        grid.setSizeFull();
+        grid.setSelectionMode(Grid.SelectionMode.NONE);
+
+        NativeSelect<Enum> ns = new NativeSelect<>();
+        ns.setEmptySelectionAllowed(false);
+        ns.setItems(Enum.values());
+        grid.addColumn(Person::getEnumValue).setCaption("Enum value")
+                .setEditorComponent(ns, Person::setEnumValue);
+        grid.getEditor().setEnabled(true);
+        return grid;
+    }
+
+    enum Enum {
+        foo, bar, baz, bizzle, quux
+    }
+
+    public static class Person {
+        Enum enumValue;
+
+        public Person(Enum progress) {
+            this.enumValue = progress;
+        }
+
+        public Enum getEnumValue() {
+            return enumValue;
+        }
+
+        public void setEnumValue(Enum enumValue) {
+            this.enumValue = enumValue;
+        }
+    }
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChange.java b/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChange.java
new file mode 100644
index 0000000000..8e1d985171
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChange.java
@@ -0,0 +1,35 @@
+package com.vaadin.tests.components.radiobuttongroup;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.RadioButtonGroup;
+
+import java.util.Arrays;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class RadioButtonGroupAfterVisibilityChange extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        RadioButtonGroup<Boolean> radio = new RadioButtonGroup<>("Radio",
+                Arrays.asList(true, false));
+        radio.setId("radioButton");
+        addComponent(radio);
+
+        Button hideButton = new Button("Hide");
+        hideButton.setId("hideB");
+        hideButton.addClickListener(event1 -> radio.setVisible(false));
+        addComponent(hideButton);
+
+        Button setAndShowButton = new Button("Set and Show");
+        setAndShowButton.setId("setAndShow");
+        setAndShowButton.addClickListener(event1 -> {
+            radio.setValue(true);
+            radio.setVisible(true);
+        });
+        addComponent(setAndShowButton);
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChangeTest.java b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChangeTest.java
new file mode 100644
index 0000000000..eff1798e79
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChangeTest.java
@@ -0,0 +1,41 @@
+package com.vaadin.tests.components.radiobuttongroup;
+
+import com.vaadin.testbench.elements.RadioButtonGroupElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import java.util.List;
+
+import static junit.framework.TestCase.assertEquals;
+import static junit.framework.TestCase.assertTrue;
+
+public class RadioButtonGroupAfterVisibilityChangeTest
+        extends MultiBrowserTest {
+
+    @Test
+    public void verifyOptionIsSelectable() {
+        openTestURL();
+
+        getRadioButtonGroupElement().selectByText("false");
+        findElement(By.id("hideB")).click();
+        findElement(By.id("setAndShow")).click();
+        isSelectedOnClientSide("true");
+        getRadioButtonGroupElement().selectByText("false");
+        isSelectedOnClientSide("false");
+    }
+
+    private RadioButtonGroupElement getRadioButtonGroupElement() {
+        RadioButtonGroupElement radioButtonGroup = $(
+                RadioButtonGroupElement.class).first();
+        return radioButtonGroup;
+    }
+
+    private void isSelectedOnClientSide(String selectedText) {
+        List<WebElement> selectOptions = findElement(By.id("radioButton"))
+                .findElements(By.className("v-select-option-selected"));
+        assertEquals("Item should be selected", selectOptions.size(), 1);
+        assertTrue(selectOptions.get(0).getText().equals(selectedText));
+    }
+}