NH-3114 - Collection inside Component cannot be mapped to a different table (#691)

* NH-3114 - create a few test cases

The first test verifies mapping-by-code generates a correct mapping, while
the second test inserts an entity with the first component set but currently
also gets the second component collection initialized with the same contents
after retrieving.

This fixture is providing explicit column- and table-names to make sure
everything ends up where we expect it.

* NH-3114 - include the full property path in collection mappings

Since PropertyPath is used as dictionary key during mapping/customization,
only considering the local part may lead to customizations being applied to
the wrong members (or being applied multiple times, leaving only the last
invocation each as effective result).

With the parent path included, collection mappings on nested contexts (such
as components) now work properly even when the nested context is repeated
on the same type (such as having multiple properties of the same component
type on a root or subclass).

Author: BhaaLseN

src/NHibernate.Test/Async/NHSpecificTest/NH3114/ExplicitByCodeFixture.cs

@@ -0,0 +1,92 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by AsyncGenerator.
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NHibernate.Cfg.MappingSchema;
+using NHibernate.Mapping.ByCode;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.NH3114
+{
+	using System.Threading.Tasks;
+	[TestFixture]
+	public class ExplicitByCodeFixtureAsync : TestCaseMappingByCode
+	{
+		protected override HbmMapping GetMappings()
+		{
+			var mapper = new ModelMapper();
+			mapper.Class<Entity>(rc =>
+			{
+				rc.Id(i => i.Id, m => m.Generator(Generators.GuidComb));
+				rc.Component(p => p.FirstComponent,
+					m =>
+					{
+						m.Set(c => c.ComponentCollection,
+							c => c.Table("FirstTable"),
+							c => c.Element());
+						m.Property(p => p.ComponentProperty, p => p.Column("FirstProperty"));
+					});
+				rc.Component(p => p.SecondComponent,
+					m =>
+					{
+						m.Set(c => c.ComponentCollection,
+							c => c.Table("SecondTable"),
+							c => c.Element());
+						m.Property(p => p.ComponentProperty, p => p.Column("SecondProperty"));
+					});
+			});
+
+			return mapper.CompileMappingForAllExplicitlyAddedEntities();
+		}
+
+		protected override void OnTearDown()
+		{
+			using (ISession session = OpenSession())
+			using (ITransaction transaction = session.BeginTransaction())
+			{
+				session.Delete("from Entity");
+
+				session.Flush();
+				transaction.Commit();
+			}
+		}
+
+		[Test]
+		public async Task Component_WithSameType_ButDifferentTables_IsStoredInTheCorrectTableAndCollectionAsync()
+		{
+			Guid previouslySavedId;
+			using (var session = OpenSession())
+			{
+				var entity = new Entity();
+				entity.FirstComponent = new Component();
+				entity.FirstComponent.ComponentProperty = "First";
+				entity.FirstComponent.ComponentCollection = new List<string> { "FirstOne", "FirstTwo", "FirstThree" };
+				// not setting entity.SecondComponent; it must not contain the contents of entity.FirstComponent later
+				await (session.SaveOrUpdateAsync(entity));
+				await (session.FlushAsync());
+				previouslySavedId = entity.Id;
+			}
+
+			using (var session = OpenSession())
+			{
+				var entity = await (session.GetAsync<Entity>(previouslySavedId));
+				Assert.IsNotNull(entity);
+				Assert.IsNotNull(entity.FirstComponent);
+				Assert.AreEqual("First", entity.FirstComponent.ComponentProperty);
+				CollectionAssert.AreEquivalent(new[] { "FirstOne", "FirstTwo", "FirstThree" }, entity.FirstComponent.ComponentCollection);
+				//Assert.IsNull(entity.SecondComponent); // cannot check SecondComponent for null, since components are apparently always initialized
+				Assert.AreNotEqual("First", entity.SecondComponent.ComponentProperty);
+				CollectionAssert.IsEmpty(entity.SecondComponent.ComponentCollection);
+			}
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/NH3114/Entity.cs

@@ -0,0 +1,17 @@
+using System;
+using System.Collections.Generic;
+
+namespace NHibernate.Test.NHSpecificTest.NH3114
+{
+	public class Component
+	{
+		public virtual string ComponentProperty { get; set; }
+		public virtual ICollection<string> ComponentCollection { get; set; } = new List<string>();
+	}
+	public class Entity
+	{
+		public virtual Guid Id { get; set; }
+		public virtual Component FirstComponent { get; set; }
+		public virtual Component SecondComponent { get; set; }
+	}
+}

src/NHibernate.Test/NHSpecificTest/NH3114/ExplicitByCodeFixture.cs

@@ -0,0 +1,107 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NHibernate.Cfg.MappingSchema;
+using NHibernate.Mapping.ByCode;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.NH3114
+{
+	[TestFixture]
+	public class ExplicitByCodeFixture : TestCaseMappingByCode
+	{
+		protected override HbmMapping GetMappings()
+		{
+			var mapper = new ModelMapper();
+			mapper.Class<Entity>(rc =>
+			{
+				rc.Id(i => i.Id, m => m.Generator(Generators.GuidComb));
+				rc.Component(p => p.FirstComponent,
+					m =>
+					{
+						m.Set(c => c.ComponentCollection,
+							c => c.Table("FirstTable"),
+							c => c.Element());
+						m.Property(p => p.ComponentProperty, p => p.Column("FirstProperty"));
+					});
+				rc.Component(p => p.SecondComponent,
+					m =>
+					{
+						m.Set(c => c.ComponentCollection,
+							c => c.Table("SecondTable"),
+							c => c.Element());
+						m.Property(p => p.ComponentProperty, p => p.Column("SecondProperty"));
+					});
+			});
+
+			return mapper.CompileMappingForAllExplicitlyAddedEntities();
+		}
+
+		protected override void OnTearDown()
+		{
+			using (ISession session = OpenSession())
+			using (ITransaction transaction = session.BeginTransaction())
+			{
+				session.Delete("from Entity");
+
+				session.Flush();
+				transaction.Commit();
+			}
+		}
+
+		[Test]
+		public void Component_WithSameType_ButDifferentTables_ShouldBeMappedAccordingly()
+		{
+			var mappings = GetMappings();
+			var modelMapping = mappings.Items.OfType<HbmClass>().FirstOrDefault();
+			Assert.IsNotNull(modelMapping);
+			var lists = modelMapping.Items.OfType<HbmComponent>();
+			Assert.AreEqual(2, lists.Count());
+			var firstMapping = lists.FirstOrDefault(l => l.Name == nameof(Entity.FirstComponent));
+			Assert.IsNotNull(firstMapping);
+			var firstMember = firstMapping.Properties.OfType<HbmProperty>().FirstOrDefault(p => p.Name == nameof(Component.ComponentProperty));
+			Assert.IsNotNull(firstMember);
+			Assert.AreEqual("FirstProperty", firstMember.column);
+			var firstCollection = firstMapping.Items.OfType<HbmSet>().FirstOrDefault();
+			Assert.IsNotNull(firstCollection);
+			Assert.AreEqual("FirstTable", firstCollection.Table);
+			var secondMapping = lists.FirstOrDefault(l => l.Name == nameof(Entity.SecondComponent));
+			Assert.IsNotNull(secondMapping);
+			var secondMember = secondMapping.Properties.OfType<HbmProperty>().FirstOrDefault(p => p.Name == nameof(Component.ComponentProperty));
+			Assert.IsNotNull(secondMember);
+			Assert.AreEqual("SecondProperty", secondMember.column);
+			var secondCollection = secondMapping.Items.OfType<HbmSet>().FirstOrDefault();
+			Assert.IsNotNull(secondCollection);
+			Assert.AreEqual("SecondTable", secondCollection.Table);
+		}
+
+		[Test]
+		public void Component_WithSameType_ButDifferentTables_IsStoredInTheCorrectTableAndCollection()
+		{
+			Guid previouslySavedId;
+			using (var session = OpenSession())
+			{
+				var entity = new Entity();
+				entity.FirstComponent = new Component();
+				entity.FirstComponent.ComponentProperty = "First";
+				entity.FirstComponent.ComponentCollection = new List<string> { "FirstOne", "FirstTwo", "FirstThree" };
+				// not setting entity.SecondComponent; it must not contain the contents of entity.FirstComponent later
+				session.SaveOrUpdate(entity);
+				session.Flush();
+				previouslySavedId = entity.Id;
+			}
+
+			using (var session = OpenSession())
+			{
+				var entity = session.Get<Entity>(previouslySavedId);
+				Assert.IsNotNull(entity);
+				Assert.IsNotNull(entity.FirstComponent);
+				Assert.AreEqual("First", entity.FirstComponent.ComponentProperty);
+				CollectionAssert.AreEquivalent(new[] { "FirstOne", "FirstTwo", "FirstThree" }, entity.FirstComponent.ComponentCollection);
+				//Assert.IsNull(entity.SecondComponent); // cannot check SecondComponent for null, since components are apparently always initialized
+				Assert.AreNotEqual("First", entity.SecondComponent.ComponentProperty);
+				CollectionAssert.IsEmpty(entity.SecondComponent.ComponentCollection);
+			}
+		}
+	}
+}

src/NHibernate/Mapping/ByCode/Impl/CustomizersImpl/PropertyContainerCustomizer.cs

@@ -157,7 +157,7 @@ public void OneToOne<TProperty>(string notVisiblePropertyOrFieldName, Action<IOn
 			if (typeof(TProperty) != propertyOrFieldType)
 			{
 				throw new MappingException(string.Format("Wrong relation type. For the property/field '{0}' of {1} was expected a one-to-one with {2} but was {3}",
-														 notVisiblePropertyOrFieldName, typeof (TEntity).FullName, typeof (TProperty).Name, propertyOrFieldType.Name));
+														 notVisiblePropertyOrFieldName, typeof(TEntity).FullName, typeof(TProperty).Name, propertyOrFieldType.Name));
 			}
 			var memberOf = member.GetMemberFromReflectedType(typeof(TEntity));
 			RegisterOneToOneMapping(mapping, member, memberOf);
@@ -222,7 +222,7 @@ protected void RegisterSetMapping<TElement>(Action<ISetPropertiesMapper<TEntity,
 		{
 			foreach (var member in members)
 			{
-				collectionMapping(new SetPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(null, member), CustomizersHolder));
+				collectionMapping(new SetPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 				mapping(new CollectionElementRelationCustomizer<TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 			}
 		}
@@ -249,7 +249,7 @@ protected void RegisterBagMapping<TElement>(Action<IBagPropertiesMapper<TEntity,
 		{
 			foreach (var member in members)
 			{
-				collectionMapping(new BagPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(null, member), CustomizersHolder));
+				collectionMapping(new BagPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 				mapping(new CollectionElementRelationCustomizer<TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 			}
 		}
@@ -276,7 +276,7 @@ protected void RegisterListMapping<TElement>(Action<IListPropertiesMapper<TEntit
 		{
 			foreach (var member in members)
 			{
-				collectionMapping(new ListPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(null, member), CustomizersHolder));
+				collectionMapping(new ListPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 				mapping(new CollectionElementRelationCustomizer<TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 			}
 		}
@@ -338,11 +338,11 @@ protected virtual void RegisterIdBagMapping<TElement>(Expression<Func<TEntity, I
 			RegisterIdBagMapping(collectionMapping, mapping, memberOf);
 		}
 
-		protected virtual void RegisterIdBagMapping<TElement>(Action<IIdBagPropertiesMapper<TEntity, TElement>> collectionMapping, Action<ICollectionElementRelation<TElement>> mapping,params MemberInfo[] members)
+		protected virtual void RegisterIdBagMapping<TElement>(Action<IIdBagPropertiesMapper<TEntity, TElement>> collectionMapping, Action<ICollectionElementRelation<TElement>> mapping, params MemberInfo[] members)
 		{
 			foreach (var member in members)
 			{
-				collectionMapping(new IdBagPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(null, member), CustomizersHolder));
+				collectionMapping(new IdBagPropertiesCustomizer<TEntity, TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 				mapping(new CollectionElementRelationCustomizer<TElement>(explicitDeclarationsHolder, new PropertyPath(PropertyPath, member), CustomizersHolder));
 			}
 		}
@@ -361,11 +361,11 @@ private static void AssertCollectionElementType<TElement>(string propertyOrField
 		{
 			System.Type collectionElementType = memberInfo.GetPropertyOrFieldType().DetermineCollectionElementType();
 
-			if (typeof (TElement) != collectionElementType)
+			if (typeof(TElement) != collectionElementType)
 			{
 				var message = string.Format(
 					"Wrong collection element type. For the property/field '{0}' of {1} was expected a generic collection of {2} but was {3}",
-					propertyOrFieldName, typeof (TEntity).FullName, typeof (TElement).Name,
+					propertyOrFieldName, typeof(TEntity).FullName, typeof(TElement).Name,
 					collectionElementType != null ? collectionElementType.Name : "unknown");
 				throw new MappingException(message);
 			}
@@ -414,7 +414,7 @@ public void Map<TKey, TElement>(string notVisiblePropertyOrFieldName, Action<IMa
 			if (!typeof(TElement).Equals(collectionElementType) || !typeof(TKey).Equals(keyType))
 			{
 				throw new MappingException(string.Format("Wrong collection element type. For the property/field '{0}' of {1} was expected a dictionary of {2}/{3} but was {4}/{5}",
-																								 notVisiblePropertyOrFieldName, typeof(TEntity).FullName, typeof(TKey).Name, keyType.Name ,typeof(TElement).Name, collectionElementType.Name));
+																								 notVisiblePropertyOrFieldName, typeof(TEntity).FullName, typeof(TKey).Name, keyType.Name, typeof(TElement).Name, collectionElementType.Name));
 			}
 			MemberInfo memberOf = member.GetMemberFromReflectedType(typeof(TEntity));
 			RegisterMapMapping<TKey, TElement>(collectionMapping, keyMapping, mapping, member, memberOf);
@@ -505,4 +505,4 @@ public static MemberInfo GetPropertyOrFieldMatchingNameOrThrow(string memberName
 			return result;
 		}
 	}
-}
+}