dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.53k stars 3.13k forks source link

Discussion on many-to-Many relationships (without CLR class for join table) #1368

Closed 0xdeafcafe closed 1 year ago

0xdeafcafe commented 9 years ago

Note:

With regard to feedback, I think it is worth reiterating some comments made a few months ago. We read and consider all feedback (and try to make the right choices based on it) no matter how it is delivered; that is, whether it is provided politely and with respect or not. Yelling at us or others with different opinions at best makes you feel good and us/them feel bad. It doesn’t achieve anything concrete. We are doing our best to prioritize features and implement them in the best way we can. We really appreciate your feedback and it makes a big difference on shaping the product. We personally appreciate it more when feedback is delivered in a respectful way, but please don’t stop providing constructive feedback.


Original issue:

I'm assuming there isn't a way to do Many-to-Many relationships in EF Core yet, as I've found nothing mentioning how to do it. Are there any examples on getting round this? Or am I just completely missing it, and it's actually there after all.

ajcvickers commented 7 years ago

@Daniel15 It's currently on our backlog and not scheduled for any of the immediate upcoming releases.

fedoranimus commented 7 years ago

That's disappointing @ajcvickers. It would appear to be an issue for many people (including myself). I know it will drastically simplify my code and make it maintainable.

KSubedi commented 7 years ago

I agree with @fedoranimus, I was hoping this feature would be available with the 1.1 release, I am disappointed to hear that its not even scheduled.

vsg24 commented 7 years ago

I can't say that I'm surprised. The whole .NET Core ecosystem is a big mess right now.

fletchsod-developer commented 7 years ago

If you really look at several other bugs, you'll find they're being fixed one by one. We're actually getting there but not fast enough.

johnkwaters commented 7 years ago

I am trying to work with the approach of creating a join entity, and it kind of works... except for this case:

Say you have a many to many from User to Role, so a user can have many roles, a role can be held by many users. I would have User ( Id, Name ), Role (Id, Name) and UserRole (UserId, RoleId).

Inserting a new User is fine, I add a couple of UserRole children, and save.

Now, if I fetch that user to a web app, and in the web app, I add another role, then send it back to my API, when I try to save the User object, I get duplicated roles. The ones that were already there get added again. If I add a unique constraint to (UserId, RoleId), then instead I get a unique constraint violation.

A related problem is if I send the User to a client, it removes a role, and then I save the user with one less role. The dropped role is not actually removed.

So...do I need to go about manipulating EF to understnad these changes to relationships in a better way for the freamework to not create duplicates and understand when 'links' need to be deleted? When I try, I keep ending up with various exceptions to do with the object already being in the context....

What is the correct pattern to use?

Daniel15 commented 7 years ago

when I try to save the User object, I get duplicated roles. The ones that were already there get added again.

The way I handle this is by only adding relationships that don't already exist. Given a set of old roles (currently in the database) and new roles (the roles that have been selected), you need to remove roles that are in old roles but not in new roles, and add roles that are in new roles that are not in old roles. Something like this:

public void SetRoles(User user, IEnumerable<int> roleIds)
{
    var newRoles = new HashSet<int>(roleIds);
    var oldRoles = new HashSet<int>(user.UserRoles.Select(x => x.RoleId));

    // Remove roles that aren't set any more
    user.UserRoles = user.UserRoles.FindAll(x => newRoles.Contains(x.RoleId));

    // Add roles that were newly-added
    foreach (var roleId in newRoles.Where(id => !oldRoles.Contains(id)))
    {
        user.UserRoles.Add(new UserRole
        {
            RoleId = roleId,
            User = user,
        });
    }
    Context.SaveChanges();
}

If you have several many-to-many relationships, you could probably pull it out as an extension method to reuse the logic.

Here's some actual code that I'm using on my blog to save a many-to-many relationship between posts and categories: https://github.com/Daniel15/Website/blob/be62cba1f5b9af64e7dd88620433562f03db9e06/Daniel15.Data/Repositories/EntityFramework/BlogRepository.cs#L368-L386

Entity Framework 6 handles this much better, as it does all of this for you. I really hope EF Core gains proper support for many-to-many relationships in the future.

johnkwaters commented 7 years ago

