Update route parameters and fix action selection. Fixes #336. Resolves #337. Resolves #341.

Chris Martinez · Chris Martinez · commit 0b3a6c84cc61 · 2018-09-30T20:22:15.000-07:00
diff --git a/src/Microsoft.AspNet.OData.Versioning/Routing/VersionedAttributeRoutingConvention.cs b/src/Microsoft.AspNet.OData.Versioning/Routing/VersionedAttributeRoutingConvention.cs
@@ -19,6 +19,7 @@
     /// </content>
     public partial class VersionedAttributeRoutingConvention : IODataRoutingConvention
     {
+        const string AttributeRouteData = nameof( AttributeRouteData );
         static readonly DefaultODataPathHandler defaultPathHandler = new DefaultODataPathHandler();
 
         /// <summary>
@@ -161,8 +162,8 @@ public virtual string SelectController( ODataPath odataPath, HttpRequestMessage
 
                 if ( action.SupportedHttpMethods.Contains( request.Method ) && template.TryMatch( odataPath, values ) )
                 {
-                    values["action"] = action.ActionName;
-                    request.Properties["AttributeRouteData"] = values;
+                    values[ODataRouteConstants.Action] = action.ActionName;
+                    request.Properties[AttributeRouteData] = values;
 
                     return action.ControllerDescriptor.ControllerName;
                 }
@@ -183,7 +184,7 @@ public virtual string SelectAction( ODataPath odataPath, HttpControllerContext c
             var request = controllerContext.Request;
             var properties = request.Properties;
 
-            if ( !properties.TryGetValue( (string) "AttributeRouteData", out var value ) || !( value is IDictionary<string, object> attributeRouteData ) )
+            if ( !properties.TryGetValue( AttributeRouteData, out var value ) || !( value is IDictionary<string, object> attributeRouteData ) )
             {
                 return null;
             }
@@ -203,7 +204,7 @@ public virtual string SelectAction( ODataPath odataPath, HttpControllerContext c
                 }
             }
 
-            return attributeRouteData["action"]?.ToString();
+            return attributeRouteData[ODataRouteConstants.Action]?.ToString();
         }
 
         IDictionary<ODataPathTemplate, HttpActionDescriptor> BuildAttributeMappings( IEnumerable<HttpControllerDescriptor> controllers )
diff --git a/src/Microsoft.AspNetCore.OData.Versioning/AspNet.OData/Routing/VersionedAttributeRoutingConvention.cs b/src/Microsoft.AspNetCore.OData.Versioning/AspNet.OData/Routing/VersionedAttributeRoutingConvention.cs
@@ -148,6 +148,8 @@ protected virtual IEnumerable<SelectControllerResult> SelectController( RouteCon
                     }
                 }
 
+                items[ODataRouteConstants.Action] = action.ActionName;
+
                 yield return new SelectControllerResult( action.ControllerName, items );
             }
         }
