Ensure implicitly versioned ActionModel has associated ControllerModel. Fixes #519

Chris Martinez · commonsensesoftware · commit 31d08faae4f5 · 2019-07-18T09:55:19.000-07:00
diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersioningApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersioningApplicationModelProvider.cs
@@ -59,8 +59,10 @@ public virtual void OnProvidersExecuted( ApplicationModelProviderContext context
                 controllers = ControllerFilter.Apply( controllers );
             }
 
-            foreach ( var controller in controllers )
+            for ( var i = 0; i < controllers.Count; i++ )
             {
+                var controller = controllers[i];
+
                 if ( !conventionBuilder.ApplyTo( controller ) )
                 {
                     ApplyAttributeOrImplicitConventions( controller, implicitVersionModel );
@@ -75,8 +77,10 @@ static bool IsDecoratedWithAttributes( ControllerModel controller )
         {
             Contract.Requires( controller != null );
 
-            foreach ( var attribute in controller.Attributes )
+            for ( var i = 0; i < controller.Attributes.Count; i++ )
             {
+                var attribute = controller.Attributes[i];
+
                 if ( attribute is IApiVersionProvider || attribute is IApiVersionNeutral )
                 {
                     return true;
@@ -91,8 +95,10 @@ static void ApplyImplicitConventions( ControllerModel controller, ApiVersionMode
             Contract.Requires( controller != null );
             Contract.Requires( implicitVersionModel != null );
 
-            foreach ( var action in controller.Actions )
+            for ( var i = 0; i < controller.Actions.Count; i++ )
             {
+                var action = controller.Actions[i];
+                action.SetProperty( controller );
                 action.SetProperty( implicitVersionModel );
             }
         }
diff --git a/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/ODataApiDescriptionProvider.cs b/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/ODataApiDescriptionProvider.cs
@@ -8,12 +8,10 @@
     using Microsoft.AspNetCore.Mvc.Abstractions;
     using Microsoft.AspNetCore.Mvc.ActionConstraints;
     using Microsoft.AspNetCore.Mvc.ApplicationModels;
-    using Microsoft.AspNetCore.Mvc.ApplicationParts;
     using Microsoft.AspNetCore.Mvc.Controllers;
     using Microsoft.AspNetCore.Mvc.Formatters;
     using Microsoft.AspNetCore.Mvc.Internal;
     using Microsoft.AspNetCore.Mvc.ModelBinding;
-    using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
     using Microsoft.AspNetCore.Mvc.Routing;
     using Microsoft.AspNetCore.Routing;
     using Microsoft.AspNetCore.Routing.Template;
@@ -26,7 +24,6 @@
     using System.Diagnostics.Contracts;
     using System.Globalization;
     using System.Linq;
-    using System.Reflection;
     using System.Threading.Tasks;
     using static Microsoft.AspNet.OData.Routing.ODataRouteActionType;
     using static Microsoft.AspNetCore.Http.StatusCodes;
@@ -132,9 +129,9 @@ public ODataApiDescriptionProvider(
         protected MvcOptions MvcOptions { get; }
 
         /// <summary>
-        /// Gets the order prescendence of the current API description provider.
+        /// Gets the order precedence of the current API description provider.
         /// </summary>
-        /// <value>The order prescendence of the current API description provider. The default value is -100.</value>
+        /// <value>The order precedence of the current API description provider. The default value is -100.</value>
         public virtual int Order => AfterApiVersioning;
 
         /// <summary>
@@ -144,10 +141,8 @@ public ODataApiDescriptionProvider(
         /// <remarks>The default implementation performs no action.</remarks>
         public virtual void OnProvidersExecuted( ApiDescriptionProviderContext context )
         {
-            var ignoreApiExplorerSettings = !Options.UseApiExplorerSettings;
             var mappings = RouteCollectionProvider.Items;
             var results = context.Results;
-
             var groupNameFormat = Options.GroupNameFormat;
             var formatProvider = CultureInfo.CurrentCulture;
 
@@ -164,8 +159,9 @@ public virtual void OnProvidersExecuted( ApiDescriptionProviderContext context )
 
                 if ( model.IsApiVersionNeutral )
                 {
-                    foreach ( var mapping in mappings )
+                    for ( var i = 0; i < mappings.Count; i++ )
                     {
+                        var mapping = mappings[i];
                         var descriptions = new List<ApiDescription>();
                         var groupName = mapping.ApiVersion.ToString( groupNameFormat, formatProvider );
 
@@ -183,17 +179,19 @@ public virtual void OnProvidersExecuted( ApiDescriptionProviderContext context )
                 }
                 else
                 {
-                    foreach ( var apiVersion in model.DeclaredApiVersions )
+                    for ( var i = 0; i < model.DeclaredApiVersions.Count; i++ )
                     {
+                        var apiVersion = model.DeclaredApiVersions[i];
                         var groupName = apiVersion.ToString( groupNameFormat, formatProvider );
 
                         if ( !mappings.TryGetValue( apiVersion, out var mappingsPerApiVersion ) )
                         {
                             continue;
                         }
 
-                        foreach ( var mapping in mappingsPerApiVersion )
+                        for ( var j = 0; j < mappingsPerApiVersion.Count; j++ )
                         {
+                            var mapping = mappingsPerApiVersion[j];
                             var descriptions = new List<ApiDescription>();
 
                             foreach ( var apiDescription in NewODataApiDescriptions( action, groupName, mapping ) )
@@ -364,7 +362,7 @@ static string BuildRelativePath( ControllerActionDescriptor action, ODataRouteBu
 
             var relativePath = action.AttributeRouteInfo?.Template;
 
-            // note: if path happens to be built adhead of time, it's expected to be qualified; rebuild it as necessary
+            // note: if path happens to be built ahead of time, it's expected to be qualified; rebuild it as necessary
             if ( string.IsNullOrEmpty( relativePath ) || !routeContext.Options.UseQualifiedNames )
             {
                 var builder = new ODataRouteBuilder( routeContext );
@@ -422,14 +420,15 @@ IEnumerable<ApiDescription> NewODataApiDescriptions( ControllerActionDescriptor
                     apiDescription.Properties[typeof( IEdmOperation )] = routeContext.Operation;
                 }
 
-                foreach ( var parameter in parameters )
+                for ( var i = 0; i < parameters.Count; i++ )
                 {
+                    var parameter = parameters[i];
                     apiDescription.ParameterDescriptions.Add( parameter );
                 }
 
-                foreach ( var apiResponseType in apiResponseTypes )
+                for ( var i = 0; i < apiResponseTypes.Count; i++ )
                 {
-                    apiDescription.SupportedResponseTypes.Add( apiResponseType );
+                    apiDescription.SupportedResponseTypes.Add( apiResponseTypes[i] );
                 }
 
                 PopulateApiVersionParameters( apiDescription, mapping.ApiVersion );
@@ -445,8 +444,9 @@ IList<ApiParameterDescription> GetParameters( ApiParameterContext context )
 
             if ( action.Parameters != null )
             {
-                foreach ( var actionParameter in action.Parameters )
+                for ( var i = 0; i < action.Parameters.Count; i++ )
                 {
+                    var actionParameter = action.Parameters[i];
                     var metadata = MetadataProvider.GetMetadataForType( actionParameter.ParameterType );
 
                     UpdateBindingInfo( context, actionParameter, metadata );
@@ -460,8 +460,9 @@ IList<ApiParameterDescription> GetParameters( ApiParameterContext context )
 
             if ( action.BoundProperties != null )
             {
-                foreach ( var actionParameter in action.BoundProperties )
+                for ( var i = 0; i < action.BoundProperties.Count; i++ )
                 {
+                    var actionParameter = action.BoundProperties[i];
                     var visitor = new PseudoModelBindingVisitor( context, actionParameter );
                     var modelMetadata = context.MetadataProvider.GetMetadataForProperty(
                         containerType: action.ControllerTypeInfo.AsType(),
diff --git a/src/Microsoft.AspNetCore.OData.Versioning/AspNetCore.Mvc/Versioning/ApiExplorerModelConvention.cs b/src/Microsoft.AspNetCore.OData.Versioning/AspNetCore.Mvc/Versioning/ApiExplorerModelConvention.cs
@@ -2,30 +2,53 @@
 {
     using Microsoft.AspNet.OData;
     using Microsoft.AspNetCore.Mvc.ApplicationModels;
+    using System.Linq;
 
     sealed class ApiExplorerModelConvention : IApplicationModelConvention
     {
         public void Apply( ApplicationModel application )
         {
-            foreach ( var controller in application.Controllers )
+            for ( var i = 0; i < application.Controllers.Count; i++ )
             {
+                var controller = application.Controllers[i];
+
                 if ( !controller.ControllerType.IsODataController() )
                 {
                     continue;
                 }
 
-                if ( controller.ApiExplorer != null )
+                if ( controller.ApiExplorer == null )
+                {
+                    var model = new ApiExplorerModel();
+                    var attribute = controller.Attributes.OfType<ApiExplorerSettingsAttribute>().FirstOrDefault();
+
+                    if ( attribute != null )
+                    {
+                        model.GroupName = attribute.GroupName;
+                        model.IsVisible = !attribute.IgnoreApi;
+                    }
+
+                    controller.ApiExplorer = new ODataApiExplorerModel( model );
+                }
+                else if ( !( controller.ApiExplorer is ODataApiExplorerModel ) )
                 {
                     controller.ApiExplorer = new ODataApiExplorerModel( controller.ApiExplorer );
                 }
 
-                foreach ( var action in controller.Actions )
+                for ( var j = 0; j < controller.Actions.Count; j++ )
                 {
-                    if ( action.ApiExplorer != null )
+                    var action = controller.Actions[j];
+
+                    if ( action.ApiExplorer == null )
+                    {
+                        action.ApiExplorer = controller.ApiExplorer;
+                    }
+                    else if ( !( action.ApiExplorer is ODataApiExplorerModel ) )
                     {
                         action.ApiExplorer = new ODataApiExplorerModel( action.ApiExplorer );
-                        action.SetProperty( action.ApiExplorer );
                     }
+
+                    action.SetProperty( action.ApiExplorer );
                 }
             }
         }

Original file line number	Diff line number	Diff line change
`@@ -59,8 +59,10 @@ public virtual void OnProvidersExecuted( ApplicationModelProviderContext context`
`59`	`59`	`controllers = ControllerFilter.Apply( controllers );`
`60`	`60`	`}`
`61`	`61`
`62`		`- foreach ( var controller in controllers )`
	`62`	`+ for ( var i = 0; i < controllers.Count; i++ )`
`63`	`63`	`{`
	`64`	`+ var controller = controllers[i];`
	`65`	`+`
`64`	`66`	`if ( !conventionBuilder.ApplyTo( controller ) )`
`65`	`67`	`{`
`66`	`68`	`ApplyAttributeOrImplicitConventions( controller, implicitVersionModel );`
`@@ -75,8 +77,10 @@ static bool IsDecoratedWithAttributes( ControllerModel controller )`
`75`	`77`	`{`
`76`	`78`	`Contract.Requires( controller != null );`
`77`	`79`
`78`		`- foreach ( var attribute in controller.Attributes )`
	`80`	`+ for ( var i = 0; i < controller.Attributes.Count; i++ )`
`79`	`81`	`{`
	`82`	`+ var attribute = controller.Attributes[i];`
	`83`	`+`
`80`	`84`	`if ( attribute is IApiVersionProvider \|\| attribute is IApiVersionNeutral )`
`81`	`85`	`{`
`82`	`86`	`return true;`
`@@ -91,8 +95,10 @@ static void ApplyImplicitConventions( ControllerModel controller, ApiVersionMode`
`91`	`95`	`Contract.Requires( controller != null );`
`92`	`96`	`Contract.Requires( implicitVersionModel != null );`
`93`	`97`
`94`		`- foreach ( var action in controller.Actions )`
	`98`	`+ for ( var i = 0; i < controller.Actions.Count; i++ )`
`95`	`99`	`{`
	`100`	`+ var action = controller.Actions[i];`
	`101`	`+ action.SetProperty( controller );`
`96`	`102`	`action.SetProperty( implicitVersionModel );`
`97`	`103`	`}`
`98`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,30 +2,53 @@`
`2`	`2`	`{`
`3`	`3`	`using Microsoft.AspNet.OData;`
`4`	`4`	`using Microsoft.AspNetCore.Mvc.ApplicationModels;`
	`5`	`+ using System.Linq;`
`5`	`6`
`6`	`7`	`sealed class ApiExplorerModelConvention : IApplicationModelConvention`
`7`	`8`	`{`
`8`	`9`	`public void Apply( ApplicationModel application )`
`9`	`10`	`{`
`10`		`- foreach ( var controller in application.Controllers )`
	`11`	`+ for ( var i = 0; i < application.Controllers.Count; i++ )`
`11`	`12`	`{`
	`13`	`+ var controller = application.Controllers[i];`
	`14`	`+`
`12`	`15`	`if ( !controller.ControllerType.IsODataController() )`
`13`	`16`	`{`
`14`	`17`	`continue;`
`15`	`18`	`}`
`16`	`19`
`17`		`- if ( controller.ApiExplorer != null )`
	`20`	`+ if ( controller.ApiExplorer == null )`
	`21`	`+ {`
	`22`	`+ var model = new ApiExplorerModel();`
	`23`	`+ var attribute = controller.Attributes.OfType<ApiExplorerSettingsAttribute>().FirstOrDefault();`
	`24`	`+`
	`25`	`+ if ( attribute != null )`
	`26`	`+ {`
	`27`	`+ model.GroupName = attribute.GroupName;`
	`28`	`+ model.IsVisible = !attribute.IgnoreApi;`
	`29`	`+ }`
	`30`	`+`
	`31`	`+ controller.ApiExplorer = new ODataApiExplorerModel( model );`
	`32`	`+ }`
	`33`	`+ else if ( !( controller.ApiExplorer is ODataApiExplorerModel ) )`
`18`	`34`	`{`
`19`	`35`	`controller.ApiExplorer = new ODataApiExplorerModel( controller.ApiExplorer );`
`20`	`36`	`}`
`21`	`37`
`22`		`- foreach ( var action in controller.Actions )`
	`38`	`+ for ( var j = 0; j < controller.Actions.Count; j++ )`
`23`	`39`	`{`
`24`		`- if ( action.ApiExplorer != null )`
	`40`	`+ var action = controller.Actions[j];`
	`41`	`+`
	`42`	`+ if ( action.ApiExplorer == null )`
	`43`	`+ {`
	`44`	`+ action.ApiExplorer = controller.ApiExplorer;`
	`45`	`+ }`
	`46`	`+ else if ( !( action.ApiExplorer is ODataApiExplorerModel ) )`
`25`	`47`	`{`
`26`	`48`	`action.ApiExplorer = new ODataApiExplorerModel( action.ApiExplorer );`
`27`		`- action.SetProperty( action.ApiExplorer );`
`28`	`49`	`}`
	`50`	`+`
	`51`	`+ action.SetProperty( action.ApiExplorer );`
`29`	`52`	`}`
`30`	`53`	`}`
`31`	`54`	`}`