Add failing test for NH-3772 + Proposed fix for API behavior for IUserCollectionType (NH-3772) when Instantiate returns an initialized collection

Author: Sebazzz

src/NHibernate.Test/NHSpecificTest/NH3772/CustomGenericCollection.cs

@@ -0,0 +1,55 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using NHibernate.Collection;
+using NHibernate.Collection.Generic;
+using NHibernate.Engine;
+using NHibernate.Persister.Collection;
+using NHibernate.UserTypes;
+
+namespace NHibernate.Test.NHSpecificTest.NH3772 {
+	public class CustomGenericCollection<T> : IUserCollectionType where T : class {
+		// Whether or not to use the failing test behavior, since it causes issues with TearDown
+		public static bool TestBehavior = false;
+
+		public IPersistentCollection Instantiate(ISessionImplementor session, ICollectionPersister persister) {
+			if (!TestBehavior) return new PersistentGenericSet<T>(session);
+
+			return new PersistentGenericSet<T>(session, new HashSet<T>(EqualityComparer<T>.Default));
+		}
+
+		public IPersistentCollection Wrap(ISessionImplementor session, object collection) {
+			var realCollection = (ISet<T>) collection;
+			return new PersistentGenericSet<T>(session, realCollection);
+		}
+
+		public IEnumerable GetElements(object collection) {
+			var realCollection = (ISet<T>) collection;
+			return realCollection.ToList();
+		}
+
+		public bool Contains(object collection, object entity) {
+			var realCollection = (ISet<T>) collection;
+			return realCollection.Contains((T) entity);
+		}
+
+		public object IndexOf(object collection, object entity) {
+			return -1; // no indexing supported
+		}
+
+		public object ReplaceElements(object original, object target, ICollectionPersister persister, object owner,
+									  IDictionary copyCache, ISessionImplementor session) {
+			var originalCollection = (ISet<T>) original;
+			var targetCollection = (ISet<T>) target;
+
+			targetCollection.Clear();
+			Utils.AddRange(targetCollection, originalCollection);
+
+			return targetCollection;
+		}
+
+		public object Instantiate(int anticipatedSize) {
+			return new HashSet<T>(EqualityComparer<T>.Default);
+		}
+	}
+}

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

@@ -0,0 +1,24 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+namespace NHibernate.Test.NHSpecificTest.NH3772 {
+	public class Entity
+	{
+		private ICollection<SubEntity> _subEntities;
+		public virtual int Id { get; set; }
+		public virtual string Name { get; set; }
+
+		public virtual ICollection<SubEntity> SubEntities {
+			get { return this._subEntities ?? (this._subEntities = new HashSet<SubEntity>()); }
+			set { this._subEntities = value; }
+		}
+	}
+
+	public class SubEntity
+	{
+		public virtual int Id { get; set; }
+		public virtual string Name { get; set; }
+	}
+}

src/NHibernate.Test/NHSpecificTest/NH3772/Fixture.cs

