Merge pull request #1643 from jkoritzinsky/fixes/1289

Dont call setter when initializing two way bindings

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);

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.

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

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)
         {

src/Avalonia.Base/Data/Core/PropertyAccessorNode.cs

@@ -10,7 +10,7 @@ using Avalonia.Data.Core.Plugins;
 
 namespace Avalonia.Data.Core
 {
-    internal class PropertyAccessorNode : ExpressionNode, ISettableNode
+    internal class PropertyAccessorNode : SettableNode
     {
         private readonly bool _enableValidation;
         private IPropertyAccessor _accessor;
@@ -23,13 +23,17 @@ 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 { return _accessor.SetValue(value, priority); } catch { }
+                try
+                {
+                    return _accessor.SetValue(value, priority);
+                }
+                catch { }
             }
 
             return false;
@@ -56,7 +60,10 @@ namespace Avalonia.Data.Core
                 () =>
                 {
                     _accessor = accessor;
-                    return Disposable.Create(() => _accessor = null);
+                    return Disposable.Create(() =>
+                    {
+                        _accessor = null;
+                    });
                 },
                 _ => accessor);
         }

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 != null && LastValue.Target.Equals(value);
+            }
+            return LastValue != null && Object.ReferenceEquals(LastValue?.Target, value);
+        }
+
+        protected abstract bool SetTargetValueCore(object value, BindingPriority priority);
+
+        public abstract Type PropertyType { get; }
+    }
+}

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs

@@ -457,6 +457,28 @@ namespace Avalonia.Base.UnitTests
             Assert.True(target.IsAnimating(Class1.FooProperty));
         }
 
+        [Fact]
+        public void TwoWay_Binding_Should_Not_Call_Setter_On_Creation()
+        {
+            var target = new Class1();
+            var source = new TestTwoWayBindingViewModel();
+
+            target.Bind(Class1.DoubleValueProperty, new Binding(nameof(source.Value), BindingMode.TwoWay) { Source = source });
+
+            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>
@@ -545,5 +567,32 @@ namespace Avalonia.Base.UnitTests
                 }
             }
         }
+
+        private class TestTwoWayBindingViewModel
+        {
+            private double _value;
+
+            public double Value
+            {
+                get => _value;
+                set
+                {
+                    _value = value;
+                    SetterCalled = true;
+                }
+            }
+
+            public double this[int index]
+            {
+                get => _value;
+                set
+                {
+                    _value = value;
+                    SetterCalled = true;
+                }
+            }
+
+            public bool SetterCalled { get; private set; }
+        }
     }
 }