From fdb0f17cece37769384cb1d1ed5bea94a1c7d540 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 7 Oct 2020 00:55:34 +0200 Subject: [PATCH 01/20] Added new ObservableObject.OnProperty... overloads --- .../ComponentModel/ObservableObject.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs index f1c9b8989b5..51139c17a84 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs @@ -26,6 +26,26 @@ public abstract class ObservableObject : INotifyPropertyChanged, INotifyProperty /// public event PropertyChangingEventHandler? PropertyChanging; + /// + /// Performs the required configuration when a property has changed, and then + /// raises the event to notify listeners of the update. + /// + /// The input instance. + protected void OnPropertyChanged(PropertyChangedEventArgs args) + { + PropertyChanged?.Invoke(this, args); + } + + /// + /// Performs the required configuration when a property is changing, and then + /// raises the event to notify listeners of the update. + /// + /// The input instance. + protected void OnPropertyChanging(PropertyChangingEventArgs args) + { + PropertyChanging?.Invoke(this, args); + } + /// /// Performs the required configuration when a property has changed, and then /// raises the event to notify listeners of the update. From 30ce3216c71f9164b305fdefc9d01c4ebf3461d3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 7 Oct 2020 01:07:11 +0200 Subject: [PATCH 02/20] Added notification to AsyncRelayCommand.CanBeCanceled --- Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs | 11 +++++++++-- Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs index d7edbca8e05..130441dcc87 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs @@ -91,15 +91,22 @@ public Task? ExecutionTask get => this.executionTask; private set { - if (SetPropertyAndNotifyOnCompletion(ref this.executionTask, value, _ => OnPropertyChanged(nameof(IsRunning)))) + if (SetPropertyAndNotifyOnCompletion(ref this.executionTask, value, _ => { + // When the task completes OnPropertyChanged(nameof(IsRunning)); + OnPropertyChanged(nameof(CanBeCanceled)); + })) + { + // When setting the task + OnPropertyChanged(nameof(IsRunning)); + OnPropertyChanged(nameof(CanBeCanceled)); } } } /// - public bool CanBeCanceled => !(this.cancelableExecute is null); + public bool CanBeCanceled => !(this.cancelableExecute is null) && IsRunning; /// public bool IsCancellationRequested => this.cancellationTokenSource?.IsCancellationRequested == true; diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index 1acf8e72a23..f818adb6a9b 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -91,15 +91,22 @@ public Task? ExecutionTask get => this.executionTask; private set { - if (SetPropertyAndNotifyOnCompletion(ref this.executionTask, value, _ => OnPropertyChanged(nameof(IsRunning)))) + if (SetPropertyAndNotifyOnCompletion(ref this.executionTask, value, _ => { + // When the task completes OnPropertyChanged(nameof(IsRunning)); + OnPropertyChanged(nameof(CanBeCanceled)); + })) + { + // When setting the task + OnPropertyChanged(nameof(IsRunning)); + OnPropertyChanged(nameof(CanBeCanceled)); } } } /// - public bool CanBeCanceled => !(this.cancelableExecute is null); + public bool CanBeCanceled => !(this.cancelableExecute is null) && IsRunning; /// public bool IsCancellationRequested => this.cancellationTokenSource?.IsCancellationRequested == true; From 634e243809b964700d736bbb9bae1f7b8efa464a Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 7 Oct 2020 01:12:31 +0200 Subject: [PATCH 03/20] Added cached args to async relay commands --- .../Input/AsyncRelayCommand.cs | 28 +++++++++++++++---- .../Input/AsyncRelayCommand{T}.cs | 12 ++++---- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs index 130441dcc87..94132a5069c 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.ComponentModel; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -18,6 +19,21 @@ namespace Microsoft.Toolkit.Mvvm.Input /// public sealed class AsyncRelayCommand : ObservableObject, IAsyncRelayCommand { + /// + /// The cached for . + /// + internal static readonly PropertyChangedEventArgs CanBeCanceledChangedEventArgs = new PropertyChangedEventArgs(nameof(CanBeCanceled)); + + /// + /// The cached for . + /// + internal static readonly PropertyChangedEventArgs IsCancellationRequestedChangedEventArgs = new PropertyChangedEventArgs(nameof(IsCancellationRequested)); + + /// + /// The cached for . + /// + internal static readonly PropertyChangedEventArgs IsRunningChangedEventArgs = new PropertyChangedEventArgs(nameof(IsRunning)); + /// /// The to invoke when is used. /// @@ -94,13 +110,13 @@ private set if (SetPropertyAndNotifyOnCompletion(ref this.executionTask, value, _ => { // When the task completes - OnPropertyChanged(nameof(IsRunning)); - OnPropertyChanged(nameof(CanBeCanceled)); + OnPropertyChanged(IsRunningChangedEventArgs); + OnPropertyChanged(CanBeCanceledChangedEventArgs); })) { // When setting the task - OnPropertyChanged(nameof(IsRunning)); - OnPropertyChanged(nameof(CanBeCanceled)); + OnPropertyChanged(IsRunningChangedEventArgs); + OnPropertyChanged(CanBeCanceledChangedEventArgs); } } } @@ -149,7 +165,7 @@ public Task ExecuteAsync(object? parameter) var cancellationTokenSource = this.cancellationTokenSource = new CancellationTokenSource(); - OnPropertyChanged(nameof(IsCancellationRequested)); + OnPropertyChanged(IsCancellationRequestedChangedEventArgs); // Invoke the cancelable command delegate with a new linked token return ExecutionTask = this.cancelableExecute!(cancellationTokenSource.Token); @@ -163,7 +179,7 @@ public void Cancel() { this.cancellationTokenSource?.Cancel(); - OnPropertyChanged(nameof(IsCancellationRequested)); + OnPropertyChanged(IsCancellationRequestedChangedEventArgs); } } } \ No newline at end of file diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index f818adb6a9b..18f9f4efa7c 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -94,13 +94,13 @@ private set if (SetPropertyAndNotifyOnCompletion(ref this.executionTask, value, _ => { // When the task completes - OnPropertyChanged(nameof(IsRunning)); - OnPropertyChanged(nameof(CanBeCanceled)); + OnPropertyChanged(AsyncRelayCommand.IsRunningChangedEventArgs); + OnPropertyChanged(AsyncRelayCommand.CanBeCanceledChangedEventArgs); })) { // When setting the task - OnPropertyChanged(nameof(IsRunning)); - OnPropertyChanged(nameof(CanBeCanceled)); + OnPropertyChanged(AsyncRelayCommand.IsRunningChangedEventArgs); + OnPropertyChanged(AsyncRelayCommand.CanBeCanceledChangedEventArgs); } } } @@ -170,7 +170,7 @@ public Task ExecuteAsync(T parameter) var cancellationTokenSource = this.cancellationTokenSource = new CancellationTokenSource(); - OnPropertyChanged(nameof(IsCancellationRequested)); + OnPropertyChanged(AsyncRelayCommand.IsCancellationRequestedChangedEventArgs); // Invoke the cancelable command delegate with a new linked token return ExecutionTask = this.cancelableExecute!(parameter, cancellationTokenSource.Token); @@ -190,7 +190,7 @@ public void Cancel() { this.cancellationTokenSource?.Cancel(); - OnPropertyChanged(nameof(IsCancellationRequested)); + OnPropertyChanged(AsyncRelayCommand.IsCancellationRequestedChangedEventArgs); } } } \ No newline at end of file From 599c76e09600e499afd576cd398e3db7b6ecd39f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 8 Oct 2020 00:01:46 +0200 Subject: [PATCH 04/20] Fixed unit test, added missing notification --- .../Input/AsyncRelayCommand.cs | 1 + .../Mvvm/Test_AsyncRelayCommand.cs | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs index 94132a5069c..e9678946137 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs @@ -180,6 +180,7 @@ public void Cancel() this.cancellationTokenSource?.Cancel(); OnPropertyChanged(IsCancellationRequestedChangedEventArgs); + OnPropertyChanged(CanBeCanceledChangedEventArgs); } } } \ No newline at end of file diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs index 09ff1d35a90..f144a40e24b 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; +using System.ComponentModel; using System.Threading.Tasks; using Microsoft.Toolkit.Mvvm.Input; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -126,24 +128,39 @@ public async Task Test_AsyncRelayCommand_WithCancellation() var command = new AsyncRelayCommand(token => tcs.Task); + List args = new List(); + + command.PropertyChanged += (s, e) => args.Add(e); + Assert.IsTrue(command.CanExecute(null)); Assert.IsTrue(command.CanExecute(new object())); - Assert.IsTrue(command.CanBeCanceled); + Assert.IsFalse(command.CanBeCanceled); Assert.IsFalse(command.IsCancellationRequested); command.Execute(null); + Assert.IsTrue(command.CanBeCanceled); Assert.IsFalse(command.IsCancellationRequested); + Assert.AreEqual(args.Count, 4); + Assert.AreEqual(args[0].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested)); + Assert.AreEqual(args[1].PropertyName, nameof(IAsyncRelayCommand.ExecutionTask)); + Assert.AreEqual(args[2].PropertyName, nameof(IAsyncRelayCommand.IsRunning)); + Assert.AreEqual(args[3].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled)); command.Cancel(); + Assert.AreEqual(args.Count, 6); + Assert.AreEqual(args[4].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested)); + Assert.AreEqual(args[5].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled)); + Assert.IsTrue(command.IsCancellationRequested); tcs.SetResult(null); await command.ExecutionTask!; + Assert.IsFalse(command.CanBeCanceled); Assert.IsTrue(command.IsCancellationRequested); } } From db2742abc81f91a2566bc0f938e0d24a292fcc39 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 8 Oct 2020 00:02:59 +0200 Subject: [PATCH 05/20] Fixed another missing property change notification --- Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index 18f9f4efa7c..61f1962b1d7 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -191,6 +191,7 @@ public void Cancel() this.cancellationTokenSource?.Cancel(); OnPropertyChanged(AsyncRelayCommand.IsCancellationRequestedChangedEventArgs); + OnPropertyChanged(AsyncRelayCommand.CanBeCanceledChangedEventArgs); } } } \ No newline at end of file From 7464bd004e57c62b3cc12693a5a3d728a11a9abe Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 8 Oct 2020 00:24:03 +0200 Subject: [PATCH 06/20] Enabled notification for ObservableValidator.HasErrors --- .../ComponentModel/ObservableValidator.cs | 59 +++++++++++++------ .../Mvvm/Test_ObservableValidator.cs | 5 ++ 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 2a96ab7b837..4b43431d81e 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -19,33 +19,28 @@ namespace Microsoft.Toolkit.Mvvm.ComponentModel /// public abstract class ObservableValidator : ObservableObject, INotifyDataErrorInfo { + /// + /// The cached for . + /// + private static readonly PropertyChangedEventArgs HasErrorsChangedEventArgs = new PropertyChangedEventArgs(nameof(HasErrors)); + /// /// The instance used to store previous validation results. /// private readonly Dictionary> errors = new Dictionary>(); + /// + /// Indicates the total number of properties with errors (not total errors). + /// This is used to allow to operate in O(1) time, as it can just + /// check whether this value is not 0 instead of having to traverse . + /// + private int totalErrors; + /// public event EventHandler? ErrorsChanged; /// - public bool HasErrors - { - get - { - // This uses the value enumerator for Dictionary.ValueCollection, so it doesn't - // allocate. Accessing this property is O(n), but we can stop as soon as we find at least one - // error in the whole entity, and doing this saves 8 bytes in the object size (no fields needed). - foreach (var value in this.errors.Values) - { - if (value.Count > 0) - { - return true; - } - } - - return false; - } - } + public bool HasErrors => this.totalErrors > 0; /// /// Compares the current and new values for a given property. If the value has changed, @@ -285,6 +280,34 @@ private void ValidateProperty(object? value, string? propertyName) new ValidationContext(this, null, null) { MemberName = propertyName }, propertyErrors); + // Update the shared counter for the number of errors, and raise the + // property changed event if necessary. We decrement the number of total + // errors if the current property is valid but it wasn't so before this + // validation, and we increment it if the validation failed after being + // correct before. The property changed event is raised whenever the + // number of total errors is either decremented to 0, or incremented to 1. + if (isValid) + { + if (errorsChanged) + { + this.totalErrors--; + + if (this.totalErrors == 0) + { + OnPropertyChanged(HasErrorsChangedEventArgs); + } + } + } + else if (!errorsChanged) + { + this.totalErrors++; + + if (this.totalErrors == 1) + { + OnPropertyChanged(HasErrorsChangedEventArgs); + } + } + // Only raise the event once if needed. This happens either when the target property // had existing errors and is now valid, or if the validation has failed and there are // new errors to broadcast, regardless of the previous validation state for the property. diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index d5f6e243f40..0e67376646b 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -19,16 +19,21 @@ public class Test_ObservableValidator public void Test_ObservableValidator_HasErrors() { var model = new Person(); + var args = new List(); Assert.IsFalse(model.HasErrors); model.Name = "No"; Assert.IsTrue(model.HasErrors); + Assert.AreEqual(args.Count, 1); + Assert.AreEqual(args[0].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); model.Name = "Valid"; Assert.IsFalse(model.HasErrors); + Assert.AreEqual(args.Count, 2); + Assert.AreEqual(args[2].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); } [TestCategory("Mvvm")] From 2f71694ff02150a5da5480ab2d95456dc5bfe1c9 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 8 Oct 2020 01:24:26 +0200 Subject: [PATCH 07/20] Fixed ObservableValidator.HasErrors unit test --- .../UnitTests.Shared/Mvvm/Test_ObservableValidator.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 0e67376646b..3f4f2c4aae6 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -21,19 +21,23 @@ public void Test_ObservableValidator_HasErrors() var model = new Person(); var args = new List(); + model.PropertyChanged += (s, e) => args.Add(e); + Assert.IsFalse(model.HasErrors); model.Name = "No"; Assert.IsTrue(model.HasErrors); - Assert.AreEqual(args.Count, 1); + Assert.AreEqual(args.Count, 2); Assert.AreEqual(args[0].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); + Assert.AreEqual(args[1].PropertyName, nameof(Person.Name)); model.Name = "Valid"; Assert.IsFalse(model.HasErrors); - Assert.AreEqual(args.Count, 2); + Assert.AreEqual(args.Count, 4); Assert.AreEqual(args[2].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); + Assert.AreEqual(args[3].PropertyName, nameof(Person.Name)); } [TestCategory("Mvvm")] From 78991fe2a86cf71348b43899d8c6abb927fdef9d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 9 Oct 2020 18:47:55 +0200 Subject: [PATCH 08/20] Tweaks to the OnPropertyChanging overloads --- .../ComponentModel/ObservableObject.cs | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs index 51139c17a84..aa314a6e4c6 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs @@ -27,42 +27,36 @@ public abstract class ObservableObject : INotifyPropertyChanged, INotifyProperty public event PropertyChangingEventHandler? PropertyChanging; /// - /// Performs the required configuration when a property has changed, and then - /// raises the event to notify listeners of the update. + /// Raises the event. /// - /// The input instance. - protected void OnPropertyChanged(PropertyChangedEventArgs args) + /// The input instance. + protected virtual void OnPropertyChanged(PropertyChangedEventArgs e) { - PropertyChanged?.Invoke(this, args); + PropertyChanged?.Invoke(this, e); } /// - /// Performs the required configuration when a property is changing, and then - /// raises the event to notify listeners of the update. + /// Raises the event. /// - /// The input instance. - protected void OnPropertyChanging(PropertyChangingEventArgs args) + /// The input instance. + protected virtual void OnPropertyChanging(PropertyChangingEventArgs e) { - PropertyChanging?.Invoke(this, args); + PropertyChanging?.Invoke(this, e); } /// - /// Performs the required configuration when a property has changed, and then - /// raises the event to notify listeners of the update. + /// Raises the event. /// /// (optional) The name of the property that changed. - /// The base implementation only raises the event. protected virtual void OnPropertyChanged([CallerMemberName] string? propertyName = null) { PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); } /// - /// Performs the required configuration when a property is changing, and then - /// raises the event to notify listeners of the update. + /// Raises the event. /// /// (optional) The name of the property that changed. - /// The base implementation only raises the event. protected virtual void OnPropertyChanging([CallerMemberName] string? propertyName = null) { PropertyChanging?.Invoke(this, new PropertyChangingEventArgs(propertyName)); From 2ceb110a2a66fbf322bbf847d3d08630ebe446df Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 9 Oct 2020 18:58:45 +0200 Subject: [PATCH 09/20] Improved OnPropertyChanging overloads --- Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs index aa314a6e4c6..cd7fec0634c 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs @@ -48,18 +48,18 @@ protected virtual void OnPropertyChanging(PropertyChangingEventArgs e) /// Raises the event. /// /// (optional) The name of the property that changed. - protected virtual void OnPropertyChanged([CallerMemberName] string? propertyName = null) + protected void OnPropertyChanged([CallerMemberName] string? propertyName = null) { - PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + OnPropertyChanged(new PropertyChangedEventArgs(propertyName)); } /// /// Raises the event. /// /// (optional) The name of the property that changed. - protected virtual void OnPropertyChanging([CallerMemberName] string? propertyName = null) + protected void OnPropertyChanging([CallerMemberName] string? propertyName = null) { - PropertyChanging?.Invoke(this, new PropertyChangingEventArgs(propertyName)); + OnPropertyChanging(new PropertyChangingEventArgs(propertyName)); } /// From 66825f9103b8f095d409c577bb060435078be357 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 12 Oct 2020 11:26:58 +0200 Subject: [PATCH 10/20] Reintroduced simplified Ioc class --- .../DependencyInjection/Ioc.cs | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs diff --git a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs new file mode 100644 index 00000000000..65b32026026 --- /dev/null +++ b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs @@ -0,0 +1,111 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading; + +#nullable enable + +namespace Microsoft.Toolkit.Mvvm.DependencyInjection +{ + /// + /// A type that facilitates the use of the type. + /// The provides the ability to configure services in a singleton, thread-safe + /// service provider instance, which can then be used to resolve service instances. + /// The first step to use this feature is to declare some services, for instance: + /// + /// public interface ILogger + /// { + /// void Log(string text); + /// } + /// + /// + /// public class ConsoleLogger : ILogger + /// { + /// void Log(string text) => Console.WriteLine(text); + /// } + /// + /// Then the services configuration should then be done at startup, by calling the + /// method and passing an instance with the services to use. That instance can + /// be from any library offering dependency injection functionality, such as Microsoft.Extensions.DependencyInjection. + /// For instance, using that library, can be used as follows in this example: + /// + /// Ioc.Default.ConfigureServices( + /// new ServiceCollection() + /// .AddSingleton<ILogger, Logger>() + /// .BuildServiceProvider()); + /// + /// Finally, you can use the instance (which implements ) + /// to retrieve the service instances from anywhere in your application, by doing as follows: + /// + /// Ioc.Default.GetService<ILogger>().Log("Hello world!"); + /// + /// + public sealed class Ioc : IServiceProvider + { + /// + /// Gets the default instance. + /// + public static Ioc Default { get; } = new Ioc(); + + /// + /// The instance to use, if initialized. + /// + private volatile IServiceProvider? serviceProvider; + + /// + public object? GetService(Type serviceType) + { + // As per section I.12.6.6 of the official CLI ECMA-335 spec: + // "[...] read and write access to properly aligned memory locations no larger than the native + // word size is atomic when all the write accesses to a location are the same size. Atomic writes + // shall alter no bits other than those written. Unless explicit layout control is used [...], + // data elements no larger than the natural word size [...] shall be properly aligned. + // Object references shall be treated as though they are stored in the native word size." + // The field being accessed here is of native int size (reference type), and is only ever accessed + // directly and atomically by a compare exchange instruction (see below), or here. We can therefore + // assume this read is thread safe with respect to accesses to this property or to invocations to one + // of the available configuration methods. So we can just read the field directly and make the necessary + // check with our local copy, without the need of paying the locking overhead from this get accessor. + IServiceProvider? provider = this.serviceProvider; + + if (provider is null) + { + ThrowInvalidOperationExceptionForMissingInitialization(); + } + + return provider!.GetService(serviceType); + } + + /// + /// Initializes the shared instance. + /// + /// The input instance to use. + public void ConfigureServices(IServiceProvider serviceProvider) + { + IServiceProvider? oldServices = Interlocked.CompareExchange(ref this.serviceProvider, serviceProvider, null); + + if (!(oldServices is null)) + { + ThrowInvalidOperationExceptionForRepeatedConfiguration(); + } + } + + /// + /// Throws an when the property is used before initialization. + /// + private static void ThrowInvalidOperationExceptionForMissingInitialization() + { + throw new InvalidOperationException("The service provider has not been configured yet"); + } + + /// + /// Throws an when a configuration is attempted more than once. + /// + private static void ThrowInvalidOperationExceptionForRepeatedConfiguration() + { + throw new InvalidOperationException("The default service provider has already been configured"); + } + } +} \ No newline at end of file From fb17f618ced8a9fd8fe9440e082c21002813a49f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 12 Oct 2020 11:27:14 +0200 Subject: [PATCH 11/20] Added generic Ioc.GetService method --- .../DependencyInjection/Ioc.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs index 65b32026026..c8a535d8c98 100644 --- a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs +++ b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs @@ -78,6 +78,24 @@ public sealed class Ioc : IServiceProvider return provider!.GetService(serviceType); } + /// + /// Tries to resolve an instance of a specified service type. + /// + /// The type of service to resolve. + /// An instance of the specified service, or . + public T? GetService() + where T : class + { + IServiceProvider? provider = this.serviceProvider; + + if (provider is null) + { + ThrowInvalidOperationExceptionForMissingInitialization(); + } + + return (T?)provider!.GetService(typeof(T)); + } + /// /// Initializes the shared instance. /// From 727e828cbab09bdfc57bb0985478f3015d47afd0 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 20 Oct 2020 19:36:22 +0200 Subject: [PATCH 12/20] Minor code refactoring and performance tweak --- .../Messaging/IMessengerExtensions.cs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs index 72caa9fb7dd..ef8c7612701 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs @@ -27,27 +27,10 @@ public static class IMessengerExtensions /// private static class MethodInfos { - /// - /// Initializes static members of the class. - /// - static MethodInfos() - { - RegisterIRecipient = ( - from methodInfo in typeof(IMessengerExtensions).GetMethods() - where methodInfo.Name == nameof(Register) && - methodInfo.IsGenericMethod && - methodInfo.GetGenericArguments().Length == 2 - let parameters = methodInfo.GetParameters() - where parameters.Length == 3 && - parameters[1].ParameterType.IsGenericType && - parameters[1].ParameterType.GetGenericTypeDefinition() == typeof(IRecipient<>) - select methodInfo).First(); - } - /// /// The instance associated with . /// - public static readonly MethodInfo RegisterIRecipient; + public static readonly MethodInfo RegisterIRecipient = new Action, Unit>(Register).Method.GetGenericMethodDefinition(); } /// From bbe8dfd475506d4397a613d7c5f9af43a3708e4b Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 30 Oct 2020 15:53:39 +0100 Subject: [PATCH 13/20] Added comments to unit tests --- .../Mvvm/Test_AsyncRelayCommand.cs | 16 ++++++++++++++++ .../Mvvm/Test_ObservableValidator.cs | 3 +++ 2 files changed, 19 insertions(+) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs index f144a40e24b..85d40de0505 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand.cs @@ -126,22 +126,33 @@ public async Task Test_AsyncRelayCommand_WithCancellation() { TaskCompletionSource tcs = new TaskCompletionSource(); + // We need to test the cancellation support here, so we use the overload with an input + // parameter, which is a cancellation token. The token is the one that is internally managed + // by the AsyncRelayCommand instance, and canceled when using IAsyncRelayCommand.Cancel(). var command = new AsyncRelayCommand(token => tcs.Task); List args = new List(); command.PropertyChanged += (s, e) => args.Add(e); + // We have no canExecute parameter, so the command can always be invoked Assert.IsTrue(command.CanExecute(null)); Assert.IsTrue(command.CanExecute(new object())); + // The command isn't running, so it can't be canceled yet Assert.IsFalse(command.CanBeCanceled); Assert.IsFalse(command.IsCancellationRequested); + // Start the command, which will return the token from our task completion source. + // We can use that to easily keep the command running while we do our tests, and then + // stop the processing by completing the source when we need (see below). command.Execute(null); + // The command is running, so it can be canceled, as we used the token overload Assert.IsTrue(command.CanBeCanceled); Assert.IsFalse(command.IsCancellationRequested); + + // Validate the various event args for all the properties that were updated when executing the command Assert.AreEqual(args.Count, 4); Assert.AreEqual(args[0].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested)); Assert.AreEqual(args[1].PropertyName, nameof(IAsyncRelayCommand.ExecutionTask)); @@ -150,16 +161,21 @@ public async Task Test_AsyncRelayCommand_WithCancellation() command.Cancel(); + // Verify that these two properties raised notifications correctly when canceling the command too. + // We need to ensure all command properties support notifications so that users can bind to them. Assert.AreEqual(args.Count, 6); Assert.AreEqual(args[4].PropertyName, nameof(IAsyncRelayCommand.IsCancellationRequested)); Assert.AreEqual(args[5].PropertyName, nameof(IAsyncRelayCommand.CanBeCanceled)); Assert.IsTrue(command.IsCancellationRequested); + // Complete the source, which will mark the command as completed too (as it returned the same task) tcs.SetResult(null); await command.ExecutionTask!; + // Verify that the command can no longer be canceled, and that the cancellation is + // instead still true, as that's reset when executing a command and not on completion. Assert.IsFalse(command.CanBeCanceled); Assert.IsTrue(command.IsCancellationRequested); } diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 3f4f2c4aae6..38da3e76aee 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -27,6 +27,9 @@ public void Test_ObservableValidator_HasErrors() model.Name = "No"; + // Verify that errors were correctly reported as changed, and that all the relevant + // properties were broadcast as well (both the changed property and HasErrors). We need + // this last one to raise notifications too so that users can bind to that in the UI. Assert.IsTrue(model.HasErrors); Assert.AreEqual(args.Count, 2); Assert.AreEqual(args[0].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); From ee1d7688b4c7fc15f810d12c9301034cdb55401d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 9 Nov 2020 00:21:39 +0100 Subject: [PATCH 14/20] Fixed a small bug in Type2.GetHashCode --- .../Messaging/Internals/Type2.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs index 411b8809ba6..f6f203a37d6 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Type2.cs @@ -65,19 +65,19 @@ public override bool Equals(object? obj) [MethodImpl(MethodImplOptions.AggressiveInlining)] public override int GetHashCode() { - unchecked - { - // To combine the two hashes, we can simply use the fast djb2 hash algorithm. - // This is not a problem in this case since we already know that the base - // RuntimeHelpers.GetHashCode method is providing hashes with a good enough distribution. - int hash = RuntimeHelpers.GetHashCode(TMessage); + // To combine the two hashes, we can simply use the fast djb2 hash algorithm. Unfortunately we + // can't really skip the callvirt here (eg. by using RuntimeHelpers.GetHashCode like in other + // cases), as there are some niche cases mentioned above that might break when doing so. + // However since this method is not generally used in a hot path (eg. the message broadcasting + // only invokes this a handful of times when initially retrieving the target mapping), this + // doesn't actually make a noticeable difference despite the minor overhead of the virtual call. + int hash = TMessage.GetHashCode(); - hash = (hash << 5) + hash; + hash = (hash << 5) + hash; - hash += RuntimeHelpers.GetHashCode(TToken); + hash += TToken.GetHashCode(); - return hash; - } + return hash; } } } From 07f1a27d85246c48639140c06b92a2487b4e63d1 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 9 Nov 2020 00:22:16 +0100 Subject: [PATCH 15/20] Minor code tweak --- Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs index a8e45b15c2e..dc414ffaeb0 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs @@ -371,8 +371,6 @@ public TMessage Send(TMessage message, TToken token) // that doesn't expose the single standard Current property. while (mappingEnumerator.MoveNext()) { - object recipient = mappingEnumerator.Key.Target; - // Pick the target handler, if the token is a match for the recipient if (mappingEnumerator.Value.TryGetValue(token, out object? handler)) { @@ -382,7 +380,7 @@ public TMessage Send(TMessage message, TToken token) // We're still using a checked span accesses here though to make sure an out of // bounds write can never happen even if an error was present in the logic above. pairs[2 * i] = handler!; - pairs[(2 * i) + 1] = recipient; + pairs[(2 * i) + 1] = mappingEnumerator.Key.Target; i++; } } From bab6d9a40b52c7678af8654f5313bf7e568ca83a Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 9 Nov 2020 15:08:04 +0100 Subject: [PATCH 16/20] Minor tweaks to ObservableValidator --- .../ComponentModel/ObservableValidator.cs | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 4b43431d81e..05bda3be72d 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -62,12 +62,14 @@ public abstract class ObservableValidator : ObservableObject, INotifyDataErrorIn /// protected bool SetProperty(ref T field, T newValue, bool validate, [CallerMemberName] string? propertyName = null) { - if (validate) + bool propertyChanged = SetProperty(ref field, newValue, propertyName); + + if (propertyChanged && validate) { ValidateProperty(newValue, propertyName); } - return SetProperty(ref field, newValue, propertyName); + return propertyChanged; } /// @@ -85,12 +87,14 @@ protected bool SetProperty(ref T field, T newValue, bool validate, [CallerMem /// if the property was changed, otherwise. protected bool SetProperty(ref T field, T newValue, IEqualityComparer comparer, bool validate, [CallerMemberName] string? propertyName = null) { - if (validate) + bool propertyChanged = SetProperty(ref field, newValue, comparer, propertyName); + + if (propertyChanged && validate) { ValidateProperty(newValue, propertyName); } - return SetProperty(ref field, newValue, comparer, propertyName); + return propertyChanged; } /// @@ -115,12 +119,14 @@ protected bool SetProperty(ref T field, T newValue, IEqualityComparer comp /// protected bool SetProperty(T oldValue, T newValue, Action callback, bool validate, [CallerMemberName] string? propertyName = null) { - if (validate) + bool propertyChanged = SetProperty(oldValue, newValue, callback, propertyName); + + if (propertyChanged && validate) { ValidateProperty(newValue, propertyName); } - return SetProperty(oldValue, newValue, callback, propertyName); + return propertyChanged; } /// @@ -139,12 +145,14 @@ protected bool SetProperty(T oldValue, T newValue, Action callback, bool v /// if the property was changed, otherwise. protected bool SetProperty(T oldValue, T newValue, IEqualityComparer comparer, Action callback, bool validate, [CallerMemberName] string? propertyName = null) { - if (validate) + bool propertyChanged = SetProperty(oldValue, newValue, comparer, callback, propertyName); + + if (propertyChanged && validate) { ValidateProperty(newValue, propertyName); } - return SetProperty(oldValue, newValue, comparer, callback, propertyName); + return propertyChanged; } /// @@ -167,12 +175,14 @@ protected bool SetProperty(T oldValue, T newValue, IEqualityComparer compa protected bool SetProperty(T oldValue, T newValue, TModel model, Action callback, bool validate, [CallerMemberName] string? propertyName = null) where TModel : class { - if (validate) + bool propertyChanged = SetProperty(oldValue, newValue, model, callback, propertyName); + + if (propertyChanged && validate) { ValidateProperty(newValue, propertyName); } - return SetProperty(oldValue, newValue, model, callback, propertyName); + return propertyChanged; } /// @@ -197,12 +207,14 @@ protected bool SetProperty(T oldValue, T newValue, TModel model, Acti protected bool SetProperty(T oldValue, T newValue, IEqualityComparer comparer, TModel model, Action callback, bool validate, [CallerMemberName] string? propertyName = null) where TModel : class { - if (validate) + bool propertyChanged = SetProperty(oldValue, newValue, comparer, model, callback, propertyName); + + if (propertyChanged && validate) { ValidateProperty(newValue, propertyName); } - return SetProperty(oldValue, newValue, comparer, model, callback, propertyName); + return propertyChanged; } /// From 2faf3ea3c9308372b340477e1e4bb1d1925b2aea Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 9 Nov 2020 18:08:14 +0100 Subject: [PATCH 17/20] Fixed unit tests --- .../UnitTests.Shared/Mvvm/Test_ObservableValidator.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 38da3e76aee..80e0bd204fd 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -32,15 +32,15 @@ public void Test_ObservableValidator_HasErrors() // this last one to raise notifications too so that users can bind to that in the UI. Assert.IsTrue(model.HasErrors); Assert.AreEqual(args.Count, 2); - Assert.AreEqual(args[0].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); - Assert.AreEqual(args[1].PropertyName, nameof(Person.Name)); + Assert.AreEqual(args[0].PropertyName, nameof(Person.Name)); + Assert.AreEqual(args[1].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); model.Name = "Valid"; Assert.IsFalse(model.HasErrors); Assert.AreEqual(args.Count, 4); - Assert.AreEqual(args[2].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); - Assert.AreEqual(args[3].PropertyName, nameof(Person.Name)); + Assert.AreEqual(args[2].PropertyName, nameof(Person.Name)); + Assert.AreEqual(args[3].PropertyName, nameof(INotifyDataErrorInfo.HasErrors)); } [TestCategory("Mvvm")] @@ -131,7 +131,6 @@ public void Test_ObservableValidator_GetErrors() [TestCategory("Mvvm")] [TestMethod] - [DataRow(null, false)] [DataRow("", false)] [DataRow("No", false)] [DataRow("This text is really, really too long for the target property", false)] From e4116ca43689a598a8f362213797aa1660ae3490 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 10 Nov 2020 18:13:04 +0100 Subject: [PATCH 18/20] Added Ioc.GetRequiredService method --- .../DependencyInjection/Ioc.cs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs index c8a535d8c98..b44819888ff 100644 --- a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs +++ b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs @@ -83,6 +83,7 @@ public sealed class Ioc : IServiceProvider /// /// The type of service to resolve. /// An instance of the specified service, or . + /// Throw if the current instance has not been initialized. public T? GetService() where T : class { @@ -96,6 +97,35 @@ public sealed class Ioc : IServiceProvider return (T?)provider!.GetService(typeof(T)); } + /// + /// Resolves an instance of a specified service type. + /// + /// The type of service to resolve. + /// An instance of the specified service, or . + /// + /// Throw if the current instance has not been initialized, or if the + /// requested service type was not registered in the service provider currently in use. + /// + public T GetRequiredService() + where T : class + { + IServiceProvider? provider = this.serviceProvider; + + if (provider is null) + { + ThrowInvalidOperationExceptionForMissingInitialization(); + } + + T? service = (T?)provider!.GetService(typeof(T)); + + if (service is null) + { + ThrowInvalidOperationExceptionForUnregisteredType(); + } + + return service!; + } + /// /// Initializes the shared instance. /// @@ -118,6 +148,14 @@ private static void ThrowInvalidOperationExceptionForMissingInitialization() throw new InvalidOperationException("The service provider has not been configured yet"); } + /// + /// Throws an when the property is missing a type registration. + /// + private static void ThrowInvalidOperationExceptionForUnregisteredType() + { + throw new InvalidOperationException("The requested service type was not registered"); + } + /// /// Throws an when a configuration is attempted more than once. /// From ffecfc47508e56a1620d4c0d79e5c719b920f1aa Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 11 Nov 2020 17:04:51 +0100 Subject: [PATCH 19/20] Added ObservableValidator.TrySetProperty methods --- .../ComponentModel/ObservableValidator.cs | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 05bda3be72d..e990fcc8b1d 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -217,6 +217,115 @@ protected bool SetProperty(T oldValue, T newValue, IEqualityComparer< return propertyChanged; } + /// + /// Tries to validate a new value for a specified property. If the validation is successful, + /// is called, otherwise no state change is performed. + /// + /// The type of the property that changed. + /// The field storing the property's value. + /// The property's value after the change occurred. + /// The resulting validation errors, if any. + /// (optional) The name of the property that changed. + /// Whether the validation was successful and the property value changed as well. + protected bool TrySetProperty(ref T field, T newValue, out IReadOnlyCollection errors, [CallerMemberName] string? propertyName = null) + { + return TryValidateProperty(newValue, propertyName, out errors) && + SetProperty(ref field, newValue, propertyName); + } + + /// + /// Tries to validate a new value for a specified property. If the validation is successful, + /// is called, otherwise no state change is performed. + /// + /// The type of the property that changed. + /// The field storing the property's value. + /// The property's value after the change occurred. + /// The instance to use to compare the input values. + /// The resulting validation errors, if any. + /// (optional) The name of the property that changed. + /// Whether the validation was successful and the property value changed as well. + protected bool TrySetProperty(ref T field, T newValue, IEqualityComparer comparer, out IReadOnlyCollection errors, [CallerMemberName] string? propertyName = null) + { + return TryValidateProperty(newValue, propertyName, out errors) && + SetProperty(ref field, newValue, comparer, propertyName); + } + + /// + /// Tries to validate a new value for a specified property. If the validation is successful, + /// is called, otherwise no state change is performed. + /// + /// The type of the property that changed. + /// The current property value. + /// The property's value after the change occurred. + /// A callback to invoke to update the property value. + /// The resulting validation errors, if any. + /// (optional) The name of the property that changed. + /// Whether the validation was successful and the property value changed as well. + protected bool TrySetProperty(T oldValue, T newValue, Action callback, out IReadOnlyCollection errors, [CallerMemberName] string? propertyName = null) + { + return TryValidateProperty(newValue, propertyName, out errors) && + SetProperty(oldValue, newValue, callback, propertyName); + } + + /// + /// Tries to validate a new value for a specified property. If the validation is successful, + /// is called, otherwise no state change is performed. + /// + /// The type of the property that changed. + /// The current property value. + /// The property's value after the change occurred. + /// The instance to use to compare the input values. + /// A callback to invoke to update the property value. + /// The resulting validation errors, if any. + /// (optional) The name of the property that changed. + /// Whether the validation was successful and the property value changed as well. + protected bool TrySetProperty(T oldValue, T newValue, IEqualityComparer comparer, Action callback, out IReadOnlyCollection errors, [CallerMemberName] string? propertyName = null) + { + return TryValidateProperty(newValue, propertyName, out errors) && + SetProperty(oldValue, newValue, comparer, callback, propertyName); + } + + /// + /// Tries to validate a new value for a specified property. If the validation is successful, + /// is called, otherwise no state change is performed. + /// + /// The type of model whose property (or field) to set. + /// The type of property (or field) to set. + /// The current property value. + /// The property's value after the change occurred. + /// The model + /// The callback to invoke to set the target property value, if a change has occurred. + /// The resulting validation errors, if any. + /// (optional) The name of the property that changed. + /// Whether the validation was successful and the property value changed as well. + protected bool TrySetProperty(T oldValue, T newValue, TModel model, Action callback, out IReadOnlyCollection errors, [CallerMemberName] string? propertyName = null) + where TModel : class + { + return TryValidateProperty(newValue, propertyName, out errors) && + SetProperty(oldValue, newValue, model, callback, propertyName); + } + + /// + /// Tries to validate a new value for a specified property. If the validation is successful, + /// is called, otherwise no state change is performed. + /// + /// The type of model whose property (or field) to set. + /// The type of property (or field) to set. + /// The current property value. + /// The property's value after the change occurred. + /// The instance to use to compare the input values. + /// The model + /// The callback to invoke to set the target property value, if a change has occurred. + /// The resulting validation errors, if any. + /// (optional) The name of the property that changed. + /// Whether the validation was successful and the property value changed as well. + protected bool TrySetProperty(T oldValue, T newValue, IEqualityComparer comparer, TModel model, Action callback, out IReadOnlyCollection errors, [CallerMemberName] string? propertyName = null) + where TModel : class + { + return TryValidateProperty(newValue, propertyName, out errors) && + SetProperty(oldValue, newValue, comparer, model, callback, propertyName); + } + /// [Pure] public IEnumerable GetErrors(string? propertyName) @@ -329,6 +438,61 @@ private void ValidateProperty(object? value, string? propertyName) } } + /// + /// Tries to validate a property with a specified name and a given input value, and returns + /// the computed errors, if any. If the property is valid, it is assumed that its value is + /// about to be set in the current object. Otherwise, no observable local state is modified. + /// + /// The value to test for the specified property. + /// The name of the property to validate. + /// The resulting validation errors, if any. + /// Thrown when is . + private bool TryValidateProperty(object? value, string? propertyName, out IReadOnlyCollection errors) + { + if (propertyName is null) + { + ThrowArgumentNullExceptionForNullPropertyName(); + } + + // Add the cached errors list for later use. + if (!this.errors.TryGetValue(propertyName!, out List? propertyErrors)) + { + propertyErrors = new List(); + + this.errors.Add(propertyName!, propertyErrors); + } + + bool hasErrors = propertyErrors.Count > 0; + + List localErrors = new List(); + + // Validate the property, by adding new errors to the local list + bool isValid = Validator.TryValidateProperty( + value, + new ValidationContext(this, null, null) { MemberName = propertyName }, + localErrors); + + // We only modify the state if the property is valid and it wasn't so before. In this case, we + // clear the cached list of errors (which is visible to consumers) and raise the necessary events. + if (isValid && hasErrors) + { + propertyErrors.Clear(); + + this.totalErrors--; + + if (this.totalErrors == 0) + { + OnPropertyChanged(HasErrorsChangedEventArgs); + } + + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(propertyName)); + } + + errors = localErrors; + + return isValid; + } + #pragma warning disable SA1204 /// /// Throws an when a property name given as input is . From 9d1b4a72c64598f23e0307b98a75c6683f2da26a Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 11 Nov 2020 18:38:43 +0100 Subject: [PATCH 20/20] Added some TrySetProperty tests --- .../Mvvm/Test_ObservableValidator.cs | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 80e0bd204fd..41c94e6310a 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -153,6 +153,60 @@ public void Test_ObservableValidator_ValidateReturn(string value, bool isValid) } } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableValidator_TrySetProperty() + { + var model = new Person(); + var events = new List(); + + model.ErrorsChanged += (s, e) => events.Add(e); + + // Set a correct value, this should update the property + Assert.IsTrue(model.TrySetName("Hello", out var errors)); + Assert.IsTrue(errors.Count == 0); + Assert.IsTrue(events.Count == 0); + Assert.AreEqual(model.Name, "Hello"); + Assert.IsFalse(model.HasErrors); + + // Invalid value #1, this should be ignored + Assert.IsFalse(model.TrySetName(null, out errors)); + Assert.IsTrue(errors.Count > 0); + Assert.IsTrue(events.Count == 0); + Assert.AreEqual(model.Name, "Hello"); + Assert.IsFalse(model.HasErrors); + + // Invalid value #2, same as above + Assert.IsFalse(model.TrySetName("This string is too long for the target property in this model and should fail", out errors)); + Assert.IsTrue(errors.Count > 0); + Assert.IsTrue(events.Count == 0); + Assert.AreEqual(model.Name, "Hello"); + Assert.IsFalse(model.HasErrors); + + // Correct value, this should update the property + Assert.IsTrue(model.TrySetName("Hello world", out errors)); + Assert.IsTrue(errors.Count == 0); + Assert.IsTrue(events.Count == 0); + Assert.AreEqual(model.Name, "Hello world"); + Assert.IsFalse(model.HasErrors); + + // Actually set an invalid value to show some errors + model.Name = "No"; + + // Errors should now be present + Assert.IsTrue(model.HasErrors); + Assert.IsTrue(events.Count == 1); + Assert.IsTrue(model.GetErrors(nameof(Person.Name)).Cast().Any()); + Assert.IsTrue(model.HasErrors); + + // Trying to set a correct property should clear the errors + Assert.IsTrue(model.TrySetName("This is fine", out errors)); + Assert.IsTrue(errors.Count == 0); + Assert.IsTrue(events.Count == 2); + Assert.IsFalse(model.HasErrors); + Assert.AreEqual(model.Name, "This is fine"); + } + public class Person : ObservableValidator { private string name; @@ -166,6 +220,11 @@ public string Name set => SetProperty(ref this.name, value, true); } + public bool TrySetName(string value, out IReadOnlyCollection errors) + { + return TrySetProperty(ref name, value, out errors, nameof(Name)); + } + private int age; [Range(0, 100)]