Use selector when determining if 1st version parameter is optional. F…

…ixes #1025
dotnet · Oct 9, 2023 · 8b0e9c4 · 8b0e9c4
1 parent 6f4a7ed
commit 8b0e9c4
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 14 deletions.
diff --git a/...rc/Asp.Versioning.WebApi.ApiExplorer/Description/ApiVersionParameterDescriptionContext.cs b/...rc/Asp.Versioning.WebApi.ApiExplorer/Description/ApiVersionParameterDescriptionContext.cs
@@ -30,9 +30,9 @@ public class ApiVersionParameterDescriptionContext : IApiVersionParameterDescrip
     public ApiVersionParameterDescriptionContext( ApiDescription apiDescription, ApiVersion apiVersion, ApiExplorerOptions options )
     {
         Options = options ?? throw new ArgumentNullException( nameof( options ) );
-        ApiDescription = apiDescription;
-        ApiVersion = apiVersion;
-        optional = options.AssumeDefaultVersionWhenUnspecified && apiVersion == options.DefaultApiVersion;
+        ApiDescription = apiDescription ?? throw new ArgumentNullException( nameof( apiDescription ) );
+        ApiVersion = apiVersion ?? throw new ArgumentNullException( nameof( apiVersion ) );
+        optional = FirstParameterIsOptional( apiDescription, apiVersion, options );
     }
 
     /// <summary>
@@ -242,4 +242,26 @@ private static void CloneFormattersAndAddMediaTypeParameter( NameValueHeaderValu
             formatters.Add( formatter );
         }
     }
+
+    private static bool FirstParameterIsOptional(
+        ApiDescription apiDescription,
+        ApiVersion apiVersion,
+        ApiExplorerOptions options )
+    {
+        if ( !options.AssumeDefaultVersionWhenUnspecified )
+        {
+            return false;
+        }
+
+        var mapping = ApiVersionMapping.Explicit | ApiVersionMapping.Implicit;
+        var model = apiDescription.ActionDescriptor.GetApiVersionMetadata().Map( mapping );
+        ApiVersion defaultApiVersion;
+
+        using ( var request = new HttpRequestMessage() )
+        {
+            defaultApiVersion = options.ApiVersionSelector.SelectVersion( request, model );
+        }
+
+        return apiVersion == defaultApiVersion;
+    }
 }
diff --git a/...sioning.WebApi.ApiExplorer.Tests/Description/ApiVersionParameterDescriptionContextTest.cs b/...sioning.WebApi.ApiExplorer.Tests/Description/ApiVersionParameterDescriptionContextTest.cs
@@ -234,11 +234,19 @@ public void add_parameter_should_add_optional_parameter_when_allowed()
         var configuration = new HttpConfiguration();
         var action = NewActionDescriptor();
         var description = new ApiDescription() { ActionDescriptor = action };
-        var version = new ApiVersion( 1, 0 );
-        var options = new ApiExplorerOptions( configuration );
+        var version = new ApiVersion( 2.0 );
+        var options = new ApiExplorerOptions( configuration )
+        {
+            ApiVersionSelector = new ConstantApiVersionSelector( version ),
+        };
 
         action.Configuration = configuration;
-        configuration.AddApiVersioning( o => o.AssumeDefaultVersionWhenUnspecified = true );
+        configuration.AddApiVersioning(
+            options =>
+            {
+                options.DefaultApiVersion = ApiVersion.Default;
+                options.AssumeDefaultVersionWhenUnspecified = true;
+            } );
 
         var context = new ApiVersionParameterDescriptionContext( description, version, options );
 
@@ -255,7 +263,7 @@ public void add_parameter_should_add_optional_parameter_when_allowed()
                 ParameterDescriptor = new
                 {
                     ParameterName = "api-version",
-                    DefaultValue = "1.0",
+                    DefaultValue = "2.0",
                     IsOptional = true,
                     Configuration = configuration,
                     ActionDescriptor = action,

diff --git a/...etCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiVersionParameterDescriptionContext.cs b/...etCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiVersionParameterDescriptionContext.cs
@@ -41,7 +41,7 @@ public ApiVersionParameterDescriptionContext(
         ApiDescription = apiDescription ?? throw new ArgumentNullException( nameof( apiDescription ) );
         ApiVersion = apiVersion ?? throw new ArgumentNullException( nameof( apiVersion ) );
         ModelMetadata = modelMetadata ?? throw new ArgumentNullException( nameof( modelMetadata ) );
-        optional = options.AssumeDefaultVersionWhenUnspecified && apiVersion == options.DefaultApiVersion;
+        optional = FirstParameterIsOptional( apiDescription, apiVersion, options );
     }
 
     // intentionally an internal property so the public contract doesn't change. this will be removed
@@ -440,4 +440,22 @@ private void RemoveAllParametersExcept( ApiParameterDescription parameter )
             }
         }
     }
