FluentValidation icon indicating copy to clipboard operation
FluentValidation copied to clipboard

[Bug] All RuleSets firing, even though only one specific in component options

Open Jaffacakes82 opened this issue 2 years ago • 6 comments

Describe the bug I am specifying an explicit RuleSet to include when using the FluentValidationValidator component in my component, e.g. <FluentValidationValidator Options="@(options => options.IncludeRuleSets(CreateUserModelValidator.ModifyUserRuleSet))" @ref="fluentValidationValidator" />. However, all RuleSets are firing which is printing duplicate validation messages on the screen.

NOTE: I am not explicitly calling Validate(), Fluent Validation seems to be checking for valid fields when those fields are updated.

Expected behavior Only the specified RuleSet is executed when updating any fields.

Screenshots Duplicate validation messages: image

Redacated validator class with RuleSets:

RuleSet(CreateUserRuleSet, () =>
{
    ...
    When(u => u.LoginEnabled && RequiresNPI(u), () =>
    {
        RuleFor(x => x.NPI)
            ...
            .Matches(new Regex("\\d{10}"))
                .WithMessage("NPIs must be 10 digit numbers");
    });
});

RuleSet(ModifyUserRuleSet, () =>
{
  When(u => u.LoginEnabled && RequiresNPI(u), () =>
  {
      RuleFor(x => x.NPI)
          ...
          .Matches(new Regex("\\d{10}"))
              .WithMessage("NPIs must be 10 digit numbers")
  });
});

RuleSet(ManualValidateNPIRuleSet, () =>
{
    RuleFor(p => p.NPI)
        ...
        .Matches(new Regex("\\d{10}"))
            .WithMessage("NPIs must be 10 digit numbers");
});

Hosting Model (is this issue happening with a certain hosting model?):

  • Blazor Server

Jaffacakes82 avatar Feb 08 '23 14:02 Jaffacakes82

We're seeing the same thing here. Updating to the newest version didn't fix the problem. It's as if all rulesets were firing whenever we updated an input's contents.

UniMichael avatar Mar 09 '23 13:03 UniMichael

Update on this (because it's a pretty big problem on our end):

I think I may have found the issue.

ValidateField and ValidateModel are implemented differently. ValidateField seems to ignore options altogether, and neither pass a PropertyChain to their contexts along with options, so the validation doesn't work fully.

UniMichael avatar Mar 21 '23 19:03 UniMichael

I have the same issue. I saw internal implementation of FluentValidation.ValidationStrategy and they do create IValidatorSelector differently:


private IValidatorSelector GetSelector() {
		IValidatorSelector selector = null;

		if (_properties != null || _ruleSets != null || _customSelector != null) {
			var selectors = new List<IValidatorSelector>(3);

			if (_customSelector != null) {
				selectors.Add(_customSelector);
			}

			if (_properties != null) {
				selectors.Add(ValidatorOptions.Global.ValidatorSelectors.MemberNameValidatorSelectorFactory(_properties.ToArray()));
			}

			if (_ruleSets != null) {
				selectors.Add(ValidatorOptions.Global.ValidatorSelectors.RulesetValidatorSelectorFactory(_ruleSets.ToArray()));
			}

			selector = selectors.Count == 1 ? selectors[0] : ValidatorOptions.Global.ValidatorSelectors.CompositeValidatorSelectorFactory(selectors);
		}
		else {
			selector = ValidatorOptions.Global.ValidatorSelectors.DefaultValidatorSelectorFactory();
		}

		return selector;
	}

is it possible to implement your ValidateField method something like this?

pit1188 avatar Apr 12 '23 10:04 pit1188

Hi

I just ran into this same issue and managed to get it working by tweaking the Blazored.FluentValidation field-level validation implementation. I'll PR it and hopefully get it into the main branch, but in the meantime, if anyone else has the same issue - you can try these.

See #205 #204

matthetherington avatar Dec 05 '23 16:12 matthetherington

Back in March, when I looked into this, the issue was that FluentValidation didn't expose enough information for ValidateField to be able to work as intended.

Admittedly, this may have changed since then (I don't know, we fixed it and moved on), but taking a look at their release page, they don't seem to have added this functionality since.

Unfortunately, our solution was to drop this library and write our own validation component that:

  • Stores the current validation errors
  • Validates the entire model when a field changes
  • Filters-out all the new errors that don't match the target field (so existing errors were untouched)

We found a way to map the FluentValidation paths (which are string paths) to what Blazor's EditContext expects (FieldIdentifiers) by using a modified version of Steven Sanderson's converter. I say modified because it used to be on this blog post according to our Git history, but I can't seem to find it there anymore. We modified it by simplifying some of the code that was in the blog post and fixing a handful of warnings about potential null reference exceptions.

There a few downsides with this, of course. We're now maintaining our own validation library (though this one doesn't seem to have had a release in a while, so that may end-up being good for us). By validating the whole model, we run into the risk of having slow validation code trigger when an unrelated field changes, though this should only happen if we did something silly like have some long-running async code in our validation, which we don't do (we've forbidden async validation altogether through code reviews).

UniMichael avatar Dec 06 '23 13:12 UniMichael

Hi @UniMichael

I've put together a PR which sounds like it's doing something similar to what you described (though the reverse) - would you mind weighing in over at https://github.com/Blazored/FluentValidation/pull/205 ?

I'm curious if it also helps with your use case

matthetherington avatar Dec 06 '23 18:12 matthetherington