Skip to content

Commit 1847ac9

Browse files
committed
Enhance endpoint selection by resolving ambiguities through constraint specificity analysis
Reduce ambiguity between constraints by defining a level of specificity between the constraints. Previously, if you had two endpoints as follows: ``` GET test/{id:required} ``` and ``` GET test/{id:guid:required} ``` The DefaultEndpointSelector would not be able to determine which endpoint is the correct one among the two, as neither violates the Route Constraint policy and therefore are considered valid candidates. From a human perspective, the endpoint that has the constraint of the GUID is more specific than simply REQUIRED, but the engine did not make that distinction. Now, the DefaultEndpointSelector can eliminate ambiguities based on their constraint level of priority. Lets say the same routes described above. The engine will get two candidates available for it, but it can remove the first one cause its less specific than second one. But it is interesting to note that if the system cannot determine the priority, it will still report the ambiguity. Let's assume that the second request was equal to the first, it would trigger AmbiguousMatchException. The currently specifity order is defined as (from higher to lower one): 1 - Strong typed route constraint (e.g int, guid, long etc.) 2 - Ranged route contraint (e.g min, max, range) 3 - Length route constraint (e.g length, minlength, maxlength) 4 - String patterns (e.g regex, alphanumerical) 5 - File and non file 6 - Unknown route constraint 7 - Required route constraint As mentioned earlier, if after processing the ambiguities he still hasn't been able to determine a specificity, he will report the ambiguity, since there is nothing to be done in this case : ).
1 parent 623d09c commit 1847ac9

File tree

2 files changed

+180
-7
lines changed

2 files changed

+180
-7
lines changed

src/Http/Routing/src/Matching/DefaultEndpointSelector.cs

