From f604fb8d87c52e29cb412c7992b70fa4fe1ca20f Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 6 Mar 2014 12:18:31 -0800 Subject: [PATCH] CR feedback again --- .../RoutingSample/HttpContextRouteEndpoint.cs | 5 ++-- samples/RoutingSample/PrefixRoute.cs | 4 +-- .../BindPathContext.cs | 2 -- src/Microsoft.AspNet.Routing/IRouter.cs | 2 +- .../RouteCollection.cs | 11 ++++---- .../Template/TemplateRoute.cs | 25 ++++++++++--------- .../Template/TemplateRouteTests.cs | 16 ++++++------ 7 files changed, 33 insertions(+), 32 deletions(-) diff --git a/samples/RoutingSample/HttpContextRouteEndpoint.cs b/samples/RoutingSample/HttpContextRouteEndpoint.cs index da72d0b11f..e256b22ed6 100644 --- a/samples/RoutingSample/HttpContextRouteEndpoint.cs +++ b/samples/RoutingSample/HttpContextRouteEndpoint.cs @@ -21,10 +21,11 @@ namespace RoutingSample context.IsHandled = true; } - public void BindPath(BindPathContext context) + public string BindPath(BindPathContext context) { - // We can generate a url for anything that the parent route deems OK. + // We don't really care what the values look like. context.IsBound = true; + return null; } } } diff --git a/samples/RoutingSample/PrefixRoute.cs b/samples/RoutingSample/PrefixRoute.cs index 833add299a..fc51a3dc09 100644 --- a/samples/RoutingSample/PrefixRoute.cs +++ b/samples/RoutingSample/PrefixRoute.cs @@ -50,9 +50,9 @@ namespace RoutingSample } } - public void BindPath(BindPathContext context) + public string BindPath(BindPathContext context) { - // Do nothing + return null; } } } diff --git a/src/Microsoft.AspNet.Routing/BindPathContext.cs b/src/Microsoft.AspNet.Routing/BindPathContext.cs index a69086b9f9..f15b18443a 100644 --- a/src/Microsoft.AspNet.Routing/BindPathContext.cs +++ b/src/Microsoft.AspNet.Routing/BindPathContext.cs @@ -20,8 +20,6 @@ namespace Microsoft.AspNet.Routing public bool IsBound { get; set; } - public string BoundPath { get; set; } - public IDictionary Values { get; private set; } } } diff --git a/src/Microsoft.AspNet.Routing/IRouter.cs b/src/Microsoft.AspNet.Routing/IRouter.cs index d83dc7a1ec..a77645d382 100644 --- a/src/Microsoft.AspNet.Routing/IRouter.cs +++ b/src/Microsoft.AspNet.Routing/IRouter.cs @@ -8,6 +8,6 @@ namespace Microsoft.AspNet.Routing { Task RouteAsync(RouteContext context); - void BindPath(BindPathContext context); + string BindPath(BindPathContext context); } } diff --git a/src/Microsoft.AspNet.Routing/RouteCollection.cs b/src/Microsoft.AspNet.Routing/RouteCollection.cs index c85876f95e..2ba7718846 100644 --- a/src/Microsoft.AspNet.Routing/RouteCollection.cs +++ b/src/Microsoft.AspNet.Routing/RouteCollection.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Threading.Tasks; -using Microsoft.AspNet.Abstractions; namespace Microsoft.AspNet.Routing { @@ -41,18 +40,20 @@ namespace Microsoft.AspNet.Routing } } - public virtual void BindPath(BindPathContext context) + public virtual string BindPath(BindPathContext context) { for (var i = 0; i < Count; i++) { var route = this[i]; - route.BindPath(context); - if (context.IsBound) + var path = route.BindPath(context); + if (path != null) { - return; + return path; } } + + return null; } } } diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs index 8d1f0a1839..7df2a0e11a 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs @@ -75,28 +75,29 @@ namespace Microsoft.AspNet.Routing.Template } } - public void BindPath(BindPathContext context) + public string BindPath(BindPathContext context) { - // Validate that the target can accept these values. - _target.BindPath(context); - if (!context.IsBound) + // Validate that the target can accept these values - if the target generates a value + // then that can short circuit. + var path = _target.BindPath(context); + if (path != null) { - return; + return path; + } + else if (!context.IsBound) + { + return null; } // This could be optimized more heavily - right now we try to do the full url // generation after validating, but we could do it in two phases. - var path = _binder.Bind(_defaults, context.AmbientValues, context.Values); + path = _binder.Bind(_defaults, context.AmbientValues, context.Values); if (path == null) { context.IsBound = false; - return; - } - else - { - Debug.Assert(context.IsBound); - context.BoundPath = path; } + + return path; } } } diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs index 4a6d40a39d..f61368f3e9 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs @@ -113,11 +113,11 @@ namespace Microsoft.AspNet.Routing.Template.Tests var context = CreateRouteBindContext(new {controller = "Home"}); // Act - route.BindPath(context); + var path = route.BindPath(context); // Assert Assert.True(context.IsBound); - Assert.Equal("Home", context.BoundPath); + Assert.Equal("Home", path); } [Fact] @@ -128,11 +128,11 @@ namespace Microsoft.AspNet.Routing.Template.Tests var context = CreateRouteBindContext(new { controller = "Home" }); // Act - route.BindPath(context); + var path = route.BindPath(context); // Assert Assert.False(context.IsBound); - Assert.Null(context.BoundPath); + Assert.Null(path); } [Fact] @@ -143,11 +143,11 @@ namespace Microsoft.AspNet.Routing.Template.Tests var context = CreateRouteBindContext(new { controller = "Home" }); // Act - route.BindPath(context); + var path = route.BindPath(context); // Assert Assert.False(context.IsBound); - Assert.Null(context.BoundPath); + Assert.Null(path); } [Fact] @@ -158,11 +158,11 @@ namespace Microsoft.AspNet.Routing.Template.Tests var context = CreateRouteBindContext(new { action = "Index"}, new { controller = "Home" }); // Act - route.BindPath(context); + var path = route.BindPath(context); // Assert Assert.True(context.IsBound); - Assert.Equal("Home/Index", context.BoundPath); + Assert.Equal("Home/Index", path); } private static BindPathContext CreateRouteBindContext(object values)