diff --git a/src/Validation/src/ValidatablePropertyInfo.cs b/src/Validation/src/ValidatablePropertyInfo.cs index 94d7eb606fa3..889703cb1e93 100644 --- a/src/Validation/src/ValidatablePropertyInfo.cs +++ b/src/Validation/src/ValidatablePropertyInfo.cs @@ -59,6 +59,23 @@ protected ValidatablePropertyInfo( /// public virtual async Task ValidateAsync(object? value, ValidateContext context, CancellationToken cancellationToken) + { + // Validate property attributes first (e.g., [Required], [Range], etc.) + var hasPropertyValidationErrors = await ValidatePropertyAttributesAsync(value, context, cancellationToken); + + // Only validate complex objects if property validation succeeds + // This matches the behavior of System.ComponentModel.DataAnnotations.Validator + if (!hasPropertyValidationErrors) + { + await ValidateComplexObjectsAsync(value, context, cancellationToken); + } + } + + /// + /// Validates only the property attributes (Required, Range, etc.) without validating complex objects. + /// Returns true if there were validation errors. + /// + internal virtual Task ValidatePropertyAttributesAsync(object? value, ValidateContext context, CancellationToken cancellationToken) { var property = DeclaringType.GetProperty(Name) ?? throw new InvalidOperationException($"Property '{Name}' not found on type '{DeclaringType.Name}'."); var propertyValue = property.GetValue(value); @@ -78,6 +95,8 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context, context.ValidationContext.DisplayName = DisplayName; context.ValidationContext.MemberName = Name; + var hadValidationErrors = false; + // Check required attribute first if (_requiredAttribute is not null || validationAttributes.TryGetRequiredAttribute(out _requiredAttribute)) { @@ -87,12 +106,49 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context, { context.AddValidationError(Name, context.CurrentValidationPath, [result.ErrorMessage], value); context.CurrentValidationPath = originalPrefix; // Restore prefix - return; + return Task.FromResult(true); } } + // Track errors before validating other attributes + var errorCountBeforeOtherValidation = context.ValidationErrors?.Count ?? 0; + // Validate any other attributes - ValidateValue(propertyValue, Name, context.CurrentValidationPath, validationAttributes, value); + ValidateValue(propertyValue, Name, context.CurrentValidationPath, validationAttributes, value, context); + + // Check if other validation introduced errors + if ((context.ValidationErrors?.Count ?? 0) > errorCountBeforeOtherValidation) + { + hadValidationErrors = true; + } + + // Restore prefix + context.CurrentValidationPath = originalPrefix; + + return Task.FromResult(hadValidationErrors); + } + + /// + /// Validates complex objects (sub-types) for this property. + /// + internal virtual async Task ValidateComplexObjectsAsync(object? value, ValidateContext context, CancellationToken cancellationToken) + { + var property = DeclaringType.GetProperty(Name) ?? throw new InvalidOperationException($"Property '{Name}' not found on type '{DeclaringType.Name}'."); + var propertyValue = property.GetValue(value); + + // Calculate and save the current path + var originalPrefix = context.CurrentValidationPath; + if (string.IsNullOrEmpty(originalPrefix)) + { + context.CurrentValidationPath = Name; + } + else + { + context.CurrentValidationPath = $"{originalPrefix}.{Name}"; + } + + context.ValidationContext.DisplayName = DisplayName; + context.ValidationContext.MemberName = Name; // Check if we've reached the maximum depth before validating complex properties if (context.CurrentDepth >= context.ValidationOptions.MaxDepth) @@ -149,27 +205,27 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context, context.CurrentDepth--; context.CurrentValidationPath = originalPrefix; } + } - void ValidateValue(object? val, string name, string errorPrefix, ValidationAttribute[] validationAttributes, object? container) + private static void ValidateValue(object? val, string name, string errorPrefix, ValidationAttribute[] validationAttributes, object? container, ValidateContext context) + { + for (var i = 0; i < validationAttributes.Length; i++) { - for (var i = 0; i < validationAttributes.Length; i++) + var attribute = validationAttributes[i]; + try { - var attribute = validationAttributes[i]; - try - { - var result = attribute.GetValidationResult(val, context.ValidationContext); - if (result is not null && result != ValidationResult.Success && result.ErrorMessage is not null) - { - var key = errorPrefix.TrimStart('.'); - context.AddOrExtendValidationErrors(name, key, [result.ErrorMessage], container); - } - } - catch (Exception ex) + var result = attribute.GetValidationResult(val, context.ValidationContext); + if (result is not null && result != ValidationResult.Success && result.ErrorMessage is not null) { var key = errorPrefix.TrimStart('.'); - context.AddOrExtendValidationErrors(name, key, [ex.Message], container); + context.AddOrExtendValidationErrors(name, key, [result.ErrorMessage], container); } } + catch (Exception ex) + { + var key = errorPrefix.TrimStart('.'); + context.AddOrExtendValidationErrors(name, key, [ex.Message], container); + } } } } diff --git a/src/Validation/src/ValidatableTypeInfo.cs b/src/Validation/src/ValidatableTypeInfo.cs index 8852f674a7e0..15b9195ac11a 100644 --- a/src/Validation/src/ValidatableTypeInfo.cs +++ b/src/Validation/src/ValidatableTypeInfo.cs @@ -64,10 +64,23 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context, { var actualType = value.GetType(); - // First validate members + // Track if there are any property-level validation errors from this type + var hasPropertyValidationErrors = false; + + // First validate only property attributes for each member + for (var i = 0; i < _membersCount; i++) + { + if (await Members[i].ValidatePropertyAttributesAsync(value, context, cancellationToken)) + { + hasPropertyValidationErrors = true; + } + context.CurrentValidationPath = originalPrefix; + } + + // Then validate complex objects for each member for (var i = 0; i < _membersCount; i++) { - await Members[i].ValidateAsync(value, context, cancellationToken); + await Members[i].ValidateComplexObjectsAsync(value, context, cancellationToken); context.CurrentValidationPath = originalPrefix; } @@ -86,8 +99,8 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context, } } - // Finally validate IValidatableObject if implemented - if (Type.ImplementsInterface(typeof(IValidatableObject)) && value is IValidatableObject validatable) + // Finally validate IValidatableObject if implemented, but only if there are no property validation errors + if (Type.ImplementsInterface(typeof(IValidatableObject)) && value is IValidatableObject validatable && !hasPropertyValidationErrors) { // Important: Set the DisplayName to the type name for top-level validations // and restore the original validation context properties diff --git a/src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.IValidatableObject.cs b/src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.IValidatableObject.cs index 590195468298..5955cb065237 100644 --- a/src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.IValidatableObject.cs +++ b/src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.IValidatableObject.cs @@ -125,6 +125,8 @@ async Task ValidateMethodCalledIfPropertyValidationsFail() await endpoint.RequestDelegate(httpContext); var problemDetails = await AssertBadRequest(httpContext); + // With the fix, IValidatableObject validation should not run when property validation fails + // So we should only get the property validation errors, not the IValidatableObject error for Value1 Assert.Collection(problemDetails.Errors, error => { @@ -141,11 +143,6 @@ async Task ValidateMethodCalledIfPropertyValidationsFail() { Assert.Equal("SubType.Value3", error.Key); Assert.Equal("The field ValidatableSubType must be 'some-value'.", error.Value.Single()); - }, - error => - { - Assert.Equal("Value1", error.Key); - Assert.Equal("The field Value1 must be between 10 and 100.", error.Value.Single()); }); } diff --git a/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs b/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs index 002ebb1582b0..17de4491c8d6 100644 --- a/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs +++ b/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs @@ -586,6 +586,96 @@ public async Task Validate_IValidatableObject_WithZeroAndMultipleMemberNames_Beh }); } + [Fact] + public async Task Validate_IValidatableObject_ShouldNotRunWhenPropertyValidationFails() + { + // Arrange + var testObjectType = new TestValidatableTypeInfo( + typeof(TestObjectWithValidation), + [ + CreatePropertyInfo(typeof(TestObjectWithValidation), typeof(string), "Name", "Name", + [new RequiredAttribute()]), + CreatePropertyInfo(typeof(TestObjectWithValidation), typeof(int), "Age", "Age", + [new RangeAttribute(1, 100)]) + ]); + + var testObject = new TestObjectWithValidation + { + Name = "", // Invalid - required field is empty + Age = 25 // Valid + }; + + var context = new ValidateContext + { + ValidationOptions = new TestValidationOptions(new Dictionary + { + { typeof(TestObjectWithValidation), testObjectType } + }), + ValidationContext = new ValidationContext(testObject) + }; + + // Act + await testObjectType.ValidateAsync(testObject, context, default); + + // Assert + Assert.NotNull(context.ValidationErrors); + Assert.Contains("Name", context.ValidationErrors.Keys); + + // The key assertion: IValidatableObject should NOT be called when property validation fails + Assert.False(testObject.IValidatableObjectWasCalled, "IValidatableObject.Validate should not be called when property validation fails"); + } + + [Fact] + public void DoesNotCallObjectValidation_WhenPropertyAttributeValidationFails() + { + // Arrange + var address = new AddressWithValidation { Street = "123" }; // Too short for MinLength(5) + var context = new ValidationContext(address); + var results = new List(); + + // Act + var isValid = Validator.TryValidateObject(address, context, results, validateAllProperties: true); + + // Assert + Assert.False(isValid); + Assert.Contains(results, r => r.ErrorMessage!.Contains("minimum length of '5'")); + Assert.DoesNotContain(results, r => r.ErrorMessage == "Address-level validation ran."); + Assert.False(address.WasValidateCalled); + } + + private class AddressWithValidation : IValidatableObject + { + [MinLength(5)] + public string Street { get; set; } = string.Empty; + + public bool WasValidateCalled { get; private set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + WasValidateCalled = true; + yield return new ValidationResult("Address-level validation ran."); + } + } + + // Test class to demonstrate the expected behavior + private class TestObjectWithValidation : IValidatableObject + { + public string Name { get; set; } = string.Empty; + public int Age { get; set; } + public bool IValidatableObjectWasCalled { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + IValidatableObjectWasCalled = true; + + // This should NOT be called if there are property validation errors + if (Age < 18) + { + yield return new ValidationResult("Age must be at least 18", new[] { nameof(Age) }); + } + } + } + // Returns no member names to validate https://github.com/dotnet/aspnetcore/issues/61739 private class GlobalErrorObject : IValidatableObject {