Skip to content

Commit 0d61f46

Browse files
fredericDelaportefredericDelaporte
authored
Fix TypedValue not always using adequate comparer with SetParameterList (#1612)
* And fix TypedValue hash-code colliding too easily
1 parent f9f0389 commit 0d61f46

File tree

16 files changed

+240
-131
lines changed

16 files changed

+240
-131
lines changed

‎src/NHibernate.Test/CacheTest/EntityWithFilters.hbm.xml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
<filter name="DescriptionLike" />
1111
<filter name="DescriptionEqualAndValueGT" />
12+
<filter name="ValueIn" />
1213
</class>
1314
<query name="EntityWithFilters.All" cache-region="aRegion" cacheable="true">
1415
from EntityWithFilters
@@ -19,5 +20,8 @@
1920
<filter-def name="DescriptionEqualAndValueGT" condition="Description = :pDesc and `Value` > :pValue">
2021
<filter-param name="pDesc" type="string"/>
2122
<filter-param name="pValue" type="int"/>
22-
</filter-def>
23-
</hibernate-mapping>
23+
</filter-def>
24+
<filter-def name="ValueIn" condition="`Value` in (:pIn)">
25+
<filter-param name="pIn" type="int"/>
26+
</filter-def>
27+
</hibernate-mapping>

‎src/NHibernate.Test/CacheTest/FilterKeyFixture.cs‎

Lines changed: 67 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Collections;
2+
using System.Collections.Generic;
23
using NHibernate.Cache;
34
using NHibernate.Impl;
45
using NUnit.Framework;
@@ -8,105 +9,125 @@ namespace NHibernate.Test.CacheTest
89
[TestFixture]
910
public class FilterKeyFixture: TestCase
1011
{
11-
protected override string MappingsAssembly
12-
{
13-
get{return "NHibernate.Test";}
14-
}
12+
protected override string MappingsAssembly => "NHibernate.Test";
1513

16-
protected override IList Mappings
17-
{
18-
get { return new[] { "CacheTest.EntityWithFilters.hbm.xml" }; }
19-
}
14+
protected override IList Mappings => new[] { "CacheTest.EntityWithFilters.hbm.xml" };
2015

2116
[Test]
2217
public void ToStringIncludeAll()
2318
{
2419
string filterName = "DescriptionLike";
2520
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
2621
f.SetParameter("pLike", "so%");
27-
var fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
28-
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionLike{'pLike'='so%'}]"));
22+
var fk = new FilterKey(f);
23+
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionLike{'pLike'='so%'}]"), "Like");
2924

3025
filterName = "DescriptionEqualAndValueGT";
3126
f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
3227
f.SetParameter("pDesc", "something").SetParameter("pValue", 10);
33-
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
34-
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionEqualAndValueGT{'pDesc'='something', 'pValue'='10'}]"));
28+
fk = new FilterKey(f);
29+
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionEqualAndValueGT{'pDesc'='something', 'pValue'='10'}]"), "Value");
3530
}
3631

3732
[Test]
3833
public void Equality()
3934
{
40-
// Equality is aware only by parameters names not values
41-
FilterKey fk, fk1;
42-
FilterDescLikeToCompare(out fk, out fk1);
43-
Assert.That(fk, Is.EqualTo(fk1));
35+
FilterDescLikeToCompare(out var fk, out var fk1, true);
36+
Assert.That(fk, Is.EqualTo(fk1), "Like");
4437

45-
FilterDescValueToCompare(out fk, out fk1);
46-
Assert.That(fk, Is.EqualTo(fk1));
38+
FilterDescValueToCompare(out fk, out fk1, true);
39+
Assert.That(fk, Is.EqualTo(fk1), "Value");
40+
41+
FilterValueInToCompare(out fk, out fk1, true);
42+
Assert.That(fk, Is.EqualTo(fk1), "In");
4743
}
4844

49-
private void FilterDescLikeToCompare(out FilterKey fk, out FilterKey fk1)
45+
private void FilterDescLikeToCompare(out FilterKey fk, out FilterKey fk1, bool sameValue)
5046
{
5147
const string filterName = "DescriptionLike";
5248
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
5349
f.SetParameter("pLike", "so%");
54-
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
50+
fk = new FilterKey(f);
5551

5652
var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
57-
f1.SetParameter("pLike", "%ing");
58-
fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
53+
f1.SetParameter("pLike", sameValue ? "so%" : "%ing");
54+
fk1 = new FilterKey(f1);
5955
}
6056

