From 844da05fe897ff3e9b6a0b87ebc12ac97cd6f839 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 17 Aug 2016 01:50:36 +0200 Subject: [PATCH] Revert "Only send BindingNotifications on error." This reverts commit 57e646583fcd540a9b2795ee5e07700e241ea4f7. For TextBox there can be 2 bindings to Text: one which is the binding to the view model which should have data validation enabled, and another binding to the TextPresenter in the template which should not have data validation enabled, or it will override the view model data validation. For this we need to be able to distinguish between the two and so bindings with data validation enabled need to always send BindingNotifications. Conflicts: src/Avalonia.Base/AvaloniaObject.cs tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs --- src/Avalonia.Base/AvaloniaObject.cs | 2 +- .../Data/Plugins/DataValidatiorBase.cs | 6 +- .../Data/Plugins/IndeiValidationPlugin.cs | 8 +- .../AvaloniaObjectTests_DataValidation.cs | 25 ++-- .../ExpressionObserverTests_DataValidation.cs | 12 +- .../ExpressionObserverTests_Observable.cs | 2 +- .../Data/ExpressionSubjectTests.cs | 8 +- .../Plugins/ExceptionValidationPluginTests.cs | 10 +- .../Plugins/IndeiValidationPluginTests.cs | 12 +- .../Data/BindingTests_Validation.cs | 126 ++++++++++++++++++ 10 files changed, 166 insertions(+), 45 deletions(-) create mode 100644 tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index d5d97324f7..80d1c253b4 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -693,7 +693,7 @@ namespace Avalonia accessor.SetValue(this, finalValue); } - if (metadata.EnableDataValidation) + if (metadata.EnableDataValidation && notification != null) { UpdateDataValidation(property, notification); } diff --git a/src/Markup/Avalonia.Markup/Data/Plugins/DataValidatiorBase.cs b/src/Markup/Avalonia.Markup/Data/Plugins/DataValidatiorBase.cs index 9b387456ca..95d269f437 100644 --- a/src/Markup/Avalonia.Markup/Data/Plugins/DataValidatiorBase.cs +++ b/src/Markup/Avalonia.Markup/Data/Plugins/DataValidatiorBase.cs @@ -68,11 +68,13 @@ namespace Avalonia.Markup.Data.Plugins /// /// The value. /// - /// Notifies the observer that the value has changed. + /// Notifies the observer that the value has changed. The value will be wrapped in a + /// if it is not already a binding notification. /// protected virtual void InnerValueChanged(object value) { - Observer.OnNext(value); + var notification = value as BindingNotification ?? new BindingNotification(value); + Observer.OnNext(notification); } } } \ No newline at end of file diff --git a/src/Markup/Avalonia.Markup/Data/Plugins/IndeiValidationPlugin.cs b/src/Markup/Avalonia.Markup/Data/Plugins/IndeiValidationPlugin.cs index 561bd66db3..8fb2568f30 100644 --- a/src/Markup/Avalonia.Markup/Data/Plugins/IndeiValidationPlugin.cs +++ b/src/Markup/Avalonia.Markup/Data/Plugins/IndeiValidationPlugin.cs @@ -40,7 +40,7 @@ namespace Avalonia.Markup.Data.Plugins { if (e.PropertyName == _name || string.IsNullOrEmpty(e.PropertyName)) { - Observer.OnNext(BuildResult(Value)); + Observer.OnNext(CreateBindingNotification(Value)); } } @@ -76,10 +76,10 @@ namespace Avalonia.Markup.Data.Plugins protected override void InnerValueChanged(object value) { - base.InnerValueChanged(BuildResult(value)); + base.InnerValueChanged(CreateBindingNotification(value)); } - private object BuildResult(object value) + private BindingNotification CreateBindingNotification(object value) { var target = (INotifyDataErrorInfo)_reference.Target; @@ -98,7 +98,7 @@ namespace Avalonia.Markup.Data.Plugins } } - return value; + return new BindingNotification(value); } private Exception GenerateException(IList errors) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs index 942eccc1c3..a74b972b90 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_DataValidation.cs @@ -14,11 +14,10 @@ namespace Avalonia.Base.UnitTests { var target = new Class1(); - target.SetValue(Class1.NonValidatedProperty, 6); + target.SetValue(Class1.NonValidatedProperty, new BindingNotification(6)); target.SetValue(Class1.NonValidatedProperty, new BindingNotification(new Exception(), BindingErrorType.Error)); target.SetValue(Class1.NonValidatedProperty, new BindingNotification(new Exception(), BindingErrorType.DataValidationError)); target.SetValue(Class1.NonValidatedProperty, new BindingNotification(7)); - target.SetValue(Class1.NonValidatedProperty, 8); Assert.Empty(target.Notifications); } @@ -28,11 +27,10 @@ namespace Avalonia.Base.UnitTests { var target = new Class1(); - target.SetValue(Class1.NonValidatedDirectProperty, 6); + target.SetValue(Class1.NonValidatedDirectProperty, new BindingNotification(6)); target.SetValue(Class1.NonValidatedDirectProperty, new BindingNotification(new Exception(), BindingErrorType.Error)); target.SetValue(Class1.NonValidatedDirectProperty, new BindingNotification(new Exception(), BindingErrorType.DataValidationError)); target.SetValue(Class1.NonValidatedDirectProperty, new BindingNotification(7)); - target.SetValue(Class1.NonValidatedDirectProperty, 8); Assert.Empty(target.Notifications); } @@ -42,20 +40,18 @@ namespace Avalonia.Base.UnitTests { var target = new Class1(); - target.SetValue(Class1.ValidatedDirectProperty, 6); + target.SetValue(Class1.ValidatedDirectProperty, new BindingNotification(6)); target.SetValue(Class1.ValidatedDirectProperty, new BindingNotification(new Exception(), BindingErrorType.Error)); target.SetValue(Class1.ValidatedDirectProperty, new BindingNotification(new Exception(), BindingErrorType.DataValidationError)); target.SetValue(Class1.ValidatedDirectProperty, new BindingNotification(7)); - target.SetValue(Class1.ValidatedDirectProperty, 8); Assert.Equal( new[] { - null, // 6 + new BindingNotification(6), new BindingNotification(new Exception(), BindingErrorType.Error), new BindingNotification(new Exception(), BindingErrorType.DataValidationError), - new BindingNotification(7), // 7 - null, // 8 + new BindingNotification(7), }, target.Notifications.AsEnumerable()); } @@ -69,11 +65,10 @@ namespace Avalonia.Base.UnitTests [!Class1.NonValidatedProperty] = source.AsBinding(), }; - source.OnNext(6); + source.OnNext(new BindingNotification(6)); source.OnNext(new BindingNotification(new Exception(), BindingErrorType.Error)); source.OnNext(new BindingNotification(new Exception(), BindingErrorType.DataValidationError)); source.OnNext(new BindingNotification(7)); - source.OnNext(8); Assert.Empty(target.Notifications); } @@ -87,20 +82,18 @@ namespace Avalonia.Base.UnitTests [!Class1.ValidatedDirectProperty] = source.AsBinding(), }; - source.OnNext(6); + source.OnNext(new BindingNotification(6)); source.OnNext(new BindingNotification(new Exception(), BindingErrorType.Error)); source.OnNext(new BindingNotification(new Exception(), BindingErrorType.DataValidationError)); source.OnNext(new BindingNotification(7)); - source.OnNext(8); Assert.Equal( new[] { - null, // 6 + new BindingNotification(6), new BindingNotification(new Exception(), BindingErrorType.Error), new BindingNotification(new Exception(), BindingErrorType.DataValidationError), - new BindingNotification(7), // 7 - null, // 8 + new BindingNotification(7), }, target.Notifications.AsEnumerable()); } diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs index a363d69486..cdcaeda4cc 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs @@ -81,16 +81,16 @@ namespace Avalonia.Markup.UnitTests.Data observer.SetValue("foo"); observer.SetValue(5); - Assert.Equal(new object[] + Assert.Equal(new[] { - 0, + new BindingNotification(0), // Value is notified twice as ErrorsChanged is always called by IndeiTest. - 5, - 5, + new BindingNotification(5), + new BindingNotification(5), // Value is first signalled without an error as validation hasn't been updated. - -5, + new BindingNotification(-5), new BindingNotification(new Exception("Must be positive"), BindingErrorType.DataValidationError, -5), // Exception is thrown by trying to set value to "foo". @@ -100,7 +100,7 @@ namespace Avalonia.Markup.UnitTests.Data // Value is set then validation is updated. new BindingNotification(new Exception("Must be positive"), BindingErrorType.DataValidationError, 5), - 5, + new BindingNotification(5), }, result); } diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs index 3b915f97a8..3263aaace2 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs @@ -85,7 +85,7 @@ namespace Avalonia.Markup.UnitTests.Data data.Next.OnNext(new Class2("foo")); sync.ExecutePostedCallbacks(); - Assert.Equal(new[] { "foo" }, result); + Assert.Equal(new[] { new BindingNotification("foo") }, result); sub.Dispose(); Assert.Equal(0, data.PropertyChangedSubscriptionCount); diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs index 16b71c87fe..5498926fe4 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs @@ -288,11 +288,11 @@ namespace Avalonia.Markup.UnitTests.Data target.OnNext("bar"); Assert.Equal( - new object[] + new[] { - "5.6", - "1.2", - "3.4", + new BindingNotification("5.6"), + new BindingNotification("1.2"), + new BindingNotification("3.4"), new BindingNotification( new InvalidCastException("Error setting 'DoubleValue': Could not convert 'bar' to 'System.Double'"), BindingErrorType.Error) diff --git a/tests/Avalonia.Markup.UnitTests/Data/Plugins/ExceptionValidationPluginTests.cs b/tests/Avalonia.Markup.UnitTests/Data/Plugins/ExceptionValidationPluginTests.cs index 7b7128ed2f..4a34791008 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/Plugins/ExceptionValidationPluginTests.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/Plugins/ExceptionValidationPluginTests.cs @@ -14,7 +14,7 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins public class ExceptionValidationPluginTests { [Fact] - public void Produces_Correct_Results() + public void Produces_BindingNotifications() { var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); var validatorPlugin = new ExceptionValidationPlugin(); @@ -28,12 +28,12 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins validator.SetValue(-2, BindingPriority.LocalValue); validator.SetValue(6, BindingPriority.LocalValue); - Assert.Equal(new object[] + Assert.Equal(new[] { - 0, - 5, + new BindingNotification(0), + new BindingNotification(5), new BindingNotification(new ArgumentOutOfRangeException("value"), BindingErrorType.DataValidationError), - 6, + new BindingNotification(6), }, result); } diff --git a/tests/Avalonia.Markup.UnitTests/Data/Plugins/IndeiValidationPluginTests.cs b/tests/Avalonia.Markup.UnitTests/Data/Plugins/IndeiValidationPluginTests.cs index 9e6657e0a1..788bc25a34 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/Plugins/IndeiValidationPluginTests.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/Plugins/IndeiValidationPluginTests.cs @@ -14,7 +14,7 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins public class IndeiValidationPluginTests { [Fact] - public void Produces_Correct_Results() + public void Produces_BindingNotifications() { var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); var validatorPlugin = new IndeiValidationPlugin(); @@ -29,19 +29,19 @@ namespace Avalonia.Markup.UnitTests.Data.Plugins data.Maximum = 10; data.Maximum = 5; - Assert.Equal(new object[] + Assert.Equal(new[] { - 0, - 5, + new BindingNotification(0), + new BindingNotification(5), // Value is first signalled without an error as validation hasn't been updated. - 6, + new BindingNotification(6), // Then the ErrorsChanged event is fired. new BindingNotification(new Exception("Must be less than Maximum"), BindingErrorType.DataValidationError, 6), // Maximum is changed to 10 so value is now valid. - 6, + new BindingNotification(6), // And Maximum is changed back to 5. new BindingNotification(new Exception("Must be less than Maximum"), BindingErrorType.DataValidationError, 6), diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs new file mode 100644 index 0000000000..8759cb42c5 --- /dev/null +++ b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs @@ -0,0 +1,126 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using Avalonia.Controls; +using Avalonia.Data; +using Avalonia.Markup.Xaml.Data; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Markup.Xaml.UnitTests.Data +{ + public class BindingTests_Validation + { + [Fact] + public void Non_Validated_Property_Does_Not_Receive_BindingNotifications() + { + var source = new ValidationTestModel { MustBePositive = 5 }; + var target = new TestControl + { + DataContext = source, + [!TestControl.NonValidatedProperty] = new Binding(nameof(source.MustBePositive)), + }; + + Assert.Empty(target.Notifications); + } + + [Fact] + public void Validated_Direct_Property_Receives_BindingNotifications() + { + var source = new ValidationTestModel { MustBePositive = 5 }; + var target = new TestControl + { + DataContext = source, + }; + + target.Bind( + TestControl.ValidatedDirectProperty, + new Binding(nameof(source.MustBePositive), BindingMode.TwoWay)); + + target.ValidatedDirect = 6; + target.ValidatedDirect = -1; + target.ValidatedDirect = 7; + + Assert.Equal( + new[] + { + new BindingNotification(5), + new BindingNotification(6), + new BindingNotification(new ArgumentOutOfRangeException("value"), BindingErrorType.DataValidationError), + new BindingNotification(7), + }, + target.Notifications.AsEnumerable()); + } + + private class TestControl : Control + { + public static readonly StyledProperty NonValidatedProperty = + AvaloniaProperty.Register( + nameof(Validated), + enableDataValidation: false); + + public static readonly StyledProperty ValidatedProperty = + AvaloniaProperty.Register( + nameof(Validated), + enableDataValidation: true); + + public static readonly DirectProperty ValidatedDirectProperty = + AvaloniaProperty.RegisterDirect( + nameof(Validated), + o => o.ValidatedDirect, + (o, v) => o.ValidatedDirect = v, + enableDataValidation: true); + + private int _direct; + + public int NonValidated + { + get { return GetValue(NonValidatedProperty); } + set { SetValue(NonValidatedProperty, value); } + } + + public int Validated + { + get { return GetValue(ValidatedProperty); } + set { SetValue(ValidatedProperty, value); } + } + + public int ValidatedDirect + { + get { return _direct; } + set { SetAndRaise(ValidatedDirectProperty, ref _direct, value); } + } + + public IList Notifications { get; } = new List(); + + protected override void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) + { + Notifications.Add(notification); + } + } + + private class ValidationTestModel : NotifyingBase + { + private int _mustBePositive; + + public int MustBePositive + { + get { return _mustBePositive; } + set + { + if (value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + + if (_mustBePositive != value) + { + _mustBePositive = value; + RaisePropertyChanged(); + } + } + } + } + } +}