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

Add a factory method for creating the CompositeValidatorSelector #1988

Closed
uhfath opened this issue Aug 8, 2022 · 10 comments
Closed

Add a factory method for creating the CompositeValidatorSelector #1988

uhfath opened this issue Aug 8, 2022 · 10 comments
Milestone

Comments

@uhfath
Copy link

uhfath commented Aug 8, 2022

Is your feature request related to a problem? Please describe.

When defining a RuleSet along with properties to be validated (like described here) the ValidationStrategy builds a CompositeValidatorSelector which then uses Any to combine IValidatorSelectors.

There are cases where it would be feasible to change Any to All since a RuleSet might define other rules for a property or there is no need to run validators not defined in a RuleSet using an explicit property.
Currently that is not possible.

Describe the solution you'd like

Something like this:

public enum ValidatorSelectorCombineMode
{
    All,
    Any,
}

When set to All the above described case will be possible.
Any is the currently default method to keep backward compatibility.

Describe alternatives you've considered

I'm willing to make a PR if this addition is welcome.

Additional Context

A sample which describes when to use this feature:

internal class ClientDetailsModelValidator : AbstractValidator<Models.ClientDetails>
{
    public ClientDetailsModelValidator()
    {
        RuleSet("name", () =>
        {
            RuleFor(m => m.Surname)
                .NotEmpty()
                .MaximumLength(50)
            ;

            RuleFor(m => m.Name)
                .NotEmpty()
                .MaximumLength(50)
            ;

            RuleFor(m => m.Patronymic)
                .MaximumLength(50)
            ;
        });

        RuleFor(m => m.Sex)
            .NotEmpty()
        ;

        RuleFor(m => m.Birthday)
            .NotEmpty()
            .LessThanOrEqualTo(DateTime.Today)
            .ExclusiveBetween(new DateTime(1900, 01, 01), new DateTime(2100, 01, 01))
        ;
    }
}

Currently it's not possible to validate a Surname property from name RuleSet separately.
Or at least I haven't found a way for it to work in a single Validate call.

@JeremySkinner
Copy link
Member

Hi, thanks for the suggestion.

This isn't something that we'll be including in the core library, but if you want to implement this in your own project you can do so by writing a custom IValidatorSelector implementation that includes the selection logic that you need for your use case.

The design philosophy behind many of FluentValidation's features is generally to provide default behaviour that covers common use cases, and then provide extensibility points for users to plug in their own logic if they want non-default behaviour, rather than trying to include lots of different options in the core library.

Thanks for wanting to be involved though!

@uhfath
Copy link
Author

uhfath commented Aug 8, 2022

Thanks for the explanation.
I get your point.
If it's ok I'll share my view of this.

The IValidatorSelector implementation was my initial step which was already taken but one of other reasons for this "PR" is that I haven't found any places in docs that mention described behavior of validator selectors and, honestly, I was expecting it works the other way around.
I imagined it would work like multiple Where expressions in LINQ that narrows the "search" and not expand it.

The extension in this case requires creation of validator selector using the same code that is part of the core since there are already multiple cases like IncludeRuleSets, IncludeAllRuleSets and IncludeRulesNotInRuleSet, etc. which are already implemented and tested.
Also if it becomes a core part then any other new kinds of strategy will be already taken care of and it could become another extension point to control how CompositeValidatorSelector might behave in future.

@JeremySkinner
Copy link
Member

Yeah I see where you're coming from. How about a compromise: we currently have global factory methods for creating the Ruleset and MemberName selectors. We could add another factory method for creating the CompositeValidatorSelector, so you can then swap out just the composite selector without having to duplicate any other logic. Would that work for you?

@uhfath
Copy link
Author

uhfath commented Aug 8, 2022

Seems quite reasonable.
Just to make sure I understand correctly - something like replacing a LanguageManager or DisplayNameResolver in ValidatorOptions.Global at startup?
That would be nice and will make customization of this part even more flexible.

@JeremySkinner
Copy link
Member

Yes, so you can currently do it with the MemberNameValidatorSelector and the RulesetValidatorSelector, eg:

// Replace RulesetValidatorSelector with a custom version:
ValidatorOptions.Global.ValidatorSelectors.RulesetValidatorSelectorFactory = (rulesets) => new MyRulesetValidatorSelector(rulesets);

...and would add something similar for the composite selector.

@JeremySkinner JeremySkinner reopened this Aug 8, 2022
@uhfath
Copy link
Author

uhfath commented Aug 8, 2022

Makes sense indeed.
Thanks!

@JeremySkinner JeremySkinner changed the title A way to control how CompositeValidatorSelector combines validator selectors Add a factory method for creating the CompositeValidatorSelector Aug 8, 2022
@JeremySkinner
Copy link
Member

I've pushed out an 11.2 release which contains the ValidatorOptions.Global.ValidatorSelectors.CompositeValidatorSelectorFactory

@JeremySkinner JeremySkinner added this to the 11.2.0 milestone Aug 8, 2022
@julealgon
Copy link

Currently it's not possible to validate a Surname property from name RuleSet separately.
Or at least I haven't found a way for it to work in a single Validate call.

What does the validate call look like @uhfath ? Just curious about the use case here.

@uhfath
Copy link
Author

uhfath commented Aug 9, 2022

I've pushed out an 11.2 release which contains the ValidatorOptions.Global.ValidatorSelectors.CompositeValidatorSelectorFactory

That was fast.
Much appreciated! Thank you!

@uhfath
Copy link
Author

uhfath commented Aug 9, 2022

Currently it's not possible to validate a Surname property from name RuleSet separately.
Or at least I haven't found a way for it to work in a single Validate call.

What does the validate call look like @uhfath ? Just curious about the use case here.

Something along the lines:

private ValidationContext<object> BuildValidationContext(object model, params string[] properties) =>
    ValidationContext<object>.CreateWithOptions(model, opts =>
    {
        if (!properties.IsEmpty())
        {
            opts.IncludeProperties(properties);
        }

        if (!string.IsNullOrWhiteSpace(RuleSet))
        {
            opts.IncludeRuleSets(RuleSet);
        }

        if (!RuleSets.IsEmpty())
        {
            opts.IncludeRuleSets(RuleSets.ToArray());
        }

        if (UseNotInRuleSet)
        {
            opts.IncludeRulesNotInRuleSet();
        }

        opts.SetSelectorCombineMode(FluentValidation.Internal.ValidatorSelectorCombineMode.All);
    });

//---------------------------

    var property = eventArgs.FieldIdentifier.FieldName;
    var context = BuildValidationContext(eventArgs.FieldIdentifier.Model, property);
    var validator = (IValidator)ServiceProvider.GetRequiredService(MakeValidatorType(eventArgs.FieldIdentifier.Model.GetType()));
    var result = validator.Validate(context);

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants