Skip to content

Commit a76647a

Browse files
Use selector when determining if 1st version parameter is optional. Fixes #1025
1 parent b39c6d7 commit a76647a

File tree

5 files changed

+63
-14
lines changed

5 files changed

+63
-14
lines changed

Diff for: src/AspNet/WebApi/src/Asp.Versioning.WebApi.ApiExplorer/Description/ApiVersionParameterDescriptionContext.cs

+25-3
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ public class ApiVersionParameterDescriptionContext : IApiVersionParameterDescrip
3030
public ApiVersionParameterDescriptionContext( ApiDescription apiDescription, ApiVersion apiVersion, ApiExplorerOptions options )
3131
{
3232
Options = options ?? throw new ArgumentNullException( nameof( options ) );
33-
ApiDescription = apiDescription;
34-
ApiVersion = apiVersion;
35-
optional = options.AssumeDefaultVersionWhenUnspecified && apiVersion == options.DefaultApiVersion;
33+
ApiDescription = apiDescription ?? throw new ArgumentNullException( nameof( apiDescription ) );
34+
ApiVersion = apiVersion ?? throw new ArgumentNullException( nameof( apiVersion ) );
35+
optional = FirstParameterIsOptional( apiDescription, apiVersion, options );
3636
}
3737

3838
/// <summary>
@@ -242,4 +242,26 @@ private static void CloneFormattersAndAddMediaTypeParameter( NameValueHeaderValu
242242
formatters.Add( formatter );
243243
}
244244
}
245+
246+
private static bool FirstParameterIsOptional(
247+
ApiDescription apiDescription,
248+
ApiVersion apiVersion,
249+
ApiExplorerOptions options )
250+
{
251+
if ( !options.AssumeDefaultVersionWhenUnspecified )
252+
{
253+
return false;
254+
}
255+
256+
var mapping = ApiVersionMapping.Explicit | ApiVersionMapping.Implicit;
257+
var model = apiDescription.ActionDescriptor.GetApiVersionMetadata().Map( mapping );
258+
ApiVersion defaultApiVersion;
259+
260+
using ( var request = new HttpRequestMessage() )
261+
{
262+
defaultApiVersion = options.ApiVersionSelector.SelectVersion( request, model );
263+
}
264+
265+
return apiVersion == defaultApiVersion;
266+
}
245267
}

Diff for: src/AspNet/WebApi/test/Asp.Versioning.WebApi.ApiExplorer.Tests/Description/ApiVersionParameterDescriptionContextTest.cs

+12-4
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,19 @@ public void add_parameter_should_add_optional_parameter_when_allowed()
234234
var configuration = new HttpConfiguration();
235235
var action = NewActionDescriptor();
236236
var description = new ApiDescription() { ActionDescriptor = action };
237-
var version = new ApiVersion( 1, 0 );
238-
var options = new ApiExplorerOptions( configuration );
237+
var version = new ApiVersion( 2.0 );
238+
var options = new ApiExplorerOptions( configuration )
239+
{
240+
ApiVersionSelector = new ConstantApiVersionSelector( version ),
241+
};
239242

240243
action.Configuration = configuration;
241-
configuration.AddApiVersioning( o => o.AssumeDefaultVersionWhenUnspecified = true );
244+
configuration.AddApiVersioning(
245+
options =>
246+
{
247+
options.DefaultApiVersion = ApiVersion.Default;
248+
options.AssumeDefaultVersionWhenUnspecified = true;
249+
} );
242250

243251
var context = new ApiVersionParameterDescriptionContext( description, version, options );
244252

