Browse Source

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
pull/15641/head
Steven Kirk 2 years ago
committed by GitHub
parent
commit
5e323b8fb1
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 1
      src/Avalonia.Base/Data/BindingNotification.cs
  2. 11
      src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs
  3. 70
      tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs

1
src/Avalonia.Base/Data/BindingNotification.cs

@ -57,7 +57,6 @@ namespace Avalonia.Data
/// <param name="value">The binding value.</param>
public BindingNotification(object? value)
{
Debug.Assert(value is not BindingNotification);
_value = value;
}

11
src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs

@ -187,13 +187,20 @@ internal abstract class ExpressionNode
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

70
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 partial class BindingExpressionTests
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 partial class BindingExpressionTests
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));

Loading…
Cancel
Save