Fix null as a SelectionModel

Provides methods for cleaning up selection models.

Change-Id: Ifd1db68ba8cd6e75942a3f700e608ee0d2dfcd15

client/src/main/java/com/vaadin/client/connectors/components/AbstractListingConnector.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright 2000-2014 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
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.client.connectors.components;
+
+import com.vaadin.client.connectors.data.HasSelection;
+import com.vaadin.client.data.DataSource;
+import com.vaadin.client.data.selection.SelectionModel;
+import com.vaadin.client.ui.AbstractComponentConnector;
+
+import elemental.json.JsonObject;
+
+/**
+ * Base connector for AbstractListings base implementations for storing and
+ * retrieving the data source and selection model. Used by default
+ * implementations for SelectionModels.
+ * 
+ * @since
+ */
+public abstract class AbstractListingConnector extends
+        AbstractComponentConnector implements HasSelection {
+
+    private SelectionModel model = null;
+    private DataSource<JsonObject> dataSource = null;
+
+    @Override
+    public void setDataSource(DataSource<JsonObject> dataSource) {
+        if (this.dataSource != null) {
+            removeDataSource(this.dataSource);
+        }
+        this.dataSource = dataSource;
+    }
+
+    @Override
+    public DataSource<JsonObject> getDataSource() {
+        return dataSource;
+    }
+
+    @Override
+    public void setSelectionModel(SelectionModel selectionModel) {
+        if (model != null) {
+            removeSelectionModel(model);
+        }
+        model = selectionModel;
+    }
+
+    @Override
+    public SelectionModel getSelectionModel() {
+        return model;
+    }
+
+    /**
+     * Method that is executed when removing an old data source.
+     * 
+     * @param dataSource
+     *            old data source
+     */
+    protected void removeDataSource(DataSource<JsonObject> dataSource) {
+        // NO-OP
+    }
+
+    /**
+     * Method that is executed when removing an old selection model.
+     * 
+     * @param selectionModel
+     *            old selection model
+     */
+    protected void removeSelectionModel(SelectionModel selectionModel) {
+        // NO-OP
+    }
+}

client/src/main/java/com/vaadin/client/connectors/components/NativeSelectConnector.java

@@ -32,7 +32,7 @@ import com.vaadin.ui.components.nativeselect.NativeSelect;
 import elemental.json.JsonObject;
 
 @Connect(NativeSelect.class)
