Skip to content

Commit ea48ea5

Browse files
LuukN2commonsensesoftware
LuukN2
authored and
commonsensesoftware
committed
Type building fixes (#394)
* Add support for generating types for action parameters that have collection parameters. * Add support for the substitution of ODataActionParameter parameters. * Added null checks and removed GetPropertyType. * Add support for building custom attributes that take params parameters. * Prevent trying to resolve dependencies for an unfinished type that has been finished already. * Support substitution of single result generic. * Do not set the type if no structured type could be found. Also prevent null pointer when no odata action parameters are found. This is necessary for CreateRef actions. * Prevent duplicate action parameter type names when actions with the same names are declared in multiple controllers. * Fixed property dependencies on types that have already been generated never being resolved. * Changed controller name source. * Added test cases for building custom attributes that take params parameters. * Added test cases for generating types for action parameters with the same name for different entity sets. (controllers)
1 parent 56c2677 commit ea48ea5

File tree

12 files changed

+218
-17
lines changed

12 files changed

+218
-17
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,14 @@ static IEnumerable<CustomAttributeBuilder> AttributesFromProperty( PropertyInfo
8484
}
8585
}
8686

87+
for ( var i = 0; i < ctorArgs.Length; i++ )
88+
{
89+
if ( ctorArgs[i] is IReadOnlyCollection<CustomAttributeTypedArgument> paramsList )
90+
{
91+
ctorArgs[i] = paramsList.Select( a => a.Value ).ToArray();
92+
}
93+
}
94+
8795
yield return new CustomAttributeBuilder(
8896
ctor,
8997
ctorArgs,

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,14 +171,15 @@ public Type NewStructuredType( IEdmStructuredType structuredType, Type clrType,
171171
}
172172

173173
/// <inheritdoc />
174-
public Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion )
174+
public Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion, string controllerName )
175175
{
176176
Arg.NotNull( services, nameof( services ) );
177177
Arg.NotNull( action, nameof( action ) );
178178
Arg.NotNull( apiVersion, nameof( apiVersion ) );
179+
Arg.NotNull( controllerName, nameof(controllerName) );
179180
Contract.Ensures( Contract.Result<Type>() != null );
180181

181-
var name = action.FullName() + "Parameters";
182+
var name = controllerName + "." + action.FullName() + "Parameters";
182183
var properties = action.Parameters.Where( p => p.Name != "bindingParameter" ).Select( p => new ClassProperty( services, assemblies, p, this ) );
183184
var signature = new ClassSignature( name, properties, apiVersion );
184185

@@ -227,7 +228,18 @@ private Type ResolveDependencies( TypeBuilder typeBuilder, EdmTypeKey typeKey )
227228
for ( var x = propertyDependencies.Count - 1; x >= 0; x-- )
228229
{
229230
var propertyDependency = propertyDependencies[x];
230-
if ( propertyDependency.DependentOnTypeKey == typeKey )
231+
Type dependentOnType = null;
232+
233+
if ( unfinishedTypes.TryGetValue( propertyDependency.DependentOnTypeKey, out var dependentOnTypeBuilder ) )
234+
{
235+
dependentOnType = dependentOnTypeBuilder;
236+
}
237+
else if ( generatedEdmTypes.TryGetValue( propertyDependency.DependentOnTypeKey, out var dependentOnTypeInfo ) )
238+
{
239+
dependentOnType = dependentOnTypeInfo;
240+
}
241+
242+
if ( dependentOnType != null)
231243
{
232244
if ( propertyDependency.IsCollection )
233245
{
@@ -274,7 +286,10 @@ private void ResolveForUnfinishedTypes()
274286
var keys = unfinishedTypes.Keys;
275287
foreach ( var key in keys )
276288
{
277-
ResolveDependencies(unfinishedTypes[key], key);
289+
if ( unfinishedTypes.TryGetValue( key, out var type ) )
290+
{
291+
ResolveDependencies(type, key);
292+
}
278293
}
279294
}
280295

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,11 @@ public interface IModelTypeBuilder
3434
/// <param name="services">The <see cref="IServiceProvider">services</see> needed to potentially substitute types.</param>
3535
/// <param name="action">The defining <see cref="IEdmAction">action</see>.</param>
3636
/// <param name="apiVersion">The <see cref="ApiVersion">API version</see> of the <paramref name="action"/> to create the parameter type for.</param>
37+
/// <param name="controllerName">The name of the controller that defines the action. Necessary for generating unique parameter types.</param>
3738
/// <returns>A strong <see cref="Type">type</see> definition for the OData <paramref name="action"/> parameters.</returns>
3839
/// <remarks><see cref="ODataActionParameters">OData action parameters</see> are modeled as a <see cref="Dictionary{TKey,TValue}">dictionary</see>,
3940
/// which is difficult to use effectively by documentation tools such as the API Explorer. The corresponding type is generated only once per
4041
/// <paramref name="apiVersion">API version</paramref>.</remarks>
41-
Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion );
42+
Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion, string controllerName );
4243
}
4344
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.OData.Edm;
77
#if WEBAPI
88
using Microsoft.Web.Http;
9+
using System.Web.Http;
910
#endif
1011
using System;
1112
using System.Collections.Generic;
@@ -26,6 +27,7 @@ public static partial class TypeExtensions
2627
static readonly Type HttpResponseType = typeof( HttpResponseMessage );
2728
static readonly Type IEnumerableOfT = typeof( IEnumerable<> );
2829
static readonly Type ODataValueOfT = typeof( ODataValue<> );
30+
static readonly Type SingleResultOfT = typeof( SingleResult<> );
2931

