Fix Exception in list static filter index assumptions (#1120)

* Add RemoveKey tests showing issue with Refresh and Filter

* Fix index out of range issue with static List Filter

* Remove unused variable in RemoveKeyFixture

* Remove unused variable in RemoveKeyFixture (really)

* Revert formatting to previous in Filter.Static.cs

* Add unit test changing order or RemoveKey call

* Update RemoveKey test names based on PR feedback

* Remove extraneous comment per PR feedback

* Move Filter-related RemoveKey tests to FilterFixture

---------

Co-authored-by: John Cummings <jcummings2sf@gmail.com>

Author: jcummings2

src/DynamicData.Tests/Cache/FilterFixture.Static.cs

@@ -6,6 +6,9 @@
 using Xunit;
 
 using DynamicData.Tests.Utilities;
+using DynamicData.Tests.Domain;
+using System.Collections.ObjectModel;
+using System.Linq;
 
 namespace DynamicData.Tests.Cache;
 
@@ -86,5 +89,55 @@ protected override IObservable<IChangeSet<Item, int>> BuildUut(
             => source.Filter(
                 filter:                     predicate,
                 suppressEmptyChangeSets:    suppressEmptyChangeSets);
+        [Fact]
+        public void AutoRefreshRemoveKeyFilterUpdate_CollectionUpdated()
+        {
+            RandomPersonGenerator generator = new();
+            using var source = new SourceCache<Person, string>(p => p.Key);
+            var people = generator.Take(100).ToArray();
+            var average = people.Average(x => x.Age);
+            ReadOnlyObservableCollection<Person> collection;
+            using var subscription = source.Connect()
+                .AutoRefresh(x => x.Age)
+                .RemoveKey()
+                .Filter(x => x.Age < average)
+                .Bind(out collection)
+                .Subscribe();
+            source.AddOrUpdate(people);
+
+            Assert.Equivalent(people.Where(x => x.Age < average), collection);
+
+            foreach (var person in people)
+            {
+                person.Age = person.Age + 1;
+            }
+            Assert.Equivalent(people.Where(x => x.Age < average), collection);
+        }
+
+        [Fact]
+        public void AutoRefreshFilterRemoveKeyUpdate_CollectionUpdated()
+        {
+            RandomPersonGenerator generator = new();
+            using var source = new SourceCache<Person, string>(p => p.Key);
+            var people = generator.Take(100).ToArray();
+            var average = people.Average(x => x.Age);
+            ReadOnlyObservableCollection<Person> collection;
+            using var subscription = source.Connect()
+                .AutoRefresh(x => x.Age)
+                .Filter(x => x.Age < average)
+                .RemoveKey()
+                .Bind(out collection)
+                .Subscribe();
+            source.AddOrUpdate(people);
+
+            Assert.Equivalent(people.Where(x => x.Age < average), collection);
+
+            foreach (var person in people)
+            {
+                person.Age = person.Age + 1;
+            }
+            Assert.Equivalent(people.Where(x => x.Age < average), collection);
+        }
     }
+
 }

src/DynamicData.Tests/Cache/RemoveKeyFixture.cs

@@ -0,0 +1,95 @@
+#region
+
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Linq;
+using System.Reactive.Disposables;
+
+using DynamicData.Binding;
+using DynamicData.Tests.Domain;
+
+using FluentAssertions;
+
+using Xunit;
+
+#endregion
+
+namespace DynamicData.Tests.Cache;
+
+public class RemoveKeyFixture : IDisposable
+{
+    private readonly RandomPersonGenerator _generator = new();
+
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "Handled with CompositeDisposable")]
+    private readonly ISourceCache<Person, string> _source;
+
+    private readonly CompositeDisposable _cleanup = new();
+
+    public RemoveKeyFixture()
+    {
+        _source = new SourceCache<Person, string>(p => p.Key);
+        _cleanup.Add(_source);
+    }
+
+    public void Dispose() => _cleanup.Dispose();
+
+    [Fact]
+    public void CacheRemoveKey_Add_KeyIsRemoved()
+    {
+        ReadOnlyObservableCollection<Person> collection;
+        _cleanup.Add(
+            _source.Connect()
+                .RemoveKey()
+                .Bind(out collection)
+                .Subscribe()
+        );
+        var people = _generator.Take(100).ToArray();
+        _source.AddOrUpdate(people);
+
+        Assert.Equivalent(people, collection);
+    }
+
+    [Fact]
+    public void CacheRemoveKey_Filter_ItemsFilterKeyIsRemoved()
+    {
+        var people = _generator.Take(100).ToArray();
+        var average = people.Average(x => x.Age);
+
+        ReadOnlyObservableCollection<Person> collection;
+        _cleanup.Add(
+            _source.Connect()
+                .RemoveKey()
+                .Filter(x => x.Age < average)
+                .Bind(out collection)
+                .Subscribe()
+        );
+        _source.AddOrUpdate(people);
+
+        Assert.Equivalent(people.Where(x => x.Age < average), collection);
+    }
+
+    [Fact]
+    public void CacheRemoveKey_AutoRefreshUpdateITems_CollectionUpdated()
+    {
+        ReadOnlyObservableCollection<Person> collection;
+        _cleanup.Add(
+            _source.Connect()
+                .AutoRefresh(x => x.Age)
+                .RemoveKey()
+                .Bind(out collection)
+                .Subscribe()
+        );
+        var people = _generator.Take(100).ToArray();
+        _source.AddOrUpdate(people);
+
+        Assert.Equivalent(people, collection);
+
+        foreach (var person in people)
+        {
+            person.Age = person.Age + 1;
+        }
+        Assert.Equivalent(people, collection);
+    }
+
+}

src/DynamicData/List/Internal/Filter.Static.cs

@@ -26,7 +26,6 @@ public static IObservable<IChangeSet<T>> Create(
 
                 var downstreamItems = new ChangeAwareList<T>();
                 var itemsBuffer = new List<T>();
-
                 var downstream = source.Select(upstreamChanges =>
                 {
                     foreach (var change in upstreamChanges)
@@ -212,13 +211,23 @@ public static IObservable<IChangeSet<T>> Create(
                                 {
                                     var isIncluded = predicate.Invoke(change.Item.Current);
 
-                                    var itemState = upstreamItemsStates[change.Item.CurrentIndex];
-                                    upstreamItemsStates[change.Item.CurrentIndex] = (
+                                    var currentIndex = change.Item.CurrentIndex;
+                                    // A Replace might have a negative CurrentIndex from a Refresh in RemoveKeyEnumerator
+                                    if (currentIndex < 0)
+                                    {
+                                        var previous = upstreamItemsStates.Select(x => x.item)
+                                            .IndexOfOptional(change.Item.Current)
+                                            .ValueOrThrow(() => new InvalidOperationException($"Cannot find index of {typeof(T).Name} -> {change.Item.Current}. Expected to be in the list"));
+                                        currentIndex = previous.Index;
+                                    }
+                                    var itemState = upstreamItemsStates[currentIndex];
+
+                                    upstreamItemsStates[currentIndex] = (
                                         item: change.Item.Current,
                                         isIncluded: isIncluded);
 
                                     var downstreamIndex = (isIncluded || itemState.isIncluded)
-                                        ? change.Item.CurrentIndex - CountExcludedItemsBefore(change.Item.CurrentIndex)
+                                        ? currentIndex - CountExcludedItemsBefore(currentIndex)
                                         : -1;
 
                                     switch (itemState.isIncluded, isIncluded)