Fix the ValidatingModelBase, in the face of WPF asking for model-wide errors

Fixes #11

Author: canton7

Stylet/IValidationAdapter.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 
 namespace Stylet
@@ -36,14 +37,16 @@ public interface IModelValidator
         /// <summary>
         /// Validate a single property by name, and return an array of validation errors for that property (or null if validation was successful)
         /// </summary>
-        /// <param name="propertyName">Property to validate</param>
-        /// <returns>Array of validation errors, or null if validation was successful</returns>
+        /// <param name="propertyName">Property to validate, or <see cref="String.Empty"/> to validate the entire model</param>
+        /// <returns>Array of validation errors, or null / empty if validation was successful</returns>
         Task<IEnumerable<string>> ValidatePropertyAsync(string propertyName);
 
         /// <summary>
         /// Validate all properties, and return the results for all properties
         /// </summary>
         /// <remarks>
+        /// Use a key of <see cref="String.Empty"/> to indicate validation errors for the entire model.
+        /// 
         /// If a property validates successfully, you MUST return a null entry for it in the returned dictionary!
         /// </remarks>
         /// <returns>A dictionary of property name => array of validation errors (or null if that property validated successfully)</returns>

Stylet/ValidatingModelBase.cs

@@ -100,13 +100,14 @@ protected virtual async Task<bool> ValidateAsync()
             if (this.Validator == null)
                 throw new InvalidOperationException("Can't run validation if a validator hasn't been set");
 
-            bool anyChanged = false;
-
             // We need the ConfigureAwait(false), as we might be called synchronously
             // However this means that the stuff after the await can be run in parallel on multiple threads
             // Therefore, we need the lock
             // However, we can't raise PropertyChanged events from within the lock, otherwise deadlock
             var results = await this.Validator.ValidateAllPropertiesAsync().ConfigureAwait(false);
+            if (results == null)
+                results = new Dictionary<string, IEnumerable<string>>();
+
             var changedProperties = new List<string>();
             await this.propertyErrorsLock.WaitAsync().ConfigureAwait(false);
             {
@@ -119,21 +120,19 @@ protected virtual async Task<bool> ValidateAsync()
                         continue;
                     else
                         this.propertyErrors[kvp.Key] = newErrors;
-                    anyChanged = true;
                     changedProperties.Add(kvp.Key);
                 }
 
                 // If they haven't included a key in their validation results, that counts as no validation error
                 foreach (var removedKey in this.propertyErrors.Keys.Except(results.Keys).ToArray())
                 {
                     this.propertyErrors[removedKey] = null;
-                    anyChanged = true;
                     changedProperties.Add(removedKey);
                 }
             }
             this.propertyErrorsLock.Release();
 
-            if (anyChanged)
+            if (changedProperties.Count > 0)
                 this.OnValidationStateChanged(changedProperties);
 
             return !this.HasErrors;
@@ -182,16 +181,16 @@ protected bool ValidateProperty([CallerMemberName] string propertyName = null)
         /// <summary>
         /// Validate a single property asynchronously, by name.
         /// </summary>
-        /// <param name="propertyName">Property to validate. Validates all properties if null or String.Empty</param>
+        /// <param name="propertyName">Property to validate. Validates the entire model if null or <see cref="String.Empty"/></param>
         /// <returns>True if the property validated successfully</returns>
         /// <remarks>If you override this, you MUST fire ErrorsChanged and call OnValidationStateChanged() if appropriate</remarks>
         protected virtual async Task<bool> ValidatePropertyAsync([CallerMemberName] string propertyName = null)
         {
             if (this.Validator == null)
                 throw new InvalidOperationException("Can't run validation if a validator hasn't been set");
 
-            if (String.IsNullOrEmpty(propertyName))
-                return await this.ValidateAsync().ConfigureAwait(false);
+            if (propertyName == null)
+                propertyName = String.Empty;
 
             // To allow synchronous calling of this method, we need to resume on the ThreadPool.
             // Therefore, we might resume on any thread, hence the need for a lock
@@ -234,12 +233,12 @@ protected override async void OnPropertyChanged(string propertyName)
         }
 
         /// <summary>
