Skip to content
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

String based Include #3921

Closed
annexus opened this issue Nov 29, 2015 · 20 comments
Closed

String based Include #3921

annexus opened this issue Nov 29, 2015 · 20 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@annexus
Copy link

annexus commented Nov 29, 2015

In the older version of EF you were able to do something like, which is useful for dynamic scenarios.

        var query = db.CampaignCreatives.AsQueryable();
        foreach (string include in includes)
        {
            query = query.Include(include);
        };

Edited July-18 by @rowanmiller

Workaround

Here is some code that adds two string based overloads of Include.

  • One allows you to do EF6 style string includes - such as context.Blogs.Include("Posts") and context.Blogs.Include("Posts.Author.Photo").
  • The second overload allows you to make use of the C# 6 nameof feature when including multiple levels context.Blogs.Include(nameof(Post.Blog), nameof(Blog.Owner), nameof(Person.Photo))
using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;

namespace ConsoleApplication5
{
    public static class Extensions
    {
        private static MethodInfo _include = typeof(EntityFrameworkQueryableExtensions)
                        .GetMethod("Include");

        private static MethodInfo _thenIncludeReference = typeof(EntityFrameworkQueryableExtensions)
            .GetMethods()
            .Where(m => m.Name == "ThenInclude")
            .Single(m => m.Name == "ThenInclude" &&
                         m.GetParameters()
                          .Single(p => p.Name == "source")
                          .ParameterType
                          .GetGenericArguments()[1].Name != typeof(ICollection<>).Name);

        private static MethodInfo _thenIncludeCollection = typeof(EntityFrameworkQueryableExtensions)
            .GetMethods()
            .Where(m => m.Name == "ThenInclude")
            .Single(m => m.Name == "ThenInclude" &&
                         m.GetParameters()
                          .Single(p => p.Name == "source")
                          .ParameterType
                          .GetGenericArguments()[1].Name == typeof(ICollection<>).Name);

        public static IQueryable<T> Include<T>(this IQueryable<T> query, string include)
        {
            return query.Include(include.Split('.'));
        }

        public static IQueryable<T> Include<T>(this IQueryable<T> query, params string[] include)
        {
            var currentType = query.ElementType;
            var previousNavWasCollection = false;

            for (int i = 0; i < include.Length; i++)
            {
                var navigationName = include[i];
                var navigationProperty = currentType.GetProperty(navigationName);
                if (navigationProperty == null)
                {
                    throw new ArgumentException($"'{navigationName}' is not a valid property of '{currentType}'");
                }

                var includeMethod = i == 0
                    ? _include.MakeGenericMethod(query.ElementType, navigationProperty.PropertyType)
                    : previousNavWasCollection
                        ? _thenIncludeCollection.MakeGenericMethod(query.ElementType, currentType, navigationProperty.PropertyType)
                        : _thenIncludeReference.MakeGenericMethod(query.ElementType, currentType, navigationProperty.PropertyType);

                var expressionParameter = Expression.Parameter(currentType);
                var expression = Expression.Lambda(
                    Expression.Property(expressionParameter, navigationName),
                    expressionParameter);

                query = (IQueryable<T>)includeMethod.Invoke(null, new object[] { query, expression });

                if (navigationProperty.PropertyType.GetInterfaces().Any(x => x.Name == typeof(ICollection<>).Name))
                {
                    previousNavWasCollection = true;
                    currentType = navigationProperty.PropertyType.GetGenericArguments().Single();
                }
                else
                {
                    previousNavWasCollection = false;
                    currentType = navigationProperty.PropertyType;
                }
            }

            return query;
        }
    }
}
@pearsonw
Copy link

pearsonw commented Dec 7, 2015

I just asked the same question here too http://stackoverflow.com/q/34124501/5647927

@rowanmiller rowanmiller changed the title Include by string missing in ef7 String based Include Dec 8, 2015
@rowanmiller rowanmiller added this to the Backlog milestone Dec 8, 2015
@Moffmo
Copy link

Moffmo commented Dec 9, 2015

I too would like this, It helps with creating dynamic repositories. I have tried to make a workaround but it seems impossible to return Expression<Func<TEntity, TProperty>> as you never know the type of TProperty

@chinookproject
Copy link

I also need this functionality.

And I'm not sure if this will be picked up along with this enhancement, but I'm also missing that you can't do a dynamic DbSet: ctx.Set(Obj.GetType()). like you can do in EF6.

@EvAlex
Copy link

EvAlex commented May 17, 2016

This should be a joke.

Before C# 6 we didn't have nameof and early versions of EF didn't have Include() method accepting lambda overload - only string. We somehow managed to get through. We made our own methods to determine property name based on lambda expression. Everything went well. Then EF embraced lambda parameter for Include() method and everything went even better.

