Update ModelMetadata with substituted type when necessary. Fixes #562.

Author: Chris Martinez

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

@@ -5,10 +5,12 @@
     using System;
     using System.Collections.Generic;
     using System.ComponentModel.DataAnnotations;
+    using System.Diagnostics;
     using System.Linq;
     using System.Reflection;
     using System.Reflection.Emit;
 
+    [DebuggerDisplay( "{Name,nq}" )]
     readonly struct ClassProperty
     {
         internal readonly Type Type;

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

@@ -7,25 +7,30 @@
 #endif
     using System;
     using System.Collections.Generic;
+    using System.Diagnostics;
     using System.Linq;
     using System.Reflection;
     using System.Reflection.Emit;
 
+    [DebuggerDisplay( "{Name,nq} ({ApiVersion,nq})" )]
     sealed class ClassSignature : IEquatable<ClassSignature>
     {
+        static readonly ConstructorInfo newOriginalType = typeof( OriginalTypeAttribute ).GetConstructors()[0];
 #if WEBAPI
         static readonly CustomAttributeBuilder[] NoAttributes = new CustomAttributeBuilder[0];
 #else
         static readonly CustomAttributeBuilder[] NoAttributes = Array.Empty<CustomAttributeBuilder>();
 #endif
-        static readonly ConstructorInfo newOriginalType = typeof( OriginalTypeAttribute ).GetConstructors()[0];
         readonly Lazy<int> hashCode;
 
         internal ClassSignature( Type originalType, IEnumerable<ClassProperty> properties, ApiVersion apiVersion )
         {
-            var attributeBuilders = new List<CustomAttributeBuilder>( originalType.DeclaredAttributes() );
+            var attributeBuilders = new List<CustomAttributeBuilder>
+            {
+                new CustomAttributeBuilder( newOriginalType, new object[] { originalType } ),
+            };
 
-            attributeBuilders.Insert( 0, new CustomAttributeBuilder( newOriginalType, new object[] { originalType } ) );
+            attributeBuilders.AddRange( originalType.DeclaredAttributes() );
 
             Name = originalType.FullName;
             Attributes = attributeBuilders.ToArray();

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

@@ -10,7 +10,6 @@
     using System;
     using System.Collections.Concurrent;
     using System.Collections.Generic;
-    using System.Diagnostics.Contracts;
     using System.Linq;
     using System.Reflection;
     using System.Reflection.Emit;
@@ -19,6 +18,7 @@
 #endif
     using static System.Globalization.CultureInfo;
     using static System.Guid;
+    using static System.Reflection.BindingFlags;
     using static System.Reflection.Emit.AssemblyBuilderAccess;
 
     /// <summary>
@@ -69,45 +69,80 @@ IDictionary<EdmTypeKey, TypeInfo> GenerateTypesForEdmModel( IEdmModel model, Api
             return ResolveDependencies( context );
         }
 
+        static void MapEdmPropertiesToClrProperties(
+            IEdmModel edmModel,
+            IEdmStructuredType edmType,
+            Dictionary<string, IEdmProperty> structuralProperties,
+            Dictionary<PropertyInfo, IEdmProperty> mappedClrProperties )
+        {
+            foreach ( var edmProperty in edmType.Properties() )
+            {
+                structuralProperties.Add( edmProperty.Name, edmProperty );
+
+                var clrProperty = edmModel.GetAnnotationValue<ClrPropertyInfoAnnotation>( edmProperty )?.ClrPropertyInfo;
+
+                if ( clrProperty != null )
+                {
+                    mappedClrProperties.Add( clrProperty, edmProperty );
+                }
+            }
+        }
+
         static Type GenerateTypeIfNeeded( IEdmStructuredType structuredType, BuilderContext context )
         {
-            var apiVersion = context.ApiVersion;
-            var edmTypes = context.EdmTypes;
-            var typeKey = new EdmTypeKey( structuredType, apiVersion );
+            var typeKey = new EdmTypeKey( structuredType, context.ApiVersion );
 
-            if ( edmTypes.TryGetValue( typeKey, out var generatedType ) )
+            if ( context.EdmTypes.TryGetValue( typeKey, out var generatedType ) )
             {
                 return generatedType;
             }
 
-            var edmModel = context.EdmModel;
-            var clrType = structuredType.GetClrType( edmModel )!;
+            var clrType = structuredType.GetClrType( context.EdmModel )!;
             var visitedEdmTypes = context.VisitedEdmTypes;
 
             visitedEdmTypes.Add( typeKey );
 
-            const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.Instance;
-
             var properties = new List<ClassProperty>();
             var structuralProperties = new Dictionary<string, IEdmProperty>( StringComparer.OrdinalIgnoreCase );
             var mappedClrProperties = new Dictionary<PropertyInfo, IEdmProperty>();
-            var clrTypeMatchesEdmType = true;
-            var hasUnfinishedTypes = false;
             var dependentProperties = new List<PropertyDependency>();
 
-            foreach ( var property in structuredType.Properties() )
-            {
-                structuralProperties.Add( property.Name, property );
-
-                var clrProperty = edmModel.GetAnnotationValue<ClrPropertyInfoAnnotation>( property )?.ClrPropertyInfo;
+            MapEdmPropertiesToClrProperties( context.EdmModel, structuredType, structuralProperties, mappedClrProperties );
+
+            var (clrTypeMatchesEdmType, hasUnfinishedTypes) =
+                BuildSignatureProperties(
+                    clrType,
+                    structuralProperties,
+                    mappedClrProperties,
+                    properties,
+                    dependentProperties,
+                    context );
+
+            return ResolveType(
+                typeKey,
+                clrType,
+                clrTypeMatchesEdmType,
+                hasUnfinishedTypes,
+                properties,
+                dependentProperties,
+                context );
+        }
 
-                if ( clrProperty != null )
-                {
-                    mappedClrProperties.Add( clrProperty, property );
-                }
-            }
+        static Tuple<bool, bool> BuildSignatureProperties(
+            Type clrType,
+            IReadOnlyDictionary<string, IEdmProperty> structuralProperties,
+            IReadOnlyDictionary<PropertyInfo, IEdmProperty> mappedClrProperties,
+            List<ClassProperty> properties,
+            List<PropertyDependency> dependentProperties,
+            BuilderContext context )
+        {
+            var edmModel = context.EdmModel;
+            var apiVersion = context.ApiVersion;
+            var visitedEdmTypes = context.VisitedEdmTypes;
+            var clrTypeMatchesEdmType = true;
+            var hasUnfinishedTypes = false;
 
-            foreach ( var property in clrType.GetProperties( bindingFlags ) )
+            foreach ( var property in clrType.GetProperties( Public | Instance ) )
             {
                 if ( !structuralProperties.TryGetValue( property.Name, out var structuralProperty ) &&
                      !mappedClrProperties.TryGetValue( property, out structuralProperty ) )
@@ -178,6 +213,21 @@ static Type GenerateTypeIfNeeded( IEdmStructuredType structuredType, BuilderCont
                 properties.Add( new ClassProperty( property, propertyType ) );
             }
 
+            return Tuple.Create( clrTypeMatchesEdmType, hasUnfinishedTypes );
+        }
+
+        static TypeInfo ResolveType(
+            EdmTypeKey typeKey,
+            Type clrType,
+            bool clrTypeMatchesEdmType,
+            bool hasUnfinishedTypes,
+            List<ClassProperty> properties,
+            List<PropertyDependency> dependentProperties,
+            BuilderContext context )
+        {
+            var apiVersion = context.ApiVersion;
+            var edmTypes = context.EdmTypes;
+
             TypeInfo type;
 
             if ( clrTypeMatchesEdmType )
@@ -240,6 +290,7 @@ static TypeBuilder CreateTypeBuilderFromSignature( ModuleBuilder moduleBuilder,
                 ref var property = ref properties[i];
                 var type = property.Type;
                 var name = property.Name;
+
                 AddProperty( typeBuilder, type, name, property.Attributes );
             }
 
@@ -253,15 +304,15 @@ static IDictionary<EdmTypeKey, TypeInfo> ResolveDependencies( BuilderContext con
 
             for ( var i = 0; i < dependencies.Count; i++ )
             {
-                var propertyDependency = dependencies[i];
-                var dependentOnType = edmTypes[propertyDependency.DependentOnTypeKey];
+                var dependency = dependencies[i];
+                var dependentOnType = edmTypes[dependency.DependentOnTypeKey];
 
-                if ( propertyDependency.IsCollection )
+                if ( dependency.IsCollection )
                 {
                     dependentOnType = IEnumerableOfT.MakeGenericType( dependentOnType ).GetTypeInfo();
                 }
 
-                AddProperty( propertyDependency.DependentType!, dependentOnType, propertyDependency.PropertyName, propertyDependency.CustomAttributes );
+                AddProperty( dependency.DependentType!, dependentOnType, dependency.PropertyName, dependency.CustomAttributes );
             }
 
             var keys = edmTypes.Keys.ToArray();
@@ -338,7 +389,7 @@ internal BuilderContext( IEdmModel edmModel, ApiVersion apiVersion, Func<ModuleB
 
             internal IDictionary<EdmTypeKey, TypeInfo> EdmTypes { get; } = new Dictionary<EdmTypeKey, TypeInfo>();
 
-            internal ICollection<EdmTypeKey> VisitedEdmTypes { get; } = new HashSet<EdmTypeKey>();
+            internal ISet<EdmTypeKey> VisitedEdmTypes { get; } = new HashSet<EdmTypeKey>();
 
             internal IList<PropertyDependency> Dependencies { get; } = new List<PropertyDependency>();
         }

src/Microsoft.AspNet.WebApi.Versioning/TupleExtensions.cs

@@ -1,7 +1,6 @@
 namespace Microsoft.Web.Http
 {
     using System;
-    using System.Diagnostics.Contracts;
 
     static class TupleExtensions
     {

src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/ModelMetadataExtensions.cs

@@ -0,0 +1,18 @@
+namespace Microsoft.AspNetCore.Mvc.ApiExplorer
+{
+    using Microsoft.AspNetCore.Mvc.ModelBinding;
+    using System;
+
+    internal static class ModelMetadataExtensions
+    {
+        internal static ModelMetadata SubstituteIfNecessary( this ModelMetadata modelMetadata, Type type )
+        {
+            if ( type.Equals( modelMetadata.ModelType ) )
+            {
+                return modelMetadata;
+            }
+
+            return new SubstitutedModelMetadata( modelMetadata, type );
+        }
+    }
+}

src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/ODataApiDescriptionProvider.cs

@@ -707,22 +707,29 @@ IReadOnlyList<ApiResponseType> GetApiResponseTypes( IReadOnlyList<IApiResponseMe
 
             foreach ( var objectType in objectTypes )
             {
+                Type type;
+
                 if ( objectType.Value == typeof( void ) )
                 {
+                    type = objectType.Value.SubstituteIfNecessary( new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder ) );
+
                     results.Add( new ApiResponseType()
                     {
                         StatusCode = objectType.Key,
-                        Type = objectType.Value.SubstituteIfNecessary( new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder ) ),
+                        Type = type,
+                        ModelMetadata = MetadataProvider.GetMetadataForType( objectType.Value ).SubstituteIfNecessary( type ),
                     } );
 
                     continue;
                 }
 
+                type = objectType.Value.SubstituteIfNecessary( new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder ) );
+
                 var apiResponseType = new ApiResponseType()
                 {
                     StatusCode = objectType.Key,
-                    Type = objectType.Value.SubstituteIfNecessary( new TypeSubstitutionContext( serviceProvider, ModelTypeBuilder ) ),
-                    ModelMetadata = MetadataProvider.GetMetadataForType( objectType.Value ),
+                    Type = type,
+                    ModelMetadata = MetadataProvider.GetMetadataForType( objectType.Value ).SubstituteIfNecessary( type ),
                 };
 
                 for ( var i = 0; i < contentTypes.Count; i++ )
@@ -779,13 +786,16 @@ void ProcessRouteParameters( ApiParameterContext context )
             var routeTemplate = TemplateParser.Parse( prefix );
             var routeParameters = new Dictionary<string, ApiParameterRouteInfo>( StringComparer.OrdinalIgnoreCase );
 
-            foreach ( var routeParameter in routeTemplate.Parameters )
+            for ( var i = 0; i < routeTemplate.Parameters.Count; i++ )
             {
+                var routeParameter = routeTemplate.Parameters[i];
                 routeParameters.Add( routeParameter.Name, CreateRouteInfo( routeParameter ) );
             }
 
-            foreach ( var parameter in context.Results )
+            for ( var i = 0; i < context.Results.Count; i++ )
             {
+                var parameter = context.Results[i];
+
                 if ( parameter.Source == Path || parameter.Source == ModelBinding || parameter.Source == Custom )
                 {
                     if ( routeParameters.TryGetValue( parameter.Name, out var routeInfo ) )

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

@@ -87,29 +87,25 @@ ApiParameterDescription CreateResult( ApiParameterDescriptionContext bindingCont
             {
                 var action = (IEdmAction) Context.RouteContext.Operation;
                 var apiVersion = Context.RouteContext.ApiVersion;
+
                 type = Context.TypeBuilder.NewActionParameters( Context.Services, action, apiVersion, Context.RouteContext.ActionDescriptor.ControllerName );
             }
             else
             {
                 type = type.SubstituteIfNecessary( new TypeSubstitutionContext( Context.Services, Context.TypeBuilder ) );
             }
 
-            if ( !type.Equals( modelMetadata.ModelType ) )
-            {
-                modelMetadata = new SubstitutedModelMetadata( modelMetadata, type );
-            }
-
             return new ApiParameterDescription()
             {
-                ModelMetadata = modelMetadata,
+                ModelMetadata = modelMetadata.SubstituteIfNecessary( type ),
                 Name = GetName( containerName, bindingContext ),
                 Source = source,
                 Type = type,
                 ParameterDescriptor = Parameter,
             };
         }
 
-        struct PropertyKey
+        readonly struct PropertyKey
         {
             public readonly Type ContainerType;
             public readonly string PropertyName;

src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/SubstitutedModelMetadata.cs

@@ -5,6 +5,8 @@
     using System;
     using System.Collections.Generic;
 
+#pragma warning disable CA1812
+
     sealed class SubstitutedModelMetadata : ModelMetadata
     {
         readonly ModelMetadata inner;