Redesign RpcDataSourceConnector pinning RPC requests (#18692)

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

Change-Id: Ic97d1dc827d45fde65bcddc0414bfe711032620c

9 files changed, 91 insertions, 390 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;
diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java
index c8f3604fd9..78c87ab23d 100644
--- a/server/src/com/vaadin/data/RpcDataProviderExtension.java
+++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java
@@ -21,14 +21,11 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import com.google.gwt.thirdparty.guava.common.collect.BiMap;
-import com.google.gwt.thirdparty.guava.common.collect.HashBiMap;
 import com.google.gwt.thirdparty.guava.common.collect.ImmutableSet;
 import com.google.gwt.thirdparty.guava.common.collect.Maps;
 import com.google.gwt.thirdparty.guava.common.collect.Sets;
@@ -43,6 +40,7 @@ import com.vaadin.data.Property.ValueChangeListener;
 import com.vaadin.data.Property.ValueChangeNotifier;
 import com.vaadin.server.AbstractExtension;
 import com.vaadin.server.ClientConnector;
+import com.vaadin.server.KeyMapper;
 import com.vaadin.shared.data.DataProviderRpc;
 import com.vaadin.shared.data.DataRequestRpc;
 import com.vaadin.shared.ui.grid.GridClientRpc;
@@ -71,214 +69,21 @@ import elemental.json.JsonObject;
 public class RpcDataProviderExtension extends AbstractExtension {
 
     /**
-     * ItemId to Key to ItemId mapper.
-     * <p>
-     * This class is used when transmitting information about items in container
-     * related to Grid. It introduces a consistent way of mapping ItemIds and
-     * its container to a String that can be mapped back to ItemId.
-     * <p>
-     * <em>Technical note:</em> This class also keeps tabs on which indices are
-     * being shown/selected, and is able to clean up after itself once the
-     * itemId &lrarr; key mapping is not needed anymore. In other words, this
-     * doesn't leak memory.
-     */
-    public class DataProviderKeyMapper implements Serializable, DataGenerator {
-        private final BiMap<Object, String> itemIdToKey = HashBiMap.create();
-        private Set<Object> pinnedItemIds = new HashSet<Object>();
-        private long rollingIndex = 0;
-
-        private DataProviderKeyMapper() {
-            // private implementation
-        }
-
-        private String nextKey() {
-            return String.valueOf(rollingIndex++);
-        }
-
-        /**
-         * Gets the key for a given item id. Creates a new key mapping if no
-         * existing mapping was found for the given item id.
-         * 
-         * @since 7.5.0
-         * @param itemId
-         *            the item id to get the key for
-         * @return the key for the given item id
-         */
-        public String getKey(Object itemId) {
-            String key = itemIdToKey.get(itemId);
-            if (key == null) {
-                key = nextKey();
-                itemIdToKey.put(itemId, key);
-            }
-            return key;
-        }
-
-        /**
-         * Gets keys for a collection of item ids.
-         * <p>
-         * If the itemIds are currently cached, the existing keys will be used.
-         * Otherwise new ones will be created.
-         * 
-         * @param itemIds
-         *            the item ids for which to get keys
-         * @return keys for the {@code itemIds}
-         */
-        public List<String> getKeys(Collection<?> itemIds) {
-            if (itemIds == null) {
-                throw new IllegalArgumentException("itemIds can't be null");
-            }
-
-            ArrayList<String> keys = new ArrayList<String>(itemIds.size());
-            for (Object itemId : itemIds) {
-                keys.add(getKey(itemId));
-            }
-            return keys;
-        }
-
-        /**
-         * Gets the registered item id based on its key.
-         * <p>
-         * A key is used to identify a particular row on both a server and a
-         * client. This method can be used to get the item id for the row key
-         * that the client has sent.
-         * 
-         * @param key
-         *            the row key for which to retrieve an item id
-         * @return the item id corresponding to {@code key}
-         * @throws IllegalStateException
-         *             if the key mapper does not have a record of {@code key} .
-         */
-        public Object getItemId(String key) throws IllegalStateException {
-            Object itemId = itemIdToKey.inverse().get(key);
-            if (itemId != null) {
-                return itemId;
-            } else {
-                throw new IllegalStateException("No item id for key " + key
-                        + " found.");
-            }
-        }
-
-        /**
-         * Gets corresponding item ids for each of the keys in a collection.
-         * 
-         * @param keys
-         *            the keys for which to retrieve item ids
-         * @return a collection of item ids for the {@code keys}
-         * @throws IllegalStateException
-         *             if one or more of keys don't have a corresponding item id
-         *             in the cache
-         */
-        public Collection<Object> getItemIds(Collection<String> keys)
-                throws IllegalStateException {
-            if (keys == null) {
-                throw new IllegalArgumentException("keys may not be null");
-            }
-
-            ArrayList<Object> itemIds = new ArrayList<Object>(keys.size());
-            for (String key : keys) {
-                itemIds.add(getItemId(key));
-            }
-            return itemIds;
-        }
-
-        /**
-         * Pin an item id to be cached indefinitely.
-         * <p>
-         * Normally when an itemId is not an active row, it is discarded from
-         * the cache. Pinning an item id will make sure that it is kept in the
-         * cache.
-         * <p>
-         * In effect, while an item id is pinned, it always has the same key.
-         * 
-         * @param itemId
-         *            the item id to pin
-         * @throws IllegalStateException
-         *             if {@code itemId} was already pinned
-         * @see #unpin(Object)
-         * @see #isPinned(Object)
-         * @see #getItemIds(Collection)
-         */
-        public void pin(Object itemId) throws IllegalStateException {
-            if (isPinned(itemId)) {
-                throw new IllegalStateException("Item id " + itemId
-                        + " was pinned already");
-            }
-            pinnedItemIds.add(itemId);
-        }
-
-        /**
-         * Unpin an item id.
-         * <p>
-         * This cancels the effect of pinning an item id. If the item id is
-         * currently inactive, it will be immediately removed from the cache.
-         * 
-         * @param itemId
-         *            the item id to unpin
-         * @throws IllegalStateException
-         *             if {@code itemId} was not pinned
-         * @see #pin(Object)
-         * @see #isPinned(Object)
-         * @see #getItemIds(Collection)
-         */
-        public void unpin(Object itemId) throws IllegalStateException {
-            if (!isPinned(itemId)) {
-                throw new IllegalStateException("Item id " + itemId
-                        + " was not pinned");
-            }
-
-            pinnedItemIds.remove(itemId);
-        }
-
-        /**
-         * Checks whether an item id is pinned or not.
-         * 
-         * @param itemId
-         *            the item id to check for pin status
-         * @return {@code true} iff the item id is currently pinned
-         */
-        public boolean isPinned(Object itemId) {
-            return pinnedItemIds.contains(itemId);
-        }
-
-        /**
-         * {@inheritDoc}
-         * 
-         * @since 7.6
-         */
-        @Override
-        public void generateData(Object itemId, Item item, JsonObject rowData) {
-            rowData.put(GridState.JSONKEY_ROWKEY, getKey(itemId));
-        }
-
-        /**
-         * Removes all inactive item id to key mapping from the key mapper.
-         * 
-         * @since 7.6
-         */
-        public void dropInactiveItems() {
-            Collection<Object> active = activeItemHandler.getActiveItemIds();
-            Iterator<Object> itemIter = itemIdToKey.keySet().iterator();
-            while (itemIter.hasNext()) {
-                Object itemId = itemIter.next();
-                if (!active.contains(itemId) && !isPinned(itemId)) {
-                    itemIter.remove();
-                }
-            }
-        }
-    }
-
-    /**
      * Class for keeping track of current items and ValueChangeListeners.
      * 
      * @since 7.6
      */
-    private class ActiveItemHandler implements Serializable {
+    private class ActiveItemHandler implements Serializable, DataGenerator {
 
         private final Map<Object, GridValueChangeListener> activeItemMap = new HashMap<Object, GridValueChangeListener>();
+        private final KeyMapper<Object> keyMapper = new KeyMapper<Object>();
         private final Set<Object> droppedItems = new HashSet<Object>();
 
         /**
-         * Registers ValueChangeListeners for given items ids.
+         * Registers ValueChangeListeners for given item ids.
+         * <p>
+         * Note: This method will clean up any unneeded listeners and key
+         * mappings
          * 
          * @param itemIds
          *            collection of new active item ids
@@ -293,7 +98,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
 
             // Remove still active rows that were "dropped"
             droppedItems.removeAll(itemIds);
-            dropListeners(droppedItems);
+            internalDropActiveItems(droppedItems);
             droppedItems.clear();
         }
 
@@ -310,11 +115,12 @@ public class RpcDataProviderExtension extends AbstractExtension {
             }
         }
 
-        private void dropListeners(Collection<Object> itemIds) {
+        private void internalDropActiveItems(Collection<Object> itemIds) {
             for (Object itemId : droppedItems) {
                 assert activeItemMap.containsKey(itemId) : "Item ID should exist in the activeItemMap";
 
                 activeItemMap.remove(itemId).removeListener();
+                keyMapper.remove(itemId);
             }
         }
 
@@ -335,6 +141,12 @@ public class RpcDataProviderExtension extends AbstractExtension {
         public Collection<GridValueChangeListener> getValueChangeListeners() {
             return new HashSet<GridValueChangeListener>(activeItemMap.values());
         }
+
+        @Override
+        public void generateData(Object itemId, Item item, JsonObject rowData) {
+            rowData.put(GridState.JSONKEY_ROWKEY, keyMapper.key(itemId));
+        }
+
     }
 
     /**
@@ -635,8 +447,6 @@ public class RpcDataProviderExtension extends AbstractExtension {
         }
     };
 
-    private final DataProviderKeyMapper keyMapper = new DataProviderKeyMapper();
-
     /** RpcDataProvider should send the current cache again. */
     private boolean refreshCache = false;
 
@@ -684,24 +494,10 @@ public class RpcDataProviderExtension extends AbstractExtension {
             }
 
             @Override
-            public void setPinned(String key, boolean isPinned) {
-                Object itemId = keyMapper.getItemId(key);
-                if (isPinned) {
-                    // Row might already be pinned if it was selected from the
-                    // server
-                    if (!keyMapper.isPinned(itemId)) {
-                        keyMapper.pin(itemId);
-                    }
-                } else {
-                    keyMapper.unpin(itemId);
-                }
-            }
-
-            @Override
             public void dropRows(JsonArray rowKeys) {
                 for (int i = 0; i < rowKeys.length(); ++i) {
-                    activeItemHandler.dropActiveItem(keyMapper
-                            .getItemId(rowKeys.getString(i)));
+                    activeItemHandler.dropActiveItem(getKeyMapper().get(
+                            rowKeys.getString(i)));
                 }
             }
         });
@@ -711,7 +507,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
                     .addItemSetChangeListener(itemListener);
         }
 
-        addDataGenerator(keyMapper);
+        addDataGenerator(activeItemHandler);
         addDataGenerator(detailComponentManager);
     }
 
