[Fix] SortedObservableCollectionAdaptor does Remove -> Add instead of…

… Replace when index not changed (#392) * Replace item if index not changed Change-Id: Ib7564fb941849b4a12b30d3b6bc4dd6ab8dabfc8 * Add unit tests Change-Id: Ib62b5916b63961e2b87b48f8d398ab37905d8497 Co-authored-by: Radu Petrisel <[email protected]>
reactivemarbles · Aug 25, 2020 · 6594137 · 6594137
1 parent 5d375e5
commit 6594137
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 2 deletions.
diff --git a/src/DynamicData.Tests/Binding/ObservableCollectionBindCacheSortedFixture.cs b/src/DynamicData.Tests/Binding/ObservableCollectionBindCacheSortedFixture.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Linq;
+using System.Reactive.Linq;
 using DynamicData.Binding;
 using DynamicData.Tests.Domain;
 using FluentAssertions;
@@ -174,5 +175,61 @@ public void TreatMovesAsRemoveAdd()
 			    latestSetWithMoves.Adds.Should().Be(0);
 		    }
 	    }
+
+        [Fact]
+        public void UpdateToSourceSendsReplaceIfSortingIsNotAffected()
+        {
+            var person1 = new Person("Adult1", 10);
+            var person2 = new Person("Adult2", 11);
+
+            NotifyCollectionChangedAction action = default;
+            _source.AddOrUpdate(person1);
+            _source.AddOrUpdate(person2);
+
+            var person2Updated = new Person("Adult2", 12);
+
+            using (_collection
+                .ObserveCollectionChanges()
+                .Select(change => change.EventArgs.Action)
+                .Subscribe(act => action = act))
+            {
+                _source.AddOrUpdate(person2Updated);
+            }
+
+            action.Should().Be(NotifyCollectionChangedAction.Replace, "The notification type should be Replace");
+            _collection.Should().Equal(person1, person2Updated);
+        }
+
+        [Fact]
+        public void UpdateToSourceSendsRemoveAndAddIfSortingIsAffected()
+        {
+            var person1 = new Person("Adult1", 10);
+            var person2 = new Person("Adult2", 11);
+            var person2Updated = new Person("Adult2", 1);
+
+            var actions = new List<NotifyCollectionChangedAction>();
+            var collection = new ObservableCollectionExtended<Person>();
+
+            using (var source = new SourceCache<Person, string>(person => person.Name))
+            using (source.Connect()
+                .Sort(SortExpressionComparer<Person>.Ascending(person => person.Age))
+                .Bind(collection)
+                .Subscribe())
+            {
+                source.AddOrUpdate(person1);
+                source.AddOrUpdate(person2);
+
+                using (collection
+                    .ObserveCollectionChanges()
+                    .Select(change => change.EventArgs.Action)
+                    .Subscribe(act => actions.Add(act)))
+                {
+                    source.AddOrUpdate(person2Updated);
+                }
+            }
+
+            actions.Should().Equal(NotifyCollectionChangedAction.Remove, NotifyCollectionChangedAction.Add);
+            collection.Should().Equal(person2Updated, person1);
+        }
     }
 }
diff --git a/src/DynamicData/Binding/SortedObservableCollectionAdaptor.cs b/src/DynamicData/Binding/SortedObservableCollectionAdaptor.cs
@@ -103,8 +103,16 @@ private void DoUpdate(ISortedChangeSet<TObject, TKey> updates, IObservableCollec
                         list.Move(update.PreviousIndex, update.CurrentIndex);
                         break;
                     case ChangeReason.Update:
-                        list.RemoveAt(update.PreviousIndex);
-                        list.Insert(update.CurrentIndex, update.Current);
+                        if (update.PreviousIndex != update.CurrentIndex)
+                        {
+                            list.RemoveAt(update.PreviousIndex);
+                            list.Insert(update.CurrentIndex, update.Current);
+                        }
+                        else
+                        {
+                            list.Replace(update.Previous.Value, update.Current);
+                        }
+
                         break;
                 }
             }