Fix lazy set when adding a transient element with overridden Equals method (#2481)

Author: maca88

src/NHibernate.Test/Async/Extralazy/ExtraLazyFixture.cs

@@ -1326,6 +1326,52 @@ public async Task SetAddAsync(bool initialize)
 			}
 		}
 
+		[Test]
+		public async Task SetAddWithOverrideEqualsAsync()
+		{
+			User gavin;
+			User robert;
+			User tom;
+
+			using (var s = OpenSession())
+			using (var t = s.BeginTransaction())
+			{
+				gavin = new User("gavin", "secret");
+				robert = new User("robert", "secret");
+				tom = new User("tom", "secret");
+				await (s.PersistAsync(gavin));
+				await (s.PersistAsync(robert));
+				await (s.PersistAsync(tom));
+
+				gavin.Followers.Add(new UserFollower(gavin, robert));
+				gavin.Followers.Add(new UserFollower(gavin, tom));
+				robert.Followers.Add(new UserFollower(robert, tom));
+
+				Assert.That(gavin.Followers.Count, Is.EqualTo(2), "Gavin's documents count after adding 2");
+				Assert.That(robert.Followers.Count, Is.EqualTo(1), "Robert's followers count after adding one");
+
+				await (t.CommitAsync());
+			}
+
+			using (var s = OpenSession())
+			using (var t = s.BeginTransaction())
+			{
+				gavin = await (s.GetAsync<User>("gavin"));
+				robert = await (s.GetAsync<User>("robert"));
+				tom = await (s.GetAsync<User>("tom"));
+
+				// Re-add
+				Assert.That(gavin.Followers.Add(new UserFollower(gavin, robert)), Is.False, "Re-adding element");
+				Assert.That(NHibernateUtil.IsInitialized(gavin.Followers), Is.True, "Documents initialization status after re-adding");
+				Assert.That(gavin.Followers, Has.Count.EqualTo(2), "Gavin's followers count after re-adding");
+
+				// Add new
+				Assert.That(robert.Followers.Add(new UserFollower(robert, gavin)), Is.True, "Adding element");
+				Assert.That(NHibernateUtil.IsInitialized(gavin.Followers), Is.True, "Documents initialization status after adding");
+				Assert.That(gavin.Followers, Has.Count.EqualTo(2), "Robert's followers count after re-adding");
+			}
+		}
+
 		[TestCase(false, false)]
 		[TestCase(false, true)]
 		[TestCase(true, false)]

src/NHibernate.Test/Extralazy/ExtraLazyFixture.cs

@@ -1315,6 +1315,52 @@ public void SetAdd(bool initialize)
 			}
 		}
 
+		[Test]
+		public void SetAddWithOverrideEquals()
+		{
+			User gavin;
+			User robert;
+			User tom;
+
+			using (var s = OpenSession())
+			using (var t = s.BeginTransaction())
+			{
+				gavin = new User("gavin", "secret");
+				robert = new User("robert", "secret");
+				tom = new User("tom", "secret");
+				s.Persist(gavin);
+				s.Persist(robert);
+				s.Persist(tom);
+
+				gavin.Followers.Add(new UserFollower(gavin, robert));
+				gavin.Followers.Add(new UserFollower(gavin, tom));
+				robert.Followers.Add(new UserFollower(robert, tom));
+
+				Assert.That(gavin.Followers.Count, Is.EqualTo(2), "Gavin's documents count after adding 2");
+				Assert.That(robert.Followers.Count, Is.EqualTo(1), "Robert's followers count after adding one");
+
+				t.Commit();
+			}
+
+			using (var s = OpenSession())
+			using (var t = s.BeginTransaction())
+			{
+				gavin = s.Get<User>("gavin");
+				robert = s.Get<User>("robert");
+				tom = s.Get<User>("tom");
+
+				// Re-add
+				Assert.That(gavin.Followers.Add(new UserFollower(gavin, robert)), Is.False, "Re-adding element");
+				Assert.That(NHibernateUtil.IsInitialized(gavin.Followers), Is.True, "Documents initialization status after re-adding");
+				Assert.That(gavin.Followers, Has.Count.EqualTo(2), "Gavin's followers count after re-adding");
+
+				// Add new
+				Assert.That(robert.Followers.Add(new UserFollower(robert, gavin)), Is.True, "Adding element");
+				Assert.That(NHibernateUtil.IsInitialized(gavin.Followers), Is.True, "Documents initialization status after adding");
+				Assert.That(gavin.Followers, Has.Count.EqualTo(2), "Robert's followers count after re-adding");
+			}
+		}
+
 		[TestCase(false, false)]
 		[TestCase(false, true)]
 		[TestCase(true, false)]

src/NHibernate.Test/Extralazy/User.cs

@@ -50,6 +50,8 @@ public virtual ISet<Photo> Photos
 
 		public virtual ISet<UserPermission> Permissions { get; set; } = new HashSet<UserPermission>();
 
+		public virtual ISet<UserFollower> Followers { get; set; } = new HashSet<UserFollower>();
+
 		public virtual IList<Company> Companies { get; set; } = new List<Company>();
 
 		public virtual IList<CreditCard> CreditCards { get; set; } = new List<CreditCard>();

src/NHibernate.Test/Extralazy/UserFollower.cs

