Can't write custom WhenNotNull extension in v10

[gabrielmaldi]

FluentValidation version

10.0.0

ASP.NET version

No response

Summary

Now that IValidationContext doesn't expose PropertyValue, I can't write custom WhenNotNull, WhenNotEmpty, etc., extension methods.

In v9 I defined these (and more) extension methods:

public static IRuleBuilderOptions<T, TProperty> WhenNotNull<T, TProperty>(this IRuleBuilderOptions<T, TProperty> rule, ApplyConditionTo applyConditionTo = ApplyConditionTo.AllValidators)
{
    return rule.Configure(config => config.ApplyCondition(context => context.PropertyValue != null, applyConditionTo));
}

public static IRuleBuilderOptions<T, string> WhenNotEmpty<T>(this IRuleBuilderOptions<T, string> rule, ApplyConditionTo applyConditionTo = ApplyConditionTo.AllValidators)
{
    return rule.Configure(config => config.ApplyCondition(context => !string.IsNullOrEmpty((string)context.PropertyValue), applyConditionTo));
}

And used them extensively like this:

public class MyClassValidator : AbstractValidator<MyClass>
{
    public MyClassValidator()
    {
        RuleFor(x => x.MyDictionary).MaximumSize(50, 40, 500).WhenNotNull(); // MaximumSize validates maxElements, maxKeyLength, maxValueLength for an IDictionary
        RuleFor(x => x.MyString).MaximumLength(10).WhenNotEmpty();
    }
}

