Register Validators as scoped instances rather than singleton

[mysticmind]

I had a setup a validation as below wherein RoleManager via constructor injection is a scoped instance and this resulted in an error Cannot consume scoped service in Singleton (validators are registered as Singleton within CarterExtensions.cs).

public class CommandValidator : AbstractValidator<Command>
{
    public CommandValidator(RoleManager<UserRole> roleManager)
    {
        RuleFor(c => c.Name).CustomAsync(async (name, context, cancel) =>
        {
            if (string.IsNullOrEmpty(name))
            {
                context.AddFailure("Role name is required");
            }
            else
            {
                var role = await roleManager.FindByNameAsync(name);

                if (role != null)
                {
                    context.AddFailure("Role name already exists");
                }
            }
        });
    }
}

It would be good to register validators as scoped instances.

@jchannon if you could confirm on this change, I would be happy to send my first PR to Carter :-)

[JoeStead]

It's probably best to make this configurable, in the cases where you can have validators as a singleton, that would be preferable because it will have an impact on performance. There have been other discussions around the use of the container / how things are registered, and I imagine those discussions will continue for a while :)

[mysticmind]

@JoeStead understand your comments around requiring it to be configurable. Let me get my head around it and see if I can make it configurable. Apparently I did send a PR to change it to scoped, I think it can be ignored.

[damianh]

@mysticmind Looks like you are doing a bit of CQRS there. You have a race condition in your validator and your domain logic is leaking - two concurrent requests with the same name will succeed the FindByNameAsync() but then you'll get duplicate / exception in next layer (domain or db). Leave the set based concerns at domain / db level where you can have strong consistency guarantees.

I would strongly advise that message validators, your first line of defense after auth, work exclusively with the message and not have any dependencies on services. (There are always exceptions OC).

My understanding is that fluent validator objects are quite expensive to instantiate due to compiled expressions etc as such should be singleton wrt to the life time of your app (I actually use static instances). I might be out of date with this understanding, but if I'm correct, the only time you might need instances if there is a some sort setting adjustable at runtime.

[mysticmind]

@damianh All your points makes sense and totally agree that fluent validator objects are quite expensive to instantiate. Besides it is best to have clear separation of concerns as you rightly pointed out.

I would strongly advise that message validators, your first line of defense after auth, work exclusively with the message and not have any dependencies on services. (There are always exceptions OC)

This is a good rule of thumb!

Also @JoeStead indicated in earlier comment around the impact on performance.

I am closing this.

[dcernach]

The ability to change the lifecycle of the registration using the DI container, in my opinion, is crucial; otherwise, it would render the use of Carter.NET in applications that are being migrated to minimal APIs unfeasible. It is quite common to have validators coupled with some sort of database access. For example, when checking the uniqueness of an email address or some other value. I understand that FluentValidation objects are quite expensive to instantiate, but in my case, and I believe in the case of others, it's a price we are willing to pay.

The automatic registration of all validators in an application as Singletons becomes a deal-breaker, even affecting previous code, as it prevents the application from starting due to errors generated during the build of the DI container. One possibility would be to disable the automatic registration of validators, along with the ability to validate the request directly at the endpoint, and leave it to the developer to manually execute the validation procedure.

[jchannon]

https://github.com/CarterCommunity/Carter/pull/324