61-
private void FilterDescValueToCompare(out FilterKey fk, out FilterKey fk1)
57+
private void FilterDescValueToCompare(out FilterKey fk, out FilterKey fk1, bool sameValue)
6258
{
6359
const string filterName = "DescriptionEqualAndValueGT";
6460
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
6561
f.SetParameter("pDesc", "something").SetParameter("pValue", 10);
66-
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
62+
fk = new FilterKey(f);
63+
64+
var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
65+
f1.SetParameter("pDesc", "something").SetParameter("pValue", sameValue ? 10 : 11);
66+
fk1 = new FilterKey(f1);
67+
}
68+
69+
private void FilterValueInToCompare(out FilterKey fk, out FilterKey fk1, bool sameValue)
70+
{
71+
const string filterName = "ValueIn";
72+
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
73+
f.SetParameterList("pIn", new HashSet<int> { 10, 11 });
74+
fk = new FilterKey(f);
6775

6876
var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
69-
f1.SetParameter("pDesc", "something").SetParameter("pValue", 11);
70-
fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
77+
f1.SetParameterList("pIn", sameValue ? (ICollection<int>)new [] { 10, 11 } : new HashSet<int> { 10, 12 });
78+
fk1 = new FilterKey(f1);
7179
}
7280

7381
[Test]
7482
public void NotEquality()
7583
{
76-
FilterKey fk, fk1;
77-
FilterDescLikeToCompare(out fk, out fk1);
84+
FilterDescLikeToCompare(out var fk, out var fk1, false);
85+
Assert.That(fk, Is.Not.EqualTo(fk1), "fk & fk1");
7886

79-
FilterKey fvk, fvk1;
80-
FilterDescValueToCompare(out fvk, out fvk1);
87+
FilterDescValueToCompare(out var fvk, out var fvk1, false);
88+
Assert.That(fvk, Is.Not.EqualTo(fvk1), "fvk & fvk1");
8189

82-
Assert.That(fk, Is.Not.EqualTo(fvk));
83-
Assert.That(fk1, Is.Not.EqualTo(fvk1));
90+
FilterValueInToCompare(out var fik, out var fik1, false);
91+
Assert.That(fik, Is.Not.EqualTo(fik1), "fik & fik1");
92+
93+
Assert.That(fk, Is.Not.EqualTo(fvk), "fk & fvk");
94+
Assert.That(fk1, Is.Not.EqualTo(fvk1), "fk1 & fvk1");
95+
Assert.That(fvk, Is.Not.EqualTo(fik), "fvk & fik");
96+
Assert.That(fvk1, Is.Not.EqualTo(fik1), "fvk1 & fik1");
8497
}
8598

8699
[Test]
87100
public void HashCode()
88101
{
89-
// HashCode is aware only by parameters names not values (should work as Equal)
90-
FilterKey fk, fk1;
91-
FilterDescLikeToCompare(out fk, out fk1);
92-
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()));
102+
FilterDescLikeToCompare(out var fk, out var fk1, true);
103+
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()), "Like");
93104

94-
FilterDescValueToCompare(out fk, out fk1);
95-
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()));
105+
FilterDescValueToCompare(out fk, out fk1, true);
106+
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()), "Value");
96107

108+
FilterValueInToCompare(out fk, out fk1, true);
109+
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()), "In");
97110
}
98111

99112
[Test]
100113
public void NotEqualHashCode()
101114
{
102-
FilterKey fk, fk1;
103-
FilterDescLikeToCompare(out fk, out fk1);
104-
105-
FilterKey fvk, fvk1;
106-
FilterDescValueToCompare(out fvk, out fvk1);
107-
108-
Assert.That(fk.GetHashCode(), Is.Not.EqualTo(fvk.GetHashCode()));
109-
Assert.That(fk1.GetHashCode(), Is.Not.EqualTo(fvk1.GetHashCode()));
115+
// GetHashCode semantic does not guarantee no collision may ever occur, but the algorithm should
116+
// generates different hashcodes for similar but inequal cases. These tests check that cache keys
117+
// for a query generated for different parameters values are no more equal.
118+
FilterDescLikeToCompare(out var fk, out var fk1, false);
119+
Assert.That(fk.GetHashCode(), Is.Not.EqualTo(fk1.GetHashCode()), "fk & fk1");
120+
121+
FilterDescValueToCompare(out var fvk, out var fvk1, false);
122+
Assert.That(fvk.GetHashCode(), Is.Not.EqualTo(fvk1.GetHashCode()), "fvk & fvk1");
123+
124+
FilterValueInToCompare(out var fik, out var fik1, false);
125+
Assert.That(fik.GetHashCode(), Is.Not.EqualTo(fik1.GetHashCode()), "fik & fik1");
126+
127+
Assert.That(fk.GetHashCode(), Is.Not.EqualTo(fvk.GetHashCode()), "fk & fvk");
128+
Assert.That(fk1.GetHashCode(), Is.Not.EqualTo(fvk1.GetHashCode()), "fk1 & fvk1");
129+
Assert.That(fvk.GetHashCode(), Is.Not.EqualTo(fik.GetHashCode()), "fvk & fik");
130+
Assert.That(fvk1.GetHashCode(), Is.Not.EqualTo(fik1.GetHashCode()), "fvk1 & fik1");
110131
}
111132
}
112133
}

