Flow: Refreshing a data provider won't update items when the item instance is not the same

[heruan]

When refreshing a Grid data provider, if the item instances change (e.g. when fetched from a database), rows are not updated if a column value has changed.

Demo: GridRefreshAll.java

[kolle1986]

Yes, this depends on hash code and equals of underlying item object, if I understand problem correctly. Can be annoying really.

[heruan]

I think it's a bug then, since when overriding the DataProvider::getId it shouldn't rely on equals/hashCode but on the ID. Certain frameworks (e.g. Spring Data with projections, or Blaze Persistence with entity views) might provide DTO instances created dynamically and you can't really override their equals/hashCode methods.

[Legioth]

I can confirm that the entire point of overriding DataProvider::getId is to make all object identity decisions based on that method instead of directly relying equals and hashCode.

Object identity should only be needed for things like tracking selection and for the more granular DataProvider::refreshItem method. refreshAll() shouldn't really depend on equals/hashCode/getId for anything, but rather throw away all old state and resend a completely new set of items to the client.

[heruan]

Does refreshAll contain some logic, or just dispatches an event? I mean, should this be fixed in flow-data or vaadin-grid-flow?

[Legioth]

refreshAll() fires an event that is handled by com.vaadin.flow.data.provider.DataCommunicator by delegating to its reset() method. I guess there's some field that isn't reset, but I cannot tell whether it's on the DataCommunicator or Grid side without further investigation.

[Legioth]

As a workaround, you can do grid.setDataProvider(grid.getDataProvider());.

[heruan]

Thank you @Legioth for the workaround; unfortunately, I can't make use of it since the callback which triggers the refresh doesn't have access to the component, just the data provider. It would be great to have this tackled down soon!

[heruan]

Any update on this? Is the demo project I provided sufficient?

[Legioth]

I've identified the issue. The problem is that DataCommunicator.keyMapper will still hold a reference to the old item instance, which then ends up being used when generating the data to send to the client.

Clearing the entire keyMapper for refreshAll() might not be safe since the client might at that moment have a request in flight that will e.g. toggle selection of some item. We can only clear a key from the mapper after the client has confirmed that it no longer holds references to that key.

Instead, we should ensure that the latest item from the data provider is also the item that stored in the key mapper. In Vaadin 8, this is done by calling keyMapper.refresh(data) when the data is being generated. In Flow, I think this needs to be done in DataCommunicator.activate since that's the method that actually fetches fresh items from the data provider.