NorthwindTraders
NorthwindTraders copied to clipboard
Duplicated fluent validation rules in Application layer?
New .NET and C# guy here, so please take this with a grain of salt.
I've learned a lot from this repository, but one thing that really sticks out for me as a potential code smell are the duplicated fluent validation rules across different commands, e.g. CreateCustomerCommandValidator.cs
and UpdateCustomerCommandValidator.cs
. On a large codebase these duplications could become very hard to keep track of.
Having listened to a few talks and read a fair amount about it, I can get why one would want to avoid inline attribute based validations, but at the same time, centralizing this concern helped avoid the duplication issue.
Is there a somewhat standardized approach to defining these validation constraints while using fluent validation?
One interesting thread I found was directly in the FluentValidation repository: https://github.com/FluentValidation/FluentValidation/issues/484
What I've ended up doing is creating additional custom validators bearing the name of the class and property being validated. I'm adding them through a static class that sits in the same file as the class whose properties are being validated.
This way, I get what I liked about inline attribute validation, which is proximity to the item being validated (class property), while still having the full power of fluent validation.
Here's a quick example:
public class InsuranceCompany
{
public string Name { get; set; }
public string Symbol { get; set; }
}
public static class InsuranceCompanyValidator
{
public static IRuleBuilderOptions<T, string> InsuranceCompanyName<T>(this IRuleBuilder<T, string> ruleBuilder)
{
return ruleBuilder
.NotEmpty()
.MinimumLength(2)
.MaximumLength(80);
}
public static IRuleBuilderOptions<T, string> InsuranceCompanySymbol<T>(this IRuleBuilder<T, string> ruleBuilder)
{
return ruleBuilder
.NotEmpty()
.MinimumLength(2)
.MaximumLength(4);
}
}
By handling properties validation in this way, I can then avoid duplicating the rules in different commands with the same underlying entities, by calling the predefined rules like this:
public class CreateInsuranceCompanyCommandValidator : AbstractValidator<CreateInsuranceCompanyCommand>
{
public CreateInsuranceCompanyCommandValidator()
{
RuleFor(company => company.Name).InsuranceCompanyName();
RuleFor(company => company.Symbol).InsuranceCompanySymbol();
}
}
Doing things this way allows me to avoid the approach I was concerned about, which is duplicating validation rules across several commands/queries or even elsewhere.
However, the downside of using these custom validators is that they become globally available, which becomes increasingly more annoying as your project grows.
While this seems to be (from what I've read at least) the idiomatic way of handling this sort of thing in FluentValidation, I admit it would be great to be able to write a self-contained rules class which can individually be called upon from instances that require the validation of the fields.
Any feedback on any of the above is highly welcome, since as I've mentioned I've been dabbling in .NET Core and C# for just a few months by now.
You can also move the validations to domain layer. Instead of validating the commands, you can just validate the entity.
Hi @sformisano Thanks for this. But I prefer attributes validation instead of fluent validation. Because it is a built-in option. And I can use to automatically generate a Swagger spec from the source code. It is so easy to create custom attributes and add support for these attributes to swagger spec. Could you please provide more details here - why do you prefer a fluent validation?
And ane more thing about request models validation:
The author uses command as a request DTO. But there is one more issue: Command it is not 100% the same as a request.
Sample: UpdateCustomerCommand.cs
has Id property. But you do not need this property in DTO because you already have it here: PUT /customers/{id}
So, what id should I use on the backend?
To solve this issue we can use a separate DTO for the request model. Sample:
public class UpdateCustomerCommand : IRequest
{
public string CustomerId { get; set; }
public UpdateCustomerDto Model { get; set; }
}
public class UpdateCustomerDto
{
[Requred] // and other validation atributes.
public string Address { get; set; }
public string City { get; set; }
// all other properties
}
public class CustomersController
{
[HttpPut("{id}")]
[ProducesResponseType(StatusCodes.Status204NoContent)]
[ProducesResponseType(StatusCodes.Status404NotFound)]
public async Task<IActionResult> Update(int id, [Requred]UpdateCustomerDto dto, CancellationToken cancellationToken)
{
await Mediator.Send(new UpdateCustomerCommand { CustomerId = id, Model = dto }, cancellationToken);
return NoContent();
}
}
And you can create a base class for DTO to avoid code duplication.
public class EditCustomerDtoBase
{
[Requred] // and other validation atributes.
public string Address { get; set; }
public string City { get; set; }
// all other properties
}
public class UpdateCustomerDto : EditCustomerDtoBase
{
// TODO: add custom properties here.
}
public class CreateCustomerDto : EditCustomerDtoBase
{
// TODO: add custom properties here.
}
On the one hand we do not duplicate code. But on the other hand - create and update requests use a common model. And when you change this common model it will affect both requests. But I believe it is not a big deal. What do you think about this approach? Thanks.
@AndreiTsvettsikh what you wrote makes a lot of sense to me, thanks a lot for the clarifications.
I also agree with you that attributes validation might just be better for most scenarios, but it seems many do not particularly like that approach including the author of this repo (and many others online) seem to have.
As I said, I'm new to both C# and .Net so I tend to look for expert opinions.
Hey all. Here's my 2 cents worth. I've been doing this a long time. I find having commands and queries via an absolute win.
We also do use fluent validation. The situation you have described where the DTO is different from the command/model. This becomes even more complicated when you have different versions of an API or endpoint(s).
So depending on the API (we have a number of approaches) we use data annotations to ensure the request/DTO is sound - has the correct data type. Things like the validation of email address perhaps. Where the command is using fluent validation for more complicated things. Sometimes there is also a duplicate. Our fluent validation is a guard against the domain from using bad data or putting the domain in an invalid state. So in our behaviour pipeline, we have an exception thrown. Some of our microservices just end up returning a 500 (with all exception details of the fluent validation errors being logged etc).
Other services use the fluent validation, but as the HTTP response is sent to the client/caller we ensure the error code and property name makes sense to the API version and dto. So that the caller can indeed make a change to then create a successful HTTP request.
Another reason for separating this is that we have a number of situations where a command is used in different ways. So we need to ensure each boundary processes any fluent validation errors as deemed appropriate. For example, we use Rabbit MQ and Http requests to access some of our 'commands' (and queries of course).
To do the converting of fluent errors to something that the client can use I leverage the ''DisplayNameResolver" delegate provided inside fluent validation. See: https://docs.fluentvalidation.net/en/latest/configuring.html#overriding-the-property-name
In our start up process (.net core) we wire up the dependencies to use fluent validation (for web api routes flow) to leverage this by creating a means to add IPropertyNameResolvers, and for each resolver, it handles a different aspect of a name resolution. We have different DI and components when using in the context of a rabbit mq message (for example). Same code different DI rules.
public static class Extensions { public static IServiceCollection AddCommandHandlerFluentValidation( this IServiceCollection services) { services.AddFluentValidation(new[] { typeof(CreateJob).Assembly });
return services
.AddSingleton<IPropertyNameResolver, EndsOnNameResolver>()
.AddSingleton<IPropertyNameResolver,
PollingFrequencyNameResolver>() .AddSingleton<IPropertyNameResolver, StartsFromNameResolver
(); }
public static IApplicationBuilder UserCommandHandlerFluentValidation
(this IApplicationBuilder app) { var resolvers = app.ApplicationServices.GetServices< IPropertyNameResolver>();
ValidatorOptions.DisplayNameResolver = (type, member, expression
) => resolvers .Select(resolver => resolver.Resolve(member)) .FirstOrDefault(resolvedName => resolvedName != null);
return app;
}
}
So far we have not had any issues and the systems we are building needs to be able to scale and handle large volumes of throughput.
Happy to chat further if required.
Cheers, Shane
On Wed, Jun 17, 2020 at 6:36 PM AndreiTsvettsikh [email protected] wrote:
Hi @sformisano https://github.com/sformisano Thanks for this. But I prefer attributes validation instead of fluent validation. Because it is a built-in option. And I can use to automatically generate a Swagger spec from the source code. It is so easy to create custom attributes and add support for these attributes to swagger spec. Could you please provide more details here - why do you prefer a fluent validation?
And ane more thing about request models validation: The author uses command as a request DTO. But there is one more issue: Command it is not 100% the same as a request. Sample: UpdateCustomerCommand.cs has Id property. But you do not need this property in DTO because you already have it here: PUT /customers/{id} So, what id should I use on the backend?
To solve this issue we can use a separate DTO for the request model. Sample:
public class UpdateCustomerCommand : IRequest { public string CustomerId { get; set; } public UpdateCustomerDto Model { get; set; } } public class UpdateCustomerDto { [Requred] // and other validation atributes. public string Address { get; set; } public string City { get; set; } // all other properties } public class CustomersController { [HttpPut("{id}")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task<IActionResult> Update(int id, [Requred]UpdateCustomerDto dto, CancellationToken cancellationToken) { await Mediator.Send(new UpdateCustomerCommand { CustomerId = id, Model = dto }, cancellationToken);
return NoContent(); }
}
And you can create a base class for DTO to avoid code duplication.
public class EditCustomerDtoBase { [Requred] // and other validation atributes. public string Address { get; set; } public string City { get; set; } // all other properties } public class UpdateCustomerDto : EditCustomerDtoBase { // TODO: add custom properties here. } public class CreateCustomerDto : EditCustomerDtoBase { // TODO: add custom properties here. }
On the one hand we do not duplicate code. But on the other hand - create and update requests use a common model. And when you change this common model it will affect both requests. But I believe it is not a big deal. What do you think about this approach? Thanks.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jasontaylordev/NorthwindTraders/issues/256#issuecomment-645181771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIF5WLKUO3KXSCCSIUXIRTRXBQAFANCNFSM4NS73ILA .
To further example to help explain how that works. You can also leverage the current HTTP request using the IHttpContextAccessor to get more details like the current version of the request (V2 etc).
public sealed class EndsOnNameResolver : IPropertyNameResolver { public string? Resolve(MemberInfo memberInfo) { if (memberInfo != null) { if (memberInfo.Name == nameof(Job.EndsOn)) { return nameof(JobRequestDto.CollectionStop); } }
return null;
}
}
@shanerogers Thanks for this!
But I want to clary a couple of implementation details if your app:
- Do you use Swagger for your microservices?
- We do not have any problems with API versioning when DTO is different from command/queries. Could you please provide more details - when it can be a problem?
Thanks.
Thank you for your interest in this project. This repository has been archived and is no longer actively maintained or supported. We appreciate your understanding. Feel free to explore the codebase and adapt it to your own needs if it serves as a useful reference. If you have any further questions or concerns, please refer to the README for more information.