Skip to content

Commit

Permalink
Add API for data destruction to DataGenerator interface (#19038)
Browse files Browse the repository at this point in the history 
This patch provides destroyData implementation for all default data
generators.

Change-Id: I1458080ee0203a27b52e604d3a30c9e5240c0383
  • Loading branch information
Teemu Suo-Anttila committed Oct 6, 2015
1 parent 5097095 commit 1d3fc17
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 39 deletions.
14 changes: 11 additions & 3 deletions server/src/com/vaadin/data/DataGenerator.java
View file
Expand Up @@ -18,16 +18,15 @@
import java.io.Serializable;

import com.vaadin.ui.Grid.AbstractGridExtension;
import com.vaadin.ui.Grid.AbstractRenderer;

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
Expand All @@ -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);

}
44 changes: 32 additions & 12 deletions server/src/com/vaadin/data/RpcDataProviderExtension.java
View file
Expand Up @@ -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();
}

Expand All @@ -115,15 +115,6 @@ public void dropActiveItem(Object itemId) {
}
}

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.
*
Expand All @@ -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();
}
}
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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<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.
Expand Down
10 changes: 10 additions & 0 deletions server/src/com/vaadin/ui/Grid.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
}

/**
Expand Down
35 changes: 22 additions & 13 deletions uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -230,12 +230,18 @@ public Component getDetails(RowReference rowReference) {
}
};

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);
}
};

Expand Down Expand Up @@ -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<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);
Expand Down
39 changes: 28 additions & 11 deletions uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
View file
Expand Up @@ -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;
Expand All @@ -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" };

Expand Down Expand Up @@ -187,17 +187,17 @@ 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);
assertEquals("Two", getGridElement().getDetails(0).getText());
}

@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);
Expand All @@ -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);
Expand All @@ -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"));
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 1d3fc17

Please sign in to comment.