3032
/// <summary>
3133
/// Substitutes the specified type, if required.
@@ -68,7 +70,11 @@ public static Type SubstituteIfNecessary( this Type type, TypeSubstitutionContex
6870
{
6971
var (apiVersion, resolver) = holder.Value;
7072
var structuredType = resolver.GetStructuredType( type );
73+
74+
if ( structuredType != null )
75+
{
7176
type = context.ModelTypeBuilder.NewStructuredType( structuredType, type, apiVersion );
77+
}
7278
}
7379

7480
return type;
@@ -106,7 +112,7 @@ static bool IsSubstitutableGeneric( Type type, Stack<Type> openTypes, out Type i
106112

107113
var typeArg = typeArgs[0];
108114

109-
if ( typeDef.Equals( IEnumerableOfT ) || typeDef.IsDelta() || typeDef.Equals( ODataValueOfT ) || typeDef.IsActionResult() )
115+
if ( typeDef.Equals( IEnumerableOfT ) || typeDef.IsDelta() || typeDef.Equals( ODataValueOfT ) || typeDef.IsActionResult() || typeDef.Equals( SingleResultOfT ))
110116
{
111117
innerType = typeArg;
112118
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,15 @@ internal ODataRouteBuilderContext(
6161

6262
if ( Operation?.IsAction() == true )
6363
{
64-
ConvertODataActionParametersToTypedModel( modelTypeBuilder, (IEdmAction) Operation );
64+
ConvertODataActionParametersToTypedModel( modelTypeBuilder, (IEdmAction) Operation, actionDescriptor.ControllerDescriptor.ControllerName );
6565
}
6666
}
6767

68-
void ConvertODataActionParametersToTypedModel( IModelTypeBuilder modelTypeBuilder, IEdmAction action )
68+
void ConvertODataActionParametersToTypedModel( IModelTypeBuilder modelTypeBuilder, IEdmAction action, string controllerName )
6969
{
7070
Contract.Requires( modelTypeBuilder != null );
7171
Contract.Requires( action != null );
72+
Contract.Requires( controllerName != null );
7273

7374
var apiVersion = new Lazy<ApiVersion>( () => EdmModel.GetAnnotationValue<ApiVersionAnnotation>( EdmModel ).ApiVersion );
7475

@@ -77,9 +78,9 @@ void ConvertODataActionParametersToTypedModel( IModelTypeBuilder modelTypeBuilde
7778
var description = ParameterDescriptions[i];
7879
var parameter = description.ParameterDescriptor;
7980

80-
if ( parameter.ParameterType.IsODataActionParameters() )
81+
if ( parameter != null && parameter.ParameterType.IsODataActionParameters() )
8182
{
82-
description.ParameterDescriptor = new ODataModelBoundParameterDescriptor( parameter, modelTypeBuilder.NewActionParameters( serviceProvider, action, apiVersion.Value ) );
83+
description.ParameterDescriptor = new ODataModelBoundParameterDescriptor( parameter, modelTypeBuilder.NewActionParameters( serviceProvider, action, apiVersion.Value, controllerName ) );
8384
break;
8485
}
8586
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ ApiParameterDescription CreateResult( ApiParameterDescriptionContext bindingCont
8888
{
8989
var action = (IEdmAction) Context.RouteContext.Operation;
9090
var apiVersion = Context.RouteContext.ApiVersion;
91-
type = Context.TypeBuilder.NewActionParameters( Context.Services, action, apiVersion );
91+
type = Context.TypeBuilder.NewActionParameters( Context.Services, action, apiVersion, Context.RouteContext.ActionDescriptor.ControllerName );
9292
}
9393
else
9494
{

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
namespace Microsoft.AspNet.OData
2+
{
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
7+
[AttributeUsage(AttributeTargets.Property)]
8+
public class AllowedRolesAttribute : Attribute
9+
{
10+
11+
public AllowedRolesAttribute( params string[] parameters)
12+
{
13+
AllowedRoles = parameters.ToList();
14+
}
15+
16+
public List<string> AllowedRoles { get; }
17+
}
18+
}

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

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ public void substitute_should_generate_type_for_action_parameters()
236236
services.AddSingleton( model );
237237

238238
// act
239-
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default );
239+
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name );
240240

241241
// assert
242242
substitutionType.GetRuntimeProperties().Should().HaveCount( 3 );
@@ -266,7 +266,7 @@ public void substitute_should_generate_type_for_action_parameters_with_substitut
266266
services.AddSingleton( model );
267267

268268
// act
269-
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default );
269+
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name );
270270

271271
// assert
272272
substitutionType.GetRuntimeProperties().Should().HaveCount( 3 );
@@ -302,7 +302,7 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio
302302
services.AddSingleton( model );
303303

304304
// act
305-
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default );
305+
var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name );
306306

307307
// assert
308308
substitutionType.GetRuntimeProperties().Should().HaveCount( 3 );
@@ -311,6 +311,72 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio
311311
substitutionType.Should().HaveProperty<IEnumerable<string>>( "topics" );
312312
}
313313

