Move DetailComponentManager from Grid to RDPE (#16644)

Change-Id: I2b65a878bb50c2b1f62135a998207a41e82fe62f

3 files changed, 271 insertions, 253 deletions

diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java
index 8d7b654468..66c17c4afa 100644
--- a/server/src/com/vaadin/data/RpcDataProviderExtension.java
+++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java
@@ -25,12 +25,15 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.logging.Logger;
 
 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;
 import com.vaadin.data.Container.Indexed;
 import com.vaadin.data.Container.Indexed.ItemAddEvent;
 import com.vaadin.data.Container.Indexed.ItemRemoveEvent;
@@ -46,13 +49,16 @@ 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.DetailsConnectorChange;
 import com.vaadin.shared.ui.grid.GridState;
 import com.vaadin.shared.ui.grid.Range;
+import com.vaadin.shared.util.SharedUtil;
+import com.vaadin.ui.Component;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.CellReference;
 import com.vaadin.ui.Grid.CellStyleGenerator;
 import com.vaadin.ui.Grid.Column;
-import com.vaadin.ui.Grid.DetailComponentManager;
+import com.vaadin.ui.Grid.DetailsGenerator;
 import com.vaadin.ui.Grid.RowReference;
 import com.vaadin.ui.Grid.RowStyleGenerator;
 import com.vaadin.ui.renderers.Renderer;
@@ -578,6 +584,253 @@ public class RpcDataProviderExtension extends AbstractExtension {
         }
     }
 
