Skip to content

Commit 03497ff

Browse files
Chris Martinezcommonsensesoftware
Chris Martinez
authored and
commonsensesoftware
committed
Test for and apply action-level conventions before falling back to implicit controller-level conventions
1 parent 29f7603 commit 03497ff

File tree

9 files changed

+232
-123
lines changed

9 files changed

+232
-123
lines changed

‎src/Common/Versioning/Conventions/ApiVersionConventionBuilder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@ bool InternalApplyTo( Model model )
113113

114114
if ( !hasExplicitConventions )
115115
{
116-
var hasNoImplicitConventions = ControllerConventions.Count == 0;
116+
var hasNoExplicitConventions = ControllerConventions.Count == 0;
117117

118-
if ( hasNoImplicitConventions )
118+
if ( hasNoExplicitConventions && !( applied = HasDecoratedActions( model ) ) )
119119
{
120120
return false;
121121
}

‎src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ApiVersionControllerSelector.cs

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ ConcurrentDictionary<string, HttpControllerDescriptorGroup> InitializeController
177177
{
178178
var descriptor = new HttpControllerDescriptor( configuration, key, type );
179179

180-
if ( conventions.Count == 0 || !conventions.ApplyTo( descriptor ) )
180+
if ( !conventions.ApplyTo( descriptor ) )
181181
{
182182
ApplyAttributeOrImplicitConventions( descriptor, actionSelector, implicitVersionModel );
183183
}
@@ -244,19 +244,26 @@ static HttpControllerDescriptor[] ApplyCollatedModels( List<HttpControllerDescri
244244

245245
var supported = new HashSet<ApiVersion>();
246246
var deprecated = new HashSet<ApiVersion>();
247-
var models = new List<ApiVersionModel>( controllers.Count );
248-
var visited = new List<Tuple<HttpActionDescriptor, ApiVersionModel>>( controllers.Count );
247+
var controllerModels = new List<ApiVersionModel>( controllers.Count );
248+
var actionModels = new List<ApiVersionModel>( controllers.Count );
249+
var visitedControllers = new List<Tuple<HttpControllerDescriptor, ApiVersionModel>>( controllers.Count );
250+
var visitedActions = new List<Tuple<HttpActionDescriptor, ApiVersionModel>>( controllers.Count );
249251

250252
for ( var i = 0; i < controllers.Count; i++ )
251253
{
252-
// 1 - collate controller versions (only useful for 400s)
254+
// 1 - collate controller versions
253255
var controller = controllers[i];
254256
var model = controller.GetApiVersionModel();
255257

256-
supported.AddRange( model.SupportedApiVersions );
257-
deprecated.AddRange( model.DeprecatedApiVersions );
258+
if ( model.IsApiVersionNeutral )
259+
{
260+
continue;
261+
}
258262

259-
// 2 - collate action versions (only required for reporting api versions)
263+
controllerModels.Add( model );
264+
visitedControllers.Add( Tuple.Create( controller, model ) );
265+
266+
// 2 - collate action versions
260267
var actions = actionSelector.GetActionMapping( controller ).SelectMany( g => g );
261268

262269
foreach ( var action in actions )
@@ -268,36 +275,31 @@ static HttpControllerDescriptor[] ApplyCollatedModels( List<HttpControllerDescri
268275
continue;
269276
}
270277

271-
models.Add( model );
272-
visited.Add( Tuple.Create( action, model ) );
278+
actionModels.Add( model );
279+
visitedActions.Add( Tuple.Create( action, model ) );
273280
}
274281
}
275282

276-
// 3 - apply collated controller model
277-
var collatedModel = new ApiVersionModel( supported, deprecated );
283+
// 3 - apply collated action model
284+
var collatedModel = actionModels.Aggregate();
278285

279-
for ( var i = 0; i < controllers.Count; i++ )
286+
for ( var i = 0; i < visitedActions.Count; i++ )
280287
{
281-
var controller = controllers[i];
282-
var model = controller.GetApiVersionModel();
288+
var (action, model) = visitedActions[i];
283289

284-
controller.SetApiVersionModel( model.Aggregate( collatedModel ) );
285-
}
286-
287-
if ( visited.Count == 0 )
288-
{
289-
return controllers.ToArray();
290+
action.SetProperty( model.Aggregate( collatedModel ) );
290291
}
291292

292-
// 4 - apply collated action model
293-
collatedModel = models.Aggregate();
293+
// 4 - apply collated controller model
294+
// note: allows controllers to report versions in 400s even when an action is unmatched
295+
controllerModels.Add( collatedModel );
296+
collatedModel = controllerModels.Aggregate();
294297

295-
for ( var i = 0; i < visited.Count; i++ )
298+
for ( var i = 0; i < visitedControllers.Count; i++ )
296299
{
297-
var item = visited[i];
298-
var (action, model) = item;
300+
var (controller, model) = visitedControllers[i];
299301

300-
action.SetProperty( model.Aggregate( collatedModel ) );
302+
controller.SetApiVersionModel( model.Aggregate( collatedModel ) );
301303
}
302304

303305
return controllers.ToArray();

‎src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ControllerSelector.cs

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
namespace Microsoft.Web.Http.Dispatcher
2+
{
3+
using Microsoft.Web.Http.Routing;
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Diagnostics.Contracts;
7+
using System.Linq;
8+
using System.Net.Http;
9+
using System.Web.Http;
10+
using System.Web.Http.Controllers;
11+
using System.Web.Http.Routing;
12+
using static Microsoft.Web.Http.Versioning.ApiVersionMapping;
13+
14+
abstract class ControllerSelector
15+
{
16+
protected ControllerSelector() { }
17+
18+
public abstract ControllerSelectionResult SelectController( ControllerSelectionContext context );
19+
20+
protected static ICollection<HttpControllerDescriptor> SelectBestCandidates( IReadOnlyList<CandidateAction> candidates, ApiVersion apiVersion )
21+
{
22+
Contract.Requires( candidates != null );
23+
Contract.Requires( apiVersion != null );
24+
Contract.Ensures( Contract.Result<ICollection<HttpControllerDescriptor>>() != null );
25+
26+
var bestMatch = default( HttpActionDescriptor );
27+
var bestMatches = new HashSet<HttpControllerDescriptor>();
28+
var implicitMatches = new HashSet<HttpControllerDescriptor>();
29+
30+
for ( var i = 0; i < candidates.Count; i++ )
31+
{
32+
var action = candidates[i].ActionDescriptor;
33+
34+
switch ( action.MappingTo( apiVersion ) )
35+
{
36+
case Explicit:
37+
bestMatch = action;
38+
bestMatches.Add( action.ControllerDescriptor );
39+
break;
40+
case Implicit:
41+
implicitMatches.Add( action.ControllerDescriptor );
42+
break;
43+
}
44+
}
45+
46+
switch ( bestMatches.Count )
47+
{
48+
case 0:
49+
bestMatches.UnionWith( implicitMatches );
50+
break;
51+
case 1:
52+
if ( bestMatch.GetApiVersionModel().IsApiVersionNeutral )
53+
{
54+
bestMatches.UnionWith( implicitMatches );
55+
}
56+
57+
break;
58+
}
59+
60+
return bestMatches;
61+
}
62+
63+
protected static bool TryDisambiguateControllerByAction(
64+
HttpRequestMessage request,
65+
IEnumerable<HttpControllerDescriptor> controllers,
66+
out HttpControllerDescriptor resolvedController )
67+
{
68+
Contract.Requires( request != null );
69+
Contract.Requires( controllers != null );
70+
71+
// note: this method should only legitimately be called to disambiguate actions
72+
// from different controller types that match the request. this can happen as a
73+
// result of inheritance with a version-neutral action on a base class. there's
74+
// still a chance the action is actually ambiguous, which is a developer mistake
75+
var matches = new HashSet<HttpControllerDescriptor>();
76+
77+
foreach ( var controller in controllers )
78+
{
79+
var configuration = controller.Configuration;
80+
var actionSelector = configuration.Services.GetActionSelector();
81+
var routeData = EnsureRouteDataSet( configuration, request );
82+
var context = new HttpControllerContext( configuration, routeData, request )
83+
{
84+
ControllerDescriptor = controller,
85+
};
86+
87+
try
88+
{
89+
if ( actionSelector.SelectAction( context ) != null )
90+
{
91+
matches.Add( controller );
92+
}
93+
}
94+
catch ( InvalidOperationException )
95+
{
96+
}
97+
catch ( HttpResponseException )
98+
{
99+
}
100+
}
101+
102+
if ( matches.Count == 1 )
103+
{
104+
resolvedController = matches.Single();
105+
return true;
106+
}
107+
108+
resolvedController = default;
109+
return false;
110+
}
111+
112+
static IHttpRouteData EnsureRouteDataSet( HttpConfiguration configuration, HttpRequestMessage request )
113+
{
114+
Contract.Requires( configuration != null );
115+
Contract.Requires( request != null );
116+
117+
var routeData = request.GetRouteData();
118+
119+
if ( routeData != null )
120+
{
121+
return routeData;
122+
}
123+
124+
if ( ( routeData = configuration.Routes.GetRouteData( request ) ) != null )
125+
{
126+
request.SetRouteData( routeData );
127+
}
128+
129+
return routeData;
130+
}
131+
}
132+
}

‎src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ConventionRouteControllerSelector.cs

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,16 @@
66
using System.Linq;
77
using System.Text;
88
using System.Web.Http;
9-
using System.Web.Http.Controllers;
109
using System.Web.Http.Routing;
11-
using static Microsoft.Web.Http.Versioning.ApiVersionMapping;
1210
using static System.Environment;
1311

14-
sealed class ConventionRouteControllerSelector : IControllerSelector
12+
sealed class ConventionRouteControllerSelector : ControllerSelector
1513
{
1614
readonly HttpControllerTypeCache controllerTypeCache;
1715

1816
internal ConventionRouteControllerSelector( HttpControllerTypeCache controllerTypeCache ) => this.controllerTypeCache = controllerTypeCache;
1917

20-
public ControllerSelectionResult SelectController( ControllerSelectionContext context )
18+
public override ControllerSelectionResult SelectController( ControllerSelectionContext context )
2119
{
2220
Contract.Requires( context != null );
2321
Contract.Ensures( Contract.Result<ControllerSelectionResult>() != null );
@@ -37,39 +35,7 @@ public ControllerSelectionResult SelectController( ControllerSelectionContext co
3735
return result;
3836
}
3937

40-
var bestMatch = default( HttpActionDescriptor );
41-
var bestMatches = new HashSet<HttpControllerDescriptor>();
42-
var implicitMatches = new HashSet<HttpControllerDescriptor>();
43-
44-
for ( var i = 0; i < context.ConventionRouteCandidates.Length; i++ )
45-
{
46-
var action = context.ConventionRouteCandidates[i].ActionDescriptor;
47-
48-
switch ( action.MappingTo( requestedVersion ) )
49-
{
50-
case Explicit:
51-
bestMatch = action;
52-
bestMatches.Add( action.ControllerDescriptor );
53-
break;
54-
case Implicit:
55-
implicitMatches.Add( action.ControllerDescriptor );
56-
break;
57-
}
58-
}
59-
60-
switch ( bestMatches.Count )
61-
{
62-
case 0:
63-
bestMatches.UnionWith( implicitMatches );
64-
break;
65-
case 1:
66-
if ( bestMatch.GetApiVersionModel().IsApiVersionNeutral )
67-
{
68-
bestMatches.UnionWith( implicitMatches );
69-
}
70-
71-
break;
72-
}
38+
var bestMatches = SelectBestCandidates( context.ConventionRouteCandidates, requestedVersion );
7339

7440
switch ( bestMatches.Count )
7541
{
@@ -80,6 +46,12 @@ public ControllerSelectionResult SelectController( ControllerSelectionContext co
8046
result.Controller.SetPossibleCandidates( context.ConventionRouteCandidates.Select( c => c.ActionDescriptor.ControllerDescriptor ).ToArray() );
8147
break;
8248
default:
49+
if ( TryDisambiguateControllerByAction( request, bestMatches, out var resolvedController ) )
50+
{
51+
result.Controller = resolvedController;
52+
break;
53+
}
54+
8355
throw CreateAmbiguousControllerException( context.RouteData.Route, controllerName, controllerTypeCache.GetControllerTypes( controllerName ) );
8456
}
8557

‎src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/DirectRouteControllerSelector.cs

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,12 @@
55
using System.Diagnostics.Contracts;
66
using System.Linq;
77
using System.Text;
8-
using System.Web.Http;
98
using System.Web.Http.Controllers;
10-
using static Microsoft.Web.Http.Versioning.ApiVersionMapping;
119
using static System.Environment;
1210

13-
sealed class DirectRouteControllerSelector : IControllerSelector
11+
sealed class DirectRouteControllerSelector : ControllerSelector
1412
{
15-
public ControllerSelectionResult SelectController( ControllerSelectionContext context )
13+
public override ControllerSelectionResult SelectController( ControllerSelectionContext context )
1614
{
1715
Contract.Requires( context != null );
1816
Contract.Ensures( Contract.Result<ControllerSelectionResult>() != null );
@@ -30,39 +28,7 @@ public ControllerSelectionResult SelectController( ControllerSelectionContext co
3028
return result;
3129
}
3230

33-
var bestMatch = default( HttpActionDescriptor );
34-
var bestMatches = new HashSet<HttpControllerDescriptor>();
35-
var implicitMatches = new HashSet<HttpControllerDescriptor>();
36-
37-
for ( var i = 0; i < context.DirectRouteCandidates.Length; i++ )
38-
{
39-
var action = context.DirectRouteCandidates[i].ActionDescriptor;
40-
41-
switch ( action.MappingTo( requestedVersion ) )
42-
{
43-
case Explicit:
44-
bestMatch = action;
45-
bestMatches.Add( action.ControllerDescriptor );
46-
break;
47-
case Implicit:
48-
implicitMatches.Add( action.ControllerDescriptor );
49-
break;
50-
}
51-
}
52-
53-
switch ( bestMatches.Count )
54-
{
55-
case 0:
56-
bestMatches.UnionWith( implicitMatches );
57-
break;
58-
case 1:
59-
if ( bestMatch.GetApiVersionModel().IsApiVersionNeutral )
60-
{
61-
bestMatches.UnionWith( implicitMatches );
62-
}
63-
64-
break;
65-
}
31+
var bestMatches = SelectBestCandidates( context.DirectRouteCandidates, requestedVersion );
6632

6733
switch ( bestMatches.Count )
6834
{
@@ -72,6 +38,12 @@ public ControllerSelectionResult SelectController( ControllerSelectionContext co
7238
result.Controller = bestMatches.Single();
7339
break;
7440
default:
41+
if ( TryDisambiguateControllerByAction( request, bestMatches, out var resolvedController ) )
42+
{
43+
result.Controller = resolvedController;
44+
break;
45+
}
46+
7547
throw CreateAmbiguousControllerException( bestMatches );
7648
}
7749

‎src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/IControllerSelector.cs

Lines changed: 0 additions & 9 deletions
This file was deleted.

0 commit comments

Comments
 (0)