AutoMap() is now aware of overridden properties in derived types (#2632)

* AutoMap() is now aware of overridden properties in derived types

Previously, `AutoMap()` would not take into account properties that
have been overridden in derived classes:

```
public class A
{
    public int Foo { get; set; }
}

public class B : A
{
    public new string Foo { get; set; }
}
```

So in this instance, mapping class `B` would result in `Foo` being mapped
as an `int`, even though it has been overridden as a `string` using the `new`
keyword.

With this commit, `AutoMap()` is now aware of overridden properties and
in this example, will map `Foo` as a `string`.

Closes #2566

* fix formatting

Author: gmarz

src/Nest/CommonAbstractions/Extensions/TypeExtensions.cs

@@ -29,7 +29,8 @@ internal static class TypeExtensions
 
 		//this contract is only used to resolve properties in class WE OWN.
 		//these are not subject to change depending on what the user passes as connectionsettings
-		private static readonly ElasticContractResolver JsonContract = new ElasticContractResolver(new ConnectionSettings(), null);
+		private static readonly ElasticContractResolver JsonContract =
+			new ElasticContractResolver(new ConnectionSettings(), null);
 
 		public delegate T ObjectActivator<out T>(params object[] args);
 
@@ -51,7 +52,7 @@ internal static object CreateGenericInstance(this Type t, Type[] closeOver, para
 			return closedType.CreateInstance(args);
 		}
 
-		internal static T CreateInstance<T>(this Type t, params object[] args) => (T)t.CreateInstance(args);
+		internal static T CreateInstance<T>(this Type t, params object[] args) => (T) t.CreateInstance(args);
 
 		internal static object CreateInstance(this Type t, params object[] args)
 		{
@@ -71,7 +72,7 @@ internal static object CreateInstance(this Type t, params object[] args)
 			var ctor = constructors.FirstOrDefault();
 			if (ctor == null)
 				throw new Exception($"Cannot create an instance of {t.FullName} because it has no constructor taking {args.Length} arguments");
-			activator = (ObjectActivator<object>)generic.Invoke(null, new[] { ctor });
+			activator = (ObjectActivator<object>) generic.Invoke(null, new[] {ctor});
 			CachedActivators.TryAdd(key, activator);
 			return activator(args);
 		}
@@ -115,11 +116,12 @@ private static ObjectActivator<T> GetActivator<T>(ConstructorInfo ctor)
 				Expression.Lambda(typeof(ObjectActivator<T>), newExp, param);
 
 			//compile it
-			ObjectActivator<T> compiled = (ObjectActivator<T>)lambda.Compile();
+			ObjectActivator<T> compiled = (ObjectActivator<T>) lambda.Compile();
 			return compiled;
 		}
 
-		internal static IList<JsonProperty> GetCachedObjectProperties(this Type t, MemberSerialization memberSerialization = MemberSerialization.OptIn)
+		internal static IList<JsonProperty> GetCachedObjectProperties(this Type t,
+			MemberSerialization memberSerialization = MemberSerialization.OptIn)
 		{
 			IList<JsonProperty> propertyDictionary;
 			if (CachedTypeProperties.TryGetValue(t, out propertyDictionary))
@@ -139,21 +141,53 @@ internal static IList<PropertyInfo> AllPropertiesCached(this Type t)
 			return propertyInfos;
 		}
 
-		/// <summary> Returns inherited properties with reflectedType set to base type</summary>
+		/// <summary>
+		/// Returns inherited properties with reflectedType set to base type
+		/// </summary>
 		private static IEnumerable<PropertyInfo> AllPropertiesNotCached(this Type type)
 		{
+			var propertiesByName = new Dictionary<string, PropertyInfo>();
 			do
 			{
 				foreach (var propertyInfo in type.GetProperties(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly))
-					yield return propertyInfo;
+				{
+					if (propertiesByName.ContainsKey(propertyInfo.Name))
+					{
+						if (IsHidingMember(propertyInfo))
+						{
+							propertiesByName[propertyInfo.Name] = propertyInfo;
+						}
+					}
+					else
+					{
+						propertiesByName.Add(propertyInfo.Name, propertyInfo);
+					}
+				}
 #if DOTNETCORE
 				type = type.GetTypeInfo()?.BaseType;
 			} while (type?.GetTypeInfo()?.BaseType != null);
 #else
 				type = type.BaseType;
 			} while (type?.BaseType != null);
 #endif
