From a3b84bb9db9a975554ad096b0621cddf55121769 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 04:46:56 +0000 Subject: [PATCH 1/7] Initial plan From 57b678f7f49268d66db83456d1a584caf140f66f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 04:57:36 +0000 Subject: [PATCH 2/7] Initial investigation and test setup Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --- .../ValidatableTypeInfoTests.cs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs b/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs index 002ebb1582b0..452c7281c5d1 100644 --- a/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs +++ b/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs @@ -586,6 +586,64 @@ 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"); + } + + // 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 { From 7a655f4be615a1dda27f1c4786cfa781e70d03e7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 05:17:31 +0000 Subject: [PATCH 3/7] Fix ValidatableTypeInfo to skip IValidatableObject validation when property validation fails Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --- src/Validation/src/PublicAPI.Unshipped.txt | 2 + src/Validation/src/ValidatablePropertyInfo.cs | 81 +++++++++++++++---- src/Validation/src/ValidatableTypeInfo.cs | 21 ++++- ...ValidationsGenerator.IValidatableObject.cs | 7 +- 4 files changed, 86 insertions(+), 25 deletions(-) diff --git a/src/Validation/src/PublicAPI.Unshipped.txt b/src/Validation/src/PublicAPI.Unshipped.txt index d7f657e38875..9be1589a6b1c 100644 --- a/src/Validation/src/PublicAPI.Unshipped.txt +++ b/src/Validation/src/PublicAPI.Unshipped.txt @@ -48,4 +48,6 @@ abstract Microsoft.Extensions.Validation.ValidatablePropertyInfo.GetValidationAt static Microsoft.Extensions.DependencyInjection.ValidationServiceCollectionExtensions.AddValidation(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action? configureOptions = null) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! virtual Microsoft.Extensions.Validation.ValidatableParameterInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidateComplexObjectsAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidatePropertyAttributesAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! virtual Microsoft.Extensions.Validation.ValidatableTypeInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! \ No newline at end of file diff --git a/src/Validation/src/ValidatablePropertyInfo.cs b/src/Validation/src/ValidatablePropertyInfo.cs index 94d7eb606fa3..2d21da50b3d3 100644 --- a/src/Validation/src/ValidatablePropertyInfo.cs +++ b/src/Validation/src/ValidatablePropertyInfo.cs @@ -59,6 +59,16 @@ protected ValidatablePropertyInfo( /// public virtual async Task ValidateAsync(object? value, ValidateContext context, CancellationToken cancellationToken) + { + await ValidatePropertyAttributesAsync(value, context, cancellationToken); + await ValidateComplexObjectsAsync(value, context, cancellationToken); + } + + /// + /// Validates only the property attributes (Required, Range, etc.) without validating complex objects. + /// Returns true if there were validation errors. + /// + public 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 +88,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 +99,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. + /// + public 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 +198,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()); }); } From 9708b8189c334b8c6e44cef7f9a37be6014e57af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 15:21:11 +0000 Subject: [PATCH 4/7] Make ValidatePropertyAttributesAsync and ValidateComplexObjectsAsync internal and remove from PublicApi Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --- src/Validation/src/PublicAPI.Unshipped.txt | 2 -- src/Validation/src/ValidatablePropertyInfo.cs | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Validation/src/PublicAPI.Unshipped.txt b/src/Validation/src/PublicAPI.Unshipped.txt index 9be1589a6b1c..d7f657e38875 100644 --- a/src/Validation/src/PublicAPI.Unshipped.txt +++ b/src/Validation/src/PublicAPI.Unshipped.txt @@ -48,6 +48,4 @@ abstract Microsoft.Extensions.Validation.ValidatablePropertyInfo.GetValidationAt static Microsoft.Extensions.DependencyInjection.ValidationServiceCollectionExtensions.AddValidation(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action? configureOptions = null) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! virtual Microsoft.Extensions.Validation.ValidatableParameterInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! -virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidateComplexObjectsAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! -virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidatePropertyAttributesAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! virtual Microsoft.Extensions.Validation.ValidatableTypeInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! \ No newline at end of file diff --git a/src/Validation/src/ValidatablePropertyInfo.cs b/src/Validation/src/ValidatablePropertyInfo.cs index 2d21da50b3d3..df2ce1b78092 100644 --- a/src/Validation/src/ValidatablePropertyInfo.cs +++ b/src/Validation/src/ValidatablePropertyInfo.cs @@ -68,7 +68,7 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context, /// Validates only the property attributes (Required, Range, etc.) without validating complex objects. /// Returns true if there were validation errors. /// - public virtual Task ValidatePropertyAttributesAsync(object? value, ValidateContext context, CancellationToken cancellationToken) + 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); @@ -124,7 +124,7 @@ public virtual Task ValidatePropertyAttributesAsync(object? value, Validat /// /// Validates complex objects (sub-types) for this property. /// - public virtual async Task ValidateComplexObjectsAsync(object? value, ValidateContext context, CancellationToken cancellationToken) + 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); From 09348678b91c211f1d8a2795fc77e01ee0d3b1d1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Jul 2025 21:55:22 +0000 Subject: [PATCH 5/7] Add test demonstrating System.ComponentModel.DataAnnotations.Validator behavior Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> --- .../ValidatableTypeInfoTests.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs b/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs index 452c7281c5d1..17de4491c8d6 100644 --- a/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs +++ b/src/Validation/test/Microsoft.Extensions.Validation.Tests/ValidatableTypeInfoTests.cs @@ -625,6 +625,38 @@ [new RangeAttribute(1, 100)]) 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 { From 3e8c7fb732d766ec60659ffcdadf066f8954fc3d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Jul 2025 22:06:02 +0000 Subject: [PATCH 6/7] Clarify ValidatePropertyAttributesAsync result handling with explicit discard and comment Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> --- src/Validation/src/ValidatablePropertyInfo.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Validation/src/ValidatablePropertyInfo.cs b/src/Validation/src/ValidatablePropertyInfo.cs index df2ce1b78092..a7bff24dceef 100644 --- a/src/Validation/src/ValidatablePropertyInfo.cs +++ b/src/Validation/src/ValidatablePropertyInfo.cs @@ -60,7 +60,13 @@ protected ValidatablePropertyInfo( /// public virtual async Task ValidateAsync(object? value, ValidateContext context, CancellationToken cancellationToken) { - await ValidatePropertyAttributesAsync(value, context, cancellationToken); + // Validate property attributes first (e.g., [Required], [Range], etc.) + // Note: We capture the result for clarity, but complex object validation + // always runs regardless of property validation results, as it handles + // null values automatically and non-null objects should be validated + // even if they fail property-level validation. + _ = await ValidatePropertyAttributesAsync(value, context, cancellationToken); + await ValidateComplexObjectsAsync(value, context, cancellationToken); } From ae00450c258531f29369a2f0b2303c3fbf0269b6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Jul 2025 22:19:16 +0000 Subject: [PATCH 7/7] Fix ValidatablePropertyInfo to skip complex object validation when property validation fails Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> --- src/Validation/src/ValidatablePropertyInfo.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Validation/src/ValidatablePropertyInfo.cs b/src/Validation/src/ValidatablePropertyInfo.cs index a7bff24dceef..889703cb1e93 100644 --- a/src/Validation/src/ValidatablePropertyInfo.cs +++ b/src/Validation/src/ValidatablePropertyInfo.cs @@ -61,13 +61,14 @@ protected ValidatablePropertyInfo( public virtual async Task ValidateAsync(object? value, ValidateContext context, CancellationToken cancellationToken) { // Validate property attributes first (e.g., [Required], [Range], etc.) - // Note: We capture the result for clarity, but complex object validation - // always runs regardless of property validation results, as it handles - // null values automatically and non-null objects should be validated - // even if they fail property-level validation. - _ = await ValidatePropertyAttributesAsync(value, context, cancellationToken); + var hasPropertyValidationErrors = await ValidatePropertyAttributesAsync(value, context, cancellationToken); - await ValidateComplexObjectsAsync(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); + } } ///