Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3857a33

Browse filesBrowse files
commonsensesoftwarecommonsensesoftware
committed
Implicit endpoint with better score should have precedence over explicit endpoint with worse score. Fixes #884 
1 parent a3ddc27 commit 3857a33
Copy full SHA for 3857a33

File tree

Expand file treeCollapse file tree

3 files changed

+106
-35
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+106
-35
lines changed

‎src/AspNetCore/Acceptance/Asp.Versioning.Mvc.Acceptance.Tests/Mvc/UsingAttributes/Controllers/OverlappingRouteTemplateController.cs

Copy file name to clipboardExpand all lines: src/AspNetCore/Acceptance/Asp.Versioning.Mvc.Acceptance.Tests/Mvc/UsingAttributes/Controllers/OverlappingRouteTemplateController.cs
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22

3+
#pragma warning disable CA1822 // Mark members as static
4+
35
namespace Asp.Versioning.Mvc.UsingAttributes.Controllers;
46

57
using Microsoft.AspNetCore.Mvc;
68

79
[ApiController]
810
[ApiVersion( "1.0" )]
11+
[ApiVersion( "2.0" )]
912
[Route( "api/v{version:apiVersion}/values" )]
1013
public class OverlappingRouteTemplateController : ControllerBase
1114
{
@@ -20,4 +23,11 @@ public class OverlappingRouteTemplateController : ControllerBase
2023

2124
[HttpGet( "{id:int}/ambiguous" )]
2225
public IActionResult Ambiguous2( int id ) => Ok();
26+
27+
[HttpGet( "[action]" )]
28+
public string Echo() => "Test";
29+
30+
[HttpGet( "[action]/{id}" )]
31+
[MapToApiVersion( "1.0" )]
32+
public string Echo( string id ) => id;
2333
}

‎src/AspNetCore/Acceptance/Asp.Versioning.Mvc.Acceptance.Tests/Mvc/UsingAttributes/given a versioned Controller/when two route templates overlap.cs

Copy file name to clipboardExpand all lines: src/AspNetCore/Acceptance/Asp.Versioning.Mvc.Acceptance.Tests/Mvc/UsingAttributes/given a versioned Controller/when two route templates overlap.cs
+36Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ namespace given_a_versioned_Controller;
44

55
using Asp.Versioning;
66
using Asp.Versioning.Mvc.UsingAttributes;
7+
using System.Net;
8+
using static System.Net.HttpStatusCode;
79

810
public class when_two_route_templates_overlap : AcceptanceTest, IClassFixture<OverlappingRouteTemplateFixture>
911
{
@@ -54,6 +56,40 @@ public async Task then_the_higher_precedence_route_should_result_in_ambiguous_ac
5456
( await act.Should().ThrowAsync<Exception>() ).And.GetType().Name.Should().Be( "AmbiguousMatchException" );
5557
}
5658

59+
[Theory]
60+
[InlineData( "api/v1/values/echo" )]
61+
[InlineData( "api/v2/values/echo" )]
62+
public async Task then_route_with_same_score_and_version_should_return_200( string requestUri )
63+
{
64+
// arrange
65+
66+
67+
// act
68+
var response = await GetAsync( requestUri );
69+
70+
// assert
71+
response.StatusCode.Should().Be( OK );
72+
}
73+
74+
[Theory]
75+
[InlineData( "api/v1/values/echo/42", OK )]
76+
#if NETCOREAPP3_1
77+
[InlineData( "api/v2/values/echo/42", BadRequest )]
78+
#else
79+
[InlineData( "api/v2/values/echo/42", NotFound )]
80+
#endif
81+
public async Task then_route_with_same_score_and_different_versions_should_return_expected_status( string requestUri, HttpStatusCode statusCode )
82+
{
83+
// arrange
84+
85+
86+
// act
87+
var response = await GetAsync( requestUri );
88+
89+
// assert
90+
response.StatusCode.Should().Be( statusCode );
91+
}
92+
5793
public when_two_route_templates_overlap( OverlappingRouteTemplateFixture fixture, ITestOutputHelper console )
5894
: base( fixture ) => console.WriteLine( fixture.DirectedGraphVisualizationUrl );
5995
}

‎src/AspNetCore/WebApi/src/Asp.Versioning.Http/Routing/ApiVersionMatcherPolicy.cs

Copy file name to clipboardExpand all lines: src/AspNetCore/WebApi/src/Asp.Versioning.Http/Routing/ApiVersionMatcherPolicy.cs
+60-35Lines changed: 60 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ namespace Asp.Versioning.Routing;
88
using Microsoft.AspNetCore.Routing.Patterns;
99
using Microsoft.Extensions.Logging;
1010
using Microsoft.Extensions.Options;
11+
using System.Buffers;
1112
using System.Runtime.CompilerServices;
1213
using System.Text.RegularExpressions;
1314
using static Asp.Versioning.ApiVersionMapping;
@@ -261,7 +262,7 @@ private static bool DifferByRouteConstraintsOnly( CandidateSet candidates )
261262

