Browse Source

Move fix up to SettableNode and ExpressionNode so all settable node types (i.e. PropertyAccessor and Indexer nodes) can get the fix.

Jeremy Koritzinsky 7 years ago
parent
commit
1c3b714a0e

+ 6 - 0
src/Avalonia.Base/Data/Core/ExpressionNode.cs

@@ -11,6 +11,7 @@ namespace Avalonia.Data.Core
 {
     internal abstract class ExpressionNode : ISubject<object>
     {
+        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<object> _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);

+ 2 - 2
src/Avalonia.Base/Data/Core/ExpressionObserver.cs

@@ -154,7 +154,7 @@ namespace Avalonia.Data.Core
         /// </returns>
         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.
         /// </summary>
-        public Type ResultType => (Leaf as ISettableNode)?.PropertyType;
+        public Type ResultType => (Leaf as SettableNode)?.PropertyType;
 
         /// <summary>
         /// Gets the leaf node.

+ 0 - 15
src/Avalonia.Base/Data/Core/ISettableNode.cs

@@ -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; }
-    }
-}

+ 3 - 3
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<string> 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<string> 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)
         {

+ 9 - 35
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<object> 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);
         }
     }
 }

+ 38 - 0
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; }
+    }
+}

+ 21 - 0
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);
+        }
+
         /// <summary>
         /// Returns an observable that returns a single value but does not complete.
         /// </summary>
@@ -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; }
         }
     }