Fix for ASP.NET Core URL Generation and 404

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;
         }
     }
-}
+}

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
             }
         }
     }
-}
+}

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" );
+        }
     }
-}
+}

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" ) );
     }

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() );
             }
         }