Lines changed: 119 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ private static void ProcessFinalCandidates(
6060
Endpoint? endpoint = null;
6161
RouteValueDictionary? values = null;
6262
int? foundScore = null;
63+
var candidatesWithSameScore = new List<CandidateState>();
64+
6365
for (var i = 0; i < candidateState.Length; i++)
6466
{
6567
ref var state = ref candidateState[i];
@@ -74,6 +76,7 @@ private static void ProcessFinalCandidates(
7476
endpoint = state.Endpoint;
7577
values = state.Values;
7678
foundScore = state.Score;
79+
candidatesWithSameScore.Add(state);
7780
}
7881
else if (foundScore < state.Score)
7982
{
@@ -85,15 +88,27 @@ private static void ProcessFinalCandidates(
8588
}
8689
else if (foundScore == state.Score)
8790
{
88-
// This is the second match we've found of the same score, so there
89-
// must be an ambiguity.
90-
//
91-
// Don't worry about the 'null == state.Score' case, it returns false.
91+
// Same score - collect for constraint specificity analysis. We cant
92+
// just dismiss these candidate and report an ambiguity, we need to
93+
// analyze them for constraint specificity.
94+
candidatesWithSameScore.Add(state);
95+
}
96+
}
9297

98+
// If we have multiple candidates with the same score, try to resolve using
99+
// constraint specificity rules
100+
if (candidatesWithSameScore.Count > 1)
101+
{
102+
var mostSpecific = SelectMostSpecificEndpoint(candidatesWithSameScore);
103+
if (mostSpecific.HasValue)
104+
{
105+
endpoint = mostSpecific.Value.Endpoint;
106+
values = mostSpecific.Value.Values;
107+
}
108+
else
109+
{
110+
// Still ambiguous after constraint analysis
93111
ReportAmbiguity(candidateState);
94-
95-
// Unreachable, ReportAmbiguity always throws.
96-
throw new NotSupportedException();
97112
}
98113
}
99114

@@ -104,6 +119,103 @@ private static void ProcessFinalCandidates(
104119
}
105120
}
106121

122+
private static CandidateState? SelectMostSpecificEndpoint(List<CandidateState> candidates)
123+
{
124+
CandidateState? mostSpecific = null;
125+
var highestSpecificity = -1;
126+
var hasAmbiguity = false;
127+
128+
foreach (var candidate in candidates)
129+
{
130+
if (candidate.Endpoint is not RouteEndpoint routeEndpoint)
131+
{
132+
continue;
133+
}
134+
135+
var specificity = CalculateConstraintSpecificity(routeEndpoint);
136+
137+
if (specificity > highestSpecificity)
138+
{
139+
highestSpecificity = specificity;
140+
mostSpecific = candidate;
141+
hasAmbiguity = false;
142+
}
143+
else if (specificity == highestSpecificity)
144+
{
145+
// Okay, note the ambiguity and continue trying
146+
// to determine a higher level of specificity.
147+
hasAmbiguity = true;
148+
}
149+
}
150+
151+
return hasAmbiguity ? null : mostSpecific;
152+
}
153+
154+
private static int CalculateConstraintSpecificity(RouteEndpoint endpoint)
155+
{
156+
var specificity = 0;
157+
var routePattern = endpoint.RoutePattern;
158+
159+
foreach (var parameter in routePattern.Parameters)
160+
{
161+
// We may have parameter without constraints, e.g. "id" in "/products/{id}"
162+
if (parameter.ParameterPolicies?.Count > 0)
163+
{
164+
foreach (var policy in parameter.ParameterPolicies)
165+
{
166+
if (policy.Content != null)
167+
{
168+
specificity += GetConstraintSpecificityWeight(policy.Content);
169+
}
170+
}
171+
}
172+
}
173+
174+
return specificity;
175+
}
176+
177+
private static int GetConstraintSpecificityWeight(string constraintName)
178+
{
179+
return constraintName.ToLowerInvariant() switch
180+
{
181+
// Strong typed constraints that are very restrictive and has
182+
// the highest specificity
183+
"guid" => 100,
184+
"datetime" => 90,
185+
"decimal" => 85,
186+
"double" => 80,
187+
"float" => 75,
188+
"long" => 70,
189+
"int" => 65,
190+
"bool" => 60,
191+
192+
// Range constraint are more restrictive than other types
193+
var range when range.StartsWith("range(", StringComparison.OrdinalIgnoreCase) => 55,
194+
var min when min.StartsWith("min(", StringComparison.OrdinalIgnoreCase) => 50,
195+
var max when max.StartsWith("max(", StringComparison.OrdinalIgnoreCase) => 50,
196+
197+
// This one is a bit odd, but are less specific than range, since defines
198+
// only the length of the value
199+
var length when length.StartsWith("length(", StringComparison.OrdinalIgnoreCase) => 45,
200+
var minlength when minlength.StartsWith("minlength(", StringComparison.OrdinalIgnoreCase) => 40,
201+
var maxlength when maxlength.StartsWith("maxlength(", StringComparison.OrdinalIgnoreCase) => 40,
202+
203+
// String patterns, which are less specific than length
204+
"alpha" => 35,
205+
var regex when regex.StartsWith("regex(", StringComparison.OrdinalIgnoreCase) => 30,
206+
207+
// File constraints
208+
"file" => 25,
209+
"nonfile" => 25,
210+
211+
// Least specific just requires non empty
212+
"required" => 10,
213+
214+
// Unknown constraint, assign medium specificity to it
215+
_ => 20
216+
};
217+
}
218+
107219
private static void ReportAmbiguity(Span<CandidateState> candidateState)
108220
{
109221
// If we get here it's the result of an ambiguity - we're OK with this

src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,28 @@ private DataSourceDependentMatcher CreateDfaMatcher(
5858
return Assert.IsType<DataSourceDependentMatcher>(factory.CreateMatcher(dataSource));
5959
}
6060

61+
[Fact]
62+
public async Task MatchAsync_SimilarEndpoints_CanDetermine_MostSpecificOne()
63+
{
64+
// Arrange
65+
var dataSource = new DefaultEndpointDataSource(new List<Endpoint>
66+
{
67+
CreateEndpoint("/test/{name:required}", 0),
68+
CreateEndpoint("/test/{id:guid:required}", 0),
69+
});
70+
71+
var matcher = CreateDfaMatcher(dataSource);
72+
73+
var httpContext = CreateContext();
74+
httpContext.Request.Path = "/test/" + Guid.NewGuid().ToString();
75+
76+
// Act
77+
await matcher.MatchAsync(httpContext);
78+
79+
// Assert
80+
Assert.Same(dataSource.Endpoints[1], httpContext.GetEndpoint());
81+
}
82+
6183
[Fact]
6284
public async Task MatchAsync_ValidRouteConstraint_EndpointMatched()
6385
{
@@ -312,6 +334,45 @@ public async Task MatchAsync_MultipleEndpointsWithDifferentRequiredValues_Endpoi
312334
Assert.Same(endpoint2, httpContext.GetEndpoint());
313335
}
314336

337+
[Fact]
338+
public async Task MatchAsync_MultipleEndpointsWithSameRequiredValues_EndpointMatched()
339+
{
340+
// Arrange
341+
var endpoint1 = CreateEndpoint(
342+
"{controller}/{action}/{id?}",
343+
0,
344+
requiredValues: new { controller = "Home", action = "Index", area = (string)null, page = (string)null });
345+
var endpoint2 = CreateEndpoint(
346+
"{controller}/{action}/{id?}",
347+
0,
348+
requiredValues: new { controller = "Login", action = "Index", area = (string)null, page = (string)null });
349+
350+
var dataSource = new DefaultEndpointDataSource(new List<Endpoint>
351+
{
352+
endpoint1,
353+
endpoint2
354+
});
355+
356+
var matcher = CreateDfaMatcher(dataSource);
357+
358+
var httpContext = CreateContext();
359+
httpContext.Request.Path = "/Home/Index/123";
360+
361+
// Act 1
362+
await matcher.MatchAsync(httpContext);
363+
364+
// Assert 1
365+
Assert.Same(endpoint1, httpContext.GetEndpoint());
366+
367+
httpContext.Request.Path = "/Login/Index/123";
368+
369+
// Act 2
370+
await matcher.MatchAsync(httpContext);
371+
372+
// Assert 2
373+
Assert.Same(endpoint2, httpContext.GetEndpoint());
374+
}
375+
315376
[Fact]
316377
public async Task MatchAsync_ParameterTransformer_EndpointMatched()
317378
{

0 commit comments

Comments
 (0)