Reset Combobox internal state (#11412)

issue in #11343 and #11385 is not reproducible on top this patch
Three different bugs are involved in this fix:
we bring the old fix(#11094) back and fixed the other related issues:
1) allow adding the same new item after dataProvider got reset, This is cause by the client side parameter `LastNewItemString`, it saves the value added before resetting. 
2) clear the pending newItem eagerly, so that the same value will not be added again.

5 files changed, 309 insertions, 19 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/VComboBox.java b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
index fbac85c2af..50d4c6588c 100644
--- a/client/src/main/java/com/vaadin/client/ui/VComboBox.java
+++ b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
@@ -23,7 +23,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
-import java.util.UUID;
 import java.util.logging.Logger;
 
 import com.google.gwt.animation.client.AnimationScheduler;
@@ -122,7 +121,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
          *            icon URI or null
          */
         public ComboBoxSuggestion(String key, String caption, String style,
-                String untranslatedIconUri) {
+                                  String untranslatedIconUri) {
             this.key = key;
             this.caption = caption;
             this.style = style;
@@ -1642,6 +1641,11 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             performSelection(selectedKey, oldSuggestionTextMatchTheOldSelection,
                     !isWaitingForFilteringResponse() || popupOpenerClicked);
 
+            // currentSuggestion should be set to match the value of the
+            // ComboBox
+            resetCurrentSuggestionBasedOnServerResponse(selectedKey,
+                    selectedCaption, selectedIconUri);
+
             cancelPendingPostFiltering();
 
             setSelectedCaption(selectedCaption);
@@ -1649,6 +1653,22 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             setSelectedItemIcon(selectedIconUri);
         }
 
+        /*
+         * Updates the current suggestion based on values provided by the
+         * server.
+         */
+        private void resetCurrentSuggestionBasedOnServerResponse(
+                String selectedKey, String selectedCaption,
+                String selectedIconUri) {
+            if (currentSuggestion == null
+                    && (selectedKey != null || selectedCaption != null)) {
+                currentSuggestion = new ComboBoxSuggestion(selectedKey,
+                        selectedCaption, "", selectedIconUri);
+            } else if (selectedKey == null && selectedCaption == null) {
+                currentSuggestion = null;
+            }
+        }
+
     }
 
     // TODO decide whether this should change - affects themes and v7