-        /// Called whenever the error state of any properties changes. Calls NotifyOfPropertyChange(() => this.HasErrors) by default
+        /// Called whenever the error state of any properties changes. Calls NotifyOfPropertyChange("HasErrors") by default
         /// </summary>
         /// <param name="changedProperties">List of property names which have changed validation state</param>
         protected virtual void OnValidationStateChanged(IEnumerable<string> changedProperties)
         {
-            this.NotifyOfPropertyChange(() => this.HasErrors);
+            this.NotifyOfPropertyChange("HasErrors");
             foreach (var property in changedProperties)
             {
                 this.RaiseErrorsChanged(property);
@@ -264,14 +263,16 @@ protected virtual void RaiseErrorsChanged(string propertyName)
         /// <returns>The validation errors for the property or entity.</returns>
         public virtual IEnumerable GetErrors(string propertyName)
         {
-            string[] errors = null;
+            string[] errors;
+
+            if (propertyName == null)
+                propertyName = String.Empty;
 
             // We'll just have to wait synchronously for this. Oh well. The lock shouldn't be long.
             // Everything that awaits uses ConfigureAwait(false), so we shouldn't deadlock if someone calls this on the main thread
             this.propertyErrorsLock.Wait();
             {
-                if (this.propertyErrors.ContainsKey(propertyName))
-                    errors = this.propertyErrors[propertyName];
+                this.propertyErrors.TryGetValue(propertyName, out errors);
             }
             this.propertyErrorsLock.Release();
 

StyletUnitTests/ValidatingModelBaseTests.cs

@@ -137,19 +137,19 @@ public void ValidatePropertyAsyncByNameCallsAdapterValidate()
         }
 
         [Test]
-        public void ValidatePropertyAsyncWithNullCallsAdapterValidate()
+        public void ValidatePropertyAsyncWitNullCallsAdapterValidatePropertyWithEmptyString()
         {
-            this.validator.Setup(x => x.ValidateAllPropertiesAsync()).ReturnsAsync(new Dictionary<string, IEnumerable<string>>()).Verifiable();
-            this.model.ValidatePropertyAsync(null).Wait();
+            this.validator.Setup(x => x.ValidatePropertyAsync(String.Empty)).ReturnsAsync(Enumerable.Empty<string>()).Verifiable();
+            this.model.ValidatePropertyAsync(String.Empty).Wait();
 
             this.validator.Verify();
         }
 
 
         [Test]
-        public void ValidatePropertyAsyncWithEmptyStringCallsAdapterValidate()
+        public void ValidatePropertyAsyncWithEmptyStringCallsAdapterValidatePropertyWithEmptyString()
         {
-            this.validator.Setup(x => x.ValidateAllPropertiesAsync()).ReturnsAsync(new Dictionary<string, IEnumerable<string>>()).Verifiable();
+            this.validator.Setup(x => x.ValidatePropertyAsync(String.Empty)).ReturnsAsync(Enumerable.Empty<string>()).Verifiable();
             this.model.ValidatePropertyAsync(String.Empty).Wait();
 
             this.validator.Verify();
@@ -342,6 +342,19 @@ public void GetErrorsReturnsErrorsForProperty()
             Assert.That(errors, Is.EquivalentTo(new[] { "error1", "error2" }));
         }
 
+        [Test]
+        public void GetErrorsWithNullReturnsModelErrors()
+        {
+            this.validator.Setup(x => x.ValidateAllPropertiesAsync()).ReturnsAsync(new Dictionary<string, IEnumerable<string>>()
+            {
+                { "", new[] { "error1", "error2" } }
+            });
+
+            this.model.Validate();
+            var errors = this.model.GetErrors(null);
+            Assert.That(errors, Is.EquivalentTo(new[] { "error1", "error2" }));
+        }
+
         [Test]
         public void SettingPropertyValidatesIfAutoValidateIsTrue()
         {