Skip to content

Commit 59069f7

Browse files
captainsafiacaptainsafia
committed
Update tests and fix implementation for more cases
1 parent e9f9a2e commit 59069f7

File tree

9 files changed

+187
-129
lines changed

9 files changed

+187
-129
lines changed

‎src/Http/Http.Abstractions/src/Validation/ValidatablePropertyInfo.cs‎

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33

44
using System.ComponentModel.DataAnnotations;
55
using System.Diagnostics.CodeAnalysis;
6+
using System.Reflection;
7+
using System.Text.Json;
8+
using System.Text.Json.Serialization;
69

710
namespace Microsoft.AspNetCore.Http.Validation;
811

@@ -33,7 +36,7 @@ protected ValidatablePropertyInfo(
3336
/// <summary>
3437
/// Gets the member type.
3538
/// </summary>
36-
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)]
39+
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicConstructors)]
3740
internal Type DeclaringType { get; }
3841

3942
/// <summary>
@@ -65,24 +68,23 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context,
6568
var validationAttributes = GetValidationAttributes();
6669

6770
// Calculate and save the current path
71+
var memberName = GetJsonPropertyName(Name, property, context.SerializerOptions?.PropertyNamingPolicy);
6872
var originalPrefix = context.CurrentValidationPath;
6973
if (string.IsNullOrEmpty(originalPrefix))
7074
{
71-
context.CurrentValidationPath = Name;
75+
context.CurrentValidationPath = memberName;
7276
}
7377
else
7478
{
75-
context.CurrentValidationPath = $"{originalPrefix}.{Name}";
79+
context.CurrentValidationPath = $"{originalPrefix}.{memberName}";
7680
}
7781

78-
context.ValidationContext.DisplayName = DisplayName;
79-
80-
// Format member name according to naming policy if available
81-
var memberName = Name;
82-
if (context.SerializerOptions?.PropertyNamingPolicy is not null)
83-
{
84-
memberName = context.SerializerOptions.PropertyNamingPolicy.ConvertName(Name);
85-
}
82+
// Format the display name and member name according to JsonPropertyName attribute first, then naming policy
83+
// If the property has a [Display] attribute (either on property or record parameter), use DisplayName directly without formatting
84+
var hasDisplayAttribute = HasDisplayAttribute(property);
85+
context.ValidationContext.DisplayName = hasDisplayAttribute
86+
? DisplayName
87+
: GetJsonPropertyName(DisplayName, property, context.SerializerOptions?.PropertyNamingPolicy);
8688
context.ValidationContext.MemberName = memberName;
8789

8890
// Check required attribute first
@@ -177,4 +179,61 @@ void ValidateValue(object? val, string errorPrefix, ValidationAttribute[] valida
177179
}
178180
}
179181
}
182+
183+
/// <summary>
184+
/// Gets the effective member name for JSON serialization, considering JsonPropertyName attribute and naming policy.
185+
/// </summary>
186+
/// <param name="targetValue">The target value to get the name for.</param>
187+
/// <param name="property">The property info to get the name for.</param>
188+
/// <param name="namingPolicy">The JSON naming policy to apply if no JsonPropertyName attribute is present.</param>
189+
/// <returns>The effective property name for JSON serialization.</returns>
190+
private static string GetJsonPropertyName(string targetValue, PropertyInfo property, JsonNamingPolicy? namingPolicy)
191+
{
192+
var jsonPropertyName = property.GetCustomAttribute<JsonPropertyNameAttribute>()?.Name;
193+
194+
if (jsonPropertyName is not null)
195+
{
196+
return jsonPropertyName;
197+
}
198+
199+
if (namingPolicy is not null)
200+
{
201+
return namingPolicy.ConvertName(targetValue);
202+
}
203+
204+
return targetValue;
205+
}
206+
207+
/// <summary>
208+
/// Determines whether the property has a DisplayAttribute, either directly on the property
209+
/// or on the corresponding constructor parameter if the declaring type is a record.
210+
/// </summary>
211+
/// <param name="property">The property to check.</param>
212+
/// <returns>True if the property has a DisplayAttribute, false otherwise.</returns>
213+
private bool HasDisplayAttribute(PropertyInfo property)
214+
{
215+
// Check if the property itself has the DisplayAttribute with a valid Name
216+
if (property.GetCustomAttribute<DisplayAttribute>() is { Name: not null })
217+
{
218+
return true;
219+
}
220+
221+
// Look for a constructor parameter matching the property name (case-insensitive)
222+
// to account for the record scenario
223+
foreach (var constructor in DeclaringType.GetConstructors())
224+
{
225+
foreach (var parameter in constructor.GetParameters())
226+
{
227+
if (string.Equals(parameter.Name, property.Name, StringComparison.OrdinalIgnoreCase))
228+
{
229+
if (parameter.GetCustomAttribute<DisplayAttribute>() is { Name: not null })
230+
{
231+
return true;
232+
}
233+
}
234+
}
235+
}
236+
237+
return false;
238+
}
180239
}

