Issue when calling Validator.TryValidateObject

[jpiquot]

Hi,

The method TryValidateObject will throw an exception when trying to validate an object. It seems that the GetObjectValidationErrors method does not check if result of alidatable.Validate(validationContext) is not null. Is this a bug or a desired design?

Regards,
Jérôme Piquot

Method : System.ComponentModel.DataAnnotations.GetObjectValidationErrors

      // Step 3: Test for IValidatableObject implementation            
        if (instance is IValidatableObject validatable)
        {
            var results = validatable.Validate(validationContext);

===> foreach (var result in results.Where(r => r != ValidationResult.Success))
{
errors.Add(new ValidationError(null, instance, result));
}
}
Call Stack :
System.ArgumentNullException: Value cannot be null.
Parameter name: source
at System.Linq.Enumerable.Where[TSource](IEnumerable1 source, Func2 predicate)
at System.ComponentModel.DataAnnotations.Validator.GetObjectValidationErrors(Object instance, ValidationContext validationContext, Boolean validateAllProperties, Boolean breakOnFirstError)
at System.ComponentModel.DataAnnotations.Validator.TryValidateObject(Object instance, ValidationContext validationContext, ICollection`1 validationResults, Boolean validateAllProperties)
at Flexmind.Framework.Database.ApplicationDbContext.ValidateObject(IValidatableObject toValidate) in C:\Dev\Tyndareus\Flexmind.Framework\Flexmind.Framework\src\Core\Flexmind.Framework.Database\ApplicationDbContext.cs:line 148
at Flexmind.Framework.Database.ApplicationDbContext.BeforeSave() in C:\Dev\Tyndareus\Flexmind.Framework\Flexmind.Framework\src\Core\Flexmind.Framework.Database\ApplicationDbContext.cs:line 127
at Flexmind.Framework.Database.ApplicationDbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) in C:\Dev\Tyndareus\Flexmind.Framework\Flexmind.Framework\src\Core\Flexmind.Framework.Database\ApplicationDbContext.cs:line 102
at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(CancellationToken cancellationToken)

[ajcvickers]

Looks like a bug to me. Any thoughts @divega?

[divega]

@ajcvickers Sure, seems like it should be an easy fix too.

[FSou1]

I suspect that an issue in these lines of code:
https://github.com/dotnet/corefx/blob/03c7617170661e1312f6ae414225ef72ea792170/src/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/Validator.cs#L432-L442
The variable results could be null and in this case I propose to skip a loop with adding if(results != null) condition.

If it's a proper way from your point of view I'll do it.

[ajcvickers]

@FSou1 Yes, that looks reasonable. It would be great if you could send a PR for this. Please also add a test.

[FSou1]

Great, I'll do it

[FSou1]

Looks like NETFX x86 Release Build — was failed but i'm not pretty sure if my changes were the root cause.

Let me know if I have to fix it.

[karelz]

@FSou1 thanks for you interest to fix the bug. I have sent you Collaborator invite, so that we can assign it to you (GH limitation). Please ping me once you accept (assigning to myself for now).

Pro-tip: Disable notifications for the repo (you will be auto-subscribed) as there's plenty (500+ per day) and opt-in into "your mentions / subscriptions" notification setting.

[FSou1]

@karelz ok, i've just accepted the invite.

Ok, i got it.

[FSou1]

@karelz Would you mind helping me to determine the proper way to close this issue? As you've already mentioned a label should be assigned and an attribute for test ignoring on NET FX have to be added.

I'm afraid i'm stuck with both of them:

1. Do not have enough access rules;
2. Not able to figure out the content of testResult.xml to retrieve helpful information;

[karelz]

@FSou1 only members have permissions to change labels - it is tied to commit permissions.
@ajcvickers adding netfx-port-consider, please double check it is something you want to eventually consider porting into Desktop.

@FSou1 let's keep PR discussion in the PR (testResults.xml)