Skip to content

Commit e59d124

Browse files
bahusoidfredericDelaporte
bahusoid
authored and
fredericDelaporte
committed
Fixed Equals method for transformers (#2000)
And reuse stateless transformer instances.
1 parent e305b67 commit e59d124

9 files changed

+90
-112
lines changed

‎src/NHibernate.Test/NHSpecificTest/NH3957/ResultTransformerEqualityFixture.cs

Lines changed: 43 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System.Collections.Generic;
2-
using System.Reflection;
32
using NHibernate.Transform;
43
using NUnit.Framework;
54

@@ -8,53 +7,20 @@ namespace NHibernate.Test.NHSpecificTest.NH3957
87
[TestFixture]
98
public class ResultTransformerEqualityFixture
109
{
11-
/// <summary>
12-
/// Allows to simulate a hashcode collision. Issue would be unpractical to test otherwise.
13-
/// Hashcode collision must be supported for avoiding unexpected and hard to reproduce failures.
14-
/// </summary>
15-
private void TweakHashcode(System.Type transformerToTweak, object hasher)
16-
{
17-
var hasherTargetField = transformerToTweak.GetField("Hasher", BindingFlags.Static | BindingFlags.NonPublic);
18-
if (!_hasherBackup.ContainsKey(transformerToTweak))
19-
_hasherBackup.Add(transformerToTweak, hasherTargetField.GetValue(null));
20-
21-
// Though hasher is a readonly field, this works at the time of this writing. If it starts breaking and cannot be fixed,
22-
// ignore those tests or throw them away.
23-
hasherTargetField.SetValue(null, hasher);
24-
}
25-
26-
private Dictionary<System.Type, object> _hasherBackup = new Dictionary<System.Type, object>();
27-
28-
[SetUp]
29-
public void Setup()
30-
{
31-
var hasherForAll = typeof(AliasToEntityMapResultTransformer)
32-
.GetField("Hasher", BindingFlags.Static | BindingFlags.NonPublic)
33-
.GetValue(null);
34-
TweakHashcode(typeof(DistinctRootEntityResultTransformer), hasherForAll);
35-
TweakHashcode(typeof(PassThroughResultTransformer), hasherForAll);
36-
TweakHashcode(typeof(RootEntityResultTransformer), hasherForAll);
37-
TweakHashcode(typeof(ToListResultTransformer), hasherForAll);
38-
}
39-
40-
[TearDown]
41-
public void TearDown()
42-
{
43-
// Restore those types hashcode. (Avoid impacting perf of other tests, avoid second level query cache
44-
// issues if it was holding cached entries (but would mean some tests have not cleaned up properly).)
45-
foreach(var backup in _hasherBackup)
46-
{
47-
TweakHashcode(backup.Key, backup.Value);
48-
}
49-
}
50-
10+
public class CustomAliasToEntityMapResultTransformer : AliasToEntityMapResultTransformer { }
11+
public class CustomDistinctRootEntityResultTransformer : DistinctRootEntityResultTransformer { }
12+
public class CustomPassThroughResultTransformer : PassThroughResultTransformer { }
13+
public class CustomRootEntityResultTransformer : RootEntityResultTransformer { }
14+
5115
// Non reg test case
5216
[Test]
5317
public void AliasToEntityMapEquality()
5418
{
5519
var transf1 = new AliasToEntityMapResultTransformer();
5620
var transf2 = new AliasToEntityMapResultTransformer();
21+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
5722

23+
Assert.That(set.Count, Is.EqualTo(1));
5824
Assert.IsTrue(transf1.Equals(transf2));
5925
Assert.IsTrue(transf2.Equals(transf1));
6026
}
@@ -63,8 +29,11 @@ public void AliasToEntityMapEquality()
6329
public void AliasToEntityMapAndDistinctRootEntityInequality()
6430
{
6531
var transf1 = new AliasToEntityMapResultTransformer();
66-
var transf2 = new DistinctRootEntityResultTransformer();
32+
var transf2 = new CustomAliasToEntityMapResultTransformer();
33+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
6734

35+
Assert.That(transf1.GetHashCode(), Is.EqualTo(transf2.GetHashCode()), "prerequisite");
36+
Assert.That(set.Count, Is.EqualTo(2));
6837
Assert.IsFalse(transf1.Equals(transf2));
6938
Assert.IsFalse(transf2.Equals(transf1));
7039
}
@@ -75,18 +44,35 @@ public void DistinctRootEntityEquality()
7544
{
7645
var transf1 = new DistinctRootEntityResultTransformer();
7746
var transf2 = new DistinctRootEntityResultTransformer();
47+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
7848

49+
Assert.That(set.Count, Is.EqualTo(1));
7950
Assert.IsTrue(transf1.Equals(transf2));
8051
Assert.IsTrue(transf2.Equals(transf1));
8152
}
8253

54+
[Test]
55+
public void DistinctRootEntityEqualityInequality()
56+
{
57+
var transf1 = new DistinctRootEntityResultTransformer();
58+
var transf2 = new CustomDistinctRootEntityResultTransformer();
59+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
60+
61+
Assert.That(transf1.GetHashCode(), Is.EqualTo(transf2.GetHashCode()), "prerequisite");
62+
Assert.That(set.Count, Is.EqualTo(2));
63+
Assert.IsFalse(transf1.Equals(transf2));
64+
Assert.IsFalse(transf2.Equals(transf1));
65+
}
66+
8367
// Non reg test case
8468
[Test]
8569
public void PassThroughEquality()
8670
{
8771
var transf1 = new PassThroughResultTransformer();
8872
var transf2 = new PassThroughResultTransformer();
73+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
8974

75+
Assert.That(set.Count, Is.EqualTo(1));
9076
Assert.IsTrue(transf1.Equals(transf2));
9177
Assert.IsTrue(transf2.Equals(transf1));
9278
}
@@ -95,8 +81,11 @@ public void PassThroughEquality()
9581
public void PassThroughAndRootEntityInequality()
9682
{
9783
var transf1 = new PassThroughResultTransformer();
98-
var transf2 = new RootEntityResultTransformer();
99-
84+
var transf2 = new CustomPassThroughResultTransformer();
85+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
86+
87+
Assert.That(transf1.GetHashCode(), Is.EqualTo(transf2.GetHashCode()), "prerequisite");
88+
Assert.That(set.Count, Is.EqualTo(2));
10089
Assert.IsFalse(transf1.Equals(transf2));
10190
Assert.IsFalse(transf2.Equals(transf1));
10291
}
@@ -107,7 +96,9 @@ public void RootEntityEquality()
10796
{
10897
var transf1 = new RootEntityResultTransformer();
10998
var transf2 = new RootEntityResultTransformer();
99+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
110100

101+
Assert.That(set.Count, Is.EqualTo(1));
111102
Assert.IsTrue(transf1.Equals(transf2));
112103
Assert.IsTrue(transf2.Equals(transf1));
113104
}
@@ -116,8 +107,11 @@ public void RootEntityEquality()
116107
public void RootEntityAndToListInequality()
117108
{
118109
var transf1 = new RootEntityResultTransformer();
119-
var transf2 = new ToListResultTransformer();
120-
110+
var transf2 = new CustomRootEntityResultTransformer();
111+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2, };
112+
113+
Assert.That(transf1.GetHashCode(), Is.EqualTo(transf2.GetHashCode()), "prerequisite");
114+
Assert.That(set.Count, Is.EqualTo(2));
121115
Assert.IsFalse(transf1.Equals(transf2));
122116
Assert.IsFalse(transf2.Equals(transf1));
123117
}
@@ -128,9 +122,11 @@ public void ToListEquality()
128122
{
129123
var transf1 = new ToListResultTransformer();
130124
var transf2 = new ToListResultTransformer();
125+
HashSet<IResultTransformer> set = new HashSet<IResultTransformer>() { transf1, transf2 };
131126

127+
Assert.That(set.Count, Is.EqualTo(1));
132128
Assert.IsTrue(transf1.Equals(transf2));
133129
Assert.IsTrue(transf2.Equals(transf1));
134130
}
135131
}
136-
}
132+
}

