From 3533b23da44acbd65500316a7cc7ed1871a56bd4 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Tue, 9 May 2017 13:56:09 -0700 Subject: [PATCH 1/4] Initial proof of concept --- samples/aspnetcore/BasicSample/Startup.cs | 6 +- .../ActionDescriptorExtensions.cs | 19 +++ .../HttpContextExtensions.cs | 15 ++ .../IApplicationBuilderExtensions.cs | 28 ++++ .../IServiceCollectionExtensions.cs | 3 +- .../Routing/ApiVersionRouteCandidates.cs | 25 ++++ .../Routing/ApiVersionRoutePolicy.cs | 139 ++++++++++++++++++ .../Routing/IApiVersionRoutePolicy.cs | 13 ++ .../Versioning/ApiVersionActionSelector.cs | 24 ++- 9 files changed, 261 insertions(+), 11 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IApplicationBuilderExtensions.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Routing/IApiVersionRoutePolicy.cs diff --git a/samples/aspnetcore/BasicSample/Startup.cs b/samples/aspnetcore/BasicSample/Startup.cs index 07e8b20a..916687ac 100644 --- a/samples/aspnetcore/BasicSample/Startup.cs +++ b/samples/aspnetcore/BasicSample/Startup.cs @@ -10,6 +10,8 @@ using System.Linq; using System.Threading.Tasks; + using Microsoft.AspNetCore.Mvc.Routing; + public class Startup { public Startup( IHostingEnvironment env ) @@ -27,7 +29,7 @@ public Startup( IHostingEnvironment env ) public void ConfigureServices( IServiceCollection services ) { services.AddMvc(); - + // reporting api versions will return the headers "api-supported-versions" and "api-deprecated-versions" services.AddApiVersioning( o => o.ReportApiVersions = true ); } @@ -36,7 +38,7 @@ public void Configure( IApplicationBuilder app, IHostingEnvironment env, ILogger { loggerFactory.AddConsole( Configuration.GetSection( "Logging" ) ); loggerFactory.AddDebug(); - app.UseMvc(); + app.UseMvc().UseApiVersioning(); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Abstractions/ActionDescriptorExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Abstractions/ActionDescriptorExtensions.cs index 986f375b..5b93d7a9 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Abstractions/ActionDescriptorExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Abstractions/ActionDescriptorExtensions.cs @@ -2,6 +2,7 @@ { using ApplicationModels; using System; + using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Linq; using Versioning; @@ -18,6 +19,24 @@ public static class ActionDescriptorExtensions static void HasAggregatedVersions( this ActionDescriptor action, bool value ) => action.Properties[VersionsAggregated] = value; + internal static void AggregateAllVersions( this ActionDescriptor action, IEnumerable matchingActions ) + { + Contract.Requires( action != null ); + Contract.Requires( matchingActions != null ); + + if ( action.HasAggregatedVersions() ) + { + return; + } + + action.HasAggregatedVersions( true ); + + var model = action.GetProperty(); + Contract.Assume( model != null ); + + action.SetProperty( model.Aggregate( matchingActions.Select( a => a.GetProperty() ).Where( m => m != null ) ) ); + } + internal static void AggregateAllVersions( this ActionDescriptor action, ActionSelectionContext context ) { Contract.Requires( action != null ); diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs index c788974b..080e8c6f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs @@ -1,6 +1,7 @@ namespace Microsoft.AspNetCore.Mvc { using Http; + using Microsoft.AspNetCore.Mvc.Routing; using System; using System.Diagnostics.Contracts; using Versioning; @@ -12,6 +13,20 @@ public static class HttpContextExtensions { const string ApiVersionPropertiesKey = "MS_" + nameof( ApiVersionRequestProperties ); + const string ApiVersionSelectionCriteriaKey = "MS_" + nameof( ApiVersionRouteCandidates ); + + internal static ApiVersionRouteCandidates ActionCandidates( this HttpContext context ) + { + Contract.Requires( context != null ); + Contract.Ensures( Contract.Result() != null ); + + if ( !context.Items.TryGetValue( ApiVersionSelectionCriteriaKey, out ApiVersionRouteCandidates criteria ) ) + { + context.Items[ApiVersionSelectionCriteriaKey] = criteria = new ApiVersionRouteCandidates(); + } + + return criteria; + } /// /// Gets the current API versioning request properties. diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IApplicationBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IApplicationBuilderExtensions.cs new file mode 100644 index 00000000..dfbf3695 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IApplicationBuilderExtensions.cs @@ -0,0 +1,28 @@ +namespace Microsoft.Extensions.DependencyInjection +{ + using Microsoft.AspNetCore.Builder; + using Microsoft.AspNetCore.Mvc.Routing; + using System; + using System.Diagnostics.Contracts; + + /// + /// Provides extension methods for the interface. + /// + [CLSCompliant( false )] + public static class IApplicationBuilderExtensions + { + /// + /// Adds API versioning to the request execution pipeline. + /// + /// The application builder to add API versioning to. + /// The original . + public static IApplicationBuilder UseApiVersioning( this IApplicationBuilder app ) + { + Arg.NotNull( app, nameof( app ) ); + Contract.Ensures( Contract.Result() != null ); + + app.UseMvc( builder => builder.Routes.Add( builder.ServiceProvider.GetRequiredService() ) ); + return app; + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs index 3443eadd..6e89399c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs @@ -41,6 +41,7 @@ public static IServiceCollection AddApiVersioning( this IServiceCollection servi services.Add( new ServiceDescriptor( typeof( IApiVersionSelector ), options.ApiVersionSelector ) ); services.Add( Singleton>( new OptionsWrapper( options ) ) ); services.Replace( Singleton() ); + services.TryAddSingleton(); if ( options.ReportApiVersions ) { @@ -58,7 +59,7 @@ public static IServiceCollection AddApiVersioning( this IServiceCollection servi mvcOptions.Conventions.Add( new ApiVersionConvention( options.DefaultApiVersion, options.Conventions ) ); } ); - services.AddRouting( mvcOptions => mvcOptions.ConstraintMap.Add( "apiVersion", typeof( ApiVersionRouteConstraint ) ) ); + services.AddRouting( routeOptions => routeOptions.ConstraintMap.Add( "apiVersion", typeof( ApiVersionRouteConstraint ) ) ); return services; } diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs new file mode 100644 index 00000000..cb12be92 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs @@ -0,0 +1,25 @@ +namespace Microsoft.AspNetCore.Mvc.Routing +{ + using Microsoft.AspNetCore.Mvc.Abstractions; + using System; + using System.Collections.Generic; + + public class ApiVersionRouteCandidates + { + public ICollection Actions { get; } = new HashSet(); + + /// + /// Gets the read-only list of controller actions matching the current route. + /// + /// A read-only list of actions matching the current route. + public ICollection MatchingActions { get; } = new HashSet(); + + /// + /// Gets or sets the currently requested API version. + /// + /// The currently requested API version. + /// This property may be null if the client did request an explicit version. Implementors should update this property when + /// implicit API version matching is allowed and a version has been selected. + public ApiVersion RequestedVersion { get; set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs new file mode 100644 index 00000000..03cd8fdc --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs @@ -0,0 +1,139 @@ +namespace Microsoft.AspNetCore.Mvc.Routing +{ + using Microsoft.AspNetCore.Http; + using Microsoft.AspNetCore.Mvc.Abstractions; + using Microsoft.AspNetCore.Mvc.Infrastructure; + using Microsoft.AspNetCore.Routing; + using System; + using System.Collections.Generic; + using System.Diagnostics.Contracts; + using System.Linq; + using System.Text; + using System.Threading.Tasks; + + /// + /// Represents the default API versioning route policy. + /// + [CLSCompliant( false )] + public class ApiVersionRoutePolicy : IApiVersionRoutePolicy + { + readonly IActionContextAccessor actionContextAccessor; + readonly IActionInvokerFactory actionInvokerFactory; + + /// + /// + /// + /// + public ApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory ) : this( actionInvokerFactory, null ) { } + + /// + /// Initializes a new instance of the class. + /// + /// The underlying action invoker factory. + /// The associated action context accessor. + public ApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory, IActionContextAccessor actionContextAccessor ) + { + Arg.NotNull( actionInvokerFactory, nameof( actionInvokerFactory ) ); + + this.actionContextAccessor = actionContextAccessor; + this.actionInvokerFactory = actionInvokerFactory; + } + + /// + /// Gets the virtual path given the specified context. + /// + /// The virtual path context used to retrieve the path data. + /// The virtual path data. The default implementation always returns null. + public virtual VirtualPathData GetVirtualPath( VirtualPathContext context ) => null; + + /// + /// Executes the API versioning route policy. + /// + /// The route context to evaluate against. + /// A task representing the asynchonrous operation. + public virtual Task RouteAsync( RouteContext context ) + { + var candidates = context.HttpContext.ActionCandidates(); + + switch ( candidates.MatchingActions.Count ) + { + case 0: + OnUnmatched( context, candidates ); + break; + case 1: + OnSingleMatch( context, candidates, candidates.MatchingActions.First() ); + break; + default: + OnMultipleMatches( context, candidates ); + break; + } + + return Task.FromResult( false ); + } + + protected virtual void OnSingleMatch( RouteContext context, ApiVersionRouteCandidates candidates, ActionDescriptor actionDescriptor ) + { + Arg.NotNull( context, nameof( context ) ); + Arg.NotNull( candidates, nameof( candidates ) ); + Arg.NotNull( actionDescriptor, nameof( actionDescriptor ) ); + + var handler = new DefaultActionHandler( actionInvokerFactory, actionContextAccessor, actionDescriptor ); + + actionDescriptor.AggregateAllVersions( candidates.MatchingActions ); + context.HttpContext.ApiVersionProperties().ApiVersion = candidates.RequestedVersion; + context.Handler = handler.Invoke; + } + + protected virtual void OnUnmatched( RouteContext context, ApiVersionRouteCandidates candidates ) + { + Arg.NotNull( context, nameof( context ) ); + Arg.NotNull( candidates, nameof( candidates ) ); + } + + protected virtual void OnMultipleMatches( RouteContext context, ApiVersionRouteCandidates candidates ) + { + Arg.NotNull( context, nameof( context ) ); + Arg.NotNull( candidates, nameof( candidates ) ); + } + + sealed class DefaultActionHandler + { + readonly IActionContextAccessor actionContextAccessor; + readonly IActionInvokerFactory actionInvokerFactory; + readonly ActionDescriptor actionDescriptor; + + internal DefaultActionHandler( + IActionInvokerFactory actionInvokerFactory, + IActionContextAccessor actionContextAccessor, + ActionDescriptor actionDescriptor ) + { + this.actionContextAccessor = actionContextAccessor; + this.actionInvokerFactory = actionInvokerFactory; + this.actionDescriptor = actionDescriptor; + } + + internal Task Invoke( HttpContext context ) + { + Contract.Requires( context != null ); + + var routeData = c.GetRouteData(); + var actionContext = new ActionContext( context, routeData, actionDescriptor ); + + if ( actionContextAccessor != null ) + { + actionContextAccessor.ActionContext = actionContext; + } + + var invoker = actionInvokerFactory.CreateInvoker( actionContext ); + + if ( invoker == null ) + { + throw new InvalidOperationException(); + //throw new InvalidOperationException( Resources.FormatActionInvokerFactory_CouldNotCreateInvoker( actionDescriptor.DisplayName ) ); + } + + return invoker.InvokeAsync(); + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/IApiVersionRoutePolicy.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/IApiVersionRoutePolicy.cs new file mode 100644 index 00000000..de3fd147 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/IApiVersionRoutePolicy.cs @@ -0,0 +1,13 @@ +namespace Microsoft.AspNetCore.Mvc.Routing +{ + using Microsoft.AspNetCore.Routing; + using System; + + /// + /// Defines the behavior of an API version route policy. + /// + [CLSCompliant( false )] + public interface IApiVersionRoutePolicy : IRouter + { + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs index dbad08c9..5c2c9d29 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs @@ -101,19 +101,27 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead if ( finalMatches == null || finalMatches.Count == 0 ) { - if ( ( invalidRequestHandler = IsValidRequest( selectionContext, candidates ) ) != null ) - { - context.Handler = invalidRequestHandler; - } + //if ( ( invalidRequestHandler = IsValidRequest( selectionContext, candidates ) ) != null ) + //{ + // context.Handler = invalidRequestHandler; + //} return null; } else if ( finalMatches.Count == 1 ) { - var selectedAction = finalMatches[0]; - selectedAction.AggregateAllVersions( selectionContext ); - httpContext.ApiVersionProperties().ApiVersion = selectionContext.RequestedVersion; - return selectedAction; + var actionCandidates = httpContext.ActionCandidates(); + + actionCandidates.Actions.AddRange( candidates ); + actionCandidates.MatchingActions.AddRange( finalMatches ); + actionCandidates.RequestedVersion = selectionContext.RequestedVersion; + + //var selectedAction = finalMatches[0]; + //selectedAction.AggregateAllVersions( selectionContext ); + //httpContext.ApiVersionProperties().ApiVersion = selectionContext.RequestedVersion; + //return selectedAction; + + return null; } else { From 55daa94856a2a9a77a1a981433a3df5422db2350 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Tue, 9 May 2017 14:25:43 -0700 Subject: [PATCH 2/4] Second set of changes --- .../Versioning/ApiVersionRequestProperties.cs | 7 +++++ .../HttpContextExtensions.cs | 14 ---------- .../Routing/ApiVersionRouteCandidates.cs | 25 ------------------ .../Routing/ApiVersionRoutePolicy.cs | 26 +++++++++---------- .../Versioning/ActionSelectionResult.cs | 23 ++++++++++++++++ 5 files changed, 43 insertions(+), 52 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs diff --git a/src/Common/Versioning/ApiVersionRequestProperties.cs b/src/Common/Versioning/ApiVersionRequestProperties.cs index 9fe9a2cc..9fdfca33 100644 --- a/src/Common/Versioning/ApiVersionRequestProperties.cs +++ b/src/Common/Versioning/ApiVersionRequestProperties.cs @@ -18,6 +18,7 @@ public partial class ApiVersionRequestProperties readonly Lazy rawApiVersion; bool apiVersionInitialized; ApiVersion apiVersion; + ActionSelectionResult selectionResult; /// /// Gets the raw, unparsed API version for the current request. @@ -58,5 +59,11 @@ public ApiVersion ApiVersion /// and is not meant to be directly used in your code. [EditorBrowsable( Never )] public string RouteParameterName { get; set; } + + /// + /// Gets the action selection result associated with the current request. + /// + /// The action selection result associated with the current request. + public ActionSelectionResult SelectionResult => selectionResult ?? ( selectionResult = new ActionSelectionResult() ); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs index 080e8c6f..99e43393 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs @@ -13,20 +13,6 @@ public static class HttpContextExtensions { const string ApiVersionPropertiesKey = "MS_" + nameof( ApiVersionRequestProperties ); - const string ApiVersionSelectionCriteriaKey = "MS_" + nameof( ApiVersionRouteCandidates ); - - internal static ApiVersionRouteCandidates ActionCandidates( this HttpContext context ) - { - Contract.Requires( context != null ); - Contract.Ensures( Contract.Result() != null ); - - if ( !context.Items.TryGetValue( ApiVersionSelectionCriteriaKey, out ApiVersionRouteCandidates criteria ) ) - { - context.Items[ApiVersionSelectionCriteriaKey] = criteria = new ApiVersionRouteCandidates(); - } - - return criteria; - } /// /// Gets the current API versioning request properties. diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs deleted file mode 100644 index cb12be92..00000000 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteCandidates.cs +++ /dev/null @@ -1,25 +0,0 @@ -namespace Microsoft.AspNetCore.Mvc.Routing -{ - using Microsoft.AspNetCore.Mvc.Abstractions; - using System; - using System.Collections.Generic; - - public class ApiVersionRouteCandidates - { - public ICollection Actions { get; } = new HashSet(); - - /// - /// Gets the read-only list of controller actions matching the current route. - /// - /// A read-only list of actions matching the current route. - public ICollection MatchingActions { get; } = new HashSet(); - - /// - /// Gets or sets the currently requested API version. - /// - /// The currently requested API version. - /// This property may be null if the client did request an explicit version. Implementors should update this property when - /// implicit API version matching is allowed and a version has been selected. - public ApiVersion RequestedVersion { get; set; } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs index 03cd8fdc..ce226d07 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs @@ -53,47 +53,47 @@ public ApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory, IActio /// A task representing the asynchonrous operation. public virtual Task RouteAsync( RouteContext context ) { - var candidates = context.HttpContext.ActionCandidates(); + var selectionResult = context.HttpContext.ApiVersionProperties().SelectionResult; - switch ( candidates.MatchingActions.Count ) + switch ( selectionResult.MatchingActions.Count ) { case 0: - OnUnmatched( context, candidates ); + OnUnmatched( context, selectionResult ); break; case 1: - OnSingleMatch( context, candidates, candidates.MatchingActions.First() ); + OnSingleMatch( context, selectionResult, selectionResult.MatchingActions.First() ); break; default: - OnMultipleMatches( context, candidates ); + OnMultipleMatches( context, selectionResult ); break; } return Task.FromResult( false ); } - protected virtual void OnSingleMatch( RouteContext context, ApiVersionRouteCandidates candidates, ActionDescriptor actionDescriptor ) + protected virtual void OnSingleMatch( RouteContext context, ApiVersionSelectionResult selectionResult, ActionDescriptor actionDescriptor ) { Arg.NotNull( context, nameof( context ) ); - Arg.NotNull( candidates, nameof( candidates ) ); + Arg.NotNull( selectionResult, nameof( selectionResult ) ); Arg.NotNull( actionDescriptor, nameof( actionDescriptor ) ); var handler = new DefaultActionHandler( actionInvokerFactory, actionContextAccessor, actionDescriptor ); - actionDescriptor.AggregateAllVersions( candidates.MatchingActions ); - context.HttpContext.ApiVersionProperties().ApiVersion = candidates.RequestedVersion; + actionDescriptor.AggregateAllVersions( selectionResult.MatchingActions ); + context.HttpContext.ApiVersionProperties().ApiVersion = selectionResult.RequestedVersion; context.Handler = handler.Invoke; } - protected virtual void OnUnmatched( RouteContext context, ApiVersionRouteCandidates candidates ) + protected virtual void OnUnmatched( RouteContext context, ApiVersionSelectionResult selectionResult ) { Arg.NotNull( context, nameof( context ) ); - Arg.NotNull( candidates, nameof( candidates ) ); + Arg.NotNull( selectionResult, nameof( selectionResult ) ); } - protected virtual void OnMultipleMatches( RouteContext context, ApiVersionRouteCandidates candidates ) + protected virtual void OnMultipleMatches( RouteContext context, ApiVersionSelectionResult selectionResult ) { Arg.NotNull( context, nameof( context ) ); - Arg.NotNull( candidates, nameof( candidates ) ); + Arg.NotNull( selectionResult, nameof( selectionResult ) ); } sealed class DefaultActionHandler diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs new file mode 100644 index 00000000..9b5b3d0c --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs @@ -0,0 +1,23 @@ +namespace Microsoft.AspNetCore.Mvc.Versioning +{ + using System; + using System.Collections.Generic; + + /// + /// Represents an API versioning action selection result for which a versioning policy can be applied. + /// + public class ActionSelectionResult + { + /// + /// Gets the collection of all candidate controller actions for the current route. + /// + /// A collection of actions candidate actions for the current route. + public ICollection CandidateActions { get; } = new HashSet(); + + /// + /// Gets the collection of controller actions matching the current route. + /// + /// A collection of actions matching the current route. + public ICollection MatchingActions { get; } = new HashSet(); + } +} \ No newline at end of file From 86ec57940238486cbf6473cdeb1db44960094458 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Wed, 10 May 2017 14:35:53 -0700 Subject: [PATCH 3/4] Completed action selector refactoring --- samples/aspnetcore/BasicSample/Startup.cs | 3 +- .../aspnetcore/ByNamespaceSample/Startup.cs | 1 + .../aspnetcore/ConventionsSample/Startup.cs | 1 + samples/aspnetcore/SwaggerSample/Startup.cs | 1 + .../Versioning/ApiVersionRequestProperties.cs | 7 - .../IServiceCollectionExtensions.cs | 1 + .../Routing/ApiVersionRoutePolicy.cs | 195 +++++++++++++++--- .../SR.Designer.cs | 9 + .../SR.resx | 3 + .../Versioning/ActionDescriptorMatch.cs | 99 +++++++++ .../Versioning/ActionSelectionResult.cs | 7 +- .../Versioning/ApiVersionActionSelector.cs | 140 +++---------- .../Versioning/ApiVersionRequestProperties.cs | 7 + .../Versioning/BadRequestHandler.cs | 6 +- .../Versioning/MethodNotAllowedHandler.cs | 6 +- .../Versioning/RequestHandler.cs | 11 +- .../AcceptanceTest.cs | 2 +- .../Basic/Controllers/Values2Controller.cs | 3 + .../Basic/Controllers/ValuesController.cs | 3 + ...a query string and split into two types.cs | 42 +++- .../TestApiVersionActionSelector.cs | 11 +- .../Versioning/WebServer.cs | 2 +- 22 files changed, 387 insertions(+), 173 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionDescriptorMatch.cs diff --git a/samples/aspnetcore/BasicSample/Startup.cs b/samples/aspnetcore/BasicSample/Startup.cs index 916687ac..79492c08 100644 --- a/samples/aspnetcore/BasicSample/Startup.cs +++ b/samples/aspnetcore/BasicSample/Startup.cs @@ -38,7 +38,8 @@ public void Configure( IApplicationBuilder app, IHostingEnvironment env, ILogger { loggerFactory.AddConsole( Configuration.GetSection( "Logging" ) ); loggerFactory.AddDebug(); - app.UseMvc().UseApiVersioning(); + app.UseMvc(); + app.UseApiVersioning(); } } } diff --git a/samples/aspnetcore/ByNamespaceSample/Startup.cs b/samples/aspnetcore/ByNamespaceSample/Startup.cs index 9cdba956..4898bc53 100644 --- a/samples/aspnetcore/ByNamespaceSample/Startup.cs +++ b/samples/aspnetcore/ByNamespaceSample/Startup.cs @@ -28,6 +28,7 @@ public void Configure( IApplicationBuilder app, IHostingEnvironment env, ILogger } app.UseMvc(); + app.UseApiVersioning(); } } } \ No newline at end of file diff --git a/samples/aspnetcore/ConventionsSample/Startup.cs b/samples/aspnetcore/ConventionsSample/Startup.cs index 7c1871b7..795f5f5f 100644 --- a/samples/aspnetcore/ConventionsSample/Startup.cs +++ b/samples/aspnetcore/ConventionsSample/Startup.cs @@ -54,6 +54,7 @@ public void Configure( IApplicationBuilder app, IHostingEnvironment env, ILogger loggerFactory.AddConsole( Configuration.GetSection( "Logging" ) ); loggerFactory.AddDebug(); app.UseMvc(); + app.UseApiVersioning(); } } } \ No newline at end of file diff --git a/samples/aspnetcore/SwaggerSample/Startup.cs b/samples/aspnetcore/SwaggerSample/Startup.cs index 4af1af8b..faa39413 100644 --- a/samples/aspnetcore/SwaggerSample/Startup.cs +++ b/samples/aspnetcore/SwaggerSample/Startup.cs @@ -84,6 +84,7 @@ public void Configure( IApplicationBuilder app, IHostingEnvironment env, ILogger loggerFactory.AddDebug(); app.UseMvc(); + app.UseApiVersioning(); app.UseSwagger(); app.UseSwaggerUI( options => diff --git a/src/Common/Versioning/ApiVersionRequestProperties.cs b/src/Common/Versioning/ApiVersionRequestProperties.cs index 9fdfca33..9fe9a2cc 100644 --- a/src/Common/Versioning/ApiVersionRequestProperties.cs +++ b/src/Common/Versioning/ApiVersionRequestProperties.cs @@ -18,7 +18,6 @@ public partial class ApiVersionRequestProperties readonly Lazy rawApiVersion; bool apiVersionInitialized; ApiVersion apiVersion; - ActionSelectionResult selectionResult; /// /// Gets the raw, unparsed API version for the current request. @@ -59,11 +58,5 @@ public ApiVersion ApiVersion /// and is not meant to be directly used in your code. [EditorBrowsable( Never )] public string RouteParameterName { get; set; } - - /// - /// Gets the action selection result associated with the current request. - /// - /// The action selection result associated with the current request. - public ActionSelectionResult SelectionResult => selectionResult ?? ( selectionResult = new ActionSelectionResult() ); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs index 6e89399c..5b657b78 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs @@ -39,6 +39,7 @@ public static IServiceCollection AddApiVersioning( this IServiceCollection servi setupAction( options ); services.Add( new ServiceDescriptor( typeof( IApiVersionReader ), options.ApiVersionReader ) ); services.Add( new ServiceDescriptor( typeof( IApiVersionSelector ), options.ApiVersionSelector ) ); + services.Add( new ServiceDescriptor( typeof( IErrorResponseProvider ), options.ErrorResponses ) ); services.Add( Singleton>( new OptionsWrapper( options ) ) ); services.Replace( Singleton() ); services.TryAddSingleton(); diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs index ce226d07..0f358065 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs @@ -1,15 +1,23 @@ namespace Microsoft.AspNetCore.Mvc.Routing { using Microsoft.AspNetCore.Http; + using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc.Abstractions; + using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Infrastructure; + using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Routing; + using Microsoft.Extensions.Logging; using System; using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Linq; - using System.Text; using System.Threading.Tasks; + using Versioning; + using static ApiVersion; + using static System.Environment; + using static System.String; + using static Versioning.ErrorCodes; /// /// Represents the default API versioning route policy. @@ -17,28 +25,64 @@ [CLSCompliant( false )] public class ApiVersionRoutePolicy : IApiVersionRoutePolicy { - readonly IActionContextAccessor actionContextAccessor; - readonly IActionInvokerFactory actionInvokerFactory; + static readonly Task CompletedTask = Task.FromResult( default( object ) ); /// - /// + /// Initializes a new instance of the class. /// - /// - public ApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory ) : this( actionInvokerFactory, null ) { } + /// The underlying action invoker factory. + /// The provider used to create error responses. + /// The . + public ApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory, IErrorResponseProvider errorResponseProvider, ILoggerFactory loggerFactory ) + : this( actionInvokerFactory, errorResponseProvider, loggerFactory, null ) { } /// /// Initializes a new instance of the class. /// /// The underlying action invoker factory. + /// The provider used to create error responses. + /// The . /// The associated action context accessor. - public ApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory, IActionContextAccessor actionContextAccessor ) + public ApiVersionRoutePolicy( + IActionInvokerFactory actionInvokerFactory, + IErrorResponseProvider errorResponseProvider, + ILoggerFactory loggerFactory, + IActionContextAccessor actionContextAccessor ) { Arg.NotNull( actionInvokerFactory, nameof( actionInvokerFactory ) ); + Arg.NotNull( errorResponseProvider, nameof( errorResponseProvider ) ); + Arg.NotNull( loggerFactory, nameof( loggerFactory ) ); - this.actionContextAccessor = actionContextAccessor; - this.actionInvokerFactory = actionInvokerFactory; + ErrorResponseProvider = errorResponseProvider; + ActionInvokerFactory = actionInvokerFactory; + Logger = loggerFactory.CreateLogger( GetType() ); + ActionContextAccessor = actionContextAccessor; } + /// + /// Gets the action invoker factory associated with the route policy. + /// + /// The associated . + protected IActionInvokerFactory ActionInvokerFactory { get; } + + /// + /// Gets the action context accessor associated with the route policy, if any. + /// + /// The associated or null. + protected IActionContextAccessor ActionContextAccessor { get; } + + /// + /// Gets the error response provider associated with the route policy. + /// + /// The provider used to create error responses for the route policy. + protected IErrorResponseProvider ErrorResponseProvider { get; } + + /// + /// Gets the logger associated with the route policy. + /// + /// The associated logger. + protected ILogger Logger { get; } + /// /// Gets the virtual path given the specified context. /// @@ -68,56 +112,160 @@ public virtual Task RouteAsync( RouteContext context ) break; } - return Task.FromResult( false ); + return CompletedTask; } - protected virtual void OnSingleMatch( RouteContext context, ApiVersionSelectionResult selectionResult, ActionDescriptor actionDescriptor ) + /// + /// Occurs when a single action is matched to the route policy. + /// + /// The current route context. + /// The current action selection result. + /// The matched action. + protected virtual void OnSingleMatch( RouteContext context, ActionSelectionResult selectionResult, ActionDescriptorMatch match ) { Arg.NotNull( context, nameof( context ) ); Arg.NotNull( selectionResult, nameof( selectionResult ) ); - Arg.NotNull( actionDescriptor, nameof( actionDescriptor ) ); + Arg.NotNull( match, nameof( match ) ); - var handler = new DefaultActionHandler( actionInvokerFactory, actionContextAccessor, actionDescriptor ); + var handler = new DefaultActionHandler( ActionInvokerFactory, ActionContextAccessor, selectionResult, match ); - actionDescriptor.AggregateAllVersions( selectionResult.MatchingActions ); - context.HttpContext.ApiVersionProperties().ApiVersion = selectionResult.RequestedVersion; + match.Action.AggregateAllVersions( selectionResult.CandidateActions ); context.Handler = handler.Invoke; } - protected virtual void OnUnmatched( RouteContext context, ApiVersionSelectionResult selectionResult ) + /// + /// Occurs when a no actions are matched by the route policy. + /// + /// The current route context. + /// The current action selection result. + protected virtual void OnUnmatched( RouteContext context, ActionSelectionResult selectionResult ) { Arg.NotNull( context, nameof( context ) ); Arg.NotNull( selectionResult, nameof( selectionResult ) ); + + context.Handler = ClientError( context, selectionResult ); } - protected virtual void OnMultipleMatches( RouteContext context, ApiVersionSelectionResult selectionResult ) + /// + /// Occurs when a multiple actions are matched to the route policy. + /// + /// The current route context. + /// The current action selection result. + /// The default implementation always throws an . + protected virtual void OnMultipleMatches( RouteContext context, ActionSelectionResult selectionResult ) { Arg.NotNull( context, nameof( context ) ); Arg.NotNull( selectionResult, nameof( selectionResult ) ); + + var actionNames = Join( NewLine, selectionResult.MatchingActions.Select( match => match.Action.DisplayName ) ); + + Logger.AmbiguousActions( actionNames ); + + var message = SR.ActionSelector_AmbiguousActions.FormatDefault( NewLine, actionNames ); + + throw new AmbiguousActionException( message ); + } + + RequestHandler ClientError( RouteContext context, ActionSelectionResult selectionResult ) + { + Contract.Requires( context != null ); + Contract.Requires( selectionResult != null ); + + const RequestHandler NotFound = default( RequestHandler ); + var candidates = selectionResult.CandidateActions; + + if ( candidates.Count == 0 ) + { + return NotFound; + } + + var httpContext = context.HttpContext; + var properties = httpContext.ApiVersionProperties(); + var code = default( string ); + var requestedVersion = default( string ); + var parsedVersion = properties.ApiVersion; + var actionNames = new Lazy( () => Join( NewLine, candidates.Select( a => a.DisplayName ) ) ); + var allowedMethods = new Lazy>( + () => new HashSet( candidates.SelectMany( c => c.ActionConstraints.OfType() ) + .SelectMany( ac => ac.HttpMethods ), + StringComparer.OrdinalIgnoreCase ) ); + var newRequestHandler = default( Func ); + + if ( parsedVersion == null ) + { + requestedVersion = properties.RawApiVersion; + + if ( IsNullOrEmpty( requestedVersion ) ) + { + code = ApiVersionUnspecified; + Logger.ApiVersionUnspecified( actionNames.Value ); + return new BadRequestHandler( ErrorResponseProvider, code, SR.ApiVersionUnspecified ); + } + else if ( TryParse( requestedVersion, out parsedVersion ) ) + { + code = UnsupportedApiVersion; + Logger.ApiVersionUnmatched( parsedVersion, actionNames.Value ); + + if ( allowedMethods.Value.Contains( httpContext.Request.Method ) ) + { + newRequestHandler = ( e, c, m ) => new BadRequestHandler( e, c, m ); + } + else + { + newRequestHandler = ( e, c, m ) => new MethodNotAllowedHandler( e, c, m, allowedMethods.Value.ToArray() ); + } + } + else + { + code = InvalidApiVersion; + Logger.ApiVersionInvalid( requestedVersion ); + newRequestHandler = ( e, c, m ) => new BadRequestHandler( e, c, m ); + } + } + else + { + requestedVersion = parsedVersion.ToString(); + code = UnsupportedApiVersion; + Logger.ApiVersionUnmatched( parsedVersion, actionNames.Value ); + + if ( allowedMethods.Value.Contains( httpContext.Request.Method ) ) + { + newRequestHandler = ( e, c, m ) => new BadRequestHandler( e, c, m ); + } + else + { + newRequestHandler = ( e, c, m ) => new MethodNotAllowedHandler( e, c, m, allowedMethods.Value.ToArray() ); + } + } + + var message = SR.VersionedResourceNotSupported.FormatDefault( httpContext.Request.GetDisplayUrl(), requestedVersion ); + return newRequestHandler( ErrorResponseProvider, code, message ); } sealed class DefaultActionHandler { readonly IActionContextAccessor actionContextAccessor; readonly IActionInvokerFactory actionInvokerFactory; - readonly ActionDescriptor actionDescriptor; + readonly ActionSelectionResult selectionResult; + readonly ActionDescriptorMatch match; internal DefaultActionHandler( IActionInvokerFactory actionInvokerFactory, IActionContextAccessor actionContextAccessor, - ActionDescriptor actionDescriptor ) + ActionSelectionResult selectionResult, + ActionDescriptorMatch match ) { this.actionContextAccessor = actionContextAccessor; this.actionInvokerFactory = actionInvokerFactory; - this.actionDescriptor = actionDescriptor; + this.selectionResult = selectionResult; + this.match = match; } internal Task Invoke( HttpContext context ) { Contract.Requires( context != null ); - var routeData = c.GetRouteData(); - var actionContext = new ActionContext( context, routeData, actionDescriptor ); + var actionContext = new ActionContext( context, match.RouteData, match.Action ); if ( actionContextAccessor != null ) { @@ -128,8 +276,7 @@ internal Task Invoke( HttpContext context ) if ( invoker == null ) { - throw new InvalidOperationException(); - //throw new InvalidOperationException( Resources.FormatActionInvokerFactory_CouldNotCreateInvoker( actionDescriptor.DisplayName ) ); + throw new InvalidOperationException( SR.CouldNotCreateInvoker.FormatDefault( match.Action.DisplayName ) ); } return invoker.InvokeAsync(); diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs index 65c860eb..21b27afe 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs @@ -124,6 +124,15 @@ internal static string ApiVersionUnspecified { } } + /// + /// Looks up a localized string similar to An action invoker could not be created for action '{0}'.. + /// + internal static string CouldNotCreateInvoker { + get { + return ResourceManager.GetString("CouldNotCreateInvoker", resourceCulture); + } + } + /// /// Looks up a localized string similar to The expression '{0}' must refer to a controller action method.. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx index 2b1d9f0a..fff2a720 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx @@ -139,6 +139,9 @@ An API version is required, but was not specified. + + An action invoker could not be created for action '{0}'. + The expression '{0}' must refer to a controller action method. diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionDescriptorMatch.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionDescriptorMatch.cs new file mode 100644 index 00000000..2a7d5eaf --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionDescriptorMatch.cs @@ -0,0 +1,99 @@ +namespace Microsoft.AspNetCore.Mvc.Versioning +{ + using Microsoft.AspNetCore.Mvc.Abstractions; + using Microsoft.AspNetCore.Routing; + using System; + + /// + /// Represents a matched action descriptor. + /// + [CLSCompliant( false )] + public class ActionDescriptorMatch : IEquatable + { + /// + /// Initializes a new instance of the class. + /// + /// The matched action. + /// The route data for the matched . + public ActionDescriptorMatch( ActionDescriptor action, RouteData routeData ) + { + Arg.NotNull( action, nameof( action ) ); + Arg.NotNull( routeData, nameof( routeData ) ); + + Action = action; + RouteData = routeData; + } + + /// + /// Gets the matched action. + /// + /// The matched action. + public ActionDescriptor Action { get; } + + /// + /// Gets the route data for the matched action. + /// + /// The route data for the matched action. + public RouteData RouteData { get; } + + /// + /// Determines whether the current object equals the specified object. + /// + /// The object to evaluate. + /// True if the current object equals the other object; otherwise, false. + public virtual bool Equals( ActionDescriptorMatch other ) => Equals( Action, other?.Action ); + + /// + /// Determines whether the current object equals the specified object. + /// + /// The object to evaluate. + /// True if the current object equals the other object; otherwise, false. + public override bool Equals( object obj ) => Equals( obj as ActionDescriptorMatch ); + + /// + /// Gets hash code for the current to object. + /// + /// A hash code. + public override int GetHashCode() => Action.GetHashCode(); + + /// + /// Determines whether two objects are equal. + /// + /// The first object to compare. + /// The second object to compare against. + /// True if the objects are equal; otherwise, false. + public static bool operator ==( ActionDescriptorMatch match1, ActionDescriptorMatch match2 ) + { + if ( ReferenceEquals( match1, null ) ) + { + return ReferenceEquals( match2, null ); + } + else if ( ReferenceEquals( match2, null ) ) + { + return false; + } + + return Equals( match1.Action, match2.Action ); + } + + /// + /// Determines whether two objects are not equal. + /// + /// The first object to compare. + /// The second object to compare against. + /// True if the objects are not equal; otherwise, false. + public static bool operator !=( ActionDescriptorMatch match1, ActionDescriptorMatch match2 ) + { + if ( ReferenceEquals( match1, null ) ) + { + return !ReferenceEquals( match2, null ); + } + else if ( ReferenceEquals( match2, null ) ) + { + return true; + } + + return !Equals( match1.Action, match2.Action ); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs index 9b5b3d0c..e7d0131e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs @@ -1,5 +1,6 @@ namespace Microsoft.AspNetCore.Mvc.Versioning { + using Microsoft.AspNetCore.Mvc.Abstractions; using System; using System.Collections.Generic; @@ -12,12 +13,14 @@ public class ActionSelectionResult /// Gets the collection of all candidate controller actions for the current route. /// /// A collection of actions candidate actions for the current route. + [CLSCompliant( false )] public ICollection CandidateActions { get; } = new HashSet(); /// /// Gets the collection of controller actions matching the current route. /// - /// A collection of actions matching the current route. - public ICollection MatchingActions { get; } = new HashSet(); + /// A collection of matching actions for the current route. + [CLSCompliant( false )] + public ICollection MatchingActions { get; } = new HashSet(); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs index 5c2c9d29..ae6a28a0 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs @@ -6,16 +6,12 @@ using Extensions.Logging; using Extensions.Options; using Http; - using Http.Extensions; using Infrastructure; using Internal; using System; using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Linq; - using static ApiVersion; - using static System.Environment; - using static System.String; using static ErrorCodes; /// @@ -28,7 +24,6 @@ public class ApiVersionActionSelector : IActionSelector readonly IActionSelectorDecisionTreeProvider decisionTreeProvider; readonly ActionConstraintCache actionConstraintCache; readonly IOptions options; - readonly ILogger logger; /// /// Initializes a new instance of the class. @@ -43,10 +38,15 @@ public ApiVersionActionSelector( IOptions options, ILoggerFactory loggerFactory ) { + Arg.NotNull( decisionTreeProvider, nameof( decisionTreeProvider ) ); + Arg.NotNull( actionConstraintCache, nameof( actionConstraintCache ) ); + Arg.NotNull( options, nameof( options ) ); + Arg.NotNull( loggerFactory, nameof( loggerFactory ) ); + this.decisionTreeProvider = decisionTreeProvider; this.actionConstraintCache = actionConstraintCache; this.options = options; - logger = loggerFactory.CreateLogger(); + Logger = loggerFactory.CreateLogger( GetType() ); } /// @@ -62,6 +62,12 @@ public ApiVersionActionSelector( /// API version when a client does not specify a version. protected IApiVersionSelector ApiVersionSelector => Options.ApiVersionSelector; + /// + /// Gets the logger associated with the action selector. + /// + /// The associated logger. + protected ILogger Logger { get; } + /// /// Selects a list of candidate actions from the specified route context. /// @@ -87,52 +93,31 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead Arg.NotNull( candidates, nameof( candidates ) ); var httpContext = context.HttpContext; - var invalidRequestHandler = default( RequestHandler ); - if ( ( invalidRequestHandler = VerifyRequestedApiVersionIsNotAmbiguous( httpContext, out var apiVersion ) ) != null ) + if ( ( context.Handler = VerifyRequestedApiVersionIsNotAmbiguous( httpContext, out var apiVersion ) ) != null ) { - context.Handler = invalidRequestHandler; return null; } var matches = EvaluateActionConstraints( context, candidates ); var selectionContext = new ActionSelectionContext( httpContext, matches, apiVersion ); var finalMatches = SelectBestActions( selectionContext ); + var properties = httpContext.ApiVersionProperties(); + var selectionResult = properties.SelectionResult; - if ( finalMatches == null || finalMatches.Count == 0 ) - { - //if ( ( invalidRequestHandler = IsValidRequest( selectionContext, candidates ) ) != null ) - //{ - // context.Handler = invalidRequestHandler; - //} + properties.ApiVersion = selectionContext.RequestedVersion; + selectionResult.CandidateActions.AddRange( candidates ); - return null; - } - else if ( finalMatches.Count == 1 ) + if ( finalMatches?.Count > 0 ) { - var actionCandidates = httpContext.ActionCandidates(); - - actionCandidates.Actions.AddRange( candidates ); - actionCandidates.MatchingActions.AddRange( finalMatches ); - actionCandidates.RequestedVersion = selectionContext.RequestedVersion; - - //var selectedAction = finalMatches[0]; - //selectedAction.AggregateAllVersions( selectionContext ); - //httpContext.ApiVersionProperties().ApiVersion = selectionContext.RequestedVersion; - //return selectedAction; - - return null; + var routeData = new RouteData( context.RouteData ); + selectionResult.MatchingActions.AddRange( finalMatches.Select( action => new ActionDescriptorMatch( action, routeData ) ) ); } - else - { - var actionNames = Join( NewLine, finalMatches.Select( a => a.DisplayName ) ); - logger.AmbiguousActions( actionNames ); - - var message = SR.ActionSelector_AmbiguousActions.FormatDefault( NewLine, actionNames ); - - throw new AmbiguousActionException( message ); - } + // note: even though we may have had a successful match, this method could be called multiple times. + // the final decision is made by the IApiVersionRoutePolicy. we return here to make sure all candidates + // have been considered. + return null; } /// @@ -199,85 +184,14 @@ RequestHandler VerifyRequestedApiVersionIsNotAmbiguous( HttpContext httpContext, } catch ( AmbiguousApiVersionException ex ) { - logger.LogInformation( ex.Message ); + Logger.LogInformation( ex.Message ); apiVersion = default( ApiVersion ); - return new BadRequestHandler( Options, AmbiguousApiVersion, ex.Message ); + return new BadRequestHandler( Options.ErrorResponses, AmbiguousApiVersion, ex.Message ); } return null; } - RequestHandler IsValidRequest( ActionSelectionContext context, IReadOnlyList candidates ) - { - Contract.Requires( context != null ); - Contract.Requires( candidates != null ); - - if ( !context.MatchingActions.Any() && !candidates.Any() ) - { - return null; - } - - var code = default( string ); - var requestedVersion = default( string ); - var parsedVersion = context.RequestedVersion; - var actionNames = new Lazy( () => Join( NewLine, candidates.Select( a => a.DisplayName ) ) ); - var allowedMethods = new Lazy>( - () => new HashSet( candidates.SelectMany( c => c.ActionConstraints.OfType() ) - .SelectMany( ac => ac.HttpMethods ), - StringComparer.OrdinalIgnoreCase ) ); - var newRequestHandler = default( Func ); - - if ( parsedVersion == null ) - { - requestedVersion = context.HttpContext.ApiVersionProperties().RawApiVersion; - - if ( IsNullOrEmpty( requestedVersion ) ) - { - code = ApiVersionUnspecified; - logger.ApiVersionUnspecified( actionNames.Value ); - return new BadRequestHandler( Options, code, SR.ApiVersionUnspecified ); - } - else if ( TryParse( requestedVersion, out parsedVersion ) ) - { - code = UnsupportedApiVersion; - logger.ApiVersionUnmatched( parsedVersion, actionNames.Value ); - - if ( allowedMethods.Value.Contains( context.HttpContext.Request.Method ) ) - { - newRequestHandler = ( o, c, m ) => new BadRequestHandler( o, c, m ); - } - else - { - newRequestHandler = ( o, c, m ) => new MethodNotAllowedHandler( o, c, m, allowedMethods.Value.ToArray() ); - } - } - else - { - code = InvalidApiVersion; - logger.ApiVersionInvalid( requestedVersion ); - newRequestHandler = ( o, c, m ) => new BadRequestHandler( o, c, m ); - } - } - else - { - requestedVersion = parsedVersion.ToString(); - code = UnsupportedApiVersion; - logger.ApiVersionUnmatched( parsedVersion, actionNames.Value ); - - if ( allowedMethods.Value.Contains( context.HttpContext.Request.Method ) ) - { - newRequestHandler = ( o, c, m ) => new BadRequestHandler( o, c, m ); - } - else - { - newRequestHandler = ( o, c, m ) => new MethodNotAllowedHandler( o, c, m, allowedMethods.Value.ToArray() ); - } - } - - var message = SR.VersionedResourceNotSupported.FormatDefault( context.HttpContext.Request.GetDisplayUrl(), requestedVersion ); - return newRequestHandler( Options, code, message ); - } - static IEnumerable MatchVersionNeutralActions( ActionSelectionContext context ) => from action in context.MatchingActions let model = action.GetProperty() @@ -393,7 +307,7 @@ IReadOnlyList EvaluateActionConstraintsCore( RouteConte if ( !constraint.Accept( constraintContext ) ) { isMatch = false; - logger.ConstraintMismatch( candidate.Action.DisplayName, candidate.Action.Id, constraint ); + Logger.ConstraintMismatch( candidate.Action.DisplayName, candidate.Action.Id, constraint ); break; } } diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionRequestProperties.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionRequestProperties.cs index 679487b1..1b55ee60 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionRequestProperties.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionRequestProperties.cs @@ -10,6 +10,7 @@ public partial class ApiVersionRequestProperties { readonly HttpContext context; + ActionSelectionResult selectionResult; /// /// Initializes a new instance of the class. @@ -24,6 +25,12 @@ public ApiVersionRequestProperties( HttpContext context ) rawApiVersion = new Lazy( GetRawApiVersion ); } + /// + /// Gets the action selection result associated with the current request. + /// + /// The action selection result associated with the current request. + public ActionSelectionResult SelectionResult => selectionResult ?? ( selectionResult = new ActionSelectionResult() ); + string GetRawApiVersion() { var reader = context.RequestServices.GetService() ?? new QueryStringApiVersionReader(); diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs index 659d4cb3..6b37c173 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs @@ -4,10 +4,10 @@ sealed class BadRequestHandler : RequestHandler { - internal BadRequestHandler( ApiVersioningOptions options, string code, string message ) - : base( options, code, message ) { } + internal BadRequestHandler( IErrorResponseProvider errorResponseProvider, string code, string message ) + : base( errorResponseProvider, code, message ) { } protected override IActionResult CreateResult( HttpContext context ) => - Options.ErrorResponses.BadRequest( context, Code, Message ); + ErrorResponses.BadRequest( context, Code, Message ); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/MethodNotAllowedHandler.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/MethodNotAllowedHandler.cs index 584594ca..edab6f8b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/MethodNotAllowedHandler.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/MethodNotAllowedHandler.cs @@ -10,8 +10,8 @@ sealed class MethodNotAllowedHandler : RequestHandler { readonly string[] allowedMethods; - internal MethodNotAllowedHandler( ApiVersioningOptions options, string code, string message, string[] allowedMethods ) - : base( options, code, message ) + internal MethodNotAllowedHandler( IErrorResponseProvider errorResponseProvider, string code, string message, string[] allowedMethods ) + : base( errorResponseProvider, code, message ) { Contract.Requires( allowedMethods != null ); this.allowedMethods = allowedMethods; @@ -19,7 +19,7 @@ internal MethodNotAllowedHandler( ApiVersioningOptions options, string code, str protected override IActionResult CreateResult( HttpContext context ) { - var result = Options.ErrorResponses.MethodNotAllowed( context, Code, Message ); + var result = ErrorResponses.MethodNotAllowed( context, Code, Message ); return allowedMethods.Length == 0 ? result : new AllowHeaderResult( result, allowedMethods ); } diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs index 879e9a81..f4476747 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs @@ -8,17 +8,17 @@ abstract class RequestHandler { - protected RequestHandler( ApiVersioningOptions options, string code, string message ) + protected RequestHandler( IErrorResponseProvider errorResponseProvider, string code, string message ) { - Contract.Requires( options != null ); + Contract.Requires( errorResponseProvider != null ); Contract.Requires( !string.IsNullOrEmpty( message ) ); - Options = options; + ErrorResponses = errorResponseProvider; Message = message; Code = code; } - protected ApiVersioningOptions Options { get; } + protected IErrorResponseProvider ErrorResponses { get; } protected string Code { get; } @@ -41,6 +41,7 @@ internal Task ExecuteAsync( HttpContext context ) return result.ExecuteResultAsync( actionContext ); } - public static implicit operator RequestDelegate( RequestHandler handler ) => handler.ExecuteAsync; + public static implicit operator RequestDelegate( RequestHandler handler ) => + handler == null ? default( RequestDelegate ) : handler.ExecuteAsync; } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/AcceptanceTest.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/AcceptanceTest.cs index 051e02ab..5fc6babc 100644 --- a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/AcceptanceTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/AcceptanceTest.cs @@ -60,7 +60,7 @@ protected virtual void Dispose( bool disposing ) TestServer CreateServer() { var builder = new WebHostBuilder() - .Configure( app => app.UseMvc( OnConfigureRoutes ) ) + .Configure( app => app.UseMvc( OnConfigureRoutes ).UseApiVersioning() ) .ConfigureServices( OnConfigureServices ); return new TestServer( builder ); diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/Values2Controller.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/Values2Controller.cs index 8c316256..bb9629d7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/Values2Controller.cs +++ b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/Values2Controller.cs @@ -9,5 +9,8 @@ public class Values2Controller : Controller { [HttpGet] public IActionResult Get() => Ok( new { Controller = nameof( Values2Controller ), Version = HttpContext.GetRequestedApiVersion().ToString() } ); + + [HttpGet( "{id:int}" )] + public IActionResult Get( int id ) => Ok( new { Controller = nameof( Values2Controller ), Id = id, Version = HttpContext.GetRequestedApiVersion().ToString() } ); } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/ValuesController.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/ValuesController.cs index 822c8350..3552a50a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/ValuesController.cs +++ b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/ValuesController.cs @@ -9,5 +9,8 @@ public class ValuesController : Controller { [HttpGet] public IActionResult Get() => Ok( new { Controller = nameof( ValuesController ), Version = HttpContext.GetRequestedApiVersion().ToString() } ); + + [HttpGet( "{id}" )] + public IActionResult Get( string id ) => Ok( new { Controller = nameof( ValuesController ), Id = id, Version = HttpContext.GetRequestedApiVersion().ToString() } ); } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when using a query string and split into two types.cs b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when using a query string and split into two types.cs index 342d9ca6..788c9198 100644 --- a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when using a query string and split into two types.cs +++ b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when using a query string and split into two types.cs @@ -4,7 +4,6 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Basic; using Microsoft.AspNetCore.Mvc.Basic.Controllers; - using System.Collections.Generic; using System.Linq; using System.Net.Http; using System.Threading.Tasks; @@ -19,20 +18,45 @@ public class when_using_a_query_string_and_split_into_two_types : BasicAcceptanc public async Task then_get_should_return_200( string controller, string apiVersion ) { // arrange - + var example = new { controller = "", version = "" }; // act var response = await GetAsync( $"api/values?api-version={apiVersion}" ).EnsureSuccessStatusCode(); - var content = await response.Content.ReadAsAsync>(); + var content = await response.Content.ReadAsExampleAsync( example ); + + // assert + response.Headers.GetValues( "api-supported-versions" ).Single().Should().Be( "1.0, 2.0" ); + content.ShouldBeEquivalentTo( new { controller = controller, version = apiVersion } ); + } + + [Fact] + public async Task then_get_with_string_id_should_return_200() + { + // arrange + var example = new { controller = "", id = "", version = "" }; + + // act + var response = await GetAsync( $"api/values/42?api-version=1.0" ).EnsureSuccessStatusCode(); + var content = await response.Content.ReadAsExampleAsync( example ); + + // assert + response.Headers.GetValues( "api-supported-versions" ).Single().Should().Be( "1.0, 2.0" ); + content.ShouldBeEquivalentTo( new { controller = nameof( ValuesController ), id = "42", version = "1.0" } ); + } + + [Fact] + public async Task then_get_with_integer_id_should_return_200() + { + // arrange + var example = new { controller = "", id = 0, version = "" }; + + // act + var response = await GetAsync( $"api/values/42?api-version=2.0" ).EnsureSuccessStatusCode(); + var content = await response.Content.ReadAsExampleAsync( example ); // assert response.Headers.GetValues( "api-supported-versions" ).Single().Should().Be( "1.0, 2.0" ); - content.ShouldBeEquivalentTo( - new Dictionary() - { - ["controller"] = controller, - ["version"] = apiVersion - } ); + content.ShouldBeEquivalentTo( new { controller = nameof( Values2Controller ), id = 42, version = "2.0" } ); } [Fact] diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/TestApiVersionActionSelector.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/TestApiVersionActionSelector.cs index ecdb56b2..b8f7bc6b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/TestApiVersionActionSelector.cs +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/TestApiVersionActionSelector.cs @@ -6,6 +6,7 @@ using Extensions.Options; using Internal; using System.Collections.Generic; + using System.Linq; public class TestApiVersionActionSelector : ApiVersionActionSelector { @@ -14,13 +15,15 @@ public TestApiVersionActionSelector( ActionConstraintCache actionConstraintCache, IOptions options, ILoggerFactory loggerFactory ) - : base( decisionTreeProvider, actionConstraintCache, options, loggerFactory ) + : base( decisionTreeProvider, actionConstraintCache, options, loggerFactory ) { } + + public override ActionDescriptor SelectBestCandidate( RouteContext context, IReadOnlyList candidates ) { + var bestCandidate = base.SelectBestCandidate( context, candidates ); + SelectedCandidate = context.HttpContext.ApiVersionProperties().SelectionResult.MatchingActions.FirstOrDefault()?.Action; + return bestCandidate; } - public override ActionDescriptor SelectBestCandidate( RouteContext context, IReadOnlyList candidates ) => - SelectedCandidate = base.SelectBestCandidate( context, candidates ); - public ActionDescriptor SelectedCandidate { get; private set; } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/WebServer.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/WebServer.cs index 2044d1f5..577841c7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/WebServer.cs +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/WebServer.cs @@ -31,7 +31,7 @@ public WebServer( Action setupApiVersioning = null, Action } var hostBuilder = new WebHostBuilder() - .Configure( app => app.UseMvc( setupRoutes ) ) + .Configure( app => app.UseMvc( setupRoutes ).UseApiVersioning() ) .ConfigureServices( services => { From 1ed2d7ceab7174957a1df3b4cd55431be6477229 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Wed, 10 May 2017 15:09:15 -0700 Subject: [PATCH 4/4] Apply "Default" prefix to route policy implementation --- .../IServiceCollectionExtensions.cs | 2 +- ...nRoutePolicy.cs => DefaultApiVersionRoutePolicy.cs} | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) rename src/Microsoft.AspNetCore.Mvc.Versioning/Routing/{ApiVersionRoutePolicy.cs => DefaultApiVersionRoutePolicy.cs} (96%) diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs index 5b657b78..18c01e42 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs @@ -42,7 +42,7 @@ public static IServiceCollection AddApiVersioning( this IServiceCollection servi services.Add( new ServiceDescriptor( typeof( IErrorResponseProvider ), options.ErrorResponses ) ); services.Add( Singleton>( new OptionsWrapper( options ) ) ); services.Replace( Singleton() ); - services.TryAddSingleton(); + services.TryAddSingleton(); if ( options.ReportApiVersions ) { diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs similarity index 96% rename from src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs rename to src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs index 0f358065..ae5f5663 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRoutePolicy.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs @@ -23,27 +23,27 @@ /// Represents the default API versioning route policy. /// [CLSCompliant( false )] - public class ApiVersionRoutePolicy : IApiVersionRoutePolicy + public class DefaultApiVersionRoutePolicy : IApiVersionRoutePolicy { static readonly Task CompletedTask = Task.FromResult( default( object ) ); /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The underlying action invoker factory. /// The provider used to create error responses. /// The . - public ApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory, IErrorResponseProvider errorResponseProvider, ILoggerFactory loggerFactory ) + public DefaultApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory, IErrorResponseProvider errorResponseProvider, ILoggerFactory loggerFactory ) : this( actionInvokerFactory, errorResponseProvider, loggerFactory, null ) { } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The underlying action invoker factory. /// The provider used to create error responses. /// The . /// The associated action context accessor. - public ApiVersionRoutePolicy( + public DefaultApiVersionRoutePolicy( IActionInvokerFactory actionInvokerFactory, IErrorResponseProvider errorResponseProvider, ILoggerFactory loggerFactory,