Index Safety for List Tranform Operator (#762)

* Added safety check to the Transfomer operator and associated unit tests

* PR Feedback

* Handle null and missing index in a single check

* Align List/Cache versions

* Subtle syntax improvement

* Rev the version

* Add file to remove coverage details from PR diff

---------

Co-authored-by: Chris Pulman <chris.pulman@yahoo.com>

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 @@ private void Transform(ChangeAwareList<TransformedItemContainer> transformed, IC
                 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 @@ private void Transform(ChangeAwareList<TransformedItemContainer> transformed, IC
                 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

@@ -1543,19 +1543,7 @@ public static IObservable<IChangeSet<T>> SubscribeMany<T>(this IObservable<IChan
     /// <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
@@ -2300,6 +2288,16 @@ public static IObservable<IChangeSet<T>> WhereReasonsAreNot<T>(this IObservable<
             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 =>

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