Thanks Daniel, that approach worked. I have one more wrinkle... the NEW version of the object comes in as a DTO which is mapped through AutoMapper to the OLD version from the DB. So your code goes in the mapping like so:

public class MappingProfile : Profile { ///

/// Constructor /// public MappingProfile() { CreateMap<Product, ProductDTO>().ReverseMap() .ForMember( dest => dest.ListItems, opt => opt.Ignore()) .AfterMap(DoProductListMapping); ... }

    private void DoProductListMapping(ProductDTO src, Product dest)
    {
        var newListItems = new HashSet<int>(src.ListItems.Select( li => li.ListItemId));
        var oldListItems = new HashSet<int>(dest.ListItems.Select(li => li.ListItemId));

        // Remove listitems that are no longer present
        dest.ListItems = dest.ListItems.ToList().FindAll(x => newListItems.Contains(x.ListItemId));

        // Add list items that were newly-added
        foreach (var listItemId in newListItems.Where(id => !oldListItems.Contains(id)))
        {
            dest.ListItems.Add(new ProductListItem
            {
                ListItemId = listItemId,
                Product = dest,
            });
        }
    }
daddydrac commented 7 years ago

.Include(x => x.TableName ) not returning relationships (from principal table to dependent table), or only returning one row of data, FIX HERE:

This was the fix to my issue, add this to Startup.cs public void ConfigureServices(IServiceCollection services) method:services.AddMvc().AddJsonOptions(options => { options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; });

Related Articles on the above:

https://benohead.com/c-circular-reference-detected-serializing-object

Note: If you want to also be able to have your API ready for TCP/IP or MVC/Razor, do not use that and just use the [JsonIgnore] attribute, which is actually a better practice than the above.

http://www.newtonsoft.com/json/help/html/serializationattributes.htm#JsonIgnoreAttribute

Also, in Startup.cs make sure you have this at the top:

using Microsoft.EntityFrameworkCore; 
using Newtonsoft.Json; 
using Project_Name_Here.Models;
Vannevelj commented 7 years ago

Can you shed some light as to why this isn't more of a priority since it's such a fundamental database aspect? I wanted to try EF again after 2 years of not using it and after even just a day I'm already ditching it because the first two models that I wanted to connect in my app require a bloated solution.

sthiakos commented 7 years ago

While considered an essential, there is a workaround and someone could argue that it's an optimization. Remember that once the relationship has some other attribute (e.g. Association Date) it suddenly becomes and entity.

I have a class folder called ManyToMany that contains these classes that will be eliminated when this features is eventually implemented.

These guys have ton of other difficult and truly essential features to implement (ahem...TPT/TPC ;) along with maintaining quality and performance. @bricelam, @ajcvickers, @divega , @rowanmiller - Thanks for all you do.

Zonciu commented 7 years ago

Is there any way to create a relationship by using object id only? For DDD rules, we should not relate aggregate roots with object reference.

There is an example:

    public class User : AggregateRoot<string>
    {
        public string Id { get; private set; }
        public string UserName { get; private set; }
        public ICollection<string> DepartmentRelationships { get; private set; } // Department id collection
    }

    public class Department : AggregateRoot<string>
    {
        public string Id { get; private set; }
        public string DepartmentName { get; private set; }
        public ICollection<string> UserRelationships { get; private set; } // User id collection
    }

Is it possible to implement, or I have to use another relationship class to relate User and Department? The relationship class:


    public class UserDepartmentRelationship
    {
        public string UserId { get; set; }
        public User User { get; set; } // Is required ?

        public string DepartmentId { get; set; }
        public Department Department { get; set; } // Is required ?
    }
ajcvickers commented 7 years ago

@Zonciu You can create a relationship with no navigations. For example:

modelBuilder.Entity<User>()
    .HasMany<UserDepartmentRelationship>()
    .WithOne();

However, if you want to have a many-to-many relationship, then you'll still need the join table. So something like:

public class User : AggregateRoot<string>
{
    public string Id { get; private set; }
    public string UserName { get; private set; }
}

public class Department : AggregateRoot<string>
{
    public string Id { get; private set; }
    public string DepartmentName { get; private set; }
}

public class UserDepartmentRelationship
{
    public string UserId { get; set; }
    public string DepartmentId { get; set; }
}

With configuration:

modelBuilder.Entity<User>()
    .HasMany<UserDepartmentRelationship>()
    .WithOne();

modelBuilder.Entity<Department>()
    .HasMany<UserDepartmentRelationship>()
    .WithOne();
Zonciu commented 7 years ago

@ajcvickers Thank you! This works:

public class User : AggregateRoot<string>
    {
        public string Id { get; private set; }
        public string UserName { get; private set; }

