Type building fixes

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

@@ -84,6 +84,14 @@ static IEnumerable<CustomAttributeBuilder> AttributesFromProperty( PropertyInfo
                     }
                 }
 
+                for ( var i = 0; i < ctorArgs.Length; i++ )
+                {
+                    if ( ctorArgs[i] is IReadOnlyCollection<CustomAttributeTypedArgument> paramsList )
+                    {
+                        ctorArgs[i] = paramsList.Select( a => a.Value ).ToArray();
+                    }
+                }
+
                 yield return new CustomAttributeBuilder(
                     ctor,
                     ctorArgs,

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

@@ -171,14 +171,15 @@ public Type NewStructuredType( IEdmStructuredType structuredType, Type clrType,
         }
 
         /// <inheritdoc />
-        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<Type>() != 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);
+                }
             }
         }
 

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

@@ -34,10 +34,11 @@ public interface IModelTypeBuilder
         /// <param name="services">The <see cref="IServiceProvider">services</see> needed to potentially substitute types.</param>
         /// <param name="action">The defining <see cref="IEdmAction">action</see>.</param>
         /// <param name="apiVersion">The <see cref="ApiVersion">API version</see> of the <paramref name="action"/> to create the parameter type for.</param>
+        /// <param name="controllerName">The name of the controller that defines the action. Necessary for generating unique parameter types.</param>
         /// <returns>A strong <see cref="Type">type</see> definition for the OData <paramref name="action"/> parameters.</returns>
         /// <remarks><see cref="ODataActionParameters">OData action parameters</see> are modeled as a <see cref="Dictionary{TKey,TValue}">dictionary</see>,
         /// which is difficult to use effectively by documentation tools such as the API Explorer. The corresponding type is generated only once per
         /// <paramref name="apiVersion">API version</paramref>.</remarks>
-        Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion );
+        Type NewActionParameters( IServiceProvider services, IEdmAction action, ApiVersion apiVersion, string controllerName );
     }
 }

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<> );
 
         /// <summary>
         /// 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<Type> 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;
             }

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<ApiVersion>( () => EdmModel.GetAnnotationValue<ApiVersionAnnotation>( 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;
                 }
             }

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
             {

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<string> AllowedRoles { get; }
+    }
+}

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<IEnumerable<string>>( "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<Contact>( "Contacts" ).EntityType;
+            var action = contact.Action( "PlanMeeting" );
+
+            action.Parameter<DateTime>( "when" );
+            action.CollectionParameter<Contact>( "attendees" );
+            action.CollectionParameter<string>( "topics" );
+
+            var employee = modelBuilder.EntitySet<Employee>( "Employees" ).EntityType;
+            action = employee.Action( "PlanMeeting" );
+
+            action.Parameter<DateTime>( "when" );
+            action.CollectionParameter<Employee>( "attendees" );
+            action.Parameter<string>( "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<DateTimeOffset>( "when" );
+            contactActionType.Should().HaveProperty<IEnumerable<Contact>>( "attendees" );
+            contactActionType.Should().HaveProperty<IEnumerable<string>>( "topics" );
+
+            employeesActionType.GetRuntimeProperties().Should().HaveCount( 3 );
+            employeesActionType.Should().HaveProperty<DateTimeOffset>( "when" );
+            Assert.NotNull(employeesActionType.GetRuntimeProperty( "attendees" ));
+            employeesActionType.Should().HaveProperty<string>( "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<Employee>( "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<AllowedRolesAttribute>();
+
+            Assert.Equal( "Manager", attributeWithParams.AllowedRoles[0] );
+            Assert.Equal( "Employer", attributeWithParams.AllowedRoles[1] );
+        }
+
         public static IEnumerable<object[]> SubstitutionNotRequiredData
         {
             get

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; }

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<string> AllowedRoles { get; }
+    }
+}