Corrects an issue where a direct route is matched, but the bound action binding does not correspond to the matched controller. Fixes #44 and resolves #43. (#48)

commonsensesoftware · web-flow · commit dbd24b30f113 · 2016-11-05T19:37:44.000-07:00
diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Controllers/AggregatedActionMapping.cs b/src/Microsoft.AspNet.WebApi.Versioning/Controllers/AggregatedActionMapping.cs
@@ -21,13 +21,15 @@ internal AggregatedActionMapping( IReadOnlyList<ILookup<string, HttpActionDescri
                 this.actionMappings = actionMappings;
             }
 
-            public IEnumerable<HttpActionDescriptor> this[string key] => actionMappings.Where( am => am.Contains( key ) ).SelectMany( am => am[key] );
+            public IEnumerable<HttpActionDescriptor> this[string key] =>
+                actionMappings.Where( am => am.Contains( key ) ).SelectMany( am => am[key] );
 
-            public int Count => actionMappings[0].Count;
+            public int Count => actionMappings.Aggregate( 0, ( count, mappings ) => count + mappings.Count );
 
             public bool Contains( string key ) => actionMappings.Any( am => am.Contains( key ) );
 
-            public IEnumerator<IGrouping<string, HttpActionDescriptor>> GetEnumerator() => actionMappings[0].GetEnumerator();
+            public IEnumerator<IGrouping<string, HttpActionDescriptor>> GetEnumerator() =>
+                actionMappings.SelectMany( am => am ).GetEnumerator();
 
             IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
         }
diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Controllers/ApiVersionActionSelector.cs b/src/Microsoft.AspNet.WebApi.Versioning/Controllers/ApiVersionActionSelector.cs
@@ -154,20 +154,11 @@ public virtual ILookup<string, HttpActionDescriptor> GetActionMapping( HttpContr
             Arg.NotNull( controllerDescriptor, nameof( controllerDescriptor ) );
             Contract.Ensures( Contract.Result<ILookup<string, HttpActionDescriptor>>() != null );
 
-            var internalSelector = GetInternalSelector( controllerDescriptor );
-            var actionMappings = new List<ILookup<string, HttpActionDescriptor>>();
+            var actionMappings = ( from descriptor in controllerDescriptor.AsEnumerable()
+                                   let selector = GetInternalSelector( descriptor )
+                                   select selector.GetActionMapping() ).ToArray();
 
-            actionMappings.Add( internalSelector.GetActionMapping() );
-
-            foreach ( var relatedControllerDescriptor in controllerDescriptor.GetRelatedCandidates() )
-            {
-                if ( relatedControllerDescriptor != controllerDescriptor )
-                {
-                    actionMappings.Add( GetInternalSelector( relatedControllerDescriptor ).GetActionMapping() );
-                }
-            }
-
-            return actionMappings.Count == 1 ? actionMappings[0] : new AggregatedActionMapping( actionMappings );
+            return actionMappings.Length == 1 ? actionMappings[0] : new AggregatedActionMapping( actionMappings );
         }
     }
 }
diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ApiVersionControllerAggregator.cs b/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ApiVersionControllerAggregator.cs
@@ -118,7 +118,7 @@ private static IEnumerable<HttpControllerDescriptor> EnumerateControllersInDataT
 
             var value = default( object );
 
-            if ( dataTokens.TryGetValue( "controller", out value ) )
+            if ( dataTokens.TryGetValue( RouteDataTokenKeys.Controller, out value ) )
             {
                 var controllerDescriptor = value as HttpControllerDescriptor;
 
@@ -130,7 +130,7 @@ private static IEnumerable<HttpControllerDescriptor> EnumerateControllersInDataT
                 yield break;
             }
 
-            if ( dataTokens.TryGetValue( "actions", out value ) )
+            if ( dataTokens.TryGetValue( RouteDataTokenKeys.Actions, out value ) )
             {
                 var actionDescriptors = value as HttpActionDescriptor[];
 
@@ -146,4 +146,4 @@ private static IEnumerable<HttpControllerDescriptor> EnumerateControllersInDataT
             }
         }
     }
-}
+}
diff --git a/src/Microsoft.AspNet.WebApi.Versioning/System.Web.Http/HttpControllerDescriptorExtensions.cs b/src/Microsoft.AspNet.WebApi.Versioning/System.Web.Http/HttpControllerDescriptorExtensions.cs
@@ -107,12 +107,50 @@ internal static void SetApiVersionModel( this HttpControllerDescriptor controlle
         internal static void SetConventionsApiVersionModel( this HttpControllerDescriptor controllerDescriptor, ApiVersionModel model ) =>
             controllerDescriptor.Properties.AddOrUpdate( ConventionsApiVersionInfoKey, model, ( key, currentModel ) => ( (ApiVersionModel) currentModel ).Aggregate( model ) );
 
-        internal static IEnumerable<HttpControllerDescriptor> GetRelatedCandidates( this HttpControllerDescriptor controllerDescriptor ) =>
-            (IEnumerable<HttpControllerDescriptor>) controllerDescriptor.Properties.GetOrAdd( RelatedControllerCandidatesKey, key => Enumerable.Empty<HttpControllerDescriptor>() );
-
         internal static void SetRelatedCandidates( this HttpControllerDescriptor controllerDescriptor, IEnumerable<HttpControllerDescriptor> value ) =>
             controllerDescriptor.Properties.AddOrUpdate( RelatedControllerCandidatesKey, value, ( key, oldValue ) => value );
 
+        internal static IEnumerable<HttpControllerDescriptor> AsEnumerable( this HttpControllerDescriptor controllerDescriptor )
+        {
+            IEnumerable<HttpControllerDescriptor> relatedCandidates;
+
+            if ( controllerDescriptor.Properties.TryGetValue( RelatedControllerCandidatesKey, out 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;
+                    }
+                }
+            }
+
+            var groupedControllerDescriptors = controllerDescriptor as IEnumerable<HttpControllerDescriptor>;
+
+            if ( groupedControllerDescriptors == null )
+            {
+                yield return controllerDescriptor;
+            }
+            else
+            {
+                foreach ( var groupedControllerDescriptor in groupedControllerDescriptors )
+                {
+                    yield return groupedControllerDescriptor;
+                }
+            }
+        }
+
         /// <summary>
         /// Gets a value indicating whether the controller is API version neutral.
         /// </summary>
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/AcceptanceTest.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/AcceptanceTest.cs
@@ -14,6 +14,7 @@
     using static System.Web.Http.IncludeErrorDetailPolicy;
 
     [Trait( "Kind", "Acceptance" )]
