From 0e0b26722f1cd2cbea8a6c18d9933a4e58d5571a Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Mon, 13 Jun 2022 19:43:56 -0700 Subject: [PATCH 1/8] Handle possible NRE. Fixes #837 --- .../src/Common/DefaultApiVersionReporter.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Common/src/Common/DefaultApiVersionReporter.cs b/src/Common/src/Common/DefaultApiVersionReporter.cs index 636762d2..5bcc451d 100644 --- a/src/Common/src/Common/DefaultApiVersionReporter.cs +++ b/src/Common/src/Common/DefaultApiVersionReporter.cs @@ -62,7 +62,6 @@ public void Report( HttpResponse response, ApiVersionModel apiVersionModel ) AddApiVersionHeader( headers, ApiDeprecatedVersions, apiVersionModel.DeprecatedApiVersions ); #if NETFRAMEWORK - var request = response.RequestMessage; var statusCode = (int) response.StatusCode; #else var context = response.HttpContext; @@ -75,13 +74,23 @@ public void Report( HttpResponse response, ApiVersionModel apiVersionModel ) } #if NETFRAMEWORK - var dependencyResolver = request.GetConfiguration().DependencyResolver; - var policyManager = dependencyResolver.GetSunsetPolicyManager(); - var name = request.GetActionDescriptor().GetApiVersionMetadata().Name; + if ( response.RequestMessage is not HttpRequestMessage request || + request.GetActionDescriptor()?.GetApiVersionMetadata() is not ApiVersionMetadata metadata ) + { + return; + } + + var name = metadata.Name; + var policyManager = request.GetConfiguration().DependencyResolver.GetSunsetPolicyManager(); var version = request.GetRequestedApiVersion(); #else + if ( context.GetEndpoint()?.Metadata.GetMetadata() is not ApiVersionMetadata metadata ) + { + return; + } + + var name = metadata.Name; var policyManager = context.RequestServices.GetRequiredService(); - var name = context.GetEndpoint()!.Metadata.GetMetadata()?.Name; var version = context.GetRequestedApiVersion(); #endif From d87c1b3385c5ad14d474e700f01dc93bcdab9ad6 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Tue, 14 Jun 2022 10:43:06 -0700 Subject: [PATCH 2/8] Fix possible NRE (netcoreapp3.1 only?) --- .../ApiExplorer/ODataApiDescriptionProvider.cs | 8 +++++++- .../VersionedApiDescriptionProvider.cs | 5 +++++ .../Abstractions/ActionDescriptorExtensions.cs | 11 +++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/AspNetCore/OData/src/Asp.Versioning.OData.ApiExplorer/ApiExplorer/ODataApiDescriptionProvider.cs b/src/AspNetCore/OData/src/Asp.Versioning.OData.ApiExplorer/ApiExplorer/ODataApiDescriptionProvider.cs index a157768e..4c9081d4 100644 --- a/src/AspNetCore/OData/src/Asp.Versioning.OData.ApiExplorer/ApiExplorer/ODataApiDescriptionProvider.cs +++ b/src/AspNetCore/OData/src/Asp.Versioning.OData.ApiExplorer/ApiExplorer/ODataApiDescriptionProvider.cs @@ -102,7 +102,13 @@ public virtual void OnProvidersExecuted( ApiDescriptionProviderContext context ) for ( var i = results.Count - 1; i >= 0; i-- ) { var result = results[i]; - var metadata = result.ActionDescriptor.EndpointMetadata.OfType().ToArray(); + + if ( result.ActionDescriptor.EndpointMetadata is not IList endpointMetadata ) + { + continue; + } + + var metadata = endpointMetadata.OfType().ToArray(); var notOData = metadata.Length == 0; if ( notOData ) diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs index 6fbbb027..4e4ff5d8 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs @@ -179,6 +179,11 @@ private static bool IsUnversioned( ActionDescriptor action ) { var endpointMetadata = action.EndpointMetadata; + if ( endpointMetadata == null ) + { + return true; + } + for ( var i = 0; i < endpointMetadata.Count; i++ ) { if ( endpointMetadata[i] is ApiVersionMetadata ) diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/Abstractions/ActionDescriptorExtensions.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/Abstractions/ActionDescriptorExtensions.cs index aca045d1..61fb9fc0 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/Abstractions/ActionDescriptorExtensions.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/Abstractions/ActionDescriptorExtensions.cs @@ -24,6 +24,11 @@ public static ApiVersionMetadata GetApiVersionMetadata( this ActionDescriptor ac var endpointMetadata = action.EndpointMetadata; + if ( endpointMetadata == null ) + { + return ApiVersionMetadata.Empty; + } + for ( var i = 0; i < endpointMetadata.Count; i++ ) { if ( endpointMetadata[i] is ApiVersionMetadata metadata ) @@ -39,6 +44,12 @@ internal static void AddOrReplaceApiVersionMetadata( this ActionDescriptor actio { var endpointMetadata = action.EndpointMetadata; + if ( endpointMetadata == null ) + { + action.EndpointMetadata = new List() { value }; + return; + } + for ( var i = 0; i < endpointMetadata.Count; i++ ) { if ( endpointMetadata[i] is not ApiVersionMetadata ) From c4a2632d8e9bffdec94c10903a3cb9d06d20c7d0 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Tue, 14 Jun 2022 10:43:37 -0700 Subject: [PATCH 3/8] Skip collation of actions that do not have API versioning metadata. Fixes #839 --- .../src/Asp.Versioning.Mvc/ApiVersionCollator.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/ApiVersionCollator.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/ApiVersionCollator.cs index 544e0c13..85aaa55d 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/ApiVersionCollator.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc/ApiVersionCollator.cs @@ -2,8 +2,10 @@ namespace Asp.Versioning; +using Asp.Versioning.ApplicationModels; using Asp.Versioning.Conventions; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Controllers; using System.Runtime.CompilerServices; using static Asp.Versioning.ApiVersionMapping; @@ -102,6 +104,9 @@ protected virtual string GetControllerName( ActionDescriptor action ) return NamingConvention.GroupName( name ); } + [MethodImpl( MethodImplOptions.AggressiveInlining )] + private static bool IsUnversioned( ActionDescriptor action ) => action.GetApiVersionMetadata() == ApiVersionMetadata.Empty; + private IEnumerable> GroupActionsByController( IList actions ) { var groups = new Dictionary>( StringComparer.OrdinalIgnoreCase ); @@ -109,6 +114,12 @@ private IEnumerable> GroupActionsByController( I for ( var i = 0; i < actions.Count; i++ ) { var action = actions[i]; + + if ( IsUnversioned( action ) ) + { + continue; + } + var key = GetControllerName( action ); if ( string.IsNullOrEmpty( key ) ) From 713ff0f6792635016e6b509029b3d97cf32427db Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Tue, 14 Jun 2022 13:21:30 -0700 Subject: [PATCH 4/8] Provide work around for ASP.NET Core bug 41773. Resolves #830 --- .../ApiVersionParameterDescriptionContext.cs | 76 ++++++++++++++++++- .../IApiVersioningBuilderExtensions.cs | 13 +++- .../VersionedApiDescriptionProvider.cs | 31 ++++++++ 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiVersionParameterDescriptionContext.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiVersionParameterDescriptionContext.cs index 3cf7c1a4..43a7e597 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiVersionParameterDescriptionContext.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiVersionParameterDescriptionContext.cs @@ -8,6 +8,7 @@ namespace Asp.Versioning.ApiExplorer; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.AspNetCore.Routing.Template; using static Asp.Versioning.ApiVersionParameterLocation; using static System.Linq.Enumerable; using static System.StringComparison; @@ -43,6 +44,11 @@ public ApiVersionParameterDescriptionContext( optional = options.AssumeDefaultVersionWhenUnspecified && apiVersion == options.DefaultApiVersion; } + // intentionally an internal property so the public contract doesn't change. this will be removed + // once the ASP.NET Core team fixes the bug + // BUG: https://github.com/dotnet/aspnetcore/issues/41773 + internal IInlineConstraintResolver? ConstraintResolver { get; set; } + /// /// Gets the associated API description. /// @@ -160,7 +166,8 @@ protected virtual void AddHeader( string name ) protected virtual void UpdateUrlSegment() { var parameter = FindByRouteConstraintType( ApiDescription ) ?? - FindByRouteConstraintName( ApiDescription, Options.RouteConstraintName ); + FindByRouteConstraintName( ApiDescription, Options.RouteConstraintName ) ?? + TryCreateFromRouteTemplate( ApiDescription, ConstraintResolver ); if ( parameter == null ) { @@ -309,6 +316,73 @@ routeInfo.Constraints is IEnumerable constraints && return default; } + private static ApiParameterDescription? TryCreateFromRouteTemplate( ApiDescription description, IInlineConstraintResolver? constraintResolver ) + { + if ( constraintResolver == null ) + { + return default; + } + + var relativePath = description.RelativePath; + + if ( string.IsNullOrEmpty( relativePath ) ) + { + return default; + } + + var constraints = new List(); + var template = TemplateParser.Parse( relativePath ); + var constraintName = default( string ); + + for ( var i = 0; i < template.Parameters.Count; i++ ) + { + var match = false; + var parameter = template.Parameters[i]; + + foreach ( var inlineConstraint in parameter.InlineConstraints ) + { + var constraint = constraintResolver.ResolveConstraint( inlineConstraint.Constraint )!; + + constraints.Add( constraint ); + + if ( constraint is ApiVersionRouteConstraint ) + { + match = true; + constraintName = inlineConstraint.Constraint; + } + } + + if ( !match ) + { + continue; + } + + constraints.TrimExcess(); + + // ASP.NET Core does not discover route parameters without using Reflection in 6.0. unclear if it will be fixed before 7.0 + // BUG: https://github.com/dotnet/aspnetcore/issues/41773 + // REF: https://github.com/dotnet/aspnetcore/blob/release/6.0/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs#L323 + var result = new ApiParameterDescription() + { + Name = parameter.Name!, + RouteInfo = new() + { + Constraints = constraints, + DefaultValue = parameter.DefaultValue, + IsOptional = parameter.IsOptional || parameter.DefaultValue != null, + }, + Source = BindingSource.Path, + }; + var token = $"{parameter.Name}:{constraintName}"; + + description.RelativePath = relativePath.Replace( token, parameter.Name, Ordinal ); + description.ParameterDescriptions.Insert( 0, result ); + return result; + } + + return default; + } + private ApiParameterDescription NewApiVersionParameter( string name, BindingSource source ) { var parameter = new ApiParameterDescription() diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/DependencyInjection/IApiVersioningBuilderExtensions.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/DependencyInjection/IApiVersioningBuilderExtensions.cs index 438a7bd5..a845bf61 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/DependencyInjection/IApiVersioningBuilderExtensions.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/DependencyInjection/IApiVersioningBuilderExtensions.cs @@ -5,6 +5,8 @@ namespace Microsoft.Extensions.DependencyInjection; using Asp.Versioning; using Asp.Versioning.ApiExplorer; using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; using static ServiceDescriptor; @@ -60,6 +62,15 @@ private static void AddApiExplorerServices( IServiceCollection services ) services.AddMvcCore().AddApiExplorer(); services.TryAddSingleton, ApiExplorerOptionsFactory>(); services.TryAddSingleton(); - services.TryAddEnumerable( Transient() ); + + // use internal constructor until ASP.NET Core fixes their bug + // BUG: https://github.com/dotnet/aspnetcore/issues/41773 + services.TryAddEnumerable( + Transient( + sp => new( + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService>() ) ) ); } } \ No newline at end of file diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs index 4e4ff5d8..bce9e94c 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/VersionedApiDescriptionProvider.cs @@ -2,10 +2,12 @@ namespace Asp.Versioning.ApiExplorer; +using Asp.Versioning.Routing; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Options; using static Asp.Versioning.ApiVersionMapping; using static System.Globalization.CultureInfo; @@ -18,6 +20,7 @@ namespace Asp.Versioning.ApiExplorer; public class VersionedApiDescriptionProvider : IApiDescriptionProvider { private readonly IOptions options; + private readonly IInlineConstraintResolver constraintResolver; private ApiVersionModelMetadata? modelMetadata; /// @@ -31,9 +34,19 @@ public VersionedApiDescriptionProvider( ISunsetPolicyManager sunsetPolicyManager, IModelMetadataProvider modelMetadataProvider, IOptions options ) + : this( sunsetPolicyManager, modelMetadataProvider, new SimpleConstraintResolver( options ), options ) { } + + // intentionally hiding IInlineConstraintResolver from public signature until ASP.NET Core fixes their bug + // BUG: https://github.com/dotnet/aspnetcore/issues/41773 + internal VersionedApiDescriptionProvider( + ISunsetPolicyManager sunsetPolicyManager, + IModelMetadataProvider modelMetadataProvider, + IInlineConstraintResolver constraintResolver, + IOptions options ) { SunsetPolicyManager = sunsetPolicyManager; ModelMetadataProvider = modelMetadataProvider; + this.constraintResolver = constraintResolver; this.options = options; } @@ -83,6 +96,7 @@ protected virtual void PopulateApiVersionParameters( ApiDescription apiDescripti var parameterSource = Options.ApiVersionParameterSource; var context = new ApiVersionParameterDescriptionContext( apiDescription, apiVersion, ModelMetadata, Options ); + context.ConstraintResolver = constraintResolver; parameterSource.AddParameters( context ); } @@ -247,4 +261,21 @@ private IEnumerable FlattenApiVersions( IList descri return versions; } + + private sealed class SimpleConstraintResolver : IInlineConstraintResolver + { + private readonly IOptions options; + + internal SimpleConstraintResolver( IOptions options ) => this.options = options; + + public IRouteConstraint? ResolveConstraint( string inlineConstraint ) + { + if ( options.Value.RouteConstraintName == inlineConstraint ) + { + return new ApiVersionRouteConstraint(); + } + + return default; + } + } } \ No newline at end of file From 1580738d4494ab2eb0e3a76c5c968f0102356552 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Tue, 14 Jun 2022 14:49:03 -0700 Subject: [PATCH 5/8] Clone IsDefaultResponse in API Explorer. Related #823 --- .../Asp.Versioning.Mvc.ApiExplorer/ApiDescriptionExtensions.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiDescriptionExtensions.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiDescriptionExtensions.cs index b6eda4e6..a4faf70f 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiDescriptionExtensions.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiDescriptionExtensions.cs @@ -175,6 +175,7 @@ internal static ApiResponseType Clone( this ApiResponseType responseType ) { var clone = new ApiResponseType() { + IsDefaultResponse = responseType.IsDefaultResponse, ModelMetadata = responseType.ModelMetadata, StatusCode = responseType.StatusCode, Type = responseType.Type, From 2ace5bbc5945bb7be57e856d8c219c3247aacc5b Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 17 Jun 2022 10:14:18 -0700 Subject: [PATCH 6/8] Retard OData to 8.0.2. Resolves #836 --- .../OData/src/Asp.Versioning.OData/Asp.Versioning.OData.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AspNetCore/OData/src/Asp.Versioning.OData/Asp.Versioning.OData.csproj b/src/AspNetCore/OData/src/Asp.Versioning.OData/Asp.Versioning.OData.csproj index 29d88a6d..53289f8f 100644 --- a/src/AspNetCore/OData/src/Asp.Versioning.OData/Asp.Versioning.OData.csproj +++ b/src/AspNetCore/OData/src/Asp.Versioning.OData/Asp.Versioning.OData.csproj @@ -25,7 +25,7 @@ - + From 51c94910f99ac92ae9dfa766dd06be6caac5a7c1 Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 17 Jun 2022 10:14:51 -0700 Subject: [PATCH 7/8] Update OData examples to use current version --- asp.sln | 3 +++ examples/AspNetCore/OData/Directory.Build.props | 11 +++++++++++ 2 files changed, 14 insertions(+) create mode 100644 examples/AspNetCore/OData/Directory.Build.props diff --git a/asp.sln b/asp.sln index 727c3fae..fbcd4c37 100644 --- a/asp.sln +++ b/asp.sln @@ -130,6 +130,9 @@ EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "WebApi", "WebApi", "{E0E64F6F-FB0C-4534-B815-2217700B50BA}" EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "OData", "OData", "{49EA6476-901C-4D4F-8E45-98BC8A2780EB}" + ProjectSection(SolutionItems) = preProject + examples\AspNetCore\OData\Directory.Build.props = examples\AspNetCore\OData\Directory.Build.props + EndProjectSection EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "BasicWebApiExample", "examples\AspNet\WebApi\BasicWebApiExample\BasicWebApiExample.csproj", "{B0457E07-161A-4ED0-949A-8CF7EFA765F5}" EndProject diff --git a/examples/AspNetCore/OData/Directory.Build.props b/examples/AspNetCore/OData/Directory.Build.props new file mode 100644 index 00000000..48fa0067 --- /dev/null +++ b/examples/AspNetCore/OData/Directory.Build.props @@ -0,0 +1,11 @@ + + + + + + + + + + \ No newline at end of file From f5cd8e50f569fb0f641cca0bd2dd673ae2b0c6fa Mon Sep 17 00:00:00 2001 From: Chris Martinez Date: Fri, 17 Jun 2022 12:46:38 -0700 Subject: [PATCH 8/8] Fix incorrect index variable in loop --- .../ApiExplorer/VersionedApiExplorer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AspNet/WebApi/src/Asp.Versioning.WebApi.ApiExplorer/ApiExplorer/VersionedApiExplorer.cs b/src/AspNet/WebApi/src/Asp.Versioning.WebApi.ApiExplorer/ApiExplorer/VersionedApiExplorer.cs index 4a706ef2..d828b34f 100644 --- a/src/AspNet/WebApi/src/Asp.Versioning.WebApi.ApiExplorer/ApiExplorer/VersionedApiExplorer.cs +++ b/src/AspNet/WebApi/src/Asp.Versioning.WebApi.ApiExplorer/ApiExplorer/VersionedApiExplorer.cs @@ -296,7 +296,7 @@ protected virtual ApiDescriptionGroupCollection InitializeApiDescriptions() for ( var j = 0; j < apiDescriptions.Count; j++ ) { - apiDescriptions[i].TryUpdateRelativePathAndRemoveApiVersionParameter( Options ); + apiDescriptions[j].TryUpdateRelativePathAndRemoveApiVersionParameter( Options ); } }