diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/ClassProperty.cs b/src/Common.OData.ApiExplorer/AspNet.OData/ClassProperty.cs index f43dd37a..0c7a5fd7 100644 --- a/src/Common.OData.ApiExplorer/AspNet.OData/ClassProperty.cs +++ b/src/Common.OData.ApiExplorer/AspNet.OData/ClassProperty.cs @@ -84,6 +84,14 @@ static IEnumerable AttributesFromProperty( PropertyInfo } } + for ( var i = 0; i < ctorArgs.Length; i++ ) + { + if ( ctorArgs[i] is IReadOnlyCollection paramsList ) + { + ctorArgs[i] = paramsList.Select( a => a.Value ).ToArray(); + } + } + yield return new CustomAttributeBuilder( ctor, ctorArgs, diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/DefaultModelTypeBuilder.cs b/src/Common.OData.ApiExplorer/AspNet.OData/DefaultModelTypeBuilder.cs index b22db6a1..6d7fbbc6 100644 --- a/src/Common.OData.ApiExplorer/AspNet.OData/DefaultModelTypeBuilder.cs +++ b/src/Common.OData.ApiExplorer/AspNet.OData/DefaultModelTypeBuilder.cs @@ -171,14 +171,15 @@ public Type NewStructuredType( IEdmStructuredType structuredType, Type clrType, } /// - public Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion ) + public Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion, string controllerName ) { Arg.NotNull( services, nameof( services ) ); Arg.NotNull( action, nameof( action ) ); Arg.NotNull( apiVersion, nameof( apiVersion ) ); + Arg.NotNull( controllerName, nameof(controllerName) ); Contract.Ensures( Contract.Result() != null ); - var name = action.FullName() + "Parameters"; + var name = controllerName + "." + action.FullName() + "Parameters"; var properties = action.Parameters.Where( p => p.Name != "bindingParameter" ).Select( p => new ClassProperty( services, assemblies, p, this ) ); var signature = new ClassSignature( name, properties, apiVersion ); @@ -227,7 +228,18 @@ private Type ResolveDependencies( TypeBuilder typeBuilder, EdmTypeKey typeKey ) for ( var x = propertyDependencies.Count - 1; x >= 0; x-- ) { var propertyDependency = propertyDependencies[x]; - if ( propertyDependency.DependentOnTypeKey == typeKey ) + Type dependentOnType = null; + + if ( unfinishedTypes.TryGetValue( propertyDependency.DependentOnTypeKey, out var dependentOnTypeBuilder ) ) + { + dependentOnType = dependentOnTypeBuilder; + } + else if ( generatedEdmTypes.TryGetValue( propertyDependency.DependentOnTypeKey, out var dependentOnTypeInfo ) ) + { + dependentOnType = dependentOnTypeInfo; + } + + if ( dependentOnType != null) { if ( propertyDependency.IsCollection ) { @@ -274,7 +286,10 @@ private void ResolveForUnfinishedTypes() var keys = unfinishedTypes.Keys; foreach ( var key in keys ) { - ResolveDependencies(unfinishedTypes[key], key); + if ( unfinishedTypes.TryGetValue( key, out var type ) ) + { + ResolveDependencies(type, key); + } } } diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/IModelTypeBuilder.cs b/src/Common.OData.ApiExplorer/AspNet.OData/IModelTypeBuilder.cs index 441a175e..e1144518 100644 --- a/src/Common.OData.ApiExplorer/AspNet.OData/IModelTypeBuilder.cs +++ b/src/Common.OData.ApiExplorer/AspNet.OData/IModelTypeBuilder.cs @@ -34,10 +34,11 @@ public interface IModelTypeBuilder /// The services needed to potentially substitute types. /// The defining action. /// The API version of the to create the parameter type for. + /// The name of the controller that defines the action. Necessary for generating unique parameter types. /// A strong type definition for the OData parameters. /// OData action parameters are modeled as a dictionary, /// which is difficult to use effectively by documentation tools such as the API Explorer. The corresponding type is generated only once per /// API version. - Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion ); + Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion, string controllerName ); } } \ No newline at end of file diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/TypeExtensions.cs b/src/Common.OData.ApiExplorer/AspNet.OData/TypeExtensions.cs index fe776e9d..6f38a480 100644 --- a/src/Common.OData.ApiExplorer/AspNet.OData/TypeExtensions.cs +++ b/src/Common.OData.ApiExplorer/AspNet.OData/TypeExtensions.cs @@ -6,6 +6,7 @@ using Microsoft.OData.Edm; #if WEBAPI using Microsoft.Web.Http; + using System.Web.Http; #endif using System; using System.Collections.Generic; @@ -26,6 +27,7 @@ public static partial class TypeExtensions static readonly Type HttpResponseType = typeof( HttpResponseMessage ); static readonly Type IEnumerableOfT = typeof( IEnumerable<> ); static readonly Type ODataValueOfT = typeof( ODataValue<> ); + static readonly Type SingleResultOfT = typeof( SingleResult<> ); /// /// Substitutes the specified type, if required. @@ -68,7 +70,11 @@ public static Type SubstituteIfNecessary( this Type type, TypeSubstitutionContex { var (apiVersion, resolver) = holder.Value; var structuredType = resolver.GetStructuredType( type ); + + if ( structuredType != null ) + { type = context.ModelTypeBuilder.NewStructuredType( structuredType, type, apiVersion ); + } } return type; @@ -106,7 +112,7 @@ static bool IsSubstitutableGeneric( Type type, Stack openTypes, out Type i var typeArg = typeArgs[0]; - if ( typeDef.Equals( IEnumerableOfT ) || typeDef.IsDelta() || typeDef.Equals( ODataValueOfT ) || typeDef.IsActionResult() ) + if ( typeDef.Equals( IEnumerableOfT ) || typeDef.IsDelta() || typeDef.Equals( ODataValueOfT ) || typeDef.IsActionResult() || typeDef.Equals( SingleResultOfT )) { innerType = typeArg; } diff --git a/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilderContext.cs b/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilderContext.cs index e5b6f3f5..2273d2ed 100644 --- a/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilderContext.cs +++ b/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilderContext.cs @@ -61,14 +61,15 @@ internal ODataRouteBuilderContext( if ( Operation?.IsAction() == true ) { - ConvertODataActionParametersToTypedModel( modelTypeBuilder, (IEdmAction) Operation ); + ConvertODataActionParametersToTypedModel( modelTypeBuilder, (IEdmAction) Operation, actionDescriptor.ControllerDescriptor.ControllerName ); } } - void ConvertODataActionParametersToTypedModel( IModelTypeBuilder modelTypeBuilder, IEdmAction action ) + void ConvertODataActionParametersToTypedModel( IModelTypeBuilder modelTypeBuilder, IEdmAction action, string controllerName ) { Contract.Requires( modelTypeBuilder != null ); Contract.Requires( action != null ); + Contract.Requires( controllerName != null ); var apiVersion = new Lazy( () => EdmModel.GetAnnotationValue( EdmModel ).ApiVersion ); @@ -77,9 +78,9 @@ void ConvertODataActionParametersToTypedModel( IModelTypeBuilder modelTypeBuilde var description = ParameterDescriptions[i]; var parameter = description.ParameterDescriptor; - if ( parameter.ParameterType.IsODataActionParameters() ) + if ( parameter != null && parameter.ParameterType.IsODataActionParameters() ) { - description.ParameterDescriptor = new ODataModelBoundParameterDescriptor( parameter, modelTypeBuilder.NewActionParameters( serviceProvider, action, apiVersion.Value ) ); + description.ParameterDescriptor = new ODataModelBoundParameterDescriptor( parameter, modelTypeBuilder.NewActionParameters( serviceProvider, action, apiVersion.Value, controllerName ) ); break; } } diff --git a/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/PseudoModelBindingVisitor.cs b/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/PseudoModelBindingVisitor.cs index 4fd45bee..d46194b1 100644 --- a/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/PseudoModelBindingVisitor.cs +++ b/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/PseudoModelBindingVisitor.cs @@ -88,7 +88,7 @@ ApiParameterDescription CreateResult( ApiParameterDescriptionContext bindingCont { var action = (IEdmAction) Context.RouteContext.Operation; var apiVersion = Context.RouteContext.ApiVersion; - type = Context.TypeBuilder.NewActionParameters( Context.Services, action, apiVersion ); + type = Context.TypeBuilder.NewActionParameters( Context.Services, action, apiVersion, Context.RouteContext.ActionDescriptor.ControllerName ); } else { diff --git a/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/AllowedRolesAttribute.cs b/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/AllowedRolesAttribute.cs new file mode 100644 index 00000000..b6ccb24b --- /dev/null +++ b/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/AllowedRolesAttribute.cs @@ -0,0 +1,18 @@ +namespace Microsoft.AspNet.OData +{ + using System; + using System.Collections.Generic; + using System.Linq; + + [AttributeUsage(AttributeTargets.Property)] + public class AllowedRolesAttribute : Attribute + { + + public AllowedRolesAttribute( params string[] parameters) + { + AllowedRoles = parameters.ToList(); + } + + public List AllowedRoles { get; } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs b/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs index 2109d3d5..624e018a 100644 --- a/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs +++ b/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs @@ -236,7 +236,7 @@ public void substitute_should_generate_type_for_action_parameters() services.AddSingleton( model ); // act - var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default ); + var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name ); // assert substitutionType.GetRuntimeProperties().Should().HaveCount( 3 ); @@ -266,7 +266,7 @@ public void substitute_should_generate_type_for_action_parameters_with_substitut services.AddSingleton( model ); // act - var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default ); + var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name ); // assert substitutionType.GetRuntimeProperties().Should().HaveCount( 3 ); @@ -302,7 +302,7 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio services.AddSingleton( model ); // act - var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default ); + var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name ); // assert substitutionType.GetRuntimeProperties().Should().HaveCount( 3 ); @@ -311,6 +311,72 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio substitutionType.Should().HaveProperty>( "topics" ); } + [Fact] + public void substitute_should_generate_types_for_actions_with_the_same_name_in_different_controllers() + { + // arrange + var modelBuilder = new ODataConventionModelBuilder(); + var contact = modelBuilder.EntitySet( "Contacts" ).EntityType; + var action = contact.Action( "PlanMeeting" ); + + action.Parameter( "when" ); + action.CollectionParameter( "attendees" ); + action.CollectionParameter( "topics" ); + + var employee = modelBuilder.EntitySet( "Employees" ).EntityType; + action = employee.Action( "PlanMeeting" ); + + action.Parameter( "when" ); + action.CollectionParameter( "attendees" ); + action.Parameter( "project" ); + + var context = NewContext( modelBuilder.GetEdmModel() ); + var model = context.Model; + + var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}"; + var operations = model.FindDeclaredOperations( qualifiedName ).Select( o => (IEdmAction) o ).ToArray(); + var services = new ServiceCollection(); + services.AddSingleton( model ); + + // act + var contactActionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operations[0], ApiVersion.Default, contact.Name ); + var employeesActionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operations[1], ApiVersion.Default, employee.Name ); + + // assert + contactActionType.Should().NotBe( employeesActionType ); + contactActionType.GetRuntimeProperties().Should().HaveCount( 3 ); + contactActionType.Should().HaveProperty( "when" ); + contactActionType.Should().HaveProperty>( "attendees" ); + contactActionType.Should().HaveProperty>( "topics" ); + + employeesActionType.GetRuntimeProperties().Should().HaveCount( 3 ); + employeesActionType.Should().HaveProperty( "when" ); + Assert.NotNull(employeesActionType.GetRuntimeProperty( "attendees" )); + employeesActionType.Should().HaveProperty( "project" ); + } + + [Fact] + public void substitute_should_get_attributes_from_property_that_has_attributes_that_takes_params() + { + // arrange + var modelBuilder = new ODataConventionModelBuilder(); + var employee = modelBuilder.EntitySet( "Employees" ).EntityType; + employee.Ignore( e => e.FirstName ); + var originalType = typeof( Employee ); + + var context = NewContext( modelBuilder.GetEdmModel() ); + + // act + var substitutionType = originalType.SubstituteIfNecessary( context ); + + // assert + var property = substitutionType.GetRuntimeProperty( "Salary" ); + var attributeWithParams = property.GetCustomAttribute(); + + Assert.Equal( "Manager", attributeWithParams.AllowedRoles[0] ); + Assert.Equal( "Employer", attributeWithParams.AllowedRoles[1] ); + } + public static IEnumerable SubstitutionNotRequiredData { get diff --git a/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs b/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs index 4991efa3..923d5942 100644 --- a/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs +++ b/test/Microsoft.AspNet.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs @@ -6,6 +6,7 @@ public class Employee public Employer Employer { get; set; } + [AllowedRoles("Manager", "Employer")] public decimal Salary { get; set; } public string FirstName { get; set; } diff --git a/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/AllowedRolesAttribute.cs b/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/AllowedRolesAttribute.cs new file mode 100644 index 00000000..b6ccb24b --- /dev/null +++ b/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/AllowedRolesAttribute.cs @@ -0,0 +1,18 @@ +namespace Microsoft.AspNet.OData +{ + using System; + using System.Collections.Generic; + using System.Linq; + + [AttributeUsage(AttributeTargets.Property)] + public class AllowedRolesAttribute : Attribute + { + + public AllowedRolesAttribute( params string[] parameters) + { + AllowedRoles = parameters.ToList(); + } + + public List AllowedRoles { get; } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs b/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs index f126d215..5ddd1e43 100644 --- a/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs +++ b/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/DefaultModelTypeBuilderTest.cs @@ -236,7 +236,7 @@ public void substitute_should_generate_type_for_action_parameters() services.AddSingleton( model ); // act - var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default ); + var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name ); // assert substitutionType.GetRuntimeProperties().Should().HaveCount( 3 ); @@ -266,7 +266,7 @@ public void substitute_should_generate_type_for_action_parameters_with_substitut services.AddSingleton( model ); // act - var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default ); + var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name ); // assert substitutionType.GetRuntimeProperties().Should().HaveCount( 3 ); @@ -302,7 +302,7 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio services.AddSingleton( model ); // act - var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default ); + var substitutionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operation, ApiVersion.Default, contact.Name ); // assert substitutionType.GetRuntimeProperties().Should().HaveCount( 3 ); @@ -311,6 +311,72 @@ public void substitute_should_generate_type_for_action_parameters_with_collectio substitutionType.Should().HaveProperty>( "topics" ); } + [Fact] + public void substitute_should_generate_types_for_actions_with_the_same_name_in_different_controllers() + { + // arrange + var modelBuilder = new ODataConventionModelBuilder(); + var contact = modelBuilder.EntitySet( "Contacts" ).EntityType; + var action = contact.Action( "PlanMeeting" ); + + action.Parameter( "when" ); + action.CollectionParameter( "attendees" ); + action.CollectionParameter( "topics" ); + + var employee = modelBuilder.EntitySet( "Employees" ).EntityType; + action = employee.Action( "PlanMeeting" ); + + action.Parameter( "when" ); + action.CollectionParameter( "attendees" ); + action.Parameter( "project" ); + + var context = NewContext( modelBuilder.GetEdmModel() ); + var model = context.Model; + + var qualifiedName = $"{model.EntityContainer.Namespace}.{action.Name}"; + var operations = model.FindDeclaredOperations( qualifiedName ).Select( o => (IEdmAction) o ).ToArray(); + var services = new ServiceCollection(); + services.AddSingleton( model ); + + // act + var contactActionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operations[0], ApiVersion.Default, contact.Name ); + var employeesActionType = context.ModelTypeBuilder.NewActionParameters( services.BuildServiceProvider(), operations[1], ApiVersion.Default, employee.Name ); + + // assert + contactActionType.Should().NotBe( employeesActionType ); + contactActionType.GetRuntimeProperties().Should().HaveCount( 3 ); + contactActionType.Should().HaveProperty( "when" ); + contactActionType.Should().HaveProperty>( "attendees" ); + contactActionType.Should().HaveProperty>( "topics" ); + + employeesActionType.GetRuntimeProperties().Should().HaveCount( 3 ); + employeesActionType.Should().HaveProperty( "when" ); + Assert.NotNull(employeesActionType.GetRuntimeProperty( "attendees" )); + employeesActionType.Should().HaveProperty( "project" ); + } + + [Fact] + public void substitute_should_get_attributes_from_property_that_has_attributes_that_takes_params() + { + // arrange + var modelBuilder = new ODataConventionModelBuilder(); + var employee = modelBuilder.EntitySet( "Employees" ).EntityType; + employee.Ignore( e => e.FirstName ); + var originalType = typeof( Employee ); + + var context = NewContext( modelBuilder.GetEdmModel() ); + + // act + var substitutionType = originalType.SubstituteIfNecessary( context ); + + // assert + var property = substitutionType.GetRuntimeProperty( "Salary" ); + var attributeWithParams = property.GetCustomAttribute(); + + Assert.Equal( "Manager", attributeWithParams.AllowedRoles[0] ); + Assert.Equal( "Employer", attributeWithParams.AllowedRoles[1] ); + } + public static IEnumerable SubstitutionNotRequiredData { get diff --git a/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs b/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs index 4991efa3..923d5942 100644 --- a/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs +++ b/test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Employee.cs @@ -6,6 +6,7 @@ public class Employee public Employer Employer { get; set; } + [AllowedRoles("Manager", "Employer")] public decimal Salary { get; set; } public string FirstName { get; set; }