+    /**
+     * A class that makes detail component related internal communication
+     * possible between {@link RpcDataProviderExtension} and grid.
+     * 
+     * @since
+     * @author Vaadin Ltd
+     */
+    public static final class DetailComponentManager implements Serializable {
+        /**
+         * This map represents all the components that have been requested for
+         * each item id.
+         * <p>
+         * Normally this map is consistent with what is displayed in the
+         * component hierarchy (and thus the DOM). The only time this map is out
+         * of sync with the DOM is between the any calls to
+         * {@link #createDetails(Object, int)} or
+         * {@link #destroyDetails(Object)}, and
+         * {@link GridClientRpc#setDetailsConnectorChanges(Set)}.
+         * <p>
+         * This is easily checked: if {@link #unattachedComponents} is
+         * {@link Collection#isEmpty() empty}, then this field is consistent
+         * with the connector hierarchy.
+         */
+        private final Map<Object, Component> visibleDetailsComponents = Maps
+                .newHashMap();
+
+        /** A lookup map for which row contains which details component. */
+        private BiMap<Integer, Component> rowIndexToDetails = HashBiMap
+                .create();
+
+        /**
+         * A copy of {@link #rowIndexToDetails} from its last stable state. Used
+         * for creating a diff against {@link #rowIndexToDetails}.
+         * 
+         * @see #getAndResetConnectorChanges()
+         */
+        private BiMap<Integer, Component> prevRowIndexToDetails = HashBiMap
+                .create();
+
+        /**
+         * A set keeping track on components that have been created, but not
+         * attached. They should be attached at some later point in time.
+         * <p>
+         * This isn't strictly requried, but it's a handy explicit log. You
+         * could find out the same thing by taking out all the other components
+         * and checking whether Grid is their parent or not.
+         */
+        private final Set<Component> unattachedComponents = Sets.newHashSet();
+
+        /**
+         * Keeps tabs on all the details that did not get a component during
+         * {@link #createDetails(Object, int)}.
+         */
+        private final Map<Object, Integer> emptyDetails = Maps.newHashMap();
+
+        private Grid grid;
+
+        /**
+         * Creates a details component by the request of the client side, with
+         * the help of the user-defined {@link DetailsGenerator}.
+         * <p>
+         * Also keeps internal bookkeeping up to date.
+         * 
+         * @param itemId
+         *            the item id for which to create the details component.
+         *            Assumed not <code>null</code> and that a component is not
+         *            currently present for this item previously
+         * @param rowIndex
+         *            the row index for {@code itemId}
+         * @throws IllegalStateException
+         *             if the current details generator provides a component
+         *             that was manually attached, or if the same instance has
+         *             already been provided
+         */
+        public void createDetails(Object itemId, int rowIndex)
+                throws IllegalStateException {
+            assert itemId != null : "itemId was null";
+            Integer newRowIndex = Integer.valueOf(rowIndex);
+
+            assert !visibleDetailsComponents.containsKey(itemId) : "itemId "
+                    + "already has a component. Should be destroyed first.";
+
+            RowReference rowReference = new RowReference(grid);
+            rowReference.set(itemId);
+
+            DetailsGenerator detailsGenerator = grid.getDetailsGenerator();
+            Component details = detailsGenerator.getDetails(rowReference);
+            if (details != null) {
+                String generatorName = detailsGenerator.getClass().getName();
+                if (details.getParent() != null) {
+                    throw new IllegalStateException(generatorName
+                            + " generated a details component that already "
+                            + "was attached. (itemId: " + itemId + ", row: "
+                            + rowIndex + ", component: " + details);
+                }
+
+                if (rowIndexToDetails.containsValue(details)) {
+                    throw new IllegalStateException(generatorName
+                            + " provided a details component that already "
+                            + "exists in Grid. (itemId: " + itemId + ", row: "
+                            + rowIndex + ", component: " + details);
+                }
+
+                visibleDetailsComponents.put(itemId, details);
+                rowIndexToDetails.put(newRowIndex, details);
+                unattachedComponents.add(details);
+
+                assert !emptyDetails.containsKey(itemId) : "Bookeeping thinks "
+                        + "itemId is empty even though we just created a "
+                        + "component for it (" + itemId + ")";
+            } else {
+                assert !emptyDetails.containsKey(itemId) : "Bookkeeping has "
+                        + "already itemId marked as empty (itemId: " + itemId
+                        + ", old index: " + emptyDetails.get(itemId)
+                        + ", new index: " + newRowIndex + ")";
+                assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping"
+                        + " already had another itemId for this empty index "
+                        + "(index: " + newRowIndex + ", new itemId: " + itemId
+                        + ")";
+                emptyDetails.put(itemId, newRowIndex);
+            }
+
+            /*
+             * Don't attach the components here. It's done by
+             * GridServerRpc.sendDetailsComponents in a separate roundtrip.
+             */
+        }
+
+        /**
+         * Destroys correctly a details component, by the request of the client
+         * side.
+         * <p>
+         * Also keeps internal bookkeeping up to date.
+         * 
+         * @param itemId
+         *            the item id for which to destroy the details component
+         */
+        public void destroyDetails(Object itemId) {
+            emptyDetails.remove(itemId);
+
+            Component removedComponent = visibleDetailsComponents
+                    .remove(itemId);
+            if (removedComponent == null) {
+                return;
+            }
+
+            rowIndexToDetails.inverse().remove(removedComponent);
+
+            removedComponent.setParent(null);
+            grid.markAsDirty();
+        }
+
+        /**
+         * Gets all details components that are currently attached to the grid.
+         * <p>
+         * Used internally by the Grid object.
+         * 
+         * @return all details components that are currently attached to the
+         *         grid
+         */
+        public Collection<Component> getComponents() {
+            Set<Component> components = new HashSet<Component>(
+                    visibleDetailsComponents.values());
+            components.removeAll(unattachedComponents);
+            return components;
+        }
+
+        /**
+         * Gets information on how the connectors have changed.
+         * <p>
+         * This method only returns the changes that have been made between two
+         * calls of this method. I.e. Calling this method once will reset the
+         * state for the next state.
+         * <p>
+         * Used internally by the Grid object.
+         * 
+         * @return information on how the connectors have changed
+         */
+        public Set<DetailsConnectorChange> getAndResetConnectorChanges() {
+            Set<DetailsConnectorChange> changes = new HashSet<DetailsConnectorChange>();
+
+            // populate diff with added/changed
+            for (Entry<Integer, Component> entry : rowIndexToDetails.entrySet()) {
+                Component component = entry.getValue();
+                assert component != null : "rowIndexToDetails contains a null component";
+
+                Integer newIndex = entry.getKey();
+                Integer oldIndex = prevRowIndexToDetails.inverse().get(
+                        component);
+
+                /*
+                 * only attach components. Detaching already happened in
+                 * destroyDetails.
+                 */
+                if (newIndex != null && oldIndex == null) {
+                    assert unattachedComponents.contains(component) : "unattachedComponents does not contain component for index "
+                            + newIndex + " (" + component + ")";
+                    component.setParent(grid);
+                    unattachedComponents.remove(component);
+                }
+
+                if (!SharedUtil.equals(oldIndex, newIndex)) {
+                    changes.add(new DetailsConnectorChange(component, oldIndex,
+                            newIndex));
+                }
+            }
+
+            // populate diff with removed
+            for (Entry<Integer, Component> entry : prevRowIndexToDetails
+                    .entrySet()) {
+                Integer oldIndex = entry.getKey();
+                Component component = entry.getValue();
+                Integer newIndex = rowIndexToDetails.inverse().get(component);
+                if (newIndex == null) {
+                    changes.add(new DetailsConnectorChange(null, oldIndex, null));
+                }
+            }
+
+            // reset diff map
+            prevRowIndexToDetails = HashBiMap.create(rowIndexToDetails);
+
+            return changes;
+        }
+
+        public void refresh(Object itemId) {
+            Component component = visibleDetailsComponents.get(itemId);
+            Integer rowIndex = null;
+            if (component != null) {
+                rowIndex = rowIndexToDetails.inverse().get(component);
+                destroyDetails(itemId);
+            } else {
+                rowIndex = emptyDetails.remove(itemId);
+            }
+
+            assert rowIndex != null : "Given itemId does not map to an "
+                    + "existing detail row (" + itemId + ")";
+            createDetails(itemId, rowIndex.intValue());
+        }
+
+        void setGrid(Grid grid) {
+            if (this.grid != null) {
+                throw new IllegalStateException("Grid may injected only once.");
+            }
+            this.grid = grid;
+        }
+    }
+
     private final Indexed container;
 
     private final ActiveRowHandler activeRowHandler = new ActiveRowHandler();
