Skip to content

Commit 7a655f4

Browse files
Copilotcaptainsafia
andcommitted
Fix ValidatableTypeInfo to skip IValidatableObject validation when property validation fails
Co-authored-by: captainsafia <[email protected]>
1 parent 57b678f commit 7a655f4

File tree

4 files changed

+86
-25
lines changed

4 files changed

+86
-25
lines changed

src/Validation/src/PublicAPI.Unshipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,6 @@ abstract Microsoft.Extensions.Validation.ValidatablePropertyInfo.GetValidationAt
4848
static Microsoft.Extensions.DependencyInjection.ValidationServiceCollectionExtensions.AddValidation(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action<Microsoft.Extensions.Validation.ValidationOptions!>? configureOptions = null) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
4949
virtual Microsoft.Extensions.Validation.ValidatableParameterInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task!
5050
virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task!
51+
virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidateComplexObjectsAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task!
52+
virtual Microsoft.Extensions.Validation.ValidatablePropertyInfo.ValidatePropertyAttributesAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<bool>!
5153
virtual Microsoft.Extensions.Validation.ValidatableTypeInfo.ValidateAsync(object? value, Microsoft.Extensions.Validation.ValidateContext! context, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task!

src/Validation/src/ValidatablePropertyInfo.cs

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ protected ValidatablePropertyInfo(
5959

6060
/// <inheritdoc />
6161
public virtual async Task ValidateAsync(object? value, ValidateContext context, CancellationToken cancellationToken)
62+
{
63+
await ValidatePropertyAttributesAsync(value, context, cancellationToken);
64+
await ValidateComplexObjectsAsync(value, context, cancellationToken);
65+
}
66+
67+
/// <summary>
68+
/// Validates only the property attributes (Required, Range, etc.) without validating complex objects.
69+
/// Returns true if there were validation errors.
70+
/// </summary>
71+
public virtual Task<bool> ValidatePropertyAttributesAsync(object? value, ValidateContext context, CancellationToken cancellationToken)
6272
{
6373
var property = DeclaringType.GetProperty(Name) ?? throw new InvalidOperationException($"Property '{Name}' not found on type '{DeclaringType.Name}'.");
6474
var propertyValue = property.GetValue(value);
@@ -78,6 +88,8 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context,
7888
context.ValidationContext.DisplayName = DisplayName;
7989
context.ValidationContext.MemberName = Name;
8090

91+
var hadValidationErrors = false;
92+
8193
// Check required attribute first
8294
if (_requiredAttribute is not null || validationAttributes.TryGetRequiredAttribute(out _requiredAttribute))
8395
{
@@ -87,12 +99,49 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context,
8799
{
88100
context.AddValidationError(Name, context.CurrentValidationPath, [result.ErrorMessage], value);
89101
context.CurrentValidationPath = originalPrefix; // Restore prefix
90-
return;
102+
return Task.FromResult(true);
91103
}
92104
}
93105

106+
// Track errors before validating other attributes
107+
var errorCountBeforeOtherValidation = context.ValidationErrors?.Count ?? 0;
108+
94109
// Validate any other attributes
95-
ValidateValue(propertyValue, Name, context.CurrentValidationPath, validationAttributes, value);
110+
ValidateValue(propertyValue, Name, context.CurrentValidationPath, validationAttributes, value, context);
111+
112+
// Check if other validation introduced errors
113+
if ((context.ValidationErrors?.Count ?? 0) > errorCountBeforeOtherValidation)
114+
{
115+
hadValidationErrors = true;
116+
}
117+
118+
// Restore prefix
119+
context.CurrentValidationPath = originalPrefix;
120+
121+
return Task.FromResult(hadValidationErrors);
122+
}
123+
124+
/// <summary>
125+
/// Validates complex objects (sub-types) for this property.
126+
/// </summary>
127+
public virtual async Task ValidateComplexObjectsAsync(object? value, ValidateContext context, CancellationToken cancellationToken)
128+
{
129+
var property = DeclaringType.GetProperty(Name) ?? throw new InvalidOperationException($"Property '{Name}' not found on type '{DeclaringType.Name}'.");
130+
var propertyValue = property.GetValue(value);
131+
132+
// Calculate and save the current path
133+
var originalPrefix = context.CurrentValidationPath;
134+
if (string.IsNullOrEmpty(originalPrefix))
135+
{
136+
context.CurrentValidationPath = Name;
137+
}
138+
else
139+
{
140+
context.CurrentValidationPath = $"{originalPrefix}.{Name}";
141+
}
142+
143+
context.ValidationContext.DisplayName = DisplayName;
144+
context.ValidationContext.MemberName = Name;
96145

97146
// Check if we've reached the maximum depth before validating complex properties
98147
if (context.CurrentDepth >= context.ValidationOptions.MaxDepth)
@@ -149,27 +198,27 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context,
149198
context.CurrentDepth--;
150199
context.CurrentValidationPath = originalPrefix;
151200
}
201+
}
152202

153-
void ValidateValue(object? val, string name, string errorPrefix, ValidationAttribute[] validationAttributes, object? container)
203+
private static void ValidateValue(object? val, string name, string errorPrefix, ValidationAttribute[] validationAttributes, object? container, ValidateContext context)
204+
{
205+
for (var i = 0; i < validationAttributes.Length; i++)
154206
{
155-
for (var i = 0; i < validationAttributes.Length; i++)
207+
var attribute = validationAttributes[i];
208+
try
156209
{
157-
var attribute = validationAttributes[i];
158-
try
159-
{
160-
var result = attribute.GetValidationResult(val, context.ValidationContext);
161-
if (result is not null && result != ValidationResult.Success && result.ErrorMessage is not null)
162-
{
163-
var key = errorPrefix.TrimStart('.');
164-
context.AddOrExtendValidationErrors(name, key, [result.ErrorMessage], container);
165-
}
166-
}
167-
catch (Exception ex)
210+
var result = attribute.GetValidationResult(val, context.ValidationContext);
211+
if (result is not null && result != ValidationResult.Success && result.ErrorMessage is not null)
168212
{
169213
var key = errorPrefix.TrimStart('.');
170-
context.AddOrExtendValidationErrors(name, key, [ex.Message], container);
214+
context.AddOrExtendValidationErrors(name, key, [result.ErrorMessage], container);
171215
}
172216
}
217+
catch (Exception ex)
218+
{
219+
var key = errorPrefix.TrimStart('.');
220+
context.AddOrExtendValidationErrors(name, key, [ex.Message], container);
221+
}
173222
}
174223
}
175224
}