+
+    private static bool FirstParameterIsOptional(
+        ApiDescription apiDescription,
+        ApiVersion apiVersion,
+        ApiExplorerOptions options )
+    {
+        if ( !options.AssumeDefaultVersionWhenUnspecified )
+        {
+            return false;
+        }
+
+        var mapping = ApiVersionMapping.Explicit | ApiVersionMapping.Implicit;
+        var model = apiDescription.ActionDescriptor.GetApiVersionMetadata().Map( mapping );
+        var context = new Microsoft.AspNetCore.Http.DefaultHttpContext();
+        var defaultApiVersion = options.ApiVersionSelector.SelectVersion( context.Request, model );
+
+        return apiVersion == defaultApiVersion;
+    }
 }
diff --git a/...pi/test/Asp.Versioning.Mvc.ApiExplorer.Tests/ApiVersionParameterDescriptionContextTest.cs b/...pi/test/Asp.Versioning.Mvc.ApiExplorer.Tests/ApiVersionParameterDescriptionContextTest.cs
@@ -249,13 +249,14 @@ public void add_parameter_should_add_descriptor_for_media_type_parameter()
     public void add_parameter_should_add_optional_parameter_when_allowed()
     {
         // arrange
-        var version = new ApiVersion( 1, 0 );
+        var version = new ApiVersion( 2.0 );
         var description = NewApiDescription( version );
         var modelMetadata = new Mock<ModelMetadata>( ModelMetadataIdentity.ForType( typeof( string ) ) ).Object;
         var options = new ApiExplorerOptions()
         {
-            DefaultApiVersion = version,
+            DefaultApiVersion = ApiVersion.Default,
             ApiVersionParameterSource = new QueryStringApiVersionReader(),
+            ApiVersionSelector = new ConstantApiVersionSelector( version ),
             AssumeDefaultVersionWhenUnspecified = true,
         };
         var context = new ApiVersionParameterDescriptionContext( description, version, modelMetadata, options );
@@ -270,7 +271,7 @@ public void add_parameter_should_add_optional_parameter_when_allowed()
                 Name = "api-version",
                 ModelMetadata = modelMetadata,
                 Source = BindingSource.Query,
-                DefaultValue = (object) "1.0",
+                DefaultValue = (object) "2.0",
                 IsRequired = false,
                 Type = typeof( string ),
             },

diff --git a/...ebApi/test/Asp.Versioning.Mvc.ApiExplorer.Tests/TestActionDescriptorCollectionProvider.cs b/...ebApi/test/Asp.Versioning.Mvc.ApiExplorer.Tests/TestActionDescriptorCollectionProvider.cs
@@ -7,9 +7,9 @@ namespace Asp.Versioning.ApiExplorer;
 
 internal sealed class TestActionDescriptorCollectionProvider : IActionDescriptorCollectionProvider
 {
-    private readonly Lazy<ActionDescriptorCollection> collection = new( CreateActionDescriptors );
+    private readonly Lazy<ActionDescriptorCollection> collection;
 
-    public TestActionDescriptorCollectionProvider() { }
+    public TestActionDescriptorCollectionProvider() => collection = new( CreateActionDescriptors );
 
     public TestActionDescriptorCollectionProvider( ActionDescriptor action, params ActionDescriptor[] otherActions )
     {
@@ -21,7 +21,7 @@ public TestActionDescriptorCollectionProvider( ActionDescriptor action, params A
         }
         else
         {
-            actions = new ActionDescriptor[otherActions.Length];
+            actions = new ActionDescriptor[otherActions.Length + 1];
             actions[0] = action;
             Array.Copy( otherActions, 0, actions, 1, otherActions.Length );
         }