JoshClose / CsvHelper

Library to help reading and writing CSV files
http://joshclose.github.io/CsvHelper/
Other
4.65k stars 1.05k forks source link

Allow multiple validations to be defined per field #2197

Open folkcoder opened 9 months ago

folkcoder commented 9 months ago

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

Currently, the ClassMap.Validate extension method only accepts a single validation expression. You can technically chain them now, but in practice the last applied validation "wins" and is the only one that has any effect.

For example, given the following code

private sealed class TestClassMap : ClassMap<CsvTestClass>
{
    public TestClassMap()
    {
        Map(m => m.StringProperty)
            .Validate(
                args => args.Field.Length < 10,
                _ => "Failed length validation"
            )
            .Validate(
                args => args.Field.Contains("green"),
                _ => "Failed color validation"
            );
    }
}

a field value of "green green green green" would pass validation despite having more than 10 characters.

This behavior makes it difficult to package common validation rules into components that can be easily shared across different class maps. If the validation expressions were additive, we could more easily build reusable validation tooling, which is particularly valuable for consistent error messaging that is returned, in our case, to a user interface. For example, we could replace the contrived example above with reusable extensions:

public static MemberMap<TClass, TMember> AddLengthValidator<TClass, TMember>(
    this MemberMap<TClass, TMember> memberMap,
    int requiredLength
) {
    Validate validate = args => !string.IsNullOrWhitespace(args.Field) && args.Field.Length < requiredLength;
    ValidateMessage validateMessage = _ =>
        $"Field must be less than {requiredLength} characters.";

    return memberMap.Validate(validate, validateMessage);
}

public static MemberMap<TClass, TMember> AddContainsValidator<TClass, TMember>(
    this MemberMap<TClass, TMember> memberMap,
    string matchText
) {
    Validate validate = args => args.Field.Contains(matchText);
    ValidateMessage validateMessage = _ =>
        $"Field must contain the text {matchText}.";

    return memberMap.Validate(validate, validateMessage);
}

private sealed class TestClassMap : ClassMap<CsvTestClass>
{
    public TestClassMap()
    {
        Map(m => m.StringProperty)
            .AddLengthValidator(10)
            .AddContainsValidator("green")
    }
}

Describe the solution you'd like

Currently, MemberMapData defines a single expression each for the validation itself and the validation message. It may make more sense for each MemberMapData instance to define a collection of validations instead, loop through each rule in the CreateGetFieldExpression call, and throw an exception if any of the validations fail.

Currently, a validation failure throws a FieldValidationException. If multiple validation rules were introduced, we'd have to consider the best way to return error information. Some potential options:

Depending on the way this goes, there could be a win for the common feature request of validating all columns of a row before throwing a validation exception. See, for instance, #1357.

As MemberMapData publicly exposes ValidateExpression and ValidateMessageExpression, this solution would be a breaking change.

Describe alternatives you've considered

Our current workaround is an extension method to add validators that combines the expression tree of the new validator with any current validation already defined in the member map data. This works for our use case but has the limitation that ALL validation messages are returned (concatenated) even if only one of the field's multiple validations fail.

Additional context

I am happy to take on the work to implement this if it's a feature that would be a good fit for the project.

fwilliamconceicao commented 5 months ago

👍 for this feature, we have the same behavior. For now, we'll do the workaround to revalidate after passing this.

dta-trifork commented 2 months ago

Perhaps you could sidestep this by using a fluent validator in the callback method and using the fluent validator response as the result and message?

A better improvement however would be to simply register an existing validator and using that. A downside is that you then have to establish a validator in addition to each map. Hmm..

Similarly, I would prefer if we could somehow get more precise error messages in general (such as column, row) for faster manual adjustment or precise reporting.