usage of ValidationContext in ValidateActionParametersAttribute #2
Conversation
|
Oh, I've forgot to fix tests. I'll try to fix it this week |
| validationContext.ObjectInstance, | ||
| validationContext); | ||
|
|
||
| if (!ReferenceEquals(validationResult, ValidationResult.Success)) |
There was a problem hiding this comment.
Please use just simple !=, that's easier to read, plus the framework seems to be using it too (for example here: https://github.com/aspnet/Mvc/blob/24eaa740f5b1736700d8d91053f60d690f4fc17e/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/ValidatableObjectAdapter.cs#L51).
| context.ActionArguments[parameter.Name] : null; | ||
|
|
||
| EvaluateValidationAttributes(parameter, argument, context.ModelState); | ||
| var argument = executingContext.ActionArguments[parameter.Name]; |
There was a problem hiding this comment.
Did you actually test that this doesn't throw if the collection doesn't contain this key? (I don't know what the runtime type of ActionArguments is, but if it's a simple Dictionary<K, V>, then it's supposed to throw.)
If it can throw, the I'd prefer to have a proper check here, even if it shouldn't happen in practice.
|
Hey @LutsenkoKirill, |
Hi Mark!
I've found that custom model validation attributes sometimes override IsValid method with two parameters:
object valueandValidationContext validationContext. That's why I decided to add following improvements in ValidateActionParametersAttribute.Hope you'll find it helpful.
Anyway, since ASP.Net Core 2.1 we will no longer need this attribute: https://andrewlock.net/coming-in-asp-net-core-2-1-top-level-mvc-parameter-validation/