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

Field-only navigation properties in many-to-many relationships can not be loaded by explicit loading #23717

Closed
anranruye opened this issue Dec 17, 2020 · 7 comments · Fixed by #24548
Labels
area-many-to-many closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@anranruye
Copy link

anranruye commented Dec 17, 2020

EF Core version: EF Core 5.0.1 / EF Core 6.0.0-alpha.1.20616.3(daily build)
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 5
Operating system: Win 7
IDE: Visual Studio 2019 16.8.0

I will use these simple entities to illustrate this issue:

    public class Post
    {
        public int Id { get; set; }

        public string Name { get; set; }

        public List<Tag> Tags { get; set; }
    }

    public class Tag
    {
        public int Id { get; set; }

        public string Name { get; set; }

        //public List<Post> Posts { get; set; }   //This navigation property is removed
    }

    public class PostTag
    {
        public int PostId { get; set; }

        public int TagId { get; set; }

        public Post Post { get; set; }

        public Tag Tag { get; set; }
    }

When configure many to many relationship by:

            modelBuilder.Entity<Tag>().HasMany<Post>()
                .WithMany(x => x.Tags)
                .UsingEntity<PostTag>(
                    j => j.HasOne(x => x.Post).WithMany().HasForeignKey(x => x.PostId),
                    j => j.HasOne(x => x.Tag).WithMany().HasForeignKey(x => x.TagId)
                );

An error is throwed by OnModelCreating method with error message:

System.InvalidOperationException: Unable to set up a many-to-many relationship between the entity types 'Tag' and 'Post' because one of the navigations was not specified. Provide a navigation in the 'HasMany' call in 'OnModelCreating'.

From the error message, we know that both navigations should exist in many to many relationship.

Change the code above to:

            modelBuilder.Entity<Tag>().HasMany<Post>("Posts")
                .WithMany(x => x.Tags)
                .UsingEntity<PostTag>(
                    j => j.HasOne(x => x.Post).WithMany().HasForeignKey(x => x.PostId),
                    j => j.HasOne(x => x.Tag).WithMany().HasForeignKey(x => x.TagId)
                );

Another error is throwed with message:

System.InvalidOperationException: The navigation 'Posts' cannot be added to the entity type 'Tag' because there is no corresponding CLR property on the underlying type and navigations properties cannot be added in shadow state.

From this error message, we know that both navigations should exist and they must be CLR property rather than shadow property.

Change the code above again to :

          modelBuilder.Entity<Post>().HasMany(x => x.Tags)
                .WithMany("Posts")
                .UsingEntity<PostTag>(
                    j => j.HasOne(x => x.Tag).WithMany().HasForeignKey(x => x.TagId),
                    j => j.HasOne(x => x.Post).WithMany().HasForeignKey(x => x.PostId)
                );

This time no error is throwed by OnModelCreating and the model building succeeded. This is not expected.

Next step we can use the DbContext configured by the above code to do some queries.

            var posts = dbContext.Posts.Select(x => new
            {
                x.Id,
                x.Name,
                x.Tags
            }).ToList();

This query with projection succeeded, which looks like a good news.

            var posts = db.Posts.Include(x => x.Tags).ToList();

This query failed with error message:

System.ArgumentException: Method 'Boolean Add(System.Object, System.Object, Boolean)' declared on type 'Microsoft.EntityFrameworkCore.Metadata.IClrCollectionAccessor' cannot be called with instance of type 'System.Object'
   at System.Linq.Expressions.Expression.ValidateCallInstanceType(Type instanceType, MethodInfo method)
   at System.Linq.Expressions.Expression.ValidateMethodAndGetParameters(Expression instance, MethodInfo method)
   at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, Expression arg0, Expression arg1, Expression arg2)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.AddToCollectionNavigation(ParameterExpression entity, ParameterExpression relatedEntity, INavigationBase navigation)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.GenerateFixup(Type entityType, Type relatedEntityType, INavigationBase navigation, INavigationBase inverseNavigation)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ProcessShaper(Expression shaperExpression, RelationalCommandCache& relationalCommandCache, LambdaExpression& relatedDataLoaders)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitShapedQuery(ShapedQueryExpression shapedQueryExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator()
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.IncludableQueryable`2.GetEnumerator()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
            var posts = db.Posts.ToList();
            foreach (var post in posts)
            {
                db.Entry(post).Collection(x => x.Tags).Load();
            }

The first query succeeded, but the explicit load failed. Error message:

System.ArgumentNullException: Value cannot be null. (Parameter 'member')
   at System.Linq.Expressions.Expression.MakeMemberAccess(Expression expression, MemberInfo member)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.BuildIncludeLambda(ISkipNavigation skipNavigation, IReadOnlyList`1 keyProperties, ValueBuffer keyValues)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.Query(DbContext context, Object[] keyValues)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.Load(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.CollectionEntry.Load()

The error messages of the two failed queries is not clear enough to point what is wrong. We can know that there seems to be something wrong with many to many relationship, but the first successful query will make things complex and really confusing.

I made two failed model configuring before the third one in this post. But consider that someone only use my third approach and only make query with projection rather than include(), then he can hardly notice the wrong configuration and take it as right
usage. When he use explicit load, he might be suddenly shocked.

I don't know whether this issue can be called a bug or not. But it do make trouble to me to research what is wrong. I do know that optional navigation property will be introduced by ef core 6, and this issue will not exist that time. However, before that time, a more clear tip should be made in somewhere to tell people what is the actual problem when they are tripped by the confusing error.

2020-12-20 Update:
After I change the missing navigation property to a private field, query with eager loading can work, query with explicit loading still can not work.
Look at #23717 (comment)

@anranruye anranruye changed the title Call CollectionNavigationBuilder<TEntity, TRelatedEntity>.WithMany(string navigationName) with an not existed clr property name should throw an error but not, which results in confusing errors when query a navigation collection Call CollectionNavigationBuilder<TEntity, TRelatedEntity>.WithMany(string navigationName) with a not existed clr property name should throw an error but not, which results in confusing errors when query a navigation collection Dec 17, 2020
@ajcvickers ajcvickers added this to the 6.0.0 milestone Dec 18, 2020
@ajcvickers
Copy link
Member

Note, support for unidirectional many-to-many relationships is tracked by #3864. Make sure to vote for that issue.

@anranruye
Copy link
Author

anranruye commented Dec 19, 2020

@ajcvickers

Before raise this issue, I searched the existed issues to avoid to raise duplicate issue. Thus I noticed issue 3864 and learned that this work is in progress. However, I will not vote for it. It is not necessary for me to hide one of the two navigation properties in this moment. I raise this issue to help others to avoid detours.

This issue is added to the 6.0.0 milestone, so does issue 3864 now. Does this mean that the two issues will be solved together in a future version after 6.0.0? If it does, I think this is not what I want. My suggestion is that the right configuration should be explained as soon as possible in documentation or method description, whether the behavior of ef core is to be changed or not. From this aspect, this issue is about document rather than about a bug. If you choose to throw an exception in the WithMany() and explain it by exception message, it's also ok.

@anranruye
Copy link
Author

anranruye commented Dec 20, 2020

@ajcvickers @AndriySvyryd
It seems that I mix up shadow state navigation property and field-only navigation property. Navigation properties
cannot be added as shadow state propertes in all kinds of relationships (not only in many-to-many relationships), they can be added as field-only properties. There is no need to point that shadow properties should not be used for many-to-many relationship in the documentation.

After I change the missing navigation property to a field, query with eager loading can work, query with explicit loading still can not work.

New Tag class:

    public class Tag
    {
        public int Id { get; set; }

        public string Name { get; set; }

        //public List<Post> Posts { get; set; }

        private List<Post> Posts;

        public List<Post> GetPosts() => Posts;
    }

And the fluent api configuration:

    modelBuilder.Entity<Post>().HasMany(x => x.Tags)
                .WithMany("Posts")
                .UsingEntity<PostTag>(
                    j => j.HasOne(x => x.Tag).WithMany().HasForeignKey(x => x.TagId),
                    j => j.HasOne(x => x.Post).WithMany().HasForeignKey(x => x.PostId)
                );

or

    modelBuilder.Entity<Tag>().HasMany<Post>("Posts")
                .WithMany(x => x.Tags)
                .UsingEntity<PostTag>(
                    j => j.HasOne(x => x.Post).WithMany().HasForeignKey(x => x.PostId),
                    j => j.HasOne(x => x.Tag).WithMany().HasForeignKey(x => x.TagId)
                );

Exception message when perform explicit loading:

System.ArgumentNullException: Value cannot be null. (Parameter 'member')
   at System.Linq.Expressions.Expression.MakeMemberAccess(Expression expression, MemberInfo member)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.BuildIncludeLambda(ISkipNavigation skipNavigation, IReadOnlyList`1 keyProperties, ValueBuffer keyValues)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.Query(DbContext context, Object[] keyValues)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.Load(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.CollectionEntry.Load()

I think this is not expected. I tried field-only navigation property(access by public method) with one-to-many relationship, the field-only navigation properties are loaded by explicit loading successfully.

Also, I made three more queries from the other end of the many-to-many relationship (the Tag end), explicit loading failed, eager loading and projection succeeded.

    // succeeded
    var tags = db.Tags.Select(x => new
            {
                x.Id,
                x.Name,
                Posts = EF.Property<List<Post>>(x, "Posts") 
            }).ToList();

    // succeeded
    var tags = db.Tags.Include("Posts").ToList();
        
    //failed
    var tags = db.Tags.ToList();
    foreach (var tag in tags)
    {
        db.Entry(tag).Collection("Posts").Load();
    }

Exception message when perform explicit loading(notice that this one is different from above):

System.ArgumentNullException: Value cannot be null. (Parameter 'member')
   at System.Linq.Expressions.Expression.MakeMemberAccess(Expression expression, MemberInfo member)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.BuildSelectManyLambda(INavigationBase navigation)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.Query(DbContext context, Object[] keyValues)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.Load(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.CollectionEntry.Load()

@anranruye
Copy link
Author

anranruye commented Dec 20, 2020

@ajcvickers

I find original issue of this post is duplicate with #23362, I omit this one when I searched the existed issues. Although I mentioned more infomation in this post, issue 23362 is raised earlier.

But what about field-only navigation property? Field-only property and shadow property are two different things, right? Maybe we can use this post to trace the new issue.

If the issue of field-only property can be fixed easier and earlier than #3864, then some people can use field-only property as a workaround before #3864 is solved.

@anranruye anranruye changed the title Call CollectionNavigationBuilder<TEntity, TRelatedEntity>.WithMany(string navigationName) with a not existed clr property name should throw an error but not, which results in confusing errors when query a navigation collection Field-only navigation properties in many-to-many relationships can not be loaded by explicit loading Dec 22, 2020
@ajcvickers ajcvickers removed this from the 6.0.0 milestone Dec 22, 2020
@ajcvickers
Copy link
Member

Confirmed bug in the loader.

@anranruye The workaround is to use a private property instead of a private field.

@ajcvickers ajcvickers assigned ajcvickers and unassigned AndriySvyryd Jan 5, 2021
@ajcvickers ajcvickers added this to the 6.0.0 milestone Jan 5, 2021
@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 Mar 30, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview4 Mar 30, 2021
@royberris
Copy link

@ajcvickers will this still work with the workaround after the fix?

The workaround is to use a private property instead of a private field.

@ajcvickers
Copy link
Member

@royberris Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-many-to-many closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants