Browse Source

Implemented `MultiBindingExpression` (#16219)

* Update BindingBase.Instance signature.

- Swap `target` and `targetProperty` order to make it consistent with other similar methods
- Make `targetProperty` nullable as it will need to be null for `MultiBinding`

* IBinding2.Instance needs to accept a null target property.

It will need to be null for `MultiBinding`.

* Attach needs to accept a null target property.

It will need to be null for `MultiBinding`.

* Initial implementation of MultiBindingExpression.

* Fix failing template binding test.

Only publish unset value if we've already published a value.

* Enabled nullability annotations.

* Added passing test for #16084.

* Remove obsolete API usages.

* Bind to Tag not Text.

Prevents test passing when it shouldn't. See https://github.com/AvaloniaUI/Avalonia/pull/16219#discussion_r1665466968

* Handle DoNothing in MultiBindingExpression.
Steven Kirk 1 year ago
parent
commit
026789037f

+ 1 - 1
src/Avalonia.Base/AvaloniaObjectExtensions.cs

@@ -375,7 +375,7 @@ namespace Avalonia
                 return new InstancedBinding(expression, BindingMode.OneWay, BindingPriority.LocalValue);
             }
 
-            BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty property, object? anchor)
+            BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty? property, object? anchor)
             {
                 return new UntypedObservableBindingExpression(_source, BindingPriority.LocalValue);
             }

+ 1 - 1
src/Avalonia.Base/Data/Core/IBinding2.cs

@@ -15,6 +15,6 @@ internal interface IBinding2 : IBinding
 {
     BindingExpressionBase Instance(
         AvaloniaObject target,
-        AvaloniaProperty targetProperty,
+        AvaloniaProperty? targetProperty,
         object? anchor);
 }

+ 133 - 0
src/Avalonia.Base/Data/Core/MultiBindingExpression.cs