@@ -255,7 +263,7 @@ public void add_parameter_should_add_optional_parameter_when_allowed()
255263
ParameterDescriptor = new
256264
{
257265
ParameterName = "api-version",
258-
DefaultValue = "1.0",
266+
DefaultValue = "2.0",
259267
IsOptional = true,
260268
Configuration = configuration,
261269
ActionDescriptor = action,

Diff for: src/AspNetCore/WebApi/src/Asp.Versioning.Mvc.ApiExplorer/ApiVersionParameterDescriptionContext.cs

+19-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public ApiVersionParameterDescriptionContext(
4141
ApiDescription = apiDescription ?? throw new ArgumentNullException( nameof( apiDescription ) );
4242
ApiVersion = apiVersion ?? throw new ArgumentNullException( nameof( apiVersion ) );
4343
ModelMetadata = modelMetadata ?? throw new ArgumentNullException( nameof( modelMetadata ) );
44-
optional = options.AssumeDefaultVersionWhenUnspecified && apiVersion == options.DefaultApiVersion;
44+
optional = FirstParameterIsOptional( apiDescription, apiVersion, options );
4545
}
4646

4747
// intentionally an internal property so the public contract doesn't change. this will be removed
@@ -440,4 +440,22 @@ private void RemoveAllParametersExcept( ApiParameterDescription parameter )
440440
}
441441
}
442442
}
443+
444+
private static bool FirstParameterIsOptional(
445+
ApiDescription apiDescription,
446+
ApiVersion apiVersion,
447+
ApiExplorerOptions options )
448+
{
449+
if ( !options.AssumeDefaultVersionWhenUnspecified )
450+
{
451+
return false;
452+
}
453+
454+
var mapping = ApiVersionMapping.Explicit | ApiVersionMapping.Implicit;
455+
var model = apiDescription.ActionDescriptor.GetApiVersionMetadata().Map( mapping );
456+
var context = new Microsoft.AspNetCore.Http.DefaultHttpContext();
457+
var defaultApiVersion = options.ApiVersionSelector.SelectVersion( context.Request, model );
458+
459+
return apiVersion == defaultApiVersion;
460+
}
443461
}

Diff for: src/AspNetCore/WebApi/test/Asp.Versioning.Mvc.ApiExplorer.Tests/ApiVersionParameterDescriptionContextTest.cs

+4-3
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,14 @@ public void add_parameter_should_add_descriptor_for_media_type_parameter()
249249
public void add_parameter_should_add_optional_parameter_when_allowed()
250250
{
251251
// arrange
252-
var version = new ApiVersion( 1, 0 );
252+
var version = new ApiVersion( 2.0 );
253253
var description = NewApiDescription( version );
254254
var modelMetadata = new Mock<ModelMetadata>( ModelMetadataIdentity.ForType( typeof( string ) ) ).Object;
255255
var options = new ApiExplorerOptions()
256256
{
257-
DefaultApiVersion = version,
257+
DefaultApiVersion = ApiVersion.Default,
258258
ApiVersionParameterSource = new QueryStringApiVersionReader(),
259+
ApiVersionSelector = new ConstantApiVersionSelector( version ),
259260
AssumeDefaultVersionWhenUnspecified = true,
260261
};
261262
var context = new ApiVersionParameterDescriptionContext( description, version, modelMetadata, options );
@@ -270,7 +271,7 @@ public void add_parameter_should_add_optional_parameter_when_allowed()
270271
Name = "api-version",
271272
ModelMetadata = modelMetadata,
272273
Source = BindingSource.Query,
273-
DefaultValue = (object) "1.0",
274+
DefaultValue = (object) "2.0",
274275
IsRequired = false,
275276
Type = typeof( string ),
276277
},

Diff for: src/AspNetCore/WebApi/test/Asp.Versioning.Mvc.ApiExplorer.Tests/TestActionDescriptorCollectionProvider.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ namespace Asp.Versioning.ApiExplorer;
77

88
internal sealed class TestActionDescriptorCollectionProvider : IActionDescriptorCollectionProvider
99
{
10-
private readonly Lazy<ActionDescriptorCollection> collection = new( CreateActionDescriptors );
10+
private readonly Lazy<ActionDescriptorCollection> collection;
1111

12-
public TestActionDescriptorCollectionProvider() { }
12+
public TestActionDescriptorCollectionProvider() => collection = new( CreateActionDescriptors );
1313

1414
public TestActionDescriptorCollectionProvider( ActionDescriptor action, params ActionDescriptor[] otherActions )
1515
{
@@ -21,7 +21,7 @@ public TestActionDescriptorCollectionProvider( ActionDescriptor action, params A
2121
}
2222
else
2323
{
24-
actions = new ActionDescriptor[otherActions.Length];
24+
actions = new ActionDescriptor[otherActions.Length + 1];
2525
actions[0] = action;
2626
Array.Copy( otherActions, 0, actions, 1, otherActions.Length );
2727
}

0 commit comments

Comments
 (0)