+			return propertiesByName.Values;
+		}
 
+		/// <summary>
+		/// Determines if a property is overriding an inherited property of its base class
+		/// </summary>
+		private static bool IsHidingMember(PropertyInfo propertyInfo)
+		{
+#if DOTNETCORE
+			var baseType = propertyInfo.DeclaringType?.GetTypeInfo()?.BaseType;
+#else
+			var baseType = propertyInfo.DeclaringType?.BaseType;
+#endif
+			var baseProperty = baseType?.GetProperty(propertyInfo.Name);
+			if (baseProperty == null) return false;
+			var derivedGetMethod = propertyInfo.GetGetMethod().GetBaseDefinition();
+			return derivedGetMethod?.ReturnType != propertyInfo.PropertyType;
 		}
 	}
 }
+

src/Nest/Mapping/Types/PropertyBase.cs

@@ -36,11 +36,12 @@ protected PropertyBase(TypeName typeName)
 		{
 			Type = typeName;
 		}
+
 #pragma warning disable 618
 		protected PropertyBase(FieldType type) : this(type.GetStringValue()) { }
 #pragma warning restore 618
 
-		protected string DebugDisplay => $"Type: {Type.DebugDisplay}, Name: {Name.DebugDisplay} ";
+		protected string DebugDisplay => $"Type: {Type?.DebugDisplay}, Name: {Name?.DebugDisplay} ";
 
 		public PropertyName Name { get; set; }
 		public virtual TypeName Type { get; set; }

src/Tests/ClientConcepts/HighLevel/Mapping/AutoMap.doc.cs

@@ -55,9 +55,9 @@ public void MappingManually()
 					.Map<Company>(m => m
 						.Properties(ps => ps
 							.Text(s => s
-								.Name(c => c.Name) //<1> map `Name` as a `string` type
+									.Name(c => c.Name) //<1> map `Name` as a `string` type
 							)
-							.Object<Employee>(o => o  //<2> map `Employees` as an `object` type, mapping each of the properties of `Employee`
+							.Object<Employee>(o => o //<2> map `Employees` as an `object` type, mapping each of the properties of `Employee`
 								.Name(c => c.Employees)
 								.Properties(eps => eps
 									.Text(s => s
@@ -117,7 +117,7 @@ public void MappingManually()
 				}
 			};
 
-			Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 		}
 
 		[U]
@@ -273,7 +273,7 @@ public void UsingAutoMap()
 			};
 
 
-			Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 		}
 		/**[IMPORTANT]
 		 * ====
@@ -353,7 +353,7 @@ public void OverridingAutoMappedProperties()
 				}
 			};
 
-			Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 
 			/**
 			 * `.AutoMap()` is idempotent; calling it before or after manually
@@ -371,7 +371,7 @@ public void OverridingAutoMappedProperties()
 					)
 				);
 
-			Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 		}
 
 		/**[[attribute-mapping]]
@@ -758,7 +758,58 @@ public void OverridingAutoMappedAttributes()
 				}
 			};
 
-			Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
+		}
+
+		public class ParentWithStringId : Parent
+		{
+			public new string Id { get; set; }
+		}
+
+		[U]
+		public void OverridingInheritedProperties()
+		{
+			var descriptor = new CreateIndexDescriptor("myindex")
+				.Mappings(ms => ms
+					.Map<ParentWithStringId>(m => m
+						.AutoMap()
+					)
+				);
+
+			var expected = new
+			{
+				mappings = new
+				{
+					parent = new
+					{
+						properties = new
+						{
+							id = new
+							{
+								type = "text",
+								fields = new
+								{
+									keyword = new
+									{
+										ignore_above = 256,
+										type = "keyword"
+									}
+								}
+							}
+						}
+					}
+				}
+			};
+
+			var settings = WithConnectionSettings(s => s
+				.InferMappingFor<ParentWithStringId>(m => m
+					.TypeName("parent")
+					.Ignore(p => p.Description)
+					.Ignore(p => p.IgnoreMe)
+				)
+			);
+
+			settings.Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 		}
 
 		/**[float]
@@ -830,7 +881,7 @@ public void IgnoringProperties()
 				)
 			);
 
-			settings.Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			settings.Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 		}
 
 		public class Parent
@@ -840,7 +891,10 @@ public class Parent
 			public string IgnoreMe { get; set; }
 		}
 
-		public class Child : Parent { }
+		public class Child : Parent
+		{
+		}
+
 		[U]
 		public void IgnoringInheritedProperties()
 		{
@@ -860,16 +914,20 @@ public void IgnoringInheritedProperties()
 					{
 						properties = new
 						{
-							desc = new {
-								fields = new {
-									keyword = new {
+							desc = new
+							{
+								fields = new
+								{
+									keyword = new
+									{
 										ignore_above = 256,
 										type = "keyword"
 									}
 								},
 								type = "text"
 							},
-							id = new {
+							id = new
+							{
 								type = "integer"
 							}
 						}
@@ -884,8 +942,9 @@ public void IgnoringInheritedProperties()
 				)
 			);
 
-			settings.Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			settings.Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 		}
+
 		/**[float]
 		 * == Mapping Recursion
 		 * If you notice in our previous `Company` and `Employee` examples, the `Employee` type is recursive
@@ -933,7 +992,7 @@ public void ControllingRecursionDepth()
 				}
 			};
 
-			Expect(expected).WhenSerializing((ICreateIndexRequest)descriptor);
+			Expect(expected).WhenSerializing((ICreateIndexRequest) descriptor);
 
 			/** Now let's specify a maxRecursion of 3 */
 			var withMaxRecursionDescriptor = new CreateIndexDescriptor("myindex")
@@ -981,7 +1040,7 @@ public void ControllingRecursionDepth()
 				}
 			};
 
