Refactors and pre-aggregates API version models in Web API. Fixes #322

Author: Chris Martinez

src/Common/Versioning/Conventions/ActionApiVersionConventionBuilderBase.cs

@@ -12,8 +12,6 @@ namespace Microsoft.AspNetCore.Mvc.Versioning.Conventions
     /// </summary>
     public partial class ActionApiVersionConventionBuilderBase
     {
-        readonly HashSet<ApiVersion> mappedVersions = new HashSet<ApiVersion>();
-
         /// <summary>
         /// Initializes a new instance of the <see cref="ActionApiVersionConventionBuilderBase"/> class.
         /// </summary>
@@ -23,6 +21,6 @@ protected ActionApiVersionConventionBuilderBase() { }
         /// Gets the collection of API versions mapped to the current action.
         /// </summary>
         /// <value>A <see cref="ICollection{T}">collection</see> of mapped <see cref="ApiVersion">API versions</see>.</value>
-        protected ICollection<ApiVersion> MappedVersions => mappedVersions;
+        protected ICollection<ApiVersion> MappedVersions { get; } = new HashSet<ApiVersion>();
     }
 }

src/Microsoft.AspNet.OData.Versioning.ApiExplorer/System.Web.Http/Controllers/HttpControllerDescriptorExtensions.cs