@@ -0,0 +1,133 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Diagnostics;
+using System.Globalization;
+using Avalonia.Data.Converters;
+
+namespace Avalonia.Data.Core;
+
+internal class MultiBindingExpression : UntypedBindingExpressionBase, IBindingExpressionSink
+{
+    private static readonly object s_uninitialized = new object();
+    private readonly IBinding[] _bindings;
+    private readonly IMultiValueConverter? _converter;
+    private readonly CultureInfo? _converterCulture;
+    private readonly object? _converterParameter;
+    private readonly UntypedBindingExpressionBase?[] _expressions;
+    private readonly object? _fallbackValue;
+    private readonly object? _targetNullValue;
+    private readonly object?[] _values;
+    private readonly ReadOnlyCollection<object?> _valuesView;
+
+    public MultiBindingExpression(
+        BindingPriority priority,
+        IList<IBinding> bindings,
+        IMultiValueConverter? converter,
+        CultureInfo? converterCulture,
+        object? converterParameter,
+        object? fallbackValue,
+        object? targetNullValue)
+            : base(priority)
+    {
+        _bindings = [.. bindings];
+        _converter = converter;
+        _converterCulture = converterCulture;
+        _converterParameter = converterParameter;
+        _expressions = new UntypedBindingExpressionBase[_bindings.Length];
+        _fallbackValue = fallbackValue;
+        _targetNullValue = targetNullValue;
+        _values = new object?[_bindings.Length];
+        _valuesView = new(_values);
+
+#if NETSTANDARD2_0
+        for (var i = 0; i < _bindings.Length; ++i)
+            _values[i] = s_uninitialized;
+#else
+        Array.Fill(_values, s_uninitialized);
+#endif
+    }
+
+    public override string Description => "MultiBinding";
+
+    protected override void StartCore()
+    {
+        if (!TryGetTarget(out var target))
+            throw new AvaloniaInternalException("MultiBindingExpression has no target.");
+
+        for (var i = 0; i < _bindings.Length; ++i)
+        {
+            var binding = _bindings[i]; 
+
+            if (binding is not IBinding2 b)
+                throw new NotSupportedException($"Unsupported IBinding implementation '{binding}'.");
+
+            var expression = b.Instance(target, null, null);
+
+            if (expression is not UntypedBindingExpressionBase e)
+                throw new NotSupportedException($"Unsupported BindingExpressionBase implementation '{expression}'.");
+
+            _expressions[i] = e;
+            e.AttachAndStart(this, target, null, Priority);
+        }
+    }
+
+    protected override void StopCore()
+    {
+        for (var i = 0; i < _expressions.Length; ++i)
+        {
+            _expressions[i]?.Dispose();
+            _expressions[i] = null;
+            _values[i] = s_uninitialized;
+        }
+    }
+
+    void IBindingExpressionSink.OnChanged(
+        UntypedBindingExpressionBase instance,
+        bool hasValueChanged,
+        bool hasErrorChanged,
+        object? value,
+        BindingError? error)
+    {
+        var i = Array.IndexOf(_expressions, instance);
+        Debug.Assert(i != -1);
+
+        _values[i] = BindingNotification.ExtractValue(value);
+        PublishValue();
+    }
+
+    void IBindingExpressionSink.OnCompleted(UntypedBindingExpressionBase instance)
+    {
+        // Nothing to do here.
+    }
+
+    private void PublishValue()
+    {
+        foreach (var v in _values)
+        {
+            if (v == s_uninitialized)
+                return;
+        }
+
+        if (_converter is not null)
+        {
+            var culture = _converterCulture ?? CultureInfo.CurrentCulture;
+            var converted = _converter.Convert(_valuesView, TargetType, _converterParameter, culture);
+
+            converted = BindingNotification.ExtractValue(converted);
+
+            if (converted != BindingOperations.DoNothing)
+            {
+                if (converted == null)
+                    converted = _targetNullValue;
+                if (converted == AvaloniaProperty.UnsetValue)
+                    converted = _fallbackValue;
+                PublishValue(converted);
+            }
+        }
+        else
+        {
+            PublishValue(_valuesView);
+        }
+    }
+}

+ 6 - 3
src/Avalonia.Base/Data/Core/UntypedBindingExpressionBase.cs

@@ -204,7 +204,7 @@ public abstract class UntypedBindingExpressionBase : BindingExpressionBase,
     internal void AttachAndStart(
         IBindingExpressionSink subscriber,
         AvaloniaObject target,
-        AvaloniaProperty targetProperty,
+        AvaloniaProperty? targetProperty,
         BindingPriority priority)
     {
         AttachCore(subscriber, null, target, targetProperty, priority);
@@ -261,7 +261,7 @@ public abstract class UntypedBindingExpressionBase : BindingExpressionBase,
         IBindingExpressionSink sink,
         ImmediateValueFrame? frame,
         AvaloniaObject target,
-        AvaloniaProperty targetProperty,
+        AvaloniaProperty? targetProperty,
         BindingPriority priority)
     {
         if (_sink is not null)
@@ -273,7 +273,7 @@ public abstract class UntypedBindingExpressionBase : BindingExpressionBase,
         _frame = frame;
         _target = new(target);
         TargetProperty = targetProperty;
-        TargetType = targetProperty.PropertyType;
+        TargetType = targetProperty?.PropertyType ?? typeof(object);
         Priority = priority;
     }
 
@@ -409,6 +409,9 @@ public abstract class UntypedBindingExpressionBase : BindingExpressionBase,
     /// <param name="error">The new binding or data validation error.</param>
     private protected void PublishValue(object? value, BindingError? error = null)
     {
+        Debug.Assert(value is not BindingNotification);
+        Debug.Assert(value != BindingOperations.DoNothing);
+
         if (!IsRunning)
             return;
 

+ 1 - 1
src/Avalonia.Base/Data/IndexerBinding.cs

@@ -31,7 +31,7 @@ namespace Avalonia.Data
             return new InstancedBinding(expression, Mode, BindingPriority.LocalValue);
         }
 
-        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty targetProperty, object? anchor)
+        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty? targetProperty, object? anchor)
         {
             return new IndexerBindingExpression(Source, Property, target, targetProperty, Mode);
         }

+ 5 - 2
src/Avalonia.Base/Data/TemplateBinding.cs

@@ -21,6 +21,7 @@ namespace Avalonia.Data
         IDisposable
     {
         private bool _isSetterValue;
+        private bool _hasPublishedValue;
 
         public TemplateBinding()
             : base(BindingPriority.Template)
@@ -82,7 +83,7 @@ namespace Avalonia.Data
             return new(target, InstanceCore(), Mode, BindingPriority.Template);
         }
 
-        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty property, object? anchor)
+        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty? property, object? anchor)
         {
             return InstanceCore();
         }
@@ -108,6 +109,7 @@ namespace Avalonia.Data
 
         protected override void StartCore()
         {
+            _hasPublishedValue = false;
             OnTemplatedParentChanged();
             if (TryGetTarget(out var target))
                 target.PropertyChanged += OnTargetPropertyChanged;
@@ -199,11 +201,12 @@ namespace Avalonia.Data
 
                 value = ConvertToTargetType(value);
                 PublishValue(value, error);
+                _hasPublishedValue = true;
 
                 if (Mode == BindingMode.OneTime)
                     Stop();
             }
-            else
+            else if (_hasPublishedValue)
             {
                 PublishValue(AvaloniaProperty.UnsetValue);
             }

+ 2 - 2
src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/CompiledBindingExtension.cs

@@ -59,11 +59,11 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions
         }
 
         private protected override BindingExpressionBase Instance(
-            AvaloniaProperty targetProperty,
             AvaloniaObject target,
+            AvaloniaProperty? targetProperty,
             object? anchor)
         {
-            var enableDataValidation = targetProperty.GetMetadata(target).EnableDataValidation ?? false;
+            var enableDataValidation = targetProperty?.GetMetadata(target).EnableDataValidation ?? false;
             return InstanceCore(target, targetProperty, anchor, enableDataValidation);
         }
 

+ 1 - 1
src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/DynamicResourceExtension.cs

@@ -56,7 +56,7 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions
             return new InstancedBinding(target, expression, BindingMode.OneWay, _priority);
         }
 
-        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty targetProperty, object? anchor)
+        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty? targetProperty, object? anchor)
         {
             if (ResourceKey is null)
                 throw new InvalidOperationException("DynamicResource must have a ResourceKey.");

+ 2 - 2
src/Markup/Avalonia.Markup/Data/Binding.cs

@@ -73,11 +73,11 @@ namespace Avalonia.Data
         }
 
         private protected override BindingExpressionBase Instance(
-            AvaloniaProperty targetProperty,
             AvaloniaObject target,
+            AvaloniaProperty? targetProperty,
             object? anchor)
         {
-            var enableDataValidation = targetProperty.GetMetadata(target).EnableDataValidation ?? false;
+            var enableDataValidation = targetProperty?.GetMetadata(target).EnableDataValidation ?? false;
             return InstanceCore(targetProperty, target, anchor, enableDataValidation);
         }
 

+ 3 - 3
src/Markup/Avalonia.Markup/Data/BindingBase.cs

@@ -95,8 +95,8 @@ namespace Avalonia.Data
             bool enableDataValidation = false);
 
         private protected abstract BindingExpressionBase Instance(
-            AvaloniaProperty targetProperty,
             AvaloniaObject target,
+            AvaloniaProperty? targetProperty,
             object? anchor);
 
         private protected (BindingMode, UpdateSourceTrigger) ResolveDefaultsFromMetadata(
@@ -118,9 +118,9 @@ namespace Avalonia.Data
             return (mode, trigger);
         }
 
-        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty property, object? anchor)
+        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty? property, object? anchor)
         {
-            return Instance(property, target, anchor);
+            return Instance(target, property, anchor);
         }
     }
 }

+ 30 - 61
src/Markup/Avalonia.Markup/Data/MultiBinding.cs

@@ -6,6 +6,7 @@ using Avalonia.Reactive;
 using Avalonia.Data.Converters;
 using Avalonia.Metadata;
 using Avalonia.Data.Core;
+using System.ComponentModel;
 
 namespace Avalonia.Data
 {
@@ -25,6 +26,16 @@ namespace Avalonia.Data
         /// </summary>
         public IMultiValueConverter? Converter { get; set; }
 
+        /// <summary>
+        /// Gets or sets the culture in which to evaluate the converter.
+        /// </summary>
+        /// <value>The default value is null.</value>
+        /// <remarks>
+        /// If this property is not set then <see cref="CultureInfo.CurrentCulture"/> will be used.
+        /// </remarks>
+        [TypeConverter(typeof(CultureInfoIetfLanguageTagConverter))]
+        public CultureInfo? ConverterCulture { get; set; }
+
         /// <summary>
         /// Gets or sets a parameter to pass to <see cref="Converter"/>.
         /// </summary>
@@ -73,33 +84,25 @@ namespace Avalonia.Data
             object? anchor = null,
             bool enableDataValidation = false)
         {
-            var input = InstanceCore(target, targetProperty);
-            var mode = Mode == BindingMode.Default ?
-                targetProperty?.GetMetadata(target).DefaultBindingMode : Mode;
-
-            switch (mode)
-            {
-                case BindingMode.OneTime:
-                    return InstancedBinding.OneTime(input, Priority);
-                case BindingMode.OneWay:
-                    return InstancedBinding.OneWay(input, Priority);
-                default:
-                    throw new NotSupportedException(
-                        "MultiBinding currently only supports OneTime and OneWay BindingMode.");
-            }
+            var expression = InstanceCore(target, targetProperty);
+            return new InstancedBinding(target, expression, Mode, Priority);
         }
 
-        BindingExpressionBase IBinding2.Instance(AvaloniaObject target, AvaloniaProperty property, object? anchor)
+        BindingExpressionBase IBinding2.Instance(
+            AvaloniaObject target,
+            AvaloniaProperty? targetProperty,
+            object? anchor)
         {
-            // TODO: Implement MultiBindingExpression instead of wrapping an observable.
-            var o = InstanceCore(target, property);
-            return new UntypedObservableBindingExpression(o, BindingPriority.LocalValue);
+            return InstanceCore(target, targetProperty);
         }
 
-        private IObservable<object?> InstanceCore(AvaloniaObject target, AvaloniaProperty? targetProperty)
+        private MultiBindingExpression InstanceCore(
+            AvaloniaObject target,
+            AvaloniaProperty? targetProperty)
         {
             var targetType = targetProperty?.PropertyType ?? typeof(object);
             var converter = Converter;
+
             // We only respect `StringFormat` if the type of the property we're assigning to will
             // accept a string. Note that this is slightly different to WPF in that WPF only applies
             // `StringFormat` for target type `string` (not `object`).
@@ -109,48 +112,14 @@ namespace Avalonia.Data
                 converter = new StringFormatMultiValueConverter(StringFormat!, converter);
             }
 
