Redesign RpcDataSourceConnector pinning RPC requests (#18692)

This patch removes DataProviderKeyMapper which was mostly dead code
already. Uses a regular KeyMapper instead.

Change-Id: Ic97d1dc827d45fde65bcddc0414bfe711032620c

3 files changed, 57 insertions, 48 deletions

diff --git a/client/src/com/vaadin/client/connectors/MultiSelectionModelConnector.java b/client/src/com/vaadin/client/connectors/MultiSelectionModelConnector.java
index c1f5d87d68..e4ad50e7ac 100644
--- a/client/src/com/vaadin/client/connectors/MultiSelectionModelConnector.java
+++ b/client/src/com/vaadin/client/connectors/MultiSelectionModelConnector.java
@@ -156,19 +156,16 @@ public class MultiSelectionModelConnector extends
         public void selectAll() {
             assert !isBeingBatchSelected() : "Can't select all in middle of a batch selection.";
 
-            Set<RowHandle<JsonObject>> rows = new HashSet<DataSource.RowHandle<JsonObject>>();
             DataSource<JsonObject> dataSource = getGrid().getDataSource();
             for (int i = availableRows.getStart(); i < availableRows.getEnd(); ++i) {
                 final JsonObject row = dataSource.getRow(i);
                 if (row != null) {
                     RowHandle<JsonObject> handle = dataSource.getHandle(row);
                     markAsSelected(handle, true);
-                    rows.add(handle);
                 }
             }
 
             getRpcProxy(MultiSelectionModelServerRpc.class).selectAll();
-            cleanRowCache(rows);
         }
 
         @Override
@@ -205,19 +202,16 @@ public class MultiSelectionModelConnector extends
         public boolean deselectAll() {
             assert !isBeingBatchSelected() : "Can't select all in middle of a batch selection.";
 
-            Set<RowHandle<JsonObject>> rows = new HashSet<DataSource.RowHandle<JsonObject>>();
             DataSource<JsonObject> dataSource = getGrid().getDataSource();
             for (int i = availableRows.getStart(); i < availableRows.getEnd(); ++i) {
                 final JsonObject row = dataSource.getRow(i);
                 if (row != null) {
                     RowHandle<JsonObject> handle = dataSource.getHandle(row);
                     markAsSelected(handle, false);
-                    rows.add(handle);
                 }
             }
 
             getRpcProxy(MultiSelectionModelServerRpc.class).deselectAll();
-            cleanRowCache(rows);
 
             return true;
         }
@@ -235,8 +229,9 @@ public class MultiSelectionModelConnector extends
 
             for (JsonObject row : rows) {
                 RowHandle<JsonObject> rowHandle = getRowHandle(row);
-                markAsSelected(rowHandle, true);
-                selected.add(rowHandle);
+                if (markAsSelected(rowHandle, true)) {
+                    selected.add(rowHandle);
+                }
             }
 
             if (!isBeingBatchSelected()) {
@@ -246,23 +241,35 @@ public class MultiSelectionModelConnector extends
         }
 
         /**
-         * Marks the JsonObject pointed by RowHandle to have selected
-         * information equal to given boolean
+         * Marks the given row to be selected or deselected. Returns true if the
+         * value actually changed.
+         * <p>
+         * Note: If selection model is in batch select state, the row will be
+         * pinned on select.
          * 
          * @param row
          *            row handle
          * @param selected
-         *            should row be selected
+         *            {@code true} if row should be selected; {@code false} if
+         *            not
+         * @return {@code true} if selected status changed; {@code false} if not
          */
-        protected void markAsSelected(RowHandle<JsonObject> row,
+        protected boolean markAsSelected(RowHandle<JsonObject> row,
                 boolean selected) {
-            row.pin();
-            if (selected) {
+            if (selected && !isSelected(row.getRow())) {
                 row.getRow().put(GridState.JSONKEY_SELECTED, true);
-            } else {
+            } else if (!selected && isSelected(row.getRow())) {
                 row.getRow().remove(GridState.JSONKEY_SELECTED);
+            } else {
+                return false;
             }
+
             row.updateRow();
+
+            if (isBeingBatchSelected()) {
+                row.pin();
+            }
+            return true;
         }
 
         /**
@@ -278,8 +285,9 @@ public class MultiSelectionModelConnector extends
 
             for (JsonObject row : rows) {
                 RowHandle<JsonObject> rowHandle = getRowHandle(row);
-                markAsSelected(rowHandle, false);
-                deselected.add(rowHandle);
+                if (markAsSelected(rowHandle, false)) {
+                    deselected.add(rowHandle);
+                }
             }
 
             if (!isBeingBatchSelected()) {
@@ -288,16 +296,38 @@ public class MultiSelectionModelConnector extends
             return true;
         }
 
+        /**
+         * Sends a deselect RPC call to server-side containing all deselected
+         * rows. Unpins any pinned rows.
+         */
         private void sendDeselected() {
             getRpcProxy(MultiSelectionModelServerRpc.class).deselect(
                     getRowKeys(deselected));
-            cleanRowCache(deselected);
+
+            if (isBeingBatchSelected()) {
+                for (RowHandle<JsonObject> row : deselected) {
+                    row.unpin();
+                }
+            }
+
+            deselected.clear();
         }
 
+        /**
+         * Sends a select RPC call to server-side containing all selected rows.
+         * Unpins any pinned rows.
+         */
         private void sendSelected() {
             getRpcProxy(MultiSelectionModelServerRpc.class).select(
                     getRowKeys(selected));
-            cleanRowCache(selected);
+
+            if (isBeingBatchSelected()) {
+                for (RowHandle<JsonObject> row : selected) {
+                    row.unpin();
+                }
+            }
+
+            selected.clear();
         }
 
         private List<String> getRowKeys(Set<RowHandle<JsonObject>> handles) {
@@ -316,13 +346,6 @@ public class MultiSelectionModelConnector extends
             return rows;
         }
 
-        private void cleanRowCache(Set<RowHandle<JsonObject>> handles) {
-            for (RowHandle<JsonObject> handle : handles) {
-                handle.unpin();
-            }
-            handles.clear();
-        }
-
         @Override
         public void startBatchSelect() {
             assert selected.isEmpty() && deselected.isEmpty() : "Row caches were not clear.";
diff --git a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java
index bcca8816b1..5daa02c3bf 100644
--- a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java
+++ b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java
@@ -190,23 +190,15 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector {
         }
 
         @Override
-        protected void pinHandle(RowHandleImpl handle) {
-            // Server only knows if something is pinned or not. No need to pin
-            // multiple times.
-            boolean pinnedBefore = handle.isPinned();
-            super.pinHandle(handle);
-            if (!pinnedBefore) {
-                rpcProxy.setPinned(getRowKey(handle.getRow()), true);
-            }
-        }
-
-        @Override
         protected void unpinHandle(RowHandleImpl handle) {
             // Row data is no longer available after it has been unpinned.
             String key = getRowKey(handle.getRow());
             super.unpinHandle(handle);
             if (!handle.isPinned()) {
-                rpcProxy.setPinned(key, false);
+                if (indexOfKey(key) == -1) {
+                    // Row out of view has been unpinned. drop it
+                    droppedRowKeys.set(droppedRowKeys.length(), key);
+                }
             }
         }
 
@@ -244,7 +236,9 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector {
 
         @Override
         protected void onDropFromCache(int rowIndex, JsonObject row) {
-            droppedRowKeys.set(droppedRowKeys.length(), getRowKey(row));
+            if (!((RowHandleImpl) getHandle(row)).isPinned()) {
+                droppedRowKeys.set(droppedRowKeys.length(), getRowKey(row));
+            }
         }
     }
 
diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
index 2438bec8df..58cd5c5f19 100644
--- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
+++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
@@ -16,8 +16,6 @@
 
 package com.vaadin.client.data;
 
-import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -120,12 +118,7 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> {
 
         @Override
         public T getRow() throws IllegalStateException {
-            if (isPinned()) {
-                return row;
-            } else {
-                throw new IllegalStateException("The row handle for key " + key
-                        + " was not pinned");
-            }
+            return row;
         }
 
         public boolean isPinned() {
@@ -197,7 +190,6 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> {
 
     private Map<Object, Integer> pinnedCounts = new HashMap<Object, Integer>();
     private Map<Object, RowHandleImpl> pinnedRows = new HashMap<Object, RowHandleImpl>();
-    protected Collection<T> temporarilyPinnedRows = Collections.emptySet();
 
     // Size not yet known
     private int size = -1;