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

Update error when a table has two foreign keys pointing to the same table #168

Open
fdahlberg opened this issue Nov 25, 2017 · 24 comments
Open
Assignees

Comments

@fdahlberg
Copy link

Hi,

I have the following model:

public class Person
    {
        public Person()
        {
            Id = Guid.NewGuid();
            WorkItems = new List<WorkItem>();
        }

        public Guid Id { get; set; }

        public string Name { get; set; }

        public ICollection<WorkItem> WorkItems { get; set; }
    }
public class WorkItem
    {
        public WorkItem()
        {
            Id = Guid.NewGuid();
        }

        public Guid Id { get; set; }

        public string Description { get; set; }

        public Person Creator { get; set; }

    }

My fluent API looks like this:

protected override void OnModelCreating(DbModelBuilder modelBuilder)
        {
            base.OnModelCreating(modelBuilder);


            modelBuilder.Entity<Person>()
                .HasMany(p => p.WorkItems)
                .WithRequired()
                .WillCascadeOnDelete();
        }

When I use Graphdiff to update this small graph, I find that the Creator_Id Column in the WorkItem table is constantly set to the same as Person_id column.
I can't understand why this is happening. Is this a bug?

My graphdiff code to update the graph looks like this:

        public static void UpdatePerson(Person person)
        {
            using (var context = new GraphDiffTestDbContext())
            {
                context.Database.Log = s => System.Diagnostics.Debug.WriteLine(s);

                context.UpdateGraph(person, map => map
                .OwnedCollection(p => p.WorkItems, with => with.AssociatedEntity(p => p.Creator))
                );

                context.SaveChanges();
            }
        }
@fdahlberg fdahlberg changed the title Update error when a table has two foreign keys to the same table Update error when a table has two foreign keys pointing to the same table Nov 25, 2017
@JonathanMagnan JonathanMagnan self-assigned this Nov 25, 2017
@JonathanMagnan
Copy link
Member

Hello @fdahlberg ,

Thank you for reporting, I will try to check this issue tomorrow or during next week.

Best Regards,

Jonathan


Help us to support this library: Donate

@cryo75
Copy link

cryo75 commented Nov 25, 2017

This is not a GraphDiff problem but most probably something wrong with your entity configuration. How does your fluent API look like?

@fdahlberg
Copy link
Author

Hi cryo75,
The fluent api that I'm using is part of the question.

@cryo75
Copy link

cryo75 commented Nov 27, 2017

How are you mapping the properties to the columns?

And where is the Person_id column? In Person on in WorkItem?

@fdahlberg
Copy link
Author

The Person_id column is created automatically by Entity Framework, because I have a naviagation property on Person (WorkItems).
The code above gives me the table structure that I'm after. But it's possible I could achieve this in a different way that would work with GraphDiff.

@cryo75
Copy link

cryo75 commented Nov 27, 2017

So you have Person_Id and Creator_Id in the workitems table? Am I understanding correctly? And why do you want 2 foreign keys pointing to the same table?

I usually do my own mapping. However, what I can see is this:

context.UpdateGraph(person, map => map .OwnedCollection(p => p.WorkItems, with => with.AssociatedEntity(p => p.Creator)) );

Shouldn't that be:

context.UpdateGraph(person, map => map.OwnedCollection(p => p.WorkItems));

@fdahlberg
Copy link
Author

That's correct! In my example (the real situation is much more complicated) I want to say that a person has a number of workitems, but the workitem is also created by a person (differenct from the owner). Therefore, as far as I can understand I need 2 foreign keys pointing to the same table (Person). Would you have solved it differently?

The Graphdiff mapping that you suggest is what I started with. I added the associated entity part as an attempt to solve it, but it didn't change anything.

@cryo75
Copy link

cryo75 commented Nov 27, 2017

This is EF-related. I guess you will need another Person property (Owner) in your WorkItem because Creator will always be the same as Person.

@fdahlberg
Copy link
Author

Tried adding a property on WorkItem called Owner and changing the fluen API to:
modelBuilder.Entity()
.HasMany(p => p.WorkItems)
.WithRequired(w => w.Owner)
.WillCascadeOnDelete();

Same result unfortunately.

Not sure it's EF related since it works fine if I add a Person object in the normal EF way. This is the code that works fine, meaning that only Owner_id is populated:

        public static void AddPerson()
        {

            var john = new Person { Name = "John" };

            var workItem1 = new WorkItem { Description = "Things to do" };

            john.WorkItems.Add(workItem1);


            using (var context = new GraphDiffTestDbContext())
            {
                context.Persons.Add(john);

                context.SaveChanges();
            }

        }

