#2520 - Fixed Effort- and ODataQueryTests and a bug in ExpressionVisitor

[gentledepp]

Fix for #2520

The ExpressionMapper has the following bug.
In ExpressionMapper.VisitBinary, it compares the types of the two expressions.
If one is of type string and the other is not, it adds a "ToString()" to the expression.

However, it does not check for the "null Constant".

Therefore, the OData query of ODataQueryTests.CanFilter_FullNameEndsWith converts the following DTO expression:

.Call System.Linq.Queryable.Where(
    .Call System.Linq.Queryable.OrderBy(
        .Constant<System.Linq.EnumerableQuery`1[AutoMapperSamples.EF.Dtos.OrderDto]>(AutoMapperSamples.EF.Dtos.OrderDto[]),
        '(.Lambda #Lambda1<System.Func`2[AutoMapperSamples.EF.Dtos.OrderDto,System.Double]>)),
    '(.Lambda #Lambda2<System.Func`2[AutoMapperSamples.EF.Dtos.OrderDto,System.Boolean]>))

.Lambda #Lambda1<System.Func`2[AutoMapperSamples.EF.Dtos.OrderDto,System.Double]>(AutoMapperSamples.EF.Dtos.OrderDto $o) {
    $o.Price
}

.Lambda #Lambda2<System.Func`2[AutoMapperSamples.EF.Dtos.OrderDto,System.Boolean]>(AutoMapperSamples.EF.Dtos.OrderDto $$it)
{
    .If (
        $$it.FullName == null || .Constant<System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]>(System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]).TypedProperty ==
        null
    ) {
        null
    } .Else {
        (System.Nullable`1[System.Boolean]).Call ($$it.FullName).EndsWith(.Constant<System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]>(System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]).TypedProperty)
    } == .Constant<System.Nullable`1[System.Boolean]>(True)
}

to the following Entity expression:

.Call System.Linq.Queryable.Where(
    .Call System.Linq.Queryable.OrderBy(
        .Call (.Call .Constant<System.Data.Entity.Core.Objects.ObjectQuery`1[AutoMapperSamples.EF.Model.Order]>(System.Data.Entity.Core.Objects.ObjectQuery`1[AutoMapperSamples.EF.Model.Order]).MergeAs(.Constant<System.Data.Entity.Core.Objects.MergeOption>(AppendOnly))
        ).IncludeSpan(.Constant<System.Data.Entity.Core.Objects.Span>(System.Data.Entity.Core.Objects.Span)),
        '(.Lambda #Lambda1<System.Func`2[AutoMapperSamples.EF.Model.Order,System.Double]>)),
    '(.Lambda #Lambda2<System.Func`2[AutoMapperSamples.EF.Model.Order,System.Boolean]>))

.Lambda #Lambda1<System.Func`2[AutoMapperSamples.EF.Model.Order,System.Double]>(AutoMapperSamples.EF.Model.Order $o) {
    $o.Price
}

.Lambda #Lambda2<System.Func`2[AutoMapperSamples.EF.Model.Order,System.Boolean]>(AutoMapperSamples.EF.Model.Order $$it) {
    .If (
        $$it.Name == .Call null.ToString() || .Constant<System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]>(System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]).TypedProperty ==
        .Call null.ToString()
    ) {
        null
    } .Else {
        (System.Nullable`1[System.Boolean]).Call ($$it.Name).EndsWith(.Constant<System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]>(System.Web.Http.OData.Query.Expressions.LinqParameterContainer+TypedLinqParameterContainer`1[System.String]).TypedProperty)
    } == .Constant<System.Nullable`1[System.Boolean]>(True)
}

As you can see, the Entity expression contains a ".Call null.ToString()" which results in an InvalidOperationException in EntityFramework.

[lbargaoanu]

This is kind of weird :) A regular method should do the job.

[gentledepp]

ya think! It is just used in this one single method and contributes to readability

[lbargaoanu]

I disagree. You can use a local method instead.

[gentledepp]

yeah - you're right with that.
I aleady pushed that change

[lbargaoanu]

I think it looks much better after the function body, like in the docs. Easier on the eyes :)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.