Now that there is nameof in C# and one would think that we can finally forget about lambdas to determine property name. There is more concious, beautiful and performant way of doing this. But wait! EF team didn't implement Include() method overload accepting string! What? How? Why? An irony.

@ajcvickers
Copy link
Contributor

Marking for re-triage. We should consider this for 1.1

@ajcvickers ajcvickers removed this from the Backlog milestone Jul 11, 2016
@rowanmiller rowanmiller modified the milestone: Backlog Jul 15, 2016
@rowanmiller
Copy link
Contributor

In triage we discussed a that using Include/ThenInclude could still work in a repository signature, however here are the issues I found with this:

  • It looks ugly
  • It requires exposing IQueryable in the method signature, which would allow any LINQ operatory to be used (thus defeating part of the purpose of a repository)
  • It requires use of EF specific methods when using a repository (which is likely designed to hide the data access technology)
public Blog GetById(int id, Func<IQueryable<Blog>, IQueryable<Blog>> includeAction)
{
    return includeAction(_context.Blogs).Single(b => b.BlogId == id);
}
var blog = repository.GetById(
    1,
    q => q.Include(b => b.Posts).ThenInclude(p => p.Author)
            .Include(b => b.Owner));

We also talked about the idea of having a custom data structure that the includes are specified in, which is then translated to the EF methods. This gets super messy with generic methods, see the workaround code I've added to the description of this issue, which translates from strings to the Include/ThenInclude methods.

After playing around with this, I am more convinced that a string based API is a really good addition and not just an easy option because it's what we had in EF6. Though one improvement over EF6 would be to allow the hierarchical navigation names to be specified as params, rather than (or perhaps in addition to) a dot separated string, to better support the nameof C# feature (see workaround code for details).

@rowanmiller
Copy link
Contributor

I've added some workaround code to the description of this issue (top of the page) that adds a string-based include method to EF Core. If you're going to use it in a production app, you probably want to look at more caching etc. as it makes extensive use of MakeGenericMethod etc.

@gdoron
Copy link

gdoron commented Jul 18, 2016

Worth mentioning using this feature is risky if the include string is an unvalidated input coming from a user.
A user can cause EF to query the entire DB and cause a DoS with a single request or cause SqlInjection.

@divega divega self-assigned this Aug 2, 2016
@Xipooo
Copy link

Xipooo commented Aug 8, 2016

While recognizing that Core means the team is moving in a different direction, it seems a lot of code we we got used to writing has to be re-learned, and a lot of our old code base is useless now. Your extension code is fine as an answer, but now requires a complete re-write of everything that calls my repository. Not only that, but at least we were able to have lambda includes that gave us intellisense. Now we have to use dot notation in a string which has no intellisense. This seems very backwards and a downgrade from EF6. I know the original request was for a string as a dynamic include, but lambda includes seems germane to the topic.

@TomDroste
Copy link

I needed to use the string based workaround for a generic method. After implementing it, it kept throwing the "$"'{navigationName}' is not a valid property of '{currentType}'"" exception. Apparently, it was triggered because of the way I declared the collection directly with the interface.

public class Example
{
    public Example()
    {
        ExampleCollection= new HashSet<Example>();
    }

    public virtual ICollection<Example> ExampleCollection { get; set; }
}

This resulted in a change in the workaround method. Because of the direct declaration of the models it the old solution only detected the IEnumerable interface and not the ICollection interface

public static class IncludeByStringsExtensions
   {
        private static MethodInfo _include = typeof(EntityFrameworkQueryableExtensions)
                        .GetMethod("Include");

        private static MethodInfo _thenIncludeReference = typeof(EntityFrameworkQueryableExtensions)
            .GetMethods()
            .Where(m => m.Name == "ThenInclude")
            .Single(m => m.Name == "ThenInclude" &&
                         m.GetParameters()
                          .Single(p => p.Name == "source")
                          .ParameterType
                          .GetGenericArguments()[1].Name != typeof(ICollection<>).Name);

        private static MethodInfo _thenIncludeCollection = typeof(EntityFrameworkQueryableExtensions)
            .GetMethods()
            .Where(m => m.Name == "ThenInclude")
            .Single(m => m.Name == "ThenInclude" &&
                         m.GetParameters()
                          .Single(p => p.Name == "source")
                          .ParameterType
                          .GetGenericArguments()[1].Name == typeof(ICollection<>).Name);

        public static IQueryable<T> Include<T>(this IQueryable<T> query, string include)
        {
            return query.Include(include.Split('.'));
        }

        public static IQueryable<T> Include<T>(this IQueryable<T> query, params string[] include)
        {
            var currentType = query.ElementType;
            var previousNavWasCollection = false;

            for (int i = 0; i < include.Length; i++)
            {
                var navigationName = include[i];
                var navigationProperty = currentType.GetProperty(navigationName);
                if (navigationProperty == null)
                {
                    throw new ArgumentException($"'{navigationName}' is not a valid property of '{currentType}'");
                }

                var includeMethod = i == 0
                    ? _include.MakeGenericMethod(query.ElementType, navigationProperty.PropertyType)
                    : previousNavWasCollection
                        ? _thenIncludeCollection.MakeGenericMethod(query.ElementType, currentType, navigationProperty.PropertyType)
                        : _thenIncludeReference.MakeGenericMethod(query.ElementType, currentType, navigationProperty.PropertyType);

                var expressionParameter = Expression.Parameter(currentType);
                var expression = Expression.Lambda(
                    Expression.Property(expressionParameter, navigationName),
                    expressionParameter);

                query = (IQueryable<T>)includeMethod.Invoke(null, new object[] { query, expression });

                var proptype = navigationProperty.PropertyType;
                if (proptype.GetTypeInfo().IsGenericType && proptype.GetGenericTypeDefinition() == typeof(ICollection<>))
                {
                    previousNavWasCollection = true;
                    currentType = navigationProperty.PropertyType.GetGenericArguments().Single();
                }
                else
                {
                    previousNavWasCollection = false;
                    currentType = navigationProperty.PropertyType;
                }
            }

            return query;
        }
    }

