Add API for data destruction to DataGenerator interface (#19038)

This patch provides destroyData implementation for all default data
generators.

Change-Id: I1458080ee0203a27b52e604d3a30c9e5240c0383

5 files changed, 103 insertions, 39 deletions

diff --git a/server/src/com/vaadin/data/DataGenerator.java b/server/src/com/vaadin/data/DataGenerator.java
index a5333b8523..5e301d8151 100644
--- a/server/src/com/vaadin/data/DataGenerator.java
+++ b/server/src/com/vaadin/data/DataGenerator.java
@@ -18,7 +18,6 @@ package com.vaadin.data;
 import java.io.Serializable;
 
 import com.vaadin.ui.Grid.AbstractGridExtension;
-import com.vaadin.ui.Grid.AbstractRenderer;
 
 import elemental.json.JsonObject;
 
@@ -26,8 +25,8 @@ import elemental.json.JsonObject;
  * Interface for {@link AbstractGridExtension}s that allows adding data to row
  * objects being sent to client by the {@link RpcDataProviderExtension}.
  * <p>
- * {@link AbstractRenderer} implements this interface to provide encoded data to
- * client for {@link Renderer}s automatically.
+ * This class also provides a way to remove any unneeded data once the data
+ * object is no longer used on the client-side.
  * 
  * @since 7.6
  * @author Vaadin Ltd
@@ -46,4 +45,13 @@ public interface DataGenerator extends Serializable {
      */
     public void generateData(Object itemId, Item item, JsonObject rowData);
 
+    /**
+     * Informs the DataGenerator that an item id has been dropped and is no
+     * longer needed.
+     * 
+     * @param itemId
+     *            removed item id
+     */
+    public void destroyData(Object itemId);
+
 }
diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java
index 78c87ab23d..45caf01587 100644
--- a/server/src/com/vaadin/data/RpcDataProviderExtension.java
+++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java
@@ -98,7 +98,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
 
             // Remove still active rows that were "dropped"
             droppedItems.removeAll(itemIds);
-            internalDropActiveItems(droppedItems);
+            internalDropItems(droppedItems);
             droppedItems.clear();
         }
 
@@ -115,15 +115,6 @@ public class RpcDataProviderExtension extends AbstractExtension {
             }
         }
 
