Convert ComboBox to inherit AbstractSingleSelect

ComboBoxState now inherits AbstractSingleSelectState, but still
effectively communicates its selection via RPC.

Change-Id: I47b6a73f275e0c4c63d70d2c2c8badc95a218157

3 files changed, 64 insertions, 102 deletions

diff --git a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java
index 81c22cffda..fa683eb305 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java
@@ -1,12 +1,12 @@
 /*
  * Copyright 2000-2016 Vaadin Ltd.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -21,6 +21,7 @@ import java.util.Optional;
 
 import com.vaadin.event.selection.SingleSelectionChange;
 import com.vaadin.event.selection.SingleSelectionListener;
+import com.vaadin.server.data.DataCommunicator;
 import com.vaadin.shared.Registration;
 import com.vaadin.shared.data.selection.SelectionModel;
 import com.vaadin.shared.data.selection.SelectionServerRpc;
@@ -35,9 +36,9 @@ import com.vaadin.util.ReflectTools;
  *
  * @param <T>
  *            the item date type
- * 
+ *
  * @see com.vaadin.shared.data.selection.SelectionModel.Single
- * 
+ *
  * @since
  */
 public abstract class AbstractSingleSelect<T> extends
@@ -49,8 +50,8 @@ public abstract class AbstractSingleSelect<T> extends
      * client. Maintaining the selection state and communicating it from the
      * server to the client is the responsibility of the implementing class.
      */
