diff --git a/src/NHibernate.Test/NHSpecificTest/NH3956/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3956/Fixture.cs new file mode 100644 index 00000000000..adf8c31e4b2 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3956/Fixture.cs @@ -0,0 +1,221 @@ +using System.Reflection; +using NHibernate.Engine.Query.Sql; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3956 +{ + [TestFixture] + public class Fixture + { + private static readonly FieldInfo HashCodeField = + typeof(NativeSQLQuerySpecification).GetField("hashCode", BindingFlags.Instance | BindingFlags.NonPublic); + + /// + /// Allows to simulate a hashcode collision. Issue would be unpractical to test otherwise. + /// Hashcode collision must be supported for avoiding unexpected and hard to reproduce failures. + /// + private void TweakHashcode(NativeSQLQuerySpecification specToTweak, int hashCode) + { + // Though hashCode is a readonly field, this works at the time of this writing. If it starts breaking and cannot be fixed, + // ignore those tests or throw them away. + HashCodeField.SetValue(specToTweak, hashCode); + } + + [Test] + public void NativeSQLQuerySpecificationEqualityOnQuery() + { + var spec1 = new NativeSQLQuerySpecification("select blah", null, null); + // Empty spaces array should be equivalent to null. Maybe results too but current implementation does not handle this. + var spec2 = new NativeSQLQuerySpecification("select blah", null, new string[0]); + + Assert.IsTrue(spec1.Equals(spec2)); + Assert.IsTrue(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnQuery() + { + var spec1 = new NativeSQLQuerySpecification("select blah", null, null); + var spec2 = new NativeSQLQuerySpecification("select blargh", null, null); + + TweakHashcode(spec2, spec1.GetHashCode()); + Assert.IsFalse(spec1.Equals(spec2)); + Assert.IsFalse(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationEqualityOnReturns() + { + var spec1 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character), + new NativeSQLQueryScalarReturn("alias2", NHibernateUtil.Decimal) + }, + null); + var spec2 = new NativeSQLQuerySpecification("select blah", + new[] + { + // Aliases ordering matters, do not get them mixed. (But type does not matter, I guess this means + // a case with a same query having sames aliases but different types is never supposed to happen + // Note that Hibernate does test other properties as of this writing: + // https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/engine/query/spi/sql/NativeSQLQueryScalarReturn.java + // And same on other INativeSQLQueryReturn implementations. + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character), + new NativeSQLQueryScalarReturn("alias2", NHibernateUtil.Decimal) + }, + // Empty spaces array should be equivalent to null. + new string[0]); + + Assert.IsTrue(spec1.Equals(spec2)); + Assert.IsTrue(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnNullReturn() + { + var spec1 = new NativeSQLQuerySpecification("select blah", null, null); + var spec2 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character) + }, + null); + + TweakHashcode(spec2, spec1.GetHashCode()); + Assert.IsFalse(spec1.Equals(spec2)); + Assert.IsFalse(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnReturnContents() + { + var spec1 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character), + new NativeSQLQueryScalarReturn("alias2", NHibernateUtil.Decimal) + }, + null); + var spec2 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character), + new NativeSQLQueryScalarReturn("alias3", NHibernateUtil.Decimal) + }, + null); + + TweakHashcode(spec2, spec1.GetHashCode()); + Assert.IsFalse(spec1.Equals(spec2)); + Assert.IsFalse(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnReturnLengthes() + { + var spec1 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character), + }, + null); + var spec2 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character), + new NativeSQLQueryScalarReturn("alias2", NHibernateUtil.Decimal) + }, + null); + + TweakHashcode(spec2, spec1.GetHashCode()); + Assert.IsFalse(spec1.Equals(spec2)); + Assert.IsFalse(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnReturnOrderings() + { + var spec1 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character), + new NativeSQLQueryScalarReturn("alias2", NHibernateUtil.Decimal) + }, + null); + var spec2 = new NativeSQLQuerySpecification("select blah", + new[] + { + new NativeSQLQueryScalarReturn("alias2", NHibernateUtil.Decimal), + new NativeSQLQueryScalarReturn("alias1", NHibernateUtil.Character) + }, + null); + + TweakHashcode(spec2, spec1.GetHashCode()); + Assert.IsFalse(spec1.Equals(spec2)); + Assert.IsFalse(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationEqualityOnSpaces() + { + var spec1 = new NativeSQLQuerySpecification("select blah", null, + new[] { "space1", "space2" }); + var spec2 = new NativeSQLQuerySpecification("select blah", null, + new[] { "space1", "space2" }); + + Assert.IsTrue(spec1.Equals(spec2)); + Assert.IsTrue(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnNullSpace() + { + var spec1 = new NativeSQLQuerySpecification("select blah", null, null); + var spec2 = new NativeSQLQuerySpecification("select blah", null, + new[] { "space1", "space2" }); + + TweakHashcode(spec2, spec1.GetHashCode()); + Assert.IsFalse(spec1.Equals(spec2)); + Assert.IsFalse(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnSpaceContents() + { + var spec1 = new NativeSQLQuerySpecification("select blah", null, + new[] { "space1", "space2" }); + var spec2 = new NativeSQLQuerySpecification("select blah", null, + new[] { "space1", "space3" }); + + TweakHashcode(spec2, spec1.GetHashCode()); + Assert.IsFalse(spec1.Equals(spec2)); + Assert.IsFalse(spec2.Equals(spec1)); + } + + [Test] + public void NativeSQLQuerySpecificationInequalityOnSpaceLengthes() + { + var spec1 = new NativeSQLQuerySpecification("select blah", null, + new[] { "space1", "space2" }); + var spec2 = new NativeSQLQuerySpecification("select blah", null, + new[] { "space1", "space2", "space3" }); + + TweakHashcode(spec2, spec1.GetHashCode()); + 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)); + } + } +} diff --git a/src/NHibernate.Test/NHibernate.Test.csproj b/src/NHibernate.Test/NHibernate.Test.csproj index bc3ee315644..da460c658d8 100644 --- a/src/NHibernate.Test/NHibernate.Test.csproj +++ b/src/NHibernate.Test/NHibernate.Test.csproj @@ -943,6 +943,7 @@ + diff --git a/src/NHibernate/Engine/Query/Sql/NativeSQLQuerySpecification.cs b/src/NHibernate/Engine/Query/Sql/NativeSQLQuerySpecification.cs index d8f4bddc82e..c2b2803c2ee 100644 --- a/src/NHibernate/Engine/Query/Sql/NativeSQLQuerySpecification.cs +++ b/src/NHibernate/Engine/Query/Sql/NativeSQLQuerySpecification.cs @@ -61,8 +61,12 @@ public override bool Equals(object obj) if (that == null) return false; - // NHibernate different impl.: NativeSQLQuerySpecification is immutable and the hash is calculated at Ctor - return hashCode == that.hashCode; + // 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 && + queryString.Equals(that.queryString) && + CollectionHelper.CollectionEquals(querySpaces, that.querySpaces) && + CollectionHelper.CollectionEquals(sqlQueryReturns, that.sqlQueryReturns); } public override int GetHashCode() diff --git a/src/NHibernate/Util/CollectionHelper.cs b/src/NHibernate/Util/CollectionHelper.cs index b8f78c5bef0..2fb88295a0c 100644 --- a/src/NHibernate/Util/CollectionHelper.cs +++ b/src/NHibernate/Util/CollectionHelper.cs @@ -216,6 +216,12 @@ public IEnumerator GetEnumerator() public static readonly ICollection EmptyCollection = EmptyMap; public static readonly IList EmptyList = new EmptyListClass(); + /// + /// Determines if two collections have equals elements, with the same ordering. + /// + /// The first collection. + /// The second collection. + /// true if collection are equals, false otherwise. public static bool CollectionEquals(ICollection c1, ICollection c2) { if (c1 == c2) @@ -620,6 +626,13 @@ public static bool SetEquals(ISet a, ISet b) return true; } + /// + /// Determines if two collections have equals elements, with the same ordering. + /// + /// The type of the elements. + /// The first collection. + /// The second collection. + /// true if collection are equals, false otherwise. public static bool CollectionEquals(ICollection c1, ICollection c2) { if (c1 == c2)