From 65ce91a57599e800aef01f7db4fef2ed5c62b6b5 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 14:44:17 -0500 Subject: [PATCH 1/8] Add RemoveKey tests showing issue with Refresh and Filter --- .../Cache/RemoveKeyFixture.cs | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 src/DynamicData.Tests/Cache/RemoveKeyFixture.cs diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs new file mode 100644 index 00000000..a49143c6 --- /dev/null +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs @@ -0,0 +1,123 @@ +#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 IComparer _comparer; + + private readonly RandomPersonGenerator _generator = new(); + + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "Handled with CompositeDisposable")] + private readonly ISourceCache _source; + + private readonly CompositeDisposable _cleanup = new(); + + public RemoveKeyFixture() + { + _comparer = SortExpressionComparer.Ascending(p => p.Age).ThenByAscending(p => p.Name); + + _source = new SourceCache(p => p.Key); + _cleanup.Add(_source); + } + + public void Dispose() => _cleanup.Dispose(); + + [Fact] + public void Add() + { + ReadOnlyObservableCollection 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 Filter() + { + var people = _generator.Take(100).ToArray(); + var average = people.Average(x => x.Age); + + ReadOnlyObservableCollection 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 Refresh() + { + ReadOnlyObservableCollection 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); + } + + [Fact] + public void RefreshFilter() + { + var people = _generator.Take(100).ToArray(); + var average = people.Average(x => x.Age); + ReadOnlyObservableCollection collection; + _cleanup.Add( + _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); + } +} From 48e50a73f45acdd6b16b40e921b92fbd53bddaeb Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 15:33:40 -0500 Subject: [PATCH 2/8] Fix index out of range issue with static List Filter --- .../List/Internal/Filter.Static.cs | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/DynamicData/List/Internal/Filter.Static.cs b/src/DynamicData/List/Internal/Filter.Static.cs index 36a49c5e..3e9d6e40 100644 --- a/src/DynamicData/List/Internal/Filter.Static.cs +++ b/src/DynamicData/List/Internal/Filter.Static.cs @@ -26,7 +26,6 @@ public static IObservable> Create( var downstreamItems = new ChangeAwareList(); var itemsBuffer = new List(); - var downstream = source.Select(upstreamChanges => { foreach (var change in upstreamChanges) @@ -177,10 +176,12 @@ public static IObservable> Create( break; case ListChangeReason.Remove: - if (upstreamItemsStates[change.Item.CurrentIndex].isIncluded) - downstreamItems.RemoveAt(change.Item.CurrentIndex - CountExcludedItemsBefore(change.Item.CurrentIndex)); + { + if (upstreamItemsStates[change.Item.CurrentIndex].isIncluded) + downstreamItems.RemoveAt(change.Item.CurrentIndex - CountExcludedItemsBefore(change.Item.CurrentIndex)); - upstreamItemsStates.RemoveAt(change.Item.CurrentIndex); + upstreamItemsStates.RemoveAt(change.Item.CurrentIndex); + } break; case ListChangeReason.RemoveRange: @@ -212,13 +213,24 @@ public static IObservable> 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) + { + // this code mimics the previous logic + 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) From df8e081467873597fe5b79b96c7213fe10539518 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 16:08:26 -0500 Subject: [PATCH 3/8] Remove unused variable in RemoveKeyFixture --- src/DynamicData.Tests/Cache/RemoveKeyFixture.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs index a49143c6..55edb54f 100644 --- a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs @@ -19,8 +19,6 @@ namespace DynamicData.Tests.Cache; public class RemoveKeyFixture : IDisposable { - private readonly IComparer _comparer; - private readonly RandomPersonGenerator _generator = new(); [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "Handled with CompositeDisposable")] From 0c739a73fcdffeeee57c2df76f4931947e0f53eb Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 16:11:00 -0500 Subject: [PATCH 4/8] Remove unused variable in RemoveKeyFixture (really) --- src/DynamicData.Tests/Cache/RemoveKeyFixture.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs index 55edb54f..9051e2a3 100644 --- a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs @@ -28,8 +28,6 @@ public class RemoveKeyFixture : IDisposable public RemoveKeyFixture() { - _comparer = SortExpressionComparer.Ascending(p => p.Age).ThenByAscending(p => p.Name); - _source = new SourceCache(p => p.Key); _cleanup.Add(_source); } From cfb2d81d26075a3dcd07c86c479adac54ec539ad Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 16:11:57 -0500 Subject: [PATCH 5/8] Revert formatting to previous in Filter.Static.cs --- src/DynamicData/List/Internal/Filter.Static.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/DynamicData/List/Internal/Filter.Static.cs b/src/DynamicData/List/Internal/Filter.Static.cs index 3e9d6e40..f67890d3 100644 --- a/src/DynamicData/List/Internal/Filter.Static.cs +++ b/src/DynamicData/List/Internal/Filter.Static.cs @@ -176,12 +176,10 @@ public static IObservable> Create( break; case ListChangeReason.Remove: - { - if (upstreamItemsStates[change.Item.CurrentIndex].isIncluded) - downstreamItems.RemoveAt(change.Item.CurrentIndex - CountExcludedItemsBefore(change.Item.CurrentIndex)); + if (upstreamItemsStates[change.Item.CurrentIndex].isIncluded) + downstreamItems.RemoveAt(change.Item.CurrentIndex - CountExcludedItemsBefore(change.Item.CurrentIndex)); - upstreamItemsStates.RemoveAt(change.Item.CurrentIndex); - } + upstreamItemsStates.RemoveAt(change.Item.CurrentIndex); break; case ListChangeReason.RemoveRange: From a414c324bcb1009d5d7eb7414bf058e24b13f3fe Mon Sep 17 00:00:00 2001 From: John Cummings Date: Thu, 25 Jun 2026 07:30:45 -0500 Subject: [PATCH 6/8] Add unit test changing order or RemoveKey call --- .../Cache/RemoveKeyFixture.cs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs index 9051e2a3..a602a8c9 100644 --- a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs @@ -93,7 +93,7 @@ public void Refresh() } [Fact] - public void RefreshFilter() + public void RefreshRemoveKeyFilter() { var people = _generator.Take(100).ToArray(); var average = people.Average(x => x.Age); @@ -116,4 +116,29 @@ public void RefreshFilter() } Assert.Equivalent(people.Where(x => x.Age < average), collection); } + + [Fact] + public void RefreshFilterRemoveKey() + { + var people = _generator.Take(100).ToArray(); + var average = people.Average(x => x.Age); + ReadOnlyObservableCollection collection; + _cleanup.Add( + _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); + } } From 9a93fd3930a9cbd7a6396a25e5cd4b0327d5e73c Mon Sep 17 00:00:00 2001 From: John Cummings Date: Thu, 25 Jun 2026 07:44:23 -0500 Subject: [PATCH 7/8] Update RemoveKey test names based on PR feedback --- src/DynamicData.Tests/Cache/RemoveKeyFixture.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs index a602a8c9..1c81744a 100644 --- a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs @@ -35,7 +35,7 @@ public RemoveKeyFixture() public void Dispose() => _cleanup.Dispose(); [Fact] - public void Add() + public void CacheRemoveKey_Add_KeyIsRemoved() { ReadOnlyObservableCollection collection; _cleanup.Add( @@ -51,7 +51,7 @@ public void Add() } [Fact] - public void Filter() + public void CacheRemoveKey_Filter_ItemsFilterKeyIsRemoved() { var people = _generator.Take(100).ToArray(); var average = people.Average(x => x.Age); @@ -70,7 +70,7 @@ public void Filter() } [Fact] - public void Refresh() + public void CacheRemoveKey_AutoRefreshUpdateITems_CollectionUpdated() { ReadOnlyObservableCollection collection; _cleanup.Add( @@ -93,7 +93,7 @@ public void Refresh() } [Fact] - public void RefreshRemoveKeyFilter() + public void Cache_AutoRefreshRemoveKeyFilterUpdate_CollectionUpdated() { var people = _generator.Take(100).ToArray(); var average = people.Average(x => x.Age); @@ -118,7 +118,7 @@ public void RefreshRemoveKeyFilter() } [Fact] - public void RefreshFilterRemoveKey() + public void Cache_AutoRefreshFilterRemoveKeyUpdate_CollectionUpdated() { var people = _generator.Take(100).ToArray(); var average = people.Average(x => x.Age); From 3aae7fade3a1538e3ebaf9d722ae8bd022a25204 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Thu, 25 Jun 2026 07:44:37 -0500 Subject: [PATCH 8/8] Remove extraneous comment per PR feedback --- src/DynamicData/List/Internal/Filter.Static.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/DynamicData/List/Internal/Filter.Static.cs b/src/DynamicData/List/Internal/Filter.Static.cs index f67890d3..48b12020 100644 --- a/src/DynamicData/List/Internal/Filter.Static.cs +++ b/src/DynamicData/List/Internal/Filter.Static.cs @@ -215,7 +215,6 @@ public static IObservable> Create( // A Replace might have a negative CurrentIndex from a Refresh in RemoveKeyEnumerator if (currentIndex < 0) { - // this code mimics the previous logic 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"));