diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.AspNetCore.Mvc.Versioning.csproj b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.AspNetCore.Mvc.Versioning.csproj index d7f8cda9..d2fc1c88 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.AspNetCore.Mvc.Versioning.csproj +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.AspNetCore.Mvc.Versioning.csproj @@ -1,7 +1,7 @@  - 1.1.0 + 1.1.1 1.1.0.0 net451;netstandard1.6 1.6.1 @@ -11,7 +11,7 @@ A service API versioning library for Microsoft ASP.NET Core. Microsoft.AspNetCore.Mvc Microsoft;AspNet;AspNetCore;Versioning - https://github.com/Microsoft/aspnet-api-versioning/releases/tag/v1.1.0 + • Fixed overlapped route action selection (Issue #148) diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs index c6605bd6..20c0cf10 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs @@ -3,7 +3,6 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc.Abstractions; - using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Routing; @@ -16,6 +15,7 @@ using Versioning; using static ApiVersion; using static System.Environment; + using static System.Linq.Enumerable; using static System.String; using static Versioning.ErrorCodes; @@ -193,10 +193,7 @@ RequestHandler ClientError( RouteContext context, ActionSelectionResult selectio var requestedVersion = default( string ); var parsedVersion = properties.ApiVersion; var actionNames = new Lazy( () => Join( NewLine, candidates.Select( a => a.DisplayName ) ) ); - var allowedMethods = new Lazy>( - () => new HashSet( candidates.SelectMany( c => c.ActionConstraints.OfType() ) - .SelectMany( ac => ac.HttpMethods ), - StringComparer.OrdinalIgnoreCase ) ); + var allowedMethods = new Lazy>( () => AllowedMethodsFromCandidates( candidates ) ); var newRequestHandler = default( Func ); if ( parsedVersion == null ) @@ -250,6 +247,29 @@ RequestHandler ClientError( RouteContext context, ActionSelectionResult selectio return newRequestHandler( ErrorResponseProvider, code, message ); } + static HashSet AllowedMethodsFromCandidates( IEnumerable candidates ) + { + Contract.Requires( candidates != null ); + Contract.Ensures( Contract.Result>() != null ); + + var httpMethods = new HashSet( StringComparer.OrdinalIgnoreCase ); + + foreach ( var candidate in candidates ) + { + if ( candidate.ActionConstraints == null ) + { + continue; + } + + foreach ( var constraint in candidate.ActionConstraints.OfType() ) + { + httpMethods.AddRange( constraint.HttpMethods ); + } + } + + return httpMethods; + } + sealed class DefaultActionHandler { readonly IActionContextAccessor actionContextAccessor; diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs index 86a061e6..9e72a155 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs +++ b/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; } diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/OverlappingRouteTemplateController.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/OverlappingRouteTemplateController.cs new file mode 100644 index 00000000..47bbf86f --- /dev/null +++ b/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 } ); + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when two route templates overlap.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when two route templates overlap.cs new file mode 100644 index 00000000..2869b444 --- /dev/null +++ b/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}" ); + } + } +} \ No newline at end of file