‎src/Http/Http.Abstractions/test/Validation/ValidatableTypeInfoTests.cs‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,12 @@ [new RequiredAttribute()])
206206
kvp =>
207207
{
208208
Assert.Equal(expectedFirstName, kvp.Key);
209-
Assert.Equal($"The FirstName field is required.", kvp.Value.First());
209+
Assert.Equal($"The {expectedFirstName} field is required.", kvp.Value.First());
210210
},
211211
kvp =>
212212
{
213213
Assert.Equal(expectedLastName, kvp.Key);
214-
Assert.Equal($"The LastName field is required.", kvp.Value.First());
214+
Assert.Equal($"The {expectedLastName} field is required.", kvp.Value.First());
215215
});
216216
}
217217

@@ -891,20 +891,17 @@ [new EmailAddressAttribute()])
891891

892892
// Assert
893893
Assert.NotNull(context.ValidationErrors);
894+
// Use [JsonPropertyName] over naming policy
894895
Assert.Collection(context.ValidationErrors,
895-
kvp =>
896+
kvp =>
896897
{
897-
// Property key uses camelCase naming policy
898898
Assert.Equal("username", kvp.Key);
899-
// Error message should also use camelCase for property names
900-
Assert.Equal("The UserName field is required.", kvp.Value.First());
899+
Assert.Equal("The username field is required.", kvp.Value.First());
901900
},
902-
kvp =>
901+
kvp =>
903902
{
904-
// Property key uses camelCase naming policy
905-
Assert.Equal("email", kvp.Key);
906-
// Error message should also use camelCase for property names
907-
Assert.Equal("The EmailAddress field is not a valid e-mail address.", kvp.Value.First());
903+
Assert.Equal("email", kvp.Key);
904+
Assert.Equal("The email field is not a valid e-mail address.", kvp.Value.First());
908905
});
909906
}
910907

@@ -1107,13 +1104,13 @@ public bool TryGetValidatableParameterInfo(ParameterInfo parameterInfo, [NotNull
11071104
}
11081105
}
11091106
}
1110-
1107+
11111108
[Fact]
11121109
public void Validate_FormatsErrorMessagesWithPropertyNamingPolicy()
11131110
{
11141111
// Arrange
11151112
var validationOptions = new ValidationOptions();
1116-
1113+
11171114
var address = new Address();
11181115
var validationContext = new ValidationContext(address);
11191116
var validateContext = new ValidateContext
@@ -1147,7 +1144,7 @@ public void Validate_PreservesCustomErrorMessagesWithPropertyNamingPolicy()
11471144
{
11481145
// Arrange
11491146
var validationOptions = new ValidationOptions();
1150-
1147+
11511148
var model = new TestModel();
11521149
var validationContext = new ValidationContext(model);
11531150
var validateContext = new ValidateContext
@@ -1175,7 +1172,7 @@ public void Validate_PreservesCustomErrorMessagesWithPropertyNamingPolicy()
11751172
Assert.Equal("customProperty", error.Key);
11761173
Assert.Equal("Custom message without standard format.", error.Value.First()); // Custom message without formatting
11771174
}
1178-
1175+
11791176
private class TestModel
11801177
{
11811178
public string? CustomProperty { get; set; }

‎src/Http/Http.Extensions/test/ValidationsGenerator/ValidationsGenerator.ComplexType.cs‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ async Task InvalidIntegerWithRangeProducesError(Endpoint endpoint)
124124
var problemDetails = await AssertBadRequest(context);
125125
Assert.Collection(problemDetails.Errors, kvp =>
126126
{
127-
Assert.Equal("IntegerWithRange", kvp.Key);
128-
Assert.Equal("The field IntegerWithRange must be between 10 and 100.", kvp.Value.Single());
127+
Assert.Equal("integerWithRange", kvp.Key);
128+
Assert.Equal("The field integerWithRange must be between 10 and 100.", kvp.Value.Single());
129129
});
130130
}
131131

@@ -143,7 +143,7 @@ async Task InvalidIntegerWithRangeAndDisplayNameProducesError(Endpoint endpoint)
143143
var problemDetails = await AssertBadRequest(context);
144144
Assert.Collection(problemDetails.Errors, kvp =>
145145
{
146-
Assert.Equal("IntegerWithRangeAndDisplayName", kvp.Key);
146+
Assert.Equal("integerWithRangeAndDisplayName", kvp.Key);
147147
Assert.Equal("The field Valid identifier must be between 10 and 100.", kvp.Value.Single());
148148
});
149149
}
@@ -162,8 +162,8 @@ async Task MissingRequiredSubtypePropertyProducesError(Endpoint endpoint)
162162
var problemDetails = await AssertBadRequest(context);
163163
Assert.Collection(problemDetails.Errors, kvp =>
164164
{
165-
Assert.Equal("PropertyWithMemberAttributes", kvp.Key);
166-
Assert.Equal("The PropertyWithMemberAttributes field is required.", kvp.Value.Single());
165+
Assert.Equal("propertyWithMemberAttributes", kvp.Key);
166+
Assert.Equal("The propertyWithMemberAttributes field is required.", kvp.Value.Single());
167167
});
168168
}
169169

