Implement CollectionHelper.GetHashCode that accepts IEqualityComparer<T>

- Also fix CollectionHelper.GetHashCode to account for the length of the collections

Co-authored-by: Frédéric Delaporte <[email protected]>

Author: hazzik

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

@@ -205,18 +205,5 @@ public void NativeSQLQuerySpecificationInequalityOnSpaceLengthes()
 			Assert.IsFalse(spec1.Equals(spec2));
 			Assert.IsFalse(spec2.Equals(spec1));
 		}
-
-		[Test]
-		public void NativeSQLQuerySpecificationInequalityOnSpaceOrderings()
-		{
-			var spec1 = new NativeSQLQuerySpecification("select blah", null,
-				new[] { "space1", "space2" });
-			var spec2 = new NativeSQLQuerySpecification("select blah", null,
-				new[] { "space2", "space1" });
-
-			TweakHashcode(spec2, spec1.GetHashCode());
-			Assert.IsFalse(spec1.Equals(spec2));
-			Assert.IsFalse(spec2.Equals(spec1));
-		}
 	}
 }

src/NHibernate.Test/UtilityTest/CollectionHelperFixture.cs

@@ -1,4 +1,6 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
+using NHibernate.Engine;
 using NHibernate.Util;
 using NUnit.Framework;
 
@@ -180,6 +182,69 @@ public void SetsWithoutSameCountShouldBeInequal()
 			Assert.That(CollectionHelper.SetEquals(c2, c1), Is.False);
 		}
 
+		[Test]
+		public void MapsWithSameContentShouldHaveSameHashCode()
+		{
+			var d1 = new Dictionary<string, string> { { "1", "2" }, { "3", "4" } };
+			var d2 = new Dictionary<string, string> { { "1", "2" }, { "3", "4" } };
+			Assert.That(
+				CollectionHelper.GetHashCode(d1, KvpStringComparer.Instance),
+				Is.EqualTo(CollectionHelper.GetHashCode(d2, KvpStringComparer.Instance)));
+		}
+
+		// Failure of following tests is not an error from GetHashCode semantic viewpoint, but it causes it
+		// to be potentially inefficients for usages in dictionnaries or hashset.
+
+		[Test]
+		public void MapsWithSameCountButDistinctKeysShouldNotHaveSameHashCode()
+		{
+			var d1 = new Dictionary<string, string> { { "1", "2" }, { "3", "4" } };
+			var d2 = new Dictionary<string, string> { { "1", "2" }, { "4", "3" } };
+			Assert.That(
+				CollectionHelper.GetHashCode(d1, KvpStringComparer.Instance),
+				Is.Not.EqualTo(CollectionHelper.GetHashCode(d2, KvpStringComparer.Instance)));
+		}
+
+		[Test]
+		public void MapsWithSameCountButDistinctValuesShouldNotHaveSameHashCode()
+		{
+			var d1 = new Dictionary<string, string> { { "1", "2" }, { "3", "4" } };
+			var d2 = new Dictionary<string, string> { { "1", "2" }, { "3", "3" } };
+			Assert.That(
+				CollectionHelper.GetHashCode(d1, KvpStringComparer.Instance),
+				Is.Not.EqualTo(CollectionHelper.GetHashCode(d2, KvpStringComparer.Instance)));
+		}
+
+		[Test]
+		public void MapsWithoutSameCountShouldNotHaveSameHashCode()
+		{
+			var d1 = new Dictionary<string, string> { { "1", "2" }, { "3", "4" } };
+			var d2 = new Dictionary<string, string> { { "1", "2" } };
+			Assert.That(
+				CollectionHelper.GetHashCode(d1, KvpStringComparer.Instance),
+				Is.Not.EqualTo(CollectionHelper.GetHashCode(d2, KvpStringComparer.Instance)));
+		}
+
+		internal class KvpStringComparer : IEqualityComparer<KeyValuePair<string, string>>
+		{
+			public static readonly KvpStringComparer Instance = new KvpStringComparer();
+
+			public bool Equals(KeyValuePair<string, string> x, KeyValuePair<string, string> y)
+			{
+				return StringComparer.Ordinal.Equals(x.Key, y.Key) &&
+					StringComparer.Ordinal.Equals(x.Value, y.Value);
+			}
+
+			public int GetHashCode(KeyValuePair<string, string> obj)
+			{
+				unchecked
+				{
+					return 397 * StringComparer.Ordinal.GetHashCode(obj.Key) ^
+						(obj.Value != null ? StringComparer.Ordinal.GetHashCode(obj.Value) : 0);
+				}
+			}
+		}
+
 		#endregion
 
 		#region List/Sequence

src/NHibernate/Cache/FilterKey.cs

