From c98d26891835174f1dce034ad3618503ddde0fb2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 14 May 2024 21:55:46 +0200 Subject: [PATCH] Allow nested BindingNotifications. (#15722) * Added failing test for #15201. * Handle nested BindingNotifications. When #13970 was written, [a check](https://github.com/AvaloniaUI/Avalonia/pull/13970/files#diff-cfb25a491b9452e1815aa2c0d71465aaf81e99792a88a04a1a2ed572fd1930ffR60) was added to ensure that nested `BindingNotification`s didn't happen, and the refactor was written with the assumption that they wouldn't happen. The problem is that they _do_ happen: when a source object implements both `INotifyDataErrorInfo` and had data annotations, then the nested data validation plugins would each wrap the value coming from the previous plugin in a new `BindingNotification`, resulting in nested `BindingNotifications`. This adds support for nested binding notifications back in - even though IMO nesting binding notifications is a bug, if we're doing it and we previously supported it then we should continue to support it. Fixes #15201 --- src/Avalonia.Base/Data/BindingNotification.cs | 1 - .../Core/ExpressionNodes/ExpressionNode.cs | 11 ++- .../BindingExpressionTests.DataValidation.cs | 70 +++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Base/Data/BindingNotification.cs b/src/Avalonia.Base/Data/BindingNotification.cs index 0c940a14610..a3a2e0c2b05 100644 --- a/src/Avalonia.Base/Data/BindingNotification.cs +++ b/src/Avalonia.Base/Data/BindingNotification.cs @@ -57,7 +57,6 @@ public class BindingNotification /// The binding value. public BindingNotification(object? value) { - Debug.Assert(value is not BindingNotification); _value = value; } diff --git a/src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs b/src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs index e8e6633ab73..8b53190f869 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs @@ -187,13 +187,20 @@ protected void SetValue(object? valueOrNotification) else if (notification.ErrorType == BindingErrorType.DataValidationError) { if (notification.HasValue) - SetValue(notification.Value, notification.Error); + { + if (notification.Value is BindingNotification n) + SetValue(n); + else + SetValue(notification.Value, notification.Error); + } else + { SetDataValidationError(notification.Error!); + } } else { - SetValue(notification.Value, null); + SetValue(notification.Value); } } else diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs index bedf24bd6d7..8e1865f2092 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using Avalonia.Data; using Avalonia.UnitTests; using Xunit; @@ -271,6 +272,39 @@ public void Updates_Data_Validation_For_Null_Value_In_Property_Chain() GC.KeepAlive(data); } + [Fact] + public void Updates_Data_Validation_For_Required_DataAnnotation() + { + var data = new DataAnnotationsViewModel(); + var target = CreateTargetWithSource( + data, + o => o.RequiredString, + enableDataValidation: true); + + AssertBindingError( + target, + TargetClass.StringProperty, + new DataValidationException("String is required!"), + BindingErrorType.DataValidationError); + } + + [Fact] + public void Handles_Indei_And_DataAnnotations_On_Same_Class() + { + // Issue #15201 + var data = new IndeiDataAnnotationsViewModel(); + var target = CreateTargetWithSource( + data, + o => o.RequiredString, + enableDataValidation: true); + + AssertBindingError( + target, + TargetClass.StringProperty, + new DataValidationException("String is required!"), + BindingErrorType.DataValidationError); + } + public class ExceptionViewModel : NotifyingBase { private int _mustBePositive; @@ -341,6 +375,42 @@ public IndeiViewModel? Inner public override IEnumerable? GetErrors(string propertyName) => null; } + private class DataAnnotationsViewModel : NotifyingBase + { + private string? _requiredString; + + [Required(ErrorMessage = "String is required!")] + public string? RequiredString + { + get { return _requiredString; } + set { _requiredString = value; RaisePropertyChanged(); } + } + } + + private class IndeiDataAnnotationsViewModel : IndeiBase + { + private string? _requiredString; + + [Required(ErrorMessage = "String is required!")] + public string? RequiredString + { + get { return _requiredString; } + set { _requiredString = value; RaisePropertyChanged(); } + } + + public override bool HasErrors => RequiredString is null; + + public override IEnumerable? GetErrors(string propertyName) + { + if (propertyName == nameof(RequiredString) && RequiredString is null) + { + return new[] { "String is required!" }; + } + + return null; + } + } + private static void AssertNoError(TargetClass target, AvaloniaProperty property) { Assert.False(target.BindingNotifications.TryGetValue(property, out var notification));