11642 refresh pagelength 0 combobox items after dataprovider update (#11653)

* Fixes #11642. ComboBox with pageLength 0 should be updated if DataProvider changes

* added comments, fixed imports

6 files changed, 223 insertions, 6 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java b/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java
index 2ef091569d..2709175c5c 100644
--- a/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java
+++ b/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java
@@ -62,6 +62,12 @@ public class ComboBoxConnector extends AbstractListingConnector
      */
     private String pendingNewItemValue = null;
 
+    /**
+     * If this flag is toggled, even unpaged data sources should be updated on
+     * reset.
+     */
+    private boolean forceDataSourceUpdate = false;
+
     @Override
     protected void init() {
         super.init();
@@ -123,6 +129,11 @@ public class ComboBoxConnector extends AbstractListingConnector
         getWidget().setEmptySelectionCaption(getState().emptySelectionCaption);
     }
 
+    @OnStateChange("forceDataSourceUpdate")
+    private void onForceDataSourceUpdate() {
+        forceDataSourceUpdate = getState().forceDataSourceUpdate;
+    }
+
     @OnStateChange({ "selectedItemKey", "selectedItemCaption",
             "selectedItemIcon" })
     private void onSelectionChange() {
@@ -503,9 +514,13 @@ public class ComboBoxConnector extends AbstractListingConnector
         @Override
         public void resetDataAndSize(int estimatedNewDataSize) {
             if (getState().pageLength == 0) {
-                if (getWidget().suggestionPopup.isShowing()) {
+                if (getWidget().suggestionPopup.isShowing()
+                        || forceDataSourceUpdate) {
                     dataSource.ensureAvailability(0, estimatedNewDataSize);
                 }
+                if (forceDataSourceUpdate) {
+                    rpc.resetForceDataSourceUpdate();
+                }
                 // else lets just wait till the popup is opened before
                 // everything is fetched to it. this could be optimized later on
                 // to fetch everything if in-memory data is used.
diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java
index c9622e1fd8..b9511450a5 100644
--- a/server/src/main/java/com/vaadin/ui/ComboBox.java
+++ b/server/src/main/java/com/vaadin/ui/ComboBox.java
@@ -28,17 +28,19 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Stream;
 
-import org.jsoup.nodes.Element;
-
-import com.vaadin.data.HasFilterableDataProvider;
-import com.vaadin.data.HasValue;
-import com.vaadin.data.ValueProvider;
 import com.vaadin.data.provider.CallbackDataProvider;
+import com.vaadin.data.provider.DataChangeEvent;
 import com.vaadin.data.provider.DataCommunicator;
 import com.vaadin.data.provider.DataGenerator;
 import com.vaadin.data.provider.DataKeyMapper;
 import com.vaadin.data.provider.DataProvider;
+import com.vaadin.data.provider.InMemoryDataProvider;
 import com.vaadin.data.provider.ListDataProvider;
+import org.jsoup.nodes.Element;
+
+import com.vaadin.data.HasFilterableDataProvider;
+import com.vaadin.data.HasValue;
+import com.vaadin.data.ValueProvider;
 import com.vaadin.event.FieldEvents;
 import com.vaadin.event.FieldEvents.BlurEvent;
 import com.vaadin.event.FieldEvents.BlurListener;
@@ -219,6 +221,11 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
             getState().currentFilterText = filterText;
             filterSlot.accept(filterText);
         }
+
+        @Override
+        public void resetForceDataSourceUpdate() {
+            getState().forceDataSourceUpdate = false;
+        }
     };
 
     /**
@@ -962,6 +969,20 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
 
         filterSlot = filter -> providerFilterSlot
                 .accept(convertOrNull.apply(filter));
+
+        // This workaround is done to fix issue #11642 for unpaged comboboxes.
+        // Data sources for on the client need to be updated after data provider
+        // refreshAll so that serverside selection works even before the dropdown
+        // is opened. Only done for in-memory data providers for performance
+        // reasons.
+        if (dataProvider instanceof InMemoryDataProvider) {
+            dataProvider.addDataProviderListener(event -> {
+                if ((!(event instanceof DataChangeEvent.DataRefreshEvent))
+                        && (getPageLength() == 0)) {
+                    getState().forceDataSourceUpdate = true;
+                }
+            });
+        }
     }
 
     /**
diff --git a/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxServerRpc.java b/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxServerRpc.java
index 32bcd64c43..8cd9fff9e9 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxServerRpc.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxServerRpc.java
@@ -40,4 +40,11 @@ public interface ComboBoxServerRpc extends ServerRpc {
      *            mode
      */
     public void setFilter(String filter);
+
+    /**
+     * Reset the force update flag once the list contents have been updated.
+     *
+     * @since
+     */
+    public void resetForceDataSourceUpdate();
 }
diff --git a/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java b/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java
index acd4fb0abc..a8ad60e33d 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java
@@ -25,6 +25,7 @@ import com.vaadin.shared.ui.AbstractSingleSelectState;
  * @since 7.0
  */
 public class ComboBoxState extends AbstractSingleSelectState {
+
     {
         // TODO ideally this would be v-combobox, but that would affect a lot of
         // themes
@@ -107,4 +108,11 @@ public class ComboBoxState extends AbstractSingleSelectState {
      */
     public String currentFilterText;
 
+    /**
+     * Ensure the data source is updated when backing dataprovider has been
+     * refreshed.
+     *
+     * @since
+     */
+    public boolean forceDataSourceUpdate;
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdate.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdate.java
new file mode 100644
index 0000000000..f361f8fb49
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdate.java
@@ -0,0 +1,116 @@
+package com.vaadin.tests.components.combobox;
+
+import java.util.ArrayList;
+import java.util.Objects;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.data.provider.ListDataProvider;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.ComboBox;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.VerticalLayout;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdate
+        extends AbstractTestUI {
+
+    private static final class Bean {
+        @Override
+        public boolean equals(Object o) {
+            if (this == o)
+                return true;
+            if (o == null || getClass() != o.getClass())
+                return false;
+            Bean bean = (Bean) o;
+            return Objects.equals(name, bean.name);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(name);
+        }
+
+        private String name;
+
+        public Bean(String name) {
+            setName(name);
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+    }
+
+    private ArrayList<Bean> list = new ArrayList<>();
+    private ArrayList<Bean> list2 = new ArrayList<>();
+
+    private int counter = 0;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        HorizontalLayout root = new HorizontalLayout();
+        root.setSizeFull();
+        VerticalLayout leftLayout = new VerticalLayout();
+        VerticalLayout rightLayout = new VerticalLayout();
+        root.addComponent(leftLayout);
+        root.addComponent(rightLayout);
+        addComponent(root);
+        final ComboBox<Bean> comboBoxPageLengthZero = new ComboBox<>();
+        comboBoxPageLengthZero.setId("combo-0");
+        comboBoxPageLengthZero.setPageLength(0);
+        final ComboBox<Bean> comboBoxRegular = new ComboBox<>();
+        comboBoxRegular.setId("combo-n");
+        comboBoxPageLengthZero.setItemCaptionGenerator(bean -> bean.getName());
+        comboBoxRegular.setItemCaptionGenerator(bean -> bean.getName());
+        ListDataProvider<Bean> dataProvider = new ListDataProvider<>(list);
+        ListDataProvider<Bean> dataProvider2 = new ListDataProvider<>(list2);
+        comboBoxPageLengthZero.setDataProvider(dataProvider);
+        comboBoxRegular.setDataProvider(dataProvider2);
+        createComponents(leftLayout, comboBoxPageLengthZero, dataProvider,
+                true);
+        createComponents(rightLayout, comboBoxRegular, dataProvider2, false);
+    }
+
+    private void createComponents(VerticalLayout layout,
+            ComboBox<Bean> comboBox, ListDataProvider<Bean> dataProvider,
+            boolean pageLengthZero) {
+        Button updateValues = new Button(
+                "1. Refresh backing list and dataprovider, select first item",
+                e -> {
+                    Bean selected = null;
+                    if (pageLengthZero) {
+                        list.clear();
+                        list.add(new Bean("foo" + (++counter)));
+                        list.add(new Bean("foo" + (++counter)));
+                        selected = list.get(0);
+                    } else {
+                        list2.clear();
+                        list2.add(new Bean("bar" + (++counter)));
+                        list2.add(new Bean("bar" + (++counter)));
+                        selected = list2.get(0);
+                    }
+                    dataProvider.refreshAll();
+                    comboBox.setValue(selected);
+                });
+
+        updateValues.setId(pageLengthZero ? "update-0" : "update-n");
+
+        Button refresh = new Button(
+                "2. Update item returned by combobox's getValue and call refreshItem on data provider",
+                e -> {
+                    Bean currentValue = comboBox.getValue();
+                    currentValue.setName(currentValue.getName() + "(updated)");
+                    dataProvider.refreshItem(currentValue);
+                });
+        refresh.setId(pageLengthZero ? "refresh-0" : "refresh-n");
+
+        layout.addComponents(updateValues, refresh, comboBox);
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdateTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdateTest.java
new file mode 100644
index 0000000000..8a4ba69c64
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdateTest.java
@@ -0,0 +1,50 @@
+package com.vaadin.tests.components.combobox;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.ComboBoxElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.openqa.selenium.By;
+
+public class ComboBoxPageLengthZeroRefreshItemAfterDataProviderUpdateTest
+        extends MultiBrowserTest {
+
+    @Test
+    public void refreshItemAfterDataProviderUpdateWithPageLengthZero() {
+        openTestURL();
+        waitForElementVisible(By.id("combo-0"));
+        ComboBoxElement pageLengthZeroCombo = $(ComboBoxElement.class)
+                .id("combo-0");
+        ButtonElement updateButton = $(ButtonElement.class).id("update-0");
+        ButtonElement refreshButton = $(ButtonElement.class).id("refresh-0");
+        String comboText = getComboBoxInputTextAfterUpdateAndRefresh(
+                pageLengthZeroCombo, updateButton, refreshButton);
+        assertEquals("Expected item containing (updated), got " + comboText,
+                true, comboText.contains("(updated)"));
+    }
+
+    @Test
+    public void refreshItemAfterDataProviderUpdateWithDefaultPageLength() {
+        openTestURL();
+        waitForElementVisible(By.id("combo-n"));
+        ComboBoxElement pageLengthRegularCombo = $(ComboBoxElement.class)
+                .id("combo-n");
+        ButtonElement updateButton = $(ButtonElement.class).id("update-n");
+        ButtonElement refreshButton = $(ButtonElement.class).id("refresh-n");
+        String comboText = getComboBoxInputTextAfterUpdateAndRefresh(
+                pageLengthRegularCombo, updateButton, refreshButton);
+        assertEquals("Expected item containing (updated), got " + comboText,
+                true, comboText.contains("(updated)"));
+    }
+
+    public String getComboBoxInputTextAfterUpdateAndRefresh(
+            ComboBoxElement combo, ButtonElement updateDataProvider,
+            ButtonElement refreshItem) {
+        updateDataProvider.click();
+        refreshItem.click();
+        return combo.getValue();
+    }
+}