diff --git a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs index e432d4736d..2b8e20e7d4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs +++ b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs @@ -8,6 +8,7 @@ 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; @@ -32,60 +33,62 @@ namespace Microsoft.AspNet.Mvc public async Task RouteAsync([NotNull] RouteContext context) { - // 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")) + // 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)) { - var actionSelector = services.GetRequiredService(); - var actionDescriptor = await actionSelector.SelectAsync(context); + var services = context.HttpContext.RequestServices; + Contract.Assert(services != null); - if (actionDescriptor == 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")) { - LogActionSelection(actionSelected: false, actionInvoked: false, handled: context.IsHandled); - return; - } + var actionSelector = services.GetRequiredService(); + var actionDescriptor = await actionSelector.SelectAsync(context); - // 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 (actionDescriptor == null) { - if (!newRouteData.Values.ContainsKey(kvp.Key)) + 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) { - newRouteData.Values.Add(kvp.Key, kvp.Value); + 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) + try { - context.RouteData = oldRouteData; - } - } + context.RouteData = newRouteData; - LogActionSelection(actionSelected: true, actionInvoked: true, handled: context.IsHandled); + 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 f3be3f42ea..2dfe574d19 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,6 +248,101 @@ 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,