From 8d633aeda407a8ad65cbe0e4d8a7d4d127bb0a3d Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 8 Jul 2024 14:24:14 +0200 Subject: [PATCH 1/8] Revert " Revert "Added Range Manipulation APIs to Collection and ObservableCollection (dotnet/corefx#35772)" (dotnet/corefx#38115)" This reverts commit 18c2fa114e6ee9ad909096c27ba12633158ba671. Manual edits: * Moved CollectionTests.netcoreapp.cs to separate folder * Resolved confict in Tests.csproj Co-Authored-By: Andrew Hoefling --- .../ObjectModel/ObservableCollection.cs | 159 ++++++- ...rvableCollection_MethodTests.netcoreapp.cs | 269 +++++++++++ .../ObservableCollection_MethodsTest.cs | 2 +- ...n_SkipCollectionChangedTests.netcoreapp.cs | 126 ++++++ .../tests/System.ObjectModel.Tests.csproj | 11 +- .../System.Runtime/ref/System.Runtime.cs | 7 + .../System.Runtime.Tests.csproj | 1 + .../ObjectModel/CollectionTests.cs | 2 +- .../ObjectModel/CollectionTests.netcoreapp.cs | 424 ++++++++++++++++++ 9 files changed, 991 insertions(+), 10 deletions(-) create mode 100644 src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodTests.netcoreapp.cs create mode 100644 src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.netcoreapp.cs create mode 100644 src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.netcoreapp.cs diff --git a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs index 48109cfa49e194..55f081c8a3c51b 100644 --- a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs +++ b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs @@ -26,6 +26,9 @@ public class ObservableCollection : Collection, INotifyCollectionChanged, [NonSerialized] private int _blockReentrancyCount; + [NonSerialized] + private bool _skipRaisingEvents; + /// /// Initializes a new instance of ObservableCollection that is empty and has default initial capacity. /// @@ -107,9 +110,95 @@ protected override void RemoveItem(int index) base.RemoveItem(index); - OnCountPropertyChanged(); - OnIndexerPropertyChanged(); - OnCollectionChanged(NotifyCollectionChangedAction.Remove, removedItem, index); + if (!_skipRaisingEvents) + { + OnCountPropertyChanged(); + OnIndexerPropertyChanged(); + OnCollectionChanged(NotifyCollectionChangedAction.Remove, removedItem, index); + } + } + + /// + /// Called by base class Collection<T> when a count of items is removed from the list; + /// raises a CollectionChanged event to any listeners. + /// + protected override void RemoveItemsRange(int index, int count) + { + CheckReentrancy(); + + T[] removedItems = null; + + bool ignore = _skipRaisingEvents; + if (!ignore) + { + _skipRaisingEvents = true; + + if (count > 0) + { + removedItems = new T[count]; + for (int i = 0; i < count; i++) + { + removedItems[i] = this[index + i]; + } + } + } + + try + { + base.RemoveItemsRange(index, count); + } + finally + { + if (!ignore) + { + _skipRaisingEvents = false; + } + } + + if (count > 0 && !_skipRaisingEvents) + { + OnCountPropertyChanged(); + OnIndexerPropertyChanged(); + OnCollectionChanged(NotifyCollectionChangedAction.Remove, removedItems, index); + } + } + + /// + /// Called by base class Collection<T> when a collection of items is added to list; + /// raises a CollectionChanged event to any listeners. + /// + protected override void ReplaceItemsRange(int index, int count, IEnumerable collection) + { + CheckReentrancy(); + + _skipRaisingEvents = true; + + T[] itemsToReplace = new T[count - index]; + for (int i = index; i < count; i++) + { + itemsToReplace[i] = this[i]; + } + + try + { + base.ReplaceItemsRange(index, count, collection); + } + finally + { + _skipRaisingEvents = false; + } + + if (!_skipRaisingEvents) + { + IList newItems = collection is IList list ? list : new List(collection); + + if (newItems.Count > 0) + { + OnCountPropertyChanged(); + OnIndexerPropertyChanged(); + OnCollectionChanged(NotifyCollectionChangedAction.Replace, itemsToReplace, newItems, index); + } + } } /// @@ -121,9 +210,51 @@ protected override void InsertItem(int index, T item) CheckReentrancy(); base.InsertItem(index, item); - OnCountPropertyChanged(); - OnIndexerPropertyChanged(); - OnCollectionChanged(NotifyCollectionChangedAction.Add, item, index); + if (!_skipRaisingEvents) + { + OnCountPropertyChanged(); + OnIndexerPropertyChanged(); + OnCollectionChanged(NotifyCollectionChangedAction.Add, item, index); + } + } + + /// + /// Called by base class Collection<T> when a collection of items is added to list; + /// raises a CollectionChanged event to any listeners. + /// + protected override void InsertItemsRange(int index, IEnumerable collection) + { + CheckReentrancy(); + + bool ignore = _skipRaisingEvents; + if (!ignore) + { + _skipRaisingEvents = true; + } + + try + { + base.InsertItemsRange(index, collection); + } + finally + { + if (!ignore) + { + _skipRaisingEvents = false; + } + } + + if (!_skipRaisingEvents) + { + IList newItems = collection is IList list ? list : new List(collection); + + if (newItems.Count > 0) + { + OnCountPropertyChanged(); + OnIndexerPropertyChanged(); + OnCollectionChanged(NotifyCollectionChangedAction.Add, newItems, index); + } + } } /// @@ -252,6 +383,14 @@ private void OnCollectionChanged(NotifyCollectionChangedAction action, object? i OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, item, index)); } + /// + /// Helper to raise CollectionChanged event to any listeners + /// + private void OnCollectionChanged(NotifyCollectionChangedAction action, IList items, int index) + { + OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, items, index)); + } + /// /// Helper to raise CollectionChanged event to any listeners /// @@ -268,6 +407,14 @@ private void OnCollectionChanged(NotifyCollectionChangedAction action, object? o OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, newItem, oldItem, index)); } + /// + /// Helper to raise CollectionChanged event to any listeners + /// + private void OnCollectionChanged(NotifyCollectionChangedAction action, IList oldItems, IList newItems, int index) + { + OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, newItems, oldItems, index)); + } + /// /// Helper to raise CollectionChanged event with action == Reset to any listeners /// diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodTests.netcoreapp.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodTests.netcoreapp.cs new file mode 100644 index 00000000000000..f15d3c3c463f96 --- /dev/null +++ b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodTests.netcoreapp.cs @@ -0,0 +1,269 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Linq; +using Xunit; + +namespace System.Collections.ObjectModel.Tests +{ + public partial class ObservableCollection_MethodTests + { + [Fact] + public static void InsertRange_NotifyCollectionChanged_Beginning_Test() + { + int[] dataToInsert = new int[] { 1, 2, 3, 4, 5 }; + int[] initialData = new int[] { 10, 11, 12, 13 }; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.InsertRange(0, dataToInsert); + + Assert.Equal(dataToInsert.Length + initialData.Length, collection.Count); + Assert.Equal(1, eventCounter); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(dataToInsert, collectionAssertion.AsSpan(0, 5).ToArray()); + Assert.Equal(initialData, collectionAssertion.AsSpan(5).ToArray()); + } + + [Fact] + public static void InsertRange_NotifyCollectionChanged_Middle_Test() + { + int[] dataToInsert = new int[] { 1, 2, 3, 4, 5 }; + int[] initialData = new int[] { 10, 11, 12, 13 }; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.InsertRange(2, dataToInsert); + + Assert.Equal(dataToInsert.Length + initialData.Length, collection.Count); + Assert.Equal(1, eventCounter); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(initialData.AsSpan(0, 2).ToArray(), collectionAssertion.AsSpan(0, 2).ToArray()); + Assert.Equal(dataToInsert, collectionAssertion.AsSpan(2, 5).ToArray()); + Assert.Equal(initialData.AsSpan(2, 2).ToArray(), collectionAssertion.AsSpan(7, 2).ToArray()); + } + + [Fact] + public static void InsertRange_NotifyCollectionChanged_End_Test() + { + int[] dataToInsert = new int[] { 1, 2, 3, 4, 5 }; + int[] initialData = new int[] { 10, 11, 12, 13 }; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.InsertRange(4, dataToInsert); + + Assert.Equal(dataToInsert.Length + initialData.Length, collection.Count); + Assert.Equal(1, eventCounter); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(initialData, collectionAssertion.AsSpan(0, 4).ToArray()); + Assert.Equal(dataToInsert, collectionAssertion.AsSpan(4).ToArray()); + } + + [Fact] + public static void AddRange_NotifyCollectionChanged_Test() + { + int[] dataToInsert = new int[] { 1, 2, 3, 4, 5 }; + int[] initialData = new int[] { 10, 11, 12, 13 }; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.AddRange(dataToInsert); + + Assert.Equal(dataToInsert.Length + initialData.Length, collection.Count); + Assert.Equal(1, eventCounter); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(initialData, collectionAssertion.AsSpan(0, 4).ToArray()); + Assert.Equal(dataToInsert, collectionAssertion.AsSpan(4).ToArray()); + } + + [Fact] + public static void AddRange_NotifyCollectionChanged_EventArgs_Test() + { + int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; + int[] actualDataAdded = new int[0]; + ObservableCollection collection = new ObservableCollection(); + collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + + collection.AddRange(dataToAdd); + + Assert.Equal(dataToAdd, actualDataAdded); + } + + [Fact] + public static void InsertRange_NotifyCollectionChanged_EventArgs_Test() + { + int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; + int[] actualDataAdded = new int[0]; + ObservableCollection collection = new ObservableCollection(); + collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + + collection.InsertRange(0, dataToAdd); + + Assert.Equal(dataToAdd, actualDataAdded); + } + + [Fact] + public static void InsertRange_NotifyCollectionChanged_EventArgs_Middle_Test() + { + int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; + int[] actualDataAdded = new int[0]; + ObservableCollection collection = new ObservableCollection(); + for (int i = 0; i < 4; i++) + { + collection.Add(i); + } + + collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + collection.InsertRange(2, dataToAdd); + + Assert.Equal(dataToAdd, actualDataAdded); + } + + [Fact] + public static void InsertRange_NotifyCollectionChanged_EventArgs_End_Test() + { + int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; + int[] actualDataAdded = new int[0]; + ObservableCollection collection = new ObservableCollection(); + for (int i = 0; i < 4; i++) + { + collection.Add(i); + } + + collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + collection.InsertRange(4, dataToAdd); + + Assert.Equal(dataToAdd, actualDataAdded); + } + + [Fact] + public static void RemoveRange_NotifyCollectionChanged_FirstTwo_Test() + { + int[] initialData = new int[] { 10, 11, 12, 13 }; + int itemsToRemove = 2; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.RemoveRange(0, itemsToRemove); + + Assert.Equal(initialData.Length - itemsToRemove, collection.Count); + Assert.Equal(1, eventCounter); + Assert.Equal(initialData.AsSpan(2).ToArray(), collection.ToArray()); + } + + [Fact] + public static void RemoveRange_NotifyCollectionChanged_MiddleTwo_Test() + { + int[] initialData = new int[] { 10, 11, 12, 13 }; + int itemsToRemove = 2; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.RemoveRange(1, itemsToRemove); + + Assert.Equal(initialData.Length - itemsToRemove, collection.Count); + Assert.Equal(1, eventCounter); + Assert.Equal(initialData[0], collection[0]); + Assert.Equal(initialData[3], collection[1]); + } + + [Fact] + public static void RemoveRange_NotifyCollectionChanged_LastTwo_Test() + { + int[] initialData = new int[] { 10, 11, 12, 13 }; + int itemsToRemove = 2; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.RemoveRange(2, itemsToRemove); + + Assert.Equal(initialData.Length - itemsToRemove, collection.Count); + Assert.Equal(1, eventCounter); + Assert.Equal(initialData.AsSpan(0, 2).ToArray(), collection.ToArray()); + } + + [Fact] + public static void RemoveRange_NotifyCollectionChanged_IntMaxValueOverflow_Test() + { + int count = 500; + ObservableCollection collection = new ObservableCollection(); + for (int i = 0; i < count; i++) + { + collection.Add(i); + } + + Assert.Throws(() => collection.RemoveRange(collection.Count - 2, int.MaxValue)); + } + + [Fact] + public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexOfZero_Test() + { + int[] initialData = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; + int[] actualDataRemoved = new int[0]; + int numberOfItemsToRemove = 4; + ObservableCollection collection = new ObservableCollection(); + foreach (int item in initialData) + { + collection.Add(item); + } + + collection.CollectionChanged += (o, e) => actualDataRemoved = e.OldItems.Cast().ToArray(); + collection.RemoveRange(0, numberOfItemsToRemove); + + Assert.Equal(initialData.Length - numberOfItemsToRemove, collection.Count); + Assert.Equal(initialData.AsSpan(0, numberOfItemsToRemove).ToArray(), actualDataRemoved); + } + + [Fact] + public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexMiddle_Test() + { + int[] initialData = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; + int[] actualDataRemoved = new int[0]; + int numberOfItemsToRemove = 4; + int startIndex = 3; + ObservableCollection collection = new ObservableCollection(); + foreach (int item in initialData) + { + collection.Add(item); + } + + collection.CollectionChanged += (o, e) => actualDataRemoved = e.OldItems.Cast().ToArray(); + collection.RemoveRange(startIndex, numberOfItemsToRemove); + + Assert.Equal(initialData.Length - numberOfItemsToRemove, collection.Count); + Assert.Equal(initialData.AsSpan(startIndex, numberOfItemsToRemove).ToArray(), actualDataRemoved); + } + + [Fact] + public static void ReplaceRange_NotifyCollectionChanged_Test() + { + int[] initialData = new int[] { 10, 11, 12, 13 }; + int[] dataToReplace = new int[] { 3, 8 }; + int eventCounter = 0; + ObservableCollection collection = new ObservableCollection(initialData); + collection.CollectionChanged += (o, e) => eventCounter++; + + collection.ReplaceRange(0, 2, dataToReplace); + + Assert.Equal(initialData.Length, collection.Count); + Assert.Equal(1, eventCounter); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(dataToReplace, collectionAssertion.AsSpan(0, 2).ToArray()); + Assert.Equal(initialData.AsSpan(2, 2).ToArray(), collectionAssertion.AsSpan(2, 2).ToArray()); + } + } +} diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs index 5faf6b8d6b2261..5dc4d2ab4d9c75 100644 --- a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs +++ b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs @@ -13,7 +13,7 @@ namespace System.Collections.ObjectModel.Tests /// that the CollectionChanged events and eventargs are fired and populated /// properly. /// - public static class PublicMethodsTest + public partial class PublicMethodsTest { /// /// Tests that is possible to Add an item to the collection. diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.netcoreapp.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.netcoreapp.cs new file mode 100644 index 00000000000000..9a9b1c6f2120e6 --- /dev/null +++ b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.netcoreapp.cs @@ -0,0 +1,126 @@ +using System.Collections.Generic; +using System.Collections.ObjectModel; +using Xunit; + +namespace System.ObjectModel.Tests.ObservableCollection +{ + public class ObservableCollection_SkipCollectionChangedTests + { + [Fact] + public void SkipCollectionChanged_AddRange_Test() + { + int collectionChangedCounter = 0; + NonNullObservableCollection collection = new NonNullObservableCollection(); + collection.Add("1"); + collection.Add("2"); + collection.Add("3"); + collection.CollectionChanged += (s, e) => collectionChangedCounter++; + + Assert.Throws(() => collection.AddRange(new string[1])); + Assert.Equal(0, collectionChangedCounter); + + collection.Add("4"); + Assert.Equal(1, collectionChangedCounter); + } + + [Fact] + public void SkipCollectionChanged_InsertRange_Test() + { + int collectionChangedCounter = 0; + NonNullObservableCollection collection = new NonNullObservableCollection(); + collection.Add("1"); + collection.Add("2"); + collection.Add("3"); + collection.CollectionChanged += (s, e) => collectionChangedCounter++; + + Assert.Throws(() => collection.InsertRange(0, new string[1])); + Assert.Equal(0, collectionChangedCounter); + + collection.Add("4"); + Assert.Equal(1, collectionChangedCounter); + } + + [Fact] + public void SkipCollectionChanged_RemoveRange_Test() + { + int collectionChangedCounter = 0; + NonNullObservableCollection collection = new NonNullObservableCollection(); + collection.Add("1"); + collection.Add("2"); + collection.Add("3"); + collection.CollectionChanged += (s, e) => collectionChangedCounter++; + + collection.RemoveRange(0, 2); + Assert.Equal(1, collectionChangedCounter); + + collection.Add("1"); + Assert.Equal(2, collectionChangedCounter); + } + + [Fact] + public void SkipCollectionChanged_RemoveRange_NoEventsRaised_Test() + { + int collectionChangedCounter = 0; + NonNullObservableCollection collection = new NonNullObservableCollection(); + collection.Add("1"); + collection.Add("2"); + collection.Add("3"); + collection.CollectionChanged += (s, e) => collectionChangedCounter++; + + collection.RemoveRange(0, 0); + + Assert.Equal(0, collectionChangedCounter); + } + + [Fact] + public void SkipCollectionChanged_ReplaceRange_Test() + { + int collectionChangedCounter = 0; + NonNullObservableCollection collection = new NonNullObservableCollection(); + collection.Add("1"); + collection.Add("2"); + collection.Add("3"); + collection.CollectionChanged += (s, e) => collectionChangedCounter++; + + Assert.Throws(() => collection.ReplaceRange(0, 2, new string[1])); + Assert.Equal(0, collectionChangedCounter); + + collection.Add("1"); + Assert.Equal(1, collectionChangedCounter); + } + + [Fact] + public void SkipCollectionChanged_ReplaceRange_Empty_Test() + { + int collectionChangedCounter = 0; + NonNullObservableCollection collection = new NonNullObservableCollection(); + collection.Add("1"); + collection.Add("2"); + collection.Add("3"); + collection.CollectionChanged += (s, e) => collectionChangedCounter++; + + collection.ReplaceRange(0, 0, new string[0]); + Assert.Equal(0, collectionChangedCounter); + + collection.Add("1"); + Assert.Equal(1, collectionChangedCounter); + } + + public class NonNullObservableCollection : ObservableCollection + { + + public NonNullObservableCollection() : base() { } + public NonNullObservableCollection(List list) : base(list) { } + + protected override void InsertItem(int index, T item) + { + if (item == null) + { + throw new ArgumentNullException(); + } + + base.InsertItem(index, item); + } + } + } +} diff --git a/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj b/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj index f015015de4d3ed..f2ad25e72195fa 100644 --- a/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj +++ b/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj @@ -33,8 +33,15 @@ - + + Common\System\Diagnostics\DebuggerAttributes.cs + + + + + + + diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index e7e61105315e09..c34130e3b56a9f 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -8332,6 +8332,7 @@ public Collection(System.Collections.Generic.IList list) { } bool System.Collections.IList.IsReadOnly { get { throw null; } } object? System.Collections.IList.this[int index] { get { throw null; } set { } } public void Add(T item) { } + public void AddRange(System.Collections.Generic.IEnumerable collection) { } public void Clear() { } protected virtual void ClearItems() { } public bool Contains(T item) { throw null; } @@ -8340,9 +8341,15 @@ public void CopyTo(T[] array, int index) { } public int IndexOf(T item) { throw null; } public void Insert(int index, T item) { } protected virtual void InsertItem(int index, T item) { } + protected virtual void InsertItemsRange(int index, System.Collections.Generic.IEnumerable collection) { } + public void InsertRange(int index, System.Collections.Generic.IEnumerable collection) { } public bool Remove(T item) { throw null; } public void RemoveAt(int index) { } protected virtual void RemoveItem(int index) { } + protected virtual void RemoveItemsRange(int index, int count) { } + public void RemoveRange(int index, int count) { } + protected virtual void ReplaceItemsRange(int index, int count, System.Collections.Generic.IEnumerable collection) { } + public void ReplaceRange(int index, int count, System.Collections.Generic.IEnumerable collection) { } protected virtual void SetItem(int index, T item) { } void System.Collections.ICollection.CopyTo(System.Array array, int index) { } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj index 1c9fa97e35c6da..8c12f46253dc79 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj @@ -70,6 +70,7 @@ + diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.cs index 2818edafd238a9..723b2acda03133 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.cs @@ -15,7 +15,7 @@ namespace System.Collections.ObjectModel.Tests /// we expect are forwarded to the underlying list, and verify that the exceptions we expect /// are thrown. /// - public class CollectionTests : CollectionTestBase + public partial class CollectionTests : CollectionTestBase { private static readonly Collection s_empty = new Collection(); diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.netcoreapp.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.netcoreapp.cs new file mode 100644 index 00000000000000..940af677362623 --- /dev/null +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.netcoreapp.cs @@ -0,0 +1,424 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using Xunit; + +namespace System.Collections.ObjectModel.Tests +{ + /// + /// Since is just a wrapper base class around an , + /// we just verify that the underlying list is what we expect, validate that the calls which + /// we expect are forwarded to the underlying list, and verify that the exceptions we expect + /// are thrown. + /// + public partial class CollectionTests : CollectionTestBase + { + [Fact] + public void Collection_AddRange_ToEmpty_Test() + { + int[] expected = new[] { 1, 2, 3, 4 }; + Collection collection = new Collection(); + + collection.AddRange(expected); + + Assert.NotNull(collection); + Assert.Equal(expected.Length, collection.Count); + Assert.Equal(expected, collection.ToArray()); + } + + [Fact] + public void Collection_AddRange_ToExisting_Test() + { + int[] initial = new int[] { 1, 2, 3, 4 }; + int[] dataToInsert = new int[] { 5, 6, 7, 8 }; + Collection collection = new Collection(); + for (int i = 0; i < initial.Length; i++) + collection.Add(initial[i]); + + collection.AddRange(dataToInsert); + + Assert.NotNull(collection); + Assert.Equal(initial.Length + dataToInsert.Length, collection.Count); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(initial, collectionAssertion.AsSpan(0, 4).ToArray()); + Assert.Equal(dataToInsert, collectionAssertion.AsSpan(4, 4).ToArray()); + } + + [Fact] + public void Collection_AddRange_Empty_Test() + { + int[] expected = new int[0]; + Collection collection = new Collection(); + + collection.AddRange(expected); + + Assert.NotNull(collection); + Assert.Equal(expected.Length, collection.Count); + } + + [Fact] + public void Collection_AddRange_Null_Test() + { + Collection collection = new Collection(); + Exception ex = Assert.Throws(() => collection.AddRange(null)); + } + + [Fact] + public void Collection_AddRange_ReadOnly_Test() + { + int[] expected = new int[] { 1, 2, 3, 4 }; + int[] baseCollection = new int[] { 5, 6, 7, 8 }; + Collection collection = new Collection(baseCollection); + + Exception ex = Assert.Throws(() => collection.AddRange(expected)); + } + + [Fact] + public void Collection_InsertRange_Beginning_Test() + { + int[] expected = new int[] { 1, 2, 3, 4, 5 }; + int[] originalCollection = new int[] { 10, 11, 12, 13 }; + List baseCollection = new List(originalCollection); + + int expectedLength = expected.Length + originalCollection.Length; + Collection collection = new Collection(baseCollection); + + collection.InsertRange(0, expected); + + Assert.NotNull(collection); + Assert.Equal(expectedLength, collection.Count); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(expected, collectionAssertion.AsSpan(0, 5).ToArray()); + Assert.Equal(originalCollection, collectionAssertion.AsSpan(5, 4).ToArray()); + } + + [Fact] + public void Collection_InsertRange_End_Test() + { + int[] expected = new int[] { 1, 2, 3, 4, 5 }; + int[] originalCollection = new int[] { 10, 11, 12, 13 }; + List baseCollection = new List(originalCollection); + + int expectedLength = expected.Length + originalCollection.Length; + Collection collection = new Collection(baseCollection); + + collection.InsertRange(expected.Length - 1, expected); + + Assert.NotNull(collection); + Assert.Equal(expectedLength, collection.Count); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(originalCollection, collectionAssertion.AsSpan(0, 4).ToArray()); + Assert.Equal(expected, collectionAssertion.AsSpan(4, 5).ToArray()); + } + + [Fact] + public void Collection_InsertRange_Middle_Test() + { + int[] expected = new int[] { 1, 2, 3, 4, 5 }; + int[] originalCollection = new int[] { 10, 11, 12, 13 }; + List baseCollection = new List(originalCollection); + + int expectedLength = expected.Length + originalCollection.Length; + Collection collection = new Collection(baseCollection); + + collection.InsertRange(2, expected); + + Assert.NotNull(collection); + Assert.Equal(expectedLength, collection.Count); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(originalCollection.AsSpan(0, 2).ToArray(), collectionAssertion.AsSpan(0, 2).ToArray()); + Assert.Equal(expected, collectionAssertion.AsSpan(2, 5).ToArray()); + Assert.Equal(originalCollection.AsSpan(2, 2).ToArray(), collectionAssertion.AsSpan(7, 2).ToArray()); + } + + [Fact] + public void Collection_InsertRange_Empty_Test() + { + List baseCollection = new List(new[] { 10, 11, 12, 13 }); + Collection collection = new Collection(baseCollection); + + collection.InsertRange(0, new int[0]); + + Assert.NotNull(collection); + Assert.Equal(baseCollection.Count, collection.Count); + Assert.Equal(baseCollection, collection.ToArray()); + } + + [Fact] + public void Collection_InsertRange_Null_Test() + { + Collection collection = new Collection(); + Exception ex = Assert.Throws(() => collection.InsertRange(0, null)); + } + + [Fact] + public void Collection_InsertRange_ReadOnly_Test() + { + int[] expected = new int[] { 1, 2, 3, 4 }; + int[] baseCollection = new int[] { 5, 6, 7, 8 }; + Collection collection = new Collection(baseCollection); + + Exception ex = Assert.Throws(() => collection.InsertRange(0, expected)); + } + + [Fact] + public void Collection_InsertRange_IndexLessThan0_Test() + { + int[] expected = new int[] { 1, 2, 3, 4 }; + Collection collection = new Collection(); + Exception ex = Assert.Throws(() => collection.InsertRange(-1, expected)); + } + + [Fact] + public void Collection_InsertRange_IndexGreaterThanCount_Test() + { + int[] expected = new int[] { 1, 2, 3, 4 }; + Collection collection = new Collection(); + Exception ex = Assert.Throws(() => collection.InsertRange(10, expected)); + } + + [Fact] + public void Collection_RemoveRange_Overflow_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + collection.Add(3); + + Assert.Throws(() => collection.RemoveRange(0, 4)); + } + + [Fact] + public void Collection_RemoveRange_IntMaxValueOverflow_Test() + { + var count = 500; + Collection collection = new Collection(); + for (int i = 0; i < count; i++) + { + collection.Add(i); + } + + Assert.Throws(() => collection.RemoveRange(collection.Count - 2, int.MaxValue)); + } + + [Fact] + public void Collection_RemoveRange_CountIsZero_Test() + { + int[] expected = new int[] { 1, 2, 3, 4, 5 }; + Collection collection = new Collection(); + for (int i = 0; i < expected.Length; i++) + { + collection.Add(expected[i]); + } + + collection.RemoveRange(0, 0); + + Assert.NotNull(collection); + Assert.Equal(expected.Length, collection.Count); + Assert.Equal(expected, collection.ToArray()); + } + + [Fact] + public void Collection_RemoveRange_CountIsLessThanZero_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + + Assert.Throws(() => collection.RemoveRange(0, -1)); + } + + [Fact] + public void Collection_RemoveRange_All_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + collection.Add(3); + + collection.RemoveRange(0, 3); + + Assert.NotNull(collection); + Assert.Equal(0, collection.Count); + } + + [Fact] + public void Collection_RemoveRange_FirstTwoItems_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + collection.Add(3); + + collection.RemoveRange(0, 2); + + Assert.NotNull(collection); + Assert.Equal(1, collection.Count); + Assert.Equal(3, collection[0]); + } + + [Fact] + public void Collection_RemoveRange_LastTwoItems_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + collection.Add(3); + + collection.RemoveRange(1, 2); + + Assert.NotNull(collection); + Assert.Equal(1, collection.Count); + Assert.Equal(1, collection[0]); + } + + [Fact] + public void Collection_RemoveRange_ZeroItems_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + collection.Add(3); + + collection.RemoveRange(0, 0); + + Assert.NotNull(collection); + Assert.Equal(3, collection.Count); + Assert.Equal(1, collection[0]); + Assert.Equal(2, collection[1]); + Assert.Equal(3, collection[2]); + } + + [Fact] + public void Collection_RemoveRange_IndexLessThanZero_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + collection.Add(3); + + AssertExtensions.Throws("index", () => collection.RemoveRange(-1, 3)); + } + + [Fact] + public void Collection_RemoveRange_IndexGreaterThanCollection_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + collection.Add(3); + + AssertExtensions.Throws("index", () => collection.RemoveRange(4, 3)); + } + + [Fact] + public void Collection_RemoveRange_ReadOnly_Test() + { + Collection collection = new Collection(new int[] { 1, 2, 3 }); + + Assert.Throws(() => collection.RemoveRange(0, 2)); + } + + [Fact] + public void Collection_ReplaceRange_FirstTwo_Test() + { + int[] initial = new int[] { 1, 2, 3, 4 }; + int[] replace = new int[] { 5, 6, 7, 8 }; + Collection collection = new Collection(); + foreach (var item in initial) + collection.Add(item); + + collection.ReplaceRange(0, 2, replace); + + Assert.NotNull(collection); + Assert.Equal(initial.Length + 2, collection.Count); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(replace, collectionAssertion.AsSpan(0, 4).ToArray()); + Assert.Equal(initial.AsSpan(2, 2).ToArray(), collectionAssertion.AsSpan(4, 2).ToArray()); + } + + [Fact] + public void Collection_ReplaceRange_LastTwo_Test() + { + int[] initial = new int[] { 1, 2, 3, 4 }; + int[] replace = new int[] { 5, 6, 7, 8 }; + Collection collection = new Collection(); + foreach (var item in initial) + collection.Add(item); + + collection.ReplaceRange(2, 2, replace); + + Assert.NotNull(collection); + Assert.Equal(initial.Length + 2, collection.Count); + + int[] collectionAssertion = collection.ToArray(); + Assert.Equal(initial.AsSpan(0, 2).ToArray(), collectionAssertion.AsSpan(0, 2).ToArray()); + Assert.Equal(replace.AsSpan(0, 4).ToArray(), collectionAssertion.AsSpan(2, 4).ToArray()); + } + + [Fact] + public void Collection_ReplaceRange_MiddleTwo_Test() + { + int[] initial = new int[] { 1, 2, 3, 4 }; + int[] replace = new int[] { 5, 6, 7, 8 }; + Collection collection = new Collection(); + foreach (var item in initial) + collection.Add(item); + + collection.ReplaceRange(1, 2, replace); + + Assert.NotNull(collection); + Assert.Equal(initial.Length + 2, collection.Count); + + Assert.Equal(initial[0], collection[0]); + Assert.Equal(replace, collection.ToArray().AsSpan(1, 4).ToArray()); + Assert.Equal(initial[3], collection[5]); + } + + [Fact] + public void Collection_ReplaceRange_NullCollection_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + + Assert.Throws(() => collection.ReplaceRange(0, 2, null)); + } + + [Fact] + public void Collection_ReplaceRange_ReadOnly_Test() + { + Collection collection = new Collection(new int[] { 1, 2, 3 }); + Assert.Throws(() => collection.ReplaceRange(0, 2, new int[] { 4, 5 })); + } + + [Fact] + public void Collection_ReplaceRange_IndexLessThanZero_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + + AssertExtensions.Throws("index", () => collection.ReplaceRange(-2, 2, new int[] { 1, 2 })); + } + + [Fact] + public void Collection_ReplaceRange_IndexGreaterThanCount_Test() + { + Collection collection = new Collection(); + collection.Add(1); + collection.Add(2); + + AssertExtensions.Throws("index", () => collection.ReplaceRange(4, 2, new int[] { 1, 2 })); + } + } +} From 3184415ebba1f968b92594ce57bc31579c15deb5 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 8 Jul 2024 22:14:21 +0200 Subject: [PATCH 2/8] Revert "Remove Collection range APIs (dotnet/coreclr#24938)" This reverts commit e059596d681b354c736dbafe0f480d15776485ac. --- .../Collections/ObjectModel/Collection.cs | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs index 4c6aa24b0a4a63..a6a5cb777a0f55 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs @@ -63,6 +63,8 @@ public void Add(T item) InsertItem(index, item); } + public void AddRange(IEnumerable collection) => InsertItemsRange(items.Count, collection); + public void Clear() { if (items.IsReadOnly) @@ -108,6 +110,26 @@ public void Insert(int index, T item) InsertItem(index, item); } + public void InsertRange(int index, IEnumerable collection) + { + if (items.IsReadOnly) + { + ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection); + } + + if (collection == null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.collection); + } + + if ((uint)index > (uint)items.Count) + { + ThrowHelper.ThrowArgumentOutOfRange_IndexException(); + } + + InsertItemsRange(index, collection!); // TODO-NULLABLE: Remove ! when [DoesNotReturn] respected + } + public bool Remove(T item) { if (items.IsReadOnly) @@ -121,6 +143,61 @@ public bool Remove(T item) return true; } + public void RemoveRange(int index, int count) + { + if (items.IsReadOnly) + { + ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection); + } + + if ((uint)index > (uint)items.Count) + { + ThrowHelper.ThrowArgumentOutOfRange_IndexException(); + } + + if (count < 0) + { + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); + } + + if (index > items.Count - count) + { + ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_InvalidOffLen); + } + + RemoveItemsRange(index, count); + } + + public void ReplaceRange(int index, int count, IEnumerable collection) + { + if (items.IsReadOnly) + { + ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection); + } + + if ((uint)index > (uint)items.Count) + { + ThrowHelper.ThrowArgumentOutOfRange_IndexException(); + } + + if (count < 0) + { + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); + } + + if (index > items.Count - count) + { + ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_InvalidOffLen); + } + + if (collection == null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.collection); + } + + ReplaceItemsRange(index, count, collection!); // TODO-NULLABLE: Remove ! when [DoesNotReturn] respected + } + public void RemoveAt(int index) { if (items.IsReadOnly) @@ -156,6 +233,42 @@ protected virtual void SetItem(int index, T item) items[index] = item; } + protected virtual void InsertItemsRange(int index, IEnumerable collection) + { + if (GetType() == typeof(Collection) && items is List list) + { + list.InsertRange(index, collection); + } + else + { + foreach (T item in collection) + { + InsertItem(index++, item); + } + } + } + + protected virtual void RemoveItemsRange(int index, int count) + { + if (GetType() == typeof(Collection) && items is List list) + { + list.RemoveRange(index, count); + } + else + { + for (int i = 0; i < count; i++) + { + RemoveItem(index); + } + } + } + + protected virtual void ReplaceItemsRange(int index, int count, IEnumerable collection) + { + RemoveItemsRange(index, count); + InsertItemsRange(index, collection); + } + bool ICollection.IsReadOnly => items.IsReadOnly; IEnumerator IEnumerable.GetEnumerator() From 48584a5e2b222034728ce0e237aa49543a013f8e Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 8 Jul 2024 22:25:10 +0200 Subject: [PATCH 3/8] Fix so code compiles after reverts * Fix calls to throwhelpers that have been renamed * Remove // TODO-NULLABLE: * Updates to project files --- .../tests/System.ObjectModel.Tests.csproj | 4 ---- .../src/System/Collections/ObjectModel/Collection.cs | 8 ++++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj b/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj index f2ad25e72195fa..e8c0c28cab668d 100644 --- a/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj +++ b/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj @@ -36,12 +36,8 @@ Common\System\Diagnostics\DebuggerAttributes.cs - - - - diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs index a6a5cb777a0f55..4e5f761fa51499 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/ObjectModel/Collection.cs @@ -124,7 +124,7 @@ public void InsertRange(int index, IEnumerable collection) if ((uint)index > (uint)items.Count) { - ThrowHelper.ThrowArgumentOutOfRange_IndexException(); + ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException(); } InsertItemsRange(index, collection!); // TODO-NULLABLE: Remove ! when [DoesNotReturn] respected @@ -152,7 +152,7 @@ public void RemoveRange(int index, int count) if ((uint)index > (uint)items.Count) { - ThrowHelper.ThrowArgumentOutOfRange_IndexException(); + ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException(); } if (count < 0) @@ -177,7 +177,7 @@ public void ReplaceRange(int index, int count, IEnumerable collection) if ((uint)index > (uint)items.Count) { - ThrowHelper.ThrowArgumentOutOfRange_IndexException(); + ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException(); } if (count < 0) @@ -195,7 +195,7 @@ public void ReplaceRange(int index, int count, IEnumerable collection) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.collection); } - ReplaceItemsRange(index, count, collection!); // TODO-NULLABLE: Remove ! when [DoesNotReturn] respected + ReplaceItemsRange(index, count, collection); } public void RemoveAt(int index) From 580bfcf5ab47b32265f22f778ad2779edb43da2b Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 8 Jul 2024 23:36:08 +0200 Subject: [PATCH 4/8] Rewrite NotifyCollectionChangedEventArgs code for Range API so it becomes easier to add a more compatible "Reset" based approach --- .../ObjectModel/ObservableCollection.cs | 86 +++++++++++-------- 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs index 55f081c8a3c51b..78e0674442b7b4 100644 --- a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs +++ b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs @@ -29,6 +29,12 @@ public class ObservableCollection : Collection, INotifyCollectionChanged, [NonSerialized] private bool _skipRaisingEvents; + /// + /// true to opt into raising with list + /// of items when a range is inserted, removed or replaced. Instead of resets + /// + private static bool RaiseRangeCollectionChangedEvents => true; + /// /// Initializes a new instance of ObservableCollection that is empty and has default initial capacity. /// @@ -126,20 +132,21 @@ protected override void RemoveItemsRange(int index, int count) { CheckReentrancy(); - T[] removedItems = null; - + NotifyCollectionChangedEventArgs collectionChangedEventArgs = EventArgsCache.ResetCollectionChanged; bool ignore = _skipRaisingEvents; if (!ignore) { _skipRaisingEvents = true; - if (count > 0) + if (RaiseRangeCollectionChangedEvents && count > 0 && CollectionChanged is not null) { - removedItems = new T[count]; + T[] removedItems = new T[count]; for (int i = 0; i < count; i++) { removedItems[i] = this[index + i]; } + + collectionChangedEventArgs = new(NotifyCollectionChangedAction.Remove, removedItems, index); } } @@ -159,7 +166,7 @@ protected override void RemoveItemsRange(int index, int count) { OnCountPropertyChanged(); OnIndexerPropertyChanged(); - OnCollectionChanged(NotifyCollectionChangedAction.Remove, removedItems, index); + OnCollectionChanged(collectionChangedEventArgs); } } @@ -171,12 +178,25 @@ protected override void ReplaceItemsRange(int index, int count, IEnumerable c { CheckReentrancy(); - _skipRaisingEvents = true; + int countBefore = default; + bool skipEvents = _skipRaisingEvents; + if (!skipEvents) + { + _skipRaisingEvents = true; + countBefore = Count; + } - T[] itemsToReplace = new T[count - index]; - for (int i = index; i < count; i++) + NotifyCollectionChangedEventArgs collectionChangedEventArgs = EventArgsCache.ResetCollectionChanged; + if (RaiseRangeCollectionChangedEvents && !skipEvents && CollectionChanged is not null) { - itemsToReplace[i] = this[i]; + T[] itemsToReplace = new T[count - index]; + for (int i = index; i < count; i++) + { + itemsToReplace[i] = this[i]; + } + + IList newItems = collection as IList ?? new List(collection); + collectionChangedEventArgs = new(NotifyCollectionChangedAction.Replace, newItems, itemsToReplace, index); } try @@ -185,18 +205,25 @@ protected override void ReplaceItemsRange(int index, int count, IEnumerable c } finally { - _skipRaisingEvents = false; + if (!skipEvents) + { + _skipRaisingEvents = false; + } } - if (!_skipRaisingEvents) + if (!skipEvents) { - IList newItems = collection is IList list ? list : new List(collection); - - if (newItems.Count > 0) + if (countBefore != Count) { OnCountPropertyChanged(); OnIndexerPropertyChanged(); - OnCollectionChanged(NotifyCollectionChangedAction.Replace, itemsToReplace, newItems, index); + OnCollectionChanged(collectionChangedEventArgs); + } + else if (count > 0) + { + // We replaced positive number of items with the same number of items, only the contents changed + OnIndexerPropertyChanged(); + OnCollectionChanged(collectionChangedEventArgs); } } } @@ -226,10 +253,12 @@ protected override void InsertItemsRange(int index, IEnumerable collection) { CheckReentrancy(); + int countBefore = default; bool ignore = _skipRaisingEvents; if (!ignore) { _skipRaisingEvents = true; + countBefore = Count; } try @@ -246,13 +275,18 @@ protected override void InsertItemsRange(int index, IEnumerable collection) if (!_skipRaisingEvents) { - IList newItems = collection is IList list ? list : new List(collection); + NotifyCollectionChangedEventArgs collectionChangedEventArgs = EventArgsCache.ResetCollectionChanged; + if (RaiseRangeCollectionChangedEvents && CollectionChanged is not null) + { + IList newItems = collection as IList ?? new List(collection); + collectionChangedEventArgs = new(NotifyCollectionChangedAction.Add, newItems, index); + } - if (newItems.Count > 0) + if (countBefore != Count) { OnCountPropertyChanged(); OnIndexerPropertyChanged(); - OnCollectionChanged(NotifyCollectionChangedAction.Add, newItems, index); + OnCollectionChanged(collectionChangedEventArgs); } } } @@ -383,14 +417,6 @@ private void OnCollectionChanged(NotifyCollectionChangedAction action, object? i OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, item, index)); } - /// - /// Helper to raise CollectionChanged event to any listeners - /// - private void OnCollectionChanged(NotifyCollectionChangedAction action, IList items, int index) - { - OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, items, index)); - } - /// /// Helper to raise CollectionChanged event to any listeners /// @@ -407,14 +433,6 @@ private void OnCollectionChanged(NotifyCollectionChangedAction action, object? o OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, newItem, oldItem, index)); } - /// - /// Helper to raise CollectionChanged event to any listeners - /// - private void OnCollectionChanged(NotifyCollectionChangedAction action, IList oldItems, IList newItems, int index) - { - OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, newItems, oldItems, index)); - } - /// /// Helper to raise CollectionChanged event with action == Reset to any listeners /// From 7e1d24fd72c8073cfce46226d89d264383578b4a Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 8 Jul 2024 23:43:37 +0200 Subject: [PATCH 5/8] rename "ignore" to "skipEvents" for rest of range APIs --- .../Collections/ObjectModel/ObservableCollection.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs index 78e0674442b7b4..c1db05381e83ee 100644 --- a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs +++ b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs @@ -133,8 +133,8 @@ protected override void RemoveItemsRange(int index, int count) CheckReentrancy(); NotifyCollectionChangedEventArgs collectionChangedEventArgs = EventArgsCache.ResetCollectionChanged; - bool ignore = _skipRaisingEvents; - if (!ignore) + bool skipEvents = _skipRaisingEvents; + if (!skipEvents) { _skipRaisingEvents = true; @@ -156,7 +156,7 @@ protected override void RemoveItemsRange(int index, int count) } finally { - if (!ignore) + if (!skipEvents) { _skipRaisingEvents = false; } @@ -254,8 +254,8 @@ protected override void InsertItemsRange(int index, IEnumerable collection) CheckReentrancy(); int countBefore = default; - bool ignore = _skipRaisingEvents; - if (!ignore) + bool skipEvents = _skipRaisingEvents; + if (!skipEvents) { _skipRaisingEvents = true; countBefore = Count; @@ -267,7 +267,7 @@ protected override void InsertItemsRange(int index, IEnumerable collection) } finally { - if (!ignore) + if (!skipEvents) { _skipRaisingEvents = false; } From 2b682811d4eb3fe7c9985ff46c884dd3c16a9e84 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Wed, 10 Jul 2024 10:05:58 +0200 Subject: [PATCH 6/8] Update tests * Rename test files (replace .netcoreapp with something more suiteable) * CollectionTests: Remove redundat Assert.NotNull(collection) since collection is created with new * ObservableCollectionTests * Rewrite tests for batch changes so they 1. verify events args Action and Index 2. use InlineData to tests both with "batch notification" and without (reset) --- .../ObservableCollection_MethodsTest.cs | 2 +- ... ObservableCollection_RangeMethodTests.cs} | 192 +++++++++++++----- ...eCollection_SkipCollectionChangedTests.cs} | 0 .../tests/System.ObjectModel.Tests.csproj | 9 +- .../System.Runtime.Tests.csproj | 2 +- ...app.cs => CollectionTests.RangeMethods.cs} | 19 +- 6 files changed, 146 insertions(+), 78 deletions(-) rename src/libraries/System.ObjectModel/tests/ObservableCollection/{ObservableCollection_MethodTests.netcoreapp.cs => ObservableCollection_RangeMethodTests.cs} (60%) rename src/libraries/System.ObjectModel/tests/ObservableCollection/{ObservableCollection_SkipCollectionChangedTests.netcoreapp.cs => ObservableCollection_SkipCollectionChangedTests.cs} (100%) rename src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/{CollectionTests.netcoreapp.cs => CollectionTests.RangeMethods.cs} (95%) diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs index 5dc4d2ab4d9c75..5faf6b8d6b2261 100644 --- a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs +++ b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodsTest.cs @@ -13,7 +13,7 @@ namespace System.Collections.ObjectModel.Tests /// that the CollectionChanged events and eventargs are fired and populated /// properly. /// - public partial class PublicMethodsTest + public static class PublicMethodsTest { /// /// Tests that is possible to Add an item to the collection. diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodTests.netcoreapp.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs similarity index 60% rename from src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodTests.netcoreapp.cs rename to src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs index f15d3c3c463f96..ccd764e46f182b 100644 --- a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_MethodTests.netcoreapp.cs +++ b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs @@ -2,12 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Specialized; using System.Linq; using Xunit; namespace System.Collections.ObjectModel.Tests { - public partial class ObservableCollection_MethodTests + public partial class ObservableCollection_RangeMethodTests { [Fact] public static void InsertRange_NotifyCollectionChanged_Beginning_Test() @@ -86,64 +87,114 @@ public static void AddRange_NotifyCollectionChanged_Test() Assert.Equal(dataToInsert, collectionAssertion.AsSpan(4).ToArray()); } - [Fact] - public static void AddRange_NotifyCollectionChanged_EventArgs_Test() + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void AddRange_NotifyCollectionChanged_EventArgs_Test(bool batchCollectionChanged) { int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; - int[] actualDataAdded = new int[0]; - ObservableCollection collection = new ObservableCollection(); - collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + ObservableCollection collection = new(); + NotifyCollectionChangedEventArgs? args = null; + collection.CollectionChanged += (o, e) => args = e; collection.AddRange(dataToAdd); - Assert.Equal(dataToAdd, actualDataAdded); + Assert.NotNull(args); + if (batchCollectionChanged) + { + Assert.Equal(NotifyCollectionChangedAction.Add, args.Action); + Assert.Equal(0, args.NewStartingIndex); + Assert.Equal(dataToAdd, args.NewItems.Cast().ToArray()); + } + else + { + Assert.Equal(NotifyCollectionChangedAction.Reset, args.Action); + } } - [Fact] - public static void InsertRange_NotifyCollectionChanged_EventArgs_Test() + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void InsertRange_NotifyCollectionChanged_EventArgs_Test(bool batchCollectionChanged) { int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; - int[] actualDataAdded = new int[0]; - ObservableCollection collection = new ObservableCollection(); - collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + ObservableCollection collection = new(); + NotifyCollectionChangedEventArgs? args = null; + collection.CollectionChanged += (o, e) => args = e; collection.InsertRange(0, dataToAdd); - Assert.Equal(dataToAdd, actualDataAdded); + Assert.NotNull(args); + if (batchCollectionChanged) + { + Assert.Equal(NotifyCollectionChangedAction.Add, args.Action); + Assert.Equal(0, args.NewStartingIndex); + Assert.Equal(dataToAdd, args.NewItems.Cast().ToArray()); + } + else + { + Assert.Equal(NotifyCollectionChangedAction.Reset, args.Action); + } } - [Fact] - public static void InsertRange_NotifyCollectionChanged_EventArgs_Middle_Test() + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void InsertRange_NotifyCollectionChanged_EventArgs_Middle_Test(bool batchCollectionChanged) { int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; - int[] actualDataAdded = new int[0]; ObservableCollection collection = new ObservableCollection(); + NotifyCollectionChangedEventArgs? args = null; + for (int i = 0; i < 4; i++) { collection.Add(i); } - collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + collection.CollectionChanged += (o, e) => args = e; collection.InsertRange(2, dataToAdd); - Assert.Equal(dataToAdd, actualDataAdded); + Assert.NotNull(args); + if (batchCollectionChanged) + { + Assert.Equal(NotifyCollectionChangedAction.Add, args.Action); + Assert.Equal(2, args.NewStartingIndex); + Assert.Equal(dataToAdd, args.NewItems.Cast().ToArray()); + } + else + { + Assert.Equal(NotifyCollectionChangedAction.Reset, args.Action); + } } - [Fact] - public static void InsertRange_NotifyCollectionChanged_EventArgs_End_Test() + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void InsertRange_NotifyCollectionChanged_EventArgs_End_Test(bool batchCollectionChanged) { int[] dataToAdd = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; - int[] actualDataAdded = new int[0]; ObservableCollection collection = new ObservableCollection(); + NotifyCollectionChangedEventArgs? args = null; + for (int i = 0; i < 4; i++) { collection.Add(i); } - collection.CollectionChanged += (o, e) => actualDataAdded = e.NewItems.Cast().ToArray(); + collection.CollectionChanged += (o, e) => args = e; collection.InsertRange(4, dataToAdd); - Assert.Equal(dataToAdd, actualDataAdded); + Assert.NotNull(args); + if (batchCollectionChanged) + { + Assert.Equal(NotifyCollectionChangedAction.Add, args.Action); + Assert.Equal(4, args.NewStartingIndex); + Assert.Equal(dataToAdd, args.NewItems.Cast().ToArray()); + } + else + { + Assert.Equal(NotifyCollectionChangedAction.Reset, args.Action); + } } [Fact] @@ -208,62 +259,97 @@ public static void RemoveRange_NotifyCollectionChanged_IntMaxValueOverflow_Test( Assert.Throws(() => collection.RemoveRange(collection.Count - 2, int.MaxValue)); } - [Fact] - public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexOfZero_Test() + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexOfZero_Test(bool batchCollectionChanged) { int[] initialData = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; - int[] actualDataRemoved = new int[0]; int numberOfItemsToRemove = 4; - ObservableCollection collection = new ObservableCollection(); - foreach (int item in initialData) - { - collection.Add(item); - } + ObservableCollection collection = new(initialData); + NotifyCollectionChangedEventArgs? args = null; - collection.CollectionChanged += (o, e) => actualDataRemoved = e.OldItems.Cast().ToArray(); + collection.CollectionChanged += (o, e) => args = e; collection.RemoveRange(0, numberOfItemsToRemove); Assert.Equal(initialData.Length - numberOfItemsToRemove, collection.Count); - Assert.Equal(initialData.AsSpan(0, numberOfItemsToRemove).ToArray(), actualDataRemoved); + + Assert.NotNull(args); + if (batchCollectionChanged) + { + Assert.Equal(NotifyCollectionChangedAction.Remove, args.Action); + Assert.Equal(0, args.OldStartingIndex); + Assert.Equal(initialData.AsSpan(0, numberOfItemsToRemove).ToArray(), args.OldItems.Cast().ToArray()); + } + else + { + Assert.Equal(NotifyCollectionChangedAction.Reset, args.Action); + } } - [Fact] - public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexMiddle_Test() + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexMiddle_Test(bool batchCollectionChanged) { int[] initialData = new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }; - int[] actualDataRemoved = new int[0]; int numberOfItemsToRemove = 4; int startIndex = 3; - ObservableCollection collection = new ObservableCollection(); - foreach (int item in initialData) - { - collection.Add(item); - } - collection.CollectionChanged += (o, e) => actualDataRemoved = e.OldItems.Cast().ToArray(); + ObservableCollection collection = new(initialData); + NotifyCollectionChangedEventArgs? args = null; + + collection.CollectionChanged += (o, e) => args = e; collection.RemoveRange(startIndex, numberOfItemsToRemove); Assert.Equal(initialData.Length - numberOfItemsToRemove, collection.Count); - Assert.Equal(initialData.AsSpan(startIndex, numberOfItemsToRemove).ToArray(), actualDataRemoved); + + Assert.NotNull(args); + if (batchCollectionChanged) + { + Assert.Equal(NotifyCollectionChangedAction.Remove, args.Action); + Assert.Equal(startIndex, args.OldStartingIndex); + Assert.Equal(initialData.AsSpan(startIndex, numberOfItemsToRemove).ToArray(), args.OldItems.Cast().ToArray()); + } + else + { + Assert.Equal(NotifyCollectionChangedAction.Reset, args.Action); + } } - [Fact] - public static void ReplaceRange_NotifyCollectionChanged_Test() + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void ReplaceRange_NotifyCollectionChanged_Test(bool batchCollectionChanged) { int[] initialData = new int[] { 10, 11, 12, 13 }; int[] dataToReplace = new int[] { 3, 8 }; - int eventCounter = 0; + int[] expectedResult = new int[] { 10, 3, 8 ,13 }; + int startIndex = 1; + int count = 2; + ObservableCollection collection = new ObservableCollection(initialData); - collection.CollectionChanged += (o, e) => eventCounter++; + NotifyCollectionChangedEventArgs? args = null; - collection.ReplaceRange(0, 2, dataToReplace); + collection.CollectionChanged += (o, e) => { Assert.Null(args); args = e; }; - Assert.Equal(initialData.Length, collection.Count); - Assert.Equal(1, eventCounter); + collection.ReplaceRange(startIndex, count, dataToReplace); - int[] collectionAssertion = collection.ToArray(); - Assert.Equal(dataToReplace, collectionAssertion.AsSpan(0, 2).ToArray()); - Assert.Equal(initialData.AsSpan(2, 2).ToArray(), collectionAssertion.AsSpan(2, 2).ToArray()); + Assert.Equal(expectedResult, collection.ToArray()); + + Assert.NotNull(args); + if (batchCollectionChanged) + { + Assert.Equal(NotifyCollectionChangedAction.Replace, args.Action); + Assert.Equal(startIndex, args.OldStartingIndex); + Assert.Equal(startIndex, args.NewStartingIndex); + Assert.Equal(initialData.AsSpan(startIndex, count).ToArray(), args.OldItems.Cast().ToArray()); + Assert.Equal(dataToReplace, args.NewItems.Cast().ToArray()); + } + else + { + Assert.Equal(NotifyCollectionChangedAction.Reset, args.Action); + } } } } diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.netcoreapp.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.cs similarity index 100% rename from src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.netcoreapp.cs rename to src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_SkipCollectionChangedTests.cs diff --git a/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj b/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj index e8c0c28cab668d..8d42c79bc5de7d 100644 --- a/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj +++ b/src/libraries/System.ObjectModel/tests/System.ObjectModel.Tests.csproj @@ -17,8 +17,10 @@ Link="Common\System\CollectionsIDictionaryTest.cs" /> + + @@ -33,11 +35,8 @@ - - Common\System\Diagnostics\DebuggerAttributes.cs - - - + diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj index 8c12f46253dc79..94992d98003c11 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj @@ -70,7 +70,6 @@ - @@ -188,6 +187,7 @@ + diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.netcoreapp.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.RangeMethods.cs similarity index 95% rename from src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.netcoreapp.cs rename to src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.RangeMethods.cs index 940af677362623..6cc14aa3014ba9 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.netcoreapp.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Collections/ObjectModel/CollectionTests.RangeMethods.cs @@ -25,7 +25,6 @@ public void Collection_AddRange_ToEmpty_Test() collection.AddRange(expected); - Assert.NotNull(collection); Assert.Equal(expected.Length, collection.Count); Assert.Equal(expected, collection.ToArray()); } @@ -35,13 +34,10 @@ public void Collection_AddRange_ToExisting_Test() { int[] initial = new int[] { 1, 2, 3, 4 }; int[] dataToInsert = new int[] { 5, 6, 7, 8 }; - Collection collection = new Collection(); - for (int i = 0; i < initial.Length; i++) - collection.Add(initial[i]); + Collection collection = new Collection(initial); collection.AddRange(dataToInsert); - Assert.NotNull(collection); Assert.Equal(initial.Length + dataToInsert.Length, collection.Count); int[] collectionAssertion = collection.ToArray(); @@ -57,7 +53,6 @@ public void Collection_AddRange_Empty_Test() collection.AddRange(expected); - Assert.NotNull(collection); Assert.Equal(expected.Length, collection.Count); } @@ -90,7 +85,6 @@ public void Collection_InsertRange_Beginning_Test() collection.InsertRange(0, expected); - Assert.NotNull(collection); Assert.Equal(expectedLength, collection.Count); int[] collectionAssertion = collection.ToArray(); @@ -110,7 +104,6 @@ public void Collection_InsertRange_End_Test() collection.InsertRange(expected.Length - 1, expected); - Assert.NotNull(collection); Assert.Equal(expectedLength, collection.Count); int[] collectionAssertion = collection.ToArray(); @@ -130,7 +123,6 @@ public void Collection_InsertRange_Middle_Test() collection.InsertRange(2, expected); - Assert.NotNull(collection); Assert.Equal(expectedLength, collection.Count); int[] collectionAssertion = collection.ToArray(); @@ -147,7 +139,6 @@ public void Collection_InsertRange_Empty_Test() collection.InsertRange(0, new int[0]); - Assert.NotNull(collection); Assert.Equal(baseCollection.Count, collection.Count); Assert.Equal(baseCollection, collection.ToArray()); } @@ -221,7 +212,6 @@ public void Collection_RemoveRange_CountIsZero_Test() collection.RemoveRange(0, 0); - Assert.NotNull(collection); Assert.Equal(expected.Length, collection.Count); Assert.Equal(expected, collection.ToArray()); } @@ -246,7 +236,6 @@ public void Collection_RemoveRange_All_Test() collection.RemoveRange(0, 3); - Assert.NotNull(collection); Assert.Equal(0, collection.Count); } @@ -260,7 +249,6 @@ public void Collection_RemoveRange_FirstTwoItems_Test() collection.RemoveRange(0, 2); - Assert.NotNull(collection); Assert.Equal(1, collection.Count); Assert.Equal(3, collection[0]); } @@ -275,7 +263,6 @@ public void Collection_RemoveRange_LastTwoItems_Test() collection.RemoveRange(1, 2); - Assert.NotNull(collection); Assert.Equal(1, collection.Count); Assert.Equal(1, collection[0]); } @@ -290,7 +277,6 @@ public void Collection_RemoveRange_ZeroItems_Test() collection.RemoveRange(0, 0); - Assert.NotNull(collection); Assert.Equal(3, collection.Count); Assert.Equal(1, collection[0]); Assert.Equal(2, collection[1]); @@ -338,7 +324,6 @@ public void Collection_ReplaceRange_FirstTwo_Test() collection.ReplaceRange(0, 2, replace); - Assert.NotNull(collection); Assert.Equal(initial.Length + 2, collection.Count); int[] collectionAssertion = collection.ToArray(); @@ -357,7 +342,6 @@ public void Collection_ReplaceRange_LastTwo_Test() collection.ReplaceRange(2, 2, replace); - Assert.NotNull(collection); Assert.Equal(initial.Length + 2, collection.Count); int[] collectionAssertion = collection.ToArray(); @@ -376,7 +360,6 @@ public void Collection_ReplaceRange_MiddleTwo_Test() collection.ReplaceRange(1, 2, replace); - Assert.NotNull(collection); Assert.Equal(initial.Length + 2, collection.Count); Assert.Equal(initial[0], collection[0]); From 11624c615cc949ba876a2d5c1c85ac352a19d070 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Wed, 10 Jul 2024 10:12:29 +0200 Subject: [PATCH 7/8] ignore tests variants for "batch changes" for now --- .../ObjectModel/ObservableCollection.cs | 14 +++++++------- .../ObservableCollection_RangeMethodTests.cs | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs index c1db05381e83ee..95c802eaac121e 100644 --- a/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs +++ b/src/libraries/System.ObjectModel/src/System/Collections/ObjectModel/ObservableCollection.cs @@ -33,7 +33,7 @@ public class ObservableCollection : Collection, INotifyCollectionChanged, /// true to opt into raising with list /// of items when a range is inserted, removed or replaced. Instead of resets /// - private static bool RaiseRangeCollectionChangedEvents => true; + private static bool RaiseBatchCollectionChangedEvents => false; /// /// Initializes a new instance of ObservableCollection that is empty and has default initial capacity. @@ -138,7 +138,7 @@ protected override void RemoveItemsRange(int index, int count) { _skipRaisingEvents = true; - if (RaiseRangeCollectionChangedEvents && count > 0 && CollectionChanged is not null) + if (RaiseBatchCollectionChangedEvents && count > 0 && CollectionChanged is not null) { T[] removedItems = new T[count]; for (int i = 0; i < count; i++) @@ -187,12 +187,12 @@ protected override void ReplaceItemsRange(int index, int count, IEnumerable c } NotifyCollectionChangedEventArgs collectionChangedEventArgs = EventArgsCache.ResetCollectionChanged; - if (RaiseRangeCollectionChangedEvents && !skipEvents && CollectionChanged is not null) + if (!skipEvents && RaiseBatchCollectionChangedEvents && CollectionChanged is not null) { - T[] itemsToReplace = new T[count - index]; - for (int i = index; i < count; i++) + T[] itemsToReplace = new T[count]; + for (int i = 0; i < count; i++) { - itemsToReplace[i] = this[i]; + itemsToReplace[i] = this[i + index]; } IList newItems = collection as IList ?? new List(collection); @@ -276,7 +276,7 @@ protected override void InsertItemsRange(int index, IEnumerable collection) if (!_skipRaisingEvents) { NotifyCollectionChangedEventArgs collectionChangedEventArgs = EventArgsCache.ResetCollectionChanged; - if (RaiseRangeCollectionChangedEvents && CollectionChanged is not null) + if (RaiseBatchCollectionChangedEvents && CollectionChanged is not null) { IList newItems = collection as IList ?? new List(collection); collectionChangedEventArgs = new(NotifyCollectionChangedAction.Add, newItems, index); diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs index ccd764e46f182b..57f03a59c5f165 100644 --- a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs +++ b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs @@ -88,7 +88,7 @@ public static void AddRange_NotifyCollectionChanged_Test() } [Theory] - [InlineData(true)] + [InlineData(true, Skip = "Reenable when AppContext switch is added to opt into multiple items in NotifyCollectionChangedEventArgs")] [InlineData(false)] public static void AddRange_NotifyCollectionChanged_EventArgs_Test(bool batchCollectionChanged) { @@ -113,7 +113,7 @@ public static void AddRange_NotifyCollectionChanged_EventArgs_Test(bool batchCol } [Theory] - [InlineData(true)] + [InlineData(true, Skip = "Reenable when AppContext switch is added to opt into multiple items in NotifyCollectionChangedEventArgs")] [InlineData(false)] public static void InsertRange_NotifyCollectionChanged_EventArgs_Test(bool batchCollectionChanged) { @@ -138,7 +138,7 @@ public static void InsertRange_NotifyCollectionChanged_EventArgs_Test(bool batch } [Theory] - [InlineData(true)] + [InlineData(true, Skip = "Reenable when AppContext switch is added to opt into multiple items in NotifyCollectionChangedEventArgs")] [InlineData(false)] public static void InsertRange_NotifyCollectionChanged_EventArgs_Middle_Test(bool batchCollectionChanged) { @@ -168,7 +168,7 @@ public static void InsertRange_NotifyCollectionChanged_EventArgs_Middle_Test(boo } [Theory] - [InlineData(true)] + [InlineData(true, Skip = "Reenable when AppContext switch is added to opt into multiple items in NotifyCollectionChangedEventArgs")] [InlineData(false)] public static void InsertRange_NotifyCollectionChanged_EventArgs_End_Test(bool batchCollectionChanged) { @@ -260,7 +260,7 @@ public static void RemoveRange_NotifyCollectionChanged_IntMaxValueOverflow_Test( } [Theory] - [InlineData(true)] + [InlineData(true, Skip = "Reenable when AppContext switch is added to opt into multiple items in NotifyCollectionChangedEventArgs")] [InlineData(false)] public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexOfZero_Test(bool batchCollectionChanged) { @@ -288,7 +288,7 @@ public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexOfZero_Tes } [Theory] - [InlineData(true)] + [InlineData(true, Skip = "Reenable when AppContext switch is added to opt into multiple items in NotifyCollectionChangedEventArgs")] [InlineData(false)] public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexMiddle_Test(bool batchCollectionChanged) { @@ -318,7 +318,7 @@ public static void RemoveRange_NotifyCollectionChanged_EventArgs_IndexMiddle_Tes } [Theory] - [InlineData(true)] + [InlineData(true, Skip = "Reenable when AppContext switch is added to opt into multiple items in NotifyCollectionChangedEventArgs")] [InlineData(false)] public static void ReplaceRange_NotifyCollectionChanged_Test(bool batchCollectionChanged) { From 40d6ee86599d0fdd70da637e9d7188ab3db05a97 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Wed, 10 Jul 2024 12:30:19 +0200 Subject: [PATCH 8/8] Remove "partial" from test class --- .../ObservableCollection_RangeMethodTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs index 57f03a59c5f165..0e48394a6f6e9c 100644 --- a/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs +++ b/src/libraries/System.ObjectModel/tests/ObservableCollection/ObservableCollection_RangeMethodTests.cs @@ -8,7 +8,12 @@ namespace System.Collections.ObjectModel.Tests { - public partial class ObservableCollection_RangeMethodTests + /// + /// Tests the public Range modification methods in ObservableCollection as well as verifies + /// that the CollectionChanged events and eventargs are fired and populated + /// properly. + /// + public static class ObservableCollection_RangeMethodTests { [Fact] public static void InsertRange_NotifyCollectionChanged_Beginning_Test()