[Bug]: ResetOnFirstTimeLoad not working with sorting

[DevEngineReq]

Describe the bug 🐞

If we use a sorting (e.g. Sort()) resetOnFirstTimeLoad is not working / has no affect.

(For SortAndBind i did not find resetOnFirstTimeLoad)

Without sorting it works as expected.

May be this is the problem

Adapt Method with ChangeSet (resetOnFirstTimeLoad is OR related) WORKS

Adapt Method with SortedChangeSet (resetOnFirstTimeLoad is && related) NOT WORKING

With version 8.0.2 it work as expected. If I reconnect to cache a reset was triggered on first time load;
With version 8.1.1 and newer a reset was only triggered if change count > threshold

Step to reproduce

See Description

Reproduction repository

https://github.com/reactivemarbles/DynamicData

Expected behavior

Reset on first time load while using sorting

Screenshots 🖼️

No response

IDE

Visual Studio

Operating system

Windows

Version

No response

Device

No response

DynamicData Version

9.0.4

Additional information ℹ️

No response

[JakenVeina]

I'll defer to @RolandPheasant for the design intent behind these operators, but I can confirm the behavior you describe, the logic of the change seems sound, and implementing the change doesn't break any existing testing.

[RolandPheasant]

Ok. This is clearly a bug and needs fixing.

However, for the latest version 9, sort has been marked obsolete and it's been replaced by SortAndBind. Therefore upgrading may be a bit painful just to see whether the new version works. Instead I'll take a look early next week and see whether I can provide a snippet for a new adaptor which you could add into your codebase.

[DevEngineReq]

Currenty we are using the Version 9.0.4. Due to that we switched from .Sort() + .Bind() to .SortAndBind()
As default we use a ReadOnlyObservableCollection together with .SortAndBind(out ,....)
In this cases the SortAndBind works because the list is already empty.

But in one case we need to use a ObservableCollectionExtended (which already contains objects) + change Binding at runtime
And for this case ResetOnFirstTimeLoad is requried.
Is there ia trick how SortAndBind does support ResetOnFirstTimeLoad. In our case it does not work.
SortAndBindOptions has no const DefaultResetOnFirstTimeLoad like BindingOptions

[JakenVeina]

But in one case we need to use a ObservableCollectionExtended (which already contains objects) + change Binding at runtime

What does this look like, in a query?

[DevEngineReq]

I created a repository with a small example project which shows the problem.
Repository

[JakenVeina]

To me, I'm not convinced this (clearing the binding target, during subscription) is a behavior that either .Bind() or .SortAndBind() are really responsible for. I'm also not convinced this shouldn't just be what the operators ALWAYS do. We'll have to talk about this one, a bit.

In the meantime, if you haven't already, this is quite easily worked-around.

If you're going to be manually switching bindings, I'd say it becomes your responsibility to just .Clear() the collection before you apply the new binding.

private void Connect()
{
    this.LastConnect = DateTime.UtcNow;
    this.cacheDisposable?.Dispose();

    this.users.Clear();

    this.cacheDisposable = this.userCache.Connect()
        .ObserveOn(RxApp.TaskpoolScheduler)
        .Filter(static x => x.Id < 10) //lower than Binding ResetThreshold to avoid reset due to too many changes
        .ObserveOn(RxApp.MainThreadScheduler)
        .SortAndBind(this.users, this.sorter)
        .Subscribe();
}

Or you can let RX and DynamicData do the binding-management for you, where the .Clear() is already baked in.

this.cacheDisposable = Observable.Interval(TimeSpan.FromSeconds(3))
    .StartWith(0)
    .ObserveOn(RxApp.MainThreadScheduler)
    .Do(_ => this.LastConnect = DateTime.UtcNow)
    .ObserveOn(RxApp.TaskpoolScheduler)
    .Select(_ => this.userCache.Connect()
        .Filter(static x => x.Id < 10))
    .Switch()
    .ObserveOn(RxApp.MainThreadScheduler)
    .SortAndBind(this.users, this.sorter)
    .Subscribe();

ResetOnFirstTimeLoad maybe still makes sense as an adapter-level behavior, I.E. it's a potential optimization for certain implementations of ObservableCollection

[DevEngineReq]

Thank you for the bugfix #926. This will help us to use the newest DynamicData Version, because the Sort() is still available and the .Bind() supports ResetOnFirstTimeLoad. Is there a release date?

We need ResetOnFirstTimeLoad not only for avoid ambiguous objects (like in my example app)
but also for our WPF Persist Selection Behavior which needs NotifyCollectionChangedAction.Reset to handle the selection in the datagrid.

A normal .clear() will remove the selection. Of course we can cache the selection and set back within the viewmodel, but at this time we do not know if the object still available in the list. (other cache, filter sorting… very dynamic)

Is there a way to bring back the ResetOnFirstTimeLoad option in SortAndBind?
Or can I do it myself and yes what would that look like, so i can use SortAndBind in the futher.

[JakenVeina]

We need ResetOnFirstTimeLoad [...] for our WPF Persist Selection Behavior which needs NotifyCollectionChangedAction.Reset to handle the selection in the datagrid.

That definitely sounds like you should be using .Switch() then. If that operator isn't triggering a Reset when switching subscriptions, I would say that's a bug we can attempt to fix.

I'm also curious how a Reset preserves the selection. A DataGrid normally preserves selections by equality-comparison, right? Do you have a custom .Equals() implementation on these objects? Or are somehow otherwise overriding equality comparison?

[RolandPheasant]

I am working to make ResetOnFirstTimeLoad and opt-in option for SortAndBind.

A system wide opt-in for this old behaviour will be achieved as follows:

DynamicDataOptions.SortAndBind = DynamicDataOptions.SortAndBind with { ResetOnFirstTimeLoad = true };

Adding this option will not lead to too much code complexity, but will help consumers maintain old behaviours. I am also thinking about adding a Scheduler options for the bindings. Then ObserveOn(MainThreadScheduler) will be redundant.

[DevEngineReq]

That would be great. Thank you.

[JakenVeina]

@RolandPheasant

I am also thinking about adding a Scheduler options for the bindings

There's another issue related to this, actually, if you hadn't spotted it. #932.