-			Expect(expectedWithMaxRecursion).WhenSerializing((ICreateIndexRequest)withMaxRecursionDescriptor);
+			Expect(expectedWithMaxRecursion).WhenSerializing((ICreateIndexRequest) withMaxRecursionDescriptor);
 		}
 
 		[U]
@@ -1002,7 +1061,7 @@ public void PutMappingAlsoAdheresToMaxRecursion()
 				}
 			};
 
-			Expect(expected).WhenSerializing((IPutMappingRequest)descriptor);
+			Expect(expected).WhenSerializing((IPutMappingRequest) descriptor);
 
 			var withMaxRecursionDescriptor = new PutMappingDescriptor<A>().AutoMap(3);
 
@@ -1039,7 +1098,7 @@ public void PutMappingAlsoAdheresToMaxRecursion()
 				}
 			};
 
-			Expect(expectedWithMaxRecursion).WhenSerializing((IPutMappingRequest)withMaxRecursionDescriptor);
+			Expect(expectedWithMaxRecursion).WhenSerializing((IPutMappingRequest) withMaxRecursionDescriptor);
 		}
 		//endhide
 
@@ -1058,15 +1117,17 @@ public class DisableDocValuesPropertyVisitor : NoopPropertyVisitor
 			public override void Visit(
 				INumberProperty type,
 				PropertyInfo propertyInfo,
-				ElasticsearchPropertyAttributeBase attribute) //<1> Override the `Visit` method on `INumberProperty` and set `DocValues = false`
+				ElasticsearchPropertyAttributeBase
+					attribute) //<1> Override the `Visit` method on `INumberProperty` and set `DocValues = false`
 			{
 				type.DocValues = false;
 			}
 
 			public override void Visit(
 				IBooleanProperty type,
 				PropertyInfo propertyInfo,
-				ElasticsearchPropertyAttributeBase attribute) //<2> Similarily, override the `Visit` method on `IBooleanProperty` and set `DocValues = false`
+				ElasticsearchPropertyAttributeBase
+					attribute) //<2> Similarily, override the `Visit` method on `IBooleanProperty` and set `DocValues = false`
 			{
 				type.DocValues = false;
 			}
@@ -1133,7 +1194,8 @@ public void UsingACustomPropertyVisitor()
 		 */
 		public class EverythingIsAStringPropertyVisitor : NoopPropertyVisitor
 		{
-			public override IProperty Visit(PropertyInfo propertyInfo, ElasticsearchPropertyAttributeBase attribute) => new TextProperty();
+			public override IProperty Visit(PropertyInfo propertyInfo,
+				ElasticsearchPropertyAttributeBase attribute) => new TextProperty();
 		}
 
 		[U]