@@ -25,9 +25,9 @@ public FilterKey(string name, IEnumerable<KeyValuePair<string, object>> @params,
 
 		public override int GetHashCode()
 		{
-			int result = 13;
+			var result = 13;
 			result = 37 * result + _filterName.GetHashCode();
-			result = 37 * result + CollectionHelper.GetHashCode(_filterParameters);
+			result = 37 * result + CollectionHelper.GetHashCode(_filterParameters, NamedParameterComparer.Instance);
 			return result;
 		}
 

src/NHibernate/Cache/NamedParameterComparer.cs

@@ -0,0 +1,26 @@
+using System;
+using System.Collections.Generic;
+using NHibernate.Engine;
+
+namespace NHibernate.Cache
+{
+	internal class NamedParameterComparer : IEqualityComparer<KeyValuePair<string, TypedValue>>
+	{
+		public static readonly NamedParameterComparer Instance = new NamedParameterComparer();
+
+		public bool Equals(KeyValuePair<string, TypedValue> x, KeyValuePair<string, TypedValue> y)
+		{
+			return StringComparer.Ordinal.Equals(x.Key, y.Key) &&
+			       (ReferenceEquals(x.Value, y.Value) || x.Value != null && y.Value != null && x.Value.Equals(y.Value));
+		}
+
+		public int GetHashCode(KeyValuePair<string, TypedValue> obj)
+		{
+			unchecked
+			{
+				return 397 * StringComparer.Ordinal.GetHashCode(obj.Key) ^
+				       (obj.Value != null ? obj.Value.GetHashCode() : 0);
+			}
+		}
+	}
+}

src/NHibernate/Cache/QueryKey.cs

@@ -161,7 +161,7 @@ public int ComputeHashCode()
 				result = 37 * result + _firstRow.GetHashCode();
 				result = 37 * result + _maxRows.GetHashCode();
 
-				result = 37 * result + (_namedParameters == null ? 0 : CollectionHelper.GetHashCode(_namedParameters));
+				result = 37 * result + (_namedParameters == null ? 0 : CollectionHelper.GetHashCode(_namedParameters, NamedParameterComparer.Instance));
 
 				for (int i = 0; i < _types.Length; i++)
 				{

src/NHibernate/Engine/Query/QueryPlanCache.cs

@@ -214,9 +214,9 @@ protected HQLQueryPlanKey(System.Type queryTypeDiscriminator, string query, bool
 				unchecked
 				{
 					var hash = query.GetHashCode();
-					hash = 29*hash + (shallow ? 1 : 0);
-					hash = 29*hash + CollectionHelper.GetHashCode(filterNames);
-					hash = 29*hash + queryTypeDiscriminator.GetHashCode();
+					hash = 29 * hash + (shallow ? 1 : 0);
+					hash = 29 * hash + CollectionHelper.GetHashCode(filterNames, filterNames.Comparer);
+					hash = 29 * hash + queryTypeDiscriminator.GetHashCode();
 					hashCode = hash;
 				}
 			}
@@ -289,7 +289,7 @@ public FilterQueryPlanKey(string query, string collectionRole, bool shallow, IDi
 				int hash = query.GetHashCode();
 				hash = 29 * hash + collectionRole.GetHashCode();
 				hash = 29 * hash + (shallow ? 1 : 0);
-				hash = 29 * hash + CollectionHelper.GetHashCode(filterNames);
+				hash = 29 * hash + CollectionHelper.GetHashCode(filterNames, filterNames.Comparer);
 				hashCode = hash;
 			}
 

src/NHibernate/Engine/Query/Sql/NativeSQLQuerySpecification.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using NHibernate.Util;
 
@@ -7,7 +8,7 @@ public class NativeSQLQuerySpecification
 	{
 		private readonly string queryString;
 		private readonly INativeSQLQueryReturn[] sqlQueryReturns;
-		private readonly ISet<string> querySpaces;
+		private readonly HashSet<string> querySpaces;
 		private readonly int hashCode;
 
 		public NativeSQLQuerySpecification(
@@ -26,10 +27,10 @@ public NativeSQLQuerySpecification(
 			int hCode = queryString.GetHashCode();
 			unchecked
 			{
-				hCode = 29 * hCode + CollectionHelper.GetHashCode(this.querySpaces);
+				hCode = 29 * hCode + CollectionHelper.GetHashCode(this.querySpaces, this.querySpaces.Comparer);
 				if (this.sqlQueryReturns != null)
 				{
-					hCode = 29 * hCode + CollectionHelper.GetHashCode(this.sqlQueryReturns);
+					hCode = 29 * hCode + ArrayHelper.ArrayGetHashCode(this.sqlQueryReturns);
 				}
 			}
 
@@ -63,10 +64,11 @@ public override bool Equals(object obj)
 
 			// NH-3956: hashcode inequality rules out equality, but hashcode equality is not enough.
 			// Code taken back from 8e92af3f and amended according to NH-1931.
-			return hashCode == that.hashCode &&
+			return
+				hashCode == that.hashCode &&
 				queryString.Equals(that.queryString) &&
-				CollectionHelper.SequenceEquals(querySpaces, that.querySpaces) &&
-				CollectionHelper.SequenceEquals<INativeSQLQueryReturn>(sqlQueryReturns, that.sqlQueryReturns);
+				CollectionHelper.SetEquals(querySpaces, that.querySpaces) &&
+				ArrayHelper.ArrayEquals(sqlQueryReturns, that.sqlQueryReturns);
 		}
 
 		public override int GetHashCode()

src/NHibernate/Mapping/Table.cs

@@ -1083,14 +1083,13 @@ public bool Equals(ForeignKeyKey x, ForeignKeyKey y)
 			{
 				// NH : Different implementation to prevent NH930 (look test)
 				return //y.referencedClassName.Equals(x.referencedClassName) &&
-					CollectionHelper.SequenceEquals<Column>(y.columns, x.columns)
-					&& CollectionHelper.SequenceEquals<Column>(y.referencedColumns, x.referencedColumns);
+					CollectionHelper.SequenceEquals(y.columns, x.columns)
+					&& CollectionHelper.SequenceEquals(y.referencedColumns, x.referencedColumns);
 			}
 
 			public int GetHashCode(ForeignKeyKey obj)
 			{
-				int result = CollectionHelper.GetHashCode(obj.columns) ^ CollectionHelper.GetHashCode(obj.referencedColumns);
-				return result;
+				return CollectionHelper.GetHashCode(obj.columns) ^ CollectionHelper.GetHashCode(obj.referencedColumns);
 			}
 
 			#endregion

src/NHibernate/Util/CollectionHelper.cs

@@ -305,27 +305,26 @@ public static bool DictionaryEquals(IDictionary a, IDictionary b)
 		/// <summary>
 		/// Computes a hash code for <paramref name="coll"/>.
 		/// </summary>
-		/// <remarks>The hash code is computed as the sum of hash codes of
-		/// individual elements, so that the value is independent of the
+		/// <remarks>The hash code is computed as the sum of hash codes of individual elements
+		/// plus a length of the collection, so that the value is independent of the
 		/// collection iteration order.
 		/// </remarks>
 		[Obsolete("It has no more usages in NHibernate and will be removed in a future version.")]
 		public static int GetHashCode(IEnumerable coll)
 		{
-			unchecked
-			{
-				int result = 0;
+			var result = 0;
 
-				foreach (object obj in coll)
+			foreach (var obj in coll)
+			{
+				unchecked
 				{
 					if (obj != null)
-					{
 						result += obj.GetHashCode();
-					}
+					result++;
 				}
-
-				return result;
 			}
+
+			return result;
 		}
 
 		/// <summary>
@@ -582,24 +581,49 @@ public IEnumerator GetEnumerator()
 		/// <summary>
 		/// Computes a hash code for <paramref name="coll"/>.
 		/// </summary>
-		/// <remarks>The hash code is computed as the sum of hash codes of
-		/// individual elements, so that the value is independent of the
+		/// <remarks>The hash code is computed as the sum of hash codes of individual elements
+		/// plus a length of the collection, so that the value is independent of the
 		/// collection iteration order.
 		/// </remarks>
 		public static int GetHashCode<T>(IEnumerable<T> coll)
 		{
-			unchecked
-			{
-				int result = 0;
+			var result = 0;
 
-				foreach (T obj in coll)
+			foreach (var obj in coll)
+			{
+				unchecked
 				{
-					if (!obj.Equals(default(T)))
+					if (!ReferenceEquals(obj, null))
 						result += obj.GetHashCode();
+					result++;
 				}
+			}
+
+			return result;
+		}
 
-				return result;
+		/// <summary>
+		/// Computes a hash code for <paramref name="coll"/>.
+		/// </summary>
+		/// <remarks>The hash code is computed as the sum of hash codes of individual elements
+		/// plus a length of the collection, so that the value is independent of the
+		/// collection iteration order.
+		/// </remarks>
+		public static int GetHashCode<T>(IEnumerable<T> coll, IEqualityComparer<T> comparer)
+		{
+			var result = 0;
+
+			foreach (var obj in coll)
+			{
+				unchecked
+				{
+					if (!ReferenceEquals(obj, null))
+						result += comparer.GetHashCode(obj);
+					result++;
+				}
 			}
+
+			return result;
 		}
 
 		/// <summary>