Fix server selection firing multiple events on client select (#13334)

This patch fixes multiple events from Single Selection Model. Multi
Selection Model is designed a bit differently.

Change-Id: I0350d7f2de873ff9fba7411c5ca8caefa156be20

2 files changed, 260 insertions, 27 deletions

diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index a1403401c7..60df269f10 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -350,6 +350,39 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
              * @see #selectAll()
              */
             boolean deselectAll();
+
+            /**
+             * Marks items as selected while deselecting all items not in the
+             * given Collection.
+             * 
+             * @param itemIds
+             *            the itemIds to mark as selected
+             * @return <code>true</code> if the selection state changed.
+             *         <code>false</code> if all the given itemIds already were
+             *         selected
+             * @throws IllegalArgumentException
+             *             if <code>itemIds</code> is <code>null</code> or given
+             *             itemIds don't exist in the container of Grid
+             */
+            boolean setSelected(Collection<?> itemIds)
+                    throws IllegalArgumentException;
+
+            /**
+             * Marks items as selected while deselecting all items not in the
+             * varargs array.
+             * 
+             * @param itemIds
+             *            the itemIds to mark as selected
+             * @return <code>true</code> if the selection state changed.
+             *         <code>false</code> if all the given itemIds already were
+             *         selected
+             * @throws IllegalArgumentException
+             *             if the <code>itemIds</code> varargs array is
+             *             <code>null</code> or given itemIds don't exist in the
+             *             container of Grid
+             */
+            boolean setSelected(Object... itemIds)
+                    throws IllegalArgumentException;
         }
 
         /**
@@ -771,6 +804,48 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
         public void reset() {
             deselectAll();
         }
+
+        @Override
+        public boolean setSelected(Collection<?> itemIds)
+                throws IllegalArgumentException {
+            if (itemIds == null) {
+                throw new IllegalArgumentException("itemIds may not be null");
+            }
+
+            checkItemIdsExist(itemIds);
+
+            boolean changed = false;
+            Set<Object> selectedRows = new HashSet<Object>(itemIds);
+            final Collection<Object> oldSelection = getSelectedRows();
+            SetView<?> added = Sets.difference(selectedRows, selection);
+            if (!added.isEmpty()) {
+                changed = true;
+                selection.addAll(added.immutableCopy());
+            }
+
+            SetView<?> removed = Sets.difference(selection, selectedRows);
+            if (!removed.isEmpty()) {
+                changed = true;
+                selection.removeAll(removed.immutableCopy());
+            }
+
+            if (changed) {
+                fireSelectionEvent(oldSelection, selection);
+            }
+
+            return changed;
+        }
+
+        @Override
+        public boolean setSelected(Object... itemIds)
+                throws IllegalArgumentException {
+            if (itemIds != null) {
+                return setSelected(Arrays.asList(itemIds));
+            } else {
+                throw new IllegalArgumentException(
+                        "Vararg array of itemIds may not be null");
+            }
+        }
     }
 
     /**
@@ -2494,37 +2569,24 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 Collection<Object> receivedSelection = getKeyMapper()
                         .getItemIds(selection);
 
-                final HashSet<Object> receivedSelectionSet = new HashSet<Object>(
-                        receivedSelection);
-                final HashSet<Object> previousSelectionSet = new HashSet<Object>(
-                        getSelectedRows());
-
                 applyingSelectionFromClient = true;
                 try {
                     SelectionModel selectionModel = getSelectionModel();
-
-                    SetView<Object> removedItemIds = Sets.difference(
-                            previousSelectionSet, receivedSelectionSet);
-                    if (!removedItemIds.isEmpty()) {
-                        if (removedItemIds.size() == 1) {
-                            deselect(removedItemIds.iterator().next());
-                        } else {
-                            assert selectionModel instanceof SelectionModel.Multi : "Got multiple deselections, but the selection model is not a SelectionModel.Multi";
-                            ((SelectionModel.Multi) selectionModel)
-                                    .deselect(removedItemIds);
-                        }
-                    }
-
-                    SetView<Object> addedItemIds = Sets.difference(
-                            receivedSelectionSet, previousSelectionSet);
-                    if (!addedItemIds.isEmpty()) {
-                        if (addedItemIds.size() == 1) {
-                            select(addedItemIds.iterator().next());
-                        } else {
-                            assert selectionModel instanceof SelectionModel.Multi : "Got multiple selections, but the selection model is not a SelectionModel.Multi";
-                            ((SelectionModel.Multi) selectionModel)
-                                    .select(addedItemIds);
+                    if (selectionModel instanceof SelectionModel.Single
+                            && selection.size() <= 1) {
+                        Object select = null;
+                        if (selection.size() == 1) {
+                            select = getKeyMapper().getItemId(selection.get(0));
                         }
+                        ((SelectionModel.Single) selectionModel).select(select);
+                    } else if (selectionModel instanceof SelectionModel.Multi) {
+                        ((SelectionModel.Multi) selectionModel)
+                                .setSelected(receivedSelection);
+                    } else {
+                        throw new IllegalStateException("SelectionModel "
+                                + selectionModel.getClass().getSimpleName()
+                                + " does not support selecting the given "
+                                + selection.size() + " items.");
                     }
                 } finally {
                     applyingSelectionFromClient = false;
diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/MultiSelectionModelTest.java b/server/tests/src/com/vaadin/tests/server/component/grid/MultiSelectionModelTest.java
new file mode 100644
index 0000000000..9b327a2f22
--- /dev/null
+++ b/server/tests/src/com/vaadin/tests/server/component/grid/MultiSelectionModelTest.java
@@ -0,0 +1,171 @@
+/*
+ * 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.tests.server.component.grid;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.vaadin.data.Container;
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.event.SelectionEvent;
+import com.vaadin.event.SelectionEvent.SelectionListener;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.MultiSelectionModel;
+import com.vaadin.ui.Grid.SelectionMode;
+
+public class MultiSelectionModelTest {
+
+    private Object itemId1Present = "itemId1Present";
+    private Object itemId2Present = "itemId2Present";
+    private Object itemId3Present = "itemId3Present";
+
+    private Object itemIdNotPresent = "itemIdNotPresent";
+    private Container.Indexed dataSource;
+    private MultiSelectionModel model;
+    private Grid grid;
+
+    private boolean expectingEvent = false;
+    private boolean expectingDeselectEvent;
+    private List<Object> select = new ArrayList<Object>();
+    private List<Object> deselect = new ArrayList<Object>();
+
+    @Before
+    public void setUp() {
+        dataSource = createDataSource();
+        grid = new Grid(dataSource);
+        grid.setSelectionMode(SelectionMode.MULTI);
+        model = (MultiSelectionModel) grid.getSelectionModel();
+    }
+
+    @After
+    public void tearDown() {
+        Assert.assertFalse("Some expected select event did not happen.",
+                expectingEvent);
+        Assert.assertFalse("Some expected deselect event did not happen.",
+                expectingDeselectEvent);
+    }
+
+    private IndexedContainer createDataSource() {
+        final IndexedContainer container = new IndexedContainer();
+        container.addItem(itemId1Present);
+        container.addItem(itemId2Present);
+        container.addItem(itemId3Present);
+        for (int i = 3; i < 10; i++) {
+            container.addItem(new Object());
+        }
+
+        return container;
+    }
+
+    @Test
+    public void testSelectAndDeselectRow() throws Throwable {
+        try {
+            expectSelectEvent(itemId1Present);
+            model.select(itemId1Present);
+            expectDeselectEvent(itemId1Present);
+            model.deselect(itemId1Present);
+        } catch (Exception e) {
+            throw e.getCause();
+        }
+
+        verifyCurrentSelection();
+    }
+
+    @Test
+    public void testAddSelection() throws Throwable {
+        try {
+            expectSelectEvent(itemId1Present);
+            model.select(itemId1Present);
+            expectSelectEvent(itemId2Present);
+            model.select(itemId2Present);
+        } catch (Exception e) {
+            throw e.getCause();
+        }
+
+        verifyCurrentSelection(itemId1Present, itemId2Present);
+    }
+
+    @Test
+    public void testSettingSelection() throws Throwable {
+        try {
+            expectSelectEvent(itemId2Present, itemId1Present);
+            model.setSelected(Arrays.asList(new Object[] { itemId1Present,
+                    itemId2Present }));
+            verifyCurrentSelection(itemId1Present, itemId2Present);
+
+            expectDeselectEvent(itemId1Present);
+            expectSelectEvent(itemId3Present);
+            model.setSelected(Arrays.asList(new Object[] { itemId3Present,
+                    itemId2Present }));
+            verifyCurrentSelection(itemId3Present, itemId2Present);
+        } catch (Exception e) {
+            throw e.getCause();
+        }
+    }
+
+    private void expectSelectEvent(Object... selectArray) {
+        select = Arrays.asList(selectArray);
+        addListener();
+    }
+
+    private void expectDeselectEvent(Object... deselectArray) {
+        deselect = Arrays.asList(deselectArray);
+        addListener();
+    }
+
+    private void addListener() {
+        if (expectingEvent) {
+            return;
+        }
+
+        expectingEvent = true;
+        grid.addSelectionListener(new SelectionListener() {
+
+            @Override
+            public void select(SelectionEvent event) {
+                Assert.assertTrue("Selection did not contain expected items",
+                        event.getAdded().containsAll(select));
+                Assert.assertTrue("Selection contained unexpected items",
+                        select.containsAll(event.getAdded()));
+                select = new ArrayList<Object>();
+
+                Assert.assertTrue("Deselection did not contain expected items",
+                        event.getRemoved().containsAll(deselect));
+                Assert.assertTrue("Deselection contained unexpected items",
+                        deselect.containsAll(event.getRemoved()));
+                deselect = new ArrayList<Object>();
+
+                grid.removeSelectionListener(this);
+                expectingEvent = false;
+            }
+        });
+    }
+
+    private void verifyCurrentSelection(Object... selection) {
+        final List<Object> selected = Arrays.asList(selection);
+        if (model.getSelectedRows().containsAll(selected)
+                && selected.containsAll(model.getSelectedRows())) {
+            return;
+        }
+        Assert.fail("Not all items were correctly selected");
+    }
+}