Skip to content

Commit 261564c

Browse files
Chris Martinezcommonsensesoftware
Chris Martinez
authored and
commonsensesoftware
committed
Fix lookup of EDM model. Fixes #675 
1 parent 93aa313 commit 261564c

File tree

9 files changed

+95
-57
lines changed

9 files changed

+95
-57
lines changed

‎src/Common.OData.ApiExplorer/AspNet.OData/ClassProperty.cs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,21 @@ internal ClassProperty( PropertyInfo clrProperty, Type propertyType )
2323
Attributes = clrProperty.DeclaredAttributes().ToArray();
2424
}
2525

26-
internal ClassProperty( IServiceProvider services, IEdmOperationParameter parameter, IModelTypeBuilder typeBuilder )
26+
internal ClassProperty( IEdmOperationParameter parameter, TypeSubstitutionContext context )
2727
{
2828
Name = parameter.Name;
2929

30-
var context = new TypeSubstitutionContext( services, typeBuilder );
31-
var edmModel = services.GetRequiredService<IEdmModel>();
32-
3330
if ( parameter.Type.IsCollection() )
3431
{
3532
var collectionType = parameter.Type.AsCollection();
36-
var elementType = collectionType.ElementType().Definition.GetClrType( edmModel )!;
33+
var elementType = collectionType.ElementType().Definition.GetClrType( context.Model )!;
3734
var substitutedType = elementType.SubstituteIfNecessary( context );
3835

3936
Type = typeof( IEnumerable<> ).MakeGenericType( substitutedType );
4037
}
4138
else
4239
{
43-
var parameterType = parameter.Type.Definition.GetClrType( edmModel )!;
40+
var parameterType = parameter.Type.Definition.GetClrType( context.Model )!;
4441

4542
Type = parameterType.SubstituteIfNecessary( context );
4643
}

‎src/Common.OData.ApiExplorer/AspNet.OData/DefaultModelTypeBuilder.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ public Type NewActionParameters( IServiceProvider services, IEdmAction action, A
5555
var key = new EdmTypeKey( fullTypeName, apiVersion );
5656
var type = paramTypes.GetOrAdd( key, _ =>
5757
{
58-
var properties = action.Parameters.Where( p => p.Name != "bindingParameter" ).Select( p => new ClassProperty( services, p, this ) );
58+
var context = new TypeSubstitutionContext( services, this, apiVersion );
59+
var properties = action.Parameters.Where( p => p.Name != "bindingParameter" ).Select( p => new ClassProperty( p, context ) );
5960
var signature = new ClassSignature( fullTypeName, properties, apiVersion );
6061
var moduleBuilder = modules.GetOrAdd( apiVersion, CreateModuleForApiVersion );
6162

‎src/Common.OData.ApiExplorer/AspNet.OData/TypeSubstitutionContext.cs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,15 @@
99
using Microsoft.Web.Http;
1010
#endif
1111
using System;
12-
using System.Collections.Generic;
13-
using System.Reflection;
1412

1513
/// <summary>
1614
/// Represents a type substitution context.
1715
/// </summary>
1816
public class TypeSubstitutionContext
1917
{
20-
private readonly Lazy<IEdmModel> model;
21-
private readonly Lazy<ApiVersion> apiVersion;
18+
readonly IServiceProvider? serviceProvider;
19+
IEdmModel? model;
20+
ApiVersion? apiVersion;
2221

2322
/// <summary>
2423
/// Initializes a new instance of the <see cref="TypeSubstitutionContext"/> class.
@@ -27,8 +26,7 @@ public class TypeSubstitutionContext
2726
/// <param name="modelTypeBuilder">The associated <see cref="IModelTypeBuilder">model type builder</see>.</param>
2827
public TypeSubstitutionContext( IEdmModel model, IModelTypeBuilder modelTypeBuilder )
2928
{
30-
this.model = new Lazy<IEdmModel>( () => model );
31-
apiVersion = new Lazy<ApiVersion>( () => Model.GetAnnotationValue<ApiVersionAnnotation>( Model )?.ApiVersion ?? ApiVersion.Default );
29+
this.model = model;
3230
ModelTypeBuilder = modelTypeBuilder;
3331
}
3432

@@ -38,24 +36,25 @@ public TypeSubstitutionContext( IEdmModel model, IModelTypeBuilder modelTypeBuil
3836
/// <param name="serviceProvider">The <see cref="IServiceProvider">service provider</see> that the
3937
/// <see cref="IEdmModel">EDM model</see> can be resolved from.</param>
4038
/// <param name="modelTypeBuilder">The associated <see cref="IModelTypeBuilder">model type builder</see>.</param>
41-
public TypeSubstitutionContext( IServiceProvider serviceProvider, IModelTypeBuilder modelTypeBuilder )
39+
/// <param name="apiVersion">The current <see cref="ApiVersion">API version</see>.</param>
40+
public TypeSubstitutionContext( IServiceProvider serviceProvider, IModelTypeBuilder modelTypeBuilder, ApiVersion apiVersion )
4241
{
43-
model = new Lazy<IEdmModel>( serviceProvider.GetRequiredService<IEdmModel> );
44-
apiVersion = new Lazy<ApiVersion>( () => Model.GetAnnotationValue<ApiVersionAnnotation>( Model )?.ApiVersion ?? ApiVersion.Default );
42+
this.apiVersion = apiVersion;
43+
this.serviceProvider = serviceProvider;
4544
ModelTypeBuilder = modelTypeBuilder;
4645
}
4746

4847
/// <summary>
4948
/// Gets the source Entity Data Model (EDM).
5049
/// </summary>
5150
/// <value>The associated <see cref="IEdmModel">EDM model</see> compared against for substitutions.</value>
52-
public IEdmModel Model => model.Value;
51+
public IEdmModel Model => model ??= serviceProvider!.GetRequiredService<IEdmModelSelector>().SelectModel( apiVersion )!;
5352

5453
/// <summary>
5554
/// Gets API version associated with the source model.
5655
/// </summary>
5756
/// <value>The associated <see cref="ApiVersion">API version</see>.</value>
58-
public ApiVersion ApiVersion => apiVersion.Value;
57+
public ApiVersion ApiVersion => apiVersion ??= model!.GetAnnotationValue<ApiVersionAnnotation>( Model )?.ApiVersion ?? ApiVersion.Neutral;
5958

6059
/// <summary>
6160
/// Gets the model type builder used to create substitution types.

‎src/Common/ApiVersion.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ internal ApiVersion( DateTime? groupVersion, int? majorVersion, int? minorVersio
168168
/// <param name="status">The status to evaluate.</param>
169169
/// <returns>True if the status is valid; otherwise, false.</returns>
170170
/// <remarks>The status must be alphabetic or alphanumeric, start with a letter, and contain no spaces.</remarks>
171-
public static bool IsValidStatus( string? status ) => IsNullOrEmpty( status ) ? false : IsMatch( status, @"^[a-zA-Z][a-zA-Z0-9]*$", Singleline );
171+
public static bool IsValidStatus( string? status ) => !IsNullOrEmpty( status ) && IsMatch( status, @"^[a-zA-Z][a-zA-Z0-9]*$", Singleline );
172172

173173
/// <summary>
174174
/// Parses the specified text into an API version.
@@ -408,7 +408,7 @@ public override int GetHashCode()
408408
/// <param name="version2">The <see cref="ApiVersion"/> to compare against.</param>
409409
/// <returns>True the first object is less than or equal to the second object; otherwise, false.</returns>
410410
public static bool operator <=( ApiVersion? version1, ApiVersion? version2 ) =>
411-
version1 is null ? true : version1.CompareTo( version2 ) <= 0;
411+
version1 is null || version1.CompareTo( version2 ) <= 0;
412412

413413
/// <summary>
414414
/// Overloads the greater than operator.
@@ -417,7 +417,7 @@ public override int GetHashCode()
417417
/// <param name="version2">The <see cref="ApiVersion"/> to compare against.</param>
418418
/// <returns>True the first object is greater than the second object; otherwise, false.</returns>
419419
public static bool operator >( ApiVersion? version1, ApiVersion? version2 ) =>
420-
version1 is null ? false : version1.CompareTo( version2 ) > 0;
420+
version1 != null && version1.CompareTo( version2 ) > 0;
421421

422422
/// <summary>
423423
/// Overloads the greater than or equal to operator.
@@ -433,7 +433,7 @@ public override int GetHashCode()
433433
/// </summary>
434434
/// <param name="other">The <see cref="ApiVersion">other</see> to evaluate.</param>
435435
/// <returns>True if the specified object is equal to the current instance; otherwise, false.</returns>
436-
public virtual bool Equals( ApiVersion? other ) => other is null ? false : GetHashCode() == other.GetHashCode();
436+
public virtual bool Equals( ApiVersion? other ) => other != null && GetHashCode() == other.GetHashCode();
437437

438438
/// <summary>
439439
/// Performs a comparison of the current object to another object and returns a value

‎src/Microsoft.AspNet.OData.Versioning.ApiExplorer/Web.Http.Description/ODataApiExplorer.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -210,12 +210,12 @@ protected virtual void ExploreQueryOptions( IEnumerable<VersionedApiDescription>
210210
queryOptions.ApplyTo( apiDescriptions, settings );
211211
}
212212

213-
ResponseDescription CreateResponseDescriptionWithRoute( HttpActionDescriptor actionDescriptor, IHttpRoute route )
213+
ResponseDescription CreateResponseDescriptionWithRoute( HttpActionDescriptor actionDescriptor, IHttpRoute route, ApiVersion apiVersion )
214214
{
215215
var description = CreateResponseDescription( actionDescriptor );
216216
var serviceProvider = actionDescriptor.Configuration.GetODataRootContainer( route );
217217
var returnType = description.ResponseType ?? description.DeclaredType;
218-
var context = new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder );
218+
var context = new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder, apiVersion );
219219

220220
description.ResponseType = returnType.SubstituteIfNecessary( context );
221221

@@ -245,7 +245,7 @@ void ExploreRouteActions(
245245
continue;
246246
}
247247

248-
var parameterDescriptions = CreateParameterDescriptions( action, route );
248+
var parameterDescriptions = CreateParameterDescriptions( action, route, apiVersion );
249249
var context = new ODataRouteBuilderContext(
250250
Configuration,
251251
apiVersion,
@@ -344,7 +344,7 @@ static bool WillReadUri( HttpParameterBinding parameterBinding )
344344
return willReadUri;
345345
}
346346

347-
ApiParameterDescription CreateParameterDescriptionFromBinding( HttpParameterBinding parameterBinding, IServiceProvider serviceProvider )
347+
ApiParameterDescription CreateParameterDescriptionFromBinding( HttpParameterBinding parameterBinding, IServiceProvider serviceProvider, ApiVersion apiVersion )
348348
{
349349
var descriptor = parameterBinding.Descriptor;
350350
var description = CreateParameterDescription( descriptor );
@@ -354,7 +354,7 @@ ApiParameterDescription CreateParameterDescriptionFromBinding( HttpParameterBind
354354
description.Source = FromBody;
355355

356356
var parameterType = descriptor.ParameterType;
357-
var context = new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder );
357+
var context = new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder, apiVersion );
358358
var substitutedType = parameterType.SubstituteIfNecessary( context );
359359

360360
if ( parameterType != substitutedType )
@@ -373,7 +373,7 @@ ApiParameterDescription CreateParameterDescriptionFromBinding( HttpParameterBind
373373
return description;
374374
}
375375

376-
IList<ApiParameterDescription> CreateParameterDescriptions( HttpActionDescriptor actionDescriptor, IHttpRoute route )
376+
IList<ApiParameterDescription> CreateParameterDescriptions( HttpActionDescriptor actionDescriptor, IHttpRoute route, ApiVersion apiVersion )
377377
{
378378
var list = new List<ApiParameterDescription>();
379379
var actionBinding = GetActionBinding( actionDescriptor );
@@ -388,7 +388,7 @@ IList<ApiParameterDescription> CreateParameterDescriptions( HttpActionDescriptor
388388
{
389389
foreach ( var binding in parameterBindings )
390390
{
391-
list.Add( CreateParameterDescriptionFromBinding( binding, serviceProvider ) );
391+
list.Add( CreateParameterDescriptionFromBinding( binding, serviceProvider, apiVersion ) );
392392
}
393393
}
394394
}
@@ -461,7 +461,7 @@ void PopulateActionDescriptions(
461461
ApiVersion apiVersion )
462462
{
463463
var documentation = DocumentationProvider?.GetDocumentation( actionDescriptor );
464-
var responseDescription = CreateResponseDescriptionWithRoute( actionDescriptor, route );
464+
var responseDescription = CreateResponseDescriptionWithRoute( actionDescriptor, route, apiVersion );
465465
var responseType = responseDescription.ResponseType ?? responseDescription.DeclaredType;
466466
var requestFormatters = new List<MediaTypeFormatter>();
467467
var responseFormatters = new List<MediaTypeFormatter>();

‎src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilderContext.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ internal ODataRouteBuilderContext(
2323
{
2424
ApiVersion = apiVersion;
2525
Services = routeMapping.Services;
26-
EdmModel = Services.GetRequiredService<IEdmModel>();
2726
routeAttribute = actionDescriptor.MethodInfo.GetCustomAttributes<ODataRouteAttribute>().FirstOrDefault();
2827
RouteTemplate = routeAttribute?.PathTemplate;
2928
RoutePrefix = routeMapping.RoutePrefix;

‎src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc/ApiExplorer/PseudoModelBindingVisitor.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,14 @@ ApiParameterDescription CreateResult( ApiParameterDescriptionContext bindingCont
8686
{
8787
var action = (IEdmAction) Context.RouteContext.Operation;
8888
var apiVersion = Context.RouteContext.ApiVersion;
89+
var controller = Context.RouteContext.ActionDescriptor.ControllerName;
8990

90-
type = Context.TypeBuilder.NewActionParameters( Context.Services, action, apiVersion, Context.RouteContext.ActionDescriptor.ControllerName );
91+
type = Context.TypeBuilder.NewActionParameters( Context.Services, action, apiVersion, controller );
9192
}
9293
else
9394
{
94-
type = type.SubstituteIfNecessary( new TypeSubstitutionContext( Context.Services, Context.TypeBuilder ) );
95+
var context = new TypeSubstitutionContext( Context.Services, Context.TypeBuilder, Context.RouteContext.ApiVersion );
96+
type = type.SubstituteIfNecessary( context );
9597
}
9698

9799
return new ApiParameterDescription()

‎test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -230,13 +230,18 @@ public void substitute_should_generate_type_for_action_parameters()
230230
action.Parameter<string>( "contactedBy" );
231231
action.Parameter<bool>( "callbackRequired" );
232232

233-
var context = NewContext( modelBuilder.GetEdmModel() );
234-
var model = context.Model;
233+
var model = modelBuilder.GetEdmModel();
234+
235+
model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );
236+
237+
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
238+
var context = NewContext( model );
235239
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
236240
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
237241
var services = new ServiceCollection();
238242

239-
services.AddSingleton( model );
243+
services.AddTransient( sp => sp.GetRequiredService<IEdmModelSelector>().SelectModel( sp ) );
244+
services.AddSingleton( selector );
240245

241246
// act
242247
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name );
@@ -263,13 +268,18 @@ public void substitute_should_generate_type_for_action_parameters_with_substitut
263268
action.Parameter<Contact>( "interviewer" );
264269
action.Parameter<Contact>( "interviewee" );
265270

266-
var context = NewContext( modelBuilder.GetEdmModel() );
267-
var model = context.Model;
271+
var model = modelBuilder.GetEdmModel();
272+
273+
model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );
274+
275+
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
276+
var context = NewContext( model );
268277
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
269278
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
270279
var services = new ServiceCollection();
271280

272-
services.AddSingleton( model );
281+
services.AddTransient( sp => sp.GetRequiredService<IEdmModelSelector>().SelectModel( sp ) );
282+
services.AddSingleton( selector );
273283

274284
// act
275285
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name );
@@ -301,13 +311,18 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio
301311
action.CollectionParameter<Contact>( "attendees" );
302312
action.CollectionParameter<string>( "topics" );
303313

304-
var context = NewContext( modelBuilder.GetEdmModel() );
305-
var model = context.Model;
314+
var model = modelBuilder.GetEdmModel();
315+
316+
model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );
317+
318+
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
319+
var context = NewContext( model );
306320
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
307321
var operation = (IEdmAction) model.FindDeclaredOperations( qualifiedName ).Single();
308322
var services = new ServiceCollection();
309323

310-
services.AddSingleton( model );
324+
services.AddTransient( sp => sp.GetRequiredService<IEdmModelSelector>().SelectModel( sp ) );
325+
services.AddSingleton( selector );
311326

312327
// act
313328
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name );
@@ -338,13 +353,18 @@ public void substitute_should_generate_types_for_actions_with_the_same_name_in_d
338353
action.CollectionParameter<Employee>( "attendees" );
339354
action.Parameter<string>( "project" );
340355

341-
var context = NewContext( modelBuilder.GetEdmModel() );
342-
var model = context.Model;
356+
var model = modelBuilder.GetEdmModel();
357+
358+
model.SetAnnotationValue( model, new ApiVersionAnnotation( ApiVersion.Default ) );
359+
360+
var selector = (IEdmModelSelector) new EdmModelSelector( new[] { model }, ApiVersion.Default );
361+
var context = NewContext( model );
343362
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
344363
var operations = model.FindDeclaredOperations( qualifiedName ).Select( o => (IEdmAction) o ).ToArray();
345364
var services = new ServiceCollection();
346365

347-
services.AddSingleton( model );
366+
services.AddTransient( sp => sp.GetRequiredService<IEdmModelSelector>().SelectModel( sp ) );
367+
services.AddSingleton( selector );
348368

349369
// act
350370
var contactActionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operations[0], ApiVersion.Default, contact.Name );

0 commit comments

Comments
 (0)