Skip to content

Commit fbe46cf

Browse files
captainsafiacaptainsafia
committed
Benchmarks, more tests, some tweaks
1 parent 03a3c06 commit fbe46cf

9 files changed

+534
-67
lines changed

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace Microsoft.AspNetCore.Http.Validation;
1313
/// </summary>
1414
public abstract class ValidatableParameterInfo
1515
{
16+
private ValidationAttribute? _requiredAttribute;
1617
/// <summary>
1718
/// Creates a new instance of <see cref="ValidatableParameterInfo"/>.
1819
/// </summary>
@@ -99,9 +100,12 @@ public virtual Task Validate(object? value, ValidatableContext context)
99100

100101
var validationAttributes = GetValidationAttributes();
101102

102-
if (IsRequired && validationAttributes.OfType<RequiredAttribute>().SingleOrDefault() is { } requiredAttribute)
103+
if (IsRequired)
103104
{
104-
var result = requiredAttribute.GetValidationResult(value, context.ValidationContext);
105+
_requiredAttribute ??= validationAttributes.OfType<RequiredAttribute>()
106+
.FirstOrDefault();
107+
Debug.Assert(_requiredAttribute is not null, "RequiredAttribute should be present if IsRequired is true");
108+
var result = _requiredAttribute.GetValidationResult(value, context.ValidationContext);
105109

106110
if (result is not null && result != ValidationResult.Success)
107111
{
@@ -112,8 +116,9 @@ public virtual Task Validate(object? value, ValidatableContext context)
112116
}
113117

114118
// Validate against validation attributes
115-
foreach (var attribute in validationAttributes)
119+
for (var i = 0; i < validationAttributes.Length; i++)
116120
{
121+
var attribute = validationAttributes[i];
117122
try
118123
{
119124
var result = attribute.GetValidationResult(value, context.ValidationContext);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,9 @@ public virtual Task Validate(object obj, ValidatableContext context)
188188

189189
void ValidateValue(object? val, string errorPrefix, ValidationAttribute[] validationAttributes)
190190
{
191-
foreach (var attribute in validationAttributes)
191+
for (var i = 0; i < validationAttributes.Length; i++)
192192
{
193+
var attribute = validationAttributes[i];
193194
try
194195
{
195196
var result = attribute.GetValidationResult(val, context.ValidationContext);

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ namespace Microsoft.AspNetCore.Http.Validation;
1212
/// </summary>
1313
public abstract class ValidatableTypeInfo
1414
{
15+
private readonly int _membersCount;
16+
private readonly int _validatableSubtypesCount;
17+
1518
/// <summary>
1619
/// Creates a new instance of <see cref="ValidatableTypeInfo"/>.
1720
/// </summary>
@@ -29,6 +32,8 @@ public ValidatableTypeInfo(
2932
Members = members;
3033
IsIValidatableObject = implementsIValidatableObject;
3134
ValidatableSubTypes = validatableSubTypes;
35+
_membersCount = members.Count;
36+
_validatableSubtypesCount = validatableSubTypes?.Count ?? 0;
3237
}
3338

3439
/// <summary>
@@ -79,17 +84,20 @@ public virtual Task Validate(object? value, ValidatableContext context)
7984
var originalPrefix = context.Prefix;
8085

8186
// First validate members
82-
foreach (var member in Members)
87+
for (var i = 0; i < _membersCount; i++)
8388
{
84-
member.Validate(value, context);
89+
Members[i].Validate(value, context);
8590
context.Prefix = originalPrefix;
8691
}
8792

8893
// Then validate sub-types if any
89-
if (ValidatableSubTypes != null)
94+
if (ValidatableSubTypes is not null)
9095
{
91-
foreach (var subType in ValidatableSubTypes)
96+
for (var i = 0; i < _validatableSubtypesCount; i++)
9297
{
98+
var subType = ValidatableSubTypes[i];
99+
// Check if the actual type is assignable to the sub-type
100+
// and validate it if it is
93101
if (subType.IsAssignableFrom(actualType))
94102
{
95103
if (context.ValidationOptions.TryGetValidatableTypeInfo(subType, out var subTypeInfo))

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public async Task CanValidateComplexTypes()
2626
2727
var app = builder.Build();
2828
29-
app.MapPost("/complex-type", (ComplexType complexType) => Results.Ok());
29+
app.MapPost("/complex-type", (ComplexType complexType) => Results.Ok("Passed"!));
3030
3131
app.Run();
3232
@@ -106,6 +106,7 @@ await VerifyEndpoint(compilation, "/complex-type", async (endpoint, serviceProvi
106106
await InvalidPropertyWithDerivedValidationAttributeProducesError(endpoint);
107107
await InvalidPropertyWithMultipleAttributesProducesError(endpoint);
108108
await InvalidPropertyWithCustomValidationProducesError(endpoint);
109+
await ValidInputProducesNoWarnings(endpoint);
109110

110111
async Task InvalidIntegerWithRangeProducesError(Endpoint endpoint)
111112
{
@@ -336,6 +337,37 @@ async Task InvalidPropertyWithCustomValidationProducesError(Endpoint endpoint)
336337
Assert.Equal("Can't use the same number value in two properties on the same class.", error);
337338
});
338339
}
340+
341+
async Task ValidInputProducesNoWarnings(Endpoint endpoint)
342+
{
343+
var payload = """
344+
{
345+
"IntegerWithRange": 50,
346+
"IntegerWithRangeAndDisplayName": 50,
347+
"PropertyWithMemberAttributes": {
348+
"RequiredProperty": "valid",
349+
"StringWithLength": "valid"
350+
},
351+
"PropertyWithoutMemberAttributes": {
352+
"RequiredProperty": "valid",
353+
"StringWithLength": "valid"
354+
},
355+
"PropertyWithInheritance": {
356+
"RequiredProperty": "valid",
357+
"StringWithLength": "valid",
358+
"EmailString": "[email protected]"
359+
},
360+
"ListOfSubTypes": [],
361+
"IntegerWithDerivedValidationAttribute": 2,
362+
"IntegerWithCustomValidation": 0,
363+
"PropertyWithMultipleAttributes": 12
364+
}
365+
""";
366+
var context = CreateHttpContextWithPayload(payload, serviceProvider);
367+
await endpoint.RequestDelegate(context);
368+
369+
Assert.Equal(200, context.Response.StatusCode);
370+
}
339371
});
340372
}
341373
}

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

Lines changed: 102 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ public async Task CanValidateRecursiveTypes()
1616
using Microsoft.Extensions.DependencyInjection;
1717
1818
var builder = WebApplication.CreateBuilder();
19-
builder.Services.AddValidation();
19+
builder.Services.AddValidation(options =>
20+
{
21+
options.MaxDepth = 8;
22+
});
2023
2124
var app = builder.Build();
2225
@@ -35,77 +38,121 @@ public class RecursiveType
3538

3639
await VerifyEndpoint(compilation, "/recursive-type", async (endpoint, serviceProvider) =>
3740
{
38-
var httpContext = CreateHttpContextWithPayload("""
41+
await ThrowsExceptionForDeeplyNestedType(endpoint);
42+
await ValidatesTypeWithLimitedNesting(endpoint);
43+
44+
async Task ThrowsExceptionForDeeplyNestedType(Endpoint endpoint)
3945
{
40-
"value": 1,
41-
"next": {
42-
"value": 2,
46+
var httpContext = CreateHttpContextWithPayload("""
47+
{
48+
"value": 1,
4349
"next": {
44-
"value": 3,
50+
"value": 2,
4551
"next": {
46-
"value": 4,
52+
"value": 3,
4753
"next": {
48-
"value": 5,
54+
"value": 4,
4955
"next": {
50-
"value": 6,
56+
"value": 5,
5157
"next": {
52-
"value": 7,
58+
"value": 6,
5359
"next": {
54-
"value": 8
60+
"value": 7,
61+
"next": {
62+
"value": 8,
63+
"next": {
64+
"value": 9,
65+
"next": {
66+
"value": 10
67+
}
68+
}
69+
}
5570
}
5671
}
5772
}
5873
}
5974
}
6075
}
6176
}
62-
}
63-
""", serviceProvider);
77+
""", serviceProvider);
6478

65-
await endpoint.RequestDelegate(httpContext);
79+
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () => await endpoint.RequestDelegate(httpContext));
80+
}
6681

67-
var problemDetails = await AssertBadRequest(httpContext);
68-
Assert.Collection(problemDetails.Errors,
69-
error =>
70-
{
71-
Assert.Equal("Value", error.Key);
72-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
73-
},
74-
error =>
75-
{
76-
Assert.Equal("Next.Value", error.Key);
77-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
78-
},
79-
error =>
80-
{
81-
Assert.Equal("Next.Next.Value", error.Key);
82-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
83-
},
84-
error =>
85-
{
86-
Assert.Equal("Next.Next.Next.Value", error.Key);
87-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
88-
},
89-
error =>
90-
{
91-
Assert.Equal("Next.Next.Next.Next.Value", error.Key);
92-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
93-
},
94-
error =>
95-
{
96-
Assert.Equal("Next.Next.Next.Next.Next.Value", error.Key);
97-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
98-
},
99-
error =>
100-
{
101-
Assert.Equal("Next.Next.Next.Next.Next.Next.Value", error.Key);
102-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
103-
},
104-
error =>
82+
async Task ValidatesTypeWithLimitedNesting(Endpoint endpoint)
83+
{
84+
var httpContext = CreateHttpContextWithPayload("""
10585
{
106-
Assert.Equal("Next.Next.Next.Next.Next.Next.Next.Value", error.Key);
107-
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
108-
});
86+
"value": 1,
87+
"next": {
88+
"value": 2,
89+
"next": {
90+
"value": 3,
91+
"next": {
92+
"value": 4,
93+
"next": {
94+
"value": 5,
95+
"next": {
96+
"value": 6,
97+
"next": {
98+
"value": 7,
99+
"next": {
100+
"value": 8
101+
}
102+
}
103+
}
104+
}
105+
}
106+
}
107+
}
108+
}
109+
""", serviceProvider);
110+
111+
await endpoint.RequestDelegate(httpContext);
112+
113+
var problemDetails = await AssertBadRequest(httpContext);
114+
Assert.Collection(problemDetails.Errors,
115+
error =>
116+
{
117+
Assert.Equal("Value", error.Key);
118+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
119+
},
120+
error =>
121+
{
122+
Assert.Equal("Next.Value", error.Key);
123+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
124+
},
125+
error =>
126+
{
127+
Assert.Equal("Next.Next.Value", error.Key);
128+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
129+
},
130+
error =>
131+
{
132+
Assert.Equal("Next.Next.Next.Value", error.Key);
133+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
134+
},
135+
error =>
136+
{
137+
Assert.Equal("Next.Next.Next.Next.Value", error.Key);
138+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
139+
},
140+
error =>
141+
{
142+
Assert.Equal("Next.Next.Next.Next.Next.Value", error.Key);
143+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
144+
},
145+
error =>
146+
{
147+
Assert.Equal("Next.Next.Next.Next.Next.Next.Value", error.Key);
148+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
149+
},
150+
error =>
151+
{
152+
Assert.Equal("Next.Next.Next.Next.Next.Next.Next.Value", error.Key);
153+
Assert.Equal("The field Value must be between 10 and 100.", error.Value.Single());
154+
});
155+
}
109156
});
110157
}
111158
}

‎src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateComplexTypes#ValidatableInfoResolver.g.verified.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ private ValidatableTypeInfo CreateComplexType()
250250
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.AspNetCore.Http.ValidationsGenerator, Version=42.42.42.42, Culture=neutral, PublicKeyToken=adb9793829ddae60", "42.42.42.42")]
251251
file static class GeneratedServiceCollectionExtensions
252252
{
253-
[global::System.Runtime.CompilerServices.InterceptsLocationAttribute(1, "2HbGOie6Bg3kAttRoJz64oEBAABQcm9ncmFtLmNz")]
253+
[global::System.Runtime.CompilerServices.InterceptsLocationAttribute(1, "I71YCOnkIuFyp29JNyKEXIEBAABQcm9ncmFtLmNz")]
254254
public static global::Microsoft.Extensions.DependencyInjection.IServiceCollection AddValidation(this global::Microsoft.Extensions.DependencyInjection.IServiceCollection services, global::System.Action<ValidationOptions>? configureOptions = null)
255255
{
256256
// Use non-extension method to avoid infinite recursion.

‎src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateRecursiveTypes#ValidatableInfoResolver.g.verified.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ private ValidatableTypeInfo CreateRecursiveType()
112112
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.AspNetCore.Http.ValidationsGenerator, Version=42.42.42.42, Culture=neutral, PublicKeyToken=adb9793829ddae60", "42.42.42.42")]
113113
file static class GeneratedServiceCollectionExtensions
114114
{
115-
[global::System.Runtime.CompilerServices.InterceptsLocationAttribute(1, "oUYTnYFMCFtXQaDfuna3SgYBAABQcm9ncmFtLmNz")]
115+
[global::System.Runtime.CompilerServices.InterceptsLocationAttribute(1, "wNU90FRNrQG/m6cp7QRaZQYBAABQcm9ncmFtLmNz")]
116116
public static global::Microsoft.Extensions.DependencyInjection.IServiceCollection AddValidation(this global::Microsoft.Extensions.DependencyInjection.IServiceCollection services, global::System.Action<ValidationOptions>? configureOptions = null)
117117
{
118118
// Use non-extension method to avoid infinite recursion.

0 commit comments

Comments
 (0)