        public ICollection<UserDepartmentRelationship> DepartmentRelationships { get; set; } =
            new List<UserDepartmentRelationship>();
    }

    public class Department : AggregateRoot<string>
    {
        public string Id { get; private set; }
        public string DepartmentName { get; private set; }

        public ICollection<UserDepartmentRelationship> UserRelationships { get; set; } =
            new List<UserDepartmentRelationship>();
    }

    public class UserDepartmentRelationship
    {
        public string UserId { get; set; }
        public string DepartmentId { get; set; }
    }

    public class Context : AbpDbContext
    {
        public Context(DbContextOptions options)
            : base(options)
        { }

        public DbSet<User> User { get; set; }
        public DbSet<Department> Department { get; set; }
        public DbSet<UserDepartmentRelationship> UserDepartmentRelationship { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<User>()
                        .HasMany(u => u.DepartmentRelationships)
                        .WithOne();

            modelBuilder.Entity<User>().HasKey(u => u.Id);

            modelBuilder.Entity<Department>()
                        .HasMany(d => d.UserRelationships)
                        .WithOne();

            modelBuilder.Entity<Department>().HasKey(d => d.Id);

            modelBuilder.Entity<UserDepartmentRelationship>()
                        .HasKey(r => new {r.UserId, r.DepartmentId});
        }
    }
SirGordon commented 7 years ago

How do I vote this up? Roadmap feels like 2.0 will ship still without this much needed feature.

ctolkien commented 7 years ago

@SirGordon this is still on the backlog, it will not be making it into the 2.0 release.

jasoncavett commented 7 years ago

@SirGordon To specifically answer your question, you can vote up this issue via the voting buttons that are on it (see the little +:smiley face: symbol in the upper right of the main post). At the time of writing there are 66 thumbs ups right now.

mahald commented 7 years ago

not making it into 2.0 cry

bassebaba commented 7 years ago

I don't really get how one's supposed to add "Tags" to a "Post" var tag = new Tag (); OK... now what? My post doesnt hold any sort of collection of tags... Only PostTags. Do I manually have to create a PostTag and then add it?

What if I have a collection of Tags? Back in teh days (normal EF), I had an ICollection on my Post and I could just do:

var tags = ctx.Tags.Where(x => x.TagID > 5).ToList();
post.tags = tags;
ctx.SaveChanges;

How do I do this in EF core?

Sorry for my stupid questions but the lack of many-to-many in EF core just messes with my head.

Edit: And then, how to use it? Like this? I.e, the way from post is to first select tags from posttag and then select TagName?

            foreach (var p in posts)
            {
               Console.WriteLine($"{p.PostId} : {p.Title}, { string.Join(",", p.PostTag.Select(t => t.Tag).Select(t => t.TagName))}");
            }
smitpatel commented 7 years ago

To add Tags to post.

Single Tag post.PostTags.Add(new PostTag { Tag = tag }); Multiple tags post.PostTags.AddRange(tags.Select(t => new PostTag { Tag = t }));

To get tags for a post

var tags = post.PostTags.Select(pt => pt.Tag);

bassebaba commented 7 years ago

@smitpatel Ok thanks. So unless this issue gets implemented, there's no "cleaner" solution?

I.e: at the current state the "code" / "application" needs to "know about" , and navigate thru "PostTags". That's really really cluttered in my opinion.

Also, Scaffold-DbContext creates "Post" as this:

    public partial class Post
    {
        public Post()
        {
            PostTag = new HashSet<PostTag>();
        }

        public int PostId { get; set; }
        public string Title { get; set; }

        public virtual ICollection<PostTag> PostTag { get; set; }
    }

ICollection has no "AddRange"...

I tried stuff like this:

        var post= ctx.Post.First();
            var tags = ctx.Tag;
            post.PostTag = tags.Select(x => new PostTag {Tag = x}).ToList();
            ctx.SaveChanges();

But that gives me errors on insert,I guess EF doesn't like that 'Im replacing the PostTag collection.

This does work: tags.ForEach(x => post.PostTag.Add(new PostTag {Tag = x}));

But gives errors if I'm trying to add a duplicate tag, so then I guess I always have to clean out duplicates before adding tags.

So either I'm completely stupid, or there are some gotchas with Many-to-Many support as of now. I guess the best thing for the moment is to place helper-functions on a partial of the Post-class with different operations, e.g "AddTagToPost(Tag t)", "AddRangeOfTagsToPost(...), ReplaceAllTagsWith(..)"

sky-code commented 7 years ago

Ok, so microsoft corporation which is always care about compatibility, can't implement so important for migration feature for 2 years and this feature still on backlog.. what a joke, brand new cloud optimized entity framework in version 2.0 not have basic compatibility with old EF )) Can anyone from EF team make a checklist of features need to be done for this issue and add this to next milestone than implement this for the next release? Really, I don't get it, so many people need this feature just for try new EF on already existing projects but it still in backlog. It's definitely have to be done in next release.

Socolin commented 7 years ago

@sky-code https://weblogs.asp.net/ricardoperes/missing-features-in-entity-framework-core-1 like this ?

sky-code commented 7 years ago

Yep, something like this, list itself is good but more important is priority order of tasks from that list. Modeling must be implemented first, than behaviour features as example at this moment for porting exists model if you use many-to-many you need to rework entire model and usage of it to new style of many-to-many with join entity, and than rework it back when this feature will be implemented and same with complex types

but behaviour features like lazy loading can have much simpler workaround, adding preloading in some cases, some queries can be re-implemented temporary and than turned back

My main point is - that modeling has to be done as the first step that there was something to work with at least

cedriclange commented 7 years ago

my aspnet core project become like a sort of unfinished one because some features cant be done the way we used to, and i think about a workaround for me ...i feel like giving up because dont eanna come back, when those features will be implemented...crazy..

On Jun 20, 2017 20:09, "Igor" notifications@github.com wrote:

Yep, something like this, list itself is good but more important is priority order of tasks from that list. Modeling must be implemented first, than behaviour features as example at this moment for porting exists model if you use many-to-many you need to rework entire model and usage of it to new style of many-to-many with join entity, and than rework it back when this feature will be implemented and same with complex types

but behaviour features like lazy loading can have much simpler workaround, adding preloading in some cases, some queries can be re-implemented temporary and than turned back

My main point is - that modeling has to be done as the first step that there was something to work with at least

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/aspnet/EntityFramework/issues/1368#issuecomment-309841409, or mute the thread https://github.com/notifications/unsubscribe-auth/AVf50pPR5MjE68rkz8dBKUb-wSr9DSPSks5sGArEgaJpZM4DPrA7 .

sky-code commented 7 years ago

I am thinking about rewrite entire project to django only because old asp.net mvc is dead but new asp.net core is not usable and will be same in next several years? I already have existing project but I got stuck with legacy asp.net for unknown amount of time and I can't use new features but need them now and old asp.net doesn't have it. I got in to a dead zone. Only one option remains, rewrite all many-to-many relationship with join entity class and rewrite all complex types to properties with backend field and later re-write all this stuff back to normal.

skendrot commented 7 years ago

As much as it makes your life easier to have M-M support, take this time to start using an ORM like Dapper. Performance is so much better.

slamotte commented 7 years ago

Why not write an extension to EF Core to add this functionality? I don't think idle threats to move to another platform will sway anyone.

Myself, I just accept the limitations of the platform and code on. Nothing is perfect, and no one ever said that EF Core was backwards compatible with legacy code.

Don't get me wrong, this really bothers me too. But not enough to make me switch platforms.

popcatalin81 commented 7 years ago

While I do use EF Core on my newest projects, with great benefits despite some functional limitations,, this is still the main issue preventing me to migrate an older large project to EF core, which I would really like to do.

sethcall commented 7 years ago

@skendrot Do you use a particular SQL Builder with Dapper?

skendrot commented 7 years ago

@sethcall I don't. I just fumble through and search online until I find the SQL that works for what I need.

sven-n commented 7 years ago

I wrote about a possible workaround in my blog post and I think it might be a good solution for some guys here. To sum it up: My application just uses a basic data model (domain model). To use it with the entity framework I create (with the help of T4 templates) inherited classes which include all the join entities and other aspects regarding entity framework core. All this stuff is hidden from the actual application code.

daddydrac commented 7 years ago

To make a relationship (and for added flexibility) make the FK nullable (int? means nullable) . Just create your data models like this, and let EFCore do the rest. Make sure you see my comment above about using the JSON serialization with Reference Loop Handling. You can use .Include() and not .WithMany(). This is a SQL DB performance enhancement as well.

PARENT TABLE:

public class Parent { public Parent() { Dependents = new List<Dependent>(); } [DatabaseGenerated(DatabaseGeneratedOption.Identity)] [Key] public int Id { get; set; } public string ParentName { get; set; } public virtual List<Dependent> Dependents { get; set; } }

DEPENDENT TABLE:

public class Dependent { [DatabaseGenerated(DatabaseGeneratedOption.Identity)] [Key]

