PersistentGenericSet: Avoid comparing values of possibly a primitive type with null, to fix NH-3590.

Author: oskarb

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

@@ -22,7 +22,7 @@ protected override void OnSetUp()
 			}
 		}
 
-		[Test, KnownBug("3590")]
+		[Test]
 		public void ShouldUpdate()
 		{
 			entity.Dates.Add(DateTime.Now);
@@ -45,7 +45,7 @@ public void ShouldUpdate()
 			}
 		}
 
-		[Test, KnownBug("3590")]
+		[Test]
 		public void ShouldMerge()
 		{
 			entity.Dates.Add(DateTime.Now);

src/NHibernate/Collection/Generic/PersistentGenericSet.cs

@@ -109,10 +109,15 @@ public override bool EqualsSnapshot(ICollectionPersister persister)
 				return false;
 			}
 
-			return !(from object obj in WrappedSet
-					 let oldValue = snapshot[(T)obj]
-					 where oldValue == null || elementType.IsDirty(oldValue, obj, Session)
-					 select obj).Any();
+
+			foreach (T obj in WrappedSet)
+			{
+				T oldValue;
+				if (!snapshot.TryGetValue(obj, out oldValue) || elementType.IsDirty(oldValue, obj, Session))
+					return false;
+			}
+
+			return true;
 		}
 
 		public override bool IsSnapshotEmpty(object snapshot)
@@ -138,10 +143,10 @@ public override void InitializeFromCache(ICollectionPersister persister, object
 			BeforeInitialize(persister, size);
 			for (int i = 0; i < size; i++)
 			{
-				var element = (T)persister.ElementType.Assemble(array[i], Session, owner);
+				var element = persister.ElementType.Assemble(array[i], Session, owner);
 				if (element != null)
 				{
-					WrappedSet.Add(element);
+					WrappedSet.Add((T) element);
 				}
 			}
 			SetInitialized();
@@ -160,10 +165,10 @@ public override string ToString()
 
 		public override object ReadFrom(IDataReader rs, ICollectionPersister role, ICollectionAliases descriptor, object owner)
 		{
-			var element = (T)role.ReadElement(rs, owner, descriptor.SuffixedElementAliases, Session);
+			var element = role.ReadElement(rs, owner, descriptor.SuffixedElementAliases, Session);
 			if (element != null)
 			{
-				_tempList.Add(element);
+				_tempList.Add((T) element);
 			}
 			return element;
 		}
@@ -219,22 +224,26 @@ public override IEnumerable GetDeletes(ICollectionPersister persister, bool inde
 
 			deletes.AddRange(sn.Where(obj => !WrappedSet.Contains(obj)));
 
-			deletes.AddRange(from obj in WrappedSet
-							 let oldValue = sn[obj]
-							 where oldValue != null && elementType.IsDirty(obj, oldValue, Session)
-							 select oldValue);
+
+			foreach (var obj in WrappedSet)
+			{
+				T oldValue;
+				if (sn.TryGetValue(obj, out oldValue) && elementType.IsDirty(obj, oldValue, Session))
+					deletes.Add(oldValue);
+			}
 
 			return deletes;
 		}
 
 		public override bool NeedsInserting(object entry, int i, IType elemType)
 		{
 			var sn = (ISetSnapshot<T>)GetSnapshot();
-			object oldKey = sn[(T)entry];
+			T oldKey;
+
 			// note that it might be better to iterate the snapshot but this is safe,
 			// assuming the user implements equals() properly, as required by the PersistentSet
 			// contract!
-			return oldKey == null || elemType.IsDirty(oldKey, entry, Session);
+			return !sn.TryGetValue((T) entry, out oldKey) || elemType.IsDirty(oldKey, entry, Session);
 		}
 
 		public override bool NeedsUpdating(object entry, int i, IType elemType)

src/NHibernate/Collection/Generic/SetHelpers/ISetSnapshot.cs

@@ -5,6 +5,6 @@ namespace NHibernate.Collection.Generic.SetHelpers
 {
 	internal interface ISetSnapshot<T> : ICollection<T>, ICollection
 	{
-		T this[T element] { get; }
+		bool TryGetValue(T element, out T value);
 	}
 }

src/NHibernate/Collection/Generic/SetHelpers/SetSnapShot.cs

@@ -88,17 +88,17 @@ public bool IsReadOnly
 			get { return ((ICollection<T>)_elements).IsReadOnly; }
 		}
 
-		public T this[T element]
+		public bool TryGetValue(T element, out T value)
 		{
-			get
+			var idx = _elements.IndexOf(element);
+			if (idx >= 0)
 			{
-				var idx = _elements.IndexOf(element);
-				if (idx >= 0)
-				{
-					return _elements[idx];
-				}
-				return default(T);
+				value = _elements[idx];
+				return true;
 			}
+
+			value = default(T);
+			return false;
 		}
 	}
 }