Dependency properties should keep custom attributes

samples/aspnetcore/SwaggerODataSample/Models/Address.cs

@@ -1,40 +1,45 @@
 namespace Microsoft.Examples.Models
 {
-    using Microsoft.AspNet.OData.Query;
-    using System;
+    using System.Runtime.Serialization;
 
     /// <summary>
     /// Represents an address.
     /// </summary>
+    [DataContract]
     public class Address
     {
         /// <summary>
         /// Gets or sets the address identifier.
         /// </summary>
+        [IgnoreDataMember]
         public int Id { get; set; }
 
         /// <summary>
         /// Gets or sets the street address.
         /// </summary>
         /// <value>The street address.</value>
+        [DataMember]
         public string Street { get; set; }
 
         /// <summary>
         /// Gets or sets the address city.
         /// </summary>
         /// <value>The address city.</value>
+        [DataMember]
         public string City { get; set; }
 
         /// <summary>
         /// Gets or sets the address state.
         /// </summary>
         /// <value>The address state.</value>
+        [DataMember]
         public string State { get; set; }
 
         /// <summary>
         /// Gets or sets the address zip code.
         /// </summary>
         /// <value>The address zip code.</value>
+        [DataMember(Name = "zip")]
         public string ZipCode { get; set; }
     }
 }

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

