FluentValidation icon indicating copy to clipboard operation
FluentValidation copied to clipboard

[Bug] Validation is not correct when validating items in a nested dictionary

Open srollinet opened this issue 2 years ago • 5 comments

Describe the bug When validating items in a nested dictionary, the validation result is true, even when items are not valid

To Reproduce I made a repro unit test here: https://github.com/srollinet/BlazoredFluentValidation/blob/DictionaryValidationIssue/Blazored.FluentValidation.Tests/NestedDictionaryTest.cs

just clone the repo and execute the test.

Expected behavior Validation result should be same as manually performed validation (valid=false)

srollinet avatar Apr 26 '22 17:04 srollinet

@srollinet are you sure you are using Blazored.FluentValidation Nuget Package ? and if so please downgrade Package version to 2.0.1.

Sharaf-Mansour avatar May 09 '22 12:05 Sharaf-Mansour

@srollinet There is a flaw in your test. On this line:

https://github.com/srollinet/BlazoredFluentValidation/blob/29e545c4321a76396e8b3cc21294a0133ee08623/Blazored.FluentValidation.Tests/NestedDictionaryTest.cs#L46

You're passing in null to the final argument, but that needs to be an instance of a FluentValidationValidator component.

However, with that said, there is definitely an issue with dictionaries.

chrissainty avatar May 09 '22 13:05 chrissainty

@chrissainty Yes, in fact I modified the method signature (temporarily) to make it testable this way. Normally it won't impact the result. In the original method, you are passing an instance of FluentValidationValidator, only to access its Options property. But it defaults to opt => opt.IncludeAllRuleSets() when Options is null.

https://github.com/srollinet/BlazoredFluentValidation/commit/29e545c4321a76396e8b3cc21294a0133ee08623#diff-e15a1de63ffa32103f6a5667b0df65a9585ed7ddb422abb99dbb9c8a14f350ae

Probably it would have been better to not change the signature and make it nullable by adding a null propagation operator at line 48

Also, while digging into the code, I realized that only one validator is used, whereas you can have multiple validators registered in the DI. Is this on purpose? If not, it probably deserves its own issue.

srollinet avatar May 10 '22 05:05 srollinet

@srollinet I've been working on this recently, but I'm struggling to get something working. Is this scenario something you've used previously with Fluent Validation on the server?

chrissainty avatar Jun 10 '22 17:06 chrissainty

@chrissainty It is the first time I have this scenario with FluentValidation, And I think it is quite uncommon.

Finally, I switched to a custom implementation based on https://github.com/ryanelian/FluentValidation.Blazor, which have a correct validation result, but I think there are still some minor glitches with this scenario.

For special cases like this, maybe just ensure that the overall validation result is consistent with a manual validation. This was my main issue. I was able to send invalid data because it was considered valid in the UI. But it was rejected later by the "server-side" validation (in this case, it is Blazor server, so I put server-side in quotes)

srollinet avatar Jun 15 '22 07:06 srollinet