From f882e14e41c7a30a09fe187248f17c65bf11a8ee Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 13 Apr 2016 16:28:47 -0500 Subject: [PATCH 01/12] Added support for IPropertyAccessor's to listen for error notifications from a target that implements INotifyDataErrorInfo. --- .../Data/Plugins/IPropertyAccessorPlugin.cs | 5 +- .../Plugins/InpcPropertyAccessorPlugin.cs | 51 ++++++- .../Plugins/PerspexPropertyAccessorPlugin.cs | 4 +- .../Data/PropertyAccessorNode.cs | 2 +- .../Data/InpcPluginTests.cs | 139 ++++++++++++++++++ .../Perspex.Markup.UnitTests.csproj | 1 + 6 files changed, 192 insertions(+), 10 deletions(-) create mode 100644 tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs diff --git a/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs index 8a431041d0..9dd4f10dbe 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; +using System.Collections; namespace Perspex.Markup.Data.Plugins { @@ -24,6 +25,7 @@ namespace Perspex.Markup.Data.Plugins /// A weak reference to the object. /// The property name. /// A function to call when the property changes. + /// A function to call when the validation status of the property changes. /// /// An interface through which future interactions with the /// property will be made. @@ -31,6 +33,7 @@ namespace Perspex.Markup.Data.Plugins IPropertyAccessor Start( WeakReference reference, string propertyName, - Action changed); + Action changed, + Action validationChanged); } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs index 64421ddc57..9eee066c61 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs @@ -9,6 +9,7 @@ using System.Reflection; using Perspex.Data; using Perspex.Logging; using Perspex.Utilities; +using System.Collections; namespace Perspex.Markup.Data.Plugins { @@ -36,6 +37,7 @@ namespace Perspex.Markup.Data.Plugins /// The object. /// The property name. /// A function to call when the property changes. + /// A function to call when the validation state of the property changes. /// /// An interface through which future interactions with the /// property will be made. @@ -43,7 +45,8 @@ namespace Perspex.Markup.Data.Plugins public IPropertyAccessor Start( WeakReference reference, string propertyName, - Action changed) + Action changed, + Action validationChanged) { Contract.Requires(reference != null); Contract.Requires(propertyName != null); @@ -54,7 +57,7 @@ namespace Perspex.Markup.Data.Plugins if (p != null) { - return new Accessor(reference, p, changed); + return new Accessor(reference, p, changed, validationChanged); } else { @@ -64,16 +67,18 @@ namespace Perspex.Markup.Data.Plugins } } - private class Accessor : IPropertyAccessor, IWeakSubscriber + private class Accessor : IPropertyAccessor, IWeakSubscriber, IWeakSubscriber { private readonly WeakReference _reference; private readonly PropertyInfo _property; private readonly Action _changed; + private readonly Action _validationChanged; public Accessor( WeakReference reference, PropertyInfo property, - Action changed) + Action changed, + Action validationChanged) { Contract.Requires(reference != null); Contract.Requires(property != null); @@ -81,12 +86,13 @@ namespace Perspex.Markup.Data.Plugins _reference = reference; _property = property; _changed = changed; + _validationChanged = validationChanged; var inpc = reference.Target as INotifyPropertyChanged; if (inpc != null) { - WeakSubscriptionManager.Subscribe( + WeakSubscriptionManager.Subscribe( inpc, nameof(inpc.PropertyChanged), this); @@ -101,6 +107,19 @@ namespace Perspex.Markup.Data.Plugins reference.Target, reference.Target.GetType()); } + + var indei = _reference.Target as INotifyDataErrorInfo; + if (indei != null) + { + if (indei.HasErrors) + { + _validationChanged(indei.GetErrors(property.Name)); + } + WeakSubscriptionManager.Subscribe( + indei, + nameof(indei.ErrorsChanged), + this); + } } public Type PropertyType => _property.PropertyType; @@ -113,11 +132,20 @@ namespace Perspex.Markup.Data.Plugins if (inpc != null) { - WeakSubscriptionManager.Unsubscribe( + WeakSubscriptionManager.Unsubscribe( inpc, nameof(inpc.PropertyChanged), this); } + + var indei = _reference.Target as INotifyDataErrorInfo; + if (indei != null) + { + WeakSubscriptionManager.Unsubscribe( + indei, + nameof(indei.ErrorsChanged), + this); + } } public bool SetValue(object value, BindingPriority priority) @@ -131,13 +159,22 @@ namespace Perspex.Markup.Data.Plugins return false; } - public void OnEvent(object sender, PropertyChangedEventArgs e) + void IWeakSubscriber.OnEvent(object sender, PropertyChangedEventArgs e) { if (e.PropertyName == _property.Name || string.IsNullOrEmpty(e.PropertyName)) { _changed(Value); } } + + void IWeakSubscriber.OnEvent(object sender, DataErrorsChangedEventArgs e) + { + if (e.PropertyName == _property.Name || string.IsNullOrEmpty(e.PropertyName)) + { + var indei = _reference.Target as INotifyDataErrorInfo; + _validationChanged(indei.GetErrors(e.PropertyName)); + } + } } } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs index bc16989a7a..d27d1e8be4 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs @@ -31,6 +31,7 @@ namespace Perspex.Markup.Data.Plugins /// A weak reference to the object. /// The property name. /// A function to call when the property changes. + /// A function to call when the validation state of the property changes. /// /// An interface through which future interactions with the /// property will be made. @@ -38,7 +39,8 @@ namespace Perspex.Markup.Data.Plugins public IPropertyAccessor Start( WeakReference reference, string propertyName, - Action changed) + Action changed, + Action validationChanged) { Contract.Requires(reference != null); Contract.Requires(propertyName != null); diff --git a/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs b/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs index 14883e449a..0f76503ff3 100644 --- a/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs +++ b/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs @@ -54,7 +54,7 @@ namespace Perspex.Markup.Data if (plugin != null) { - _accessor = plugin.Start(reference, PropertyName, SetCurrentValue); + _accessor = plugin.Start(reference, PropertyName, SetCurrentValue, _ => { }); if (_accessor != null) { diff --git a/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs b/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs new file mode 100644 index 0000000000..3a32e3eabc --- /dev/null +++ b/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs @@ -0,0 +1,139 @@ +using Perspex.Markup.Data.Plugins; +using System; +using System.Collections; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace Perspex.Markup.UnitTests.Data +{ + public class InpcPluginTests + { + private class InpcTest : INotifyPropertyChanged, INotifyDataErrorInfo + { + private int noValidationTest; + + public int NoValidationTest + { + get { return noValidationTest; } + set { noValidationTest = value; NotifyPropertyChanged(); } + } + + public bool HasErrors + { + get + { + return NonNegative < 0; + } + } + + private int nonNegative; + + public int NonNegative + { + get { return nonNegative; } + set + { + var old = nonNegative; + nonNegative = value; + NotifyPropertyChanged(); + if (old * value < 0) // If signs are different + { + NotifyErrorsChanged(); + } + } + } + + + public event EventHandler ErrorsChanged; + public event PropertyChangedEventHandler PropertyChanged; + + public IEnumerable GetErrors(string propertyName) + { + if (string.IsNullOrEmpty(propertyName) || propertyName == nameof(NonNegative)) + { + if (NonNegative < 0) + { + yield return "Invalid Value"; + } + } + } + + private void NotifyPropertyChanged([CallerMemberName] string property = "") + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(property)); + } + + private void NotifyErrorsChanged([CallerMemberName] string property = "") + { + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(property)); + } + } + + [Fact] + public void Calls_Change_Callback_When_Value_Changes() + { + var plugin = new InpcPropertyAccessorPlugin(); + var source = new InpcTest { NoValidationTest = 0 }; + var changeFired = false; + plugin.Start(new WeakReference(source), nameof(InpcTest.NoValidationTest), _ => changeFired = true, _ => { }); + source.NoValidationTest = 1; + + Assert.True(changeFired); + } + + [Fact] + public void ValidationChanged_Does_Not_Fire_When_NonValidated_Value_Changes() + { + var plugin = new InpcPropertyAccessorPlugin(); + var source = new InpcTest { NoValidationTest = 0 }; + var validationFired = false; + plugin.Start(new WeakReference(source), nameof(InpcTest.NoValidationTest), _ => { }, _ => validationFired = true); + source.NoValidationTest = 1; + + Assert.False(validationFired); + } + + [Fact] + public void ValidationChanged_Does_Not_Fire_When_Validation_Does_Not_Change() + { + var plugin = new InpcPropertyAccessorPlugin(); + var source = new InpcTest { NonNegative = 3 }; + var validationFired = false; + plugin.Start(new WeakReference(source), nameof(InpcTest.NonNegative), _ => { }, _ => validationFired = true); + source.NonNegative = 5; + + Assert.False(validationFired); + } + + [Fact] + public void ValidationChanged_Fires_On_Start_If_Has_Errors() + { + var plugin = new InpcPropertyAccessorPlugin(); + var source = new InpcTest { NonNegative = -5 }; + + Assert.True(source.HasErrors); + + var validationFired = false; + plugin.Start(new WeakReference(source), nameof(InpcTest.NonNegative), _ => { }, _ => validationFired = true); + Assert.True(validationFired); + } + + + + [Fact] + public void ValidationChanged_Fires_When_Validation_Changes() + { + var plugin = new InpcPropertyAccessorPlugin(); + var source = new InpcTest { NonNegative = 5 }; + var validationFired = false; + plugin.Start(new WeakReference(source), nameof(InpcTest.NonNegative), _ => { }, _ => validationFired = true); + source.NonNegative = -1; + Assert.True(validationFired); + } + } +} diff --git a/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj b/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj index f32a584d1d..fa2b551600 100644 --- a/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj +++ b/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj @@ -97,6 +97,7 @@ + From 59b88fe7e90cadcc4f6eb2085fcdef59ea2a172f Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 13 Apr 2016 16:42:04 -0500 Subject: [PATCH 02/12] Added copyright header to some new files. --- src/Markup/Perspex.Markup/Data/CommonPropertyNames.cs | 7 ++----- tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs | 9 ++++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Markup/Perspex.Markup/Data/CommonPropertyNames.cs b/src/Markup/Perspex.Markup/Data/CommonPropertyNames.cs index 5866539886..5082a89862 100644 --- a/src/Markup/Perspex.Markup/Data/CommonPropertyNames.cs +++ b/src/Markup/Perspex.Markup/Data/CommonPropertyNames.cs @@ -1,8 +1,5 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. namespace Perspex.Markup.Data { diff --git a/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs b/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs index 3a32e3eabc..1c97d5c386 100644 --- a/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs +++ b/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs @@ -1,12 +1,11 @@ -using Perspex.Markup.Data.Plugins; +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using Perspex.Markup.Data.Plugins; using System; using System.Collections; -using System.Collections.Generic; using System.ComponentModel; -using System.Linq; using System.Runtime.CompilerServices; -using System.Text; -using System.Threading.Tasks; using Xunit; namespace Perspex.Markup.UnitTests.Data From 76f5249f5912d569c140bd5734c83654ba9370e0 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 14 Apr 2016 00:51:50 -0500 Subject: [PATCH 03/12] Created modular validation plugin system instead of being hard-coded to just INotifyDataErrorInfo. Current plugins are for Exception validation and INotifyDataErrorInfo. Currently not in use anywhere. --- .../ExceptionValidationCheckerPlugin.cs | 52 +++++++++++++ .../Data/Plugins/IValidationCheckerPlugin.cs | 14 ++++ .../Plugins/IndeiValidationCheckerPlugin.cs | 73 +++++++++++++++++++ .../Data/Plugins/ValidationCheckerBase.cs | 34 +++++++++ .../Perspex.Markup/Perspex.Markup.csproj | 4 + src/Perspex.Base/Data/ValidationStatus.cs | 20 +++++ src/Perspex.Base/Perspex.Base.csproj | 1 + .../Data/InpcPluginTests.cs | 8 +- 8 files changed, 204 insertions(+), 2 deletions(-) create mode 100644 src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs create mode 100644 src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs create mode 100644 src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs create mode 100644 src/Markup/Perspex.Markup/Data/Plugins/ValidationCheckerBase.cs create mode 100644 src/Perspex.Base/Data/ValidationStatus.cs diff --git a/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs new file mode 100644 index 0000000000..dc8b9b18ad --- /dev/null +++ b/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs @@ -0,0 +1,52 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Perspex.Data; + +namespace Perspex.Markup.Data.Plugins +{ + public class ExceptionValidationCheckerPlugin : IValidationCheckerPlugin + { + public ValidationCheckerBase Start(WeakReference reference, string name, IPropertyAccessor accessor, Action callback) + { + return new ExceptionValidationChecker(reference, name, accessor, callback); + } + + private class ExceptionValidationChecker : ValidationCheckerBase + { + public ExceptionValidationChecker(WeakReference reference, string name, IPropertyAccessor accessor, Action callback) + : base(reference, name, accessor, callback) + { + } + + public override bool SetValue(object value, BindingPriority priority) + { + try + { + var success = base.SetValue(value, priority); + SendValidationCallback(new ExceptionValidationStatus(null)); + return success; + } + catch (Exception ex) + { + SendValidationCallback(new ExceptionValidationStatus(ex)); + } + return false; + } + } + + private class ExceptionValidationStatus : ValidationStatus + { + public ExceptionValidationStatus(Exception exception) + { + Exception = exception; + } + + public Exception Exception { get; } + + public override bool IsValid => Exception != null; + } + } +} diff --git a/src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs new file mode 100644 index 0000000000..07a10f235f --- /dev/null +++ b/src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs @@ -0,0 +1,14 @@ +using Perspex.Data; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Perspex.Markup.Data.Plugins +{ + public interface IValidationCheckerPlugin + { + ValidationCheckerBase Start(WeakReference reference, string name, IPropertyAccessor accessor, Action callback); + } +} diff --git a/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs new file mode 100644 index 0000000000..dfd63ef2b6 --- /dev/null +++ b/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs @@ -0,0 +1,73 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Perspex.Data; +using System.ComponentModel; +using System.Collections; +using Perspex.Utilities; + +namespace Perspex.Markup.Data.Plugins +{ + class IndeiValidationCheckerPlugin : IValidationCheckerPlugin + { + public ValidationCheckerBase Start(WeakReference reference, string name, IPropertyAccessor accessor, Action callback) + { + return new IndeiValidationChecker(reference, name, accessor, callback); + } + + private class IndeiValidationChecker : ValidationCheckerBase, IWeakSubscriber + { + public IndeiValidationChecker(WeakReference reference, string name, IPropertyAccessor accessor, Action callback) + : base(reference, name, accessor, callback) + { + var target = reference.Target as INotifyDataErrorInfo; + if (target != null) + { + if (target.HasErrors) + { + SendValidationCallback(new IndeiValidationStatus(target.GetErrors(name))); + } + WeakSubscriptionManager.Subscribe( + target, + nameof(target.ErrorsChanged), + this); + } + } + + public override void Dispose() + { + base.Dispose(); + var target = _reference.Target as INotifyDataErrorInfo; + if (target != null) + { + WeakSubscriptionManager.Unsubscribe( + target, + nameof(target.ErrorsChanged), + this); + } + } + + public void OnEvent(object sender, DataErrorsChangedEventArgs e) + { + if (e.PropertyName == _name || string.IsNullOrEmpty(e.PropertyName)) + { + var indei = _reference.Target as INotifyDataErrorInfo; + SendValidationCallback(new IndeiValidationStatus(indei.GetErrors(e.PropertyName))); + } + } + } + + private class IndeiValidationStatus : ValidationStatus + { + public IndeiValidationStatus(IEnumerable errors) + { + Errors = errors; + } + public override bool IsValid => !Errors.OfType().Any(); + + public IEnumerable Errors { get; } + } + } +} diff --git a/src/Markup/Perspex.Markup/Data/Plugins/ValidationCheckerBase.cs b/src/Markup/Perspex.Markup/Data/Plugins/ValidationCheckerBase.cs new file mode 100644 index 0000000000..8ab7b7e43e --- /dev/null +++ b/src/Markup/Perspex.Markup/Data/Plugins/ValidationCheckerBase.cs @@ -0,0 +1,34 @@ +using System; +using Perspex.Data; + +namespace Perspex.Markup.Data.Plugins +{ + public abstract class ValidationCheckerBase : IPropertyAccessor + { + protected readonly WeakReference _reference; + protected readonly string _name; + private readonly IPropertyAccessor _accessor; + private readonly Action _callback; + + protected ValidationCheckerBase(WeakReference reference, string name, IPropertyAccessor accessor, Action callback) + { + _reference = reference; + _name = name; + _accessor = accessor; + _callback = callback; + } + + public Type PropertyType => _accessor.PropertyType; + + public object Value => _accessor.Value; + + public virtual void Dispose() => _accessor.Dispose(); + + public virtual bool SetValue(object value, BindingPriority priority) => _accessor.SetValue(value, priority); + + protected void SendValidationCallback(ValidationStatus status) + { + _callback?.Invoke(status); + } + } +} \ No newline at end of file diff --git a/src/Markup/Perspex.Markup/Perspex.Markup.csproj b/src/Markup/Perspex.Markup/Perspex.Markup.csproj index 5427d97346..443991b120 100644 --- a/src/Markup/Perspex.Markup/Perspex.Markup.csproj +++ b/src/Markup/Perspex.Markup/Perspex.Markup.csproj @@ -46,6 +46,10 @@ + + + + diff --git a/src/Perspex.Base/Data/ValidationStatus.cs b/src/Perspex.Base/Data/ValidationStatus.cs new file mode 100644 index 0000000000..9effaa8239 --- /dev/null +++ b/src/Perspex.Base/Data/ValidationStatus.cs @@ -0,0 +1,20 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Perspex.Data +{ + /// + /// Contains information on if the current object passed validation. + /// Subclasses of this class contain additional information depending on the method of validation checking. + /// + public abstract class ValidationStatus + { + public abstract bool IsValid { get; } + } +} diff --git a/src/Perspex.Base/Perspex.Base.csproj b/src/Perspex.Base/Perspex.Base.csproj index df240ee3b8..cc8cb6ada6 100644 --- a/src/Perspex.Base/Perspex.Base.csproj +++ b/src/Perspex.Base/Perspex.Base.csproj @@ -44,6 +44,7 @@ Properties\SharedAssemblyInfo.cs + diff --git a/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs b/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs index 1c97d5c386..74bd6bd7b3 100644 --- a/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs +++ b/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs @@ -19,7 +19,11 @@ namespace Perspex.Markup.UnitTests.Data public int NoValidationTest { get { return noValidationTest; } - set { noValidationTest = value; NotifyPropertyChanged(); } + set + { + noValidationTest = value; + NotifyPropertyChanged(); + } } public bool HasErrors @@ -79,7 +83,7 @@ namespace Perspex.Markup.UnitTests.Data var plugin = new InpcPropertyAccessorPlugin(); var source = new InpcTest { NoValidationTest = 0 }; var changeFired = false; - plugin.Start(new WeakReference(source), nameof(InpcTest.NoValidationTest), _ => changeFired = true, _ => { }); + var accessor = plugin.Start(new WeakReference(source), nameof(InpcTest.NoValidationTest), _ => changeFired = true, _ => { }); source.NoValidationTest = 1; Assert.True(changeFired); From ee486113e520bf8228eef729d93f94f98c74d7f6 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sun, 17 Apr 2016 19:13:21 -0500 Subject: [PATCH 04/12] Finished wiring up validation support. Now exceptions and INotifyDataErrorInfo implementers will set controls into their ":invalid" pseudo-class state. --- .../Perspex.Markup/Data/ExpressionNode.cs | 5 + .../Perspex.Markup/Data/ExpressionObserver.cs | 11 ++ .../Perspex.Markup/Data/ExpressionSubject.cs | 1 + .../ExceptionValidationCheckerPlugin.cs | 23 ++- .../Data/Plugins/IPropertyAccessorPlugin.cs | 4 +- .../Data/Plugins/IValidationCheckerPlugin.cs | 22 +++ .../Plugins/IndeiValidationCheckerPlugin.cs | 24 ++- .../Plugins/InpcPropertyAccessorPlugin.cs | 44 +----- .../Plugins/PerspexPropertyAccessorPlugin.cs | 4 +- .../Data/PropertyAccessorNode.cs | 11 +- src/Perspex.Base/IPriorityValueOwner.cs | 9 ++ src/Perspex.Base/PerspexObject.cs | 13 ++ src/Perspex.Base/PriorityBindingEntry.cs | 8 +- src/Perspex.Base/PriorityLevel.cs | 11 ++ src/Perspex.Base/PriorityValue.cs | 10 ++ src/Perspex.Controls/Control.cs | 30 ++++ .../Data/ExceptionValidatorTests.cs | 99 ++++++++++++ .../Data/ExpressionObserverTests_Property.cs | 5 +- .../Data/IndeiValidatorTests.cs | 120 +++++++++++++++ .../Data/InpcPluginTests.cs | 142 ------------------ .../Perspex.Markup.UnitTests.csproj | 3 +- 21 files changed, 398 insertions(+), 201 deletions(-) create mode 100644 tests/Perspex.Markup.UnitTests/Data/ExceptionValidatorTests.cs create mode 100644 tests/Perspex.Markup.UnitTests/Data/IndeiValidatorTests.cs delete mode 100644 tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs diff --git a/src/Markup/Perspex.Markup/Data/ExpressionNode.cs b/src/Markup/Perspex.Markup/Data/ExpressionNode.cs index e989a72177..a120aa8901 100644 --- a/src/Markup/Perspex.Markup/Data/ExpressionNode.cs +++ b/src/Markup/Perspex.Markup/Data/ExpressionNode.cs @@ -105,6 +105,11 @@ namespace Perspex.Markup.Data CurrentValue = reference; } + protected virtual void SendValidationStatus(ValidationStatus status) + { + _subject?.OnNext(status); + } + protected virtual void Unsubscribe(object target) { } diff --git a/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs b/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs index e5b0d1ac78..082aaf652b 100644 --- a/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs +++ b/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs @@ -28,6 +28,17 @@ namespace Perspex.Markup.Data new InpcPropertyAccessorPlugin(), }; + /// + /// An ordered collection of validation checker plugins that can be used to customize + /// the validation of view model and model data. + /// + public static readonly IList ValidationCheckers = + new List + { + new IndeiValidationCheckerPlugin(), + new ExceptionValidationCheckerPlugin() + }; + private readonly WeakReference _root; private readonly Func _rootGetter; private readonly IObservable _rootObservable; diff --git a/src/Markup/Perspex.Markup/Data/ExpressionSubject.cs b/src/Markup/Perspex.Markup/Data/ExpressionSubject.cs index 46df3ce490..2ba0a5b2db 100644 --- a/src/Markup/Perspex.Markup/Data/ExpressionSubject.cs +++ b/src/Markup/Perspex.Markup/Data/ExpressionSubject.cs @@ -155,6 +155,7 @@ namespace Perspex.Markup.Data { var converted = value as BindingError ?? + value as ValidationStatus ?? Converter.Convert( value, _targetType, diff --git a/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs index dc8b9b18ad..4c918b8b46 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs @@ -7,8 +7,17 @@ using Perspex.Data; namespace Perspex.Markup.Data.Plugins { + /// + /// Validates properties that report errors by throwing exceptions. + /// public class ExceptionValidationCheckerPlugin : IValidationCheckerPlugin { + + /// + public bool Match(WeakReference reference) => true; + + + /// public ValidationCheckerBase Start(WeakReference reference, string name, IPropertyAccessor accessor, Action callback) { return new ExceptionValidationChecker(reference, name, accessor, callback); @@ -37,16 +46,24 @@ namespace Perspex.Markup.Data.Plugins } } - private class ExceptionValidationStatus : ValidationStatus + /// + /// Describes the current validation status after setting a property value. + /// + public class ExceptionValidationStatus : ValidationStatus { - public ExceptionValidationStatus(Exception exception) + internal ExceptionValidationStatus(Exception exception) { Exception = exception; } + /// + /// The thrown exception. If there was no thrown exception, null. + /// public Exception Exception { get; } - public override bool IsValid => Exception != null; + + /// + public override bool IsValid => Exception == null; } } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs index 9dd4f10dbe..2f36352983 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/IPropertyAccessorPlugin.cs @@ -25,7 +25,6 @@ namespace Perspex.Markup.Data.Plugins /// A weak reference to the object. /// The property name. /// A function to call when the property changes. - /// A function to call when the validation status of the property changes. /// /// An interface through which future interactions with the /// property will be made. @@ -33,7 +32,6 @@ namespace Perspex.Markup.Data.Plugins IPropertyAccessor Start( WeakReference reference, string propertyName, - Action changed, - Action validationChanged); + Action changed); } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs index 07a10f235f..9aff3cf3cc 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/IValidationCheckerPlugin.cs @@ -7,8 +7,30 @@ using System.Threading.Tasks; namespace Perspex.Markup.Data.Plugins { + /// + /// Defines how view model data validation is observed by an . + /// public interface IValidationCheckerPlugin { + + /// + /// Checks whether the data uses a validation scheme supported by this plugin. + /// + /// A weak reference to the data. + /// true if this plugin can observe the validation; otherwise, false. + bool Match(WeakReference reference); + + /// + /// Starts monitering the validation state of an object for the given property. + /// + /// A weak reference to the object. + /// The property name. + /// An underlying to access the property. + /// A function to call when the validation state changes. + /// + /// A subclass through which future interactions with the + /// property will be made. + /// ValidationCheckerBase Start(WeakReference reference, string name, IPropertyAccessor accessor, Action callback); } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs index dfd63ef2b6..0959f9b6fc 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs @@ -10,8 +10,18 @@ using Perspex.Utilities; namespace Perspex.Markup.Data.Plugins { - class IndeiValidationCheckerPlugin : IValidationCheckerPlugin + /// + /// Validates properties on objects that implement . + /// + public class IndeiValidationCheckerPlugin : IValidationCheckerPlugin { + /// + public bool Match(WeakReference reference) + { + return reference.Target is INotifyDataErrorInfo; + } + + /// public ValidationCheckerBase Start(WeakReference reference, string name, IPropertyAccessor accessor, Action callback) { return new IndeiValidationChecker(reference, name, accessor, callback); @@ -59,14 +69,22 @@ namespace Perspex.Markup.Data.Plugins } } - private class IndeiValidationStatus : ValidationStatus + /// + /// Describes the current validation status of a property as reported by an object that implements . + /// + public class IndeiValidationStatus : ValidationStatus { - public IndeiValidationStatus(IEnumerable errors) + internal IndeiValidationStatus(IEnumerable errors) { Errors = errors; } + + /// public override bool IsValid => !Errors.OfType().Any(); + /// + /// The errors on the given property and on the object as a whole. + /// public IEnumerable Errors { get; } } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs index 9eee066c61..a16903ea42 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/InpcPropertyAccessorPlugin.cs @@ -37,7 +37,6 @@ namespace Perspex.Markup.Data.Plugins /// The object. /// The property name. /// A function to call when the property changes. - /// A function to call when the validation state of the property changes. /// /// An interface through which future interactions with the /// property will be made. @@ -45,8 +44,7 @@ namespace Perspex.Markup.Data.Plugins public IPropertyAccessor Start( WeakReference reference, string propertyName, - Action changed, - Action validationChanged) + Action changed) { Contract.Requires(reference != null); Contract.Requires(propertyName != null); @@ -57,7 +55,7 @@ namespace Perspex.Markup.Data.Plugins if (p != null) { - return new Accessor(reference, p, changed, validationChanged); + return new Accessor(reference, p, changed); } else { @@ -67,18 +65,16 @@ namespace Perspex.Markup.Data.Plugins } } - private class Accessor : IPropertyAccessor, IWeakSubscriber, IWeakSubscriber + private class Accessor : IPropertyAccessor, IWeakSubscriber { private readonly WeakReference _reference; private readonly PropertyInfo _property; private readonly Action _changed; - private readonly Action _validationChanged; public Accessor( WeakReference reference, PropertyInfo property, - Action changed, - Action validationChanged) + Action changed) { Contract.Requires(reference != null); Contract.Requires(property != null); @@ -86,7 +82,6 @@ namespace Perspex.Markup.Data.Plugins _reference = reference; _property = property; _changed = changed; - _validationChanged = validationChanged; var inpc = reference.Target as INotifyPropertyChanged; @@ -107,19 +102,6 @@ namespace Perspex.Markup.Data.Plugins reference.Target, reference.Target.GetType()); } - - var indei = _reference.Target as INotifyDataErrorInfo; - if (indei != null) - { - if (indei.HasErrors) - { - _validationChanged(indei.GetErrors(property.Name)); - } - WeakSubscriptionManager.Subscribe( - indei, - nameof(indei.ErrorsChanged), - this); - } } public Type PropertyType => _property.PropertyType; @@ -137,15 +119,6 @@ namespace Perspex.Markup.Data.Plugins nameof(inpc.PropertyChanged), this); } - - var indei = _reference.Target as INotifyDataErrorInfo; - if (indei != null) - { - WeakSubscriptionManager.Unsubscribe( - indei, - nameof(indei.ErrorsChanged), - this); - } } public bool SetValue(object value, BindingPriority priority) @@ -166,15 +139,6 @@ namespace Perspex.Markup.Data.Plugins _changed(Value); } } - - void IWeakSubscriber.OnEvent(object sender, DataErrorsChangedEventArgs e) - { - if (e.PropertyName == _property.Name || string.IsNullOrEmpty(e.PropertyName)) - { - var indei = _reference.Target as INotifyDataErrorInfo; - _validationChanged(indei.GetErrors(e.PropertyName)); - } - } } } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs index d27d1e8be4..bc16989a7a 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/PerspexPropertyAccessorPlugin.cs @@ -31,7 +31,6 @@ namespace Perspex.Markup.Data.Plugins /// A weak reference to the object. /// The property name. /// A function to call when the property changes. - /// A function to call when the validation state of the property changes. /// /// An interface through which future interactions with the /// property will be made. @@ -39,8 +38,7 @@ namespace Perspex.Markup.Data.Plugins public IPropertyAccessor Start( WeakReference reference, string propertyName, - Action changed, - Action validationChanged) + Action changed) { Contract.Requires(reference != null); Contract.Requires(propertyName != null); diff --git a/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs b/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs index 0f76503ff3..72cf80e79e 100644 --- a/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs +++ b/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs @@ -50,11 +50,16 @@ namespace Perspex.Markup.Data if (instance != null && instance != PerspexProperty.UnsetValue) { - var plugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(reference)); + var accessorPlugin = ExpressionObserver.PropertyAccessors.FirstOrDefault(x => x.Match(reference)); - if (plugin != null) + if (accessorPlugin != null) { - _accessor = plugin.Start(reference, PropertyName, SetCurrentValue, _ => { }); + _accessor = accessorPlugin.Start(reference, PropertyName, SetCurrentValue); + var validationPlugin = ExpressionObserver.ValidationCheckers.FirstOrDefault(x => x.Match(reference)); + if (validationPlugin != null) + { + _accessor = validationPlugin.Start(reference, PropertyName, _accessor, SendValidationStatus); + } if (_accessor != null) { diff --git a/src/Perspex.Base/IPriorityValueOwner.cs b/src/Perspex.Base/IPriorityValueOwner.cs index aa79864794..2a3192dfca 100644 --- a/src/Perspex.Base/IPriorityValueOwner.cs +++ b/src/Perspex.Base/IPriorityValueOwner.cs @@ -1,6 +1,8 @@ // Copyright (c) The Perspex Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using Perspex.Data; + namespace Perspex { /// @@ -15,5 +17,12 @@ namespace Perspex /// The old value. /// The new value. void Changed(PriorityValue sender, object oldValue, object newValue); + + /// + /// Called when the validation state of a changes. + /// + /// The source of the change. + /// The validation status. + void ValidationChanged(PriorityValue sender, ValidationStatus status); } } diff --git a/src/Perspex.Base/PerspexObject.cs b/src/Perspex.Base/PerspexObject.cs index 0f28f598d8..9e89357c89 100644 --- a/src/Perspex.Base/PerspexObject.cs +++ b/src/Perspex.Base/PerspexObject.cs @@ -403,6 +403,7 @@ namespace Perspex IDisposable subscription = null; subscription = source + .Where(x => !(x is ValidationStatus)) .Select(x => CastOrDefault(x, property.PropertyType)) .Do(_ => { }, () => s_directBindings.Remove(subscription)) .Subscribe(x => DirectBindingSet(property, x)); @@ -505,6 +506,18 @@ namespace Perspex } } + /// + void IPriorityValueOwner.ValidationChanged(PriorityValue sender, ValidationStatus status) + { + var property = sender.Property; + ValidationChanged(property, status); + } + + protected virtual void ValidationChanged(PerspexProperty property, ValidationStatus status) + { + + } + /// Delegate[] IPerspexObjectDebug.GetPropertyChangedSubscribers() { diff --git a/src/Perspex.Base/PriorityBindingEntry.cs b/src/Perspex.Base/PriorityBindingEntry.cs index be6c451a71..61aeb89323 100644 --- a/src/Perspex.Base/PriorityBindingEntry.cs +++ b/src/Perspex.Base/PriorityBindingEntry.cs @@ -99,7 +99,13 @@ namespace Perspex _owner.Error(this, bindingError); } - if (bindingError == null || bindingError.UseFallbackValue) + var validationStatus = value as ValidationStatus; + + if (validationStatus != null) + { + _owner.Validation(this, validationStatus); + } + else if (bindingError == null || bindingError.UseFallbackValue) { Value = bindingError == null ? value : bindingError.FallbackValue; _owner.Changed(this); diff --git a/src/Perspex.Base/PriorityLevel.cs b/src/Perspex.Base/PriorityLevel.cs index a3167f1aa3..a5f078f73c 100644 --- a/src/Perspex.Base/PriorityLevel.cs +++ b/src/Perspex.Base/PriorityLevel.cs @@ -164,6 +164,17 @@ namespace Perspex _owner.LevelError(this, error); } + /// + /// Invoked when an entry in reports validation status. + /// + /// The entry that completed. + /// The validation status. + public void Validation(PriorityBindingEntry entry, ValidationStatus validationStatus) + { + _owner.LevelValidation(this, validationStatus); + } + + /// /// Activates the first binding that has a value. /// diff --git a/src/Perspex.Base/PriorityValue.cs b/src/Perspex.Base/PriorityValue.cs index 7a81466d17..ffd37e62f3 100644 --- a/src/Perspex.Base/PriorityValue.cs +++ b/src/Perspex.Base/PriorityValue.cs @@ -178,6 +178,16 @@ namespace Perspex } } + /// + /// Called whenever a priority level validation state changes. + /// + /// The priority level of the changed entry. + /// The validation status. + public void LevelValidation(PriorityLevel priorityLevel, ValidationStatus validationStatus) + { + _owner.ValidationChanged(this, validationStatus); + } + /// /// Called when a priority level encounters an error. /// diff --git a/src/Perspex.Controls/Control.cs b/src/Perspex.Controls/Control.cs index 74899a336a..c4a99d8119 100644 --- a/src/Perspex.Controls/Control.cs +++ b/src/Perspex.Controls/Control.cs @@ -86,6 +86,12 @@ namespace Perspex.Controls public static readonly RoutedEvent RequestBringIntoViewEvent = RoutedEvent.Register("RequestBringIntoView", RoutingStrategies.Bubble); + /// + /// Defines the property. + /// + public static readonly DirectProperty ValidationStatusProperty = + PerspexProperty.RegisterDirect(nameof(ValidationStatus), c=> c.ValidationStatus); + private int _initCount; private string _name; private IControl _parent; @@ -108,6 +114,7 @@ namespace Perspex.Controls PseudoClass(IsEnabledCoreProperty, x => !x, ":disabled"); PseudoClass(IsFocusedProperty, ":focus"); PseudoClass(IsPointerOverProperty, ":pointerover"); + PseudoClass(ValidationStatusProperty, status => status != null && !status.IsValid, ":invalid"); } /// @@ -399,6 +406,29 @@ namespace Perspex.Controls /// protected IPseudoClasses PseudoClasses => Classes; + private ValidationStatus validationStatus; + + /// + /// The current validation status of the control. + /// + public ValidationStatus ValidationStatus + { + get + { + return validationStatus; + } + private set + { + SetAndRaise(ValidationStatusProperty, ref validationStatus, value); + } + } + + protected override void ValidationChanged(PerspexProperty property, ValidationStatus status) + { + base.ValidationChanged(property, status); + ValidationStatus = status; + } + /// /// Sets the control's logical parent. /// diff --git a/tests/Perspex.Markup.UnitTests/Data/ExceptionValidatorTests.cs b/tests/Perspex.Markup.UnitTests/Data/ExceptionValidatorTests.cs new file mode 100644 index 0000000000..3725bf3934 --- /dev/null +++ b/tests/Perspex.Markup.UnitTests/Data/ExceptionValidatorTests.cs @@ -0,0 +1,99 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using Perspex.Data; +using Perspex.Markup.Data.Plugins; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace Perspex.Markup.UnitTests.Data +{ + public class ExceptionValidatorTests + { + public class Data : INotifyPropertyChanged + { + private int nonValidated; + + public int NonValidated + { + get { return nonValidated; } + set { nonValidated = value; NotifyPropertyChanged(); } + } + + private int mustBePositive; + + public int MustBePositive + { + get { return mustBePositive; } + set + { + if (value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + mustBePositive = value; + } + } + + public event PropertyChangedEventHandler PropertyChanged; + + private void NotifyPropertyChanged([CallerMemberName] string propertyName = "") + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + } + } + + [Fact] + public void Setting_Non_Validating_Triggers_Validation() + { + var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); + var validatorPlugin = new ExceptionValidationCheckerPlugin(); + var data = new Data(); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.NonValidated), _ => { }); + ValidationStatus status = null; + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.NonValidated), accessor, s => status = s); + + validator.SetValue(5, BindingPriority.LocalValue); + + Assert.NotNull(status); + } + + [Fact] + public void Setting_Validating_Property_To_Valid_Value_Returns_Successful_ValidationStatus() + { + var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); + var validatorPlugin = new ExceptionValidationCheckerPlugin(); + var data = new Data(); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), _ => { }); + ValidationStatus status = null; + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), accessor, s => status = s); + + validator.SetValue(5, BindingPriority.LocalValue); + + Assert.True(status.IsValid); + } + + + + [Fact] + public void Setting_Validating_Property_To_Invalid_Value_Returns_Failed_ValidationStatus() + { + var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); + var validatorPlugin = new ExceptionValidationCheckerPlugin(); + var data = new Data(); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), _ => { }); + ValidationStatus status = null; + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), accessor, s => status = s); + + validator.SetValue(-5, BindingPriority.LocalValue); + + Assert.False(status.IsValid); + } + } +} diff --git a/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs b/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs index 1a201dcd9c..464a08afa7 100644 --- a/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs +++ b/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs @@ -300,13 +300,14 @@ namespace Perspex.Markup.UnitTests.Data Assert.False(target.SetValue("baz")); } - [Fact] + [Fact(Skip = "Validation captures the exception")] public void SetValue_Should_Throw_For_Wrong_Type() { var data = new Class1 { Foo = "foo" }; var target = new ExpressionObserver(data, "Foo"); - Assert.Throws(() => target.SetValue(1.2)); + Assert.Throws(() => + target.SetValue(1.2)); } [Fact] diff --git a/tests/Perspex.Markup.UnitTests/Data/IndeiValidatorTests.cs b/tests/Perspex.Markup.UnitTests/Data/IndeiValidatorTests.cs new file mode 100644 index 0000000000..fd9ce418e0 --- /dev/null +++ b/tests/Perspex.Markup.UnitTests/Data/IndeiValidatorTests.cs @@ -0,0 +1,120 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + + +using Perspex.Data; +using Perspex.Markup.Data.Plugins; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; +using Xunit; +using System.Collections; + +namespace Perspex.Markup.UnitTests.Data +{ + public class IndeiValidatorTests + { + public class Data : INotifyPropertyChanged, INotifyDataErrorInfo + { + private int nonValidated; + + public int NonValidated + { + get { return nonValidated; } + set { nonValidated = value; NotifyPropertyChanged(); } + } + + private int mustBePositive; + + public int MustBePositive + { + get { return mustBePositive; } + set + { + mustBePositive = value; + NotifyErrorsChanged(); + } + } + + public bool HasErrors + { + get + { + return MustBePositive > 0; + } + } + + public event PropertyChangedEventHandler PropertyChanged; + public event EventHandler ErrorsChanged; + + private void NotifyPropertyChanged([CallerMemberName] string propertyName = "") + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + } + + private void NotifyErrorsChanged([CallerMemberName] string propertyName = "") + { + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(propertyName)); + } + + public IEnumerable GetErrors(string propertyName) + { + if (propertyName == nameof(MustBePositive) && MustBePositive <= 0) + { + yield return $"{nameof(MustBePositive)} must be positive"; + } + } + } + + [Fact] + public void Setting_Non_Validating_Does_Not_Trigger_Validation() + { + var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); + var validatorPlugin = new IndeiValidationCheckerPlugin(); + var data = new Data(); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.NonValidated), _ => { }); + ValidationStatus status = null; + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.NonValidated), accessor, s => status = s); + + validator.SetValue(5, BindingPriority.LocalValue); + + Assert.Null(status); + } + + [Fact] + public void Setting_Validating_Property_To_Valid_Value_Returns_Successful_ValidationStatus() + { + var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); + var validatorPlugin = new IndeiValidationCheckerPlugin(); + var data = new Data(); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), _ => { }); + ValidationStatus status = null; + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), accessor, s => status = s); + + validator.SetValue(5, BindingPriority.LocalValue); + + Assert.True(status.IsValid); + } + + + + [Fact] + public void Setting_Validating_Property_To_Invalid_Value_Returns_Failed_ValidationStatus() + { + var inpcAccessorPlugin = new InpcPropertyAccessorPlugin(); + var validatorPlugin = new IndeiValidationCheckerPlugin(); + var data = new Data(); + var accessor = inpcAccessorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), _ => { }); + ValidationStatus status = null; + var validator = validatorPlugin.Start(new WeakReference(data), nameof(data.MustBePositive), accessor, s => status = s); + + validator.SetValue(-5, BindingPriority.LocalValue); + + Assert.False(status.IsValid); + } + } +} diff --git a/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs b/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs deleted file mode 100644 index 74bd6bd7b3..0000000000 --- a/tests/Perspex.Markup.UnitTests/Data/InpcPluginTests.cs +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright (c) The Perspex Project. All rights reserved. -// Licensed under the MIT license. See licence.md file in the project root for full license information. - -using Perspex.Markup.Data.Plugins; -using System; -using System.Collections; -using System.ComponentModel; -using System.Runtime.CompilerServices; -using Xunit; - -namespace Perspex.Markup.UnitTests.Data -{ - public class InpcPluginTests - { - private class InpcTest : INotifyPropertyChanged, INotifyDataErrorInfo - { - private int noValidationTest; - - public int NoValidationTest - { - get { return noValidationTest; } - set - { - noValidationTest = value; - NotifyPropertyChanged(); - } - } - - public bool HasErrors - { - get - { - return NonNegative < 0; - } - } - - private int nonNegative; - - public int NonNegative - { - get { return nonNegative; } - set - { - var old = nonNegative; - nonNegative = value; - NotifyPropertyChanged(); - if (old * value < 0) // If signs are different - { - NotifyErrorsChanged(); - } - } - } - - - public event EventHandler ErrorsChanged; - public event PropertyChangedEventHandler PropertyChanged; - - public IEnumerable GetErrors(string propertyName) - { - if (string.IsNullOrEmpty(propertyName) || propertyName == nameof(NonNegative)) - { - if (NonNegative < 0) - { - yield return "Invalid Value"; - } - } - } - - private void NotifyPropertyChanged([CallerMemberName] string property = "") - { - PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(property)); - } - - private void NotifyErrorsChanged([CallerMemberName] string property = "") - { - ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(property)); - } - } - - [Fact] - public void Calls_Change_Callback_When_Value_Changes() - { - var plugin = new InpcPropertyAccessorPlugin(); - var source = new InpcTest { NoValidationTest = 0 }; - var changeFired = false; - var accessor = plugin.Start(new WeakReference(source), nameof(InpcTest.NoValidationTest), _ => changeFired = true, _ => { }); - source.NoValidationTest = 1; - - Assert.True(changeFired); - } - - [Fact] - public void ValidationChanged_Does_Not_Fire_When_NonValidated_Value_Changes() - { - var plugin = new InpcPropertyAccessorPlugin(); - var source = new InpcTest { NoValidationTest = 0 }; - var validationFired = false; - plugin.Start(new WeakReference(source), nameof(InpcTest.NoValidationTest), _ => { }, _ => validationFired = true); - source.NoValidationTest = 1; - - Assert.False(validationFired); - } - - [Fact] - public void ValidationChanged_Does_Not_Fire_When_Validation_Does_Not_Change() - { - var plugin = new InpcPropertyAccessorPlugin(); - var source = new InpcTest { NonNegative = 3 }; - var validationFired = false; - plugin.Start(new WeakReference(source), nameof(InpcTest.NonNegative), _ => { }, _ => validationFired = true); - source.NonNegative = 5; - - Assert.False(validationFired); - } - - [Fact] - public void ValidationChanged_Fires_On_Start_If_Has_Errors() - { - var plugin = new InpcPropertyAccessorPlugin(); - var source = new InpcTest { NonNegative = -5 }; - - Assert.True(source.HasErrors); - - var validationFired = false; - plugin.Start(new WeakReference(source), nameof(InpcTest.NonNegative), _ => { }, _ => validationFired = true); - Assert.True(validationFired); - } - - - - [Fact] - public void ValidationChanged_Fires_When_Validation_Changes() - { - var plugin = new InpcPropertyAccessorPlugin(); - var source = new InpcTest { NonNegative = 5 }; - var validationFired = false; - plugin.Start(new WeakReference(source), nameof(InpcTest.NonNegative), _ => { }, _ => validationFired = true); - source.NonNegative = -1; - Assert.True(validationFired); - } - } -} diff --git a/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj b/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj index fa2b551600..cd1b9f4aa9 100644 --- a/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj +++ b/tests/Perspex.Markup.UnitTests/Perspex.Markup.UnitTests.csproj @@ -85,6 +85,7 @@ + @@ -97,7 +98,7 @@ - + From 6a8e5e239072f4a54cf48521919dbdfe8fb70364 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sun, 17 Apr 2016 20:40:20 -0500 Subject: [PATCH 05/12] Fixed validation propogation on direct properties. --- src/Perspex.Base/PerspexObject.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Perspex.Base/PerspexObject.cs b/src/Perspex.Base/PerspexObject.cs index 9e89357c89..c0a361783a 100644 --- a/src/Perspex.Base/PerspexObject.cs +++ b/src/Perspex.Base/PerspexObject.cs @@ -401,17 +401,22 @@ namespace Perspex GetDescription(source)); IDisposable subscription = null; + IDisposable validationSubcription = null; subscription = source .Where(x => !(x is ValidationStatus)) .Select(x => CastOrDefault(x, property.PropertyType)) .Do(_ => { }, () => s_directBindings.Remove(subscription)) .Subscribe(x => DirectBindingSet(property, x)); + validationSubcription = source.OfType().Subscribe(x => ValidationChanged(property, x)); s_directBindings.Add(subscription); + s_directBindings.Add(validationSubcription); return Disposable.Create(() => { + validationSubcription.Dispose(); + s_directBindings.Remove(validationSubcription); subscription.Dispose(); s_directBindings.Remove(subscription); }); From e399424911c4d497747f223ff3d5964858d98823 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sun, 17 Apr 2016 20:43:21 -0500 Subject: [PATCH 06/12] Added missing documentation. --- src/Perspex.Base/Data/ValidationStatus.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Perspex.Base/Data/ValidationStatus.cs b/src/Perspex.Base/Data/ValidationStatus.cs index 9effaa8239..df0dede70d 100644 --- a/src/Perspex.Base/Data/ValidationStatus.cs +++ b/src/Perspex.Base/Data/ValidationStatus.cs @@ -15,6 +15,9 @@ namespace Perspex.Data /// public abstract class ValidationStatus { + /// + /// True when the data passes validation; otherwise, false. + /// public abstract bool IsValid { get; } } } From 774c6e6fa437b64ad69fee74ae7fea0ed51e7029 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 18 Apr 2016 13:39:53 -0500 Subject: [PATCH 07/12] Made validation enable-able during binding construction, similar to WPF. Now validation events are only raised when the validation type is enabled on the property. --- .../Perspex.Markup.Xaml/Data/Binding.cs | 4 +- .../MarkupExtensions/BindingExtension.cs | 2 + .../ExceptionValidationCheckerPlugin.cs | 5 + .../Plugins/IndeiValidationCheckerPlugin.cs | 5 + src/Perspex.Base/Data/BindingOperations.cs | 4 +- src/Perspex.Base/Data/InstancedBinding.cs | 21 +++- src/Perspex.Base/Data/ValidationMethods.cs | 13 ++ src/Perspex.Base/Data/ValidationStatus.cs | 7 ++ src/Perspex.Base/IPerspexObject.cs | 8 +- src/Perspex.Base/Perspex.Base.csproj | 1 + src/Perspex.Base/PerspexObject.cs | 17 ++- src/Perspex.Base/PriorityBindingEntry.cs | 10 +- src/Perspex.Base/PriorityLevel.cs | 5 +- src/Perspex.Base/PriorityValue.cs | 5 +- .../Data/BindingTests_TemplatedParent.cs | 6 +- .../Data/BindingTests_Validation.cs | 112 ++++++++++++++++++ .../Perspex.Markup.Xaml.UnitTests.csproj | 1 + .../SelectorTests_Child.cs | 4 +- .../SelectorTests_Descendent.cs | 4 +- .../TestControlBase.cs | 4 +- .../TestTemplatedControl.cs | 4 +- 21 files changed, 212 insertions(+), 30 deletions(-) create mode 100644 src/Perspex.Base/Data/ValidationMethods.cs create mode 100644 tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs diff --git a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs index a99582cea7..f2dbd263a0 100644 --- a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs +++ b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs @@ -77,6 +77,8 @@ namespace Perspex.Markup.Xaml.Data /// public object Source { get; set; } + public ValidationMethods ValidationMethods { get; set; } + /// public InstancedBinding Initiate( IPerspexObject target, @@ -126,7 +128,7 @@ namespace Perspex.Markup.Xaml.Data FallbackValue, Priority); - return new InstancedBinding(subject, Mode, Priority); + return new InstancedBinding(subject, Mode, Priority, ValidationMethods); } private static PathInfo ParsePath(string path) diff --git a/src/Markup/Perspex.Markup.Xaml/MarkupExtensions/BindingExtension.cs b/src/Markup/Perspex.Markup.Xaml/MarkupExtensions/BindingExtension.cs index 1e996b2b2a..d0dcd069a3 100644 --- a/src/Markup/Perspex.Markup.Xaml/MarkupExtensions/BindingExtension.cs +++ b/src/Markup/Perspex.Markup.Xaml/MarkupExtensions/BindingExtension.cs @@ -29,6 +29,7 @@ namespace Perspex.Markup.Xaml.MarkupExtensions Mode = Mode, Path = Path, Priority = Priority, + ValidationMethods = ValidationMethods }; } @@ -40,5 +41,6 @@ namespace Perspex.Markup.Xaml.MarkupExtensions public string Path { get; set; } public BindingPriority Priority { get; set; } = BindingPriority.LocalValue; public object Source { get; set; } + public ValidationMethods ValidationMethods { get; set; } = ValidationMethods.None; } } \ No newline at end of file diff --git a/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs index 4c918b8b46..5110f4713e 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/ExceptionValidationCheckerPlugin.cs @@ -64,6 +64,11 @@ namespace Perspex.Markup.Data.Plugins /// public override bool IsValid => Exception == null; + + public override bool Match(ValidationMethods enabledMethods) + { + return (enabledMethods & ValidationMethods.Exceptions) != 0; + } } } } diff --git a/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs b/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs index 0959f9b6fc..ee4e87076f 100644 --- a/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs +++ b/src/Markup/Perspex.Markup/Data/Plugins/IndeiValidationCheckerPlugin.cs @@ -86,6 +86,11 @@ namespace Perspex.Markup.Data.Plugins /// The errors on the given property and on the object as a whole. /// public IEnumerable Errors { get; } + + public override bool Match(ValidationMethods enabledMethods) + { + return (enabledMethods & ValidationMethods.INotifyDataErrorInfo) != 0; + } } } } diff --git a/src/Perspex.Base/Data/BindingOperations.cs b/src/Perspex.Base/Data/BindingOperations.cs index efa0c8500d..dbeacfbe90 100644 --- a/src/Perspex.Base/Data/BindingOperations.cs +++ b/src/Perspex.Base/Data/BindingOperations.cs @@ -44,10 +44,10 @@ namespace Perspex.Data { case BindingMode.Default: case BindingMode.OneWay: - return target.Bind(property, binding.Observable ?? binding.Subject, binding.Priority); + return target.Bind(property, binding.Observable ?? binding.Subject, binding.Priority, binding.ValidationMethods); case BindingMode.TwoWay: return new CompositeDisposable( - target.Bind(property, binding.Subject, binding.Priority), + target.Bind(property, binding.Subject, binding.Priority, binding.ValidationMethods), target.GetObservable(property).Subscribe(binding.Subject)); case BindingMode.OneTime: var source = binding.Subject ?? binding.Observable; diff --git a/src/Perspex.Base/Data/InstancedBinding.cs b/src/Perspex.Base/Data/InstancedBinding.cs index 545d690aa4..3b911fef35 100644 --- a/src/Perspex.Base/Data/InstancedBinding.cs +++ b/src/Perspex.Base/Data/InstancedBinding.cs @@ -29,12 +29,16 @@ namespace Perspex.Data /// /// The value used for the binding. /// + /// The validation methods for this binding. /// The binding priority. - public InstancedBinding(object value, BindingPriority priority = BindingPriority.LocalValue) + public InstancedBinding(object value, + BindingPriority priority = BindingPriority.LocalValue, + ValidationMethods methods = ValidationMethods.None) { Mode = BindingMode.OneTime; Priority = priority; Value = value; + ValidationMethods = methods; } /// @@ -43,10 +47,12 @@ namespace Perspex.Data /// The observable for a one-way binding. /// The binding mode. /// The binding priority. + /// The validation methods for this binding. public InstancedBinding( IObservable observable, BindingMode mode = BindingMode.OneWay, - BindingPriority priority = BindingPriority.LocalValue) + BindingPriority priority = BindingPriority.LocalValue, + ValidationMethods methods = ValidationMethods.None) { Contract.Requires(observable != null); @@ -60,6 +66,7 @@ namespace Perspex.Data Mode = mode; Priority = priority; Observable = observable; + ValidationMethods = methods; } /// @@ -68,16 +75,19 @@ namespace Perspex.Data /// The subject for a two-way binding. /// The binding mode. /// The binding priority. + /// The validation methods for this binding. public InstancedBinding( ISubject subject, BindingMode mode = BindingMode.OneWay, - BindingPriority priority = BindingPriority.LocalValue) + BindingPriority priority = BindingPriority.LocalValue, + ValidationMethods methods = ValidationMethods.None) { Contract.Requires(subject != null); Mode = mode; Priority = priority; Subject = subject; + ValidationMethods = methods; } /// @@ -104,5 +114,10 @@ namespace Perspex.Data /// Gets the subject for a two-way binding. /// public ISubject Subject { get; } + + /// + /// Gets the validation methods for this binding. + /// + public ValidationMethods ValidationMethods { get; } } } diff --git a/src/Perspex.Base/Data/ValidationMethods.cs b/src/Perspex.Base/Data/ValidationMethods.cs new file mode 100644 index 0000000000..491a7bbe50 --- /dev/null +++ b/src/Perspex.Base/Data/ValidationMethods.cs @@ -0,0 +1,13 @@ +using System; + +namespace Perspex.Data +{ + [Flags] + public enum ValidationMethods + { + None = 0, + Exceptions = 1, + INotifyDataErrorInfo = 2, + All = -1 + } +} \ No newline at end of file diff --git a/src/Perspex.Base/Data/ValidationStatus.cs b/src/Perspex.Base/Data/ValidationStatus.cs index df0dede70d..ac2600c13c 100644 --- a/src/Perspex.Base/Data/ValidationStatus.cs +++ b/src/Perspex.Base/Data/ValidationStatus.cs @@ -19,5 +19,12 @@ namespace Perspex.Data /// True when the data passes validation; otherwise, false. /// public abstract bool IsValid { get; } + + /// + /// Checks if this validation status came from a currently enabled method of validation checking. + /// + /// The enabled methods of validation checking. + /// True if enabled; otherwise, false. + public abstract bool Match(ValidationMethods enabledMethods); } } diff --git a/src/Perspex.Base/IPerspexObject.cs b/src/Perspex.Base/IPerspexObject.cs index 4290bfe792..c92898bffb 100644 --- a/src/Perspex.Base/IPerspexObject.cs +++ b/src/Perspex.Base/IPerspexObject.cs @@ -67,13 +67,15 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. + /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue); + BindingPriority priority = BindingPriority.LocalValue, + ValidationMethods validation = ValidationMethods.None); /// /// Binds a to an observable. @@ -82,12 +84,14 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. + /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue); + BindingPriority priority = BindingPriority.LocalValue, + ValidationMethods validation = ValidationMethods.None); } } \ No newline at end of file diff --git a/src/Perspex.Base/Perspex.Base.csproj b/src/Perspex.Base/Perspex.Base.csproj index cc8cb6ada6..234af801ca 100644 --- a/src/Perspex.Base/Perspex.Base.csproj +++ b/src/Perspex.Base/Perspex.Base.csproj @@ -44,6 +44,7 @@ Properties\SharedAssemblyInfo.cs + diff --git a/src/Perspex.Base/PerspexObject.cs b/src/Perspex.Base/PerspexObject.cs index c0a361783a..08b91a9afb 100644 --- a/src/Perspex.Base/PerspexObject.cs +++ b/src/Perspex.Base/PerspexObject.cs @@ -375,13 +375,15 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. + /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// public IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue) + BindingPriority priority = BindingPriority.LocalValue, + ValidationMethods validation = ValidationMethods.None) { Contract.Requires(property != null); VerifyAccess(); @@ -408,15 +410,16 @@ namespace Perspex .Select(x => CastOrDefault(x, property.PropertyType)) .Do(_ => { }, () => s_directBindings.Remove(subscription)) .Subscribe(x => DirectBindingSet(property, x)); - validationSubcription = source.OfType().Subscribe(x => ValidationChanged(property, x)); + validationSubcription = source + .OfType() + .Where(v => v.Match(validation)) + .Subscribe(x => ValidationChanged(property, x)); s_directBindings.Add(subscription); - s_directBindings.Add(validationSubcription); return Disposable.Create(() => { validationSubcription.Dispose(); - s_directBindings.Remove(validationSubcription); subscription.Dispose(); s_directBindings.Remove(subscription); }); @@ -444,7 +447,7 @@ namespace Perspex GetDescription(source), priority); - return v.Add(source, (int)priority); + return v.Add(source, (int)priority, validation); } } @@ -455,13 +458,15 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. + /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// public IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue) + BindingPriority priority = BindingPriority.LocalValue, + ValidationMethods validation = ValidationMethods.None) { Contract.Requires(property != null); diff --git a/src/Perspex.Base/PriorityBindingEntry.cs b/src/Perspex.Base/PriorityBindingEntry.cs index 61aeb89323..4f923e3a9f 100644 --- a/src/Perspex.Base/PriorityBindingEntry.cs +++ b/src/Perspex.Base/PriorityBindingEntry.cs @@ -13,6 +13,7 @@ namespace Perspex { private PriorityLevel _owner; private IDisposable _subscription; + private ValidationMethods _validation; /// /// Initializes a new instance of the class. @@ -21,10 +22,12 @@ namespace Perspex /// /// The binding index. Later bindings should have higher indexes. /// - public PriorityBindingEntry(PriorityLevel owner, int index) + /// The validation settings for the binding. + public PriorityBindingEntry(PriorityLevel owner, int index, ValidationMethods validation) { _owner = owner; Index = index; + _validation = validation; } /// @@ -103,7 +106,10 @@ namespace Perspex if (validationStatus != null) { - _owner.Validation(this, validationStatus); + if (validationStatus.Match(_validation)) + { + _owner.Validation(this, validationStatus); + } } else if (bindingError == null || bindingError.UseFallbackValue) { diff --git a/src/Perspex.Base/PriorityLevel.cs b/src/Perspex.Base/PriorityLevel.cs index a5f078f73c..3049c793d9 100644 --- a/src/Perspex.Base/PriorityLevel.cs +++ b/src/Perspex.Base/PriorityLevel.cs @@ -97,12 +97,13 @@ namespace Perspex /// Adds a binding. /// /// The binding to add. + /// Validation settings for the binding. /// A disposable used to remove the binding. - public IDisposable Add(IObservable binding) + public IDisposable Add(IObservable binding, ValidationMethods validation) { Contract.Requires(binding != null); - var entry = new PriorityBindingEntry(this, _nextIndex++); + var entry = new PriorityBindingEntry(this, _nextIndex++, validation); var node = Bindings.AddFirst(entry); entry.Start(binding); diff --git a/src/Perspex.Base/PriorityValue.cs b/src/Perspex.Base/PriorityValue.cs index ffd37e62f3..155fa47c06 100644 --- a/src/Perspex.Base/PriorityValue.cs +++ b/src/Perspex.Base/PriorityValue.cs @@ -77,12 +77,13 @@ namespace Perspex /// /// The binding. /// The binding priority. + /// Validation settings for the binding. /// /// A disposable that will remove the binding. /// - public IDisposable Add(IObservable binding, int priority) + public IDisposable Add(IObservable binding, int priority, ValidationMethods validation = ValidationMethods.None) { - return GetLevel(priority).Add(binding); + return GetLevel(priority).Add(binding, validation); } /// diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs index 1979d5d9bf..9cfcac0825 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs +++ b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs @@ -32,7 +32,8 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Verify(x => x.Bind( TextBox.TextProperty, It.IsAny>(), - BindingPriority.TemplatedParent)); + BindingPriority.TemplatedParent, + ValidationMethods.None)); } [Fact] @@ -52,7 +53,8 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Verify(x => x.Bind( TextBox.TextProperty, It.IsAny>(), - BindingPriority.TemplatedParent)); + BindingPriority.TemplatedParent, + ValidationMethods.None)); } private Mock CreateTarget( diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs new file mode 100644 index 0000000000..799486bd5f --- /dev/null +++ b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs @@ -0,0 +1,112 @@ +using Perspex.Controls; +using Perspex.Markup.Xaml.Data; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace Perspex.Markup.Xaml.UnitTests.Data +{ + public class BindingTests_Validation + { + + public class Data : INotifyPropertyChanged + { + private string mustbeNonEmpty; + + public string MustBeNonEmpty + { + get { return mustbeNonEmpty; } + set + { + if (string.IsNullOrEmpty(value)) + { + throw new ArgumentException(nameof(value)); + } + mustbeNonEmpty = value; + } + } + + public event PropertyChangedEventHandler PropertyChanged; + + private void NotifyPropertyChanged([CallerMemberName] string propertyName = "") + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + } + } + + [Fact] + public void Disabled_Validation_Should_Not_Trigger_Validation_Change_Direct() + { + var source = new Data { MustBeNonEmpty = "Test" }; + var target = new TextBlock { DataContext = source }; + var binding = new Binding + { + Path = nameof(source.MustBeNonEmpty), + Mode = Perspex.Data.BindingMode.TwoWay, + ValidationMethods = Perspex.Data.ValidationMethods.None + }; + target.Bind(TextBlock.TextProperty, binding); + + target.Text = ""; + + Assert.Null(target.ValidationStatus); + } + + [Fact] + public void Enabled_Validation_Should_Trigger_Validation_Change_Direct() + { + var source = new Data { MustBeNonEmpty = "Test" }; + var target = new TextBlock { DataContext = source }; + var binding = new Binding + { + Path = nameof(source.MustBeNonEmpty), + Mode = Perspex.Data.BindingMode.TwoWay, + ValidationMethods = Perspex.Data.ValidationMethods.All + }; + target.Bind(TextBlock.TextProperty, binding); + + target.Text = ""; + Assert.NotNull(target.ValidationStatus); + } + + [Fact] + public void Disabled_Validation_Should_Not_Trigger_Validation_Change_Styled() + { + var source = new Data { MustBeNonEmpty = "Test" }; + var target = new TextBlock { DataContext = source }; + var binding = new Binding + { + Path = nameof(source.MustBeNonEmpty), + Mode = Perspex.Data.BindingMode.TwoWay, + ValidationMethods = Perspex.Data.ValidationMethods.None + }; + target.Bind(Control.TagProperty, binding); + + target.Tag = ""; + + Assert.Null(target.ValidationStatus); + } + + [Fact] + public void Enabled_Validation_Should_Trigger_Validation_Change_Styled() + { + var source = new Data { MustBeNonEmpty = "Test" }; + var target = new TextBlock { DataContext = source }; + var binding = new Binding + { + Path = nameof(source.MustBeNonEmpty), + Mode = Perspex.Data.BindingMode.TwoWay, + ValidationMethods = Perspex.Data.ValidationMethods.All + }; + target.Bind(Control.TagProperty, binding); + + target.Tag = ""; + Assert.NotNull(target.ValidationStatus); + } + } +} diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Perspex.Markup.Xaml.UnitTests.csproj b/tests/Perspex.Markup.Xaml.UnitTests/Perspex.Markup.Xaml.UnitTests.csproj index 243c0c53f4..cf3e856d75 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Perspex.Markup.Xaml.UnitTests.csproj +++ b/tests/Perspex.Markup.Xaml.UnitTests/Perspex.Markup.Xaml.UnitTests.csproj @@ -94,6 +94,7 @@ + diff --git a/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs b/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs index 465610b045..b6cb7a0d91 100644 --- a/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs +++ b/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs @@ -129,7 +129,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority, ValidationMethods validation) { throw new NotImplementedException(); } @@ -139,7 +139,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) { throw new NotImplementedException(); } diff --git a/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs b/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs index 286a5ec269..83f0672a6f 100644 --- a/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs +++ b/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs @@ -160,7 +160,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) { throw new NotImplementedException(); } @@ -170,7 +170,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) { throw new NotImplementedException(); } diff --git a/tests/Perspex.Styling.UnitTests/TestControlBase.cs b/tests/Perspex.Styling.UnitTests/TestControlBase.cs index d5bd85f1dd..ac613ac75e 100644 --- a/tests/Perspex.Styling.UnitTests/TestControlBase.cs +++ b/tests/Perspex.Styling.UnitTests/TestControlBase.cs @@ -62,12 +62,12 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) { throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) { throw new NotImplementedException(); } diff --git a/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs b/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs index 3e16e8a18c..156fdfa035 100644 --- a/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs +++ b/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs @@ -57,12 +57,12 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) { throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) { throw new NotImplementedException(); } From 814b6ebac7fd5422afaca3ba3acaf4b4482984c8 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 18 Apr 2016 13:44:13 -0500 Subject: [PATCH 08/12] Removed now unused test. --- .../Data/ExpressionObserverTests_Property.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs b/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs index 464a08afa7..72c4704dbb 100644 --- a/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs +++ b/tests/Perspex.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs @@ -300,16 +300,6 @@ namespace Perspex.Markup.UnitTests.Data Assert.False(target.SetValue("baz")); } - [Fact(Skip = "Validation captures the exception")] - public void SetValue_Should_Throw_For_Wrong_Type() - { - var data = new Class1 { Foo = "foo" }; - var target = new ExpressionObserver(data, "Foo"); - - Assert.Throws(() => - target.SetValue(1.2)); - } - [Fact] public async void Should_Handle_Null_Root() { From 6714806dc044cf195055c6a28f20cb3dd5bd78ed Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 18 Apr 2016 13:51:27 -0500 Subject: [PATCH 09/12] Added a missing documentation comment. --- src/Markup/Perspex.Markup.Xaml/Data/Binding.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs index f2dbd263a0..8ce4aa7302 100644 --- a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs +++ b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs @@ -77,6 +77,9 @@ namespace Perspex.Markup.Xaml.Data /// public object Source { get; set; } + /// + /// Gets or sets the validation methods for the binding to use. + /// public ValidationMethods ValidationMethods { get; set; } /// From da5fd06fcd67301bb3c91e3b6c8f46af67ae00e6 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 19 Apr 2016 13:47:15 -0500 Subject: [PATCH 10/12] Moved validation filtering into ExpressionObserver and out of InstancedBinding, PerspexObject, and the Priority* system. Renamed some methods in this system to be more descriptive. --- .../Perspex.Markup.Xaml/Data/Binding.cs | 10 +++---- .../Perspex.Markup/Data/ExpressionObserver.cs | 21 +++++++++------ src/Perspex.Base/Data/BindingOperations.cs | 4 +-- src/Perspex.Base/Data/InstancedBinding.cs | 20 +++----------- src/Perspex.Base/IPerspexObject.cs | 8 ++---- src/Perspex.Base/IPriorityValueOwner.cs | 2 +- src/Perspex.Base/PerspexObject.cs | 26 +++++++++---------- src/Perspex.Base/PriorityBindingEntry.cs | 9 ++----- src/Perspex.Base/PriorityLevel.cs | 4 +-- src/Perspex.Base/PriorityValue.cs | 6 ++--- src/Perspex.Controls/Control.cs | 5 ++-- .../Data/BindingTests_TemplatedParent.cs | 6 ++--- .../SelectorTests_Child.cs | 4 +-- .../SelectorTests_Descendent.cs | 4 +-- .../TestControlBase.cs | 4 +-- .../TestTemplatedControl.cs | 4 +-- 16 files changed, 59 insertions(+), 78 deletions(-) diff --git a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs index 8ce4aa7302..5a3f647013 100644 --- a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs +++ b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs @@ -131,7 +131,7 @@ namespace Perspex.Markup.Xaml.Data FallbackValue, Priority); - return new InstancedBinding(subject, Mode, Priority, ValidationMethods); + return new InstancedBinding(subject, Mode, Priority); } private static PathInfo ParsePath(string path) @@ -207,7 +207,7 @@ namespace Perspex.Markup.Xaml.Data var result = new ExpressionObserver( () => target.GetValue(Control.DataContextProperty), path, - update); + update, ValidationMethods); return result; } @@ -218,7 +218,7 @@ namespace Perspex.Markup.Xaml.Data .OfType() .Select(x => x.GetObservable(Control.DataContextProperty)) .Switch(), - path); + path, ValidationMethods); } } @@ -228,7 +228,7 @@ namespace Perspex.Markup.Xaml.Data var result = new ExpressionObserver( ControlLocator.Track(target, elementName), - path); + path, ValidationMethods); return result; } @@ -236,7 +236,7 @@ namespace Perspex.Markup.Xaml.Data { Contract.Requires(source != null); - return new ExpressionObserver(source, path); + return new ExpressionObserver(source, path, ValidationMethods); } private ExpressionObserver CreateTemplatedParentObserver( diff --git a/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs b/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs index 082aaf652b..d7fd25d237 100644 --- a/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs +++ b/src/Markup/Perspex.Markup/Data/ExpressionObserver.cs @@ -47,18 +47,20 @@ namespace Perspex.Markup.Data private IDisposable _updateSubscription; private int _count; private readonly ExpressionNode _node; + private ValidationMethods _methods; /// /// Initializes a new instance of the class. /// /// The root object. /// The expression. - public ExpressionObserver(object root, string expression) + /// The validation methods to enable on this observer. + public ExpressionObserver(object root, string expression, ValidationMethods methods = ValidationMethods.None) { Contract.Requires(expression != null); _root = new WeakReference(root); - + _methods = methods; if (!string.IsNullOrWhiteSpace(expression)) { _node = ExpressionNodeBuilder.Build(expression); @@ -72,13 +74,14 @@ namespace Perspex.Markup.Data /// /// An observable which provides the root object. /// The expression. - public ExpressionObserver(IObservable rootObservable, string expression) + /// The validation methods to enable on this observer. + public ExpressionObserver(IObservable rootObservable, string expression, ValidationMethods methods = ValidationMethods.None) { Contract.Requires(rootObservable != null); Contract.Requires(expression != null); _rootObservable = rootObservable; - + _methods = methods; if (!string.IsNullOrWhiteSpace(expression)) { _node = ExpressionNodeBuilder.Build(expression); @@ -93,10 +96,12 @@ namespace Perspex.Markup.Data /// A function which gets the root object. /// The expression. /// An observable which triggers a re-read of the getter. + /// The validation methods to enable on this observer. public ExpressionObserver( Func rootGetter, string expression, - IObservable update) + IObservable update, + ValidationMethods methods = ValidationMethods.None) { Contract.Requires(rootGetter != null); Contract.Requires(expression != null); @@ -104,7 +109,7 @@ namespace Perspex.Markup.Data _rootGetter = rootGetter; _update = update; - + _methods = methods; if (!string.IsNullOrWhiteSpace(expression)) { _node = ExpressionNodeBuilder.Build(expression); @@ -216,8 +221,8 @@ namespace Perspex.Markup.Data { source = source.TakeUntil(_update.LastOrDefaultAsync()); } - - var subscription = source.Subscribe(observer); + var validationFiltered = source.Where(o => (o as ValidationStatus)?.Match(_methods) ?? true); + var subscription = validationFiltered.Subscribe(observer); return Disposable.Create(() => { diff --git a/src/Perspex.Base/Data/BindingOperations.cs b/src/Perspex.Base/Data/BindingOperations.cs index dbeacfbe90..efa0c8500d 100644 --- a/src/Perspex.Base/Data/BindingOperations.cs +++ b/src/Perspex.Base/Data/BindingOperations.cs @@ -44,10 +44,10 @@ namespace Perspex.Data { case BindingMode.Default: case BindingMode.OneWay: - return target.Bind(property, binding.Observable ?? binding.Subject, binding.Priority, binding.ValidationMethods); + return target.Bind(property, binding.Observable ?? binding.Subject, binding.Priority); case BindingMode.TwoWay: return new CompositeDisposable( - target.Bind(property, binding.Subject, binding.Priority, binding.ValidationMethods), + target.Bind(property, binding.Subject, binding.Priority), target.GetObservable(property).Subscribe(binding.Subject)); case BindingMode.OneTime: var source = binding.Subject ?? binding.Observable; diff --git a/src/Perspex.Base/Data/InstancedBinding.cs b/src/Perspex.Base/Data/InstancedBinding.cs index 3b911fef35..50daf56e97 100644 --- a/src/Perspex.Base/Data/InstancedBinding.cs +++ b/src/Perspex.Base/Data/InstancedBinding.cs @@ -29,16 +29,13 @@ namespace Perspex.Data /// /// The value used for the binding. /// - /// The validation methods for this binding. /// The binding priority. public InstancedBinding(object value, - BindingPriority priority = BindingPriority.LocalValue, - ValidationMethods methods = ValidationMethods.None) + BindingPriority priority = BindingPriority.LocalValue) { Mode = BindingMode.OneTime; Priority = priority; Value = value; - ValidationMethods = methods; } /// @@ -47,12 +44,10 @@ namespace Perspex.Data /// The observable for a one-way binding. /// The binding mode. /// The binding priority. - /// The validation methods for this binding. public InstancedBinding( IObservable observable, BindingMode mode = BindingMode.OneWay, - BindingPriority priority = BindingPriority.LocalValue, - ValidationMethods methods = ValidationMethods.None) + BindingPriority priority = BindingPriority.LocalValue) { Contract.Requires(observable != null); @@ -66,7 +61,6 @@ namespace Perspex.Data Mode = mode; Priority = priority; Observable = observable; - ValidationMethods = methods; } /// @@ -75,19 +69,16 @@ namespace Perspex.Data /// The subject for a two-way binding. /// The binding mode. /// The binding priority. - /// The validation methods for this binding. public InstancedBinding( ISubject subject, BindingMode mode = BindingMode.OneWay, - BindingPriority priority = BindingPriority.LocalValue, - ValidationMethods methods = ValidationMethods.None) + BindingPriority priority = BindingPriority.LocalValue) { Contract.Requires(subject != null); Mode = mode; Priority = priority; Subject = subject; - ValidationMethods = methods; } /// @@ -114,10 +105,5 @@ namespace Perspex.Data /// Gets the subject for a two-way binding. /// public ISubject Subject { get; } - - /// - /// Gets the validation methods for this binding. - /// - public ValidationMethods ValidationMethods { get; } } } diff --git a/src/Perspex.Base/IPerspexObject.cs b/src/Perspex.Base/IPerspexObject.cs index c92898bffb..4290bfe792 100644 --- a/src/Perspex.Base/IPerspexObject.cs +++ b/src/Perspex.Base/IPerspexObject.cs @@ -67,15 +67,13 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. - /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue, - ValidationMethods validation = ValidationMethods.None); + BindingPriority priority = BindingPriority.LocalValue); /// /// Binds a to an observable. @@ -84,14 +82,12 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. - /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue, - ValidationMethods validation = ValidationMethods.None); + BindingPriority priority = BindingPriority.LocalValue); } } \ No newline at end of file diff --git a/src/Perspex.Base/IPriorityValueOwner.cs b/src/Perspex.Base/IPriorityValueOwner.cs index 2a3192dfca..1afc005bad 100644 --- a/src/Perspex.Base/IPriorityValueOwner.cs +++ b/src/Perspex.Base/IPriorityValueOwner.cs @@ -23,6 +23,6 @@ namespace Perspex /// /// The source of the change. /// The validation status. - void ValidationChanged(PriorityValue sender, ValidationStatus status); + void DataValidationChanged(PriorityValue sender, ValidationStatus status); } } diff --git a/src/Perspex.Base/PerspexObject.cs b/src/Perspex.Base/PerspexObject.cs index 08b91a9afb..4cd6998b76 100644 --- a/src/Perspex.Base/PerspexObject.cs +++ b/src/Perspex.Base/PerspexObject.cs @@ -375,15 +375,13 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. - /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// public IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue, - ValidationMethods validation = ValidationMethods.None) + BindingPriority priority = BindingPriority.LocalValue) { Contract.Requires(property != null); VerifyAccess(); @@ -412,8 +410,7 @@ namespace Perspex .Subscribe(x => DirectBindingSet(property, x)); validationSubcription = source .OfType() - .Where(v => v.Match(validation)) - .Subscribe(x => ValidationChanged(property, x)); + .Subscribe(x => DataValidation(property, x)); s_directBindings.Add(subscription); @@ -447,7 +444,7 @@ namespace Perspex GetDescription(source), priority); - return v.Add(source, (int)priority, validation); + return v.Add(source, (int)priority); } } @@ -458,19 +455,17 @@ namespace Perspex /// The property. /// The observable. /// The priority of the binding. - /// The validation methods of the binding. /// /// A disposable which can be used to terminate the binding. /// public IDisposable Bind( PerspexProperty property, IObservable source, - BindingPriority priority = BindingPriority.LocalValue, - ValidationMethods validation = ValidationMethods.None) + BindingPriority priority = BindingPriority.LocalValue) { Contract.Requires(property != null); - return Bind((PerspexProperty)property, source.Select(x => (object)x), priority); + return Bind(property, source.Select(x => (object)x), priority); } /// @@ -517,13 +512,18 @@ namespace Perspex } /// - void IPriorityValueOwner.ValidationChanged(PriorityValue sender, ValidationStatus status) + void IPriorityValueOwner.DataValidationChanged(PriorityValue sender, ValidationStatus status) { var property = sender.Property; - ValidationChanged(property, status); + DataValidation(property, status); } - protected virtual void ValidationChanged(PerspexProperty property, ValidationStatus status) + /// + /// Called when the validation state on a tracked property is changed. + /// + /// The property whose validation state changed. + /// The new validation state. + protected virtual void DataValidation(PerspexProperty property, ValidationStatus status) { } diff --git a/src/Perspex.Base/PriorityBindingEntry.cs b/src/Perspex.Base/PriorityBindingEntry.cs index 4f923e3a9f..cc5cd66c2e 100644 --- a/src/Perspex.Base/PriorityBindingEntry.cs +++ b/src/Perspex.Base/PriorityBindingEntry.cs @@ -13,7 +13,6 @@ namespace Perspex { private PriorityLevel _owner; private IDisposable _subscription; - private ValidationMethods _validation; /// /// Initializes a new instance of the class. @@ -23,11 +22,10 @@ namespace Perspex /// The binding index. Later bindings should have higher indexes. /// /// The validation settings for the binding. - public PriorityBindingEntry(PriorityLevel owner, int index, ValidationMethods validation) + public PriorityBindingEntry(PriorityLevel owner, int index) { _owner = owner; Index = index; - _validation = validation; } /// @@ -106,10 +104,7 @@ namespace Perspex if (validationStatus != null) { - if (validationStatus.Match(_validation)) - { - _owner.Validation(this, validationStatus); - } + _owner.Validation(this, validationStatus); } else if (bindingError == null || bindingError.UseFallbackValue) { diff --git a/src/Perspex.Base/PriorityLevel.cs b/src/Perspex.Base/PriorityLevel.cs index 3049c793d9..2ab957f645 100644 --- a/src/Perspex.Base/PriorityLevel.cs +++ b/src/Perspex.Base/PriorityLevel.cs @@ -99,11 +99,11 @@ namespace Perspex /// The binding to add. /// Validation settings for the binding. /// A disposable used to remove the binding. - public IDisposable Add(IObservable binding, ValidationMethods validation) + public IDisposable Add(IObservable binding) { Contract.Requires(binding != null); - var entry = new PriorityBindingEntry(this, _nextIndex++, validation); + var entry = new PriorityBindingEntry(this, _nextIndex++); var node = Bindings.AddFirst(entry); entry.Start(binding); diff --git a/src/Perspex.Base/PriorityValue.cs b/src/Perspex.Base/PriorityValue.cs index 155fa47c06..c0e855654c 100644 --- a/src/Perspex.Base/PriorityValue.cs +++ b/src/Perspex.Base/PriorityValue.cs @@ -81,9 +81,9 @@ namespace Perspex /// /// A disposable that will remove the binding. /// - public IDisposable Add(IObservable binding, int priority, ValidationMethods validation = ValidationMethods.None) + public IDisposable Add(IObservable binding, int priority) { - return GetLevel(priority).Add(binding, validation); + return GetLevel(priority).Add(binding); } /// @@ -186,7 +186,7 @@ namespace Perspex /// The validation status. public void LevelValidation(PriorityLevel priorityLevel, ValidationStatus validationStatus) { - _owner.ValidationChanged(this, validationStatus); + _owner.DataValidationChanged(this, validationStatus); } /// diff --git a/src/Perspex.Controls/Control.cs b/src/Perspex.Controls/Control.cs index c4a99d8119..f59811b79c 100644 --- a/src/Perspex.Controls/Control.cs +++ b/src/Perspex.Controls/Control.cs @@ -423,9 +423,10 @@ namespace Perspex.Controls } } - protected override void ValidationChanged(PerspexProperty property, ValidationStatus status) + /// + protected override void DataValidation(PerspexProperty property, ValidationStatus status) { - base.ValidationChanged(property, status); + base.DataValidation(property, status); ValidationStatus = status; } diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs index 9cfcac0825..1979d5d9bf 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs +++ b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_TemplatedParent.cs @@ -32,8 +32,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Verify(x => x.Bind( TextBox.TextProperty, It.IsAny>(), - BindingPriority.TemplatedParent, - ValidationMethods.None)); + BindingPriority.TemplatedParent)); } [Fact] @@ -53,8 +52,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Verify(x => x.Bind( TextBox.TextProperty, It.IsAny>(), - BindingPriority.TemplatedParent, - ValidationMethods.None)); + BindingPriority.TemplatedParent)); } private Mock CreateTarget( diff --git a/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs b/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs index b6cb7a0d91..465610b045 100644 --- a/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs +++ b/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs @@ -129,7 +129,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority, ValidationMethods validation) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority) { throw new NotImplementedException(); } @@ -139,7 +139,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) { throw new NotImplementedException(); } diff --git a/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs b/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs index 83f0672a6f..62c460403b 100644 --- a/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs +++ b/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs @@ -160,7 +160,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) { throw new NotImplementedException(); } @@ -170,7 +170,7 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) { throw new NotImplementedException(); } diff --git a/tests/Perspex.Styling.UnitTests/TestControlBase.cs b/tests/Perspex.Styling.UnitTests/TestControlBase.cs index ac613ac75e..ba9646d5f4 100644 --- a/tests/Perspex.Styling.UnitTests/TestControlBase.cs +++ b/tests/Perspex.Styling.UnitTests/TestControlBase.cs @@ -62,12 +62,12 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) { throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) { throw new NotImplementedException(); } diff --git a/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs b/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs index 156fdfa035..b67cfd79a4 100644 --- a/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs +++ b/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs @@ -57,12 +57,12 @@ namespace Perspex.Styling.UnitTests throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) { throw new NotImplementedException(); } - public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue, ValidationMethods validation = ValidationMethods.None) + public IDisposable Bind(PerspexProperty property, IObservable source, BindingPriority priority = BindingPriority.LocalValue) { throw new NotImplementedException(); } From aab4331a52e209ec48fd1ed796b224399efeda6e Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 19 Apr 2016 16:40:31 -0500 Subject: [PATCH 11/12] Changed validation on controls to allow notifications from multiple types of validation and correctly report if it is valid. Changed initalization of validation plugins to nest within each other so there can be multiple active at once. --- .../Perspex.Markup/Data/ExpressionNode.cs | 10 ++++++- .../Data/PropertyAccessorNode.cs | 8 +++--- src/Perspex.Controls/Control.cs | 10 +++---- .../ControlValidationStatus.cs | 27 +++++++++++++++++++ src/Perspex.Controls/Perspex.Controls.csproj | 1 + .../Perspex.Controls.UnitTests.csproj | 3 --- .../Data/BindingTests_Validation.cs | 9 +++---- 7 files changed, 51 insertions(+), 17 deletions(-) create mode 100644 src/Perspex.Controls/ControlValidationStatus.cs diff --git a/src/Markup/Perspex.Markup/Data/ExpressionNode.cs b/src/Markup/Perspex.Markup/Data/ExpressionNode.cs index a120aa8901..ac1cad5b61 100644 --- a/src/Markup/Perspex.Markup/Data/ExpressionNode.cs +++ b/src/Markup/Perspex.Markup/Data/ExpressionNode.cs @@ -107,7 +107,15 @@ namespace Perspex.Markup.Data protected virtual void SendValidationStatus(ValidationStatus status) { - _subject?.OnNext(status); + //Even if elements only bound to sub-values, send validation changes along so they will be surfaced to the UI level. + if (_subject != null) + { + _subject.OnNext(status); + } + else + { + Next.SendValidationStatus(status); + } } protected virtual void Unsubscribe(object target) diff --git a/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs b/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs index 72cf80e79e..e0561ecfcf 100644 --- a/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs +++ b/src/Markup/Perspex.Markup/Data/PropertyAccessorNode.cs @@ -55,10 +55,12 @@ namespace Perspex.Markup.Data if (accessorPlugin != null) { _accessor = accessorPlugin.Start(reference, PropertyName, SetCurrentValue); - var validationPlugin = ExpressionObserver.ValidationCheckers.FirstOrDefault(x => x.Match(reference)); - if (validationPlugin != null) + foreach (var validationPlugin in ExpressionObserver.ValidationCheckers.Where(x => x.Match(reference))) { - _accessor = validationPlugin.Start(reference, PropertyName, _accessor, SendValidationStatus); + if (validationPlugin != null) + { + _accessor = validationPlugin.Start(reference, PropertyName, _accessor, SendValidationStatus); + } } if (_accessor != null) diff --git a/src/Perspex.Controls/Control.cs b/src/Perspex.Controls/Control.cs index f59811b79c..601975a19e 100644 --- a/src/Perspex.Controls/Control.cs +++ b/src/Perspex.Controls/Control.cs @@ -89,8 +89,8 @@ namespace Perspex.Controls /// /// Defines the property. /// - public static readonly DirectProperty ValidationStatusProperty = - PerspexProperty.RegisterDirect(nameof(ValidationStatus), c=> c.ValidationStatus); + public static readonly DirectProperty ValidationStatusProperty = + PerspexProperty.RegisterDirect(nameof(ValidationStatus), c=> c.ValidationStatus); private int _initCount; private string _name; @@ -406,12 +406,12 @@ namespace Perspex.Controls /// protected IPseudoClasses PseudoClasses => Classes; - private ValidationStatus validationStatus; + private ControlValidationStatus validationStatus = new ControlValidationStatus(); /// /// The current validation status of the control. /// - public ValidationStatus ValidationStatus + public ControlValidationStatus ValidationStatus { get { @@ -427,7 +427,7 @@ namespace Perspex.Controls protected override void DataValidation(PerspexProperty property, ValidationStatus status) { base.DataValidation(property, status); - ValidationStatus = status; + ValidationStatus.UpdateValidationStatus(status); } /// diff --git a/src/Perspex.Controls/ControlValidationStatus.cs b/src/Perspex.Controls/ControlValidationStatus.cs new file mode 100644 index 0000000000..aad65a1a0d --- /dev/null +++ b/src/Perspex.Controls/ControlValidationStatus.cs @@ -0,0 +1,27 @@ +using Perspex.Data; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Perspex.Controls +{ + public class ControlValidationStatus : ValidationStatus, INotifyPropertyChanged + { + private Dictionary propertyValidation = new Dictionary(); + + public override bool IsValid => propertyValidation.Values.All(status => status.IsValid); + + public event PropertyChangedEventHandler PropertyChanged; + + public override bool Match(ValidationMethods enabledMethods) => true; + + public void UpdateValidationStatus(ValidationStatus status) + { + propertyValidation[status.GetType()] = status; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs("")); + } + } +} diff --git a/src/Perspex.Controls/Perspex.Controls.csproj b/src/Perspex.Controls/Perspex.Controls.csproj index 8c62f1d66c..3bf7b4be9c 100644 --- a/src/Perspex.Controls/Perspex.Controls.csproj +++ b/src/Perspex.Controls/Perspex.Controls.csproj @@ -46,6 +46,7 @@ + diff --git a/tests/Perspex.Controls.UnitTests/Perspex.Controls.UnitTests.csproj b/tests/Perspex.Controls.UnitTests/Perspex.Controls.UnitTests.csproj index 932af42724..47635bf006 100644 --- a/tests/Perspex.Controls.UnitTests/Perspex.Controls.UnitTests.csproj +++ b/tests/Perspex.Controls.UnitTests/Perspex.Controls.UnitTests.csproj @@ -196,9 +196,6 @@ - - - diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs index 799486bd5f..551f930406 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs +++ b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests_Validation.cs @@ -13,7 +13,6 @@ namespace Perspex.Markup.Xaml.UnitTests.Data { public class BindingTests_Validation { - public class Data : INotifyPropertyChanged { private string mustbeNonEmpty; @@ -54,7 +53,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Text = ""; - Assert.Null(target.ValidationStatus); + Assert.True(target.ValidationStatus.IsValid); } [Fact] @@ -71,7 +70,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Bind(TextBlock.TextProperty, binding); target.Text = ""; - Assert.NotNull(target.ValidationStatus); + Assert.False(target.ValidationStatus.IsValid); } [Fact] @@ -89,7 +88,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Tag = ""; - Assert.Null(target.ValidationStatus); + Assert.True(target.ValidationStatus.IsValid); } [Fact] @@ -106,7 +105,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data target.Bind(Control.TagProperty, binding); target.Tag = ""; - Assert.NotNull(target.ValidationStatus); + Assert.False(target.ValidationStatus.IsValid); } } } From 287cc3c22e58c58ab2564cf37aaa20458beb16a2 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Apr 2016 10:05:26 -0500 Subject: [PATCH 12/12] Fixes failing tests for when an ExpressionSubject/ExpressionObserver is never subscribed to. --- src/Markup/Perspex.Markup/Data/ExpressionNode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Markup/Perspex.Markup/Data/ExpressionNode.cs b/src/Markup/Perspex.Markup/Data/ExpressionNode.cs index ac1cad5b61..b68c1ed426 100644 --- a/src/Markup/Perspex.Markup/Data/ExpressionNode.cs +++ b/src/Markup/Perspex.Markup/Data/ExpressionNode.cs @@ -114,7 +114,7 @@ namespace Perspex.Markup.Data } else { - Next.SendValidationStatus(status); + Next?.SendValidationStatus(status); } }