diff --git a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs index 2b8e20e7d4..e432d4736d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs +++ b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs @@ -8,7 +8,6 @@ using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.Logging; -using Microsoft.AspNet.RequestContainer; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Logging; @@ -33,62 +32,60 @@ namespace Microsoft.AspNet.Mvc public async Task RouteAsync([NotNull] RouteContext context) { - // This will create a scope if needed. This was likely already been done by routing, but - // we can enforce it just in case we're used in some other scenario. - using (RequestServicesContainer.EnsureRequestServices(context.HttpContext, services: null)) + // TODO: Throw an error here that's descriptive enough so that + // users understand they should call the per request scoped middleware + // or set HttpContext.Services manually + var services = context.HttpContext.RequestServices; + Contract.Assert(services != null); + + // Verify if AddMvc was done before calling UseMvc + // We use the MvcMarkerService to make sure if all the services were added. + MvcServicesHelper.ThrowIfMvcNotRegistered(services); + + EnsureLogger(context.HttpContext); + using (_logger.BeginScope("MvcRouteHandler.RouteAsync")) { - var services = context.HttpContext.RequestServices; - Contract.Assert(services != null); + var actionSelector = services.GetRequiredService(); + var actionDescriptor = await actionSelector.SelectAsync(context); - // Verify if AddMvc was done before calling UseMvc - // We use the MvcMarkerService to make sure if all the services were added. - MvcServicesHelper.ThrowIfMvcNotRegistered(services); - - EnsureLogger(context.HttpContext); - using (_logger.BeginScope("MvcRouteHandler.RouteAsync")) + if (actionDescriptor == null) { - var actionSelector = services.GetRequiredService(); - var actionDescriptor = await actionSelector.SelectAsync(context); - - if (actionDescriptor == null) - { - LogActionSelection(actionSelected: false, actionInvoked: false, handled: context.IsHandled); - return; - } - - // Replacing the route data allows any code running here to dirty the route values or data-tokens - // without affecting something upstream. - var oldRouteData = context.RouteData; - var newRouteData = new RouteData(oldRouteData); - - if (actionDescriptor.RouteValueDefaults != null) - { - foreach (var kvp in actionDescriptor.RouteValueDefaults) - { - if (!newRouteData.Values.ContainsKey(kvp.Key)) - { - newRouteData.Values.Add(kvp.Key, kvp.Value); - } - } - } - - try - { - context.RouteData = newRouteData; - - await InvokeActionAsync(context, actionDescriptor); - context.IsHandled = true; - } - finally - { - if (!context.IsHandled) - { - context.RouteData = oldRouteData; - } - } - - LogActionSelection(actionSelected: true, actionInvoked: true, handled: context.IsHandled); + LogActionSelection(actionSelected: false, actionInvoked: false, handled: context.IsHandled); + return; } + + // Replacing the route data allows any code running here to dirty the route values or data-tokens + // without affecting something upstream. + var oldRouteData = context.RouteData; + var newRouteData = new RouteData(oldRouteData); + + if (actionDescriptor.RouteValueDefaults != null) + { + foreach (var kvp in actionDescriptor.RouteValueDefaults) + { + if (!newRouteData.Values.ContainsKey(kvp.Key)) + { + newRouteData.Values.Add(kvp.Key, kvp.Value); + } + } + } + + try + { + context.RouteData = newRouteData; + + await InvokeActionAsync(context, actionDescriptor); + context.IsHandled = true; + } + finally + { + if (!context.IsHandled) + { + context.RouteData = oldRouteData; + } + } + + LogActionSelection(actionSelected: true, actionInvoked: true, handled: context.IsHandled); } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs index 2dfe574d19..f3be3f42ea 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs @@ -2,11 +2,11 @@ // 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.AspNet.Http; using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.Logging; -using Microsoft.AspNet.PipelineCore; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Logging; @@ -248,101 +248,6 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(initialRouter, Assert.Single(actionRouteData.Routers)); } - [Fact] - public async Task RouteAsync_CreatesNewServiceScope() - { - // Arrange - var httpContext = new DefaultHttpContext(); - - var services = new Mock(MockBehavior.Strict); - httpContext.ApplicationServices = services.Object; - - var requestServices = new Mock(MockBehavior.Strict); - requestServices - .Setup(s => s.GetService(It.IsAny())) - .Returns(t => services.Object.GetService(t)); - - var scope = new Mock(MockBehavior.Strict); - scope - .SetupGet(s => s.ServiceProvider) - .Returns(requestServices.Object); - scope - .Setup(s => s.Dispose()) - .Verifiable(); - - var scopeFactory = new Mock(MockBehavior.Strict); - scopeFactory - .Setup(f => f.CreateScope()) - .Returns(scope.Object); - - // Needed by RequestContainer to create a new scope - services - .Setup(s => s.GetService(typeof(IServiceProvider))) - .Returns(services.Object); - services - .Setup(s => s.GetService(typeof(IContextAccessor))) - .Returns(new ContextAccessor()); - services - .Setup(s => s.GetService(typeof(IServiceScopeFactory))) - .Returns(scopeFactory.Object); - - // These services will be resolved from the request container - var action = new Mock(); - - var actionSelector = new Mock(); - actionSelector - .Setup(a => a.SelectAsync(It.IsAny())) - .Returns(Task.FromResult(action.Object)); - requestServices - .Setup(s => s.GetService(typeof(IActionSelector))) - .Returns(actionSelector.Object); - - var invoker = new Mock(); - invoker - .Setup(i => i.InvokeAsync()) - .Returns(Task.FromResult(true)) - .Verifiable(); - - var invokerFactory = new Mock(); - invokerFactory - .Setup(f => f.CreateInvoker(It.IsAny())) - .Returns(invoker.Object); - requestServices - .Setup(s => s.GetService(typeof(IActionInvokerFactory))) - .Returns(invokerFactory.Object); - - requestServices - .Setup(s => s.GetService(typeof(IContextAccessor))) - .Returns(new ContextAccessor()); - - requestServices - .Setup(s => s.GetService(typeof(MvcMarkerService))) - .Returns(new MvcMarkerService()); - - requestServices - .Setup(s => s.GetService(typeof(ILoggerFactory))) - .Returns(NullLoggerFactory.Instance); - - requestServices - .Setup(s => s.GetService(typeof(IOptions))) - .Returns(new MockMvcOptionsAccessor()); - - var context = new RouteContext(httpContext); - var handler = new MvcRouteHandler(); - - // Act - await handler.RouteAsync(context); - - // Assert - // - // If we created the scope it should be disposed - scope.Verify(s => s.Dispose()); - - // We can't directly verify that the httpContext.RequestServices property was set, - // but we can verify that our invoker (only reachable through request services) was used. - invoker.Verify(); - } - private RouteContext CreateRouteContext( IActionSelector actionSelector = null, IActionInvokerFactory invokerFactory = null,