Implement NewItemProvider to replace NewItemHandler (#10606)

4 files changed, 156 insertions, 33 deletions

diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java
index f8468d1acd..9500d08c96 100644
--- a/server/src/main/java/com/vaadin/ui/ComboBox.java
+++ b/server/src/main/java/com/vaadin/ui/ComboBox.java
@@ -22,7 +22,10 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.stream.Stream;
 
 import org.jsoup.nodes.Element;
@@ -70,8 +73,8 @@ import elemental.json.JsonObject;
  */
 @SuppressWarnings("serial")
 public class ComboBox<T> extends AbstractSingleSelect<T>
-        implements FieldEvents.BlurNotifier,
-        FieldEvents.FocusNotifier, HasFilterableDataProvider<T, String> {
+        implements FieldEvents.BlurNotifier, FieldEvents.FocusNotifier,
+        HasFilterableDataProvider<T, String> {
 
     /**
      * A callback method for fetching items. The callback is provided with a
@@ -118,12 +121,28 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
      * </p>
      *
      * @since 8.0
+     * @deprecated Since 8.4 replaced by {@link NewItemProvider}.
      */
+    @Deprecated
     @FunctionalInterface
     public interface NewItemHandler extends SerializableConsumer<String> {
     }
 
     /**
+     * Provider function that adds a new item based on user input when the new
+     * items allowed mode is active. After the new item handling is complete,
+     * this function should return {@code Optional.of(text)} for the completion
+     * of automatic selection handling. If automatic selection is not wished
+     * for, always return {@code Optional.isEmpty()}.
+     *
+     * @since 8.4
+     */
+    @FunctionalInterface
+    public interface NewItemProvider<T>
+            extends SerializableFunction<String, Optional<T>> {
+    }
+
+    /**
      * Item style generator class for declarative support.
      * <p>
      * Provides a straightforward mapping between an item and its style.
@@ -166,7 +185,15 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
         public void createNewItem(String itemValue) {
             // New option entered
             if (itemValue != null && !itemValue.isEmpty()) {
-                if (getNewItemHandler() != null) {
+                if (getNewItemProvider() != null) {
+                    Optional<T> item = getNewItemProvider().apply(itemValue);
+                    if (!item.isPresent()) {
+                        // ensure the client resets the value to previous
+                        // selection
+                        getRpcProxy(ComboBoxClientRpc.class)
+                                .newItemNotAdded(itemValue);
+                    }
+                } else if (getNewItemHandler() != null) {
                     getNewItemHandler().accept(itemValue);
                 } else {
                     // selection handling is needed at the client even if
@@ -187,8 +214,14 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
     /**
      * Handler for new items entered by the user.
      */
+    @Deprecated
     private NewItemHandler newItemHandler;
 
+    /**
+     * Provider function for new items entered by the user.
+     */
+    private NewItemProvider<T> newItemProvider;
+
     private StyleGenerator<T> itemStyleGenerator = item -> null;
 
     private String currentFilterText;
@@ -441,9 +474,11 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
 
     /**
      * Returns true if the user can enter text into the field to either filter
-     * the selections or enter a new value if new item handler is set (see
-     * {@link #setNewItemHandler(NewItemHandler)}. If text input is disabled,
-     * the comboBox will work in the same way as a {@link NativeSelect}
+     * the selections or enter a new value if new item provider or handler is
+     * set (see {@link #setNewItemProvider(NewItemProvider)} (recommended) and
+     * {@link #setNewItemHandler(NewItemHandler)} (deprecated)). If text input
+     * is disabled, the comboBox will work in the same way as a
+     * {@link NativeSelect}
      *
      * @return true if text input is allowed
      */
@@ -667,17 +702,42 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
 
     /**
      * Sets the handler that is called when user types a new item. The creation
-     * of new items is allowed when a new item handler has been set.
+     * of new items is allowed when a new item handler has been set. If new item
+     * provider is also set, the new item handler is ignored.
      *
      * @param newItemHandler
      *            handler called for new items, null to only permit the
-     *            selection of existing items
+     *            selection of existing items, all options ignored if new item
+     *            provider is set
      * @since 8.0
+     * @deprecated Since 8.4 use {@link #setNewItemProvider(NewItemProvider)}
+     *             instead.
      */
+    @Deprecated
     public void setNewItemHandler(NewItemHandler newItemHandler) {
+        getLogger().log(Level.WARNING,
+                "NewItemHandler is deprecated. Please use NewItemProvider instead.");
         this.newItemHandler = newItemHandler;
-        getState().allowNewItems = newItemHandler != null;
-        markAsDirty();
+        getState(true).allowNewItems = newItemProvider != null
+                || newItemHandler != null;
+    }
+
+    /**
+     * Sets the provider function that is called when user types a new item. The
+     * creation of new items is allowed when a new item provider has been set.
+     * If a deprecated new item handler is also set it is ignored in favor of
+     * new item provider.
+     *
+     * @param newItemProvider
+     *            provider function that is called for new items, null to only
+     *            permit the selection of existing items or to use a deprecated
+     *            new item handler if set
+     * @since 8.4
+     */
+    public void setNewItemProvider(NewItemProvider<T> newItemProvider) {
+        this.newItemProvider = newItemProvider;
+        getState(true).allowNewItems = newItemProvider != null
+                || newItemHandler != null;
     }
 
     /**
@@ -685,11 +745,24 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
      * in the data provider).
      *
      * @return new item handler or null if none specified
+     * @deprecated Since 8.4 use {@link #getNewItemProvider()} instead.
      */
+    @Deprecated
     public NewItemHandler getNewItemHandler() {
         return newItemHandler;
     }
 
+    /**
+     * Returns the provider function that is called when the user enters a new
+     * item (not present in the data provider).
+     *
+     * @since 8.4
+     * @return new item provider or null if none specified
+     */
+    public NewItemProvider<T> getNewItemProvider() {
+        return newItemProvider;
+    }
+
     // HasValue methods delegated to the selection model
 
     @Override
@@ -871,4 +944,8 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
         @Override
         public boolean test(String itemCaption, String filterText);
     }
+
+    private static Logger getLogger() {
+        return Logger.getLogger(ComboBox.class.getName());
+    }
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxNewItemProvider.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxNewItemProvider.java
new file mode 100644
index 0000000000..71f8503216
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxNewItemProvider.java
@@ -0,0 +1,35 @@
+package com.vaadin.tests.components.combobox;
+
+import java.util.Collections;
+import java.util.Optional;
+
+public class ComboBoxNewItemProvider
+        extends ComboBoxSelectingNewItemValueChange {
+
+    @Override
+    protected void configureNewItemHandling() {
+        comboBox.setNewItemProvider(text -> {
+            if (Boolean.TRUE.equals(delay.getValue())) {
+                try {
+                    Thread.sleep(2000);
+                } catch (InterruptedException e1) {
+                    e1.printStackTrace();
+                }
+            }
+            if (Boolean.TRUE.equals(reject.getValue())) {
+                valueChangeLabel.setValue("item " + text + " discarded");
+                return Optional.empty();
+            } else {
+                items.add(text);
+                Collections.sort(items);
+                valueChangeLabel
+                        .setValue("adding new item... count: " + items.size());
+                comboBox.getDataProvider().refreshAll();
+                if (Boolean.TRUE.equals(noSelection.getValue())) {
+                    return Optional.empty();
+                }
+            }
+            return Optional.of(text);
+        });
+    }
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChange.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChange.java
index a65e34be63..a6f5e8e297 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChange.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChange.java
@@ -16,7 +16,7 @@ import com.vaadin.ui.Notification;
 
 public class ComboBoxSelectingNewItemValueChange extends ComboBoxSelecting {
 
-    private final class CustomComboBox extends ComboBox<String> {
+    final class CustomComboBox extends ComboBox<String> {
         private CustomComboBox(String caption, Collection<String> options) {
             super(caption, options);
         }
@@ -52,28 +52,7 @@ public class ComboBoxSelectingNewItemValueChange extends ComboBoxSelecting {
             }
         });
 
-        comboBox.setNewItemHandler(text -> {
-            if (Boolean.TRUE.equals(delay.getValue())) {
-                try {
-                    Thread.sleep(2000);
-                } catch (InterruptedException e1) {
-                    e1.printStackTrace();
-                }
-            }
-            if (Boolean.TRUE.equals(reject.getValue())) {
-                valueChangeLabel.setValue("item " + text + " discarded");
-                comboBox.getComboBoxClientRpc().newItemNotAdded(text);
-            } else {
-                items.add(text);
-                Collections.sort(items);
-                valueChangeLabel
-                        .setValue("adding new item... count: " + items.size());
-                if (Boolean.TRUE.equals(noSelection.getValue())) {
-                    comboBox.getComboBoxClientRpc().newItemNotAdded(text);
-                }
-                comboBox.getDataProvider().refreshAll();
-            }
-        });
+        configureNewItemHandling();
 
         comboBox.addValueChangeListener(e -> {
             ++valueChangeEventCount;
@@ -110,6 +89,32 @@ public class ComboBoxSelectingNewItemValueChange extends ComboBoxSelecting {
                 resetButton, delay, reject, noSelection);
     }
 
+    @SuppressWarnings("deprecation")
+    protected void configureNewItemHandling() {
+        comboBox.setNewItemHandler(text -> {
+            if (Boolean.TRUE.equals(delay.getValue())) {
+                try {
+                    Thread.sleep(2000);
+                } catch (InterruptedException e1) {
+                    e1.printStackTrace();
+                }
+            }
+            if (Boolean.TRUE.equals(reject.getValue())) {
+                valueChangeLabel.setValue("item " + text + " discarded");
+                comboBox.getComboBoxClientRpc().newItemNotAdded(text);
+            } else {
+                items.add(text);
+                Collections.sort(items);
+                valueChangeLabel
+                        .setValue("adding new item... count: " + items.size());
+                if (Boolean.TRUE.equals(noSelection.getValue())) {
+                    comboBox.getComboBoxClientRpc().newItemNotAdded(text);
+                }
+                comboBox.getDataProvider().refreshAll();
+            }
+        });
+    }
+
     private void initItems() {
         items.clear();
         for (char c = 'a'; c <= 'z'; c++) {
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxNewItemProviderTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxNewItemProviderTest.java
new file mode 100644
index 0000000000..b7c3a4f7d6
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxNewItemProviderTest.java
@@ -0,0 +1,6 @@
+package com.vaadin.tests.components.combobox;
+
+public class ComboBoxNewItemProviderTest
+        extends ComboBoxSelectingNewItemValueChangeTest {
+    // same tests using NewItemProvider instead of NewItemHandler
+}