-public class NativeSelectConnector extends AbstractComponentConnector implements
+public class NativeSelectConnector extends AbstractListingConnector implements
         HasSelection {
 
     private final class NativeSelectDataChangeHandler implements
@@ -46,12 +46,12 @@ public class NativeSelectConnector extends AbstractComponentConnector implements
         public void dataUpdated(int firstRowIndex, int numberOfRows) {
             for (int i = 0; i < numberOfRows; ++i) {
                 int index = i + firstRowIndex;
-                JsonObject item = dataSource.getRow(index);
+                JsonObject item = getDataSource().getRow(index);
                 getWidget().setItemText(index,
                         item.getString(DataProviderConstants.NAME));
                 getWidget().setValue(index,
                         item.getString(DataProviderConstants.KEY));
-                if (selectionModel.isSelected(item)) {
+                if (getSelectionModel().isSelected(item)) {
                     getWidget().setSelectedIndex(index);
                 }
             }
@@ -69,11 +69,11 @@ public class NativeSelectConnector extends AbstractComponentConnector implements
         public void dataAvailable(int firstRowIndex, int numberOfRows) {
             if (getWidget().getItemCount() == firstRowIndex) {
                 for (int i = 0; i < numberOfRows; ++i) {
-                    JsonObject item = dataSource.getRow(i + firstRowIndex);
+                    JsonObject item = getDataSource().getRow(i + firstRowIndex);
                     getWidget().addItem(
                             item.getString(DataProviderConstants.NAME),
                             item.getString(DataProviderConstants.KEY));
-                    if (selectionModel.isSelected(item)) {
+                    if (getSelectionModel().isSelected(item)) {
                         getWidget().setSelectedIndex(i + firstRowIndex);
                     }
                 }
@@ -84,11 +84,11 @@ public class NativeSelectConnector extends AbstractComponentConnector implements
         public void dataAdded(int firstRowIndex, int numberOfRows) {
             if (getWidget().getItemCount() == firstRowIndex) {
                 for (int i = 0; i < numberOfRows; ++i) {
-                    JsonObject item = dataSource.getRow(i + firstRowIndex);
+                    JsonObject item = getDataSource().getRow(i + firstRowIndex);
                     getWidget().addItem(
                             item.getString(DataProviderConstants.NAME),
                             item.getString(DataProviderConstants.KEY));
-                    if (selectionModel.isSelected(item)) {
+                    if (getSelectionModel().isSelected(item)) {
                         getWidget().setSelectedIndex(i + firstRowIndex);
                     }
                 }
@@ -98,8 +98,6 @@ public class NativeSelectConnector extends AbstractComponentConnector implements
         }
     }
 
-    private DataSource<JsonObject> dataSource;
-    private SelectionModel selectionModel;
     private boolean scheduled;
 
     @Override
@@ -115,30 +113,22 @@ public class NativeSelectConnector extends AbstractComponentConnector implements
 
             @Override
             public void onChange(ChangeEvent event) {
-                if (selectionModel == null) {
+                if (getSelectionModel() == null) {
                     return;
                 }
                 int index = getWidget().getSelectedIndex();
-                JsonObject selected = dataSource.getRow(index);
-                selectionModel.select(selected);
+                JsonObject selected = getDataSource().getRow(index);
+                getSelectionModel().select(selected);
             }
         });
     }
 
     @Override
     public void setDataSource(DataSource<JsonObject> dataSource) {
-        if (this.dataSource != null) {
-            dataSource.setDataChangeHandler(null);
-        }
-        this.dataSource = dataSource;
+        super.setDataSource(dataSource);
         dataSource.setDataChangeHandler(new NativeSelectDataChangeHandler());
     }
 
-    @Override
-    public void setSelectionModel(SelectionModel selectionModel) {
-        this.selectionModel = selectionModel;
-    }
-
     private void resetContent() {
         if (scheduled) {
             return;
@@ -148,12 +138,12 @@ public class NativeSelectConnector extends AbstractComponentConnector implements
             @Override
             public void execute() {
                 getWidget().clear();
-                for (int i = 0; i < dataSource.size(); ++i) {
-                    JsonObject item = dataSource.getRow(i);
+                for (int i = 0; i < getDataSource().size(); ++i) {
+                    JsonObject item = getDataSource().getRow(i);
                     getWidget().addItem(
                             item.getString(DataProviderConstants.NAME),
                             item.getString(DataProviderConstants.KEY));
-                    if (selectionModel.isSelected(item)) {
+                    if (getSelectionModel().isSelected(item)) {
                         getWidget().setSelectedIndex(i);
                     }
                 }

client/src/main/java/com/vaadin/client/connectors/components/grid/GridConnector.java

@@ -18,6 +18,7 @@ package com.vaadin.client.connectors.components.grid;
 import java.util.Collection;
 import java.util.logging.Logger;
 
+import com.vaadin.client.connectors.components.AbstractListingConnector;
 import com.vaadin.client.connectors.data.HasSelection;
 import com.vaadin.client.data.DataSource;
 import com.vaadin.client.data.selection.SelectionModel;
@@ -31,7 +32,7 @@ import com.vaadin.shared.ui.Connect;
 import elemental.json.JsonObject;
 
 @Connect(com.vaadin.ui.components.grid.Grid.class)
-public class GridConnector extends AbstractComponentConnector implements
+public class GridConnector extends AbstractListingConnector implements
         HasSelection {
 
     /**
@@ -44,15 +45,15 @@ public class GridConnector extends AbstractComponentConnector implements
     private final class SelectionModelAdapter
             implements
             com.vaadin.client.widget.grid.selection.SelectionModel.Single<JsonObject> {
-        private final SelectionModel selectionModel;
+        private final SelectionModel model;
 
         private SelectionModelAdapter(SelectionModel selectionModel) {
-            this.selectionModel = selectionModel;
+            this.model = selectionModel;
         }
 
         @Override
         public boolean isSelected(JsonObject row) {
-            return selectionModel.isSelected(row);
+            return model != null ? model.isSelected(row) : false;
         }
 
         @Override
@@ -75,13 +76,17 @@ public class GridConnector extends AbstractComponentConnector implements
 
         @Override
         public boolean select(JsonObject row) {
-            selectionModel.select(row);
+            if (model != null) {
+                model.select(row);
+            }
             return false;
         }
 
         @Override
         public boolean deselect(JsonObject row) {
-            selectionModel.deselect(row);
+            if (model != null) {
+                model.deselect(row);
+            }
             return false;
         }
 
@@ -114,11 +119,13 @@ public class GridConnector extends AbstractComponentConnector implements
 
     @Override
     public void setDataSource(DataSource<JsonObject> dataSource) {
+        super.setDataSource(dataSource);
         getWidget().setDataSource(dataSource);
     }
 
     @Override
     public void setSelectionModel(final SelectionModel selectionModel) {
+        super.setSelectionModel(selectionModel);
         getWidget()
                 .setSelectionModel(new SelectionModelAdapter(selectionModel));
     }

client/src/main/java/com/vaadin/client/connectors/data/HasDataSource.java

@@ -25,11 +25,18 @@ import elemental.json.JsonObject;
 public interface HasDataSource {
 
     /**
-     * Sets the data source for this Conenctor.
+     * Sets the data source for this Connector.
      * 
      * @param dataSource
      *            new data source
      */
     public void setDataSource(DataSource<JsonObject> dataSource);
 
+    /**
+     * Gets the current data source for this Connector.
+     * 
+     * @return data source
+     */
+    DataSource<JsonObject> getDataSource();
+
 }

client/src/main/java/com/vaadin/client/connectors/data/HasSelection.java

@@ -23,10 +23,17 @@ import com.vaadin.client.data.selection.SelectionModel;
 public interface HasSelection extends HasDataSource {
 
     /**
-     * Sets the selection model for this Conenctor.
+     * Sets the selection model for this Connector.
      * 
      * @param selectionModel
      *            selection model
      */
     public void setSelectionModel(SelectionModel selectionModel);
+
+    /**
+     * Gets the current selection model for this Connector.
+     * 
+     * @return selection model
+     */
+    public SelectionModel getSelectionModel();
 }

client/src/main/java/com/vaadin/client/connectors/selection/AbstractSelectionConnector.java

@@ -16,6 +16,7 @@
 package com.vaadin.client.connectors.selection;
 
 import com.vaadin.client.ServerConnector;
+import com.vaadin.client.connectors.components.AbstractListingConnector;
 import com.vaadin.client.connectors.data.HasSelection;
 import com.vaadin.client.data.selection.SelectionModel;
 import com.vaadin.client.extensions.AbstractExtensionConnector;
@@ -26,6 +27,8 @@ import elemental.json.JsonObject;
 public abstract class AbstractSelectionConnector extends
         AbstractExtensionConnector {
 
+    private SelectionModel model = null;
+
     @Override
     protected void extend(ServerConnector target) {
         if (!(target instanceof HasSelection)) {
@@ -35,7 +38,8 @@ public abstract class AbstractSelectionConnector extends
         // TODO: Provide SelectionModel API
         // TODO: Should this use "Registration" approach for easy and safe
         // removal?
-        ((HasSelection) target).setSelectionModel(createSelectionModel());
+        model = createSelectionModel();
+        ((HasSelection) target).setSelectionModel(getSelectionModel());
     }
 
     /**
@@ -63,4 +67,13 @@ public abstract class AbstractSelectionConnector extends
         String key = DataProviderConstants.SELECTED;
         return item.hasKey(key) && item.getBoolean(key);
     }
+
+    @Override
+    public AbstractListingConnector getParent() {
+        return (AbstractListingConnector) super.getParent();
+    }
+
+    protected SelectionModel getSelectionModel() {
+        return model;
+    }
 }

client/src/main/java/com/vaadin/client/connectors/selection/NullSelectionConnector.java

@@ -1,55 +0,0 @@
-/*
- * Copyright 2000-2014 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
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-package com.vaadin.client.connectors.selection;
-
-import com.vaadin.client.ServerConnector;
-import com.vaadin.client.connectors.data.HasSelection;
-import com.vaadin.client.data.selection.SelectionModel;
-import com.vaadin.client.data.selection.SelectionModel.Single;
-import com.vaadin.server.communication.data.typed.SelectionModel.NullSelection;
-import com.vaadin.shared.ui.Connect;
-
-import elemental.json.JsonObject;
-
-@Connect(NullSelection.class)
-public class NullSelectionConnector extends AbstractSelectionConnector {
-
-    @Override
-    protected void extend(ServerConnector target) {
-        if (target instanceof HasSelection) {
-            super.extend(target);
-        }
-    }
-
-    @Override
-    protected SelectionModel createSelectionModel() {
-        return new SelectionModel() {
-
-            @Override
-            public void select(JsonObject item) {
-            }
-
-            @Override
-            public void deselect(JsonObject item) {
-            }
-
-            @Override
-            public boolean isSelected(JsonObject item) {
-                return false;
-            }
-        };
-    }
-}

client/src/main/java/com/vaadin/client/connectors/selection/SingleSelectionConnector.java

@@ -15,6 +15,10 @@
  */
 package com.vaadin.client.connectors.selection;
 
+import java.util.logging.Logger;
+
+import com.vaadin.client.ServerConnector;
+import com.vaadin.client.connectors.components.AbstractListingConnector;
 import com.vaadin.client.data.selection.SelectionModel;
 import com.vaadin.client.data.selection.SelectionModel.Single;
 import com.vaadin.shared.data.selection.SelectionServerRpc;
@@ -60,6 +64,15 @@ public class SingleSelectionConnector extends AbstractSelectionConnector {
         }
     }
 
+    private AbstractListingConnector parent;
+
+    @Override
+    protected void extend(ServerConnector target) {
+        super.extend(target);
+
+        parent = getParent();
+    }
+
     @Override
     protected SelectionModel createSelectionModel() {
         return new SingleSelection(getRpcProxy(SelectionServerRpc.class));
@@ -68,6 +81,11 @@ public class SingleSelectionConnector extends AbstractSelectionConnector {
     @Override
     public void onUnregister() {
         super.onUnregister();
+
+        if (parent.getSelectionModel() == getSelectionModel()) {
+            // Remove from parent.
+            parent.setSelectionModel(null);
+        }
     }
 
 }

server/src/main/java/com/vaadin/server/communication/data/typed/SelectionModel.java

@@ -90,46 +90,4 @@ public interface SelectionModel<T> extends Serializable, ListingExtension<T> {
      *            deselected value
      */
     void deselect(T value);
-
-    /**
-     * Dummy selection model.
-     * 
-     * @param <T>
-     *            selected data type
-     */
-    public static class NullSelection<T> extends AbstractSelectionModel<T>
-            implements SelectionModel.Single<T> {
-
-        @Override
-        public void setValue(T value) {
-            // NO-OP
-        }
-
-        @Override
-        public T getValue() {
-            return null;
-        }
-
-        @Override
-        public Registration onChange(Handler<T> onChange) {
-            return () -> {
-                // NO-OP
-            };
-        }
-
-        @Override
-        public List<T> getSelected() {
-            return Collections.emptyList();
-        }
-
-        @Override
-        public void select(T value) {
-            // NO-OP
-        }
-
-        @Override
-        public void deselect(T value) {
-            // NO-OP
-        }
-    }
 }

server/src/main/java/com/vaadin/server/communication/data/typed/SingleSelection.java

@@ -113,4 +113,13 @@ public class SingleSelection<T> extends AbstractSelectionModel<T> implements
             setValue(null);
         }
     }
+
+    @Override
+    public void remove() {
+        if (getValue() != null) {
+            refresh(getValue());
+        }
+
+        super.remove();
+    }
 }

uitest/src/main/java/com/vaadin/tests/databinding/ListingTestUI.java

@@ -8,8 +8,10 @@ import java.util.stream.Stream;
 import com.vaadin.annotations.Theme;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.server.communication.data.typed.DataSource;
+import com.vaadin.server.communication.data.typed.SingleSelection;
 import com.vaadin.tests.components.AbstractTestUI;
 import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
 import com.vaadin.ui.Notification;
 import com.vaadin.ui.VerticalLayout;
 import com.vaadin.ui.components.grid.Grid;
@@ -90,6 +92,22 @@ public class ListingTestUI extends AbstractTestUI {
         grid.addColumn("toString", Bean::toString);
         grid.setDataSource(DataSource.create(Bean.generateRandomBeans()));
 
+        addComponent(new Button("Toggle Grid Selection",
+                new Button.ClickListener() {
+
+                    private boolean hasSelection = true;
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        if (hasSelection) {
+                            grid.setSelectionModel(null);
+                        } else {
+                            grid.setSelectionModel(new SingleSelection<>());
+                        }
+                        hasSelection = !hasSelection;
+                    }
+                }));
+
     }
 
     private List<String> createOptions() {