Update action selection for routes with overlapping route templates. Fixes #148. Relates to #147.

Author: Chris Martinez

src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.AspNetCore.Mvc.Versioning.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
  <PropertyGroup>
-  <VersionPrefix>1.1.0</VersionPrefix>
+  <VersionPrefix>1.1.1</VersionPrefix>
   <AssemblyVersion>1.1.0.0</AssemblyVersion>
   <TargetFrameworks>net451;netstandard1.6</TargetFrameworks>
   <NetStandardImplicitPackageVersion>1.6.1</NetStandardImplicitPackageVersion>
@@ -11,7 +11,7 @@
   <Description>A service API versioning library for Microsoft ASP.NET Core.</Description>
   <RootNamespace>Microsoft.AspNetCore.Mvc</RootNamespace>
   <PackageTags>Microsoft;AspNet;AspNetCore;Versioning</PackageTags>
-  <PackageReleaseNotes>https://github.com/Microsoft/aspnet-api-versioning/releases/tag/v1.1.0</PackageReleaseNotes>
+  <PackageReleaseNotes>• Fixed overlapped route action selection (Issue #148)</PackageReleaseNotes>
  </PropertyGroup>
 
  <ItemGroup>

src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs

@@ -120,7 +120,8 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
                 var selectedAction = finalMatches[0];
 
                 // note: short-circuit if the api version policy has already been applied to the match
-                if ( selectedAction.VersionPolicyIsApplied() )
+                // and no other better match has already been selected
+                if ( selectedAction.VersionPolicyIsApplied() && selectionResult.BestMatch == null )
                 {
                     httpContext.ApiVersionProperties().ApiVersion = selectionContext.RequestedVersion;
                     return selectedAction;
@@ -137,7 +138,7 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
             }
 
             // note: even though we may have had a successful match, this method could be called multiple times. the final decision
-            // is made by the IApiVersionRoutePolicy. we return here to make sure all candidates have been considered.
+            // is made by the IApiVersionRoutePolicy. we return here to make sure all candidates have been considered at least once.
             selectionResult.EndIteration();
             return null;
         }

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/OverlappingRouteTemplateController.cs

@@ -0,0 +1,16 @@
+namespace Microsoft.AspNetCore.Mvc.Basic.Controllers
+{
+    using Microsoft.AspNetCore.Mvc;
+    using System;
+
+    [ApiVersion( "1.0" )]
+    [Route( "api/v{version:apiVersion}/values" )]
+    public class OverlappingRouteTemplateController : Controller
+    {
+        [HttpGet( "{id:int}/{childId}" )]
+        public IActionResult Get( int id, string childId ) => Ok( new { id, childId } );
+
+        [HttpGet( "{id:int}/children" )]
+        public IActionResult Get( int id ) => Ok( new { id } );
+    }
+}

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when two route templates overlap.cs

@@ -0,0 +1,51 @@
+namespace given_a_versioned_Controller
+{
+    using FluentAssertions;
+    using Microsoft.AspNetCore.Mvc;
+    using Microsoft.AspNetCore.Mvc.Basic;
+    using Microsoft.AspNetCore.Mvc.Basic.Controllers;
+    using System.Reflection;
+    using System.Threading.Tasks;
+    using Xunit;
+
+    public class when_two_route_templates_overlap : BasicAcceptanceTest
+    {
+        public when_two_route_templates_overlap()
+        {
+            FilteredControllerTypes.Clear();
+            FilteredControllerTypes.Add( typeof( OverlappingRouteTemplateController ).GetTypeInfo() );
+        }
+
+        [Fact]
+        public async Task then_the_higher_precedence_route_should_be_selected_during_the_first_request()
+        {
+            // arrange
+            var response = await Client.GetAsync( "api/v1/values/42/children" );
+            var result1 = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+
+            // act
+            response = await Client.GetAsync( "api/v1/values/42/abc" );
+            var result2 = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+
+            // assert
+            result1.Should().Be( "{\"id\":42}" );
+            result2.Should().Be( "{\"id\":42,\"childId\":\"abc\"}" );
+        }
+
+        [Fact]
+        public async Task then_the_higher_precedence_route_should_be_selected_during_the_second_request()
+        {
+            // arrange
+            var response = await Client.GetAsync( "api/v1/values/42/abc" );
+            var result1 = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+
+            // act
+            response = await Client.GetAsync( "api/v1/values/42/children" );
+            var result2 = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+
+            // assert
+            result1.Should().Be( "{\"id\":42,\"childId\":\"abc\"}" );
+            result2.Should().Be( "{\"id\":42}" );
+        }
+    }
+}