scottksmith95 / LINQKit

LINQKit is a free set of extensions for LINQ to SQL and Entity Framework power users.
MIT License
1.65k stars 165 forks source link

Version 1.1.19 introduces a breaking change to ExpressionExpander.Visit #129

Closed buvinghausen closed 4 years ago

buvinghausen commented 4 years ago

I have put together a simple repository that reproduces the System.InvalidOperationException that I get when I use the latest version of LINQKit here

When I use version 1.1.17 everything works as expected and both console write lines produce true. When I use version 1.1.19 I get the following exception.

System.InvalidOperationException: 'Invoke cannot evaluate LambdaExpression from 'sd => (sd.Id == new Guid("08d87544-4aae-8e73-afd5-772ab0a086a1"))'. Ensure that your function/property/member returns LambdaExpression'

For now I've locked in using version 1.1.17 but can you provide some guidance on what I'm doing wrong in the expression wireup?

StefH commented 4 years ago

Seems like a bug was introduced by https://github.com/scottksmith95/LINQKit/pull/126

I'll take a look.

StefH commented 4 years ago

I've unlisted version 1.1.18 and 1.1.19 from NuGet.

@sdanyliv : can you please take a look and fix this? (I also noticed that the unit tests did not work anymore?) image

buvinghausen commented 4 years ago

Thanks @StefH

sdanyliv commented 4 years ago

Sure. Looking at.

sdanyliv commented 4 years ago

All tests are green. Looking at proposed repository with reproducing steps.

StefH commented 4 years ago

@sdanyliv Do you mean that the test are green? Or you fixed some code an now they are green?

sdanyliv commented 4 years ago

On my workstation all test are green. I would never propose PR without running tests.

StefH commented 4 years ago

This code :

protected LambdaExpression EvaluateTarget(Expression target)
        {
            if (target is LambdaExpression)
            {
                return (LambdaExpression) target;
            }

            if (target.NodeType == ExpressionType.Call)
            {
                var mc = (MethodCallExpression) target;
                if (mc.Method.Name == "Compile" && mc.Method.DeclaringType?.GetGenericTypeDefinition() == typeof(Expression<>))
                {
                    target = mc.Object;
                }
            }

            var lambda = target.EvaluateExpression() as LambdaExpression;

            if (lambda == null)
            {
                throw new InvalidOperationException($"Invoke cannot evaluate LambdaExpression from '{target}'. Ensure that your function/property/member returns LambdaExpression");
            }

            return lambda;
        }

is the fix?

Adding: if (target is LambdaExpression) { return (LambdaExpression) target; }

sdanyliv commented 4 years ago

Yes, I have prepared PR.

StefH commented 4 years ago

OK. Thanks a lot. I did also start on a PR, and will use that one to merge to master.

I copied your test class. Thanks for that one.

StefH commented 4 years ago

New version will be uploaded tomorrow.

sdanyliv commented 4 years ago

Another case which were not covered by unit tests. Good to have them.

StefH commented 4 years ago

@sdanyliv and @buvinghausen
1.1.20-preview-01 is released on nuget

buvinghausen commented 4 years ago

@StefH I have verified that 1.1.20-preview-01 works with our stack. Thank you and @sdanyliv on the quick turnaround that was awesome.