From 183ecd85d6b4841f43c882a0122e6fa90dda841e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 11 Jul 2018 11:47:33 +1200 Subject: [PATCH] Fix MVC integration with UseEndpoint (#8047) --- .../MvcApplicationBuilderExtensions.cs | 1 - .../Internal/MvcEndpointDataSource.cs | 25 ++++- .../Internal/MvcEndpointDataSourceTests.cs | 76 ++++++------- .../RouteDataTest.cs | 103 ------------------ .../RoutingTestsBase.cs | 80 ++++++++++++++ .../Controllers/RouteDataController.cs} | 6 +- 6 files changed, 144 insertions(+), 147 deletions(-) delete mode 100644 test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs rename test/WebSites/{BasicWebSite/Controllers/RoutingController.cs => RoutingWebSite/Controllers/RouteDataController.cs} (90%) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs index 8e0ed3bd2f..c35d40d0f1 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs @@ -122,7 +122,6 @@ namespace Microsoft.AspNetCore.Builder configureRoutes(routeBuilder); mvcEndpointDataSource.ConventionalEndpointInfos.AddRange(routeBuilder.EndpointInfos); - mvcEndpointDataSource.InitializeEndpoints(); return app.UseEndpoint(); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs index dab81b662b..786ce62b61 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs @@ -24,8 +24,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal private readonly IServiceProvider _serviceProvider; private readonly IActionDescriptorChangeProvider[] _actionDescriptorChangeProviders; private readonly List _endpoints; + private readonly object _lock = new object(); private IChangeToken _changeToken; + private bool _initialized; public MvcEndpointDataSource( IActionDescriptorCollectionProvider actions, @@ -62,7 +64,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal ConventionalEndpointInfos = new List(); } - public void InitializeEndpoints() + private void InitializeEndpoints() { foreach (var action in _actions.ActionDescriptors.Items) { @@ -387,8 +389,27 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - public override IReadOnlyList Endpoints => _endpoints; + public override IReadOnlyList Endpoints + { + get + { + if (!_initialized) + { + lock (_lock) + { + if (!_initialized) + { + InitializeEndpoints(); + _initialized = true; + } + } + } + return _endpoints; + } + } + + // REVIEW: Infos added after endpoints are initialized will not be used public List ConventionalEndpointInfos { get; } private class RouteNameMetadata : IRouteNameMetadata diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs index f3b6d778df..635169ee32 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs @@ -59,10 +59,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal var dataSource = CreateMvcEndpointDataSource(mockDescriptorProvider.Object); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - var endpoint = Assert.Single(dataSource.Endpoints); + var endpoint = Assert.Single(endpoints); var matcherEndpoint = Assert.IsType(endpoint); var endpointValue = matcherEndpoint.RequiredValues["Name"]; @@ -115,10 +115,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal new MvcEndpointInvokerFactory(actionInvokerProviderMock.Object)); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - var endpoint = Assert.Single(dataSource.Endpoints); + var endpoint = Assert.Single(endpoints); var matcherEndpoint = Assert.IsType(endpoint); var invokerDelegate = matcherEndpoint.Invoker((next) => Task.CompletedTask); @@ -192,7 +192,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, endpointInfoRoute)); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert var inspectors = finalEndpointTemplates @@ -200,7 +200,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal .ToArray(); // Assert - Assert.Collection(dataSource.Endpoints, inspectors); + Assert.Collection(endpoints, inspectors); } [Theory] @@ -219,7 +219,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, endpointInfoRoute)); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert var inspectors = finalEndpointTemplates @@ -227,7 +227,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal .ToArray(); // Assert - Assert.Collection(dataSource.Endpoints, inspectors); + Assert.Collection(endpoints, inspectors); } [Fact] @@ -243,10 +243,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal new RouteValueDictionary(new { action = "TestAction" }))); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - Assert.Collection(dataSource.Endpoints, + Assert.Collection(endpoints, (e) => Assert.Equal("TestController", Assert.IsType(e).Template), (e) => Assert.Equal("TestController/TestAction", Assert.IsType(e).Template)); } @@ -266,10 +266,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal constraints: new RouteValueDictionary(new { action = "(TestAction1|TestAction2)" }))); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - Assert.Collection(dataSource.Endpoints, + Assert.Collection(endpoints, (e) => Assert.Equal("TestController/TestAction1", Assert.IsType(e).Template), (e) => Assert.Equal("TestController/TestAction2", Assert.IsType(e).Template)); } @@ -291,14 +291,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal endpointInfoRoute)); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; var inspectors = finalEndpointTemplates .Select(t => new Action(e => Assert.Equal(t, Assert.IsType(e).Template))) .ToArray(); // Assert - Assert.Collection(dataSource.Endpoints, inspectors); + Assert.Collection(endpoints, inspectors); } [Fact] @@ -312,10 +312,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal CreateEndpointInfo(string.Empty, "named/{controller}/{action}/{id?}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - var endpoint = Assert.Single(dataSource.Endpoints); + var endpoint = Assert.Single(endpoints); var matcherEndpoint = Assert.IsType(endpoint); var routeNameMetadata = matcherEndpoint.Metadata.GetMetadata(); Assert.Null(routeNameMetadata); @@ -333,11 +333,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal CreateEndpointInfo("namedRoute", "named/{controller}/{action}/{id?}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert Assert.Collection( - dataSource.Endpoints, + endpoints, (ep) => { var matcherEndpoint = Assert.IsType(ep); @@ -372,11 +372,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal "named/{controller}/{action}/{id?}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert Assert.Collection( - dataSource.Endpoints, + endpoints, (ep) => { var matcherEndpoint = Assert.IsType(ep); @@ -413,10 +413,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, "{controller}/{action}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - Assert.Empty(dataSource.Endpoints); + Assert.Empty(endpoints); } // Since area, controller, action and page are special, check to see if the followin test succeeds for a @@ -431,10 +431,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, "{controller}/{action}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - Assert.Empty(dataSource.Endpoints); + Assert.Empty(endpoints); } [Fact] @@ -447,10 +447,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, "{area}/{controller}/{action}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - Assert.Empty(dataSource.Endpoints); + Assert.Empty(endpoints); } [Fact] @@ -463,10 +463,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, "{area=admin}/{controller}/{action}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - Assert.Empty(dataSource.Endpoints); + Assert.Empty(endpoints); } [Fact] @@ -479,10 +479,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, "{area}/{controller}/{action}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - Assert.Empty(dataSource.Endpoints); + Assert.Empty(endpoints); } [Fact] @@ -495,10 +495,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, "{controller}/{action}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - var endpoint = Assert.Single(dataSource.Endpoints); + var endpoint = Assert.Single(endpoints); var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar", matcherEndpoint.Template); AssertIsSubset(expectedDefaults, matcherEndpoint.Defaults); @@ -517,10 +517,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal CreateEndpointInfo(string.Empty, "{controller=Home}/{action=Index}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - var endpoint = Assert.Single(dataSource.Endpoints); + var endpoint = Assert.Single(endpoints); var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar", matcherEndpoint.Template); AssertIsSubset(expectedDefaults, matcherEndpoint.Defaults); @@ -540,10 +540,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal CreateEndpointInfo(string.Empty, "{controller=Home}/{action=Index}/{subscription=general}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - var endpoint = Assert.Single(dataSource.Endpoints); + var endpoint = Assert.Single(endpoints); var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar/{subscription=general}", matcherEndpoint.Template); AssertIsSubset(expectedDefaults, matcherEndpoint.Defaults); @@ -562,10 +562,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal CreateEndpointInfo(string.Empty, "{controller=Home}/{action=Index}")); // Act - dataSource.InitializeEndpoints(); + var endpoints = dataSource.Endpoints; // Assert - var endpoint = Assert.Single(dataSource.Endpoints); + var endpoint = Assert.Single(endpoints); var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar", matcherEndpoint.Template); AssertIsSubset(expectedDefaults, matcherEndpoint.Defaults); diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs deleted file mode 100644 index 3ea689b1a4..0000000000 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs +++ /dev/null @@ -1,103 +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 System.Collections.Generic; -using System.Net; -using System.Net.Http; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Mvc.Internal; -using Microsoft.AspNetCore.Routing; -using Newtonsoft.Json; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.FunctionalTests -{ - public class RouteDataTest : IClassFixture> - { - public RouteDataTest(MvcTestFixture fixture) - { - Client = fixture.CreateDefaultClient(); - } - - public HttpClient Client { get; } - - [Fact] - public async Task RouteData_Routers_ConventionalRoute() - { - // Arrange & Act - var response = await Client.GetAsync("http://localhost/Routing/Conventional"); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - - var body = await response.Content.ReadAsStringAsync(); - var result = JsonConvert.DeserializeObject(body); - - Assert.Equal( - new string[] - { - typeof(RouteCollection).FullName, - typeof(Route).FullName, - typeof(MvcRouteHandler).FullName, - }, - result.Routers); - } - - [Fact] - public async Task RouteData_Routers_AttributeRoute() - { - // Arrange & Act - var response = await Client.GetAsync("http://localhost/Routing/Attribute"); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - - var body = await response.Content.ReadAsStringAsync(); - var result = JsonConvert.DeserializeObject(body); - - Assert.Equal(new string[] - { - typeof(RouteCollection).FullName, - typeof(AttributeRoute).FullName, - typeof(MvcAttributeRouteHandler).FullName, - }, - result.Routers); - } - - // Verifies that components in the MVC pipeline can modify datatokens - // without impacting any static data. - // - // This does two request, to verify that the data in the route is not modified - [Fact] - public async Task RouteData_DataTokens_FilterCanSetDataTokens() - { - // Arrange - var response = await Client.GetAsync("http://localhost/Routing/DataTokens"); - - // Guard - var body = await response.Content.ReadAsStringAsync(); - var result = JsonConvert.DeserializeObject(body); - Assert.Single(result.DataTokens); - Assert.Single(result.DataTokens, kvp => kvp.Key == "actionName" && ((string)kvp.Value) == "DataTokens"); - - // Act - response = await Client.GetAsync("http://localhost/Routing/Conventional"); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - - body = await response.Content.ReadAsStringAsync(); - result = JsonConvert.DeserializeObject(body); - - Assert.Single(result.DataTokens); - Assert.Single(result.DataTokens, kvp => kvp.Key == "actionName" && ((string)kvp.Value) == "Conventional"); - } - - private class ResultData - { - public Dictionary DataTokens { get; set; } - - public string[] Routers { get; set; } - } - } -} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTestsBase.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTestsBase.cs index a5ea33a0f1..99c2710762 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTestsBase.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTestsBase.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Net; using System.Net.Http; using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Routing; using Newtonsoft.Json; using Xunit; @@ -22,6 +23,85 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public HttpClient Client { get; } + [Fact] + public async Task RouteData_Routers_ConventionalRoute() + { + // Arrange & Act + var response = await Client.GetAsync("http://localhost/RouteData/Conventional"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal( + new string[] + { + typeof(RouteCollection).FullName, + typeof(Route).FullName, + typeof(MvcRouteHandler).FullName, + }, + result.Routers); + } + + [Fact] + public async Task RouteData_Routers_AttributeRoute() + { + // Arrange & Act + var response = await Client.GetAsync("http://localhost/RouteData/Attribute"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal(new string[] + { + typeof(RouteCollection).FullName, + typeof(AttributeRoute).FullName, + typeof(MvcAttributeRouteHandler).FullName, + }, + result.Routers); + } + + // Verifies that components in the MVC pipeline can modify datatokens + // without impacting any static data. + // + // This does two request, to verify that the data in the route is not modified + [Fact] + public async Task RouteData_DataTokens_FilterCanSetDataTokens() + { + // Arrange + var response = await Client.GetAsync("http://localhost/RouteData/DataTokens"); + + // Guard + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + Assert.Single(result.DataTokens); + Assert.Single(result.DataTokens, kvp => kvp.Key == "actionName" && ((string)kvp.Value) == "DataTokens"); + + // Act + response = await Client.GetAsync("http://localhost/RouteData/Conventional"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + body = await response.Content.ReadAsStringAsync(); + result = JsonConvert.DeserializeObject(body); + + Assert.Single(result.DataTokens); + Assert.Single(result.DataTokens, kvp => kvp.Key == "actionName" && ((string)kvp.Value) == "Conventional"); + } + + private class ResultData + { + public Dictionary DataTokens { get; set; } + + public string[] Routers { get; set; } + } + [Fact] public virtual async Task ConventionalRoutedController_ActionIsReachable() { diff --git a/test/WebSites/BasicWebSite/Controllers/RoutingController.cs b/test/WebSites/RoutingWebSite/Controllers/RouteDataController.cs similarity index 90% rename from test/WebSites/BasicWebSite/Controllers/RoutingController.cs rename to test/WebSites/RoutingWebSite/Controllers/RouteDataController.cs index c5576b4685..e02f385707 100644 --- a/test/WebSites/BasicWebSite/Controllers/RoutingController.cs +++ b/test/WebSites/RoutingWebSite/Controllers/RouteDataController.cs @@ -6,16 +6,16 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; -namespace BasicWebSite +namespace RoutingWebSite.Controllers { - public class RoutingController : Controller + public class RouteDataController : Controller { public object Conventional() { return GetData(); } - [Route("Routing/Attribute")] + [Route("RouteData/Attribute")] public object Attribute() { return GetData();