@@ -185,13 +185,13 @@ async Task InvalidRequiredSubtypePropertyProducesError(Endpoint endpoint)
185185
Assert.Collection(problemDetails.Errors,
186186
kvp =>
187187
{
188-
Assert.Equal("PropertyWithMemberAttributes.RequiredProperty", kvp.Key);
189-
Assert.Equal("The RequiredProperty field is required.", kvp.Value.Single());
188+
Assert.Equal("propertyWithMemberAttributes.requiredProperty", kvp.Key);
189+
Assert.Equal("The requiredProperty field is required.", kvp.Value.Single());
190190
},
191191
kvp =>
192192
{
193-
Assert.Equal("PropertyWithMemberAttributes.StringWithLength", kvp.Key);
194-
Assert.Equal("The field StringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
193+
Assert.Equal("propertyWithMemberAttributes.stringWithLength", kvp.Key);
194+
Assert.Equal("The field stringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
195195
});
196196
}
197197

@@ -214,18 +214,18 @@ async Task InvalidSubTypeWithInheritancePropertyProducesError(Endpoint endpoint)
214214
Assert.Collection(problemDetails.Errors,
215215
kvp =>
216216
{
217-
Assert.Equal("PropertyWithInheritance.EmailString", kvp.Key);
218-
Assert.Equal("The EmailString field is not a valid e-mail address.", kvp.Value.Single());
217+
Assert.Equal("propertyWithInheritance.emailString", kvp.Key);
218+
Assert.Equal("The emailString field is not a valid e-mail address.", kvp.Value.Single());
219219
},
220220
kvp =>
221221
{
222-
Assert.Equal("PropertyWithInheritance.RequiredProperty", kvp.Key);
223-
Assert.Equal("The RequiredProperty field is required.", kvp.Value.Single());
222+
Assert.Equal("propertyWithInheritance.requiredProperty", kvp.Key);
223+
Assert.Equal("The requiredProperty field is required.", kvp.Value.Single());
224224
},
225225
kvp =>
226226
{
227-
Assert.Equal("PropertyWithInheritance.StringWithLength", kvp.Key);
228-
Assert.Equal("The field StringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
227+
Assert.Equal("propertyWithInheritance.stringWithLength", kvp.Key);
228+
Assert.Equal("The field stringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
229229
});
230230
}
231231

@@ -257,18 +257,18 @@ async Task InvalidListOfSubTypesProducesError(Endpoint endpoint)
257257
Assert.Collection(problemDetails.Errors,
258258
kvp =>
259259
{
260-
Assert.Equal("ListOfSubTypes[0].RequiredProperty", kvp.Key);
261-
Assert.Equal("The RequiredProperty field is required.", kvp.Value.Single());
260+
Assert.Equal("listOfSubTypes[0].requiredProperty", kvp.Key);
261+
Assert.Equal("The requiredProperty field is required.", kvp.Value.Single());
262262
},
263263
kvp =>
264264
{
265-
Assert.Equal("ListOfSubTypes[0].StringWithLength", kvp.Key);
266-
Assert.Equal("The field StringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
265+
Assert.Equal("listOfSubTypes[0].stringWithLength", kvp.Key);
266+
Assert.Equal("The field stringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
267267
},
268268
kvp =>
269269
{
270-
Assert.Equal("ListOfSubTypes[1].StringWithLength", kvp.Key);
271-
Assert.Equal("The field StringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
270+
Assert.Equal("listOfSubTypes[1].stringWithLength", kvp.Key);
271+
Assert.Equal("The field stringWithLength must be a string with a maximum length of 10.", kvp.Value.Single());
272272
});
273273
}
274274

@@ -286,7 +286,7 @@ async Task InvalidPropertyWithDerivedValidationAttributeProducesError(Endpoint e
286286
var problemDetails = await AssertBadRequest(context);
287287
Assert.Collection(problemDetails.Errors, kvp =>
288288
{
289-
Assert.Equal("IntegerWithDerivedValidationAttribute", kvp.Key);
289+
Assert.Equal("integerWithDerivedValidationAttribute", kvp.Key);
290290
Assert.Equal("Value must be an even number", kvp.Value.Single());
291291
});
292292
}
@@ -305,15 +305,15 @@ async Task InvalidPropertyWithMultipleAttributesProducesError(Endpoint endpoint)
305305
var problemDetails = await AssertBadRequest(context);
306306
Assert.Collection(problemDetails.Errors, kvp =>
307307
{
308-
Assert.Equal("PropertyWithMultipleAttributes", kvp.Key);
308+
Assert.Equal("propertyWithMultipleAttributes", kvp.Key);
309309
Assert.Collection(kvp.Value,
310310
error =>
311311
{
312-
Assert.Equal("The field PropertyWithMultipleAttributes is invalid.", error);
312+
Assert.Equal("The field propertyWithMultipleAttributes is invalid.", error);
313313
},
314314
error =>
315315
{
316-
Assert.Equal("The field PropertyWithMultipleAttributes must be between 10 and 100.", error);
316+
Assert.Equal("The field propertyWithMultipleAttributes must be between 10 and 100.", error);
317317
});
318318
});
319319
}
@@ -333,7 +333,7 @@ async Task InvalidPropertyWithCustomValidationProducesError(Endpoint endpoint)
333333
var problemDetails = await AssertBadRequest(context);
334334
Assert.Collection(problemDetails.Errors, kvp =>
335335
{
336-
Assert.Equal("IntegerWithCustomValidation", kvp.Key);
336+
Assert.Equal("integerWithCustomValidation", kvp.Key);
337337
var error = Assert.Single(kvp.Value);
338338
Assert.Equal("Can't use the same number value in two properties on the same class.", error);
339339
});

‎src/Http/Http.Extensions/test/ValidationsGenerator/ValidationsGenerator.IValidatableObject.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,23 +128,24 @@ async Task ValidateMethodCalledIfPropertyValidationsFail()
128128
Assert.Collection(problemDetails.Errors,
129129
error =>
130130
{
131-
Assert.Equal("Value2", error.Key);
131+
Assert.Equal("value2", error.Key);
132132
Assert.Collection(error.Value,
133-
msg => Assert.Equal("The Value2 field is required.", msg));
133+
msg => Assert.Equal("The value2 field is required.", msg));
134134
},
135135
error =>
136136
{
137-
Assert.Equal("SubType.RequiredProperty", error.Key);
138-
Assert.Equal("The RequiredProperty field is required.", error.Value.Single());
137+
Assert.Equal("subType.requiredProperty", error.Key);
138+
Assert.Equal("The requiredProperty field is required.", error.Value.Single());
139139
},
140140
error =>
141141
{
142-
Assert.Equal("SubType.Value3", error.Key);
142+
Assert.Equal("subType.value3", error.Key);
143143
Assert.Equal("The field ValidatableSubType must be 'some-value'.", error.Value.Single());
144144
},
145145
error =>
146146
{
147-
Assert.Equal("Value1", error.Key);
147+
Assert.Equal("value1", error.Key);
148+
// The error message is generated using nameof(Value1) in the IValidateObject implementation
148149
Assert.Equal("The field Value1 must be between 10 and 100.", error.Value.Single());
149150
});
150151
}
@@ -169,12 +170,12 @@ async Task ValidateForSubtypeInvokedFirst()
169170
Assert.Collection(problemDetails.Errors,
170171
error =>
171172
{
172-
Assert.Equal("SubType.Value3", error.Key);
173+
Assert.Equal("subType.value3", error.Key);
173174
Assert.Equal("The field ValidatableSubType must be 'some-value'.", error.Value.Single());
174175
},
175176
error =>
176177
{
177-
Assert.Equal("Value1", error.Key);
178+
Assert.Equal("value1", error.Key);
178179
Assert.Equal("The field Value1 must be between 10 and 100.", error.Value.Single());
179180
});
180181
}
@@ -199,7 +200,7 @@ async Task ValidateForTopLevelInvoked()
199200
Assert.Collection(problemDetails.Errors,
200201
error =>
201202
{
202-
Assert.Equal("Value1", error.Key);
203+
Assert.Equal("value1", error.Key);
203204
Assert.Equal("The field Value1 must be between 10 and 100.", error.Value.Single());
204205
});
205206
}

‎src/Http/Http.Extensions/test/ValidationsGenerator/ValidationsGenerator.NoOp.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,8 @@ await VerifyEndpoint(compilation, "/complex-type", async (endpoint, serviceProvi
170170
var problemDetails = await AssertBadRequest(context);
171171
Assert.Collection(problemDetails.Errors, kvp =>
172172
{
173-
Assert.Equal("IntegerWithRange", kvp.Key);
174-
Assert.Equal("The field IntegerWithRange must be between 10 and 100.", kvp.Value.Single());
173+
Assert.Equal("integerWithRange", kvp.Key);
174+
Assert.Equal("The field integerWithRange must be between 10 and 100.", kvp.Value.Single());
175175
});
176176
});
177177
}

0 commit comments

Comments
 (0)