Skip to content

Commit 584d928

Browse files
davidfowlpranavkm
davidfowl
and
pranavkm
authored
Added ControllerRequestDelegateFactory (#27773)
* Added IRequestDelegateFactory abstraction - Today there's lots of indirection to figure out which invoker is responsible for executing a particular request based on the `ActionContext`. The `IActionInvokerFactory` calls through `IActionInvokerProviders` and an `IActionInvoker` is picked as a result of this. The default implementations based this decision on the type of action descriptor (though that's not required) which means its possible to avoid this per request indirection and select the invoker based on the descriptor. The `ControllerRequestDelegateFactory` creates a `RequestDelegate` for the endpoint don't go through indirection to find the ControllerActionInvoker. PS: This code path breaks extensibility for IActionInvoker* types that want to take over invocation of existing ControllerActionDescriptor since it no longer calls them in the endpoint routing code path. This may warrant a compatibility switch * Added compatibility switch to enable action invokers - This will allow people to turn off the request delegate factory if they were using that extensiblity point before. * Update src/Mvc/Mvc.Core/src/MvcOptions.cs Co-authored-by: Pranav K <[email protected]> * Add public API Co-authored-by: Pranav K <[email protected]>
1 parent aeb982d commit 584d928

File tree

11 files changed

+190
-17
lines changed

11 files changed

+190
-17
lines changed

‎src/Mvc/Mvc.Core/src/DependencyInjection/MvcCoreServiceCollectionExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ internal static void AddMvcCoreServices(IServiceCollection services)
273273
services.TryAddSingleton<OrderedEndpointsSequenceProviderCache>();
274274
services.TryAddSingleton<ControllerActionEndpointDataSourceIdProvider>();
275275
services.TryAddSingleton<ActionEndpointFactory>();
276+
services.TryAddSingleton<IRequestDelegateFactory, ControllerRequestDelegateFactory>();
276277
services.TryAddSingleton<DynamicControllerEndpointSelectorCache>();
277278
services.TryAddEnumerable(ServiceDescriptor.Singleton<MatcherPolicy, DynamicControllerEndpointMatcherPolicy>());
278279

‎src/Mvc/Mvc.Core/src/MvcOptions.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections;
66
using System.Collections.Generic;
77
using System.ComponentModel.DataAnnotations;
8+
using Microsoft.AspNetCore.Mvc.Abstractions;
89
using Microsoft.AspNetCore.Mvc.ApplicationModels;
910
using Microsoft.AspNetCore.Mvc.Controllers;
1011
using Microsoft.AspNetCore.Mvc.Filters;
@@ -137,6 +138,14 @@ public MvcOptions()
137138
/// </summary>
138139
public bool SuppressOutputFormatterBuffering { get; set; }
139140

141+
/// <summary>
142+
/// Gets or sets the flag that determines if MVC should use action invoker extensibility. This will allow
143+
/// custom <see cref="IActionInvokerFactory"/> and <see cref="IActionInvokerProvider"/> execute during the request pipeline.
144+
/// </summary>
145+
/// <remarks>This only applies when <see cref="EnableEndpointRouting"/> is true.</remarks>
146+
/// <value>Defaults to <see langword="false" /> indicating that action invokers are unused by default.</value>
147+
public bool EnableActionInvokers { get; set; }
148+
140149
/// <summary>
141150
/// Gets or sets the maximum number of validation errors that are allowed by this application before further
142151
/// errors are ignored.

‎src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,8 @@ Microsoft.AspNetCore.Mvc.ModelMetadataTypeAttribute
632632
Microsoft.AspNetCore.Mvc.MvcOptions
633633
Microsoft.AspNetCore.Mvc.MvcOptions.AllowEmptyInputInBodyModelBinding.get -> bool
634634
Microsoft.AspNetCore.Mvc.MvcOptions.AllowEmptyInputInBodyModelBinding.set -> void
635+
Microsoft.AspNetCore.Mvc.MvcOptions.EnableActionInvokers.get -> bool
636+
Microsoft.AspNetCore.Mvc.MvcOptions.EnableActionInvokers.set -> void
635637
Microsoft.AspNetCore.Mvc.MvcOptions.EnableEndpointRouting.get -> bool
636638
Microsoft.AspNetCore.Mvc.MvcOptions.EnableEndpointRouting.set -> void
637639
Microsoft.AspNetCore.Mvc.MvcOptions.MaxIAsyncEnumerableBufferLimit.get -> int

