Skip to content

Commit c9ef58d

Browse files
fredericDelaportefredericDelaporte
committed
NH-3954: fix and test cases.
1 parent 0f88aa1 commit c9ef58d

File tree

5 files changed

+324
-20
lines changed

5 files changed

+324
-20
lines changed

‎src/NHibernate.Test/NHSpecificTest/NH3954/Entity.cs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
using System;
2+
using System.Collections.Generic;
3+
4+
namespace NHibernate.Test.NHSpecificTest.NH3954
5+
{
6+
class Entity1
7+
{
8+
}
9+
class Entity1FakeProxy
10+
{
11+
}
12+
class Entity1FakeProxy2
13+
{
14+
}
15+
16+
class Entity2
17+
{
18+
}
19+
class Entity2FakeProxy
20+
{
21+
}
22+
class Entity2FakeProxy2
23+
{
24+
}
25+
26+
class Entity3
27+
{
28+
}
29+
class Entity3FakeProxy
30+
{
31+
}
32+
class Entity3FakeProxy2
33+
{
34+
}
35+
36+
class Entity4
37+
{
38+
}
39+
class Entity4FakeProxy
40+
{
41+
}
42+
class Entity4FakeProxy2
43+
{
44+
}
45+
46+
class Entity5
47+
{
48+
}
49+
class Entity5FakeProxy
50+
{
51+
}
52+
class Entity5FakeProxy2
53+
{
54+
}
55+
}

‎src/NHibernate.Test/NHSpecificTest/NH3954/EqualsFixture.cs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
using System;
2+
using System.Reflection;
3+
using NHibernate.Proxy;
4+
using NHibernate.Proxy.DynamicProxy;
5+
using NHibernate.Type;
6+
using NUnit.Framework;
7+
8+
namespace NHibernate.Test.NHSpecificTest.NH3954
9+
{
10+
[TestFixture]
11+
public class EqualsFixture
12+
{
13+
private static readonly FieldInfo HashCodeField =
14+
typeof(ProxyCacheEntry).GetField("_hashCode", BindingFlags.Instance | BindingFlags.NonPublic);
15+
16+
/// <summary>
17+
/// Allows to simulate a hashcode collision. Issue would be unpractical to test otherwise.
18+
/// Hashcode collision must be supported for avoiding unexpected and hard to reproduce failures.
19+
/// </summary>
20+
private void TweakEntry(ProxyCacheEntry entryToTweak, int hashcode)
21+
{
22+
// Though hashCode is a readonly field, this works at the time of this writing. If it starts breaking and cannot be fixed,
23+
// ignore those tests or throw them away.
24+
HashCodeField.SetValue(entryToTweak, hashcode);
25+
}
26+
27+
// Non reg test case
28+
[Test]
29+
public void TypeEquality()
30+
{
31+
var entry1 = new ProxyCacheEntry(typeof(Entity1), null);
32+
var entry2 = new ProxyCacheEntry(typeof(Entity1), new System.Type[0]);
33+
Assert.IsTrue(entry1.Equals(entry2));
34+
Assert.IsTrue(entry2.Equals(entry1));
35+
}
36+
37+
[Test]
38+
public void TypeInequality()
39+
{
40+
var entry1 = new ProxyCacheEntry(typeof(Entity1), null);
41+
var entry2 = new ProxyCacheEntry(typeof(Entity2), null);
42+
TweakEntry(entry2, entry1.GetHashCode());
43+
Assert.IsFalse(entry1.Equals(entry2));
44+
Assert.IsFalse(entry2.Equals(entry1));
45+
}
46+
47+
// Non reg test case
48+
[Test]
49+
public void InterfaceEquality()
50+
{
51+
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy) });
52+
// Interfaces order inverted on purpose: must be supported.
53+
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IProxy), typeof(INHibernateProxy) });
54+
Assert.IsTrue(entry1.Equals(entry2));
55+
Assert.IsTrue(entry2.Equals(entry1));
56+
}
57+
58+
// Non reg test case
59+
[Test]
60+
public void InterfaceEqualityWithLotOfUnordererdAndDupInterfaces()
61+
{
62+
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy), typeof(IType), typeof(IDisposable), typeof(IFilter) });
63+
// Interfaces order inverted and duplicated on purpose: must be supported.
64+
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IType), typeof(IProxy), typeof(IFilter), typeof(IDisposable), typeof(IType), typeof(IFilter), typeof(INHibernateProxy) });
65+
Assert.IsTrue(entry1.Equals(entry2));
66+
Assert.IsTrue(entry2.Equals(entry1));
67+
}
68+
69+
[Test]
70+
public void InterfaceInequality()
71+
{
72+
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy) });
73+
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IProxy) });
74+
TweakEntry(entry2, entry1.GetHashCode());
75+
Assert.IsFalse(entry1.Equals(entry2));
76+
Assert.IsFalse(entry2.Equals(entry1));
77+
}
78+
}
79+
}

‎src/NHibernate.Test/NHSpecificTest/NH3954/ProxyCacheFixture.cs

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
using System.Collections.Concurrent;
2+
using System.Reflection;
3+
using NHibernate.Proxy;
4+
using NHibernate.Proxy.DynamicProxy;
5+
using NUnit.Framework;
6+
7+
namespace NHibernate.Test.NHSpecificTest.NH3954
8+
{
9+
[TestFixture, Explicit("Demonstrates bug impact on cache, but which tests will fail is a bit unpredictable")]
10+
public class ProxyCacheFixture
11+
{
12+
private ProxyCache _cache;
13+
private ConcurrentDictionary<ProxyCacheEntry, System.Type> _internalCache;
14+
private int _hashCode1;
15+
private int _hashCode2;
16+
17+
private static readonly FieldInfo InternalCacheField =
18+
typeof(ProxyCache).GetField("cache", BindingFlags.Static | BindingFlags.NonPublic);
19+
20+
[SetUp]
21+
public void SetUp()
22+
{
23+
_cache = new ProxyCache();
24+
25+
_internalCache = (ConcurrentDictionary<ProxyCacheEntry, System.Type>)InternalCacheField.GetValue(null);
26+
27+
_cache.StoreProxyType(typeof(Entity1FakeProxy), typeof(Entity1));
28+
_cache.StoreProxyType(typeof(Entity2FakeProxy), typeof(Entity2), typeof(INHibernateProxy));
29+
_cache.StoreProxyType(typeof(Entity3FakeProxy), typeof(Entity3));
30+
_cache.StoreProxyType(typeof(Entity4FakeProxy), typeof(Entity4), typeof(IProxy));
31+
_cache.StoreProxyType(typeof(Entity5FakeProxy), typeof(Entity5), typeof(INHibernateProxy), typeof(IProxy));
32+
33+
// Artificially inject other entries with same hashcodes
34+
_hashCode1 = new ProxyCacheEntry(typeof(Entity1), null).GetHashCode();
35+
Inject(new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy) }), _hashCode1, typeof(Entity1FakeProxy2));
36+
Inject(new ProxyCacheEntry(typeof(Entity3), null), _hashCode1, typeof(Entity3FakeProxy2));
37+
38+
_hashCode2 = new ProxyCacheEntry(typeof(Entity2), new[] { typeof(INHibernateProxy) }).GetHashCode();
39+
Inject(new ProxyCacheEntry(typeof(Entity2), null), _hashCode2, typeof(Entity2FakeProxy2));
40+
Inject(new ProxyCacheEntry(typeof(Entity4), new[] { typeof(IProxy) }), _hashCode2, typeof(Entity4FakeProxy2));
41+
Inject(new ProxyCacheEntry(typeof(Entity5), new[] { typeof(INHibernateProxy), typeof(IProxy) }), _hashCode2, typeof(Entity5FakeProxy2));
42+
}
43+
44+
private void Inject(ProxyCacheEntry entryToTweak, int hashcode, System.Type result)
45+
{
46+
TweakEntry(entryToTweak, hashcode);
47+
_internalCache[entryToTweak] = result;
48+
}
49+
50+
private static readonly FieldInfo HashCodeField =
51+
typeof(ProxyCacheEntry).GetField("_hashCode", BindingFlags.Instance | BindingFlags.NonPublic);
52+
53+
/// <summary>
54+
/// Allows to simulate a hashcode collision. Issue would be unpractical to test otherwise.
55+
/// Hashcode collision must be supported for avoiding unexpected and hard to reproduce failures.
56+
/// </summary>
57+
private void TweakEntry(ProxyCacheEntry entryToTweak, int hashcode)
58+
{
59+
// Though hashCode is a readonly field, this works at the time of this writing. If it starts breaking and cannot be fixed,
60+
// ignore those tests or throw them away.
61+
HashCodeField.SetValue(entryToTweak, hashcode);
62+
}
63+
64+
[Test]
65+
public void ProxyCacheEntity1FakeProxy()
66+
{
67+
var result = _cache.GetProxyType(typeof(Entity1));
68+
Assert.AreEqual(typeof(Entity1FakeProxy), result);
69+
}
70+
71+
[Test]
72+
public void ProxyCacheEntity1FakeProxy2()
73+
{
74+
var entry = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy) });
75+
TweakEntry(entry, _hashCode1);
76+
var result = _internalCache[entry];
77+
Assert.AreEqual(typeof(Entity1FakeProxy2), result);
78+
}
79+
80+
[Test]
81+
public void ProxyCacheEntity2FakeProxy()
82+
{
83+
var result = _cache.GetProxyType(typeof(Entity2), typeof(INHibernateProxy));
84+
Assert.AreEqual(typeof(Entity2FakeProxy), result);
85+
}
86+
87+
[Test]
88+
public void ProxyCacheEntity2FakeProxy2()
89+
{
90+
var entry = new ProxyCacheEntry(typeof(Entity2), null);
91+
TweakEntry(entry, _hashCode2);
92+
var result = _internalCache[entry];
93+
Assert.AreEqual(typeof(Entity2FakeProxy2), result);
94+
}
95+
96+
[Test]
97+
public void ProxyCacheEntity3FakeProxy()
98+
{
99+
var result = _cache.GetProxyType(typeof(Entity3));
100+
Assert.AreEqual(typeof(Entity3FakeProxy), result);
101+
}
102+
103+
[Test]
104+
public void ProxyCacheEntity3FakeProxy2()
105+
{
106+
var entry = new ProxyCacheEntry(typeof(Entity3), null);
107+
TweakEntry(entry, _hashCode1);
108+
var result = _internalCache[entry];
109+
Assert.AreEqual(typeof(Entity3FakeProxy2), result);
110+
}
111+
112+
[Test]
113+
public void ProxyCacheEntity4FakeProxy()
114+
{
115+
var result = _cache.GetProxyType(typeof(Entity4), typeof(IProxy));
116+
Assert.AreEqual(typeof(Entity4FakeProxy), result);
117+
}
118+
119+
[Test]
120+
public void ProxyCacheEntity4FakeProxy2()
121+
{
122+
var entry = new ProxyCacheEntry(typeof(Entity4), new[] { typeof(IProxy) });
123+
TweakEntry(entry, _hashCode2);
124+
var result = _internalCache[entry];
125+
Assert.AreEqual(typeof(Entity4FakeProxy2), result);
126+
}
127+
128+
[Test]
129+
public void ProxyCacheEntity5FakeProxy()
130+
{
131+
var result = _cache.GetProxyType(typeof(Entity5), typeof(IProxy), typeof(INHibernateProxy));
132+
Assert.AreEqual(typeof(Entity5FakeProxy), result);
133+
}
134+
135+
[Test]
136+
public void ProxyCacheEntity5FakeProxy2()
137+
{
138+
// Interfaces order inverted on purpose: must be supported.
139+
var entry = new ProxyCacheEntry(typeof(Entity5), new[] { typeof(IProxy), typeof(INHibernateProxy) });
140+
TweakEntry(entry, _hashCode2);
141+
var result = _internalCache[entry];
142+
Assert.AreEqual(typeof(Entity5FakeProxy2), result);
143+
}
144+
145+
[Test]
146+
public void ProxyCacheNone()
147+
{
148+
// Beware not testing the lookup failure of any combination, even tweaked, actually added in cache.
149+
// (Otherwise the test may starts failing unexpectedly sometimes, as the original bug ...)
150+
// This one was not added in anyway.
151+
System.Type result;
152+
Assert.IsFalse(_cache.TryGetProxyType(typeof(Entity2), new[] { typeof(IProxy) }, out result));
153+
}
154+
}
155+
}

