Skip to content

Fix TypedValue not always using adequate comparer with SetParameterList #1612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 28, 2018
Merged

Fix TypedValue not always using adequate comparer with SetParameterList #1612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
45aa0fa
Fix TypedValue not using adequate comparer with SetParameterList
fredericDelaporte Mar 13, 2018
cacde04
Specify to TypeValue whether the value is a list or not
fredericDelaporte Mar 13, 2018
e91f9e6
Fix TypedValue hash-code colliding too easily
fredericDelaporte May 11, 2018
bfafb14
Enable query cache key hashcode tests
fredericDelaporte May 15, 2018
02cd325
Add shortcuts in TypedValue comparators
fredericDelaporte May 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/NHibernate.Test/CacheTest/EntityWithFilters.hbm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

<filter name="DescriptionLike" />
<filter name="DescriptionEqualAndValueGT" />
<filter name="ValueIn" />
</class>
<query name="EntityWithFilters.All" cache-region="aRegion" cacheable="true">
from EntityWithFilters
Expand All @@ -19,5 +20,8 @@
<filter-def name="DescriptionEqualAndValueGT" condition="Description = :pDesc and `Value` > :pValue">
<filter-param name="pDesc" type="string"/>
<filter-param name="pValue" type="int"/>
</filter-def>
</hibernate-mapping>
</filter-def>
<filter-def name="ValueIn" condition="`Value` in (:pIn)">
<filter-param name="pIn" type="int"/>
</filter-def>
</hibernate-mapping>
113 changes: 67 additions & 46 deletions src/NHibernate.Test/CacheTest/FilterKeyFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections;
using System.Collections.Generic;
using NHibernate.Cache;
using NHibernate.Impl;
using NUnit.Framework;
Expand All @@ -8,105 +9,125 @@ namespace NHibernate.Test.CacheTest
[TestFixture]
public class FilterKeyFixture: TestCase
{
protected override string MappingsAssembly
{
get{return "NHibernate.Test";}
}
protected override string MappingsAssembly => "NHibernate.Test";

protected override IList Mappings
{
get { return new[] { "CacheTest.EntityWithFilters.hbm.xml" }; }
}
protected override IList Mappings => new[] { "CacheTest.EntityWithFilters.hbm.xml" };

[Test]
public void ToStringIncludeAll()
{
string filterName = "DescriptionLike";
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f.SetParameter("pLike", "so%");
var fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionLike{'pLike'='so%'}]"));
var fk = new FilterKey(f);
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionLike{'pLike'='so%'}]"), "Like");

filterName = "DescriptionEqualAndValueGT";
f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f.SetParameter("pDesc", "something").SetParameter("pValue", 10);
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionEqualAndValueGT{'pDesc'='something', 'pValue'='10'}]"));
fk = new FilterKey(f);
Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionEqualAndValueGT{'pDesc'='something', 'pValue'='10'}]"), "Value");
}

[Test]
public void Equality()
{
// Equality is aware only by parameters names not values
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong, but not contradicted by the test because the test was bugged.

FilterKey fk, fk1;
FilterDescLikeToCompare(out fk, out fk1);
Assert.That(fk, Is.EqualTo(fk1));
FilterDescLikeToCompare(out var fk, out var fk1, true);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can be equal only with same value. The NotEquality test is completed with an assert showcasing that a different value causes inequality.

Assert.That(fk, Is.EqualTo(fk1), "Like");

FilterDescValueToCompare(out fk, out fk1);
Assert.That(fk, Is.EqualTo(fk1));
FilterDescValueToCompare(out fk, out fk1, true);
Assert.That(fk, Is.EqualTo(fk1), "Value");

FilterValueInToCompare(out fk, out fk1, true);
Assert.That(fk, Is.EqualTo(fk1), "In");
}

private void FilterDescLikeToCompare(out FilterKey fk, out FilterKey fk1)
private void FilterDescLikeToCompare(out FilterKey fk, out FilterKey fk1, bool sameValue)
{
const string filterName = "DescriptionLike";
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f.SetParameter("pLike", "so%");
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
fk = new FilterKey(f);

var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f1.SetParameter("pLike", "%ing");
fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here lies the test bug, affecting all tests: the second key was construct with the first filter values...

f1.SetParameter("pLike", sameValue ? "so%" : "%ing");
fk1 = new FilterKey(f1);
}

private void FilterDescValueToCompare(out FilterKey fk, out FilterKey fk1)
private void FilterDescValueToCompare(out FilterKey fk, out FilterKey fk1, bool sameValue)
{
const string filterName = "DescriptionEqualAndValueGT";
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f.SetParameter("pDesc", "something").SetParameter("pValue", 10);
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
fk = new FilterKey(f);

var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f1.SetParameter("pDesc", "something").SetParameter("pValue", sameValue ? 10 : 11);
fk1 = new FilterKey(f1);
}

