From 1cc25d02db8f1a640ca39f2698391b30ef75c88a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 30 Sep 2021 15:21:22 +0200 Subject: [PATCH] Merge pull request #6485 from AvaloniaUI/fixes/itemssourceview-fixes Tweaks to ItemsSourceView --- src/Avalonia.Controls/ItemsSourceView.cs | 98 ++++++++++++------- .../ItemsSourceViewTests.cs | 63 ++++++++++++ 2 files changed, 124 insertions(+), 37 deletions(-) create mode 100644 tests/Avalonia.Controls.UnitTests/ItemsSourceViewTests.cs diff --git a/src/Avalonia.Controls/ItemsSourceView.cs b/src/Avalonia.Controls/ItemsSourceView.cs index b2663f3213..c2d20495ef 100644 --- a/src/Avalonia.Controls/ItemsSourceView.cs +++ b/src/Avalonia.Controls/ItemsSourceView.cs @@ -32,8 +32,8 @@ namespace Avalonia.Controls /// public static ItemsSourceView Empty { get; } = new ItemsSourceView(Array.Empty()); - private protected readonly IList _inner; - private INotifyCollectionChanged? _notifyCollectionChanged; + private IList? _inner; + private NotifyCollectionChangedEventHandler? _collectionChanged; /// /// Initializes a new instance of the ItemsSourceView class for the specified data source. @@ -42,27 +42,22 @@ namespace Avalonia.Controls public ItemsSourceView(IEnumerable source) { source = source ?? throw new ArgumentNullException(nameof(source)); - - if (source is IList list) - { - _inner = list; - } - else if (source is IEnumerable objectEnumerable) + _inner = source switch { - _inner = new List(objectEnumerable); - } - else - { - _inner = new List(source.Cast()); - } - - ListenToCollectionChanges(); + ItemsSourceView _ => throw new ArgumentException("Cannot wrap an existing ItemsSourceView.", nameof(source)), + IList list => list, + INotifyCollectionChanged _ => throw new ArgumentException( + "Collection implements INotifyCollectionChanged by not IList.", + nameof(source)), + IEnumerable iObj => new List(iObj), + _ => new List(source.Cast()) + }; } /// /// Gets the number of items in the collection. /// - public int Count => _inner.Count; + public int Count => Inner.Count; /// /// Gets a value that indicates whether the items source can provide a unique key for each item. @@ -72,6 +67,19 @@ namespace Avalonia.Controls /// public bool HasKeyIndexMapping => false; + /// + /// Gets the inner collection. + /// + public IList Inner + { + get + { + if (_inner is null) + ThrowDisposed(); + return _inner!; + } + } + /// /// Retrieves the item at the specified index. /// @@ -82,15 +90,38 @@ namespace Avalonia.Controls /// /// Occurs when the collection has changed to indicate the reason for the change and which items changed. /// - public event NotifyCollectionChangedEventHandler? CollectionChanged; + public event NotifyCollectionChangedEventHandler? CollectionChanged + { + add + { + if (_collectionChanged is null && Inner is INotifyCollectionChanged incc) + { + incc.CollectionChanged += OnCollectionChanged; + } + + _collectionChanged += value; + } + + remove + { + _collectionChanged -= value; + + if (_collectionChanged is null && Inner is INotifyCollectionChanged incc) + { + incc.CollectionChanged -= OnCollectionChanged; + } + } + } /// public void Dispose() { - if (_notifyCollectionChanged != null) + if (_inner is INotifyCollectionChanged incc) { - _notifyCollectionChanged.CollectionChanged -= OnCollectionChanged; + incc.CollectionChanged -= OnCollectionChanged; } + + _inner = null; } /// @@ -98,9 +129,9 @@ namespace Avalonia.Controls /// /// The index. /// The item. - public object? GetAt(int index) => _inner[index]; + public object? GetAt(int index) => Inner[index]; - public int IndexOf(object? item) => _inner.IndexOf(item); + public int IndexOf(object? item) => Inner.IndexOf(item); public static ItemsSourceView GetOrCreate(IEnumerable? items) { @@ -146,7 +177,7 @@ namespace Avalonia.Controls internal void AddListener(ICollectionChangedListener listener) { - if (_inner is INotifyCollectionChanged incc) + if (Inner is INotifyCollectionChanged incc) { CollectionChangedEventManager.Instance.AddListener(incc, listener); } @@ -154,7 +185,7 @@ namespace Avalonia.Controls internal void RemoveListener(ICollectionChangedListener listener) { - if (_inner is INotifyCollectionChanged incc) + if (Inner is INotifyCollectionChanged incc) { CollectionChangedEventManager.Instance.RemoveListener(incc, listener); } @@ -162,22 +193,15 @@ namespace Avalonia.Controls protected void OnItemsSourceChanged(NotifyCollectionChangedEventArgs args) { - CollectionChanged?.Invoke(this, args); - } - - private void ListenToCollectionChanges() - { - if (_inner is INotifyCollectionChanged incc) - { - incc.CollectionChanged += OnCollectionChanged; - _notifyCollectionChanged = incc; - } + _collectionChanged?.Invoke(this, args); } private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { OnItemsSourceChanged(e); } + + private void ThrowDisposed() => throw new ObjectDisposedException(nameof(ItemsSourceView)); } public class ItemsSourceView : ItemsSourceView, IReadOnlyList @@ -216,10 +240,10 @@ namespace Avalonia.Controls /// The index. /// The item. [return: MaybeNull] - public new T GetAt(int index) => (T)_inner[index]; + public new T GetAt(int index) => (T)Inner[index]; - public IEnumerator GetEnumerator() => _inner.Cast().GetEnumerator(); - IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator(); + public IEnumerator GetEnumerator() => Inner.Cast().GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => Inner.GetEnumerator(); public static new ItemsSourceView GetOrCreate(IEnumerable? items) { diff --git a/tests/Avalonia.Controls.UnitTests/ItemsSourceViewTests.cs b/tests/Avalonia.Controls.UnitTests/ItemsSourceViewTests.cs new file mode 100644 index 0000000000..529b3b1aa8 --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/ItemsSourceViewTests.cs @@ -0,0 +1,63 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.Text; +using Avalonia.Collections; +using Avalonia.Diagnostics; +using Xunit; + +namespace Avalonia.Controls.UnitTests +{ + public class ItemsSourceViewTests + { + [Fact] + public void Only_Subscribes_To_Source_CollectionChanged_When_CollectionChanged_Subscribed() + { + var source = new AvaloniaList(); + var target = new ItemsSourceView(source); + var debug = (INotifyCollectionChangedDebug)source; + + Assert.Null(debug.GetCollectionChangedSubscribers()); + + void Handler(object sender, NotifyCollectionChangedEventArgs e) { } + target.CollectionChanged += Handler; + + Assert.NotNull(debug.GetCollectionChangedSubscribers()); + Assert.Equal(1, debug.GetCollectionChangedSubscribers().Length); + + target.CollectionChanged -= Handler; + + Assert.Null(debug.GetCollectionChangedSubscribers()); + } + + [Fact] + public void Cannot_Wrap_An_ItemsSourceView_In_Another() + { + var source = new ItemsSourceView(new string[0]); + Assert.Throws(() => new ItemsSourceView(source)); + } + + [Fact] + public void Cannot_Create_ItemsSourceView_With_Collection_That_Implements_INCC_But_Not_List() + { + var source = new InvalidCollection(); + Assert.Throws(() => new ItemsSourceView(source)); + } + + private class InvalidCollection : INotifyCollectionChanged, IEnumerable + { + public event NotifyCollectionChangedEventHandler CollectionChanged; + + public IEnumerator GetEnumerator() + { + yield break; + } + + IEnumerator IEnumerable.GetEnumerator() + { + yield break; + } + } + } +}