‎src/NHibernate.Test/NHibernate.Test.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,9 @@
742742
<Compile Include="NHSpecificTest\NH3961\DateParametersComparedTo.cs" />
743743
<Compile Include="NHSpecificTest\NH3963\Entity.cs" />
744744
<Compile Include="NHSpecificTest\NH3963\MappedAsFixture.cs" />
745+
<Compile Include="NHSpecificTest\NH3954\Entity.cs" />
746+
<Compile Include="NHSpecificTest\NH3954\EqualsFixture.cs" />
747+
<Compile Include="NHSpecificTest\NH3954\ProxyCacheFixture.cs" />
745748
<Compile Include="NHSpecificTest\NH3950\Entity.cs" />
746749
<Compile Include="NHSpecificTest\NH3950\Fixture.cs" />
747750
<Compile Include="NHSpecificTest\NH3952\Entity.cs" />

‎src/NHibernate/Proxy/DynamicProxy/ProxyCacheEntry.cs

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,51 +12,63 @@
1212
namespace NHibernate.Proxy.DynamicProxy
1313
{
1414
[Serializable]
15-
public class ProxyCacheEntry
15+
public class ProxyCacheEntry : IEquatable<ProxyCacheEntry>
1616
{
17-
private readonly int hashCode;
17+
private readonly int _hashCode;
1818

1919
public ProxyCacheEntry(System.Type baseType, System.Type[] interfaces)
2020
{
2121
if (baseType == null)
22-
{
23-
throw new ArgumentNullException("baseType");
24-
}
22+
throw new ArgumentNullException(nameof(baseType));
2523
BaseType = baseType;
2624
Interfaces = interfaces ?? new System.Type[0];
25+
_uniqueInterfaces = new HashSet<System.Type>(Interfaces);
2726

28-
if (Interfaces.Length == 0)
27+
if (Interfaces.Count == 0)
2928
{
30-
hashCode = baseType.GetHashCode();
29+
_hashCode = baseType.GetHashCode();
3130
return;
3231
}
33-
34-
// duplicated type exclusion
35-
var set = new HashSet<System.Type>(Interfaces) { baseType };
36-
hashCode = 59;
32+
33+
var set = new List<System.Type>(_uniqueInterfaces) { baseType };
34+
_hashCode = 59;
3735
foreach (System.Type type in set)
3836
{
39-
hashCode ^= type.GetHashCode();
37+
// This simple implementation is nonsensitive to list order. If changing it for a sensitive one,
38+
// take care of ordering the list.
39+
_hashCode ^= type.GetHashCode();
4040
}
4141
}
4242

43-
public System.Type BaseType { get; private set; }
44-
public System.Type[] Interfaces { get; private set; }
43+
public System.Type BaseType { get; }
44+
public IReadOnlyCollection<System.Type> Interfaces { get; }
45+
46+
[NonSerialized]
47+
private HashSet<System.Type> _uniqueInterfaces;
4548

4649
public override bool Equals(object obj)
4750
{
4851
var that = obj as ProxyCacheEntry;
49-
if (ReferenceEquals(null, that))
52+
return Equals(that);
53+
}
54+
55+
public bool Equals(ProxyCacheEntry other)
56+
{
57+
if (ReferenceEquals(null, other) ||
58+
// hashcode inequality allows an early exit, but their equality is not enough for guaranteeing instances equality.
59+
_hashCode != other._hashCode ||
60+
BaseType != other.BaseType)
5061
{
5162
return false;
5263
}
5364

54-
return hashCode == that.GetHashCode();
55-
}
65+
// In case the instance was de-serialized, the _uniqueInterfaces will be null. Re-fetch its _uniqueInterfaces if needed.
66+
if (_uniqueInterfaces == null)
67+
_uniqueInterfaces = new HashSet<System.Type>(Interfaces);
5668

57-
public override int GetHashCode()
58-
{
59-
return hashCode;
69+
return _uniqueInterfaces.SetEquals(other.Interfaces);
6070
}
71+
72+
public override int GetHashCode() => _hashCode;
6173
}
6274
}

0 commit comments

Comments
 (0)