-        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);
-            }
-        }
-
         /**
          * Gets a collection copy of currently active item ids.
          * 
@@ -147,6 +138,15 @@ public class RpcDataProviderExtension extends AbstractExtension {
             rowData.put(GridState.JSONKEY_ROWKEY, keyMapper.key(itemId));
         }
 
+        @Override
+        public void destroyData(Object itemId) {
+            keyMapper.remove(itemId);
+            GridValueChangeListener removed = activeItemMap.remove(itemId);
+
+            if (removed != null) {
+                removed.removeListener();
+            }
+        }
     }
 
     /**
@@ -372,6 +372,13 @@ public class RpcDataProviderExtension extends AbstractExtension {
                                 .getConnectorId() : ""));
             }
         }
+
+        @Override
+        public void destroyData(Object itemId) {
+            if (visibleDetails.contains(itemId)) {
+                destroyDetails(itemId);
+            }
+        }
     }
 
     private final Indexed container;
@@ -734,8 +741,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
     public void setParent(ClientConnector parent) {
         if (parent == null) {
             // We're being detached, release various listeners
-            activeItemHandler.internalDropActiveItems(activeItemHandler
-                    .getActiveItemIds());
+            internalDropItems(activeItemHandler.getActiveItemIds());
 
             if (container instanceof ItemSetChangeNotifier) {
                 ((ItemSetChangeNotifier) container)
@@ -750,6 +756,20 @@ public class RpcDataProviderExtension extends AbstractExtension {
     }
 
     /**
+     * Informs all DataGenerators than an item id has been dropped.
+     * 
+     * @param droppedItemIds
+     *            collection of dropped item ids
+     */
+    private void internalDropItems(Collection<Object> droppedItemIds) {
+        for (Object itemId : droppedItemIds) {
+            for (DataGenerator generator : dataGenerators) {
+                generator.destroyData(itemId);
+            }
+        }
+    }
+
+    /**
      * Informs this data provider that given columns have been removed from
      * grid.
      * 
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index f0e7b664e0..cee045128f 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -1131,6 +1131,11 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
         }
 
         @Override
+        public void destroyData(Object itemId) {
+            // NO-OP
+        }
+
+        @Override
         protected Object getItemId(String rowKey) {
             return rowKey != null ? super.getItemId(rowKey) : null;
         }
@@ -1880,6 +1885,11 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
             data.put(columnKeys.key(cell.getPropertyId()), AbstractRenderer
                     .encodeValue(modelValue, renderer, converter, getLocale()));
         }
+
+        @Override
+        public void destroyData(Object itemId) {
+            // NO-OP
+        }
     }
 
     /**
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
index 479ece71ca..598ac420fc 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
@@ -20,10 +20,12 @@ import java.text.DecimalFormatSymbols;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Random;
 
 import com.vaadin.data.Container.Filter;
@@ -190,8 +192,6 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
         }
     };
 
-    private Panel detailsPanel;
-
     private final DetailsGenerator detailedDetailsGenerator = new DetailsGenerator() {
         @Override
         public Component getDetails(final RowReference rowReference) {
@@ -230,12 +230,18 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
         }
     };
 
-    private final DetailsGenerator hierarchicalDetailsGenerator = new DetailsGenerator() {
+    private Map<Object, Panel> detailsMap = new HashMap<Object, Panel>();
+
+    private final DetailsGenerator persistingDetailsGenerator = new DetailsGenerator() {
         @Override
         public Component getDetails(RowReference rowReference) {
-            detailsPanel = new Panel();
-            detailsPanel.setContent(new Label("One"));
-            return detailsPanel;
+            Object itemId = rowReference.getItemId();
+            if (!detailsMap.containsKey(itemId)) {
+                Panel panel = new Panel();
+                panel.setContent(new Label("One"));
+                detailsMap.put(itemId, panel);
+            }
+            return detailsMap.get(itemId);
         }
     };
 
@@ -1514,18 +1520,21 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                 watchingDetailsGenerator);
         createClickAction("Detailed", "Generators", swapDetailsGenerator,
                 detailedDetailsGenerator);
-        createClickAction("Hierarchical", "Generators", swapDetailsGenerator,
-                hierarchicalDetailsGenerator);
+        createClickAction("Persisting", "Generators", swapDetailsGenerator,
+                persistingDetailsGenerator);
 
         createClickAction("- Change Component", "Generators",
                 new Command<Grid, Void>() {
                     @Override
                     public void execute(Grid c, Void value, Object data) {
-                        Label label = (Label) detailsPanel.getContent();
-                        if (label.getValue().equals("One")) {
-                            detailsPanel.setContent(new Label("Two"));
-                        } else {
-                            detailsPanel.setContent(new Label("One"));
+                        for (Object id : detailsMap.keySet()) {
+                            Panel panel = detailsMap.get(id);
+                            Label label = (Label) panel.getContent();
+                            if (label.getValue().equals("One")) {
+                                panel.setContent(new Label("Two"));
+                            } else {
+                                panel.setContent(new Label("One"));
+                            }
                         }
                     }
                 }, null);
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
index a9ab7027db..f13ea9c073 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
@@ -23,9 +23,9 @@ import static org.junit.Assert.fail;
 
 import org.junit.Before;
 import org.junit.Test;
-import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
 
+import com.vaadin.testbench.By;
 import com.vaadin.testbench.TestBenchElement;
 import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
@@ -48,8 +48,8 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
             "Component", "Details", "Generators", "NULL" };
     private static final String[] DETAILS_GENERATOR_WATCHING = new String[] {
             "Component", "Details", "Generators", "\"Watching\"" };
-    private static final String[] DETAILS_GENERATOR_HIERARCHICAL = new String[] {
-            "Component", "Details", "Generators", "Hierarchical" };
+    private static final String[] DETAILS_GENERATOR_PERSISTING = new String[] {
+            "Component", "Details", "Generators", "Persisting" };
     private static final String[] CHANGE_HIERARCHY = new String[] {
             "Component", "Details", "Generators", "- Change Component" };
 
@@ -187,8 +187,8 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     }
 
     @Test
-    public void hierarchyChangesWorkInDetails() {
-        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+    public void persistingChangesWorkInDetails() {
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         assertEquals("One", getGridElement().getDetails(0).getText());
         selectMenuPath(CHANGE_HIERARCHY);
@@ -196,8 +196,8 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     }
 
     @Test
-    public void hierarchyChangesWorkInDetailsWhileOutOfView() {
-        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+    public void persistingChangesWorkInDetailsWhileOutOfView() {
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         assertEquals("One", getGridElement().getDetails(0).getText());
         scrollGridVerticallyTo(10000);
@@ -207,6 +207,23 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     }
 
     @Test
+    public void persistingChangesWorkInDetailsWhenNotAttached() {
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        assertEquals("One", getGridElement().getDetails(0).getText());
+
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        assertFalse("Details should be detached", getGridElement()
+                .isElementPresent(By.vaadin("#details[0]")));
+
+        selectMenuPath(CHANGE_HIERARCHY);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+
+        assertEquals("Two", getGridElement().getDetails(0).getText());
+
+    }
+
+    @Test
     public void swappingDetailsGenerators_noDetailsShown() {
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
         selectMenuPath(DETAILS_GENERATOR_NULL);
@@ -215,7 +232,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test
     public void swappingDetailsGenerators_shownDetails() {
-        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         assertTrue("Details should contain 'One' at first", getGridElement()
                 .getDetails(0).getText().contains("One"));
@@ -237,7 +254,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     @Test
     public void swappingDetailsGenerators_whileDetailsScrolledOut_showAfter() {
         scrollGridVerticallyTo(1000);
-        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
         scrollGridVerticallyTo(0);
@@ -249,7 +266,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test
     public void swappingDetailsGenerators_whileDetailsScrolledOut_showBefore() {
-        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
         scrollGridVerticallyTo(1000);
@@ -261,7 +278,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test
     public void swappingDetailsGenerators_whileDetailsScrolledOut_showBeforeAndAfter() {
-        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
         scrollGridVerticallyTo(1000);