From 5f1a003e4f4078549c45938284e90fb85d531d80 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Thu, 8 Aug 2019 14:11:44 +0200 Subject: [PATCH 1/6] Add failing unit test for TwoWay binding issue. --- .../Data/BindingTests.cs | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/tests/Avalonia.Markup.UnitTests/Data/BindingTests.cs b/tests/Avalonia.Markup.UnitTests/Data/BindingTests.cs index d19accb0ad..0ba06980af 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/BindingTests.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/BindingTests.cs @@ -60,6 +60,80 @@ namespace Avalonia.Markup.UnitTests.Data Assert.Equal("baz", source.Foo); } + [Fact] + public void TwoWay_Binding_Should_Be_Set_Up_GC_Collect() + { + var source = new WeakRefSource { Foo = null }; + var target = new TestControl { DataContext = source }; + + var binding = new Binding + { + Path = "Foo", + Mode = BindingMode.TwoWay + }; + + target.Bind(TestControl.ValueProperty, binding); + + var ref1 = AssignValue(target, "ref1"); + + Assert.Equal(ref1.Target, source.Foo); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + + var ref2 = AssignValue(target, "ref2"); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + + target.Value = null; + + Assert.Null(source.Foo); + } + + private class DummyObject : ICloneable + { + private readonly string _val; + + public DummyObject(string val) + { + _val = val; + } + + public object Clone() + { + return new DummyObject(_val); + } + + protected bool Equals(DummyObject other) + { + return string.Equals(_val, other._val); + } + + public override bool Equals(object obj) + { + if (ReferenceEquals(null, obj)) return false; + if (ReferenceEquals(this, obj)) return true; + if (obj.GetType() != this.GetType()) return false; + return Equals((DummyObject) obj); + } + + public override int GetHashCode() + { + return (_val != null ? _val.GetHashCode() : 0); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private WeakReference AssignValue(TestControl source, string val) + { + var obj = new DummyObject(val); + + source.Value = obj; + + return new WeakReference(obj); + } + [Fact] public void OneTime_Binding_Should_Be_Set_Up() { @@ -568,12 +642,70 @@ namespace Avalonia.Markup.UnitTests.Data } } + public class WeakRefSource : INotifyPropertyChanged + { + private WeakReference _foo; + + public object Foo + { + get + { + if (_foo == null) + { + return null; + } + + if (_foo.TryGetTarget(out object target)) + { + if (target is ICloneable cloneable) + { + return cloneable.Clone(); + } + + return target; + } + + return null; + } + set + { + _foo = new WeakReference(value); + + RaisePropertyChanged(); + } + } + + public event PropertyChangedEventHandler PropertyChanged; + + private void RaisePropertyChanged([CallerMemberName] string prop = "") + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(prop)); + } + } + private class OldDataContextViewModel { public int Foo { get; set; } = 1; public int Bar { get; set; } = 2; } + private class TestControl : Control + { + public static readonly DirectProperty ValueProperty = + AvaloniaProperty.RegisterDirect( + nameof(Value), + o => o.Value, + (o, v) => o.Value = v); + + private object _value; + + public object Value + { + get => _value; + set => SetAndRaise(ValueProperty, ref _value, value); + } + } + private class OldDataContextTest : Control { public static readonly StyledProperty FooProperty = From 868b5ea840094c6f701fc6295e9fd44e0f606196 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Thu, 8 Aug 2019 19:18:08 +0200 Subject: [PATCH 2/6] Update WeakReference usages to use the generic one. --- .../Data/Core/AvaloniaPropertyAccessorNode.cs | 6 ++-- src/Avalonia.Base/Data/Core/ExpressionNode.cs | 26 +++++++++-------- .../Data/Core/ExpressionObserver.cs | 8 +++--- .../Data/Core/IndexerExpressionNode.cs | 11 ++++++-- .../Data/Core/IndexerNodeBase.cs | 5 ++-- .../Plugins/AvaloniaPropertyAccessorPlugin.cs | 4 +-- .../DataAnnotationsValidationPlugin.cs | 23 ++++++++------- .../Core/Plugins/ExceptionValidationPlugin.cs | 8 +++--- .../Core/Plugins/IDataValidationPlugin.cs | 5 ++-- .../Core/Plugins/IPropertyAccessorPlugin.cs | 3 +- .../Data/Core/Plugins/IStreamPlugin.cs | 4 +-- .../Core/Plugins/IndeiValidationPlugin.cs | 28 +++++++++++++------ .../Plugins/InpcPropertyAccessorPlugin.cs | 23 +++++++++------ .../Data/Core/Plugins/MethodAccessorPlugin.cs | 21 ++++++++++---- .../Core/Plugins/ObservableStreamPlugin.cs | 12 ++++---- .../Data/Core/Plugins/TaskStreamPlugin.cs | 13 ++++++--- .../Data/Core/PropertyAccessorNode.cs | 6 ++-- src/Avalonia.Base/Data/Core/SettableNode.cs | 18 ++++++++++-- src/Avalonia.Base/Data/Core/StreamNode.cs | 2 +- src/Avalonia.Input/Gestures.cs | 12 ++++---- .../Markup/Parsers/Nodes/ElementNameNode.cs | 2 +- .../Markup/Parsers/Nodes/FindAncestorNode.cs | 4 +-- .../Markup/Parsers/Nodes/StringIndexerNode.cs | 26 +++++++++++------ .../Core/ExpressionObserverTests_Property.cs | 2 +- .../DataAnnotationsValidationPluginTests.cs | 14 +++++----- .../Plugins/ExceptionValidationPluginTests.cs | 4 +-- .../Plugins/IndeiValidationPluginTests.cs | 8 +++--- 27 files changed, 183 insertions(+), 115 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/AvaloniaPropertyAccessorNode.cs b/src/Avalonia.Base/Data/Core/AvaloniaPropertyAccessorNode.cs index 28c0dce518..0a33eeb2c1 100644 --- a/src/Avalonia.Base/Data/Core/AvaloniaPropertyAccessorNode.cs +++ b/src/Avalonia.Base/Data/Core/AvaloniaPropertyAccessorNode.cs @@ -24,7 +24,7 @@ namespace Avalonia.Data.Core { try { - if (Target.IsAlive && Target.Target is IAvaloniaObject obj) + if (Target.TryGetTarget(out object target) && target is IAvaloniaObject obj) { obj.SetValue(_property, value, priority); return true; @@ -37,9 +37,9 @@ namespace Avalonia.Data.Core } } - protected override void StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { - if (reference.Target is IAvaloniaObject obj) + if (reference.TryGetTarget(out object target) && target is IAvaloniaObject obj) { _subscription = new AvaloniaPropertyObservable(obj, _property).Subscribe(ValueChanged); } diff --git a/src/Avalonia.Base/Data/Core/ExpressionNode.cs b/src/Avalonia.Base/Data/Core/ExpressionNode.cs index 8a2dd46b86..ce40b3e517 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionNode.cs @@ -8,27 +8,27 @@ namespace Avalonia.Data.Core public abstract class ExpressionNode { private static readonly object CacheInvalid = new object(); - protected static readonly WeakReference UnsetReference = - new WeakReference(AvaloniaProperty.UnsetValue); + protected static readonly WeakReference UnsetReference = + new WeakReference(AvaloniaProperty.UnsetValue); - private WeakReference _target = UnsetReference; + private WeakReference _target = UnsetReference; private Action _subscriber; private bool _listening; - protected WeakReference LastValue { get; private set; } + protected WeakReference LastValue { get; private set; } public abstract string Description { get; } public ExpressionNode Next { get; set; } - public WeakReference Target + public WeakReference Target { get { return _target; } set { Contract.Requires(value != null); - var oldTarget = _target?.Target; - var newTarget = value.Target; + _target.TryGetTarget(out var oldTarget); + value.TryGetTarget(out object newTarget); if (!ReferenceEquals(oldTarget, newTarget)) { @@ -72,9 +72,11 @@ namespace Avalonia.Data.Core _subscriber = null; } - protected virtual void StartListeningCore(WeakReference reference) + protected virtual void StartListeningCore(WeakReference reference) { - ValueChanged(reference.Target); + reference.TryGetTarget(out object target); + + ValueChanged(target); } protected virtual void StopListeningCore() @@ -96,7 +98,7 @@ namespace Avalonia.Data.Core if (notification == null) { - LastValue = new WeakReference(value); + LastValue = new WeakReference(value); if (Next != null) { @@ -109,7 +111,7 @@ namespace Avalonia.Data.Core } else { - LastValue = new WeakReference(notification.Value); + LastValue = new WeakReference(notification.Value); if (Next != null) { @@ -125,7 +127,7 @@ namespace Avalonia.Data.Core private void StartListening() { - var target = _target.Target; + _target.TryGetTarget(out object target); if (target == null) { diff --git a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs index 65f26df011..7060fd3451 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs @@ -78,7 +78,7 @@ namespace Avalonia.Data.Core _node = node; Description = description; - _root = new WeakReference(root); + _root = new WeakReference(root); } /// @@ -120,7 +120,7 @@ namespace Avalonia.Data.Core Contract.Requires(update != null); Description = description; _node = node; - _node.Target = new WeakReference(rootGetter()); + _node.Target = new WeakReference(rootGetter()); _root = update.Select(x => rootGetter()); } @@ -285,13 +285,13 @@ namespace Avalonia.Data.Core if (_root is IObservable observable) { _rootSubscription = observable.Subscribe( - x => _node.Target = new WeakReference(x != AvaloniaProperty.UnsetValue ? x : null), + x => _node.Target = new WeakReference(x != AvaloniaProperty.UnsetValue ? x : null), x => PublishCompleted(), () => PublishCompleted()); } else { - _node.Target = (WeakReference)_root; + _node.Target = (WeakReference)_root; } } diff --git a/src/Avalonia.Base/Data/Core/IndexerExpressionNode.cs b/src/Avalonia.Base/Data/Core/IndexerExpressionNode.cs index 4206a99e3d..a3852cc371 100644 --- a/src/Avalonia.Base/Data/Core/IndexerExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/IndexerExpressionNode.cs @@ -36,7 +36,9 @@ namespace Avalonia.Data.Core { try { - _setDelegate.DynamicInvoke(Target.Target, value); + Target.TryGetTarget(out object target); + + _setDelegate.DynamicInvoke(target, value); return true; } catch (Exception) @@ -64,6 +66,11 @@ namespace Avalonia.Data.Core return _expression.Indexer == null || _expression.Indexer.Name == e.PropertyName; } - protected override int? TryGetFirstArgumentAsInt() => _firstArgumentDelegate.DynamicInvoke(Target.Target) as int?; + protected override int? TryGetFirstArgumentAsInt() + { + Target.TryGetTarget(out object target); + + return _firstArgumentDelegate.DynamicInvoke(target) as int?; + } } } diff --git a/src/Avalonia.Base/Data/Core/IndexerNodeBase.cs b/src/Avalonia.Base/Data/Core/IndexerNodeBase.cs index 5e09bbcc2f..47d5147ac2 100644 --- a/src/Avalonia.Base/Data/Core/IndexerNodeBase.cs +++ b/src/Avalonia.Base/Data/Core/IndexerNodeBase.cs @@ -13,9 +13,10 @@ namespace Avalonia.Data.Core { private IDisposable _subscription; - protected override void StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { - var target = reference.Target; + reference.TryGetTarget(out object target); + var incc = target as INotifyCollectionChanged; var inpc = target as INotifyPropertyChanged; var inputs = new List>(); diff --git a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs index 8d2ed905ee..ab4a109cc2 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs @@ -31,12 +31,12 @@ namespace Avalonia.Data.Core.Plugins /// An interface through which future interactions with the /// property will be made. /// - public IPropertyAccessor Start(WeakReference reference, string propertyName) + public IPropertyAccessor Start(WeakReference reference, string propertyName) { Contract.Requires(reference != null); Contract.Requires(propertyName != null); - var instance = reference.Target; + reference.TryGetTarget(out object instance); var o = (AvaloniaObject)instance; var p = LookupProperty(o, propertyName); diff --git a/src/Avalonia.Base/Data/Core/Plugins/DataAnnotationsValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/DataAnnotationsValidationPlugin.cs index 33c40abea8..f5b545d2ff 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/DataAnnotationsValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/DataAnnotationsValidationPlugin.cs @@ -15,9 +15,11 @@ namespace Avalonia.Data.Core.Plugins public class DataAnnotationsValidationPlugin : IDataValidationPlugin { /// - public bool Match(WeakReference reference, string memberName) + public bool Match(WeakReference reference, string memberName) { - return reference.Target? + reference.TryGetTarget(out object target); + + return target? .GetType() .GetRuntimeProperty(memberName)? .GetCustomAttributes() @@ -25,25 +27,22 @@ namespace Avalonia.Data.Core.Plugins } /// - public IPropertyAccessor Start(WeakReference reference, string name, IPropertyAccessor inner) + public IPropertyAccessor Start(WeakReference reference, string name, IPropertyAccessor inner) { return new Accessor(reference, name, inner); } - private class Accessor : DataValidationBase + private sealed class Accessor : DataValidationBase { - private ValidationContext _context; + private readonly ValidationContext _context; - public Accessor(WeakReference reference, string name, IPropertyAccessor inner) + public Accessor(WeakReference reference, string name, IPropertyAccessor inner) : base(inner) { - _context = new ValidationContext(reference.Target); - _context.MemberName = name; - } + reference.TryGetTarget(out object target); - public override bool SetValue(object value, BindingPriority priority) - { - return base.SetValue(value, priority); + _context = new ValidationContext(target); + _context.MemberName = name; } protected override void InnerValueChanged(object value) diff --git a/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs index eabfa31d4b..f305912fe1 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/ExceptionValidationPlugin.cs @@ -12,17 +12,17 @@ namespace Avalonia.Data.Core.Plugins public class ExceptionValidationPlugin : IDataValidationPlugin { /// - public bool Match(WeakReference reference, string memberName) => true; + public bool Match(WeakReference reference, string memberName) => true; /// - public IPropertyAccessor Start(WeakReference reference, string name, IPropertyAccessor inner) + public IPropertyAccessor Start(WeakReference reference, string name, IPropertyAccessor inner) { return new Validator(reference, name, inner); } - private class Validator : DataValidationBase + private sealed class Validator : DataValidationBase { - public Validator(WeakReference reference, string name, IPropertyAccessor inner) + public Validator(WeakReference reference, string name, IPropertyAccessor inner) : base(inner) { } diff --git a/src/Avalonia.Base/Data/Core/Plugins/IDataValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/IDataValidationPlugin.cs index 2c3a9a53b4..5b1af22f14 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IDataValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IDataValidationPlugin.cs @@ -16,7 +16,7 @@ namespace Avalonia.Data.Core.Plugins /// A weak reference to the object. /// The name of the member to validate. /// True if the plugin can handle the object; otherwise false. - bool Match(WeakReference reference, string memberName); + bool Match(WeakReference reference, string memberName); /// /// Starts monitoring the data validation state of a property on an object. @@ -28,8 +28,7 @@ namespace Avalonia.Data.Core.Plugins /// An interface through which future interactions with the /// property will be made. /// - IPropertyAccessor Start( - WeakReference reference, + IPropertyAccessor Start(WeakReference reference, string propertyName, IPropertyAccessor inner); } diff --git a/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessorPlugin.cs index 539f518083..a0021fa4d4 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IPropertyAccessorPlugin.cs @@ -28,8 +28,7 @@ namespace Avalonia.Data.Core.Plugins /// An interface through which future interactions with the /// property will be made. /// - IPropertyAccessor Start( - WeakReference reference, + IPropertyAccessor Start(WeakReference reference, string propertyName); } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/IStreamPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/IStreamPlugin.cs index b80d9d75c8..3df578d25b 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IStreamPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IStreamPlugin.cs @@ -15,7 +15,7 @@ namespace Avalonia.Data.Core.Plugins /// /// A weak reference to the value. /// True if the plugin can handle the value; otherwise false. - bool Match(WeakReference reference); + bool Match(WeakReference reference); /// /// Starts producing output based on the specified value. @@ -24,6 +24,6 @@ namespace Avalonia.Data.Core.Plugins /// /// An observable that produces the output for the value. /// - IObservable Start(WeakReference reference); + IObservable Start(WeakReference reference); } } diff --git a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs index 2e83e0c25e..e1f6d1590d 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs @@ -15,20 +15,25 @@ namespace Avalonia.Data.Core.Plugins public class IndeiValidationPlugin : IDataValidationPlugin { /// - public bool Match(WeakReference reference, string memberName) => reference.Target is INotifyDataErrorInfo; + public bool Match(WeakReference reference, string memberName) + { + reference.TryGetTarget(out object target); + + return target is INotifyDataErrorInfo; + } /// - public IPropertyAccessor Start(WeakReference reference, string name, IPropertyAccessor accessor) + public IPropertyAccessor Start(WeakReference reference, string name, IPropertyAccessor accessor) { return new Validator(reference, name, accessor); } private class Validator : DataValidationBase, IWeakSubscriber { - WeakReference _reference; - string _name; + private readonly WeakReference _reference; + private readonly string _name; - public Validator(WeakReference reference, string name, IPropertyAccessor inner) + public Validator(WeakReference reference, string name, IPropertyAccessor inner) : base(inner) { _reference = reference; @@ -45,7 +50,7 @@ namespace Avalonia.Data.Core.Plugins protected override void SubscribeCore() { - var target = _reference.Target as INotifyDataErrorInfo; + var target = GetReferenceTarget() as INotifyDataErrorInfo; if (target != null) { @@ -60,7 +65,7 @@ namespace Avalonia.Data.Core.Plugins protected override void UnsubscribeCore() { - var target = _reference.Target as INotifyDataErrorInfo; + var target = GetReferenceTarget() as INotifyDataErrorInfo; if (target != null) { @@ -80,7 +85,7 @@ namespace Avalonia.Data.Core.Plugins private BindingNotification CreateBindingNotification(object value) { - var target = (INotifyDataErrorInfo)_reference.Target; + var target = (INotifyDataErrorInfo)GetReferenceTarget(); if (target != null) { @@ -100,6 +105,13 @@ namespace Avalonia.Data.Core.Plugins return new BindingNotification(value); } + private object GetReferenceTarget() + { + _reference.TryGetTarget(out object target); + + return target; + } + private Exception GenerateException(IList errors) { if (errors.Count == 1) diff --git a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs index 4047489ccc..4716b45340 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs @@ -28,12 +28,12 @@ namespace Avalonia.Data.Core.Plugins /// An interface through which future interactions with the /// property will be made. /// - public IPropertyAccessor Start(WeakReference reference, string propertyName) + public IPropertyAccessor Start(WeakReference reference, string propertyName) { Contract.Requires(reference != null); Contract.Requires(propertyName != null); - var instance = reference.Target; + reference.TryGetTarget(out object instance); var p = instance.GetType().GetRuntimeProperties().FirstOrDefault(x => x.Name == propertyName); if (p != null) @@ -50,11 +50,11 @@ namespace Avalonia.Data.Core.Plugins private class Accessor : PropertyAccessorBase, IWeakSubscriber { - private readonly WeakReference _reference; + private readonly WeakReference _reference; private readonly PropertyInfo _property; private bool _eventRaised; - public Accessor(WeakReference reference, PropertyInfo property) + public Accessor(WeakReference reference, PropertyInfo property) { Contract.Requires(reference != null); Contract.Requires(property != null); @@ -69,7 +69,7 @@ namespace Avalonia.Data.Core.Plugins { get { - var o = _reference.Target; + var o = GetReferenceTarget(); return (o != null) ? _property.GetValue(o) : null; } } @@ -79,7 +79,7 @@ namespace Avalonia.Data.Core.Plugins if (_property.CanWrite) { _eventRaised = false; - _property.SetValue(_reference.Target, value); + _property.SetValue(GetReferenceTarget(), value); if (!_eventRaised) { @@ -109,7 +109,7 @@ namespace Avalonia.Data.Core.Plugins protected override void UnsubscribeCore() { - var inpc = _reference.Target as INotifyPropertyChanged; + var inpc = GetReferenceTarget() as INotifyPropertyChanged; if (inpc != null) { @@ -120,6 +120,13 @@ namespace Avalonia.Data.Core.Plugins } } + private object GetReferenceTarget() + { + _reference.TryGetTarget(out object target); + + return target; + } + private void SendCurrentValue() { try @@ -132,7 +139,7 @@ namespace Avalonia.Data.Core.Plugins private void SubscribeToChanges() { - var inpc = _reference.Target as INotifyPropertyChanged; + var inpc = GetReferenceTarget() as INotifyPropertyChanged; if (inpc != null) { diff --git a/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs index e48c671a13..c19ee8dba7 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/MethodAccessorPlugin.cs @@ -9,12 +9,12 @@ namespace Avalonia.Data.Core.Plugins public bool Match(object obj, string methodName) => obj.GetType().GetRuntimeMethods().Any(x => x.Name == methodName); - public IPropertyAccessor Start(WeakReference reference, string methodName) + public IPropertyAccessor Start(WeakReference reference, string methodName) { Contract.Requires(reference != null); Contract.Requires(methodName != null); - var instance = reference.Target; + reference.TryGetTarget(out object instance); var method = instance.GetType().GetRuntimeMethods().FirstOrDefault(x => x.Name == methodName); if (method != null) @@ -35,9 +35,9 @@ namespace Avalonia.Data.Core.Plugins } } - private class Accessor : PropertyAccessorBase + private sealed class Accessor : PropertyAccessorBase { - public Accessor(WeakReference reference, MethodInfo method) + public Accessor(WeakReference reference, MethodInfo method) { Contract.Requires(reference != null); Contract.Requires(method != null); @@ -61,8 +61,17 @@ namespace Avalonia.Data.Core.Plugins var genericTypeParameters = paramTypes.Concat(new[] { returnType }).ToArray(); PropertyType = Type.GetType($"System.Func`{genericTypeParameters.Length}").MakeGenericType(genericTypeParameters); } - - Value = method.IsStatic ? method.CreateDelegate(PropertyType) : method.CreateDelegate(PropertyType, reference.Target); + + if (method.IsStatic) + { + Value = method.CreateDelegate(PropertyType); + } + else + { + reference.TryGetTarget(out object target); + + Value = method.CreateDelegate(PropertyType, target); + } } public override Type PropertyType { get; } diff --git a/src/Avalonia.Base/Data/Core/Plugins/ObservableStreamPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/ObservableStreamPlugin.cs index c41097c274..ef5ce05821 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/ObservableStreamPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/ObservableStreamPlugin.cs @@ -20,9 +20,11 @@ namespace Avalonia.Data.Core.Plugins /// /// A weak reference to the value. /// True if the plugin can handle the value; otherwise false. - public virtual bool Match(WeakReference reference) + public virtual bool Match(WeakReference reference) { - return reference.Target.GetType().GetInterfaces().Any(x => + reference.TryGetTarget(out object target); + + return target != null && target.GetType().GetInterfaces().Any(x => x.IsGenericType && x.GetGenericTypeDefinition() == typeof(IObservable<>)); } @@ -34,9 +36,9 @@ namespace Avalonia.Data.Core.Plugins /// /// An observable that produces the output for the value. /// - public virtual IObservable Start(WeakReference reference) + public virtual IObservable Start(WeakReference reference) { - var target = reference.Target; + reference.TryGetTarget(out object target); // If the observable returns a reference type then we can cast it. if (target is IObservable result) @@ -46,7 +48,7 @@ namespace Avalonia.Data.Core.Plugins // If the observable returns a value type then we need to call Observable.Select on it. // First get the type of T in `IObservable`. - var sourceType = reference.Target.GetType().GetInterfaces().First(x => + var sourceType = target.GetType().GetInterfaces().First(x => x.IsGenericType && x.GetGenericTypeDefinition() == typeof(IObservable<>)).GetGenericArguments()[0]; diff --git a/src/Avalonia.Base/Data/Core/Plugins/TaskStreamPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/TaskStreamPlugin.cs index 16862f576d..a3d2714747 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/TaskStreamPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/TaskStreamPlugin.cs @@ -19,7 +19,12 @@ namespace Avalonia.Data.Core.Plugins /// /// A weak reference to the value. /// True if the plugin can handle the value; otherwise false. - public virtual bool Match(WeakReference reference) => reference.Target is Task; + public virtual bool Match(WeakReference reference) + { + reference.TryGetTarget(out object target); + + return target is Task; + } /// /// Starts producing output based on the specified value. @@ -28,11 +33,11 @@ namespace Avalonia.Data.Core.Plugins /// /// An observable that produces the output for the value. /// - public virtual IObservable Start(WeakReference reference) + public virtual IObservable Start(WeakReference reference) { - var task = reference.Target as Task; + reference.TryGetTarget(out object target); - if (task != null) + if (target is Task task) { var resultProperty = task.GetType().GetRuntimeProperty("Result"); diff --git a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs index df8f46a7d7..70f53b8b88 100644 --- a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs +++ b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs @@ -37,9 +37,11 @@ namespace Avalonia.Data.Core return false; } - protected override void StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { - var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(reference.Target, PropertyName)); + reference.TryGetTarget(out object target); + + var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(target, PropertyName)); var accessor = plugin?.Start(reference, PropertyName); if (_enableValidation && Next == null) diff --git a/src/Avalonia.Base/Data/Core/SettableNode.cs b/src/Avalonia.Base/Data/Core/SettableNode.cs index 7c839acb78..eb98b9e8d6 100644 --- a/src/Avalonia.Base/Data/Core/SettableNode.cs +++ b/src/Avalonia.Base/Data/Core/SettableNode.cs @@ -19,11 +19,25 @@ namespace Avalonia.Data.Core { return false; } + + if (LastValue == null) + { + return false; + } + + bool isLastValueAlive = LastValue.TryGetTarget(out object lastValue); + + if (!isLastValueAlive) + { + return false; + } + if (PropertyType.IsValueType) { - return LastValue?.Target != null && LastValue.Target.Equals(value); + return lastValue.Equals(value); } - return LastValue != null && Object.ReferenceEquals(LastValue?.Target, value); + + return ReferenceEquals(lastValue, value); } protected abstract bool SetTargetValueCore(object value, BindingPriority priority); diff --git a/src/Avalonia.Base/Data/Core/StreamNode.cs b/src/Avalonia.Base/Data/Core/StreamNode.cs index 6fc178e7f8..183e0662aa 100644 --- a/src/Avalonia.Base/Data/Core/StreamNode.cs +++ b/src/Avalonia.Base/Data/Core/StreamNode.cs @@ -12,7 +12,7 @@ namespace Avalonia.Data.Core public override string Description => "^"; - protected override void StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { foreach (var plugin in ExpressionObserver.StreamHandlers) { diff --git a/src/Avalonia.Input/Gestures.cs b/src/Avalonia.Input/Gestures.cs index 02dda45e99..bb8c8b8c40 100644 --- a/src/Avalonia.Input/Gestures.cs +++ b/src/Avalonia.Input/Gestures.cs @@ -31,7 +31,7 @@ namespace Avalonia.Input RoutedEvent.Register( "ScrollGestureEnded", RoutingStrategies.Bubble, typeof(Gestures)); - private static WeakReference s_lastPress; + private static WeakReference s_lastPress; static Gestures() { @@ -47,11 +47,11 @@ namespace Avalonia.Input if (e.ClickCount <= 1) { - s_lastPress = new WeakReference(e.Source); + s_lastPress = new WeakReference(e.Source); } - else if (s_lastPress?.IsAlive == true && e.ClickCount == 2 && s_lastPress.Target == e.Source) + else if (s_lastPress != null && e.ClickCount == 2 && e.MouseButton != MouseButton.Right) { - if (e.MouseButton != MouseButton.Right) + if (s_lastPress.TryGetTarget(out var target) && target == e.Source) { e.Source.RaiseEvent(new RoutedEventArgs(DoubleTappedEvent)); } @@ -65,10 +65,10 @@ namespace Avalonia.Input { var e = (PointerReleasedEventArgs)ev; - if (s_lastPress?.IsAlive == true && s_lastPress.Target == e.Source) + if (s_lastPress.TryGetTarget(out var target) && target == e.Source) { var et = e.MouseButton != MouseButton.Right ? TappedEvent : RightTappedEvent; - ((IInteractive)s_lastPress.Target).RaiseEvent(new RoutedEventArgs(et)); + e.Source.RaiseEvent(new RoutedEventArgs(et)); } } } diff --git a/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/ElementNameNode.cs b/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/ElementNameNode.cs index 981e93c534..7eec80fc00 100644 --- a/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/ElementNameNode.cs +++ b/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/ElementNameNode.cs @@ -19,7 +19,7 @@ namespace Avalonia.Markup.Parsers.Nodes public override string Description => $"#{_name}"; - protected override void StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { if (_nameScope.TryGetTarget(out var scope)) _subscription = NameScopeLocator.Track(scope, _name).Subscribe(ValueChanged); diff --git a/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/FindAncestorNode.cs b/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/FindAncestorNode.cs index 221df44327..321a85c1d7 100644 --- a/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/FindAncestorNode.cs +++ b/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/FindAncestorNode.cs @@ -31,9 +31,9 @@ namespace Avalonia.Markup.Parsers.Nodes } } - protected override void StartListeningCore(WeakReference reference) + protected override void StartListeningCore(WeakReference reference) { - if (reference.Target is ILogical logical) + if (reference.TryGetTarget(out object target) && target is ILogical logical) { _subscription = ControlLocator.Track(logical, _level, _ancestorType).Subscribe(ValueChanged); } diff --git a/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/StringIndexerNode.cs b/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/StringIndexerNode.cs index ea847bde11..a11879238b 100644 --- a/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/StringIndexerNode.cs +++ b/src/Markup/Avalonia.Markup/Markup/Parsers/Nodes/StringIndexerNode.cs @@ -26,9 +26,11 @@ namespace Avalonia.Markup.Parsers.Nodes protected override bool SetTargetValueCore(object value, BindingPriority priority) { - var typeInfo = Target.Target.GetType().GetTypeInfo(); - var list = Target.Target as IList; - var dictionary = Target.Target as IDictionary; + Target.TryGetTarget(out object target); + + var typeInfo = target.GetType().GetTypeInfo(); + var list = target as IList; + var dictionary = target as IDictionary; var indexerProperty = GetIndexer(typeInfo); var indexerParameters = indexerProperty?.GetIndexParameters(); @@ -53,7 +55,7 @@ namespace Avalonia.Markup.Parsers.Nodes // Try special cases where we can validate indices if (typeInfo.IsArray) { - return SetValueInArray((Array)Target.Target, intArgs, value); + return SetValueInArray((Array)target, intArgs, value); } else if (Arguments.Count == 1) { @@ -83,14 +85,14 @@ namespace Avalonia.Markup.Parsers.Nodes else { // Fallback to unchecked access - indexerProperty.SetValue(Target.Target, value, convertedObjectArray); + indexerProperty.SetValue(target, value, convertedObjectArray); return true; } } else { // Fallback to unchecked access - indexerProperty.SetValue(Target.Target, value, convertedObjectArray); + indexerProperty.SetValue(target, value, convertedObjectArray); return true; } } @@ -98,7 +100,7 @@ namespace Avalonia.Markup.Parsers.Nodes // multidimensional indexer, which doesn't take the same number of arguments else if (typeInfo.IsArray) { - SetValueInArray((Array)Target.Target, value); + SetValueInArray((Array)target, value); return true; } return false; @@ -126,7 +128,15 @@ namespace Avalonia.Markup.Parsers.Nodes public IList Arguments { get; } - public override Type PropertyType => GetIndexer(Target.Target.GetType().GetTypeInfo())?.PropertyType; + public override Type PropertyType + { + get + { + Target.TryGetTarget(out object target); + + return GetIndexer(target.GetType().GetTypeInfo())?.PropertyType; + } + } protected override object GetValue(object target) { diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs index b56afa33a4..a2ef8eedad 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_Property.cs @@ -574,7 +574,7 @@ namespace Avalonia.Base.UnitTests.Data.Core var source = new Class1 { Foo = "foo" }; var target = new PropertyAccessorNode("Foo", false); Assert.NotNull(target); - target.Target = new WeakReference(source); + target.Target = new WeakReference(source); target.Subscribe(_ => { }); target.Unsubscribe(); target.Unsubscribe(); diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/DataAnnotationsValidationPluginTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/DataAnnotationsValidationPluginTests.cs index 378c225e23..435ead0b80 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/DataAnnotationsValidationPluginTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/DataAnnotationsValidationPluginTests.cs @@ -20,7 +20,7 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins var target = new DataAnnotationsValidationPlugin(); var data = new Data(); - Assert.True(target.Match(new WeakReference(data), nameof(Data.Between5And10))); + Assert.True(target.Match(new WeakReference(data), nameof(Data.Between5And10))); } [Fact] @@ -29,7 +29,7 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins var target = new DataAnnotationsValidationPlugin(); var data = new Data(); - Assert.True(target.Match(new WeakReference(data), nameof(Data.PhoneNumber))); + Assert.True(target.Match(new WeakReference(data), nameof(Data.PhoneNumber))); } [Fact] @@ -38,7 +38,7 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins var target = new DataAnnotationsValidationPlugin(); var data = new Data(); - Assert.False(target.Match(new WeakReference(data), nameof(Data.Unvalidated))); + Assert.False(target.Match(new WeakReference(data), nameof(Data.Unvalidated))); } [Fact] @@ -47,8 +47,8 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); var validatorPlugin = new DataAnnotationsValidationPlugin(); var data = new Data(); - var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.Between5And10)); - var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Between5And10), accessor); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.Between5And10)); + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Between5And10), accessor); var result = new List(); var errmsg = new RangeAttribute(5, 10).FormatErrorMessage(nameof(Data.Between5And10)); @@ -79,8 +79,8 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); var validatorPlugin = new DataAnnotationsValidationPlugin(); var data = new Data(); - var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.PhoneNumber)); - var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.PhoneNumber), accessor); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.PhoneNumber)); + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.PhoneNumber), accessor); var result = new List(); validator.Subscribe(x => result.Add(x)); diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/ExceptionValidationPluginTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/ExceptionValidationPluginTests.cs index 2a307f9a61..6bd5fe5093 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/ExceptionValidationPluginTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/ExceptionValidationPluginTests.cs @@ -19,8 +19,8 @@ namespace Avalonia.Base.UnitTests.Data.Core.Plugins var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); var validatorPlugin = new ExceptionValidationPlugin(); var data = new Data(); - var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive)); - var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), accessor); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive)); + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), accessor); var result = new List(); validator.Subscribe(x => result.Add(x)); diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs index 383030cb6c..db0f5b0c77 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs @@ -18,8 +18,8 @@ namespace Avalonia.Base.UnitTests.Data.Core.Plugins var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); var validatorPlugin = new IndeiValidationPlugin(); var data = new Data { Maximum = 5 }; - var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.Value)); - var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Value), accessor); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.Value)); + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Value), accessor); var result = new List(); validator.Subscribe(x => result.Add(x)); @@ -53,8 +53,8 @@ namespace Avalonia.Base.UnitTests.Data.Core.Plugins var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); var validatorPlugin = new IndeiValidationPlugin(); var data = new Data { Maximum = 5 }; - var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.Value)); - var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Value), accessor); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.Value)); + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.Value), accessor); Assert.Equal(0, data.ErrorsChangedSubscriptionCount); validator.Subscribe(_ => { }); From 78866d54ed1ce78df089eee8fe1387c0ef0c989a Mon Sep 17 00:00:00 2001 From: "all.owing" Date: Wed, 21 Aug 2019 22:51:59 +0500 Subject: [PATCH 3/6] Allowed pass object as validation error --- .../Core/Plugins/IndeiValidationPlugin.cs | 11 +++++---- .../Data/DataValidationException.cs | 21 ++++++++++++++++ src/Avalonia.Controls/DataValidationErrors.cs | 24 ++++++++++++------- 3 files changed, 43 insertions(+), 13 deletions(-) create mode 100644 src/Avalonia.Base/Data/DataValidationException.cs diff --git a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs index 2e83e0c25e..7abbcab245 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/IndeiValidationPlugin.cs @@ -85,8 +85,9 @@ namespace Avalonia.Data.Core.Plugins if (target != null) { var errors = target.GetErrors(_name)? - .Cast() - .Where(x => x != null).ToList(); + .Cast() + .Where(x => x != null) + .ToList(); if (errors?.Count > 0) { @@ -100,16 +101,16 @@ namespace Avalonia.Data.Core.Plugins return new BindingNotification(value); } - private Exception GenerateException(IList errors) + private Exception GenerateException(IList errors) { if (errors.Count == 1) { - return new Exception(errors[0]); + return new DataValidationException(errors[0]); } else { return new AggregateException( - errors.Select(x => new Exception(x))); + errors.Select(x => new DataValidationException(x))); } } } diff --git a/src/Avalonia.Base/Data/DataValidationException.cs b/src/Avalonia.Base/Data/DataValidationException.cs new file mode 100644 index 0000000000..e38e6c30ad --- /dev/null +++ b/src/Avalonia.Base/Data/DataValidationException.cs @@ -0,0 +1,21 @@ +using System; + +namespace Avalonia.Data +{ + /// + /// Exception, which wrap validation errors. + /// + public class DataValidationException : Exception + { + /// + /// Initializes a new instance of the class. + /// + /// Data of validation error. + public DataValidationException(object errorData) : base(errorData?.ToString()) + { + ErrorData = errorData; + } + + public object ErrorData { get; } + } +} diff --git a/src/Avalonia.Controls/DataValidationErrors.cs b/src/Avalonia.Controls/DataValidationErrors.cs index f0d7f8257e..50b387e636 100644 --- a/src/Avalonia.Controls/DataValidationErrors.cs +++ b/src/Avalonia.Controls/DataValidationErrors.cs @@ -22,8 +22,8 @@ namespace Avalonia.Controls /// /// Defines the DataValidationErrors.Errors attached property. /// - public static readonly AttachedProperty> ErrorsProperty = - AvaloniaProperty.RegisterAttached>("Errors"); + public static readonly AttachedProperty> ErrorsProperty = + AvaloniaProperty.RegisterAttached>("Errors"); /// /// Defines the DataValidationErrors.HasErrors attached property. @@ -76,7 +76,7 @@ namespace Avalonia.Controls private static void ErrorsChanged(AvaloniaPropertyChangedEventArgs e) { var control = (Control)e.Sender; - var errors = (IEnumerable)e.NewValue; + var errors = (IEnumerable)e.NewValue; var hasErrors = false; if (errors != null && errors.Any()) @@ -91,11 +91,11 @@ namespace Avalonia.Controls classes.Set(":error", (bool)e.NewValue); } - public static IEnumerable GetErrors(Control control) + public static IEnumerable GetErrors(Control control) { return control.GetValue(ErrorsProperty); } - public static void SetErrors(Control control, IEnumerable errors) + public static void SetErrors(Control control, IEnumerable errors) { control.SetValue(ErrorsProperty, errors); } @@ -112,14 +112,14 @@ namespace Avalonia.Controls return control.GetValue(HasErrorsProperty); } - private static IEnumerable UnpackException(Exception exception) + private static IEnumerable UnpackException(Exception exception) { if (exception != null) { var aggregate = exception as AggregateException; var exceptions = aggregate == null ? - (IEnumerable)new[] { exception } : - aggregate.InnerExceptions; + new[] { GetExceptionData(exception) } : + aggregate.InnerExceptions.Select(GetExceptionData).ToArray(); var filtered = exceptions.Where(x => !(x is BindingChainException)).ToList(); if (filtered.Count > 0) @@ -130,5 +130,13 @@ namespace Avalonia.Controls return null; } + + private static object GetExceptionData(Exception exception) + { + if (exception is DataValidationException dataValidationException) + return dataValidationException.ErrorData; + + return exception; + } } } From a6b65a39de30f4f8d0740f647976261bc98c518e Mon Sep 17 00:00:00 2001 From: "all.owing" Date: Wed, 21 Aug 2019 23:32:16 +0500 Subject: [PATCH 4/6] Test fix --- .../Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs b/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs index 4aaf0ab5b7..0f4a0759aa 100644 --- a/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs +++ b/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs @@ -58,7 +58,7 @@ namespace Avalonia.Controls.UnitTests Assert.Null(DataValidationErrors.GetErrors(target)); target.Text = "20"; - IEnumerable errors = DataValidationErrors.GetErrors(target); + IEnumerable errors = DataValidationErrors.GetErrors(target); Assert.Single(errors); Assert.IsType(errors.Single()); target.Text = "1"; From 6dbe617ed6baddac4e90614a23a99b7e2c5fbdf1 Mon Sep 17 00:00:00 2001 From: "all.owing" Date: Thu, 22 Aug 2019 09:37:50 +0500 Subject: [PATCH 5/6] One more tests fix --- .../Data/Core/ExpressionObserverTests_DataValidation.cs | 4 ++-- .../Data/Core/Plugins/IndeiValidationPluginTests.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_DataValidation.cs b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_DataValidation.cs index b66dd610dd..c472fffb38 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_DataValidation.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_DataValidation.cs @@ -100,7 +100,7 @@ namespace Avalonia.Base.UnitTests.Data.Core // Value is first signalled without an error as validation hasn't been updated. new BindingNotification(-5), - new BindingNotification(new Exception("Must be positive"), BindingErrorType.DataValidationError, -5), + new BindingNotification(new DataValidationException("Must be positive"), BindingErrorType.DataValidationError, -5), // Exception is thrown by trying to set value to "foo". new BindingNotification( @@ -108,7 +108,7 @@ namespace Avalonia.Base.UnitTests.Data.Core BindingErrorType.DataValidationError), // Value is set then validation is updated. - new BindingNotification(new Exception("Must be positive"), BindingErrorType.DataValidationError, 5), + new BindingNotification(new DataValidationException("Must be positive"), BindingErrorType.DataValidationError, 5), new BindingNotification(5), }, result); diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs index 383030cb6c..2423900c7a 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/Plugins/IndeiValidationPluginTests.cs @@ -37,13 +37,13 @@ namespace Avalonia.Base.UnitTests.Data.Core.Plugins new BindingNotification(6), // Then the ErrorsChanged event is fired. - new BindingNotification(new Exception("Must be less than Maximum"), BindingErrorType.DataValidationError, 6), + new BindingNotification(new DataValidationException("Must be less than Maximum"), BindingErrorType.DataValidationError, 6), // Maximum is changed to 10 so value is now valid. new BindingNotification(6), // And Maximum is changed back to 5. - new BindingNotification(new Exception("Must be less than Maximum"), BindingErrorType.DataValidationError, 6), + new BindingNotification(new DataValidationException("Must be less than Maximum"), BindingErrorType.DataValidationError, 6), }, result); } From 5afc0f5395231476769b1a59fb764adb2f9af6a5 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Sun, 25 Aug 2019 23:56:53 +0200 Subject: [PATCH 6/6] Unify AddRange and InsertRange. Fix ranged versions causing multiple enumerations. Get rid of unnecessary allocations (notifications, enumerators). --- src/Avalonia.Base/Collections/AvaloniaList.cs | 229 +++++++++++++----- .../Collections/AvaloniaListTests.cs | 17 ++ 2 files changed, 187 insertions(+), 59 deletions(-) diff --git a/src/Avalonia.Base/Collections/AvaloniaList.cs b/src/Avalonia.Base/Collections/AvaloniaList.cs index 4d4a561b08..3c8d4ca7e6 100644 --- a/src/Avalonia.Base/Collections/AvaloniaList.cs +++ b/src/Avalonia.Base/Collections/AvaloniaList.cs @@ -55,15 +55,15 @@ namespace Avalonia.Collections /// public class AvaloniaList : IAvaloniaList, IList, INotifyCollectionChangedDebug { - private List _inner; + private readonly List _inner; private NotifyCollectionChangedEventHandler _collectionChanged; /// /// Initializes a new instance of the class. /// public AvaloniaList() - : this(Enumerable.Empty()) { + _inner = new List(); } /// @@ -89,8 +89,8 @@ namespace Avalonia.Collections /// public event NotifyCollectionChangedEventHandler CollectionChanged { - add { _collectionChanged += value; } - remove { _collectionChanged -= value; } + add => _collectionChanged += value; + remove => _collectionChanged -= value; } /// @@ -150,7 +150,7 @@ namespace Avalonia.Collections T old = _inner[index]; - if (!object.Equals(old, value)) + if (!EqualityComparer.Default.Equals(old, value)) { _inner[index] = value; @@ -187,45 +187,38 @@ namespace Avalonia.Collections Validate?.Invoke(item); int index = _inner.Count; _inner.Add(item); - NotifyAdd(new[] { item }, index); + NotifyAdd(item, index); } /// /// Adds multiple items to the collection. /// /// The items. - public virtual void AddRange(IEnumerable items) - { - Contract.Requires(items != null); - - var list = (items as IList) ?? items.ToList(); - - if (list.Count > 0) - { - if (Validate != null) - { - foreach (var item in list) - { - Validate((T)item); - } - } - - int index = _inner.Count; - _inner.AddRange(items); - NotifyAdd(list, index); - } - } + public virtual void AddRange(IEnumerable items) => InsertRange(_inner.Count, items); /// /// Removes all items from the collection. /// public virtual void Clear() { - if (this.Count > 0) + if (Count > 0) { - var old = _inner; - _inner = new List(); - NotifyReset(old); + if (_collectionChanged != null) + { + var e = ResetBehavior == ResetBehavior.Reset ? + EventArgsCache.ResetCollectionChanged : + new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, _inner.ToList(), 0); + + _inner.Clear(); + + _collectionChanged(this, e); + } + else + { + _inner.Clear(); + } + + NotifyCountChanged(); } } @@ -253,9 +246,20 @@ namespace Avalonia.Collections /// Returns an enumerator that enumerates the items in the collection. /// /// An . - public IEnumerator GetEnumerator() + IEnumerator IEnumerable.GetEnumerator() { - return _inner.GetEnumerator(); + return new Enumerator(_inner); + } + + /// + IEnumerator IEnumerable.GetEnumerator() + { + return new Enumerator(_inner); + } + + public Enumerator GetEnumerator() + { + return new Enumerator(_inner); } /// @@ -289,7 +293,7 @@ namespace Avalonia.Collections { Validate?.Invoke(item); _inner.Insert(index, item); - NotifyAdd(new[] { item }, index); + NotifyAdd(item, index); } /// @@ -301,20 +305,83 @@ namespace Avalonia.Collections { Contract.Requires(items != null); - var list = (items as IList) ?? items.ToList(); + bool willRaiseCollectionChanged = _collectionChanged != null; + bool hasValidation = Validate != null; - if (list.Count > 0) + if (items is IList list) { - if (Validate != null) + if (list.Count > 0) { - foreach (var item in list) + if (list is ICollection collection) { - Validate((T)item); + if (hasValidation) + { + foreach (T item in collection) + { + Validate(item); + } + } + + _inner.InsertRange(index, collection); + NotifyAdd(list, index); + } + else + { + using (IEnumerator en = items.GetEnumerator()) + { + int insertIndex = index; + + while (en.MoveNext()) + { + T item = en.Current; + + if (hasValidation) + { + Validate(item); + } + + _inner.Insert(insertIndex++, item); + } + } + + NotifyAdd(list, index); } } + } + else + { + using (IEnumerator en = items.GetEnumerator()) + { + if (en.MoveNext()) + { + // Avoid allocating list for collection notification if there is no event subscriptions. + List notificationItems = willRaiseCollectionChanged ? + new List() : + null; + + int insertIndex = index; + + do + { + T item = en.Current; + + if (hasValidation) + { + Validate(item); + } - _inner.InsertRange(index, items); - NotifyAdd((items as IList) ?? items.ToList(), index); + _inner.Insert(insertIndex++, item); + + if (willRaiseCollectionChanged) + { + notificationItems.Add(item); + } + + } while (en.MoveNext()); + + NotifyAdd(notificationItems, index); + } + } } } @@ -382,7 +449,7 @@ namespace Avalonia.Collections if (index != -1) { _inner.RemoveAt(index); - NotifyRemove(new[] { item }, index); + NotifyRemove(item , index); return true; } @@ -412,7 +479,7 @@ namespace Avalonia.Collections { T item = _inner[index]; _inner.RemoveAt(index); - NotifyRemove(new[] { item }, index); + NotifyRemove(item , index); } /// @@ -480,12 +547,6 @@ namespace Avalonia.Collections _inner.CopyTo((T[])array, index); } - /// - IEnumerator IEnumerable.GetEnumerator() - { - return _inner.GetEnumerator(); - } - /// Delegate[] INotifyCollectionChangedDebug.GetCollectionChangedSubscribers() => _collectionChanged?.GetInvocationList(); @@ -505,13 +566,29 @@ namespace Avalonia.Collections NotifyCountChanged(); } + /// + /// Raises the event with a add action. + /// + /// The item that was added. + /// The starting index. + private void NotifyAdd(T item, int index) + { + if (_collectionChanged != null) + { + var e = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, new[] { item }, index); + _collectionChanged(this, e); + } + + NotifyCountChanged(); + } + /// /// Raises the event when the property /// changes. /// private void NotifyCountChanged() { - PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Count))); + PropertyChanged?.Invoke(this, EventArgsCache.CountPropertyChanged); } /// @@ -531,23 +608,57 @@ namespace Avalonia.Collections } /// - /// Raises the event with a reset action. + /// Raises the event with a remove action. /// - /// The items that were removed. - private void NotifyReset(IList t) + /// The item that was removed. + /// The starting index. + private void NotifyRemove(T item, int index) { if (_collectionChanged != null) { - NotifyCollectionChangedEventArgs e; - - e = ResetBehavior == ResetBehavior.Reset ? - new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset) : - new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, t, 0); - + var e = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, new[] { item }, index); _collectionChanged(this, e); } NotifyCountChanged(); } + + /// + /// Enumerates the elements of a . + /// + public struct Enumerator : IEnumerator + { + private List.Enumerator _innerEnumerator; + + public Enumerator(List inner) + { + _innerEnumerator = inner.GetEnumerator(); + } + + public bool MoveNext() + { + return _innerEnumerator.MoveNext(); + } + + void IEnumerator.Reset() + { + ((IEnumerator)_innerEnumerator).Reset(); + } + + public T Current => _innerEnumerator.Current; + + object IEnumerator.Current => Current; + + public void Dispose() + { + _innerEnumerator.Dispose(); + } + } + } + + internal static class EventArgsCache + { + internal static readonly PropertyChangedEventArgs CountPropertyChanged = new PropertyChangedEventArgs(nameof(AvaloniaList.Count)); + internal static readonly NotifyCollectionChangedEventArgs ResetCollectionChanged = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset); } } diff --git a/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs b/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs index 8a38a00493..5c01e6a588 100644 --- a/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs +++ b/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs @@ -148,6 +148,23 @@ namespace Avalonia.Base.UnitTests.Collections Assert.True(raised); } + [Fact] + public void AddRange_Items_Should_Raise_Correct_CollectionChanged() + { + var target = new AvaloniaList(); + + var eventItems = new List(); + + target.CollectionChanged += (sender, args) => + { + eventItems.AddRange(args.NewItems.Cast()); + }; + + target.AddRange(Enumerable.Range(0,10).Select(i => new object())); + + Assert.Equal(eventItems, target); + } + [Fact] public void Replacing_Item_Should_Raise_CollectionChanged() {