ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.27k stars 748 forks source link

EF Core + AutoMapper ProjectTo - SQL code not projected correctly #4114

Closed eduardomhg closed 2 years ago

eduardomhg commented 3 years ago

Is there an existing issue for this?

Describe the bug

I'm using EF Core (v 5.0.9) and Automapper (v 10.1.1) with a very simple example database (the one described here).

I've set up a GraphQL server with HotChocolate and defined some simple GraphQL queries with [UseDbContext] and [UseProjection]. The generated SQL code is projected depending on the selection of fields, exactly as expected.

However when I add a layer of simple DTO classes, define maps with Automapper and then use ProjectTo in the queries, the projection stops working properly when there's a 1-to-1 relationship (interestingly it doesn't seem to happen for 1-to-many).

So for example the following query:

{
  postsDto {
    postId,
    title,
    blog {
      url
    }
  }
}

Results in the following SQL code:

SELECT [p].[PostId], [p].[Title], CAST(0 AS bit), [b].[BlogId], [b].[Url], [p0].[BlogId], [p0].[Content], [p0].[PostId], [p0].[Title]
FROM [Posts] AS [p]
INNER JOIN [Blogs] AS [b] ON [p].[BlogId] = [b].[BlogId]
LEFT JOIN [Posts] AS [p0] ON [b].[BlogId] = [p0].[BlogId]
ORDER BY [p].[PostId], [b].[BlogId], [p0].[PostId]

Which has fields that I didn't ask for plus it makes and extra JOIN back to the Posts table (because the Blog Dto exposes a Posts property, even though I haven't asked for it in the GraphQL query).

I also experience the problem described here https://github.com/ChilliCream/hotchocolate/issues/3008.

Steps to reproduce

  1. Create a simple EF Core DbContext and Entities exactly as described here .
  2. Create a ASP.NET 5.0 project and configure HotChocolate as described here
  3. Install Automapper 10.1.1 and create Dto classes for Blog and Posts, and then configure simple mappings:
    public class PostDto
    {
        public int PostId { get; set; }
        public string Title { get; set; }
        public string Content { get; set; }
        public int BlogId { get; set; }
        public BlogDto Blog { get; set; }
    }
    public class PostProfile : Profile
    {
        public PostProfile()
        {
            CreateMap<Post, PostDto>();
        }
    }
        public void ConfigureServices(IServiceCollection services)
        {
            services
                .AddGraphQLServer()
                .AddProjections()
                .AddQueryType<Query>();

            services.AddMiniProfiler
               (options =>
               { options.RouteBasePath = "/profiler"; })
                     .AddEntityFramework();

            services.AddPooledDbContextFactory<BloggingContext>(options =>
            {
               ...
            });

            services.AddAutoMapper(typeof(BlogProfile));
        }    
  1. Define a simple projected query using UseDbContext and UseProjection:
        [UseDbContext(typeof(BloggingContext))]
        [UseProjection]
        public IQueryable<BlogDto> GetBlogsDto(
               [ScopedService] BloggingContext context)
               => context.Blogs.ProjectTo<BlogDto>(mapper.ConfigurationProvider);

        [UseDbContext(typeof(BloggingContext))]
        [UseProjection]
        public IQueryable<PostDto> GetPostsDto(
               [ScopedService] BloggingContext context)
               => context.Posts.ProjectTo<PostDto>(mapper.ConfigurationProvider);
  1. Run the following query and check the generated SQL code using the miniprofiler

    {
    postsDto {
    postId,
    title,
    blog {
      url
    }
    }
    }
  2. The generated SQL code is incorrect for me:

SELECT [p].[PostId], [p].[Title], CAST(0 AS bit), [b].[BlogId], [b].[Url], [p0].[BlogId], [p0].[Content], [p0].[PostId], [p0].[Title]
FROM [Posts] AS [p]
INNER JOIN [Blogs] AS [b] ON [p].[BlogId] = [b].[BlogId]
LEFT JOIN [Posts] AS [p0] ON [b].[BlogId] = [p0].[BlogId]
ORDER BY [p].[PostId], [b].[BlogId], [p0].[PostId]
  1. Note this issue doesn't happen if the EF Core entities are use directly (no DTOs or automapper), and also if the relationship is one-to-many such as the inverse query:
{
  blogsDto {
    blogId,
    posts {
      postId,
      title
    }
  }
}

