diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index a7614d38264..58bf60577d1 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -26,6 +26,18 @@ public abstract class ObservableValidator : ObservableObject, INotifyDataErrorIn /// private static readonly ConditionalWeakTable> EntityValidatorMap = new(); + /// + /// The instance used to track display names for properties to validate. + /// + /// + /// This is necessary because we want to reuse the same instance for all validations, but + /// with the same behavior with repsect to formatted names that new instances would have provided. The issue is that the + /// property is not refreshed when we set , + /// so we need to replicate the same logic to retrieve the right display name for properties to validate and update that + /// property manually right before passing the context to and proceed with the normal functionality. + /// + private static readonly ConditionalWeakTable> DisplayNamesMap = new(); + /// /// The cached for . /// @@ -68,7 +80,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); } @@ -80,7 +92,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); } @@ -541,6 +553,7 @@ protected void ValidateProperty(object? value, [CallerMemberName] string? proper // Validate the property, by adding new errors to the existing list this.validationContext.MemberName = propertyName; + this.validationContext.DisplayName = GetDisplayNameForProperty(propertyName!); bool isValid = Validator.TryValidateProperty(value, this.validationContext, propertyErrors); @@ -611,6 +624,7 @@ private bool TryValidateProperty(object? value, string? propertyName, out IReadO // Validate the property, by adding new errors to the local list this.validationContext.MemberName = propertyName; + this.validationContext.DisplayName = GetDisplayNameForProperty(propertyName!); bool isValid = Validator.TryValidateProperty(value, this.validationContext, localErrors); @@ -688,6 +702,36 @@ private void ClearErrorsForProperty(string propertyName) ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(propertyName)); } + /// + /// Gets the display name for a given property. It could be a custom name or just the property name. + /// + /// The target property name being validated. + /// The display name for the property. + private string GetDisplayNameForProperty(string propertyName) + { + static Dictionary GetDisplayNames(Type type) + { + Dictionary displayNames = new(); + + foreach (PropertyInfo property in type.GetProperties(BindingFlags.Instance | BindingFlags.Public)) + { + if (property.GetCustomAttribute() is DisplayAttribute attribute && + attribute.GetName() is string displayName) + { + displayNames.Add(property.Name, displayName); + } + } + + return displayNames; + } + + // This method replicates the logic of DisplayName and GetDisplayName from the + // ValidationContext class. See the original source in the BCL for more details. + DisplayNamesMap.GetValue(GetType(), static t => GetDisplayNames(t)).TryGetValue(propertyName, out string? displayName); + + return displayName ?? propertyName; + } + #pragma warning disable SA1204 /// /// Throws an when a property name given as input is . diff --git a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs index 615c659af6f..cc6463c4826 100644 --- a/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs +++ b/UnitTests/UnitTests.Shared/Mvvm/Test_ObservableValidator.cs @@ -392,6 +392,28 @@ public void Test_ObservableValidator_CustomValidationWithInjectedService() Assert.AreEqual(model.GetErrors(nameof(ValidationWithServiceModel.Name)).ToArray().Length, 2); } + [TestCategory("Mvvm")] + [TestMethod] + public void Test_ObservableValidator_ValidationWithFormattedDisplayName() + { + var model = new ValidationWithDisplayName(); + + Assert.IsTrue(model.HasErrors); + + // We need to order because there is no guaranteed order on the members of a type + ValidationResult[] allErrors = model.GetErrors().OrderBy(error => error.ErrorMessage).ToArray(); + + Assert.AreEqual(allErrors.Length, 2); + + Assert.AreEqual(allErrors[0].MemberNames.Count(), 1); + Assert.AreEqual(allErrors[0].MemberNames.Single(), nameof(ValidationWithDisplayName.StringMayNotBeEmpty)); + Assert.AreEqual(allErrors[0].ErrorMessage, $"FIRST: {nameof(ValidationWithDisplayName.StringMayNotBeEmpty)}."); + + Assert.AreEqual(allErrors[1].MemberNames.Count(), 1); + Assert.AreEqual(allErrors[1].MemberNames.Single(), nameof(ValidationWithDisplayName.AnotherRequiredField)); + Assert.AreEqual(allErrors[1].ErrorMessage, $"SECOND: {nameof(ValidationWithDisplayName.AnotherRequiredField)}."); + } + public class Person : ObservableValidator { private string name; @@ -579,5 +601,35 @@ public static ValidationResult ValidateName(string name, ValidationContext conte return new ValidationResult("The name contains invalid characters"); } } + + /// + /// Test model for validation with a formatted display name string on each property. + /// This is issue #1 from https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/3763. + /// + public class ValidationWithDisplayName : ObservableValidator + { + public ValidationWithDisplayName() + { + ValidateAllProperties(); + } + + private string stringMayNotBeEmpty; + + [Required(AllowEmptyStrings = false, ErrorMessage = "FIRST: {0}.")] + public string StringMayNotBeEmpty + { + get => this.stringMayNotBeEmpty; + set => SetProperty(ref this.stringMayNotBeEmpty, value, true); + } + + private string anotherRequiredField; + + [Required(AllowEmptyStrings = false, ErrorMessage = "SECOND: {0}.")] + public string AnotherRequiredField + { + get => this.anotherRequiredField; + set => SetProperty(ref this.anotherRequiredField, value, true); + } + } } }