From f2a1a4542e9849189207b53a54b06911b09f00be Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 13 Feb 2019 18:52:07 -0800 Subject: [PATCH] Introduce dynamic endpoints and fix #7011 (#7445) * Add IDynamicEndpointMetadata for dynamic endpoints * Use a dynamic endpoint policy for pages --- .../Routing/src/IDynamicEndpointMetadata.cs | 34 ++++++ src/Http/Routing/src/Matching/CandidateSet.cs | 53 ++++++++- .../Routing/src/Matching/MatcherPolicy.cs | 30 +++++- .../test/UnitTests/MatcherPolicyTest.cs | 92 ++++++++++++++++ .../UnitTests/Matching/CandidateSetTest.cs | 87 ++++++++++++++- .../ActionEndpointDatasourceBenchmark.cs | 5 +- .../MvcCoreServiceCollectionExtensions.cs | 1 - .../Infrastructure/ActionContextAccessor.cs | 11 ++ .../Infrastructure/ControllerActionInvoker.cs | 3 +- .../ControllerActionInvokerProvider.cs | 14 +++ .../Infrastructure/ResourceInvoker.cs | 5 + .../Routing/ActionEndpointFactory.cs | 31 +++--- .../Routing/MvcAttributeRouteHandler.cs | 22 +--- .../Routing/MvcEndpointInvokerFactory.cs | 41 ------- .../Routing/MvcRouteHandler.cs | 20 ---- .../CompiledPageActionDescriptor.cs | 6 ++ .../MvcRazorPagesMvcCoreBuilderExtensions.cs | 4 + .../Infrastructure/DefaultPageLoader.cs | 22 +++- .../PageActionDescriptorProvider.cs | 10 ++ .../Infrastructure/PageActionInvoker.cs | 2 + .../PageActionInvokerProvider.cs | 43 ++++++++ .../Infrastructure/PageLoaderMatcherPolicy.cs | 89 +++++++++++++++ .../Filters/MiddlewareFilterTest.cs | 1 + .../ControllerActionInvokerTest.cs | 2 + .../ActionEndpointDataSourceBaseTest.cs | 4 +- .../Routing/ActionEndpointFactoryTest.cs | 101 +----------------- .../Routing/MvcRouteHandlerTests.cs | 5 +- .../Infrastructure/DefaultPageLoaderTest.cs | 61 +++++++++++ .../PageActionDescriptorProviderTest.cs | 3 +- .../Infrastructure/PageActionInvokerTest.cs | 1 + .../Policy/src/AuthorizationMiddleware.cs | 7 -- 31 files changed, 589 insertions(+), 221 deletions(-) create mode 100644 src/Http/Routing/src/IDynamicEndpointMetadata.cs create mode 100644 src/Http/Routing/test/UnitTests/MatcherPolicyTest.cs delete mode 100644 src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcEndpointInvokerFactory.cs create mode 100644 src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageLoaderMatcherPolicy.cs diff --git a/src/Http/Routing/src/IDynamicEndpointMetadata.cs b/src/Http/Routing/src/IDynamicEndpointMetadata.cs new file mode 100644 index 0000000000..9823add575 --- /dev/null +++ b/src/Http/Routing/src/IDynamicEndpointMetadata.cs @@ -0,0 +1,34 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing.Matching; + +namespace Microsoft.AspNetCore.Routing +{ + /// + /// A metadata interface that can be used to specify that the associated + /// will be dynamically replaced during matching. + /// + /// + /// + /// and related derived interfaces signal to + /// implementations that an has dynamic behavior + /// and thus cannot have its characteristics cached. + /// + /// + /// Using dynamic endpoints can be useful because the default matcher implementation does not + /// supply extensibility for how URLs are processed. Routing implementations that have dynamic + /// behavior can apply their dynamic logic after URL processing, by replacing a endpoints as + /// part of a . + /// + /// + public interface IDynamicEndpointMetadata + { + /// + /// Returns a value that indicates whether the associated endpoint has dynamic matching + /// behavior. + /// + bool IsDynamic { get; } + } +} diff --git a/src/Http/Routing/src/Matching/CandidateSet.cs b/src/Http/Routing/src/Matching/CandidateSet.cs index 4a004018b6..42392268fa 100644 --- a/src/Http/Routing/src/Matching/CandidateSet.cs +++ b/src/Http/Routing/src/Matching/CandidateSet.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -281,6 +281,57 @@ namespace Microsoft.AspNetCore.Routing.Matching } } + /// + /// Replaces the at the provided with the + /// provided . + /// + /// The candidate index. + /// + /// The to replace the original at + /// the . If the candidate will be marked + /// as invalid. + /// + /// + /// The to replace the original at + /// the . + /// + public void ReplaceEndpoint(int index, Endpoint endpoint, RouteValueDictionary values) + { + // Friendliness for inlining + if ((uint)index >= Count) + { + ThrowIndexArgumentOutOfRangeException(); + } + + switch (index) + { + case 0: + _state0 = new CandidateState(endpoint, values, _state0.Score); + break; + + case 1: + _state1 = new CandidateState(endpoint, values, _state1.Score); + break; + + case 2: + _state2 = new CandidateState(endpoint, values, _state2.Score); + break; + + case 3: + _state3 = new CandidateState(endpoint, values, _state3.Score); + break; + + default: + _additionalCandidates[index - 4] = new CandidateState(endpoint, values, _additionalCandidates[index - 4].Score); + break; + } + + if (endpoint == null) + { + SetValidity(index, false); + } + } + private static void ThrowIndexArgumentOutOfRangeException() { throw new ArgumentOutOfRangeException("index"); diff --git a/src/Http/Routing/src/Matching/MatcherPolicy.cs b/src/Http/Routing/src/Matching/MatcherPolicy.cs index 05a654efd9..f3fdc05fb1 100644 --- a/src/Http/Routing/src/Matching/MatcherPolicy.cs +++ b/src/Http/Routing/src/Matching/MatcherPolicy.cs @@ -1,6 +1,9 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Matching; namespace Microsoft.AspNetCore.Routing @@ -24,5 +27,30 @@ namespace Microsoft.AspNetCore.Routing /// property. /// public abstract int Order { get; } + + /// + /// Returns a value that indicates whether the provided contains + /// one or more dynamic endpoints. + /// + /// The set of endpoints. + /// true if a dynamic endpoint is found; otherwise returns false. + protected static bool ContainsDynamicEndpoints(IReadOnlyList endpoints) + { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + for (var i = 0; i < endpoints.Count; i++) + { + var metadata = endpoints[i].Metadata.GetMetadata(); + if (metadata?.IsDynamic == true) + { + return true; + } + } + + return false; + } } } diff --git a/src/Http/Routing/test/UnitTests/MatcherPolicyTest.cs b/src/Http/Routing/test/UnitTests/MatcherPolicyTest.cs new file mode 100644 index 0000000000..3c62aa1ecc --- /dev/null +++ b/src/Http/Routing/test/UnitTests/MatcherPolicyTest.cs @@ -0,0 +1,92 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing.Patterns; +using Xunit; + +namespace Microsoft.AspNetCore.Routing +{ + public class MatcherPolicyTest + { + [Fact] + public void ContainsDynamicEndpoint_FindsDynamicEndpoint() + { + // Arrange + var endpoints = new Endpoint[] + { + CreateEndpoint("1"), + CreateEndpoint("2"), + CreateEndpoint("3", new DynamicEndpointMetadata(isDynamic: true)), + }; + + // Act + var result = TestMatcherPolicy.ContainsDynamicEndpoints(endpoints); + + // Assert + Assert.True(result); + } + + [Fact] + public void ContainsDynamicEndpoint_DoesNotFindDynamicEndpoint() + { + // Arrange + var endpoints = new Endpoint[] + { + CreateEndpoint("1"), + CreateEndpoint("2"), + CreateEndpoint("3", new DynamicEndpointMetadata(isDynamic: false)), + }; + + // Act + var result = TestMatcherPolicy.ContainsDynamicEndpoints(endpoints); + + // Assert + Assert.False(result); + } + + [Fact] + public void ContainsDynamicEndpoint_DoesNotFindDynamicEndpoint_Empty() + { + // Arrange + var endpoints = new Endpoint[]{ }; + + // Act + var result = TestMatcherPolicy.ContainsDynamicEndpoints(endpoints); + + // Assert + Assert.False(result); + } + + private RouteEndpoint CreateEndpoint(string template, params object[] metadata) + { + return new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse(template), + 0, + new EndpointMetadataCollection(metadata), + "test"); + } + + private class DynamicEndpointMetadata : IDynamicEndpointMetadata + { + public DynamicEndpointMetadata(bool isDynamic) + { + IsDynamic = isDynamic; + } + + public bool IsDynamic { get; } + } + + private class TestMatcherPolicy : MatcherPolicy + { + public override int Order => throw new System.NotImplementedException(); + + public new static bool ContainsDynamicEndpoints(IReadOnlyList endpoints) + { + return MatcherPolicy.ContainsDynamicEndpoints(endpoints); + } + } + } +} diff --git a/src/Http/Routing/test/UnitTests/Matching/CandidateSetTest.cs b/src/Http/Routing/test/UnitTests/Matching/CandidateSetTest.cs index 470103e283..1254e24031 100644 --- a/src/Http/Routing/test/UnitTests/Matching/CandidateSetTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/CandidateSetTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -55,6 +55,91 @@ namespace Microsoft.AspNetCore.Routing.Matching } } + // We special case low numbers of candidates, so we want to verify that it works correctly for a variety + // of input sizes. + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + [InlineData(5)] // this is the break-point where we start to use a list. + [InlineData(6)] + [InlineData(31)] + [InlineData(32)] // this is the break point where we use a BitArray + [InlineData(33)] + public void ReplaceEndpoint_WithEndpoint(int count) + { + // Arrange + var endpoints = new RouteEndpoint[count]; + for (var i = 0; i < endpoints.Length; i++) + { + endpoints[i] = CreateEndpoint($"/{i}"); + } + + var builder = CreateDfaMatcherBuilder(); + var candidates = builder.CreateCandidates(endpoints); + + var candidateSet = new CandidateSet(candidates); + + for (var i = 0; i < candidateSet.Count; i++) + { + ref var state = ref candidateSet[i]; + + var endpoint = CreateEndpoint($"/test{i}"); + var values = new RouteValueDictionary(); + + // Act + candidateSet.ReplaceEndpoint(i, endpoint, values); + + // Assert + Assert.Same(endpoint, state.Endpoint); + Assert.Same(values, state.Values); + Assert.True(candidateSet.IsValidCandidate(i)); + } + } + + // We special case low numbers of candidates, so we want to verify that it works correctly for a variety + // of input sizes. + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + [InlineData(5)] // this is the break-point where we start to use a list. + [InlineData(6)] + [InlineData(31)] + [InlineData(32)] // this is the break point where we use a BitArray + [InlineData(33)] + public void ReplaceEndpoint_WithEndpoint_Null(int count) + { + // Arrange + var endpoints = new RouteEndpoint[count]; + for (var i = 0; i < endpoints.Length; i++) + { + endpoints[i] = CreateEndpoint($"/{i}"); + } + + var builder = CreateDfaMatcherBuilder(); + var candidates = builder.CreateCandidates(endpoints); + + var candidateSet = new CandidateSet(candidates); + + for (var i = 0; i < candidateSet.Count; i++) + { + ref var state = ref candidateSet[i]; + + // Act + candidateSet.ReplaceEndpoint(i, null, null); + + // Assert + Assert.Null(state.Endpoint); + Assert.Null(state.Values); + Assert.False(candidateSet.IsValidCandidate(i)); + } + } + // We special case low numbers of candidates, so we want to verify that it works correctly for a variety // of input sizes. [Theory] diff --git a/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ActionEndpointDatasourceBenchmark.cs b/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ActionEndpointDatasourceBenchmark.cs index 2d5b951f01..ec59bae164 100644 --- a/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ActionEndpointDatasourceBenchmark.cs +++ b/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ActionEndpointDatasourceBenchmark.cs @@ -114,10 +114,7 @@ namespace Microsoft.AspNetCore.Mvc.Performance { var dataSource = new ActionEndpointDataSource( actionDescriptorCollectionProvider, - new ActionEndpointFactory( - new MockRoutePatternTransformer(), - new MvcEndpointInvokerFactory( - new ActionInvokerFactory(Array.Empty())))); + new ActionEndpointFactory(new MockRoutePatternTransformer())); return dataSource; } diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index ca26f6740e..dfd4fc09d8 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -272,7 +272,6 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); - services.TryAddSingleton(); // // Middleware pipeline filter related diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionContextAccessor.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionContextAccessor.cs index c12acdc36c..242721fb5b 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionContextAccessor.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ActionContextAccessor.cs @@ -7,6 +7,8 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { public class ActionContextAccessor : IActionContextAccessor { + internal static readonly IActionContextAccessor Null = new NullActionContextAccessor(); + private static readonly AsyncLocal _storage = new AsyncLocal(); public ActionContext ActionContext @@ -14,5 +16,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure get { return _storage.Value; } set { _storage.Value = value; } } + + private class NullActionContextAccessor : IActionContextAccessor + { + public ActionContext ActionContext + { + get => null; + set { } + } + } } } \ No newline at end of file diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvoker.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvoker.cs index 099797fbe8..580280e11d 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvoker.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvoker.cs @@ -27,11 +27,12 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure internal ControllerActionInvoker( ILogger logger, DiagnosticListener diagnosticListener, + IActionContextAccessor actionContextAccessor, IActionResultTypeMapper mapper, ControllerContext controllerContext, ControllerActionInvokerCacheEntry cacheEntry, IFilterMetadata[] filters) - : base(diagnosticListener, logger, mapper, controllerContext, filters, controllerContext.ValueProviderFactories) + : base(diagnosticListener, logger, actionContextAccessor, mapper, controllerContext, filters, controllerContext.ValueProviderFactories) { if (cacheEntry == null) { diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvokerProvider.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvokerProvider.cs index b01501960f..4ce970061c 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvokerProvider.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ControllerActionInvokerProvider.cs @@ -21,6 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure private readonly ILogger _logger; private readonly DiagnosticListener _diagnosticListener; private readonly IActionResultTypeMapper _mapper; + private readonly IActionContextAccessor _actionContextAccessor; public ControllerActionInvokerProvider( ControllerActionInvokerCache controllerActionInvokerCache, @@ -28,6 +29,17 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure ILoggerFactory loggerFactory, DiagnosticListener diagnosticListener, IActionResultTypeMapper mapper) + : this(controllerActionInvokerCache, optionsAccessor, loggerFactory, diagnosticListener, mapper, null) + { + } + + public ControllerActionInvokerProvider( + ControllerActionInvokerCache controllerActionInvokerCache, + IOptions optionsAccessor, + ILoggerFactory loggerFactory, + DiagnosticListener diagnosticListener, + IActionResultTypeMapper mapper, + IActionContextAccessor actionContextAccessor) { _controllerActionInvokerCache = controllerActionInvokerCache; _valueProviderFactories = optionsAccessor.Value.ValueProviderFactories.ToArray(); @@ -35,6 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure _logger = loggerFactory.CreateLogger(); _diagnosticListener = diagnosticListener; _mapper = mapper; + _actionContextAccessor = actionContextAccessor ?? ActionContextAccessor.Null; } public int Order => -1000; @@ -61,6 +74,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var invoker = new ControllerActionInvoker( _logger, _diagnosticListener, + _actionContextAccessor, _mapper, controllerContext, cacheEntry, diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ResourceInvoker.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ResourceInvoker.cs index ff969d90b0..d82d9ba391 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ResourceInvoker.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ResourceInvoker.cs @@ -18,6 +18,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { protected readonly DiagnosticListener _diagnosticListener; protected readonly ILogger _logger; + protected readonly IActionContextAccessor _actionContextAccessor; protected readonly IActionResultTypeMapper _mapper; protected readonly ActionContext _actionContext; protected readonly IFilterMetadata[] _filters; @@ -39,6 +40,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure public ResourceInvoker( DiagnosticListener diagnosticListener, ILogger logger, + IActionContextAccessor actionContextAccessor, IActionResultTypeMapper mapper, ActionContext actionContext, IFilterMetadata[] filters, @@ -46,6 +48,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { _diagnosticListener = diagnosticListener ?? throw new ArgumentNullException(nameof(diagnosticListener)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _actionContextAccessor = actionContextAccessor ?? throw new ArgumentNullException(nameof(actionContextAccessor)); _mapper = mapper ?? throw new ArgumentNullException(nameof(mapper)); _actionContext = actionContext ?? throw new ArgumentNullException(nameof(actionContext)); @@ -58,6 +61,8 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { try { + _actionContextAccessor.ActionContext = _actionContext; + _diagnosticListener.BeforeAction( _actionContext.ActionDescriptor, _actionContext.HttpContext, diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionEndpointFactory.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionEndpointFactory.cs index d1392c80b3..48a38655bf 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionEndpointFactory.cs @@ -9,32 +9,25 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNetCore.Mvc.Routing { internal class ActionEndpointFactory { private readonly RoutePatternTransformer _routePatternTransformer; - private readonly MvcEndpointInvokerFactory _invokerFactory; - public ActionEndpointFactory( - RoutePatternTransformer routePatternTransformer, - MvcEndpointInvokerFactory invokerFactory) + public ActionEndpointFactory(RoutePatternTransformer routePatternTransformer) { if (routePatternTransformer == null) { throw new ArgumentNullException(nameof(routePatternTransformer)); } - if (invokerFactory == null) - { - throw new ArgumentNullException(nameof(invokerFactory)); - } - _routePatternTransformer = routePatternTransformer; - _invokerFactory = invokerFactory; } public void AddEndpoints( @@ -188,13 +181,27 @@ namespace Microsoft.AspNetCore.Mvc.Routing bool suppressPathMatching, IReadOnlyList> conventions) { + + // We don't want to close over the retrieve the Invoker Factory in ActionEndpointFactory as + // that creates cycles in DI. Since we're creating this delegate at startup time + // we don't want to create all of the things we use at runtime until the action + // actually matches. + // + // The request delegate is already a closure here because we close over + // the action descriptor. + IActionInvokerFactory invokerFactory = null; + RequestDelegate requestDelegate = (context) => { var routeData = context.GetRouteData(); - var actionContext = new ActionContext(context, routeData, action); - var invoker = _invokerFactory.CreateInvoker(actionContext); + if (invokerFactory == null) + { + invokerFactory = context.RequestServices.GetRequiredService(); + } + + var invoker = invokerFactory.CreateInvoker(actionContext); return invoker.InvokeAsync(); }; diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcAttributeRouteHandler.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcAttributeRouteHandler.cs index f32032f760..3237bb9647 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcAttributeRouteHandler.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcAttributeRouteHandler.cs @@ -14,32 +14,17 @@ namespace Microsoft.AspNetCore.Mvc.Routing { internal class MvcAttributeRouteHandler : IRouter { - private readonly IActionContextAccessor _actionContextAccessor; private readonly IActionInvokerFactory _actionInvokerFactory; private readonly IActionSelector _actionSelector; private readonly ILogger _logger; - private DiagnosticListener _diagnosticListener; + private readonly DiagnosticListener _diagnosticListener; public MvcAttributeRouteHandler( IActionInvokerFactory actionInvokerFactory, IActionSelector actionSelector, DiagnosticListener diagnosticListener, ILoggerFactory loggerFactory) - : this(actionInvokerFactory, actionSelector, diagnosticListener, loggerFactory, actionContextAccessor: null) { - } - - public MvcAttributeRouteHandler( - IActionInvokerFactory actionInvokerFactory, - IActionSelector actionSelector, - DiagnosticListener diagnosticListener, - ILoggerFactory loggerFactory, - IActionContextAccessor actionContextAccessor) - { - // The IActionContextAccessor is optional. We want to avoid the overhead of using CallContext - // if possible. - _actionContextAccessor = actionContextAccessor; - _actionInvokerFactory = actionInvokerFactory; _actionSelector = actionSelector; _diagnosticListener = diagnosticListener; @@ -94,11 +79,6 @@ namespace Microsoft.AspNetCore.Mvc.Routing var routeData = c.GetRouteData(); var actionContext = new ActionContext(context.HttpContext, routeData, actionDescriptor); - if (_actionContextAccessor != null) - { - _actionContextAccessor.ActionContext = actionContext; - } - var invoker = _actionInvokerFactory.CreateInvoker(actionContext); if (invoker == null) { diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcEndpointInvokerFactory.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcEndpointInvokerFactory.cs deleted file mode 100644 index f0d608b168..0000000000 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcEndpointInvokerFactory.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Mvc.Abstractions; -using Microsoft.AspNetCore.Mvc.Infrastructure; - -namespace Microsoft.AspNetCore.Mvc.Routing -{ - internal sealed class MvcEndpointInvokerFactory : IActionInvokerFactory - { - private readonly IActionInvokerFactory _invokerFactory; - private readonly IActionContextAccessor _actionContextAccessor; - - public MvcEndpointInvokerFactory( - IActionInvokerFactory invokerFactory) - : this(invokerFactory, actionContextAccessor: null) - { - } - - public MvcEndpointInvokerFactory( - IActionInvokerFactory invokerFactory, - IActionContextAccessor actionContextAccessor) - { - _invokerFactory = invokerFactory; - - // The IActionContextAccessor is optional. We want to avoid the overhead of using CallContext - // if possible. - _actionContextAccessor = actionContextAccessor; - } - - public IActionInvoker CreateInvoker(ActionContext actionContext) - { - if (_actionContextAccessor != null) - { - _actionContextAccessor.ActionContext = actionContext; - } - - return _invokerFactory.CreateInvoker(actionContext); - } - } -} diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcRouteHandler.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcRouteHandler.cs index 3d7e1add1e..62564f15e5 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcRouteHandler.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/MvcRouteHandler.cs @@ -13,7 +13,6 @@ namespace Microsoft.AspNetCore.Mvc.Routing { internal class MvcRouteHandler : IRouter { - private readonly IActionContextAccessor _actionContextAccessor; private readonly IActionInvokerFactory _actionInvokerFactory; private readonly IActionSelector _actionSelector; private readonly ILogger _logger; @@ -24,21 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing IActionSelector actionSelector, DiagnosticListener diagnosticListener, ILoggerFactory loggerFactory) - : this(actionInvokerFactory, actionSelector, diagnosticListener, loggerFactory, actionContextAccessor: null) { - } - - public MvcRouteHandler( - IActionInvokerFactory actionInvokerFactory, - IActionSelector actionSelector, - DiagnosticListener diagnosticListener, - ILoggerFactory loggerFactory, - IActionContextAccessor actionContextAccessor) - { - // The IActionContextAccessor is optional. We want to avoid the overhead of using CallContext - // if possible. - _actionContextAccessor = actionContextAccessor; - _actionInvokerFactory = actionInvokerFactory; _actionSelector = actionSelector; _diagnosticListener = diagnosticListener; @@ -82,11 +67,6 @@ namespace Microsoft.AspNetCore.Mvc.Routing var routeData = c.GetRouteData(); var actionContext = new ActionContext(context.HttpContext, routeData, actionDescriptor); - if (_actionContextAccessor != null) - { - _actionContextAccessor.ActionContext = actionContext; - } - var invoker = _actionInvokerFactory.CreateInvoker(actionContext); if (invoker == null) { diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/CompiledPageActionDescriptor.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/CompiledPageActionDescriptor.cs index cd81ea8888..5152f2736c 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/CompiledPageActionDescriptor.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/CompiledPageActionDescriptor.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Reflection; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; namespace Microsoft.AspNetCore.Mvc.RazorPages @@ -59,5 +60,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages /// Gets or sets the of the page. /// public TypeInfo PageTypeInfo { get; set; } + + /// + /// Gets or sets the associated of this page. + /// + public Endpoint Endpoint { get; set; } } } diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs index 29f279836e..aca9562661 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.AspNetCore.Mvc.RazorPages; using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; +using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; using Resources = Microsoft.AspNetCore.Mvc.RazorPages.Resources; @@ -82,6 +83,9 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddEnumerable( ServiceDescriptor.Transient, RazorPagesRazorViewEngineOptionsSetup>()); + // Routing + services.TryAddEnumerable(ServiceDescriptor.Singleton()); + // Action description and invocation services.TryAddEnumerable( ServiceDescriptor.Singleton()); diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageLoader.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageLoader.cs index 0534d276ba..425bd1ab15 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageLoader.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageLoader.cs @@ -5,10 +5,12 @@ using System; using System.Collections.Generic; using System.Linq; using System.Reflection; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Razor.Compilation; -using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; +using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure @@ -17,12 +19,14 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure { private readonly IPageApplicationModelProvider[] _applicationModelProviders; private readonly IViewCompilerProvider _viewCompilerProvider; + private readonly ActionEndpointFactory _endpointFactory; private readonly PageConventionCollection _conventions; private readonly FilterCollection _globalFilters; public DefaultPageLoader( IEnumerable applicationModelProviders, IViewCompilerProvider viewCompilerProvider, + ActionEndpointFactory endpointFactory, IOptions pageOptions, IOptions mvcOptions) { @@ -30,6 +34,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure .OrderBy(p => p.Order) .ToArray(); _viewCompilerProvider = viewCompilerProvider; + _endpointFactory = endpointFactory; _conventions = pageOptions.Value.Conventions; _globalFilters = mvcOptions.Value.Filters; } @@ -59,7 +64,20 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure ApplyConventions(_conventions, context.PageApplicationModel); - return CompiledPageActionDescriptorBuilder.Build(context.PageApplicationModel, _globalFilters); + var compiled = CompiledPageActionDescriptorBuilder.Build(context.PageApplicationModel, _globalFilters); + + // We need to create an endpoint for routing to use and attach it to the CompiledPageActionDescriptor... + // routing for pages is two-phase. First we perform routing using the route info - we can do this without + // compiling/loading the page. Then once we have a match we load the page and we can create an endpoint + // with all of the information we get from the compiled action descriptor. + var endpoints = new List(); + _endpointFactory.AddEndpoints(endpoints, compiled, Array.Empty(), Array.Empty>()); + + // In some test scenarios there's no route so the endpoint isn't created. This is fine because + // it won't happen for real. + compiled.Endpoint = endpoints.SingleOrDefault(); + + return compiled; } internal static void ApplyConventions( diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs index 61ab2a7a07..f1dc8bd237 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Routing; +using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure @@ -106,6 +107,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure descriptor.RouteValues.Add("page", model.ViewEnginePath); } + // Mark all pages as a "dynamic endpoint" - this is how we deal with the compilation of pages + // in endpoint routing. + descriptor.EndpointMetadata.Add(new DynamicEndpointMetadata()); + actions.Add(descriptor); } } @@ -138,5 +143,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure // Combine transformed page route with template return AttributeRouteModel.CombineTemplates(transformedPageRoute, pageRouteMetadata.RouteTemplate); } + + private class DynamicEndpointMetadata : IDynamicEndpointMetadata + { + public bool IsDynamic => true; + } } } \ No newline at end of file diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvoker.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvoker.cs index 8bcdad905b..7ac20bfe05 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvoker.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvoker.cs @@ -41,6 +41,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure IPageHandlerMethodSelector handlerMethodSelector, DiagnosticListener diagnosticListener, ILogger logger, + IActionContextAccessor actionContextAccessor, IActionResultTypeMapper mapper, PageContext pageContext, IFilterMetadata[] filterMetadata, @@ -51,6 +52,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure : base( diagnosticListener, logger, + actionContextAccessor, mapper, pageContext, filterMetadata, diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvokerProvider.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvokerProvider.cs index ffb9f93305..3747076656 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvokerProvider.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionInvokerProvider.cs @@ -38,6 +38,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure private readonly DiagnosticListener _diagnosticListener; private readonly ILogger _logger; private readonly IActionResultTypeMapper _mapper; + private readonly IActionContextAccessor _actionContextAccessor; + private volatile InnerCache _currentCache; public PageActionInvokerProvider( @@ -57,6 +59,45 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure DiagnosticListener diagnosticListener, ILoggerFactory loggerFactory, IActionResultTypeMapper mapper) + : this( + loader, + pageFactoryProvider, + modelFactoryProvider, + razorPageFactoryProvider, + collectionProvider, + filterProviders, + parameterBinder, + modelMetadataProvider, + modelBinderFactory, + tempDataFactory, + mvcOptions, + htmlHelperOptions, + selector, + diagnosticListener, + loggerFactory, + mapper, + actionContextAccessor: null) + { + } + + public PageActionInvokerProvider( + IPageLoader loader, + IPageFactoryProvider pageFactoryProvider, + IPageModelFactoryProvider modelFactoryProvider, + IRazorPageFactoryProvider razorPageFactoryProvider, + IActionDescriptorCollectionProvider collectionProvider, + IEnumerable filterProviders, + ParameterBinder parameterBinder, + IModelMetadataProvider modelMetadataProvider, + IModelBinderFactory modelBinderFactory, + ITempDataDictionaryFactory tempDataFactory, + IOptions mvcOptions, + IOptions htmlHelperOptions, + IPageHandlerMethodSelector selector, + DiagnosticListener diagnosticListener, + ILoggerFactory loggerFactory, + IActionResultTypeMapper mapper, + IActionContextAccessor actionContextAccessor) { _loader = loader; _pageFactoryProvider = pageFactoryProvider; @@ -75,6 +116,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure _diagnosticListener = diagnosticListener; _logger = loggerFactory.CreateLogger(); _mapper = mapper; + _actionContextAccessor = actionContextAccessor ?? ActionContextAccessor.Null; } public int Order { get; } = -1000; @@ -154,6 +196,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure _selector, _diagnosticListener, _logger, + _actionContextAccessor, _mapper, pageContext, filters, diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageLoaderMatcherPolicy.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageLoaderMatcherPolicy.cs new file mode 100644 index 0000000000..d41a445b48 --- /dev/null +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageLoaderMatcherPolicy.cs @@ -0,0 +1,89 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Matching; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure +{ + internal class PageLoaderMatcherPolicy : MatcherPolicy, IEndpointSelectorPolicy + { + private readonly IPageLoader _loader; + + public PageLoaderMatcherPolicy(IPageLoader loader) + { + if (loader == null) + { + throw new ArgumentNullException(nameof(loader)); + } + + _loader = loader; + } + + public override int Order => int.MinValue + 100; + + public bool AppliesToEndpoints(IReadOnlyList endpoints) + { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + if (!ContainsDynamicEndpoints(endpoints)) + { + // Pages are always dynamic endpoints. + return false; + } + + for (var i = 0; i < endpoints.Count; i++) + { + var page = endpoints[i].Metadata.GetMetadata(); + if (page != null) + { + // Found a page + return true; + } + } + + return false; + } + + public Task ApplyAsync(HttpContext httpContext, EndpointSelectorContext context, CandidateSet candidates) + { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (candidates == null) + { + throw new ArgumentNullException(nameof(candidates)); + } + + for (var i = 0; i < candidates.Count; i++) + { + ref var candidate = ref candidates[i]; + var endpoint = (RouteEndpoint)candidate.Endpoint; + + var page = endpoint.Metadata.GetMetadata(); + if (page != null) + { + var compiled = _loader.Load(page); + candidates.ReplaceEndpoint(i, compiled.Endpoint, candidate.Values); + } + } + + return Task.CompletedTask; + } + } +} diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Filters/MiddlewareFilterTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Filters/MiddlewareFilterTest.cs index 8d72ce9a36..414d4f0135 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Filters/MiddlewareFilterTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Filters/MiddlewareFilterTest.cs @@ -396,6 +396,7 @@ namespace Microsoft.AspNetCore.Mvc.Filters : base( logger, diagnosticListener, + ActionContextAccessor.Null, mapper, CreateControllerContext(actionContext, valueProviderFactories, maxAllowedErrorsInModelState), CreateCacheEntry((ControllerActionDescriptor)actionContext.ActionDescriptor, controllerFactory), diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ControllerActionInvokerTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ControllerActionInvokerTest.cs index 03e70b7f9e..e74cdcfdee 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ControllerActionInvokerTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ControllerActionInvokerTest.cs @@ -1343,6 +1343,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var invoker = new ControllerActionInvoker( new NullLoggerFactory().CreateLogger(), new DiagnosticListener("Microsoft.AspNetCore"), + ActionContextAccessor.Null, new ActionResultTypeMapper(), controllerContext, cacheEntry, @@ -1623,6 +1624,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var invoker = new ControllerActionInvoker( logger, diagnosticSource, + ActionContextAccessor.Null, new ActionResultTypeMapper(), controllerContext, cacheEntry, diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointDataSourceBaseTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointDataSourceBaseTest.cs index e4aacf1cc1..1f743fe5d0 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointDataSourceBaseTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointDataSourceBaseTest.cs @@ -148,9 +148,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing var serviceProvider = services.BuildServiceProvider(); - var endpointFactory = new ActionEndpointFactory( - serviceProvider.GetRequiredService(), - new MvcEndpointInvokerFactory(new ActionInvokerFactory(Array.Empty()))); + var endpointFactory = new ActionEndpointFactory(serviceProvider.GetRequiredService()); return CreateDataSource(actions, endpointFactory); } diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointFactoryTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointFactoryTest.cs index c4bb99e799..61c3453168 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointFactoryTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionEndpointFactoryTest.cs @@ -32,112 +32,13 @@ namespace Microsoft.AspNetCore.Mvc.Routing }); Services = serviceCollection.BuildServiceProvider(); - InvokerFactory = new Mock(MockBehavior.Strict); - Factory = new ActionEndpointFactory( - Services.GetRequiredService(), - new MvcEndpointInvokerFactory(InvokerFactory.Object)); + Factory = new ActionEndpointFactory(Services.GetRequiredService()); } internal ActionEndpointFactory Factory { get; } - internal Mock InvokerFactory { get; } - internal IServiceProvider Services { get; } - [Fact] - public async Task AddEndpoints_AttributeRouted_UsesActionInvoker() - { - // Arrange - var values = new - { - action = "Test", - controller = "Test", - page = (string)null, - }; - - var action = CreateActionDescriptor(values, pattern: "/Test"); - - var endpointFeature = new EndpointSelectorContext - { - RouteValues = new RouteValueDictionary() - }; - - var featureCollection = new FeatureCollection(); - featureCollection.Set(endpointFeature); - featureCollection.Set(endpointFeature); - featureCollection.Set(endpointFeature); - - var httpContextMock = new Mock(); - httpContextMock.Setup(m => m.Features).Returns(featureCollection); - - var actionInvokerCalled = false; - var actionInvokerMock = new Mock(); - actionInvokerMock.Setup(m => m.InvokeAsync()).Returns(() => - { - actionInvokerCalled = true; - return Task.CompletedTask; - }); - - InvokerFactory - .Setup(m => m.CreateInvoker(It.IsAny())) - .Returns(actionInvokerMock.Object); - - // Act - var endpoint = CreateAttributeRoutedEndpoint(action); - - // Assert - await endpoint.RequestDelegate(httpContextMock.Object); - - Assert.True(actionInvokerCalled); - } - - [Fact] - public async Task AddEndpoints_ConventionalRouted_UsesActionInvoker() - { - // Arrange - var values = new - { - action = "Test", - controller = "Test", - page = (string)null, - }; - - var action = CreateActionDescriptor(values); - - var endpointFeature = new EndpointSelectorContext - { - RouteValues = new RouteValueDictionary() - }; - - var featureCollection = new FeatureCollection(); - featureCollection.Set(endpointFeature); - featureCollection.Set(endpointFeature); - featureCollection.Set(endpointFeature); - - var httpContextMock = new Mock(); - httpContextMock.Setup(m => m.Features).Returns(featureCollection); - - var actionInvokerCalled = false; - var actionInvokerMock = new Mock(); - actionInvokerMock.Setup(m => m.InvokeAsync()).Returns(() => - { - actionInvokerCalled = true; - return Task.CompletedTask; - }); - - InvokerFactory - .Setup(m => m.CreateInvoker(It.IsAny())) - .Returns(actionInvokerMock.Object); - - // Act - var endpoint = CreateConventionalRoutedEndpoint(action, "{controller}/{action}"); - - // Assert - await endpoint.RequestDelegate(httpContextMock.Object); - - Assert.True(actionInvokerCalled); - } - [Fact] public void AddEndpoints_ConventionalRouted_WithEmptyRouteName_CreatesMetadataWithEmptyRouteName() { diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/MvcRouteHandlerTests.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/MvcRouteHandlerTests.cs index b13958aa0a..87b25333a5 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/MvcRouteHandlerTests.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/MvcRouteHandlerTests.cs @@ -56,8 +56,6 @@ namespace Microsoft.AspNetCore.Mvc.Routing ILoggerFactory loggerFactory = null, object diagnosticListener = null) { - var actionContextAccessor = new ActionContextAccessor(); - if (actionDescriptor == null) { var mockAction = new Mock(); @@ -105,8 +103,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing invokerFactory, actionSelector, diagnosticSource, - loggerFactory, - actionContextAccessor); + loggerFactory); } private RouteContext CreateRouteContext() diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/DefaultPageLoaderTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/DefaultPageLoaderTest.cs index ecbb41a8a7..b9c0b8462e 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/DefaultPageLoaderTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/DefaultPageLoaderTest.cs @@ -4,8 +4,11 @@ using System; using System.Reflection; using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Razor.Compilation; +using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Razor.Hosting; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Moq; @@ -25,6 +28,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure var razorPagesOptions = Options.Create(new RazorPagesOptions()); var mvcOptions = Options.Create(new MvcOptions()); + var endpointFactory = new ActionEndpointFactory(Mock.Of()); var provider1 = new Mock(); var provider2 = new Mock(); @@ -75,6 +79,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure var loader = new DefaultPageLoader( providers, compilerProvider, + endpointFactory, razorPagesOptions, mvcOptions); @@ -86,6 +91,60 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure provider2.Verify(); } + [Fact] + public void Load_CreatesEndpoint_WithRoute() + { + // Arrange + var descriptor = new PageActionDescriptor() + { + AttributeRouteInfo = new AttributeRouteInfo() + { + Template = "/test", + }, + }; + + var transformer = new Mock(); + transformer + .Setup(t => t.SubstituteRequiredValues(It.IsAny(), It.IsAny())) + .Returns((p, v) => p); + + var compilerProvider = GetCompilerProvider(); + + var razorPagesOptions = Options.Create(new RazorPagesOptions()); + var mvcOptions = Options.Create(new MvcOptions()); + var endpointFactory = new ActionEndpointFactory(transformer.Object); + + var provider = new Mock(); + + var pageApplicationModel = new PageApplicationModel(descriptor, typeof(object).GetTypeInfo(), Array.Empty()); + + provider.Setup(p => p.OnProvidersExecuting(It.IsAny())) + .Callback((PageApplicationModelProviderContext c) => + { + Assert.Null(c.PageApplicationModel); + c.PageApplicationModel = pageApplicationModel; + }) + .Verifiable(); + + var providers = new[] + { + provider.Object, + }; + + var loader = new DefaultPageLoader( + providers, + compilerProvider, + endpointFactory, + razorPagesOptions, + mvcOptions); + + // Act + var result = loader.Load(descriptor); + + // Assert + Assert.NotNull(result.Endpoint); + } + [Fact] public void Load_InvokesApplicationModelProviders_WithTheRightOrder() { @@ -94,6 +153,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure var compilerProvider = GetCompilerProvider(); var razorPagesOptions = Options.Create(new RazorPagesOptions()); var mvcOptions = Options.Create(new MvcOptions()); + var endpointFactory = new ActionEndpointFactory(Mock.Of()); var provider1 = new Mock(); provider1.SetupGet(p => p.Order).Returns(10); @@ -138,6 +198,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure var loader = new DefaultPageLoader( providers, compilerProvider, + endpointFactory, razorPagesOptions, mvcOptions); diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionDescriptorProviderTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionDescriptorProviderTest.cs index 2573072e01..a42d2ec032 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionDescriptorProviderTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionDescriptorProviderTest.cs @@ -232,8 +232,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure var result = Assert.Single(context.Results); var descriptor = Assert.IsType(result); Assert.Equal(model.RelativePath, descriptor.RelativePath); - var actual = Assert.Single(descriptor.EndpointMetadata); - Assert.Same(expected, actual); + Assert.Single(descriptor.EndpointMetadata, expected); } [Fact] diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionInvokerTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionInvokerTest.cs index f270e195d4..3a948272a9 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionInvokerTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Infrastructure/PageActionInvokerTest.cs @@ -1519,6 +1519,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure selector.Object, diagnosticListener ?? new DiagnosticListener("Microsoft.AspNetCore"), logger ?? NullLogger.Instance, + ActionContextAccessor.Null, new ActionResultTypeMapper(), pageContext, filters ?? Array.Empty(), diff --git a/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs b/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs index 6284eea15d..318054fb36 100644 --- a/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs +++ b/src/Security/Authorization/Policy/src/AuthorizationMiddleware.cs @@ -46,13 +46,6 @@ namespace Microsoft.AspNetCore.Authorization var endpoint = context.GetEndpoint(); - // Workaround for https://github.com/aspnet/AspNetCore/issues/7011. Do not use the AuthorizationMiddleware for Razor Pages - if (endpoint != null && endpoint.Metadata.Any(m => m.GetType().FullName == "Microsoft.AspNetCore.Mvc.ApplicationModels.PageRouteMetadata")) - { - await _next(context); - return; - } - // Flag to indicate to other systems, e.g. MVC, that authorization middleware was run for this request context.Items[AuthorizationMiddlewareInvokedKey] = AuthorizationMiddlewareInvokedValue;