-    public abstract class AbstractSingleSelection implements
-            SelectionModel.Single<T> {
+    public abstract class AbstractSingleSelection
+            implements SelectionModel.Single<T> {
 
         /**
          * Creates a new {@code SimpleSingleSelection} instance.
@@ -89,7 +90,7 @@ public abstract class AbstractSingleSelect<T> extends
         /**
          * Returns the communication key of the selected item or {@code null} if
          * no item is selected.
-         * 
+         *
          * @return the key of the selected item if any, {@code null} otherwise.
          */
         protected abstract String getSelectedKey();
@@ -97,7 +98,7 @@ public abstract class AbstractSingleSelect<T> extends
         /**
          * Sets the selected item based on the given communication key. If the
          * key is {@code null}, clears the current selection if any.
-         * 
+         *
          * @param key
          *            the key of the selected item or {@code null} to clear
          *            selection
@@ -109,7 +110,7 @@ public abstract class AbstractSingleSelect<T> extends
          * select component is {@linkplain Component#isReadOnly()} or if the
          * selection would not change. Otherwise updates the selection and fires
          * a selection change event with {@code isUserOriginated == true}.
-         * 
+         *
          * @param key
          *            the key of the item to select or {@code null} to clear
          *            selection
@@ -131,7 +132,7 @@ public abstract class AbstractSingleSelect<T> extends
          * Sets the selection based on server API call. Does nothing if the
          * selection would not change; otherwise updates the selection and fires
          * a selection change event with {@code isUserOriginated == false}.
-         * 
+         *
          * @param item
          *            the item to select or {@code null} to clear selection
          */
@@ -150,7 +151,7 @@ public abstract class AbstractSingleSelect<T> extends
 
         /**
          * Returns whether the given key maps to the currently selected item.
-         * 
+         *
          * @param key
          *            the key to test or {@code null} to test whether nothing is
          *            selected
@@ -164,7 +165,7 @@ public abstract class AbstractSingleSelect<T> extends
 
         /**
          * Returns the communication key assigned to the given item.
-         * 
+         *
          * @param item
          *            the item whose key to return
          * @return the assigned key
@@ -181,7 +182,7 @@ public abstract class AbstractSingleSelect<T> extends
         /**
          * Returns the item that the given key is assigned to, or {@code null}
          * if there is no such item.
-         * 
+         *
          * @param key
          *            the key whose item to return
          * @return the associated item if any, {@code null} otherwise.
@@ -228,6 +229,37 @@ public abstract class AbstractSingleSelect<T> extends
                     SingleSelectionChange.class);
 
     /**
+     * Creates a new {@code AbstractListing} with a default data communicator.
+     * <p>
+     * <strong>Note:</strong> This constructor does not set a selection model
+     * for the new listing. The invoking constructor must explicitly call
+     * {@link #setSelectionModel(SelectionModel)} with an
+     * {@link AbstractSingleSelect.AbstractSingleSelection} .
+     */
+    protected AbstractSingleSelect() {
+    }
+
+    /**
+     * Creates a new {@code AbstractSingleSelect} with the given custom data
+     * communicator.
+     * <p>
+     * <strong>Note:</strong> This method is for creating an
+     * {@code AbstractSingleSelect} with a custom communicator. In the common
+     * case {@link AbstractSingleSelect#AbstractSingleSelect()} should be used.
+     * <p>
+     * <strong>Note:</strong> This constructor does not set a selection model
+     * for the new listing. The invoking constructor must explicitly call
+     * {@link #setSelectionModel(SelectionModel)} with an
+     * {@link AbstractSingleSelect.AbstractSingleSelection} .
+     *
+     * @param dataCommunicator
+     *            the data communicator to use, not null
+     */
+    protected AbstractSingleSelect(DataCommunicator<T> dataCommunicator) {
+        super(dataCommunicator);
+    }
+
+    /**
      * Adds a selection listener to this select. The listener is called when the
      * value of this select is changed either by the user or programmatically.
      *
@@ -246,7 +278,7 @@ public abstract class AbstractSingleSelect<T> extends
     /**
      * Returns the currently selected item, or an empty optional if no item is
      * selected.
-     * 
+     *
      * @return an optional of the selected item if any, an empty optional
      *         otherwise
      */
@@ -257,7 +289,7 @@ public abstract class AbstractSingleSelect<T> extends
     /**
      * Sets the current selection to the given item or clears selection if given
      * {@code null}.
-     * 
+     *
      * @param item
      *            the item to select or {@code null} to clear selection
      */
diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java
index c76ae4a728..39c2bad8ba 100644
--- a/server/src/main/java/com/vaadin/ui/ComboBox.java
+++ b/server/src/main/java/com/vaadin/ui/ComboBox.java
@@ -17,11 +17,8 @@
 package com.vaadin.ui;
 
 import java.io.Serializable;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.List;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -33,8 +30,6 @@ import com.vaadin.event.FieldEvents.BlurListener;
 import com.vaadin.event.FieldEvents.FocusAndBlurServerRpcImpl;
 import com.vaadin.event.FieldEvents.FocusEvent;
 import com.vaadin.event.FieldEvents.FocusListener;
-import com.vaadin.event.selection.SingleSelectionChange;
-import com.vaadin.event.selection.SingleSelectionListener;
 import com.vaadin.server.KeyMapper;
 import com.vaadin.server.Resource;
 import com.vaadin.server.ResourceReference;
@@ -43,7 +38,6 @@ import com.vaadin.server.data.DataKeyMapper;
 import com.vaadin.server.data.DataSource;
 import com.vaadin.shared.Registration;
 import com.vaadin.shared.data.DataCommunicatorConstants;
-import com.vaadin.shared.data.selection.SelectionModel;
 import com.vaadin.shared.ui.combobox.ComboBoxClientRpc;
 import com.vaadin.shared.ui.combobox.ComboBoxConstants;
 import com.vaadin.shared.ui.combobox.ComboBoxServerRpc;
@@ -60,90 +54,27 @@ import elemental.json.JsonObject;
  * @author Vaadin Ltd
  */
 @SuppressWarnings("serial")
-public class ComboBox<T> extends
-        AbstractListing<T, ComboBox<T>.ComboBoxSelectionModel> implements
-        HasValue<T>, FieldEvents.BlurNotifier, FieldEvents.FocusNotifier {
+public class ComboBox<T> extends AbstractSingleSelect<T> implements HasValue<T>,
+        FieldEvents.BlurNotifier, FieldEvents.FocusNotifier {
 
     /**
      * Custom single selection model for ComboBox.
      */
-    protected class ComboBoxSelectionModel implements SelectionModel.Single<T> {
-        private Optional<T> selectedItem = Optional.empty();
-        private List<SingleSelectionListener<T>> listeners = new ArrayList<>(2);
-
+    protected class ComboBoxSelectionModel extends SimpleSingleSelection {
         @Override
-        public void deselect(T item) {
-            selectedItem.ifPresent(selected -> {
-                if (selected.equals(item)) {
-                    setValue(Optional.empty(), false);
-                }
-            });
-        }
+        protected void doSetSelectedKey(String key) {
+            super.doSetSelectedKey(key);
 
-        /**
-         * Sets the selection. This method is primarily for internal use.
-         *
-         * @param value
-         *            optional new value
-         * @param userOriginated
-         *            true if the value comes from user input, false otherwise
-         */
-        protected void setValue(Optional<T> value, boolean userOriginated) {
-            if (selectedItem.equals(value)) {
-                return;
+            String selectedCaption = null;
+            T value = getDataCommunicator().getKeyMapper().get(key);
+            if (value != null) {
+                selectedCaption = getItemCaptionProvider().apply(value);
             }
-            selectedItem = value;
-            String key = value
-                    .map(v -> getDataCommunicator().getKeyMapper().key(v))
-                    .orElse(null);
-            String selectedCaption = value
-                    .map(v -> getItemCaptionProvider().apply(v)).orElse(null);
+            // FIXME now overlap between state and RPC
             getRpcProxy(ComboBoxClientRpc.class).setSelectedItem(key,
                     selectedCaption);
-
-            fireEvent(userOriginated);
         }
 
-        /**
-         * Adds a selection listener to this select. The listener is called when
-         * the value of this select is changed either by the user or
-         * programmatically.
-         *
-         * @param listener
-         *            the value change listener, not null
-         * @return a registration for the listener
-         */
-        public Registration addSelectionListener(
-                SingleSelectionListener<T> listener) {
-            Objects.requireNonNull(listener, "listener cannot be null");
-            listeners.add(listener);
-            return () -> listeners.remove(listener);
-        }
-
-        @Override
-        public void select(T item) {
-            Objects.requireNonNull(item);
-            setValue(Optional.of(item), false);
-        }
-
-        @Override
-        public Optional<T> getSelectedItem() {
-            return selectedItem;
-        }
-
-        /**
-         * Fires a selection change event to all listeners.
-         *
-         * @param userOriginated
-         *            true to indicate that the change was caused directly by a
-         *            user action, false for programmatically set values
-         */
-        protected void fireEvent(boolean userOriginated) {
-            for (SingleSelectionListener<T> listener : listeners) {
-                listener.accept(new SingleSelectionChange<T>(ComboBox.this,
-                        getSelectedItem().orElse(null), userOriginated));
-            }
-        }
     }
 
     /**
@@ -223,10 +154,9 @@ public class ComboBox<T> extends
         public void setSelectedItem(String key) {
             // it seems both of these happen, and mean empty selection...
             if (key == null || "".equals(key)) {
-                getSelectionModel().setValue(Optional.empty(), true);
+                getSelectionModel().setSelectedFromClient(null);
             } else {
-                T item = getDataCommunicator().getKeyMapper().get(key);
-                getSelectionModel().setValue(Optional.ofNullable(item), true);
+                getSelectionModel().setSelectedFromClient(key);
             }
         }
 
@@ -657,7 +587,7 @@ public class ComboBox<T> extends
 
     @Override
     public void setValue(T value) {
-        getSelectionModel().setValue(Optional.ofNullable(value), false);
+        getSelectionModel().setSelectedFromServer(value);
 
     }
 
@@ -670,7 +600,7 @@ public class ComboBox<T> extends
     @Override
     public Registration addValueChangeListener(
             HasValue.ValueChangeListener<? super T> listener) {
-        return getSelectionModel().addSelectionListener(event -> {
+        return addSelectionListener(event -> {
             ((ValueChangeListener<T>) listener)
                     .accept(new ValueChange<T>(event.getConnector(),
                             event.getValue(), event.isUserOriginated()));
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 7aa4cbfc72..e68d606e67 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
@@ -17,14 +17,14 @@ package com.vaadin.shared.ui.combobox;
 
 import com.vaadin.shared.annotations.DelegateToWidget;
 import com.vaadin.shared.annotations.NoLayout;
-import com.vaadin.shared.ui.TabIndexState;
+import com.vaadin.shared.ui.AbstractSingleSelectState;
 
 /**
  * Shared state for the ComboBox component.
  *
  * @since 7.0
  */
-public class ComboBoxState extends TabIndexState {
+public class ComboBoxState extends AbstractSingleSelectState {
     {
         // TODO ideally this would be v-combobox, but that would affect a lot of
         // themes