Validation failure should include contextual data #146
Description
Liquid uses Fluent Validation to validate data coming from the client. Unfortunately, it doesn't take full advantage of the power this library provides us with.
The Framework has an input validation middleware built in. This middleware catches every InvalidInputException
and immediately respond with a Bad Request:
This middleware is used in conjunction with LightController
's ValidateInput<T>
and Factory<T>
methods. The former populates an internal list with errors from the view model and the latter throws if that list contains anything:
The current implementation of input validation only includes the error code, but this is not enough since frontend clients frequently need to map these failures to input fields an the UI to improve the user experience (and are extremely helpful for debugging purposes). It's hiding essential information.
Let's take an example. Consider these view models:
public class TestVM : LightViewModel<TestVM>
{
public string StringProperty { get; set; }
public NestedVM Nested { get; set; }
public IEnumerable<NestedVM> NestedList { get; set; }
public override void Validate()
{
RuleFor(x => x.StringProperty).NotEmpty().WithErrorCode("ERR_STRING_PROPERTY_REQUIRED");
}
}
public class NestedVM : LightViewModel<NestedVM>
{
public string StringProperty { get; set; }
public override void Validate()
{
RuleFor(x => x.StringProperty).NotEmpty().WithErrorCode("ERR_STRING_PROPERTY_REQUIRED");
}
}
and this request body:
{
"nested": {
},
"nestedList": [
{
}
]
}
If we'd validate it, the Framework would return this:
{
"critics": [
{
"code": "ERR_STRING_PROPERTY_REQUIRED",
"message": "ERR_STRING_PROPERTY_REQUIRED",
"type": "Error"
},
{
"code": "ERR_STRING_PROPERTY_REQUIRED",
"message": "ERR_STRING_PROPERTY_REQUIRED",
"type": "Error"
},
{
"code": "ERR_STRING_PROPERTY_REQUIRED",
"message": "ERR_STRING_PROPERTY_REQUIRED",
"type": "Error"
}
]
}
Since those messages are identical, how am I supposed to differentiate failures? It may look innocent, but it actually increased our pre-production troubleshooting as we had lots of properties (including nested ones) with the same name.
FluentValidation has a ValidationFailure class that contains metadata about the property being validated. Just look at how it format errors:
{
"isValid": false,
"errors": [
{
"propertyName": "StringProperty",
"errorMessage": "'String Property' must not be empty.",
"attemptedValue": null,
"customState": null,
"severity": 0,
"errorCode": "ERR_STRING_PROPERTY_REQUIRED",
"formattedMessageArguments": [],
"formattedMessagePlaceholderValues": {
"PropertyName": "String Property",
"PropertyValue": null
},
"resourceName": "NotEmptyValidator"
},
{
"propertyName": "Nested.StringProperty",
"errorMessage": "'Nested. String Property' must not be empty.",
"attemptedValue": null,
"customState": null,
"severity": 0,
"errorCode": "ERR_STRING_PROPERTY_REQUIRED",
"formattedMessageArguments": [],
"formattedMessagePlaceholderValues": {
"PropertyName": "Nested. String Property",
"PropertyValue": null
},
"resourceName": "NotEmptyValidator"
},
{
"propertyName": "NestedList[0].StringProperty",
"errorMessage": "'String Property' must not be empty.",
"attemptedValue": null,
"customState": null,
"severity": 0,
"errorCode": "ERR_STRING_PROPERTY_REQUIRED",
"formattedMessageArguments": [],
"formattedMessagePlaceholderValues": {
"PropertyName": "String Property",
"PropertyValue": null
},
"resourceName": "NotEmptyValidator"
}
],
"ruleSetsExecuted": [
"default"
]
}
It's much more detailed and I can clearly see what's going on. I suggest that we should be doing this as well.