Fix RadioButtonGroup selection updates to client (#9749)

This patch provides a simple fix for the majority of issues. There are still issues that should be fixes by refactoring parts of the logic in AbstractSingleSelect.

This patch does not unify the handling of empty values in the TestBench elements of various AbstractSingleSelects.

Fixes #9494

2 files changed, 196 insertions, 0 deletions

diff --git a/uitest/src/main/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelection.java b/uitest/src/main/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelection.java
new file mode 100644
index 0000000000..55f61b4134
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelection.java
@@ -0,0 +1,68 @@
+package com.vaadin.tests.components.abstractsingleselect;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.data.provider.Query;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.AbstractSingleSelect;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.ComboBox;
+import com.vaadin.ui.NativeSelect;
+import com.vaadin.ui.RadioButtonGroup;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+@SuppressWarnings({ "unchecked", "rawtypes" })
+public class AbstractSingleSelection extends AbstractTestUI {
+
+    /* Initial placeholder component */
+    AbstractSingleSelect<String> component = new ComboBox<>();
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        NativeSelect<Class<? extends AbstractSingleSelect>> componentSelect = new NativeSelect<>();
+        componentSelect.setItems(RadioButtonGroup.class, NativeSelect.class,
+                ComboBox.class);
+        componentSelect.setItemCaptionGenerator(Class::getSimpleName);
+
+        componentSelect.setEmptySelectionAllowed(false);
+        componentSelect.addValueChangeListener(singleSelectClass -> {
+            createComponent(singleSelectClass.getValue());
+        });
+
+        addComponent(componentSelect);
+        addComponent(component); // This will be replaced in createComponent
+        addComponent(
+                new Button("Deselect", e -> component.setSelectedItem(null)));
+        addComponent(new Button("Select Bar",
+                e -> component.setSelectedItem("Bar")));
+        addComponent(new Button("Refresh",
+                e -> component.getDataProvider().refreshAll()));
+
+        // Select a value from native select to create the initial component
+        componentSelect.getDataProvider().fetch(new Query<>()).findFirst()
+                .ifPresent(componentSelect::setSelectedItem);
+    }
+
+    private void createComponent(
+            Class<? extends AbstractSingleSelect> singleSelectClass) {
+        try {
+            AbstractSingleSelect<String> select = singleSelectClass
+                    .newInstance();
+            select.setItems("Foo", "Bar", "Baz", "Reset");
+            select.setSelectedItem("Bar");
+
+            select.addValueChangeListener(e -> {
+                if ("Reset".equals(e.getValue())) {
+                    select.setSelectedItem("Bar");
+                }
+            });
+
+            select.setId("testComponent");
+
+            replaceComponent(component, select);
+            component = select;
+        } catch (InstantiationException | IllegalAccessException e1) {
+            throw new RuntimeException("Component creation failed", e1);
+        }
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelectionTest.java b/uitest/src/test/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelectionTest.java
new file mode 100644
index 0000000000..4623dac714
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelectionTest.java
@@ -0,0 +1,128 @@
+package com.vaadin.tests.components.abstractsingleselect;
+
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized.Parameters;
+import org.openqa.selenium.NoSuchElementException;
+
+import com.vaadin.testbench.elements.AbstractSingleSelectElement;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.ComboBoxElement;
+import com.vaadin.testbench.elements.NativeSelectElement;
+import com.vaadin.testbench.elements.RadioButtonGroupElement;
+import com.vaadin.tests.tb3.ParameterizedTB3Runner;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+@RunWith(ParameterizedTB3Runner.class)
+public class AbstractSingleSelectionTest extends SingleBrowserTest {
+
+    private static final Map<String, Class<? extends AbstractSingleSelectElement>> elementClasses = new LinkedHashMap<>();
+
+    @Parameters
+    public static Collection<String> getElementClassNames() {
+        if (elementClasses.isEmpty()) {
+            elementClasses.put("RadioButtonGroup",
+                    RadioButtonGroupElement.class);
+            elementClasses.put("NativeSelect", NativeSelectElement.class);
+            elementClasses.put("ComboBox", ComboBoxElement.class);
+        }
+
+        return elementClasses.keySet();
+    }
+
+    private String elementClassName;
+
+    public void setElementClassName(String elementClassName) {
+        this.elementClassName = elementClassName;
+    }
+
+    @Before
+    public void before() {
+        openTestURL();
+    }
+
+    @Test
+    public void testSelectNull() {
+        $(NativeSelectElement.class).first().selectByText(elementClassName);
+
+        assertInitial();
+
+        $(ButtonElement.class).caption("Deselect").first().click();
+
+        AbstractSingleSelectElement selectElement = getSelectElement();
+        // TODO: TB API behavior should be unified.
+        if (selectElement instanceof RadioButtonGroupElement) {
+            Assert.assertNull("No value should be selected",
+                    selectElement.getValue());
+        } else if (selectElement instanceof ComboBoxElement) {
+            Assert.assertTrue("No value should be selected",
+                    selectElement.getValue().isEmpty());
+        } else {
+            // NativeSelectElement throws if no value is selected.
+            try {
+                selectElement.getValue();
+                Assert.fail("No value should be selected");
+            } catch (NoSuchElementException e) {
+                // All is fine.
+            }
+        }
+    }
+
+    @Test
+    public void testSelectOnClientAndRefresh() {
+        $(NativeSelectElement.class).first().selectByText(elementClassName);
+
+        assertInitial();
+
+        AbstractSingleSelectElement select = getSelectElement();
+        select.selectByText("Baz");
+        Assert.assertEquals("Value should change", "Baz", select.getValue());
+
+        $(ButtonElement.class).caption("Refresh").first().click();
+        Assert.assertEquals("Value should stay the same through refreshAll",
+                "Baz", select.getValue());
+    }
+
+    @Test
+    public void testSelectOnClientAndResetValueOnServer() {
+        $(NativeSelectElement.class).first().selectByText(elementClassName);
+
+        assertInitial();
+
+        AbstractSingleSelectElement select = getSelectElement();
+        select.selectByText("Baz");
+        Assert.assertEquals("Value should change", "Baz", select.getValue());
+
+        $(ButtonElement.class).caption("Select Bar").first().click();
+        Assert.assertEquals("Original value should be selected again", "Bar",
+                select.getValue());
+    }
+
+    @Test
+    public void testSelectOnClientAndResetValueOnServerInListener() {
+        $(NativeSelectElement.class).first().selectByText(elementClassName);
+
+        assertInitial();
+
+        AbstractSingleSelectElement rbg = getSelectElement();
+        rbg.selectByText("Reset");
+        // Selecting "Reset" selects "Bar" on server. Value was initially "Bar"
+        Assert.assertEquals("Original value should be selected again", "Bar",
+                rbg.getValue());
+    }
+
+    private void assertInitial() {
+        Assert.assertEquals("Initial state unexpected", "Bar",
+                getSelectElement().getValue());
+    }
+
+    private AbstractSingleSelectElement getSelectElement() {
+        return $(elementClasses.get(elementClassName)).id("testComponent");
+    }
+}