From b07f316292bf86295eb0c1d7e1858fd131fb5738 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 14:44:17 -0500 Subject: [PATCH 1/9] 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 000000000..a49143c6a --- /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 178a84a3091a45205ba69543fdefdb143adfa318 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 15:33:40 -0500 Subject: [PATCH 2/9] 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 36a49c5e9..3e9d6e402 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 b81b69e72fdd6880a58347bdc26cffe0d6306a84 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 16:08:26 -0500 Subject: [PATCH 3/9] 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 a49143c6a..55edb54f3 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 1a25cc9bc769df21f8a4384da15cdb7a91e84624 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 16:11:00 -0500 Subject: [PATCH 4/9] 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 55edb54f3..9051e2a33 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 4438c24c3985440e95b2d95cc480e646bbd12276 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 24 Jun 2026 16:11:57 -0500 Subject: [PATCH 5/9] 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 3e9d6e402..f67890d3f 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 5379d1ce2691ca5417f506b2a090b469cf29ee2a Mon Sep 17 00:00:00 2001 From: John Cummings Date: Thu, 25 Jun 2026 07:30:45 -0500 Subject: [PATCH 6/9] 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 9051e2a33..a602a8c9a 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 7b6b765fb6af200b23ec5d747b79940c66700117 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Thu, 25 Jun 2026 07:44:23 -0500 Subject: [PATCH 7/9] 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 a602a8c9a..1c81744af 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 6da70e2f3b6cd1490981bfb6f92e19e6d868dfe0 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Thu, 25 Jun 2026 07:44:37 -0500 Subject: [PATCH 8/9] 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 f67890d3f..48b12020c 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")); From 7d02d18bf9535088f2896daafe2b82b1f7f98c61 Mon Sep 17 00:00:00 2001 From: John Cummings Date: Wed, 1 Jul 2026 11:05:43 -0500 Subject: [PATCH 9/9] Move Filter-related RemoveKey tests to FilterFixture --- .../Cache/FilterFixture.Static.cs | 53 +++++++++++++++++++ .../Cache/RemoveKeyFixture.cs | 49 ----------------- 2 files changed, 53 insertions(+), 49 deletions(-) diff --git a/src/DynamicData.Tests/Cache/FilterFixture.Static.cs b/src/DynamicData.Tests/Cache/FilterFixture.Static.cs index f3d61ac0a..1ca4de0cd 100644 --- a/src/DynamicData.Tests/Cache/FilterFixture.Static.cs +++ b/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> BuildUut( => source.Filter( filter: predicate, suppressEmptyChangeSets: suppressEmptyChangeSets); + [Fact] + public void AutoRefreshRemoveKeyFilterUpdate_CollectionUpdated() + { + RandomPersonGenerator generator = new(); + using var source = new SourceCache(p => p.Key); + var people = generator.Take(100).ToArray(); + var average = people.Average(x => x.Age); + ReadOnlyObservableCollection 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(p => p.Key); + var people = generator.Take(100).ToArray(); + var average = people.Average(x => x.Age); + ReadOnlyObservableCollection 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); + } } + } diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs index 1c81744af..04d27f619 100644 --- a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs @@ -92,53 +92,4 @@ public void CacheRemoveKey_AutoRefreshUpdateITems_CollectionUpdated() Assert.Equivalent(people, collection); } - [Fact] - public void Cache_AutoRefreshRemoveKeyFilterUpdate_CollectionUpdated() - { - 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); - } - - [Fact] - public void Cache_AutoRefreshFilterRemoveKeyUpdate_CollectionUpdated() - { - 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); - } }