From d0c6430e749c581a970ad50599924b31aa781b32 Mon Sep 17 00:00:00 2001 From: Roland Pheasant Date: Mon, 4 Dec 2023 23:50:15 +0000 Subject: [PATCH 1/6] Include index =0 for inital change set on source list. Fixes #723 --- .../List/SourceListFixture.cs | 24 +++++++++++++++++++ src/DynamicData/List/SourceList.cs | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 src/DynamicData.Tests/List/SourceListFixture.cs diff --git a/src/DynamicData.Tests/List/SourceListFixture.cs b/src/DynamicData.Tests/List/SourceListFixture.cs new file mode 100644 index 000000000..dc37bdaed --- /dev/null +++ b/src/DynamicData.Tests/List/SourceListFixture.cs @@ -0,0 +1,24 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using FluentAssertions; +using Xunit; + +namespace DynamicData.Tests.List; + +public class SourceListFixture +{ + [Fact] + public void InitialChangeIsRange() + { + var source = new SourceList(); + source.Add("A"); + var changeSets = new List>(); + + source.Connect().Subscribe(changeSets.Add).Dispose(); + + + changeSets[0].First().Type.Should().Be(ChangeType.Range); + changeSets[0].First().Range.Index.Should().Be(0); + } +} diff --git a/src/DynamicData/List/SourceList.cs b/src/DynamicData/List/SourceList.cs index 8be553eb7..7494601f6 100644 --- a/src/DynamicData/List/SourceList.cs +++ b/src/DynamicData/List/SourceList.cs @@ -85,7 +85,7 @@ public IObservable> Connect(Func? predicate = null) observer.OnNext( new ChangeSet { - new(ListChangeReason.AddRange, _readerWriter.Items) + new(ListChangeReason.AddRange, _readerWriter.Items, 0) }); } From 91c6e5ca78127c54105ec6741187c544e152e6a1 Mon Sep 17 00:00:00 2001 From: Roland Pheasant Date: Tue, 5 Dec 2023 00:45:11 +0000 Subject: [PATCH 2/6] Add clone of AvaloniaDictionary to the test project for testing --- .../Binding/AvaloniaDictionaryFixture.cs | 278 ++++++++++++++++++ 1 file changed, 278 insertions(+) create mode 100644 src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs diff --git a/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs b/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs new file mode 100644 index 000000000..86baaea7e --- /dev/null +++ b/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs @@ -0,0 +1,278 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using DynamicData.Binding; +using DynamicData.Tests.Domain; +using FluentAssertions; +using Xunit; + +namespace DynamicData.Tests.Binding; + +public class AvaloniaDictionaryFixture +{ + private readonly AvaloniaDictionary _collection; + private readonly ChangeSetAggregator _results; + + public AvaloniaDictionaryFixture() + { + _collection = new AvaloniaDictionary(); + _results = _collection.ToObservableChangeSet, KeyValuePair>() + .Transform(x=>x.Value) + .AsAggregator(); + } + + [Fact] + public void Add() + { + var person = new Person("Someone",10, "M"); + + _collection.Add("Someone", person); + + _results.Messages.Count.Should().Be(1); + _results.Data.Count.Should().Be(1); + _results.Data.Items.First().Should().Be(person); + } + +} + + +public interface IAvaloniaDictionary + : IDictionary, + IAvaloniaReadOnlyDictionary, + IDictionary + where TKey : notnull +{ +} + + +public interface IAvaloniaReadOnlyDictionary + : IReadOnlyDictionary, + INotifyCollectionChanged, + INotifyPropertyChanged + where TKey : notnull +{ +} + +public class AvaloniaDictionary : IAvaloniaDictionary where TKey : notnull +{ + private Dictionary _inner; + + /// + /// Initializes a new instance of the class. + /// + public AvaloniaDictionary() + { + _inner = new Dictionary(); + } + + /// + /// Initializes a new instance of the class. + /// + public AvaloniaDictionary(int capacity) + { + _inner = new Dictionary(capacity); + } + + /// + /// Occurs when the collection changes. + /// + public event NotifyCollectionChangedEventHandler? CollectionChanged; + + /// + /// Raised when a property on the collection changes. + /// + public event PropertyChangedEventHandler? PropertyChanged; + + /// + public int Count => _inner.Count; + + /// + public bool IsReadOnly => false; + + /// + public ICollection Keys => _inner.Keys; + + /// + public ICollection Values => _inner.Values; + + bool IDictionary.IsFixedSize => ((IDictionary)_inner).IsFixedSize; + + ICollection IDictionary.Keys => ((IDictionary)_inner).Keys; + + ICollection IDictionary.Values => ((IDictionary)_inner).Values; + + bool ICollection.IsSynchronized => ((IDictionary)_inner).IsSynchronized; + + object ICollection.SyncRoot => ((IDictionary)_inner).SyncRoot; + + IEnumerable IReadOnlyDictionary.Keys => _inner.Keys; + + IEnumerable IReadOnlyDictionary.Values => _inner.Values; + + /// + /// Gets or sets the named resource. + /// + /// The resource key. + /// The resource, or null if not found. + public TValue this[TKey key] + { + get + { + return _inner[key]; + } + + set + { + bool replace = _inner.TryGetValue(key, out var old); + _inner[key] = value; + + if (replace) + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs($"{CommonPropertyNames.IndexerName}[{key}]")); + + if (CollectionChanged != null) + { + var e = new NotifyCollectionChangedEventArgs( + NotifyCollectionChangedAction.Replace, + new KeyValuePair(key, value), + new KeyValuePair(key, old!)); + CollectionChanged(this, e); + } + } + else + { + NotifyAdd(key, value); + } + } + } + + object? IDictionary.this[object key] { get => ((IDictionary)_inner)[key]; set => ((IDictionary)_inner)[key] = value; } + + /// + public void Add(TKey key, TValue value) + { + _inner.Add(key, value); + NotifyAdd(key, value); + } + + /// + public void Clear() + { + var old = _inner; + + _inner = new Dictionary(); + + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Count))); + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(CommonPropertyNames.IndexerName)); + + + if (CollectionChanged != null) + { + var e = new NotifyCollectionChangedEventArgs( + NotifyCollectionChangedAction.Remove, + old.ToArray(), + -1); + CollectionChanged(this, e); + } + } + + /// + public bool ContainsKey(TKey key) => _inner.ContainsKey(key); + + /// + public void CopyTo(KeyValuePair[] array, int arrayIndex) + { + ((IDictionary)_inner).CopyTo(array, arrayIndex); + } + + /// + public IEnumerator> GetEnumerator() => _inner.GetEnumerator(); + + /// + public bool Remove(TKey key) + { + if (_inner.TryGetValue(key, out var value)) + { + _inner.Remove(key); + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Count))); + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs($"{CommonPropertyNames.IndexerName}[{key}]")); + + if (CollectionChanged != null) + { + var e = new NotifyCollectionChangedEventArgs( + NotifyCollectionChangedAction.Remove, + new[] { new KeyValuePair(key, value) }, + -1); + CollectionChanged(this, e); + } + + return true; + } + else + { + return false; + } + } + + /// + public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value) => _inner.TryGetValue(key, out value); + /// + IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator(); + + /// + void ICollection.CopyTo(Array array, int index) => ((ICollection)_inner).CopyTo(array, index); + + /// + void ICollection>.Add(KeyValuePair item) + { + Add(item.Key, item.Value); + } + + /// + bool ICollection>.Contains(KeyValuePair item) + { + return _inner.Contains(item); + } + + /// + bool ICollection>.Remove(KeyValuePair item) + { + return Remove(item.Key); + } + + /// + void IDictionary.Add(object key, object? value) => Add((TKey)key, (TValue)value!); + + /// + bool IDictionary.Contains(object key) => ((IDictionary)_inner).Contains(key); + + /// + IDictionaryEnumerator IDictionary.GetEnumerator() => ((IDictionary)_inner).GetEnumerator(); + + /// + void IDictionary.Remove(object key) => Remove((TKey)key); + + private void NotifyAdd(TKey key, TValue value) + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Count))); + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs($"{CommonPropertyNames.IndexerName}[{key}]")); + + + if (CollectionChanged != null) + { + var e = new NotifyCollectionChangedEventArgs( + NotifyCollectionChangedAction.Add, + new[] { new KeyValuePair(key, value) }, + -1); + CollectionChanged(this, e); + } + } +} +internal static class CommonPropertyNames +{ + public const string IndexerName = "Item"; +} From b9c6a2092ce4483789020e7ce945eed06b346a3d Mon Sep 17 00:00:00 2001 From: Roland Pheasant Date: Tue, 5 Dec 2023 01:41:18 +0000 Subject: [PATCH 3/6] Add tests for Avalonia + throw the dreaded -1 exception. Fixes #710. Fixes #683 -> hopefully ! --- .editorconfig | 1 + .../Binding/AvaloniaDictionaryFixture.cs | 25 +++++++++++ .../Binding/ObservableCollectionEx.cs | 43 ++++++++++++++++--- .../Kernel/IndexMinusOneException.cs | 24 +++++++++++ 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 src/DynamicData/Kernel/IndexMinusOneException.cs diff --git a/.editorconfig b/.editorconfig index 0454e2555..dc24ddddc 100644 --- a/.editorconfig +++ b/.editorconfig @@ -699,6 +699,7 @@ csharp_style_namespace_declarations = block_scoped:silent csharp_style_prefer_method_group_conversion = true:silent csharp_style_prefer_top_level_statements = true:silent csharp_style_prefer_primary_constructors = true:suggestion +dotnet_diagnostic.RCS1194.severity = none # C++ Files [*.{cpp,h,in}] diff --git a/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs b/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs index 86baaea7e..e0a7ec40b 100644 --- a/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs +++ b/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs @@ -5,6 +5,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Linq; + using DynamicData.Binding; using DynamicData.Tests.Domain; using FluentAssertions; @@ -37,6 +38,30 @@ public void Add() _results.Data.Items.First().Should().Be(person); } + [Fact] + public void Replace() + { + var person1 = new Person("Someone", 10, "M"); + var person2 = new Person("Someone", 11, "M"); + + _collection.Add("Someone", person1); + _collection["Someone"] = person2; + + _results.Data.Count.Should().Be(1); + _results.Data.Items.First().Should().Be(person2); + } + + + [Fact] + public void Remove() + { + var person = new Person("Someone", 10, "M"); + + _collection.Add("Someone", person); + _collection.Remove(person.Key); + + _results.Data.Count.Should().Be(0); + } } diff --git a/src/DynamicData/Binding/ObservableCollectionEx.cs b/src/DynamicData/Binding/ObservableCollectionEx.cs index ccb137368..6bd844d1c 100644 --- a/src/DynamicData/Binding/ObservableCollectionEx.cs +++ b/src/DynamicData/Binding/ObservableCollectionEx.cs @@ -6,6 +6,7 @@ using System.Collections.Specialized; using System.Reactive; using System.Reactive.Linq; +using DynamicData.Kernel; namespace DynamicData.Binding; @@ -156,13 +157,17 @@ public static IObservable> ToObservableChangeSet(t { case NotifyCollectionChangedAction.Add when changes.NewItems is not null: { + var newIndex = changes.NewStartingIndex == -1 + ? list.Count + : changes.NewStartingIndex; + if (changes.NewItems.Count == 1 && changes.NewItems[0] is T item) { - list.Insert(changes.NewStartingIndex, item); + list.Insert(newIndex, item); } else { - list.InsertRange(changes.NewItems.Cast(), changes.NewStartingIndex); + list.InsertRange(changes.NewItems.Cast(), newIndex); } break; @@ -170,7 +175,14 @@ public static IObservable> ToObservableChangeSet(t case NotifyCollectionChangedAction.Remove when changes.OldItems is not null: { - if (changes.OldItems.Count == 1) + if (changes.OldStartingIndex == -1) + { + foreach (var item in changes.OldItems.Cast()) + { + list.Remove(item); + } + } + else if (changes.OldItems.Count == 1) { list.RemoveAt(changes.OldStartingIndex); } @@ -182,14 +194,35 @@ public static IObservable> ToObservableChangeSet(t break; } - case NotifyCollectionChangedAction.Replace when changes.NewItems is not null && changes.NewItems[0] is T replacedItem: - list[changes.NewStartingIndex] = replacedItem; + case NotifyCollectionChangedAction.Replace when changes.NewItems is not null && + changes.NewItems[0] is T replacedItem: + { + if (changes.NewStartingIndex == -1) + { + var original = changes.OldItems!.Cast().SingleOrDefault(); + var oldIndex = list.IndexOf(original!); + + list[oldIndex] = replacedItem; + } + else + { + list[changes.NewStartingIndex] = replacedItem; + } + } + break; case NotifyCollectionChangedAction.Reset: list.Clear(); list.AddRange(source); break; case NotifyCollectionChangedAction.Move: + + if (changes.OldStartingIndex == -1) + throw new IndexMinusOneException("Move -> OldStartingIndex"); + + if (changes.NewStartingIndex == -1) + throw new IndexMinusOneException("Move -> NewStartingIndex"); + list.Move(changes.OldStartingIndex, changes.NewStartingIndex); break; } diff --git a/src/DynamicData/Kernel/IndexMinusOneException.cs b/src/DynamicData/Kernel/IndexMinusOneException.cs new file mode 100644 index 000000000..019a92b2d --- /dev/null +++ b/src/DynamicData/Kernel/IndexMinusOneException.cs @@ -0,0 +1,24 @@ +// Copyright (c) 2011-2023 Roland Pheasant. All rights reserved. +// Roland Pheasant licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Runtime.CompilerServices; + +namespace DynamicData.Kernel; + +/// +public class IndexMinusOneException : Exception +{ + private const string DefaultMessage = "Index of minus one detected. Please raise an issue for DynamicData with an exact re-production"; + + /// + /// Initializes a new instance of the class. + /// + /// Some context. + /// The calling member. + /// The calling line number.. + public IndexMinusOneException(string context, [CallerMemberName] string? member = null, [CallerLineNumber] int line = 0) + : base($"{DefaultMessage}. Context={context}. Member={member}. Line={line}") + { + } +} From 6a3f8e1c6052557526bcfd6b0d5adc3d997d87ac Mon Sep 17 00:00:00 2001 From: Roland Pheasant Date: Tue, 5 Dec 2023 03:23:49 +0000 Subject: [PATCH 4/6] Dear Editor.Config, you've got to be kidding me, Telling me I should mark something with [Serializable] when that was a bad idea in 2004 and is totally evil in 2023. Binary formatting support required?? Please grow up and remove this as a suggestion to spare people the problem of having to turn it off. --- .editorconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/.editorconfig b/.editorconfig index dc24ddddc..9881231de 100644 --- a/.editorconfig +++ b/.editorconfig @@ -29,6 +29,7 @@ dotnet_style_prefer_compound_assignment = true:suggestion dotnet_style_prefer_simplified_interpolation = true:suggestion dotnet_style_namespace_match_folder = true:suggestion dotnet_style_prefer_collection_expression = true:suggestion +dotnet_diagnostic.CA2237.severity = none [project.json] indent_size = 2 From 77275856276b7b360fe3420ddaff54eca5607be2 Mon Sep 17 00:00:00 2001 From: Roland Pheasant Date: Tue, 5 Dec 2023 03:39:43 +0000 Subject: [PATCH 5/6] Add comment regarding avalonia dictionarytest --- .../Binding/AvaloniaDictionaryFixture.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs b/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs index e0a7ec40b..f069592f4 100644 --- a/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs +++ b/src/DynamicData.Tests/Binding/AvaloniaDictionaryFixture.cs @@ -82,6 +82,14 @@ public interface IAvaloniaReadOnlyDictionary { } + +/* + Copied from Avalionia because an issue was raised due to compatibility issues with ToObservableChangeSet(). + +There's not other way of testing it. + +See https://github.com/AvaloniaUI/Avalonia/blob/d7c82a1a6f7eb95b2214f20a281fa0581fb7b792/src/Avalonia.Base/Collections/AvaloniaDictionary.cs#L17 + */ public class AvaloniaDictionary : IAvaloniaDictionary where TKey : notnull { private Dictionary _inner; From dc92e449f92e13fbc4fcb8d14426afb3166ab3df Mon Sep 17 00:00:00 2001 From: Roland Pheasant Date: Tue, 5 Dec 2023 03:48:11 +0000 Subject: [PATCH 6/6] Use UnspecifiedIndexException --- .../Binding/ObservableCollectionEx.cs | 4 ++-- .../Kernel/IndexMinusOneException.cs | 24 ------------------- 2 files changed, 2 insertions(+), 26 deletions(-) delete mode 100644 src/DynamicData/Kernel/IndexMinusOneException.cs diff --git a/src/DynamicData/Binding/ObservableCollectionEx.cs b/src/DynamicData/Binding/ObservableCollectionEx.cs index 6bd844d1c..938847ec9 100644 --- a/src/DynamicData/Binding/ObservableCollectionEx.cs +++ b/src/DynamicData/Binding/ObservableCollectionEx.cs @@ -218,10 +218,10 @@ public static IObservable> ToObservableChangeSet(t case NotifyCollectionChangedAction.Move: if (changes.OldStartingIndex == -1) - throw new IndexMinusOneException("Move -> OldStartingIndex"); + throw new UnspecifiedIndexException("Move -> OldStartingIndex"); if (changes.NewStartingIndex == -1) - throw new IndexMinusOneException("Move -> NewStartingIndex"); + throw new UnspecifiedIndexException("Move -> NewStartingIndex"); list.Move(changes.OldStartingIndex, changes.NewStartingIndex); break; diff --git a/src/DynamicData/Kernel/IndexMinusOneException.cs b/src/DynamicData/Kernel/IndexMinusOneException.cs deleted file mode 100644 index 019a92b2d..000000000 --- a/src/DynamicData/Kernel/IndexMinusOneException.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) 2011-2023 Roland Pheasant. All rights reserved. -// Roland Pheasant licenses this file to you under the MIT license. -// See the LICENSE file in the project root for full license information. - -using System.Runtime.CompilerServices; - -namespace DynamicData.Kernel; - -/// -public class IndexMinusOneException : Exception -{ - private const string DefaultMessage = "Index of minus one detected. Please raise an issue for DynamicData with an exact re-production"; - - /// - /// Initializes a new instance of the class. - /// - /// Some context. - /// The calling member. - /// The calling line number.. - public IndexMinusOneException(string context, [CallerMemberName] string? member = null, [CallerLineNumber] int line = 0) - : base($"{DefaultMessage}. Context={context}. Member={member}. Line={line}") - { - } -}