Index Safety for List Tranform Operator

codecov.yaml

@@ -0,0 +1,3 @@
+coverage:
+  status:           #Code coverage status will be posted to pull requests based on targets defined below.
+    comments: off   #Optional. Details are off by default.

src/DynamicData.Tests/List/TransformFixture.cs

@@ -4,7 +4,6 @@
 using DynamicData.Tests.Domain;
 
 using FluentAssertions;
-
 using Xunit;
 
 namespace DynamicData.Tests.List;
@@ -186,4 +185,40 @@ public void TransformOnRefresh()
 
         results.Messages.Last().First().Reason.Should().Be(ListChangeReason.Replace);
     }
+
+    [Fact]
+    public void TransformOnReplace()
+    {
+        // Arrange
+        var person1 = new Person("Bob", 37);
+        var person2 = new Person("Bob", 38);
+
+        _source.Add(person1);
+
+        // Act
+        _source.Replace(person1, person2);
+
+        // Assert
+        _results.Messages.Count.Should().Be(2, "Should be 2 messages");
+        _results.Data.Count.Should().Be(1, "Should be 1 item in the cache");
+        _results.Data.Items.First().Should().Be(_transformFactory(person2), "Should be same person");
+    }
+
+    [Fact]
+    public void TransformOnReplaceWithoutIndex()
+    {
+        // Arrange
+        var person1 = new Person("Bob", 37);
+        var person2 = new Person("Bob", 38);
+        var results = _source.Connect().RemoveIndex().Transform(_transformFactory).AsAggregator();
+        _source.Add(person1);
+
+        // Act
+        _source.Replace(person1, person2);
+
+        // Assert
+        results.Messages.Count.Should().Be(2, "Should be 2 messages");
+        results.Data.Count.Should().Be(1, "Should be 1 item in the cache");
+        results.Data.Items.First().Should().Be(_transformFactory(person2), "Should be same person");
+    }
 }

src/DynamicData/List/Internal/Transformer.cs