@oxozle
Copy link

oxozle commented Aug 31, 2016

I found another solutions. I pass Func to Include:
List<T> Get(Expression<Func<T, bool>> filter = null, Func<IQueryable<T>, IOrderedQueryable<T>> orderBy = null, Func<DbSet<T>, IQueryable<T>> includeAction = null);

ajcvickers added a commit that referenced this issue Sep 1, 2016
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 1, 2016
@rpokrovskij
Copy link

rpokrovskij commented Oct 15, 2016

The problem really is not about "string includes" but about impossibility to create the function that can "dynamically" generate includes from its own parameters, and that means create simple abstract interface functions. Yes, it could be C# problem not EFCore team... but if the only one way to provide highly important practically (e.g. to migrate EF6 IRepository pattern realisation) and theoretically uncurryng operation on your lambdas chain, is do the step back from "strongly typed" to "dynamically typed" in this case, please be brave enough to do this step.

P.S. People do not use EF6 Include(string) directly, people were happy to have theirs own Include(List<string>). And EFCore clearly fails there because it is become impossible to have Include( allIncludesConfigurationAsOneParameter).

@gdoron
Copy link

gdoron commented Oct 15, 2016

@rpokrovskij Maybe you are better off with Mongo or other NoSQL Databases?
EF is a strongly typed ORM, I don't expect it to change.

@rpokrovskij
Copy link

rpokrovskij commented Oct 15, 2016

@gdoron one function that hides some C# limitations (the impossibility to have simple enough uncurred function to setup includes) do not make EF "not strongly typed"... Of course you can push users to use reflections but that shows only that you just do not understand that interface should be complete (there should be the way to unchain/uncurry "EFCore" includes to one call, because, if you are going to more abstraction, "to set up includes" is one operation). Also I'm sure that it is possible to provide uncurred strongly typed function the way as oxozle promote (of course hiding DbSet).

@ajcvickers
Copy link
Contributor

@eqbalsajadi String-based include was shipped in EF Core 1.1. That stackoverflow is out-of-date.

@eqbalsajadi
Copy link

@ajcvickers Thank you

@ggirard07
Copy link

ggirard07 commented Jul 31, 2018

@ajcvickers I don't get how the string support solves the issue of including multi-level include.

When we used to write something like this where my service was in charge to then handling the include:

Task<Order> GetOrderByIdAsync(string orderId, params Expression<Func<Order, object>>[] includeProperties);
...
await m_OrdersService.GetOrderByIdAsync(orderId, o => o.Lines.Select(l => l.Releases));

Is this the new way to go to keep my service in charge of handling the include?

Task<Order> GetOrderByIdAsync(string orderId, params string[] includePaths);
...
await m_OrdersService.GetOrderByIdAsync(orderId, $"{nameof(Order.Lines)}.{nameof(OrderLine.Releases)}")

In that case the include string can break if someone decide to refactor the entities without noticing the issue!
Only solution I see in that case is exposing the IQueryable as @rowanmiller exposed in his post, with the associated concerns...

@ajcvickers
Copy link
Contributor

@ggirard07 Because you can write any API pattern you want, including the one for EF6, and then convert (e.g. parse the lambda expression) to a simple list of concatenated strings before passing to EF.

@ggirard07
Copy link

@ajcvickers Thanks for the information. EF6 was way more user friendly on this by avoiding any required knowledge about Expression.
No one in our team has experience with Expression and we try to avoid those by any way as it is a real pain to debug in that case.

@ajcvickers ajcvickers modified the milestones: 1.1.0-preview1, 1.1.0 Oct 15, 2022
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests