When using URL segment API versioning with the ApiVersionRouteConstraint, URL generation and 400 responses are now correctly returned. Fixes #18, #19, and #20 (#22)

commonsensesoftware · web-flow · commit 9f5a60521ad2 · 2016-09-14T11:11:21.000-07:00
diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteConstraint.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteConstraint.cs
@@ -3,9 +3,9 @@
     using AspNetCore.Routing;
     using Http;
     using System;
-    using System.Diagnostics.Contracts;
     using static ApiVersion;
     using static AspNetCore.Routing.RouteDirection;
+    using static System.String;
 
     /// <summary>
     /// Represents a route constraint for service <see cref="ApiVersion">API versions</see>.
@@ -24,12 +24,13 @@ public sealed class ApiVersionRouteConstraint : IRouteConstraint
         /// <returns>True if the route constraint is matched; otherwise, false.</returns>
         public bool Match( HttpContext httpContext, IRouter route, string routeKey, RouteValueDictionary values, RouteDirection routeDirection )
         {
-            if ( routeDirection != IncomingRequest )
+            var value = default( string );
+
+            if ( routeDirection == UrlGeneration )
             {
-                return false;
+                return !IsNullOrEmpty( routeKey ) && values.TryGetValue( routeKey, out value ) && !IsNullOrEmpty( value );
             }
 
-            var value = default( string );
             var requestedVersion = default( ApiVersion );
 
             if ( !values.TryGetValue( routeKey, out value ) || !TryParse( value, out requestedVersion ) )
@@ -41,4 +42,4 @@ public bool Match( HttpContext httpContext, IRouter route, string routeKey, Rout
             return true;
         }
     }
-}
+}
diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs
@@ -218,35 +218,37 @@ private BadRequestHandler IsValidRequest( ActionSelectionContext context )
                 return null;
             }
 
-            var requestedVersion = context.HttpContext.GetRawRequestedApiVersion();
+            var code = default( string );
+            var requestedVersion = default( string );
             var parsedVersion = context.RequestedVersion;
             var actionNames = new Lazy<string>( () => Join( NewLine, context.MatchingActions.Select( a => a.DisplayName ) ) );
 
-            if ( IsNullOrEmpty( requestedVersion ) )
+            if ( parsedVersion == null )
             {
-                if ( parsedVersion == null )
+                requestedVersion = context.HttpContext.GetRawRequestedApiVersion();
+
+                if ( IsNullOrEmpty( requestedVersion ) )
                 {
                     logger.ApiVersionUnspecified( actionNames.Value );
+                    return null;
+                }
+                else if ( TryParse( requestedVersion, out parsedVersion ) )
+                {
+                    code = "UnsupportedApiVersion";
+                    logger.ApiVersionUnmatched( parsedVersion, actionNames.Value );
                 }
                 else
                 {
-                    logger.ApiVersionUnspecified( parsedVersion, actionNames.Value );
+                    code = "InvalidApiVersion";
+                    logger.ApiVersionInvalid( requestedVersion );
                 }
-                return null;
             }
-
-            var code = default( string );
-
-            if ( TryParse( requestedVersion, out parsedVersion ) )
+            else
             {
+                requestedVersion = parsedVersion.ToString();
                 code = "UnsupportedApiVersion";
                 logger.ApiVersionUnmatched( parsedVersion, actionNames.Value );
             }
-            else
-            {
-                code = "InvalidApiVersion";
-                logger.ApiVersionInvalid( requestedVersion );
-            }
 
             var message = SR.VersionedResourceNotSupported.FormatDefault( context.HttpContext.Request.GetDisplayUrl(), requestedVersion );
             return new BadRequestHandler( code, message );
@@ -410,4 +412,4 @@ private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraintsCore( Ro
             }
         }
     }
-}
+}
diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Routing/ApiVersionRouteConstraintTest.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Routing/ApiVersionRouteConstraintTest.cs
@@ -1,17 +1,59 @@
 ﻿namespace Microsoft.AspNetCore.Mvc.Routing
 {
     using AspNetCore.Routing;
+    using Builder;
+    using Extensions.DependencyInjection;
+    using Extensions.ObjectPool;
     using FluentAssertions;
     using Http;
     using Moq;
+    using System;
     using System.Collections.Generic;
+    using System.Text.Encodings.Web;
+    using System.Threading.Tasks;
     using Xunit;
     using static AspNetCore.Routing.RouteDirection;
+    using static System.String;
 
     public class ApiVersionRouteConstraintTest
     {
-        [Fact]
-        public void match_should_return_false_for_url_generation()
+        private class PassThroughRouter : IRouter
+        {
+            public VirtualPathData GetVirtualPath( VirtualPathContext context ) => null;
+
+            public Task RouteAsync( RouteContext context )
+            {
+                context.Handler = c => Task.CompletedTask;
+                return Task.CompletedTask;
+            }
+        }
+
+        private static ServiceCollection CreateServices()
+        {
+            var services = new ServiceCollection();
+
+            services.AddOptions();
+            services.AddLogging();
+            services.AddRouting();
+            services.AddSingleton<ObjectPoolProvider, DefaultObjectPoolProvider>()
+                    .AddSingleton( UrlEncoder.Default );
+
+            return services;
+        }
+
+        private static IRouteBuilder CreateRouteBuilder( IServiceProvider services )
+        {
+            var app = new Mock<IApplicationBuilder>();
+            app.SetupGet( a => a.ApplicationServices ).Returns( services );
+            return new RouteBuilder( app.Object ) { DefaultHandler = new PassThroughRouter() };
+        }
+
+        [Theory]
+        [InlineData( "apiVersion", "1", true )]
+        [InlineData( "apiVersion", null, false )]
+        [InlineData( "apiVersion", "", false )]
+        [InlineData( null, "", false )]
+        public void match_should_return_expected_result_for_url_generation( string key, string value, bool expected )
         {
             // arrange
             var httpContext = new Mock<HttpContext>().Object;
@@ -20,11 +62,16 @@ public void match_should_return_false_for_url_generation()
             var routeDirection = UrlGeneration;
             var constraint = new ApiVersionRouteConstraint();
 
+            if ( !IsNullOrEmpty( key ) )
+            {
+                values[key] = value;
+            }
+
             // act
-            var matched = constraint.Match( httpContext, route, null, values, routeDirection );
+            var matched = constraint.Match( httpContext, route, key, values, routeDirection );
 
             // assert
-            matched.Should().BeFalse();
+            matched.Should().Be( expected );
         }
 
         [Fact]
@@ -87,5 +134,27 @@ public void match_should_return_true_when_matched()
             // assert
             matched.Should().BeTrue();
         }
+
+        [Fact]
+        public void url_helper_should_create_route_link_with_api_version_constriant()
+        {
+            // arrange
+            var services = CreateServices().AddApiVersioning();
+            var provider = services.BuildServiceProvider();
+            var routeBuilder = CreateRouteBuilder( provider );
+            var actionContext = new ActionContext() { HttpContext = new DefaultHttpContext() { RequestServices = provider } };
+
+            routeBuilder.MapRoute( "default", "v{version:apiVersion}/{controller}/{action}" );
+            actionContext.RouteData = new RouteData();
+            actionContext.RouteData.Routers.Add( routeBuilder.Build() );
+
+            var urlHelper = new UrlHelper( actionContext );
+
+            // act
+            var url = urlHelper.Link( "default", new { version = "1", controller = "Store", action = "Buy" } );
+
+            // assert
+            url.Should().Be( "/v1/Store/Buy" );
+        }
     }
-}
+}
diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/OrdersController.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/OrdersController.cs
@@ -1,16 +1,17 @@
 ﻿namespace Microsoft.AspNetCore.Mvc.Simulators
 {
+    using Routing;
     using System;
     using System.Threading.Tasks;
 
     [ApiVersion( "2015-11-15" )]
     [ApiVersion( "2016-06-06" )]
     public class OrdersController : Controller
     {
-        [MapToApiVersion( "2015-11-15" )]
-        public Task<IActionResult> Get_2015_11_15() => Task.FromResult<IActionResult>( Ok( "Version 2015-11-15" ) );
+        [HttpGet]
+        public Task<IActionResult> Get() => Task.FromResult<IActionResult>( Ok( "Version 2015-11-15" ) );
 
-        [Route( "orders" )]
+        [HttpGet]
         [MapToApiVersion( "2016-06-06" )]
         public Task<IActionResult> Get_2016_06_06() => Task.FromResult<IActionResult>( Ok( "Version 2016-06-06" ) );
     }
diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs
@@ -251,50 +251,32 @@ public async Task select_best_candidate_should_assume_configured_default_api_ver
 
         [Fact]
         public async Task select_best_candidate_should_use_api_version_selector_for_conventionX2Dbased_controller_when_allowed()
-        {
-            // arrange
-            var controllerType = typeof( OrdersController ).GetTypeInfo();
-            Action<ApiVersioningOptions> versioningSetup = o =>
-            {
-                o.AssumeDefaultVersionWhenUnspecified = true;
-                o.ApiVersionSelector = new ConstantApiVersionSelector( new ApiVersion( new DateTime( 2015, 11, 15 ) ) );
-            };
-            Action<IRouteBuilder> routesSetup = r => r.MapRoute( "default", "api/{controller}/{action=Get_2015_11_15}/{id?}" );
-
-            using ( var server = new WebServer( versioningSetup, routesSetup ) )
-            {
-                await server.Client.GetAsync( "api/orders" );
-
-                // act
-                var action = ( (TestApiVersionActionSelector) server.Services.GetRequiredService<IActionSelector>() ).SelectedCandidate;
-
-                // assert
-                action.As<ControllerActionDescriptor>().ControllerTypeInfo.Should().Be( controllerType );
-            }
-        }
-
-        [Fact]
-        public async Task select_best_candidate_should_use_api_version_selector_for_attributeX2Dbased_controller_when_allowed()
         {
             // arrange
             var controllerType = typeof( OrdersController ).GetTypeInfo();
             Action<ApiVersioningOptions> versioningSetup = o =>
             {
                 o.AssumeDefaultVersionWhenUnspecified = true;
                 o.ApiVersionSelector = new LowestImplementedApiVersionSelector( o );
+
             };
-            Action<IRouteBuilder> routesSetup = r => r.MapRoute( "default", "{controller}/{action=Get_2015_11_15}/{id?}" );
+            Action<IRouteBuilder> routesSetup = r => r.MapRoute( "default", "api/{controller}/{action=Get}/{id?}" );
 
             using ( var server = new WebServer( versioningSetup, routesSetup ) )
             {
-                await server.Client.GetAsync( "orders" );
+                await server.Client.GetAsync( "api/orders" );
 
                 // act
                 var action = ( (TestApiVersionActionSelector) server.Services.GetRequiredService<IActionSelector>() ).SelectedCandidate;
 
                 // assert
-                action.As<ControllerActionDescriptor>().ControllerTypeInfo.Should().Be( controllerType );
-                action.As<ControllerActionDescriptor>().ActionName.Should().Be( nameof( OrdersController.Get_2015_11_15 ) );
+                action.As<ControllerActionDescriptor>().ShouldBeEquivalentTo(
+                    new
+                    {
+                        ControllerTypeInfo = controllerType,
+                        ActionName = nameof( OrdersController.Get )
+                    },
+                    options => options.ExcludingMissingMembers() );
             }
         }
 

Original file line number	Diff line number	Diff line change
`@@ -3,9 +3,9 @@`
`3`	`3`	`using AspNetCore.Routing;`
`4`	`4`	`using Http;`
`5`	`5`	`using System;`
`6`		`- using System.Diagnostics.Contracts;`
`7`	`6`	`using static ApiVersion;`
`8`	`7`	`using static AspNetCore.Routing.RouteDirection;`
	`8`	`+ using static System.String;`
`9`	`9`
`10`	`10`	`/// <summary>`
`11`	`11`	`/// Represents a route constraint for service <see cref="ApiVersion">API versions</see>.`
`@@ -24,12 +24,13 @@ public sealed class ApiVersionRouteConstraint : IRouteConstraint`
`24`	`24`	`/// <returns>True if the route constraint is matched; otherwise, false.</returns>`
`25`	`25`	`public bool Match( HttpContext httpContext, IRouter route, string routeKey, RouteValueDictionary values, RouteDirection routeDirection )`
`26`	`26`	`{`
`27`		`- if ( routeDirection != IncomingRequest )`
	`27`	`+ var value = default( string );`
	`28`	`+`
	`29`	`+ if ( routeDirection == UrlGeneration )`
`28`	`30`	`{`
`29`		`- return false;`
	`31`	`+ return !IsNullOrEmpty( routeKey ) && values.TryGetValue( routeKey, out value ) && !IsNullOrEmpty( value );`
`30`	`32`	`}`
`31`	`33`
`32`		`- var value = default( string );`
`33`	`34`	`var requestedVersion = default( ApiVersion );`
`34`	`35`
`35`	`36`	`if ( !values.TryGetValue( routeKey, out value ) \|\| !TryParse( value, out requestedVersion ) )`
`@@ -41,4 +42,4 @@ public bool Match( HttpContext httpContext, IRouter route, string routeKey, Rout`
`41`	`42`	`return true;`
`42`	`43`	`}`
`43`	`44`	`}`
`44`		`-}`
	`45`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -218,35 +218,37 @@ private BadRequestHandler IsValidRequest( ActionSelectionContext context )`
`218`	`218`	`return null;`
`219`	`219`	`}`
`220`	`220`
`221`		`- var requestedVersion = context.HttpContext.GetRawRequestedApiVersion();`
	`221`	`+ var code = default( string );`
	`222`	`+ var requestedVersion = default( string );`
`222`	`223`	`var parsedVersion = context.RequestedVersion;`
`223`	`224`	`var actionNames = new Lazy<string>( () => Join( NewLine, context.MatchingActions.Select( a => a.DisplayName ) ) );`
`224`	`225`
`225`		`- if ( IsNullOrEmpty( requestedVersion ) )`
	`226`	`+ if ( parsedVersion == null )`
`226`	`227`	`{`
`227`		`- if ( parsedVersion == null )`
	`228`	`+ requestedVersion = context.HttpContext.GetRawRequestedApiVersion();`
	`229`	`+`
	`230`	`+ if ( IsNullOrEmpty( requestedVersion ) )`
`228`	`231`	`{`
`229`	`232`	`logger.ApiVersionUnspecified( actionNames.Value );`
	`233`	`+ return null;`
	`234`	`+ }`
	`235`	`+ else if ( TryParse( requestedVersion, out parsedVersion ) )`
	`236`	`+ {`
	`237`	`+ code = "UnsupportedApiVersion";`
	`238`	`+ logger.ApiVersionUnmatched( parsedVersion, actionNames.Value );`
`230`	`239`	`}`
`231`	`240`	`else`
`232`	`241`	`{`
`233`		`- logger.ApiVersionUnspecified( parsedVersion, actionNames.Value );`
	`242`	`+ code = "InvalidApiVersion";`
	`243`	`+ logger.ApiVersionInvalid( requestedVersion );`
`234`	`244`	`}`
`235`		`- return null;`
`236`	`245`	`}`
`237`		`-`
`238`		`- var code = default( string );`
`239`		`-`
`240`		`- if ( TryParse( requestedVersion, out parsedVersion ) )`
	`246`	`+ else`
`241`	`247`	`{`
	`248`	`+ requestedVersion = parsedVersion.ToString();`
`242`	`249`	`code = "UnsupportedApiVersion";`
`243`	`250`	`logger.ApiVersionUnmatched( parsedVersion, actionNames.Value );`
`244`	`251`	`}`
`245`		`- else`
`246`		`- {`
`247`		`- code = "InvalidApiVersion";`
`248`		`- logger.ApiVersionInvalid( requestedVersion );`
`249`		`- }`
`250`	`252`
`251`	`253`	`var message = SR.VersionedResourceNotSupported.FormatDefault( context.HttpContext.Request.GetDisplayUrl(), requestedVersion );`
`252`	`254`	`return new BadRequestHandler( code, message );`
`@@ -410,4 +412,4 @@ private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraintsCore( Ro`
`410`	`412`	`}`
`411`	`413`	`}`
`412`	`414`	`}`
`413`		`-}`
	`415`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -1,16 +1,17 @@`
