From cfc786505f66ba962ee6a2a6890c5325e8e2f778 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 12 Nov 2020 13:01:47 +0100 Subject: [PATCH 01/24] Enabled C# 9, switched to target typed new() --- .../ComponentModel/ObservableObject.cs | 8 +++--- .../ComponentModel/ObservableRecipient.cs | 2 +- .../ComponentModel/ObservableValidator.cs | 10 +++---- .../DependencyInjection/Ioc.cs | 2 +- .../Input/AsyncRelayCommand.cs | 8 +++--- .../Input/AsyncRelayCommand{T}.cs | 2 +- .../Messaging/IMessengerExtensions.cs | 3 +-- .../DictionarySlim{TKey,TValue}.cs | 2 +- .../AsyncCollectionRequestMessage{T}.cs | 6 ++--- .../Messages/CollectionRequestMessage{T}.cs | 2 +- .../Messaging/StrongReferenceMessenger.cs | 20 +++++++------- .../Messaging/WeakReferenceMessenger.cs | 27 +++++++------------ .../Microsoft.Toolkit.Mvvm.csproj | 2 +- 13 files changed, 42 insertions(+), 52 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs index cd7fec0634c..d8f9c05270e 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs @@ -340,7 +340,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, // instance. This will result in no further allocations after the first time this method is called for a given // generic type. We only pay the cost of the virtual call to the delegate, but this is not performance critical // code and that overhead would still be much lower than the rest of the method anyway, so that's fine. - return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new TaskNotifier(), newValue, _ => { }, propertyName); + return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, _ => { }, propertyName); } /// @@ -362,7 +362,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, /// protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, Action callback, [CallerMemberName] string? propertyName = null) { - return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new TaskNotifier(), newValue, callback, propertyName); + return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, callback, propertyName); } /// @@ -401,7 +401,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, /// protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, [CallerMemberName] string? propertyName = null) { - return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new TaskNotifier(), newValue, _ => { }, propertyName); + return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, _ => { }, propertyName); } /// @@ -424,7 +424,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNoti /// protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, Action?> callback, [CallerMemberName] string? propertyName = null) { - return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new TaskNotifier(), newValue, callback, propertyName); + return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, callback, propertyName); } /// diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs index 5a43d09aea4..dc5d5c85690 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs @@ -116,7 +116,7 @@ protected virtual void OnDeactivated() /// protected virtual void Broadcast(T oldValue, T newValue, string? propertyName) { - var message = new PropertyChangedMessage(this, propertyName, oldValue, newValue); + PropertyChangedMessage message = new(this, propertyName, oldValue, newValue); Messenger.Send(message); } diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index e990fcc8b1d..887973ea050 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -22,12 +22,12 @@ public abstract class ObservableValidator : ObservableObject, INotifyDataErrorIn /// /// The cached for . /// - private static readonly PropertyChangedEventArgs HasErrorsChangedEventArgs = new PropertyChangedEventArgs(nameof(HasErrors)); + private static readonly PropertyChangedEventArgs HasErrorsChangedEventArgs = new(nameof(HasErrors)); /// /// The instance used to store previous validation results. /// - private readonly Dictionary> errors = new Dictionary>(); + private readonly Dictionary> errors = new(); /// /// Indicates the total number of properties with errors (not total errors). @@ -380,7 +380,7 @@ private void ValidateProperty(object? value, string? propertyName) // If the property isn't present in the dictionary, add it now to avoid allocations. if (!this.errors.TryGetValue(propertyName!, out List? propertyErrors)) { - propertyErrors = new List(); + propertyErrors = new(); this.errors.Add(propertyName!, propertyErrors); } @@ -457,14 +457,14 @@ private bool TryValidateProperty(object? value, string? propertyName, out IReadO // Add the cached errors list for later use. if (!this.errors.TryGetValue(propertyName!, out List? propertyErrors)) { - propertyErrors = new List(); + propertyErrors = new(); this.errors.Add(propertyName!, propertyErrors); } bool hasErrors = propertyErrors.Count > 0; - List localErrors = new List(); + List localErrors = new(); // Validate the property, by adding new errors to the local list bool isValid = Validator.TryValidateProperty( diff --git a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs index b44819888ff..0986c3d5622 100644 --- a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs +++ b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs @@ -47,7 +47,7 @@ public sealed class Ioc : IServiceProvider /// /// Gets the default instance. /// - public static Ioc Default { get; } = new Ioc(); + public static Ioc Default { get; } = new(); /// /// The instance to use, if initialized. diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs index e9678946137..ace4db46b4b 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs @@ -22,17 +22,17 @@ public sealed class AsyncRelayCommand : ObservableObject, IAsyncRelayCommand /// /// The cached for . /// - internal static readonly PropertyChangedEventArgs CanBeCanceledChangedEventArgs = new PropertyChangedEventArgs(nameof(CanBeCanceled)); + internal static readonly PropertyChangedEventArgs CanBeCanceledChangedEventArgs = new(nameof(CanBeCanceled)); /// /// The cached for . /// - internal static readonly PropertyChangedEventArgs IsCancellationRequestedChangedEventArgs = new PropertyChangedEventArgs(nameof(IsCancellationRequested)); + internal static readonly PropertyChangedEventArgs IsCancellationRequestedChangedEventArgs = new(nameof(IsCancellationRequested)); /// /// The cached for . /// - internal static readonly PropertyChangedEventArgs IsRunningChangedEventArgs = new PropertyChangedEventArgs(nameof(IsRunning)); + internal static readonly PropertyChangedEventArgs IsRunningChangedEventArgs = new(nameof(IsRunning)); /// /// The to invoke when is used. @@ -163,7 +163,7 @@ public Task ExecuteAsync(object? parameter) // Cancel the previous operation, if one is pending this.cancellationTokenSource?.Cancel(); - var cancellationTokenSource = this.cancellationTokenSource = new CancellationTokenSource(); + CancellationTokenSource cancellationTokenSource = this.cancellationTokenSource = new(); OnPropertyChanged(IsCancellationRequestedChangedEventArgs); diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index 61f1962b1d7..8a6087f146c 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -168,7 +168,7 @@ public Task ExecuteAsync(T parameter) // Cancel the previous operation, if one is pending this.cancellationTokenSource?.Cancel(); - var cancellationTokenSource = this.cancellationTokenSource = new CancellationTokenSource(); + CancellationTokenSource cancellationTokenSource = this.cancellationTokenSource = new(); OnPropertyChanged(AsyncRelayCommand.IsCancellationRequestedChangedEventArgs); diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs index ef8c7612701..5eb1d32757d 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs @@ -47,8 +47,7 @@ private static class DiscoveredRecipients /// /// The instance used to track the preloaded registration actions for each recipient. /// - public static readonly ConditionalWeakTable[]> RegistrationMethods - = new ConditionalWeakTable[]>(); + public static readonly ConditionalWeakTable[]> RegistrationMethods = new(); } /// diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs index 11012a1e350..b8b9a508b4b 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Internals/Microsoft.Collections.Extensions/DictionarySlim{TKey,TValue}.cs @@ -334,7 +334,7 @@ private Entry[] Resize() /// [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Enumerator GetEnumerator() => new Enumerator(this); + public Enumerator GetEnumerator() => new(this); /// /// Enumerator for . diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs index da7cb85f25d..f6597f5292b 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs @@ -22,13 +22,13 @@ public class AsyncCollectionRequestMessage : IAsyncEnumerable /// operations that can be executed in parallel, or instances, which can be used so that multiple /// asynchronous operations are only started sequentially from and do not overlap in time. /// - private readonly List<(Task?, Func>?)> responses = new List<(Task?, Func>?)>(); + private readonly List<(Task?, Func>?)> responses = new(); /// /// The instance used to link the token passed to /// and the one passed to all subscribers to the message. /// - private readonly CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); + private readonly CancellationTokenSource cancellationTokenSource = new(); /// /// Gets the instance that will be linked to the @@ -102,7 +102,7 @@ public async Task> GetResponsesAsync(CancellationToken ca cancellationToken.Register(this.cancellationTokenSource.Cancel); } - List results = new List(this.responses.Count); + List results = new(this.responses.Count); await foreach (var response in this.WithCancellation(cancellationToken)) { diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messages/CollectionRequestMessage{T}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messages/CollectionRequestMessage{T}.cs index e1acd67143e..d24bc99c49a 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messages/CollectionRequestMessage{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messages/CollectionRequestMessage{T}.cs @@ -16,7 +16,7 @@ namespace Microsoft.Toolkit.Mvvm.Messaging.Messages /// The type of request to make. public class CollectionRequestMessage : IEnumerable { - private readonly List responses = new List(); + private readonly List responses = new(); /// /// Gets the message responses. diff --git a/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs index dc414ffaeb0..ece9853ded3 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs @@ -71,7 +71,7 @@ public sealed class StrongReferenceMessenger : IMessenger /// so that all the existing handlers can be removed without having to dynamically create /// the generic types for the containers of the various dictionaries mapping the handlers. /// - private readonly DictionarySlim> recipientsMap = new DictionarySlim>(); + private readonly DictionarySlim> recipientsMap = new(); /// /// The instance for types combination. @@ -81,12 +81,12 @@ public sealed class StrongReferenceMessenger : IMessenger /// Each method relies on to get the type-safe instance /// of the class for each pair of generic arguments in use. /// - private readonly DictionarySlim typesMap = new DictionarySlim(); + private readonly DictionarySlim typesMap = new(); /// /// Gets the default instance. /// - public static StrongReferenceMessenger Default { get; } = new StrongReferenceMessenger(); + public static StrongReferenceMessenger Default { get; } = new(); /// public bool IsRegistered(object recipient, TToken token) @@ -100,7 +100,7 @@ public bool IsRegistered(object recipient, TToken token) return false; } - var key = new Recipient(recipient); + Recipient key = new(recipient); return mapping!.ContainsKey(key); } @@ -116,7 +116,7 @@ public void Register(TRecipient recipient, TToken { // Get the registration list for this recipient Mapping mapping = GetOrAddMapping(); - var key = new Recipient(recipient); + Recipient key = new(recipient); ref DictionarySlim? map = ref mapping.GetOrAddValueRef(key); map ??= new DictionarySlim(); @@ -147,7 +147,7 @@ public void UnregisterAll(object recipient) lock (this.recipientsMap) { // If the recipient has no registered messages at all, ignore - var key = new Recipient(recipient); + Recipient key = new(recipient); if (!this.recipientsMap.TryGetValue(key, out HashSet? set)) { @@ -196,7 +196,7 @@ public void UnregisterAll(object recipient, TToken token) Monitor.Enter(this.recipientsMap, ref lockTaken); // Get the shared set of mappings for the recipient, if present - var key = new Recipient(recipient); + Recipient key = new(recipient); if (!this.recipientsMap.TryGetValue(key, out HashSet? set)) { @@ -295,7 +295,7 @@ public void Unregister(object recipient, TToken token) return; } - var key = new Recipient(recipient); + Recipient key = new(recipient); if (!mapping!.TryGetValue(key, out DictionarySlim? dictionary)) { @@ -443,7 +443,7 @@ private bool TryGetMapping(out Mapping? mapp where TMessage : class where TToken : IEquatable { - var key = new Type2(typeof(TMessage), typeof(TToken)); + Type2 key = new(typeof(TMessage), typeof(TToken)); if (this.typesMap.TryGetValue(key, out IMapping? target)) { @@ -472,7 +472,7 @@ private Mapping GetOrAddMapping() where TMessage : class where TToken : IEquatable { - var key = new Type2(typeof(TMessage), typeof(TToken)); + Type2 key = new(typeof(TMessage), typeof(TToken)); ref IMapping? target = ref this.typesMap.GetOrAddValueRef(key); target ??= new Mapping(); diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index d17e332ba5d..f4a0ddceb43 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -46,12 +46,12 @@ public sealed class WeakReferenceMessenger : IMessenger /// /// The map of currently registered recipients for all message types. /// - private readonly DictionarySlim recipientsMap = new DictionarySlim(); + private readonly DictionarySlim recipientsMap = new(); /// /// Gets the default instance. /// - public static WeakReferenceMessenger Default { get; } = new WeakReferenceMessenger(); + public static WeakReferenceMessenger Default { get; } = new(); /// public bool IsRegistered(object recipient, TToken token) @@ -60,7 +60,7 @@ public bool IsRegistered(object recipient, TToken token) { lock (this.recipientsMap) { - Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); + Type2 type2 = new(typeof(TMessage), typeof(TToken)); // Get the conditional table associated with the target recipient, for the current pair // of token and message types. If it exists, check if there is a matching token. @@ -79,7 +79,7 @@ public void Register(TRecipient recipient, TToken { lock (this.recipientsMap) { - Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); + Type2 type2 = new(typeof(TMessage), typeof(TToken)); // Get the conditional table for the pair of type arguments, or create it if it doesn't exist ref RecipientsTable? mapping = ref this.recipientsMap.GetOrAddValueRef(type2); @@ -148,7 +148,7 @@ public void Unregister(object recipient, TToken token) { lock (this.recipientsMap) { - var type2 = new Type2(typeof(TMessage), typeof(TToken)); + Type2 type2 = new(typeof(TMessage), typeof(TToken)); var enumerator = this.recipientsMap.GetEnumerator(); // Traverse all the existing token and message pairs matching the current type @@ -174,7 +174,7 @@ public TMessage Send(TMessage message, TToken token) lock (this.recipientsMap) { - Type2 type2 = new Type2(typeof(TMessage), typeof(TToken)); + Type2 type2 = new(typeof(TMessage), typeof(TToken)); // Try to get the target table if (!this.recipientsMap.TryGetValue(type2, out RecipientsTable? table)) @@ -302,22 +302,13 @@ internal sealed class ConditionalWeakTable /// /// The underlying instance. /// - private readonly System.Runtime.CompilerServices.ConditionalWeakTable table; + private readonly System.Runtime.CompilerServices.ConditionalWeakTable table = new(); /// /// A supporting linked list to store keys in . This is needed to expose /// the ability to enumerate existing keys when there is no support for that in the BCL. /// - private readonly LinkedList> keys; - - /// - /// Initializes a new instance of the class. - /// - public ConditionalWeakTable() - { - this.table = new System.Runtime.CompilerServices.ConditionalWeakTable(); - this.keys = new LinkedList>(); - } + private readonly LinkedList> keys = new(); /// public bool TryGetValue(TKey key, out TValue value) @@ -365,7 +356,7 @@ public IEnumerator> GetEnumerator() if (node.Value.TryGetTarget(out TKey? target) && this.table.TryGetValue(target!, out TValue value)) { - yield return new KeyValuePair(target, value); + yield return new(target, value); } else { diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index 24b6ccc43b5..7105b6e5143 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -2,7 +2,7 @@ netstandard2.0;netstandard2.1 - 8.0 + 9.0 enable true Windows Community Toolkit MVVM Toolkit From b4f66bd7533d86a78cdd255038fcff27e0543eee Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 12 Nov 2020 14:01:18 +0100 Subject: [PATCH 02/24] Memory improvements for weak messenger on NS2.0 --- .../Messaging/WeakReferenceMessenger.cs | 87 ++++++++++++++++--- 1 file changed, 77 insertions(+), 10 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index f4a0ddceb43..e8bc3890e86 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -346,25 +346,92 @@ public bool Remove(TKey key) } /// - public IEnumerator> GetEnumerator() + [Pure] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public Enumerator GetEnumerator() => new(this); + + /// + /// A custom enumerator that traverses items in a instance. + /// + public ref struct Enumerator { - for (LinkedListNode>? node = this.keys.First; !(node is null);) + /// + /// The owner instance for the enumerator. + /// + private readonly ConditionalWeakTable owner; + + /// + /// The current , if any. + /// + private LinkedListNode>? node; + + /// + /// The current to return. + /// + private KeyValuePair current; + + /// + /// Indicates whether or not has been called at least once. + /// + private bool isFirstMoveNextPending; + + /// + /// Initializes a new instance of the struct. + /// + /// The owner instance for the enumerator. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public Enumerator(ConditionalWeakTable owner) { - LinkedListNode>? next = node.Next; + this.owner = owner; + this.node = null; + this.current = default; + this.isFirstMoveNextPending = true; + } + + /// + public bool MoveNext() + { + LinkedListNode>? node; - // Get the key and value for the current node - if (node.Value.TryGetTarget(out TKey? target) && - this.table.TryGetValue(target!, out TValue value)) + if (!isFirstMoveNextPending) { - yield return new(target, value); + node = this.node!.Next; } else { - // If the current key has been collected, trim the list - this.keys.Remove(node); + node = this.owner.keys.First; + + this.isFirstMoveNextPending = false; } - node = next; + while (node is not null) + { + // Get the key and value for the current node + if (node.Value.TryGetTarget(out TKey? target) && + this.owner.table.TryGetValue(target!, out TValue value)) + { + this.node = node; + this.current = new KeyValuePair(target, value); + + return true; + } + else + { + // If the current key has been collected, trim the list + this.owner.keys.Remove(node); + } + + node = node.Next; + } + + return false; + } + + /// + public readonly KeyValuePair Current + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => this.current; } } } From 820e07800c7c584d521c6451dff294a418e68ebd Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 12 Nov 2020 14:16:17 +0100 Subject: [PATCH 03/24] Added generic ObservableValidator.GetErrors return --- .../ComponentModel/ObservableValidator.cs | 25 +++++++++--------- .../Mvvm/Test_ObservableValidator.cs | 26 +++++++++---------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 887973ea050..8408d3d724a 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -326,14 +326,22 @@ protected bool TrySetProperty(T oldValue, T newValue, IEqualityCompar SetProperty(oldValue, newValue, comparer, model, callback, propertyName); } - /// + /// [Pure] - public IEnumerable GetErrors(string? propertyName) + public IEnumerable GetErrors(string? propertyName) { // Entity-level errors when the target property is null or empty if (string.IsNullOrEmpty(propertyName)) { - return this.GetAllErrors(); + // Local function to gather all the entity-level errors + [Pure] + [MethodImpl(MethodImplOptions.NoInlining)] + IEnumerable GetAllErrors() + { + return this.errors.Values.SelectMany(errors => errors); + } + + return GetAllErrors(); } // Property-level errors, if any @@ -350,16 +358,9 @@ public IEnumerable GetErrors(string? propertyName) return Array.Empty(); } - /// - /// Implements the logic for entity-level errors gathering for . - /// - /// An instance with all the errors in . + /// [Pure] - [MethodImpl(MethodImplOptions.NoInlining)] - private IEnumerable GetAllErrors() - { - return this.errors.Values.SelectMany(errors => errors); - } + IEnumerable INotifyDataErrorInfo.GetErrors(string? propertyName) => GetErrors(propertyName); /// /// Validates a property with a specified name and a given input value. diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 41c94e6310a..a0878c91aa5 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -88,33 +88,33 @@ public void Test_ObservableValidator_GetErrors() { var model = new Person(); - Assert.AreEqual(model.GetErrors(null).Cast().Count(), 0); - Assert.AreEqual(model.GetErrors(string.Empty).Cast().Count(), 0); - Assert.AreEqual(model.GetErrors("ThereIsntAPropertyWithThisName").Cast().Count(), 0); - Assert.AreEqual(model.GetErrors(nameof(Person.Name)).Cast().Count(), 0); + Assert.AreEqual(model.GetErrors(null).Count(), 0); + Assert.AreEqual(model.GetErrors(string.Empty).Count(), 0); + Assert.AreEqual(model.GetErrors("ThereIsntAPropertyWithThisName").Count(), 0); + Assert.AreEqual(model.GetErrors(nameof(Person.Name)).Count(), 0); model.Name = "Foo"; - var errors = model.GetErrors(nameof(Person.Name)).Cast().ToArray(); + var errors = model.GetErrors(nameof(Person.Name)).ToArray(); Assert.AreEqual(errors.Length, 1); Assert.AreEqual(errors[0].MemberNames.First(), nameof(Person.Name)); - Assert.AreEqual(model.GetErrors("ThereIsntAPropertyWithThisName").Cast().Count(), 0); + Assert.AreEqual(model.GetErrors("ThereIsntAPropertyWithThisName").Count(), 0); - errors = model.GetErrors(null).Cast().ToArray(); + errors = model.GetErrors(null).ToArray(); Assert.AreEqual(errors.Length, 1); Assert.AreEqual(errors[0].MemberNames.First(), nameof(Person.Name)); - errors = model.GetErrors(string.Empty).Cast().ToArray(); + errors = model.GetErrors(string.Empty).ToArray(); Assert.AreEqual(errors.Length, 1); Assert.AreEqual(errors[0].MemberNames.First(), nameof(Person.Name)); model.Age = -1; - errors = model.GetErrors(null).Cast().ToArray(); + errors = model.GetErrors(null).ToArray(); Assert.AreEqual(errors.Length, 2); Assert.IsTrue(errors.Any(e => e.MemberNames.First().Equals(nameof(Person.Name)))); @@ -122,7 +122,7 @@ public void Test_ObservableValidator_GetErrors() model.Age = 26; - errors = model.GetErrors(null).Cast().ToArray(); + errors = model.GetErrors(null).ToArray(); Assert.AreEqual(errors.Length, 1); Assert.IsTrue(errors.Any(e => e.MemberNames.First().Equals(nameof(Person.Name)))); @@ -145,11 +145,11 @@ public void Test_ObservableValidator_ValidateReturn(string value, bool isValid) if (isValid) { - Assert.IsTrue(!model.GetErrors(nameof(Person.Name)).Cast().Any()); + Assert.IsTrue(!model.GetErrors(nameof(Person.Name)).Any()); } else { - Assert.IsTrue(model.GetErrors(nameof(Person.Name)).Cast().Any()); + Assert.IsTrue(model.GetErrors(nameof(Person.Name)).Any()); } } @@ -196,7 +196,7 @@ public void Test_ObservableValidator_TrySetProperty() // 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.GetErrors(nameof(Person.Name)).Any()); Assert.IsTrue(model.HasErrors); // Trying to set a correct property should clear the errors From 77b0ae2dc986712b82444e272fd72426ea26c96c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 19 Nov 2020 00:36:42 +0100 Subject: [PATCH 04/24] Changed command CanExecute args to Predicate --- Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs | 6 +++--- Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index 8a6087f146c..e403e3add36 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -29,7 +29,7 @@ public sealed class AsyncRelayCommand : ObservableObject, IAsyncRelayCommand< /// /// The optional action to invoke when is used. /// - private readonly Func? canExecute; + private readonly Predicate? canExecute; /// /// The instance to use to cancel . @@ -65,7 +65,7 @@ public AsyncRelayCommand(Func cancelableExecute) /// The execution logic. /// The execution status logic. /// See notes in . - public AsyncRelayCommand(Func execute, Func canExecute) + public AsyncRelayCommand(Func execute, Predicate canExecute) { this.execute = execute; this.canExecute = canExecute; @@ -77,7 +77,7 @@ public AsyncRelayCommand(Func execute, Func canExecute) /// The cancelable execution logic. /// The execution status logic. /// See notes in . - public AsyncRelayCommand(Func cancelableExecute, Func canExecute) + public AsyncRelayCommand(Func cancelableExecute, Predicate canExecute) { this.cancelableExecute = cancelableExecute; this.canExecute = canExecute; diff --git a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs index 0288e8dbb52..02c9126ed70 100644 --- a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs @@ -29,7 +29,7 @@ public sealed class RelayCommand : IRelayCommand /// /// The optional action to invoke when is used. /// - private readonly Func? canExecute; + private readonly Predicate? canExecute; /// public event EventHandler? CanExecuteChanged; @@ -54,7 +54,7 @@ public RelayCommand(Action execute) /// The execution logic. /// The execution status logic. /// See notes in . - public RelayCommand(Action execute, Func canExecute) + public RelayCommand(Action execute, Predicate canExecute) { this.execute = execute; this.canExecute = canExecute; From 47f495c42ad0fa3f721eb5f8378d7301d311ca65 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 19 Nov 2020 00:44:09 +0100 Subject: [PATCH 05/24] Improved nullability annotations in commands --- .../Input/AsyncRelayCommand.cs | 4 +-- .../Input/AsyncRelayCommand{T}.cs | 30 +++++++++---------- .../Input/Interfaces/IAsyncRelayCommand{T}.cs | 2 +- .../Input/Interfaces/IRelayCommand{T}.cs | 4 +-- .../Input/RelayCommand{T}.cs | 16 +++++----- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs index ace4db46b4b..30f68bbc91f 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand.cs @@ -122,7 +122,7 @@ private set } /// - public bool CanBeCanceled => !(this.cancelableExecute is null) && IsRunning; + public bool CanBeCanceled => this.cancelableExecute is not null && IsRunning; /// public bool IsCancellationRequested => this.cancellationTokenSource?.IsCancellationRequested == true; @@ -155,7 +155,7 @@ public Task ExecuteAsync(object? parameter) if (CanExecute(parameter)) { // Non cancelable command delegate - if (!(this.execute is null)) + if (this.execute is not null) { return ExecutionTask = this.execute(); } diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index e403e3add36..5314a10c30c 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -19,17 +19,17 @@ public sealed class AsyncRelayCommand : ObservableObject, IAsyncRelayCommand< /// /// The to invoke when is used. /// - private readonly Func? execute; + private readonly Func? execute; /// /// The cancelable to invoke when is used. /// - private readonly Func? cancelableExecute; + private readonly Func? cancelableExecute; /// /// The optional action to invoke when is used. /// - private readonly Predicate? canExecute; + private readonly Predicate? canExecute; /// /// The instance to use to cancel . @@ -44,7 +44,7 @@ public sealed class AsyncRelayCommand : ObservableObject, IAsyncRelayCommand< /// /// The execution logic. /// See notes in . - public AsyncRelayCommand(Func execute) + public AsyncRelayCommand(Func execute) { this.execute = execute; } @@ -54,7 +54,7 @@ public AsyncRelayCommand(Func execute) /// /// The cancelable execution logic. /// See notes in . - public AsyncRelayCommand(Func cancelableExecute) + public AsyncRelayCommand(Func cancelableExecute) { this.cancelableExecute = cancelableExecute; } @@ -65,7 +65,7 @@ public AsyncRelayCommand(Func cancelableExecute) /// The execution logic. /// The execution status logic. /// See notes in . - public AsyncRelayCommand(Func execute, Predicate canExecute) + public AsyncRelayCommand(Func execute, Predicate canExecute) { this.execute = execute; this.canExecute = canExecute; @@ -77,7 +77,7 @@ public AsyncRelayCommand(Func execute, Predicate canExecute) /// The cancelable execution logic. /// The execution status logic. /// See notes in . - public AsyncRelayCommand(Func cancelableExecute, Predicate canExecute) + public AsyncRelayCommand(Func cancelableExecute, Predicate canExecute) { this.cancelableExecute = cancelableExecute; this.canExecute = canExecute; @@ -106,7 +106,7 @@ private set } /// - public bool CanBeCanceled => !(this.cancelableExecute is null) && IsRunning; + public bool CanBeCanceled => this.cancelableExecute is not null && IsRunning; /// public bool IsCancellationRequested => this.cancellationTokenSource?.IsCancellationRequested == true; @@ -122,7 +122,7 @@ public void NotifyCanExecuteChanged() /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool CanExecute(T parameter) + public bool CanExecute(T? parameter) { return this.canExecute?.Invoke(parameter) != false; } @@ -138,12 +138,12 @@ parameter is null && return true; } - return CanExecute((T)parameter!); + return CanExecute((T?)parameter); } /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Execute(T parameter) + public void Execute(T? parameter) { ExecuteAsync(parameter); } @@ -151,16 +151,16 @@ public void Execute(T parameter) /// public void Execute(object? parameter) { - ExecuteAsync((T)parameter!); + ExecuteAsync((T?)parameter); } /// - public Task ExecuteAsync(T parameter) + public Task ExecuteAsync(T? parameter) { if (CanExecute(parameter)) { // Non cancelable command delegate - if (!(this.execute is null)) + if (this.execute is not null) { return ExecutionTask = this.execute(parameter); } @@ -182,7 +182,7 @@ public Task ExecuteAsync(T parameter) /// public Task ExecuteAsync(object? parameter) { - return ExecuteAsync((T)parameter!); + return ExecuteAsync((T?)parameter); } /// diff --git a/Microsoft.Toolkit.Mvvm/Input/Interfaces/IAsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/Interfaces/IAsyncRelayCommand{T}.cs index 843547a01c8..694fa3c5b3a 100644 --- a/Microsoft.Toolkit.Mvvm/Input/Interfaces/IAsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/Interfaces/IAsyncRelayCommand{T}.cs @@ -18,6 +18,6 @@ public interface IAsyncRelayCommand : IAsyncRelayCommand, IRelayCommand /// /// The input parameter. /// The representing the async operation being executed. - Task ExecuteAsync(T parameter); + Task ExecuteAsync(T? parameter); } } diff --git a/Microsoft.Toolkit.Mvvm/Input/Interfaces/IRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/Interfaces/IRelayCommand{T}.cs index 33fedadfad4..b68e5e1861d 100644 --- a/Microsoft.Toolkit.Mvvm/Input/Interfaces/IRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/Interfaces/IRelayCommand{T}.cs @@ -18,13 +18,13 @@ public interface IRelayCommand : IRelayCommand /// The input parameter. /// Whether or not the current command can be executed. /// Use this overload to avoid boxing, if is a value type. - bool CanExecute(T parameter); + bool CanExecute(T? parameter); /// /// Provides a strongly-typed variant of . /// /// The input parameter. /// Use this overload to avoid boxing, if is a value type. - void Execute(T parameter); + void Execute(T? parameter); } } diff --git a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs index 02c9126ed70..fdbce414e55 100644 --- a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs @@ -24,12 +24,12 @@ public sealed class RelayCommand : IRelayCommand /// /// The to invoke when is used. /// - private readonly Action execute; + private readonly Action execute; /// /// The optional action to invoke when is used. /// - private readonly Predicate? canExecute; + private readonly Predicate? canExecute; /// public event EventHandler? CanExecuteChanged; @@ -43,7 +43,7 @@ public sealed class RelayCommand : IRelayCommand /// nullable parameter, it is recommended that if is a reference type, /// you should always declare it as nullable, and to always perform checks within . /// - public RelayCommand(Action execute) + public RelayCommand(Action execute) { this.execute = execute; } @@ -54,7 +54,7 @@ public RelayCommand(Action execute) /// The execution logic. /// The execution status logic. /// See notes in . - public RelayCommand(Action execute, Predicate canExecute) + public RelayCommand(Action execute, Predicate canExecute) { this.execute = execute; this.canExecute = canExecute; @@ -68,7 +68,7 @@ public void NotifyCanExecuteChanged() /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool CanExecute(T parameter) + public bool CanExecute(T? parameter) { return this.canExecute?.Invoke(parameter) != false; } @@ -83,12 +83,12 @@ parameter is null && return true; } - return CanExecute((T)parameter!); + return CanExecute((T?)parameter); } /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Execute(T parameter) + public void Execute(T? parameter) { if (CanExecute(parameter)) { @@ -99,7 +99,7 @@ public void Execute(T parameter) /// public void Execute(object? parameter) { - Execute((T)parameter!); + Execute((T?)parameter); } } } \ No newline at end of file From 905307f7434ae3d460e4cb091040d86270639617 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 19 Nov 2020 01:09:35 +0100 Subject: [PATCH 06/24] Minor codegen improvements --- Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs | 2 +- Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index 5314a10c30c..37687a689dd 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -131,7 +131,7 @@ public bool CanExecute(T? parameter) [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool CanExecute(object? parameter) { - if (typeof(T).IsValueType && + if (default(T) is not null && parameter is null && this.canExecute is null) { diff --git a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs index fdbce414e55..48914ab240f 100644 --- a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs @@ -76,7 +76,7 @@ public bool CanExecute(T? parameter) /// public bool CanExecute(object? parameter) { - if (typeof(T).IsValueType && + if (default(T) is not null && parameter is null && this.canExecute is null) { From 09ed2f2f0f66d14257b3e575afbaf802be9ba16b Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 20 Nov 2020 14:26:24 +0100 Subject: [PATCH 07/24] Minor code tweaks --- Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs | 4 ++-- .../ComponentModel/ObservableValidator.cs | 2 +- Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs | 6 +++--- Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs index d8f9c05270e..c7e4f053306 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs @@ -340,7 +340,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, // instance. This will result in no further allocations after the first time this method is called for a given // generic type. We only pay the cost of the virtual call to the delegate, but this is not performance critical // code and that overhead would still be much lower than the rest of the method anyway, so that's fine. - return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, _ => { }, propertyName); + return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, static _ => { }, propertyName); } /// @@ -401,7 +401,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, /// protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, [CallerMemberName] string? propertyName = null) { - return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, _ => { }, propertyName); + return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, static _ => { }, propertyName); } /// diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 8408d3d724a..76f5ade510e 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -338,7 +338,7 @@ public IEnumerable GetErrors(string? propertyName) [MethodImpl(MethodImplOptions.NoInlining)] IEnumerable GetAllErrors() { - return this.errors.Values.SelectMany(errors => errors); + return this.errors.Values.SelectMany(static errors => errors); } return GetAllErrors(); diff --git a/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs index 5eb1d32757d..f48045826e2 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/IMessengerExtensions.cs @@ -138,7 +138,7 @@ static Action GetRegistrationAction(Type type, Metho // For more info on this, see the related issue at https://github.com/dotnet/roslyn/issues/5835. Action[] registrationActions = DiscoveredRecipients.RegistrationMethods.GetValue( recipient.GetType(), - t => LoadRegistrationMethodsForType(t)); + static t => LoadRegistrationMethodsForType(t)); foreach (Action registrationAction in registrationActions) { @@ -157,7 +157,7 @@ static Action GetRegistrationAction(Type type, Metho public static void Register(this IMessenger messenger, IRecipient recipient) where TMessage : class { - messenger.Register, TMessage, Unit>(recipient, default, (r, m) => r.Receive(m)); + messenger.Register, TMessage, Unit>(recipient, default, static (r, m) => r.Receive(m)); } /// @@ -174,7 +174,7 @@ public static void Register(this IMessenger messenger, IRecipi where TMessage : class where TToken : IEquatable { - messenger.Register, TMessage, TToken>(recipient, token, (r, m) => r.Receive(m)); + messenger.Register, TMessage, TToken>(recipient, token, static (r, m) => r.Receive(m)); } /// diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index e8bc3890e86..b3f15157d67 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -87,7 +87,7 @@ public void Register(TRecipient recipient, TToken mapping ??= new RecipientsTable(); // Get or create the handlers dictionary for the target recipient - var map = Unsafe.As>(mapping.GetValue(recipient, _ => new DictionarySlim())); + var map = Unsafe.As>(mapping.GetValue(recipient, static _ => new DictionarySlim())); // Add the new registration entry ref object? registeredHandler = ref map.GetOrAddValueRef(token); From ff82101ad039bfde46aa914793a47e68feb943b0 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 21 Nov 2020 22:09:03 +0100 Subject: [PATCH 08/24] Updated NuGet packages --- Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index 7105b6e5143..25ea121fa4e 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -19,17 +19,19 @@ - + - + - + + + From a28668d25da7784c59c931597cbf3c78a79f4e17 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 21 Nov 2020 22:14:24 +0100 Subject: [PATCH 09/24] Minor code style tweaks --- Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs | 2 +- .../Messaging/Messages/AsyncCollectionRequestMessage{T}.cs | 2 +- Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs | 4 ++-- Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs index 0986c3d5622..2ee61f657fd 100644 --- a/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs +++ b/Microsoft.Toolkit.Mvvm/DependencyInjection/Ioc.cs @@ -134,7 +134,7 @@ public void ConfigureServices(IServiceProvider serviceProvider) { IServiceProvider? oldServices = Interlocked.CompareExchange(ref this.serviceProvider, serviceProvider, null); - if (!(oldServices is null)) + if (oldServices is not null) { ThrowInvalidOperationExceptionForRepeatedConfiguration(); } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs b/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs index f6597f5292b..6a89a34b8cf 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/Messages/AsyncCollectionRequestMessage{T}.cs @@ -129,7 +129,7 @@ public async IAsyncEnumerator GetAsyncEnumerator(CancellationToken cancellati yield break; } - if (!(task is null)) + if (task is not null) { yield return await task.ConfigureAwait(false); } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs index ece9853ded3..9cef2bc2fb1 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/StrongReferenceMessenger.cs @@ -124,7 +124,7 @@ public void Register(TRecipient recipient, TToken // Add the new registration entry ref object? registeredHandler = ref map.GetOrAddValueRef(token); - if (!(registeredHandler is null)) + if (registeredHandler is not null) { ThrowInvalidOperationExceptionForDuplicateRegistration(); } @@ -273,7 +273,7 @@ public void UnregisterAll(object recipient, TToken token) // Remove references to avoid leaks coming from the shared memory pool. // We manually create a span and clear it as a small optimization, as // arrays rented from the pool can be larger than the requested size. - if (!(maps is null)) + if (maps is not null) { maps.AsSpan(0, i).Clear(); diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index b3f15157d67..b27adfa51f1 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -92,7 +92,7 @@ public void Register(TRecipient recipient, TToken // Add the new registration entry ref object? registeredHandler = ref map.GetOrAddValueRef(token); - if (!(registeredHandler is null)) + if (registeredHandler is not null) { ThrowInvalidOperationExceptionForDuplicateRegistration(); } From 446a93b316cc90ee610c9c41a89e9236551be1df Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 11 Dec 2020 13:26:40 +0100 Subject: [PATCH 10/24] Fixed NRE when calling CanExecute(null) over value types --- Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs | 5 ++--- Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs index 37687a689dd..66441a32cb5 100644 --- a/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/AsyncRelayCommand{T}.cs @@ -132,10 +132,9 @@ public bool CanExecute(T? parameter) public bool CanExecute(object? parameter) { if (default(T) is not null && - parameter is null && - this.canExecute is null) + parameter is null) { - return true; + return false; } return CanExecute((T?)parameter); diff --git a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs index 48914ab240f..f8bf29c263d 100644 --- a/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs +++ b/Microsoft.Toolkit.Mvvm/Input/RelayCommand{T}.cs @@ -77,10 +77,9 @@ public bool CanExecute(T? parameter) public bool CanExecute(object? parameter) { if (default(T) is not null && - parameter is null && - this.canExecute is null) + parameter is null) { - return true; + return false; } return CanExecute((T?)parameter); From 504c3e259d0c21a749b8c433667b5cd2a41b38b5 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 14 Jan 2021 19:31:49 +0100 Subject: [PATCH 11/24] Switched ObservableValidator.ValidateProperty to protected --- .../ComponentModel/ObservableValidator.cs | 3 +- .../Mvvm/Test_ObservableValidator.cs | 109 ++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 76f5ade510e..482866324f0 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -364,11 +364,12 @@ IEnumerable GetAllErrors() /// /// Validates a property with a specified name and a given input value. + /// If any changes are detected, the event will be raised. /// /// The value to test for the specified property. /// The name of the property to validate. /// Thrown when is . - private void ValidateProperty(object? value, string? propertyName) + protected void ValidateProperty(object? value, [CallerMemberName] string? propertyName = null) { if (propertyName is null) { diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index a0878c91aa5..30342bb5763 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -2,6 +2,7 @@ // 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.Collections.Generic; using System.ComponentModel; using System.ComponentModel.DataAnnotations; @@ -207,6 +208,60 @@ public void Test_ObservableValidator_TrySetProperty() Assert.AreEqual(model.Name, "This is fine"); } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableValidator_ValidateProperty() + { + var model = new ComparableModel(); + var events = new List(); + + model.ErrorsChanged += (s, e) => events.Add(e); + + // Set a correct value for both properties, first A then B + model.A = 42; + model.B = 30; + + Assert.AreEqual(events.Count, 0); + Assert.IsFalse(model.HasErrors); + + // Make B greater than A, hence invalidating A + model.B = 50; + + Assert.AreEqual(events.Count, 1); + Assert.AreEqual(events.Last().PropertyName, nameof(ComparableModel.A)); + Assert.IsTrue(model.HasErrors); + + events.Clear(); + + // Make A greater than B, hence making it valid again + model.A = 51; + + Assert.AreEqual(events.Count, 1); + Assert.AreEqual(events.Last().PropertyName, nameof(ComparableModel.A)); + Assert.AreEqual(model.GetErrors(nameof(ComparableModel.A)).Count(), 0); + Assert.IsFalse(model.HasErrors); + + events.Clear(); + + // Make A smaller than B, hence invalidating it + model.A = 49; + + Assert.AreEqual(events.Count, 1); + Assert.AreEqual(events.Last().PropertyName, nameof(ComparableModel.A)); + Assert.AreEqual(model.GetErrors(nameof(ComparableModel.A)).Count(), 1); + Assert.IsTrue(model.HasErrors); + + events.Clear(); + + // Lower B, hence making A valid again + model.B = 20; + + Assert.AreEqual(events.Count, 1); + Assert.AreEqual(events.Last().PropertyName, nameof(ComparableModel.A)); + Assert.AreEqual(model.GetErrors(nameof(ComparableModel.A)).Count(), 0); + Assert.IsFalse(model.HasErrors); + } + public class Person : ObservableValidator { private string name; @@ -234,5 +289,59 @@ public int Age set => SetProperty(ref this.age, value, true); } } + + /// + /// Test model for linked properties, to test instance. + /// See https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/3665 for the original request for this feature. + /// + public class ComparableModel : ObservableValidator + { + private int a; + + [Range(10, 100)] + [GreaterThan(nameof(B))] + public int A + { + get => this.a; + set => SetProperty(ref this.a, value, true); + } + + private int b; + + [Range(20, 80)] + public int B + { + get => this.b; + set + { + SetProperty(ref this.b, value, true); + ValidateProperty(A, nameof(A)); + } + } + } + + public sealed class GreaterThanAttribute : ValidationAttribute + { + public GreaterThanAttribute(string propertyName) + { + PropertyName = propertyName; + } + + public string PropertyName { get; } + + protected override ValidationResult IsValid(object value, ValidationContext validationContext) + { + object + instance = validationContext.ObjectInstance, + otherValue = instance.GetType().GetProperty(PropertyName).GetValue(instance); + + if (((IComparable)value).CompareTo(otherValue) > 0) + { + return ValidationResult.Success; + } + + return new ValidationResult("The current value is smaller than the other one"); + } + } } } From 5da7ece4d903addce29ba24f0ec7ca6db8ac4b2c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 15 Jan 2021 13:43:28 +0100 Subject: [PATCH 12/24] Fixed unit tests with RelayCommand and value types --- .../UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs | 7 ++++--- UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs index bf6320fa083..9b7201b5b88 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_AsyncRelayCommand{T}.cs @@ -125,8 +125,8 @@ public void Test_AsyncRelayCommandOfT_NullWithValueType() return Task.CompletedTask; }); - // Special case for null value types - Assert.IsTrue(command.CanExecute(null)); + Assert.IsFalse(command.CanExecute(null)); + Assert.ThrowsException(() => command.Execute(null)); command = new AsyncRelayCommand( i => @@ -135,7 +135,8 @@ public void Test_AsyncRelayCommandOfT_NullWithValueType() return Task.CompletedTask; }, i => i > 0); - Assert.ThrowsException(() => command.CanExecute(null)); + Assert.IsFalse(command.CanExecute(null)); + Assert.ThrowsException(() => command.Execute(null)); } } } diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs index 1c606b4b884..2fdd1ba824c 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_RelayCommand{T}.cs @@ -74,12 +74,13 @@ public void Test_RelayCommand_NullWithValueType() var command = new RelayCommand(i => n = i); - // Special case for null value types - Assert.IsTrue(command.CanExecute(null)); + Assert.IsFalse(command.CanExecute(null)); + Assert.ThrowsException(() => command.Execute(null)); command = new RelayCommand(i => n = i, i => i > 0); - Assert.ThrowsException(() => command.CanExecute(null)); + Assert.IsFalse(command.CanExecute(null)); + Assert.ThrowsException(() => command.Execute(null)); } } } From 54ab0f9ef4fec9d8573e4eb62bf7cc65daac186f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 22 Jan 2021 20:10:13 +0100 Subject: [PATCH 13/24] Added ObservableValidator.ClearErrors(string) API --- .../ComponentModel/ObservableValidator.cs | 77 ++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 482866324f0..cb776cd6645 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -326,11 +326,31 @@ protected bool TrySetProperty(T oldValue, T newValue, IEqualityCompar SetProperty(oldValue, newValue, comparer, model, callback, propertyName); } + /// + /// Clears the validation errors for a specified property or for the entire entity. + /// + /// + /// The name of the property to clear validation errors for. + /// If a or empty name is used, all entity-level errors will be cleared. + /// + protected void ClearErrors(string? propertyName = null) + { + // Clear entity-level errors when the target property is null or empty + if (string.IsNullOrEmpty(propertyName)) + { + ClearAllErrors(); + } + else + { + ClearErrorsForProperty(propertyName!); + } + } + /// [Pure] - public IEnumerable GetErrors(string? propertyName) + public IEnumerable GetErrors(string? propertyName = null) { - // Entity-level errors when the target property is null or empty + // Get entity-level errors when the target property is null or empty if (string.IsNullOrEmpty(propertyName)) { // Local function to gather all the entity-level errors @@ -495,6 +515,59 @@ private bool TryValidateProperty(object? value, string? propertyName, out IReadO return isValid; } + /// + /// Clears all the current errors for the entire entity. + /// + private void ClearAllErrors() + { + if (this.totalErrors == 0) + { + return; + } + + // Clear the errors for all properties with at least one error, and raise the + // ErrorsChanged event for those properties. Other properties will be ignored. + foreach (var propertyInfo in this.errors) + { + bool hasErrors = propertyInfo.Value.Count > 0; + + propertyInfo.Value.Clear(); + + if (hasErrors) + { + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(propertyInfo.Key)); + } + } + + this.totalErrors = 0; + + OnPropertyChanged(HasErrorsChangedEventArgs); + } + + /// + /// Clears all the current errors for a target property. + /// + /// The name of the property to clear errors for. + private void ClearErrorsForProperty(string propertyName) + { + if (!this.errors.TryGetValue(propertyName!, out List? propertyErrors) || + propertyErrors.Count == 0) + { + return; + } + + propertyErrors.Clear(); + + this.totalErrors--; + + if (this.totalErrors == 0) + { + OnPropertyChanged(HasErrorsChangedEventArgs); + } + + ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(propertyName)); + } + #pragma warning disable SA1204 /// /// Throws an when a property name given as input is . From 02843c677d490ee99ecf4a9d3fd85db01d8ebc77 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 22 Jan 2021 20:16:10 +0100 Subject: [PATCH 14/24] Added unit tests for ClearErrors --- .../Mvvm/Test_ObservableValidator.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 30342bb5763..4b26b10a2e7 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -262,6 +262,42 @@ public void Test_ObservableValidator_ValidateProperty() Assert.IsFalse(model.HasErrors); } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableValidator_ClearErrors() + { + var model = new Person(); + var events = new List(); + + model.ErrorsChanged += (s, e) => events.Add(e); + + model.Age = -2; + + Assert.IsTrue(model.HasErrors); + Assert.IsTrue(model.GetErrors(nameof(Person.Age)).Any()); + + model.ClearErrors(nameof(Person.Age)); + + Assert.IsFalse(model.HasErrors); + Assert.IsTrue(events.Count == 2); + + model.Age = 200; + model.Name = "Bo"; + events.Clear(); + + Assert.IsTrue(model.HasErrors); + Assert.IsTrue(model.GetErrors(nameof(Person.Age)).Any()); + Assert.IsTrue(model.GetErrors(nameof(Person.Name)).Any()); + + model.ClearErrors(null); + Assert.IsFalse(model.HasErrors); + Assert.IsFalse(model.GetErrors(nameof(Person.Age)).Any()); + Assert.IsFalse(model.GetErrors(nameof(Person.Name)).Any()); + Assert.IsTrue(events.Count == 2); + Assert.IsTrue(events[0].PropertyName == nameof(Person.Age)); + Assert.IsTrue(events[1].PropertyName == nameof(Person.Name)); + } + public class Person : ObservableValidator { private string name; @@ -288,6 +324,11 @@ public int Age get => this.age; set => SetProperty(ref this.age, value, true); } + + public new void ClearErrors(string propertyName) + { + base.ClearErrors(propertyName); + } } /// From 0437ee452f6d64d7c0e781426af2da82a797bdee Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 22 Jan 2021 20:42:32 +0100 Subject: [PATCH 15/24] Added ObservableValidator.ValidateAllProperties() API --- .../ComponentModel/ObservableValidator.cs | 25 ++++++++ .../Mvvm/Test_ObservableValidator.cs | 60 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index cb776cd6645..371b67bb9e8 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -9,6 +9,7 @@ using System.ComponentModel.DataAnnotations; using System.Diagnostics.Contracts; using System.Linq; +using System.Reflection; using System.Runtime.CompilerServices; namespace Microsoft.Toolkit.Mvvm.ComponentModel @@ -382,6 +383,30 @@ IEnumerable GetAllErrors() [Pure] IEnumerable INotifyDataErrorInfo.GetErrors(string? propertyName) => GetErrors(propertyName); + /// + /// Validates all the properties in the current instance and updates all the tracked errors. + /// If any changes are detected, the event will be raised. + /// + /// + /// Only public instance properties (excluding custom indexers) that have at least one + /// applied to them will be validated. All other + /// members in the current instance will be ignored. None of the processed properties + /// will be modified - they will only be used to retrieve their values and validate them. + /// + protected void ValidateAllProperties() + { + foreach (PropertyInfo propertyInfo in + GetType() + .GetProperties(BindingFlags.Instance | BindingFlags.Public) + .Where(static p => p.GetIndexParameters().Length == 0 && + p.GetCustomAttributes(true).Any())) + { + object? propertyValue = propertyInfo.GetValue(this); + + ValidateProperty(propertyValue, propertyInfo.Name); + } + } + /// /// Validates a property with a specified name and a given input value. /// If any changes are detected, the event will be raised. diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 4b26b10a2e7..9dc0564881e 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -298,6 +298,47 @@ public void Test_ObservableValidator_ClearErrors() Assert.IsTrue(events[1].PropertyName == nameof(Person.Name)); } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableValidator_ValidateAllProperties() + { + var model = new PersonWithDeferredValidation(); + var events = new List(); + + model.ErrorsChanged += (s, e) => events.Add(e); + + model.ValidateAllProperties(); + + Assert.IsTrue(model.HasErrors); + Assert.IsTrue(events.Count == 2); + + // Note: we can't use an index here because the order used to return properties + // from reflection APIs is an implementation detail and might change at any time. + Assert.IsTrue(events.Any(e => e.PropertyName == nameof(Person.Name))); + Assert.IsTrue(events.Any(e => e.PropertyName == nameof(Person.Age))); + + events.Clear(); + + model.Name = "James"; + model.Age = 42; + + model.ValidateAllProperties(); + + Assert.IsFalse(model.HasErrors); + Assert.IsTrue(events.Count == 2); + Assert.IsTrue(events.Any(e => e.PropertyName == nameof(Person.Name))); + Assert.IsTrue(events.Any(e => e.PropertyName == nameof(Person.Age))); + + events.Clear(); + + model.Age = -10; + + model.ValidateAllProperties(); + Assert.IsTrue(model.HasErrors); + Assert.IsTrue(events.Count == 1); + Assert.IsTrue(events.Any(e => e.PropertyName == nameof(Person.Age))); + } + public class Person : ObservableValidator { private string name; @@ -331,6 +372,25 @@ public int Age } } + public class PersonWithDeferredValidation : ObservableValidator + { + [MinLength(4)] + [MaxLength(20)] + [Required] + public string Name { get; set; } + + [Range(18, 100)] + public int Age { get; set; } + + // Extra property with no validation + public float Foo { get; set; } = float.NaN; + + public new void ValidateAllProperties() + { + base.ValidateAllProperties(); + } + } + /// /// Test model for linked properties, to test instance. /// See https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/3665 for the original request for this feature. From 7351c538bbe7372662fbf722187c5e26edf2b9a0 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 22 Jan 2021 20:49:22 +0100 Subject: [PATCH 16/24] Added caching for reflection setup in ValidateAllProperties --- .../ComponentModel/ObservableValidator.cs | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 371b67bb9e8..64090b9a386 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -20,6 +20,11 @@ namespace Microsoft.Toolkit.Mvvm.ComponentModel /// public abstract class ObservableValidator : ObservableObject, INotifyDataErrorInfo { + /// + /// The instance used to track properties to validate for a given viewmodel type. + /// + private static readonly ConditionalWeakTable ValidatableProperties = new(); + /// /// The cached for . /// @@ -395,11 +400,21 @@ IEnumerable GetAllErrors() /// protected void ValidateAllProperties() { - foreach (PropertyInfo propertyInfo in - GetType() - .GetProperties(BindingFlags.Instance | BindingFlags.Public) - .Where(static p => p.GetIndexParameters().Length == 0 && - p.GetCustomAttributes(true).Any())) + // Helper method to discover all the properties to validate in the current viewmodel type + static PropertyInfo[] GetValidatableProperties(Type type) + { + return ( + from property in type.GetProperties(BindingFlags.Instance | BindingFlags.Public) + where property.GetIndexParameters().Length == 0 && + property.GetCustomAttributes(true).Any() + select property).ToArray(); + } + + // Get or compute the cached list of properties to validate. Here we're using a static lambda to ensure the + // delegate is cached by the C# compiler, see the related issue at https://github.com/dotnet/roslyn/issues/5835. + PropertyInfo[] propertyInfos = ValidatableProperties.GetValue(GetType(), static t => GetValidatableProperties(t)); + + foreach (PropertyInfo propertyInfo in propertyInfos) { object? propertyValue = propertyInfo.GetValue(this); From 48157f061ebbb74e2ddf4cba1efe4254a47bb505 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 26 Jan 2021 23:09:39 +0100 Subject: [PATCH 17/24] Added missing nullability annotations --- .../ComponentModel/ObservableValidator.cs | 2 +- .../Messaging/WeakReferenceMessenger.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 64090b9a386..fea68625123 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -371,7 +371,7 @@ IEnumerable GetAllErrors() } // Property-level errors, if any - if (this.errors.TryGetValue(propertyName!, out List errors)) + if (this.errors.TryGetValue(propertyName!, out List? errors)) { return errors; } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index b27adfa51f1..9f5ca7b5ad8 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -67,7 +67,7 @@ public bool IsRegistered(object recipient, TToken token) return this.recipientsMap.TryGetValue(type2, out RecipientsTable? table) && table!.TryGetValue(recipient, out IDictionarySlim? mapping) && - Unsafe.As>(mapping).ContainsKey(token); + Unsafe.As>(mapping)!.ContainsKey(token); } } @@ -133,9 +133,9 @@ public void UnregisterAll(object recipient, TToken token) while (enumerator.MoveNext()) { if (enumerator.Key.TToken == typeof(TToken) && - enumerator.Value.TryGetValue(recipient, out IDictionarySlim mapping)) + enumerator.Value.TryGetValue(recipient, out IDictionarySlim? mapping)) { - Unsafe.As>(mapping).TryRemove(token, out _); + Unsafe.As>(mapping)!.TryRemove(token, out _); } } } @@ -156,9 +156,9 @@ public void Unregister(object recipient, TToken token) while (enumerator.MoveNext()) { if (enumerator.Key.Equals(type2) && - enumerator.Value.TryGetValue(recipient, out IDictionarySlim mapping)) + enumerator.Value.TryGetValue(recipient, out IDictionarySlim? mapping)) { - Unsafe.As>(mapping).TryRemove(token, out _); + Unsafe.As>(mapping)!.TryRemove(token, out _); } } } @@ -311,7 +311,7 @@ internal sealed class ConditionalWeakTable private readonly LinkedList> keys = new(); /// - public bool TryGetValue(TKey key, out TValue value) + public bool TryGetValue(TKey key, out TValue? value) { return this.table.TryGetValue(key, out value); } From e8b624a1bb3ea5c96c63e659e973c1dd831ed19a Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 26 Jan 2021 23:10:10 +0100 Subject: [PATCH 18/24] Added .NET 5 target (and removed built-in package) --- Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj | 10 +++------- UnitTests/UnitTests.NetCore/UnitTests.NetCore.csproj | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index 25ea121fa4e..99c619287a2 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -1,7 +1,7 @@  - netstandard2.0;netstandard2.1 + netstandard2.0;netstandard2.1;net5.0 9.0 enable true @@ -20,18 +20,14 @@ + - - - - - - + diff --git a/UnitTests/UnitTests.NetCore/UnitTests.NetCore.csproj b/UnitTests/UnitTests.NetCore/UnitTests.NetCore.csproj index f4f7296db23..cbc4cc88a16 100644 --- a/UnitTests/UnitTests.NetCore/UnitTests.NetCore.csproj +++ b/UnitTests/UnitTests.NetCore/UnitTests.NetCore.csproj @@ -16,7 +16,7 @@ - + From d70e6d95edc8f64e5ab576290079a406b6d7daab Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 26 Jan 2021 23:12:58 +0100 Subject: [PATCH 19/24] Added missing types to NuGet description --- Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj index 99c619287a2..44587ab886e 100644 --- a/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj +++ b/Microsoft.Toolkit.Mvvm/Microsoft.Toolkit.Mvvm.csproj @@ -10,8 +10,11 @@ This package includes a .NET Standard MVVM library with helpers such as: - ObservableObject: a base class for objects implementing the INotifyPropertyChanged interface. - ObservableRecipient: a base class for observable objects with support for the IMessenger service. + - ObservableValidator: a base class for objects implementing the INotifyDataErrorInfo interface. - RelayCommand: a simple delegate command implementing the ICommand interface. - - Messenger: a messaging system to exchange messages through different loosely-coupled objects. + - AsyncRelayCommand: a delegate command supporting asynchronous operations and cancellation. + - WeakReferenceMessenger: a messaging system to exchange messages through different loosely-coupled objects. + - StrongReferenceMessenger: a high-performance messaging system that trades weak references for speed. - Ioc: a helper class to configure dependency injection service containers. UWP Toolkit Windows MVVM MVVMToolkit observable Ioc dependency injection services extensions helpers From fefb49948c973736516dbf8b3b888285f7de9343 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 8 Feb 2021 16:16:04 +0100 Subject: [PATCH 20/24] Added custom ValidationContext to ObservableValidator --- .../ComponentModel/ObservableValidator.cs | 49 ++++++++++++++++--- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index fea68625123..d33326e5e55 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -30,6 +30,11 @@ public abstract class ObservableValidator : ObservableObject, INotifyDataErrorIn /// private static readonly PropertyChangedEventArgs HasErrorsChangedEventArgs = new(nameof(HasErrors)); + /// + /// The instance currenty in use. + /// + private readonly ValidationContext validationContext; + /// /// The instance used to store previous validation results. /// @@ -45,6 +50,36 @@ public abstract class ObservableValidator : ObservableObject, INotifyDataErrorIn /// public event EventHandler? ErrorsChanged; + /// + /// Initializes a new instance of the class. + /// This constructor will create a new that will + /// be used to validate all properties, which will reference the current instance + /// and no additional services or validation properties and settings. + /// + protected ObservableValidator() + { + this.validationContext = new ValidationContext(this); + } + + /// + /// Initializes a new instance of the class. + /// This constructor will store the input instance, + /// and it will use it to validate all properties for the current viewmodel. + /// + /// + /// The instance to use to validate properties. + /// + /// This instance will be passed to all + /// calls executed by the current viewmodel, and its property will be updated every time + /// before the call is made to set the name of the property being validated. The property name will not be reset after that, so the + /// value of will always indicate the name of the last property that was validated, if any. + /// + /// + protected ObservableValidator(ValidationContext validationContext) + { + this.validationContext = validationContext; + } + /// public bool HasErrors => this.totalErrors > 0; @@ -458,10 +493,9 @@ protected void ValidateProperty(object? value, [CallerMemberName] string? proper } // Validate the property, by adding new errors to the existing list - bool isValid = Validator.TryValidateProperty( - value, - new ValidationContext(this, null, null) { MemberName = propertyName }, - propertyErrors); + this.validationContext.MemberName = propertyName; + + bool isValid = Validator.TryValidateProperty(value, this.validationContext, propertyErrors); // Update the shared counter for the number of errors, and raise the // property changed event if necessary. We decrement the number of total @@ -529,10 +563,9 @@ private bool TryValidateProperty(object? value, string? propertyName, out IReadO List localErrors = new(); // Validate the property, by adding new errors to the local list - bool isValid = Validator.TryValidateProperty( - value, - new ValidationContext(this, null, null) { MemberName = propertyName }, - localErrors); + this.validationContext.MemberName = propertyName; + + bool isValid = Validator.TryValidateProperty(value, this.validationContext, 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. From 69b133c87c8cde417a76a50bc23d96666550bd4b Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 8 Feb 2021 19:12:30 +0100 Subject: [PATCH 21/24] Tweaked ObservableValidator constructors, added tests --- .../ComponentModel/ObservableValidator.cs | 23 +++++++++ .../Mvvm/Test_ObservableValidator.cs | 49 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index d33326e5e55..06ba5c51ad5 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -61,6 +61,29 @@ protected ObservableValidator() this.validationContext = new ValidationContext(this); } + /// + /// Initializes a new instance of the class. + /// This constructor will create a new that will + /// be used to validate all properties, which will reference the current instance. + /// + /// A set of key/value pairs to make available to consumers. + protected ObservableValidator(IDictionary items) + { + this.validationContext = new ValidationContext(this, items); + } + + /// + /// Initializes a new instance of the class. + /// This constructor will create a new that will + /// be used to validate all properties, which will reference the current instance. + /// + /// An instance to make available during validation. + /// A set of key/value pairs to make available to consumers. + protected ObservableValidator(IServiceProvider serviceProvider, IDictionary items) + { + this.validationContext = new ValidationContext(this, serviceProvider, items); + } + /// /// Initializes a new instance of the class. /// This constructor will store the input instance, diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 9dc0564881e..f93636523d4 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -339,6 +339,22 @@ public void Test_ObservableValidator_ValidateAllProperties() Assert.IsTrue(events.Any(e => e.PropertyName == nameof(Person.Age))); } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableValidator_CustomValidation() + { + var items = new Dictionary { [nameof(CustomValidationModel.A)] = 42 }; + var model = new CustomValidationModel(items); + var events = new List(); + + model.ErrorsChanged += (s, e) => events.Add(e); + + model.A = 10; + + Assert.IsFalse(model.HasErrors); + Assert.AreEqual(events.Count, 0); + } + public class Person : ObservableValidator { private string name; @@ -444,5 +460,38 @@ protected override ValidationResult IsValid(object value, ValidationContext vali return new ValidationResult("The current value is smaller than the other one"); } } + + /// + /// Test model for custom validation properties. + /// See https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/3729 for the original request for this feature. + /// + public class CustomValidationModel : ObservableValidator + { + public CustomValidationModel(IDictionary items) + : base(items) + { + } + + private int a; + + [CustomValidation(typeof(CustomValidationModel), nameof(ValidateA))] + public int A + { + get => this.a; + set => SetProperty(ref this.a, value, true); + } + + public static ValidationResult ValidateA(int x, ValidationContext context) + { + Assert.AreEqual(context.MemberName, nameof(A)); + + if ((int)context.Items[nameof(A)] == 42) + { + return ValidationResult.Success; + } + + return new ValidationResult("Missing the magic number"); + } + } } } From ae01304b8730f4f2d712c0c339d015b8792c057f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 8 Feb 2021 20:13:15 +0100 Subject: [PATCH 22/24] Added missing nullability annotations --- Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs | 4 ++-- Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 06ba5c51ad5..4ab7418b1ec 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -67,7 +67,7 @@ protected ObservableValidator() /// be used to validate all properties, which will reference the current instance. /// /// A set of key/value pairs to make available to consumers. - protected ObservableValidator(IDictionary items) + protected ObservableValidator(IDictionary items) { this.validationContext = new ValidationContext(this, items); } @@ -79,7 +79,7 @@ protected ObservableValidator(IDictionary items) /// /// An instance to make available during validation. /// A set of key/value pairs to make available to consumers. - protected ObservableValidator(IServiceProvider serviceProvider, IDictionary items) + protected ObservableValidator(IServiceProvider serviceProvider, IDictionary items) { this.validationContext = new ValidationContext(this, serviceProvider, items); } diff --git a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs index 9f5ca7b5ad8..5da0c5e743d 100644 --- a/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs +++ b/Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs @@ -408,7 +408,7 @@ public bool MoveNext() { // Get the key and value for the current node if (node.Value.TryGetTarget(out TKey? target) && - this.owner.table.TryGetValue(target!, out TValue value)) + this.owner.table.TryGetValue(target!, out TValue? value)) { this.node = node; this.current = new KeyValuePair(target, value); From 2e10189d4da3e536915c68cd6a0c8a418f072644 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 9 Feb 2021 22:36:36 +0100 Subject: [PATCH 23/24] Switched to LINQ expressions for ValidateAllProperties --- .../ComponentModel/ObservableValidator.cs | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index 4ab7418b1ec..a7614d38264 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -9,6 +9,7 @@ using System.ComponentModel.DataAnnotations; using System.Diagnostics.Contracts; using System.Linq; +using System.Linq.Expressions; using System.Reflection; using System.Runtime.CompilerServices; @@ -21,9 +22,9 @@ namespace Microsoft.Toolkit.Mvvm.ComponentModel public abstract class ObservableValidator : ObservableObject, INotifyDataErrorInfo { /// - /// The instance used to track properties to validate for a given viewmodel type. + /// The instance used to track compiled delegates to validate entities. /// - private static readonly ConditionalWeakTable ValidatableProperties = new(); + private static readonly ConditionalWeakTable> EntityValidatorMap = new(); /// /// The cached for . @@ -458,26 +459,49 @@ IEnumerable GetAllErrors() /// protected void ValidateAllProperties() { - // Helper method to discover all the properties to validate in the current viewmodel type - static PropertyInfo[] GetValidatableProperties(Type type) + static Action GetValidationAction(Type type) { - return ( + // MyViewModel inst0 = (MyViewModel)arg0; + ParameterExpression arg0 = Expression.Parameter(typeof(object)); + UnaryExpression inst0 = Expression.Convert(arg0, type); + + // Get a reference to ValidateProperty(object, string) + MethodInfo validateMethod = typeof(ObservableValidator).GetMethod(nameof(ValidateProperty), BindingFlags.Instance | BindingFlags.NonPublic)!; + + // We want a single compiled LINQ expression that validates all properties in the + // actual type of the executing viewmodel at once. We do this by creating a block + // expression with the unrolled invocations of all properties to validate. + // Essentially, the body will contain the following code: + // =============================================================================== + // { + // inst0.ValidateProperty(inst0.Property0, nameof(MyViewModel.Property0)); + // inst0.ValidateProperty(inst0.Property1, nameof(MyViewModel.Property1)); + // ... + // } + // =============================================================================== + // We also add an explicit object conversion to represent boxing, if a given property + // is a value type. It will just be a no-op if the value is a reference type already. + // Note that this generated code is technically accessing a protected method from + // ObservableValidator externally, but that is fine because IL doesn't really have + // a concept of member visibility, that's purely a C# build-time feature. + BlockExpression body = Expression.Block( from property in type.GetProperties(BindingFlags.Instance | BindingFlags.Public) where property.GetIndexParameters().Length == 0 && property.GetCustomAttributes(true).Any() - select property).ToArray(); + let getter = property.GetMethod + where getter is not null + select Expression.Call(inst0, validateMethod, new Expression[] + { + Expression.Convert(Expression.Call(inst0, getter), typeof(object)), + Expression.Constant(property.Name) + })); + + return Expression.Lambda>(body, arg0).Compile(); } // Get or compute the cached list of properties to validate. Here we're using a static lambda to ensure the // delegate is cached by the C# compiler, see the related issue at https://github.com/dotnet/roslyn/issues/5835. - PropertyInfo[] propertyInfos = ValidatableProperties.GetValue(GetType(), static t => GetValidatableProperties(t)); - - foreach (PropertyInfo propertyInfo in propertyInfos) - { - object? propertyValue = propertyInfo.GetValue(this); - - ValidateProperty(propertyValue, propertyInfo.Name); - } + EntityValidatorMap.GetValue(GetType(), static t => GetValidationAction(t))(this); } /// From 4792ea2f514a2e8243fda34ce6443d2224c43bb1 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 11 Feb 2021 00:47:19 +0100 Subject: [PATCH 24/24] Added unit test for validation with an injected service --- .../Mvvm/Test_ObservableValidator.cs | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index f93636523d4..615c659af6f 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -7,6 +7,7 @@ using System.ComponentModel; using System.ComponentModel.DataAnnotations; using System.Linq; +using System.Text.RegularExpressions; using Microsoft.Toolkit.Mvvm.ComponentModel; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -355,6 +356,42 @@ public void Test_ObservableValidator_CustomValidation() Assert.AreEqual(events.Count, 0); } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableValidator_CustomValidationWithInjectedService() + { + var model = new ValidationWithServiceModel(new FancyService()); + var events = new List(); + + model.ErrorsChanged += (s, e) => events.Add(e); + + model.Name = "This is a valid name"; + + Assert.IsFalse(model.HasErrors); + Assert.AreEqual(events.Count, 0); + + model.Name = "This is invalid238!!"; + + Assert.IsTrue(model.HasErrors); + Assert.AreEqual(events.Count, 1); + Assert.AreEqual(events[0].PropertyName, nameof(ValidationWithServiceModel.Name)); + Assert.AreEqual(model.GetErrors(nameof(ValidationWithServiceModel.Name)).ToArray().Length, 1); + + model.Name = "This is valid but it is too long so the validation will fail anyway"; + + Assert.IsTrue(model.HasErrors); + Assert.AreEqual(events.Count, 2); + Assert.AreEqual(events[1].PropertyName, nameof(ValidationWithServiceModel.Name)); + Assert.AreEqual(model.GetErrors(nameof(ValidationWithServiceModel.Name)).ToArray().Length, 1); + + model.Name = "This is both too long and it also contains invalid characters, a real disaster!"; + + Assert.IsTrue(model.HasErrors); + Assert.AreEqual(events.Count, 3); + Assert.AreEqual(events[2].PropertyName, nameof(ValidationWithServiceModel.Name)); + Assert.AreEqual(model.GetErrors(nameof(ValidationWithServiceModel.Name)).ToArray().Length, 2); + } + public class Person : ObservableValidator { private string name; @@ -493,5 +530,54 @@ public static ValidationResult ValidateA(int x, ValidationContext context) return new ValidationResult("Missing the magic number"); } } + + public interface IFancyService + { + bool Validate(string name); + } + + public class FancyService : IFancyService + { + public bool Validate(string name) + { + return Regex.IsMatch(name, @"^[A-Za-z ]+$"); + } + } + + /// + /// Test model for custom validation with an injected service. + /// See https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/3750 for the original request for this feature. + /// + public class ValidationWithServiceModel : ObservableValidator + { + private readonly IFancyService service; + + public ValidationWithServiceModel(IFancyService service) + { + this.service = service; + } + + private string name; + + [MaxLength(25, ErrorMessage = "The name is too long")] + [CustomValidation(typeof(ValidationWithServiceModel), nameof(ValidateName))] + public string Name + { + get => this.name; + set => SetProperty(ref this.name, value, true); + } + + public static ValidationResult ValidateName(string name, ValidationContext context) + { + bool isValid = ((ValidationWithServiceModel)context.ObjectInstance).service.Validate(name); + + if (isValid) + { + return ValidationResult.Success; + } + + return new ValidationResult("The name contains invalid characters"); + } + } } }