+    [Trait( "Framework", "Web API" )]
     public abstract class AcceptanceTest : IDisposable
     {
         private sealed class FilteredControllerTypeResolver : List<Type>, IHttpControllerTypeResolver
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/ByNamespaceAcceptanceTest.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/ByNamespaceAcceptanceTest.cs
@@ -2,11 +2,34 @@
 {
     using Microsoft.Web.Http.Routing;
     using System.Web.Http;
+    using System.Web.Http.Routing;
     using static System.Web.Http.RouteParameter;
 
     public abstract class ByNamespaceAcceptanceTest : AcceptanceTest
     {
-        protected ByNamespaceAcceptanceTest()
+        protected enum SetupKind
+        {
+            None,
+            HelloWorld,
+            Agreements
+        }
+
+        protected ByNamespaceAcceptanceTest(SetupKind kind)
+        {
+            switch ( kind )
+            {
+                case SetupKind.HelloWorld:
+                    ConfigureHellWorld();
+                    break;
+                case SetupKind.Agreements:
+                    ConfigureAgreements();
+                    break;
+            }
+
+            Configuration.EnsureInitialized();
+        }
+
+        private void ConfigureAgreements()
         {
             FilteredControllerTypes.Add( typeof( Controllers.V1.AgreementsController ) );
             FilteredControllerTypes.Add( typeof( Controllers.V2.AgreementsController ) );
@@ -25,7 +48,27 @@ protected ByNamespaceAcceptanceTest()
                 new { accountId = Optional },
                 new { apiVersion = new ApiVersionRouteConstraint() } );
 
-            Configuration.EnsureInitialized();
+        }
+
+        private void ConfigureHellWorld()
+        {
+            FilteredControllerTypes.Add( typeof( Controllers.V1.HelloWorldController ) );
+            FilteredControllerTypes.Add( typeof( Controllers.V2.HelloWorldController ) );
+            FilteredControllerTypes.Add( typeof( Controllers.V3.HelloWorldController ) );
+
+            var constraintResolver = new DefaultInlineConstraintResolver()
+            {
+                ConstraintMap = { ["apiVersion"] = typeof( ApiVersionRouteConstraint ) }
+            };
+
+            Configuration.MapHttpAttributeRoutes( constraintResolver );
+            Configuration.AddApiVersioning(
+                options =>
+                {
+                    options.ReportApiVersions = true;
+                    options.DefaultApiVersion = new ApiVersion( 2, 0 );
+                    options.AssumeDefaultVersionWhenUnspecified = true;
+                } );   
         }
     }
 }
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/Controllers/V1/HelloWorldController.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/Controllers/V1/HelloWorldController.cs
@@ -0,0 +1,14 @@
+﻿namespace Microsoft.Web.Http.ByNamespace.Controllers.V1
+{
+    using Microsoft.Web.Http;
+    using Models;
+    using System.Web.Http;
+
+    [ApiVersion( "1.0", Deprecated = true )]
+    [Route( "api/HelloWorld" )]
+    [Route( "api/{version:apiVersion}/HelloWorld" )]
+    public class HelloWorldController : ApiController
+    {
+        public IHttpActionResult Get() => Ok( "V1" );
+    }
+}
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/Controllers/V2/HelloWorldController.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/Controllers/V2/HelloWorldController.cs
@@ -0,0 +1,14 @@
+﻿namespace Microsoft.Web.Http.ByNamespace.Controllers.V2
+{
+    using Microsoft.Web.Http;
+    using Models;
+    using System.Web.Http;
+
+    [ApiVersion( "2.0" )]
+    [Route( "api/HelloWorld" )]
+    [Route( "api/{version:apiVersion}/HelloWorld" )]
+    public class HelloWorldController : ApiController
+    {
+        public IHttpActionResult Get() => Ok( "V2" );
+    }
+}
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/Controllers/V3/HelloWorldController.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/Controllers/V3/HelloWorldController.cs
@@ -0,0 +1,14 @@
+﻿namespace Microsoft.Web.Http.ByNamespace.Controllers.V3
+{
+    using Microsoft.Web.Http;
+    using Models;
+    using System.Web.Http;
+
+    [ApiVersion( "3.0" )]
+    [Route( "api/HelloWorld" )]
+    [Route( "api/{version:apiVersion}/HelloWorld" )]
+    public class HelloWorldController : ApiController
+    {
+        public IHttpActionResult Get() => Ok( "V3" );
+    }
+}
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/given/a_query_string_versioned_ApiController_per_namespace.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/given/a_query_string_versioned_ApiController_per_namespace.cs
@@ -3,7 +3,6 @@
     using FluentAssertions;
     using Microsoft.Web;
     using Microsoft.Web.Http.ByNamespace;
-    using System.Collections.Generic;
     using System.Linq;
     using System.Net.Http;
     using System.Threading.Tasks;
