Skip to content

Commit 41a608d

Browse files
authored
Fixed premature evaluation of the collection within the Cache TrueFor operators, causing premature and potentially incorrect emissions to occur, when items in the collection publish values immediately upon subscription. (#923)
1 parent 8fd1124 commit 41a608d

3 files changed

Lines changed: 45 additions & 13 deletions

File tree

.editorconfig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ dotnet_diagnostic.SA1110.severity = error
345345
dotnet_diagnostic.SA1111.severity = error
346346
dotnet_diagnostic.SA1112.severity = error
347347
dotnet_diagnostic.SA1113.severity = error
348-
dotnet_diagnostic.SA1114.severity = error
348+
dotnet_diagnostic.SA1114.severity = none
349349
dotnet_diagnostic.SA1115.severity = error
350350
dotnet_diagnostic.SA1116.severity = none
351351
dotnet_diagnostic.SA1117.severity = error

src/DynamicData.Tests/Cache/TrueForAnyFixture.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
using System.Reactive.Linq;
33
using System.Reactive.Subjects;
44

5+
using DynamicData.Tests.Utilities;
6+
57
using FluentAssertions;
68

79
using Xunit;
@@ -77,6 +79,26 @@ public void MultipleValuesReturnTrue()
7779
subscribed.Dispose();
7880
}
7981

82+
// https://github.com/reactivemarbles/DynamicData/issues/922
83+
[Fact]
84+
public void ValuesPublishedOnSubscriptionDoNotTriggerPrematureOutput()
85+
{
86+
var item1 = new ObjectWithObservable(1);
87+
var item2 = new ObjectWithObservable(2);
88+
89+
item2.InvokeObservable(true);
90+
91+
_source.AddOrUpdate(item1);
92+
_source.AddOrUpdate(item2);
93+
94+
using var subscription = _observable
95+
.ValidateSynchronization()
96+
.RecordValues(out var results);
97+
98+
results.RecordedValues.Count.Should().Be(1, because: "No items were added to the source, and no value changes were made to the items");
99+
results.RecordedValues[0].Should().Be(true, because: "One of the two items in the source has a true value");
100+
}
101+
80102
private class ObjectWithObservable(int id)
81103
{
82104
private readonly ISubject<bool> _changed = new Subject<bool>();

src/DynamicData/Cache/Internal/TrueFor.cs

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,26 @@ internal sealed class TrueFor<TObject, TKey, TValue>(IObservable<IChangeSet<TObj
1818

1919
private readonly IObservable<IChangeSet<TObject, TKey>> _source = source ?? throw new ArgumentNullException(nameof(source));
2020

21-
public IObservable<bool> Run() => Observable.Create<bool>(
22-
observer =>
23-
{
24-
var transformed = _source.Transform(t => new ObservableWithValue<TObject, TValue>(t, _observableSelector(t))).Publish();
25-
var inlineChanges = transformed.MergeMany(t => t.Observable);
26-
var queried = transformed.ToCollection();
27-
28-
// nb: we do not care about the inline change because we are only monitoring it to cause a re-evaluation of all items
29-
var publisher = queried.CombineLatest(inlineChanges, (items, _) => _collectionMatcher(items)).DistinctUntilChanged().SubscribeSafe(observer);
30-
31-
return new CompositeDisposable(publisher, transformed.Connect());
32-
});
21+
public IObservable<bool> Run()
22+
=> Observable.Create<bool>(observer =>
23+
{
24+
var itemsWithValues = _source
25+
.Transform(item => new ObservableWithValue<TObject, TValue>(
26+
item: item,
27+
source: _observableSelector.Invoke(item)))
28+
.Publish();
29+
30+
var subscription = Observable.CombineLatest(
31+
// Make sure we subscribe to ALL of the items before we make the first evaluation of the collection, so any values published on-subscription don't trigger a re-evaluation of the matcher method.
32+
first: itemsWithValues.MergeMany(item => item.Observable),
33+
second: itemsWithValues.ToCollection(),
34+
// We don't need to actually look at the changed values, we just need them as a trigger to re-evaluate the matcher method.
35+
resultSelector: (_, itemsWithValues) => _collectionMatcher.Invoke(itemsWithValues))
36+
.DistinctUntilChanged()
37+
.SubscribeSafe(observer);
38+
39+
return new CompositeDisposable(
40+
subscription,
41+
itemsWithValues.Connect());
42+
});
3343
}

0 commit comments

Comments
 (0)