Raise Replace events for collection notifications

CHANGELOG.md

@@ -46,6 +46,7 @@
 
 ### Enhancements
 * Added nullability annotations to the Realm assembly. Now methods returning reference types are correctly annotated to indicate whether the returned value may or may not be null. (Issue [#3248](https://github.com/realm/realm-dotnet/issues/3248))
+* Replacing a value at an index (i.e. `myList[1] = someObj`) will now correctly `CollectionChange` notifications with the `Replace` action. (Issue [#2854](https://github.com/realm/realm-dotnet/issues/2854))
 
 ### Fixed
 

Realm/Realm/DatabaseTypes/RealmCollectionBase.cs

@@ -311,28 +311,37 @@ private void OnChange(IRealmCollection<T> sender, ChangeSet? change)
 
                 var raiseAdded = TryGetConsecutive(change.InsertedIndices, i => this[i], out var addedItems, out var addedStartIndex);
 
-                if (raiseAdded || raiseRemoved)
-                {
-                    if ((raiseAdded && raiseRemoved) ||
-                        (raiseAdded && addedItems == null) ||
-                        (raiseRemoved && removedItems == null))
-                    {
-                        RaiseCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
-                        RaisePropertyChanged();
-                        return;
-                    }
+                var raiseReplaced = TryGetConsecutive(change.NewModifiedIndices, i => this[i], out var replacedItems, out var replacedStartIndex);
 
+                // Only raise specialized notifications if we have exactly one change type to report
+                if (raiseAdded + raiseReplaced + raiseRemoved == 1)
+                {
                     if (removedItems != null)
                     {
                         RaiseCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removedItems, removedStartIndex));
                         RaisePropertyChanged();
                     }
-
-                    if (addedItems != null)
+                    else if (addedItems != null)
                     {
                         RaiseCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, addedItems, addedStartIndex));
                         RaisePropertyChanged();
                     }
+                    else if (replacedItems != null)
+                    {
+                        // Until we get a snapshot of the old collection, we won't be able to provide meaningful value for old items.
+                        var oldItems = Enumerable.Range(0, replacedItems.Count).Select(_ => InvalidObject.Instance).ToList();
+                        RaiseCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, replacedItems, oldItems, replacedStartIndex));
+                        RaisePropertyChanged();
+                    }
+                    else
+                    {
+                        Debug.Assert(false, "This should never happen");
+                    }
+                }
+                else
+                {
+                    RaiseCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
+                    RaisePropertyChanged();
                 }
             }
         }
@@ -348,7 +357,7 @@ protected void RaisePropertyChanged()
             _propertyChanged?.Invoke(this, new PropertyChangedEventArgs("Item[]"));
         }
 
-        private static bool TryGetConsecutive(int[] indices, Func<int, object?> getter, out IList? items, out int startIndex)
+        private static int TryGetConsecutive(int[] indices, Func<int, object?> getter, out IList? items, out int startIndex)
         {
             items = null;
 
@@ -360,13 +369,13 @@ private static bool TryGetConsecutive(int[] indices, Func<int, object?> getter,
                     items = Enumerable.Range(startIndex, indices.Length)
                                       .Select(getter)
                                       .ToList();
-                }
 
-                return true;
+                    return 1;
+                }
             }
 
             startIndex = -1;
-            return false;
+            return 0;
         }
 
         [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "This is called when subscribing to events and the dispose token is retained by the collection.")]

Tests/Realm.Tests/Database/NotificationTests.cs

@@ -878,6 +878,50 @@ public void ListMove_SingleMovedItemTests(int oldIndex, int newIndex)
             Assert.That(args.NewItems, Is.EquivalentTo(new[] { movedObject }));
         }
 
+        [Test]
+        public void ListReplace_RaisesReplaceNotifications()
+        {
+            var container = new OrderedContainer();
+            for (var i = 0; i < 5; i++)
+            {
+                container.Items.Add(new OrderedObject
+                {
+                    Order = i
+                });
+            }
+
+            _realm.Write(() => _realm.Add(container));
+
+            var eventArgs = new List<NotifyCollectionChangedEventArgs>();
+            var propertyEventArgs = new List<string>();
+
+            var collection = container.Items.AsRealmCollection();
+            collection.CollectionChanged += (sender, e) => eventArgs.Add(e);
+            collection.PropertyChanged += (sender, e) => propertyEventArgs.Add(e.PropertyName);
+
+            var oldItem = container.Items[1];
+            _realm.Write(() => container.Items[1] = container.Items[4]);
+
+            _realm.Refresh();
+
+            Assert.That(eventArgs.Count, Is.EqualTo(1));
+            Assert.That(eventArgs[0].Action, Is.EqualTo(NotifyCollectionChangedAction.Replace));
+            Assert.That(eventArgs[0].OldStartingIndex, Is.EqualTo(1));
+            Assert.That(eventArgs[0].NewStartingIndex, Is.EqualTo(1));
+            Assert.That(eventArgs[0].OldItems, Is.EquivalentTo(new[] { InvalidObject.Instance }));
+            Assert.That(eventArgs[0].NewItems, Is.EquivalentTo(new[] { container.Items[4] }));
+            Assert.That(propertyEventArgs.Count, Is.EqualTo(2));
+            Assert.That(propertyEventArgs, Is.EquivalentTo(new[] { "Count", "Item[]" }));
+
+            // Verify that modifying an object doesn't raise notifications
+            _realm.Write(() => container.Items[2].Order = 999);
+            _realm.Refresh();
+
+            // No notifications should have arrived
+            Assert.That(eventArgs.Count, Is.EqualTo(1));
+            Assert.That(propertyEventArgs.Count, Is.EqualTo(2));
+        }
+
         // Adds 5 OrderedObject to a List, executes moveAction and returns the single change notification argument.
         private NotifyCollectionChangedEventArgs TestMoves(Action<IList<OrderedObject>> moveAction, NotifyCollectionChangedAction expectedAction)
         {