fix: use targeted item refresh when signal-bound item value changes (#23913) (#23922)

- Add a two-argument `refreshItem(T newItem, T oldItem)` overload to `DataProvider`, `DataView`, and related classes, enabling targeted refresh when an item's identity changes (e.g., signal-bound items replaced with new instances)
- `DataRefreshEvent` now carries an optional old item reference via `getOldItem()`, allowing`DataCommunicator` and `KeyMapper` to remap from the old identity to the new one instead of failing to find the item
- `HierarchicalDataCommunicator` throws `UnsupportedOperationException` for identity-changing refreshes, deferring TreeGrid `bindItems` support to a future change

Fixes #8938

Co-authored-by: Artur Signell <artur@vaadin.com>

Author: vaadin-bot

flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataProvider.java

@@ -82,6 +82,11 @@ public void refreshItem(T item) {
         fireEvent(new DataRefreshEvent<>(this, item));
     }
 
+    @Override
+    public void refreshItem(T newItem, T oldItem) {
+        fireEvent(new DataRefreshEvent<>(this, newItem, oldItem));
+    }
+
     /**
      * Registers a new listener with the specified activation method to listen
      * events generated by this component. If the activation method does not

flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataView.java

@@ -177,6 +177,13 @@ public void refreshItem(T item) {
         dataProviderSupplier.get().refreshItem(item);
     }
 
+    @Override
+    public void refreshItem(T newItem, T oldItem) {
+        Objects.requireNonNull(newItem, NULL_ITEM_ERROR_MESSAGE);
+        Objects.requireNonNull(oldItem, NULL_ITEM_ERROR_MESSAGE);
+        dataProviderSupplier.get().refreshItem(newItem, oldItem);
+    }
+
     @Override
     public void refreshAll() {
         dataProviderSupplier.get().refreshAll();

flow-data/src/main/java/com/vaadin/flow/data/provider/DataChangeEvent.java

@@ -45,6 +45,7 @@ public class DataChangeEvent<T> extends EventObject {
     public static class DataRefreshEvent<T> extends DataChangeEvent<T> {
 
         private final T item;
+        private final T oldItem;
         private boolean refreshChildren;
 
         /**
@@ -57,7 +58,7 @@ public static class DataRefreshEvent<T> extends DataChangeEvent<T> {
          *            the updated item, not null
          */
         public DataRefreshEvent(DataProvider<T, ?> source, T item) {
-            this(source, item, false);
+            this(source, item, null, false);
         }
 
         /**
@@ -74,9 +75,48 @@ public DataRefreshEvent(DataProvider<T, ?> source, T item) {
          */
         public DataRefreshEvent(DataProvider<T, ?> source, T item,
                 boolean refreshChildren) {
+            this(source, item, null, refreshChildren);
+        }
+
+        /**
+         * Creates a new data refresh event originating from the given data
+         * provider, carrying the previous item instance so that downstream
+         * consumers can remap their internal state.
+         *
+         * @param source
+         *            the data provider, not null
+         * @param item
+         *            the updated item, not null
+         * @param oldItem
+         *            the old item before the update, or null if the identity
+         *            has not changed
+         */
+        public DataRefreshEvent(DataProvider<T, ?> source, T item, T oldItem) {
+            this(source, item, oldItem, false);
+        }
+
+        /**
+         * Creates a new data refresh event originating from the given data
+         * provider, carrying the previous item instance so that downstream
+         * consumers can remap their internal state.
+         *
+         * @param source
+         *            the data provider, not null
+         * @param item
+         *            the updated item, not null
+         * @param oldItem
+         *            the old item before the update, or null if the identity
+         *            has not changed
+         * @param refreshChildren
+         *            whether, in hierarchical providers, subelements should be
+         *            refreshed as well
+         */
+        public DataRefreshEvent(DataProvider<T, ?> source, T item, T oldItem,
+                boolean refreshChildren) {
             super(source);
             Objects.requireNonNull(item, "Refreshed item can't be null");
             this.item = item;
+            this.oldItem = oldItem;
             this.refreshChildren = refreshChildren;
         }
 
@@ -89,6 +129,16 @@ public T getItem() {
             return item;
         }
 
+        /**
+         * Gets the old item before the update. If no old item was provided,
+         * returns the current item as a safe default.
+         *
+         * @return the old item, or the current item if no old item was provided
+         */
+        public T getOldItem() {
+            return oldItem != null ? oldItem : item;
+        }
+
         /**
          * Gets the a boolean whether the refresh is supposed to be
          * refreshChildren (in hierarchical data providers).

flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java

@@ -427,9 +427,22 @@ public void reset() {
      *            updated data object; not {@code null}
      */
     public void refresh(T data) {
+        refresh(data, data);
+    }
+
+    /**
+     * Informs the DataCommunicator that a data object has been updated,
+     * providing the old data object for identity remapping in the KeyMapper.
+     *
+     * @param data
+     *            the new data object; not {@code null}
+     * @param oldData
+     *            the old data object used to find the existing key mapping
+     */
+    protected void refresh(T data, T oldData) {
         Objects.requireNonNull(data,
                 "DataCommunicator can not refresh null object");
-        getKeyMapper().refresh(data);
+        getKeyMapper().refresh(data, oldData);
         dataGenerator.refreshData(data);
         updatedData.add(data);
         requestFlushUpdatedData();
@@ -1141,7 +1154,7 @@ private void handleAttach() {
     }
 
     protected void handleDataRefreshEvent(DataRefreshEvent<T> event) {
-        refresh(event.getItem());
+        refresh(event.getItem(), event.getOldItem());
     }
 
     private void handleDetach() {

flow-data/src/main/java/com/vaadin/flow/data/provider/DataKeyMapper.java

@@ -82,6 +82,23 @@ public interface DataKeyMapper<T> extends Serializable {
      */
     void refresh(T dataObject);
 
+    /**
+     * Updates any existing mappings of given data object, using the old data
+     * object to find the existing mapping. This is useful when the identity of
+     * the data object has changed (e.g., the object was replaced with a new
+     * instance).
+     *
+     * @param dataObject
+     *            the new data object to update to
+     * @param oldDataObject
+     *            the old data object to find the existing mapping
+     */
+    default void refresh(T dataObject, T oldDataObject) {
+        throw new UnsupportedOperationException(
+                "This DataKeyMapper does not support identity remapping. "
+                        + "Override refresh(T, T) to handle item replacement.");
+    }
+
     /**
      * Takes identifier getter into use and updates existing mappings
      *

flow-data/src/main/java/com/vaadin/flow/data/provider/DataProvider.java

@@ -131,6 +131,24 @@ default void refreshItem(T item, boolean refreshChildren) {
         refreshItem(item);
     }
 
+    /**
+     * Refreshes an item that has changed identity, passing the old item so that
+     * downstream consumers (e.g. key mappers) can remap from the old identity
+     * to the new one.
+     * <p>
+     * The default implementation simply delegates to
+     * {@link #refreshItem(Object)}, discarding the old item. Implementations
+     * that need to remap internal state should override this method.
+     *
+     * @param newItem
+     *            the new item after the update, not null
+     * @param oldItem
+     *            the old item before the update, not null
+     */
+    default void refreshItem(T newItem, T oldItem) {
+        refreshItem(newItem);
+    }
+
     /**
      * Refreshes all data based on currently available data in the underlying
      * provider.

flow-data/src/main/java/com/vaadin/flow/data/provider/DataProviderWrapper.java

@@ -78,6 +78,11 @@ public void refreshItem(T item) {
         dataProvider.refreshItem(item);
     }
 
+    @Override
+    public void refreshItem(T newItem, T oldItem) {
+        dataProvider.refreshItem(newItem, oldItem);
+    }
+
     @Override
     public Object getId(T item) {
         return dataProvider.getId(item);

flow-data/src/main/java/com/vaadin/flow/data/provider/DataView.java

@@ -119,6 +119,21 @@ public interface DataView<T> extends Serializable {
      */
     void refreshItem(T item);
 
+    /**
+     * Refreshes an item that has changed identity, carrying the old item
+     * through the data pipeline so that internal mappings (e.g. key mappers)
+     * can be remapped from the old identity to the new one.
+     * <p>
+     * This method delegates the update to
+     * {@link DataProvider#refreshItem(Object, Object)}.
+     *
+     * @param newItem
+     *            the new item after the update, not null
+     * @param oldItem
+     *            the old item before the update, not null
+     */
+    void refreshItem(T newItem, T oldItem);
+
     /**
      * Notifies the component that all the items should be refreshed.
      */

flow-data/src/main/java/com/vaadin/flow/data/provider/DataViewUtils.java

@@ -23,8 +23,8 @@
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.ComponentUtil;
 import com.vaadin.flow.data.binder.HasDataProvider;
+import com.vaadin.flow.function.SerializableBiConsumer;
 import com.vaadin.flow.function.SerializableComparator;
-import com.vaadin.flow.function.SerializableConsumer;
 import com.vaadin.flow.function.SerializableFunction;
 import com.vaadin.flow.function.SerializablePredicate;
 import com.vaadin.flow.internal.nodefeature.SignalBindingFeature;
@@ -340,14 +340,14 @@ private static <T, V extends DataView<T>> V bindItemsInternal(
      * @param refreshAll
      *            callback to refresh all items
      * @param refreshItem
-     *            callback to refresh a single item
+     *            callback to replace a single item, accepting old and new items
      * @param <T>
      *            item type
      */
     private static <T> void setupItemsEffect(Component component,
             Signal<? extends List<? extends Signal<T>>> itemsSignal,
             List<T> backingList, Runnable refreshAll,
-            SerializableConsumer<T> refreshItem) {
+            SerializableBiConsumer<T, T> refreshItem) {
         // List to store inner effect registrations
         List<Registration> innerEffectRegistrations = new ArrayList<>();
 
@@ -390,23 +390,24 @@ private static <T> void setupItemsEffect(Component component,
      * @param backingList
      *            the backing list to update
      * @param refreshItem
-     *            callback to refresh the item
+     *            callback to replace the item, accepting old and new items
      * @param <T>
      *            item type
      * @return the registration for the effect
      */
     private static <T> Registration createItemEffect(Component component,
             Signal<T> itemSignal, int index, List<T> backingList,
-            SerializableConsumer<T> refreshItem) {
+            SerializableBiConsumer<T, T> refreshItem) {
         return Signal.effect(component, context -> {
             // register a dependency on the initial run
             T newValue = itemSignal.get();
 
             // Skip refreshItem on the first run since refreshAll was just
             // called
             if (!context.isInitialRun()) {
+                T oldValue = backingList.get(index);
                 backingList.set(index, newValue);
-                refreshItem.accept(newValue);
+                refreshItem.accept(newValue, oldValue);
             }
         });
     }

flow-data/src/main/java/com/vaadin/flow/data/provider/KeyMapper.java

@@ -163,6 +163,17 @@ public void refresh(V dataObject) {
         }
     }
 
+    @Override
+    public void refresh(V dataObject, V oldDataObject) {
+        Object oldId = identifierGetter.apply(oldDataObject);
+        String key = objectIdKeyMap.remove(oldId);
+        if (key != null) {
+            Object newId = identifierGetter.apply(dataObject);
+            objectIdKeyMap.put(newId, key);
+            keyObjectMap.put(key, dataObject);
+        }
+    }
+
     /**
      * Gets all mapped objects.
      */