[Bug]: BindingListAdaptor does not support Refresh/Evaluate for items

[Metadorius]

Describe the bug 🐞

I noticed that when I call Refresh on an item in SourceCache, it doesn't propagate to my WinForms-based UI (it uses BindingList) it's bound to. Upon further examination of the problem I found that while SourceCache itself seems to be fine, the BindingList doesn't receive any notifications. It seems the culprit is the BindingListAdaptor which just doesn't have any sort of Refresh handling and just does nothing when Refresh comes, despite BindingList being able to handle such changes (see ResetItem; although this is just my assumption, I am still investigating):

DynamicData/src/DynamicData/Binding/BindingListAdaptor.cs

Line 112 in ec02c3b

switch (update.Reason)

Step to reproduce

1. Create a SourceCache with some items.
2. Insert a Do with logging to see the changes emitted.
3. Bind to a BindingList.
4. Subscribe to the binding list's change event with another logging check.
5. Call Refresh(item) on SourceCache.

Reproduction repository

https://github.com/Metadorius/DynamicData.BindingListRefreshRepro

Expected behavior

Both SourceCache and BindingList should reflect the change.

DynamicData Version

7.12.1

Additional information ℹ️

Running on .NET Framework 4.8.

[Metadorius]

On another thought lemme isolate the issue, I'll be back with a reproduction repo.

[Metadorius]

This actually happens in a SortedBindingListAdaptor too. For some reason Refresh is unhandled properly.

Here's a repro repo: https://github.com/Metadorius/DynamicData.BindingListRefreshRepro

[Metadorius]

@RolandPheasant any thoughts on this? maybe I could assist with fixing that, I am just unsure on how should I proceed if I do, since Refresh seems to be a "reset all" type of notification? Maybe it would be better to emit Update?

[RolandPheasant]

Refresh is purely to instruction for an operator to check again. For example when a filter is property based it may be that a property changes so if the filter is:

var filtered = mySource.Connect()
    .AutoRefresh(x => x.IsSelected) // monitor inline property changes
    .Filter(x => x.IsSelected)  // filter on that property and respond to refresh when the property changes

If IsSelected changes, the filter receives a Refresh for the changed record, the filtered result will change. In other words the refresh causes a change in the filter result and the refresh event get's promoted to and Add, or a Remove.

The same concept applies to many but not all operators. In the case of a binding list, a refresh which represents an inline change the list itself cannot respond as a property change cannot result in any operation on the binding list itself. Therefore it is ignored.

What were you expecting, or is there some subtlety of winforms binding that I am missing?

[Metadorius]

Well what I was expecting is that when I call cache.Refresh(item), the BindingList which that item is in gets an update event on that item. Maybe I don't fully understand the intricacies, but from what I observed in BindingList source code there is such a thing:

        //
        // Summary:
        //     Raises a System.ComponentModel.BindingList`1.ListChanged event of type System.ComponentModel.ListChangedType.ItemChanged
        //     for the item at the specified position.
        //
        // Parameters:
        //   position:
        //     A zero-based index of the item to be reset.
        public void ResetItem(int position)
        {
            FireListChanged(ListChangedType.ItemChanged, position);
        }

I am using ReactiveUI and DynamicData with DevExpress on WinForms and noticed that whenever I update something in my collection's items it won't get immediately updated in the UI, even if I call Refresh on the item manually.

[RolandPheasant]

Ah, I get it now. I did not know about reset item - it's been at least 12 years since I last used binding list! The solution is:

      case ChangeReason.Refresh:
          {
              var index = list.IndexOf(update.Current);
              if (index != -1)
                  list.ResetItem(index);
              break;
          }

in the adaptor. I'll have to apply the fix to the sorted adaptor too, which is a little trickier.

[RolandPheasant]

This should do it: #678

I'll write some tests hopefully tomorrow.

[RolandPheasant]

I have release 7.13.1 which should fix this issue. Any issues, get back to me here.

[Metadorius]

Everything is working as intended with this change! The binding list receives the refresh events now and the UI updates accordingly to it (from what I understand, tracking the in-collection item property changes is out of scope (or I haven't read the doc carefully enough), so without manual refresh sending it doesn't work, expectedly).

Thanks much for the fix 👍

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.