Move registration of filter to route handlers

captainsafia · captainsafia · commit c6f88cfe0bcc · 2025-03-02T18:33:12.000-08:00
diff --git a/src/Http/Http.Abstractions/src/Extensions/EndpointBuilder.cs b/src/Http/Http.Abstractions/src/Extensions/EndpointBuilder.cs
@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Http.Validation;
 
 namespace Microsoft.AspNetCore.Builder;
 
@@ -16,7 +15,7 @@ public abstract class EndpointBuilder
     /// <summary>
     /// Gets the list of filters that apply to this endpoint.
     /// </summary>
-    public IList<Func<EndpointFilterFactoryContext, EndpointFilterDelegate, EndpointFilterDelegate>> FilterFactories => _filterFactories ??= [ValidationEndpointFilterFactory.Create];
+    public IList<Func<EndpointFilterFactoryContext, EndpointFilterDelegate, EndpointFilterDelegate>> FilterFactories => _filterFactories ??= new();
 
     /// <summary>
     /// Gets or sets the delegate used to process requests for the endpoint.
diff --git a/src/Http/Http.Extensions/gen/Microsoft.AspNetCore.Http.ValidationsGenerator/Emitters/ValidationsGenerator.Emitter.cs b/src/Http/Http.Extensions/gen/Microsoft.AspNetCore.Http.ValidationsGenerator/Emitters/ValidationsGenerator.Emitter.cs
@@ -135,7 +135,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,
             // Use non-extension method to avoid infinite recursion.
             return ValidationServiceCollectionExtensions.AddValidation(services, options =>
             {
-                options.Resolvers.Add(new GeneratedValidatableInfoResolver());
+                options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());
                 if (configureOptions is not null)
                 {
                     configureOptions(options);
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/ValidationsGenerator.IValidatableObject.cs b/src/Http/Http.Extensions/test/ValidationsGenerator/ValidationsGenerator.IValidatableObject.cs
@@ -5,8 +5,8 @@ namespace Microsoft.AspNetCore.Http.ValidationsGenerator.Tests;
 
 public partial class ValidationsGeneratorTests : ValidationsGeneratorTestBase
 {
-    // [Fact]
-    internal async Task CanValidateIValidatableObject()
+    [Fact]
+    public async Task CanValidateIValidatableObject()
     {
         var source = """
 using System.Collections.Generic;
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateComplexTypes#ValidatableInfoResolver.g.verified.cs b/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateComplexTypes#ValidatableInfoResolver.g.verified.cs
@@ -276,7 +276,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,
             // Use non-extension method to avoid infinite recursion.
             return ValidationServiceCollectionExtensions.AddValidation(services, options =>
             {
-                options.Resolvers.Add(new GeneratedValidatableInfoResolver());
+                options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());
                 if (configureOptions is not null)
                 {
                     configureOptions(options);
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateIValidatableObject#ValidatableInfoResolver.g.verified.cs b/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateIValidatableObject#ValidatableInfoResolver.g.verified.cs
@@ -221,10 +221,17 @@ private ValidatableParameterInfo CreateParameterInfomodel()
     file static class GeneratedServiceCollectionExtensions
     {
         [global::System.Runtime.CompilerServices.InterceptsLocationAttribute(1, "ZbyAIis29Y/4fT/GGaRWq7ABAABQcm9ncmFtLmNz")]
-        public static IServiceCollection AddValidation(this IServiceCollection services)
+        public static IServiceCollection AddValidation(this IServiceCollection services, Action<ValidationOptions>? configureOptions = null)
         {
-            services.AddSingleton<global::Microsoft.AspNetCore.Http.Validation.IValidatableInfoResolver>(new GeneratedValidatableInfoResolver());
-            return services;
+            // Use non-extension method to avoid infinite recursion.
+            return ValidationServiceCollectionExtensions.AddValidation(services, options =>
+            {
+                options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());
+                if (configureOptions is not null)
+                {
+                    configureOptions(options);
+                }
+            });
         }
     }
 
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateParameters#ValidatableInfoResolver.g.verified.cs b/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateParameters#ValidatableInfoResolver.g.verified.cs
@@ -191,7 +191,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,
             // Use non-extension method to avoid infinite recursion.
             return ValidationServiceCollectionExtensions.AddValidation(services, options =>
             {
-                options.Resolvers.Add(new GeneratedValidatableInfoResolver());
+                options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());
                 if (configureOptions is not null)
                 {
                     configureOptions(options);
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidatePolymorphicTypes#ValidatableInfoResolver.g.verified.cs b/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidatePolymorphicTypes#ValidatableInfoResolver.g.verified.cs
@@ -295,7 +295,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,
             // Use non-extension method to avoid infinite recursion.
             return ValidationServiceCollectionExtensions.AddValidation(services, options =>
             {
-                options.Resolvers.Add(new GeneratedValidatableInfoResolver());
+                options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());
                 if (configureOptions is not null)
                 {
                     configureOptions(options);
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateRecursiveTypes#ValidatableInfoResolver.g.verified.cs b/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateRecursiveTypes#ValidatableInfoResolver.g.verified.cs
@@ -159,7 +159,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,
             // Use non-extension method to avoid infinite recursion.
             return ValidationServiceCollectionExtensions.AddValidation(services, options =>
             {
-                options.Resolvers.Add(new GeneratedValidatableInfoResolver());
+                options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());
                 if (configureOptions is not null)
                 {
                     configureOptions(options);
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateTypesWithAttribute#ValidatableInfoResolver.g.verified.cs b/src/Http/Http.Extensions/test/ValidationsGenerator/snapshots/ValidationsGeneratorTests.CanValidateTypesWithAttribute#ValidatableInfoResolver.g.verified.cs
@@ -260,7 +260,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,
             // Use non-extension method to avoid infinite recursion.
             return ValidationServiceCollectionExtensions.AddValidation(services, options =>
             {
-                options.Resolvers.Add(new GeneratedValidatableInfoResolver());
+                options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());
                 if (configureOptions is not null)
                 {
                     configureOptions(options);
diff --git a/src/Http/Http.Extensions/test/ValidationsGenerator/validations-design-doc.md b/src/Http/Http.Extensions/test/ValidationsGenerator/validations-design-doc.md
@@ -157,9 +157,10 @@ internal static class ValidationEndpointFilterFactory
         }
         var validatableParameters = parameters
             .Select(p => options.TryGetValidatableParameterInfo(p, out var validatableParameter) ? validatableParameter : null);
+        var validatableContext = new ValidatableContext { ValidationOptions = options };
         return async (context) =>
         {
-            var validationErrors = new Dictionary<string, string[]>();
+            validatableContext.ValidationErrors?.Clear();
 
             for (var i = 0; i < context.Arguments.Count; i++)
             {
@@ -171,15 +172,15 @@ internal static class ValidationEndpointFilterFactory
                     continue;
                 }
                 var validationContext = new ValidationContext(argument, context.HttpContext.RequestServices, items: null);
-                var validatableContext = new ValidatableContext(validationContext, string.Empty, options, validationErrors);
+                validatableContext.ValidationContext = validationContext;
                 await validatableParameter.Validate(argument, validatableContext);
             }
 
-            if (validationErrors.Count > 0)
+            if (validatableContext.ValidationErrors is { Count: > 0 })
             {
                 context.HttpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
                 context.HttpContext.Response.ContentType = "application/problem+json";
-                return await ValueTask.FromResult(new HttpValidationProblemDetails(validationErrors));
+                return await ValueTask.FromResult(new HttpValidationProblemDetails(validatableContext.ValidationErrors));
             }
 
             return await next(context);
diff --git a/src/Http/Routing/src/RouteEndpointDataSource.cs b/src/Http/Routing/src/RouteEndpointDataSource.cs
@@ -6,8 +6,10 @@
 using System.Runtime.CompilerServices;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Validation;
 using Microsoft.AspNetCore.Routing.Patterns;
 using Microsoft.Extensions.FileProviders;
+using Microsoft.Extensions.Options;
 using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.Routing;
@@ -59,7 +61,18 @@ public RouteHandlerBuilder AddRouteHandler(
         Func<Delegate, RequestDelegateFactoryOptions, RequestDelegateMetadataResult?, RequestDelegateResult> createHandlerRequestDelegateFunc,
         MethodInfo methodInfo)
     {
-        var conventions = new ThrowOnAddAfterEndpointBuiltConventionCollection();
+        // Initialize all route handlers with validation convention if validation options
+        // are registered.
+        var conventions = new ThrowOnAddAfterEndpointBuiltConventionCollection
+        {
+            (builder) =>
+            {
+                if (builder.ApplicationServices.GetService(typeof(IOptions<ValidationOptions>)) is not null)
+                {
+                    builder.FilterFactories.Add(ValidationEndpointFilterFactory.Create);
+                }
+            }
+        };
         var finallyConventions = new ThrowOnAddAfterEndpointBuiltConventionCollection();
 
         var routeAttributes = RouteAttributes.RouteHandler;
diff --git a/src/Http/Routing/src/ValidationEndpointFilterFactory.cs b/src/Http/Routing/src/ValidationEndpointFilterFactory.cs
@@ -23,6 +23,10 @@ public static EndpointFilterDelegate Create(EndpointFilterFactoryContext context
         }
         var validatableParameters = parameters
             .Select(p => options.TryGetValidatableParameterInfo(p, out var validatableParameter) ? validatableParameter : null);
+        if (validatableParameters.All(p => p is null))
+        {
+            return next;
+        }
         var validatableContext = new ValidatableContext { ValidationOptions = options };
         return async (context) =>
         {
@@ -49,7 +53,7 @@ public static EndpointFilterDelegate Create(EndpointFilterFactoryContext context
                 return await ValueTask.FromResult(new HttpValidationProblemDetails(validatableContext.ValidationErrors));
             }
 
-            return await next(context);
+            return next;
         };
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,`
`135`	`135`	`// Use non-extension method to avoid infinite recursion.`
`136`	`136`	`return ValidationServiceCollectionExtensions.AddValidation(services, options =>`
`137`	`137`	`{`
`138`		`- options.Resolvers.Add(new GeneratedValidatableInfoResolver());`
	`138`	`+ options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());`
`139`	`139`	`if (configureOptions is not null)`
`140`	`140`	`{`
`141`	`141`	`configureOptions(options);`
Original file line number	Diff line number	Diff line change
`@@ -5,8 +5,8 @@ namespace Microsoft.AspNetCore.Http.ValidationsGenerator.Tests;`
`5`	`5`
`6`	`6`	`public partial class ValidationsGeneratorTests : ValidationsGeneratorTestBase`
`7`	`7`	`{`
`8`		`- // [Fact]`
`9`		`- internal async Task CanValidateIValidatableObject()`
	`8`	`+ [Fact]`
	`9`	`+ public async Task CanValidateIValidatableObject()`
`10`	`10`	`{`
`11`	`11`	`var source = """`
`12`	`12`	`using System.Collections.Generic;`
Original file line number	Diff line number	Diff line change
`@@ -276,7 +276,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,`
`276`	`276`	`// Use non-extension method to avoid infinite recursion.`
`277`	`277`	`return ValidationServiceCollectionExtensions.AddValidation(services, options =>`
`278`	`278`	`{`
`279`		`- options.Resolvers.Add(new GeneratedValidatableInfoResolver());`
	`279`	`+ options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());`
`280`	`280`	`if (configureOptions is not null)`
`281`	`281`	`{`
`282`	`282`	`configureOptions(options);`
Original file line number	Diff line number	Diff line change
`@@ -221,10 +221,17 @@ private ValidatableParameterInfo CreateParameterInfomodel()`
`221`	`221`	`file static class GeneratedServiceCollectionExtensions`
`222`	`222`	`{`
`223`	`223`	`[global::System.Runtime.CompilerServices.InterceptsLocationAttribute(1, "ZbyAIis29Y/4fT/GGaRWq7ABAABQcm9ncmFtLmNz")]`
`224`		`- public static IServiceCollection AddValidation(this IServiceCollection services)`
	`224`	`+ public static IServiceCollection AddValidation(this IServiceCollection services, Action<ValidationOptions>? configureOptions = null)`
`225`	`225`	`{`
`226`		`- services.AddSingleton<global::Microsoft.AspNetCore.Http.Validation.IValidatableInfoResolver>(new GeneratedValidatableInfoResolver());`
`227`		`- return services;`
	`226`	`+ // Use non-extension method to avoid infinite recursion.`
	`227`	`+ return ValidationServiceCollectionExtensions.AddValidation(services, options =>`
	`228`	`+ {`
	`229`	`+ options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());`
	`230`	`+ if (configureOptions is not null)`
	`231`	`+ {`
	`232`	`+ configureOptions(options);`
	`233`	`+ }`
	`234`	`+ });`
`228`	`235`	`}`
`229`	`236`	`}`
`230`	`237`
Original file line number	Diff line number	Diff line change
`@@ -191,7 +191,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,`
`191`	`191`	`// Use non-extension method to avoid infinite recursion.`
`192`	`192`	`return ValidationServiceCollectionExtensions.AddValidation(services, options =>`
`193`	`193`	`{`
`194`		`- options.Resolvers.Add(new GeneratedValidatableInfoResolver());`
	`194`	`+ options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());`
`195`	`195`	`if (configureOptions is not null)`
`196`	`196`	`{`
`197`	`197`	`configureOptions(options);`
Original file line number	Diff line number	Diff line change
`@@ -295,7 +295,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,`
`295`	`295`	`// Use non-extension method to avoid infinite recursion.`
`296`	`296`	`return ValidationServiceCollectionExtensions.AddValidation(services, options =>`
`297`	`297`	`{`
`298`		`- options.Resolvers.Add(new GeneratedValidatableInfoResolver());`
	`298`	`+ options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());`
`299`	`299`	`if (configureOptions is not null)`
`300`	`300`	`{`
`301`	`301`	`configureOptions(options);`
Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,`
`159`	`159`	`// Use non-extension method to avoid infinite recursion.`
`160`	`160`	`return ValidationServiceCollectionExtensions.AddValidation(services, options =>`
`161`	`161`	`{`
`162`		`- options.Resolvers.Add(new GeneratedValidatableInfoResolver());`
	`162`	`+ options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());`
`163`	`163`	`if (configureOptions is not null)`
`164`	`164`	`{`
`165`	`165`	`configureOptions(options);`
Original file line number	Diff line number	Diff line change
`@@ -260,7 +260,7 @@ public static IServiceCollection AddValidation(this IServiceCollection services,`
`260`	`260`	`// Use non-extension method to avoid infinite recursion.`
`261`	`261`	`return ValidationServiceCollectionExtensions.AddValidation(services, options =>`
`262`	`262`	`{`
`263`		`- options.Resolvers.Add(new GeneratedValidatableInfoResolver());`
	`263`	`+ options.Resolvers.Insert(0, new GeneratedValidatableInfoResolver());`
`264`	`264`	`if (configureOptions is not null)`
`265`	`265`	`{`
`266`	`266`	`configureOptions(options);`
Original file line number	Diff line number	Diff line change
`@@ -157,9 +157,10 @@ internal static class ValidationEndpointFilterFactory`
`157`	`157`	`}`
`158`	`158`	`var validatableParameters = parameters`
`159`	`159`	`.Select(p => options.TryGetValidatableParameterInfo(p, out var validatableParameter) ? validatableParameter : null);`
	`160`	`+ var validatableContext = new ValidatableContext { ValidationOptions = options };`
`160`	`161`	`return async (context) =>`
`161`	`162`	`{`
`162`		`- var validationErrors = new Dictionary<string, string[]>();`
	`163`	`+ validatableContext.ValidationErrors?.Clear();`
`163`	`164`
`164`	`165`	`for (var i = 0; i < context.Arguments.Count; i++)`
`165`	`166`	`{`
`@@ -171,15 +172,15 @@ internal static class ValidationEndpointFilterFactory`
`171`	`172`	`continue;`
`172`	`173`	`}`
`173`	`174`	`var validationContext = new ValidationContext(argument, context.HttpContext.RequestServices, items: null);`
`174`		`- var validatableContext = new ValidatableContext(validationContext, string.Empty, options, validationErrors);`
	`175`	`+ validatableContext.ValidationContext = validationContext;`
`175`	`176`	`await validatableParameter.Validate(argument, validatableContext);`
`176`	`177`	`}`
`177`	`178`
`178`		`- if (validationErrors.Count > 0)`
	`179`	`+ if (validatableContext.ValidationErrors is { Count: > 0 })`
`179`	`180`	`{`
`180`	`181`	`context.HttpContext.Response.StatusCode = StatusCodes.Status400BadRequest;`
`181`	`182`	`context.HttpContext.Response.ContentType = "application/problem+json";`
`182`		`- return await ValueTask.FromResult(new HttpValidationProblemDetails(validationErrors));`
	`183`	`+ return await ValueTask.FromResult(new HttpValidationProblemDetails(validatableContext.ValidationErrors));`
`183`	`184`	`}`
`184`	`185`
`185`	`186`	`return await next(context);`