Skip to content

Commit

Permalink
IEdmModelSelector should use IApiVersionSelector when possible for un…
Browse files Browse the repository at this point in the history 
…specified API version. Fixes #745
  • Loading branch information
@commonsensesoftware
commonsensesoftware committed Mar 21, 2022
1 parent 2c0ab86 commit 3267a33
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 53 deletions.
117 changes: 97 additions & 20 deletions src/Common.OData/OData.Edm/EdmModelSelector.cs
View file
Expand Up @@ -3,60 +3,105 @@
#if !WEBAPI
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Versioning;
#endif
using Microsoft.Extensions.DependencyInjection;
#if WEBAPI
using Microsoft.Web.Http;
using Microsoft.Web.Http.Versioning;
#endif
using System;
using System.Collections.Generic;
using System.Linq;
#if WEBAPI
using System.Net.Http;
using System.Web.Http;
using HttpRequest = System.Net.Http.HttpRequestMessage;
#endif

/// <summary>
/// Represents an <see cref="IEdmModelSelector">EDM model selector</see>.
/// </summary>
#if WEBAPI
#if !WEBAPI
[CLSCompliant( false )]
#endif
public class EdmModelSelector : IEdmModelSelector
{
readonly ApiVersion maxVersion;
readonly IApiVersionSelector selector;

/// <summary>
/// Initializes a new instance of the <see cref="EdmModelSelector"/> class.
/// </summary>
/// <param name="models">The <see cref="IEnumerable{T}">sequence</see> of <see cref="IEdmModel">models</see> to select from.</param>
/// <param name="defaultApiVersion">The default <see cref="ApiVersion">API version</see>.</param>
[Obsolete( "This constructor will be removed in the next major version. Use the constructor with IApiVersionSelector instead." )]
public EdmModelSelector( IEnumerable<IEdmModel> models, ApiVersion defaultApiVersion )
: this( models, defaultApiVersion, new ConstantApiVersionSelector( defaultApiVersion ) ) { }

/// <summary>
/// Initializes a new instance of the <see cref="EdmModelSelector"/> class.
/// </summary>
/// <param name="models">The <see cref="IEnumerable{T}">sequence</see> of <see cref="IEdmModel">models</see> to select from.</param>
/// <param name="defaultApiVersion">The default <see cref="ApiVersion">API version</see>.</param>
/// <param name="apiVersionSelector">The <see cref="IApiVersionSelector">selector</see> used to choose API versions.</param>
public EdmModelSelector( IEnumerable<IEdmModel> models, ApiVersion defaultApiVersion, IApiVersionSelector apiVersionSelector )
{
var versions = new List<ApiVersion>();
var collection = new Dictionary<ApiVersion, IEdmModel>();
if ( models == null )
{
throw new ArgumentNullException( nameof( models ) );
}

if ( defaultApiVersion == null )
{
throw new ArgumentNullException( nameof( defaultApiVersion ) );
}

selector = apiVersionSelector ?? throw new ArgumentNullException( nameof( apiVersionSelector ) );

foreach ( var model in models ?? throw new ArgumentNullException( nameof( models ) ) )
List<ApiVersion> versions;
Dictionary<ApiVersion, IEdmModel> collection;

switch ( models )
{
var annotation = model.GetAnnotationValue<ApiVersionAnnotation>( model );
case IList<IEdmModel> list:
versions = new( list.Count );
collection = new( list.Count );

if ( annotation == null )
{
throw new ArgumentException( LocalSR.MissingAnnotation.FormatDefault( typeof( ApiVersionAnnotation ).Name ) );
}
for ( var i = 0; i < list.Count; i++ )
{
AddVersionFromModel( list[i], versions, collection );
}

var version = annotation.ApiVersion;
break;
case IReadOnlyList<IEdmModel> list:
versions = new( list.Count );
collection = new( list.Count );

collection.Add( version, model );
versions.Add( version );
for ( var i = 0; i < list.Count; i++ )
{
AddVersionFromModel( list[i], versions, collection );
}

break;
default:
versions = new();
collection = new();

foreach ( var model in models )
{
AddVersionFromModel( model, versions, collection );
}
#if !WEBAPI
collection.TrimExcess();
#endif
break;
}

versions.Sort();
#pragma warning disable IDE0056 // Use index operator (cannot be used in web api)
maxVersion = versions.Count == 0 ? defaultApiVersion : versions[versions.Count - 1];
#pragma warning restore IDE0056
#if !WEBAPI
collection.TrimExcess();
#endif
ApiVersions = versions.ToArray();
Models = collection;
}
Expand Down Expand Up @@ -85,13 +130,45 @@ public EdmModelSelector( IEnumerable<IEdmModel> models, ApiVersion defaultApiVer
return default;
}

#if WEBAPI
var version = serviceProvider.GetService<HttpRequestMessage>()?.GetRequestedApiVersion();
#else
var version = serviceProvider.GetService<HttpRequest>()?.HttpContext.GetRequestedApiVersion();
var request = serviceProvider.GetService<HttpRequest>();

if ( request is null )
{
return Models[maxVersion];
}

var version = request
#if !WEBAPI
.HttpContext
#endif
.GetRequestedApiVersion();

if ( version is null )
{
var model = new ApiVersionModel( ApiVersions, Enumerable.Empty<ApiVersion>() );

if ( ( version = selector.SelectVersion( request, model ) ) is null )
{
return Models[maxVersion];
}
}

return Models.TryGetValue( version, out var edm ) ? edm : Models[maxVersion];
}