‎src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-
#nullable enable

‎src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -21,8 +21,9 @@ internal class ActionEndpointFactory
2121
{
2222
private readonly RoutePatternTransformer _routePatternTransformer;
2323
private readonly RequestDelegate _requestDelegate;
24+
private readonly IRequestDelegateFactory[] _requestDelegateFactories;
2425

25-
public ActionEndpointFactory(RoutePatternTransformer routePatternTransformer)
26+
public ActionEndpointFactory(RoutePatternTransformer routePatternTransformer, IEnumerable<IRequestDelegateFactory> requestDelegateFactories)
2627
{
2728
if (routePatternTransformer == null)
2829
{
@@ -31,6 +32,7 @@ public ActionEndpointFactory(RoutePatternTransformer routePatternTransformer)
3132

3233
_routePatternTransformer = routePatternTransformer;
3334
_requestDelegate = CreateRequestDelegate();
35+
_requestDelegateFactories = requestDelegateFactories.ToArray();
3436
}
3537

3638
public void AddEndpoints(
@@ -102,9 +104,11 @@ public void AddEndpoints(
102104
continue;
103105
}
104106

107+
var requestDelegate = CreateRequestDelegate(action, route.DataTokens) ?? _requestDelegate;
108+
105109
// We suppress link generation for each conventionally routed endpoint. We generate a single endpoint per-route
106110
// to handle link generation.
107-
var builder = new RouteEndpointBuilder(_requestDelegate, updatedRoutePattern, route.Order)
111+
var builder = new RouteEndpointBuilder(requestDelegate, updatedRoutePattern, route.Order)
108112
{
109113
DisplayName = action.DisplayName,
110114
};
@@ -123,6 +127,7 @@ public void AddEndpoints(
123127
}
124128
else
125129
{
130+
var requestDelegate = CreateRequestDelegate(action) ?? _requestDelegate;
126131
var attributeRoutePattern = RoutePatternFactory.Parse(action.AttributeRouteInfo.Template);
127132

128133
// Modify the route and required values to ensure required values can be successfully subsituted.
@@ -142,7 +147,7 @@ public void AddEndpoints(
142147
$"To fix this error, choose a different parameter name.");
143148
}
144149

145-
var builder = new RouteEndpointBuilder(_requestDelegate, updatedRoutePattern, action.AttributeRouteInfo.Order)
150+
var builder = new RouteEndpointBuilder(requestDelegate, updatedRoutePattern, action.AttributeRouteInfo.Order)
146151
{
147152
DisplayName = action.DisplayName,
148153
};
@@ -405,6 +410,20 @@ private void AddActionDataToBuilder(
405410
}
406411
}
407412

413+
private RequestDelegate CreateRequestDelegate(ActionDescriptor action, RouteValueDictionary dataTokens = null)
414+
{
415+
foreach (var factory in _requestDelegateFactories)
416+
{
417+
var rd = factory.CreateRequestDelegate(action, dataTokens);
418+
if (rd != null)
419+
{
420+
return rd;
421+
}
422+
}
423+
424+
return null;
425+
}
426+
408427
private static RequestDelegate CreateRequestDelegate()
409428
{
410429
// We don't want to close over the Invoker Factory in ActionEndpointFactory as

‎src/Mvc/Mvc.Core/src/Routing/ControllerRequestDelegateFactory.cs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
using System.Collections.Generic;
2+
using System.Diagnostics;
3+
using System.Linq;
4+
using Microsoft.AspNetCore.Http;
5+
using Microsoft.AspNetCore.Mvc.Abstractions;
6+
using Microsoft.AspNetCore.Mvc.Controllers;
7+
using Microsoft.AspNetCore.Mvc.Infrastructure;
8+
using Microsoft.AspNetCore.Mvc.ModelBinding;
9+
using Microsoft.AspNetCore.Routing;
10+
using Microsoft.Extensions.Logging;
11+
using Microsoft.Extensions.Options;
12+
13+
namespace Microsoft.AspNetCore.Mvc.Routing
14+
{
15+
internal class ControllerRequestDelegateFactory : IRequestDelegateFactory
16+
{
17+
private readonly ControllerActionInvokerCache _controllerActionInvokerCache;
18+
private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories;
19+
private readonly int _maxModelValidationErrors;
20+
private readonly ILogger _logger;
21+
private readonly DiagnosticListener _diagnosticListener;
22+
private readonly IActionResultTypeMapper _mapper;
23+
private readonly IActionContextAccessor _actionContextAccessor;
24+
private readonly bool _enableActionInvokers;
25+
26+
public ControllerRequestDelegateFactory(
27+
ControllerActionInvokerCache controllerActionInvokerCache,
28+
IOptions<MvcOptions> optionsAccessor,
29+
ILoggerFactory loggerFactory,
30+
DiagnosticListener diagnosticListener,
31+
IActionResultTypeMapper mapper)
32+
: this(controllerActionInvokerCache, optionsAccessor, loggerFactory, diagnosticListener, mapper, null)
33+
{
34+
}
35+
36+
public ControllerRequestDelegateFactory(
37+
ControllerActionInvokerCache controllerActionInvokerCache,
38+
IOptions<MvcOptions> optionsAccessor,
39+
ILoggerFactory loggerFactory,
40+
DiagnosticListener diagnosticListener,
41+
IActionResultTypeMapper mapper,
42+
IActionContextAccessor actionContextAccessor)
43+
{
44+
_controllerActionInvokerCache = controllerActionInvokerCache;
45+
_valueProviderFactories = optionsAccessor.Value.ValueProviderFactories.ToArray();
46+
_maxModelValidationErrors = optionsAccessor.Value.MaxModelValidationErrors;
47+
_enableActionInvokers = optionsAccessor.Value.EnableActionInvokers;
48+
_logger = loggerFactory.CreateLogger<ControllerActionInvoker>();
49+
_diagnosticListener = diagnosticListener;
50+
_mapper = mapper;
51+
_actionContextAccessor = actionContextAccessor ?? ActionContextAccessor.Null;
52+
}
53+
54+
public RequestDelegate CreateRequestDelegate(ActionDescriptor actionDescriptor, RouteValueDictionary dataTokens)
55+
{
56+
// Fallback to action invoker extensibility so that invokers can override any default behaviors
57+
if (!_enableActionInvokers && actionDescriptor is ControllerActionDescriptor)
58+
{
59+
return context =>
60+
{
61+
RouteData routeData = null;
62+
63+
if (dataTokens is null or { Count: 0 })
64+
{
65+
routeData = new RouteData(context.Request.RouteValues);
66+
}
67+
else
68+
{
69+
routeData = new RouteData();
70+
routeData.PushState(router: null, context.Request.RouteValues, dataTokens);
71+
}
72+
73+
var actionContext = new ActionContext(context, routeData, actionDescriptor);
74+
75+
var controllerContext = new ControllerContext(actionContext)
76+
{
77+
// PERF: These are rarely going to be changed, so let's go copy-on-write.
78+
ValueProviderFactories = new CopyOnWriteList<IValueProviderFactory>(_valueProviderFactories)
79+
};
80+
81+
controllerContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
82+
83+
var (cacheEntry, filters) = _controllerActionInvokerCache.GetCachedResult(controllerContext);
84+
85+
var invoker = new ControllerActionInvoker(
86+
_logger,
87+
_diagnosticListener,
88+
_actionContextAccessor,
89+
_mapper,
90+
controllerContext,
91+
cacheEntry,
92+
filters);
93+
94+
return invoker.InvokeAsync();
95+
};
96+
}
97+
98+
return null;
99+
}
100+
}
101+
}

‎src/Mvc/Mvc.Core/src/Routing/IRequestDelegateFactory.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Http;
5+
using Microsoft.AspNetCore.Mvc.Abstractions;
6+
using Microsoft.AspNetCore.Routing;
7+
8+
namespace Microsoft.AspNetCore.Mvc.Routing
9+
{
10+
/// <summary>
11+
/// Internal interfaces that allows us to optimize the request execution path based on ActionDescriptor
12+
/// </summary>
13+
internal interface IRequestDelegateFactory
14+
{
15+
RequestDelegate CreateRequestDelegate(ActionDescriptor actionDescriptor, RouteValueDictionary dataTokens);
16+
}
17+
}

‎src/Mvc/Mvc.Core/test/Routing/ActionEndpointDataSourceBaseTest.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Linq;
67
using System.Threading;
78
using Microsoft.AspNetCore.Mvc.Abstractions;
89
using Microsoft.AspNetCore.Mvc.Infrastructure;
@@ -148,7 +149,7 @@ protected private ActionEndpointDataSourceBase CreateDataSource(IActionDescripto
148149

149150
var serviceProvider = services.BuildServiceProvider();
150151

151-
var endpointFactory = new ActionEndpointFactory(serviceProvider.GetRequiredService<RoutePatternTransformer>());
152+
var endpointFactory = new ActionEndpointFactory(serviceProvider.GetRequiredService<RoutePatternTransformer>(), Enumerable.Empty<IRequestDelegateFactory>());
152153

153154
return CreateDataSource(actions, endpointFactory);
154155
}
@@ -168,4 +169,4 @@ protected abstract ActionDescriptor CreateActionDescriptor(
168169
string pattern = null,
169170
IList<object> metadata = null);
170171
}
171-
}
172+
}

‎src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -32,7 +32,7 @@ public ActionEndpointFactoryTest()
3232
});
3333

3434
Services = serviceCollection.BuildServiceProvider();
35-
Factory = new ActionEndpointFactory(Services.GetRequiredService<RoutePatternTransformer>());
35+
Factory = new ActionEndpointFactory(Services.GetRequiredService<RoutePatternTransformer>(), Enumerable.Empty<IRequestDelegateFactory>());
3636
}
3737

3838
internal ActionEndpointFactory Factory { get; }
@@ -261,6 +261,29 @@ public void AddEndpoints_AttributeRouted_WithRouteName_EndpointCreated()
261261
Assert.Equal("Test", endpoint.Metadata.GetMetadata<IEndpointNameMetadata>().EndpointName);
262262
}
263263

264+
[Fact]
265+
public void RequestDelegateFactoryWorks()
266+
{
267+
// Arrange
268+
var values = new { controller = "TestController", action = "TestAction", page = (string)null };
269+
var action = CreateActionDescriptor(values, "{controller}/{action}/{page}");
270+
action.AttributeRouteInfo.Name = "Test";
271+
RequestDelegate del = context => Task.CompletedTask;
272+
var requestDelegateFactory = new Mock<IRequestDelegateFactory>();
273+
requestDelegateFactory.Setup(m => m.CreateRequestDelegate(action, It.IsAny<RouteValueDictionary>())).Returns(del);
274+
275+
// Act
276+
var factory = new ActionEndpointFactory(Services.GetRequiredService<RoutePatternTransformer>(), new[] { requestDelegateFactory.Object });
277+
278+
var endpoints = new List<Endpoint>();
279+
factory.AddEndpoints(endpoints, new HashSet<string>(), action, Array.Empty<ConventionalRouteEntry>(), Array.Empty<Action<EndpointBuilder>>(), createInertEndpoints: false);
280+
281+
var endpoint = Assert.IsType<RouteEndpoint>(Assert.Single(endpoints));
282+
283+
// Assert
284+
Assert.Same(del, endpoint.RequestDelegate);
285+
}
286+
264287
[Fact]
265288
public void AddEndpoints_ConventionalRouted_WithMatchingConstraint_CreatesEndpoint()
266289
{

‎src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageLoaderTest.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Linq;
56
using System.Reflection;
67
using System.Threading.Tasks;
78
using Microsoft.AspNetCore.Mvc.Abstractions;
@@ -38,7 +39,7 @@ public async Task LoadAsync_InvokesApplicationModelProviders()
3839
var compilerProvider = GetCompilerProvider();
3940

4041
var mvcOptions = Options.Create(new MvcOptions());
41-
var endpointFactory = new ActionEndpointFactory(Mock.Of<RoutePatternTransformer>());
42+
var endpointFactory = new ActionEndpointFactory(Mock.Of<RoutePatternTransformer>(), Enumerable.Empty<IRequestDelegateFactory>());
4243

4344
var provider1 = new Mock<IPageApplicationModelProvider>();
4445
var provider2 = new Mock<IPageApplicationModelProvider>();
@@ -122,7 +123,7 @@ public async Task LoadAsync_CreatesEndpoint_WithRoute()
122123
var compilerProvider = GetCompilerProvider();
123124

124125
var mvcOptions = Options.Create(new MvcOptions());
125-
var endpointFactory = new ActionEndpointFactory(transformer.Object);
126+
var endpointFactory = new ActionEndpointFactory(transformer.Object, Enumerable.Empty<IRequestDelegateFactory>());
126127

127128
var provider = new Mock<IPageApplicationModelProvider>();
128129

@@ -163,7 +164,7 @@ public async Task LoadAsync_InvokesApplicationModelProviders_WithTheRightOrder()
163164
var descriptor = new PageActionDescriptor();
164165
var compilerProvider = GetCompilerProvider();
165166
var mvcOptions = Options.Create(new MvcOptions());
166-
var endpointFactory = new ActionEndpointFactory(Mock.Of<RoutePatternTransformer>());
167+
var endpointFactory = new ActionEndpointFactory(Mock.Of<RoutePatternTransformer>(), Enumerable.Empty<IRequestDelegateFactory>());
167168

168169
var provider1 = new Mock<IPageApplicationModelProvider>();
169170
provider1.SetupGet(p => p.Order).Returns(10);
@@ -241,7 +242,7 @@ public async Task LoadAsync_CachesResults()
241242
var compilerProvider = GetCompilerProvider();
242243

243244
var mvcOptions = Options.Create(new MvcOptions());
244-
var endpointFactory = new ActionEndpointFactory(transformer.Object);
245+
var endpointFactory = new ActionEndpointFactory(transformer.Object, Enumerable.Empty<IRequestDelegateFactory>());
245246

246247
var provider = new Mock<IPageApplicationModelProvider>();
247248

@@ -296,7 +297,7 @@ public async Task LoadAsync_UpdatesResults()
296297
var compilerProvider = GetCompilerProvider();
297298

298299
var mvcOptions = Options.Create(new MvcOptions());
299-
var endpointFactory = new ActionEndpointFactory(transformer.Object);
300+
var endpointFactory = new ActionEndpointFactory(transformer.Object, Enumerable.Empty<IRequestDelegateFactory>());
300301

301302
var provider = new Mock<IPageApplicationModelProvider>();
302303

‎src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ControllerActionEndpointDatasourceBenchmark.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ private ControllerActionEndpointDataSource CreateDataSource(IActionDescriptorCol
111111
var dataSource = new ControllerActionEndpointDataSource(
112112
new ControllerActionEndpointDataSourceIdProvider(),
113113
actionDescriptorCollectionProvider,
114-
new ActionEndpointFactory(new MockRoutePatternTransformer()),
114+
new ActionEndpointFactory(new MockRoutePatternTransformer(), Enumerable.Empty<IRequestDelegateFactory>()),
115115
new OrderedEndpointsSequenceProvider());
116116

117117
return dataSource;

0 commit comments

Comments
 (0)