From 1d3fc17f38e8dbf3ee2328be221202e1774bd56b Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Mon, 5 Oct 2015 16:41:56 +0300 Subject: [PATCH] Add API for data destruction to DataGenerator interface (#19038) This patch provides destroyData implementation for all default data generators. Change-Id: I1458080ee0203a27b52e604d3a30c9e5240c0383 --- server/src/com/vaadin/data/DataGenerator.java | 14 ++++-- .../vaadin/data/RpcDataProviderExtension.java | 44 ++++++++++++++----- server/src/com/vaadin/ui/Grid.java | 10 +++++ .../grid/basicfeatures/GridBasicFeatures.java | 35 +++++++++------ .../server/GridDetailsServerTest.java | 39 +++++++++++----- 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 a5333b85238..5e301d81517 100644 --- a/server/src/com/vaadin/data/DataGenerator.java +++ b/server/src/com/vaadin/data/DataGenerator.java @@ -18,7 +18,6 @@ import java.io.Serializable; import com.vaadin.ui.Grid.AbstractGridExtension; -import com.vaadin.ui.Grid.AbstractRenderer; import elemental.json.JsonObject; @@ -26,8 +25,8 @@ * Interface for {@link AbstractGridExtension}s that allows adding data to row * objects being sent to client by the {@link RpcDataProviderExtension}. *

- * {@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 78c87ab23d3..45caf01587f 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -98,7 +98,7 @@ public void addActiveItems(Collection itemIds) { // Remove still active rows that were "dropped" droppedItems.removeAll(itemIds); - internalDropActiveItems(droppedItems); + internalDropItems(droppedItems); droppedItems.clear(); } @@ -115,15 +115,6 @@ public void dropActiveItem(Object itemId) { } } - private void internalDropActiveItems(Collection 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 void generateData(Object itemId, Item item, JsonObject rowData) { 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 void generateData(Object itemId, Item item, JsonObject rowData) { .getConnectorId() : "")); } } + + @Override + public void destroyData(Object itemId) { + if (visibleDetails.contains(itemId)) { + destroyDetails(itemId); + } + } } private final Indexed container; @@ -734,8 +741,7 @@ public void refreshCache() { 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) @@ -749,6 +755,20 @@ public void setParent(ClientConnector parent) { super.setParent(parent); } + /** + * Informs all DataGenerators than an item id has been dropped. + * + * @param droppedItemIds + * collection of dropped item ids + */ + private void internalDropItems(Collection 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 f0e7b664e0b..cee045128f2 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -1130,6 +1130,11 @@ public void generateData(Object itemId, Item item, JsonObject rowData) { } } + @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 @@ private void writeData(CellReference cell, JsonObject data) { 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 479ece71ca9..598ac420fcb 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.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 void columnVisibilityChanged(ColumnVisibilityChangeEvent event) { } }; - private Panel detailsPanel; - private final DetailsGenerator detailedDetailsGenerator = new DetailsGenerator() { @Override public Component getDetails(final RowReference rowReference) { @@ -230,12 +230,18 @@ public Component getDetails(RowReference rowReference) { } }; - private final DetailsGenerator hierarchicalDetailsGenerator = new DetailsGenerator() { + private Map detailsMap = new HashMap(); + + 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 void execute(Grid g, Boolean visible, Object itemId) { watchingDetailsGenerator); createClickAction("Detailed", "Generators", swapDetailsGenerator, detailedDetailsGenerator); - createClickAction("Hierarchical", "Generators", swapDetailsGenerator, - hierarchicalDetailsGenerator); + createClickAction("Persisting", "Generators", swapDetailsGenerator, + persistingDetailsGenerator); createClickAction("- Change Component", "Generators", new Command() { @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 a9ab7027db0..f13ea9c0733 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 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 void almostLastItemIdIsRendered() { } @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 void hierarchyChangesWorkInDetails() { } @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); @@ -206,6 +206,23 @@ public void hierarchyChangesWorkInDetailsWhileOutOfView() { assertEquals("Two", getGridElement().getDetails(0).getText()); } + @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); @@ -215,7 +232,7 @@ public void swappingDetailsGenerators_noDetailsShown() { @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 void swappingDetailsGenerators_whileDetailsScrolledOut_showNever() { @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 void swappingDetailsGenerators_whileDetailsScrolledOut_showAfter() { @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 void swappingDetailsGenerators_whileDetailsScrolledOut_showBefore() { @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);