Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow AddFluentValidation to be called directly on IServiceCollection #1726

Closed
Sherif-Ahmed opened this issue May 1, 2021 · 4 comments · Fixed by #1727
Closed

Allow AddFluentValidation to be called directly on IServiceCollection #1726

Sherif-Ahmed opened this issue May 1, 2021 · 4 comments · Fixed by #1727
Milestone

Comments

@Sherif-Ahmed
Copy link
Contributor

Sherif-Ahmed commented May 1, 2021

Is your feature request related to a problem?

Not a problem, but enhancement for easy to use in some situations like OrchardCore.
In OrchardCore I don't have access to IMvcBuilder (I can work around it), but if I have an extension point so that I can call services.AddFluentValidation() it will be good.

Solution

I take a look at the code and figured out that you are just using IMvcBuilder to call mvcBuilder.AddMvcOptions which internally is calling services.Configure<MvcOptions>() so we can have a new extension point that calls directly services.Configure<MvcOptions>()

public static IServiceCollection AddFluentValidation(this IServiceCollection services, Action<FluentValidationMvcConfiguration> configurationExpression = null) {
    var config = new FluentValidationMvcConfiguration(ValidatorOptions.Global);
    configurationExpression?.Invoke(config);

    RegisterServices(services, config);

    services.Configure<MvcOptions>(options => {
        // Check if the providers have already been added.
        // We shouldn't have to do this, but there's a bug in the ASP.NET Core integration
        // testing components that can cause Configureservices to be called multple times
        // meaning we end up with duplicates.
        if (!options.ModelMetadataDetailsProviders.Any(x => x is FluentValidationBindingMetadataProvider)) {
	    options.ModelMetadataDetailsProviders.Add(new FluentValidationBindingMetadataProvider());
        }

        if (!options.ModelValidatorProviders.Any(x => x is FluentValidationModelValidatorProvider)) {
	    options.ModelValidatorProviders.Insert(0, new FluentValidationModelValidatorProvider(
	        config.ImplicitlyValidateChildProperties,
		config.ImplicitlyValidateRootCollectionElements));
        }
    });

    return services;
}
@JeremySkinner
Copy link
Member

Hi, thanks for the suggestion - that seems like a reasonable addition. If you'd like to put together a pull request then I'd be happy to review it. I think the PR would need to do the following:

  • Add the new overload
  • Refactor existing overloads to remove as much duplication as possible
  • Add new integration test that checks that calling services.AddFluentValidation() prior to services.AddMvc() doesn't break anything

@JeremySkinner JeremySkinner added the Status: Awaiting PR Awaiting a pull request. label May 1, 2021
@Sherif-Ahmed
Copy link
Contributor Author

The PR added please verify it.

@JeremySkinner
Copy link
Member

Thanks, I'll aim to do that this evening or tomorrow.

@JeremySkinner JeremySkinner removed the Status: Awaiting PR Awaiting a pull request. label May 1, 2021
@JeremySkinner JeremySkinner added this to the 10.2 milestone May 1, 2021
@JeremySkinner
Copy link
Member

Merged, and will be in the 10.2 release.

@JeremySkinner JeremySkinner changed the title Add Extension point to register FluentValidation without IMvcBuilder Allow AddFluentValidation to be called directly on IServiceCollection May 1, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants