diff --git a/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataProvider.java b/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataProvider.java index 8edb919c594..3dc86d4817b 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataProvider.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataProvider.java @@ -23,6 +23,8 @@ import java.util.function.Consumer; import com.vaadin.flow.data.provider.DataChangeEvent.DataRefreshEvent; +import com.vaadin.flow.data.provider.DataChangeEvent.ItemAddedEvent; +import com.vaadin.flow.data.provider.DataChangeEvent.ItemRemovedEvent; import com.vaadin.flow.function.SerializableConsumer; import com.vaadin.flow.shared.Registration; @@ -82,6 +84,16 @@ public void refreshItem(T item) { fireEvent(new DataRefreshEvent<>(this, item)); } + @Override + public void notifyItemAdded(T item) { + fireEvent(new ItemAddedEvent<>(this, item)); + } + + @Override + public void notifyItemRemoved(T item) { + fireEvent(new ItemRemovedEvent<>(this, item)); + } + /** * Registers a new listener with the specified activation method to listen * events generated by this component. If the activation method does not diff --git a/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractListDataView.java b/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractListDataView.java index ffd4a1ae13f..295782179e8 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractListDataView.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractListDataView.java @@ -210,7 +210,7 @@ public AbstractListDataView addItem(T item) { final ListDataProvider dataProvider = getDataProvider(); if (!contains(item)) { dataProvider.getItems().add(item); - dataProvider.refreshAll(); + dataProvider.notifyItemAdded(item); } return this; } @@ -269,8 +269,11 @@ public AbstractListDataView addItemsBefore(Collection items, @Override public AbstractListDataView removeItem(T item) { final ListDataProvider dataProvider = getDataProvider(); - removeItemIfPresent(item, dataProvider); - dataProvider.refreshAll(); + if (removeItemIfPresent(item, dataProvider)) { + dataProvider.notifyItemRemoved(item); + } else { + dataProvider.refreshAll(); + } return this; } @@ -315,8 +318,10 @@ protected void validateItemIndex(int itemIndex) { } } - private void removeItemIfPresent(T item, ListDataProvider dataProvider) { - dataProvider.getItems().removeIf(nextItem -> equals(item, nextItem)); + private boolean removeItemIfPresent(T item, + ListDataProvider dataProvider) { + return dataProvider.getItems() + .removeIf(nextItem -> equals(item, nextItem)); } private void addItemOnTarget(T item, T target, @@ -348,7 +353,7 @@ private void addItemOnTarget(T item, T target, removeItemIfPresent(item, dataProvider); itemList.add(insertItemsIndexProvider .apply(getItemIndex(target, itemList.stream())), item); - dataProvider.refreshAll(); + dataProvider.notifyItemAdded(item); } private void addItemCollectionOnTarget(Collection items, T target, diff --git a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataChangeEvent.java b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataChangeEvent.java index caa15c89b00..c91ed94136a 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataChangeEvent.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataChangeEvent.java @@ -101,6 +101,87 @@ public boolean isRefreshChildren() { } } + /** + * An event fired when a single item has been added to a + * {@code DataProvider}. + *

+ * Listeners that don't need fine-grained handling can simply treat this as + * any other {@link DataChangeEvent}. {@link DataCommunicator} uses this + * event to perform a more efficient update than a full refresh while still + * picking up the new total item count and the position of the new item with + * respect to current sorting and filtering. + * + * @param + * the data type + */ + public static class ItemAddedEvent extends DataChangeEvent { + + private final T item; + + /** + * Creates a new event originating from the given data provider. + * + * @param source + * the data provider, not null + * @param item + * the added item, not null + */ + public ItemAddedEvent(DataProvider source, T item) { + super(source); + Objects.requireNonNull(item, "Added item can't be null"); + this.item = item; + } + + /** + * Gets the added item. + * + * @return the added item, never null + */ + public T getItem() { + return item; + } + } + + /** + * An event fired when a single item has been removed from a + * {@code DataProvider}. + *

+ * Listeners that don't need fine-grained handling can simply treat this as + * any other {@link DataChangeEvent}. {@link DataCommunicator} uses this + * event to perform a more efficient update than a full refresh while still + * picking up the new total item count. + * + * @param + * the data type + */ + public static class ItemRemovedEvent extends DataChangeEvent { + + private final T item; + + /** + * Creates a new event originating from the given data provider. + * + * @param source + * the data provider, not null + * @param item + * the removed item, not null + */ + public ItemRemovedEvent(DataProvider source, T item) { + super(source); + Objects.requireNonNull(item, "Removed item can't be null"); + this.item = item; + } + + /** + * Gets the removed item. + * + * @return the removed item, never null + */ + public T getItem() { + return item; + } + } + /** * Creates a new {@code DataChangeEvent} event originating from the given * data provider. diff --git a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java index ca9158ab95a..4a1809bc2d8 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java @@ -44,6 +44,8 @@ import com.vaadin.flow.component.UI; import com.vaadin.flow.data.provider.ArrayUpdater.Update; import com.vaadin.flow.data.provider.DataChangeEvent.DataRefreshEvent; +import com.vaadin.flow.data.provider.DataChangeEvent.ItemAddedEvent; +import com.vaadin.flow.data.provider.DataChangeEvent.ItemRemovedEvent; import com.vaadin.flow.dom.Element; import com.vaadin.flow.function.SerializableComparator; import com.vaadin.flow.function.SerializableConsumer; @@ -1131,6 +1133,10 @@ private void handleAttach() { .addDataProviderListener(event -> { if (event instanceof DataRefreshEvent) { handleDataRefreshEvent((DataRefreshEvent) event); + } else if (event instanceof ItemAddedEvent) { + handleItemAddedEvent((ItemAddedEvent) event); + } else if (event instanceof ItemRemovedEvent) { + handleItemRemovedEvent((ItemRemovedEvent) event); } else { reset(); } @@ -1144,6 +1150,40 @@ protected void handleDataRefreshEvent(DataRefreshEvent event) { refresh(event.getItem()); } + /** + * Handles a notification that a single item has been added to the + * underlying data. The default implementation triggers a viewport refresh + * and a size re-fetch without discarding per-item state for unrelated + * items, which is significantly cheaper than {@link #reset()} when only one + * item changes. + * + * @param event + * the added-item event, not {@code null} + */ + protected void handleItemAddedEvent(ItemAddedEvent event) { + sizeReset = true; + refreshViewport(); + } + + /** + * Handles a notification that a single item has been removed from the + * underlying data. The default implementation removes the removed item from + * the {@link KeyMapper} (if active) and triggers a viewport refresh with a + * size re-fetch, without discarding per-item state for unrelated items. + * + * @param event + * the removed-item event, not {@code null} + */ + protected void handleItemRemovedEvent(ItemRemovedEvent event) { + T removed = event.getItem(); + if (keyMapper.has(removed)) { + dataGenerator.destroyData(removed); + keyMapper.remove(removed); + } + sizeReset = true; + refreshViewport(); + } + private void handleDetach() { if (future != null) { future.cancel(true); diff --git a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataProvider.java b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataProvider.java index 391421e7f71..03fd76184cf 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataProvider.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataProvider.java @@ -137,6 +137,57 @@ default void refreshItem(T item, boolean refreshChildren) { */ void refreshAll(); + /** + * Notifies listeners that a single item has been added to the underlying + * data and the listening components should refresh accordingly. This allows + * components to perform an incremental update without rebuilding the + * per-item state for unrelated items, which can be significantly faster + * than {@link #refreshAll()} when only a single item changes. + *

+ * Where the new item appears in the rendered output is still determined by + * the configured sorting and filtering: the implementation is expected to + * have already added the item to the underlying data structure before + * calling this method. + *

+ * The default implementation falls back to {@link #refreshAll()} so that + * custom data providers do not need to opt in to the new behavior. Data + * providers that fire {@link DataChangeEvent}s should override this method + * to fire a {@link DataChangeEvent.ItemAddedEvent} instead, which lets + * {@link DataCommunicator} perform an efficient update. + * + * @param item + * the added item; not {@code null} + */ + default void notifyItemAdded(T item) { + Objects.requireNonNull(item, "Added item cannot be null."); + refreshAll(); + } + + /** + * Notifies listeners that a single item has been removed from the + * underlying data and the listening components should refresh accordingly. + * This allows components to perform an incremental update without + * rebuilding the per-item state for unrelated items, which can be + * significantly faster than {@link #refreshAll()} when only a single item + * changes. + *

+ * The implementation is expected to have already removed the item from the + * underlying data structure before calling this method. + *

+ * The default implementation falls back to {@link #refreshAll()} so that + * custom data providers do not need to opt in to the new behavior. Data + * providers that fire {@link DataChangeEvent}s should override this method + * to fire a {@link DataChangeEvent.ItemRemovedEvent} instead, which lets + * {@link DataCommunicator} perform an efficient update. + * + * @param item + * the removed item; not {@code null} + */ + default void notifyItemRemoved(T item) { + Objects.requireNonNull(item, "Removed item cannot be null."); + refreshAll(); + } + /** * Gets an identifier for the given item. This identifier is used by the * framework to determine equality between two items. diff --git a/flow-data/src/test/java/com/vaadin/flow/data/provider/AbstractListDataViewTest.java b/flow-data/src/test/java/com/vaadin/flow/data/provider/AbstractListDataViewTest.java index 6a1f034781d..7eaf2bf2dc1 100644 --- a/flow-data/src/test/java/com/vaadin/flow/data/provider/AbstractListDataViewTest.java +++ b/flow-data/src/test/java/com/vaadin/flow/data/provider/AbstractListDataViewTest.java @@ -402,6 +402,52 @@ void removeItem_notInList_dataSetNotChanged() { assertEquals(3, dataView.getItemCount()); } + @Test + void addItem_firesItemAddedEventInsteadOfRefreshAll() { + AtomicReference> received = new AtomicReference<>(); + dataProvider.addDataProviderListener(received::set); + + dataView.addItem("new"); + + DataChangeEvent event = received.get(); + assertNotNull(event, "Expected an event to be fired"); + assertTrue(event instanceof DataChangeEvent.ItemAddedEvent, + "Expected an ItemAddedEvent, got " + event.getClass()); + assertEquals("new", + ((DataChangeEvent.ItemAddedEvent) event).getItem()); + } + + @Test + void removeItem_firesItemRemovedEventInsteadOfRefreshAll() { + AtomicReference> received = new AtomicReference<>(); + dataProvider.addDataProviderListener(received::set); + + dataView.removeItem("middle"); + + DataChangeEvent event = received.get(); + assertNotNull(event, "Expected an event to be fired"); + assertTrue(event instanceof DataChangeEvent.ItemRemovedEvent, + "Expected an ItemRemovedEvent, got " + event.getClass()); + assertEquals("middle", + ((DataChangeEvent.ItemRemovedEvent) event).getItem()); + } + + @Test + void removeItem_notPresent_fallsBackToRefreshAll() { + AtomicReference> received = new AtomicReference<>(); + dataProvider.addDataProviderListener(received::set); + + dataView.removeItem("not present"); + + DataChangeEvent event = received.get(); + assertNotNull(event, "Expected an event to be fired"); + // When nothing is removed, the data provider has no item to notify + // about, so it falls back to the original refreshAll behavior + assertFalse(event instanceof DataChangeEvent.DataRefreshEvent); + assertFalse(event instanceof DataChangeEvent.ItemRemovedEvent); + assertFalse(event instanceof DataChangeEvent.ItemAddedEvent); + } + @Test void addItemBefore_itemIsAddedAtExpectedPosition() { dataView.addItemBefore("newItem", "middle"); diff --git a/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java b/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java index b37f90650dc..3e1f47a59bf 100644 --- a/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java +++ b/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java @@ -443,6 +443,73 @@ void sameKeyDifferentInstance_latestInstanceUsed( assertSame(updatedItem, dataCommunicator.getKeyMapper().get(key)); } + @ParameterizedTest + @ValueSource(booleans = { true, false }) + void notifyItemAdded_viewportRefreshedWithoutDestroyingItemState( + boolean dataProviderWithParallelStream) { + this.dataProviderWithParallelStream = dataProviderWithParallelStream; + List items = new ArrayList<>(); + for (int i = 0; i < 2; i++) { + items.add(new Item(i)); + } + ListDataProvider dataProvider = new ListDataProvider<>(items); + dataCommunicator.setDataProvider(dataProvider, null); + + dataCommunicator.setViewportRange(0, 50); + fakeClientCommunication(); + // Reset mock interactions accumulated during the initial flush so we + // can assert specifically what notifyItemAdded triggers + Mockito.reset(dataGenerator); + lastSet = null; + + Item newItem = new Item(2); + items.add(newItem); + dataProvider.notifyItemAdded(newItem); + fakeClientCommunication(); + + // The viewport is resent to pick up the new item and its size + assertEquals(Range.withLength(0, 3), lastSet); + // Per-item state for unrelated items must not be discarded + Mockito.verify(dataGenerator, Mockito.never()).destroyAllData(); + } + + @ParameterizedTest + @ValueSource(booleans = { true, false }) + void notifyItemRemoved_itemKeyDroppedAndViewportRefreshedWithoutDestroyingItemState( + boolean dataProviderWithParallelStream) { + this.dataProviderWithParallelStream = dataProviderWithParallelStream; + List items = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + items.add(new Item(i)); + } + ListDataProvider dataProvider = new ListDataProvider<>(items); + dataCommunicator.setDataProvider(dataProvider, null); + + dataCommunicator.setViewportRange(0, 50); + fakeClientCommunication(); + + Item removed = items.get(1); + String removedKey = dataCommunicator.getKeyMapper().key(removed); + assertNotNull(removedKey); + + Mockito.reset(dataGenerator); + lastSet = null; + + items.remove(removed); + dataProvider.notifyItemRemoved(removed); + fakeClientCommunication(); + + // Viewport is resent with the new size + assertEquals(Range.withLength(0, 2), lastSet); + // The removed item's key is dropped immediately so any in-flight + // client interaction for it can no longer resolve to a stale instance + assertFalse(dataCommunicator.getKeyMapper().has(removed)); + // Per-item state for unrelated items must not be discarded; only the + // removed item should have its state cleaned up + Mockito.verify(dataGenerator, Mockito.never()).destroyAllData(); + Mockito.verify(dataGenerator).destroyData(removed); + } + @ParameterizedTest @ValueSource(booleans = { true, false }) void dataProviderReturnsLessItemsThanRequested_aNewSizeQueryIsPerformed(