@@ -0,0 +1,82 @@
+using System.Diagnostics;
+using System.Linq;
+using NHibernate.Cfg.MappingSchema;
+using NHibernate.Linq;
+using NHibernate.Mapping.ByCode;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.NH3772 {
+	public class Fixture : TestCaseMappingByCode {
+		protected override HbmMapping GetMappings() {
+			var mapper = new ModelMapper();
+			mapper.Class<Entity>(rc => {
+				rc.Id(x => x.Id, m => m.Generator(Generators.Identity));
+				rc.Property(x => x.Name);
+
+				rc.Set(x => x.SubEntities,
+					   x => {
+						   x.Type<CustomGenericCollection<SubEntity>>();
+					   },
+					   x => {
+						   x.ManyToMany();
+					   });
+			});
+
+			mapper.Class<SubEntity>(rc => {
+				rc.Id(x => x.Id, m => m.Generator(Generators.Identity));
+				rc.Property(x => x.Name);
+			});
+
+			return mapper.CompileMappingForAllExplicitlyAddedEntities();
+		}
+
+		protected override void OnSetUp() {
+			using (var session = this.OpenSession()) {
+				using (var transaction = session.BeginTransaction()) {
+					var e1 = new Entity {Name = "Bob"};
+					session.Save(e1);
+
+					var e2 = new Entity {Name = "Sally"};
+					session.Save(e2);
+
+					var s1 = new SubEntity {Name = "Bob"};
+					session.Save(s1);
+
+					var s2 = new SubEntity {Name = "Sally"};
+					session.Save(s2);
+
+					session.Flush();
+					transaction.Commit();
+				}
+
+			}
+			CustomGenericCollection<SubEntity>.TestBehavior = true;
+		}
+
+		protected override void OnTearDown() {
+			CustomGenericCollection<SubEntity>.TestBehavior = false;
+
+			using (var session = this.OpenSession()) {
+				using (var transaction = session.BeginTransaction()) {
+					session.Delete("from NHibernate.Test.NHSpecificTest.NH3772.Entity");
+					session.Delete("from NHibernate.Test.NHSpecificTest.NH3772.SubEntity");
+
+					session.Flush();
+
+					transaction.Commit();
+				}
+			}
+		}
+
+		[Test]
+		public void CustomCollectionType_ThrowsHibernateException_WhenUserCollectionTypeReturnsInitializedPersistentCollection() {
+			using (var session = this.OpenSession()) {
+				using (session.BeginTransaction()) {
+					TestDelegate action = () => (from e in session.Query<Entity>() where e.Name == "Bob" select e).First();
+
+					Assert.That(action, Throws.InstanceOf<HibernateException>().And.Message.EqualTo("UserCollectionType.Instantiate should return a non-initialized persistent collection. Implement UserCollectionType.Instantiate(int anticipatedSize) to actually create the collection that needs to be wrapped by the persistent collection."));
+				}
+			}
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/NH3772/Utils.cs

@@ -0,0 +1,11 @@
+using System.Collections.Generic;
+
+namespace NHibernate.Test.NHSpecificTest.NH3772 {
+	public static class Utils {
+		public static void AddRange<T>(ICollection<T> collection, IEnumerable<T> items) {
+			foreach (var item in items) {
+				collection.Add(item);
+			}
+		}
+	}
+}

src/NHibernate.Test/NHibernate.Test.csproj

@@ -859,6 +859,10 @@
     <Compile Include="NHSpecificTest\NH3604\FixtureByCode.cs" />
     <Compile Include="NHSpecificTest\NH3754\Fixture.cs" />
     <Compile Include="NHSpecificTest\NH3754\Model.cs" />
+    <Compile Include="NHSpecificTest\NH3772\CustomGenericCollection.cs" />
+    <Compile Include="NHSpecificTest\NH3772\Entity.cs" />
+    <Compile Include="NHSpecificTest\NH3772\Fixture.cs" />
+    <Compile Include="NHSpecificTest\NH3772\Utils.cs" />
     <Compile Include="NHSpecificTest\NH646\Domain.cs" />
     <Compile Include="NHSpecificTest\NH646\Fixture.cs" />
     <Compile Include="NHSpecificTest\NH3234\Fixture.cs" />

src/NHibernate/Type/CustomCollectionType.cs

@@ -47,7 +47,17 @@ public IUserCollectionType UserType
 
 		public override IPersistentCollection Instantiate(ISessionImplementor session, ICollectionPersister persister, object key)
 		{
-			return userType.Instantiate(session, persister);
+			IPersistentCollection createdCollection = userType.Instantiate(session, persister);
+			EnsureNotInitialized(createdCollection);
+			return createdCollection;
+		}
+
+		private static void EnsureNotInitialized(IPersistentCollection createdCollection) 
+		{
+			if (createdCollection.WasInitialized) {
+				throw new HibernateException(
+					"UserCollectionType.Instantiate should return a non-initialized persistent collection. Implement UserCollectionType.Instantiate(int anticipatedSize) to actually create the collection that needs to be wrapped by the persistent collection.");
+			}
 		}
 
 		public override IPersistentCollection Wrap(ISessionImplementor session, object collection)
@@ -80,8 +90,7 @@ public override object IndexOf(object collection, object entity)
 			return userType.IndexOf(collection, entity);
 		}
 
-		public override object ReplaceElements(object original, object target, object owner, IDictionary copyCache,
-		                                       ISessionImplementor session)
+		public override object ReplaceElements(object original, object target, object owner, IDictionary copyCache, ISessionImplementor session)
 		{
 			ICollectionPersister cp = session.Factory.GetCollectionPersister(Role);
 			return userType.ReplaceElements(original, target, cp, owner, copyCache, session);