262263
for ( var i = 0; i < candidates.Count; i++ )
263264
{
264-
ref var candidate = ref candidates[i];
265+
ref readonly var candidate = ref candidates[i];
265266

266267
if ( candidate.Endpoint is not RouteEndpoint endpoint )
267268
{
@@ -291,81 +292,85 @@ private static bool DifferByRouteConstraintsOnly( CandidateSet candidates )
291292

292293
private static (bool Matched, bool HasCandidates) MatchApiVersion( CandidateSet candidates, ApiVersion? apiVersion )
293294
{
294-
List<int>? bestMatches = default;
295-
List<int>? implicitMatches = default;
295+
var total = candidates.Count;
296+
var count = 0;
297+
var array = default( Match[] );
298+
var bestMatch = default( Match? );
296299
var hasCandidates = false;
300+
Span<Match> matches =
301+
total <= 16
302+
? stackalloc Match[total]
303+
: ( array = ArrayPool<Match>.Shared.Rent( total ) ).AsSpan();
297304

298-
for ( var i = 0; i < candidates.Count; i++ )
305+
for ( var i = 0; i < total; i++ )
299306
{
300307
if ( !candidates.IsValidCandidate( i ) )
301308
{
302309
continue;
303310
}
304311

305312
hasCandidates = true;
306-
ref var candidate = ref candidates[i];
313+
ref readonly var candidate = ref candidates[i];
307314
var metadata = candidate.Endpoint.Metadata.GetMetadata<ApiVersionMetadata>();
308315

309316
if ( metadata == null )
310317
{
311318
continue;
312319
}
313320

314-
// remember whether the candidate is currently valid. a matching api version will not
315-
// make the candidate valid; however, we want to short-circuit with 400 if no candidates
316-
// match the api version at all.
321+
var score = candidate.Score;
322+
bool isExplicit;
323+
324+
// perf: always make the candidate invalid so we only need to loop through the
325+
// final, best matches for any remaining candidates
326+
candidates.SetValidity( i, false );
327+
317328
switch ( metadata.MappingTo( apiVersion ) )
318329
{
319330
case Explicit:
320-
bestMatches ??= new();
321-
bestMatches.Add( i );
331+
isExplicit = true;
322332
break;
323333
case Implicit:
324-
implicitMatches ??= new();
325-
implicitMatches.Add( i );
334+
isExplicit = metadata.IsApiVersionNeutral;
326335
break;
336+
default:
337+
continue;
327338
}
328339

329-
// perf: always make the candidate invalid so we only need to loop through the
330-
// final, best matches for any remaining candidates
331-
candidates.SetValidity( i, false );
332-
}
340+
var match = new Match( i, score, isExplicit );
333341

334-
if ( bestMatches is null )
335-
{
336-
if ( implicitMatches is null )
337-
{
338-
return (false, hasCandidates);
339-
}
342+
matches[count++] = match;
340343

341-
for ( var i = 0; i < implicitMatches.Count; i++ )
344+
if ( !bestMatch.HasValue || match.CompareTo( bestMatch.Value ) > 0 )
342345
{
343-
candidates.SetValidity( implicitMatches[i], true );
346+
bestMatch = match;
344347
}
345-
346-
return (true, hasCandidates);
347348
}
348349

349-
if ( bestMatches.Count == 1 && implicitMatches is not null )
350+
var matched = false;
351+
352+
if ( bestMatch.HasValue )
350353
{
351-
ref var candidate = ref candidates[bestMatches[0]];
352-
var metadata = candidate.Endpoint.Metadata.GetMetadata<ApiVersionMetadata>()!;
354+
matched = true;
355+
var match = bestMatch.Value;
353356

354-
if ( metadata.IsApiVersionNeutral )
357+
for ( var i = 0; i < count; i++ )
355358
{
356-
for ( var i = 0; i < implicitMatches.Count; i++ )
359+
ref readonly var otherMatch = ref matches[i];
360+
361+
if ( match.CompareTo( otherMatch ) == 0 )
357362
{
358-
candidates.SetValidity( implicitMatches[i], true );
363+
candidates.SetValidity( otherMatch.Index, true );
359364
}
360365
}
361366
}
362367

363-
for ( var i = 0; i < bestMatches.Count; i++ )
368+
if ( array is not null )
364369
{
365-
candidates.SetValidity( bestMatches[i], true );
370+
ArrayPool<Match>.Shared.Return( array );
366371
}
367372

368-
return (true, hasCandidates);
373+
return (matched, hasCandidates);
369374
}
370375

371376
private ApiVersion TrySelectApiVersion( HttpContext httpContext, CandidateSet candidates )
@@ -393,4 +398,24 @@ private ApiVersion TrySelectApiVersion( HttpContext httpContext, CandidateSet ca
393398

394399
bool INodeBuilderPolicy.AppliesToEndpoints( IReadOnlyList<Endpoint> endpoints ) =>
395400
!ContainsDynamicEndpoints( endpoints ) && AppliesToEndpoints( endpoints );
401+
402+
private readonly struct Match
403+
{
404+
internal readonly int Index;
405+
internal readonly int Score;
406+
internal readonly bool IsExplicit;
407+
408+
internal Match( int index, int score, bool isExplicit )
409+
{
410+
Index = index;
411+
Score = score;
412+
IsExplicit = isExplicit;
413+
}
414+
415+
internal int CompareTo( in Match other )
416+
{
417+
var result = -Score.CompareTo( other.Score );
418+
return result == 0 ? IsExplicit.CompareTo( other.IsExplicit ) : result;
419+
}
420+
}
396421
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.