From 1c3b714a0ec25166a8f3b15291f432b09da9b883 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sat, 2 Jun 2018 13:02:13 -0500 Subject: [PATCH] Move fix up to SettableNode and ExpressionNode so all settable node types (i.e. PropertyAccessor and Indexer nodes) can get the fix. --- src/Avalonia.Base/Data/Core/ExpressionNode.cs | 6 +++ .../Data/Core/ExpressionObserver.cs | 4 +- src/Avalonia.Base/Data/Core/ISettableNode.cs | 15 ------- src/Avalonia.Base/Data/Core/IndexerNode.cs | 6 +-- .../Data/Core/PropertyAccessorNode.cs | 44 ++++--------------- src/Avalonia.Base/Data/Core/SettableNode.cs | 38 ++++++++++++++++ .../AvaloniaObjectTests_Binding.cs | 21 +++++++++ 7 files changed, 79 insertions(+), 55 deletions(-) delete mode 100644 src/Avalonia.Base/Data/Core/ISettableNode.cs create mode 100644 src/Avalonia.Base/Data/Core/SettableNode.cs diff --git a/src/Avalonia.Base/Data/Core/ExpressionNode.cs b/src/Avalonia.Base/Data/Core/ExpressionNode.cs index ae70cacdba..ac7e97a4b1 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionNode.cs @@ -11,6 +11,7 @@ namespace Avalonia.Data.Core { internal abstract class ExpressionNode : ISubject { + private static readonly object CacheInvalid = new object(); protected static readonly WeakReference UnsetReference = new WeakReference(AvaloniaProperty.UnsetValue); @@ -18,6 +19,8 @@ namespace Avalonia.Data.Core private IDisposable _valueSubscription; private IObserver _observer; + protected WeakReference LastValue { get; private set; } + public abstract string Description { get; } public ExpressionNode Next { get; set; } @@ -61,6 +64,7 @@ namespace Avalonia.Data.Core { _valueSubscription?.Dispose(); _valueSubscription = null; + LastValue = null; nextSubscription?.Dispose(); _observer = null; }); @@ -120,6 +124,7 @@ namespace Avalonia.Data.Core if (notification == null) { + LastValue = new WeakReference(value); if (Next != null) { Next.Target = new WeakReference(value); @@ -131,6 +136,7 @@ namespace Avalonia.Data.Core } else { + LastValue = new WeakReference(notification.Value); if (Next != null) { Next.Target = new WeakReference(notification.Value); diff --git a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs index 7719f93a02..14bc09f5b7 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs @@ -154,7 +154,7 @@ namespace Avalonia.Data.Core /// public bool SetValue(object value, BindingPriority priority = BindingPriority.LocalValue) { - if (Leaf is ISettableNode settable) + if (Leaf is SettableNode settable) { var node = _node; while (node != null) @@ -188,7 +188,7 @@ namespace Avalonia.Data.Core /// Gets the type of the expression result or null if the expression could not be /// evaluated. /// - public Type ResultType => (Leaf as ISettableNode)?.PropertyType; + public Type ResultType => (Leaf as SettableNode)?.PropertyType; /// /// Gets the leaf node. diff --git a/src/Avalonia.Base/Data/Core/ISettableNode.cs b/src/Avalonia.Base/Data/Core/ISettableNode.cs deleted file mode 100644 index 7788407833..0000000000 --- a/src/Avalonia.Base/Data/Core/ISettableNode.cs +++ /dev/null @@ -1,15 +0,0 @@ -using Avalonia.Data; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace Avalonia.Data.Core -{ - interface ISettableNode - { - bool SetTargetValue(object value, BindingPriority priority); - Type PropertyType { get; } - } -} diff --git a/src/Avalonia.Base/Data/Core/IndexerNode.cs b/src/Avalonia.Base/Data/Core/IndexerNode.cs index 47e82fa2d3..633d3558ee 100644 --- a/src/Avalonia.Base/Data/Core/IndexerNode.cs +++ b/src/Avalonia.Base/Data/Core/IndexerNode.cs @@ -15,7 +15,7 @@ using Avalonia.Data; namespace Avalonia.Data.Core { - internal class IndexerNode : ExpressionNode, ISettableNode + internal class IndexerNode : SettableNode { public IndexerNode(IList arguments) { @@ -52,7 +52,7 @@ namespace Avalonia.Data.Core return Observable.Merge(inputs).StartWith(GetValue(target)); } - public bool SetTargetValue(object value, BindingPriority priority) + protected override bool SetTargetValueCore(object value, BindingPriority priority) { var typeInfo = Target.Target.GetType().GetTypeInfo(); var list = Target.Target as IList; @@ -154,7 +154,7 @@ namespace Avalonia.Data.Core public IList Arguments { get; } - public Type PropertyType => GetIndexer(Target.Target.GetType().GetTypeInfo())?.PropertyType; + public override Type PropertyType => GetIndexer(Target.Target.GetType().GetTypeInfo())?.PropertyType; private object GetValue(object target) { diff --git a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs index c958cfdb25..9d657b3144 100644 --- a/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs +++ b/src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs @@ -10,12 +10,10 @@ using Avalonia.Data.Core.Plugins; namespace Avalonia.Data.Core { - internal class PropertyAccessorNode : ExpressionNode, ISettableNode + internal class PropertyAccessorNode : SettableNode { - private static readonly object CacheInvalid = new object(); private readonly bool _enableValidation; private IPropertyAccessor _accessor; - private WeakReference _lastValue = null; public PropertyAccessorNode(string propertyName, bool enableValidation) { @@ -25,22 +23,15 @@ namespace Avalonia.Data.Core public override string Description => PropertyName; public string PropertyName { get; } - public Type PropertyType => _accessor?.PropertyType; + public override Type PropertyType => _accessor?.PropertyType; - public bool SetTargetValue(object value, BindingPriority priority) + protected override bool SetTargetValueCore(object value, BindingPriority priority) { if (_accessor != null) { try { - if (ShouldNotSet(value)) - { - return true; - } - else - { - return _accessor.SetValue(value, priority); - } + return _accessor.SetValue(value, priority); } catch { } } @@ -48,15 +39,6 @@ namespace Avalonia.Data.Core return false; } - private bool ShouldNotSet(object value) - { - if (PropertyType.IsValueType) - { - return _lastValue?.Target.Equals(value) ?? false; - } - return Object.ReferenceEquals(_lastValue?.Target ?? CacheInvalid, value); - } - protected override IObservable StartListeningCore(WeakReference reference) { var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(reference.Target, PropertyName)); @@ -78,20 +60,12 @@ namespace Avalonia.Data.Core () => { _accessor = accessor; - return Disposable.Create(() => _accessor = null); - }, - _ => accessor).Select(value => - { - if (value is BindingNotification notification) + return Disposable.Create(() => { - _lastValue = notification.HasValue ? new WeakReference(notification.Value) : null; - } - else - { - _lastValue = new WeakReference(value); - } - return value; - }); + _accessor = null; + }); + }, + _ => accessor); } } } diff --git a/src/Avalonia.Base/Data/Core/SettableNode.cs b/src/Avalonia.Base/Data/Core/SettableNode.cs new file mode 100644 index 0000000000..1f6ec0bff4 --- /dev/null +++ b/src/Avalonia.Base/Data/Core/SettableNode.cs @@ -0,0 +1,38 @@ +using Avalonia.Data; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Avalonia.Data.Core +{ + internal abstract class SettableNode : ExpressionNode + { + public bool SetTargetValue(object value, BindingPriority priority) + { + if (ShouldNotSet(value)) + { + return true; + } + return SetTargetValueCore(value, priority); + } + + private bool ShouldNotSet(object value) + { + if (PropertyType == null) + { + return false; + } + if (PropertyType.IsValueType) + { + return LastValue?.Target.Equals(value) ?? false; + } + return LastValue != null && Object.ReferenceEquals(LastValue?.Target, value); + } + + protected abstract bool SetTargetValueCore(object value, BindingPriority priority); + + public abstract Type PropertyType { get; } + } +} diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs index 65d37503b6..4638aa84a5 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs @@ -468,6 +468,17 @@ namespace Avalonia.Base.UnitTests Assert.False(source.SetterCalled); } + [Fact] + public void TwoWay_Binding_Should_Not_Call_Setter_On_Creation_Indexer() + { + var target = new Class1(); + var source = new TestTwoWayBindingViewModel(); + + target.Bind(Class1.DoubleValueProperty, new Binding("[0]", BindingMode.TwoWay) { Source = source }); + + Assert.False(source.SetterCalled); + } + /// /// Returns an observable that returns a single value but does not complete. /// @@ -571,6 +582,16 @@ namespace Avalonia.Base.UnitTests } } + public double this[int index] + { + get => _value; + set + { + _value = value; + SetterCalled = true; + } + } + public bool SetterCalled { get; private set; } } }