static void AddVersionFromModel( IEdmModel model, IList<ApiVersion> versions, IDictionary<ApiVersion, IEdmModel> collection )
{
var annotation = model.GetAnnotationValue<ApiVersionAnnotation>( model );

if ( annotation == null )
{
throw new ArgumentException( LocalSR.MissingAnnotation.FormatDefault( typeof( ApiVersionAnnotation ).Name ) );
}

var version = annotation.ApiVersion;

return version != null && Models.TryGetValue( version, out var model ) ? model : Models[maxVersion];
collection.Add( version, model );
versions.Add( version );
}
}
}
16 changes: 10 additions & 6 deletions src/Microsoft.AspNet.OData.Versioning/OData/IContainerBuilderExtensions.cs
View file
Expand Up @@ -26,9 +26,11 @@ public static class IContainerBuilderExtensions
.AddService( Transient, sp => sp.GetRequiredService<IEdmModelSelector>().SelectModel( sp ) )
.AddService(
Singleton,
sp => (IEdmModelSelector) new EdmModelSelector(
models,
sp.GetRequiredService<HttpConfiguration>().GetApiVersioningOptions().DefaultApiVersion ) )
sp =>
{
var options = sp.GetRequiredService<HttpConfiguration>().GetApiVersioningOptions();
return (IEdmModelSelector) new EdmModelSelector( models, options.DefaultApiVersion, options.ApiVersionSelector );
} )
.AddService(
Singleton,
sp => CreateDefaultWithAttributeRouting(
Expand All @@ -50,9 +52,11 @@ public static class IContainerBuilderExtensions
.AddService( Transient, sp => sp.GetRequiredService<IEdmModelSelector>().SelectModel( sp ) )
.AddService(
Singleton,
sp => (IEdmModelSelector) new EdmModelSelector(
models,
sp.GetRequiredService<HttpConfiguration>().GetApiVersioningOptions().DefaultApiVersion ) )
sp =>
{
var options = sp.GetRequiredService<HttpConfiguration>().GetApiVersioningOptions();
return (IEdmModelSelector) new EdmModelSelector( models, options.DefaultApiVersion, options.ApiVersionSelector );
} )
.AddService( Singleton, sp => AddOrUpdate( routingConventions.ToList() ).AsEnumerable() );
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.AspNet.WebApi.Versioning/Controllers/ApiVersionActionSelector.cs
View file
Expand Up @@ -14,7 +14,7 @@
/// </summary>
public partial class ApiVersionActionSelector : IHttpActionSelector
{
readonly object cacheKey = new object();
readonly object cacheKey = new();
ActionSelectorCacheItem? fastCache;

/// <summary>
Expand Down
15 changes: 5 additions & 10 deletions ...ft.AspNetCore.OData.Versioning/AspNetCore.Mvc/Versioning/ODataApiVersionActionSelector.cs
View file
Expand Up @@ -106,7 +106,7 @@ public override IReadOnlyList<ActionDescriptor> SelectCandidates( RouteContext c
{
foreach ( var candidate in candidates )
{
if ( !( candidate.AttributeRouteInfo is ODataAttributeRouteInfo info ) ||
if ( candidate.AttributeRouteInfo is not ODataAttributeRouteInfo info ||
!comparer.Equals( routePrefix, info.RoutePrefix ) )
{
continue;
Expand Down Expand Up @@ -237,16 +237,11 @@ static bool RequestHasBody( HttpContext context )
{
var method = context.Request.Method.ToUpperInvariant();

switch ( method )
return method switch
{
case "POST":
case "PUT":
case "PATCH":
case "MERGE":
return true;
}

return false;
"POST" or "PUT" or "PATCH" or "MERGE" => true,
_ => false,
};
}

static bool ActionAcceptsMethod( ActionDescriptor action, string method ) =>
Expand Down
16 changes: 10 additions & 6 deletions src/Microsoft.AspNetCore.OData.Versioning/OData/IContainerBuilderExtensions.cs
View file
Expand Up @@ -32,9 +32,11 @@ public static class IContainerBuilderExtensions
Singleton,
child => child.WithParent(
serviceProvider,
sp => (IEdmModelSelector) new EdmModelSelector(
models,
sp.GetRequiredService<IOptions<ApiVersioningOptions>>().Value.DefaultApiVersion ) ) )
sp =>
{
var options = sp.GetRequiredService<IOptions<ApiVersioningOptions>>().Value;
return (IEdmModelSelector) new EdmModelSelector( models, options.DefaultApiVersion, options.ApiVersionSelector );
} ) )
.AddService(
Singleton,
child => child.WithParent( serviceProvider, sp => CreateDefaultWithAttributeRouting( routeName, sp ).AsEnumerable() ) );
Expand All @@ -59,9 +61,11 @@ public static class IContainerBuilderExtensions
Singleton,
child => child.WithParent(
serviceProvider,
sp => (IEdmModelSelector) new EdmModelSelector(
models,
sp.GetRequiredService<IOptions<ApiVersioningOptions>>().Value.DefaultApiVersion ) ) )
sp =>
{
var options = sp.GetRequiredService<IOptions<ApiVersioningOptions>>().Value;
return (IEdmModelSelector) new EdmModelSelector( models, options.DefaultApiVersion, options.ApiVersionSelector );
} ) )
.AddService( Singleton, child => child.WithParent( serviceProvider, sp => AddOrUpdate( routingConventions.ToList() ).AsEnumerable() ) );
}
}
2 changes: 1 addition & 1 deletion test/Acceptance.Test.Shared/AcceptanceTest.cs
View file
Expand Up @@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Mvc
public abstract partial class AcceptanceTest
{
const string JsonMediaType = "application/json";
static readonly HttpMethod Patch = new HttpMethod( "PATCH" );
static readonly HttpMethod Patch = new( "PATCH" );
readonly HttpServerFixture fixture;

protected AcceptanceTest( HttpServerFixture fixture ) => this.fixture = fixture;
Expand Down
9 changes: 5 additions & 4 deletions ...oft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs
View file
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData.Edm;
using Microsoft.Web.Http;
using Microsoft.Web.Http.Versioning;
using System;
using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -234,7 +235,7 @@ public void substitute_should_generate_type_for_action_parameters()

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
Expand Down Expand Up @@ -272,7 +273,7 @@ public void substitute_should_generate_type_for_action_parameters_with_substitut

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
Expand Down Expand Up @@ -315,7 +316,7 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
Expand Down Expand Up @@ -357,7 +358,7 @@ public void substitute_should_generate_types_for_actions_with_the_same_name_in_d

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operations = model.FindDeclaredOperations( qualifiedName ).Select( o => (IEdmAction) o ).ToArray();
Expand Down
11 changes: 6 additions & 5 deletions ...AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs
View file
Expand Up @@ -2,16 +2,17 @@
{
using FluentAssertions;
using FluentAssertions.Common;
using System.Runtime.Serialization;
using Microsoft.AspNet.OData.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Versioning;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData.Edm;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Reflection.Emit;
using System.Runtime.Serialization;
using Xunit;

public class DefaultModelTypeBuilderTest
Expand Down Expand Up @@ -236,7 +237,7 @@ public void substitute_should_generate_type_for_action_parameters()

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
Expand Down Expand Up @@ -274,7 +275,7 @@ public void substitute_should_generate_type_for_action_parameters_with_substitut

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
Expand Down Expand Up @@ -317,7 +318,7 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
Expand Down Expand Up @@ -359,7 +360,7 @@ public void substitute_should_generate_types_for_actions_with_the_same_name_in_d

model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );

var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default, new ConstantApiVersionSelector( ApiVersion.Default ) );
var context = NewContext( model );
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
var operations = model.FindDeclaredOperations( qualifiedName ).Select( o => (IEdmAction) o ).ToArray();
Expand Down

0 comments on commit 3267a33

Please sign in to comment.