‎src/NHibernate.Test/CacheTest/QueryKeyFixture.cs‎

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,95 +14,90 @@ public class QueryKeyFixture : TestCase
1414
private readonly SqlString SqlAll =
1515
new SqlString("select entitywith0_.id as id0_, entitywith0_.Description as Descript2_0_, entitywith0_.Value as Value0_ from EntityWithFilters entitywith0_");
1616

17-
protected override string MappingsAssembly
18-
{
19-
get { return "NHibernate.Test"; }
20-
}
17+
protected override string MappingsAssembly => "NHibernate.Test";
2118

22-
protected override IList Mappings
23-
{
24-
get { return new[] { "CacheTest.EntityWithFilters.hbm.xml" }; }
25-
}
19+
protected override IList Mappings => new[] { "CacheTest.EntityWithFilters.hbm.xml" };
2620

2721
[Test]
2822
public void EqualityWithFilters()
2923
{
30-
QueryKey qk, qk1;
31-
QueryKeyFilterDescLikeToCompare(out qk, out qk1);
32-
Assert.That(qk, Is.EqualTo(qk1));
24+
QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, true);
25+
Assert.That(qk, Is.EqualTo(qk1), "Like");
3326

34-
QueryKeyFilterDescValueToCompare(out qk, out qk1);
35-
Assert.That(qk, Is.EqualTo(qk1));
27+
QueryKeyFilterDescValueToCompare(out qk, out qk1, true);
28+
Assert.That(qk, Is.EqualTo(qk1), "Value");
3629
}
3730

38-
private void QueryKeyFilterDescLikeToCompare(out QueryKey qk, out QueryKey qk1)
31+
private void QueryKeyFilterDescLikeToCompare(out QueryKey qk, out QueryKey qk1, bool sameValue)
3932
{
4033
const string filterName = "DescriptionLike";
4134
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
4235
f.SetParameter("pLike", "so%");
43-
var fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
36+
var fk = new FilterKey(f);
4437
ISet<FilterKey> fks = new HashSet<FilterKey> { fk };
4538
qk = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
4639

4740
var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
48-
f1.SetParameter("pLike", "%ing");
49-
var fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
41+
f1.SetParameter("pLike", sameValue ? "so%" : "%ing");
42+
var fk1 = new FilterKey(f1);
5043
fks = new HashSet<FilterKey> { fk1 };
5144
qk1 = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
5245
}
5346

54-
private void QueryKeyFilterDescValueToCompare(out QueryKey qk, out QueryKey qk1)
47+
private void QueryKeyFilterDescValueToCompare(out QueryKey qk, out QueryKey qk1, bool sameValue)
5548
{
5649
const string filterName = "DescriptionEqualAndValueGT";
5750

5851
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
5952
f.SetParameter("pDesc", "something").SetParameter("pValue", 10);
60-
var fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
53+
var fk = new FilterKey(f);
6154
ISet<FilterKey> fks = new HashSet<FilterKey> { fk };
6255
qk = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
6356

6457
var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
65-
f1.SetParameter("pDesc", "something").SetParameter("pValue", 11);
66-
var fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
58+
f1.SetParameter("pDesc", "something").SetParameter("pValue", sameValue ? 10 : 11);
59+
var fk1 = new FilterKey(f1);
6760
fks = new HashSet<FilterKey> { fk1 };
6861
qk1 = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
6962
}
7063

7164
[Test]
7265
public void NotEqualityWithFilters()
7366
{
74-
QueryKey qk, qk1;
75-
QueryKeyFilterDescLikeToCompare(out qk, out qk1);
67+
QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, false);
68+
Assert.That(qk, Is.Not.EqualTo(qk1), "qk & qk1");
7669

77-
QueryKey qvk, qvk1;
78-
QueryKeyFilterDescValueToCompare(out qvk, out qvk1);
70+
QueryKeyFilterDescValueToCompare(out var qvk, out var qvk1, false);
71+
Assert.That(qvk, Is.Not.EqualTo(qvk1), "qvk & qvk1");
7972

