Skip to content

Commit 3b76937

Browse files
davidfowlpranavkm
davidfowl
and
pranavkm
authored
Added PageRequestDelegateFactory (#28134)
* Added PageRequestDelegateFactory - Implemented the PageRequestDelegateFactory for razor pages - Refactored the PageActionInvokerProvider into a PageActionInvokerCache to mimic the controller implementation and to share logic. - Moved the cache to be part of the CompiledPageActionDescriptor itself. This simplifies the looked and expiration as it is tied to the CompiledPageActionDescriptor instance. - Skip the page loader for already compiled pages * Updated the service collection tests * Fixed another test * Apply suggestions from code review Co-authored-by: Pranav K <[email protected]> * Remove extra ctor and use default value Co-authored-by: Pranav K <[email protected]>
1 parent 5c2cb8b commit 3b76937

17 files changed

+452
-438
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,9 @@ internal static void AddMvcCoreServices(IServiceCollection services)
273273
services.TryAddSingleton<OrderedEndpointsSequenceProviderCache>();
274274
services.TryAddSingleton<ControllerActionEndpointDataSourceIdProvider>();
275275
services.TryAddSingleton<ActionEndpointFactory>();
276-
services.TryAddSingleton<IRequestDelegateFactory, ControllerRequestDelegateFactory>();
277276
services.TryAddSingleton<DynamicControllerEndpointSelectorCache>();
278277
services.TryAddEnumerable(ServiceDescriptor.Singleton<MatcherPolicy, DynamicControllerEndpointMatcherPolicy>());
278+
services.TryAddEnumerable(ServiceDescriptor.Singleton<IRequestDelegateFactory, ControllerRequestDelegateFactory>());
279279

280280
//
281281
// Middleware pipeline filter related

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

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -54,48 +54,48 @@ public ControllerRequestDelegateFactory(
5454
public RequestDelegate CreateRequestDelegate(ActionDescriptor actionDescriptor, RouteValueDictionary dataTokens)
5555
{
5656
// Fallback to action invoker extensibility so that invokers can override any default behaviors
57-
if (!_enableActionInvokers && actionDescriptor is ControllerActionDescriptor)
57+
if (_enableActionInvokers || actionDescriptor is not ControllerActionDescriptor)
5858
{
59-
return context =>
60-
{
61-
RouteData routeData = null;
59+
return null;
60+
}
6261

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-
}
62+
return context =>
63+
{
64+
RouteData routeData = null;
7265

73-
var actionContext = new ActionContext(context, routeData, actionDescriptor);
66+
if (dataTokens is null or { Count: 0 })
67+
{
68+
routeData = new RouteData(context.Request.RouteValues);
69+
}
70+
else
71+
{
72+
routeData = new RouteData();
73+
routeData.PushState(router: null, context.Request.RouteValues, dataTokens);
74+
}
7475

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-
};
76+
var actionContext = new ActionContext(context, routeData, actionDescriptor);
8077

81-
controllerContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
78+
var controllerContext = new ControllerContext(actionContext)
79+
{
80+
// PERF: These are rarely going to be changed, so let's go copy-on-write.
81+
ValueProviderFactories = new CopyOnWriteList<IValueProviderFactory>(_valueProviderFactories)
82+
};
8283

83-
var (cacheEntry, filters) = _controllerActionInvokerCache.GetCachedResult(controllerContext);
84+
controllerContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
8485

85-
var invoker = new ControllerActionInvoker(
86-
_logger,
87-
_diagnosticListener,
88-
_actionContextAccessor,
89-
_mapper,
90-
controllerContext,
91-
cacheEntry,
92-
filters);
86+
var (cacheEntry, filters) = _controllerActionInvokerCache.GetCachedResult(controllerContext);
9387

94-
return invoker.InvokeAsync();
95-
};
96-
}
88+
var invoker = new ControllerActionInvoker(
89+
_logger,
90+
_diagnosticListener,
91+
_actionContextAccessor,
92+
_mapper,
93+
controllerContext,
94+
cacheEntry,
95+
filters);
9796

98-
return null;
97+
return invoker.InvokeAsync();
98+
};
9999
}
100100
}
101101
}

