diff options
15 files changed, 288 insertions, 33 deletions
diff --git a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java index 7cd6b7390f..c9523fc34b 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java +++ b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java @@ -178,6 +178,12 @@ public class DataCommunicator<T, F> extends AbstractExtension { // Drop the registered key getKeyMapper().remove(data); } + + @Override + public void destroyAllData() { + activeData.clear(); + getKeyMapper().removeAll(); + } } private final Collection<DataGenerator<T>> generators = new LinkedHashSet<>(); @@ -354,6 +360,13 @@ public class DataCommunicator<T, F> extends AbstractExtension { } } + private void dropAllData() { + for (DataGenerator<T> g : generators) { + g.destroyAllData(); + } + handler.destroyAllData(); + } + /** * Informs the DataProvider that the collection has changed. */ @@ -460,6 +473,7 @@ public class DataCommunicator<T, F> extends AbstractExtension { Objects.requireNonNull(dataProvider, "data provider cannot be null"); this.dataProvider = dataProvider; detachDataProviderListener(); + dropAllData(); /* * This introduces behavior which influence on the client-server * communication: now the very first response to the client will always diff --git a/server/src/main/java/com/vaadin/data/provider/DataGenerator.java b/server/src/main/java/com/vaadin/data/provider/DataGenerator.java index f7ffa78ab6..5e88f7ff66 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataGenerator.java +++ b/server/src/main/java/com/vaadin/data/provider/DataGenerator.java @@ -55,4 +55,11 @@ public interface DataGenerator<T> extends Serializable { */ public default void destroyData(T item) { } + + /** + * Informs the {@code DataGenerator} that all data has been dropped. This + * method should clean up any unneeded information stored for items. + */ + public default void destroyAllData() { + } } diff --git a/server/src/main/java/com/vaadin/ui/AbstractListing.java b/server/src/main/java/com/vaadin/ui/AbstractListing.java index cf285c1d96..cacc9240f2 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractListing.java +++ b/server/src/main/java/com/vaadin/ui/AbstractListing.java @@ -15,14 +15,12 @@ */ package com.vaadin.ui; -import java.util.List; import java.util.Objects; import org.jsoup.nodes.Attributes; import org.jsoup.nodes.Element; import com.vaadin.data.Listing; -import com.vaadin.data.SelectionModel; import com.vaadin.data.provider.DataCommunicator; import com.vaadin.data.provider.DataGenerator; import com.vaadin.data.provider.DataProvider; @@ -374,10 +372,7 @@ public abstract class AbstractListing<T> extends AbstractComponent setItemIconGenerator( new DeclarativeIconGenerator<>(getItemIconGenerator())); - List<T> readItems = readItems(design, context); - if (!readItems.isEmpty() && this instanceof Listing) { - ((Listing<T, ?>) this).setItems(readItems); - } + readItems(design, context); } /** @@ -387,10 +382,8 @@ public abstract class AbstractListing<T> extends AbstractComponent * The element to obtain the state from * @param context * The DesignContext instance used for parsing the design - * - * @return the items read from the design */ - protected abstract List<T> readItems(Element design, DesignContext context); + protected abstract void readItems(Element design, DesignContext context); /** * Reads an Item from a design and inserts it into the data source. diff --git a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java index 63c2d66d5a..ce79da25a1 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java @@ -25,10 +25,10 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; -import com.vaadin.server.SerializableConsumer; import org.jsoup.nodes.Element; import com.vaadin.data.HasValue; +import com.vaadin.data.Listing; import com.vaadin.data.SelectionModel; import com.vaadin.data.SelectionModel.Multi; import com.vaadin.data.provider.DataGenerator; @@ -36,6 +36,7 @@ import com.vaadin.event.selection.MultiSelectionEvent; import com.vaadin.event.selection.MultiSelectionListener; import com.vaadin.server.Resource; import com.vaadin.server.ResourceReference; +import com.vaadin.server.SerializableConsumer; import com.vaadin.server.SerializablePredicate; import com.vaadin.shared.Registration; import com.vaadin.shared.data.selection.MultiSelectServerRpc; @@ -117,6 +118,11 @@ public abstract class AbstractMultiSelect<T> extends AbstractListing<T> @Override public void destroyData(T data) { } + + @Override + public void destroyAllData() { + AbstractMultiSelect.this.deselectAll(); + } } /** @@ -422,14 +428,16 @@ public abstract class AbstractMultiSelect<T> extends AbstractListing<T> } @Override - protected List<T> readItems(Element design, DesignContext context) { + protected void readItems(Element design, DesignContext context) { Set<T> selected = new HashSet<>(); List<T> items = design.children().stream() .map(child -> readItem(child, selected, context)) .collect(Collectors.toList()); deselectAll(); + if (!items.isEmpty() && this instanceof Listing) { + ((Listing<T, ?>) this).setItems(items); + } selected.forEach(this::select); - return items; } /** diff --git a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java index 521a0b518b..f98d15bf10 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java @@ -26,7 +26,7 @@ import java.util.stream.Collectors; import org.jsoup.nodes.Element; import com.vaadin.data.HasValue; -import com.vaadin.data.SelectionModel; +import com.vaadin.data.Listing; import com.vaadin.data.SelectionModel.Single; import com.vaadin.data.provider.DataCommunicator; import com.vaadin.event.selection.SingleSelectionEvent; @@ -321,13 +321,15 @@ public abstract class AbstractSingleSelect<T> extends AbstractListing<T> } @Override - protected List<T> readItems(Element design, DesignContext context) { + protected void readItems(Element design, DesignContext context) { Set<T> selected = new HashSet<>(); List<T> items = design.children().stream() .map(child -> readItem(child, selected, context)) .collect(Collectors.toList()); + if (!items.isEmpty() && this instanceof Listing) { + ((Listing<T, ?>) this).setItems(items); + } selected.forEach(this::setValue); - return items; } /** diff --git a/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java b/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java index ddc0945bc3..a4c48f3515 100644 --- a/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java +++ b/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java @@ -17,7 +17,6 @@ package com.vaadin.ui; import java.util.Collection; -import java.util.List; import java.util.Set; import org.jsoup.nodes.Element; @@ -167,9 +166,9 @@ public class CheckBoxGroup<T> extends AbstractMultiSelect<T> } @Override - protected List<T> readItems(Element design, DesignContext context) { + protected void readItems(Element design, DesignContext context) { setItemEnabledProvider(new DeclarativeItemEnabledProvider<>()); - return super.readItems(design, context); + super.readItems(design, context); } @SuppressWarnings({ "unchecked", "rawtypes" }) diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java index 67a8c79983..f742a22adc 100644 --- a/server/src/main/java/com/vaadin/ui/ComboBox.java +++ b/server/src/main/java/com/vaadin/ui/ComboBox.java @@ -18,7 +18,6 @@ package com.vaadin.ui; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -610,9 +609,9 @@ public class ComboBox<T> extends AbstractSingleSelect<T> } @Override - protected List<T> readItems(Element design, DesignContext context) { + protected void readItems(Element design, DesignContext context) { setStyleGenerator(new DeclarativeStyleGenerator<>(getStyleGenerator())); - return super.readItems(design, context); + super.readItems(design, context); } @SuppressWarnings({ "unchecked", "rawtypes" }) diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index b405896447..16d995b4ae 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -2895,8 +2895,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, } @Override - protected List<T> readItems(Element design, DesignContext context) { - return Collections.emptyList(); + protected void readItems(Element design, DesignContext context) { + // Grid handles reading of items in Grid#readData } @Override @@ -3043,11 +3043,12 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, List<DeclarativeValueProvider<T>> providers) { getSelectionModel().deselectAll(); List<T> items = new ArrayList<>(); + List<T> selectedItems = new ArrayList<>(); for (Element row : body.children()) { T item = deserializeDeclarativeRepresentation(row.attr("item")); items.add(item); if (row.hasAttr("selected")) { - getSelectionModel().select(item); + selectedItems.add(item); } Elements cells = row.children(); int i = 0; @@ -3058,6 +3059,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, } setItems(items); + selectedItems.forEach(getSelectionModel()::select); } private void writeStructure(Element design, DesignContext designContext) { diff --git a/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java b/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java index 5d4d44d169..592781cc56 100644 --- a/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java +++ b/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java @@ -17,7 +17,6 @@ package com.vaadin.ui; import java.util.Collection; -import java.util.List; import java.util.Objects; import java.util.Set; @@ -236,9 +235,9 @@ public class RadioButtonGroup<T> extends AbstractSingleSelect<T> } @Override - protected List<T> readItems(Element design, DesignContext context) { + protected void readItems(Element design, DesignContext context) { setItemEnabledProvider(new DeclarativeItemEnabledProvider<>()); - return super.readItems(design, context); + super.readItems(design, context); } @SuppressWarnings({ "unchecked", "rawtypes" }) diff --git a/server/src/main/java/com/vaadin/ui/components/grid/AbstractSelectionModel.java b/server/src/main/java/com/vaadin/ui/components/grid/AbstractSelectionModel.java index 0371d2be3a..e38c350694 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/AbstractSelectionModel.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/AbstractSelectionModel.java @@ -44,6 +44,11 @@ public abstract class AbstractSelectionModel<T> extends AbstractGridExtension<T> } @Override + public void destroyAllData() { + deselectAll(); + } + + @Override protected AbstractSelectionModelState getState() { return (AbstractSelectionModelState) super.getState(); } diff --git a/server/src/test/java/com/vaadin/data/provider/ReplaceDataProviderTest.java b/server/src/test/java/com/vaadin/data/provider/ReplaceDataProviderTest.java new file mode 100644 index 0000000000..0510022e5e --- /dev/null +++ b/server/src/test/java/com/vaadin/data/provider/ReplaceDataProviderTest.java @@ -0,0 +1,110 @@ +package com.vaadin.data.provider; + +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +import java.util.List; +import java.util.function.IntFunction; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import org.junit.Test; + +import com.vaadin.server.SerializablePredicate; + +public class ReplaceDataProviderTest { + + public static class BeanWithEquals extends Bean { + + BeanWithEquals(int id) { + super(id); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + BeanWithEquals that = (BeanWithEquals) o; + + return id == that.id; + } + + @Override + public int hashCode() { + return id; + } + } + + public static class Bean { + protected final int id; + private final String fluff; + + Bean(int id) { + this.id = id; + this.fluff = "Fluff #" + id; + } + + public int getId() { + return id; + } + + @SuppressWarnings("unused") + public String getFluff() { + return fluff; + } + + } + + @Test + public void testBeanEquals() { + doTest(BeanWithEquals::new); + } + + @Test + public void testBeanSame() { + doTest(Bean::new); + } + + private <SOME_BEAN> void doTest(IntFunction<SOME_BEAN> beanConstructor) { + + DataCommunicator<SOME_BEAN, SerializablePredicate<SOME_BEAN>> dataCommunicator = new DataCommunicator<>(); + + List<SOME_BEAN> beans1 = createCollection(beanConstructor); + + ListDataProvider<SOME_BEAN> dataProvider = new ListDataProvider<>( + beans1); + + dataCommunicator.setDataProvider(dataProvider); + dataCommunicator.pushData(1, beans1.stream()); + + SOME_BEAN bean1_17 = beans1.get(17); + String key1_17 = dataCommunicator.getKeyMapper().key(bean1_17); + + assertSame(bean1_17, dataCommunicator.getKeyMapper().get(key1_17)); + + List<SOME_BEAN> beans2 = createCollection(beanConstructor); + + dataProvider = new ListDataProvider<>(beans2); + dataCommunicator.setDataProvider(dataProvider); + dataCommunicator.pushData(1, beans2.stream()); + + SOME_BEAN bean2_17 = beans2.get(17); + String key2_17 = dataCommunicator.getKeyMapper().key(bean2_17); + + assertSame(bean2_17, dataCommunicator.getKeyMapper().get(key2_17)); + assertNotEquals(key2_17, key1_17); + assertNull(dataCommunicator.getKeyMapper().get(key1_17)); + } + + private <SOME_BEAN> List<SOME_BEAN> createCollection( + IntFunction<SOME_BEAN> beanConstructor) { + return IntStream.range(1, 100).mapToObj(beanConstructor) + .collect(Collectors.toList()); + } +} diff --git a/server/src/test/java/com/vaadin/ui/AbstractListingTest.java b/server/src/test/java/com/vaadin/ui/AbstractListingTest.java index 819ed9feb3..212a15f32f 100644 --- a/server/src/test/java/com/vaadin/ui/AbstractListingTest.java +++ b/server/src/test/java/com/vaadin/ui/AbstractListingTest.java @@ -40,9 +40,8 @@ public class AbstractListingTest { } @Override - protected List<String> readItems(Element design, + protected void readItems(Element design, DesignContext context) { - return null; } @Override diff --git a/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java b/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java index 1b814921e7..9bbcc2ca43 100644 --- a/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java +++ b/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java @@ -60,9 +60,8 @@ public class AbstractSingleSelectTest { } @Override - protected List<Person> readItems(Element design, + protected void readItems(Element design, DesignContext context) { - return null; } @Override @@ -254,9 +253,8 @@ public class AbstractSingleSelectTest { } @Override - protected List<String> readItems(Element design, + protected void readItems(Element design, DesignContext context) { - return null; } }; diff --git a/uitest/src/main/java/com/vaadin/tests/data/ReplaceDataProvider.java b/uitest/src/main/java/com/vaadin/tests/data/ReplaceDataProvider.java new file mode 100644 index 0000000000..8a33cae95d --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/data/ReplaceDataProvider.java @@ -0,0 +1,67 @@ +package com.vaadin.tests.data; + +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Grid; + +public class ReplaceDataProvider extends AbstractTestUI { + + private static class TestClass { + public String someField; + public int hash; + + public TestClass(int hash) { + this.hash = hash; + someField = "a"; + } + + @Override + public int hashCode() { + return hash; + } + + @Override + public boolean equals(Object obj) { + return true; + } + } + + @Override + protected void setup(VaadinRequest request) { + Grid<TestClass> grid = new Grid<>(); + grid.addColumn(item -> item.someField); + + List<TestClass> listOfClasses = IntStream.range(0, 10) + .mapToObj(TestClass::new).collect(Collectors.toList()); + for (int i = 0; i < 10; i++) { + listOfClasses.add(new TestClass(10)); + } + + grid.setItems(listOfClasses); + + Button replaceBtn = new Button("replace data provider"); + replaceBtn.addClickListener(clickEvent -> { + List<TestClass> newList = IntStream.range(0, 10) + .mapToObj(TestClass::new).collect(Collectors.toList()); + newList.get(0).someField = "b"; + grid.setItems(newList); + }); + + Button replaceAndSelectBtn = new Button( + "replace data provider and select second"); + replaceAndSelectBtn.addClickListener(clickEvent -> { + List<TestClass> newList = IntStream.range(0, 10) + .mapToObj(TestClass::new).collect(Collectors.toList()); + newList.get(0).someField = "b"; + grid.setItems(newList); + grid.getSelectionModel().select(newList.get(1)); + }); + + addComponents(replaceBtn, replaceAndSelectBtn, grid); + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/data/ReplaceDataProviderTest.java b/uitest/src/test/java/com/vaadin/tests/data/ReplaceDataProviderTest.java new file mode 100644 index 0000000000..7fa495cf24 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/data/ReplaceDataProviderTest.java @@ -0,0 +1,53 @@ +package com.vaadin.tests.data; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.tests.tb3.SingleBrowserTest; + +public class ReplaceDataProviderTest extends SingleBrowserTest { + + @Before + public void setUp() { + openTestURL(); + } + + @Test + public void test_grid_data_communication_with_replaced_data_provider() { + GridElement grid = $(GridElement.class).first(); + ButtonElement replaceDataProviderButton = $(ButtonElement.class) + .first(); + + Assert.assertEquals(20, grid.getRowCount()); + grid.getCell(0, 0).click(); + assertCellText("a", 0, 0); + + replaceDataProviderButton.click(); + + Assert.assertEquals(10, grid.getRowCount()); + assertCellText("b", 0, 0); + for (int i = 1; i < 10; i++) { + assertCellText("a", i, 0); + } + + Assert.assertFalse(grid.getRow(0).isSelected()); + + grid.getCell(0, 0).click(); + assertCellText("b", 0, 0); + + // This button should replace the data provider and do a server side + // select on the second item + $(ButtonElement.class).get(1).click(); + grid.getRow(1).isSelected(); + } + + private void assertCellText(String text, int rowIndex, int colIndex) { + String firstCellText = $(GridElement.class).first() + .getCell(rowIndex, colIndex) + .getText(); + Assert.assertEquals(text, firstCellText); + } +} |