Relevant log output

No response

Additional Context?

I have tested this with the latest stable version (11.3.5) and the latest perelease version (12.0.0-preview30), getting the same result.

Product

Hot Chocolate

Version

11.3.5

valentinmarinro commented 3 years ago

Hey @tobias-tengler, I also have this issue, and it is reproducing even if I make a manual projection without AutoMapper to a DTO like this:

public IQueryable<ContactDto> GetContacts()
{
    return DbContext.Set<Contact>().Select(c => new ContactDto
    {
        Id = c.Id,
        Name = c.Name,
        Age = c.Age,
        Children = c.Children.Select(cc => new ChildrenDto
        {
            Id = c.Id,
            Name = c.Name,
            Age = c.Age
        })
    });
}

Hey @eduardomhg , have you found any workaround on this one?

oysandvik94 commented 3 years ago

Hi, I also have this issue. I'm pretty sure it's the same problem as https://github.com/ChilliCream/hotchocolate/issues/2373.

oysandvik94 commented 3 years ago

This issue also happens without AutoMapper, using just regular naive manual mapping. It looks like the null check described in #2373 is the issue. When querying for a single attribute on a collection (where the projection works as expected) you will see that it projects the ID of the other entity implicitly, which I'm guessing means that the null check is done only on the ID, while for single entities it does the null check on the entire entity.

gmavritsakis commented 2 years ago

Same here, using .Net 6, HotChocolate 12.3.2 and AutoMapper 8.1.1 Flattening the target models (using automapper) fixes the problem but this is not a viable solution. This is a game stopper everywhere we want to use nested objects without data loaders.

PascalSenn commented 2 years ago

Can someone provide a reproduction of bug in form of a very simple project? so we can have a look at it?

PascalSenn commented 2 years ago

So I have investigated what is going on here. Given the configuration in the issue description and the following query:

    {
      postsdto {
        postid
        title
        blog {
          url
        }
      }
    }

This is what projections generate:

.Lambda #Lambda1<System.Func`2[HotChocolate.Data.Projections.AutomapperTests+PostDto,HotChocolate.Data.Projections.AutomapperTests+PostDto]>(HotChocolate.Data.Projections.AutomapperTests+PostDto $_s1)
{
    .New HotChocolate.Data.Projections.AutomapperTests+PostDto(){
        PostId = $_s1.PostId,
        Title = $_s1.Title,
        Blog = .If ($_s1.Blog != null) {
            .New HotChocolate.Data.Projections.AutomapperTests+BlogDto(){
                Url = ($_s1.Blog).Url
            }
        } .Else {
            .Default(HotChocolate.Data.Projections.AutomapperTests+BlogDto)
        }
    }
}

As soon as this is applied to the IQueryable that was mapped with automapper:

DbSet<Post>()
    .Select(dtoPost => new PostDto{ 
        Blog = dtoPost.Blog == null ? null : new BlogDto{ 
            BlogId = dtoPost.Blog.BlogId, 
            Name = dtoPost.Blog.Name, 
            Posts = dtoPost.Blog.Posts
                .Select(dtoPost => new PostDto{ 
                    BlogId = dtoPost.BlogId, 
                    Content = dtoPost.Content, 
                    PostId = dtoPost.PostId, 
                    Title = dtoPost.Title 
                }
                )
                .ToList(), 
            Url = dtoPost.Blog.Url 
        }
        , 
        BlogId = dtoPost.BlogId, 
        Content = dtoPost.Content, 
        PostId = dtoPost.PostId, 
        Title = dtoPost.Title 
    }
    )
    .Select(_s1 => new PostDto{ 
        PostId = _s1.PostId, 
        Title = _s1.Title, 
        Blog = _s1.Blog != null ? new BlogDto{ Url = _s1.Blog.Url }
         : default(BlogDto) 
    }
    )

So you see that the root of the problem is, that the projections of auto mapper are not merged with the projections of the UseProjections.

