WAF: SynchronizingList fix add elements to internal mapping for two-w…

…ay sync
jbe2277 · Oct 3, 2023 · cb473b1 · cb473b1
1 parent 2ce9311
commit cb473b1
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 14 deletions.
diff --git a/src/System.Waf/System.Waf/System.Waf.Core.Test/Foundation/SynchronizingListTest.cs b/src/System.Waf/System.Waf/System.Waf.Core.Test/Foundation/SynchronizingListTest.cs
@@ -119,11 +119,13 @@ public void SynchronizeWithGetOriginalItemTest()
                 Assert.AreEqual(3, e.NewStartingIndex);
                 Assert.AreEqual(originalList.Last(), e.NewItems!.Cast<MyDataModel>().Single().Model);
             };
-            AssertCollectionChangeEventsCalled(handler, () => originalList.Add(new MyModel()));
-            originalList.Remove(originalList.Last());
             var dataModelToAdd = new MyDataModel(new MyModel());
             AssertCollectionChangeEventsCalled(handler, () => synchronizingList.Add(dataModelToAdd));
             Assert.AreSame(dataModelToAdd, synchronizingList.Last());
+            Assert.AreEqual(3, originalList.IndexOf(dataModelToAdd.Model));
+            originalList.Remove(originalList.Last());
+            Assert.AreEqual(-1, originalList.IndexOf(dataModelToAdd.Model));
+            AssertCollectionChangeEventsCalled(handler, () => originalList.Add(new MyModel()));
 
             // Check insert at index 0 operation with collection changed event.
             handler = (sender, e) =>
@@ -132,11 +134,13 @@ public void SynchronizeWithGetOriginalItemTest()
                 Assert.AreEqual(0, e.NewStartingIndex);
                 Assert.AreEqual(originalList[0], e.NewItems!.Cast<MyDataModel>().Single().Model);
             };
-            AssertCollectionChangeEventsCalled(handler, () => originalList.Insert(0, new MyModel()));
-            originalList.RemoveAt(0);
             var dataModelToInsert = new MyDataModel(new MyModel());
             AssertCollectionChangeEventsCalled(handler, () => synchronizingList.Insert(0, dataModelToInsert));
             Assert.AreSame(dataModelToInsert, synchronizingList[0]);
+            Assert.AreEqual(0, originalList.IndexOf(dataModelToInsert.Model));
+            originalList.RemoveAt(0);
+            Assert.AreEqual(-1, originalList.IndexOf(dataModelToInsert.Model));
+            AssertCollectionChangeEventsCalled(handler, () => originalList.Insert(0, new MyModel()));
 
             // Check replace operation with collection changed event.
             handler = (sender, e) =>
@@ -145,10 +149,12 @@ public void SynchronizeWithGetOriginalItemTest()
                 Assert.AreEqual(0, e.NewStartingIndex);
                 Assert.AreEqual(originalList[0], e.NewItems!.Cast<MyDataModel>().Single().Model);
             };
-            AssertCollectionChangeEventsCalled(handler, () => originalList[0] = new MyModel());
             var dataModelToReplace = new MyDataModel(new MyModel());
             AssertCollectionChangeEventsCalled(handler, () => synchronizingList[0] = dataModelToReplace);
             Assert.AreSame(dataModelToReplace, synchronizingList[0]);
+            Assert.AreEqual(0, originalList.IndexOf(dataModelToReplace.Model));
+            AssertCollectionChangeEventsCalled(handler, () => originalList[0] = new MyModel());
+            Assert.AreEqual(-1, originalList.IndexOf(dataModelToReplace.Model));
 
             // Compare the collections
             AssertHelper.SequenceEqual(originalList, synchronizingList.Select(dm => dm.Model));

diff --git a/src/System.Waf/System.Waf/System.Waf.Core/Foundation/SynchronizingList.cs b/src/System.Waf/System.Waf/System.Waf.Core/Foundation/SynchronizingList.cs
@@ -1,7 +1,6 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
-using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 
 namespace System.Waf.Foundation
@@ -127,7 +126,7 @@ protected override void InsertItem(int index, T item)
             if (!innerChange)
             {
                 outerChange = true;
-                try { ModifyOriginalList().Insert(index, GetOriginalItem(item)); }
+                try { ModifyOriginalList().Insert(index, GetOriginalItemToAdd(item)); }
                 finally { outerChange = false; }
             }
             base.InsertItem(index, item);
@@ -140,7 +139,7 @@ protected override void SetItem(int index, T item)
             if (!innerChange)
             {
                 outerChange = true;
-                try { ModifyOriginalList()[index] = GetOriginalItem(item); }
+                try { ModifyOriginalList()[index] = GetOriginalItemToAdd(item); }
                 finally { outerChange = false; }
             }
             base.SetItem(index, item);
@@ -173,20 +172,22 @@ private ObservableCollection<TOriginal> ModifyOriginalList()
             return originalList!;
         }
 
-        private TOriginal GetOriginalItem(T item)
+        private TOriginal GetOriginalItemToAdd(T item)
         {
             if (getOriginalItem is null) throw new NotSupportedException("Operation is not supported because getOriginalItem parameter was not set.");
-            return getOriginalItem(item);
+            var originalItem = getOriginalItem(item);
+            mapping.Add((originalItem, item));
+            return originalItem;
         }
 
-        private T CreateItem([AllowNull] TOriginal oldItem)
+        private T CreateItem(TOriginal originalItem)
         {
-            T newItem = factory(oldItem!);
-            mapping.Add((oldItem!, newItem));
+            T newItem = factory(originalItem);
+            mapping.Add((originalItem, newItem));
             return newItem;
         }
 
-        private void RemoveCore([AllowNull] TOriginal oldItem)
+        private void RemoveCore(TOriginal oldItem)
         {
             var tuple = mapping.First(t => originalItemComparer.Equals(t.original, oldItem!));
             mapping.Remove(tuple);