private void FilterValueInToCompare(out FilterKey fk, out FilterKey fk1, bool sameValue)
{
const string filterName = "ValueIn";
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f.SetParameterList("pIn", new HashSet<int> { 10, 11 });
fk = new FilterKey(f);

var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f1.SetParameter("pDesc", "something").SetParameter("pValue", 11);
fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same test bug than in FilterDescLikeToCompare, fixed too.

f1.SetParameterList("pIn", sameValue ? (ICollection<int>)new [] { 10, 11 } : new HashSet<int> { 10, 12 });
fk1 = new FilterKey(f1);
}

[Test]
public void NotEquality()
{
FilterKey fk, fk1;
FilterDescLikeToCompare(out fk, out fk1);
FilterDescLikeToCompare(out var fk, out var fk1, false);
Assert.That(fk, Is.Not.EqualTo(fk1), "fk & fk1");

FilterKey fvk, fvk1;
FilterDescValueToCompare(out fvk, out fvk1);
FilterDescValueToCompare(out var fvk, out var fvk1, false);
Assert.That(fvk, Is.Not.EqualTo(fvk1), "fvk & fvk1");

Assert.That(fk, Is.Not.EqualTo(fvk));
Assert.That(fk1, Is.Not.EqualTo(fvk1));
FilterValueInToCompare(out var fik, out var fik1, false);
Assert.That(fik, Is.Not.EqualTo(fik1), "fik & fik1");

Assert.That(fk, Is.Not.EqualTo(fvk), "fk & fvk");
Assert.That(fk1, Is.Not.EqualTo(fvk1), "fk1 & fvk1");
Assert.That(fvk, Is.Not.EqualTo(fik), "fvk & fik");
Assert.That(fvk1, Is.Not.EqualTo(fik1), "fvk1 & fik1");
}

[Test]
public void HashCode()
{
// HashCode is aware only by parameters names not values (should work as Equal)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately true for the current hashcode implementation, which is bad for performances. But in fact it should ideally not be equal, since actually filter with different parameters values are not equals.

FilterKey fk, fk1;
FilterDescLikeToCompare(out fk, out fk1);
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()));
FilterDescLikeToCompare(out var fk, out var fk1, true);
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()), "Like");

FilterDescValueToCompare(out fk, out fk1);
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()));
FilterDescValueToCompare(out fk, out fk1, true);
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()), "Value");

FilterValueInToCompare(out fk, out fk1, true);
Assert.That(fk.GetHashCode(), Is.EqualTo(fk1.GetHashCode()), "In");
}

[Test]
public void NotEqualHashCode()
{
FilterKey fk, fk1;
FilterDescLikeToCompare(out fk, out fk1);

FilterKey fvk, fvk1;
FilterDescValueToCompare(out fvk, out fvk1);

Assert.That(fk.GetHashCode(), Is.Not.EqualTo(fvk.GetHashCode()));
Assert.That(fk1.GetHashCode(), Is.Not.EqualTo(fvk1.GetHashCode()));
// GetHashCode semantic does not guarantee no collision may ever occur, but the algorithm should
// generates different hashcodes for similar but inequal cases. These tests check that cache keys
// for a query generated for different parameters values are no more equal.
FilterDescLikeToCompare(out var fk, out var fk1, false);
Assert.That(fk.GetHashCode(), Is.Not.EqualTo(fk1.GetHashCode()), "fk & fk1");

FilterDescValueToCompare(out var fvk, out var fvk1, false);
Assert.That(fvk.GetHashCode(), Is.Not.EqualTo(fvk1.GetHashCode()), "fvk & fvk1");

FilterValueInToCompare(out var fik, out var fik1, false);
Assert.That(fik.GetHashCode(), Is.Not.EqualTo(fik1.GetHashCode()), "fik & fik1");

Assert.That(fk.GetHashCode(), Is.Not.EqualTo(fvk.GetHashCode()), "fk & fvk");
Assert.That(fk1.GetHashCode(), Is.Not.EqualTo(fvk1.GetHashCode()), "fk1 & fvk1");
Assert.That(fvk.GetHashCode(), Is.Not.EqualTo(fik.GetHashCode()), "fvk & fik");
Assert.That(fvk1.GetHashCode(), Is.Not.EqualTo(fik1.GetHashCode()), "fvk1 & fik1");
}
}
}
85 changes: 40 additions & 45 deletions src/NHibernate.Test/CacheTest/QueryKeyFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,95 +14,90 @@ public class QueryKeyFixture : TestCase
private readonly SqlString SqlAll =
new SqlString("select entitywith0_.id as id0_, entitywith0_.Description as Descript2_0_, entitywith0_.Value as Value0_ from EntityWithFilters entitywith0_");

protected override string MappingsAssembly
{
get { return "NHibernate.Test"; }
}
protected override string MappingsAssembly => "NHibernate.Test";

protected override IList Mappings
{
get { return new[] { "CacheTest.EntityWithFilters.hbm.xml" }; }
}
protected override IList Mappings => new[] { "CacheTest.EntityWithFilters.hbm.xml" };

