-
Notifications
You must be signed in to change notification settings - Fork 936
NH-3952: removing usages of NHibernate.Linq.EnumerableHelper #557
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
Changes from 7 commits
0d634f5
b79fff9
c385ebe
2b496e8
2964ad7
a8f6ff3
7ae87b2
b746a13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
|
||
namespace NHibernate.Test.NHSpecificTest.NH3952 | ||
{ | ||
class Entity | ||
{ | ||
public virtual Guid Id { get; set; } | ||
public virtual string Name { get; set; } | ||
public virtual Guid? ParentId { get; set; } | ||
public virtual ISet<Entity> Children { get; set; } | ||
public virtual string[] Hobbies { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,185 @@ | ||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Linq; | ||
using System.Reflection; | ||
using NHibernate.Linq; | ||
using NUnit.Framework; | ||
|
||
namespace NHibernate.Test.NHSpecificTest.NH3952 | ||
{ | ||
[TestFixture] | ||
public class Fixture : BugTestCase | ||
{ | ||
protected override void OnSetUp() | ||
{ | ||
using (ISession session = OpenSession()) | ||
using (ITransaction transaction = session.BeginTransaction()) | ||
{ | ||
var e1 = new Entity { Name = "Bob" }; | ||
session.Save(e1); | ||
|
||
var e2 = new Entity { Name = "Sally", ParentId = e1.Id, Hobbies = new[] { "Inline skate", "Sailing" } }; | ||
session.Save(e2); | ||
|
||
var e3 = new Entity { Name = "Max", ParentId = e1.Id }; | ||
session.Save(e3); | ||
|
||
session.Flush(); | ||
transaction.Commit(); | ||
} | ||
} | ||
|
||
protected override void OnTearDown() | ||
{ | ||
using (ISession session = OpenSession()) | ||
using (ITransaction transaction = session.BeginTransaction()) | ||
{ | ||
session.Delete("from System.Object"); | ||
|
||
session.Flush(); | ||
transaction.Commit(); | ||
} | ||
} | ||
|
||
[Test] | ||
public void SimpleNestedSelect() | ||
{ | ||
using (ISession session = OpenSession()) | ||
using (session.BeginTransaction()) | ||
{ | ||
var result = session.Query<Entity>() | ||
.Select(e => new { e.Name, children = e.Children.Select(c => c.Name).ToArray() }) | ||
.OrderBy(e => e.Name) | ||
.ToList(); | ||
|
||
Assert.AreEqual(3, result.Count); | ||
Assert.AreEqual(2, result[0].children.Length); | ||
Assert.AreEqual("Bob", result[0].Name); | ||
Assert.Contains("Max", result[0].children); | ||
Assert.Contains("Sally", result[0].children); | ||
Assert.AreEqual(0, result[1].children.Length + result[2].children.Length); | ||
} | ||
} | ||
|
||
[Test] | ||
public void ArraySelect() | ||
{ | ||
using (ISession session = OpenSession()) | ||
using (session.BeginTransaction()) | ||
{ | ||
var result = session.Query<Entity>() | ||
.Select(e => new { e.Name, e.Hobbies }) | ||
.OrderBy(e => e.Name) | ||
.ToList(); | ||
|
||
Assert.AreEqual(3, result.Count); | ||
Assert.AreEqual(2, result[2].Hobbies.Length); | ||
Assert.AreEqual("Sally", result[2].Name); | ||
Assert.Contains("Inline skate", result[2].Hobbies); | ||
Assert.Contains("Sailing", result[2].Hobbies); | ||
Assert.AreEqual(0, result[0].Hobbies.Length + result[1].Hobbies.Length); | ||
} | ||
} | ||
|
||
private static readonly MethodInfo CastMethodDefinition = ReflectionHelper.GetMethodDefinition( | ||
() => Enumerable.Cast<object>(null)); | ||
|
||
private static readonly MethodInfo CastMethod = ReflectionHelper.GetMethod( | ||
() => Enumerable.Cast<int>(null)); | ||
|
||
[Test, Explicit("Just a blunt perf comparison among some reflection patterns used in NH")] | ||
public void ReflectionBluntPerfCompare() | ||
{ | ||
var swCached = new Stopwatch(); | ||
swCached.Start(); | ||
for (var i = 0; i < 1000; i++) | ||
{ | ||
Trace.TraceInformation(CastMethod.ToString()); | ||
} | ||
swCached.Stop(); | ||
|
||
var swCachedDef = new Stopwatch(); | ||
swCachedDef.Start(); | ||
for (var i = 0; i < 1000; i++) | ||
{ | ||
var cast = CastMethodDefinition.MakeGenericMethod(new[] { typeof(int) }); | ||
Trace.TraceInformation(cast.ToString()); | ||
} | ||
swCachedDef.Stop(); | ||
|
||
var swRefl2 = new Stopwatch(); | ||
swRefl2.Start(); | ||
for (var i = 0; i < 1000; i++) | ||
{ | ||
var cast = GetMethod2(Enumerable.Cast<int>, (IEnumerable<int>)null); | ||
Trace.TraceInformation(cast.ToString()); | ||
} | ||
swRefl2.Stop(); | ||
|
||
var swRefl2Def = new Stopwatch(); | ||
swRefl2Def.Start(); | ||
for (var i = 0; i < 1000; i++) | ||
{ | ||
var cast = GetMethodDefinition2(Enumerable.Cast<object>, (IEnumerable<object>)null) | ||
.MakeGenericMethod(new[] { typeof(int) }); | ||
Trace.TraceInformation(cast.ToString()); | ||
} | ||
swRefl2Def.Stop(); | ||
|
||
var swRefl = new Stopwatch(); | ||
swRefl.Start(); | ||
for (var i = 0; i < 1000; i++) | ||
{ | ||
var cast = ReflectionHelper.GetMethod(() => Enumerable.Cast<int>(null)); | ||
Trace.TraceInformation(cast.ToString()); | ||
} | ||
swRefl.Stop(); | ||
|
||
var swReflDef = new Stopwatch(); | ||
swReflDef.Start(); | ||
for (var i = 0; i < 1000; i++) | ||
{ | ||
var cast = ReflectionHelper.GetMethodDefinition(() => Enumerable.Cast<object>(null)) | ||
.MakeGenericMethod(new[] { typeof(int) }); | ||
Trace.TraceInformation(cast.ToString()); | ||
} | ||
swReflDef.Stop(); | ||
|
||
var swEnHlp = new Stopwatch(); | ||
swEnHlp.Start(); | ||
for (var i = 0; i < 1000; i++) | ||
{ | ||
// Testing the obsolete helper perf. Disable obsolete warning. Remove this swEnHlp part of the test if this helper is to be removed. | ||
#pragma warning disable 0618 | ||
var cast = EnumerableHelper.GetMethod("Cast", new[] { typeof(IEnumerable) }, new[] { typeof(int) }); | ||
#pragma warning restore 0618 | ||
Trace.TraceInformation(cast.ToString()); | ||
} | ||
swEnHlp.Stop(); | ||
|
||
Assert.Pass(@"Blunt perf timings: | ||
Cached method: {0} | ||
Cached method definition + make gen: {1} | ||
Hazzik GetMethod: {5} | ||
Hazzik GetMethodDefinition + make gen: {6} | ||
ReflectionHelper.GetMethod: {2} | ||
ReflectionHelper.GetMethodDefinition + make gen: {3} | ||
EnumerableHelper.GetMethod(generic overload): {4}", | ||
swCached.Elapsed, swCachedDef.Elapsed, swRefl.Elapsed, swReflDef.Elapsed, swEnHlp.Elapsed, | ||
swRefl2.Elapsed, swRefl2Def.Elapsed); | ||
} | ||
|
||
public static MethodInfo GetMethod2<T, TResult>(Func<T, TResult> func, T arg1) | ||
{ | ||
return func.Method; | ||
} | ||
|
||
public static MethodInfo GetMethodDefinition2<T, TResult>(Func<T, TResult> func, T arg1) | ||
{ | ||
var method = GetMethod2(func, arg1); | ||
return method.IsGenericMethod ? method.GetGenericMethodDefinition() : method; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?xml version="1.0" encoding="utf-8" ?> | ||
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test" namespace="NHibernate.Test.NHSpecificTest.NH3952"> | ||
|
||
<class name="Entity"> | ||
<id name="Id" generator="guid.comb" /> | ||
<property name="Name" /> | ||
<property name="ParentId" /> | ||
<set name="Children"> | ||
<key column="ParentId"/> | ||
<one-to-many class="Entity" /> | ||
</set> | ||
<array name="Hobbies" table="Hobby"> | ||
<key column="EntityId" /> | ||
<index column="`Index`"/> | ||
<element type="string" length="50"> | ||
<column name="Name" /> | ||
</element> | ||
</array> | ||
</class> | ||
|
||
</hibernate-mapping> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
using System; | ||
using System.Collections; | ||
using System.Linq; | ||
using System.Linq.Expressions; | ||
using System.Reflection; | ||
|
@@ -57,7 +56,7 @@ public static MethodInfo GetMethod(Expression<System.Action> method) | |
if (method == null) | ||
throw new ArgumentNullException("method"); | ||
|
||
return ((MethodCallExpression) method.Body).Method; | ||
return ((MethodCallExpression)method.Body).Method; | ||
} | ||
|
||
/// <summary> | ||
|
@@ -94,7 +93,46 @@ internal static System.Type GetPropertyOrFieldType(this MemberInfo memberInfo) | |
} | ||
} | ||
|
||
// TODO rename / remove - reflection helper above is better | ||
internal static class ReflectionCache | ||
{ | ||
// When adding a method to this cache, please follow the naming convention of those subclasses and fields: | ||
// - Add your method to a subclass named according to the type holding the method, and suffixed with "Methods". | ||
// - Name the field according to the method name. | ||
// - If the method has overloads, suffix it with "With" followed by its parameter names. Do not list parameters | ||
// common to all overloads. | ||
// - If the method is a generic method definition, add "Definition" as final suffix. | ||
// - If the method is generic, suffix it with "On" followed by its generic parameter type names. | ||
// Avoid caching here narrow cases, such as those using specific types and unlikely to be used by many classes. | ||
// Cache them instead in classes using them. | ||
internal static class EnumerableMethods | ||
{ | ||
internal static readonly MethodInfo AggregateDefinition = | ||
ReflectionHelper.GetMethodDefinition(() => Enumerable.Aggregate<object>(null, null)); | ||
internal static readonly MethodInfo AggregateWithSeedDefinition = | ||
ReflectionHelper.GetMethodDefinition(() => Enumerable.Aggregate<object, object>(null, null, null)); | ||
internal static readonly MethodInfo AggregateWithSeedAndResultSelectorDefinition = | ||
ReflectionHelper.GetMethodDefinition(() => Enumerable.Aggregate<object, object, object>(null, null, null, null)); | ||
|
||
internal static readonly MethodInfo CastDefinition = | ||
ReflectionHelper.GetMethodDefinition(() => Enumerable.Cast<object>(null)); | ||
internal static readonly MethodInfo CastOnObjectArray = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this needs to be moved back to NestedSelectRewriter There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was unsure about that one, agreed. |
||
ReflectionHelper.GetMethod(() => Enumerable.Cast<object[]>(null)); | ||
|
||
internal static readonly MethodInfo GroupByWithElementSelectorDefinition = ReflectionHelper.GetMethodDefinition( | ||
() => Enumerable.GroupBy<object, object, object>(null, null, (Func<object, object>)null)); | ||
|
||
internal static readonly MethodInfo SelectDefinition = | ||
ReflectionHelper.GetMethodDefinition(() => Enumerable.Select<object, object>(null, (Func<object, object>)null)); | ||
|
||
internal static readonly MethodInfo ToArrayDefinition = | ||
ReflectionHelper.GetMethodDefinition(() => Enumerable.ToArray<object>(null)); | ||
|
||
internal static readonly MethodInfo ToListDefinition = | ||
ReflectionHelper.GetMethodDefinition(() => Enumerable.ToList<object>(null)); | ||
} | ||
} | ||
|
||
[Obsolete("Please use ReflectionHelper instead")] | ||
public static class EnumerableHelper | ||
{ | ||
public static MethodInfo GetMethod(string name, System.Type[] parameterTypes) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Linq.Expressions; | ||
|
@@ -16,7 +15,12 @@ namespace NHibernate.Linq.NestedSelects | |
{ | ||
static class NestedSelectRewriter | ||
{ | ||
private static readonly MethodInfo CastMethod = EnumerableHelper.GetMethod("Cast", new[] { typeof (IEnumerable) }, new[] { typeof (object[]) }); | ||
private static readonly MethodInfo GroupByMethod = ReflectionHelper.GetMethod( | ||
() => Enumerable.GroupBy<object[], Tuple, Tuple>(null, null, (Func<object[], Tuple>)null)); | ||
private static readonly MethodInfo WhereMethod = ReflectionHelper.GetMethod( | ||
() => Enumerable.Where<Tuple>(null, (Func<Tuple, bool>)null)); | ||
private static readonly MethodInfo SelectMethodDefinition = ReflectionHelper.GetMethodDefinition( | ||
() => Enumerable.Select<Tuple, object>(null, (Func<Tuple, object>)null)); | ||
|
||
public static void ReWrite(QueryModel queryModel, ISessionFactory sessionFactory) | ||
{ | ||
|
@@ -51,16 +55,12 @@ public static void ReWrite(QueryModel queryModel, ISessionFactory sessionFactory | |
var keySelector = CreateSelector(elementExpression, 0); | ||
|
||
var elementSelector = CreateSelector(elementExpression, 1); | ||
|
||
var groupBy = EnumerableHelper.GetMethod("GroupBy", | ||
new[] { typeof (IEnumerable<>), typeof (Func<,>), typeof (Func<,>) }, | ||
new[] { typeof (object[]), typeof (Tuple), typeof (Tuple) }); | ||
|
||
|
||
var input = Expression.Parameter(typeof (IEnumerable<object>), "input"); | ||
|
||
var lambda = Expression.Lambda( | ||
Expression.Call(groupBy, | ||
Expression.Call(CastMethod, input), | ||
Expression.Call(GroupByMethod, | ||
Expression.Call(ReflectionCache.EnumerableMethods.CastOnObjectArray, input), | ||
keySelector, | ||
elementSelector), | ||
input); | ||
|
@@ -154,24 +154,16 @@ private static LambdaExpression MakeSelector(ICollection<ExpressionHolder> eleme | |
private static Expression SubCollectionQuery(System.Type collectionType, System.Type elementType, Expression source, Expression predicate, Expression selector) | ||
{ | ||
// source.Where(predicate).Select(selector).ToList(); | ||
var whereMethod = EnumerableHelper.GetMethod("Where", | ||
new[] { typeof (IEnumerable<>), typeof (Func<,>) }, | ||
new[] { typeof (Tuple) }); | ||
|
||
|
||
var selectMethod = EnumerableHelper.GetMethod("Select", | ||
new[] { typeof (IEnumerable<>), typeof (Func<,>) }, | ||
new[] { typeof (Tuple), elementType }); | ||
var selectMethod = SelectMethodDefinition.MakeGenericMethod(new[] { typeof(Tuple), elementType }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, missed it. |
||
|
||
var select = Expression.Call(selectMethod, | ||
Expression.Call(whereMethod, source, predicate), | ||
Expression.Call(WhereMethod, source, predicate), | ||
selector); | ||
|
||
if (collectionType.IsArray) | ||
{ | ||
var toArrayMethod = EnumerableHelper.GetMethod("ToArray", | ||
new[] { typeof(IEnumerable<>) }, | ||
new[] { elementType }); | ||
var toArrayMethod = ReflectionCache.EnumerableMethods.ToArrayDefinition.MakeGenericMethod(new[] { elementType }); | ||
|
||
var array = Expression.Call(toArrayMethod, @select); | ||
return array; | ||
|
@@ -181,9 +173,7 @@ private static Expression SubCollectionQuery(System.Type collectionType, System. | |
if (constructor != null) | ||
return Expression.New(constructor, (Expression) @select); | ||
|
||
var toListMethod = EnumerableHelper.GetMethod("ToList", | ||
new[] { typeof (IEnumerable<>) }, | ||
new[] { elementType }); | ||
var toListMethod = ReflectionCache.EnumerableMethods.ToListDefinition.MakeGenericMethod(new[] { elementType }); | ||
|
||
return Expression.Call(Expression.Call(toListMethod, @select), | ||
"AsReadonly", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this to a separate class in
NHibernate.Util
namespace.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Way more cleaner.