React to routing breaking changes

src/Microsoft.AspNet.Mvc.Core/Infrastructure/MvcRouteHandler.cs

@@ -59,73 +59,61 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
                 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))
+                    if (!context.RouteData.Values.ContainsKey(kvp.Key))
                     {
-                        newRouteData.Values.Add(kvp.Key, kvp.Value);
+                        context.RouteData.Values.Add(kvp.Key, kvp.Value);
                     }
                 }
             }
 
             // Removing RouteGroup from RouteValues to simulate the result of conventional routing
-            newRouteData.Values.Remove(TreeRouter.RouteGroupKey);
+            context.RouteData.Values.Remove(TreeRouter.RouteGroupKey);
 
+            context.Handler = (c) => InvokeActionAsync(c, actionDescriptor);
+        }
+
+        private async Task InvokeActionAsync(HttpContext httpContext, ActionDescriptor actionDescriptor)
+        {
+            var routeData = httpContext.GetRouteData();
             try
             {
-                context.RouteData = newRouteData;
+                _diagnosticSource.BeforeAction(actionDescriptor, httpContext, routeData);
-
-                _diagnosticSource.BeforeAction(actionDescriptor, context.HttpContext, context.RouteData);
 
                 using (_logger.ActionScope(actionDescriptor))
                 {
                     _logger.ExecutingAction(actionDescriptor);
 
                     var startTime = Environment.TickCount;
-                    await InvokeActionAsync(context, actionDescriptor);
+
-                    context.IsHandled = true;
+                    var actionContext = new ActionContext(httpContext, routeData, actionDescriptor);
+                    if (_actionContextAccessor != null)
+                    {
+                        _actionContextAccessor.ActionContext = actionContext;
+                    }
+
+                    var invoker = _actionInvokerFactory.CreateInvoker(actionContext);
+                    if (invoker == null)
+                    {
+                        throw new InvalidOperationException(
+                            Resources.FormatActionInvokerFactory_CouldNotCreateInvoker(
+                                actionDescriptor.DisplayName));
+                    }
+
+                    await invoker.InvokeAsync();
 
                     _logger.ExecutedAction(actionDescriptor, startTime);
                 }
             }
             finally
             {
-                _diagnosticSource.AfterAction(actionDescriptor, context.HttpContext, context.RouteData);
+                _diagnosticSource.AfterAction(actionDescriptor, httpContext, routeData);
-
-                if (!context.IsHandled)
-                {
-                    context.RouteData = oldRouteData;
-                }
             }
         }
 
-        private Task InvokeActionAsync(RouteContext context, ActionDescriptor actionDescriptor)
-        {
-            var actionContext = new ActionContext(context.HttpContext, context.RouteData, actionDescriptor);
-
-            if (_actionContextAccessor != null)
-            {
-                _actionContextAccessor.ActionContext = actionContext;
-            }
-
-            var invoker = _actionInvokerFactory.CreateInvoker(actionContext);
-            if (invoker == null)
-            {
-                throw new InvalidOperationException(
-                    Resources.FormatActionInvokerFactory_CouldNotCreateInvoker(
-                        actionDescriptor.DisplayName));
-            }
-
-            return invoker.InvokeAsync();
-        }
-
         private void EnsureServices(HttpContext context)
         {
             if (_servicesRetrieved)

src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs

@@ -282,9 +282,14 @@ namespace Microsoft.AspNet.Mvc.Routing
 
             routeInfo.Constraints = constraintBuilder.Build();
 
-            routeInfo.Defaults = routeInfo.ParsedTemplate.Parameters
+            routeInfo.Defaults = new RouteValueDictionary();
-                .Where(p => p.DefaultValue != null)
+            foreach (var parameter in routeInfo.ParsedTemplate.Parameters)
-                .ToDictionary(p => p.Name, p => p.DefaultValue, StringComparer.OrdinalIgnoreCase);
+            {
+                if (parameter.DefaultValue != null)
+                {
+                    routeInfo.Defaults.Add(parameter.Name, parameter.DefaultValue);
+                }
+            }
 
             return routeInfo;
         }
@@ -293,9 +298,9 @@ namespace Microsoft.AspNet.Mvc.Routing
         {
             public ActionDescriptor ActionDescriptor { get; set; }
 
-            public IReadOnlyDictionary<string, IRouteConstraint> Constraints { get; set; }
+            public IDictionary<string, IRouteConstraint> Constraints { get; set; }
 
-            public IReadOnlyDictionary<string, object> Defaults { get; set; }
+            public RouteValueDictionary Defaults { get; set; }
 
             public string ErrorMessage { get; set; }
 

test/Microsoft.AspNet.Mvc.Core.Test/Builder/MvcAreaRouteBuilderExtensionsTest.cs

@@ -2,11 +2,9 @@
 // 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.Linq;
 using Microsoft.AspNet.Routing;
 using Microsoft.AspNet.Routing.Constraints;
-using Microsoft.AspNet.Routing.Template;
 using Microsoft.Extensions.DependencyInjection;
 using Moq;
 using Xunit;
@@ -29,7 +27,7 @@ namespace Microsoft.AspNet.Builder
             builder.MapAreaRoute(name: null, areaName: "admin", template: "site/Admin/");
 
             // Assert
-            var route = Assert.IsType<TemplateRoute>((Assert.Single(builder.Routes)));
+            var route = Assert.IsType<Route>((Assert.Single(builder.Routes)));
 
             Assert.Null(route.Name);
             Assert.Equal("site/Admin/", route.RouteTemplate);
@@ -68,7 +66,7 @@ namespace Microsoft.AspNet.Builder
                 defaults: new { action = "Home" });
 
             // Assert
-            var route = Assert.IsType<TemplateRoute>((Assert.Single(builder.Routes)));
+            var route = Assert.IsType<Route>((Assert.Single(builder.Routes)));
 
             Assert.Equal("admin_area", route.Name);
             Assert.Equal("site/Admin/", route.RouteTemplate);
@@ -113,7 +111,7 @@ namespace Microsoft.AspNet.Builder
                 constraints: new { id = new IntRouteConstraint() });
 
             // Assert
-            var route = Assert.IsType<TemplateRoute>((Assert.Single(builder.Routes)));
+            var route = Assert.IsType<Route>((Assert.Single(builder.Routes)));
 
             Assert.Equal("admin_area", route.Name);
             Assert.Equal("site/Admin/", route.RouteTemplate);
@@ -164,7 +162,7 @@ namespace Microsoft.AspNet.Builder
                 dataTokens: new { some_token = "hello" });
 
             // Assert
-            var route = Assert.IsType<TemplateRoute>((Assert.Single(builder.Routes)));
+            var route = Assert.IsType<Route>((Assert.Single(builder.Routes)));
 
             Assert.Equal("admin_area", route.Name);
             Assert.Equal("site/Admin/", route.RouteTemplate);
@@ -221,7 +219,7 @@ namespace Microsoft.AspNet.Builder
                 dataTokens: new { some_token = "hello" });
 
             // Assert
-            var route = Assert.IsType<TemplateRoute>((Assert.Single(builder.Routes)));
+            var route = Assert.IsType<Route>((Assert.Single(builder.Routes)));
 
             Assert.Equal("admin_area", route.Name);
             Assert.Equal("site/Admin/", route.RouteTemplate);

test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs

@@ -20,7 +20,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
     public class MvcRouteHandlerTests
     {
         [Fact]
-        public async Task RouteAsync_Success_LogsCorrectValues()
+        public async Task RouteHandler_Success_LogsCorrectValues()
         {
             // Arrange
             var sink = new TestSink();
@@ -35,9 +35,10 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             var context = CreateRouteContext(actionDescriptor: actionDescriptor.Object, loggerFactory: loggerFactory);
 
             var handler = new MvcRouteHandler();
+            await handler.RouteAsync(context);
 
             // Act
-            await handler.RouteAsync(context);
+            await context.Handler(context.HttpContext);
 
             // Assert
             Assert.Single(sink.Scopes);
@@ -77,10 +78,9 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
         }
 
         [Fact]
-        public async Task RouteAsync_CreatesNewRouteData()
+        public async Task RouteHandler_RemovesRouteGroupFromRouteValues()
         {
             // Arrange
-            RouteData actionRouteData = null;
             var invoker = new Mock<IActionInvoker>();
             invoker
                 .Setup(i => i.InvokeAsync())
@@ -91,49 +91,6 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
                 .Setup(f => f.CreateInvoker(It.IsAny<ActionContext>()))
                 .Returns<ActionContext>((c) =>
                 {
-                    actionRouteData = c.RouteData;
-                    return invoker.Object;
-                });
-
-            var initialRouter = Mock.Of<IRouter>();
-
-            var context = CreateRouteContext(invokerFactory: invokerFactory.Object);
-            var handler = new MvcRouteHandler();
-
-            var originalRouteData = context.RouteData;
-            originalRouteData.Routers.Add(initialRouter);
-            originalRouteData.Values.Add("action", "Index");
-
-            // Act
-            await handler.RouteAsync(context);
-
-            // Assert
-            Assert.NotSame(originalRouteData, context.RouteData);
-            Assert.NotSame(originalRouteData, actionRouteData);
-            Assert.Same(actionRouteData, context.RouteData);
-
-            // The new routedata is a copy
-            Assert.Equal("Index", context.RouteData.Values["action"]);
-
-            Assert.Equal(initialRouter, Assert.Single(context.RouteData.Routers));
-        }
-
-        [Fact]
-        public async Task RouteAsync_RemovesRouteGroupFromRouteValues()
-        {
-            // Arrange
-            RouteData actionRouteData = null;
-            var invoker = new Mock<IActionInvoker>();
-            invoker
-                .Setup(i => i.InvokeAsync())
-                .Returns(Task.FromResult(true));
-
-            var invokerFactory = new Mock<IActionInvokerFactory>();
-            invokerFactory
-                .Setup(f => f.CreateInvoker(It.IsAny<ActionContext>()))
-                .Returns<ActionContext>((c) =>
-                {
-                    actionRouteData = c.RouteData;
                     return invoker.Object;
                 });
 
@@ -147,59 +104,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             await handler.RouteAsync(context);
 
             // Assert
-            Assert.NotSame(originalRouteData, context.RouteData);
+            Assert.Same(originalRouteData, context.RouteData);
-            Assert.NotSame(originalRouteData, actionRouteData);
+
-            Assert.Same(actionRouteData, context.RouteData);
 
-            // The new routedata is a copy
             Assert.False(context.RouteData.Values.ContainsKey(TreeRouter.RouteGroupKey));
         }
 
         [Fact]
-        public async Task RouteAsync_ResetsRouteDataOnException()
+        public async Task RouteHandler_WritesDiagnostic_ActionSelected()
-        {
-            // Arrange
-            RouteData actionRouteData = null;
-            var invoker = new Mock<IActionInvoker>();
-            invoker
-                .Setup(i => i.InvokeAsync())
-                .Throws(new Exception());
-
-            var invokerFactory = new Mock<IActionInvokerFactory>();
-            invokerFactory
-                .Setup(f => f.CreateInvoker(It.IsAny<ActionContext>()))
-                .Returns<ActionContext>((c) =>
-                {
-                    actionRouteData = c.RouteData;
-                    c.RouteData.Values.Add("action", "Index");
-                    return invoker.Object;
-                });
-
-            var context = CreateRouteContext(invokerFactory: invokerFactory.Object);
-            var handler = new MvcRouteHandler();
-
-            var initialRouter = Mock.Of<IRouter>();
-
-            var originalRouteData = context.RouteData;
-            originalRouteData.Routers.Add(initialRouter);
-
-            // Act
-            await Assert.ThrowsAsync<Exception>(() => handler.RouteAsync(context));
-
-            // Assert
-            Assert.Same(originalRouteData, context.RouteData);
-            Assert.NotSame(originalRouteData, actionRouteData);
-            Assert.NotSame(actionRouteData, context.RouteData);
-
-            // The new routedata is a copy
-            Assert.Null(context.RouteData.Values["action"]);
-            Assert.Equal("Index", actionRouteData.Values["action"]);
-
-            Assert.Equal(initialRouter, Assert.Single(actionRouteData.Routers));
-        }
-
-        [Fact]
-        public async Task RouteAsync_WritesDiagnostic_ActionSelected()
         {
             // Arrange
             var listener = new TestDiagnosticListener();
@@ -208,9 +120,10 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             context.RouteData.Values.Add("tag", "value");
 
             var handler = new MvcRouteHandler();
+            await handler.RouteAsync(context);
 
             // Act
-            await handler.RouteAsync(context);
+            await context.Handler(context.HttpContext);
 
             // Assert
             Assert.NotNull(listener.BeforeAction?.ActionDescriptor);
@@ -224,7 +137,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
         }
 
         [Fact]
-        public async Task RouteAsync_WritesDiagnostic_ActionInvoked()
+        public async Task RouteHandler_WritesDiagnostic_ActionInvoked()
         {
             // Arrange
             var listener = new TestDiagnosticListener();
@@ -232,9 +145,10 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             var context = CreateRouteContext(diagnosticListener: listener);
 
             var handler = new MvcRouteHandler();
+            await handler.RouteAsync(context);
 
             // Act
-            await handler.RouteAsync(context);
+            await context.Handler(context.HttpContext);
 
             // Assert
             Assert.NotNull(listener.AfterAction?.ActionDescriptor);
@@ -293,22 +207,42 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
                 diagnosticSource.SubscribeWithAdapter(diagnosticListener);
             }
 
+            var routingFeature = new RoutingFeature();
+
             var httpContext = new Mock<HttpContext>();
-            httpContext.Setup(h => h.RequestServices.GetService(typeof(IActionContextAccessor)))
+            httpContext
+                .Setup(h => h.RequestServices.GetService(typeof(IActionContextAccessor)))
                 .Returns(new ActionContextAccessor());
-            httpContext.Setup(h => h.RequestServices.GetService(typeof(IActionSelector)))
+            httpContext
+                .Setup(h => h.RequestServices.GetService(typeof(IActionSelector)))
                 .Returns(actionSelector);
-            httpContext.Setup(h => h.RequestServices.GetService(typeof(IActionInvokerFactory)))
+            httpContext
+                .Setup(h => h.RequestServices.GetService(typeof(IActionInvokerFactory)))
                 .Returns(invokerFactory);
-            httpContext.Setup(h => h.RequestServices.GetService(typeof(ILoggerFactory)))
+            httpContext
+                .Setup(h => h.RequestServices.GetService(typeof(ILoggerFactory)))
                 .Returns(loggerFactory);
-            httpContext.Setup(h => h.RequestServices.GetService(typeof(MvcMarkerService)))
+            httpContext
-                 .Returns(new MvcMarkerService());
+                .Setup(h => h.RequestServices.GetService(typeof(MvcMarkerService)))
-            httpContext.Setup(h => h.RequestServices.GetService(typeof(IOptions<MvcOptions>)))
+                .Returns(new MvcMarkerService());
-                 .Returns(optionsAccessor);
+            httpContext
-            httpContext.Setup(h => h.RequestServices.GetService(typeof(DiagnosticSource)))
+                .Setup(h => h.RequestServices.GetService(typeof(IOptions<MvcOptions>)))
+                .Returns(optionsAccessor);
+            httpContext
+                .Setup(h => h.RequestServices.GetService(typeof(DiagnosticSource)))
                 .Returns(diagnosticSource);
-            return new RouteContext(httpContext.Object);
+            httpContext
+                .Setup(h => h.Features[typeof(IRoutingFeature)])
+                .Returns(routingFeature);
+
+            var routeContext = new RouteContext(httpContext.Object);
+            routingFeature.RouteData = routeContext.RouteData;
+            return routeContext;
+        }
+
+        private class RoutingFeature : IRoutingFeature
+        {
+            public RouteData RouteData { get; set; }
         }
     }
 }

test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTest.cs

@@ -19,6 +19,8 @@ namespace Microsoft.AspNet.Mvc.Routing
 {
     public class AttributeRouteTest
     {
+        private static readonly RequestDelegate NullHandler = (c) => Task.FromResult(0);
+
         // This test verifies that AttributeRoute can respond to changes in the AD collection. It does this
         // by running a successful request, then removing that action and verifying the next route isn't
         // successful.
@@ -29,7 +31,7 @@ namespace Microsoft.AspNet.Mvc.Routing
             var handler = new Mock<IRouter>(MockBehavior.Strict);
             handler
                 .Setup(h => h.RouteAsync(It.IsAny<RouteContext>()))
-                .Callback<RouteContext>(c => c.IsHandled = true)
+                .Callback<RouteContext>(c => c.Handler = NullHandler)
                 .Returns(Task.FromResult(true))
                 .Verifiable();
 
@@ -85,7 +87,7 @@ namespace Microsoft.AspNet.Mvc.Routing
             await route.RouteAsync(context);
 
             // Assert 1
-            Assert.True(context.IsHandled);
+            Assert.NotNull(context.Handler);
             Assert.Equal("5", context.RouteData.Values["id"]);
             Assert.Equal("2", context.RouteData.Values[TreeRouter.RouteGroupKey]);
 
@@ -103,7 +105,7 @@ namespace Microsoft.AspNet.Mvc.Routing
             await route.RouteAsync(context);
 
             // Assert 2
-            Assert.False(context.IsHandled);
+            Assert.Null(context.Handler);
             Assert.Empty(context.RouteData.Values);
 
             handler.Verify(h => h.RouteAsync(It.IsAny<RouteContext>()), Times.Once());

test/Microsoft.AspNet.Mvc.Core.Test/Routing/UrlHelperTest.cs

@@ -1020,7 +1020,7 @@ namespace Microsoft.AspNet.Mvc.Routing
 
             public Task RouteAsync(RouteContext context)
             {
-                context.IsHandled = true;
+                context.Handler = (c) => Task.FromResult(0);
                 return Task.FromResult(false);
             }
         }

test/Microsoft.AspNet.Mvc.FunctionalTests/RouteDataTest.cs

@@ -39,7 +39,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
                 new string[]
                 {
                     typeof(RouteCollection).FullName,
-                    typeof(TemplateRoute).FullName,
+                    typeof(Route).FullName,
                     typeof(MvcRouteHandler).FullName,
                 },
                 result.Routers);

test/WebSites/CustomRouteWebSite/LocalizedRoute.cs

@@ -32,32 +32,12 @@ namespace CustomRouteWebSite
 
         public async Task RouteAsync(RouteContext context)
         {
-            // Saving and restoring the original route data ensures that any values we
+            context.RouteData.Routers.Add(_next);
-            // add won't 'leak' if action selection doesn't match.
-            var oldRouteData = context.RouteData;
-
-            // For diagnostics and link-generation purposes, routing should include
-            // a list of IRoute instances that lead to the ultimate destination.
-            // It's the responsibility of each IRouter to add the 'next' before 
-            // calling it.
-            var newRouteData = new RouteData(oldRouteData);
-            newRouteData.Routers.Add(_next);
 
             var locale = GetLocale(context.HttpContext) ?? "en-US";
-            newRouteData.Values.Add("locale", locale);
+            context.RouteData.Values.Add("locale", locale);
 
-            try
+            await _next.RouteAsync(context);
-            {
-                context.RouteData = newRouteData;
-                await _next.RouteAsync(context);
-            }
-            finally
-            {
-                if (!context.IsHandled)
-                {
-                    context.RouteData = oldRouteData;
-                }
-            }
         }
 
         private string GetLocale(HttpContext context)