I read the upgrade guide (and also #1672) but still couldn't get it to work.

Thanks!

[JeremySkinner]

Hi,

This was actually a bug in 9.x - the property value shouldn't be available to access at this point. Conditions are designed to prevent a rule from running before it has a chance to read the property value. This is important particularly when it comes to properties that are lazily loaded or expensive to calculate. Allowing access to the property value here bypasses this, as it means the library has to always read the property value at least once prior to the condition running. For 10.0 this was fixed, but it does mean that what you're trying to do here isn't possible anymore.

I didn't mention this in the upgrade guide as I believed this was an internal implementation detail and I hadn't foreseen that it might be used by end-users in this way.

The correct way to handle this scenario is to pass a func into a call to When. You are then explicitly choosing to read the property value early, rather than the library always having to do it:

RuleFor(x => x.MyDictionary).MaximumSize(50, 40, 500).When(x => x.MyDictionary != null);

That being said I appreciate that this may be a big change for you, so I'll have a think about whether there's a way to add something into the library that would enable your use case without having to reintroduce the problematic code. It's just passed midnight for me, so I won't look at this now but will try and have a play with it tomorrow.

[gabrielmaldi]

Jeremy, thanks a lot for your incredibly fast reply – and even more being that's so late for you, I really appreciate it!

I completely understand and support the design decision, thank you for the thorough explanation.

I can just replace all my calls to my WhenXXX extension methods with calls to When(x => x.Property...).

If it's posible, I think it would be ideal if there were a way to write a WhenNotNull() extension method to avoid repeating the same property selector function both in RuleFor and When, which I would guess it's a common use case, at least for me.

Just to add to the usefulness of the use-case: I use WhenNotEmpty not just for string properties, but also for Uri, where the check is context.PropertyValue != null && !string.IsNullOrEmpty(((Uri)context.PropertyValue).OriginalString). So then I can just validate several Uri properties by calling RuleFor(x => x.OnSuccessUrl).ValidUri().WhenNotEmpty(), RuleFor(x => x.OnErrorUrl).ValidUri().WhenNotEmpty(), RuleFor(x => x.OnCancelUrl).ValidUri().WhenNotEmpty(), etc.

Thanks again!

---

For completeness:

public static IRuleBuilderOptions<T, Uri> ValidUri<T>(this IRuleBuilder<T, Uri> ruleBuilder)
{
    return ruleBuilder
        .Must(uri =>
        {
            return uri != null && !string.IsNullOrEmpty(uri.OriginalString) &&
                Uri.TryCreate(uri.OriginalString, UriKind.Absolute, out var newUri) &&
                (newUri.Scheme == Uri.UriSchemeHttp || newUri.Scheme == Uri.UriSchemeHttps);
        })
        .WithMessage("'{PropertyName}' must be a valid http:// or https:// URL.");
}

[JeremySkinner]

Thanks for the additional info.

I think there are 2 ways that we could enable this. Option 1 is to expose the PropertyFunc on the rule, so you'd then be able to implement your WhenNotNull method like this:

public static IRuleBuilderOptions<T, TProperty> WhenNotNull<T, TProperty>(this IRuleBuilderOptions<T, TProperty> rule, ApplyConditionTo applyConditionTo = ApplyConditionTo.AllValidators)
{
    return rule.Configure(config => 
    {
      config.ApplyCondition(context => config.PropertyFunc((T)context.InstanceToValidate) != null, applyConditionTo);
    });
}

Option 2 would be to add a second level of conditions that run after the property value has been obtained, which would be exposed with an additional overload of ApplyCondition:

public static IRuleBuilderOptions<T, TProperty> WhenNotNull<T, TProperty>(this IRuleBuilderOptions<T, TProperty> rule, ApplyConditionTo applyConditionTo = ApplyConditionTo.AllValidators)
{
    return rule.Configure(config => 
    {
      config.ApplyCondition((context, value)=> value != null, applyConditionTo);
    });
}

Both have their upsides and downsides, and both would require changing the IValidationRule<T> interface, so would be technically a breaking change (although this is not an interface that users should implement directly, so could possibly get away with making either change without having to do a major release).

Option 1 is potentially more open to abuse, but simpler to implement. Option 2 makes condition handling more complex and adds an additional step to the validation process. I'm currently leaning towards option 1

[gabrielmaldi]

If it were up to me, I'd definitely go with option 1. Option 2 needs to be designed, implemented, tested, documented, and maintained. It's way more complex than just exposing PropertyFunc, which fulfills the use case.

Another question: what's the difference between ApplyCondition and ApplySharedCondition? Which one would you choose for WhenNotNull? (Option 2 would also need to support ApplySharedCondition).

[JeremySkinner]

ApplySharedCondition is used for top-level conditions that apply to one or more complete rule chains, while ApplyCondition is used when you call When on a specific part of a rule chain.

// shared condition - shared across multiple rule chains 
When(x => x.Foo, () => {
  RuleFor(x => x.Bar).NotNull().GreaterThan(0);
  RuleFor(x => x.Baz).NotNull().GreaterThan(0);
});

// normal condition 
RuleFor(x => x.Bar).NotNull().GreaterThan(0).When(x=>x.Foo);

option 2 wouldn’t apply to shared conditions, as they’re not tied to a specific property.

Anyway, thanks for the feedback - I think option 1 is the way to go. I’ll try and do a patch release shortly.

[gabrielmaldi]

Great! I'll give it a test run when it's ready.

Thanks again for taking the time to not only address issues but also explain so thoroughly 😊.

[JeremySkinner]

I've pushed out 10.0.1 to nuget with this change. I had to do it slightly differently and expose a public method rather than exposing the PropretyFunc directly. You can call this new method inside your extension like this:

public static IRuleBuilderOptions<T, TProperty> WhenNotNull<T, TProperty>(this IRuleBuilderOptions<T, TProperty> rule, ApplyConditionTo applyConditionTo = ApplyConditionTo.AllValidators)
{
    return rule.Configure(config => 
    {
      config.ApplyCondition(context => config.GetPropertyValue(context.InstanceToValidate) != null, applyConditionTo);
    });
}

It's untyped, returning object but there's no way around this unfortunately as the TProperty on the extension method won't match the property value when working with collections (inside RuleForEach collections, TProperty references the type of the current collection element, rather than the collection property type returned by the PropertyFunc, so the new method has to return object in order to correctly receive the collection reference when called as part of RuleForEach).

[gabrielmaldi]

Just upgraded to 10.0.1, refactored my extension methods to use GetPropertyValue and now everything works perfectly!

Thanks a lot!