src/Validation/src/ValidatableTypeInfo.cs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,23 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context,
6464
{
6565
var actualType = value.GetType();
6666

67-
// First validate members
67+
// Track if there are any property-level validation errors from this type
68+
var hasPropertyValidationErrors = false;
69+
70+
// First validate only property attributes for each member
71+
for (var i = 0; i < _membersCount; i++)
72+
{
73+
if (await Members[i].ValidatePropertyAttributesAsync(value, context, cancellationToken))
74+
{
75+
hasPropertyValidationErrors = true;
76+
}
77+
context.CurrentValidationPath = originalPrefix;
78+
}
79+
80+
// Then validate complex objects for each member
6881
for (var i = 0; i < _membersCount; i++)
6982
{
70-
await Members[i].ValidateAsync(value, context, cancellationToken);
83+
await Members[i].ValidateComplexObjectsAsync(value, context, cancellationToken);
7184
context.CurrentValidationPath = originalPrefix;
7285
}
7386

@@ -86,8 +99,8 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context,
8699
}
87100
}
88101

89-
// Finally validate IValidatableObject if implemented
90-
if (Type.ImplementsInterface(typeof(IValidatableObject)) && value is IValidatableObject validatable)
102+
// Finally validate IValidatableObject if implemented, but only if there are no property validation errors
103+
if (Type.ImplementsInterface(typeof(IValidatableObject)) && value is IValidatableObject validatable && !hasPropertyValidationErrors)
91104
{
92105
// Important: Set the DisplayName to the type name for top-level validations
93106
// and restore the original validation context properties

src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.IValidatableObject.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ async Task ValidateMethodCalledIfPropertyValidationsFail()
125125
await endpoint.RequestDelegate(httpContext);
126126

127127
var problemDetails = await AssertBadRequest(httpContext);
128+
// With the fix, IValidatableObject validation should not run when property validation fails
129+
// So we should only get the property validation errors, not the IValidatableObject error for Value1
128130
Assert.Collection(problemDetails.Errors,
129131
error =>
130132
{
@@ -141,11 +143,6 @@ async Task ValidateMethodCalledIfPropertyValidationsFail()
141143
{
142144
Assert.Equal("SubType.Value3", error.Key);
143145
Assert.Equal("The field ValidatableSubType must be 'some-value'.", error.Value.Single());
144-
},
145-
error =>
146-
{
147-
Assert.Equal("Value1", error.Key);
148-
Assert.Equal("The field Value1 must be between 10 and 100.", error.Value.Single());
149146
});
150147
}
151148

0 commit comments

Comments
 (0)