[Test]
public void EqualityWithFilters()
{
QueryKey qk, qk1;
QueryKeyFilterDescLikeToCompare(out qk, out qk1);
Assert.That(qk, Is.EqualTo(qk1));
QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, true);
Assert.That(qk, Is.EqualTo(qk1), "Like");

QueryKeyFilterDescValueToCompare(out qk, out qk1);
Assert.That(qk, Is.EqualTo(qk1));
QueryKeyFilterDescValueToCompare(out qk, out qk1, true);
Assert.That(qk, Is.EqualTo(qk1), "Value");
}

private void QueryKeyFilterDescLikeToCompare(out QueryKey qk, out QueryKey qk1)
private void QueryKeyFilterDescLikeToCompare(out QueryKey qk, out QueryKey qk1, bool sameValue)
{
const string filterName = "DescriptionLike";
var f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f.SetParameter("pLike", "so%");
var fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
var fk = new FilterKey(f);
ISet<FilterKey> fks = new HashSet<FilterKey> { fk };
qk = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);

var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f1.SetParameter("pLike", "%ing");
var fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
f1.SetParameter("pLike", sameValue ? "so%" : "%ing");
var fk1 = new FilterKey(f1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same test bug as in FilterKeyFixture.

fks = new HashSet<FilterKey> { fk1 };
qk1 = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
}

private void QueryKeyFilterDescValueToCompare(out QueryKey qk, out QueryKey qk1)
private void QueryKeyFilterDescValueToCompare(out QueryKey qk, out QueryKey qk1, bool sameValue)
{
const string filterName = "DescriptionEqualAndValueGT";

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

var f1 = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f1.SetParameter("pDesc", "something").SetParameter("pValue", 11);
var fk1 = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
f1.SetParameter("pDesc", "something").SetParameter("pValue", sameValue ? 10 : 11);
var fk1 = new FilterKey(f1);
fks = new HashSet<FilterKey> { fk1 };
qk1 = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
}

[Test]
public void NotEqualityWithFilters()
{
QueryKey qk, qk1;
QueryKeyFilterDescLikeToCompare(out qk, out qk1);
QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, false);
Assert.That(qk, Is.Not.EqualTo(qk1), "qk & qk1");

QueryKey qvk, qvk1;
QueryKeyFilterDescValueToCompare(out qvk, out qvk1);
QueryKeyFilterDescValueToCompare(out var qvk, out var qvk1, false);
Assert.That(qvk, Is.Not.EqualTo(qvk1), "qvk & qvk1");

Assert.That(qk, Is.Not.EqualTo(qvk));
Assert.That(qk1, Is.Not.EqualTo(qvk1));
Assert.That(qk, Is.Not.EqualTo(qvk), "qk & qvk");
Assert.That(qk1, Is.Not.EqualTo(qvk1), "qk1 & qvk1");
}

[Test]
public void HashCodeWithFilters()
{
QueryKey qk, qk1;
QueryKeyFilterDescLikeToCompare(out qk, out qk1);
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()));
QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, true);
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()), "Like");

QueryKeyFilterDescValueToCompare(out qk, out qk1);
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()));
QueryKeyFilterDescValueToCompare(out qk, out qk1, true);
Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()), "Value");
}

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

QueryKey qvk, qvk1;
QueryKeyFilterDescValueToCompare(out qvk, out qvk1);
QueryKeyFilterDescValueToCompare(out var qvk, out var qvk1, false);
Assert.That(qvk.GetHashCode(), Is.Not.EqualTo(qvk1.GetHashCode()), "qvk & qvk1");

Assert.That(qk.GetHashCode(), Is.Not.EqualTo(qvk.GetHashCode()));
Assert.That(qk1.GetHashCode(), Is.Not.EqualTo(qvk1.GetHashCode()));
Assert.That(qk.GetHashCode(), Is.Not.EqualTo(qvk.GetHashCode()), "qk & qvk");
Assert.That(qk1.GetHashCode(), Is.Not.EqualTo(qvk1.GetHashCode()), "qk1 & qvk1");
}

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

filterName = "DescriptionEqualAndValueGT";
f = new FilterImpl(Sfi.GetFilterDefinition(filterName));
f.SetParameter("pDesc", "something").SetParameter("pValue", 10);
fk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
fk = new FilterKey(f);
fks = new HashSet<FilterKey> { fk };
qk = new QueryKey(Sfi, SqlAll, new QueryParameters(), fks, null);
Assert.That(qk.ToString(), Does.Contain(string.Format("filters: ['{0}']", fk)));
Assert.That(qk.ToString(), Does.Contain($"filters: ['{fk}']"), "Value");
}

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

filterName = "DescriptionEqualAndValueGT";
var fv = new FilterImpl(Sfi.GetFilterDefinition(filterName));
fv.SetParameter("pDesc", "something").SetParameter("pValue", 10);
var fvk = new FilterKey(filterName, f.Parameters, f.FilterDefinition.ParameterTypes);
var fvk = new FilterKey(fv);

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