When I run the following graphdiff it doesn't work:

        public static void UpdatePerson()
        {

            var jenny = new Person { Name = "Jenny" };

            var workItem1 = new WorkItem { Description = "Stuff to do" };

            jenny.WorkItems.Add(workItem1);

            using (var context = new GraphDiffTestDbContext())
            {
                context.Database.Log = s => System.Diagnostics.Debug.WriteLine(s);

                context.UpdateGraph(jenny, map => map
                .OwnedCollection(p => p.WorkItems)
                );

                context.SaveChanges();
            }
        }

After having run these two methods the database looks like this:

2017-11-27_11h16_59

So what I can't understand is why Creator_id is populated when I use graphdiff.

@cryo75
Copy link

cryo75 commented Nov 27, 2017

Can you set up a sample project, zip it and attach it here so I can have a look at it.

@fdahlberg
Copy link
Author

This is the project I'm using to test this issue:

GraphDiffTest.zip

@fdahlberg
Copy link
Author

I know that it works fine when you're not using GraphDiff. My problem is when you use GraphDiff...

@cryo75
Copy link

cryo75 commented Nov 27, 2017

I noticed. I'm getting the same Id's when using GraphDiff. Trying to see what the problem is.

@fdahlberg
Copy link
Author

Excellent! Then we're on the same page!

@JonathanMagnan
Copy link
Member

Hello @fdahlberg ,

Sorry for the delay, we forget to add this issue to our task list.

My developer tried your scenario, one problem he noted is that you call base.OnModelCreating(modelBuilder); before configuring your model. If you switch the call to put it at the end, the code works fine.

protected override void OnModelCreating(DbModelBuilder modelBuilder)
{
	modelBuilder.Entity<Person>()
		.HasMany(p => p.WorkItems)
		.WithRequired()
		.WillCascadeOnDelete();
		
	base.OnModelCreating(modelBuilder);
}

Let me know if that once when moving base.OnModelCreating at the end of the method.

Best Regards,

Jonathan


Help us to support this library: Donate

@JonathanMagnan
Copy link
Member

Hello @fdahlberg ,

I will close this issue since it seems to be solved.

Feel free to reopen it if there is something else.

Best Regards,

Jonathan


Help us to support this library: Donate

@fdahlberg
Copy link
Author

Hi Jonathan,

I tried the suggested solution but couldn't see any difference. Acccording to the documentation for OnModelCreating the base implementation does nothing. So how could it matter whether I call it first or last or not at all in the method?

Kind Regards
Fred

@JonathanMagnan
Copy link
Member

Hello @fdahlberg ,

I don't understand either, I just know that if we put this line before it doesn't work, but when we put this line after, it works on our side.

Could you at least try it?

Best Regards,

Jonathan


Help us to support this library: Donate

@fdahlberg
Copy link
Author

Hi Jonathan,

I have tried it. It didn't work for me.
I've found a workaround (changed my db structure) so it's not terribly important for me anymore. But would be nice if you could fix it. I find it hard to believe that moving that line (base.OnModelCreating ) could make any difference though. Would be very interesting to understand how, if that's actaully the case.

Fred

@JonathanMagnan
Copy link
Member

We will investigate again, don't worry ;)

I'm also curious why it started to work on our side magically!

Best Regards,

Jonathan


Help us to support this library: Donate

@hprutsman
Copy link

Hi @JonathanMagnan, was this ever looked at again? I have come across this same problem and am wondering if there will be a fix coming?

@JonathanMagnan
Copy link
Member

Hello @hprutsman ,

To be honest, yes it will be looked again but not in a very short term.

We are currently trying to close all issues related to EF Effort than we might move to this one.

One problem with this request if I remember well is how FK are handled. They are a huge flaw in the logic with this scenario that requires some heavy work on our side. The logic simply doesn't work.

We will need soon to implement GraphDiff for Entity Framework Classic, so it will allow us to look deeper on how we can better do this library but meanwhile, there is nothing planned to fix this.

Best Regards,

Jonathan

@Bykiev
Copy link

Bykiev commented Oct 28, 2019

@JonathanMagnan, hi!

Thank you for this awesome library. I faced with the same problem in EF6, any news when it'll be fixed?

@Bykiev
Copy link

Bykiev commented May 11, 2022

@JonathanMagnan, is any progress on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants