From 688dbc4fdc24e19ccad196c9cf21a9b69d43b6ec Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 16 Feb 2023 18:56:23 +0100 Subject: [PATCH 1/9] Moved EnableDataValdiation to AvaloniaPropertyMetadata Support data validation in StyledProperty bindings Co-Authored-By: Tom Edwards <109803929+tomenscape@users.noreply.github.com> --- src/Avalonia.Base/AvaloniaObject.cs | 11 +++++--- src/Avalonia.Base/AvaloniaObjectExtensions.cs | 4 +-- src/Avalonia.Base/AvaloniaProperty.cs | 13 ++++++--- src/Avalonia.Base/AvaloniaPropertyMetadata.cs | 18 ++++++++++++- src/Avalonia.Base/DirectPropertyMetadata`1.cs | 27 +++---------------- src/Avalonia.Base/StyledElement.cs | 1 + src/Avalonia.Base/StyledPropertyMetadata`1.cs | 6 +++-- .../AvaloniaPropertyTests.cs | 1 + 8 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index d89d6f3690..bf1624eab3 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -776,11 +776,16 @@ namespace Avalonia break; } - var metadata = property.GetMetadata(GetType()); + UpdateDataValidationCore(property, value.Type, value.Error); + } - if (metadata.EnableDataValidation == true) + internal void UpdateDataValidationCore(AvaloniaProperty property, + BindingValueType state, + Exception? error) + { + if (property.GetMetadata(GetType()) is { EnableDataValidation: true }) { - UpdateDataValidation(property, value.Type, value.Error); + UpdateDataValidation(property, state, error); } } diff --git a/src/Avalonia.Base/AvaloniaObjectExtensions.cs b/src/Avalonia.Base/AvaloniaObjectExtensions.cs index 6231483ff8..9fbf680a5c 100644 --- a/src/Avalonia.Base/AvaloniaObjectExtensions.cs +++ b/src/Avalonia.Base/AvaloniaObjectExtensions.cs @@ -199,13 +199,11 @@ namespace Avalonia property = property ?? throw new ArgumentNullException(nameof(property)); binding = binding ?? throw new ArgumentNullException(nameof(binding)); - var metadata = property.GetMetadata(target.GetType()) as IDirectPropertyMetadata; - var result = binding.Initiate( target, property, anchor, - metadata?.EnableDataValidation ?? false); + property.GetMetadata(target.GetType()).EnableDataValidation ?? false); if (result != null) { diff --git a/src/Avalonia.Base/AvaloniaProperty.cs b/src/Avalonia.Base/AvaloniaProperty.cs index 45ab293a89..24244c5068 100644 --- a/src/Avalonia.Base/AvaloniaProperty.cs +++ b/src/Avalonia.Base/AvaloniaProperty.cs @@ -227,6 +227,7 @@ namespace Avalonia /// The default binding mode for the property. /// A value validation callback. /// A value coercion callback. + /// Whether the property is interested in data validation. /// A public static StyledProperty Register( string name, @@ -234,7 +235,8 @@ namespace Avalonia bool inherits = false, BindingMode defaultBindingMode = BindingMode.OneWay, Func? validate = null, - Func? coerce = null) + Func? coerce = null, + bool enableDataValidation = false) where TOwner : AvaloniaObject { _ = name ?? throw new ArgumentNullException(nameof(name)); @@ -242,7 +244,8 @@ namespace Avalonia var metadata = new StyledPropertyMetadata( defaultValue, defaultBindingMode: defaultBindingMode, - coerce: coerce); + coerce: coerce, + enableDataValidation: enableDataValidation); var result = new StyledProperty( name, @@ -253,7 +256,7 @@ namespace Avalonia AvaloniaPropertyRegistry.Instance.Register(typeof(TOwner), result); return result; } - + /// /// /// A method that gets called before and after the property starts being notified on an @@ -267,6 +270,7 @@ namespace Avalonia BindingMode defaultBindingMode, Func? validate, Func? coerce, + bool enableDataValidation, Action? notifying) where TOwner : AvaloniaObject { @@ -275,7 +279,8 @@ namespace Avalonia var metadata = new StyledPropertyMetadata( defaultValue, defaultBindingMode: defaultBindingMode, - coerce: coerce); + coerce: coerce, + enableDataValidation: enableDataValidation); var result = new StyledProperty( name, diff --git a/src/Avalonia.Base/AvaloniaPropertyMetadata.cs b/src/Avalonia.Base/AvaloniaPropertyMetadata.cs index 2963567b14..62bb65351f 100644 --- a/src/Avalonia.Base/AvaloniaPropertyMetadata.cs +++ b/src/Avalonia.Base/AvaloniaPropertyMetadata.cs @@ -13,10 +13,13 @@ namespace Avalonia /// Initializes a new instance of the class. /// /// The default binding mode. + /// Whether the property is interested in data validation. public AvaloniaPropertyMetadata( - BindingMode defaultBindingMode = BindingMode.Default) + BindingMode defaultBindingMode = BindingMode.Default, + bool? enableDataValidation = null) { _defaultBindingMode = defaultBindingMode; + EnableDataValidation = enableDataValidation; } /// @@ -31,6 +34,17 @@ namespace Avalonia } } + /// + /// Gets a value indicating whether the property is interested in data validation. + /// + /// + /// Data validation is validation performed at the target of a binding, for example in a + /// view model using the INotifyDataErrorInfo interface. Only certain properties on a + /// control (such as a TextBox's Text property) will be interested in receiving data + /// validation messages so this feature must be explicitly enabled by setting this flag. + /// + public bool? EnableDataValidation { get; private set; } + /// /// Merges the metadata with the base metadata. /// @@ -44,6 +58,8 @@ namespace Avalonia { _defaultBindingMode = baseMetadata.DefaultBindingMode; } + + EnableDataValidation ??= baseMetadata.EnableDataValidation; } } } diff --git a/src/Avalonia.Base/DirectPropertyMetadata`1.cs b/src/Avalonia.Base/DirectPropertyMetadata`1.cs index fe1cdd0e65..451ff6ce00 100644 --- a/src/Avalonia.Base/DirectPropertyMetadata`1.cs +++ b/src/Avalonia.Base/DirectPropertyMetadata`1.cs @@ -21,10 +21,9 @@ namespace Avalonia TValue unsetValue = default!, BindingMode defaultBindingMode = BindingMode.Default, bool? enableDataValidation = null) - : base(defaultBindingMode) + : base(defaultBindingMode, enableDataValidation) { UnsetValue = unsetValue; - EnableDataValidation = enableDataValidation; } /// @@ -32,16 +31,6 @@ namespace Avalonia /// public TValue UnsetValue { get; private set; } - /// - /// Gets a value indicating whether the property is interested in data validation. - /// - /// - /// Data validation is validation performed at the target of a binding, for example in a - /// view model using the INotifyDataErrorInfo interface. Only certain properties on a - /// control (such as a TextBox's Text property) will be interested in receiving data - /// validation messages so this feature must be explicitly enabled by setting this flag. - /// - public bool? EnableDataValidation { get; private set; } /// object? IDirectPropertyMetadata.UnsetValue => UnsetValue; @@ -51,19 +40,9 @@ namespace Avalonia { base.Merge(baseMetadata, property); - var src = baseMetadata as DirectPropertyMetadata; - - if (src != null) + if (baseMetadata is DirectPropertyMetadata src) { - if (UnsetValue == null) - { - UnsetValue = src.UnsetValue; - } - - if (EnableDataValidation == null) - { - EnableDataValidation = src.EnableDataValidation; - } + UnsetValue ??= src.UnsetValue; } } } diff --git a/src/Avalonia.Base/StyledElement.cs b/src/Avalonia.Base/StyledElement.cs index 5b8dac2f53..10a0c5da0e 100644 --- a/src/Avalonia.Base/StyledElement.cs +++ b/src/Avalonia.Base/StyledElement.cs @@ -46,6 +46,7 @@ namespace Avalonia defaultBindingMode: BindingMode.OneWay, validate: null, coerce: null, + enableDataValidation: false, notifying: DataContextNotifying); /// diff --git a/src/Avalonia.Base/StyledPropertyMetadata`1.cs b/src/Avalonia.Base/StyledPropertyMetadata`1.cs index c71973fde8..6f10de3651 100644 --- a/src/Avalonia.Base/StyledPropertyMetadata`1.cs +++ b/src/Avalonia.Base/StyledPropertyMetadata`1.cs @@ -16,11 +16,13 @@ namespace Avalonia /// The default value of the property. /// The default binding mode. /// A value coercion callback. + /// Whether the property is interested in data validation. public StyledPropertyMetadata( Optional defaultValue = default, BindingMode defaultBindingMode = BindingMode.Default, - Func? coerce = null) - : base(defaultBindingMode) + Func? coerce = null, + bool enableDataValidation = false) + : base(defaultBindingMode, enableDataValidation) { _defaultValue = defaultValue; CoerceValue = coerce; diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs index 7e932373c2..181596a681 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs @@ -198,6 +198,7 @@ namespace Avalonia.Base.UnitTests defaultBindingMode: BindingMode.OneWay, validate: null, coerce: null, + enableDataValidation: false, notifying: FooNotifying); public int NotifyCount { get; private set; } From eabc9493fae49de2aa02fc8d3658a2914da18871 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 16 Feb 2023 23:22:46 +0100 Subject: [PATCH 2/9] Added tests for styled property data validation. --- .../AvaloniaObjectTests_DataValidation.cs | 258 +++++++++------ .../Data/BindingTests_DataValidation.cs | 293 +++++++++++++++--- 2 files changed, 420 insertions(+), 131 deletions(-) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs index d48e58136a..71eb521f9d 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs @@ -1,115 +1,170 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Reactive.Subjects; using Avalonia.Data; using Avalonia.UnitTests; using Xunit; +#nullable enable + namespace Avalonia.Base.UnitTests { public class AvaloniaObjectTests_DataValidation { - [Fact] - public void Binding_Non_Validated_Styled_Property_Does_Not_Call_UpdateDataValidation() + public abstract class TestBase + where T : AvaloniaProperty { - var target = new Class1(); - var source = new Subject>(); + [Fact] + public void Binding_Non_Validated_Property_Does_Not_Call_UpdateDataValidation() + { + var target = new Class1(); + var source = new Subject>(); + var property = GetNonValidatedProperty(); - target.Bind(Class1.NonValidatedProperty, source); - source.OnNext(6); - source.OnNext(BindingValue.BindingError(new Exception())); - source.OnNext(BindingValue.DataValidationError(new Exception())); - source.OnNext(6); + target.Bind(property, source); + source.OnNext(6); + source.OnNext(BindingValue.BindingError(new Exception())); + source.OnNext(BindingValue.DataValidationError(new Exception())); + source.OnNext(6); - Assert.Empty(target.Notifications); - } + Assert.Empty(target.Notifications); + } - [Fact] - public void Binding_Non_Validated_Direct_Property_Does_Not_Call_UpdateDataValidation() - { - var target = new Class1(); - var source = new Subject>(); + [Fact] + public void Binding_Validated_Property_Calls_UpdateDataValidation() + { + var target = new Class1(); + var source = new Subject>(); + var property = GetProperty(); + var error1 = new Exception(); + var error2 = new Exception(); - target.Bind(Class1.NonValidatedDirectProperty, source); - source.OnNext(6); - source.OnNext(BindingValue.BindingError(new Exception())); - source.OnNext(BindingValue.DataValidationError(new Exception())); - source.OnNext(6); + target.Bind(property, source); + source.OnNext(6); + source.OnNext(BindingValue.DataValidationError(error1)); + source.OnNext(BindingValue.BindingError(error2)); + source.OnNext(7); - Assert.Empty(target.Notifications); - } + Assert.Equal(new Notification[] + { + new(BindingValueType.Value, 6, null), + new(BindingValueType.DataValidationError, 6, error1), + new(BindingValueType.BindingError, 0, error2), + new(BindingValueType.Value, 7, null), + }, target.Notifications); + } - [Fact] - public void Binding_Validated_Direct_Property_Calls_UpdateDataValidation() - { - var target = new Class1(); - var source = new Subject>(); - - target.Bind(Class1.ValidatedDirectIntProperty, source); - source.OnNext(6); - source.OnNext(BindingValue.BindingError(new Exception())); - source.OnNext(BindingValue.DataValidationError(new Exception())); - source.OnNext(7); - - var result = target.Notifications; - Assert.Equal(4, result.Count); - Assert.Equal(BindingValueType.Value, result[0].type); - Assert.Equal(6, result[0].value); - Assert.Equal(BindingValueType.BindingError, result[1].type); - Assert.Equal(BindingValueType.DataValidationError, result[2].type); - Assert.Equal(BindingValueType.Value, result[3].type); - Assert.Equal(7, result[3].value); + [Fact] + public void Binding_Validated_Property_Calls_UpdateDataValidation_Untyped() + { + var target = new Class1(); + var source = new Subject(); + var property = GetProperty(); + var error1 = new Exception(); + var error2 = new Exception(); + + target.Bind(property, source); + source.OnNext(6); + source.OnNext(new BindingNotification(error1, BindingErrorType.DataValidationError)); + source.OnNext(new BindingNotification(error2, BindingErrorType.Error)); + source.OnNext(7); + + Assert.Equal(new Notification[] + { + new(BindingValueType.Value, 6, null), + new(BindingValueType.DataValidationError, 6, error1), + new(BindingValueType.BindingError, 0, error2), + new(BindingValueType.Value, 7, null), + }, target.Notifications); + } + + [Fact] + public void Binding_Overridden_Validated_Property_Calls_UpdateDataValidation() + { + var target = new Class2(); + var source = new Subject>(); + var property = GetNonValidatedProperty(); + + // Class2 overrides the non-validated property metadata to enable data validation. + target.Bind(property, source); + source.OnNext(1); + + Assert.Equal(1, target.Notifications.Count); + } + + protected abstract T GetProperty(); + protected abstract T GetNonValidatedProperty(); } - [Fact] - public void Binding_Overridden_Validated_Direct_Property_Calls_UpdateDataValidation() + public class DirectPropertyTests : TestBase> { - var target = new Class2(); - var source = new Subject>(); + [Fact] + public void Bound_Validated_String_Property_Can_Be_Set_To_Null() + { + var source = new ViewModel + { + StringValue = "foo", + }; + + var target = new Class1 + { + [!Class1.ValidatedDirectStringProperty] = new Binding + { + Path = nameof(ViewModel.StringValue), + Source = source, + }, + }; - // Class2 overrides `NonValidatedDirectProperty`'s metadata to enable data validation. - target.Bind(Class1.NonValidatedDirectProperty, source); - source.OnNext(1); + Assert.Equal("foo", target.ValidatedDirectString); - Assert.Equal(1, target.Notifications.Count); + source.StringValue = null; + + Assert.Null(target.ValidatedDirectString); + } + + protected override DirectPropertyBase GetProperty() => Class1.ValidatedDirectIntProperty; + protected override DirectPropertyBase GetNonValidatedProperty() => Class1.NonValidatedDirectIntProperty; } - [Fact] - public void Bound_Validated_Direct_String_Property_Can_Be_Set_To_Null() + public class StyledPropertyTests : TestBase> { - var source = new ViewModel + [Fact] + public void Bound_Validated_String_Property_Can_Be_Set_To_Null() { - StringValue = "foo", - }; + var source = new ViewModel + { + StringValue = "foo", + }; - var target = new Class1 - { - [!Class1.ValidatedDirectStringProperty] = new Binding + var target = new Class1 { - Path = nameof(ViewModel.StringValue), - Source = source, - }, - }; + [!Class1.ValidatedDirectStringProperty] = new Binding + { + Path = nameof(ViewModel.StringValue), + Source = source, + }, + }; + + Assert.Equal("foo", target.ValidatedDirectString); - Assert.Equal("foo", target.ValidatedDirectString); + source.StringValue = null; - source.StringValue = null; + Assert.Null(target.ValidatedDirectString); + } - Assert.Null(target.ValidatedDirectString); + protected override StyledProperty GetProperty() => Class1.ValidatedStyledIntProperty; + protected override StyledProperty GetNonValidatedProperty() => Class1.NonValidatedStyledIntProperty; } + private record class Notification(BindingValueType type, object? value, Exception? error); + private class Class1 : AvaloniaObject { - public static readonly StyledProperty NonValidatedProperty = - AvaloniaProperty.Register( - nameof(NonValidated)); - - public static readonly DirectProperty NonValidatedDirectProperty = + public static readonly DirectProperty NonValidatedDirectIntProperty = AvaloniaProperty.RegisterDirect( - nameof(NonValidatedDirect), - o => o.NonValidatedDirect, - (o, v) => o.NonValidatedDirect = v); + nameof(NonValidatedDirectInt), + o => o.NonValidatedDirectInt, + (o, v) => o.NonValidatedDirectInt = v); public static readonly DirectProperty ValidatedDirectIntProperty = AvaloniaProperty.RegisterDirect( @@ -118,27 +173,30 @@ namespace Avalonia.Base.UnitTests (o, v) => o.ValidatedDirectInt = v, enableDataValidation: true); - public static readonly DirectProperty ValidatedDirectStringProperty = - AvaloniaProperty.RegisterDirect( + public static readonly DirectProperty ValidatedDirectStringProperty = + AvaloniaProperty.RegisterDirect( nameof(ValidatedDirectString), o => o.ValidatedDirectString, (o, v) => o.ValidatedDirectString = v, enableDataValidation: true); + public static readonly StyledProperty NonValidatedStyledIntProperty = + AvaloniaProperty.Register( + nameof(NonValidatedStyledInt)); + + public static readonly StyledProperty ValidatedStyledIntProperty = + AvaloniaProperty.Register( + nameof(ValidatedStyledInt), + enableDataValidation: true); + private int _nonValidatedDirect; private int _directInt; - private string _directString; - - public int NonValidated - { - get { return GetValue(NonValidatedProperty); } - set { SetValue(NonValidatedProperty, value); } - } + private string? _directString; - public int NonValidatedDirect + public int NonValidatedDirectInt { get { return _directInt; } - set { SetAndRaise(NonValidatedDirectProperty, ref _nonValidatedDirect, value); } + set { SetAndRaise(NonValidatedDirectIntProperty, ref _nonValidatedDirect, value); } } public int ValidatedDirectInt @@ -147,20 +205,32 @@ namespace Avalonia.Base.UnitTests set { SetAndRaise(ValidatedDirectIntProperty, ref _directInt, value); } } - public string ValidatedDirectString + public string? ValidatedDirectString { get { return _directString; } set { SetAndRaise(ValidatedDirectStringProperty, ref _directString, value); } } - public List<(BindingValueType type, object value)> Notifications { get; } = new(); + public int NonValidatedStyledInt + { + get { return GetValue(NonValidatedStyledIntProperty); } + set { SetValue(NonValidatedStyledIntProperty, value); } + } + + public int ValidatedStyledInt + { + get => GetValue(ValidatedStyledIntProperty); + set => SetValue(ValidatedStyledIntProperty, value); + } + + public List Notifications { get; } = new(); protected override void UpdateDataValidation( AvaloniaProperty property, BindingValueType state, - Exception error) + Exception? error) { - Notifications.Add((state, GetValue(property))); + Notifications.Add(new(state, GetValue(property), error)); } } @@ -168,16 +238,18 @@ namespace Avalonia.Base.UnitTests { static Class2() { - NonValidatedDirectProperty.OverrideMetadata( + NonValidatedDirectIntProperty.OverrideMetadata( new DirectPropertyMetadata(enableDataValidation: true)); + NonValidatedStyledIntProperty.OverrideMetadata( + new StyledPropertyMetadata(enableDataValidation: true)); } } public class ViewModel : NotifyingBase { - private string _stringValue; + private string? _stringValue; - public string StringValue + public string? StringValue { get { return _stringValue; } set { _stringValue = value; RaisePropertyChanged(); } diff --git a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs index 505eddb146..de2d01a2ac 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs @@ -1,72 +1,289 @@ using System; -using System.Reactive.Linq; +using System.Collections; +using System.ComponentModel; using Avalonia.Controls; using Avalonia.Data; -using Avalonia.Data.Core; -using Avalonia.Markup.Data; +using Avalonia.Styling; +using Avalonia.UnitTests; using Xunit; +#nullable enable + namespace Avalonia.Markup.UnitTests.Data { public class BindingTests_DataValidation { - [Fact] - public void Initiate_Should_Not_Enable_Data_Validation_With_BindingPriority_LocalValue() + public abstract class TestBase + where T : AvaloniaProperty { - var textBlock = new TextBlock + [Fact] + public void Setter_Exception_Causes_DataValidation_Error() + { + var (target, property) = CreateTarget(); + var binding = new Binding(nameof(ExceptionValidatingModel.Value)) + { + Mode = BindingMode.TwoWay + }; + + target.DataContext = new ExceptionValidatingModel(); + target.Bind(property, binding); + + Assert.Equal(20, target.GetValue(property)); + + target.SetValue(property, 200); + + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + + target.SetValue(property, 10); + + Assert.Equal(10, target.GetValue(property)); + Assert.Null(target.DataValidationError); + } + + [Fact] + public void Indei_Error_Causes_DataValidation_Error() { - DataContext = new Class1(), - }; + var (target, property) = CreateTarget(); + var binding = new Binding(nameof(IndeiValidatingModel.Value)) + { + Mode = BindingMode.TwoWay + }; + + target.DataContext = new IndeiValidatingModel(); + target.Bind(property, binding); + + Assert.Equal(20, target.GetValue(property)); + + target.SetValue(property, 200); + + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value.", target.DataValidationError?.Message); + + target.SetValue(property, 10); - var target = new Binding(nameof(Class1.Foo)); - var instanced = target.Initiate(textBlock, TextBlock.TextProperty, enableDataValidation: false); - var subject = (BindingExpression)instanced.Source; - object result = null; + Assert.Equal(10, target.GetValue(property)); + Assert.Null(target.DataValidationError); + } - subject.Subscribe(x => result = x); + private protected abstract (DataValidationTestControl, T) CreateTarget(); + } - Assert.IsType(result); + public class DirectPropertyTests : TestBase> + { + private protected override (DataValidationTestControl, DirectPropertyBase) CreateTarget() + { + return (new ValidatedDirectPropertyClass(), ValidatedDirectPropertyClass.ValueProperty); + } } - [Fact] - public void Initiate_Should_Enable_Data_Validation_With_BindingPriority_LocalValue() + public class StyledPropertyTests : TestBase> { - var textBlock = new TextBlock + [Fact] + public void Style_Binding_Supports_Indei_Data_Validation() + { + var (target, property) = CreateTarget(); + var binding = new Binding(nameof(IndeiValidatingModel.Value)) + { + Mode = BindingMode.TwoWay + }; + + var root = new TestRoot + { + DataContext = new IndeiValidatingModel(), + Styles = + { + new Style(x => x.Is()) + { + Setters = + { + new Setter(property, binding) + } + } + }, + Child = target, + }; + + root.LayoutManager.ExecuteInitialLayoutPass(); + + Assert.Equal(20, target.GetValue(property)); + + target.SetValue(property, 200); + + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value.", target.DataValidationError?.Message); + + target.SetValue(property, 10); + + Assert.Equal(10, target.GetValue(property)); + Assert.Null(target.DataValidationError); + } + + [Fact] + public void Style_With_Activator_Binding_Supports_Indei_Data_Validation() + { + var (target, property) = CreateTarget(); + var binding = new Binding(nameof(IndeiValidatingModel.Value)) + { + Mode = BindingMode.TwoWay + }; + + var model = new IndeiValidatingModel + { + Value = 200, + }; + + var root = new TestRoot + { + DataContext = model, + Styles = + { + new Style(x => x.Is().Class("foo")) + { + Setters = + { + new Setter(property, binding) + } + } + }, + Child = target, + }; + + root.LayoutManager.ExecuteInitialLayoutPass(); + target.Classes.Add("foo"); + + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value.", target.DataValidationError?.Message); + + target.Classes.Remove("foo"); + Assert.Equal(0, target.GetValue(property)); + Assert.Null(target.DataValidationError); + + target.Classes.Add("foo"); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value.", target.DataValidationError?.Message); + + target.SetValue(property, 10); + + Assert.Equal(10, target.GetValue(property)); + Assert.Null(target.DataValidationError); + } + + private protected override (DataValidationTestControl, StyledProperty) CreateTarget() { - DataContext = new Class1(), - }; + return (new ValidatedStyledPropertyClass(), ValidatedStyledPropertyClass.ValueProperty); + } + } - var target = new Binding(nameof(Class1.Foo)); - var instanced = target.Initiate(textBlock, TextBlock.TextProperty, enableDataValidation: true); - var subject = (BindingExpression)instanced.Source; - object result = null; + internal class DataValidationTestControl : Control + { + public Exception? DataValidationError { get; protected set; } + } - subject.Subscribe(x => result = x); + private class ValidatedStyledPropertyClass : DataValidationTestControl + { + public static readonly StyledProperty ValueProperty = + AvaloniaProperty.Register( + "Value", + enableDataValidation: true); + + public int Value + { + get => GetValue(ValueProperty); + set => SetValue(ValueProperty, value); + } - Assert.Equal(new BindingNotification("foo"), result); + protected override void UpdateDataValidation(AvaloniaProperty property, BindingValueType state, Exception? error) + { + if (property == ValueProperty) + { + DataValidationError = state.HasAnyFlag(BindingValueType.DataValidationError) ? error : null; + } + } } - [Fact] - public void Initiate_Should_Not_Enable_Data_Validation_With_BindingPriority_TemplatedParent() + private class ValidatedDirectPropertyClass : DataValidationTestControl { - var textBlock = new TextBlock + public static readonly DirectProperty ValueProperty = + AvaloniaProperty.RegisterDirect( + "Value", + o => o.Value, + (o, v) => o.Value = v, + enableDataValidation: true); + + private int _value; + + public int Value { - DataContext = new Class1(), - }; + get => _value; + set => SetAndRaise(ValueProperty, ref _value, value); + } - var target = new Binding(nameof(Class1.Foo)) { Priority = BindingPriority.Template }; - var instanced = target.Initiate(textBlock, TextBlock.TextProperty, enableDataValidation: true); - var subject = (BindingExpression)instanced.Source; - object result = null; + protected override void UpdateDataValidation(AvaloniaProperty property, BindingValueType state, Exception? error) + { + if (property == ValueProperty) + { + DataValidationError = state.HasAnyFlag(BindingValueType.DataValidationError) ? error : null; + } + } + } - subject.Subscribe(x => result = x); + private class ExceptionValidatingModel + { + public const int MaxValue = 100; + private int _value = 20; - Assert.IsType(result); + public int Value + { + get => _value; + set + { + if (value > MaxValue) + throw new ArgumentOutOfRangeException(nameof(value)); + _value = value; + } + } } - private class Class1 + private class IndeiValidatingModel : INotifyDataErrorInfo { - public string Foo { get; set; } = "foo"; + public const int MaxValue = 100; + private bool _hasErrors; + private int _value = 20; + + public int Value + { + get => _value; + set + { + _value = value; + HasErrors = value > MaxValue; + } + } + + public bool HasErrors + { + get => _hasErrors; + private set + { + if (_hasErrors != value) + { + _hasErrors = value; + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(nameof(Value))); + } + } + } + + public event EventHandler? ErrorsChanged; + + public IEnumerable GetErrors(string? propertyName) + { + if (propertyName == nameof(Value) && _value > MaxValue) + yield return "Invalid value."; + } } } } From d787a72f7f66621e0bc9a793fd1f5bc34291645b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 17 Feb 2023 11:29:31 +0100 Subject: [PATCH 3/9] Initial impl of data validation for styled properties. Currently only local value bindings implemented. --- src/Avalonia.Base/AvaloniaObject.cs | 16 ++++---- .../LocalValueBindingObserver.cs | 29 +++++++++------ .../LocalValueUntypedBindingObserver.cs | 37 +++++++++---------- src/Avalonia.Base/PropertyStore/ValueStore.cs | 28 ++++++++------ .../AvaloniaObjectTests_Binding.cs | 7 ++-- 5 files changed, 62 insertions(+), 55 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index bf1624eab3..f3a046ef80 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -776,19 +776,19 @@ namespace Avalonia break; } - UpdateDataValidationCore(property, value.Type, value.Error); - } + var metadata = property.GetMetadata(GetType()); - internal void UpdateDataValidationCore(AvaloniaProperty property, - BindingValueType state, - Exception? error) - { - if (property.GetMetadata(GetType()) is { EnableDataValidation: true }) + if (metadata.EnableDataValidation == true) { - UpdateDataValidation(property, state, error); + UpdateDataValidation(property, value.Type, value.Error); } } + internal void OnUpdateDataValidation(AvaloniaProperty property, BindingValueType state, Exception? error) + { + UpdateDataValidation(property, state, error); + } + /// /// Gets a description of an observable that van be used in logs. /// diff --git a/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs index 5908d9e535..24eb00b2fe 100644 --- a/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs +++ b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs @@ -9,6 +9,7 @@ namespace Avalonia.PropertyStore IDisposable { private readonly ValueStore _owner; + private readonly bool _hasDataValidation; private IDisposable? _subscription; private T? _defaultValue; private bool _isDefaultValueInitialized; @@ -17,6 +18,7 @@ namespace Avalonia.PropertyStore { _owner = owner; Property = property; + _hasDataValidation = property.GetMetadata(owner.Owner.GetType()).EnableDataValidation ?? false; } public StyledProperty Property { get;} @@ -51,7 +53,10 @@ namespace Avalonia.PropertyStore if (property.ValidateValue?.Invoke(value) == false) value = instance.GetCachedDefaultValue(); - owner.SetValue(property, value, BindingPriority.LocalValue); + owner.SetLocalValue(property, value); + + if (instance._hasDataValidation) + owner.Owner.OnUpdateDataValidation(property, BindingValueType.Value, null); } if (Dispatcher.UIThread.CheckAccess()) @@ -74,23 +79,23 @@ namespace Avalonia.PropertyStore { var owner = instance._owner; var property = instance.Property; + var originalType = value.Type; LoggingUtils.LogIfNecessary(owner.Owner, property, value); + // Revert to the default value if the binding value fails validation, or if + // there was no value (though not if there was a data validation error). + if ((value.HasValue && property.ValidateValue?.Invoke(value.Value) == false) || + (!value.HasValue && value.Type != BindingValueType.DataValidationError)) + value = value.WithValue(instance.GetCachedDefaultValue()); + if (value.HasValue) - { - var effectiveValue = value.Value; - if (property.ValidateValue?.Invoke(effectiveValue) == false) - effectiveValue = instance.GetCachedDefaultValue(); - owner.SetValue(property, effectiveValue, BindingPriority.LocalValue); - } - else - { - owner.SetValue(property, instance.GetCachedDefaultValue(), BindingPriority.LocalValue); - } + owner.SetLocalValue(property, value.Value); + if (instance._hasDataValidation) + owner.Owner.OnUpdateDataValidation(property, originalType, value.Error); } - if (value.Type is BindingValueType.DoNothing or BindingValueType.DataValidationError) + if (value.Type is BindingValueType.DoNothing) return; if (Dispatcher.UIThread.CheckAccess()) diff --git a/src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs b/src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs index 46e6ed810a..cda11faa1a 100644 --- a/src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs +++ b/src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using Avalonia.Data; using Avalonia.Threading; @@ -8,6 +9,7 @@ namespace Avalonia.PropertyStore IDisposable { private readonly ValueStore _owner; + private readonly bool _hasDataValidation; private IDisposable? _subscription; private T? _defaultValue; private bool _isDefaultValueInitialized; @@ -16,6 +18,7 @@ namespace Avalonia.PropertyStore { _owner = owner; Property = property; + _hasDataValidation = property.GetMetadata(owner.Owner.GetType()).EnableDataValidation ?? false; } public StyledProperty Property { get; } @@ -35,32 +38,28 @@ namespace Avalonia.PropertyStore public void OnCompleted() => _owner.OnLocalValueBindingCompleted(Property, this); public void OnError(Exception error) => OnCompleted(); + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = TrimmingMessages.ImplicitTypeConvertionSupressWarningMessage)] public void OnNext(object? value) { - static void Execute(LocalValueUntypedBindingObserver instance, object? value) + static void Execute(LocalValueUntypedBindingObserver instance, object? untypedValue) { var owner = instance._owner; var property = instance.Property; + var value = BindingValue.FromUntyped(untypedValue, property.PropertyType); + var originalType = value.Type; - if (value is BindingNotification n) - { - value = n.Value; - LoggingUtils.LogIfNecessary(owner.Owner, property, n); - } + LoggingUtils.LogIfNecessary(owner.Owner, property, value); - if (value == AvaloniaProperty.UnsetValue) - { - owner.SetValue(property, instance.GetCachedDefaultValue(), BindingPriority.LocalValue); - } - else if (UntypedValueUtils.TryConvertAndValidate(property, value, out var typedValue)) - { - owner.SetValue(property, typedValue, BindingPriority.LocalValue); - } - else - { - owner.SetValue(property, instance.GetCachedDefaultValue(), BindingPriority.LocalValue); - LoggingUtils.LogInvalidValue(owner.Owner, property, typeof(T), value); - } + // Revert to the default value if the binding value fails validation, or if + // there was no value (though not if there was a data validation error). + if ((value.HasValue && property.ValidateValue?.Invoke(value.Value) == false) || + (!value.HasValue && value.Type != BindingValueType.DataValidationError)) + value = value.WithValue(instance.GetCachedDefaultValue()); + + if (value.HasValue) + owner.SetLocalValue(property, value.Value); + if (instance._hasDataValidation) + owner.Owner.OnUpdateDataValidation(property, originalType, value.Error); } if (value == BindingOperations.DoNothing) diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index ec6ed392c1..9efc91d44d 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -193,18 +193,7 @@ namespace Avalonia.PropertyStore } else { - if (TryGetEffectiveValue(property, out var existing)) - { - var effective = (EffectiveValue)existing; - effective.SetLocalValueAndRaise(this, property, value); - } - else - { - var effectiveValue = CreateEffectiveValue(property); - AddEffectiveValue(property, effectiveValue); - effectiveValue.SetLocalValueAndRaise(this, property, value); - } - + SetLocalValue(property, value); return null; } } @@ -223,6 +212,21 @@ namespace Avalonia.PropertyStore } } + public void SetLocalValue(StyledProperty property, T value) + { + if (TryGetEffectiveValue(property, out var existing)) + { + var effective = (EffectiveValue)existing; + effective.SetLocalValueAndRaise(this, property, value); + } + else + { + var effectiveValue = CreateEffectiveValue(property); + AddEffectiveValue(property, effectiveValue); + effectiveValue.SetLocalValueAndRaise(this, property, value); + } + } + public object? GetValue(AvaloniaProperty property) { if (_effectiveValues.TryGetValue(property, out var v)) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs index b6036bba8f..9f74d2fc08 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs @@ -888,7 +888,8 @@ namespace Avalonia.Base.UnitTests var target = new Class1(); var source = new Subject(); var called = false; - var expectedMessageTemplate = "Error in binding to {Target}.{Property}: expected {ExpectedType}, got {Value} ({ValueType})"; + var expectedMessageTemplate = "Error in binding to {Target}.{Property}: {Message}"; + var message = "Unable to convert object 'foo' of type 'System.String' to type 'System.Double'."; LogCallback checkLogMessage = (level, area, src, mt, pv) => { @@ -898,9 +899,7 @@ namespace Avalonia.Base.UnitTests src == target && pv[0].GetType() == typeof(Class1) && (AvaloniaProperty)pv[1] == Class1.QuxProperty && - (Type)pv[2] == typeof(double) && - (string)pv[3] == "foo" && - (Type)pv[4] == typeof(string)) + (string)pv[2] == message) { called = true; } From 98fa10416f33509c8ef4a2007d67ad336eb4b1a8 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 17 Feb 2023 11:38:49 +0100 Subject: [PATCH 4/9] Use same logic for typed and untyped observers. However we still need to split the observers into two classes because otherwise we can't implement both `IObservable` and `IObservable` on the same object. --- .../LocalValueBindingObserver.cs | 119 ++---------------- ...er.cs => LocalValueBindingObserverBase.cs} | 57 +++++++-- src/Avalonia.Base/PropertyStore/ValueStore.cs | 2 +- 3 files changed, 55 insertions(+), 123 deletions(-) rename src/Avalonia.Base/PropertyStore/{LocalValueUntypedBindingObserver.cs => LocalValueBindingObserverBase.cs} (60%) diff --git a/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs index 24eb00b2fe..9e9b4a3190 100644 --- a/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs +++ b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs @@ -1,126 +1,25 @@ using System; +using System.Diagnostics.CodeAnalysis; using Avalonia.Data; -using Avalonia.Threading; namespace Avalonia.PropertyStore { - internal class LocalValueBindingObserver : IObserver, - IObserver>, - IDisposable + internal class LocalValueBindingObserver : LocalValueBindingObserverBase, + IObserver { - private readonly ValueStore _owner; - private readonly bool _hasDataValidation; - private IDisposable? _subscription; - private T? _defaultValue; - private bool _isDefaultValueInitialized; - public LocalValueBindingObserver(ValueStore owner, StyledProperty property) + : base(owner, property) { - _owner = owner; - Property = property; - _hasDataValidation = property.GetMetadata(owner.Owner.GetType()).EnableDataValidation ?? false; - } - - public StyledProperty Property { get;} - - public void Start(IObservable source) - { - _subscription = source.Subscribe(this); - } - - public void Start(IObservable> source) - { - _subscription = source.Subscribe(this); - } - - public void Dispose() - { - _subscription?.Dispose(); - _subscription = null; - _owner.OnLocalValueBindingCompleted(Property, this); } - public void OnCompleted() => _owner.OnLocalValueBindingCompleted(Property, this); - public void OnError(Exception error) => OnCompleted(); + public void Start(IObservable source) => _subscription = source.Subscribe(this); - public void OnNext(T value) + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = TrimmingMessages.ImplicitTypeConvertionSupressWarningMessage)] + public void OnNext(object? value) { - static void Execute(LocalValueBindingObserver instance, T value) - { - var owner = instance._owner; - var property = instance.Property; - - if (property.ValidateValue?.Invoke(value) == false) - value = instance.GetCachedDefaultValue(); - - owner.SetLocalValue(property, value); - - if (instance._hasDataValidation) - owner.Owner.OnUpdateDataValidation(property, BindingValueType.Value, null); - } - - if (Dispatcher.UIThread.CheckAccess()) - { - Execute(this, value); - } - else - { - // To avoid allocating closure in the outer scope we need to capture variables - // locally. This allows us to skip most of the allocations when on UI thread. - var instance = this; - var newValue = value; - Dispatcher.UIThread.Post(() => Execute(instance, newValue)); - } - } - - public void OnNext(BindingValue value) - { - static void Execute(LocalValueBindingObserver instance, BindingValue value) - { - var owner = instance._owner; - var property = instance.Property; - var originalType = value.Type; - - LoggingUtils.LogIfNecessary(owner.Owner, property, value); - - // Revert to the default value if the binding value fails validation, or if - // there was no value (though not if there was a data validation error). - if ((value.HasValue && property.ValidateValue?.Invoke(value.Value) == false) || - (!value.HasValue && value.Type != BindingValueType.DataValidationError)) - value = value.WithValue(instance.GetCachedDefaultValue()); - - if (value.HasValue) - owner.SetLocalValue(property, value.Value); - if (instance._hasDataValidation) - owner.Owner.OnUpdateDataValidation(property, originalType, value.Error); - } - - if (value.Type is BindingValueType.DoNothing) + if (value == BindingOperations.DoNothing) return; - - if (Dispatcher.UIThread.CheckAccess()) - { - Execute(this, value); - } - else - { - // To avoid allocating closure in the outer scope we need to capture variables - // locally. This allows us to skip most of the allocations when on UI thread. - var instance = this; - var newValue = value; - Dispatcher.UIThread.Post(() => Execute(instance, newValue)); - } - } - - private T GetCachedDefaultValue() - { - if (!_isDefaultValueInitialized) - { - _defaultValue = Property.GetDefaultValue(_owner.Owner.GetType()); - _isDefaultValueInitialized = true; - } - - return _defaultValue!; + base.OnNext(BindingValue.FromUntyped(value, Property.PropertyType)); } } } diff --git a/src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserverBase.cs similarity index 60% rename from src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs rename to src/Avalonia.Base/PropertyStore/LocalValueBindingObserverBase.cs index cda11faa1a..85de33d9e0 100644 --- a/src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs +++ b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserverBase.cs @@ -1,29 +1,34 @@ using System; -using System.Diagnostics.CodeAnalysis; using Avalonia.Data; using Avalonia.Threading; namespace Avalonia.PropertyStore { - internal class LocalValueUntypedBindingObserver : IObserver, + internal class LocalValueBindingObserverBase : IObserver, + IObserver>, IDisposable { private readonly ValueStore _owner; private readonly bool _hasDataValidation; - private IDisposable? _subscription; + protected IDisposable? _subscription; private T? _defaultValue; private bool _isDefaultValueInitialized; - public LocalValueUntypedBindingObserver(ValueStore owner, StyledProperty property) + protected LocalValueBindingObserverBase(ValueStore owner, StyledProperty property) { _owner = owner; Property = property; _hasDataValidation = property.GetMetadata(owner.Owner.GetType()).EnableDataValidation ?? false; } - public StyledProperty Property { get; } + public StyledProperty Property { get;} - public void Start(IObservable source) + public void Start(IObservable source) + { + _subscription = source.Subscribe(this); + } + + public void Start(IObservable> source) { _subscription = source.Subscribe(this); } @@ -38,14 +43,42 @@ namespace Avalonia.PropertyStore public void OnCompleted() => _owner.OnLocalValueBindingCompleted(Property, this); public void OnError(Exception error) => OnCompleted(); - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = TrimmingMessages.ImplicitTypeConvertionSupressWarningMessage)] - public void OnNext(object? value) + public void OnNext(T value) + { + static void Execute(LocalValueBindingObserverBase instance, T value) + { + var owner = instance._owner; + var property = instance.Property; + + if (property.ValidateValue?.Invoke(value) == false) + value = instance.GetCachedDefaultValue(); + + owner.SetLocalValue(property, value); + + if (instance._hasDataValidation) + owner.Owner.OnUpdateDataValidation(property, BindingValueType.Value, null); + } + + if (Dispatcher.UIThread.CheckAccess()) + { + Execute(this, value); + } + else + { + // To avoid allocating closure in the outer scope we need to capture variables + // locally. This allows us to skip most of the allocations when on UI thread. + var instance = this; + var newValue = value; + Dispatcher.UIThread.Post(() => Execute(instance, newValue)); + } + } + + public void OnNext(BindingValue value) { - static void Execute(LocalValueUntypedBindingObserver instance, object? untypedValue) + static void Execute(LocalValueBindingObserverBase instance, BindingValue value) { var owner = instance._owner; var property = instance.Property; - var value = BindingValue.FromUntyped(untypedValue, property.PropertyType); var originalType = value.Type; LoggingUtils.LogIfNecessary(owner.Owner, property, value); @@ -62,14 +95,14 @@ namespace Avalonia.PropertyStore owner.Owner.OnUpdateDataValidation(property, originalType, value.Error); } - if (value == BindingOperations.DoNothing) + if (value.Type is BindingValueType.DoNothing) return; if (Dispatcher.UIThread.CheckAccess()) { Execute(this, value); } - else if (value != BindingOperations.DoNothing) + else { // To avoid allocating closure in the outer scope we need to capture variables // locally. This allows us to skip most of the allocations when on UI thread. diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index 9efc91d44d..0a5084466f 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -104,7 +104,7 @@ namespace Avalonia.PropertyStore { if (priority == BindingPriority.LocalValue) { - var observer = new LocalValueUntypedBindingObserver(this, property); + var observer = new LocalValueBindingObserver(this, property); DisposeExistingLocalValueBinding(property); _localValueBindings ??= new(); _localValueBindings[property.Id] = observer; From 172a75e438cc27cca4194863e0e2817732beb735 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 17 Feb 2023 12:13:00 +0100 Subject: [PATCH 5/9] Support data validation on style bindings. --- .../PropertyStore/BindingEntryBase.cs | 28 +++++++++++++++---- .../PropertyStore/ImmediateValueFrame.cs | 6 ++-- .../SourceUntypedBindingEntry.cs | 3 +- .../PropertyStore/TypedBindingEntry.cs | 6 ++-- .../PropertyStore/UntypedBindingEntry.cs | 3 +- .../Styling/PropertySetterBindingInstance.cs | 2 +- src/Avalonia.Base/Styling/Setter.cs | 3 +- 7 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs index e1ff0970c2..4a00abf1ce 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using Avalonia.Reactive; using Avalonia.Data; using Avalonia.Threading; +using static Avalonia.Rendering.Composition.Animations.PropertySetSnapshot; namespace Avalonia.PropertyStore { @@ -13,6 +14,7 @@ namespace Avalonia.PropertyStore { private static readonly IDisposable s_creating = Disposable.Empty; private static readonly IDisposable s_creatingQuiet = Disposable.Create(() => { }); + private readonly bool _hasDataValidation; private IDisposable? _subscription; private bool _hasValue; private TValue? _value; @@ -20,6 +22,7 @@ namespace Avalonia.PropertyStore private bool _isDefaultValueInitialized; protected BindingEntryBase( + AvaloniaObject target, ValueFrame frame, AvaloniaProperty property, IObservable> source) @@ -27,9 +30,11 @@ namespace Avalonia.PropertyStore Frame = frame; Source = source; Property = property; + _hasDataValidation = property.GetMetadata(target.GetType()).EnableDataValidation ?? false; } protected BindingEntryBase( + AvaloniaObject target, ValueFrame frame, AvaloniaProperty property, IObservable source) @@ -37,6 +42,7 @@ namespace Avalonia.PropertyStore Frame = frame; Source = source; Property = property; + _hasDataValidation = property.GetMetadata(target.GetType()).EnableDataValidation ?? false; } public bool HasValue @@ -79,6 +85,9 @@ namespace Avalonia.PropertyStore { _subscription?.Dispose(); _subscription = null; + + if (_hasDataValidation) + Frame.Owner?.Owner.OnUpdateDataValidation(Property, BindingValueType.UnsetValue, null); } object? IValueEntry.GetValue() @@ -111,20 +120,29 @@ namespace Avalonia.PropertyStore { static void Execute(BindingEntryBase instance, BindingValue value) { - if (instance.Frame.Owner is null) + if (instance.Frame.Owner is not { } valueStore) return; - LoggingUtils.LogIfNecessary(instance.Frame.Owner.Owner, instance.Property, value); + var owner = valueStore.Owner; + var property = instance.Property; + var originalType = value.Type; + + LoggingUtils.LogIfNecessary(owner, property, value); - var effectiveValue = value.HasValue ? value.Value : instance.GetCachedDefaultValue(); + if (!value.HasValue && value.Type != BindingValueType.DataValidationError) + value = value.WithValue(instance.GetCachedDefaultValue()); - if (!instance._hasValue || !EqualityComparer.Default.Equals(instance._value, effectiveValue)) + if (value.HasValue && + (!instance._hasValue || !EqualityComparer.Default.Equals(instance._value, value.Value))) { - instance._value = effectiveValue; + instance._value = value.Value; instance._hasValue = true; if (instance._subscription is not null && instance._subscription != s_creatingQuiet) instance.Frame.Owner?.OnBindingValueChanged(instance, instance.Frame.Priority); } + + if (instance._hasDataValidation) + owner.OnUpdateDataValidation(property, originalType, value.Error); } if (value.Type == BindingValueType.DoNothing) diff --git a/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs b/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs index 7e9f3ab312..222d857aa3 100644 --- a/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs +++ b/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs @@ -18,7 +18,7 @@ namespace Avalonia.PropertyStore StyledProperty property, IObservable> source) { - var e = new TypedBindingEntry(this, property, source); + var e = new TypedBindingEntry(Owner!.Owner, this, property, source); Add(e); return e; } @@ -27,7 +27,7 @@ namespace Avalonia.PropertyStore StyledProperty property, IObservable source) { - var e = new TypedBindingEntry(this, property, source); + var e = new TypedBindingEntry(Owner!.Owner, this, property, source); Add(e); return e; } @@ -36,7 +36,7 @@ namespace Avalonia.PropertyStore StyledProperty property, IObservable source) { - var e = new SourceUntypedBindingEntry(this, property, source); + var e = new SourceUntypedBindingEntry(Owner!.Owner, this, property, source); Add(e); return e; } diff --git a/src/Avalonia.Base/PropertyStore/SourceUntypedBindingEntry.cs b/src/Avalonia.Base/PropertyStore/SourceUntypedBindingEntry.cs index b82714817b..99c6a3ee9d 100644 --- a/src/Avalonia.Base/PropertyStore/SourceUntypedBindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/SourceUntypedBindingEntry.cs @@ -12,10 +12,11 @@ namespace Avalonia.PropertyStore private readonly Func? _validate; public SourceUntypedBindingEntry( + AvaloniaObject target, ValueFrame frame, StyledProperty property, IObservable source) - : base(frame, property, source) + : base(target, frame, property, source) { _validate = property.ValidateValue; } diff --git a/src/Avalonia.Base/PropertyStore/TypedBindingEntry.cs b/src/Avalonia.Base/PropertyStore/TypedBindingEntry.cs index 550f5c0001..c209138605 100644 --- a/src/Avalonia.Base/PropertyStore/TypedBindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/TypedBindingEntry.cs @@ -10,18 +10,20 @@ namespace Avalonia.PropertyStore internal sealed class TypedBindingEntry : BindingEntryBase { public TypedBindingEntry( + AvaloniaObject target, ValueFrame frame, StyledProperty property, IObservable source) - : base(frame, property, source) + : base(target, frame, property, source) { } public TypedBindingEntry( + AvaloniaObject target, ValueFrame frame, StyledProperty property, IObservable> source) - : base(frame, property, source) + : base(target, frame, property, source) { } diff --git a/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs b/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs index a77d7fddb6..e3a7607479 100644 --- a/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs @@ -12,10 +12,11 @@ namespace Avalonia.PropertyStore private readonly Func? _validate; public UntypedBindingEntry( + AvaloniaObject target, ValueFrame frame, AvaloniaProperty property, IObservable source) - : base(frame, property, source) + : base(target, frame, property, source) { _validate = ((IStyledPropertyAccessor)property).ValidateValue; } diff --git a/src/Avalonia.Base/Styling/PropertySetterBindingInstance.cs b/src/Avalonia.Base/Styling/PropertySetterBindingInstance.cs index 826b45582d..be5a999771 100644 --- a/src/Avalonia.Base/Styling/PropertySetterBindingInstance.cs +++ b/src/Avalonia.Base/Styling/PropertySetterBindingInstance.cs @@ -15,7 +15,7 @@ namespace Avalonia.Styling AvaloniaProperty property, BindingMode mode, IObservable source) - : base(instance, property, source) + : base(target, instance, property, source) { _target = target; _mode = mode; diff --git a/src/Avalonia.Base/Styling/Setter.cs b/src/Avalonia.Base/Styling/Setter.cs index 093597c6a0..5586132301 100644 --- a/src/Avalonia.Base/Styling/Setter.cs +++ b/src/Avalonia.Base/Styling/Setter.cs @@ -99,7 +99,8 @@ namespace Avalonia.Styling { if (!Property!.IsDirect) { - var i = binding.Initiate(target, Property)!; + var hasDataValidation = Property.GetMetadata(target.GetType()).EnableDataValidation ?? false; + var i = binding.Initiate(target, Property, enableDataValidation: hasDataValidation)!; var mode = i.Mode; if (mode == BindingMode.Default) From 23267b7967f73135419a9fbbc18b554aafca214a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 17 Feb 2023 12:35:59 +0100 Subject: [PATCH 6/9] Added additional failing tests. --- .../Data/BindingTests_DataValidation.cs | 104 ++++++++++++++++-- 1 file changed, 97 insertions(+), 7 deletions(-) diff --git a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs index de2d01a2ac..b76c86e814 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs @@ -59,7 +59,7 @@ namespace Avalonia.Markup.UnitTests.Data Assert.Equal(200, target.GetValue(property)); Assert.IsType(target.DataValidationError); - Assert.Equal("Invalid value.", target.DataValidationError?.Message); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); target.SetValue(property, 10); @@ -81,7 +81,7 @@ namespace Avalonia.Markup.UnitTests.Data public class StyledPropertyTests : TestBase> { [Fact] - public void Style_Binding_Supports_Indei_Data_Validation() + public void Style_Binding_Supports_Data_Validation() { var (target, property) = CreateTarget(); var binding = new Binding(nameof(IndeiValidatingModel.Value)) @@ -113,7 +113,7 @@ namespace Avalonia.Markup.UnitTests.Data Assert.Equal(200, target.GetValue(property)); Assert.IsType(target.DataValidationError); - Assert.Equal("Invalid value.", target.DataValidationError?.Message); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); target.SetValue(property, 10); @@ -122,7 +122,7 @@ namespace Avalonia.Markup.UnitTests.Data } [Fact] - public void Style_With_Activator_Binding_Supports_Indei_Data_Validation() + public void Style_With_Activator_Binding_Supports_Data_Validation() { var (target, property) = CreateTarget(); var binding = new Binding(nameof(IndeiValidatingModel.Value)) @@ -156,7 +156,7 @@ namespace Avalonia.Markup.UnitTests.Data Assert.Equal(200, target.GetValue(property)); Assert.IsType(target.DataValidationError); - Assert.Equal("Invalid value.", target.DataValidationError?.Message); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); target.Classes.Remove("foo"); Assert.Equal(0, target.GetValue(property)); @@ -164,7 +164,7 @@ namespace Avalonia.Markup.UnitTests.Data target.Classes.Add("foo"); Assert.IsType(target.DataValidationError); - Assert.Equal("Invalid value.", target.DataValidationError?.Message); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); target.SetValue(property, 10); @@ -172,6 +172,96 @@ namespace Avalonia.Markup.UnitTests.Data Assert.Null(target.DataValidationError); } + [Fact] + public void Data_Validation_Can_Switch_Between_Style_And_LocalValue_Binding() + { + var (target, property) = CreateTarget(); + var model1 = new IndeiValidatingModel { Value = 200 }; + var model2 = new IndeiValidatingModel { Value = 300 }; + var binding1 = new Binding(nameof(IndeiValidatingModel.Value)); + var binding2 = new Binding(nameof(IndeiValidatingModel.Value)) { Source = model2 }; + + var root = new TestRoot + { + DataContext = model1, + Styles = + { + new Style(x => x.Is()) + { + Setters = + { + new Setter(property, binding1) + } + } + }, + Child = target, + }; + + root.LayoutManager.ExecuteInitialLayoutPass(); + + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); + + var sub = target.Bind(property, binding2); + Assert.Equal(300, target.GetValue(property)); + Assert.Equal("Invalid value: 300.", target.DataValidationError?.Message); + + sub.Dispose(); + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); + } + + + [Fact] + public void Data_Validation_Can_Switch_Between_Style_And_StyleTrigger_Binding() + { + var (target, property) = CreateTarget(); + var model1 = new IndeiValidatingModel { Value = 200 }; + var model2 = new IndeiValidatingModel { Value = 300 }; + var binding1 = new Binding(nameof(IndeiValidatingModel.Value)); + var binding2 = new Binding(nameof(IndeiValidatingModel.Value)) { Source = model2 }; + + var root = new TestRoot + { + DataContext = model1, + Styles = + { + new Style(x => x.Is()) + { + Setters = + { + new Setter(property, binding1) + } + }, + new Style(x => x.Is().Class("foo")) + { + Setters = + { + new Setter(property, binding2) + } + }, + }, + Child = target, + }; + + root.LayoutManager.ExecuteInitialLayoutPass(); + + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); + + target.Classes.Add("foo"); + Assert.Equal(300, target.GetValue(property)); + Assert.Equal("Invalid value: 300.", target.DataValidationError?.Message); + + target.Classes.Remove("foo"); + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); + } + private protected override (DataValidationTestControl, StyledProperty) CreateTarget() { return (new ValidatedStyledPropertyClass(), ValidatedStyledPropertyClass.ValueProperty); @@ -282,7 +372,7 @@ namespace Avalonia.Markup.UnitTests.Data public IEnumerable GetErrors(string? propertyName) { if (propertyName == nameof(Value) && _value > MaxValue) - yield return "Invalid value."; + yield return $"Invalid value: {_value}."; } } } From 7b2a659725f15fdf23eea73e384017ddadf89e10 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 20 Feb 2023 11:52:58 +0100 Subject: [PATCH 7/9] Added IValueEntry.GetDataValidationState. Implemented only on `BindingEntryBase` as only bindings can supply data validation state. --- .../PropertyStore/BindingEntryBase.cs | 67 +++++++++++++------ .../PropertyStore/IValueEntry.cs | 11 +++ .../PropertyStore/ImmediateValueEntry.cs | 8 +++ .../Styling/PropertySetterTemplateInstance.cs | 8 +++ src/Avalonia.Base/Styling/Setter.cs | 7 ++ 5 files changed, 82 insertions(+), 19 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs index 4a00abf1ce..2fe3844a74 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs @@ -14,23 +14,18 @@ namespace Avalonia.PropertyStore { private static readonly IDisposable s_creating = Disposable.Empty; private static readonly IDisposable s_creatingQuiet = Disposable.Create(() => { }); - private readonly bool _hasDataValidation; private IDisposable? _subscription; private bool _hasValue; private TValue? _value; - private TValue? _defaultValue; - private bool _isDefaultValueInitialized; + private UncommonFields? _uncommon; protected BindingEntryBase( AvaloniaObject target, ValueFrame frame, AvaloniaProperty property, IObservable> source) + : this(target, frame, property, (object)source) { - Frame = frame; - Source = source; - Property = property; - _hasDataValidation = property.GetMetadata(target.GetType()).EnableDataValidation ?? false; } protected BindingEntryBase( @@ -38,11 +33,21 @@ namespace Avalonia.PropertyStore ValueFrame frame, AvaloniaProperty property, IObservable source) + : this(target, frame, property, (object)source) + { + } + + private BindingEntryBase( + AvaloniaObject target, + ValueFrame frame, + AvaloniaProperty property, + object source) { Frame = frame; - Source = source; Property = property; - _hasDataValidation = property.GetMetadata(target.GetType()).EnableDataValidation ?? false; + Source = source; + if (property.GetMetadata(target.GetType()).EnableDataValidation == true) + _uncommon = new() { _hasDataValidation = true }; } public bool HasValue @@ -74,6 +79,20 @@ namespace Avalonia.PropertyStore return _value!; } + public bool GetDataValidationState(out BindingValueType state, out Exception? error) + { + if (_uncommon?._hasDataValidation == true) + { + state = _uncommon._dataValidationState; + error = _uncommon._dataValidationError; + return true; + } + + state = BindingValueType.Value; + error = null; + return false; + } + public void Start() => Start(true); public void OnCompleted() => BindingCompleted(); @@ -85,9 +104,6 @@ namespace Avalonia.PropertyStore { _subscription?.Dispose(); _subscription = null; - - if (_hasDataValidation) - Frame.Owner?.Owner.OnUpdateDataValidation(Property, BindingValueType.UnsetValue, null); } object? IValueEntry.GetValue() @@ -132,6 +148,12 @@ namespace Avalonia.PropertyStore if (!value.HasValue && value.Type != BindingValueType.DataValidationError) value = value.WithValue(instance.GetCachedDefaultValue()); + if (instance._uncommon?._hasDataValidation == true) + { + instance._uncommon._dataValidationState = value.Type; + instance._uncommon._dataValidationError = value.Error; + } + if (value.HasValue && (!instance._hasValue || !EqualityComparer.Default.Equals(instance._value, value.Value))) { @@ -140,9 +162,6 @@ namespace Avalonia.PropertyStore if (instance._subscription is not null && instance._subscription != s_creatingQuiet) instance.Frame.Owner?.OnBindingValueChanged(instance, instance.Frame.Priority); } - - if (instance._hasDataValidation) - owner.OnUpdateDataValidation(property, originalType, value.Error); } if (value.Type == BindingValueType.DoNothing) @@ -170,13 +189,23 @@ namespace Avalonia.PropertyStore private TValue GetCachedDefaultValue() { - if (!_isDefaultValueInitialized) + if (_uncommon?._isDefaultValueInitialized != true) { - _defaultValue = GetDefaultValue(Frame.Owner!.Owner.GetType()); - _isDefaultValueInitialized = true; + _uncommon ??= new(); + _uncommon._defaultValue = GetDefaultValue(Frame.Owner!.Owner.GetType()); + _uncommon._isDefaultValueInitialized = true; } - return _defaultValue!; + return _uncommon._defaultValue!; + } + + private class UncommonFields + { + public TValue? _defaultValue; + public bool _isDefaultValueInitialized; + public bool _hasDataValidation; + public BindingValueType _dataValidationState; + public Exception? _dataValidationError; } } } diff --git a/src/Avalonia.Base/PropertyStore/IValueEntry.cs b/src/Avalonia.Base/PropertyStore/IValueEntry.cs index 271d85f8bc..5898bef491 100644 --- a/src/Avalonia.Base/PropertyStore/IValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/IValueEntry.cs @@ -1,4 +1,5 @@ using System; +using Avalonia.Data; namespace Avalonia.PropertyStore { @@ -22,6 +23,16 @@ namespace Avalonia.PropertyStore /// object? GetValue(); + /// + /// Gets the data validation state if supported. + /// + /// The binding validation state. + /// The current binding error, if any. + /// + /// True if the entry supports data validation, otherwise false. + /// + bool GetDataValidationState(out BindingValueType state, out Exception? error); + /// /// Called when the value entry is removed from the value store. /// diff --git a/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs b/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs index d8a353dc70..16b96eff5d 100644 --- a/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs @@ -1,4 +1,5 @@ using System; +using Avalonia.Data; namespace Avalonia.PropertyStore { @@ -27,5 +28,12 @@ namespace Avalonia.PropertyStore object? IValueEntry.GetValue() => _value; T IValueEntry.GetValue() => _value; + + bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error) + { + state = BindingValueType.Value; + error = null; + return false; + } } } diff --git a/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs b/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs index 7a39407ba2..7604c26244 100644 --- a/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs +++ b/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs @@ -1,4 +1,5 @@ using System; +using Avalonia.Data; using Avalonia.PropertyStore; namespace Avalonia.Styling @@ -19,6 +20,13 @@ namespace Avalonia.Styling public object? GetValue() => _value ??= _template.Build(); + bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error) + { + state = BindingValueType.Value; + error = null; + return false; + } + void IValueEntry.Unsubscribe() { } } } diff --git a/src/Avalonia.Base/Styling/Setter.cs b/src/Avalonia.Base/Styling/Setter.cs index 5586132301..9b009be6d2 100644 --- a/src/Avalonia.Base/Styling/Setter.cs +++ b/src/Avalonia.Base/Styling/Setter.cs @@ -90,6 +90,13 @@ namespace Avalonia.Styling object? IValueEntry.GetValue() => Value; + bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error) + { + state = BindingValueType.Value; + error = null; + return false; + } + private AvaloniaProperty EnsureProperty() { return Property ?? throw new InvalidOperationException("Setter.Property must be set."); From a899185afb4bc95d18f48561b73eb38727dd4f40 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 20 Feb 2023 11:55:58 +0100 Subject: [PATCH 8/9] Update data validation from EffectiveValue. Requires `ValueEntry`/`BaseValueEntry` to be available to `EffectiveValue`. Also change the tests to only test values coming back from the model in the style binding tests, as they don't work when setting local values currently. This will be fixed later. --- .../PropertyStore/BindingEntryBase.cs | 1 - .../PropertyStore/EffectiveValue.cs | 51 ++++++++++--------- .../PropertyStore/EffectiveValue`1.cs | 26 +++++++--- src/Avalonia.Base/PropertyStore/ValueStore.cs | 7 +-- .../Data/BindingTests_DataValidation.cs | 9 ++-- 5 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs index 2fe3844a74..a841803ee1 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using Avalonia.Reactive; using Avalonia.Data; using Avalonia.Threading; -using static Avalonia.Rendering.Composition.Animations.PropertySetSnapshot; namespace Avalonia.PropertyStore { diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue.cs index 78f0ad46b7..11a4dd7893 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue.cs @@ -11,9 +11,6 @@ namespace Avalonia.PropertyStore /// internal abstract class EffectiveValue { - private IValueEntry? _valueEntry; - private IValueEntry? _baseValueEntry; - /// /// Gets the current effective value as a boxed value. /// @@ -29,6 +26,16 @@ namespace Avalonia.PropertyStore /// public BindingPriority BasePriority { get; protected set; } + /// + /// Gets the active value entry for the current effective value. + /// + public IValueEntry? ValueEntry { get; private set; } + + /// + /// Gets the active value entry for the current base value. + /// + public IValueEntry? BaseValueEntry { get; private set; } + /// /// Gets a value indicating whether the was overridden by a call to /// . @@ -63,14 +70,14 @@ namespace Avalonia.PropertyStore { if (Priority == BindingPriority.Unset) { - _valueEntry?.Unsubscribe(); - _valueEntry = null; + ValueEntry?.Unsubscribe(); + ValueEntry = null; } if (BasePriority == BindingPriority.Unset) { - _baseValueEntry?.Unsubscribe(); - _baseValueEntry = null; + BaseValueEntry?.Unsubscribe(); + BaseValueEntry = null; } } @@ -135,40 +142,34 @@ namespace Avalonia.PropertyStore // value, then the current entry becomes our base entry. if (Priority > BindingPriority.LocalValue && Priority < BindingPriority.Inherited) { - Debug.Assert(_valueEntry is not null); - _baseValueEntry = _valueEntry; - _valueEntry = null; + Debug.Assert(ValueEntry is not null); + BaseValueEntry = ValueEntry; + ValueEntry = null; } - if (_valueEntry != entry) + if (ValueEntry != entry) { - _valueEntry?.Unsubscribe(); - _valueEntry = entry; + ValueEntry?.Unsubscribe(); + ValueEntry = entry; } } else if (Priority <= BindingPriority.Animation) { // We've received a non-animation value and have an active animation value, so the // new entry becomes our base entry. - if (_baseValueEntry != entry) + if (BaseValueEntry != entry) { - _baseValueEntry?.Unsubscribe(); - _baseValueEntry = entry; + BaseValueEntry?.Unsubscribe(); + BaseValueEntry = entry; } } - else if (_valueEntry != entry) + else if (ValueEntry != entry) { // Both the current value and the new value are non-animation values, so the new // entry replaces the existing entry. - _valueEntry?.Unsubscribe(); - _valueEntry = entry; + ValueEntry?.Unsubscribe(); + ValueEntry = entry; } } - - protected void UnsubscribeValueEntries() - { - _valueEntry?.Unsubscribe(); - _baseValueEntry?.Unsubscribe(); - } } } diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs index c469034f9b..0788b39459 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using Avalonia.Data; +using static Avalonia.Rendering.Composition.Animations.PropertySetSnapshot; namespace Avalonia.PropertyStore { @@ -61,6 +62,12 @@ namespace Avalonia.PropertyStore UpdateValueEntry(value, priority); SetAndRaiseCore(owner, (StyledProperty)value.Property, GetValue(value), priority, false); + + if (priority > BindingPriority.LocalValue && + value.GetDataValidationState(out var state, out var error)) + { + owner.Owner.OnUpdateDataValidation(value.Property, state, error); + } } public void SetLocalValueAndRaise( @@ -128,12 +135,10 @@ namespace Avalonia.PropertyStore public override void DisposeAndRaiseUnset(ValueStore owner, AvaloniaProperty property) { - UnsubscribeValueEntries(); - DisposeAndRaiseUnset(owner, (StyledProperty)property); - } + ValueEntry?.Unsubscribe(); + BaseValueEntry?.Unsubscribe(); - public void DisposeAndRaiseUnset(ValueStore owner, StyledProperty property) - { + var p = (StyledProperty)property; BindingPriority priority; T oldValue; @@ -150,9 +155,16 @@ namespace Avalonia.PropertyStore if (!EqualityComparer.Default.Equals(oldValue, Value)) { - owner.Owner.RaisePropertyChanged(property, Value, oldValue, priority, true); + owner.Owner.RaisePropertyChanged(p, Value, oldValue, priority, true); if (property.Inherits) - owner.OnInheritedEffectiveValueDisposed(property, Value); + owner.OnInheritedEffectiveValueDisposed(p, Value); + } + + if (ValueEntry?.GetDataValidationState(out _, out _) ?? + BaseValueEntry?.GetDataValidationState(out _, out _) ?? + false) + { + owner.Owner.OnUpdateDataValidation(p, BindingValueType.UnsetValue, null); } } diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index 0a5084466f..0887f11ec9 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -838,8 +838,6 @@ namespace Avalonia.PropertyStore break; } - current?.EndReevaluation(); - if (current?.Priority == BindingPriority.Unset) { if (current.BasePriority == BindingPriority.Unset) @@ -852,6 +850,8 @@ namespace Avalonia.PropertyStore current.RemoveAnimationAndRaise(this, property); } } + + current?.EndReevaluation(); } finally { @@ -923,7 +923,6 @@ namespace Avalonia.PropertyStore for (var i = _effectiveValues.Count - 1; i >= 0; --i) { _effectiveValues.GetKeyValue(i, out var key, out var e); - e.EndReevaluation(); if (e.Priority == BindingPriority.Unset) { @@ -933,6 +932,8 @@ namespace Avalonia.PropertyStore if (i > _effectiveValues.Count) break; } + + e.EndReevaluation(); } } finally diff --git a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs index b76c86e814..5de703deb1 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs @@ -89,9 +89,10 @@ namespace Avalonia.Markup.UnitTests.Data Mode = BindingMode.TwoWay }; + var model = new IndeiValidatingModel(); var root = new TestRoot { - DataContext = new IndeiValidatingModel(), + DataContext = model, Styles = { new Style(x => x.Is()) @@ -109,13 +110,13 @@ namespace Avalonia.Markup.UnitTests.Data Assert.Equal(20, target.GetValue(property)); - target.SetValue(property, 200); + model.Value = 200; Assert.Equal(200, target.GetValue(property)); Assert.IsType(target.DataValidationError); Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); - target.SetValue(property, 10); + model.Value = 10; Assert.Equal(10, target.GetValue(property)); Assert.Null(target.DataValidationError); @@ -166,7 +167,7 @@ namespace Avalonia.Markup.UnitTests.Data Assert.IsType(target.DataValidationError); Assert.Equal("Invalid value: 200.", target.DataValidationError?.Message); - target.SetValue(property, 10); + model.Value = 10; Assert.Equal(10, target.GetValue(property)); Assert.Null(target.DataValidationError); From 40ab20146f6950e21b073b6446b8f3f6e9ad6f1c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 21 Feb 2023 11:42:45 +0100 Subject: [PATCH 9/9] Clear data validation when binding completes. --- .../PropertyStore/DirectBindingObserver.cs | 11 ++++- .../DirectUntypedBindingObserver.cs | 5 +++ .../LocalValueBindingObserverBase.cs | 9 +++- .../AvaloniaObjectTests_DataValidation.cs | 42 +++++++++++++++++++ .../Data/BindingTests_DataValidation.cs | 24 +++++++++++ 5 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/DirectBindingObserver.cs b/src/Avalonia.Base/PropertyStore/DirectBindingObserver.cs index cbe2435953..4bf98e3f7b 100644 --- a/src/Avalonia.Base/PropertyStore/DirectBindingObserver.cs +++ b/src/Avalonia.Base/PropertyStore/DirectBindingObserver.cs @@ -9,11 +9,13 @@ namespace Avalonia.PropertyStore IDisposable { private readonly ValueStore _owner; + private readonly bool _hasDataValidation; private IDisposable? _subscription; public DirectBindingObserver(ValueStore owner, DirectPropertyBase property) { _owner = owner; + _hasDataValidation = property.GetMetadata(owner.Owner.GetType())?.EnableDataValidation ?? false; Property = property; } @@ -33,10 +35,17 @@ namespace Avalonia.PropertyStore { _subscription?.Dispose(); _subscription = null; + OnCompleted(); + } + + public void OnCompleted() + { _owner.OnLocalValueBindingCompleted(Property, this); + + if (_hasDataValidation) + _owner.Owner.OnUpdateDataValidation(Property, BindingValueType.UnsetValue, null); } - public void OnCompleted() => _owner.OnLocalValueBindingCompleted(Property, this); public void OnError(Exception error) => OnCompleted(); public void OnNext(T value) diff --git a/src/Avalonia.Base/PropertyStore/DirectUntypedBindingObserver.cs b/src/Avalonia.Base/PropertyStore/DirectUntypedBindingObserver.cs index 5d60b44bef..1cf108df9b 100644 --- a/src/Avalonia.Base/PropertyStore/DirectUntypedBindingObserver.cs +++ b/src/Avalonia.Base/PropertyStore/DirectUntypedBindingObserver.cs @@ -10,11 +10,13 @@ namespace Avalonia.PropertyStore IDisposable { private readonly ValueStore _owner; + private readonly bool _hasDataValidation; private IDisposable? _subscription; public DirectUntypedBindingObserver(ValueStore owner, DirectPropertyBase property) { _owner = owner; + _hasDataValidation = property.GetMetadata(owner.Owner.GetType())?.EnableDataValidation ?? false; Property = property; } @@ -30,6 +32,9 @@ namespace Avalonia.PropertyStore _subscription?.Dispose(); _subscription = null; _owner.OnLocalValueBindingCompleted(Property, this); + + if (_hasDataValidation) + _owner.Owner.OnUpdateDataValidation(Property, BindingValueType.UnsetValue, null); } public void OnCompleted() => _owner.OnLocalValueBindingCompleted(Property, this); diff --git a/src/Avalonia.Base/PropertyStore/LocalValueBindingObserverBase.cs b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserverBase.cs index 85de33d9e0..5d920cf88d 100644 --- a/src/Avalonia.Base/PropertyStore/LocalValueBindingObserverBase.cs +++ b/src/Avalonia.Base/PropertyStore/LocalValueBindingObserverBase.cs @@ -37,10 +37,17 @@ namespace Avalonia.PropertyStore { _subscription?.Dispose(); _subscription = null; + OnCompleted(); + } + + public void OnCompleted() + { + if (_hasDataValidation) + _owner.Owner.OnUpdateDataValidation(Property, BindingValueType.UnsetValue, null); + _owner.OnLocalValueBindingCompleted(Property, this); } - public void OnCompleted() => _owner.OnLocalValueBindingCompleted(Property, this); public void OnError(Exception error) => OnCompleted(); public void OnNext(T value) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs index 71eb521f9d..12cd39046b 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs @@ -92,6 +92,48 @@ namespace Avalonia.Base.UnitTests Assert.Equal(1, target.Notifications.Count); } + [Fact] + public void Disposing_Binding_Subscription_Clears_DataValidation() + { + var target = new Class1(); + var source = new Subject>(); + var property = GetProperty(); + var error = new Exception(); + var sub = target.Bind(property, source); + + source.OnNext(6); + source.OnNext(BindingValue.DataValidationError(error)); + sub.Dispose(); + + Assert.Equal(new Notification[] + { + new(BindingValueType.Value, 6, null), + new(BindingValueType.DataValidationError, 6, error), + new(BindingValueType.UnsetValue, 6, null), + }, target.Notifications); + } + + [Fact] + public void Completing_Binding_Clears_DataValidation() + { + var target = new Class1(); + var source = new Subject>(); + var property = GetProperty(); + var error = new Exception(); + + target.Bind(property, source); + source.OnNext(6); + source.OnNext(BindingValue.DataValidationError(error)); + source.OnCompleted(); + + Assert.Equal(new Notification[] + { + new(BindingValueType.Value, 6, null), + new(BindingValueType.DataValidationError, 6, error), + new(BindingValueType.UnsetValue, 6, null), + }, target.Notifications); + } + protected abstract T GetProperty(); protected abstract T GetNonValidatedProperty(); } diff --git a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs index 5de703deb1..35e9370c4c 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_DataValidation.cs @@ -67,6 +67,30 @@ namespace Avalonia.Markup.UnitTests.Data Assert.Null(target.DataValidationError); } + [Fact] + public void Disposing_Binding_Subscription_Clears_DataValidation() + { + var (target, property) = CreateTarget(); + var binding = new Binding(nameof(ExceptionValidatingModel.Value)) + { + Mode = BindingMode.TwoWay + }; + + target.DataContext = new IndeiValidatingModel + { + Value = 200, + }; + + var sub = target.Bind(property, binding); + + Assert.Equal(200, target.GetValue(property)); + Assert.IsType(target.DataValidationError); + + sub.Dispose(); + + Assert.Null(target.DataValidationError); + } + private protected abstract (DataValidationTestControl, T) CreateTarget(); }