From 6ba1151c411612d2be93ad9801a7576a707b4851 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Wed, 22 Feb 2017 16:55:40 -0800 Subject: [PATCH 1/8] Use all candidates instead of just filtered matches. Fixes #83. --- .../Versioning/ApiVersionActionSelector.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs index eb0b6060..53aa652c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs @@ -212,7 +212,7 @@ private BadRequestHandler IsValidRequest( ActionSelectionContext context, IReadO var code = default( string ); var requestedVersion = default( string ); var parsedVersion = context.RequestedVersion; - var actionNames = new Lazy( () => Join( NewLine, context.MatchingActions.Select( a => a.DisplayName ) ) ); + var actionNames = new Lazy( () => Join( NewLine, candidates.Select( a => a.DisplayName ) ) ); if ( parsedVersion == null ) { From 448f02eba2ca39a391ce0bad991573ce97956f81 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Thu, 23 Feb 2017 06:11:36 -0800 Subject: [PATCH 2/8] Reduce dependencies to Microsoft.AspNetMvc.Mvc.Core and update to version 1.1.1 per MSA4010983. Resolves #66. Fixes #78. --- samples/aspnetcore/BasicSample/project.json | 2 +- .../aspnetcore/ConventionsSample/project.json | 2 +- .../IServiceCollectionExtensions.cs | 2 +- .../project.json | 5 +- .../project.json | 4 +- .../project.json | 4 +- .../project.json | 4 +- .../project.json | 62 ++++++++++--------- .../project.json | 6 +- 9 files changed, 47 insertions(+), 44 deletions(-) diff --git a/samples/aspnetcore/BasicSample/project.json b/samples/aspnetcore/BasicSample/project.json index 1bb80957..6639a882 100644 --- a/samples/aspnetcore/BasicSample/project.json +++ b/samples/aspnetcore/BasicSample/project.json @@ -4,7 +4,7 @@ "version": "1.0.0", "type": "platform" }, - "Microsoft.AspNetCore.Mvc": "1.0.0", + "Microsoft.AspNetCore.Mvc": "1.1.1", "Microsoft.AspNetCore.Server.IISIntegration": "1.0.0", "Microsoft.AspNetCore.Server.Kestrel": "1.0.0", "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0", diff --git a/samples/aspnetcore/ConventionsSample/project.json b/samples/aspnetcore/ConventionsSample/project.json index ec338d9c..5e2ffa7a 100644 --- a/samples/aspnetcore/ConventionsSample/project.json +++ b/samples/aspnetcore/ConventionsSample/project.json @@ -4,7 +4,7 @@ "version": "1.0.0", "type": "platform" }, - "Microsoft.AspNetCore.Mvc": "1.0.0", + "Microsoft.AspNetCore.Mvc": "1.1.1", "Microsoft.AspNetCore.Server.IISIntegration": "1.0.0", "Microsoft.AspNetCore.Server.Kestrel": "1.0.0", "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0", 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 794c4e43..3443eadd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs @@ -47,7 +47,7 @@ public static IServiceCollection AddApiVersioning( this IServiceCollection servi services.AddSingleton(); } - services.AddMvc( + services.AddMvcCore( mvcOptions => { if ( options.ReportApiVersions ) diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/project.json b/src/Microsoft.AspNetCore.Mvc.Versioning/project.json index 0371f21f..6595ab62 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/project.json +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/project.json @@ -7,13 +7,12 @@ "language": "en", "dependencies": { - "NETStandard.Library": "1.6.0", - "Microsoft.AspNetCore.Mvc": "1.0.0" + "NETStandard.Library": "1.6.1", + "Microsoft.AspNetCore.Mvc.Core": "1.1.1" }, "frameworks": { "net451": { - }, "netstandard1.6": { "imports": "dnxcore50" diff --git a/test/Microsoft.AspNet.OData.Versioning.Tests/project.json b/test/Microsoft.AspNet.OData.Versioning.Tests/project.json index 4978e1f6..cebbbfb7 100644 --- a/test/Microsoft.AspNet.OData.Versioning.Tests/project.json +++ b/test/Microsoft.AspNet.OData.Versioning.Tests/project.json @@ -3,10 +3,10 @@ "dependencies": { "Microsoft.AspNet.OData": "5.9.1", - "FluentAssertions": "4.12.0", + "FluentAssertions": "4.19.0", "more.xunit": "2.1.0", "xunit.runner.visualstudio": "2.1.0", - "Moq": "4.5.16", + "Moq": "4.7.0", "Microsoft.AspNet.WebApi.Core": "5.2.3", "Microsoft.AspNet.OData.Versioning": { "target": "project", diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/project.json b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/project.json index c1ebc873..144e7724 100644 --- a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/project.json +++ b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/project.json @@ -3,10 +3,10 @@ "dependencies": { "Microsoft.AspNet.OData": "5.9.1", - "FluentAssertions": "4.12.0", + "FluentAssertions": "4.19.0", "more.xunit": "2.1.0", "xunit.runner.visualstudio": "2.1.0", - "Moq": "4.5.16", + "Moq": "4.7.0", "Microsoft.AspNet.WebApi.Core": "5.2.3", "Microsoft.AspNet.WebApi.Versioning": { "target": "project", diff --git a/test/Microsoft.AspNet.WebApi.Versioning.Tests/project.json b/test/Microsoft.AspNet.WebApi.Versioning.Tests/project.json index 3dd0ce03..b588bbd2 100644 --- a/test/Microsoft.AspNet.WebApi.Versioning.Tests/project.json +++ b/test/Microsoft.AspNet.WebApi.Versioning.Tests/project.json @@ -3,10 +3,10 @@ "dependencies": { "Microsoft.AspNet.WebApi.Core": "5.2.3", - "FluentAssertions": "4.12.0", + "FluentAssertions": "4.19.0", "more.xunit": "2.1.0", "xunit.runner.visualstudio": "2.1.0", - "Moq": "4.5.16", + "Moq": "4.7.0", "Microsoft.AspNet.WebApi.Versioning": { "target": "project", "version": "" diff --git a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/project.json b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/project.json index 449c67ac..88ada322 100644 --- a/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/project.json +++ b/test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/project.json @@ -1,37 +1,39 @@ { - "version": "1.0.0-*", - "testRunner": "xunit", + "version": "1.0.0-*", + "testRunner": "xunit", - "dependencies": { - "dotnet-test-xunit": "2.2.0-preview2-build1029", - "Microsoft.AspNetCore.TestHost": "1.0.0", - "Microsoft.NETCore.App": { - "type": "platform", - "version": "1.0.0" + "dependencies": { + "dotnet-test-xunit": "2.2.0-preview2-build1029", + "Microsoft.DotNet.InternalAbstractions": "1.0.1-beta-003206", + "Microsoft.AspNetCore.Mvc": "1.1.1", + "Microsoft.AspNetCore.TestHost": "1.0.0", + "Microsoft.NETCore.App": { + "type": "platform", + "version": "1.0.0" + }, + "FluentAssertions": "4.12.0", + "Moq": "4.7.0", + "Newtonsoft.Json": "9.0.1", + "System.Diagnostics.TraceSource": "4.0.0", + "System.Net.Http": "4.3.0", + "Microsoft.AspNetCore.Mvc.Versioning": { + "target": "project", + "version": "" + }, + "more.xunit": "2.2.0-beta2-build3300", + "Microsoft.AspNet.WebApi.Client": "5.2.3", + "System.Runtime.Serialization.Xml": "4.1.1" }, - "FluentAssertions": "4.12.0", - "Moq": "4.6.36-alpha", - "Newtonsoft.Json": "9.0.1", - "System.Diagnostics.TraceSource": "4.0.0", - "System.Net.Http": "4.1.0", - "Microsoft.AspNetCore.Mvc.Versioning": { - "target": "project", - "version": "" - }, - "more.xunit": "2.2.0-beta2-build3300", - "Microsoft.AspNet.WebApi.Client": "5.2.3", - "System.Runtime.Serialization.Xml": "4.1.1" - }, - "frameworks": { - "netcoreapp1.0": { - "imports": [ "dnxcore50", "portable-net451+win8" ] - } - }, + "frameworks": { + "netcoreapp1.0": { + "imports": [ "dnxcore50", "portable-net451+win8" ] + } + }, - "buildOptions": { - "copyToOutput": { - "include": [ "xunit.runner.json" ] + "buildOptions": { + "copyToOutput": { + "include": [ "xunit.runner.json" ] + } } - } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/project.json b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/project.json index 90257095..c4527390 100644 --- a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/project.json +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/project.json @@ -4,6 +4,8 @@ "dependencies": { "dotnet-test-xunit": "2.2.0-preview2-build1029", + "Microsoft.DotNet.InternalAbstractions": "1.0.1-beta-003206", + "Microsoft.AspNetCore.Mvc": "1.1.1", "Microsoft.AspNetCore.TestHost": "1.0.0", "Microsoft.NETCore.App": { "type": "platform", @@ -11,10 +13,10 @@ }, "FluentAssertions": "4.12.0", - "Moq": "4.6.36-alpha", + "Moq": "4.7.0", "Newtonsoft.Json": "9.0.1", "System.Diagnostics.TraceSource": "4.0.0", - "System.Net.Http": "4.1.0", + "System.Net.Http": "4.3.0", "Microsoft.AspNetCore.Mvc.Versioning": { "target": "project", "version": "" From 2b3935f996e4000c5e1df36b80428eef9c79c0f1 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 3 Mar 2017 06:37:24 -0800 Subject: [PATCH 3/8] Strongly-typed resources should be scope internal --- .../SR.Designer.cs | 75 +++++++++++++------ .../SR.resx | 11 ++- .../SR.Designer.cs | 35 +++++---- .../SR.resx | 3 + 4 files changed, 86 insertions(+), 38 deletions(-) diff --git a/src/Microsoft.AspNet.WebApi.Versioning/SR.Designer.cs b/src/Microsoft.AspNet.WebApi.Versioning/SR.Designer.cs index 4f01371a..da5863e7 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/SR.Designer.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/SR.Designer.cs @@ -22,7 +22,7 @@ namespace Microsoft.Web.Http { // with the /str option, or rebuild your VS project. [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] - public class SR { + internal class SR { private static global::System.Resources.ResourceManager resourceMan; @@ -35,7 +35,7 @@ internal SR() { /// Returns the cached ResourceManager instance used by this class. /// [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - public static global::System.Resources.ResourceManager ResourceManager { + internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.AspNet.WebApi.Versioning.SR", typeof(SR).GetTypeInfo().Assembly); @@ -50,7 +50,7 @@ internal SR() { /// resource lookups using this strongly typed resource class. /// [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - public static global::System.Globalization.CultureInfo Culture { + internal static global::System.Globalization.CultureInfo Culture { get { return resourceCulture; } @@ -62,7 +62,7 @@ internal SR() { /// /// Looks up a localized string similar to {0} on type {1}. /// - public static string ActionSelector_AmbiguousMatchType { + internal static string ActionSelector_AmbiguousMatchType { get { return ResourceManager.GetString("ActionSelector_AmbiguousMatchType", resourceCulture); } @@ -71,7 +71,7 @@ public static string ActionSelector_AmbiguousMatchType { /// /// Looks up a localized string similar to No action was found on the controller '{0}' that matches the name '{1}'.. /// - public static string ApiControllerActionSelector_ActionNameNotFound { + internal static string ApiControllerActionSelector_ActionNameNotFound { get { return ResourceManager.GetString("ApiControllerActionSelector_ActionNameNotFound", resourceCulture); } @@ -80,7 +80,7 @@ public static string ApiControllerActionSelector_ActionNameNotFound { /// /// Looks up a localized string similar to No action was found on the controller '{0}' that matches the request.. /// - public static string ApiControllerActionSelector_ActionNotFound { + internal static string ApiControllerActionSelector_ActionNotFound { get { return ResourceManager.GetString("ApiControllerActionSelector_ActionNotFound", resourceCulture); } @@ -89,16 +89,16 @@ public static string ApiControllerActionSelector_ActionNotFound { /// /// Looks up a localized string similar to Multiple actions were found that match the request: {0}. /// - public static string ApiControllerActionSelector_AmbiguousMatch { + internal static string ApiControllerActionSelector_AmbiguousMatch { get { return ResourceManager.GetString("ApiControllerActionSelector_AmbiguousMatch", resourceCulture); } } /// - /// Looks up a localized string similar to The requested resource does not support http method '{0}'.. + /// Looks up a localized string similar to The requested resource does not support HTTP method '{0}'.. /// - public static string ApiControllerActionSelector_HttpMethodNotSupported { + internal static string ApiControllerActionSelector_HttpMethodNotSupported { get { return ResourceManager.GetString("ApiControllerActionSelector_HttpMethodNotSupported", resourceCulture); } @@ -107,7 +107,7 @@ public static string ApiControllerActionSelector_HttpMethodNotSupported { /// /// Looks up a localized string similar to The specified API group version '{0}' is invalid.. /// - public static string ApiVersionBadGroupVersion { + internal static string ApiVersionBadGroupVersion { get { return ResourceManager.GetString("ApiVersionBadGroupVersion", resourceCulture); } @@ -116,7 +116,7 @@ public static string ApiVersionBadGroupVersion { /// /// Looks up a localized string similar to The specified API version status '{0}' is invalid.. /// - public static string ApiVersionBadStatus { + internal static string ApiVersionBadStatus { get { return ResourceManager.GetString("ApiVersionBadStatus", resourceCulture); } @@ -125,7 +125,7 @@ public static string ApiVersionBadStatus { /// /// Looks up a localized string similar to The specified API version is invalid.. /// - public static string ApiVersionInvalidFormat { + internal static string ApiVersionInvalidFormat { get { return ResourceManager.GetString("ApiVersionInvalidFormat", resourceCulture); } @@ -134,7 +134,7 @@ public static string ApiVersionInvalidFormat { /// /// Looks up a localized string similar to The format '{0}' is invalid or not supported.. /// - public static string ApiVersionInvalidFormatCode { + internal static string ApiVersionInvalidFormatCode { get { return ResourceManager.GetString("ApiVersionInvalidFormatCode", resourceCulture); } @@ -143,7 +143,7 @@ public static string ApiVersionInvalidFormatCode { /// /// Looks up a localized string similar to The requested API version '{0}' is not supported.. /// - public static string ApiVersionNotSupported { + internal static string ApiVersionNotSupported { get { return ResourceManager.GetString("ApiVersionNotSupported", resourceCulture); } @@ -152,7 +152,7 @@ public static string ApiVersionNotSupported { /// /// Looks up a localized string similar to An API version is required, but was not specified.. /// - public static string ApiVersionUnspecified { + internal static string ApiVersionUnspecified { get { return ResourceManager.GetString("ApiVersionUnspecified", resourceCulture); } @@ -161,7 +161,7 @@ public static string ApiVersionUnspecified { /// /// Looks up a localized string similar to No route providing a controller name was found to match request URI '{0}'.. /// - public static string ControllerNameNotFound { + internal static string ControllerNameNotFound { get { return ResourceManager.GetString("ControllerNameNotFound", resourceCulture); } @@ -170,7 +170,7 @@ public static string ControllerNameNotFound { /// /// Looks up a localized string similar to Multiple types were found that match the controller named '{0}'. This can happen if the route that services this request ('{1}') found multiple controllers defined with the same name but differing namespaces, which is not supported.{3}{3}The request for '{0}' has found the following matching controllers:{2}. /// - public static string DefaultControllerFactory_ControllerNameAmbiguous_WithRouteTemplate { + internal static string DefaultControllerFactory_ControllerNameAmbiguous_WithRouteTemplate { get { return ResourceManager.GetString("DefaultControllerFactory_ControllerNameAmbiguous_WithRouteTemplate", resourceCulture); } @@ -179,7 +179,7 @@ public static string DefaultControllerFactory_ControllerNameAmbiguous_WithRouteT /// /// Looks up a localized string similar to No type was found that matches the controller named '{0}'.. /// - public static string DefaultControllerFactory_ControllerNameNotFound { + internal static string DefaultControllerFactory_ControllerNameNotFound { get { return ResourceManager.GetString("DefaultControllerFactory_ControllerNameNotFound", resourceCulture); } @@ -188,7 +188,7 @@ public static string DefaultControllerFactory_ControllerNameNotFound { /// /// Looks up a localized string similar to Multiple controller types were found that match the URL. This can happen if attribute routes on multiple controllers match the requested URL.{1}{1}The request has found the following matching controller types: {0}. /// - public static string DirectRoute_AmbiguousController { + internal static string DirectRoute_AmbiguousController { get { return ResourceManager.GetString("DirectRoute_AmbiguousController", resourceCulture); } @@ -197,7 +197,7 @@ public static string DirectRoute_AmbiguousController { /// /// Looks up a localized string similar to The expression '{0}' must refer to a controller action method.. /// - public static string InvalidActionMethodExpression { + internal static string InvalidActionMethodExpression { get { return ResourceManager.GetString("InvalidActionMethodExpression", resourceCulture); } @@ -206,7 +206,7 @@ public static string InvalidActionMethodExpression { /// /// Looks up a localized string similar to The following API versions were requested: {0}. At most, only a single API version may be specified. Please update the intended API version and retry the request.. /// - public static string MultipleDifferentApiVersionsRequested { + internal static string MultipleDifferentApiVersionsRequested { get { return ResourceManager.GetString("MultipleDifferentApiVersionsRequested", resourceCulture); } @@ -215,28 +215,55 @@ public static string MultipleDifferentApiVersionsRequested { /// /// Looks up a localized string similar to No HTTP resource was found that matches the request URI '{0}'.. /// - public static string ResourceNotFound { + internal static string ResourceNotFound { get { return ResourceManager.GetString("ResourceNotFound", resourceCulture); } } + /// + /// Looks up a localized string similar to No route providing a controller name with API version '{2}' was found to match HTTP method '{1}' and request URI '{0}'.. + /// + internal static string VersionedActionNameNotFound { + get { + return ResourceManager.GetString("VersionedActionNameNotFound", resourceCulture); + } + } + /// /// Looks up a localized string similar to No route providing a controller name with API version '{1}' was found to match request URI '{0}'.. /// - public static string VersionedControllerNameNotFound { + internal static string VersionedControllerNameNotFound { get { return ResourceManager.GetString("VersionedControllerNameNotFound", resourceCulture); } } + /// + /// Looks up a localized string similar to The requested resource with API version '{0}' does not support HTTP method '{1}'.. + /// + internal static string VersionedMethodNotSupported { + get { + return ResourceManager.GetString("VersionedMethodNotSupported", resourceCulture); + } + } + /// /// Looks up a localized string similar to The HTTP resource that matches the request URI '{0}' does not support the API version '{1}'.. /// - public static string VersionedResourceNotSupported { + internal static string VersionedResourceNotSupported { get { return ResourceManager.GetString("VersionedResourceNotSupported", resourceCulture); } } + + /// + /// Looks up a localized string similar to At least one IApiVersionReader must be specified.. + /// + internal static string ZeroApiVersionReaders { + get { + return ResourceManager.GetString("ZeroApiVersionReaders", resourceCulture); + } + } } } diff --git a/src/Microsoft.AspNet.WebApi.Versioning/SR.resx b/src/Microsoft.AspNet.WebApi.Versioning/SR.resx index d40dafcf..eec4585b 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/SR.resx +++ b/src/Microsoft.AspNet.WebApi.Versioning/SR.resx @@ -130,7 +130,7 @@ Multiple actions were found that match the request: {0} - The requested resource does not support http method '{0}'. + The requested resource does not support HTTP method '{0}'. The specified API group version '{0}' is invalid. @@ -171,10 +171,19 @@ No HTTP resource was found that matches the request URI '{0}'. + + No route providing a controller name with API version '{2}' was found to match HTTP method '{1}' and request URI '{0}'. + No route providing a controller name with API version '{1}' was found to match request URI '{0}'. + + The requested resource with API version '{0}' does not support HTTP method '{1}'. + The HTTP resource that matches the request URI '{0}' does not support the API version '{1}'. + + At least one IApiVersionReader must be specified. + \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs index 46a04395..7a4f292b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.Designer.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Mvc { // with the /str option, or rebuild your VS project. [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] - public class SR { + internal class SR { private static global::System.Resources.ResourceManager resourceMan; @@ -35,7 +35,7 @@ internal SR() { /// Returns the cached ResourceManager instance used by this class. /// [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - public static global::System.Resources.ResourceManager ResourceManager { + internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.AspNetCore.Mvc.Versioning.SR", typeof(SR).GetTypeInfo().Assembly); @@ -50,7 +50,7 @@ internal SR() { /// resource lookups using this strongly typed resource class. /// [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - public static global::System.Globalization.CultureInfo Culture { + internal static global::System.Globalization.CultureInfo Culture { get { return resourceCulture; } @@ -62,7 +62,7 @@ internal SR() { /// /// Looks up a localized string similar to Multiple actions matched. The following actions matched route data and had all constraints satisfied:{0}{0}{1}. /// - public static string ActionSelector_AmbiguousActions { + internal static string ActionSelector_AmbiguousActions { get { return ResourceManager.GetString("ActionSelector_AmbiguousActions", resourceCulture); } @@ -71,7 +71,7 @@ public static string ActionSelector_AmbiguousActions { /// /// Looks up a localized string similar to The specified API group version '{0}' is invalid.. /// - public static string ApiVersionBadGroupVersion { + internal static string ApiVersionBadGroupVersion { get { return ResourceManager.GetString("ApiVersionBadGroupVersion", resourceCulture); } @@ -80,7 +80,7 @@ public static string ApiVersionBadGroupVersion { /// /// Looks up a localized string similar to The specified API version status '{0}' is invalid.. /// - public static string ApiVersionBadStatus { + internal static string ApiVersionBadStatus { get { return ResourceManager.GetString("ApiVersionBadStatus", resourceCulture); } @@ -89,7 +89,7 @@ public static string ApiVersionBadStatus { /// /// Looks up a localized string similar to The specified API version is invalid.. /// - public static string ApiVersionInvalidFormat { + internal static string ApiVersionInvalidFormat { get { return ResourceManager.GetString("ApiVersionInvalidFormat", resourceCulture); } @@ -98,7 +98,7 @@ public static string ApiVersionInvalidFormat { /// /// Looks up a localized string similar to The format '{0}' is invalid or not supported.. /// - public static string ApiVersionInvalidFormatCode { + internal static string ApiVersionInvalidFormatCode { get { return ResourceManager.GetString("ApiVersionInvalidFormatCode", resourceCulture); } @@ -107,7 +107,7 @@ public static string ApiVersionInvalidFormatCode { /// /// Looks up a localized string similar to The requested API version '{0}' is not supported.. /// - public static string ApiVersionNotSupported { + internal static string ApiVersionNotSupported { get { return ResourceManager.GetString("ApiVersionNotSupported", resourceCulture); } @@ -116,7 +116,7 @@ public static string ApiVersionNotSupported { /// /// Looks up a localized string similar to An API version is required, but was not specified.. /// - public static string ApiVersionUnspecified { + internal static string ApiVersionUnspecified { get { return ResourceManager.GetString("ApiVersionUnspecified", resourceCulture); } @@ -125,7 +125,7 @@ public static string ApiVersionUnspecified { /// /// Looks up a localized string similar to The expression '{0}' must refer to a controller action method.. /// - public static string InvalidActionMethodExpression { + internal static string InvalidActionMethodExpression { get { return ResourceManager.GetString("InvalidActionMethodExpression", resourceCulture); } @@ -134,7 +134,7 @@ public static string InvalidActionMethodExpression { /// /// Looks up a localized string similar to The following API versions were requested: {0}. At most, only a single API version may be specified. Please update the intended API version and retry the request.. /// - public static string MultipleDifferentApiVersionsRequested { + internal static string MultipleDifferentApiVersionsRequested { get { return ResourceManager.GetString("MultipleDifferentApiVersionsRequested", resourceCulture); } @@ -143,10 +143,19 @@ public static string MultipleDifferentApiVersionsRequested { /// /// Looks up a localized string similar to The HTTP resource that matches the request URI '{0}' does not support the API version '{1}'.. /// - public static string VersionedResourceNotSupported { + internal static string VersionedResourceNotSupported { get { return ResourceManager.GetString("VersionedResourceNotSupported", resourceCulture); } } + + /// + /// Looks up a localized string similar to At least one IApiVersionReader must be specified.. + /// + internal static string ZeroApiVersionReaders { + get { + return ResourceManager.GetString("ZeroApiVersionReaders", resourceCulture); + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx index 338dfdc5..d3a54aa7 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/SR.resx @@ -148,4 +148,7 @@ The HTTP resource that matches the request URI '{0}' does not support the API version '{1}'. + + At least one IApiVersionReader must be specified. + \ No newline at end of file From d48c835efc188d8601df9fd936c612f1ca604eb1 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 3 Mar 2017 07:16:15 -0800 Subject: [PATCH 4/8] Support reading API version from URL segment. Fixes #91 --- .../Versioning/UrlSegmentApiVersionReader.cs | 25 ++++++ .../Routing/ApiVersionRouteConstraint.cs | 27 +++++-- .../HttpRequestMessageExtensions.cs | 33 +++++++- .../Versioning/UrlSegmentApiVersionReader.cs | 77 +++++++++++++++++++ .../HttpContextExtensions.cs | 33 +++++++- .../Routing/ApiVersionRouteConstraint.cs | 24 ++++-- ...eader.cs => UrlSegmentApiVersionReader.cs} | 26 +++---- .../UrlSegmentApiVersionReaderTest.cs | 50 ++++++++++++ .../Routing/ApiVersionRouteConstraintTest.cs | 35 +++++---- .../UrlSegmentApiVersionReaderTest.cs | 48 ++++++++++++ 10 files changed, 332 insertions(+), 46 deletions(-) create mode 100644 src/Common/Versioning/UrlSegmentApiVersionReader.cs create mode 100644 src/Microsoft.AspNet.WebApi.Versioning/Versioning/UrlSegmentApiVersionReader.cs rename src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/{QueryStringOrHeaderApiVersionReader.cs => UrlSegmentApiVersionReader.cs} (55%) create mode 100644 test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs diff --git a/src/Common/Versioning/UrlSegmentApiVersionReader.cs b/src/Common/Versioning/UrlSegmentApiVersionReader.cs new file mode 100644 index 00000000..4df5a424 --- /dev/null +++ b/src/Common/Versioning/UrlSegmentApiVersionReader.cs @@ -0,0 +1,25 @@ +#if WEBAPI +namespace Microsoft.Web.Http.Versioning +#else +namespace Microsoft.AspNetCore.Mvc.Versioning +#endif +{ +#if WEBAPI + using Routing; +#else + using Microsoft.AspNetCore.Routing; + using Routing; +#endif + using System.Diagnostics.Contracts; + + /// + /// Represents a service API version reader that reads the value from a path segment in the request URL. + /// + public partial class UrlSegmentApiVersionReader : IApiVersionReader + { + /// + /// Initializes a new instance of the class. + /// + public UrlSegmentApiVersionReader() { } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Routing/ApiVersionRouteConstraint.cs b/src/Microsoft.AspNet.WebApi.Versioning/Routing/ApiVersionRouteConstraint.cs index 0e01df77..6e561e37 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/Routing/ApiVersionRouteConstraint.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/Routing/ApiVersionRouteConstraint.cs @@ -1,6 +1,5 @@ namespace Microsoft.Web.Http.Routing { - using System; using System.Collections.Generic; using System.Net.Http; using System.Web.Http; @@ -25,22 +24,36 @@ public sealed class ApiVersionRouteConstraint : IHttpRouteConstraint /// True if the route constraint is matched; otherwise, false. public bool Match( HttpRequestMessage request, IHttpRoute route, string parameterName, IDictionary values, HttpRouteDirection routeDirection ) { + if ( IsNullOrEmpty( parameterName ) ) + { + return false; + } + var value = default( string ); + if ( values.TryGetValue( parameterName, out value ) ) + { + request.SetRouteParameterName( parameterName ); + } + else + { + return false; + } + if ( routeDirection == UriGeneration ) { - return !IsNullOrEmpty( parameterName ) && values.TryGetValue( parameterName, out value ) && !IsNullOrEmpty( value ); + return !IsNullOrEmpty( value ); } var requestedVersion = default( ApiVersion ); - if ( !values.TryGetValue( parameterName, out value ) || !TryParse( value, out requestedVersion ) ) + if ( TryParse( value, out requestedVersion ) ) { - return false; + request.SetRequestedApiVersion( requestedVersion ); + return true; } - request.SetRequestedApiVersion( requestedVersion ); - return true; + return false; } } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/System.Web.Http/HttpRequestMessageExtensions.cs b/src/Microsoft.AspNet.WebApi.Versioning/System.Web.Http/HttpRequestMessageExtensions.cs index cf8ae938..bd5b365d 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/System.Web.Http/HttpRequestMessageExtensions.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/System.Web.Http/HttpRequestMessageExtensions.cs @@ -5,12 +5,14 @@ using Diagnostics.Contracts; using Microsoft; using Microsoft.Web.Http; + using Microsoft.Web.Http.Routing; using Microsoft.Web.Http.Versioning; using Net; using Net.Http; using System; - using static Microsoft.Web.Http.ApiVersion; using static ComponentModel.EditorBrowsableState; + using static Microsoft.Web.Http.ApiVersion; + using static System.String; /// /// Provides extension methods for the class. @@ -18,6 +20,7 @@ public static class HttpRequestMessageExtensions { private const string ApiVersionKey = "MS_" + nameof( ApiVersion ); + private const string ApiVersionRouteParameterName = "MS_" + nameof( ApiVersionRouteConstraint ) + "_ParameterName"; [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Handled by the caller." )] private static HttpResponseMessage CreateErrorResponse( this HttpRequestMessage request, HttpStatusCode statusCode, Func errorCreator ) @@ -140,5 +143,33 @@ public static void SetRequestedApiVersion( this HttpRequestMessage request, ApiV request.Properties[ApiVersionKey] = version; } } + + internal static string GetRouteParameterNameAssignedByApiVersionRouteConstraint( this HttpRequestMessage request ) + { + Contract.Requires( request != null ); + + var parameterName = default( string ); + + if ( request.Properties.TryGetValue( ApiVersionRouteParameterName, out parameterName ) ) + { + return parameterName; + } + + return null; + } + + internal static void SetRouteParameterName( this HttpRequestMessage request, string parameterName ) + { + Contract.Requires( request != null ); + + if ( IsNullOrEmpty( parameterName ) ) + { + request.Properties.Remove( ApiVersionRouteParameterName ); + } + else + { + request.Properties[ApiVersionRouteParameterName] = parameterName; + } + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/UrlSegmentApiVersionReader.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/UrlSegmentApiVersionReader.cs new file mode 100644 index 00000000..247b1ee7 --- /dev/null +++ b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/UrlSegmentApiVersionReader.cs @@ -0,0 +1,77 @@ +namespace Microsoft.Web.Http.Versioning +{ + using Routing; + using System.Diagnostics.Contracts; + using System.Net.Http; + using System.Web.Http; + using System.Web.Http.Routing; + using static System.String; + + /// + /// Provides the implementation for ASP.NET Web API. + /// + public partial class UrlSegmentApiVersionReader + { + /// + /// Reads the service API version value from a request. + /// + /// The HTTP request to read the API version from. + /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. + /// Multiple, different API versions were requested. + public virtual string Read( HttpRequestMessage request ) + { + Arg.NotNull( request, nameof( request ) ); + + var routeData = request.GetRouteData(); + + if ( routeData == null ) + { + return null; + } + + var key = request.GetRouteParameterNameAssignedByApiVersionRouteConstraint(); + var subRouteData = routeData.GetSubRoutes() ?? new[] { routeData }; + var value = default( object ); + + if ( IsNullOrEmpty( key ) ) + { + foreach ( var subRouteDatum in subRouteData ) + { + key = GetRouteParameterNameFromConstraintNameInTemplate( subRouteDatum ); + + if ( key != null && subRouteDatum.Values.TryGetValue( key, out value ) ) + { + return value.ToString(); + } + } + } + else + { + foreach ( var subRouteDatum in subRouteData ) + { + if ( subRouteDatum.Values.TryGetValue( key, out value ) ) + { + return value.ToString(); + } + } + } + + return null; + } + + static string GetRouteParameterNameFromConstraintNameInTemplate( IHttpRouteData routeData ) + { + Contract.Requires( routeData != null ); + + foreach ( var constraint in routeData.Route.Constraints ) + { + if ( constraint.Value is ApiVersionRouteConstraint ) + { + return constraint.Key; + } + } + + return null; + } + } +} \ 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 45bda46c..8f0fa9fa 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/HttpContextExtensions.cs @@ -1,12 +1,14 @@ namespace Microsoft.AspNetCore.Mvc { using Http; + using Routing; using System; using System.ComponentModel; using System.Diagnostics.Contracts; using Versioning; using static ApiVersion; using static System.ComponentModel.EditorBrowsableState; + using static System.String; /// /// Provides extension methods for the class. @@ -15,6 +17,7 @@ public static class HttpContextExtensions { private const string ApiVersionKey = "MS_" + nameof( ApiVersion ); + private const string ApiVersionRouteParameterName = "MS_" + nameof( ApiVersionRouteConstraint ) + "_ParameterName"; /// /// Gets the current raw, unparsed service API version requested. @@ -76,5 +79,33 @@ internal static void SetRequestedApiVersion( this HttpContext context, ApiVersio context.Items[ApiVersionKey] = version; } } + + internal static string GetRouteParameterNameAssignedByApiVersionRouteConstraint( this HttpContext context ) + { + Contract.Requires( context != null ); + + var parameterName = default( string ); + + if ( context.Items.TryGetValue( ApiVersionRouteParameterName, out parameterName ) ) + { + return parameterName; + } + + return null; + } + + internal static void SetRouteParameterName( this HttpContext context, string parameterName ) + { + Contract.Requires( context != null ); + + if ( IsNullOrEmpty( parameterName ) ) + { + context.Items.Remove( ApiVersionRouteParameterName ); + } + else + { + context.Items[ApiVersionRouteParameterName] = parameterName; + } + } } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteConstraint.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteConstraint.cs index 600d0e92..a3b211f5 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteConstraint.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionRouteConstraint.cs @@ -24,22 +24,36 @@ public sealed class ApiVersionRouteConstraint : IRouteConstraint /// True if the route constraint is matched; otherwise, false. public bool Match( HttpContext httpContext, IRouter route, string routeKey, RouteValueDictionary values, RouteDirection routeDirection ) { + if ( IsNullOrEmpty( routeKey ) ) + { + return false; + } + var value = default( string ); + if ( values.TryGetValue( routeKey, out value ) ) + { + httpContext.SetRouteParameterName( routeKey ); + } + else + { + return false; + } + if ( routeDirection == UrlGeneration ) { - return !IsNullOrEmpty( routeKey ) && values.TryGetValue( routeKey, out value ) && !IsNullOrEmpty( value ); + return !IsNullOrEmpty( value ); } var requestedVersion = default( ApiVersion ); - if ( !values.TryGetValue( routeKey, out value ) || !TryParse( value, out requestedVersion ) ) + if ( TryParse( value, out requestedVersion ) ) { - return false; + httpContext.SetRequestedApiVersion( requestedVersion ); + return true; } - httpContext.SetRequestedApiVersion( requestedVersion ); - return true; + return false; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/QueryStringOrHeaderApiVersionReader.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/UrlSegmentApiVersionReader.cs similarity index 55% rename from src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/QueryStringOrHeaderApiVersionReader.cs rename to src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/UrlSegmentApiVersionReader.cs index 70304aa8..25cfcdd7 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/QueryStringOrHeaderApiVersionReader.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/UrlSegmentApiVersionReader.cs @@ -1,16 +1,15 @@ namespace Microsoft.AspNetCore.Mvc.Versioning { + using AspNetCore.Routing; using Http; using System; - using System.Collections.Generic; - using System.Linq; using static System.String; /// /// Provides the implementation for ASP.NET Core. /// [CLSCompliant( false )] - public partial class QueryStringOrHeaderApiVersionReader + public partial class UrlSegmentApiVersionReader { /// /// Reads the service API version value from a request. @@ -18,24 +17,19 @@ public partial class QueryStringOrHeaderApiVersionReader /// The HTTP request to read the API version from. /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. /// Multiple, different API versions were requested. - public override string Read( HttpRequest request ) + public virtual string Read( HttpRequest request ) { - var version = base.Read( request ); - var versions = new HashSet( StringComparer.OrdinalIgnoreCase ); + Arg.NotNull( request, nameof( request ) ); - if ( !IsNullOrEmpty( version ) ) - { - versions.Add( version ); - } - - version = ReadFromHeader( request, HeaderNames ); + var context = request.HttpContext; + var key = context.GetRouteParameterNameAssignedByApiVersionRouteConstraint(); - if ( !IsNullOrEmpty( version ) ) + if ( IsNullOrEmpty( key ) ) { - versions.Add( version ); + return null; } - return versions.EnsureZeroOrOneApiVersions(); + return context.GetRouteValue( key )?.ToString(); } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs new file mode 100644 index 00000000..855784e2 --- /dev/null +++ b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs @@ -0,0 +1,50 @@ +namespace Microsoft.Web.Http.Versioning +{ + using FluentAssertions; + using Routing; + using System; + using System.Net.Http; + using System.Web.Http; + using System.Web.Http.Routing; + using Xunit; + using static System.Net.Http.HttpMethod; + + public class UrlSegmentApiVersionReaderTest + { + [Fact] + public void read_should_retrieve_version_from_url() + { + // arrange + var requestedVersion = "2"; + var configuration = NewConfiguration(); + var request = new HttpRequestMessage( Get, $"http://localhost/api/v{requestedVersion}/test" ); + var reader = new UrlSegmentApiVersionReader(); + + configuration.EnsureInitialized(); + + var routeData = configuration.Routes.GetRouteData( request ); + + request.SetConfiguration( configuration ); + request.SetRouteData( routeData ); + + // act + var version = reader.Read( request ); + + // assert + version.Should().Be( requestedVersion ); + } + + static HttpConfiguration NewConfiguration() + { + var configuration = new HttpConfiguration(); + var constraintResolver = new DefaultInlineConstraintResolver() + { + ConstraintMap = { ["apiVersion"] = typeof( ApiVersionRouteConstraint ) } + }; + + configuration.MapHttpAttributeRoutes( constraintResolver ); + + return configuration; + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Routing/ApiVersionRouteConstraintTest.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Routing/ApiVersionRouteConstraintTest.cs index 2a7d139c..1c7a4be0 100644 --- a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Routing/ApiVersionRouteConstraintTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Routing/ApiVersionRouteConstraintTest.cs @@ -17,7 +17,7 @@ public class ApiVersionRouteConstraintTest { - private class PassThroughRouter : IRouter + class PassThroughRouter : IRouter { public VirtualPathData GetVirtualPath( VirtualPathContext context ) => null; @@ -28,7 +28,16 @@ public Task RouteAsync( RouteContext context ) } } - private static ServiceCollection CreateServices() + static HttpContext NewHttpContext() + { + var httpContext = new Mock(); + + httpContext.SetupProperty( hc => hc.Items, new Dictionary() ); + + return httpContext.Object; + } + + static ServiceCollection CreateServices() { var services = new ServiceCollection(); @@ -41,7 +50,7 @@ private static ServiceCollection CreateServices() return services; } - private static IRouteBuilder CreateRouteBuilder( IServiceProvider services ) + static IRouteBuilder CreateRouteBuilder( IServiceProvider services ) { var app = new Mock(); app.SetupGet( a => a.ApplicationServices ).Returns( services ); @@ -56,7 +65,7 @@ private static IRouteBuilder CreateRouteBuilder( IServiceProvider services ) public void match_should_return_expected_result_for_url_generation( string key, string value, bool expected ) { // arrange - var httpContext = new Mock().Object; + var httpContext = NewHttpContext(); var route = new Mock().Object; var values = new RouteValueDictionary(); var routeDirection = UrlGeneration; @@ -78,16 +87,14 @@ public void match_should_return_expected_result_for_url_generation( string key, public void match_should_return_false_when_route_key_is_missing() { // arrange - var httpContext = new Mock(); + var httpContext = NewHttpContext(); var route = new Mock().Object; var values = new RouteValueDictionary(); var routeDirection = IncomingRequest; var constraint = new ApiVersionRouteConstraint(); - httpContext.SetupProperty( c => c.Items, new Dictionary() ); - // act - var matched = constraint.Match( httpContext.Object, route, "version", values, routeDirection ); + var matched = constraint.Match( httpContext, route, "version", values, routeDirection ); // assert matched.Should().BeFalse(); @@ -100,17 +107,15 @@ public void match_should_return_false_when_route_key_is_missing() public void match_should_return_false_when_route_parameter_is_invalid( string version ) { // arrange - var httpContext = new Mock(); + var httpContext = NewHttpContext(); var route = new Mock().Object; var routeKey = nameof( version ); var values = new RouteValueDictionary() { [routeKey] = version }; var routeDirection = IncomingRequest; var constraint = new ApiVersionRouteConstraint(); - httpContext.SetupProperty( c => c.Items, new Dictionary() ); - // act - var matched = constraint.Match( httpContext.Object, route, routeKey, values, routeDirection ); + var matched = constraint.Match( httpContext, route, routeKey, values, routeDirection ); // assert matched.Should().BeFalse(); @@ -120,16 +125,14 @@ public void match_should_return_false_when_route_parameter_is_invalid( string ve public void match_should_return_true_when_matched() { // arrange - var httpContext = new Mock(); + var httpContext = NewHttpContext(); var route = new Mock().Object; var values = new RouteValueDictionary() { ["version"] = "2.0" }; var routeDirection = IncomingRequest; var constraint = new ApiVersionRouteConstraint(); - httpContext.SetupProperty( c => c.Items, new Dictionary() ); - // act - var matched = constraint.Match( httpContext.Object, route, "version", values, routeDirection ); + var matched = constraint.Match( httpContext, route, "version", values, routeDirection ); // assert matched.Should().BeTrue(); diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs new file mode 100644 index 00000000..06384634 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/UrlSegmentApiVersionReaderTest.cs @@ -0,0 +1,48 @@ +namespace Microsoft.AspNetCore.Mvc.Versioning +{ + using AspNetCore.Routing; + using FluentAssertions; + using Http; + using Http.Features; + using Moq; + using System.Collections.Generic; + using Xunit; + + public class UrlSegmentApiVersionReaderTest + { + [Fact] + public void read_should_retrieve_version_from_url() + { + // arrange + var requestedVersion = "2"; + var request = RequestAfterApiVersionConstraintHasBeenMatched( requestedVersion ); + var reader = new UrlSegmentApiVersionReader(); + + // act + var version = reader.Read( request ); + + // assert + version.Should().Be( requestedVersion ); + } + + static HttpRequest RequestAfterApiVersionConstraintHasBeenMatched( string requestedVersion ) + { + const string ParmaterName = "version"; + const string ItemKey = "MS_ApiVersionRouteConstraint_ParameterName"; + var request = new Mock(); + var routeData = new RouteData() { Values = { [ParmaterName] = requestedVersion } }; + var feature = new RoutingFeature() { RouteData = routeData }; + var featureCollection = new Mock(); + var items = new Dictionary() { [ItemKey] = ParmaterName }; + var httpContext = new Mock(); + var reader = new UrlSegmentApiVersionReader(); + + featureCollection.SetupGet( fc => fc[typeof( IRoutingFeature )] ).Returns( feature ); + httpContext.SetupProperty( c => c.Items, items ); + httpContext.SetupGet( c => c.Features ).Returns( featureCollection.Object ); + request.SetupGet( r => r.HttpContext ).Returns( httpContext.Object ); + + return request.Object; + } + } +} \ No newline at end of file From ff4b50deb0d86e7a38274c26cba59d5e1a699590 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 3 Mar 2017 07:50:54 -0800 Subject: [PATCH 5/8] Refactored API version readers to use composition over inheritance --- .../AdvancedODataWebApiSample/Startup.cs | 11 +-- src/Common/Versioning/ApiVersionReader.cs | 89 +++++++++++++++++-- src/Common/Versioning/ApiVersioningOptions.cs | 9 +- .../Versioning/HeaderApiVersionReader.cs | 6 +- .../Versioning/QueryStringApiVersionReader.cs | 30 +++++-- .../QueryStringOrHeaderApiVersionReader.cs | 39 -------- .../Versioning/ApiVersionReader.cs | 70 --------------- .../Versioning/HeaderApiVersionReader.cs | 25 +++++- .../Versioning/QueryStringApiVersionReader.cs | 16 +++- .../QueryStringOrHeaderApiVersionReader.cs | 42 --------- .../Versioning/ApiVersionReader.cs | 67 -------------- .../Versioning/HeaderApiVersionReader.cs | 26 +++++- .../Versioning/QueryStringApiVersionReader.cs | 13 ++- .../OData/Advanced/AdvancedAcceptanceTest.cs | 11 +-- .../HttpRequestMessageExtensionsTest.cs | 4 +- .../QueryStringApiVersionReaderTest.cs | 3 +- ...QueryStringOrHeaderApiVersionReaderTest.cs | 8 +- .../IServiceCollectionExtensionsTest.cs | 6 +- ...QueryStringOrHeaderApiVersionReaderTest.cs | 8 +- 19 files changed, 206 insertions(+), 277 deletions(-) delete mode 100644 src/Common/Versioning/QueryStringOrHeaderApiVersionReader.cs delete mode 100644 src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersionReader.cs delete mode 100644 src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringOrHeaderApiVersionReader.cs delete mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionReader.cs diff --git a/samples/webapi/AdvancedODataWebApiSample/Startup.cs b/samples/webapi/AdvancedODataWebApiSample/Startup.cs index b36b6b17..ef3e728c 100644 --- a/samples/webapi/AdvancedODataWebApiSample/Startup.cs +++ b/samples/webapi/AdvancedODataWebApiSample/Startup.cs @@ -24,14 +24,9 @@ public void Configuration( IAppBuilder appBuilder ) { o.ReportApiVersions = true; o.AssumeDefaultVersionWhenUnspecified = true; - o.ApiVersionReader = new QueryStringOrHeaderApiVersionReader() - { - HeaderNames = - { - "api-version", - "x-ms-version" - } - }; + o.ApiVersionReader = ApiVersionReader.Combine( + new QueryStringApiVersionReader(), + new HeaderApiVersionReader( "api-version", "x-ms-version" ) ); } ); configuration.EnableCaseInsensitive( true ); configuration.EnableUnqualifiedNameCall( true ); diff --git a/src/Common/Versioning/ApiVersionReader.cs b/src/Common/Versioning/ApiVersionReader.cs index 43311e5d..a91edbb4 100644 --- a/src/Common/Versioning/ApiVersionReader.cs +++ b/src/Common/Versioning/ApiVersionReader.cs @@ -4,19 +4,98 @@ namespace Microsoft.Web.Http.Versioning namespace Microsoft.AspNetCore.Mvc.Versioning #endif { +#if !WEBAPI + using Http; +#endif using System; using System.Linq; + using System.Collections.Generic; + using System.Diagnostics.Contracts; +#if WEBAPI + using HttpRequest = System.Net.Http.HttpRequestMessage; +#endif + using static System.String; /// - /// Represents the base implementation of a service API version reader. + /// Provides utility functions for service API version readers. /// - public abstract partial class ApiVersionReader : IApiVersionReader + public static class ApiVersionReader { /// - /// Initializes a new instance of the class. + /// Returns a new API version reader that is a combination of the specified set. /// - protected ApiVersionReader() + /// The array of + /// API version readers to combine. + /// A new, unioned API version reader. +#if !WEBAPI + [CLSCompliant( false )] +#endif + public static IApiVersionReader Combine( params IApiVersionReader[] apiVersionReaders ) + { + Arg.NotNull( apiVersionReaders, nameof( apiVersionReaders ) ); + Contract.Ensures( Contract.Result() != null ); + Contract.EndContractBlock(); + + if ( apiVersionReaders.Length == 0 ) + { + throw new ArgumentException( SR.ZeroApiVersionReaders, nameof( apiVersionReaders ) ); + } + + return new CombinedApiVersionReader( apiVersionReaders ); + } + + /// + /// Returns a new API version reader that is a combination of the specified set. + /// + /// The sequence of + /// API version readers to combine. + /// A new, unioned API version reader. +#if !WEBAPI + [CLSCompliant( false )] +#endif + public static IApiVersionReader Combine( IEnumerable apiVersionReaders ) { + Arg.NotNull( apiVersionReaders, nameof( apiVersionReaders ) ); + Contract.Ensures( Contract.Result() != null ); + Contract.EndContractBlock(); + + var items = apiVersionReaders.ToArray(); + + if ( items.Length == 0 ) + { + throw new ArgumentException( SR.ZeroApiVersionReaders, nameof( apiVersionReaders ) ); + } + + return new CombinedApiVersionReader( items ); + } + + sealed class CombinedApiVersionReader : IApiVersionReader + { + readonly IApiVersionReader[] apiVersionReaders; + + internal CombinedApiVersionReader( IApiVersionReader[] apiVersionReaders ) + { + Contract.Requires( apiVersionReaders != null ); + Contract.Requires( apiVersionReaders.Length > 0 ); + this.apiVersionReaders = apiVersionReaders; + } + + public string Read( HttpRequest request ) + { + var versions = new HashSet( StringComparer.OrdinalIgnoreCase ); + + foreach ( var apiVersionReader in apiVersionReaders ) + { + var version = apiVersionReader.Read( request ); + + if ( !IsNullOrEmpty( version ) ) + { + versions.Add( version ); + } + } + + return versions.EnsureZeroOrOneApiVersions(); + } } } -} +} \ No newline at end of file diff --git a/src/Common/Versioning/ApiVersioningOptions.cs b/src/Common/Versioning/ApiVersioningOptions.cs index 819a6115..c88b122e 100644 --- a/src/Common/Versioning/ApiVersioningOptions.cs +++ b/src/Common/Versioning/ApiVersioningOptions.cs @@ -7,6 +7,11 @@ namespace Microsoft.AspNetCore.Mvc.Versioning using Conventions; using System; using System.Diagnostics.Contracts; +#if WEBAPI + using static Microsoft.Web.Http.Versioning.ApiVersionReader; +#else + using static Microsoft.AspNetCore.Mvc.Versioning.ApiVersionReader; +#endif /// /// Represents the possible API versioning options for services. @@ -14,7 +19,7 @@ namespace Microsoft.AspNetCore.Mvc.Versioning public partial class ApiVersioningOptions { private ApiVersion defaultApiVersion = ApiVersion.Default; - private IApiVersionReader apiVersionReader = new QueryStringApiVersionReader(); + private IApiVersionReader apiVersionReader = Combine( new QueryStringApiVersionReader(), new UrlSegmentApiVersionReader() ); private IApiVersionSelector apiVersionSelector; private ApiVersionConventionBuilder conventions = new ApiVersionConventionBuilder(); @@ -140,7 +145,7 @@ public ApiVersionConventionBuilder Conventions } set { - Arg.NotNull( conventions, nameof( conventions ) ); + Arg.NotNull( value, nameof( value ) ); conventions = value; } } diff --git a/src/Common/Versioning/HeaderApiVersionReader.cs b/src/Common/Versioning/HeaderApiVersionReader.cs index 32cc4c34..033863a1 100644 --- a/src/Common/Versioning/HeaderApiVersionReader.cs +++ b/src/Common/Versioning/HeaderApiVersionReader.cs @@ -11,14 +11,12 @@ namespace Microsoft.AspNetCore.Mvc.Versioning /// /// Represents a service API version reader that reads the value from a HTTP header. /// - public partial class HeaderApiVersionReader : ApiVersionReader + public partial class HeaderApiVersionReader : IApiVersionReader { /// /// Initializes a new instance of the class. /// - public HeaderApiVersionReader() - { - } + public HeaderApiVersionReader() { } /// /// Initializes a new instance of the class. diff --git a/src/Common/Versioning/QueryStringApiVersionReader.cs b/src/Common/Versioning/QueryStringApiVersionReader.cs index b9ad181f..62d4c8c3 100644 --- a/src/Common/Versioning/QueryStringApiVersionReader.cs +++ b/src/Common/Versioning/QueryStringApiVersionReader.cs @@ -4,21 +4,21 @@ namespace Microsoft.Web.Http.Versioning namespace Microsoft.AspNetCore.Mvc.Versioning #endif { + using Routing; using System; - using System.Linq; + using System.Diagnostics.Contracts; /// /// Represents a service API version reader that reads the value from the query string in a URL. /// - public partial class QueryStringApiVersionReader : ApiVersionReader + public partial class QueryStringApiVersionReader : IApiVersionReader { + string parameterName = "api-version"; + /// /// Initializes a new instance of the class. /// - public QueryStringApiVersionReader() - { - ParameterName = "api-version"; - } + public QueryStringApiVersionReader() { } /// /// Initializes a new instance of the class. @@ -31,10 +31,22 @@ public QueryStringApiVersionReader( string parameterName ) } /// - /// Gets the name of the query parameter to read the service API version from. + /// Gets or sets the name of the query parameter to read the service API version from. /// /// The name of the query parameter to read the service API version from. /// The default value is "api-version". - protected string ParameterName { get; } + public string ParameterName + { + get + { + Contract.Ensures( !string.IsNullOrEmpty( parameterName ) ); + return parameterName; + } + set + { + Arg.NotNullOrEmpty( value, nameof( value ) ); + parameterName = value; + } + } } -} +} \ No newline at end of file diff --git a/src/Common/Versioning/QueryStringOrHeaderApiVersionReader.cs b/src/Common/Versioning/QueryStringOrHeaderApiVersionReader.cs deleted file mode 100644 index 7cedca05..00000000 --- a/src/Common/Versioning/QueryStringOrHeaderApiVersionReader.cs +++ /dev/null @@ -1,39 +0,0 @@ -#if WEBAPI -namespace Microsoft.Web.Http.Versioning -#else -namespace Microsoft.AspNetCore.Mvc.Versioning -#endif -{ - using System; - using System.Collections.Generic; - using static System.StringComparer; - - /// - /// Represents a service API version reader that reads the value from the query string in a URL or HTTP header. - /// - public partial class QueryStringOrHeaderApiVersionReader : QueryStringApiVersionReader - { - /// - /// Initializes a new instance of the class. - /// - public QueryStringOrHeaderApiVersionReader() - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The name of the query string parameter to read the service API version from. - public QueryStringOrHeaderApiVersionReader( string parameterName ) - : base( parameterName ) - { - } - - /// - /// Gets a collection of HTTP header names that the service API version can be read from. - /// - /// A collection of HTTP header names. - /// HTTP header names are evaluated in a case-insensitive manner. - public ICollection HeaderNames { get; } = new HashSet( OrdinalIgnoreCase ); - } -} diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersionReader.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersionReader.cs deleted file mode 100644 index 5b172cca..00000000 --- a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersionReader.cs +++ /dev/null @@ -1,70 +0,0 @@ -namespace Microsoft.Web.Http.Versioning -{ - using System; - using System.Collections.Generic; - using System.Linq; - using System.Net.Http; - using static System.String; - using static System.StringComparison; - - /// - /// Provides the implementation for ASP.NET Web API. - /// - public abstract partial class ApiVersionReader - { - /// - /// Reads the service API version value from a request. - /// - /// The HTTP request to read the API version from. - /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. - public abstract string Read( HttpRequestMessage request ); - - /// - /// Reads the service API version value from the query string of a request. - /// - /// The HTTP request to read the API version from. - /// The name of the query parameter to read from. - /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. - /// Multiple, different API versions were requested. - protected static string ReadFromQueryString( HttpRequestMessage request, string parameterName ) - { - Arg.NotNull( request, nameof( request ) ); - Arg.NotNullOrEmpty( parameterName, nameof( parameterName ) ); - - var parameters = from pair in request.GetQueryNameValuePairs() - where parameterName.Equals( pair.Key, OrdinalIgnoreCase ) && pair.Value.Length > 0 - select pair.Value; - var versions = new HashSet( parameters, StringComparer.OrdinalIgnoreCase ); - - return versions.EnsureZeroOrOneApiVersions(); - } - - /// - /// Reads the service API version value from a HTTP header of a request. - /// - /// The HTTP request to read the API version from. - /// The sequence of HTTP header names to extract the API version from. - /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. - /// When multiple HTTP header names are specified, the first matching HTTP header will be used. - /// Multiple, different API versions were requested. - protected static string ReadFromHeader( HttpRequestMessage request, IEnumerable headerNames ) - { - Arg.NotNull( request, nameof( request ) ); - - var headers = request.Headers; - var versions = new HashSet( StringComparer.OrdinalIgnoreCase ); - - foreach ( var name in headerNames ) - { - IEnumerable values; - - if ( headers.TryGetValues( name, out values ) ) - { - versions.AddRange( values.Where( v => !IsNullOrEmpty( v ) ) ); - } - } - - return versions.EnsureZeroOrOneApiVersions(); - } - } -} diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/HeaderApiVersionReader.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/HeaderApiVersionReader.cs index 16405595..cb306a47 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/HeaderApiVersionReader.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/HeaderApiVersionReader.cs @@ -1,7 +1,10 @@ namespace Microsoft.Web.Http.Versioning { using System; + using System.Collections.Generic; + using System.Linq; using System.Net.Http; + using static System.String; /// /// Provides the implementation for ASP.NET Web API. @@ -14,6 +17,24 @@ public partial class HeaderApiVersionReader /// The HTTP request to read the API version from. /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. /// Multiple, different API versions were requested. - public override string Read( HttpRequestMessage request ) => ReadFromHeader( request, HeaderNames ); + public virtual string Read( HttpRequestMessage request ) + { + Arg.NotNull( request, nameof( request ) ); + + var headers = request.Headers; + var versions = new HashSet( StringComparer.OrdinalIgnoreCase ); + + foreach ( var name in HeaderNames ) + { + var values = default( IEnumerable ); + + if ( headers.TryGetValues( name, out values ) ) + { + versions.AddRange( values.Where( v => !IsNullOrEmpty( v ) ) ); + } + } + + return versions.EnsureZeroOrOneApiVersions(); + } } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringApiVersionReader.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringApiVersionReader.cs index db52f487..1b121a21 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringApiVersionReader.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringApiVersionReader.cs @@ -1,8 +1,10 @@ namespace Microsoft.Web.Http.Versioning { using System; + using System.Collections.Generic; using System.Linq; using System.Net.Http; + using static System.StringComparison; /// /// Provides the implementation for ASP.NET Web API. @@ -15,6 +17,16 @@ public partial class QueryStringApiVersionReader /// The HTTP request to read the API version from. /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. /// Multiple, different API versions were requested. - public override string Read( HttpRequestMessage request ) => ReadFromQueryString( request, ParameterName ); + public virtual string Read( HttpRequestMessage request ) + { + Arg.NotNull( request, nameof( request ) ); + + var parameters = from pair in request.GetQueryNameValuePairs() + where ParameterName.Equals( pair.Key, OrdinalIgnoreCase ) && pair.Value.Length > 0 + select pair.Value; + var versions = new HashSet( parameters, StringComparer.OrdinalIgnoreCase ); + + return versions.EnsureZeroOrOneApiVersions(); + } } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringOrHeaderApiVersionReader.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringOrHeaderApiVersionReader.cs deleted file mode 100644 index b06d3b04..00000000 --- a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/QueryStringOrHeaderApiVersionReader.cs +++ /dev/null @@ -1,42 +0,0 @@ -namespace Microsoft.Web.Http.Versioning -{ - using System; - using System.Collections.Generic; - using System.Linq; - using System.Net.Http; - using static System.String; - - /// - /// Provides the implementation for ASP.NET Web API. - /// - public partial class QueryStringOrHeaderApiVersionReader - { - /// - /// Reads the service API version value from a request. - /// - /// The HTTP request to read the API version from. - /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. - /// Multiple, different API versions were requested. - public override string Read( HttpRequestMessage request ) - { - Arg.NotNull( request, nameof( request ) ); - - var version = base.Read( request ); - var versions = new HashSet( StringComparer.OrdinalIgnoreCase ); - - if ( !IsNullOrEmpty( version )) - { - versions.Add( version ); - } - - version = ReadFromHeader( request, HeaderNames ); - - if ( !IsNullOrEmpty( version ) ) - { - versions.Add( version ); - } - - return versions.EnsureZeroOrOneApiVersions(); - } - } -} diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionReader.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionReader.cs deleted file mode 100644 index 6d018c5a..00000000 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionReader.cs +++ /dev/null @@ -1,67 +0,0 @@ -namespace Microsoft.AspNetCore.Mvc.Versioning -{ - using Extensions.Primitives; - using Http; - using System; - using System.Collections.Generic; - using System.Linq; - using static System.String; - - /// - /// Represents the base implementation of a service API version reader. - /// - [CLSCompliant( false )] - public abstract partial class ApiVersionReader : IApiVersionReader - { - /// - /// Reads the service API version value from a request. - /// - /// The HTTP request to read the API version from. - /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. - public abstract string Read( HttpRequest request ); - - /// - /// Reads the service API version value from the query string of a request. - /// - /// The HTTP request to read the API version from. - /// The name of the query parameter to read from. - /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. - /// Multiple, different API versions were requested. - protected static string ReadFromQueryString( HttpRequest request, string parameterName ) - { - Arg.NotNull( request, nameof( request ) ); - Arg.NotNullOrEmpty( parameterName, nameof( parameterName ) ); - - var versions = new HashSet( request.Query[parameterName].Where( v => !IsNullOrEmpty( v ) ), StringComparer.OrdinalIgnoreCase ); - return versions.EnsureZeroOrOneApiVersions(); - } - - /// - /// Reads the service API version value from a HTTP header of a request. - /// - /// The HTTP request to read the API version from. - /// The sequence of HTTP header names to extract the API version from. - /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. - /// When multiple HTTP header names are specified, the first matching HTTP header will be used. - /// Multiple, different API versions were requested. - protected static string ReadFromHeader( HttpRequest request, IEnumerable headerNames ) - { - Arg.NotNull( request, nameof( request ) ); - - var headers = request.Headers; - var versions = new HashSet( StringComparer.OrdinalIgnoreCase ); - - foreach ( var name in headerNames ) - { - StringValues values; - - if ( headers.TryGetValue( name, out values ) ) - { - versions.AddRange( values.Where( v => !IsNullOrEmpty( v ) ) ); - } - } - - return versions.EnsureZeroOrOneApiVersions(); - } - } -} diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/HeaderApiVersionReader.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/HeaderApiVersionReader.cs index 990d6ee4..aa6513fb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/HeaderApiVersionReader.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/HeaderApiVersionReader.cs @@ -1,7 +1,11 @@ namespace Microsoft.AspNetCore.Mvc.Versioning { + using Extensions.Primitives; using Http; using System; + using System.Collections.Generic; + using System.Linq; + using static System.String; /// /// Provides the implementation for ASP.NET Core. @@ -15,6 +19,24 @@ public partial class HeaderApiVersionReader /// The HTTP request to read the API version from. /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. /// Multiple, different API versions were requested. - public override string Read( HttpRequest request ) => ReadFromHeader( request, HeaderNames ); + public virtual string Read( HttpRequest request ) + { + Arg.NotNull( request, nameof( request ) ); + + var headers = request.Headers; + var versions = new HashSet( StringComparer.OrdinalIgnoreCase ); + + foreach ( var name in HeaderNames ) + { + var values = default( StringValues ); + + if ( headers.TryGetValue( name, out values ) ) + { + versions.AddRange( values.Where( v => !IsNullOrEmpty( v ) ) ); + } + } + + return versions.EnsureZeroOrOneApiVersions(); + } } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/QueryStringApiVersionReader.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/QueryStringApiVersionReader.cs index 7d463255..4e12a014 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/QueryStringApiVersionReader.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/QueryStringApiVersionReader.cs @@ -2,6 +2,9 @@ { using Http; using System; + using System.Collections.Generic; + using System.Linq; + using static System.String; /// /// Provides the implementation for ASP.NET Core. @@ -15,6 +18,12 @@ public partial class QueryStringApiVersionReader /// The HTTP request to read the API version from. /// The raw, unparsed service API version value read from the request or null if request does not contain an API version. /// Multiple, different API versions were requested. - public override string Read( HttpRequest request ) => ReadFromQueryString( request, ParameterName ); + public virtual string Read( HttpRequest request ) + { + Arg.NotNull( request, nameof( request ) ); + + var versions = new HashSet( request.Query[ParameterName].Where( v => !IsNullOrEmpty( v ) ), StringComparer.OrdinalIgnoreCase ); + return versions.EnsureZeroOrOneApiVersions(); + } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/AdvancedAcceptanceTest.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/AdvancedAcceptanceTest.cs index 562965a6..3115559e 100644 --- a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/AdvancedAcceptanceTest.cs +++ b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/AdvancedAcceptanceTest.cs @@ -24,14 +24,9 @@ protected AdvancedAcceptanceTest() { options.ReportApiVersions = true; options.AssumeDefaultVersionWhenUnspecified = true; - options.ApiVersionReader = new QueryStringOrHeaderApiVersionReader() - { - HeaderNames = - { - "api-version", - "x-ms-version" - } - }; + options.ApiVersionReader = ApiVersionReader.Combine( + new QueryStringApiVersionReader(), + new HeaderApiVersionReader( "api-version", "x-ms-version" ) ); } ); var modelBuilder = new VersionedODataModelBuilder( Configuration ) diff --git a/test/Microsoft.AspNet.WebApi.Versioning.Tests/System.Web.Http/HttpRequestMessageExtensionsTest.cs b/test/Microsoft.AspNet.WebApi.Versioning.Tests/System.Web.Http/HttpRequestMessageExtensionsTest.cs index 4dab9673..dd8f3ee8 100644 --- a/test/Microsoft.AspNet.WebApi.Versioning.Tests/System.Web.Http/HttpRequestMessageExtensionsTest.cs +++ b/test/Microsoft.AspNet.WebApi.Versioning.Tests/System.Web.Http/HttpRequestMessageExtensionsTest.cs @@ -45,7 +45,7 @@ public void get_requested_api_version_should_return_null_when_header_is_nullX2C_ // arrange var configuration = new HttpConfiguration(); var request = new HttpRequestMessage(); - var versionReader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version", "x-ms-version" } }; + var versionReader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version", "x-ms-version" ) ); configuration.AddApiVersioning( o => o.ApiVersionReader = versionReader ); request.SetConfiguration( configuration ); @@ -91,7 +91,7 @@ public void get_requested_api_version_should_return_expected_value_from_header( var requestedVersion = new ApiVersion( 1, 0 ); var configuration = new HttpConfiguration(); var request = new HttpRequestMessage(); - var versionReader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { headerName } }; + var versionReader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( headerName ) ); configuration.AddApiVersioning( o => o.ApiVersionReader = versionReader ); request.SetConfiguration( configuration ); diff --git a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringApiVersionReaderTest.cs b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringApiVersionReaderTest.cs index 3b9148da..5011e1df 100644 --- a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringApiVersionReaderTest.cs +++ b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringApiVersionReaderTest.cs @@ -1,5 +1,4 @@ -using Xunit; -namespace Microsoft.Web.Http.Versioning +namespace Microsoft.Web.Http.Versioning { using FluentAssertions; using System; diff --git a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs index a403851c..89ed4e9e 100644 --- a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs +++ b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs @@ -15,7 +15,7 @@ public void read_should_retrieve_version_from_header( string headerName, string { // arrange var request = new HttpRequestMessage(); - var reader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version", "x-ms-version" } }; + var reader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version", "x-ms-version" ) ); request.Headers.TryAddWithoutValidation( headerName, requestedVersion ); @@ -31,7 +31,7 @@ public void read_should_throw_exception_when_ambiguous_api_versions_are_requeste { // arrange var request = new HttpRequestMessage( Get, "http://localhost/test?api-version=2.0" ); - var reader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version" } }; + var reader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version" ) ); request.Headers.TryAddWithoutValidation( "api-version", "1.0" ); @@ -47,7 +47,7 @@ public void read_should_not_throw_exception_when_duplicate_api_versions_are_requ { // arrange var request = new HttpRequestMessage( Get, "http://localhost/test?api-version=1.0" ); - var reader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version" } }; + var reader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version" ) ); request.Headers.TryAddWithoutValidation( "api-version", "1.0" ); @@ -58,4 +58,4 @@ public void read_should_not_throw_exception_when_duplicate_api_versions_are_requ version.Should().Be( "1.0" ); } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensionsTest.cs index 01aba873..2b0f1af8 100644 --- a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensionsTest.cs @@ -33,7 +33,7 @@ public void add_api_versioning_should_configure_mvc_with_default_options() routeConfiguration.Configure( routeOptions ); // assert - services.Single( sd => sd.ServiceType == typeof( IApiVersionReader ) ).ImplementationInstance.Should().BeOfType(); + services.Single( sd => sd.ServiceType == typeof( IApiVersionReader ) ).ImplementationInstance.Should().NotBeNull(); services.Single( sd => sd.ServiceType == typeof( IApiVersionSelector ) ).ImplementationInstance.Should().BeOfType(); services.Single( sd => sd.ServiceType == typeof( IActionSelector ) ).ImplementationType.Should().Be( typeof( ApiVersionActionSelector ) ); mvcOptions.Conventions.Single().Should().BeOfType(); @@ -53,7 +53,7 @@ public void add_api_versioning_should_configure_mvc_with_custom_options() o => { o.ReportApiVersions = true; - o.ApiVersionReader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version" } }; + o.ApiVersionReader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version" ) ); o.ApiVersionSelector = new ConstantApiVersionSelector( new ApiVersion( DateTime.Today ) ); } ); @@ -66,7 +66,7 @@ public void add_api_versioning_should_configure_mvc_with_custom_options() routeConfiguration.Configure( routeOptions ); // assert - services.Single( sd => sd.ServiceType == typeof( IApiVersionReader ) ).ImplementationInstance.Should().BeOfType(); + services.Single( sd => sd.ServiceType == typeof( IApiVersionReader ) ).ImplementationInstance.Should().NotBeNull(); services.Single( sd => sd.ServiceType == typeof( IApiVersionSelector ) ).ImplementationInstance.Should().BeOfType(); services.Single( sd => sd.ServiceType == typeof( IActionSelector ) ).ImplementationType.Should().Be( typeof( ApiVersionActionSelector ) ); services.Single( sd => sd.ServiceType == typeof( ReportApiVersionsAttribute ) ).ImplementationType.Should().Be( typeof( ReportApiVersionsAttribute ) ); diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs index 2349c3a7..69c4da01 100644 --- a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/QueryStringOrHeaderApiVersionReaderTest.cs @@ -17,7 +17,7 @@ public void read_should_retrieve_version_from_header( string headerName, string // arrange var headers = new HeaderDictionary() { [headerName] = requestedVersion }; var request = new Mock(); - var reader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version", "x-ms-version" } }; + var reader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version", "x-ms-version" ) ); request.SetupGet( r => r.Query ).Returns( Mock.Of() ); request.SetupGet( r => r.Headers ).Returns( headers ); @@ -36,7 +36,7 @@ public void read_should_throw_exception_when_ambiguous_api_versions_are_requeste var query = new Mock(); var headers = new HeaderDictionary() { ["api-version"] = new StringValues( new[] { "1.0" } ) }; var request = new Mock(); - var reader = new QueryStringOrHeaderApiVersionReader(){ HeaderNames = { "api-version" } }; + var reader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version" ) ); query.SetupGet( q => q["api-version"] ).Returns( new StringValues( "2.0" ) ); request.SetupProperty( r => r.Query, query.Object ); @@ -56,7 +56,7 @@ public void read_should_not_throw_exception_when_duplicate_api_versions_are_requ var query = new Mock(); var headers = new HeaderDictionary() { ["api-version"] = new StringValues( new[] { "1.0" } ) }; var request = new Mock(); - var reader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version" } }; + var reader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version" ) ); query.SetupGet( q => q["api-version"] ).Returns( new StringValues( "1.0" ) ); request.SetupProperty( r => r.Query, query.Object ); @@ -69,4 +69,4 @@ public void read_should_not_throw_exception_when_duplicate_api_versions_are_requ version.Should().Be( "1.0" ); } } -} +} \ No newline at end of file From a8174e7165c3cb53f12b1c4cad6b3416ab79db83 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 3 Mar 2017 07:56:55 -0800 Subject: [PATCH 6/8] Added new support for providing custom 400 and 405 responses related to API versioning --- ApiVersioningWithSamples.sln | 4 +- src/Common/Versioning/ApiVersioningOptions.cs | 25 +++- src/Common/Versioning/ErrorResponseContext.cs | 30 +++++ .../Versioning/IErrorResponseProvider.cs | 36 ++++++ .../DefaultErrorResponseProvider.cs | 122 ++++++++++++++++++ .../Versioning/ErrorResponseContext.cs | 34 +++++ .../DefaultErrorResponseProvider.cs | 69 ++++++++++ .../Versioning/ErrorResponseContext.cs | 36 ++++++ 8 files changed, 354 insertions(+), 2 deletions(-) create mode 100644 src/Common/Versioning/ErrorResponseContext.cs create mode 100644 src/Common/Versioning/IErrorResponseProvider.cs create mode 100644 src/Microsoft.AspNet.WebApi.Versioning/Versioning/DefaultErrorResponseProvider.cs create mode 100644 src/Microsoft.AspNet.WebApi.Versioning/Versioning/ErrorResponseContext.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/DefaultErrorResponseProvider.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ErrorResponseContext.cs diff --git a/ApiVersioningWithSamples.sln b/ApiVersioningWithSamples.sln index 096ade46..8fe4e022 100644 --- a/ApiVersioningWithSamples.sln +++ b/ApiVersioningWithSamples.sln @@ -49,14 +49,16 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Versioning", "Versioning", src\Common\Versioning\ConstantApiVersionSelector.cs = src\Common\Versioning\ConstantApiVersionSelector.cs src\Common\Versioning\CurrentImplementationApiVersionSelector.cs = src\Common\Versioning\CurrentImplementationApiVersionSelector.cs src\Common\Versioning\DefaultApiVersionSelector.cs = src\Common\Versioning\DefaultApiVersionSelector.cs + src\Common\Versioning\ErrorResponseContext.cs = src\Common\Versioning\ErrorResponseContext.cs src\Common\Versioning\HeaderApiVersionReader.cs = src\Common\Versioning\HeaderApiVersionReader.cs src\Common\Versioning\IApiVersionNeutral.cs = src\Common\Versioning\IApiVersionNeutral.cs src\Common\Versioning\IApiVersionProvider.cs = src\Common\Versioning\IApiVersionProvider.cs src\Common\Versioning\IApiVersionReader.cs = src\Common\Versioning\IApiVersionReader.cs src\Common\Versioning\IApiVersionSelector.cs = src\Common\Versioning\IApiVersionSelector.cs + src\Common\Versioning\IErrorResponseProvider.cs = src\Common\Versioning\IErrorResponseProvider.cs src\Common\Versioning\LowestImplementedApiVersionSelector.cs = src\Common\Versioning\LowestImplementedApiVersionSelector.cs src\Common\Versioning\QueryStringApiVersionReader.cs = src\Common\Versioning\QueryStringApiVersionReader.cs - src\Common\Versioning\QueryStringOrHeaderApiVersionReader.cs = src\Common\Versioning\QueryStringOrHeaderApiVersionReader.cs + src\Common\Versioning\UrlSegmentApiVersionReader.cs = src\Common\Versioning\UrlSegmentApiVersionReader.cs EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "samples", "samples", "{915BB224-B1D0-4E27-A348-67FCC77AAA44}" diff --git a/src/Common/Versioning/ApiVersioningOptions.cs b/src/Common/Versioning/ApiVersioningOptions.cs index c88b122e..7ab8b76b 100644 --- a/src/Common/Versioning/ApiVersioningOptions.cs +++ b/src/Common/Versioning/ApiVersioningOptions.cs @@ -21,6 +21,7 @@ public partial class ApiVersioningOptions private ApiVersion defaultApiVersion = ApiVersion.Default; private IApiVersionReader apiVersionReader = Combine( new QueryStringApiVersionReader(), new UrlSegmentApiVersionReader() ); private IApiVersionSelector apiVersionSelector; + private IErrorResponseProvider errorResponseProvider = new DefaultErrorResponseProvider(); private ApiVersionConventionBuilder conventions = new ApiVersionConventionBuilder(); /// @@ -85,7 +86,7 @@ public ApiVersion DefaultApiVersion /// service API version specified by a client. The default value is the /// , which only reads the service API version from /// the "api-version" query string parameter. Replace the default value with an alternate - /// implementation, such as the , which + /// implementation, such as the , which /// can read the service API version from additional information like HTTP headers. #if !WEBAPI [CLSCompliant( false )] @@ -149,5 +150,27 @@ public ApiVersionConventionBuilder Conventions conventions = value; } } + + /// + /// Gets or sets the object used to generate HTTP error responses related to API versioning. + /// + /// An error response provider object. + /// The default value is an instance of the . +#if !WEBAPI + [CLSCompliant( false )] +#endif + public IErrorResponseProvider ErrorResponses + { + get + { + Contract.Ensures( errorResponseProvider != null ); + return errorResponseProvider; + } + set + { + Arg.NotNull( value, nameof( value ) ); + errorResponseProvider = value; + } + } } } \ No newline at end of file diff --git a/src/Common/Versioning/ErrorResponseContext.cs b/src/Common/Versioning/ErrorResponseContext.cs new file mode 100644 index 00000000..c36cbb35 --- /dev/null +++ b/src/Common/Versioning/ErrorResponseContext.cs @@ -0,0 +1,30 @@ +#if WEBAPI +namespace Microsoft.Web.Http.Versioning +#else +namespace Microsoft.AspNetCore.Mvc.Versioning +#endif +{ + /// + /// Represents the contextual information used when generating HTTP error responses related to API versioning. + /// + public partial class ErrorResponseContext + { + /// + /// Gets the associated error code. + /// + /// The associated error code. + public string Code { get; } + + /// + /// Gets the associated error message. + /// + /// The error message. + public string Message { get; } + + /// + /// Gets the detailed error message. + /// + /// The detailed error message, if any. + public string MessageDetail { get; } + } +} \ No newline at end of file diff --git a/src/Common/Versioning/IErrorResponseProvider.cs b/src/Common/Versioning/IErrorResponseProvider.cs new file mode 100644 index 00000000..db98628e --- /dev/null +++ b/src/Common/Versioning/IErrorResponseProvider.cs @@ -0,0 +1,36 @@ +#if WEBAPI +namespace Microsoft.Web.Http.Versioning +#else +namespace Microsoft.AspNetCore.Mvc.Versioning +#endif +{ +#if WEBAPI + using IActionResult = System.Net.Http.HttpResponseMessage; +#else + using Http; +#endif + using System; + + /// + /// Defines the behavior of an object that provides HTTP error responses related to API versioning. + /// +#if !WEBAPI + [CLSCompliant( false )] +#endif + public interface IErrorResponseProvider + { + /// + /// Creates and returns a new HTTP 400 (Bad Request) given the provided context. + /// + /// The error context used to generate response. + /// The generated response. + IActionResult BadRequest( ErrorResponseContext context ); + + /// + /// Creates and returns a new HTTP 405 (Method Not Allowed) given the provided context. + /// + /// The error context used to generate response. + /// The generated response. + IActionResult MethodNotAllowed( ErrorResponseContext context ); + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/DefaultErrorResponseProvider.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/DefaultErrorResponseProvider.cs new file mode 100644 index 00000000..292784e4 --- /dev/null +++ b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/DefaultErrorResponseProvider.cs @@ -0,0 +1,122 @@ +namespace Microsoft.Web.Http.Versioning +{ + using System.Diagnostics.Contracts; + using System.Net; + using System.Net.Http; + using System.Web.Http; + using static System.String; + + /// + /// Represents the default implementation for creating HTTP error responses related to API versioning. + /// + public class DefaultErrorResponseProvider : IErrorResponseProvider + { + /// + /// Creates and returns a new HTTP 400 (Bad Request) given the provided context. + /// + /// The error context used to generate response. + /// The generated response. + public virtual HttpResponseMessage BadRequest( ErrorResponseContext context ) + { + Arg.NotNull( context, nameof( context ) ); + return CreateErrorResponse( context, HttpStatusCode.BadRequest ); + } + + /// + /// Creates and returns a new HTTP 405 (Method Not Allowed) given the provided context. + /// + /// The error context used to generate response. + /// The generated response. + public virtual HttpResponseMessage MethodNotAllowed( ErrorResponseContext context ) + { + Arg.NotNull( context, nameof( context ) ); + return CreateErrorResponse( context, HttpStatusCode.MethodNotAllowed ); + } + + static HttpResponseMessage CreateErrorResponse( ErrorResponseContext context, HttpStatusCode statusCode ) + { + Contract.Requires( context != null ); + Contract.Ensures( Contract.Result() != null ); + + var error = IsODataRequest( context ) ? CreateODataError( context ) : CreateWebApiError( context ); + return context.Request.CreateErrorResponse( statusCode, error ); + } + + static HttpResponseMessage CreateWebApiBadRequest( ErrorResponseContext context ) => + context.Request.CreateErrorResponse( HttpStatusCode.BadRequest, CreateWebApiError( context ) ); + + static HttpResponseMessage CreateODataBadRequest( ErrorResponseContext context ) => + context.Request.CreateErrorResponse( HttpStatusCode.BadRequest, CreateODataError( context ) ); + + static bool IsODataRequest( ErrorResponseContext context ) + { + Contract.Requires( context != null ); + + var request = context.Request; + var routeValues = request.GetRouteData(); + + if ( routeValues == null ) + { + return false; + } + + if ( !routeValues.Values.ContainsKey( "odataPath" ) ) + { + return false; + } + + return request.GetConfiguration()?.Formatters.JsonFormatter == null; + } + + static HttpError CreateWebApiError( ErrorResponseContext context ) + { + Contract.Requires( context != null ); + Contract.Ensures( Contract.Result() != null ); + + var error = new HttpError(); + var root = new HttpError() { ["Error"] = error }; + + if ( !IsNullOrEmpty( context.Code ) ) + { + error["Code"] = context.Code; + } + + if ( !IsNullOrEmpty( context.Message ) ) + { + error.Message = context.Message; + } + + if ( !IsNullOrEmpty( context.MessageDetail ) && context.Request.ShouldIncludeErrorDetail() == true ) + { + error["InnerError"] = new HttpError( context.MessageDetail ); + } + + return root; + } + + static HttpError CreateODataError( ErrorResponseContext context ) + { + Contract.Requires( context != null ); + Contract.Ensures( Contract.Result() != null ); + + var error = new HttpError(); + + if ( !IsNullOrEmpty( context.Code ) ) + { + error[HttpErrorKeys.ErrorCodeKey] = context.Code; + } + + if ( !IsNullOrEmpty( context.Message ) ) + { + error.Message = context.Message; + } + + if ( !IsNullOrEmpty( context.MessageDetail ) && context.Request.ShouldIncludeErrorDetail() == true ) + { + error[HttpErrorKeys.MessageDetailKey] = context.MessageDetail; + } + + return error; + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ErrorResponseContext.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ErrorResponseContext.cs new file mode 100644 index 00000000..bd79a893 --- /dev/null +++ b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ErrorResponseContext.cs @@ -0,0 +1,34 @@ +namespace Microsoft.Web.Http.Versioning +{ + using System; + using System.Net.Http; + + /// + /// Provides additional implementation specific to ASP.NET Web API. + /// + public partial class ErrorResponseContext + { + /// + /// Initializes a new instance of the class. + /// + /// The current HTTP request. + /// The associated error code. + /// The error message. + /// The detailed error message, if any. + public ErrorResponseContext( HttpRequestMessage request, string code, string message, string messageDetail ) + { + Arg.NotNull( request, nameof( request ) ); + + Request = request; + Code = code; + Message = message; + MessageDetail = messageDetail; + } + + /// + /// Gets the current HTTP request. + /// + /// The current HTTP request. + public HttpRequestMessage Request { get; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/DefaultErrorResponseProvider.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/DefaultErrorResponseProvider.cs new file mode 100644 index 00000000..96143db5 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/DefaultErrorResponseProvider.cs @@ -0,0 +1,69 @@ +namespace Microsoft.AspNetCore.Mvc.Versioning +{ + using Hosting; + using System; + using System.Collections.Generic; + using System.Diagnostics.Contracts; + using static Http.StatusCodes; + using static System.String; + + /// + /// Represents the default implementation for creating HTTP error responses related to API versioning. + /// + [CLSCompliant( false )] + public class DefaultErrorResponseProvider : IErrorResponseProvider + { + /// + /// Creates and returns a new HTTP 400 (Bad Request) given the provided context. + /// + /// The error context used to generate response. + /// The generated response. + public virtual IActionResult BadRequest( ErrorResponseContext context ) + { + Arg.NotNull( context, nameof( context ) ); + return new BadRequestObjectResult( CreateErrorContent( context ) ); + } + + /// + /// Creates and returns a new HTTP 405 (Method Not Allowed) given the provided context. + /// + /// The error context used to generate response. + /// The generated response. + public virtual IActionResult MethodNotAllowed( ErrorResponseContext context ) + { + Arg.NotNull( context, nameof( context ) ); + return new ObjectResult( CreateErrorContent( context ) ) { StatusCode = Status405MethodNotAllowed }; + } + + static object CreateErrorContent( ErrorResponseContext context ) + { + Contract.Requires( context != null ); + Contract.Ensures( Contract.Result() != null ); + + var error = new Dictionary(); + var root = new Dictionary() { ["Error"] = error }; + + if ( !IsNullOrEmpty( context.Code ) ) + { + error["Code"] = context.Code; + } + + if ( !IsNullOrEmpty( context.Message ) ) + { + error["Message"] = context.Message; + } + + if ( !IsNullOrEmpty( context.MessageDetail ) ) + { + var environment = (IHostingEnvironment) context.Request.HttpContext.RequestServices.GetService( typeof( IHostingEnvironment ) ); + + if ( environment?.IsDevelopment() == true ) + { + error["InnerError"] = new Dictionary() { ["Message"] = context.MessageDetail }; + } + } + + return root; + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ErrorResponseContext.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ErrorResponseContext.cs new file mode 100644 index 00000000..911bab30 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ErrorResponseContext.cs @@ -0,0 +1,36 @@ +namespace Microsoft.AspNetCore.Mvc.Versioning +{ + using Http; + using System; + + /// + /// Provides additional implementation specific to ASP.NET Core. + /// + public partial class ErrorResponseContext + { + /// + /// Initializes a new instance of the class. + /// + /// The current HTTP request. + /// The associated error code. + /// The error message. + /// The detailed error message, if any. + [CLSCompliant( false )] + public ErrorResponseContext( HttpRequest request, string code, string message, string messageDetail ) + { + Arg.NotNull( request, nameof( request ) ); + + Request = request; + Code = code; + Message = message; + MessageDetail = messageDetail; + } + + /// + /// Gets the current HTTP request. + /// + /// The current HTTP request. + [CLSCompliant( false )] + public HttpRequest Request { get; } + } +} \ No newline at end of file From 0ecdcdd91ce24ddbb0a26973d38aaf976daeeeab Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 3 Mar 2017 07:59:02 -0800 Subject: [PATCH 7/8] Refactored error respones to return 405 over 400 when appropriate. Fixes #65 --- .../Controllers/ActionSelectorCacheItem.cs | 63 ++------- .../ApiVersionControllerSelector.cs | 3 +- .../HttpResponseExceptionFactory.cs | 86 ++++++++++-- .../Versioning/ApiVersioningOptions.cs | 126 ------------------ .../Versioning/CreateBadRequestDelegate.cs | 16 --- .../Versioning/ApiVersionActionSelector.cs | 40 ++++-- .../Versioning/ApiVersioningOptions.cs | 68 ---------- .../Versioning/BadRequestHandler.cs | 40 +----- .../Versioning/CreateBadRequestDelegate.cs | 17 --- .../Versioning/MethodNotAllowedHandler.cs | 55 ++++++++ .../Versioning/RequestHandler.cs | 46 +++++++ ...le_ODataController_split_into_two_types.cs | 18 ++- ...ed_ODataController_split_into_two_types.cs | 18 ++- ...ed_ODataController_split_into_two_types.cs | 18 ++- ..._split_into_two_types_using_conventions.cs | 18 ++- ..._split_into_two_types_using_conventions.cs | 18 ++- .../ApiVersionControllerSelectorTest.cs | 30 ++++- .../AttributeRoutedTestController.cs | 3 + .../ApiVersionActionSelectorTest.cs | 6 +- 19 files changed, 345 insertions(+), 344 deletions(-) delete mode 100644 src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersioningOptions.cs delete mode 100644 src/Microsoft.AspNet.WebApi.Versioning/Versioning/CreateBadRequestDelegate.cs delete mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersioningOptions.cs delete mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/CreateBadRequestDelegate.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/MethodNotAllowedHandler.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Controllers/ActionSelectorCacheItem.cs b/src/Microsoft.AspNet.WebApi.Versioning/Controllers/ActionSelectorCacheItem.cs index 602e2ea2..ee01cfdf 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/Controllers/ActionSelectorCacheItem.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/Controllers/ActionSelectorCacheItem.cs @@ -35,6 +35,7 @@ private sealed class ActionSelectorCacheItem private readonly CandidateAction[] combinedCandidateActions; private readonly IDictionary actionParameterNames = new Dictionary(); private readonly ILookup combinedActionNameMapping; + private readonly HashSet allowedMethods = new HashSet(); private StandardActionSelectionCache standardActions; internal ActionSelectorCacheItem( HttpControllerDescriptor controllerDescriptor ) @@ -54,6 +55,7 @@ internal ActionSelectorCacheItem( HttpControllerDescriptor controllerDescriptor var actionDescriptor = new ReflectedHttpActionDescriptor( controllerDescriptor, method ); var actionBinding = actionDescriptor.ActionBinding; + allowedMethods.AddRange( actionDescriptor.SupportedHttpMethods ); combinedCandidateActions[i] = new CandidateAction( actionDescriptor ); actionParameterNames.Add( @@ -203,19 +205,18 @@ private HttpResponseMessage CreateSelectionError( HttpControllerContext controll { Contract.Ensures( Contract.Result() != null ); - if ( !controllerContext.ControllerDescriptor.GetApiVersionModel().IsApiVersionNeutral ) - { - return CreateBadRequestResponse( controllerContext ); - } - var actionsFoundByParams = FindMatchingActions( controllerContext, ignoreVerbs: true ); - if ( actionsFoundByParams.Count > 0 ) + if ( actionsFoundByParams.Count == 0 ) { - return CreateMethodNotAllowedResponse( controllerContext, actionsFoundByParams ); + return CreateActionNotFoundResponse( controllerContext ); } - return CreateActionNotFoundResponse( controllerContext ); + var request = controllerContext.Request; + var versionNeutral = controllerContext.ControllerDescriptor.GetApiVersionModel().IsApiVersionNeutral; + var exceptionFactory = new HttpResponseExceptionFactory( request ); + + return exceptionFactory.CreateMethodNotAllowedResponse( versionNeutral, allowedMethods ); } [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Caller is responsible for disposing of response instance." )] @@ -229,30 +230,6 @@ private static HttpResponseMessage CreateBadRequestResponse( HttpControllerConte return exceptionFactory.CreateBadRequestResponse( request.GetRequestedApiVersion() ); } - [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Caller is responsible for disposing of response instance." )] - private static HttpResponseMessage CreateMethodNotAllowedResponse( HttpControllerContext controllerContext, IEnumerable allowedCandidates ) - { - Contract.Requires( controllerContext != null ); - Contract.Requires( allowedCandidates != null ); - Contract.Ensures( Contract.Result() != null ); - - var incomingMethod = controllerContext.Request.Method; - var response = controllerContext.Request.CreateErrorResponse( MethodNotAllowed, SR.ApiControllerActionSelector_HttpMethodNotSupported.FormatDefault( incomingMethod ) ); - var methods = new HashSet(); - - foreach ( var candidate in allowedCandidates ) - { - methods.UnionWith( candidate.SupportedHttpMethods ); - } - - foreach ( var method in methods ) - { - response.Content.Headers.Allow.Add( method.ToString() ); - } - - return response; - } - [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Handled by the caller." )] private HttpResponseMessage CreateActionNotFoundResponse( HttpControllerContext controllerContext ) { @@ -264,22 +241,6 @@ private HttpResponseMessage CreateActionNotFoundResponse( HttpControllerContext return controllerContext.Request.CreateErrorResponse( NotFound, message, messageDetail ); } - [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Handled by the caller." )] - private HttpResponseMessage CreateActionNotFoundResponse( HttpControllerContext controllerContext, string actionName ) - { - Contract.Requires( controllerContext != null ); - Contract.Ensures( Contract.Result() != null ); - - if ( !controllerContext.ControllerDescriptor.GetApiVersionModel().IsApiVersionNeutral ) - { - return CreateBadRequestResponse( controllerContext ); - } - - var message = SR.ResourceNotFound.FormatDefault( controllerContext.Request.RequestUri ); - var messageDetail = SR.ApiControllerActionSelector_ActionNameNotFound.FormatDefault( controllerDescriptor.ControllerName, actionName ); - return controllerContext.Request.CreateErrorResponse( NotFound, message, messageDetail ); - } - private static List GetInitialCandidateWithParameterListForDirectRoutes( HttpControllerContext controllerContext, IEnumerable subRoutes, bool ignoreVerbs ) { Contract.Requires( controllerContext != null ); @@ -344,7 +305,11 @@ private CandidateAction[] GetInitialCandidateList( HttpControllerContext control if ( actionsFoundByName.Length == 0 ) { - throw new HttpResponseException( CreateActionNotFoundResponse( controllerContext, actionName ) ); + var request = controllerContext.Request; + var versionNeutral = controllerContext.ControllerDescriptor.GetApiVersionModel().IsApiVersionNeutral; + var exceptionFactory = new HttpResponseExceptionFactory( request ); + + throw exceptionFactory.NewMethodNotAllowedException( versionNeutral, allowedMethods ); } var candidatesFoundByName = new CandidateAction[actionsFoundByName.Length]; diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ApiVersionControllerSelector.cs b/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ApiVersionControllerSelector.cs index c9fc1382..18645ba2 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ApiVersionControllerSelector.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/ApiVersionControllerSelector.cs @@ -165,7 +165,8 @@ private static void EnsureRequestHasValidApiVersion( HttpRequestMessage request catch ( AmbiguousApiVersionException ex ) { var options = request.GetApiVersioningOptions(); - throw new HttpResponseException( options.CreateBadRequest( request, "AmbiguousApiVersion", ex.Message, null ) ); + var context = new ErrorResponseContext( request, "AmbiguousApiVersion", ex.Message, messageDetail: null ); + throw new HttpResponseException( options.ErrorResponses.BadRequest( context ) ); } } } diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/HttpResponseExceptionFactory.cs b/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/HttpResponseExceptionFactory.cs index 0000d6f0..86e74ea5 100644 --- a/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/HttpResponseExceptionFactory.cs +++ b/src/Microsoft.AspNet.WebApi.Versioning/Dispatcher/HttpResponseExceptionFactory.cs @@ -1,17 +1,23 @@ namespace Microsoft.Web.Http.Dispatcher { + using System; + using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Contracts; + using System.Linq; using System.Net.Http; using System.Web.Http; + using System.Web.Http.Controllers; using System.Web.Http.Dispatcher; using System.Web.Http.Tracing; using Versioning; using static ApiVersion; using static System.Net.HttpStatusCode; + using static System.String; internal sealed class HttpResponseExceptionFactory { + const string Allow = nameof( Allow ); private static readonly string ControllerSelectorCategory = typeof( IHttpControllerSelector ).FullName; private readonly HttpRequestMessage request; @@ -30,8 +36,9 @@ internal HttpResponseException NewNotFoundOrBadRequestException( ControllerSelec CreateBadRequest( conventionRouteResult, directRouteResult ) ?? CreateNotFound( conventionRouteResult ); [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Created exception cannot be disposed. Handled by the caller." )] - internal HttpResponseMessage CreateBadRequestResponse( ApiVersion requestedVersion ) => - requestedVersion == null ? CreateBadRequestForUnspecifiedApiVersionOrInvalidApiVersion() : CreateBadRequestForUnsupportedApiVersion( requestedVersion ); + internal HttpResponseMessage CreateBadRequestResponse( ApiVersion requestedVersion ) => requestedVersion == null ? + CreateBadRequestForUnspecifiedApiVersionOrInvalidApiVersion( versionNeutral: false ) : + CreateBadRequestForUnsupportedApiVersion( requestedVersion ); [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Created exception cannot be disposed. Handled by the caller." )] internal HttpResponseException CreateBadRequest( ApiVersion requestedVersion ) => new HttpResponseException( CreateBadRequestResponse( requestedVersion ) ); @@ -60,17 +67,24 @@ private HttpResponseException CreateBadRequest( ControllerSelectionResult conven } [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Created exception cannot be disposed. Handled by the caller." )] - private HttpResponseMessage CreateBadRequestForUnspecifiedApiVersionOrInvalidApiVersion() + private HttpResponseMessage CreateBadRequestForUnspecifiedApiVersionOrInvalidApiVersion( bool versionNeutral ) { var requestedVersion = request.GetRawRequestedApiVersion(); var parsedVersion = default( ApiVersion ); var message = default( string ); + var context = default( ErrorResponseContext ); - if ( string.IsNullOrEmpty( requestedVersion ) ) + if ( IsNullOrEmpty( requestedVersion ) ) { + if ( versionNeutral ) + { + return null; + } + message = SR.ApiVersionUnspecified; TraceWriter.Info( request, ControllerSelectorCategory, message ); - return Options.CreateBadRequest( request, "ApiVersionUnspecified", message, messageDetail: null ); + context = new ErrorResponseContext( request, "ApiVersionUnspecified", message, messageDetail: null ); + return Options.ErrorResponses.BadRequest( context ); } else if ( TryParse( requestedVersion, out parsedVersion ) ) { @@ -79,10 +93,11 @@ private HttpResponseMessage CreateBadRequestForUnspecifiedApiVersionOrInvalidApi message = SR.VersionedResourceNotSupported.FormatDefault( request.RequestUri, requestedVersion ); var messageDetail = SR.VersionedControllerNameNotFound.FormatDefault( request.RequestUri, requestedVersion ); + context = new ErrorResponseContext( request, "InvalidApiVersion", message, messageDetail ); TraceWriter.Info( request, ControllerSelectorCategory, message ); - return Options.CreateBadRequest( request, "InvalidApiVersion", message, messageDetail ); + return Options.ErrorResponses.BadRequest( context ); } [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Created exception cannot be disposed. Handled by the caller." )] @@ -93,12 +108,67 @@ private HttpResponseMessage CreateBadRequestForUnsupportedApiVersion( ApiVersion var message = SR.VersionedResourceNotSupported.FormatDefault( request.RequestUri, requestedVersion ); var messageDetail = SR.VersionedControllerNameNotFound.FormatDefault( request.RequestUri, requestedVersion ); + var context = new ErrorResponseContext( request, "UnsupportedApiVersion", message, messageDetail ); + + TraceWriter.Info( request, ControllerSelectorCategory, message ); + + return Options.ErrorResponses.BadRequest( context ); + } + + [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Created exception cannot be disposed. Handled by the caller." )] + internal HttpResponseMessage CreateMethodNotAllowedResponse( bool versionNeutral, IEnumerable allowedMethods ) + { + Contract.Requires( allowedMethods != null ); + + var response = CreateBadRequestForUnspecifiedApiVersionOrInvalidApiVersion( versionNeutral ); + + if ( response != null ) + { + return response; + } + + var requestedMethod = request.Method; + var version = request.GetRequestedApiVersion()?.ToString() ?? "(null)"; + var message = default( string ); + var messageDetail = default( string ); + + if ( versionNeutral ) + { + message = SR.VersionedResourceNotSupported.FormatDefault( request.RequestUri, version ); + messageDetail = SR.VersionedControllerNameNotFound.FormatDefault( request.RequestUri, version ); + } + else + { + message = SR.VersionedMethodNotSupported.FormatDefault( version, requestedMethod ); + messageDetail = SR.VersionedActionNameNotFound.FormatDefault( request.RequestUri, requestedMethod, version ); + } TraceWriter.Info( request, ControllerSelectorCategory, message ); - return Options.CreateBadRequest( request, "UnsupportedApiVersion", message, messageDetail ); + var context = new ErrorResponseContext( request, "UnsupportedApiVersion", message, messageDetail ); + + response = Options.ErrorResponses.MethodNotAllowed( context ); + + if ( response.Content == null ) + { + response.Content = new StringContent( Empty ); + response.Content.Headers.ContentType = null; + } + + var headers = response.Content.Headers; + + if ( headers.Allow.Count == 0 ) + { + headers.Allow.AddRange( allowedMethods.Select( m => m.Method ) ); + } + + return response; } + [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Created exception cannot be disposed. Handled by the caller." )] + internal HttpResponseException NewMethodNotAllowedException( bool versionNeutral, IEnumerable allowedMethods ) => + new HttpResponseException( CreateMethodNotAllowedResponse( versionNeutral, allowedMethods ) ); + [SuppressMessage( "Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Created exception cannot be disposed. Handled by the caller." )] private HttpResponseException CreateNotFound( ControllerSelectionResult conventionRouteResult ) { @@ -107,7 +177,7 @@ private HttpResponseException CreateNotFound( ControllerSelectionResult conventi var message = SR.ResourceNotFound.FormatDefault( request.RequestUri ); var messageDetail = default( string ); - if ( string.IsNullOrEmpty( conventionRouteResult.ControllerName ) ) + if ( IsNullOrEmpty( conventionRouteResult.ControllerName ) ) { messageDetail = SR.ControllerNameNotFound.FormatDefault( request.RequestUri ); } diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersioningOptions.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersioningOptions.cs deleted file mode 100644 index fbd553ad..00000000 --- a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/ApiVersioningOptions.cs +++ /dev/null @@ -1,126 +0,0 @@ -namespace Microsoft.Web.Http.Versioning -{ - using System.Diagnostics.Contracts; - using System.Net.Http; - using System.Net.Http.Formatting; - using System.Web.Http; - using static System.Net.HttpStatusCode; - using static System.String; - - /// - /// Provides additional implementation specific to Microsoft ASP.NET Web API. - /// - public partial class ApiVersioningOptions - { - private CreateBadRequestDelegate createBadRequest = CreateDefaultBadRequest; - - /// - /// Gets or sets the function to used to create HTTP 400 (Bad Request) responses related to API versioning. - /// - /// The function to used to create a HTTP 400 (Bad Request) - /// response related to API versioning. - /// The default value generates responses that are compliant with the Microsoft REST API Guidelines. - /// This option should only be changed by service authors that intentionally want to deviate from the - /// established guidance. - public CreateBadRequestDelegate CreateBadRequest - { - get - { - Contract.Ensures( createBadRequest != null ); - return createBadRequest; - } - set - { - Arg.NotNull( value, nameof( value ) ); - createBadRequest = value; - } - } - - private static HttpResponseMessage CreateDefaultBadRequest( HttpRequestMessage request, string code, string message, string messageDetail ) - { - if ( request == null || !IsODataRequest( request ) ) - { - return CreateWebApiBadRequest( request, code, message, messageDetail ); - } - - return CreateODataBadRequest( request, code, message, messageDetail ); - } - - private static HttpResponseMessage CreateWebApiBadRequest( HttpRequestMessage request, string code, string message, string messageDetail ) - { - var error = new HttpError(); - var root = new HttpError() { ["Error"] = error }; - - if ( !IsNullOrEmpty( code ) ) - { - error["Code"] = code; - } - - if ( !IsNullOrEmpty( message ) ) - { - error.Message = message; - } - - if ( !IsNullOrEmpty( messageDetail ) && request?.ShouldIncludeErrorDetail() == true ) - { - error["InnerError"] = new HttpError( messageDetail ); - } - - if ( request == null ) - { - return new HttpResponseMessage( BadRequest ) - { - Content = new ObjectContent( root, new JsonMediaTypeFormatter() ) - }; - } - - return request.CreateErrorResponse( BadRequest, root ); - } - - private static bool IsODataRequest( HttpRequestMessage request ) - { - if ( request == null ) - { - return false; - } - - var routeValues = request.GetRouteData(); - - if ( routeValues == null ) - { - return false; - } - - if ( !routeValues.Values.ContainsKey( "odataPath" ) ) - { - return false; - } - - return request.GetConfiguration()?.Formatters.JsonFormatter == null; - } - - private static HttpResponseMessage CreateODataBadRequest( HttpRequestMessage request, string code, string message, string messageDetail ) - { - Contract.Requires( request != null ); - - var error = new HttpError(); - - if ( !IsNullOrEmpty( code ) ) - { - error[HttpErrorKeys.ErrorCodeKey] = code; - } - - if ( !IsNullOrEmpty( message ) ) - { - error.Message = message; - } - - if ( !IsNullOrEmpty( messageDetail ) && request?.ShouldIncludeErrorDetail() == true ) - { - error[HttpErrorKeys.MessageDetailKey] = messageDetail; - } - - return request.CreateErrorResponse( BadRequest, error ); - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/CreateBadRequestDelegate.cs b/src/Microsoft.AspNet.WebApi.Versioning/Versioning/CreateBadRequestDelegate.cs deleted file mode 100644 index 61da91dd..00000000 --- a/src/Microsoft.AspNet.WebApi.Versioning/Versioning/CreateBadRequestDelegate.cs +++ /dev/null @@ -1,16 +0,0 @@ -namespace Microsoft.Web.Http.Versioning -{ - using System; - using System.Net.Http; - using System.Web.Http; - - /// - /// Represents the function invoked to create a HTTP 400 (Bad Request) response related to API versioning. - /// - /// The current HTTP request. - /// The associated error code. - /// The error message. - /// The detailed error message, if any. - /// A HTTP response representing for status code 400 (Bad Request). - public delegate HttpResponseMessage CreateBadRequestDelegate( HttpRequestMessage request, string code, string message, string messageDetail ); -} \ 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 53aa652c..4d61759b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs @@ -87,11 +87,11 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead var httpContext = context.HttpContext; var apiVersion = default( ApiVersion ); - var badRequest = default( BadRequestHandler ); + var invalidRequestHandler = default( RequestHandler ); - if ( ( badRequest = VerifyRequestedApiVersionIsNotAmbiguous( httpContext, out apiVersion ) ) != null ) + if ( ( invalidRequestHandler = VerifyRequestedApiVersionIsNotAmbiguous( httpContext, out apiVersion ) ) != null ) { - context.Handler = badRequest; + context.Handler = invalidRequestHandler; return null; } @@ -101,9 +101,9 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead if ( finalMatches == null || finalMatches.Count == 0 ) { - if ( ( badRequest = IsValidRequest( selectionContext, candidates ) ) != null ) + if ( ( invalidRequestHandler = IsValidRequest( selectionContext, candidates ) ) != null ) { - context.Handler = badRequest; + context.Handler = invalidRequestHandler; } return null; @@ -181,7 +181,7 @@ where ActionIsSatisfiedBy( action, model, context.RequestedVersion, implicitMatc return bestMatches; } - private BadRequestHandler VerifyRequestedApiVersionIsNotAmbiguous( HttpContext httpContext, out ApiVersion apiVersion ) + private RequestHandler VerifyRequestedApiVersionIsNotAmbiguous( HttpContext httpContext, out ApiVersion apiVersion ) { Contract.Requires( httpContext != null ); @@ -199,7 +199,7 @@ private BadRequestHandler VerifyRequestedApiVersionIsNotAmbiguous( HttpContext h return null; } - private BadRequestHandler IsValidRequest( ActionSelectionContext context, IReadOnlyList candidates ) + private RequestHandler IsValidRequest( ActionSelectionContext context, IReadOnlyList candidates ) { Contract.Requires( context != null ); Contract.Requires( candidates != null ); @@ -213,6 +213,11 @@ private BadRequestHandler IsValidRequest( ActionSelectionContext context, IReadO 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 ) { @@ -228,11 +233,21 @@ private BadRequestHandler IsValidRequest( ActionSelectionContext context, IReadO { 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 @@ -240,10 +255,19 @@ private BadRequestHandler IsValidRequest( ActionSelectionContext context, IReadO 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 new BadRequestHandler( Options, code, message ); + return newRequestHandler( Options, code, message ); } private static IEnumerable MatchVersionNeutralActions( ActionSelectionContext context ) => diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersioningOptions.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersioningOptions.cs deleted file mode 100644 index 0cc9b679..00000000 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersioningOptions.cs +++ /dev/null @@ -1,68 +0,0 @@ -namespace Microsoft.AspNetCore.Mvc.Versioning -{ - using Hosting; - using Http; - using System; - using System.Collections.Generic; - using System.Diagnostics.Contracts; - using static System.String; - - /// - /// Provides additional implementation specific to Microsoft ASP.NET Web API. - /// - public partial class ApiVersioningOptions - { - private CreateBadRequestDelegate createBadRequest = CreateDefaultBadRequest; - - /// - /// Gets or sets the function to used to create HTTP 400 (Bad Request) responses related to API versioning. - /// - /// The function to used to create a HTTP 400 (Bad Request) - /// response related to API versioning. - /// The default value generates responses that are compliant with the Microsoft REST API Guidelines. - /// This option should only be changed by service authors that intentionally want to deviate from the - /// established guidance. - [CLSCompliant( false )] - public CreateBadRequestDelegate CreateBadRequest - { - get - { - Contract.Ensures( createBadRequest != null ); - return createBadRequest; - } - set - { - Arg.NotNull( value, nameof( value ) ); - createBadRequest = value; - } - } - - private static BadRequestObjectResult CreateDefaultBadRequest( HttpRequest request, string code, string message, string messageDetail ) - { - var error = new Dictionary(); - var root = new Dictionary() { ["Error"] = error }; - - if ( !IsNullOrEmpty( code ) ) - { - error["Code"] = code; - } - - if ( !IsNullOrEmpty( message ) ) - { - error["Message"] = message; - } - - if ( !IsNullOrEmpty( messageDetail ) ) - { - var environment = (IHostingEnvironment) request?.HttpContext.RequestServices.GetService( typeof( IHostingEnvironment ) ); - - if ( environment?.IsDevelopment() == true ) - { - error["InnerError"] = new Dictionary() { ["Message"] = messageDetail }; - } - } - - return new BadRequestObjectResult( root ); - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs index 9fae5e7a..470778b2 100644 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/BadRequestHandler.cs @@ -1,46 +1,16 @@ namespace Microsoft.AspNetCore.Mvc.Versioning { - using Abstractions; - using AspNetCore.Routing; using Http; - using System.Diagnostics.Contracts; - using System.Threading.Tasks; - internal sealed class BadRequestHandler + internal sealed class BadRequestHandler : RequestHandler { - private readonly ApiVersioningOptions options; - private readonly string code; - private readonly string message; - - internal BadRequestHandler( ApiVersioningOptions options, string message ) - : this( options, null, message ) - { - } - internal BadRequestHandler( ApiVersioningOptions options, string code, string message ) - { - Contract.Requires( options != null ); - Contract.Requires( !string.IsNullOrEmpty( message ) ); + : base( options, code, message ) { } - this.options = options; - this.message = message; - this.code = code; - } - - internal async Task ExecuteAsync( HttpContext context ) + protected override IActionResult CreateResult( HttpContext context ) { - Contract.Assume( context != null ); - - var actionContext = new ActionContext() - { - HttpContext = context, - RouteData = context.GetRouteData(), - ActionDescriptor = new ActionDescriptor() - }; - var result = options.CreateBadRequest( context.Request, code, message, null ); - await result.ExecuteResultAsync( actionContext ); + var errorContext = new ErrorResponseContext( context.Request, Code, Message, messageDetail: null ); + return Options.ErrorResponses.BadRequest( errorContext ); } - - public static implicit operator RequestDelegate( BadRequestHandler handler ) => handler.ExecuteAsync; } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/CreateBadRequestDelegate.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/CreateBadRequestDelegate.cs deleted file mode 100644 index ba5a8bb0..00000000 --- a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/CreateBadRequestDelegate.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace Microsoft.AspNetCore.Mvc.Versioning -{ - using AspNetCore.Mvc; - using Http; - using System; - - /// - /// Represents the function invoked to create a HTTP 400 (Bad Request) response related to API versioning. - /// - /// The current HTTP request. - /// The associated error code. - /// The error message. - /// The detailed error message, if any. - /// A HTTP response representing for status code 400 (Bad Request). - [CLSCompliant( false )] - public delegate BadRequestObjectResult CreateBadRequestDelegate( HttpRequest request, string code, string message, string messageDetail ); -} \ 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 new file mode 100644 index 00000000..8a5d239e --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/MethodNotAllowedHandler.cs @@ -0,0 +1,55 @@ +namespace Microsoft.AspNetCore.Mvc.Versioning +{ + using Extensions.Primitives; + using Http; + using System; + using System.Diagnostics.Contracts; + using System.Threading.Tasks; + + internal sealed class MethodNotAllowedHandler : RequestHandler + { + readonly string[] allowedMethods; + + internal MethodNotAllowedHandler( ApiVersioningOptions options, string code, string message, string[] allowedMethods ) + : base( options, code, message ) + { + Contract.Requires( allowedMethods != null ); + this.allowedMethods = allowedMethods; + } + + protected override IActionResult CreateResult( HttpContext context ) + { + var errorContext = new ErrorResponseContext( context.Request, Code, Message, messageDetail: null ); + var result = Options.ErrorResponses.MethodNotAllowed( errorContext ); + return allowedMethods.Length == 0 ? result : new AllowHeaderResult( result, allowedMethods ); + } + + sealed class AllowHeaderResult : IActionResult + { + const string Allow = nameof( Allow ); + readonly IActionResult inner; + readonly string[] allowedMethods; + + internal AllowHeaderResult( IActionResult inner, string[] allowedMethods ) + { + Contract.Requires( inner != null ); + Contract.Requires( allowedMethods != null ); + + this.inner = inner; + this.allowedMethods = allowedMethods; + } + + public Task ExecuteResultAsync( ActionContext context ) + { + var headers = context.HttpContext.Response?.Headers; + + if ( headers != null && !headers.ContainsKey( Allow ) ) + { + headers.Add( Allow, new StringValues( allowedMethods ) ); + } + + return inner.ExecuteResultAsync( context ); + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs new file mode 100644 index 00000000..90d77414 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/RequestHandler.cs @@ -0,0 +1,46 @@ +namespace Microsoft.AspNetCore.Mvc.Versioning +{ + using Abstractions; + using AspNetCore.Routing; + using Http; + using System.Diagnostics.Contracts; + using System.Threading.Tasks; + + internal abstract class RequestHandler + { + protected RequestHandler( ApiVersioningOptions options, string code, string message ) + { + Contract.Requires( options != null ); + Contract.Requires( !string.IsNullOrEmpty( message ) ); + + Options = options; + Message = message; + Code = code; + } + + protected ApiVersioningOptions Options { get; } + + protected string Code { get; } + + protected string Message { get; } + + protected abstract IActionResult CreateResult( HttpContext context ); + + internal Task ExecuteAsync( HttpContext context ) + { + Contract.Requires( context != null ); + + var result = CreateResult( context ); + var actionContext = new ActionContext() + { + HttpContext = context, + RouteData = context.GetRouteData(), + ActionDescriptor = new ActionDescriptor() + }; + + return result.ExecuteResultAsync( actionContext ); + } + + public static implicit operator RequestDelegate( RequestHandler handler ) => handler.ExecuteAsync; + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/given/a_mix-in_people_ODataController_split_into_two_types.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/given/a_mix-in_people_ODataController_split_into_two_types.cs index 93cbcf96..7801a491 100644 --- a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/given/a_mix-in_people_ODataController_split_into_two_types.cs +++ b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Advanced/given/a_mix-in_people_ODataController_split_into_two_types.cs @@ -140,8 +140,7 @@ public async Task _patch_should_return_400_when_updating_member_that_does_not_ex [Theory] [InlineData( "1.0" )] [InlineData( "3.0" )] - [InlineData( "4.0" )] - public async Task _patch_should_return_400_when_version_is_unsupported( string apiVersion ) + public async Task _patch_should_return_405_when_version_could_be_supported( string apiVersion ) { // arrange var person = new { lastName = "Me" }; @@ -150,6 +149,21 @@ public async Task _patch_should_return_400_when_version_is_unsupported( string a var response = await PatchAsync( $"api/people(42)?api-version={apiVersion}", person ); var content = await response.Content.ReadAsAsync(); + // assert + response.StatusCode.Should().Be( MethodNotAllowed ); + content.Error.Code.Should().Be( "UnsupportedApiVersion" ); + } + + [Fact] + public async Task _patch_should_return_400_when_version_is_unsupported() + { + // arrange + var person = new { lastName = "Me" }; + + // act + var response = await PatchAsync( $"api/people(42)?api-version=4.0", person ); + var content = await response.Content.ReadAsAsync(); + // assert response.StatusCode.Should().Be( BadRequest ); content.Error.Code.Should().Be( "UnsupportedApiVersion" ); diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_query_string_versioned_ODataController_split_into_two_types.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_query_string_versioned_ODataController_split_into_two_types.cs index e8822329..d70ebcbf 100644 --- a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_query_string_versioned_ODataController_split_into_two_types.cs +++ b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_query_string_versioned_ODataController_split_into_two_types.cs @@ -61,8 +61,7 @@ public async Task _patch_should_return_204() [Theory] [InlineData( "api/people(42)?api-version=1.0" )] [InlineData( "api/people(42)?api-version=3.0" )] - [InlineData( "api/people(42)?api-version=4.0" )] - public async Task _patch_should_return_400_when_version_is_unsupported( string requestUrl ) + public async Task _patch_should_return_405_when_version_could_be_supported( string requestUrl ) { // arrange var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; @@ -71,6 +70,21 @@ public async Task _patch_should_return_400_when_version_is_unsupported( string r var response = await PatchAsync( requestUrl, person ); var content = await response.Content.ReadAsAsync(); + // assert + response.StatusCode.Should().Be( MethodNotAllowed ); + content.Error.Code.Should().Be( "UnsupportedApiVersion" ); + } + + [Fact] + public async Task _patch_should_return_400_when_version_is_unsupported() + { + // arrange + var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; + + // act + var response = await PatchAsync( "api/people(42)?api-version=4.0", person ); + var content = await response.Content.ReadAsAsync(); + // assert response.StatusCode.Should().Be( BadRequest ); content.Error.Code.Should().Be( "UnsupportedApiVersion" ); diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_url_versioned_ODataController_split_into_two_types.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_url_versioned_ODataController_split_into_two_types.cs index a38c54a5..f8a39efd 100644 --- a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_url_versioned_ODataController_split_into_two_types.cs +++ b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Basic/given/a_url_versioned_ODataController_split_into_two_types.cs @@ -46,8 +46,7 @@ public async Task _patch_should_return_204() [Theory] [InlineData( "v1/people(42)" )] [InlineData( "v3/people(42)" )] - [InlineData( "v4/people(42)" )] - public async Task _patch_should_return_400_when_version_is_unsupported( string requestUrl ) + public async Task _patch_should_return_405_when_version_could_be_supported( string requestUrl ) { // arrange var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; @@ -56,6 +55,21 @@ public async Task _patch_should_return_400_when_version_is_unsupported( string r var response = await PatchAsync( requestUrl, person ); var content = await response.Content.ReadAsAsync(); + // assert + response.StatusCode.Should().Be( MethodNotAllowed ); + content.Error.Code.Should().Be( "UnsupportedApiVersion" ); + } + + [Fact] + public async Task _patch_should_return_400_when_version_is_unsupported() + { + // arrange + var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; + + // act + var response = await PatchAsync( "v4/people(42)", person ); + var content = await response.Content.ReadAsAsync(); + // assert response.StatusCode.Should().Be( BadRequest ); content.Error.Code.Should().Be( "UnsupportedApiVersion" ); diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_query_string_versioned_ODataController_split_into_two_types_using_conventions.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_query_string_versioned_ODataController_split_into_two_types_using_conventions.cs index bb062a65..7e8f1f19 100644 --- a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_query_string_versioned_ODataController_split_into_two_types_using_conventions.cs +++ b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_query_string_versioned_ODataController_split_into_two_types_using_conventions.cs @@ -61,8 +61,7 @@ public async Task _patch_should_return_204() [Theory] [InlineData( "api/people(42)?api-version=1.0" )] [InlineData( "api/people(42)?api-version=3.0" )] - [InlineData( "api/people(42)?api-version=4.0" )] - public async Task _patch_should_return_400_when_version_is_unsupported( string requestUrl ) + public async Task _patch_should_return_405_when_version_could_be_supported( string requestUrl ) { // arrange var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; @@ -71,6 +70,21 @@ public async Task _patch_should_return_400_when_version_is_unsupported( string r var response = await PatchAsync( requestUrl, person ); var content = await response.Content.ReadAsAsync(); + // assert + response.StatusCode.Should().Be( MethodNotAllowed ); + content.Error.Code.Should().Be( "UnsupportedApiVersion" ); + } + + [Fact] + public async Task _patch_should_return_400_when_version_is_unsupported() + { + // arrange + var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; + + // act + var response = await PatchAsync( "api/people(42)?api-version=4.0", person ); + var content = await response.Content.ReadAsAsync(); + // assert response.StatusCode.Should().Be( BadRequest ); content.Error.Code.Should().Be( "UnsupportedApiVersion" ); diff --git a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_url_versioned_ODataController_split_into_two_types_using_conventions.cs b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_url_versioned_ODataController_split_into_two_types_using_conventions.cs index fd57cfee..1a0e6f73 100644 --- a/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_url_versioned_ODataController_split_into_two_types_using_conventions.cs +++ b/test/Microsoft.AspNet.WebApi.Acceptance.Tests/OData/Conventions/given/a_url_versioned_ODataController_split_into_two_types_using_conventions.cs @@ -46,8 +46,7 @@ public async Task _patch_should_return_204() [Theory] [InlineData( "v1/people(42)" )] [InlineData( "v3/people(42)" )] - [InlineData( "v4/people(42)" )] - public async Task _patch_should_return_400_when_version_is_unsupported( string requestUrl ) + public async Task _patch_should_return_405_when_version_could_be_supported( string requestUrl ) { // arrange var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; @@ -56,6 +55,21 @@ public async Task _patch_should_return_400_when_version_is_unsupported( string r var response = await PatchAsync( requestUrl, person ); var content = await response.Content.ReadAsAsync(); + // assert + response.StatusCode.Should().Be( MethodNotAllowed ); + content.Error.Code.Should().Be( "UnsupportedApiVersion" ); + } + + [Fact] + public async Task _patch_should_return_400_when_version_is_unsupported() + { + // arrange + var person = new { id = 42, firstName = "John", lastName = "Doe", email = "john.doe@somewhere.com" }; + + // act + var response = await PatchAsync( "v4/people(42)", person ); + var content = await response.Content.ReadAsAsync(); + // assert response.StatusCode.Should().Be( BadRequest ); content.Error.Code.Should().Be( "UnsupportedApiVersion" ); diff --git a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Dispatcher/ApiVersionControllerSelectorTest.cs b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Dispatcher/ApiVersionControllerSelectorTest.cs index deb040e9..c1c805a2 100644 --- a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Dispatcher/ApiVersionControllerSelectorTest.cs +++ b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Dispatcher/ApiVersionControllerSelectorTest.cs @@ -422,6 +422,30 @@ public void select_controller_should_return_400_when_no_version_is_specified_and [Fact] public void select_controller_should_return_400_for_unmatched_action() + { + // arrange + var configuration = AttributeRoutingEnabledConfiguration; + var request = new HttpRequestMessage( Get, "http://localhost/api/test/1?api-version=2.0" ); + + configuration.AddApiVersioning(); + configuration.EnsureInitialized(); + + var routeData = configuration.Routes.GetRouteData( request ); + + request.SetConfiguration( configuration ); + request.SetRouteData( routeData ); + + Action selectController = () => configuration.Services.GetHttpControllerSelector().SelectController( request ); + + // act + var response = selectController.ShouldThrow().Subject.Single().Response; + + // assert + response.StatusCode.Should().Be( BadRequest ); + } + + [Fact] + public void select_controller_should_return_405_for_unmatched_action() { // arrange var configuration = AttributeRoutingEnabledConfiguration; @@ -452,7 +476,7 @@ public void select_controller_should_return_400_for_unmatched_action() var response = selectAction.ShouldThrow().Subject.Single().Response; // assert - response.StatusCode.Should().Be( BadRequest ); + response.StatusCode.Should().Be( MethodNotAllowed ); } [Fact] @@ -943,7 +967,7 @@ public void select_controller_should_return_400_when_requested_api_version_is_am var request = new HttpRequestMessage( Get, "http://localhost/api/test?api-version=2.0" ); request.Headers.TryAddWithoutValidation( "api-version", "1.0" ); - configuration.AddApiVersioning( o => o.ApiVersionReader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version" } } ); + configuration.AddApiVersioning( o => o.ApiVersionReader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version" ) ) ); configuration.EnsureInitialized(); var routeData = configuration.Routes.GetRouteData( request ); @@ -976,7 +1000,7 @@ public async Task select_controller_should_resolve_controller_with_api_versionX2 { options.AssumeDefaultVersionWhenUnspecified = true; options.DefaultApiVersion = new ApiVersion( new DateTime( 2015, 11, 15 ) ); - options.ApiVersionReader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version", "x-ms-version" } }; + options.ApiVersionReader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version", "x-ms-version" ) ); } ); configuration.Routes.MapHttpRoute( "Admin-1", "admin", new { controller = "admin", action = "Get" } ); configuration.Routes.MapHttpRoute( "Admin-2", "admin/seedData", new { controller = "admin", action = "SeedData" } ); diff --git a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Simulators/AttributeRoutedTestController.cs b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Simulators/AttributeRoutedTestController.cs index 1de43bb7..ddc5210c 100644 --- a/test/Microsoft.AspNet.WebApi.Versioning.Tests/Simulators/AttributeRoutedTestController.cs +++ b/test/Microsoft.AspNet.WebApi.Versioning.Tests/Simulators/AttributeRoutedTestController.cs @@ -9,5 +9,8 @@ public sealed class AttributeRoutedTestController : ApiController { [Route] public Task Get() => Task.FromResult( "Test" ); + + [Route( "{id}" )] + public Task Get( string id ) => Task.FromResult( "Test" ); } } diff --git a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs index ff1dba5d..0bc2e3fb 100644 --- a/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs @@ -176,7 +176,7 @@ public async Task select_best_candidate_should_return_404_for_unmatched_controll } [Fact] - public async Task select_best_candidate_should_return_400_for_unmatched_action() + public async Task select_best_candidate_should_return_405_for_unmatched_action() { // arrange var request = new HttpRequestMessage( Post, "api/attributed?api-version=1.0" ); @@ -187,7 +187,7 @@ public async Task select_best_candidate_should_return_400_for_unmatched_action() var response = await server.Client.SendAsync( request ); // assert - response.StatusCode.Should().Be( BadRequest ); + response.StatusCode.Should().Be( MethodNotAllowed ); } } @@ -467,7 +467,7 @@ public async Task select_best_candidate_should_return_correct_controller_for_ver public async Task select_controller_should_return_400_when_requested_api_version_is_ambiguous() { // arrange - Action versioningSetup = o => o.ApiVersionReader = new QueryStringOrHeaderApiVersionReader() { HeaderNames = { "api-version" } }; + Action versioningSetup = o => o.ApiVersionReader = ApiVersionReader.Combine( new QueryStringApiVersionReader(), new HeaderApiVersionReader( "api-version" ) ); using ( var server = new WebServer( versioningSetup ) ) { From ce3175944dbae458f3f38ec5be3ee11dcc54db90 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 3 Mar 2017 08:09:37 -0800 Subject: [PATCH 8/8] Corrected license text to be compliant with Microsoft CELA guidelines --- LICENSE | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/LICENSE b/LICENSE index e646c6b2..4b3ba9df 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ -The MIT License (MIT) +MIT License -Copyright (c) 2015 Commonsense Software +Copyright (c) Microsoft Corporation. All rights reserved. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -18,5 +18,4 @@ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. - +SOFTWARE \ No newline at end of file