@@ -1707,7 +1727,9 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
     /** For internal use only. May be removed or replaced in the future. */
     public boolean initDone = false;
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     */
     public String lastFilter = "";
 
     /**
@@ -1802,6 +1824,16 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
         return s.getMarginWidth() + s.getBorderWidth() + s.getPaddingWidth();
     }
 
+    /**
+     * This method will reset the saved item string which is added last time.
+     */
+    public void resetLastNewItemString() {
+        // Clean the temp string eagerly in order to re-add the same value again
+        // after data provider got reset.
+        // Fixes issue https://github.com/vaadin/framework/issues/11317
+        lastNewItemString = null;
+    }
+
     /*
      * (non-Javadoc)
      *
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 3aa58083cf..2ef091569d 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
@@ -130,11 +130,9 @@ public class ComboBoxConnector extends AbstractListingConnector
             getWidget().selectedOptionKey = null;
             getWidget().currentSuggestion = null;
         }
-        if (isNewItemStillPending()
-                && pendingNewItemValue == getState().selectedItemCaption) {
-            // no automated selection handling required
-            clearNewItemHandling();
-        }
+
+        clearNewItemHandlingIfMatch(getState().selectedItemCaption);
+
         getDataReceivedHandler().updateSelectionFromServer(
                 getState().selectedItemKey, getState().selectedItemCaption,
                 getState().selectedItemIcon);
@@ -188,10 +186,11 @@ public class ComboBoxConnector extends AbstractListingConnector
         if (itemValue != null && !itemValue.equals(pendingNewItemValue)) {
             // clear any previous handling as outdated
             clearNewItemHandling();
+
+            pendingNewItemValue = itemValue;
+            rpc.createNewItem(itemValue);
+            getDataReceivedHandler().clearPendingNavigation();
         }
-        pendingNewItemValue = itemValue;
-        rpc.createNewItem(itemValue);
-        getDataReceivedHandler().clearPendingNavigation();
     }
 
     /**
@@ -359,7 +358,7 @@ public class ComboBoxConnector extends AbstractListingConnector
 
         updateSuggestions(start, end);
         getWidget().setTotalSuggestions(getDataSource().size());
-
+        getWidget().resetLastNewItemString();
         getDataReceivedHandler().dataReceived();
     }
 
@@ -451,10 +450,7 @@ public class ComboBoxConnector extends AbstractListingConnector
      * versions.
      */
     private void clearNewItemHandling() {
-        // never clear pending value before it has been handled
-        if (!isNewItemStillPending()) {
-            pendingNewItemValue = null;
-        }
+        pendingNewItemValue = null;
     }
 
     /**
diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java
index a976782ecc..8fadca6c35 100644
--- a/server/src/main/java/com/vaadin/ui/ComboBox.java
+++ b/server/src/main/java/com/vaadin/ui/ComboBox.java
@@ -191,6 +191,15 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
                 if (getNewItemProvider() != null) {
                     Optional<T> item = getNewItemProvider().apply(itemValue);
                     added = item.isPresent();
+                    // Fixes issue https://github.com/vaadin/framework/issues/11343
+                    // Update the internal selection state immediately to avoid
+                    // client side hanging. This is needed for cases that user
+                    // interaction fires multi events (like adding and deleting)
+                    // on a new item during the same round trip.
+                    item.ifPresent(value -> {
+                        setSelectedItem(value, true);
+                        getDataCommunicator().reset();
+                    });
                 } else if (getNewItemHandler() != null) {
                     getNewItemHandler().accept(itemValue);
                     // Up to the user to tell if no item was added.
@@ -446,7 +455,7 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
      * @since 8.0
      */
     public void setDataProvider(CaptionFilter captionFilter,
-            ListDataProvider<T> listDataProvider) {
+                                ListDataProvider<T> listDataProvider) {
         Objects.requireNonNull(listDataProvider,
                 "List data provider cannot be null");
 
@@ -610,12 +619,11 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
      * <p>
      * The empty string {@code ""} is the default empty selection caption.
      *
+     * @return the empty selection caption, not {@code null}
      * @see #setEmptySelectionAllowed(boolean)
      * @see #isEmptySelectionAllowed()
      * @see #setEmptySelectionCaption(String)
      * @see #isSelected(Object)
-     *
-     * @return the empty selection caption, not {@code null}
      * @since 8.0
      */
     public String getEmptySelectionCaption() {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddNewItemAndResetProviderAtSameRound.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddNewItemAndResetProviderAtSameRound.java
new file mode 100644
index 0000000000..c5ea5a23ac
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddNewItemAndResetProviderAtSameRound.java
@@ -0,0 +1,99 @@
+package com.vaadin.tests.components.combobox;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.CheckBox;
+import com.vaadin.ui.ComboBox;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Notification;
+
+public class ComboBoxAddNewItemAndResetProviderAtSameRound
+        extends AbstractTestUIWithLog {
+
+    ComboBox<String> comboBox;
+    List<String> items = new ArrayList<>();
+    Label resetLabel = new Label("Reset Label");
+    Label valueLabel = new Label("Value Label");
+    CheckBox delay = new CheckBox("Slow adding process", false);
+
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        initItems();
+        comboBox = new ComboBox(null, items);
+        comboBox.setTextInputAllowed(true);
+        comboBox.setEmptySelectionAllowed(true);
+        comboBox.addValueChangeListener(event -> {
+            valueLabel.setValue(comboBox.getValue());
+            log("ComboBox value : " + comboBox.getValue());
+        });
+
+        configureNewItemHandling();
+
+        Button checkButton = new Button("Check ComboBox value", e -> {
+            Notification.show("selected: " + comboBox.getValue());
+        });
+
+        Button resetButton = new Button("Reset options", e -> {
+            comboBox.setValue(null);
+            initItems();
+            comboBox.getDataProvider().refreshAll();
+            resetLabel.setValue("Reset");
+            valueLabel.setValue("Value is reset");
+            log("DataProvider has been reset");
+        });
+
+        resetLabel.setId("reset-label");
+        valueLabel.setId("value-label");
+        delay.setId("delay");
+        resetButton.setId("reset");
+
+        Button button = new Button("Button for clicking only");
+        button.setId("button-for-click");
+
+        HorizontalLayout hl = new HorizontalLayout(checkButton, button);
+        addComponents(comboBox, resetLabel, valueLabel, hl, resetButton, delay);
+    }
+
+    private void configureNewItemHandling() {
+        comboBox.setNewItemProvider(text -> {
+            if (delay.getValue()) {
+                try {
+                    Thread.sleep(2000);
+                } catch (InterruptedException e1) {
+                    e1.printStackTrace();
+                }
+            }
+
+            items.add(text);
+            Collections.sort(items);
+
+            comboBox.getDataProvider().refreshAll();
+            log("New item has been added");
+            return Optional.of(text);
+        });
+    }
+
+    private void initItems() {
+        items.clear();
+        StringBuilder builder = new StringBuilder();
+        for (char c = 'a'; c <= 'z'; c++) {
+            for (int i = 0; i < 100; i++) {
+                builder.setLength(0);
+                items.add(builder.append(c).append(i).toString());
+            }
+        }
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11343;
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddNewItemAndResetProviderAtSameRoundTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddNewItemAndResetProviderAtSameRoundTest.java
new file mode 100644
index 0000000000..a1a7a1f685
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddNewItemAndResetProviderAtSameRoundTest.java
@@ -0,0 +1,155 @@
+package com.vaadin.tests.components.combobox;
+
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.interactions.Actions;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.CheckBoxElement;
+import com.vaadin.testbench.elements.ComboBoxElement;
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+import static org.junit.Assert.assertTrue;
+
+public class ComboBoxAddNewItemAndResetProviderAtSameRoundTest
+        extends SingleBrowserTest {
+
+    protected enum SelectionType {
+        ENTER, TAB, CLICK_OUT;
+    }
+
+    private ComboBoxElement comboBoxElement;
+    private LabelElement valueLabelElement;
+    private String inputValue = "000";
+
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+        openTestURL();
+        waitForElementPresent(By.className("v-filterselect"));
+        waitForElementPresent(By.id("reset-label"));
+        waitForElementPresent(By.id("value-label"));
+        comboBoxElement = $(ComboBoxElement.class).first();
+    }
+
+    /**
+     * Scenario: add new item and reset the data provider in the same round,
+     * then add the same value again with ENTER
+     */
+    @Test
+    public void addNewItemAndReset_reAddWithEnter() {
+        itemHandling(SelectionType.ENTER, inputValue);
+    }
+
+    /**
+     * Scenario: add new item and reset the data provider in the same round,
+     * then add the same value again with TAB
+     */
+    @Test
+    public void addNewItemAndReset_reAddWithTab() {
+        itemHandling(SelectionType.TAB, inputValue);
+    }
+
+    /**
+     * Scenario: add new item and reset the data provider in the same round,
+     * then add the same value again with clicking out side of the CB
+     */
+    @Test
+    public void addNewItemAndReset_reAddWithClickOut() {
+        itemHandling(SelectionType.CLICK_OUT, inputValue);
+    }
+
+    /**
+     * Scenario: add new item and reset the data provider in the same round with
+     * 2 seconds delay, then add the same value again with ENTER
+     */
+    @Test
+    public void slowAddNewItemAndReset_reAddWithEnter() {
+        delay(true);
+        itemHandling(SelectionType.ENTER, inputValue);
+    }
+
+    /**
+     * Scenario: add new item and reset the data provider in the same round with
+     * 2 seconds delay, then add the same value again with TAB
+     */
+    @Test
+    public void slowAddNewItemAndReset_reAddWithTab() {
+        delay(true);
+        itemHandling(SelectionType.TAB, inputValue);
+    }
+
+    /**
+     * Scenario: add new item and reset the data provider in the same round with
+     * 2 seconds delay, then add the same value again with clicking out side
+     */
+    @Test
+    public void slowAddNewItemAndReset_reAddWithClickOut() {
+        delay(true);
+        itemHandling(SelectionType.CLICK_OUT, inputValue);
+    }
+
+    private void itemHandling(SelectionType selectionType, String input) {
+        assertValueLabelText("Value Label");
+        sendKeysToInput(input);
+        sleep(1000);
+
+        // reset the dataProvider
+        reset();
+
+        // re-add the same value and select
+        sendKeysToInput(input);
+        performSelect(selectionType);
+
+        assertLogMessage();
+    }
+
+    private void assertLogMessage() {
+        sleep(2000);
+        //current test is not stable for collecting all the logs,
+        //so that we need to do the assertion with full log and contents.
+        assertTrue("The full log should contain the following text",
+                getLogs().toString().contains("ComboBox value : 000"));
+        assertTrue("The full log should contain the following text",
+                getLogs().toString().contains("New item has been added"));
+        assertTrue("The full log should contain the following text",
+                getLogs().toString().contains("DataProvider has been reset"));
+    }
+
+    private void sendKeysToInput(CharSequence... keys) {
+        new Actions(getDriver()).moveToElement(comboBoxElement).perform();
+        comboBoxElement.sendKeys(keys);
+    }
+
+    private void performSelect(SelectionType selectionType) {
+        switch (selectionType) {
+        case ENTER:
+            sendKeysToInput(Keys.ENTER);
+            break;
+        case TAB:
+            sendKeysToInput(Keys.TAB);
+            break;
+        case CLICK_OUT:
+            $(ButtonElement.class).id("button-for-click").click();
+            break;
+        }
+    }
+
+    private void assertValueLabelText(String value) {
+        valueLabelElement = $(LabelElement.class).id("value-label");
+        waitUntil(driver -> value.equals(valueLabelElement.getText()));
+    }
+
+    private void delay(boolean delay) {
+        CheckBoxElement checkBox = $(CheckBoxElement.class).id("delay");
+        if (delay != checkBox.isChecked()) {
+            checkBox.click();
+        }
+    }
+
+    private void reset() {
+        $(ButtonElement.class).id("reset").click();
+    }
+}