@@ -787,7 +583,6 @@ public class RpcDataProviderExtension extends AbstractExtension {
         rpc.setRowData(firstRowToPush, rows);
 
         activeItemHandler.addActiveItems(itemIds);
-        keyMapper.dropInactiveItems();
     }
 
     private JsonObject getRowData(Collection<Column> columns, Object itemId) {
@@ -939,7 +734,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
     public void setParent(ClientConnector parent) {
         if (parent == null) {
             // We're being detached, release various listeners
-            activeItemHandler.dropListeners(activeItemHandler
+            activeItemHandler.internalDropActiveItems(activeItemHandler
                     .getActiveItemIds());
 
             if (container instanceof ItemSetChangeNotifier) {
@@ -987,8 +782,8 @@ public class RpcDataProviderExtension extends AbstractExtension {
         refreshCache();
     }
 
-    public DataProviderKeyMapper getKeyMapper() {
-        return keyMapper;
+    public KeyMapper<Object> getKeyMapper() {
+        return activeItemHandler.keyMapper;
     }
 
     protected Grid getGrid() {
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 204f771ee5..215df3a6a3 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -57,7 +57,6 @@ import com.vaadin.data.DataGenerator;
 import com.vaadin.data.Item;
 import com.vaadin.data.Property;
 import com.vaadin.data.RpcDataProviderExtension;
-import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper;
 import com.vaadin.data.RpcDataProviderExtension.DetailComponentManager;
 import com.vaadin.data.Validator.InvalidValueException;
 import com.vaadin.data.fieldgroup.DefaultFieldGroupFieldFactory;
@@ -3856,7 +3855,7 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
          * @return the item id corresponding to {@code key}
          */
         protected Object getItemId(String rowKey) {
-            return getParentGrid().getKeyMapper().getItemId(rowKey);
+            return getParentGrid().getKeyMapper().get(rowKey);
         }
 
         /**
@@ -4139,7 +4138,7 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
             @Override
             public void itemClick(String rowKey, String columnId,
                     MouseEventDetails details) {
-                Object itemId = getKeyMapper().getItemId(rowKey);
+                Object itemId = getKeyMapper().get(rowKey);
                 Item item = datasource.getItem(itemId);
                 Object propertyId = getPropertyIdByColumnId(columnId);
                 fireEvent(new ItemClickEvent(Grid.this, item, itemId,
@@ -4222,20 +4221,20 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
 
             @Override
             public void editorOpen(String rowKey) {
-                fireEvent(new EditorOpenEvent(Grid.this, getKeyMapper()
-                        .getItemId(rowKey)));
+                fireEvent(new EditorOpenEvent(Grid.this, getKeyMapper().get(
+                        rowKey)));
             }
 
             @Override
             public void editorMove(String rowKey) {
-                fireEvent(new EditorMoveEvent(Grid.this, getKeyMapper()
-                        .getItemId(rowKey)));
+                fireEvent(new EditorMoveEvent(Grid.this, getKeyMapper().get(
+                        rowKey)));
             }
 
             @Override
             public void editorClose(String rowKey) {
-                fireEvent(new EditorCloseEvent(Grid.this, getKeyMapper()
-                        .getItemId(rowKey)));
+                fireEvent(new EditorCloseEvent(Grid.this, getKeyMapper().get(
+                        rowKey)));
             }
         });
 
@@ -5299,7 +5298,7 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
      * 
      * @return the key mapper being used by the data source
      */
-    DataProviderKeyMapper getKeyMapper() {
+    KeyMapper<Object> getKeyMapper() {
         return datasourceExtension.getKeyMapper();
     }
 
diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java b/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java
deleted file mode 100644
index 9ecf131c5b..0000000000
--- a/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java
+++ /dev/null
@@ -1,88 +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.tests.server.component.grid;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-import java.util.Arrays;
-
-import org.junit.Before;
-import org.junit.Test;
-
-import com.vaadin.data.Container;
-import com.vaadin.data.Container.Indexed;
-import com.vaadin.data.Item;
-import com.vaadin.data.Property;
-import com.vaadin.data.RpcDataProviderExtension;
-import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper;
-import com.vaadin.data.util.IndexedContainer;
-
-public class DataProviderExtension {
-    private RpcDataProviderExtension dataProvider;
-    private DataProviderKeyMapper keyMapper;
-    private Container.Indexed container;
-
-    private static final Object ITEM_ID1 = "itemid1";
-    private static final Object ITEM_ID2 = "itemid2";
-    private static final Object ITEM_ID3 = "itemid3";
-
-    private static final Object PROPERTY_ID1_STRING = "property1";
-
-    @Before
-    public void setup() {
-        container = new IndexedContainer();
-        populate(container);
-
-        dataProvider = new RpcDataProviderExtension(container);
-        keyMapper = dataProvider.getKeyMapper();
-    }
-
-    private static void populate(Indexed container) {
-        container.addContainerProperty(PROPERTY_ID1_STRING, String.class, "");
-        for (Object itemId : Arrays.asList(ITEM_ID1, ITEM_ID2, ITEM_ID3)) {
-            final Item item = container.addItem(itemId);
-            @SuppressWarnings("unchecked")
-            final Property<String> stringProperty = item
-                    .getItemProperty(PROPERTY_ID1_STRING);
-            stringProperty.setValue(itemId.toString());
-        }
-    }
-
-    @Test
-    public void pinBasics() {
-        assertFalse("itemId1 should not start as pinned",
-                keyMapper.isPinned(ITEM_ID2));
-
-        keyMapper.pin(ITEM_ID1);
-        assertTrue("itemId1 should now be pinned", keyMapper.isPinned(ITEM_ID1));
-
-        keyMapper.unpin(ITEM_ID1);
-        assertFalse("itemId1 should not be pinned anymore",
-                keyMapper.isPinned(ITEM_ID2));
-    }
-
-    @Test(expected = IllegalStateException.class)
-    public void doublePinning() {
-        keyMapper.pin(ITEM_ID1);
-        keyMapper.pin(ITEM_ID1);
-    }
-
-    @Test(expected = IllegalStateException.class)
-    public void nonexistentUnpin() {
-        keyMapper.unpin(ITEM_ID1);
-    }
-}
diff --git a/shared/src/com/vaadin/shared/data/DataRequestRpc.java b/shared/src/com/vaadin/shared/data/DataRequestRpc.java
index b66965fae9..4b553dda68 100644
--- a/shared/src/com/vaadin/shared/data/DataRequestRpc.java
+++ b/shared/src/com/vaadin/shared/data/DataRequestRpc.java
@@ -47,20 +47,6 @@ public interface DataRequestRpc extends ServerRpc {
             int firstCachedRowIndex, int cacheSize);
 
     /**
-     * Informs the server that an item referenced with a key pinned status has
-     * changed. This is a delayed call that happens along with next rpc call to
-     * server.
-     * 
-     * @param key
-     *            key mapping to item
-     * @param isPinned
-     *            pinned status of referenced item
-     */
-    @Delayed
-    @NoLoadingIndicator
-    public void setPinned(String key, boolean isPinned);
-
-    /**
      * Informs the server that items have been dropped from the client cache.
      * 
      * @since 7.6
diff --git a/uitest/src/com/vaadin/tests/components/grid/CustomRendererTest.java b/uitest/src/com/vaadin/tests/components/grid/CustomRendererTest.java
index 7d706ecd30..d5e01a9de8 100644
--- a/uitest/src/com/vaadin/tests/components/grid/CustomRendererTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/CustomRendererTest.java
@@ -47,8 +47,8 @@ public class CustomRendererTest extends MultiBrowserTest {
                 .getText());
 
         grid.getCell(0, 1).click();
-        assertEquals("row: 0, key: 0", grid.getCell(0, 1).getText());
-        assertEquals("key: 0, itemId: " + CustomRenderer.ITEM_ID,
+        assertEquals("row: 0, key: 1", grid.getCell(0, 1).getText());
+        assertEquals("key: 1, itemId: " + CustomRenderer.ITEM_ID,
                 findDebugLabel().getText());
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java b/uitest/src/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java
index b38178b156..2e86053ef3 100644
--- a/uitest/src/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java
@@ -43,6 +43,6 @@ public class JavaScriptRenderersTest extends MultiBrowserTest {
         // Verify onbrowserevent
         cell_1_1.click();
         Assert.assertTrue(cell_1_1.getText().startsWith(
-                "Clicked 1 with key 1 at"));
+                "Clicked 1 with key 2 at"));
     }
 }