From 082a03b7c4c3df71479ad22faf9cffeb047f9748 Mon Sep 17 00:00:00 2001 From: Evan <109839359+Evan260@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:20:44 +0000 Subject: [PATCH] Handle DataValidationError notifications returned by ConvertBack (#22171) * Handle DataValidationError notifications returned by ConvertBack A converter returning a BindingNotification with DataValidationError fell through to the target type converter, which replaced it with a cast error that stringified the notification. Publish the original error instead. * Add additional failing tests. There are more `BindingNotification` cases to handle. * Handle the other `BindingNotification` cases. --------- Co-authored-by: grokys --- .../Data/Core/BindingExpression.cs | 48 ++++++-- .../BindingExpressionTests.DataValidation.cs | 111 ++++++++++++++++++ 2 files changed, 151 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/BindingExpression.cs b/src/Avalonia.Base/Data/Core/BindingExpression.cs index 31b8338dd6..2fba66742f 100644 --- a/src/Avalonia.Base/Data/Core/BindingExpression.cs +++ b/src/Avalonia.Base/Data/Core/BindingExpression.cs @@ -3,11 +3,9 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.Linq.Expressions; using System.Text; using Avalonia.Data.Converters; using Avalonia.Data.Core.ExpressionNodes; -using Avalonia.Data.Core.Parsers; using Avalonia.Input; using Avalonia.Interactivity; using Avalonia.Logging; @@ -316,13 +314,12 @@ internal class BindingExpression : UntypedBindingExpressionBase, IDescription, I if (_nodes.Count == 0 || LeafNode is not ISettableNode setter || setter.ValueType is not { } type) return false; - if (Converter is { } converter && - value != AvaloniaProperty.UnsetValue && - value != BindingOperations.DoNothing) - { - value = ConvertBack(converter, ConverterCulture, ConverterParameter, value, type); - } + // Invoke any converter on the value before writing it to the source. If the converter + // returns an error then we don't write the value to the source and return false. + if (!TryConvertBack(type, ref value)) + return false; + // A converter may return DoNothing. if (value == BindingOperations.DoNothing) return true; @@ -586,6 +583,41 @@ internal class BindingExpression : UntypedBindingExpressionBase, IDescription, I return AvaloniaProperty.UnsetValue; } + private bool TryConvertBack(Type valueType, ref object? value) + { + if (Converter is { } converter && + value != AvaloniaProperty.UnsetValue && + value != BindingOperations.DoNothing) + { + value = ConvertBack(converter, ConverterCulture, ConverterParameter, value, valueType); + + if (value is BindingNotification notification) + { + if (notification.Error is { } error) + { + switch (notification.ErrorType) + { + case BindingErrorType.DataValidationError: + if (IsDataValidationEnabled) + OnDataValidationError(notification.Error); + break; + default: + if (ShouldLogError(out var target)) + Log(target, error.Message); + PublishValue(UnchangedValue, new(error, BindingErrorType.Error)); + break; + } + + return false; + } + + value = notification.Value; + } + } + + return true; + } + /// /// Uncommonly used fields are separated out to reduce memory usage. /// diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs index 9889a380cc..b5aa31939a 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs @@ -2,8 +2,10 @@ using System.Collections; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using System.Globalization; using System.Linq; using Avalonia.Data; +using Avalonia.Data.Converters; using Avalonia.Data.Core.Plugins; using Avalonia.UnitTests; using Xunit; @@ -576,6 +578,115 @@ public partial class BindingExpressionTests } } + private class InvalidIdConverter(BindingErrorType errorType) : IValueConverter + { + private readonly BindingErrorType _errorType = errorType; + + public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture) => value; + + public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture) + { + if (value is int i) + return i; + + if (value is string s && int.TryParse(s, out var parsed)) + return parsed; + + return new BindingNotification( + new FormatException($"'{value}' is not a valid ID."), + _errorType); + } + } + + [Fact] + public void ConvertBack_DataValidationError_Updates_Data_Validation() + { + var data = new ViewModel { IntValue = 1 }; + var target = CreateTargetWithSource( + data, + o => o.IntValue, + converter: new InvalidIdConverter(BindingErrorType.DataValidationError), + enableDataValidation: true, + mode: BindingMode.TwoWay, + targetProperty: TargetClass.TagProperty); + + target.Tag = "42"; + + AssertNoError(target, TargetClass.TagProperty); + + target.Tag = "0x555g"; + + Assert.Equal(42, data.IntValue); + AssertBindingError( + target, + TargetClass.TagProperty, + new FormatException("'0x555g' is not a valid ID."), + BindingErrorType.DataValidationError); + + GC.KeepAlive(data); + } + + [Fact] + public void ConvertBack_Error_Updates_Data_Validation() + { + var data = new ViewModel { IntValue = 1 }; + var target = CreateTargetWithSource( + data, + o => o.IntValue, + converter: new InvalidIdConverter(BindingErrorType.Error), + enableDataValidation: true, + mode: BindingMode.TwoWay, + targetProperty: TargetClass.TagProperty); + + target.Tag = "42"; + + AssertNoError(target, TargetClass.TagProperty); + + target.Tag = "0x555g"; + + Assert.Equal(42, data.IntValue); + AssertBindingError( + target, + TargetClass.TagProperty, + new FormatException("'0x555g' is not a valid ID."), + BindingErrorType.Error); + + GC.KeepAlive(data); + } + + [Fact] + public void ConvertBack_Notification_With_Value_Writes_Value() + { + var data = new ViewModel { IntValue = 1 }; + var target = CreateTargetWithSource( + data, + o => o.IntValue, + converter: new FuncValueConverter(v => new BindingNotification(42)), + enableDataValidation: true, + mode: BindingMode.TwoWay, + targetProperty: TargetClass.TagProperty); + + target.Tag = "foo"; + + Assert.Equal(42, data.IntValue); + AssertNoError(target, TargetClass.TagProperty); + + GC.KeepAlive(data); + } + + private class FuncValueConverter : IValueConverter + { + private readonly Func _convertBack; + + public FuncValueConverter(Func convertBack) => _convertBack = convertBack; + + public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture) + => value; + + public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture) + => _convertBack(value); + } + private static void AssertNoError(TargetClass target, AvaloniaProperty property) { Assert.False(target.BindingNotifications.TryGetValue(property, out var notification));