‎src/Mvc/Mvc.Core/test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,13 @@ private Dictionary<Type, Type[]> MultiRegistrationServiceTypes
285285
typeof(ControllerActionInvokerProvider),
286286
}
287287
},
288+
{
289+
typeof(IRequestDelegateFactory),
290+
new Type[]
291+
{
292+
typeof(ControllerRequestDelegateFactory)
293+
}
294+
},
288295
{
289296
typeof(IFilterProvider),
290297
new Type[]
@@ -366,4 +373,4 @@ private void AssertContainsSingle(
366373
}
367374
}
368375
}
369-
}
376+
}

‎src/Mvc/Mvc.RazorPages/src/CompiledPageActionDescriptor.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
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

4+
using System;
45
using System.Collections.Generic;
56
using System.Reflection;
67
using Microsoft.AspNetCore.Http;
@@ -65,5 +66,13 @@ public CompiledPageActionDescriptor(PageActionDescriptor actionDescriptor)
6566
/// Gets or sets the associated <see cref="Endpoint"/> of this page.
6667
/// </summary>
6768
public Endpoint Endpoint { get; set; }
69+
70+
internal PageActionInvokerCacheEntry CacheEntry { get; set; }
71+
72+
internal override CompiledPageActionDescriptor CompiledPageDescriptor
73+
{
74+
get => this;
75+
set => throw new InvalidOperationException("Setting the compiled descriptor on a compiled descriptor is not allowed.");
76+
}
6877
}
6978
}

‎src/Mvc/Mvc.RazorPages/src/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using Microsoft.AspNetCore.Mvc.Razor;
99
using Microsoft.AspNetCore.Mvc.RazorPages;
1010
using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
11+
using Microsoft.AspNetCore.Mvc.Routing;
1112
using Microsoft.AspNetCore.Routing;
1213
using Microsoft.Extensions.DependencyInjection.Extensions;
1314
using Microsoft.Extensions.Options;
@@ -131,6 +132,9 @@ internal static void AddRazorPagesServices(IServiceCollection services)
131132

132133
services.TryAddEnumerable(
133134
ServiceDescriptor.Singleton<IActionInvokerProvider, PageActionInvokerProvider>());
135+
services.TryAddEnumerable(
136+
ServiceDescriptor.Singleton<IRequestDelegateFactory, PageRequestDelegateFactory>());
137+
services.TryAddSingleton<PageActionInvokerCache>();
134138

135139
// Page and Page model creation and activation
136140
services.TryAddSingleton<IPageModelActivatorProvider, DefaultPageModelActivatorProvider>();

‎src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs

Lines changed: 7 additions & 41 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;
@@ -11,7 +11,6 @@
1111
using Microsoft.AspNetCore.Http;
1212
using Microsoft.AspNetCore.Mvc.ApplicationModels;
1313
using Microsoft.AspNetCore.Mvc.Filters;
14-
using Microsoft.AspNetCore.Mvc.Infrastructure;
1514
using Microsoft.AspNetCore.Mvc.Razor.Compilation;
1615
using Microsoft.AspNetCore.Mvc.Routing;
1716
using Microsoft.Extensions.Options;
@@ -20,23 +19,19 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
2019
{
2120
internal class DefaultPageLoader : PageLoader
2221
{
23-
private readonly IActionDescriptorCollectionProvider _collectionProvider;
2422
private readonly IPageApplicationModelProvider[] _applicationModelProviders;
2523
private readonly IViewCompilerProvider _viewCompilerProvider;
2624
private readonly ActionEndpointFactory _endpointFactory;
2725
private readonly PageConventionCollection _conventions;
2826
private readonly FilterCollection _globalFilters;
29-
private volatile InnerCache _currentCache;
3027

3128
public DefaultPageLoader(
32-
IActionDescriptorCollectionProvider actionDescriptorCollectionProvider,
3329
IEnumerable<IPageApplicationModelProvider> applicationModelProviders,
3430
IViewCompilerProvider viewCompilerProvider,
3531
ActionEndpointFactory endpointFactory,
3632
IOptions<RazorPagesOptions> pageOptions,
3733
IOptions<MvcOptions> mvcOptions)
3834
{
39-
_collectionProvider = actionDescriptorCollectionProvider;
4035
_applicationModelProviders = applicationModelProviders
4136
.OrderBy(p => p.Order)
4237
.ToArray();
@@ -49,23 +44,6 @@ public DefaultPageLoader(
4944

5045
private IViewCompiler Compiler => _viewCompilerProvider.GetCompiler();
5146

52-
private ConcurrentDictionary<PageActionDescriptor, Task<CompiledPageActionDescriptor>> CurrentCache
53-
{
54-
get
55-
{
56-
var current = _currentCache;
57-
var actionDescriptors = _collectionProvider.ActionDescriptors;
58-
59-
if (current == null || current.Version != actionDescriptors.Version)
60-
{
61-
current = new InnerCache(actionDescriptors.Version);
62-
_currentCache = current;
63-
}
64-
65-
return current.Entries;
66-
}
67-
}
68-
6947
public override Task<CompiledPageActionDescriptor> LoadAsync(PageActionDescriptor actionDescriptor)
7048
=> LoadAsync(actionDescriptor, EndpointMetadataCollection.Empty);
7149

@@ -76,13 +54,14 @@ internal Task<CompiledPageActionDescriptor> LoadAsync(PageActionDescriptor actio
7654
throw new ArgumentNullException(nameof(actionDescriptor));
7755
}
7856

79-
var cache = CurrentCache;
80-
if (cache.TryGetValue(actionDescriptor, out var compiledDescriptorTask))
57+
var task = actionDescriptor.CompiledPageActionDescriptorTask;
58+
59+
if (task != null)
8160
{
82-
return compiledDescriptorTask;
61+
return task;
8362
}
8463

85-
return cache.GetOrAdd(actionDescriptor, LoadAsyncCore(actionDescriptor, endpointMetadata));
64+
return actionDescriptor.CompiledPageActionDescriptorTask = LoadAsyncCore(actionDescriptor, endpointMetadata);
8665
}
8766

8867
private async Task<CompiledPageActionDescriptor> LoadAsyncCore(PageActionDescriptor actionDescriptor, EndpointMetadataCollection endpointMetadata)
@@ -182,18 +161,5 @@ IEnumerable<TConvention> GetConventions<TConvention>(
182161
attributes.OfType<TConvention>());
183162
}
184163
}
185-
186-
private sealed class InnerCache
187-
{
188-
public InnerCache(int version)
189-
{
190-
Version = version;
191-
Entries = new ConcurrentDictionary<PageActionDescriptor, Task<CompiledPageActionDescriptor>>();
192-
}
193-
194-
public ConcurrentDictionary<PageActionDescriptor, Task<CompiledPageActionDescriptor>> Entries { get; }
195-
196-
public int Version { get; }
197-
}
198164
}
199-
}
165+
}

‎src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvoker.cs

Lines changed: 1 addition & 5 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;
@@ -10,7 +10,6 @@
1010
using Microsoft.AspNetCore.Mvc.Abstractions;
1111
using Microsoft.AspNetCore.Mvc.Filters;
1212
using Microsoft.AspNetCore.Mvc.Infrastructure;
13-
using Microsoft.AspNetCore.Mvc.ModelBinding;
1413
using Microsoft.AspNetCore.Mvc.Rendering;
1514
using Microsoft.AspNetCore.Mvc.ViewFeatures;
1615
using Microsoft.Extensions.Internal;
@@ -22,7 +21,6 @@ internal class PageActionInvoker : ResourceInvoker, IActionInvoker
2221
{
2322
private readonly IPageHandlerMethodSelector _selector;
2423
private readonly PageContext _pageContext;
25-
private readonly ParameterBinder _parameterBinder;
2624
private readonly ITempDataDictionaryFactory _tempDataFactory;
2725
private readonly HtmlHelperOptions _htmlHelperOptions;
2826
private readonly CompiledPageActionDescriptor _actionDescriptor;
@@ -46,7 +44,6 @@ public PageActionInvoker(
4644
PageContext pageContext,
4745
IFilterMetadata[] filterMetadata,
4846
PageActionInvokerCacheEntry cacheEntry,
49-
ParameterBinder parameterBinder,
5047
ITempDataDictionaryFactory tempDataFactory,
5148
HtmlHelperOptions htmlHelperOptions)
5249
: base(
@@ -61,7 +58,6 @@ public PageActionInvoker(
6158
_selector = handlerMethodSelector;
6259
_pageContext = pageContext;
6360
CacheEntry = cacheEntry;
64-
_parameterBinder = parameterBinder;
6561
_tempDataFactory = tempDataFactory;
6662
_htmlHelperOptions = htmlHelperOptions;
6763

0 commit comments

Comments
 (0)