@@ -96,14 +96,26 @@ static Type GenerateTypeIfNeeded( IEdmStructuredType structuredType, BuilderCont
             const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.Instance;
 
             var properties = new List<ClassProperty>();
-            var structuralProperties = structuredType.Properties().ToDictionary( p => p.Name, StringComparer.OrdinalIgnoreCase );
+            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;
+                if ( clrProperty != null )
+                {
+                    mappedClrProperties.Add( clrProperty, property );
+                }
+            }
+
             foreach ( var property in clrType.GetProperties( bindingFlags ) )
             {
-                if ( !structuralProperties.TryGetValue( property.Name, out var structuralProperty ) )
+                if ( !structuralProperties.TryGetValue( property.Name, out var structuralProperty ) &&
+                     !mappedClrProperties.TryGetValue( property, out structuralProperty ) )
                 {
                     clrTypeMatchesEdmType = false;
                     continue;
@@ -129,7 +141,7 @@ static Type GenerateTypeIfNeeded( IEdmStructuredType structuredType, BuilderCont
                         {
                             clrTypeMatchesEdmType = false;
                             hasUnfinishedTypes = true;
-                            dependentProperties.Add( new PropertyDependency( elementKey, true, property.Name ) );
+                            dependentProperties.Add( new PropertyDependency( elementKey, true, property.Name, property.DeclaredAttributes() ) );
                             continue;
                         }
 
@@ -162,7 +174,7 @@ static Type GenerateTypeIfNeeded( IEdmStructuredType structuredType, BuilderCont
                     {
                         clrTypeMatchesEdmType = false;
                         hasUnfinishedTypes = true;
-                        dependentProperties.Add( new PropertyDependency( propertyTypeKey, false, property.Name ) );
+                        dependentProperties.Add( new PropertyDependency( propertyTypeKey, false, property.Name, property.DeclaredAttributes() ) );
                         continue;
                     }
                 }
@@ -232,12 +244,7 @@ static TypeBuilder CreateTypeBuilderFromSignature( ModuleBuilder moduleBuilder,
             {
                 var type = property.Type;
                 var name = property.Name;
-                var propertyBuilder = AddProperty( typeBuilder, type, name );
-
-                foreach ( var attribute in property.Attributes )
-                {
-                    propertyBuilder.SetCustomAttribute( attribute );
-                }
+                AddProperty( typeBuilder, type, name, property.Attributes );
             }
 
             return typeBuilder;
@@ -258,7 +265,7 @@ static IDictionary<EdmTypeKey, TypeInfo> ResolveDependencies( BuilderContext con
                     dependentOnType = IEnumerableOfT.MakeGenericType( dependentOnType ).GetTypeInfo();
                 }
 
-                AddProperty( propertyDependency.DependentType, dependentOnType, propertyDependency.PropertyName );
+                AddProperty( propertyDependency.DependentType, dependentOnType, propertyDependency.PropertyName, propertyDependency.CustomAttributes );
             }
 
             var keys = edmTypes.Keys.ToArray();
@@ -276,7 +283,7 @@ static IDictionary<EdmTypeKey, TypeInfo> ResolveDependencies( BuilderContext con
             return edmTypes;
         }
 
-        static PropertyBuilder AddProperty( TypeBuilder addTo, Type shouldBeAdded, string name )
+        static PropertyBuilder AddProperty( TypeBuilder addTo, Type shouldBeAdded, string name, IEnumerable<CustomAttributeBuilder> customAttributes )
         {
             const MethodAttributes propertyMethodAttributes = MethodAttributes.Public | MethodAttributes.SpecialName | MethodAttributes.HideBySig;
             var field = addTo.DefineField( "_" + name, shouldBeAdded, FieldAttributes.Private );
@@ -297,6 +304,11 @@ static PropertyBuilder AddProperty( TypeBuilder addTo, Type shouldBeAdded, strin
             propertyBuilder.SetGetMethod( getter );
             propertyBuilder.SetSetMethod( setter );
 
+            foreach ( var attribute in customAttributes )
+            {
+                propertyBuilder.SetCustomAttribute( attribute );
+            }
+
             return propertyBuilder;
         }
 

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

@@ -1,5 +1,6 @@
 namespace Microsoft.AspNet.OData
 {
+    using System.Collections.Generic;
     using System.Reflection.Emit;
 
     /// <summary>
@@ -13,13 +14,15 @@ internal class PropertyDependency
         /// <param name="dependentOnTypeKey">The key of the type the property has a dependency on.</param>
         /// <param name="propertyName">The name of the property.</param>
         /// <param name="isCollection">Whether the property is a collection or not.</param>
-        internal PropertyDependency( EdmTypeKey dependentOnTypeKey,  bool isCollection, string propertyName )
+        /// <param name="customAttributes">A collection of custom attribute builders.</param>
+        internal PropertyDependency( EdmTypeKey dependentOnTypeKey,  bool isCollection, string propertyName, IEnumerable<CustomAttributeBuilder> customAttributes )
         {
             Arg.NotNull<EdmTypeKey>( dependentOnTypeKey, nameof( dependentOnTypeKey ) );
             Arg.NotNull<string>( propertyName, nameof( propertyName ) );
 
             DependentOnTypeKey = dependentOnTypeKey;
             PropertyName = propertyName;
+            CustomAttributes = customAttributes;
             IsCollection = isCollection;
         }
 
@@ -42,5 +45,10 @@ internal PropertyDependency( EdmTypeKey dependentOnTypeKey,  bool isCollection,
         /// Gets a value indicating whether the property is a collection.
         /// </summary>
         internal bool IsCollection { get; }
+
+        /// <summary>
+        /// Gets custom attribute builders of the property.
+        /// </summary>
+        internal IEnumerable<CustomAttributeBuilder> CustomAttributes { get; }
     }
 }

test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Contact.cs

@@ -1,19 +1,27 @@
 namespace Microsoft.AspNet.OData
 {
     using System.Collections.Generic;
+    using System.Runtime.Serialization;
 
+    [DataContract]
     public class Contact
     {
+        [DataMember]
         public int ContactId { get; set; }
 
+        [DataMember( Name = "first_name" )]
         public string FirstName { get; set; }
 
+        [DataMember]
         public string LastName { get; set; }
 
-        public string Email { get; set; }
+        [DataMember]
+        public Email Email { get; set; }
 
+        [DataMember( Name = "telephone" )]
         public string Phone { get; set; }
 
+        [DataMember]
         public List<Address> Addresses { get; set; }
     }
 }

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

@@ -1,6 +1,8 @@
 namespace Microsoft.AspNet.OData
 {
     using FluentAssertions;
+    using FluentAssertions.Common;
+    using System.Runtime.Serialization;
     using Microsoft.AspNet.OData.Builder;
     using Microsoft.AspNetCore.Mvc;
     using Microsoft.Extensions.DependencyInjection;
@@ -207,7 +209,7 @@ public void type_should_match_edm_with_child_entity_substitution( Type originalT
             nextType.Should().HaveProperty<int>( nameof( Contact.ContactId ) );
             nextType.Should().HaveProperty<string>( nameof( Contact.FirstName ) );
             nextType.Should().HaveProperty<string>( nameof( Contact.LastName ) );
-            nextType.Should().HaveProperty<string>( nameof( Contact.Email ) );
+            nextType.Should().HaveProperty<Email>( nameof( Contact.Email ) );
             nextType.Should().HaveProperty<string>( nameof( Contact.Phone ) );
             nextType = nextType.GetRuntimeProperty( nameof( Contact.Addresses ) ).PropertyType.GetGenericArguments()[0];
             nextType.GetRuntimeProperties().Should().HaveCount( 5 );
@@ -408,6 +410,61 @@ public void substitute_should_resolve_types_that_reference_a_model_that_match_th
             substitutionType.Should().NotBeOfType<TypeBuilder>();
         }
 
+        [Fact]
+        public void substituted_type_should_have_renamed_with_attribute_properties_from_original_type()
+        {
+            // arrange
+            var modelBuilder = new ODataConventionModelBuilder();
+            modelBuilder.EntitySet<Contact>( "Contacts" );
+
+            var context = NewContext( modelBuilder.GetEdmModel() );
+            var originalType = typeof( Contact );
+
+            // act
+            var substitutedType = originalType.SubstituteIfNecessary( context );
+
+            // assert
+            substitutedType.Should().HaveProperty<string>( nameof( Contact.FirstName ) );
+            substitutedType.GetRuntimeProperty( nameof( Contact.FirstName ) ).Should().NotBeNull();
+            substitutedType.GetRuntimeProperty( nameof( Contact.FirstName ) ).HasAttribute<DataMemberAttribute>();
+        }
+
+        [Fact]
+        public void substituted_type_should_keep_custom_attributes_on_dependency_property()
+        {
+            // arrange
+            var modelBuilder = new ODataConventionModelBuilder();
+            modelBuilder.EntitySet<Contact>( "Contacts" );
+
+            var context = NewContext( modelBuilder.GetEdmModel() );
+            var originalType = typeof( Contact );
+
+            // act
+            var substitutedType = originalType.SubstituteIfNecessary( context );
+
+            // assert
+            substitutedType.GetRuntimeProperty( nameof( Contact.Email ) ).Should().NotBeNull();
+            substitutedType.GetRuntimeProperty( nameof( Contact.Email ) ).HasAttribute<DataMemberAttribute>();
+        }
+
+        [Fact]
+        public void substituted_type_should_keep_custom_attributes_on_collection_dependency_property()
+        {
+            // arrange
+            var modelBuilder = new ODataConventionModelBuilder();
+            modelBuilder.EntitySet<Contact>( "Contacts" );
+
+            var context = NewContext( modelBuilder.GetEdmModel() );
+            var originalType = typeof( Contact );
+
+            // act
+            var substitutedType = originalType.SubstituteIfNecessary( context );
+
+            // assert
+            substitutedType.GetRuntimeProperty( nameof( Contact.Addresses ) ).Should().NotBeNull();
+            substitutedType.GetRuntimeProperty( nameof( Contact.Addresses ) ).HasAttribute<DataMemberAttribute>();
+        }
+
         public static IEnumerable<object[]> SubstitutionNotRequiredData
         {
             get

test/Microsoft.AspNetCore.OData.Versioning.ApiExplorer.Tests/AspNet.OData/Email.cs

@@ -0,0 +1,14 @@
+namespace Microsoft.AspNet.OData
+{
+    using System.Runtime.Serialization;
+
+    [DataContract]
+    public class Email
+    {
+        [DataMember]
+        public string Server { get; set; }
+
+        [DataMember]
+        public string Username { get; set; }
+    }
+}