    public int Id { get; set; }
    public string DependentName { get; set; }
    public int? ParentsId { get; set; }
    [ForeignKey("ParentsId")]

    public Parent Parents { get; set; }

}

}

alojoshka commented 6 years ago

Is there any approximate time schedule for implementing pure many-to-many support in EF Core (without join table as entity)? we are waiting for this feature to start porting all our projects to Ef Core.

alojoshka commented 6 years ago

I think many-to-many feature was always one of the weak points even of EF6. In NHibernate the implementation was much better - it just worked simply and stable without the need of difficult and critical updates of both connected lists (adding, deleting and modifying connected lists on any side caused needed operations in join table without the need to modify both collections before saving the context). Hopefully the Ef Core will have improved many-to-many scenario.

mhosman commented 6 years ago

+1, please implement Many To Many via shadow state entities!!!!

alojoshka commented 6 years ago

please implement Many To Many via shadow state entities!!!!

for us it means "forget about Ef Core for now". This solution adds a lot of shim to every m-m entity, and it is not implementable for business applications with hundreds of entities. Unfortunately.

popcatalin81 commented 6 years ago

@rowanmiller @bricelam @ajcvickers Now that 2.0 is out, do you have any updates for us regarding this item? A roadmap update would be really helpful.

fletchsod-developer commented 6 years ago

If I were you, I wouldn't keep waiting for this feature (along with 1-to-many, 1-to-1, etc.) to be fixed and move on to thinking outside of the box. Because you will end up waiting 10 years later and still no changes.

We end up creating Sparse Matrix to store fields & values (like pebble pieces) as C# class properties. That is our database tables and we used C# linq objects to write script to manipulate the data ourselves. Since then we had made progress on our own without depending on incomplete EF Core package.

That is how we think outside of the box. You need to do the same as well because it will be a long time wait.

alojoshka commented 6 years ago

It sounds like writing own ORM. What is the task of Ef Core then, if it supports a lot of advanced features on many platforms, but doesn't support out of the box the most basic things - reading and saving data? Many-to-many scenario is not just an abstraction, it's a model that describes business logic, and I can't understand the logic of Ef Core developer's team, when in 2017 I end up with writing my own sql queries or a tricky mess of code and helpers just to support the simplest every-day business scenario.

fletchsod-developer commented 6 years ago

Exactly! We're far ahead on the new "Cloud-based Web Application" now than we were 3 years ago. We now no longer have to deal with frequent JOIN bugs. We noticed EF Core's sql query & returned result aren't the same for every same call in JOIN. So we moved business logic outside of EF Core & instead passed in seperate data back to EF Core tables for CRUD changes individually. We will move the business logic back into EF Core when it works better & support features we need. Frustrating though. We also are having to deal with Angular 4 & TypeScript mess too.

alojoshka commented 6 years ago

It would be fare then to announce it publicly that the Ef Core will be cloud-based only and will never support many-to-many pattern in a classic way, and remove this topic from the list of issues, so developer can decide whether to change the entire philosophy of the database development, or to choose a more appropriate ORM and platform for their future projects. But this topic is still in "hight priority features" for future releases, like a teaser, making us to wait for years now, and you clearly say that we don't need this feature at all and it will not be implemented at all. Very frustrating.

mhosman commented 6 years ago

Something important... Many to many relationships works on EF Core. The only different thing is that you need to create a Class for the join table. This discussion is about removing the need of creating that class.

alojoshka commented 6 years ago

the problem is that this join entity always should be mentioned not only while saving the context, but for fetching data and building projections, and it is extremely difficult to migrate existing business logic to Ef Core, making it implementable only for the new applications, written from scratch. As it was mentioned many time in the discussion above, the lack of the possibility to migrate old applications just because of the delay in implementing this feature make a lot of companies to restrain from starting to use Ef Core generally. Starting to use not tested ORM for new projects is too risky while there is no practical possibility to port the existing applications to Ef Core and to test it in practical existing scenarios.

KhalloufHassan commented 6 years ago

@alojosgka Did they really say they won't do it? I was so hyped for asp core for a week and I watched all mva courses about it! I even convinced my manager to start our new project using it only to find out today this isn't supported and spatial data isn't supported either

I'm not sure we can start the project this way, how hard is it to use ef6 on asp core and move to ef core when the features are ready?

I know this will limit us to iis and windows only but it's not like starting on the old mvc will give us this feature anyways!

Or should I just start the project using mvc 5 amd forget about core, they said it's production ready, I didn't even start working and I found 2 deal braekers :(

bassebaba commented 6 years ago

I'm with @alojoshka here. I use the database first approach since I'm on legacy systems with existing/old databases. Many-to-many gets really messy and I actually wish I would have explored other possibilities than ef core, but when I found out about this it was too late.

@KhalloufHassan please be cautious. "Release" and "production ready" does not mean the same thing for Microsoft as for the rest of us. Migrating a ~40 project solution has been the biggest pain I have experienced, ever in coding. We have a class-file with "workarounds" for all things .net core that "doesnt work" in order to share solutions in the team, and that class is now over 5000 lines long... It must have been hunderds of hours of work to find and workaround all those problems....

I have no hope what so ever that this issue will be fixed, and my number one priority will be to try to replace EF core, but i havent had the time to research if there are any viable options for .net core.

MikeFoden commented 6 years ago

It's now open source, so feel free to contribute to help 'em out guys!

popcatalin81 commented 6 years ago

@MikeFoden this particular feature is unfortunately out of reach of individual contributors as it touches almost all subsystems in non trivial ways: metadata, entity tracking, query translation, reverse engineering and more.

Which means the only chance to see it implemented is if the EF team decides to do it.

leus commented 6 years ago

What @popcatalin81 says, and further more, even if one individual decides to bite the bullet and implement it, chances are it won't be merged because of its scope. (I believe this actually happened.)

alojoshka commented 6 years ago

actually, to solve the problem in a positive way, the only thing is needed and highly awaited from the Ef Core team is to set the approximate date of the release which will implement this feature. Of course each developer team and business has it's own priorities and has the right to decide whether to wait or to look for and alternative solution. I am really tired to plan our next bunch of projects to move to Ef Core in some months, because m-m feature is in the priority list for Ef Core team, and after years not even planned. As I've understood now, this delay has some philosophical reasons as well, not only the lack of time of the developers (which we all suffer from time to time and can easily forgive to others). Now in October we planned the shift our main solution to Ef Core. It has some b2b and b2c hotel booking systems, air company booking and site etc. - the whole solution is almost 10GB(checked right now). The shift was planned 1.5 years before! After long and deep analysis, the only feature which is the absolute stopper is this m-m - our DAL is quite plain, based on the pure POCO entities, but is too huge to change m-m relations to sth like "m-m with join entity". What should I write to our CEO? That in 1.5 years of keeping this feature in hight priority list it is not even planned for next months and it's developers try to convince me that I don't need it at all? Really, together with @bassebaba I put my next biggest priority to find the alternate product, which has its development team able to set its priorities according to the business needs and not only its imaginary icon of developer's needs.