@@ -82,14 +82,26 @@
                 case ListChangeReason.Refresh:
                     {
                         var change = item.Item;
+                        var index = change.CurrentIndex;
+                        if (index < 0)
+                        {
+                            // Find the corresponding index
+                            var current = transformed.FirstOrDefault(x => x.Source.Equals(change.Current));
+                            index = current switch
+                            {
+                                TransformedItemContainer tic when transformed.IndexOf(tic) is int i and (>= 0) => i,
+                                _ => throw new UnspecifiedIndexException($"Cannot find index of {change.Current}"),
+                            };
+                        }
+
                         if (_transformOnRefresh)
                         {
-                            Optional<TDestination> previous = transformed[change.CurrentIndex].Destination;
-                            transformed[change.CurrentIndex] = _containerFactory(change.Current, previous, change.CurrentIndex);
+                            Optional<TDestination> previous = transformed[index].Destination;
+                            transformed[index] = _containerFactory(change.Current, previous, index);
                         }
                         else
                         {
-                            transformed.RefreshAt(change.CurrentIndex);
+                            transformed.RefreshAt(index);
                         }
 
                         break;
@@ -98,15 +110,31 @@
                 case ListChangeReason.Replace:
                     {
                         var change = item.Item;
-                        Optional<TDestination> previous = transformed[change.PreviousIndex].Destination;
-                        if (change.CurrentIndex == change.PreviousIndex)
+
+                        if (change.CurrentIndex == -1 || change.PreviousIndex == -1)
                         {
-                            transformed[change.CurrentIndex] = _containerFactory(change.Current, previous, change.CurrentIndex);
+                            // Find the original, with it's corresponding index
+                            var previous = transformed.First(x => x.Source.Equals(change.Previous.Value));
+                            var index = transformed.IndexOf(previous);
+                            if (index == -1)
+                            {
+                                throw new UnspecifiedIndexException($"Cannot find index of {change.Previous.Value}");
+                            }
+
+                            transformed[index] = _containerFactory(change.Current, previous.Destination, index);
                         }
                         else
                         {
-                            transformed.RemoveAt(change.PreviousIndex);
-                            transformed.Insert(change.CurrentIndex, _containerFactory(change.Current, Optional<TDestination>.None, change.CurrentIndex));
+                            Optional<TDestination> previous = transformed[change.PreviousIndex].Destination;
+                            if (change.CurrentIndex == change.PreviousIndex)
+                            {
+                                transformed[change.CurrentIndex] = _containerFactory(change.Current, previous, change.CurrentIndex);
+                            }
+                            else
+                            {
+                                transformed.RemoveAt(change.PreviousIndex);
+                                transformed.Insert(change.CurrentIndex, _containerFactory(change.Current, Optional<TDestination>.None, change.CurrentIndex));
+                            }
                         }
 
                         break;

src/DynamicData/List/ObservableListEx.cs

@@ -129,7 +129,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> And<T>(this IObservableList<IObservableList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.And);
 
     /// <summary>
     /// Dynamically apply a logical And operator between the items in the outer observable list.
@@ -139,7 +139,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> And<T>(this IObservableList<ISourceList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.And);
 
     /// <summary>
     /// Converts the source list to an read only observable list.
@@ -604,7 +604,7 @@
         where T : notnull
     {
         if (source is null)
             throw new ArgumentNullException(nameof(source));
 
         return new DisposeMany<T>(source).Run();
     }
@@ -676,7 +676,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> Except<T>(this IObservableList<IObservableList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.Except);
 
     /// <summary>
     /// Dynamically apply a logical Except operator. Items from the first observable list are included when an equivalent item does not exist in the other sources.
@@ -685,7 +685,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> Except<T>(this IObservableList<ISourceList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.Except);
 
     /// <summary>
     /// Removes items from the cache according to the value specified by the time selector function.
@@ -1253,7 +1253,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> Or<T>(this IObservableList<IObservableList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.Or);
 
     /// <summary>
     /// Dynamically apply a logical Or operator between the items in the outer observable list.
@@ -1263,7 +1263,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> Or<T>(this IObservableList<ISourceList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.Or);
 
     /// <summary>
     /// Applies paging to the data source.
@@ -1505,7 +1505,7 @@
     /// <param name="source">The source observable of change set values.</param>
     /// <returns>An observable which emits a change set.</returns>
     public static IObservable<IChangeSet<T>> StartWithEmpty<T>(this IObservable<IChangeSet<T>> source)
         where T : notnull => source.StartWith(ChangeSet<T>.Empty);
 
     /// <summary>
     /// Subscribes to each item when it is added to the stream and unsubscribes when it is removed.  All items will be unsubscribed when the stream is disposed.
@@ -1543,19 +1543,7 @@
     /// <param name="source">The source observable change set.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> SuppressRefresh<T>(this IObservable<IChangeSet<T>> source)
-        where T : notnull
-    {
-        if (source == null)
-        {
-            throw new ArgumentNullException(nameof(source));
-        }
-
-        return source.Select(changes =>
-        {
-            var filtered = changes.Where(c => c.Reason != ListChangeReason.Refresh);
-            return new ChangeSet<T>(filtered);
-        });
-    }
+        where T : notnull => source.WhereReasonsAreNot(ListChangeReason.Refresh);
 
     /// <summary>
     /// Transforms an observable sequence of observable lists into a single sequence
@@ -1820,7 +1808,7 @@
     /// <param name="comparer">The sort comparer.</param>
     /// <returns>An observable which emits the read only collection.</returns>
     public static IObservable<IReadOnlyCollection<TObject>> ToSortedCollection<TObject>(this IObservable<IChangeSet<TObject>> source, IComparer<TObject> comparer)
         where TObject : notnull => source.QueryWhenChanged(
             query =>
             {
                 var items = query.AsList();
@@ -2300,6 +2288,16 @@
             throw new ArgumentException("Must enter at least 1 reason", nameof(reasons));
         }
 
+        if (reasons.Length == 1 && reasons[0] == ListChangeReason.Refresh)
+        {
+            // If only refresh changes are removed, then there's no need to remove the indexes
+            return source.Select(changes =>
+            {
+                var filtered = changes.Where(c => c.Reason != ListChangeReason.Refresh);
+                return new ChangeSet<T>(filtered);
+            }).NotEmpty();
+        }
+
         var matches = new HashSet<ListChangeReason>(reasons);
         return source.Select(
             updates =>
@@ -2348,7 +2346,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> Xor<T>(this IObservableList<IObservableList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.Xor);
 
     /// <summary>
     /// Dynamically apply a logical Xor operator between the items in the outer observable list.
@@ -2358,7 +2356,7 @@
     /// <param name="sources">The source.</param>
     /// <returns>An observable which emits the change set.</returns>
     public static IObservable<IChangeSet<T>> Xor<T>(this IObservableList<ISourceList<T>> sources)
         where T : notnull => sources.Combine(CombineOperator.Xor);
 
     private static IObservable<IChangeSet<T>> Combine<T>(this ICollection<IObservable<IChangeSet<T>>> sources, CombineOperator type)
         where T : notnull

version.json

@@ -1,5 +1,5 @@
 {
-    "version": "8.2",
+    "version": "8.3",
     "publicReleaseRefSpec": [
         "^refs/heads/main$", // we release out of master
         "^refs/heads/preview/.*", // we release previews