‎src/NHibernate/Criterion/CriteriaSpecification.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ public static class CriteriaSpecification
3131

3232
static CriteriaSpecification()
3333
{
34-
AliasToEntityMap = new AliasToEntityMapResultTransformer();
35-
RootEntity = new RootEntityResultTransformer();
36-
DistinctRootEntity = new DistinctRootEntityResultTransformer();
37-
Projection = new PassThroughResultTransformer();
34+
AliasToEntityMap = AliasToEntityMapResultTransformer.Instance;
35+
RootEntity = RootEntityResultTransformer.Instance;
36+
DistinctRootEntity = DistinctRootEntityResultTransformer.Instance;
37+
Projection = PassThroughResultTransformer.Instance;
3838
InnerJoin = JoinType.InnerJoin;
3939
FullJoin = JoinType.FullJoin;
4040
LeftJoin = JoinType.LeftOuterJoin;

‎src/NHibernate/Transform/AliasToEntityMapResultTransformer.cs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace NHibernate.Transform
66
[Serializable]
77
public class AliasToEntityMapResultTransformer : AliasedTupleSubsetResultTransformer
88
{
9-
private static readonly object Hasher = new object();
9+
internal static readonly AliasToEntityMapResultTransformer Instance = new AliasToEntityMapResultTransformer();
1010

1111
public override object TransformTuple(object[] tuple, string[] aliases)
1212
{
@@ -37,18 +37,16 @@ public override bool IsTransformedValueATupleElement(string[] aliases, int tuple
3737

3838
public override bool Equals(object obj)
3939
{
40-
if (obj == null || obj.GetHashCode() != Hasher.GetHashCode())
41-
{
40+
if (ReferenceEquals(obj, this))
41+
return true;
42+
if (obj == null)
4243
return false;
43-
}
44-
// NH-3957: do not rely on hashcode alone.
45-
// Must be the exact same type
46-
return obj.GetType() == typeof(AliasToEntityMapResultTransformer);
44+
return obj.GetType() == GetType();
4745
}
4846

4947
public override int GetHashCode()
5048
{
51-
return Hasher.GetHashCode();
49+
return System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(Instance);
5250
}
5351
}
54-
}
52+
}

‎src/NHibernate/Transform/CacheableResultTransformer.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public class CacheableResultTransformer : IResultTransformer
1919
// is private (as it should be for a singleton)
2020
//private const PassThroughResultTransformer ACTUAL_TRANSFORMER =
2121
// PassThroughResultTransformer.INSTANCE;
22-
private readonly PassThroughResultTransformer _actualTransformer = new PassThroughResultTransformer();
22+
private readonly PassThroughResultTransformer _actualTransformer = PassThroughResultTransformer.Instance;
2323

2424
public bool AutoDiscoverTypes { get; }
2525

@@ -358,7 +358,7 @@ public override bool Equals(object o)
358358
if (this == o)
359359
return true;
360360

361-
if (o == null || typeof (CacheableResultTransformer) != o.GetType())
361+
if (o == null || GetType() != o.GetType())
362362
return false;
363363

364364
var that = (CacheableResultTransformer) o;

‎src/NHibernate/Transform/DistinctRootEntityResultTransformer.cs

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace NHibernate.Transform
1010
public class DistinctRootEntityResultTransformer : IResultTransformer, ITupleSubsetResultTransformer
1111
{
1212
private static readonly INHibernateLogger log = NHibernateLogger.For(typeof(DistinctRootEntityResultTransformer));
13-
private static readonly object Hasher = new object();
13+
internal static readonly DistinctRootEntityResultTransformer Instance = new DistinctRootEntityResultTransformer();
1414

1515
internal sealed class Identity
1616
{
@@ -62,33 +62,26 @@ public IList TransformList(IList list)
6262

6363
public bool[] IncludeInTransform(String[] aliases, int tupleLength)
6464
{
65-
//return RootEntityResultTransformer.INSTANCE.includeInTransform(aliases, tupleLength);
66-
var transformer = new RootEntityResultTransformer();
67-
return transformer.IncludeInTransform(aliases, tupleLength);
65+
return RootEntityResultTransformer.Instance.IncludeInTransform(aliases, tupleLength);
6866
}
6967

70-
7168
public bool IsTransformedValueATupleElement(String[] aliases, int tupleLength)
7269
{
73-
//return RootEntityResultTransformer.INSTANCE.isTransformedValueATupleElement(null, tupleLength);
74-
var transformer = new RootEntityResultTransformer();
75-
return transformer.IsTransformedValueATupleElement(null, tupleLength);
70+
return RootEntityResultTransformer.Instance.IsTransformedValueATupleElement(null, tupleLength);
7671
}
7772

7873
public override bool Equals(object obj)
7974
{
80-
if (obj == null || obj.GetHashCode() != Hasher.GetHashCode())
81-
{
75+
if (ReferenceEquals(obj, this))
76+
return true;
77+
if (obj == null)
8278
return false;
83-
}
84-
// NH-3957: do not rely on hashcode alone.
85-
// Must be the exact same type
86-
return obj.GetType() == typeof(DistinctRootEntityResultTransformer);
79+
return obj.GetType() == GetType();
8780
}
8881

8982
public override int GetHashCode()
9083
{
91-
return Hasher.GetHashCode();
84+
return RuntimeHelpers.GetHashCode(Instance);
9285
}
9386
}
9487
}

‎src/NHibernate/Transform/PassThroughResultTransformer.cs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace NHibernate.Transform
77
[Serializable]
88
public class PassThroughResultTransformer : IResultTransformer, ITupleSubsetResultTransformer
99
{
10-
private static readonly object Hasher = new object();
10+
internal static readonly PassThroughResultTransformer Instance = new PassThroughResultTransformer();
1111

1212
#region IResultTransformer Members
1313

@@ -58,21 +58,18 @@ internal object[] UntransformToTuple(object transformed, bool isSingleResult)
5858
return isSingleResult ? new[] {transformed} : (object[]) transformed;
5959
}
6060

61-
6261
public override bool Equals(object obj)
6362
{
64-
if (obj == null || obj.GetHashCode() != Hasher.GetHashCode())
65-
{
63+
if (ReferenceEquals(obj, this))
64+
return true;
65+
if (obj == null)
6666
return false;
67-
}
68-
// NH-3957: do not rely on hashcode alone.
69-
// Must be the exact same type
70-
return obj.GetType() == typeof(PassThroughResultTransformer);
67+
return obj.GetType() == GetType();
7168
}
7269

7370
public override int GetHashCode()
7471
{
75-
return Hasher.GetHashCode();
72+
return System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(Instance);
7673
}
7774
}
78-
}
75+
}

