Fix RpcDataSource to use RPC for row pins/unpins (#13334)

This patch removes the temprarilyPinnedRows workaround from
AbstractRemoteDataSource and refactors the whole feature to be part of
RpcDataSource where it should be.

Change-Id: Id55020dd11dda3dcf54dfe3c1b41af8e495c1c0c

2 files changed, 19 insertions, 63 deletions

diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java
index 99ff57089c..68f40f6cf3 100644
--- a/server/src/com/vaadin/data/RpcDataProviderExtension.java
+++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java
@@ -49,8 +49,6 @@ import com.vaadin.shared.ui.grid.Range;
 import com.vaadin.ui.components.grid.Grid;
 import com.vaadin.ui.components.grid.GridColumn;
 import com.vaadin.ui.components.grid.Renderer;
-import com.vaadin.ui.components.grid.selection.SelectionChangeEvent;
-import com.vaadin.ui.components.grid.selection.SelectionChangeListener;
 
 import elemental.json.Json;
 import elemental.json.JsonArray;
@@ -650,20 +648,9 @@ public class RpcDataProviderExtension extends AbstractExtension {
         rpc = getRpcProxy(DataProviderRpc.class);
 
         registerRpc(new DataRequestRpc() {
-            private Collection<String> allTemporarilyPinnedKeys = new ArrayList<String>();
-
             @Override
             public void requestRows(int firstRow, int numberOfRows,
-                    int firstCachedRowIndex, int cacheSize,
-                    List<String> temporarilyPinnedKeys) {
-
-                for (String key : temporarilyPinnedKeys) {
-                    Object itemId = keyMapper.getItemId(key);
-                    if (!keyMapper.isPinned(itemId)) {
-                        keyMapper.pin(itemId);
-                    }
-                }
-                allTemporarilyPinnedKeys.addAll(temporarilyPinnedKeys);
+                    int firstCachedRowIndex, int cacheSize) {
 
                 Range active = Range.withLength(firstRow, numberOfRows);
                 if (cacheSize != 0) {
@@ -682,52 +669,12 @@ public class RpcDataProviderExtension extends AbstractExtension {
             }
 
             @Override
-            public void releaseTemporarilyPinnedKeys() {
-                /*
-                 * This needs to be done deferredly since the selection event
-                 * comes after this RPC call.
-                 */
-
-                final SelectionChangeListener listener = new SelectionChangeListener() {
-                    @Override
-                    public void selectionChange(SelectionChangeEvent event) {
-                        for (String tempPinnedKey : allTemporarilyPinnedKeys) {
-                            /*
-                             * TODO: this could be moved into a field instead of
-                             * inline to reduce indentations.
-                             */
-
-                            /*
-                             * This works around the fact that when deselecting
-                             * and leaping through the cache, the client tries
-                             * to send a deselect event even though a row never
-                             * was selected. So, it tries to unpin something
-                             * that never was temporarily pinned.
-                             * 
-                             * If the same thing would happen while selecting
-                             * (instead of deselecting), the row would be
-                             * pinned, not because of the temporary pinning, but
-                             * because it's selected.
-                             */
-                            if (!keyMapper.isPinned(tempPinnedKey)) {
-                                continue;
-                            }
-
-                            Object itemId = keyMapper.getItemId(tempPinnedKey);
-                            Integer index = keyMapper.indexToItemId.inverse()
-                                    .get(itemId);
-                            if (!getGrid().isSelected(itemId)
-                                    && !activeRowHandler.activeRange
-                                            .contains(index.intValue())) {
-                                keyMapper.unpin(itemId);
-                            }
-                        }
-                        allTemporarilyPinnedKeys = new ArrayList<String>();
-                        getGrid().removeSelectionChangeListener(this);
-                    }
-                };
-
-                getGrid().addSelectionChangeListener(listener);
+            public void setPinned(String key, boolean isPinned) {
+                if (isPinned) {
+                    keyMapper.pin(keyMapper.getItemId(key));
+                } else {
+                    keyMapper.unpin(keyMapper.getItemId(key));
+                }
             }
         });
 
diff --git a/server/src/com/vaadin/ui/components/grid/Grid.java b/server/src/com/vaadin/ui/components/grid/Grid.java
index fe03d589c0..8203f9c344 100644
--- a/server/src/com/vaadin/ui/components/grid/Grid.java
+++ b/server/src/com/vaadin/ui/components/grid/Grid.java
@@ -305,9 +305,18 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
         addSelectionChangeListener(new SelectionChangeListener() {
             @Override
             public void selectionChange(SelectionChangeEvent event) {
-                for (Object removedItemId : event.getRemoved()) {
-                    getKeyMapper().unpin(removedItemId);
-                }
+                /*
+                 * This listener nor anything else in the server side should
+                 * never unpin anything from KeyMapper. Pinning is mostly a
+                 * client feature and is only used when selecting something from
+                 * the server side. This is to ensure that client has the
+                 * correct key from server when the selected row is first
+                 * loaded.
+                 * 
+                 * Once client has gotten info that it is supposed to select a
+                 * row, it will pin the data from the client side as well and it
+                 * will be unpinned once it gets deselected.
+                 */
 
                 for (Object addedItemId : event.getAdded()) {
                     if (!getKeyMapper().isPinned(addedItemId)) {