Fix ExpressionKeyVisitor to produce unique keys for anon types (#1532)

Author: mkasperski

src/NHibernate.DomainModel/NHSpecific/AnonymousType1.cs

@@ -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;
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH1526/AnonymousType1.cs

@@ -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();
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH1526/Fixture.cs

@@ -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>());
+		}
+	}
+}

src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs

@@ -192,7 +192,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(')');
@@ -212,7 +212,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;
@@ -240,7 +240,7 @@ 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(']');
 			}
 		}