‎src/NHibernate/Transform/RootEntityResultTransformer.cs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace NHibernate.Transform
77
[Serializable]
88
public class RootEntityResultTransformer : IResultTransformer, ITupleSubsetResultTransformer
99
{
10-
private static readonly object Hasher = new object();
10+
internal static readonly RootEntityResultTransformer Instance = new RootEntityResultTransformer();
1111

1212
public object TransformTuple(object[] tuple, string[] aliases)
1313
{
@@ -19,13 +19,11 @@ public IList TransformList(IList collection)
1919
return collection;
2020
}
2121

22-
2322
public bool IsTransformedValueATupleElement(String[] aliases, int tupleLength)
2423
{
2524
return true;
2625
}
2726

28-
2927
public bool[] IncludeInTransform(String[] aliases, int tupleLength)
3028
{
3129
bool[] includeInTransform;
@@ -43,18 +41,16 @@ public bool[] IncludeInTransform(String[] aliases, int tupleLength)
4341

4442
public override bool Equals(object obj)
4543
{
46-
if (obj == null || obj.GetHashCode() != Hasher.GetHashCode())
47-
{
44+
if (ReferenceEquals(obj, this))
45+
return true;
46+
if (obj == null)
4847
return false;
49-
}
50-
// NH-3957: do not rely on hashcode alone.
51-
// Must be the exact same type
52-
return obj.GetType() == typeof(RootEntityResultTransformer);
48+
return obj.GetType() == GetType();
5349
}
5450

5551
public override int GetHashCode()
5652
{
57-
return Hasher.GetHashCode();
53+
return System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(Instance);
5854
}
5955
}
60-
}
56+
}

0 commit comments

Comments
 (0)