@@ -685,7 +938,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
      */
     private Set<Object> visibleDetails = new HashSet<Object>();
 
-    private DetailComponentManager detailComponentManager;
+    private final DetailComponentManager detailComponentManager = new DetailComponentManager();
 
     /**
      * Creates a new data provider using the given container.
@@ -693,10 +946,8 @@ public class RpcDataProviderExtension extends AbstractExtension {
      * @param container
      *            the container to make available
      */
-    public RpcDataProviderExtension(Indexed container,
-            DetailComponentManager detailComponentManager) {
+    public RpcDataProviderExtension(Indexed container) {
         this.container = container;
-        this.detailComponentManager = detailComponentManager;
         rpc = getRpcProxy(DataProviderRpc.class);
 
         registerRpc(new DataRequestRpc() {
@@ -884,9 +1135,12 @@ public class RpcDataProviderExtension extends AbstractExtension {
      * 
      * @param component
      *            the remote data grid component to extend
+     * @param columnKeys
+     *            the key mapper for columns
      */
     public void extend(Grid component, KeyMapper<Object> columnKeys) {
         this.columnKeys = columnKeys;
+        detailComponentManager.setGrid(component);
         super.extend(component);
     }
 
@@ -1171,4 +1425,9 @@ public class RpcDataProviderExtension extends AbstractExtension {
          */
         return container.indexOfId(itemId);
     }
+
+    /** Gets the detail component manager for this data provider */
+    public DetailComponentManager getDetailComponentManager() {
+        return detailComponentManager;
+    }
 }
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 0e035ae524..4488789406 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -37,9 +37,6 @@ import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-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.Maps;
 import com.google.gwt.thirdparty.guava.common.collect.Sets;
 import com.google.gwt.thirdparty.guava.common.collect.Sets.SetView;
 import com.vaadin.data.Container;
@@ -52,6 +49,7 @@ 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;
 import com.vaadin.data.fieldgroup.FieldGroup;
@@ -80,7 +78,6 @@ import com.vaadin.server.KeyMapper;
 import com.vaadin.server.VaadinSession;
 import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.data.sort.SortDirection;
-import com.vaadin.shared.ui.grid.DetailsConnectorChange;
 import com.vaadin.shared.ui.grid.EditorClientRpc;
 import com.vaadin.shared.ui.grid.EditorServerRpc;
 import com.vaadin.shared.ui.grid.GridClientRpc;
@@ -2996,246 +2993,6 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
     }
 
     /**
-     * A class that makes detail component related internal communication
-     * possible between {@link RpcDataProviderExtension} and grid.
-     * 
-     * @since
-     * @author Vaadin Ltd
-     */
-    public final class DetailComponentManager implements Serializable {
-        /**
-         * This map represents all the components that have been requested for
-         * each item id.
-         * <p>
-         * Normally this map is consistent with what is displayed in the
-         * component hierarchy (and thus the DOM). The only time this map is out
-         * of sync with the DOM is between the any calls to
-         * {@link #createDetails(Object, int)} or
-         * {@link #destroyDetails(Object)}, and
-         * {@link GridClientRpc#setDetailsConnectorChanges(Set)}.
-         * <p>
-         * This is easily checked: if {@link #unattachedComponents} is
-         * {@link Collection#isEmpty() empty}, then this field is consistent
-         * with the connector hierarchy.
-         */
-        private final Map<Object, Component> visibleDetailsComponents = Maps
-                .newHashMap();
-
-        /** A lookup map for which row contains which details component. */
-        private BiMap<Integer, Component> rowIndexToDetails = HashBiMap
-                .create();
-
-        /**
-         * A copy of {@link #rowIndexToDetails} from its last stable state. Used
-         * for creating a diff against {@link #rowIndexToDetails}.
-         * 
-         * @see #getAndResetConnectorChanges()
-         */
-        private BiMap<Integer, Component> prevRowIndexToDetails = HashBiMap
-                .create();
-
-        /**
-         * A set keeping track on components that have been created, but not
-         * attached. They should be attached at some later point in time.
-         * <p>
-         * This isn't strictly requried, but it's a handy explicit log. You
-         * could find out the same thing by taking out all the other components
-         * and checking whether Grid is their parent or not.
-         */
-        private final Set<Component> unattachedComponents = Sets.newHashSet();
-
-        /**
-         * Keeps tabs on all the details that did not get a component during
-         * {@link #createDetails(Object, int)}.
-         */
-        private final Map<Object, Integer> emptyDetails = Maps.newHashMap();
-
-        /**
-         * Creates a details component by the request of the client side, with
-         * the help of the user-defined {@link DetailsGenerator}.
-         * <p>
-         * Also keeps internal bookkeeping up to date.
-         * 
-         * @param itemId
-         *            the item id for which to create the details component.
-         *            Assumed not <code>null</code> and that a component is not
-         *            currently present for this item previously
-         * @param rowIndex
-         *            the row index for {@code itemId}
-         * @throws IllegalStateException
-         *             if the current details generator provides a component
-         *             that was manually attached, or if the same instance has
-         *             already been provided
-         */
-        public void createDetails(Object itemId, int rowIndex)
-                throws IllegalStateException {
-            assert itemId != null : "itemId was null";
-            Integer newRowIndex = Integer.valueOf(rowIndex);
-
-            assert !visibleDetailsComponents.containsKey(itemId) : "itemId "
-                    + "already has a component. Should be destroyed first.";
-
-            RowReference rowReference = new RowReference(Grid.this);
-            rowReference.set(itemId);
-
-            Component details = getDetailsGenerator().getDetails(rowReference);
-            if (details != null) {
-                if (details.getParent() != null) {
-                    String generatorName = getDetailsGenerator().getClass()
-                            .getName();
-                    throw new IllegalStateException(generatorName
-                            + " generated a details component that already "
-                            + "was attached. (itemId: " + itemId + ", row: "
-                            + rowIndex + ", component: " + details);
-                }
-
-                if (rowIndexToDetails.containsValue(details)) {
-                    String generatorName = getDetailsGenerator().getClass()
-                            .getName();
-                    throw new IllegalStateException(generatorName
-                            + " provided a details component that already "
-                            + "exists in Grid. (itemId: " + itemId + ", row: "
-                            + rowIndex + ", component: " + details);
-                }
-
-                visibleDetailsComponents.put(itemId, details);
-                rowIndexToDetails.put(newRowIndex, details);
-                unattachedComponents.add(details);
-
-                assert !emptyDetails.containsKey(itemId) : "Bookeeping thinks "
-                        + "itemId is empty even though we just created a "
-                        + "component for it (" + itemId + ")";
-            } else {
-                assert !emptyDetails.containsKey(itemId) : "Bookkeeping has "
-                        + "already itemId marked as empty (itemId: " + itemId
-                        + ", old index: " + emptyDetails.get(itemId)
-                        + ", new index: " + newRowIndex + ")";
-                assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping"
-                        + " already had another itemId for this empty index "
-                        + "(index: " + newRowIndex + ", new itemId: " + itemId
-                        + ")";
-                emptyDetails.put(itemId, newRowIndex);
-            }
-
-            /*
-             * Don't attach the components here. It's done by
-             * GridServerRpc.sendDetailsComponents in a separate roundtrip.
-             */
-        }
-
-        /**
-         * Destroys correctly a details component, by the request of the client
-         * side.
-         * <p>
-         * Also keeps internal bookkeeping up to date.
-         * 
-         * @param itemId
-         *            the item id for which to destroy the details component
-         */
-        public void destroyDetails(Object itemId) {
-            emptyDetails.remove(itemId);
-
-            Component removedComponent = visibleDetailsComponents
-                    .remove(itemId);
-            if (removedComponent == null) {
-                return;
-            }
-
-            rowIndexToDetails.inverse().remove(removedComponent);
-
-            removedComponent.setParent(null);
-            markAsDirty();
-        }
-
-        /**
-         * Gets all details components that are currently attached to the grid.
-         * <p>
-         * Used internally by the Grid object.
-         * 
-         * @return all details components that are currently attached to the
-         *         grid
-         */
-        Collection<Component> getComponents() {
-            Set<Component> components = new HashSet<Component>(
-                    visibleDetailsComponents.values());
-            components.removeAll(unattachedComponents);
-            return components;
-        }
-
-        /**
-         * Gets information on how the connectors have changed.
-         * <p>
-         * This method only returns the changes that have been made between two
-         * calls of this method. I.e. Calling this method once will reset the
-         * state for the next state.
-         * <p>
-         * Used internally by the Grid object.
-         * 
-         * @return information on how the connectors have changed
-         */
-        Set<DetailsConnectorChange> getAndResetConnectorChanges() {
-            Set<DetailsConnectorChange> changes = new HashSet<DetailsConnectorChange>();
-
-            // populate diff with added/changed
-            for (Entry<Integer, Component> entry : rowIndexToDetails.entrySet()) {
-                Component component = entry.getValue();
-                assert component != null : "rowIndexToDetails contains a null component";
-
-                Integer newIndex = entry.getKey();
-                Integer oldIndex = prevRowIndexToDetails.inverse().get(
-                        component);
-
-                /*
-                 * only attach components. Detaching already happened in
-                 * destroyDetails.
-                 */
-                if (newIndex != null && oldIndex == null) {
-                    assert unattachedComponents.contains(component) : "unattachedComponents does not contain component for index "
-                            + newIndex + " (" + component + ")";
-                    component.setParent(Grid.this);
-                    unattachedComponents.remove(component);
-                }
-
-                if (!SharedUtil.equals(oldIndex, newIndex)) {
-                    changes.add(new DetailsConnectorChange(component, oldIndex,
-                            newIndex));
-                }
-            }
-
-            // populate diff with removed
-            for (Entry<Integer, Component> entry : prevRowIndexToDetails
-                    .entrySet()) {
-                Integer oldIndex = entry.getKey();
-                Component component = entry.getValue();
-                Integer newIndex = rowIndexToDetails.inverse().get(component);
-                if (newIndex == null) {
-                    changes.add(new DetailsConnectorChange(null, oldIndex, null));
-                }
-            }
-
-            // reset diff map
-            prevRowIndexToDetails = HashBiMap.create(rowIndexToDetails);
-
-            return changes;
-        }
-
-        public void refresh(Object itemId) {
-            Component component = visibleDetailsComponents.get(itemId);
-            Integer rowIndex = null;
-            if (component != null) {
-                rowIndex = rowIndexToDetails.inverse().get(component);
-                destroyDetails(itemId);
-            } else {
-                rowIndex = emptyDetails.remove(itemId);
-            }
-
-            assert rowIndex != null : "Given itemId does not map to an existing detail row ("
-                    + itemId + ")";
-            createDetails(itemId, rowIndex.intValue());
-        }
-    }
-
-    /**
      * The data source attached to the grid
      */
     private Container.Indexed datasource;
@@ -3349,7 +3106,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
      */
     private DetailsGenerator detailsGenerator = DetailsGenerator.NULL;
 
-    private final DetailComponentManager detailComponentManager = new DetailComponentManager();
+    private DetailComponentManager detailComponentManager = null;
 
     private static final Method SELECTION_CHANGE_METHOD = ReflectTools
             .findMethod(SelectionListener.class, "select", SelectionEvent.class);
@@ -3765,10 +3522,12 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             sortOrder.clear();
         }
 
-        datasourceExtension = new RpcDataProviderExtension(container,
-                detailComponentManager);
+        datasourceExtension = new RpcDataProviderExtension(container);
         datasourceExtension.extend(this, columnKeys);
 
+        detailComponentManager = datasourceExtension
+                .getDetailComponentManager();
+
         /*
          * selectionModel == null when the invocation comes from the
          * constructor.
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
index 54f5dcdbc7..9ecf131c5b 100644
--- a/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java
+++ b/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java
@@ -47,7 +47,7 @@ public class DataProviderExtension {
         container = new IndexedContainer();
         populate(container);
 
-        dataProvider = new RpcDataProviderExtension(container, null);
+        dataProvider = new RpcDataProviderExtension(container);
         keyMapper = dataProvider.getKeyMapper();
     }