80-
Assert.That(qk, Is.Not.EqualTo(qvk));
81-
Assert.That(qk1, Is.Not.EqualTo(qvk1));
73+
Assert.That(qk, Is.Not.EqualTo(qvk), "qk & qvk");
74+
Assert.That(qk1, Is.Not.EqualTo(qvk1), "qk1 & qvk1");
8275
}
8376

8477
[Test]
8578
public void HashCodeWithFilters()
8679
{
87-
QueryKey qk, qk1;
88-
QueryKeyFilterDescLikeToCompare(out qk, out qk1);
89-
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()));
80+
QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, true);
81+
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()), "Like");
9082

91-
QueryKeyFilterDescValueToCompare(out qk, out qk1);
92-
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()));
83+
QueryKeyFilterDescValueToCompare(out qk, out qk1, true);
84+
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()), "Value");
9385
}
9486

9587
[Test]
9688
public void NotEqualHashCodeWithFilters()
9789
{
98-
QueryKey qk, qk1;
99-
QueryKeyFilterDescLikeToCompare(out qk, out qk1);
90+
// GetHashCode semantic does not guarantee no collision may ever occur, but the algorithm should
91+
// generates different hashcodes for similar but inequal cases. These tests check that cache keys
92+
// for a query generated for different parameters values are no more equal.
93+
QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, false);
94+
Assert.That(qk.GetHashCode(), Is.Not.EqualTo(qk1.GetHashCode()), "qk & qk1");
10095

101-
QueryKey qvk, qvk1;
102-
QueryKeyFilterDescValueToCompare(out qvk, out qvk1);
96+
QueryKeyFilterDescValueToCompare(out var qvk, out var qvk1, false);
97+
Assert.That(qvk.GetHashCode(), Is.Not.EqualTo(qvk1.GetHashCode()), "qvk & qvk1");
10398

104-
Assert.That(qk.GetHashCode(), Is.Not.EqualTo(qvk.GetHashCode()));
105-
Assert.That(qk1.GetHashCode(), Is.Not.EqualTo(qvk1.GetHashCode()));
99+
Assert.That(qk.GetHashCode(), Is.Not.EqualTo(qvk.GetHashCode()), "qk & qvk");
100+
Assert.That(qk1.GetHashCode(), Is.Not.EqualTo(qvk1.GetHashCode()), "qk1 & qvk1");
106101
}
107102

108103
[Test]
@@ -111,18 +106,18 @@ public void ToStringWithFilters()
111106
string filterName = "DescriptionLike";
112107
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
113108
f.SetParameter("pLike", "so%");
114-
var fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
109+
var fk = new FilterKey(f);
115110
ISet<FilterKey> fks = new HashSet<FilterKey> { fk };
116111
var qk = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
117-
Assert.That(qk.ToString(), Does.Contain(string.Format("filters: ['{0}']",fk)));
112+
Assert.That(qk.ToString(), Does.Contain($"filters: ['{fk}']"), "Like");
118113

119114
filterName = "DescriptionEqualAndValueGT";
120115
f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
121116
f.SetParameter("pDesc", "something").SetParameter("pValue", 10);
122-
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
117+
fk = new FilterKey(f);
123118
fks = new HashSet<FilterKey> { fk };
124119
qk = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
125-
Assert.That(qk.ToString(), Does.Contain(string.Format("filters: ['{0}']", fk)));
120+
Assert.That(qk.ToString(), Does.Contain($"filters: ['{fk}']"), "Value");
126121
}
127122

128123
[Test]
@@ -131,16 +126,16 @@ public void ToStringWithMoreFilters()
131126
string filterName = "DescriptionLike";
132127
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
133128
f.SetParameter("pLike", "so%");
134-
var fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
129+
var fk = new FilterKey(f);
135130

136131
filterName = "DescriptionEqualAndValueGT";
137132
var fv = new FilterImpl(Sfi.GetFilterDefinition(filterName));
138133
fv.SetParameter("pDesc", "something").SetParameter("pValue", 10);
139-
var fvk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
134+
var fvk = new FilterKey(fv);
140135

141136
ISet<FilterKey> fks = new HashSet<FilterKey> { fk, fvk };
142137
var qk = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
143-
Assert.That(qk.ToString(), Does.Contain(string.Format("filters: ['{0}', '{1}']", fk, fvk)));
138+
Assert.That(qk.ToString(), Does.Contain($"filters: ['{fk}', '{fvk}']"));
144139
}
145140
}
146141
}

0 commit comments

Comments
 (0)