-            var children = Bindings.Select(x => x.Initiate(target, null));
-
-            return children.Select(x => x?.Source)
-                .Where(x => x is not null)!
-                .CombineLatest()
-                .Select(x => ConvertValue(x, targetType, converter))
-                .Where(x => x != BindingOperations.DoNothing);
-        }
-
-        private object ConvertValue(IList<object?> values, Type targetType, IMultiValueConverter? converter)
-        {
-            for (var i = 0; i < values.Count; ++i)
-            {
-                if (values[i] is BindingNotification notification)
-                {
-                    values[i] = notification.Value;
-                }
-            }
-
-            var culture = CultureInfo.CurrentCulture;
-            values = new System.Collections.ObjectModel.ReadOnlyCollection<object?>(values);
-            object? converted;
-            if (converter != null)
-            {
-                converted = converter.Convert(values, targetType, ConverterParameter, culture);
-            }
-            else
-            {
-                converted = values;
-            }
-
-            if (converted == null)
-            {
-                converted = TargetNullValue;
-            }
-
-            if (converted == AvaloniaProperty.UnsetValue)
-            {
-                converted = FallbackValue;
-            }
-
-            return converted;
+            return new MultiBindingExpression(
+                Priority,
+                Bindings,
+                converter,
+                ConverterCulture,
+                ConverterParameter,
+                FallbackValue,
+                TargetNullValue);
         }
     }
 }

+ 69 - 16
tests/Avalonia.Markup.UnitTests/Data/MultiBindingTests.cs

@@ -3,19 +3,21 @@ using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 using System.Reactive.Linq;
-using Moq;
-using Avalonia.Controls;
-using Xunit;
 using System.Threading.Tasks;
-using Avalonia.Data.Converters;
+using Avalonia.Controls;
 using Avalonia.Data;
+using Avalonia.Data.Converters;
+using Avalonia.UnitTests;
+using Xunit;
+
+#nullable enable
 
 namespace Avalonia.Markup.UnitTests.Data
 {
     public class MultiBindingTests
     {
         [Fact]
-        public async Task OneWay_Binding_Should_Be_Set_Up()
+        public void OneWay_Binding_Should_Be_Set_Up()
         {
             var source = new { A = 1, B = 2, C = 3 };
             var binding = new MultiBinding
@@ -30,14 +32,13 @@ namespace Avalonia.Markup.UnitTests.Data
             };
 
             var target = new Control { DataContext = source };
-            var observable = binding.Initiate(target, null).Source;
-            var result = await observable.Take(1);
+            target.Bind(Control.TagProperty, binding);
 
-            Assert.Equal("1,2,3", result);
+            Assert.Equal("1,2,3", target.Tag);
         }
 
         [Fact]
-        public async Task Nested_MultiBinding_Should_Be_Set_Up()
+        public void Nested_MultiBinding_Should_Be_Set_Up()
         {
             var source = new { A = 1, B = 2, C = 3 };
             var binding = new MultiBinding
@@ -59,10 +60,9 @@ namespace Avalonia.Markup.UnitTests.Data
             };
 
             var target = new Control { DataContext = source };
-            var observable = binding.Initiate(target, null).Source;
-            var result = await observable.Take(1);
+            target.Bind(Control.TagProperty, binding);
 
-            Assert.Equal("1,2,3", result);
+            Assert.Equal("1,2,3", target.Tag);
         }
 
         [Fact]
@@ -202,9 +202,49 @@ namespace Avalonia.Markup.UnitTests.Data
             Assert.Equal("1,2,3-BindingNotification", target.Text);
         }
 
