From 6770d44170252de4f1f2dd39f1aa606bb358f27a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98ystein=20Krog?= Date: Thu, 18 Aug 2022 16:24:10 +0200 Subject: [PATCH] feature: Change TransformAsync implementation to be more similar to normal Transform (#625) Add TransformOnRefresh option to List.TransformAsync --- .../List/TransformAsyncFixture.cs | 216 +++++++++++------- .../List/TransformFixture.cs | 37 +++ .../List/Internal/TransformAsync.cs | 157 ++++++------- src/DynamicData/List/Internal/Transformer.cs | 6 +- src/DynamicData/List/ObservableListEx.cs | 100 +++++++- 5 files changed, 338 insertions(+), 178 deletions(-) mode change 100644 => 100755 src/DynamicData.Tests/List/TransformAsyncFixture.cs diff --git a/src/DynamicData.Tests/List/TransformAsyncFixture.cs b/src/DynamicData.Tests/List/TransformAsyncFixture.cs old mode 100644 new mode 100755 index 87450cf4e..aae29e775 --- a/src/DynamicData.Tests/List/TransformAsyncFixture.cs +++ b/src/DynamicData.Tests/List/TransformAsyncFixture.cs @@ -1,7 +1,10 @@ using System; +using System.Linq; using System.Threading.Tasks; using DynamicData.Tests.Domain; +using FluentAssertions; +using Xunit; namespace DynamicData.Tests.List; @@ -35,90 +38,131 @@ public void Dispose() */ - //[Fact] - //public async Task Add() - //{ - // var person = new Person("Adult1", 50); - // _source.Add(person); - - // _results.Messages.Count.Should().Be(1, "Should be 1 updates"); - // _results.Data.Count.Should().Be(1, "Should be 1 item in the cache"); - - // var transformed = await _transformFactory(person); - // _results.Data.Items.First().Should().Be(transformed, "Should be same person"); - //} - - //[Fact] - //public void Remove() - //{ - // const string key = "Adult1"; - // var person = new Person(key, 50); - - // _source.Add(person); - // _source.Remove(person); - - // _results.Messages.Count.Should().Be(2, "Should be 2 updates"); - // _results.Messages.Count.Should().Be(2, "Should be 2 updates"); - // _results.Messages[0].Adds.Should().Be(1, "Should be 80 addes"); - // _results.Messages[1].Removes.Should().Be(1, "Should be 80 removes"); - // _results.Data.Count.Should().Be(0, "Should be nothing cached"); - //} - - //[Fact] - //public void Update() - //{ - // const string key = "Adult1"; - // var newperson = new Person(key, 50); - // var updated = new Person(key, 51); - - // _source.Add(newperson); - // _source.Add(updated); - - // _results.Messages.Count.Should().Be(2, "Should be 2 updates"); - // _results.Messages[0].Adds.Should().Be(1, "Should be 1 adds"); - // _results.Messages[0].Replaced.Should().Be(0, "Should be 1 update"); - //} - - //[Fact] - //public async Task BatchOfUniqueUpdates() - //{ - // var people = Enumerable.Range(1, 100).Select(i => new Person("Name" + i, i)).ToArray(); - - // _source.AddRange(people); - - // _results.Messages.Count.Should().Be(1, "Should be 1 updates"); - // _results.Messages[0].Adds.Should().Be(100, "Should return 100 adds"); - - // var tasks = people.Select(_transformFactory); - // var result = await Task.WhenAll(tasks); - - // var transformed = result.OrderBy(p => p.Age).ToArray(); - // _results.Data.Items.OrderBy(p => p.Age).Should().BeEquivalentTo(_results.Data.Items.OrderBy(p => p.Age), "Incorrect transform result"); - //} - - //[Fact] - //public void SameKeyChanges() - //{ - // var people = Enumerable.Range(1, 10).Select(i => new Person("Name", i)).ToArray(); - - // _source.AddRange(people); - - // _results.Messages.Count.Should().Be(1, "Should be 1 updates"); - // _results.Messages[0].Adds.Should().Be(10, "Should return 10 adds"); - // _results.Data.Count.Should().Be(10, "Should result in 10 records"); - //} - - //[Fact] - //public void Clear() - //{ - // var people = Enumerable.Range(1, 100).Select(l => new Person("Name" + l, l)).ToArray(); - - // _source.AddRange(people); - // _source.Clear(); - - // _results.Messages.Count.Should().Be(2, "Should be 2 updates"); - // _results.Messages[0].Adds.Should().Be(100, "Should be 80 addes"); - // _results.Messages[1].Removes.Should().Be(100, "Should be 80 removes"); - // _results.Data.Count.Should().Be(0, "Should be nothing cached"); - //} + [Fact] + public async Task Add() + { + var person = new Person("Adult1", 50); + _source.Add(person); + + _results.Messages.Count.Should().Be(1, "Should be 1 updates"); + _results.Data.Count.Should().Be(1, "Should be 1 item in the cache"); + + var transformed = await _transformFactory(person); + _results.Data.Items.First().Should().Be(transformed, "Should be same person"); + } + + [Fact] + public void Remove() + { + const string key = "Adult1"; + var person = new Person(key, 50); + + _source.Add(person); + _source.Remove(person); + + _results.Messages.Count.Should().Be(2, "Should be 2 updates"); + _results.Messages.Count.Should().Be(2, "Should be 2 updates"); + _results.Messages[0].Adds.Should().Be(1, "Should be 80 addes"); + _results.Messages[1].Removes.Should().Be(1, "Should be 80 removes"); + _results.Data.Count.Should().Be(0, "Should be nothing cached"); + } + + [Fact] + public void Update() + { + const string key = "Adult1"; + var newperson = new Person(key, 50); + var updated = new Person(key, 51); + + _source.Add(newperson); + _source.Add(updated); + + _results.Messages.Count.Should().Be(2, "Should be 2 updates"); + _results.Messages[0].Adds.Should().Be(1, "Should be 1 adds"); + _results.Messages[0].Replaced.Should().Be(0, "Should be 1 update"); + } + + [Fact] + public async Task BatchOfUniqueUpdates() + { + var people = Enumerable.Range(1, 100).Select(i => new Person("Name" + i, i)).ToArray(); + + _source.AddRange(people); + + _results.Messages.Count.Should().Be(1, "Should be 1 updates"); + _results.Messages[0].Adds.Should().Be(100, "Should return 100 adds"); + + var tasks = people.Select(_transformFactory); + var result = await Task.WhenAll(tasks); + + var transformed = result.OrderBy(p => p.Age).ToArray(); + _results.Data.Items.OrderBy(p => p.Age).Should().BeEquivalentTo(_results.Data.Items.OrderBy(p => p.Age), "Incorrect transform result"); + } + + [Fact] + public void SameKeyChanges() + { + var people = Enumerable.Range(1, 10).Select(i => new Person("Name", i)).ToArray(); + + _source.AddRange(people); + + _results.Messages.Count.Should().Be(1, "Should be 1 updates"); + _results.Messages[0].Adds.Should().Be(10, "Should return 10 adds"); + _results.Data.Count.Should().Be(10, "Should result in 10 records"); + } + + [Fact] + public void Clear() + { + var people = Enumerable.Range(1, 100).Select(l => new Person("Name" + l, l)).ToArray(); + + _source.AddRange(people); + _source.Clear(); + + _results.Messages.Count.Should().Be(2, "Should be 2 updates"); + _results.Messages[0].Adds.Should().Be(100, "Should be 80 addes"); + _results.Messages[1].Removes.Should().Be(100, "Should be 80 removes"); + _results.Data.Count.Should().Be(0, "Should be nothing cached"); + } + + /// + /// This test is disabled as it was flaky. + /// https://github.com/reactivemarbles/DynamicData/pull/625 + /// + [Fact] + private void TransformOnRefresh() + { + var items = Enumerable.Range(1, 100).Select(i => new Person("Person" + i, 1)).ToArray(); + + //result should only be true when all items are set to true + using var list = new SourceList(); + using var results = list.Connect().AutoRefresh(p => p.Age) + .TransformAsync(Task.FromResult, transformOnRefresh: true).AsAggregator(); + list.AddRange(items); + + results.Data.Count.Should().Be(100); + results.Messages.Count.Should().Be(1); + + items[0].Age = 10; + results.Data.Count.Should().Be(100); + results.Messages.Count.Should().Be(2); + + results.Messages[1].First().Reason.Should().Be(ListChangeReason.Replace); + + //remove an item and check no change is fired + var toRemove = items[1]; + list.Remove(toRemove); + results.Data.Count.Should().Be(99); + results.Messages.Count.Should().Be(3); + toRemove.Age = 100; + results.Messages.Count.Should().Be(3); + + //add it back in and check it updates + list.Add(toRemove); + results.Messages.Count.Should().Be(4); + toRemove.Age = 101; + results.Messages.Count.Should().Be(5); + + results.Messages.Last().First().Reason.Should().Be(ListChangeReason.Replace); + } } diff --git a/src/DynamicData.Tests/List/TransformFixture.cs b/src/DynamicData.Tests/List/TransformFixture.cs index e06fe7601..77be742b6 100644 --- a/src/DynamicData.Tests/List/TransformFixture.cs +++ b/src/DynamicData.Tests/List/TransformFixture.cs @@ -162,4 +162,41 @@ public void MultipleSubscribersShouldNotShareState() transformed.Transform(o => o).Subscribe(); transformed.Transform(o => o).Subscribe(); } + + [Fact] + public void TransformOnRefresh() + { + var items = Enumerable.Range(1, 100).Select(i => new Person("Person" + i, 1)).ToArray(); + + //result should only be true when all items are set to true + using var list = new SourceList(); + using var results = list.Connect().AutoRefresh(p => p.Age) + .Transform(v => v, transformOnRefresh: true).AsAggregator(); + list.AddRange(items); + + results.Data.Count.Should().Be(100); + results.Messages.Count.Should().Be(1); + + items[0].Age = 10; + results.Data.Count.Should().Be(100); + results.Messages.Count.Should().Be(2); + + results.Messages[1].First().Reason.Should().Be(ListChangeReason.Replace); + + //remove an item and check no change is fired + var toRemove = items[1]; + list.Remove(toRemove); + results.Data.Count.Should().Be(99); + results.Messages.Count.Should().Be(3); + toRemove.Age = 100; + results.Messages.Count.Should().Be(3); + + //add it back in and check it updates + list.Add(toRemove); + results.Messages.Count.Should().Be(4); + toRemove.Age = 101; + results.Messages.Count.Should().Be(5); + + results.Messages.Last().First().Reason.Should().Be(ListChangeReason.Replace); + } } diff --git a/src/DynamicData/List/Internal/TransformAsync.cs b/src/DynamicData/List/Internal/TransformAsync.cs index 685409221..3dadfa906 100644 --- a/src/DynamicData/List/Internal/TransformAsync.cs +++ b/src/DynamicData/List/Internal/TransformAsync.cs @@ -4,16 +4,20 @@ using System.Reactive.Linq; using System.Reactive.Threading.Tasks; +using DynamicData.Binding; +using DynamicData.Kernel; namespace DynamicData.List.Internal; internal class TransformAsync { - private readonly Func> _containerFactory; + private readonly Func, int, Task.TransformedItemContainer>> _containerFactory; private readonly IObservable> _source; + private readonly bool _transformOnRefresh; - public TransformAsync(IObservable> source, Func> factory) + public TransformAsync(IObservable> source, + Func, int, Task> factory, bool transformOnRefresh) { if (factory is null) { @@ -21,44 +25,45 @@ public TransformAsync(IObservable> source, Func + _transformOnRefresh = transformOnRefresh; + _containerFactory = async (item, prev, index) => { - var destination = await factory(item).ConfigureAwait(false); - return new TransformedItemContainer(item, destination); + var destination = await factory(item, prev, index).ConfigureAwait(false); + return new Transformer.TransformedItemContainer(item, destination); }; } - public IObservable> Run() + public IObservable> Run() => Observable.Defer(RunImpl); + + private IObservable> RunImpl() { - return Observable.Create>( - observer => - { - var state = new ChangeAwareList(); - var asyncLock = new SemaphoreSlim(1, 1); + var state = new ChangeAwareList.TransformedItemContainer>(); + var asyncLock = new SemaphoreSlim(1, 1); - return _source.Select( - async changes => - { - try - { - await asyncLock.WaitAsync().ConfigureAwait(false); - await Transform(state, changes).ConfigureAwait(false); - return state; - } - finally - { - asyncLock.Release(); - } - }).Select(tasks => tasks.ToObservable()).SelectMany(items => items).Select( - transformed => - { - var changed = transformed.CaptureChanges(); - return changed.Transform(container => container.Destination); - }).SubscribeSafe(observer); + return _source.Select( + async changes => + { + try + { + await asyncLock.WaitAsync().ConfigureAwait(false); + await Transform(state, changes).ConfigureAwait(false); + return state; + } + finally + { + asyncLock.Release(); + } + }).Select(tasks => tasks.ToObservable()).SelectMany(items => items).Select( + transformed => + { + var changed = transformed.CaptureChanges(); + return changed.Transform(container => container.Destination); }); } - private async Task Transform(ChangeAwareList transformed, IChangeSet changes) + private async Task Transform( + ChangeAwareList.TransformedItemContainer> transformed, + IChangeSet changes) { if (changes is null) { @@ -74,12 +79,16 @@ private async Task Transform(ChangeAwareList transform var change = item.Item; if (change.CurrentIndex < 0 | change.CurrentIndex >= transformed.Count) { - var container = await _containerFactory(item.Item.Current).ConfigureAwait(false); + var container = + await _containerFactory(item.Item.Current, Optional.None, + transformed.Count).ConfigureAwait(false); transformed.Add(container); } else { - var container = await _containerFactory(item.Item.Current).ConfigureAwait(false); + var container = + await _containerFactory(item.Item.Current, Optional.None, + change.CurrentIndex).ConfigureAwait(false); transformed.Insert(change.CurrentIndex, container); } @@ -88,25 +97,44 @@ private async Task Transform(ChangeAwareList transform case ListChangeReason.AddRange: { - var tasks = item.Range.Select(_containerFactory); + var startIndex = item.Range.Index < 0 ? transformed.Count : item.Range.Index; + var tasks = item.Range.Select((t, idx) => _containerFactory(t, Optional.None, idx + startIndex)); var containers = await Task.WhenAll(tasks).ConfigureAwait(false); transformed.AddOrInsertRange(containers, item.Range.Index); break; } + case ListChangeReason.Refresh: + { + var change = item.Item; + if (_transformOnRefresh) + { + Optional previous = transformed[change.CurrentIndex].Destination; + var container = await _containerFactory(change.Current, previous, change.CurrentIndex) + .ConfigureAwait(false); + transformed[change.CurrentIndex] = container; + } + else + { + transformed.RefreshAt(change.CurrentIndex); + } + + break; + } + case ListChangeReason.Replace: { var change = item.Item; - var container = await _containerFactory(item.Item.Current).ConfigureAwait(false); + Optional previous = transformed[change.PreviousIndex].Destination; if (change.CurrentIndex == change.PreviousIndex) { - transformed[change.CurrentIndex] = container; + transformed[change.CurrentIndex] = await _containerFactory(change.Current, previous, change.CurrentIndex); } else { transformed.RemoveAt(change.PreviousIndex); - transformed.Insert(change.CurrentIndex, container); + transformed.Insert(change.CurrentIndex, await _containerFactory(change.Current, Optional.None, change.CurrentIndex)); } break; @@ -152,7 +180,9 @@ private async Task Transform(ChangeAwareList transform case ListChangeReason.Clear: { // i.e. need to store transformed reference so we can correctly clear - var toClear = new Change(ListChangeReason.Clear, transformed); + var toClear = + new Change.TransformedItemContainer>( + ListChangeReason.Clear, transformed); transformed.ClearOrRemoveMany(toClear); break; @@ -167,7 +197,8 @@ private async Task Transform(ChangeAwareList transform throw new UnspecifiedIndexException("Cannot move as an index was not specified"); } - if (transformed is IExtendedList collection) + if (transformed is IExtendedList.TransformedItemContainer> + collection) { collection.Move(change.PreviousIndex, change.CurrentIndex); } @@ -183,52 +214,4 @@ private async Task Transform(ChangeAwareList transform } } } - - private class TransformedItemContainer : IEquatable - { - public TransformedItemContainer(TSource source, TDestination destination) - { - Source = source; - Destination = destination; - } - - public TDestination Destination { get; } - - public TSource Source { get; } - - public static bool operator ==(TransformedItemContainer left, TransformedItemContainer right) - { - return Equals(left, right); - } - - public static bool operator !=(TransformedItemContainer left, TransformedItemContainer right) - { - return !Equals(left, right); - } - - public bool Equals(TransformedItemContainer? other) - { - if (ReferenceEquals(null, other)) - { - return false; - } - - if (ReferenceEquals(this, other)) - { - return true; - } - - return EqualityComparer.Default.Equals(Source, other.Source); - } - - public override bool Equals(object? obj) - { - return obj is TransformedItemContainer item && Equals(item); - } - - public override int GetHashCode() - { - return Source is null ? 0 : EqualityComparer.Default.GetHashCode(Source); - } - } } diff --git a/src/DynamicData/List/Internal/Transformer.cs b/src/DynamicData/List/Internal/Transformer.cs index 0204060e0..7eac9d75a 100644 --- a/src/DynamicData/List/Internal/Transformer.cs +++ b/src/DynamicData/List/Internal/Transformer.cs @@ -82,15 +82,15 @@ private void Transform(ChangeAwareList transformed, IC case ListChangeReason.Refresh: { + var change = item.Item; if (_transformOnRefresh) { - var change = item.Item; Optional previous = transformed[change.CurrentIndex].Destination; transformed[change.CurrentIndex] = _containerFactory(change.Current, previous, change.CurrentIndex); } else { - transformed.RefreshAt(item.Item.CurrentIndex); + transformed.RefreshAt(change.CurrentIndex); } break; @@ -175,7 +175,7 @@ private void Transform(ChangeAwareList transformed, IC } } - private sealed class TransformedItemContainer : IEquatable + internal sealed class TransformedItemContainer : IEquatable { public TransformedItemContainer(TSource source, TDestination destination) { diff --git a/src/DynamicData/List/ObservableListEx.cs b/src/DynamicData/List/ObservableListEx.cs index b1e33ead6..d96544511 100644 --- a/src/DynamicData/List/ObservableListEx.cs +++ b/src/DynamicData/List/ObservableListEx.cs @@ -1909,13 +1909,109 @@ public static IObservable> TransformThe type of the destination. /// The source. /// The transform factory. + /// Should a new transform be applied when a refresh event is received. + /// A an observable change set of the transformed object. + /// + /// source + /// or + /// valueSelector. + /// + public static IObservable> TransformAsync( + this IObservable> source, Func> transformFactory, + bool transformOnRefresh = false) + { + if (source is null) + { + throw new ArgumentNullException(nameof(source)); + } + + if (transformFactory is null) + { + throw new ArgumentNullException(nameof(transformFactory)); + } + + return source.TransformAsync((t, _, _) => transformFactory(t), transformOnRefresh); + } + + /// + /// Projects each update item to a new form using the specified transform function. + /// + /// The type of the source. + /// The type of the destination. + /// The source. + /// The transform factory. + /// Should a new transform be applied when a refresh event is received. + /// A an observable change set of the transformed object. + /// + /// source + /// or + /// valueSelector. + /// + public static IObservable> TransformAsync( + this IObservable> source, Func> transformFactory, + bool transformOnRefresh = false) + { + if (source is null) + { + throw new ArgumentNullException(nameof(source)); + } + + if (transformFactory is null) + { + throw new ArgumentNullException(nameof(transformFactory)); + } + + return source.TransformAsync((t, _, i) => transformFactory(t, i), transformOnRefresh); + } + + /// + /// Projects each update item to a new form using the specified transform function. + /// + /// The type of the source. + /// The type of the destination. + /// The source. + /// The transform factory. + /// Should a new transform be applied when a refresh event is received. + /// A an observable change set of the transformed object. + /// + /// source + /// or + /// valueSelector. + /// + public static IObservable> TransformAsync( + this IObservable> source, Func, Task> transformFactory, + bool transformOnRefresh = false) + { + if (source is null) + { + throw new ArgumentNullException(nameof(source)); + } + + if (transformFactory is null) + { + throw new ArgumentNullException(nameof(transformFactory)); + } + + return source.TransformAsync((t, d, _) => transformFactory(t, d), transformOnRefresh); + } + + /// + /// Projects each update item to a new form using the specified transform function. + /// + /// The type of the source. + /// The type of the destination. + /// The source. + /// The transform factory. + /// Should a new transform be applied when a refresh event is received. /// A an observable change set of the transformed object. /// /// source /// or /// valueSelector. /// - public static IObservable> TransformAsync(this IObservable> source, Func> transformFactory) + public static IObservable> TransformAsync( + this IObservable> source, Func, int, Task> transformFactory, + bool transformOnRefresh = false) { if (source is null) { @@ -1927,7 +2023,7 @@ public static IObservable> TransformAsync(source, transformFactory).Run(); + return new TransformAsync(source, transformFactory, transformOnRefresh).Run(); } ///