CommunityToolkit / CommunityToolkit/dotnet
Refactor ObservableValidator to be AOT Compatible
- Dominant language
- C#
- Stars
- 3.8k
- Forks
- 400
- PR merge metrics
- No merged PRs in 30d
Description
### Overview
I am primarily using the MVVM Toolkit for its AOT compatibility. I love using it, and it works great! However, the functionality falls apart when using the `ObservableValidator`. Even though you can manually annotate your ViewModels in a way that properties are preserved and then suppress the warnings, you know are now irrelevant, this process is tedious and error-prone. Therefore, I would like to improve the implementation of the ObservableValidator.
Basically, as far as I can see, the current implementation has trimming/dynamic code annotations for
- Constructing a new ObservableValidator (The ValidationContext when it's constructed)
- Validating a new property (The Validator.TryValidateProperty searches for attributes on a property that it has not seen before / the ValidationContext needs DisplayName / MemberName)
- The ValidateAll method tries to find all properties to validate (there's already a source generator that addresses this in theory)
The goal would be to have an `ObservableValidator` that no longer needs any of these annotations.
The base assumptions, I have:
1. Using a ValidationContext is generally safe if DisplayName and MemberName are set manually (to my knowledge, the Options generator relies on this as well)
2. Instead of relying on the internal `ValidationAttributeStore` used by the Validator, we can replace this functionality with a source-generated discovery
3. Instead of just providing the fast-path with a source generator, we only provide the source-generated version
4. The ObservableValidator source generation should be able to live in parallel to the existing ObservableProperty source generation, since the `ValidateProperty` methods are independent
### API breakdown
The API would be identical to the current version, except that every class inheriting the `ObservableValidator` would need to be `partial`.
Internally, I see multiple different options for the code emitted by the generator. Right now, my proposal would be virtual methods for `TryValidatePropertyCore` and `ValidateAllPropertiesCore` which are chained when inheritance comes into play.
Attributes could be discovered reflection and a helper `GetValidationAttributes` method.
### Usage example
```csharp
public partial class MyTestViewModel : ObservableValidator
{
private string? userName;
private string? password;
// Could also be a partial method/field annotated with [ObservableProperty] with the get/set pattern auto-generated by the other source generator
[Required(ErrorMessage = "Missing {0}.")]
[Display(Name = "User Name")]
[StringLength(20, MinimumLength = 3)]
public string? UserName
{
get => this.userName;
set => SetProperty(ref this.userName, value, validate: true);
}
[Required]
[StringLength(20, MinimumLength = 3)]
public string? Password
{
get => this.password;
set => SetProperty(ref this.password, value, validate: true);
}
}
public sealed partial class SecondTestViewModel : MyTestViewModel
{
private int? counter;
private double test;
public SecondTestViewModel() => ValidateAllProperties();
[Range(10, 20)]
public int? Counter
{
get => this.counter;
set => SetProperty(ref this.counter, value, validate: true);
}
[CustomValidation(typeof(SecondTestViewModel), nameof(Validate))]
public double Test
{
get => this.test;
set => SetProperty(ref this.test, value, validate: true);
}
public static ValidationResult? Validate(double value, ValidationContext context)
{
SecondTestViewModel instance = (SecondTestViewModel)context.ObjectInstance;
return value < instance.Counter ? ValidationResult.Success : new ValidationResult("Invalid value");
}
}
// Example generation
partial class MyTestViewModel
{
private static readonly DisplayAttribute __UserNameDisplayAttribute =
typeof(MyTestViewModel).GetProperty(nameof(UserName))!.GetCustomAttribute()!;
private static readonly IEnumerable ___UserNameValidationAttributes =
typeof(MyTestViewModel).GetProperty(nameof(UserName))!.GetValidationAttributes();
private static readonly IEnumerable ___PasswordValidationAttributes =
typeof(MyTestViewModel).GetProperty(nameof(Password))!.GetValidationAttributes();
///
protected override void ValidateAllPropertiesCore()
{
ValidateProperty(UserName, nameof(UserName));
ValidateProperty(Password, nameof(Password));
base.ValidateAllPropertiesCore();
}
///
protected override ValidationStatus TryValidatePropertyCore(object? value, string propertyName, ICollection errors)
{
return (propertyName) switch
{
nameof(UserName) => TryValidateValue(value, nameof(UserName), __UserNameDisplayAttribute.GetName() ?? "UserName", ___UserNameValidationAttributes, errors),
nameof(Password) => TryValidateValue(value, nameof(Password), "Password", ___PasswordValidationAttributes, errors),
_ => base.TryValidatePropertyCore(value, propertyName, errors)
};
}
}
partial class SecondTestViewModel
{
private static readonly IEnumerable __CounterValidationAttributes =
typeof(SecondTestViewModel).GetProperty(nameof(Counter))!.GetValidationAttributes();
private static readonly IEnumerable __TestValidationAttributes =
typeof(SecondTestViewModel).GetProperty(nameof(Test))!.GetValidationAttributes();
///
protected override void ValidateAllPropertiesCore()
{
ValidateProperty(Counter, nameof(Counter));
ValidateProperty(Test, nameof(Test));
base.ValidateAllPropertiesCore();
}
///
protected override ValidationStatus TryValidatePropertyCore(object? value, string propertyName, ICollection errors)
{
return (propertyName) switch
{
nameof(Counter) => TryValidateValue(value, nameof(Counter), "Counter", __CounterValidationAttributes, errors),
nameof(Test) => TryValidateValue(value, nameof(Test), "Test", __TestValidationAttributes, errors),
_ => base.TryValidatePropertyCore(value, propertyName, errors)
};
}
}
```
### Breaking change?
I'm not sure
### Alternatives
Alternatives I have thought of (there are probably a lot more):
- Emitting code per class into a helper class (similar to the ValidateAllProperties generator right now) and use reflection to get the type info the first time we are constructing the `ObservableValidator`
- Emitting validation attributes directly instead of using reflection to discover them (the discovery should be saved, but it's still reflection)
Additionally, I have considered just dumping the MVVM toolkit for this and writing a separate package for this. However, if there's a reasonable path to get this into the MVVM toolkit, I would prefer that since its much more difficult to get a decent integration with the ObservableProperty generator when using a separate package.
### Additional context
_No response_
### Help us help you
Yes, I'd like to be assigned to work on this item
Contributor guide
Assessment
This issue has not been assessed yet.