@@ -5,35 +5,8 @@
 
     static class HttpControllerDescriptorExtensions
     {
-        const string RelatedControllerCandidatesKey = "MS_RelatedControllerCandidates";
-
         internal static IEnumerable<HttpControllerDescriptor> AsEnumerable( this HttpControllerDescriptor controllerDescriptor )
         {
-            if ( controllerDescriptor.Properties.TryGetValue( RelatedControllerCandidatesKey, out object value ) )
-            {
-                if ( value is IEnumerable<HttpControllerDescriptor> relatedCandidates )
-                {
-                    using ( var relatedControllerDescriptors = relatedCandidates.GetEnumerator() )
-                    {
-                        if ( relatedControllerDescriptors.MoveNext() )
-                        {
-                            yield return controllerDescriptor;
-
-                            do
-                            {
-                                if ( relatedControllerDescriptors.Current != controllerDescriptor )
-                                {
-                                    yield return relatedControllerDescriptors.Current;
-                                }
-                            }
-                            while ( relatedControllerDescriptors.MoveNext() );
-
-                            yield break;
-                        }
-                    }
-                }
-            }
-
             if ( controllerDescriptor is IEnumerable<HttpControllerDescriptor> groupedControllerDescriptors )
             {
                 foreach ( var groupedControllerDescriptor in groupedControllerDescriptors )

src/Microsoft.AspNet.OData.Versioning.ApiExplorer/System.Web.Http/HttpRouteCollectionExtensions.cs

@@ -1,10 +1,7 @@
 namespace System.Web.Http
 {
-    using System.Collections.Generic;
     using System.Diagnostics.Contracts;
-    using System.Reflection;
     using System.Web.Http.Routing;
-    using static System.Reflection.BindingFlags;
 
     static class HttpRouteCollectionExtensions
     {
@@ -13,9 +10,7 @@ internal static string GetRouteName( this HttpRouteCollection routes, IHttpRoute
             Contract.Requires( routes != null );
             Contract.Requires( route != null );
 
-            var items = CopyRouteEntries( routes );
-
-            foreach ( var item in items )
+            foreach ( var item in routes.ToDictionary() )
             {
                 if ( Equals( item.Value, route ) )
                 {
@@ -25,61 +20,5 @@ internal static string GetRouteName( this HttpRouteCollection routes, IHttpRoute
 
             return null;
         }
-
-        static KeyValuePair<string, IHttpRoute>[] CopyRouteEntries( HttpRouteCollection routes )
-        {
-            Contract.Requires( routes != null );
-            Contract.Ensures( Contract.Result<KeyValuePair<string, IHttpRoute>[]>() != null );
-
-            var items = new KeyValuePair<string, IHttpRoute>[routes.Count];
-
-            try
-            {
-                routes.CopyTo( items, 0 );
-            }
-            catch ( NotSupportedException ) when ( routes.GetType().FullName == "System.Web.Http.WebHost.Routing.HostedHttpRouteCollection" )
-            {
-                var keys = GetRouteKeys( routes );
-
-                for ( var i = 0; i < keys.Count; i++ )
-                {
-                    var key = keys[i];
-                    var route = routes[key];
-
-                    items[i] = new KeyValuePair<string, IHttpRoute>( key, route );
-                }
-            }
-
-            return items;
-        }
-
-        static IReadOnlyList<string> GetRouteKeys( HttpRouteCollection routes )
-        {
-            Contract.Requires( routes != null );
-            Contract.Ensures( Contract.Result<IReadOnlyList<string>>() != null );
-
-            var collection = GetKeys( routes );
-            var keys = new string[collection.Count];
-
-            collection.CopyTo( keys, 0 );
-
-            return keys;
-        }
-
-        static ICollection<string> GetKeys( HttpRouteCollection routes )
-        {
-            Contract.Requires( routes != null );
-            Contract.Ensures( Contract.Result<ICollection<string>>() != null );
-
-            // HACK: System.Web.Routing.RouteCollection doesn't expose the names associated with registered routes. The
-            // HostedHttpRouteCollection could have provided an adapter to support it, but didn't. Instead, it always throws
-            // NotSupportedException for the HttpRouteCollection.CopyTo method. This only happens when hosted on IIS. The
-            // only way to get the keys is use reflection to poke at the underlying dictionary.
-            var routeCollection = routes.GetType().GetField( "_routeCollection", Instance | NonPublic ).GetValue( routes );
-            var dictionary = routeCollection.GetType().GetField( "_namedMap", Instance | NonPublic ).GetValue( routeCollection );
-            var keys = (ICollection<string>) dictionary.GetType().GetRuntimeProperty( "Keys" ).GetValue( dictionary, null );
-
-            return keys;
-        }
     }
 }

src/Microsoft.AspNet.OData.Versioning/Builder/VersionedODataModelBuilder.cs

@@ -3,8 +3,8 @@
     using Microsoft.Web.Http;
     using Microsoft.Web.Http.Versioning;
     using Microsoft.Web.Http.Versioning.Conventions;
+    using System;
     using System.Collections.Generic;
-    using System.Diagnostics.CodeAnalysis;
     using System.Diagnostics.Contracts;
     using System.Linq;
     using System.Web.Http;
@@ -23,7 +23,6 @@ public partial class VersionedODataModelBuilder
         /// <remarks>This constructor resolves the current <see cref="IHttpControllerSelector"/> from the
         /// <see cref="ServicesExtensions.GetHttpControllerSelector(ServicesContainer)"/> extension method via the
         /// <see cref="HttpConfiguration.Services"/>.</remarks>
-        [SuppressMessage( "Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "0", Justification = "Validated by a code contract." )]
         public VersionedODataModelBuilder( HttpConfiguration configuration )
         {
             Arg.NotNull( configuration, nameof( configuration ) );
@@ -55,15 +54,18 @@ protected virtual IReadOnlyList<ApiVersion> GetApiVersions()
             var assembliesResolver = services.GetAssembliesResolver();
             var typeResolver = services.GetHttpControllerTypeResolver();
             var controllerTypes = typeResolver.GetControllerTypes( assembliesResolver ).Where( TypeExtensions.IsODataController );
-            var conventions = Options.Conventions;
+            var controllerDescriptors = services.GetHttpControllerSelector().GetControllerMapping().Values;
             var supported = new HashSet<ApiVersion>();
             var deprecated = new HashSet<ApiVersion>();
 
             foreach ( var controllerType in controllerTypes )
             {
-                var descriptor = new HttpControllerDescriptor( Configuration, string.Empty, controllerType );
+                var descriptor = FindControllerDescriptor( controllerDescriptors, controllerType );
 
-                conventions.ApplyTo( descriptor );
+                if ( descriptor == null )
+                {
+                    continue;
+                }
 
                 var model = descriptor.GetApiVersionModel();
                 var versions = model.SupportedApiVersions;
@@ -116,5 +118,31 @@ protected virtual void ConfigureMetadataController( IEnumerable<ApiVersion> supp
 
             controllerBuilder.ApplyTo( controllerDescriptor );
         }
+
+        static HttpControllerDescriptor FindControllerDescriptor( IEnumerable<HttpControllerDescriptor> controllerDescriptors, Type controllerType )
+        {
+            Contract.Requires( controllerDescriptors != null );
+            Contract.Requires( controllerType != null );
+
+            foreach ( var controllerDescriptor in controllerDescriptors )
+            {
+                if ( controllerDescriptor is IEnumerable<HttpControllerDescriptor> groupedControllerDescriptors )
+                {
+                    foreach ( var groupedControllerDescriptor in groupedControllerDescriptors )
+                    {
+                        if ( controllerType.Equals( groupedControllerDescriptor.ControllerType ) )
+                        {
+                            return groupedControllerDescriptor;
+                        }
+                    }
+                }
+                else if ( controllerType.Equals( controllerDescriptor.ControllerType ) )
+                {
+                    return controllerDescriptor;
+                }
+            }
+
+            return default;
+        }
     }
 }

src/Microsoft.AspNet.OData.Versioning/System.Web.Http/HttpConfigurationExtensions.cs

@@ -610,12 +610,9 @@ public static IEdmModel GetEdmModel( this HttpConfiguration configuration, ApiVe
             Arg.NotNull( configuration, nameof( configuration ) );
             Arg.NotNull( apiVersion, nameof( apiVersion ) );
 
-            var allRoutes = configuration.Routes;
-            var routes = new KeyValuePair<string, IHttpRoute>[allRoutes.Count];
+            var routes = configuration.Routes.ToDictionary();
             var containers = configuration.GetRootContainerMappings();
 
-            allRoutes.CopyTo( routes, 0 );
-
             foreach ( var route in routes )
             {
                 if ( !( route.Value is ODataRoute odataRoute ) )

src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/Description/VersionedApiExplorer.cs

@@ -218,7 +218,7 @@ protected virtual ApiDescriptionGroupCollection InitializeApiDescriptions()
 
             var routes = FlattenRoutes( Configuration.Routes ).ToArray();
 
-            foreach ( var apiVersion in FlattenApiVersions() )
+            foreach ( var apiVersion in FlattenApiVersions( controllerMappings ) )
             {
                 foreach ( var route in routes )
                 {
@@ -441,15 +441,16 @@ static IEnumerable<IHttpRoute> FlattenRoutes( IEnumerable<IHttpRoute> routes )
             }
         }
 
-        IEnumerable<ApiVersion> FlattenApiVersions()
+        IEnumerable<ApiVersion> FlattenApiVersions( IDictionary<string, HttpControllerDescriptor> controllerMapping )
         {
+            Contract.Requires( controllerMapping != null );
             Contract.Ensures( Contract.Result<IEnumerable<ApiVersion>>() != null );
 
             var services = Configuration.Services;
             var assembliesResolver = services.GetAssembliesResolver();
             var typeResolver = services.GetHttpControllerTypeResolver();
             var controllerTypes = typeResolver.GetControllerTypes( assembliesResolver );
-            var options = Configuration.GetApiVersioningOptions();
+            var controllerDescriptors = controllerMapping.Values;
             var declared = new HashSet<ApiVersion>();
             var supported = new HashSet<ApiVersion>();
             var deprecated = new HashSet<ApiVersion>();
@@ -458,13 +459,12 @@ IEnumerable<ApiVersion> FlattenApiVersions()
 
             foreach ( var controllerType in controllerTypes )
             {
-                var descriptor = new HttpControllerDescriptor()
-                {
-                    Configuration = Configuration,
-                    ControllerType = controllerType,
-                };
+                var descriptor = FindControllerDescriptor( controllerDescriptors, controllerType );
 
-                options.Conventions.ApplyTo( descriptor );
+                if ( descriptor == null )
+                {
+                    continue;
+                }
 
                 var model = descriptor.GetApiVersionModel();
 
@@ -494,13 +494,39 @@ IEnumerable<ApiVersion> FlattenApiVersions()
 
             if ( supported.Count == 0 )
             {
-                supported.Add( options.DefaultApiVersion );
+                supported.Add( Configuration.GetApiVersioningOptions().DefaultApiVersion );
                 return supported;
             }
 
             return supported.OrderBy( v => v );
         }
 
+        static HttpControllerDescriptor FindControllerDescriptor( IEnumerable<HttpControllerDescriptor> controllerDescriptors, Type controllerType )
+        {
+            Contract.Requires( controllerDescriptors != null );
+            Contract.Requires( controllerType != null );
+
+            foreach ( var controllerDescriptor in controllerDescriptors )
+            {
+                if ( controllerDescriptor is IEnumerable<HttpControllerDescriptor> groupedControllerDescriptors )
+                {
+                    foreach ( var groupedControllerDescriptor in groupedControllerDescriptors )
+                    {
+                        if ( controllerType.Equals( groupedControllerDescriptor.ControllerType ) )
+                        {
+                            return groupedControllerDescriptor;
+                        }
+                    }
+                }
+                else if ( controllerType.Equals( controllerDescriptor.ControllerType ) )
+                {
+                    return controllerDescriptor;
+                }
+            }
+
+            return default;
+        }
+
         static HttpControllerDescriptor GetDirectRouteController( CandidateAction[] directRouteCandidates, ApiVersion apiVersion )
         {
             Contract.Requires( apiVersion != null );
@@ -1050,9 +1076,7 @@ static bool MatchRegexConstraint( IHttpRoute route, string parameterName, string
             }
 
             // note that we don't support custom constraint (IHttpRouteConstraint) because it might rely on the request and some runtime states
-            var constraintsRule = constraint as string;
-
-            if ( constraintsRule == null )
+            if ( !( constraint is string constraintsRule ) )
             {
                 return true;
             }