+        [Fact]
+        public void Converter_Should_Be_Called_On_PropertyChanged_Even_If_Property_Not_Changed()
+        {
+            // Issue #16084
+            var data = new TestModel();
+            var target = new TextBlock { DataContext = data };
+
+            var binding = new MultiBinding
+            {
+                Converter = new TestModelMemberConverter(),
+                Bindings =
+                {
+                    new Binding(),
+                    new Binding(nameof(data.NotifyingValue)),
+                },
+            };
+
+            target.Bind(TextBlock.TextProperty, binding);
+            Assert.Equal("0", target.Text);
+
+            data.NonNotifyingValue = 1;
+            Assert.Equal("0", target.Text);
+
+            data.NotifyingValue = new object();
+            Assert.Equal("1", target.Text);
+        }
+
+        private partial class TestModel : NotifyingBase
+        {
+            private object? _notifyingValue;
+
+            public int? NonNotifyingValue { get; set; } = 0;
+
+            public object? NotifyingValue
+            {
+                get => _notifyingValue;
+                set => SetField(ref _notifyingValue, value);
+            }
+        }
+
         private class ConcatConverter : IMultiValueConverter
         {
-            public object Convert(IList<object> values, Type targetType, object parameter, CultureInfo culture)
+            public object? Convert(IList<object?> values, Type targetType, object? parameter, CultureInfo culture)
             {
                 return string.Join(",", values);
             }
@@ -212,7 +252,7 @@ namespace Avalonia.Markup.UnitTests.Data
 
         private class UnsetValueConverter : IMultiValueConverter
         {
-            public object Convert(IList<object> values, Type targetType, object parameter, CultureInfo culture)
+            public object? Convert(IList<object?> values, Type targetType, object? parameter, CultureInfo culture)
             {
                 return AvaloniaProperty.UnsetValue;
             }
@@ -220,7 +260,7 @@ namespace Avalonia.Markup.UnitTests.Data
 
         private class NullValueConverter : IMultiValueConverter
         {
-            public object Convert(IList<object> values, Type targetType, object parameter, CultureInfo culture)
+            public object? Convert(IList<object?> values, Type targetType, object? parameter, CultureInfo culture)
             {
                 return null;
             }
@@ -228,7 +268,7 @@ namespace Avalonia.Markup.UnitTests.Data
 
         private class BindingNotificationConverter : IMultiValueConverter
         {
-            public object Convert(IList<object> values, Type targetType, object parameter, CultureInfo culture)
+            public object? Convert(IList<object?> values, Type targetType, object? parameter, CultureInfo culture)
             {
                 return new BindingNotification(
                     new ArgumentException(),
@@ -236,5 +276,18 @@ namespace Avalonia.Markup.UnitTests.Data
                     string.Join(",", values) + "-BindingNotification");
             }
         }
+
+        private class TestModelMemberConverter : IMultiValueConverter
+        {
+            public object? Convert(IList<object?> values, Type targetType, object? parameter, CultureInfo culture)
+            {
+                if (values[0] is not TestModel model)
+                {
+                    return string.Empty;
+                }
+
+                return model.NonNotifyingValue.ToString();
+            }
+        }
     }
 }

+ 5 - 5
tests/Avalonia.Markup.Xaml.UnitTests/Converters/MultiValueConverterTests.cs

@@ -21,12 +21,12 @@ namespace Avalonia.Markup.Xaml.UnitTests.Converters
         xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
         xmlns:c='clr-namespace:Avalonia.Markup.Xaml.UnitTests.Converters;assembly=Avalonia.Markup.Xaml.UnitTests'>
     <TextBlock Name='textBlock'>
-        <TextBlock.Text>
+        <TextBlock.Tag>
             <MultiBinding Converter='{x:Static c:TestMultiValueConverter.Instance}' FallbackValue='bar'>
                 <Binding Path='Item1' />
                 <Binding Path='Item2' />
             </MultiBinding>
-        </TextBlock.Text>
+        </TextBlock.Tag>
     </TextBlock>
 </Window>";
                 var window = (Window)AvaloniaRuntimeXamlLoader.Load(xaml);
@@ -35,13 +35,13 @@ namespace Avalonia.Markup.Xaml.UnitTests.Converters
                 window.ApplyTemplate();
 
                 window.DataContext = Tuple.Create(2, 2);
-                Assert.Equal("foo", textBlock.Text);
+                Assert.Equal("foo", textBlock.Tag);
 
                 window.DataContext = Tuple.Create(-3, 3);
-                Assert.Equal("foo", textBlock.Text);
+                Assert.Equal("foo", textBlock.Tag);
 
                 window.DataContext = Tuple.Create(0, 2);
-                Assert.Equal("bar", textBlock.Text);
+                Assert.Equal("bar", textBlock.Tag);
             }
         }
     }