Merge pull request #5954 from peppy/fix-bindable-list-fucked

Fix `BindableList` being unsafe for complex bind trees
ppy · Jul 31, 2023 · e72f4fe · e72f4fe
2 parents eb6a632 + c28aac1
commit e72f4fe
Show file tree

Hide file tree

Showing 2 changed files with 168 additions and 77 deletions.
diff --git a/osu.Framework.Tests/Bindables/BindableListTest.cs b/osu.Framework.Tests/Bindables/BindableListTest.cs
@@ -377,6 +377,26 @@ public void TestAddWithListContainingItemsDoesNotOverrideItems(string str)
             Assert.Contains(item, bindableStringList);
         }
 
+        [Test]
+        public void TestAddBranchingBinds()
+        {
+            var b1 = new BindableList<int> { 1, 2, 3, 4, 5 };
+
+            var b2 = b1.GetBoundCopy();
+            var b3 = b1.GetBoundCopy();
+
+            var b4 = new BindableList<int>();
+            b4.BindTo(b2);
+            b4.BindTo(b3);
+
+            b1.Add(6);
+
+            Assert.That(b1.Count, Is.EqualTo(6));
+            Assert.That(b2.Count, Is.EqualTo(6));
+            Assert.That(b3.Count, Is.EqualTo(6));
+            Assert.That(b4.Count, Is.EqualTo(6));
+        }
+
         #endregion
 
         #region .AddRange(items)
@@ -435,6 +455,26 @@ IEnumerable<int> valueEnumerable()
             Assert.That(counter, Is.EqualTo(1));
         }
 
+        [Test]
+        public void TestAddRangeBranchingBinds()
+        {
+            var b1 = new BindableList<int> { 1, 2, 3, 4, 5 };
+
+            var b2 = b1.GetBoundCopy();
+            var b3 = b1.GetBoundCopy();
+
+            var b4 = new BindableList<int>();
+            b4.BindTo(b2);
+            b4.BindTo(b3);
+
+            b1.AddRange(new[] { 6, 7 });
+
+            Assert.That(b1.Count, Is.EqualTo(7));
+            Assert.That(b2.Count, Is.EqualTo(7));
+            Assert.That(b3.Count, Is.EqualTo(7));
+            Assert.That(b4.Count, Is.EqualTo(7));
+        }
+
         #endregion
 
         #region .Move(item)
@@ -574,6 +614,28 @@ public void TestMoveNotifiesBoundListSubscriptions()
             Assert.That(triggeredArgsB2, Is.Not.Null);
         }
 
+        [Test]
+        public void TestMoveRangeBranchingBinds()
+        {
+            var b1 = new BindableList<int> { 1, 2, 3, 4, 5 };
+
+            var b2 = b1.GetBoundCopy();
+            var b3 = b1.GetBoundCopy();
+
+            var b4 = new BindableList<int>();
+
+            b4.BindTo(b2);
+            b4.BindTo(b3);
+
+            b1.Move(0, 1);
+
+            foreach (var list in new[] { b1, b2, b3, b4 })
+            {
+                Assert.That(list[0], Is.EqualTo(2));
+                Assert.That(list[1], Is.EqualTo(1));
+            }
+        }
+
         #endregion
 
         #region .Insert
@@ -648,6 +710,26 @@ public void TestInsertInsertsItemAtIndexInBoundList()
             });
         }
 
+        [Test]
+        public void TestInsertBranchingBinds()
+        {
+            var b1 = new BindableList<int> { 1, 2, 3, 4, 5 };
+
+            var b2 = b1.GetBoundCopy();
+            var b3 = b1.GetBoundCopy();
+
+            var b4 = new BindableList<int>();
+            b4.BindTo(b2);
+            b4.BindTo(b3);
+
+            b1.Insert(0, 0);
+
+            Assert.That(b1.Count, Is.EqualTo(6));
+            Assert.That(b2.Count, Is.EqualTo(6));
+            Assert.That(b3.Count, Is.EqualTo(6));
+            Assert.That(b4.Count, Is.EqualTo(6));
+        }
+
         #endregion
 
         #region .Remove(item)
@@ -995,6 +1077,21 @@ public void TestRemoveAtNotifiesBoundLists()
             Assert.That(triggeredArgs.OldStartingIndex, Is.EqualTo(0));
         }
 
+        [Test]
+        public void TestRemoveAtBranchingBinds()
+        {
+            var b1 = new BindableList<int> { 1, 2, 3, 4, 5 };
+
+            var b2 = b1.GetBoundCopy();
+            var b3 = b1.GetBoundCopy();
+
+            var b4 = new BindableList<int>();
+            b4.BindTo(b2);
+            b4.BindTo(b3);
+
+            b1.RemoveAt(b1.Count - 1);
+        }
+
         #endregion
 
         #region .RemoveAll(match)
@@ -1349,7 +1446,7 @@ public void TestDisabledNotifiesBoundLists()
 
         #endregion
 
-        #region .GetEnumberator()
+        #region .GetEnumerator()
 
         [Test]
         public void TestGetEnumeratorDoesNotReturnNull()