Skip to content

Commit

Permalink
Allow nested BindingNotifications. (#15722)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
grokys committed May 14, 2024
1 parent 864ecfd commit 5e323b8
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 3 deletions.
1 change: 0 additions & 1 deletion src/Avalonia.Base/Data/BindingNotification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ public class BindingNotification
/// <param name="value">The binding value.</param>
public BindingNotification(object? value)
{
Debug.Assert(value is not BindingNotification);
_value = value;
}

Expand Down
11 changes: 9 additions & 2 deletions src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 5e323b8

Please sign in to comment.