There are a couple of possibilities we have here.

  1. We could rewrite the expression tree based on what we have projected, but if you start writing an expression tree rewriter you open pandoras box.
  2. We could also let the projection provider read the automapper configuration and project based on that, but this would require a significant change in the projection visitation logic and possible breaking changes
  3. EF Core could understand chained selects better but this would require investigation why they do not do it already, a PR ant then maybe a possible fix in EF Core 7

So i tried out a few things (pretty much in the order that is listed above). But then, i had a deeper look into the the automapper documentation. And voila: https://docs.automapper.org/en/stable/Queryable-Extensions.html#explicit-expansion

So if we configure the Profile like this:

    public class PostProfile : Profile
    {
        public PostProfile()
        {
            CreateMap<Post, PostDto>().ForAllMembers(x => x.ExplicitExpansion());
        }
    }

and do some magic in the resolver:

        [UseDbContext(typeof(BloggingContext))]
        [UseSqlLogging]
        [UseProjection]
        public IQueryable<PostDto> GetPostsWorking(
            [Service] IMapper mapper,
            [ScopedService] BloggingContext dbContext,
            IResolverContext context)
        {
            // ensure sorting is only applied once
            context.LocalContextData =
                context.LocalContextData.SetItem(QueryableProjectionProvider.SkipProjectionKey,
                    true);

            var visitorContext =
                new QueryableProjectionContext(
                    context,
                    context.ObjectType,
                    context.Selection.Field.Type.UnwrapRuntimeType());
            var visitor = new QueryableProjectionVisitor();
            visitor.Visit(visitorContext);

            Expression<Func<PostDto, object>> projection =
                visitorContext.Project<PostDto, object>();

            var res = dbContext.Posts
                .ProjectTo<PostDto>(mapper.ConfigurationProvider, projection);
            return res;
        }

We get this result:

DbSet<Post>()
    .Select(dtoPost => new PostDto{ 
        Blog = dtoPost.Blog == null ? null : new BlogDto{ Url = dtoPost.Blog.Url }
        , 
        PostId = dtoPost.PostId, 
        Title = dtoPost.Title 
    }
    )

That generates the following SQL

SELECT 0, "b"."Url", "p"."PostId", "p"."Title"
FROM "Posts" AS "p"
INNER JOIN "Blogs" AS "b" ON "p"."BlogId" = "b"."BlogId"

This requires a very minor API change in projections. I guess we could also create a HotChocolate.Data.Projections.AutoMapper package that abstracts this a little.

Could everyone involved life with this API:

        [UseDbContext(typeof(BloggingContext))]
        [UseProjection]
        public IQueryable<PostDto> GetPostsWorking(
            [ScopedService] BloggingContext dbContext,
            IResolverContext context)
        {
            return dbContext.Posts.ProjectTo<Post, PostDto>(context);
        }
eduardomhg commented 2 years ago

Sorry guys but I think I may be missing something here.

I've applied the following changes to the simple Blogs/Posts project:

  1. Update HotChocolate to 12.6.0
  2. Update Automapper to 11.0.1
  3. Install HotChocolate.Data.AutoMapper 12.6.0
  4. Add .ForAllMembers(x => x.ExplicitExpansion()) to all Automapper mappings
  5. Change the ProjectTo call in the query to the extension method provided by @PascalSenn , passing the ResolverContext as a parameter.

Then if run the following query:

{ posts { postId, title, blog { blogId, url } } }

I get nulls in all the fields other than the IDs.

Also, if I remove the explicit expansion from the mapping, the query returns the correct data but the SQL is not projected correctly (original issue).

Can someone tell me what I'm missing?

MarkoH17 commented 2 years ago

@eduardomhg I am also seeing this issue. Did you ever find a solution?

PascalSenn commented 2 years ago

@eduardomhg https://github.com/ChilliCream/hotchocolate/issues/4724#issuecomment-1041940763

It seems like the problem is automapper 11. Projections to work with 10.1. This is something that should be fixed updstream in the automapper repository. But watch out. the resulting expressions are often to complex for EF to understand. So if you want to use this in combination with filtering, you could run into issues

eduardomhg commented 2 years ago

Hi @PascalSenn, yes, I can confirm that downgrading Automapper to version 10.1.1 (and in my case AutoMapper.Extensions.Microsoft.DependencyInjection to 8.1.1) does fix the issue and works as I expect (and you intended).