@@ -161,18 +163,20 @@ public IEnumerable<ControllerActionDescriptor> SelectAction( RouteContext routeC
         {
             Arg.NotNull( routeContext, nameof( routeContext ) );
 
-            var services = routeContext.HttpContext.RequestServices;
+            var httpContext = routeContext.HttpContext;
+            var services = httpContext.RequestServices;
             var actionCollectionProvider = services.GetRequiredService<IActionDescriptorCollectionProvider>();
             var actionDescriptors = actionCollectionProvider.ActionDescriptors.Items.OfType<ControllerActionDescriptor>().ToArray();
+            var comparer = StringComparer.OrdinalIgnoreCase;
 
             foreach ( var controllerResult in SelectController( routeContext ) )
             {
                 var controllerName = controllerResult.ControllerName;
-                var attributeRouteData = controllerResult.Values;
+                var actionName = controllerResult.Values[ODataRouteConstants.Action].ToString();
 
                 foreach ( var action in actionDescriptors )
                 {
-                    if ( action.ControllerName == controllerName )
+                    if ( action.ControllerName == controllerName && comparer.Equals( actionName, action.ActionName ) )
                     {
                         yield return action;
                     }
diff --git a/src/Microsoft.AspNetCore.OData.Versioning/AspNetCore.Mvc/Versioning/ODataApiVersionActionSelector.cs b/src/Microsoft.AspNetCore.OData.Versioning/AspNetCore.Mvc/Versioning/ODataApiVersionActionSelector.cs
@@ -25,8 +25,6 @@
     [CLSCompliant( false )]
     public class ODataApiVersionActionSelector : ApiVersionActionSelector
     {
-        static readonly string ActionKey = ODataRouteConstants.Action;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="ODataApiVersionActionSelector"/> class.
         /// </summary>
@@ -53,7 +51,7 @@ public override IReadOnlyList<ActionDescriptor> SelectCandidates( RouteContext c
 
             var odataPath = context.HttpContext.ODataFeature().Path;
             var routeValues = context.RouteData.Values;
-            var notODataCandidate = odataPath == null || routeValues.ContainsKey( ActionKey );
+            var notODataCandidate = odataPath == null || routeValues.ContainsKey( ODataRouteConstants.Action );
 
             if ( notODataCandidate )
             {
@@ -75,14 +73,16 @@ public override IReadOnlyList<ActionDescriptor> SelectCandidates( RouteContext c
             {
                 var actions = convention.SelectAction( context );
 
-                if ( actions != null )
+                if ( actions == null )
+                {
+                    continue;
+                }
+
+                foreach ( var action in actions )
                 {
-                    foreach ( var action in actions )
+                    if ( visited.Add( action ) )
                     {
-                        if ( visited.Add( action ) )
-                        {
-                            possibleCandidates.Add( new ActionCandidate( action ) );
-                        }
+                        possibleCandidates.Add( new ActionCandidate( action ) );
                     }
                 }
             }
@@ -162,14 +162,7 @@ public override ActionDescriptor SelectBestCandidate( RouteContext context, IRea
 
             selectionResult.AddMatches( finalMatches );
 
-            var bestCandidate = RoutePolicy.Evaluate( context, selectionResult );
-
-            if ( bestCandidate is ControllerActionDescriptor controllerAction )
-            {
-                context.RouteData.Values[ActionKey] = controllerAction.ActionName;
-            }
-
-            return bestCandidate;
+            return RoutePolicy.Evaluate( context, selectionResult );
         }
 
         [DebuggerDisplay( "{Action.DisplayName,nq}" )]

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@`
`19`	`19`	`/// </content>`
`20`	`20`	`public partial class VersionedAttributeRoutingConvention : IODataRoutingConvention`
`21`	`21`	`{`
	`22`	`+ const string AttributeRouteData = nameof( AttributeRouteData );`
`22`	`23`	`static readonly DefaultODataPathHandler defaultPathHandler = new DefaultODataPathHandler();`
`23`	`24`
`24`	`25`	`/// <summary>`
`@@ -161,8 +162,8 @@ public virtual string SelectController( ODataPath odataPath, HttpRequestMessage`
`161`	`162`
`162`	`163`	`if ( action.SupportedHttpMethods.Contains( request.Method ) && template.TryMatch( odataPath, values ) )`
`163`	`164`	`{`
`164`		`- values["action"] = action.ActionName;`
`165`		`- request.Properties["AttributeRouteData"] = values;`
	`165`	`+ values[ODataRouteConstants.Action] = action.ActionName;`
	`166`	`+ request.Properties[AttributeRouteData] = values;`
`166`	`167`
`167`	`168`	`return action.ControllerDescriptor.ControllerName;`
`168`	`169`	`}`
`@@ -183,7 +184,7 @@ public virtual string SelectAction( ODataPath odataPath, HttpControllerContext c`
`183`	`184`	`var request = controllerContext.Request;`
`184`	`185`	`var properties = request.Properties;`
`185`	`186`
`186`		`- if ( !properties.TryGetValue( (string) "AttributeRouteData", out var value ) \|\| !( value is IDictionary<string, object> attributeRouteData ) )`
	`187`	`+ if ( !properties.TryGetValue( AttributeRouteData, out var value ) \|\| !( value is IDictionary<string, object> attributeRouteData ) )`
`187`	`188`	`{`
`188`	`189`	`return null;`
`189`	`190`	`}`
`@@ -203,7 +204,7 @@ public virtual string SelectAction( ODataPath odataPath, HttpControllerContext c`
`203`	`204`	`}`
`204`	`205`	`}`
`205`	`206`
`206`		`- return attributeRouteData["action"]?.ToString();`
	`207`	`+ return attributeRouteData[ODataRouteConstants.Action]?.ToString();`
`207`	`208`	`}`
`208`	`209`
`209`	`210`	`IDictionary<ODataPathTemplate, HttpActionDescriptor> BuildAttributeMappings( IEnumerable<HttpControllerDescriptor> controllers )`
Original file line number	Diff line number	Diff line change
`@@ -148,6 +148,8 @@ protected virtual IEnumerable<SelectControllerResult> SelectController( RouteCon`
`148`	`148`	`}`
`149`	`149`	`}`
`150`	`150`
	`151`	`+ items[ODataRouteConstants.Action] = action.ActionName;`
	`152`	`+`
`151`	`153`	`yield return new SelectControllerResult( action.ControllerName, items );`
`152`	`154`	`}`
`153`	`155`	`}`
`@@ -161,18 +163,20 @@ public IEnumerable<ControllerActionDescriptor> SelectAction( RouteContext routeC`
`161`	`163`	`{`
`162`	`164`	`Arg.NotNull( routeContext, nameof( routeContext ) );`
`163`	`165`
`164`		`- var services = routeContext.HttpContext.RequestServices;`
	`166`	`+ var httpContext = routeContext.HttpContext;`
	`167`	`+ var services = httpContext.RequestServices;`
`165`	`168`	`var actionCollectionProvider = services.GetRequiredService<IActionDescriptorCollectionProvider>();`
`166`	`169`	`var actionDescriptors = actionCollectionProvider.ActionDescriptors.Items.OfType<ControllerActionDescriptor>().ToArray();`
	`170`	`+ var comparer = StringComparer.OrdinalIgnoreCase;`
`167`	`171`
`168`	`172`	`foreach ( var controllerResult in SelectController( routeContext ) )`
`169`	`173`	`{`
`170`	`174`	`var controllerName = controllerResult.ControllerName;`
`171`		`- var attributeRouteData = controllerResult.Values;`
	`175`	`+ var actionName = controllerResult.Values[ODataRouteConstants.Action].ToString();`
`172`	`176`
`173`	`177`	`foreach ( var action in actionDescriptors )`
`174`	`178`	`{`
`175`		`- if ( action.ControllerName == controllerName )`
	`179`	`+ if ( action.ControllerName == controllerName && comparer.Equals( actionName, action.ActionName ) )`
`176`	`180`	`{`
`177`	`181`	`yield return action;`
`178`	`182`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,8 +25,6 @@`
`25`	`25`	`[CLSCompliant( false )]`
`26`	`26`	`public class ODataApiVersionActionSelector : ApiVersionActionSelector`
`27`	`27`	`{`
`28`		`- static readonly string ActionKey = ODataRouteConstants.Action;`
`29`		`-`
`30`	`28`	`/// <summary>`
`31`	`29`	`/// Initializes a new instance of the <see cref="ODataApiVersionActionSelector"/> class.`
`32`	`30`	`/// </summary>`
`@@ -53,7 +51,7 @@ public override IReadOnlyList<ActionDescriptor> SelectCandidates( RouteContext c`
`53`	`51`
`54`	`52`	`var odataPath = context.HttpContext.ODataFeature().Path;`
`55`	`53`	`var routeValues = context.RouteData.Values;`
`56`		`- var notODataCandidate = odataPath == null \|\| routeValues.ContainsKey( ActionKey );`
	`54`	`+ var notODataCandidate = odataPath == null \|\| routeValues.ContainsKey( ODataRouteConstants.Action );`
`57`	`55`
`58`	`56`	`if ( notODataCandidate )`
`59`	`57`	`{`
`@@ -75,14 +73,16 @@ public override IReadOnlyList<ActionDescriptor> SelectCandidates( RouteContext c`
`75`	`73`	`{`
`76`	`74`	`var actions = convention.SelectAction( context );`
`77`	`75`
`78`		`- if ( actions != null )`
	`76`	`+ if ( actions == null )`
	`77`	`+ {`
	`78`	`+ continue;`
	`79`	`+ }`
	`80`	`+`
	`81`	`+ foreach ( var action in actions )`
`79`	`82`	`{`
`80`		`- foreach ( var action in actions )`
	`83`	`+ if ( visited.Add( action ) )`
`81`	`84`	`{`
`82`		`- if ( visited.Add( action ) )`
`83`		`- {`
`84`		`- possibleCandidates.Add( new ActionCandidate( action ) );`
`85`		`- }`
	`85`	`+ possibleCandidates.Add( new ActionCandidate( action ) );`
`86`	`86`	`}`
`87`	`87`	`}`
`88`	`88`	`}`
`@@ -162,14 +162,7 @@ public override ActionDescriptor SelectBestCandidate( RouteContext context, IRea`
`162`	`162`
`163`	`163`	`selectionResult.AddMatches( finalMatches );`
`164`	`164`
`165`		`- var bestCandidate = RoutePolicy.Evaluate( context, selectionResult );`
`166`		`-`
`167`		`- if ( bestCandidate is ControllerActionDescriptor controllerAction )`
`168`		`- {`
`169`		`- context.RouteData.Values[ActionKey] = controllerAction.ActionName;`
`170`		`- }`
`171`		`-`
`172`		`- return bestCandidate;`
	`165`	`+ return RoutePolicy.Evaluate( context, selectionResult );`
`173`	`166`	`}`
`174`	`167`
`175`	`168`	`[DebuggerDisplay( "{Action.DisplayName,nq}" )]`