Skip to content

Commit

Permalink
Fixed premature evaluation of the collection within the Cache `TrueFo…
Browse files Browse the repository at this point in the history
…r` operators, causing premature and potentially incorrect emissions to occur, when items in the collection publish values immediately upon subscription. (#923)
  • Loading branch information
JakenVeina authored Jul 27, 2024
1 parent 8fd1124 commit 41a608d
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 13 deletions.
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ dotnet_diagnostic.SA1110.severity = error
dotnet_diagnostic.SA1111.severity = error
dotnet_diagnostic.SA1112.severity = error
dotnet_diagnostic.SA1113.severity = error
dotnet_diagnostic.SA1114.severity = error
dotnet_diagnostic.SA1114.severity = none
dotnet_diagnostic.SA1115.severity = error
dotnet_diagnostic.SA1116.severity = none
dotnet_diagnostic.SA1117.severity = error
Expand Down
22 changes: 22 additions & 0 deletions src/DynamicData.Tests/Cache/TrueForAnyFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Reactive.Linq;
using System.Reactive.Subjects;

using DynamicData.Tests.Utilities;

using FluentAssertions;

using Xunit;
Expand Down Expand Up @@ -77,6 +79,26 @@ public void MultipleValuesReturnTrue()
subscribed.Dispose();
}

// https://github.com/reactivemarbles/DynamicData/issues/922
[Fact]
public void ValuesPublishedOnSubscriptionDoNotTriggerPrematureOutput()
{
var item1 = new ObjectWithObservable(1);
var item2 = new ObjectWithObservable(2);

item2.InvokeObservable(true);

_source.AddOrUpdate(item1);
_source.AddOrUpdate(item2);

using var subscription = _observable
.ValidateSynchronization()
.RecordValues(out var results);

results.RecordedValues.Count.Should().Be(1, because: "No items were added to the source, and no value changes were made to the items");
results.RecordedValues[0].Should().Be(true, because: "One of the two items in the source has a true value");
}

private class ObjectWithObservable(int id)
{
private readonly ISubject<bool> _changed = new Subject<bool>();
Expand Down
34 changes: 22 additions & 12 deletions src/DynamicData/Cache/Internal/TrueFor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,26 @@ internal sealed class TrueFor<TObject, TKey, TValue>(IObservable<IChangeSet<TObj

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

public IObservable<bool> Run() => Observable.Create<bool>(
observer =>
{
var transformed = _source.Transform(t => new ObservableWithValue<TObject, TValue>(t, _observableSelector(t))).Publish();
var inlineChanges = transformed.MergeMany(t => t.Observable);
var queried = transformed.ToCollection();

// nb: we do not care about the inline change because we are only monitoring it to cause a re-evaluation of all items
var publisher = queried.CombineLatest(inlineChanges, (items, _) => _collectionMatcher(items)).DistinctUntilChanged().SubscribeSafe(observer);

return new CompositeDisposable(publisher, transformed.Connect());
});
public IObservable<bool> Run()
=> Observable.Create<bool>(observer =>
{
var itemsWithValues = _source
.Transform(item => new ObservableWithValue<TObject, TValue>(
item: item,
source: _observableSelector.Invoke(item)))
.Publish();

var subscription = Observable.CombineLatest(
// 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.
first: itemsWithValues.MergeMany(item => item.Observable),
second: itemsWithValues.ToCollection(),
// We don't need to actually look at the changed values, we just need them as a trigger to re-evaluate the matcher method.
resultSelector: (_, itemsWithValues) => _collectionMatcher.Invoke(itemsWithValues))
.DistinctUntilChanged()
.SubscribeSafe(observer);

return new CompositeDisposable(
subscription,
itemsWithValues.Connect());
});
}

0 comments on commit 41a608d

Please sign in to comment.