@@ -12,28 +11,24 @@
 
     public class _a_query_string_versioned_ApiController_per_namespace : ByNamespaceAcceptanceTest
     {
+        public _a_query_string_versioned_ApiController_per_namespace() : base( SetupKind.Agreements ) { }
+
         [Theory]
         [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V1.AgreementsController", "1.0" )]
         [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V2.AgreementsController", "2.0" )]
         [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V3.AgreementsController", "3.0" )]
         public async Task _get_should_return_200( string controller, string apiVersion )
         {
             // arrange
-
+            var example = new { Controller = "", ApiVersion = "", AccountId = "" };
 
             // act
             var response = await GetAsync( $"api/agreements/42?api-version={apiVersion}" ).EnsureSuccessStatusCode();
-            var content = await response.Content.ReadAsAsync<IDictionary<string, string>>();
+            var content = await response.Content.ReadAsExampleAsync( example );
 
             // assert
             response.Headers.GetValues( "api-supported-versions" ).Single().Should().Be( "1.0, 2.0, 3.0" );
-            content.ShouldBeEquivalentTo(
-                new Dictionary<string, string>()
-                {
-                    ["Controller"] = controller,
-                    ["ApiVersion"] = apiVersion,
-                    ["AccountId"] = "42"
-                } );
+            content.ShouldBeEquivalentTo( new { Controller = controller, ApiVersion = apiVersion, AccountId = "42" } );
         }
 
         [Fact]
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/given/a_url_versioned_ApiController_per_namespace.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/given/a_url_versioned_ApiController_per_namespace.cs
@@ -3,7 +3,6 @@
     using FluentAssertions;
     using Microsoft.Web;
     using Microsoft.Web.Http.ByNamespace;
-    using System.Collections.Generic;
     using System.Linq;
     using System.Net.Http;
     using System.Threading.Tasks;
@@ -12,28 +11,24 @@
 
     public class _a_url_versioned_ApiController_per_namespace : ByNamespaceAcceptanceTest
     {
+        public _a_url_versioned_ApiController_per_namespace() : base( SetupKind.Agreements ) { }
+
         [Theory]
         [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V1.AgreementsController", "1" )]
         [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V2.AgreementsController", "2" )]
         [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V3.AgreementsController", "3" )]
         public async Task _get_should_return_200( string controller, string apiVersion )
         {
             // arrange
-
+            var example = new { Controller = "", ApiVersion = "", AccountId = "" };
 
             // act
             var response = await GetAsync( $"v{apiVersion}/agreements/42" ).EnsureSuccessStatusCode();
-            var content = await response.Content.ReadAsAsync<IDictionary<string, string>>();
+            var content = await response.Content.ReadAsExampleAsync( example );
 
             // assert
             response.Headers.GetValues( "api-supported-versions" ).Single().Should().Be( "1.0, 2.0, 3.0" );
-            content.ShouldBeEquivalentTo(
-                new Dictionary<string, string>()
-                {
-                    ["Controller"] = controller,
-                    ["ApiVersion"] = apiVersion,
-                    ["AccountId"] = "42"
-                } );
+            content.ShouldBeEquivalentTo( new { Controller = controller, ApiVersion = apiVersion, AccountId = "42" } );
         }
 
         [Fact]
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/given/a_url_versioned_and_attribute-routed_ApiController_per_namespace.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/Http/ByNamespace/given/a_url_versioned_and_attribute-routed_ApiController_per_namespace.cs
@@ -0,0 +1,43 @@
+﻿namespace given
+{
+    using FluentAssertions;
+    using Microsoft.Web.Http.ByNamespace;
+    using System.Threading.Tasks;
+    using Xunit;
+
+    public class _a_url_versioned_and_attributeX2Drouted_ApiController_per_namespace : ByNamespaceAcceptanceTest
+    {
+        public _a_url_versioned_and_attributeX2Drouted_ApiController_per_namespace() : base( SetupKind.HelloWorld ) { }
+
+        [Fact]
+        public async Task _then_get_should_should_return_200_when_a_version_is_unspecified()
+        {
+            // arrange
+
+
+            // act
+            var response = await GetAsync( "api/helloworld" );
+            var content = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+
+            // assert
+            content.Should().Be( "\"V2\"" );
+        }
+
+        [Theory]
+        [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V1.HelloWorldController", "1" )]
+        [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V2.HelloWorldController", "2" )]
+        [InlineData( "Microsoft.Web.Http.ByNamespace.Controllers.V3.HelloWorldController", "3" )]
+        public async Task _get_should_return_200( string controller, string apiVersion )
+        {
+            // arrange
+            var expected = $"\"V{apiVersion}\"";
+
+            // act
+            var response = await GetAsync( $"api/{apiVersion}/helloworld" );
+            var content = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+
+            // assert
+            content.Should().Be( expected );
+        }
+    }
+}
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/AdvancedAcceptanceTest.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/AdvancedAcceptanceTest.cs
@@ -19,7 +19,6 @@ protected AdvancedAcceptanceTest()
             FilteredControllerTypes.Add( typeof( PeopleController ) );
             FilteredControllerTypes.Add( typeof( People2Controller ) );
 
-            Configuration.MapHttpAttributeRoutes();
             Configuration.AddApiVersioning(
                 options =>
                 {
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/Controllers/Orders3Controller.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/Controllers/Orders3Controller.cs
@@ -6,13 +6,10 @@
 
     [ApiVersion( "3.0" )]
     [ControllerName( "Orders" )]
-    [RoutePrefix( "api/orders" )]
     public class Orders3Controller : ApiController
     {
-        [Route]
         public IHttpActionResult Get() => Ok( new[] { new Order() { Id = 1, Customer = $"Customer v{Request.GetRequestedApiVersion()}" } } );
 
-        [Route( "{key}" )]
         public IHttpActionResult Get( int key ) => Ok( new Order() { Id = key, Customer = $"Customer v{Request.GetRequestedApiVersion()}" } );
     }
 }
diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/ODataAcceptanceTest.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/ODataAcceptanceTest.cs
diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/AcceptanceTest.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/AcceptanceTest.cs
diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Conventions/given/a_query_string_versioned_Controller_split_into_two_types_using_conventions.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Conventions/given/a_query_string_versioned_Controller_split_into_two_types_using_conventions.cs

Original file line number	Diff line number	Diff line change
`@@ -21,13 +21,15 @@ internal AggregatedActionMapping( IReadOnlyList<ILookup<string, HttpActionDescri`
`21`	`21`	`this.actionMappings = actionMappings;`
`22`	`22`	`}`
`23`	`23`
`24`		`- public IEnumerable<HttpActionDescriptor> this[string key] => actionMappings.Where( am => am.Contains( key ) ).SelectMany( am => am[key] );`
	`24`	`+ public IEnumerable<HttpActionDescriptor> this[string key] =>`
	`25`	`+ actionMappings.Where( am => am.Contains( key ) ).SelectMany( am => am[key] );`
`25`	`26`
`26`		`- public int Count => actionMappings[0].Count;`
	`27`	`+ public int Count => actionMappings.Aggregate( 0, ( count, mappings ) => count + mappings.Count );`
`27`	`28`
`28`	`29`	`public bool Contains( string key ) => actionMappings.Any( am => am.Contains( key ) );`
`29`	`30`
`30`		`- public IEnumerator<IGrouping<string, HttpActionDescriptor>> GetEnumerator() => actionMappings[0].GetEnumerator();`
	`31`	`+ public IEnumerator<IGrouping<string, HttpActionDescriptor>> GetEnumerator() =>`
	`32`	`+ actionMappings.SelectMany( am => am ).GetEnumerator();`
`31`	`33`
`32`	`34`	`IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();`
`33`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,7 @@ private static IEnumerable<HttpControllerDescriptor> EnumerateControllersInDataT`
`118`	`118`
`119`	`119`	`var value = default( object );`
`120`	`120`
`121`		`- if ( dataTokens.TryGetValue( "controller", out value ) )`
	`121`	`+ if ( dataTokens.TryGetValue( RouteDataTokenKeys.Controller, out value ) )`
`122`	`122`	`{`
`123`	`123`	`var controllerDescriptor = value as HttpControllerDescriptor;`
`124`	`124`
`@@ -130,7 +130,7 @@ private static IEnumerable<HttpControllerDescriptor> EnumerateControllersInDataT`
`130`	`130`	`yield break;`
`131`	`131`	`}`
`132`	`132`
`133`		`- if ( dataTokens.TryGetValue( "actions", out value ) )`
	`133`	`+ if ( dataTokens.TryGetValue( RouteDataTokenKeys.Actions, out value ) )`
`134`	`134`	`{`
`135`	`135`	`var actionDescriptors = value as HttpActionDescriptor[];`
`136`	`136`
`@@ -146,4 +146,4 @@ private static IEnumerable<HttpControllerDescriptor> EnumerateControllersInDataT`
`146`	`146`	`}`
`147`	`147`	`}`
`148`	`148`	`}`
`149`		`-}`
	`149`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,7 @@`
`14`	`14`	`using static System.Web.Http.IncludeErrorDetailPolicy;`
`15`	`15`
`16`	`16`	`[Trait( "Kind", "Acceptance" )]`
	`17`	`+ [Trait( "Framework", "Web API" )]`
`17`	`18`	`public abstract class AcceptanceTest : IDisposable`
`18`	`19`	`{`
`19`	`20`	`private sealed class FilteredControllerTypeResolver : List<Type>, IHttpControllerTypeResolver`