`1`	`1`	`namespace Microsoft.AspNetCore.Mvc.Simulators`
`2`	`2`	`{`
	`3`	`+ using Routing;`
`3`	`4`	`using System;`
`4`	`5`	`using System.Threading.Tasks;`
`5`	`6`
`6`	`7`	`[ApiVersion( "2015-11-15" )]`
`7`	`8`	`[ApiVersion( "2016-06-06" )]`
`8`	`9`	`public class OrdersController : Controller`
`9`	`10`	`{`
`10`		`- [MapToApiVersion( "2015-11-15" )]`
`11`		`- public Task<IActionResult> Get_2015_11_15() => Task.FromResult<IActionResult>( Ok( "Version 2015-11-15" ) );`
	`11`	`+ [HttpGet]`
	`12`	`+ public Task<IActionResult> Get() => Task.FromResult<IActionResult>( Ok( "Version 2015-11-15" ) );`
`12`	`13`
`13`		`- [Route( "orders" )]`
	`14`	`+ [HttpGet]`
`14`	`15`	`[MapToApiVersion( "2016-06-06" )]`
`15`	`16`	`public Task<IActionResult> Get_2016_06_06() => Task.FromResult<IActionResult>( Ok( "Version 2016-06-06" ) );`
`16`	`17`	`}`