So do you think this is a bug that should be raised in Automapper and not a behavior change that has caused your ProjectTo to not work properly anymore? Just asking to see what the next step should be.

BlackDice051 commented 1 year ago

@eduardomhg #4724 (comment)

It seems like the problem is automapper 11. Projections to work with 10.1. This is something that should be fixed updstream in the automapper repository. But watch out. the resulting expressions are often to complex for EF to understand. So if you want to use this in combination with filtering, you could run into issues

From what I see in the AutoMapper documentation examples regarding explicit expansion (https://docs.automapper.org/en/stable/Queryable-Extensions.html#explicit-expansion), I think, that the projection expression you've fed to AutoMapper's ProjectTo method never supposed to work.

AutoMapper's ProjectTo method wants an expression for each expanded member separately and not one expression, containing the entire initialization.

Here is the sample GraphQL query from my test project:

{
  orders {
      id
      name
      description
      customer {
        id,
        contacts{
          position,
          name,
          value
        }
      },
      items{
        position
      }
  }
}

Your current ProjectTo method from HotChocolate.Data.AutoMapper package generates this single projection expression in my setup:

{
    _s1 => Convert(
    new OrderDto() 
    {
        Id = _s1.Id, 
        Name = _s1.Name, 
        Description = _s1.Description,
        Customer = new CustomerDto() 
        {   
            Id = _s1.Customer.Id, 
            Contacts = _s1.Customer.Contacts.Select(p2 => new CustomerContactDto() 
            {
                Position = p2.Position, 
                Name = p2.Name, 
                Value = p2.Value
            }).ToArray()
        }, 
        Items = _s1.Items.Select(p1 => new OrderItemDto() 
        {
            Position = p1.Position
        }).ToArray()
    }, Object)
}

I've managed to rewrite your code and break this projection expression into multiple separate expand expressions:

p_0 => Convert(p_0.Id, Object)
p_0 => p_0.Name
p_0 => p_0.Description
p_0 => Convert(p_0.Customer.Id, Object)
p_0 => p_0.Customer.Contacts.Select(p_1 => Convert(p_1.Position, Object))
p_0 => p_0.Customer.Contacts.Select(p_1 => p_1.Name)
p_0 => p_0.Customer.Contacts.Select(p_1 => p_1.Value)
p_0 => p_0.Items.Select(p_1 => Convert(p_1.Position, Object))

This way I've got a correct projection SQL code even with AutoMapper 12.0.1:

SELECT t."Id", t."Name", t."Description", FALSE, c."Id", c0."Position", c0."Name", c0."Value", c0."CustomerId", o0."Position", o0."OrderId"
      FROM (
          SELECT o."Id", o."CustomerId", o."Description", o."Name"
          FROM "Orders" AS o
          LIMIT @__p_0
      ) AS t
      INNER JOIN "Customers" AS c ON t."CustomerId" = c."Id"
      LEFT JOIN "CustomerContacts" AS c0 ON c."Id" = c0."CustomerId"
      LEFT JOIN "OrderItem" AS o0 ON t."Id" = o0."OrderId"
      ORDER BY t."Id", c."Id", c0."CustomerId", c0."Position", o0."OrderId"

The projection separation is somewhat naive, and will possibly break on some edge cases, but here it is, feel free to test and enhance it:

public static class AutoMapperQueryableExtensions
{
    public static IQueryable<TDest> ProjectTo<TSource, TDest>(this IQueryable<TSource> query, IResolverContext context)
    {
        IMapper mapper = context.Service<IMapper>();

        context.LocalContextData =
            context.LocalContextData.SetItem(QueryableProjectionProvider.SkipProjectionKey,
                true);

        var visitorContext =
            new QueryableProjectionContext(
                context,
                context.ObjectType,
                context.Selection.Field.Type.UnwrapRuntimeType(), false);
        var visitor = new QueryableProjectionVisitor();
        visitor.Visit(visitorContext);

#pragma warning disable CS8631
        Expression<Func<TDest, object?>> projection = visitorContext.Project<TDest, object?>();
#pragma warning restore CS8631

        List<Expression<Func<TDest, object?>>> membersToExpand = new();

        VisitRoot(projection, membersToExpand);

        return query.ProjectTo(mapper.ConfigurationProvider, membersToExpand.ToArray());
    }

    private static void VisitRoot<TDest>(Expression<Func<TDest, object?>> root, List<Expression<Func<TDest, object?>>> expressions)
    {
        UnaryExpression lambdaBody = (UnaryExpression)root.Body;

        MemberInitExpression memberInit = (MemberInitExpression)lambdaBody.Operand;

        int level = 0;

        ParameterExpression parameter = MakeParameter(memberInit.Type, level);

        foreach (var expression in VisitMemberInit(memberInit, parameter, level))
        {
            var lambda = Expression.Lambda<Func<TDest, object?>>(expression, parameter);

            expressions.Add(lambda);
        }
    }

    private static IEnumerable<Expression> VisitMemberInit(MemberInitExpression memberInit,
        Expression path, int level)
    {
        var memberAssignments = memberInit.Bindings.Cast<MemberAssignment>();

        foreach (MemberAssignment memberAssignment in memberAssignments)
        {
            foreach (Expression expression in VisitMemberAssignment(memberAssignment, path, level))
            {
                yield return expression;
            }
        }
    }

    private static IEnumerable<Expression> VisitMemberAssignment(MemberAssignment memberAssignment, Expression path, int level)
    {
        var memberExpression = memberAssignment.Expression;

        var member = (PropertyInfo)memberAssignment.Member;

        if (memberExpression is MemberExpression)
        {
            yield return VisitPrimitive(member, path);
        }

        if (memberExpression is MemberInitExpression nestedMemberInit)
        {
            foreach (Expression expression in VisitNestedObject(nestedMemberInit, member, path, level))
            {
                yield return expression;
            }
        }

        if (memberExpression is MethodCallExpression toArrayCallExpr)
        {
            foreach (Expression expression in VisitNestedCollection(toArrayCallExpr, member, path, level))
            {
                yield return expression;
            }
        }
    }

    private static Expression VisitPrimitive(PropertyInfo member, Expression path)
    {
        Expression memberAccess = Expression.MakeMemberAccess(path, member);

        if (member is { PropertyType.IsValueType: true })
        {
            memberAccess = Expression.Convert(memberAccess, typeof(object));
        }

        return memberAccess;
    }

    private static IEnumerable<Expression> VisitNestedObject(MemberInitExpression nestedMemberInit, PropertyInfo member, Expression path, int level)
    {
        var nestedPath = Expression.MakeMemberAccess(path, member);

        foreach (Expression expression in VisitMemberInit(nestedMemberInit, nestedPath, level))
        {
            yield return expression;
        }
    }

    private static IEnumerable<Expression> VisitNestedCollection(MethodCallExpression toArrayCall, PropertyInfo member, Expression path, int level)
    {
        if (toArrayCall.Arguments[0] is not MethodCallExpression selectCall) 
            yield break;

        if (selectCall.Arguments[1] is not LambdaExpression selectLambda) 
            yield break;

        var nestedPath = Expression.MakeMemberAccess(path, member);

        var memberInit = (MemberInitExpression)selectLambda.Body;

        level++;

        ParameterExpression parameter = MakeParameter(memberInit.Type, level);

        var selectMethod = typeof(Enumerable)
            .GetMethods(BindingFlags.Static | BindingFlags.Public)
            .Single(mi => mi.Name == nameof(Enumerable.Select) &&
                          mi.GetParameters()[1].ParameterType.GetGenericArguments().Length == 2)
            .MakeGenericMethod(parameter.Type, typeof(object));

        foreach (var expression in VisitMemberInit(memberInit, parameter, level))
        {
            LambdaExpression lambda = Expression.Lambda(expression, parameter);

            yield return Expression.Call(null, selectMethod, nestedPath, lambda);
        }
    }

    private static ParameterExpression MakeParameter(Type type, int lambdaLevel)
    {
        return Expression.Parameter(type, "p_" + lambdaLevel);
    }
}

P.S.:

I'm using this setup: AutoMapper - 12.0.1 HotChocolate.AspNetCore - 13.0.5 HotChocolate.Data.AutoMapper - 13.0.5 HotChocolate.Data.EntityFramework - 13.0.5 Microsoft.EntityFrameworkCore - 7.0.5