diff --git a/.github/instructions/dynamicdata-cache.instructions.md b/.github/instructions/dynamicdata-cache.instructions.md index 45b1874bb..35449d80a 100644 --- a/.github/instructions/dynamicdata-cache.instructions.md +++ b/.github/instructions/dynamicdata-cache.instructions.md @@ -808,6 +808,17 @@ Converts `IChangeSet` into `IObservable>` — one emission per Converts `IChangeSet` to `IChangeSet` — drops the key to produce a list changeset. +Positions are tracked independently per subscription by cache key, not item equality, so equal values and shared object references remain distinct entries. Partial streams keep unspecified indexes where the observed history cannot establish a position. + +| Input | Output | +|-------|--------| +| **Add** | Individual Add, preserving the supplied index or unspecified-index marker. | +| **Update** | Remove of the previous value followed by Add of the current value; known key positions identify the removal. | +| **Remove** | Individual Remove using the supplied or known key position. | +| **Refresh** | Self-Replace using the known key position, or unspecified indexes if unknown. | +| **Moved** | Moved with the supplied positions. | +| **OnError / OnCompleted** | Forwards the terminal notification. | + ### EnsureUniqueKeys Validates that all keys in each changeset are unique. Throws if duplicates detected. diff --git a/.github/instructions/dynamicdata-list.instructions.md b/.github/instructions/dynamicdata-list.instructions.md index 1f969b3ff..c976af7df 100644 --- a/.github/instructions/dynamicdata-list.instructions.md +++ b/.github/instructions/dynamicdata-list.instructions.md @@ -529,6 +529,8 @@ cache.Connect() .RemoveKey() // IChangeSet → IChangeSet ``` +`RemoveKey()` tracks known positions by cache key before discarding keys from the output. Equal-valued entries retain separate positions; partial streams preserve unspecified indexes where positions cannot be inferred. Updates remain Remove/Add pairs and refreshes remain self-Replaces. + --- ## Writing a New List Operator diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.Compatibility.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.Compatibility.cs new file mode 100644 index 000000000..0128c2f4f --- /dev/null +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.Compatibility.cs @@ -0,0 +1,532 @@ +using System; +using System.Reactive.Subjects; + +using Xunit; + +using DynamicData.Tests.Utilities; + +namespace DynamicData.Tests.Cache; + +public partial class RemoveKeyFixture +{ + /// + /// Preview is a hot stream without an initial snapshot. Unobserved refreshes, updates, and removals + /// retain the historical projection and unknown indexes rather than requiring complete cache history. + /// + [Theory] + [InlineData(ChangeReason.Refresh)] + [InlineData(ChangeReason.Update)] + [InlineData(ChangeReason.Remove)] + public void UnknownKeyChanges_KeepUnspecifiedIndexes(ChangeReason reason) + { + // Arrange: subscribe after the original item was added, so its list position is unknown. + using var source = new TestSourceCache(static item => item.Key); + var previous = CreateEqualItem(_identityRandomizer.Int()); + var current = new EqualItem(previous.Key, ~previous.EqualityValue, isIncluded: true); + source.AddOrUpdate(previous); + + // Partial deltas cannot be materialized or validated against an initially empty list. + using var subscription = source.Preview() + .RemoveKey() + .ValidateSynchronization() + .RecordValues(out var results); + + // Act: deliver a genuine cache operation without its initial addition. + switch (reason) + { + case ChangeReason.Refresh: + source.Refresh(new[] { previous }); + break; + + case ChangeReason.Update: + source.AddOrUpdate(current); + break; + + case ChangeReason.Remove: + source.RemoveKey(previous.Key); + break; + } + + // Assert: project the original reasons, payloads, and unknown indexes without an error. + Assert.Null(results.Error); + var changes = Assert.Single(results.RecordedValues); + switch (reason) + { + case ChangeReason.Refresh: + var refresh = Assert.Single(changes); + Assert.Equal(ListChangeReason.Replace, refresh.Reason); + Assert.Equal(-1, refresh.Item.CurrentIndex); + Assert.Equal(-1, refresh.Item.PreviousIndex); + Assert.Same(previous, refresh.Item.Current); + Assert.Same(previous, refresh.Item.Previous.Value); + break; + + case ChangeReason.Update: + Assert.Collection(changes, + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(previous, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(current, change.Item.Current); + }); + break; + + case ChangeReason.Remove: + var removal = Assert.Single(changes); + Assert.Equal(ListChangeReason.Remove, removal.Reason); + Assert.Equal(-1, removal.Item.CurrentIndex); + Assert.Same(previous, removal.Item.Current); + break; + } + + // Act: complete the source after the partial operation. + source.Complete(); + + // Assert: the operation did not terminate the subscription prematurely. + Assert.Null(results.Error); + Assert.True(results.HasCompleted); + } + + /// + /// Supplied indexes can refer to unobserved slots and must pass through without indexing a shorter local list. + /// Refresh metadata retains its legacy special case: an untracked key still produces an unindexed self-replacement. + /// + [Theory] + [InlineData(ChangeReason.Add, true, false)] + [InlineData(ChangeReason.Update, true, true)] + [InlineData(ChangeReason.Update, true, false)] + [InlineData(ChangeReason.Update, false, true)] + [InlineData(ChangeReason.Remove, true, false)] + [InlineData(ChangeReason.Moved, true, true)] + [InlineData(ChangeReason.Refresh, true, false)] + public void UntrackedIndexedChanges_PreserveSuppliedMetadata(ChangeReason reason, bool supplyCurrentIndex, bool supplyPreviousIndex) + { + // Arrange: indexes deliberately exceed the subscription's empty observed history. + using var source = new Subject>(); + var previous = CreateEqualItem(_identityRandomizer.Int()); + var current = reason is ChangeReason.Update + ? new EqualItem(previous.Key, ~previous.EqualityValue, isIncluded: true) + : previous; + var previousIndex = supplyPreviousIndex ? _identityRandomizer.Int(3, 30) : -1; + var currentIndex = supplyCurrentIndex ? _identityRandomizer.Int(31, 60) : -1; + var input = reason switch + { + ChangeReason.Update => new Change(reason, current.Key, current, previous, currentIndex, previousIndex), + ChangeReason.Moved => new Change(current.Key, current, currentIndex, previousIndex), + _ => new Change(reason, current.Key, current, currentIndex) + }; + + using var subscription = source + .RemoveKey() + .ValidateSynchronization() + .RecordValues(out var results); + + // Act: deliver one indexed change without an initial snapshot. + source.OnNext(new ChangeSet { input }); + + // Assert: no range restriction is imposed by the local tracking collection. + Assert.Null(results.Error); + var changes = Assert.Single(results.RecordedValues); + if (reason is ChangeReason.Update) + { + Assert.Collection(changes, + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(previousIndex, change.Item.CurrentIndex); + Assert.Same(previous, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(currentIndex, change.Item.CurrentIndex); + Assert.Same(current, change.Item.Current); + }); + } + else + { + var change = Assert.Single(changes); + var expectedReason = reason switch + { + ChangeReason.Add => ListChangeReason.Add, + ChangeReason.Remove => ListChangeReason.Remove, + ChangeReason.Moved => ListChangeReason.Moved, + _ => ListChangeReason.Replace + }; + Assert.Equal(expectedReason, change.Reason); + Assert.Equal(reason is ChangeReason.Refresh ? -1 : currentIndex, change.Item.CurrentIndex); + Assert.Equal(reason is ChangeReason.Moved ? previousIndex : -1, change.Item.PreviousIndex); + Assert.Same(current, change.Item.Current); + if (reason is ChangeReason.Refresh) + Assert.Same(current, change.Item.Previous.Value); + } + + if (reason is ChangeReason.Add or ChangeReason.Update or ChangeReason.Moved) + { + // Act: refresh the resulting key without supplying a new position. + source.OnNext(new ChangeSet + { + new(ChangeReason.Refresh, current.Key, current) + }); + + // Assert: reuse a supplied destination across an unobserved gap; an unspecified destination stays unknown. + Assert.Null(results.Error); + var refresh = Assert.Single(results.RecordedValues[^1]); + Assert.Equal(ListChangeReason.Replace, refresh.Reason); + Assert.Equal(currentIndex, refresh.Item.CurrentIndex); + Assert.Equal(currentIndex, refresh.Item.PreviousIndex); + Assert.Same(current, refresh.Item.Current); + Assert.Same(current, refresh.Item.Previous.Value); + } + } + + /// + /// An unknown refresh is nonstructural: it must not discard other keys' known positions. + /// Its missing history does prevent guessing the destination of a later unindexed append. + /// + [Fact] + public void UnknownRefresh_PreservesKnownPositionsWithoutGuessingTheAppendIndex() + { + // Arrange: two observed keys and an unobserved key all contain equal values. + using var source = new Subject>(); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + var unknown = CreateEqualItem(equalityValue); + var appended = CreateEqualItem(equalityValue); + + using var subscription = source + .RemoveKey() + .ValidateSynchronization() + .RecordValues(out var results); + source.OnNext(new ChangeSet + { + new(ChangeReason.Add, first.Key, first), + new(ChangeReason.Add, second.Key, second) + }); + Assert.Null(results.Error); + + // Act: refresh the unknown key, then a known key, without changing any positions. + source.OnNext(new ChangeSet + { + new(ChangeReason.Refresh, unknown.Key, unknown), + new(ChangeReason.Refresh, second.Key, second) + }); + + // Assert: only the genuinely untracked key retains unknown indexes. + Assert.Null(results.Error); + Assert.Collection(results.RecordedValues[^1], + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Equal(-1, change.Item.PreviousIndex); + Assert.Same(unknown, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(1, change.Item.CurrentIndex); + Assert.Equal(1, change.Item.PreviousIndex); + Assert.Same(second, change.Item.Current); + }); + + // Act: append without an index, after discovering that some source contents were never observed. + source.OnNext(new ChangeSet + { + new(ChangeReason.Add, appended.Key, appended), + new(ChangeReason.Refresh, appended.Key, appended), + new(ChangeReason.Refresh, second.Key, second) + }); + + // Assert: do not fabricate an end position, and do not lose the previously known second slot. + Assert.Null(results.Error); + Assert.Collection(results.RecordedValues[^1], + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(appended, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(appended, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(1, change.Item.CurrentIndex); + Assert.Same(second, change.Item.Current); + }); + } + + /// + /// An unknown removal position cannot safely shift other tracked positions. Preserve projection, discard + /// uncertain inference, and allow later supplied indexes to establish positions again. + /// + [Theory] + [InlineData(ChangeReason.Remove)] + [InlineData(ChangeReason.Update)] + public void UnknownStructuralChanges_InvalidateUncertainPositionsAndAllowIndexedRecovery(ChangeReason reason) + { + // Arrange: an equal but untracked key can have occupied an unknown position before the known key. + using var source = new Subject>(); + var equalityValue = _identityRandomizer.Int(); + var known = CreateEqualItem(equalityValue); + var unknown = CreateEqualItem(equalityValue); + var replacement = new EqualItem(unknown.Key, ~equalityValue, isIncluded: true); + var recovered = CreateEqualItem(equalityValue); + var recoveredIndex = _identityRandomizer.Int(3, 30); + + using var subscription = source + .RemoveKey() + .ValidateSynchronization() + .RecordValues(out var results); + source.OnNext(new ChangeSet { new(ChangeReason.Add, known.Key, known) }); + Assert.Null(results.Error); + + // Act: change an untracked key without supplying its previous position. + source.OnNext(new ChangeSet + { + reason is ChangeReason.Update + ? new Change(reason, unknown.Key, replacement, unknown) + : new Change(reason, unknown.Key, unknown) + }); + + // Assert: the partial operation still projects its exact changes with unspecified positions. + Assert.Null(results.Error); + if (reason is ChangeReason.Update) + { + Assert.Collection(results.RecordedValues[^1], + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(unknown, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(replacement, change.Item.Current); + }); + } + else + { + var removal = Assert.Single(results.RecordedValues[^1]); + Assert.Equal(ListChangeReason.Remove, removal.Reason); + Assert.Equal(-1, removal.Item.CurrentIndex); + Assert.Same(unknown, removal.Item.Current); + } + + // Act: refresh the formerly known key, then provide an explicit position for a new key. + source.OnNext(new ChangeSet + { + new(ChangeReason.Refresh, known.Key, known), + new(ChangeReason.Add, recovered.Key, recovered, recoveredIndex), + new(ChangeReason.Refresh, recovered.Key, recovered) + }); + + // Assert: uncertain positions remain unknown, while the supplied position can be reused safely. + Assert.Null(results.Error); + Assert.Collection(results.RecordedValues[^1], + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(known, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(recoveredIndex, change.Item.CurrentIndex); + Assert.Same(recovered, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(recoveredIndex, change.Item.CurrentIndex); + Assert.Equal(recoveredIndex, change.Item.PreviousIndex); + Assert.Same(recovered, change.Item.Current); + }); + } + + /// + /// A supplied removal index in an unobserved gap shifts only known positions after that index. + /// Tracking must neither allocate the gap nor remove an equal-valued known neighbor instead. + /// + [Fact] + public void UnknownIndexedRemoval_ShiftsKnownSparsePositions() + { + // Arrange: observe two indexed additions with an unobserved gap between them. + using var source = new Subject>(); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + var unknown = CreateEqualItem(equalityValue); + var firstIndex = _identityRandomizer.Int(3, 30); + var removalIndex = firstIndex + _identityRandomizer.Int(1, 30); + var secondIndex = removalIndex + _identityRandomizer.Int(1, 30); + + using var subscription = source + .RemoveKey() + .ValidateSynchronization() + .RecordValues(out var results); + source.OnNext(new ChangeSet + { + new(ChangeReason.Add, first.Key, first, firstIndex), + new(ChangeReason.Add, second.Key, second, secondIndex) + }); + Assert.Null(results.Error); + + // Act: remove an unobserved key from the gap, then refresh both known keys. + source.OnNext(new ChangeSet + { + new(ChangeReason.Remove, unknown.Key, unknown, removalIndex), + new(ChangeReason.Refresh, first.Key, first), + new(ChangeReason.Refresh, second.Key, second) + }); + + // Assert: preserve the supplied removal and adjust only the subsequent known position. + Assert.Null(results.Error); + Assert.Collection(results.RecordedValues[^1], + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(removalIndex, change.Item.CurrentIndex); + Assert.Same(unknown, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(firstIndex, change.Item.CurrentIndex); + Assert.Equal(firstIndex, change.Item.PreviousIndex); + Assert.Same(first, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(secondIndex - 1, change.Item.CurrentIndex); + Assert.Equal(secondIndex - 1, change.Item.PreviousIndex); + Assert.Same(second, change.Item.Current); + }); + } + + /// + /// A supplied index that contradicts observed history remains authoritative for projection. + /// Uncertain inferred positions must be discarded rather than used to address another equal item. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ConflictingSuppliedIndexes_DoNotInventSubsequentPositions(bool removeKnownKey) + { + // Arrange: inferred positions are inconsistent with a later supplied removal position. + using var source = new Subject>(); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + var removed = removeKnownKey ? second : CreateEqualItem(equalityValue); + + using var subscription = source + .RemoveKey() + .ValidateSynchronization() + .RecordValues(out var results); + source.OnNext(new ChangeSet + { + new(ChangeReason.Add, first.Key, first), + new(ChangeReason.Add, second.Key, second) + }); + Assert.Null(results.Error); + + // Act: pass through the conflicting supplied index, then refresh an untouched key. + source.OnNext(new ChangeSet + { + new(ChangeReason.Remove, removed.Key, removed, 0), + new(ChangeReason.Refresh, first.Key, first) + }); + + // Assert: preserve the original removal metadata without pretending the other key's position is still known. + Assert.Null(results.Error); + Assert.Collection(results.RecordedValues[^1], + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(0, change.Item.CurrentIndex); + Assert.Same(removed, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Equal(-1, change.Item.PreviousIndex); + Assert.Same(first, change.Item.Current); + }); + } + + /// + /// Projection accepts boundary index metadata without allocating missing list contents. + /// An inferred position beyond the index type's range must become unknown instead of wrapping. + /// + [Fact] + public void BoundaryIndexMetadata_DoesNotAllocateGapsOrWrapTrackedPositions() + { + // Arrange: values and keys are generated; int.MaxValue exercises the positional metadata boundary. + using var source = new Subject>(); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + + using var subscription = source + .RemoveKey() + .ValidateSynchronization() + .RecordValues(out var results); + + // Act: inserting before the furthest representable position makes that older position unrepresentable. + source.OnNext(new ChangeSet + { + new(ChangeReason.Add, first.Key, first, int.MaxValue), + new(ChangeReason.Add, second.Key, second, 0), + new(ChangeReason.Refresh, first.Key, first), + new(ChangeReason.Refresh, second.Key, second) + }); + + // Assert: original indexes pass through and only still-representable known positions are inferred. + Assert.Null(results.Error); + Assert.Collection(Assert.Single(results.RecordedValues), + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(int.MaxValue, change.Item.CurrentIndex); + Assert.Same(first, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(0, change.Item.CurrentIndex); + Assert.Same(second, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Equal(-1, change.Item.PreviousIndex); + Assert.Same(first, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(0, change.Item.CurrentIndex); + Assert.Equal(0, change.Item.PreviousIndex); + Assert.Same(second, change.Item.Current); + }); + } +} diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.Identity.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.Identity.cs new file mode 100644 index 000000000..c63dc6c44 --- /dev/null +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.Identity.cs @@ -0,0 +1,566 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reactive.Subjects; + +using Bogus; +using Xunit; + +using DynamicData.Tests.Utilities; + +namespace DynamicData.Tests.Cache; + +public partial class RemoveKeyFixture +{ + private const int IdentitySeed = 0x1165; + + private readonly Randomizer _identityRandomizer = new(IdentitySeed); + + /// + /// Refreshing one cache key must re-filter its own list slot, including when an earlier value compares equal. + /// The contract is the same for an initial snapshot and for additions after subscription. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public void EqualValuesAreRefreshed_FilterRemovesTheChangedKey(bool itemsExistBeforeSubscription) + { + // Arrange: equal values with distinct keys and different inclusion states. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue, isIncluded: false); + var second = CreateEqualItem(equalityValue); + var items = new[] { first, second }; + + Assert.NotEqual(first.Key, second.Key); + Assert.NotSame(first, second); + Assert.Equal(first, second); + + if (itemsExistBeforeSubscription) + source.AddOrUpdate(items); + + using var subscription = source.Connect() + .RemoveKey() + .Filter(static item => item.IsIncluded) + .ValidateSynchronization() + .ValidateChangeSets() + .ToCollection() + .RecordValues(out var results); + + if (!itemsExistBeforeSubscription) + source.AddOrUpdate(items); + + // Assert the baseline before refreshing the included key. + Assert.Null(results.Error); + Assert.NotEmpty(results.RecordedValues); + Assert.Same(second, Assert.Single(results.RecordedValues[^1])); + + // Act: only the second key is refreshed, and neither item now matches. + second.IsIncluded = false; + source.Refresh(new[] { second }); + + // Assert: no stale equal item survives in the materialized collection. + Assert.All(source.Items, static item => Assert.False(item.IsIncluded)); + Assert.Null(results.Error); + Assert.Empty(results.RecordedValues[^1]); + Assert.False(results.HasCompleted); + } + + /// + /// Two keys containing the identical reference must still retain separate inclusion states. + /// Refreshing both keys must remove both list occurrences, not repeatedly re-filter the first slot. + /// + [Fact] + public void SharedReferenceIsRefreshed_FilterTracksEachKeySeparately() + { + // Arrange: a transform retains two source keys while projecting the same object for both. + using var source = new TestSourceCache, Guid>(static entry => entry.Key); + var shared = CreateEqualItem(_identityRandomizer.Int()); + var first = new KeyValuePair(_identityRandomizer.Guid(), shared); + var second = new KeyValuePair(_identityRandomizer.Guid(), shared); + Assert.NotEqual(first.Key, second.Key); + + using var subscription = source.Connect() + .Transform(static entry => entry.Value) + .RemoveKey() + .Filter(static item => item.IsIncluded) + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var results); + + source.AddOrUpdate(new[] { first, second }); + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(shared, item), + item => Assert.Same(shared, item)); + + // Act: both keyed refreshes are delivered in one changeset. + shared.IsIncluded = false; + source.Refresh(new[] { first, second }); + + // Assert: both occurrences were independently removed. + Assert.Null(results.Error); + Assert.Empty(results.RecordedItems); + } + + /// + /// Value-type equality must not replace cache-key identity when refresh re-evaluates a predicate. + /// + [Fact] + public void EqualValueTypesAreRefreshed_FilterRemovesTheChangedKey() + { + // Arrange: the predicate's state can change without replacing either immutable struct. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var first = new EqualValue(_identityRandomizer.Guid(), equalityValue); + var second = new EqualValue(_identityRandomizer.Guid(), equalityValue); + var includedKeys = new HashSet { second.Key }; + + Assert.NotEqual(first.Key, second.Key); + Assert.Equal(first, second); + + using var subscription = source.Connect() + .RemoveKey() + .Filter(item => includedKeys.Contains(item.Key)) + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var results); + + source.AddOrUpdate(new[] { first, second }); + Assert.Null(results.Error); + Assert.Equal(second.Key, Assert.Single(results.RecordedItems).Key); + + // Act: the previously included key stops matching. + includedKeys.Remove(second.Key); + source.Refresh(new[] { second }); + + // Assert: the earlier equal struct did not absorb the second key's refresh. + Assert.Null(results.Error); + Assert.Empty(results.RecordedItems); + } + + /// + /// Unindexed cache refreshes retain the public self-replacement reason but identify their exact list slot. + /// Tracking must preserve the original individual additions and their unspecified append indexes. + /// + [Fact] + public void RefreshWithoutSourceIndex_IdentifiesTheChangedKey() + { + // Arrange: both values compare equal, so only their keys identify the refreshed position. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + + using var subscription = source.Connect() + .RemoveKey() + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var results); + + source.AddOrUpdate(new[] { first, second }); + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + Assert.Collection(results.RecordedChangeSets[0], + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(first, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(second, change.Item.Current); + }); + + // Act: refresh the second key without source index metadata. + source.Refresh(new[] { second }); + + // Assert: the original Replace reason is preserved, with both indexes set to the second slot. + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + var replacement = Assert.Single(results.RecordedChangeSets[^1]); + Assert.Equal(ListChangeReason.Replace, replacement.Reason); + Assert.Equal(1, replacement.Item.CurrentIndex); + Assert.Equal(1, replacement.Item.PreviousIndex); + Assert.True(replacement.Item.Previous.HasValue); + Assert.Same(second, replacement.Item.Previous.Value); + Assert.Same(second, replacement.Item.Current); + } + + /// + /// A true update removes the previous value by key and appends the replacement when no index is supplied. + /// The public remove/add reasons and the untouched equal item's identity must be preserved. + /// + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void EqualValuesAreUpdated_OnlyTheChangedKeyIsReplaced(bool replacementEqualsPrevious, bool updateFirstItem) + { + // Arrange: exercise both a same-position update and an update that appends after the other key. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + var previous = updateFirstItem ? first : second; + var unchanged = updateFirstItem ? second : first; + var replacement = new EqualItem( + previous.Key, + replacementEqualsPrevious ? equalityValue : ~equalityValue, + isIncluded: true); + var withoutKeys = source.Connect().RemoveKey(); + + using var rawSubscription = withoutKeys + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var rawResults); + using var subscription = withoutKeys + .Filter(static item => item.IsIncluded) + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var results); + + source.AddOrUpdate(new[] { first, second }); + Assert.Null(rawResults.Error); + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + + // Act: replace one key with a different instance, optionally unequal to its previous value. + source.AddOrUpdate(replacement); + + // Assert: preserve remove/add semantics and the actual previous and current instances. + Assert.Null(rawResults.Error); + Assert.Collection(rawResults.RecordedChangeSets[^1], + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(updateFirstItem ? 0 : 1, change.Item.CurrentIndex); + Assert.Same(previous, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(replacement, change.Item.Current); + }); + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(unchanged, item), + item => Assert.Same(replacement, item)); + + // Act: refresh the replacement at its new position after it stops matching. + replacement.IsIncluded = false; + source.Refresh(new[] { replacement }); + + // Assert: only the untouched equal item's reference remains. + Assert.Null(rawResults.Error); + Assert.Null(results.Error); + Assert.Same(unchanged, Assert.Single(results.RecordedItems)); + } + + /// + /// Removing a cache key removes only its list occurrence; re-addition and a batched clear retain exact membership. + /// + [Fact] + public void EqualValuesAreRemoved_OnlyTheChangedKeyIsRemoved() + { + // Arrange: both equal values are initially included. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + + using var subscription = source.Connect() + .RemoveKey() + .Filter(static item => item.IsIncluded) + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var results); + + source.AddOrUpdate(new[] { first, second }); + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + + // Act: remove only the second key. + source.RemoveKey(second.Key); + + // Assert: equal-value lookup must not remove the first reference instead. + Assert.Null(results.Error); + Assert.Same(first, Assert.Single(results.RecordedItems)); + + // Act: re-add the removed key. + source.AddOrUpdate(second); + + // Assert: its old position did not leave behind a duplicate or stale entry. + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + + // Act: remove both keys in one changeset. + source.Clear(); + + // Assert: both positions were removed despite their indexes shifting within the batch. + Assert.Null(results.Error); + Assert.Empty(results.RecordedItems); + } + + /// + /// Supplied indexed updates and movements must keep key tracking aligned for later unindexed refreshes and removals. + /// + [Fact] + public void IndexedMovesAndUpdates_KeepEqualValuesAtTheirKeyedPositions() + { + // Arrange: sorting supplies indexes for two values that otherwise compare equal. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var items = new[] { CreateEqualItem(equalityValue), CreateEqualItem(equalityValue) } + .OrderBy(static item => item.Key) + .ToArray(); + var first = items[0]; + var second = items[1]; + var replacement = new EqualItem(second.Key, equalityValue, isIncluded: true); + using var comparers = new BehaviorSubject>( + Comparer.Create(static (left, right) => left.Key.CompareTo(right.Key))); + + using var subscription = source.Connect() + .Sort(comparers) + .RemoveKey() + .Filter(static item => item.IsIncluded) + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var results); + + source.AddOrUpdate(items); + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + + // Act: update the second key without changing its sorted position. + source.AddOrUpdate(replacement); + + // Assert: the new reference occupies that key's indexed slot. + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(replacement, item)); + + // Act: reverse the sort order, producing indexed movement. + comparers.OnNext(Comparer.Create(static (left, right) => right.Key.CompareTo(left.Key))); + + // Assert: both exact references follow the move. + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(replacement, item), + item => Assert.Same(first, item)); + + // Act: refresh the now-second slot after excluding it. + first.IsIncluded = false; + source.Refresh(new[] { first }); + + // Assert: a correct count is insufficient; the included key's reference must remain. + Assert.Null(results.Error); + Assert.Same(replacement, Assert.Single(results.RecordedItems)); + + // Act: remove the excluded key, then the included key. + source.RemoveKey(first.Key); + + // Assert: removing an excluded item does not disturb its equal included neighbor. + Assert.Null(results.Error); + Assert.Same(replacement, Assert.Single(results.RecordedItems)); + + // Act: remove the final included key after the prior removal shifted its source index. + source.RemoveKey(replacement.Key); + + // Assert: no item remains at a stale tracked position. + Assert.Null(results.Error); + Assert.Empty(results.RecordedItems); + } + + /// + /// Later operations in one changeset use the positions left by earlier removals, without changing operation reasons. + /// + [Fact] + public void BatchedKeyChanges_UsePositionsAfterEarlierChanges() + { + // Arrange: three equal values whose keys identify distinct positions. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + var third = CreateEqualItem(equalityValue); + var replacement = new EqualItem(third.Key, ~equalityValue, isIncluded: true); + + using var subscription = source.Connect() + .RemoveKey() + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var results); + + source.AddOrUpdate(new[] { first, second, third }); + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item), + item => Assert.Same(third, item)); + + // Act: remove the leading slot, refresh the next key, and update the final key in one edit. + source.Edit(updater => + { + updater.Remove(first); + updater.Refresh(second); + updater.AddOrUpdate(replacement); + }); + + // Assert: every emitted index describes the list immediately before that operation. + Assert.Null(results.Error); + Assert.Collection(results.RecordedItems, + item => Assert.Same(second, item), + item => Assert.Same(replacement, item)); + Assert.Collection(results.RecordedChangeSets[^1], + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(0, change.Item.CurrentIndex); + Assert.Same(first, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Replace, change.Reason); + Assert.Equal(0, change.Item.CurrentIndex); + Assert.Equal(0, change.Item.PreviousIndex); + Assert.Same(second, change.Item.Previous.Value); + Assert.Same(second, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Remove, change.Reason); + Assert.Equal(1, change.Item.CurrentIndex); + Assert.Same(third, change.Item.Current); + }, + change => + { + Assert.Equal(ListChangeReason.Add, change.Reason); + Assert.Equal(-1, change.Item.CurrentIndex); + Assert.Same(replacement, change.Item.Current); + }); + + // Act: clear the remaining keys in one edit. + source.Clear(); + + // Assert: retain the original individual removals instead of coalescing them into Clear. + Assert.Null(results.Error); + Assert.Empty(results.RecordedItems); + Assert.Collection(results.RecordedChangeSets[^1], + change => Assert.Equal(ListChangeReason.Remove, change.Reason), + change => Assert.Equal(ListChangeReason.Remove, change.Reason)); + } + + /// + /// Each subscription to the same RemoveKey observable starts with its own key positions and disposes independently. + /// + [Fact] + public void SubscriptionsStartedAtDifferentTimes_TrackKeysIndependently() + { + // Arrange: the second subscription starts after the first has processed the initial additions. + using var source = new TestSourceCache(static item => item.Key); + var equalityValue = _identityRandomizer.Int(); + var first = CreateEqualItem(equalityValue); + var second = CreateEqualItem(equalityValue); + var withoutKeys = source.Connect().RemoveKey(); + + using var firstSubscription = withoutKeys + .Filter(static item => item.IsIncluded) + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var firstResults); + + source.AddOrUpdate(new[] { first, second }); + Assert.Null(firstResults.Error); + Assert.Collection(firstResults.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + + using var secondSubscription = withoutKeys + .Filter(static item => item.IsIncluded) + .ValidateSynchronization() + .ValidateChangeSets() + .RecordListItems(out var secondResults); + Assert.Null(secondResults.Error); + Assert.Collection(secondResults.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + + // Act: one source removal must affect both independently tracked subscriptions. + source.RemoveKey(second.Key); + + // Assert: neither subscription has absorbed the other's initial snapshot or removal. + Assert.Null(firstResults.Error); + Assert.Null(secondResults.Error); + Assert.Same(first, Assert.Single(firstResults.RecordedItems)); + Assert.Same(first, Assert.Single(secondResults.RecordedItems)); + + // Act: dispose one subscription, then re-add the key for the remaining subscriber. + firstSubscription.Dispose(); + source.AddOrUpdate(second); + + // Assert: the disposed observer remains unchanged while the active one receives the addition. + Assert.Null(firstResults.Error); + Assert.Null(secondResults.Error); + Assert.Same(first, Assert.Single(firstResults.RecordedItems)); + Assert.Collection(secondResults.RecordedItems, + item => Assert.Same(first, item), + item => Assert.Same(second, item)); + } + + private EqualItem CreateEqualItem(int equalityValue, bool isIncluded = true) + => new(_identityRandomizer.Guid(), equalityValue, isIncluded); + + // Cache keys are deliberately excluded from value equality. Equal values must still occupy distinct list slots. + private sealed class EqualItem : IEquatable + { + public EqualItem(Guid key, int equalityValue, bool isIncluded) + { + Key = key; + EqualityValue = equalityValue; + IsIncluded = isIncluded; + } + + public Guid Key { get; } + + public int EqualityValue { get; } + + public bool IsIncluded { get; set; } + + public bool Equals(EqualItem? other) + => other is not null && EqualityValue == other.EqualityValue; + + public override bool Equals(object? obj) + => obj is EqualItem other && Equals(other); + + public override int GetHashCode() + => EqualityValue; + } + + private readonly record struct EqualValue(Guid Key, int EqualityValue) + { + public bool Equals(EqualValue other) + => EqualityValue == other.EqualityValue; + + public override int GetHashCode() + => EqualityValue; + } +} diff --git a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs index 04d27f619..e2602b669 100644 --- a/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs +++ b/src/DynamicData.Tests/Cache/RemoveKeyFixture.cs @@ -1,4 +1,4 @@ -#region +#region using System; using System.Collections.Generic; @@ -12,12 +12,13 @@ using FluentAssertions; using Xunit; +using Xunit.Abstractions; #endregion namespace DynamicData.Tests.Cache; -public class RemoveKeyFixture : IDisposable +public partial class RemoveKeyFixture : IDisposable { private readonly RandomPersonGenerator _generator = new(); @@ -26,8 +27,9 @@ public class RemoveKeyFixture : IDisposable private readonly CompositeDisposable _cleanup = new(); - public RemoveKeyFixture() + public RemoveKeyFixture(ITestOutputHelper output) { + output.WriteLine($"Bogus seed: {IdentitySeed}"); _source = new SourceCache(p => p.Key); _cleanup.Add(_source); } @@ -47,7 +49,7 @@ public void CacheRemoveKey_Add_KeyIsRemoved() var people = _generator.Take(100).ToArray(); _source.AddOrUpdate(people); - Assert.Equivalent(people, collection); + Assert.Equivalent(people, collection, strict: true); } [Fact] @@ -66,7 +68,7 @@ public void CacheRemoveKey_Filter_ItemsFilterKeyIsRemoved() ); _source.AddOrUpdate(people); - Assert.Equivalent(people.Where(x => x.Age < average), collection); + Assert.Equivalent(people.Where(x => x.Age < average), collection, strict: true); } [Fact] @@ -83,13 +85,13 @@ public void CacheRemoveKey_AutoRefreshUpdateITems_CollectionUpdated() var people = _generator.Take(100).ToArray(); _source.AddOrUpdate(people); - Assert.Equivalent(people, collection); + Assert.Equivalent(people, collection, strict: true); foreach (var person in people) { person.Age = person.Age + 1; } - Assert.Equivalent(people, collection); + Assert.Equivalent(people, collection, strict: true); } } diff --git a/src/DynamicData/Cache/ObservableCacheEx.RemoveKey.cs b/src/DynamicData/Cache/ObservableCacheEx.RemoveKey.cs index 6e9f3ecd6..f7ed0c08b 100644 --- a/src/DynamicData/Cache/ObservableCacheEx.RemoveKey.cs +++ b/src/DynamicData/Cache/ObservableCacheEx.RemoveKey.cs @@ -15,6 +15,7 @@ using DynamicData.Binding; using DynamicData.Cache; using DynamicData.Cache.Internal; +using DynamicData.Kernel; // ReSharper disable once CheckNamespace @@ -27,12 +28,20 @@ public static partial class ObservableCacheEx { /// /// Strips the key from a cache changeset, converting to - /// (list changeset). All indexed changes are dropped (sorting is not supported). + /// (list changeset). Cache keys are tracked to supply known list indexes, + /// keeping entries with equal values at distinct positions. /// /// The type of the object. /// The type of the key. /// The source to strip keys from, producing an unkeyed list changeset. /// A list changeset stream without key information. + /// + /// Supplied addition, update, removal, and move indexes are preserved. Additions retain unspecified indexes when supplied, + /// including the addition produced by an update; their append positions are tracked internally when known. + /// Updates produce a removal followed by an addition; refreshes produce a replacement of the item with itself. + /// Partial streams retain unspecified indexes where positions cannot be inferred. An unindexed removal or update + /// of an untracked key invalidates inferred positions; later indexed changes can establish known positions again. + /// /// /// public static IObservable> RemoveKey(this IObservable> source) @@ -41,11 +50,178 @@ public static IObservable> RemoveKey(this IOb { source.ThrowArgumentNullExceptionIfNull(nameof(source)); - return source.Select( - changes => + return Observable.Defer( + () => { - var enumerator = new RemoveKeyEnumerator(changes); - return new ChangeSet(enumerator); + // Store only observed positions, not placeholders for unobserved items in a partial stream. + // Mirror known positions by key so refresh lookup does not scan the ordered list. + var keys = new List>(); + var indexesByKey = new Dictionary(); + var canInferAppendIndex = true; + + return source.Select( + changes => + { + // Preserve individual change reasons rather than coalescing them into ranges or Clear. + var result = new ChangeSet(changes.Count + changes.Updates); + + foreach (var change in changes.ToConcreteType()) + { + switch (change.Reason) + { + case ChangeReason.Add: + { + InsertKey(change.Key, change.CurrentIndex); + result.Add(new Change(ListChangeReason.Add, change.Current, change.CurrentIndex)); + } + + break; + + case ChangeReason.Refresh: + { + // Cache refresh indexes are not positional. Preserve the legacy unknown-index + // self-replacement when this subscription has not observed the key's position. + var index = FindIndex(change.Key); + if (index < 0) + { + canInferAppendIndex = false; + } + + result.Add(new Change(ListChangeReason.Replace, change.Current, change.Current, index, index)); + } + + break; + + case ChangeReason.Moved: + RemoveKeyPosition(change.Key, change.PreviousIndex); + InsertKey(change.Key, change.CurrentIndex); + result.Add(new Change(change.Current, change.CurrentIndex, change.PreviousIndex)); + break; + + case ChangeReason.Update: + { + var previousIndex = RemoveKeyPosition(change.Key, change.PreviousIndex); + result.Add(new Change(ListChangeReason.Remove, change.Previous.Value, previousIndex)); + + InsertKey(change.Key, change.CurrentIndex); + result.Add(new Change(ListChangeReason.Add, change.Current, change.CurrentIndex)); + } + + break; + + case ChangeReason.Remove: + { + var index = RemoveKeyPosition(change.Key, change.CurrentIndex); + result.Add(new Change(ListChangeReason.Remove, change.Current, index)); + } + + break; + } + } + + return result; + }); + + int FindIndex(TKey key) + => indexesByKey.TryGetValue(key, out var index) ? index : -1; + + void InsertKey(TKey key, int suppliedIndex) + { + var index = suppliedIndex >= 0 ? suppliedIndex : canInferAppendIndex ? keys.Count : suppliedIndex; + if (index < 0) + { + // An append after an incomplete history has no known absolute position. + return; + } + + if (index > keys.Count) + { + canInferAppendIndex = false; + } + + var slot = keys.Count; + while (slot > 0 && keys[slot - 1].Index >= index) + { + --slot; + var item = keys[slot]; + if (item.Index == int.MaxValue) + { + // The shifted position is not representable; do not invent a wrapped index. + keys.RemoveAt(slot); + indexesByKey.Remove(item.Item); + canInferAppendIndex = false; + } + else + { + var shiftedIndex = item.Index + 1; + keys[slot] = new ItemWithIndex(item.Item, shiftedIndex); + indexesByKey[item.Item] = shiftedIndex; + } + } + + keys.Insert(slot, new ItemWithIndex(key, index)); + indexesByKey[key] = index; + } + + int RemoveKeyPosition(TKey key, int suppliedIndex) + { + // Supplied positions in a complete stream can be read directly. + var knownIndex = canInferAppendIndex + && suppliedIndex >= 0 + && suppliedIndex < keys.Count + && EqualityComparer.Default.Equals(keys[suppliedIndex].Item, key) + ? suppliedIndex + : FindIndex(key); + var index = suppliedIndex >= 0 ? suppliedIndex : knownIndex >= 0 ? knownIndex : suppliedIndex; + + if (knownIndex < 0) + { + canInferAppendIndex = false; + } + + if (index < 0 || (knownIndex >= 0 && index != knownIndex)) + { + // An unknown removal location, or a conflicting supplied index, makes subsequent + // inferred positions unsafe. Keep projecting the supplied change instead of throwing. + keys.Clear(); + indexesByKey.Clear(); + canInferAppendIndex = false; + return index; + } + + for (var slot = keys.Count - 1; slot >= 0; --slot) + { + var item = keys[slot]; + if (item.Index < index) + { + break; + } + + if (item.Index == index) + { + if (knownIndex >= 0) + { + keys.RemoveAt(slot); + indexesByKey.Remove(item.Item); + } + else + { + // The supplied removal occupies another key's inferred position. + keys.Clear(); + indexesByKey.Clear(); + canInferAppendIndex = false; + } + + break; + } + + var shiftedIndex = item.Index - 1; + keys[slot] = new ItemWithIndex(item.Item, shiftedIndex); + indexesByKey[item.Item] = shiftedIndex; + } + + return index; + } }); }