Skip to content

Fix ExpressionKeyVisitor to produce unique keys for anon types #1532

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.

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
Jan 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
72 changes: 72 additions & 0 deletions src/NHibernate.DomainModel/NHSpecific/AnonymousType1.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
using System;
using System.Linq;
using System.Linq.Expressions;

// Simulates compiler-generated, non-namespaced anonymous type with one property
internal class AnonymousType1<TProp1>
{
public TProp1 Prop1 { get; set; }
}

namespace NHibernate.DomainModel.NHSpecific
{
public class AnonymousTypeQueryExpressionProviderFromNHibernateDomainModelAssembly
{
private readonly TypedQueryExpressionProvider<AnonymousType1<string>> _provider
= new TypedQueryExpressionProvider<AnonymousType1<string>>();

public System.Type GetAnonymousType()
{
return _provider.GetSuppliedType();
}

public Expression GetExpressionOfMethodCall()
{
return _provider.GetExpressionOfMethodCall();
}

public Expression GetExpressionOfNew()
{
return _provider.GetExpressionOfNew();
}

public Expression GetExpressionOfTypeBinary()
{
return _provider.GetExpressionOfTypeBinary();
}
}

public class TypedQueryExpressionProvider<T> where T : new ()
{
public System.Type GetSuppliedType()
{
return typeof(T);
}

public Expression GetExpressionOfMethodCall()
{
Expression<Func<object>> exp = () =>
Enumerable.Empty<object>().Select(o => (T)o).ToList();

return exp;
}

public Expression GetExpressionOfNew()
{
// adds .GetHashCode to make sure the .ToList is always of same generic type
// so that the only variable part is the 'new T()'
Expression<Func<object>> exp = () =>
Enumerable.Empty<object>().Select(o => new T().GetHashCode()).ToList();

return exp;
}

public Expression GetExpressionOfTypeBinary()
{
Expression<Func<object>> exp = () =>
Enumerable.Empty<object>().Select(o => o is T).ToList();

return exp;
}
}
}
39 changes: 39 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH1526/AnonymousType1.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using System.Linq.Expressions;
using NHibernate.DomainModel.NHSpecific;

// Simulates a compiler-generated, non-namespaced anonymous type with one property
// Exactly the same as the one in NHibernate.DomainModel.NHSpecific
internal class AnonymousType1<TProp1>
{
public TProp1 Prop1 { get; set; }
}

namespace NHibernate.Test.NHSpecificTest.GH1526
{
// Produces an Expression that has the above AnonymousType1 embedded in it
public class AnonymousTypeQueryExpressionProviderFromNHibernateTestAssembly
{
private readonly TypedQueryExpressionProvider<AnonymousType1<string>> _provider
= new TypedQueryExpressionProvider<AnonymousType1<string>>();

public System.Type GetAnonymousType()
{
return _provider.GetSuppliedType();
}

public Expression GetExpressionOfMethodCall()
{
return _provider.GetExpressionOfMethodCall();
}

public Expression GetExpressionOfNew()
{
return _provider.GetExpressionOfNew();
}

public Expression GetExpressionOfTypeBinary()
{
return _provider.GetExpressionOfTypeBinary();
}
}
}
77 changes: 77 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH1526/Fixture.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
using System.Collections.Generic;
using System.Linq.Expressions;
using NHibernate.DomainModel.NHSpecific;
using NHibernate.Linq.Visitors;
using NHibernate.Param;
using NUnit.Framework;

namespace NHibernate.Test.NHSpecificTest.GH1526
{
[TestFixture]
public class Fixture
{
private readonly AnonymousTypeQueryExpressionProviderFromNHibernateTestAssembly _providerFromNHTest
= new AnonymousTypeQueryExpressionProviderFromNHibernateTestAssembly();

private readonly AnonymousTypeQueryExpressionProviderFromNHibernateDomainModelAssembly _providerFromNHDoMo
= new AnonymousTypeQueryExpressionProviderFromNHibernateDomainModelAssembly();

[OneTimeSetUp]
public void OneTimeSetUp()
{
// all the tests in this fixture run under condition, that
// the types used from the two providers are different,
// but have exactly the same System.Type.FullName when inspected

var type1 = _providerFromNHTest.GetAnonymousType();
var type2 = _providerFromNHDoMo.GetAnonymousType();

Assert.That(type1.FullName, Is.EqualTo(type2.FullName),
"The two tested types must have the same FullName for demonstrating the bug.");

Assert.That(type1, Is.Not.EqualTo(type2),
"The two tested types must not be the same for demonstrating the bug.");
}

[Test]
public void ShouldCreateDifferentKeys_MethodCallExpression()
{
var exp1 = _providerFromNHTest.GetExpressionOfMethodCall();
var exp2 = _providerFromNHDoMo.GetExpressionOfMethodCall();

var key1 = GetCacheKey(exp1);
var key2 = GetCacheKey(exp2);

Assert.That(key1, Is.Not.EqualTo(key2));
}

[Test]
public void ShouldCreateDifferentKeys_NewExpression()
{
var exp1 = _providerFromNHTest.GetExpressionOfNew();
var exp2 = _providerFromNHDoMo.GetExpressionOfNew();

var key1 = GetCacheKey(exp1);
var key2 = GetCacheKey(exp2);

Assert.That(key1, Is.Not.EqualTo(key2));
}

[Test]
public void ShouldCreateDifferentKeys_TypeBinaryExpression()
{
var exp1 = _providerFromNHTest.GetExpressionOfTypeBinary();
var exp2 = _providerFromNHDoMo.GetExpressionOfTypeBinary();

var key1 = GetCacheKey(exp1);
var key2 = GetCacheKey(exp2);

Assert.That(key1, Is.Not.EqualTo(key2));
}

private static string GetCacheKey(Expression exp)
{
return ExpressionKeyVisitor.Visit(exp, new Dictionary<ConstantExpression, NamedParameter>());
}
}
}
8 changes: 4 additions & 4 deletions src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ protected override Expression VisitMethodCall(MethodCallExpression expression)
protected override Expression VisitNew(NewExpression expression)
{
_string.Append("new ");
_string.Append(expression.Constructor.DeclaringType.Name);
_string.Append(expression.Constructor.DeclaringType.AssemblyQualifiedName);
_string.Append('(');
Visit(expression.Arguments, AppendCommas);
_string.Append(')');
Expand All @@ -210,7 +210,7 @@ protected override Expression VisitTypeBinary(TypeBinaryExpression expression)
_string.Append("IsType(");
Visit(expression.Expression);
_string.Append(", ");
_string.Append(expression.TypeOperand.FullName);
_string.Append(expression.TypeOperand.AssemblyQualifiedName);
_string.Append(")");

return expression;
Expand Down Expand Up @@ -238,9 +238,9 @@ private void VisitMethod(MethodInfo methodInfo)
if (methodInfo.IsGenericMethod)
{
_string.Append('[');
_string.Append(string.Join(",", methodInfo.GetGenericArguments().Select(a => a.FullName).ToArray()));
_string.Append(string.Join(",", methodInfo.GetGenericArguments().Select(a => a.AssemblyQualifiedName).ToArray()));
_string.Append(']');
}
}
}
}
}