Bug Fix: Consolidate changes made to Group Caches into a single Chang…

…eSet (#863)

Fixes #851 by leveraging the code from #852 to suspend the notifications about changes to the group contents until all of the groups have been updated.

src/DynamicData.Tests/Cache/GroupOnDynamicFixture.cs

@@ -34,16 +34,15 @@ public class GroupOnDynamicFixture : IDisposable
     private readonly GroupChangeSetAggregator<Person, string, string> _groupResults;
     private readonly Faker<Person> _faker;
     private readonly Randomizer _randomizer;
-    private readonly Subject<Func<Person, string, string>> _keySelectionSubject = new ();
+    private readonly BehaviorSubject<Func<Person, string, string>?> _keySelectionSubject = new (null);
     private readonly Subject<Unit> _regroupSubject = new ();
-    private Func<Person, string, string>? _groupKeySelector;
 
     public GroupOnDynamicFixture()
     {
         unchecked { _randomizer = new((int)0xc001_d00d); }
         _faker = Fakers.Person.Clone().WithSeed(_randomizer);
         _results = _cache.Connect().AsAggregator();
-        _groupResults = _cache.Connect().Group(_keySelectionSubject.Do(func => _groupKeySelector = func), _regroupSubject).AsAggregator();
+        _groupResults = _cache.Connect().Group(KeySelectionObservable, _regroupSubject).AsAggregator();
     }
 
     [Theory]
@@ -151,6 +150,7 @@ public void ResultContainsAllAddedChildren()
         // Assert
         _results.Data.Count.Should().Be(InitialCount);
         _results.Messages.Count.Should().Be(1, "The child observables fire on subscription so everything should appear as a single changeset");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().Be(1));
         VerifyGroupingResults();
     }
 
@@ -167,6 +167,7 @@ public void ResultContainsAddedValues()
         // Assert
         _results.Data.Count.Should().Be(InitialCount + AddCount);
         _results.Messages.Count.Should().Be(2, "Initial Adds and then the subsequent Additions should each be a single message");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().BeLessThanOrEqualTo(2));
         VerifyGroupingResults();
     }
 
@@ -183,6 +184,7 @@ public void ResultDoesNotContainRemovedValues()
         // Assert
         _results.Data.Count.Should().Be(InitialCount - RemoveCount);
         _results.Messages.Count.Should().Be(2, "1 for Adds and 1 for Removes");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().BeLessThanOrEqualTo(2));
         VerifyGroupingResults();
     }
 
@@ -201,6 +203,7 @@ public void ResultContainsUpdatedValues()
         // Assert
         _results.Data.Count.Should().Be(InitialCount, "Only replacements were made");
         _results.Messages.Count.Should().Be(2, "1 for Adds and 1 for Updates");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().BeLessThanOrEqualTo(2));
         VerifyGroupingResults();
     }
 
@@ -303,7 +306,7 @@ public void ResultFailsIfSourceFails()
         InitialPopulate();
         var expectedError = new Exception("Expected");
         var throwObservable = Observable.Throw<IChangeSet<Person, string>>(expectedError);
-        using var results = _cache.Connect().Concat(throwObservable).Group(_keySelectionSubject, _regroupSubject).AsAggregator();
+        using var results = _cache.Connect().Concat(throwObservable).Group(KeySelectionObservable, _regroupSubject).AsAggregator();
 
         // Act
         _cache.Dispose();
@@ -349,10 +352,12 @@ public void Dispose()
         _regroupSubject.Dispose();
     }
 
+    private IObservable<Func<Person, string, string>> KeySelectionObservable => _keySelectionSubject.Where(v => v is not null).Select(v => v!);
+
     private void InitialPopulate() => _cache.AddOrUpdate(_faker.Generate(InitialCount));
 
     private void VerifyGroupingResults() =>
-        VerifyGroupingResults(_cache, _results, _groupResults, _groupKeySelector);
+        VerifyGroupingResults(_cache, _results, _groupResults, _keySelectionSubject.Value);
 
     private static void VerifyGroupingResults(ISourceCache<Person, string> cache, ChangeSetAggregator<Person, string> cacheResults, GroupChangeSetAggregator<Person, string, string> groupResults, Func<Person, string, string>? groupKeySelector)
     {
@@ -374,7 +379,7 @@ private static void VerifyGroupingResults(ISourceCache<Person, string> cache, Ch
         expectedGroupings.ForEach(grouping => grouping.Should().BeEquivalentTo(groupResults.Groups.Lookup(grouping.Key).Value.Data.Items));
 
         // No groups should be empty
-        groupResults.Groups.Items.ForEach(group => group.Data.Count.Should().BeGreaterThan(0));
+        groupResults.Groups.Items.ForEach(group => group.Data.Count.Should().BeGreaterThan(0, "Empty groups should be removed"));
     }
 
     private void ForceRegroup() => _regroupSubject.OnNext(Unit.Default);

src/DynamicData.Tests/Cache/GroupOnObservableFixture.cs

@@ -1,15 +1,16 @@
 using System;
 using System.Linq;
+using System.Reactive.Linq;
+using System.Threading.Tasks;
+
 using Bogus;
-using DynamicData.Tests.Domain;
 using DynamicData.Binding;
-using System.Reactive.Linq;
+using DynamicData.Kernel;
+using DynamicData.Tests.Domain;
 using FluentAssertions;
 using Xunit;
 
 using Person = DynamicData.Tests.Domain.Person;
-using System.Threading.Tasks;
-using DynamicData.Kernel;
 
 namespace DynamicData.Tests.Cache;
 
@@ -50,6 +51,7 @@ public void ResultContainsAllInitialChildren()
         // Assert
         _results.Data.Count.Should().Be(InitialCount);
         _results.Messages.Count.Should().Be(1, "The child observables fire on subscription so everything should appear as a single changeset");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().Be(1));
         VerifyGroupingResults();
     }
 
@@ -65,6 +67,7 @@ public void ResultContainsAddedValues()
         // Assert
         _results.Data.Count.Should().Be(InitialCount + AddCount);
         _results.Messages.Count.Should().Be(2, "Initial Adds and then the subsequent Additions should each be a single message");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().BeLessThanOrEqualTo(2));
         VerifyGroupingResults();
     }
 
@@ -80,6 +83,7 @@ public void ResultDoesNotContainRemovedValues()
         // Assert
         _results.Data.Count.Should().Be(InitialCount - RemoveCount);
         _results.Messages.Count.Should().Be(2, "1 for Adds and 1 for Removes");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().BeLessThanOrEqualTo(2));
         VerifyGroupingResults();
     }
 
@@ -97,6 +101,7 @@ public void ResultContainsUpdatedValues()
         // Assert
         _results.Data.Count.Should().Be(InitialCount, "Only replacements were made");
         _results.Messages.Count.Should().Be(2, "1 for Adds and 1 for Updates");
+        _groupResults.Groups.Items.ForEach(group => group.Messages.Count.Should().BeLessThanOrEqualTo(2));
         VerifyGroupingResults();
     }
 
@@ -115,6 +120,7 @@ public void GroupRemovedWhenEmpty()
         // Assert
         _cache.Items.Select(p => p.FavoriteColor).Distinct().Count().Should().Be(colorCount - 1);
         _results.Messages.Count.Should().Be(2, "1 for Adds and 1 for Removes");
+        _groupResults.Data.Count.Should().Be(colorCount - 1, "{0} colors were used and then all of the {1} were removed", colorCount, removeColor);
         _groupResults.Messages.Count.Should().Be(2, "1 for Adds and 1 for Removes");
         _groupResults.Summary.Overall.Adds.Should().Be(colorCount);
         _groupResults.Summary.Overall.Removes.Should().Be(1);
@@ -142,9 +148,11 @@ public void GroupNotRemovedIfAddedBackImmediately()
         // Assert
         _cache.Items.Select(p => p.FavoriteColor).Distinct().Count().Should().Be(colorCount);
         _results.Messages.Count.Should().Be(2, "1 for Adds and 1 for Other Added Value");
+        _groupResults.Data.Count.Should().Be(colorCount);
         _groupResults.Messages.Count.Should().Be(1, "Shouldn't be removed/re-added");
         _groupResults.Summary.Overall.Adds.Should().Be(colorCount);
         _groupResults.Summary.Overall.Removes.Should().Be(0);
+        _groupResults.Groups.Lookup(removeColor).Value.Data.Count.Should().Be(1, "All the {0} were removed and then 1 was added back", removeColor);
         VerifyGroupingResults();
     }
 
@@ -329,6 +337,9 @@ private static void VerifyGroupingResults(ISourceCache<Person, string> cache, Ch
 
         // Check each group
         expectedGroupings.ForEach(grouping => grouping.Should().BeEquivalentTo(groupResults.Groups.Lookup(grouping.Key).Value.Data.Items));
+
+        // No groups should be empty
+        groupResults.Groups.Items.ForEach(group => group.Data.Count.Should().BeGreaterThan(0, "Empty groups should be removed"));
     }
 
     private static IObservable<Color> CreateFavoriteColorObservable(Person person, string key) =>