[Bug]: RemoveKey with AutoRefresh and Filter throws

[jcummings2]

Describe the bug 🐞

Upgrading to 9.4.31 caused code with the pattern mentioned in the "Steps to Reproduce" section below to fail in some situations.

It can result in the following Exception:

System.ArgumentOutOfRangeException : Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')

The bug was introduced with the rewrite of the static list filter logic in #1063. The issue ultimately stems from what seems to a be long standing feature/bug of RemoveKeyEnumerator that sets the CurrentIndex to -1 when a list is not passed into the constructor (or technically when the item isn't found when a list is passed but no callers actually pass a list). Pull request #1120 with unit tests that demonstrate the problem and a fix.

Step to reproduce

        ReadOnlyObservableCollection<Person> collection;
        using var source = new SourceCache<Person, string>(p => p.Key);
        using var subscription = source.Connect()
                .AutoRefresh(x => x.Age)
                .RemoveKey()
                .Filter(x => x.Age > 50)
                .Bind(out collection)
                .Subscribe();

Reproduction repository

https://github.com/jcummings2/DynamicData

Expected behavior

List should be properly updated with filtered list of items from cache.

Screenshots 🖼️

No response

IDE

No response

Operating system

No response

Version

No response

Device

No response

DynamicData Version

9.4.31

Additional information ℹ️

Worked fine in 9.4.1

[jcummings2]

As noted in the PR, moving the RemoveKey() call to just before Bind() avoids the exception. This detail means code following this pattern works:

        ReadOnlyObservableCollection<Person> collection;
        using var source = new SourceCache<Person, string>(p => p.Key);
        using var subscription = source.Connect()
                .AutoRefresh(x => x.Age)
                .Filter(x => x.Age > 50)
                .RemoveKey()
                .Bind(out collection)
                .Subscribe();

[JakenVeina]

Arguably, this should be the recommended solution. If I had my way, we'd deprecate and remove list-domain filter operators entirely. They're a massive pain to implement properly and/or performantly.

But as I mentioned over in the PR, there's probably quite a few scenarios littered around where list indexes can get stripped out of changesets. If we want to get rid of that behavior, thay really needs to be its own effort. I'm fine with this targeted fix until then.

[JakenVeina]

Should be fixed by #1120. I'll get a patch release published within the next couple days, alongside another couple bugfixes.