GraphDiff icon indicating copy to clipboard operation
GraphDiff copied to clipboard

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

Open fdahlberg opened this issue 7 years ago • 24 comments

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 avatar Nov 25 '17 13:11 fdahlberg

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

JonathanMagnan avatar Nov 25 '17 17:11 JonathanMagnan

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

cryo75 avatar Nov 25 '17 17:11 cryo75

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

fdahlberg avatar Nov 27 '17 08:11 fdahlberg

How are you mapping the properties to the columns?

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

cryo75 avatar Nov 27 '17 08:11 cryo75

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.

fdahlberg avatar Nov 27 '17 08:11 fdahlberg

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));

cryo75 avatar Nov 27 '17 09:11 cryo75

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.

fdahlberg avatar Nov 27 '17 09:11 fdahlberg

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.

cryo75 avatar Nov 27 '17 09:11 cryo75

Tried adding a property on WorkItem called Owner and changing the fluen API to: modelBuilder.Entity<Person>() .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.

fdahlberg avatar Nov 27 '17 10:11 fdahlberg

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

cryo75 avatar Nov 27 '17 10:11 cryo75

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

GraphDiffTest.zip

fdahlberg avatar Nov 27 '17 10:11 fdahlberg

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

fdahlberg avatar Nov 27 '17 11:11 fdahlberg

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

cryo75 avatar Nov 27 '17 11:11 cryo75

Excellent! Then we're on the same page!

fdahlberg avatar Nov 27 '17 11:11 fdahlberg

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 avatar Dec 18 '17 15:12 JonathanMagnan

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

JonathanMagnan avatar Jan 09 '18 20:01 JonathanMagnan

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

fdahlberg avatar Jan 10 '18 08:01 fdahlberg

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

JonathanMagnan avatar Jan 10 '18 13:01 JonathanMagnan

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

fdahlberg avatar Jan 10 '18 13:01 fdahlberg

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

JonathanMagnan avatar Jan 10 '18 13:01 JonathanMagnan

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?

hprutsman avatar Nov 12 '18 19:11 hprutsman

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

JonathanMagnan avatar Nov 12 '18 21:11 JonathanMagnan

@JonathanMagnan, hi!

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

Bykiev avatar Oct 28 '19 13:10 Bykiev

@JonathanMagnan, is any progress on this?

Bykiev avatar May 11 '22 10:05 Bykiev