@@ -0,0 +1,46 @@
+using System;
+
+namespace NHibernate.Test.Extralazy
+{
+	public class UserFollower : IEquatable<UserFollower>
+	{
+		public UserFollower(User user, User follower)
+		{
+			User = user;
+			Follower = follower;
+		}
+
+		protected UserFollower()
+		{
+		}
+
+		public virtual int Id { get; set; }
+
+		public virtual User User { get; set; }
+
+		public virtual User Follower { get; set; }
+
+		public override bool Equals(object obj)
+		{
+			if (obj == null) return false;
+			if (ReferenceEquals(this, obj)) return true;
+			if (obj.GetType() != GetType()) return false;
+			return Equals((UserFollower) obj);
+		}
+
+		public virtual bool Equals(UserFollower other)
+		{
+			if (other == null) return false;
+			if (ReferenceEquals(this, other)) return true;
+			return Equals(User.Name, other.User.Name) && Equals(Follower.Name, other.Follower.Name);
+		}
+
+		public override int GetHashCode()
+		{
+			unchecked
+			{
+				return (User.Name.GetHashCode() * 397) ^ Follower.Name.GetHashCode();
+			}
+		}
+	}
+}

src/NHibernate.Test/Extralazy/UserGroup.hbm.xml

@@ -30,6 +30,10 @@
 			<key column="owner"/>
 			<one-to-many class="Document"/>
 		</set>
+    <set name="Followers" inverse="true" lazy="true" cascade="all,delete-orphan">
+      <key column="userId"/>
+      <one-to-many class="UserFollower"/>
+    </set>
 		<set name="Photos" inverse="true" lazy="true" where="Title like 'PRV%'" cascade="all,delete-orphan">
 			<key column="owner"/>
 			<one-to-many class="Photo"/>
@@ -77,6 +81,13 @@
     <property name="OriginalIndex" />
     <many-to-one name="Owner" not-null="true"/>
   </class>
+  <class name="UserFollower" table="user_follower">
+    <id name="Id">
+      <generator class="native"/>
+    </id>
+    <many-to-one name="User" column="userId" not-null="true"/>
+    <many-to-one name="Follower" column="followerId" not-null="true"/>
+  </class>
   <class name="CreditCard" table="credit_card">
     <id name="Id">
       <generator class="native"/>

src/NHibernate/Async/Collection/AbstractPersistentCollection.cs

@@ -74,13 +74,14 @@ public abstract partial class AbstractPersistentCollection : IPersistentCollecti
 			return null;
 		}
 
-		internal async Task<bool> IsTransientAsync(object element, CancellationToken cancellationToken)
+		internal async Task<bool> CanSkipElementExistenceCheckAsync(object element, CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();
 			var queryableCollection = (IQueryableCollection) Session.Factory.GetCollectionPersister(Role);
 			return
 				queryableCollection != null &&
 				queryableCollection.ElementType.IsEntityType &&
+				!queryableCollection.ElementPersister.EntityMetamodel.OverridesEquals &&
 				!element.IsProxy() &&
 				!Session.PersistenceContext.IsEntryFor(element) &&
 				await (ForeignKeys.IsTransientFastAsync(queryableCollection.ElementPersister.EntityName, element, Session, cancellationToken)).ConfigureAwait(false) == true;

src/NHibernate/Collection/AbstractPersistentCollection.cs

@@ -775,12 +775,13 @@ private AbstractQueueOperationTracker TryFlushAndGetQueueOperationTracker(string
 			return queueOperationTracker;
 		}
 
-		internal bool IsTransient(object element)
+		internal bool CanSkipElementExistenceCheck(object element)
 		{
 			var queryableCollection = (IQueryableCollection) Session.Factory.GetCollectionPersister(Role);
 			return
 				queryableCollection != null &&
 				queryableCollection.ElementType.IsEntityType &&
+				!queryableCollection.ElementPersister.EntityMetamodel.OverridesEquals &&
 				!element.IsProxy() &&
 				!Session.PersistenceContext.IsEntryFor(element) &&
 				ForeignKeys.IsTransientFast(queryableCollection.ElementPersister.EntityName, element, Session) == true;

src/NHibernate/Collection/Generic/PersistentGenericSet.cs

@@ -314,8 +314,8 @@ public bool Contains(T item)
 		public bool Add(T o)
 		{
 			// Skip checking the element existence in the database if we know that the element
-			// is transient and the operation queue is enabled
-			if (WasInitialized || !IsOperationQueueEnabled || !IsTransient(o))
+			// is transient, the mapped class does not override Equals method and the operation queue is enabled
+			if (WasInitialized || !IsOperationQueueEnabled || !CanSkipElementExistenceCheck(o))
 			{
 				var exists = IsOperationQueueEnabled ? ReadElementExistence(o, out _) : null;
 				if (!exists.HasValue)

src/NHibernate/Tuple/Entity/EntityMetamodel.cs

@@ -90,6 +90,7 @@ public EntityMetamodel(PersistentClass persistentClass, ISessionFactoryImplement
 			type = persistentClass.MappedClass;
 			rootType = persistentClass.RootClazz.MappedClass;
 			rootTypeAssemblyQualifiedName = rootType == null ? null : rootType.AssemblyQualifiedName;
+			OverridesEquals = type != null && ReflectHelper.OverridesEquals(type); // type will be null for dynamic entities
 
 			identifierProperty = PropertyFactory.BuildIdentifierProperty(persistentClass,
 			                                                             sessionFactory.GetIdentifierGenerator(rootName));
@@ -549,6 +550,8 @@ public StandardProperty[] Properties
 			get { return properties; }
 		}
 
+		internal bool OverridesEquals { get; private set; }
+
 		public int GetPropertyIndex(string propertyName)
 		{
 			int? index = GetPropertyIndexOrNull(propertyName);