314+
[Fact]
315+
public void substitute_should_generate_types_for_actions_with_the_same_name_in_different_controllers()
316+
{
317+
// arrange
318+
var modelBuilder = new ODataConventionModelBuilder();
319+
var contact = modelBuilder.EntitySet<Contact>( "Contacts" ).EntityType;
320+
var action = contact.Action( "PlanMeeting" );
321+
322+
action.Parameter<DateTime>( "when" );
323+
action.CollectionParameter<Contact>( "attendees" );
324+
action.CollectionParameter<string>( "topics" );
325+
326+
var employee = modelBuilder.EntitySet<Employee>( "Employees" ).EntityType;
327+
action = employee.Action( "PlanMeeting" );
328+
329+
action.Parameter<DateTime>( "when" );
330+
action.CollectionParameter<Employee>( "attendees" );
331+
action.Parameter<string>( "project" );
332+
333+
var context = NewContext( modelBuilder.GetEdmModel() );
334+
var model = context.Model;
335+
336+
var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}";
337+
var operations = model.FindDeclaredOperations( qualifiedName ).Select( o => (IEdmAction) o ).ToArray();
338+
var services = new ServiceCollection();
339+
services.AddSingleton( model );
340+
341+
// act
342+
var contactActionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operations[0], ApiVersion.Default, contact.Name );
343+
var employeesActionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operations[1], ApiVersion.Default, employee.Name );
344+
345+
// assert
346+
contactActionType.Should().NotBe( employeesActionType );
347+
contactActionType.GetRuntimeProperties().Should().HaveCount( 3 );
348+
contactActionType.Should().HaveProperty<DateTimeOffset>( "when" );
349+
contactActionType.Should().HaveProperty<IEnumerable<Contact>>( "attendees" );
350+
contactActionType.Should().HaveProperty<IEnumerable<string>>( "topics" );
351+
352+
employeesActionType.GetRuntimeProperties().Should().HaveCount( 3 );
353+
employeesActionType.Should().HaveProperty<DateTimeOffset>( "when" );
354+
Assert.NotNull(employeesActionType.GetRuntimeProperty( "attendees" ));
355+
employeesActionType.Should().HaveProperty<string>( "project" );
356+
}
357+
358+
[Fact]
359+
public void substitute_should_get_attributes_from_property_that_has_attributes_that_takes_params()
360+
{
361+
// arrange
362+
var modelBuilder = new ODataConventionModelBuilder();
363+
var employee = modelBuilder.EntitySet<Employee>( "Employees" ).EntityType;
364+
employee.Ignore( e => e.FirstName );
365+
var originalType = typeof( Employee );
366+
367+
var context = NewContext( modelBuilder.GetEdmModel() );
368+
369+
// act
370+
var substitutionType = originalType.SubstituteIfNecessary( context );
371+
372+
// assert
373+
var property = substitutionType.GetRuntimeProperty( "Salary" );
374+
var attributeWithParams = property.GetCustomAttribute<AllowedRolesAttribute>();
375+
376+
Assert.Equal( "Manager", attributeWithParams.AllowedRoles[0] );
377+
Assert.Equal( "Employer", attributeWithParams.AllowedRoles[1] );
378+
}
379+
314380
public static IEnumerable<object[]> SubstitutionNotRequiredData
315381
{
316382
get

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ public class Employee
66

77
public Employer Employer { get; set; }
88

9+
[AllowedRoles("Manager", "Employer")]
910
public decimal Salary { get; set; }
1011

1112
public string FirstName { get; set; }

‎test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/AllowedRolesAttribute.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
namespace Microsoft.AspNet.OData
2+
{
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
7+
[AttributeUsage(AttributeTargets.Property)]
8+
public class AllowedRolesAttribute : Attribute
9+
{
10+
11+
public AllowedRolesAttribute( params string[] parameters)
12+
{
13+
AllowedRoles = parameters.ToList();
14+
}
15+
16+
public List<string> AllowedRoles { get; }
17+
}
18+
}

0 commit comments

Comments
 (0)