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

Unhandled Exception: System.InvalidOperationException: Unable to save changes because a circular dependency was detected in the data to be saved #11888

Closed
bugproof opened this issue May 3, 2018 · 10 comments

Comments

@bugproof
Copy link

bugproof commented May 3, 2018

I'm attempting to do this:

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

    public string Producer { get; set; }
    public string Model { get; set; }
    public int Year { get; set; }

    public int? OwnerId { get; set; }

    [ForeignKey("OwnerId")]
    public Person Owner { get; set; }
}

// Can have many cars but one active car
class Person
{
    public int Id { get; set; }

    public string Name { get; set; }
    public string Surname { get; set; }

    [ForeignKey("ActiveCarId")]
    public Car ActiveCar { get; set; }
    [InverseProperty("Owner")]
    public ICollection<Car> OwnedCars { get; set; }
}

//Usage

using (var dbContext = new AppDbContext())
{
    var person = new Person()
    {
        Name = "Captain",
        Surname = "Morgan",
        ActiveCar = new Car()
        {
            Producer = "Super Cars",
            Model = "XXX",
            Year = 2018
        }
    };

    person.ActiveCar.Owner = person;

    dbContext.People.Add(person);
    int numAffected = dbContext.SaveChanges();
    if (numAffected == 0)
    {
        Console.WriteLine("Something went wrong during saving");
    }

}

Results in:

Unhandled Exception: System.InvalidOperationException: Unable to save changes because a circular dependency was detected in the data to be saved: 'Person [Added] <- Owner { 'OwnerId' } Car [Added] <- ActiveCar { 'ActiveCarId' } Person [Added]'.

Is this possible somehow?

Expected result:

image
image

@smitpatel
Copy link
Member

Save entities in multiple SaveChanges.

@bugproof
Copy link
Author

bugproof commented May 3, 2018

Thanks @smitpatel it worked. Is there any other possibly better way to workaround this?

using (var dbContext = new AppDbContext())
{
    var person = new Person()
    {
        Name = "Captain",
        Surname = "Morgan"
    };

    dbContext.People.Add(person);
    dbContext.SaveChanges();


    var car = new Car()
    {
        Producer = "Super Cars",
        Model = "XXX",
        Year = 2018,
        Owner = person
    };

    person.ActiveCar = car;

    dbContext.SaveChanges();
}

I'm thinking of a way to automate this... so far came up with this:

public override int SaveChanges(bool acceptAllChangesOnSuccess)
{
    foreach (var entry in ChangeTracker.Entries())
    {
        foreach (var @ref in entry.References)
        {
            var targetEntry = @ref.TargetEntry;
            if (targetEntry == null)
            {
                continue;
            }

            if (targetEntry.State == EntityState.Added)
            {
                // Check if there are circular references, and if so, update them
                var circularReference = targetEntry.References.SingleOrDefault(r => r.Metadata.ForeignKey.PrincipalEntityType.ClrType == entry.Metadata.ClrType);
                if (circularReference != null)
                {
                    // We can't set circular reference with 1 SaveChanges call 
                    var value = circularReference.CurrentValue;
                    circularReference.CurrentValue = null;
                    base.SaveChanges(acceptAllChangesOnSuccess);

                    circularReference.CurrentValue = value;
                }
            }
        }
    }
    
    return base.SaveChanges(acceptAllChangesOnSuccess);
}

@smitpatel
Copy link
Member

There is cyclic dependency here. When you add both the entities, both of them have temporary values. You need to figure out value of Owner.Id to save OwnerId & same way Car.Id to save ActiveCarId. So someone needs to break the tie. Save one of them first, use the value to save another and then update the first one. In this case tie breaking is easy to understand but it could get difficult in complex scenarios.
So the easiest solution for user is to do the tie breaking by calling SaveChanges multiple times.

@bugproof bugproof closed this as completed May 3, 2018
@bugproof
Copy link
Author

bugproof commented May 3, 2018

@smitpatel Thank you very much. I think that's solved for now, would be nice if it was somehow magically resolved by EF core out of the box. It seems like I will need to manually control transaction in my case:

https://docs.microsoft.com/en-us/ef/core/saving/transactions

So two calls to SaveChanges are 1 transaction.

@ajcvickers
Copy link
Member

Duplicate of #1699

@ajcvickers ajcvickers marked this as a duplicate of #1699 May 3, 2018
@bugproof
Copy link
Author

bugproof commented May 4, 2018

@ajcvickers Yeah that's what I'm trying to accomplish with overrides right now - detect circular dependencies, call SaveChanges again under the hood. Could you suggest any better way than overriding SaveChanges to do this with one call to SaveChanges? Where can I find this update pipeline? That's what I did:

public override int SaveChanges(bool acceptAllChangesOnSuccess)
{
    var pendingReferenceUpdates = new Dictionary<ReferenceEntry, object>();

    foreach (var entry in ChangeTracker.Entries())
    {
        // Process references to allow circular references in one call to SaveChanges
        foreach (var reference in entry.References)
        {
            var referenceTarget = reference.TargetEntry;
            if (referenceTarget == null)
            {
                continue;
            }

            // reference target entry state is added but it's not in the database yet, that means it must be updated later with a second SaveChanges call
            if (referenceTarget.State == EntityState.Added && reference.CurrentValue != null)
            {
                pendingReferenceUpdates[reference] = reference.CurrentValue;
                reference.CurrentValue = null;
            }

            foreach (var circularReference in referenceTarget.References.Where(targetRef => referenceTarget.State == EntityState.Added && targetRef.Metadata.ClrType == entry.Metadata.ClrType && targetRef.CurrentValue != null))
            {
                if (circularReference.CurrentValue != null)
                {
                    pendingReferenceUpdates[circularReference] = circularReference.CurrentValue;
                    circularReference.CurrentValue = null;
                }
            }
        }
    }
    
    if (pendingReferenceUpdates.Count == 0)
    {
        return base.SaveChanges(acceptAllChangesOnSuccess);
    }

    int numAffected = 0;
    using (var transaction = Database.BeginTransaction())
    {
        numAffected = base.SaveChanges(true);

        // Apply pending changes now
        foreach (var kv in pendingReferenceUpdates)
        {
            kv.Key.CurrentValue = kv.Value;
        }

        numAffected += base.SaveChanges(acceptAllChangesOnSuccess);
        transaction.Commit();
    }

    return numAffected;
}

@ajcvickers
Copy link
Member

@enemyofthedawn Not suggesting anything different, just marking this issue as a duplicate of an existing issue.

@dharmaturtle
Copy link

Unfortunately calling .SaveChanges() multiple times won't work if the relationship is required, e.g. OwnerId is not nullable. Are there any other possible fixes?

@pha3z
Copy link

pha3z commented Nov 22, 2020

Google brought me here. And the proposed solutions aren't always desirable, so I thought I would point out another option that worked well for my case.

Caveat 1) This won't work if you are doing code-first (which I highly recommend NEVER doing anyway. The drawbacks of code-first typically outweigh the benefits in anything except the most simplistic project).
Caveat 2) This will only work if you have built your own entities manually -or- you are micro-tweaking them (instead of depending strictly on EF automatic scaffolds), AND if you can accept dropping one of the navigation properties that's causing the circular dependency. As an aside, if you always relied on scaffolded or code-first, try the traditional ORM approach: build the schema with SQL, build the EF models to match. Its more manual labor, but the quality pays off with fewer quirks you have to work-around and a more maintainable solution. You're also going to have a far better database since you will be forced to build the schema first and then basically double-check your work when you build the EF model. You will catch problems this way.

First: If you have a foreign key in the database causing circular dependency set it to DEFERRABLE INITIALLY DEFERRED (Postgres terminology, but SqlServer should also have this capability). This will make it so the database will allow missing foreign key until the transaction is complete.

Second: Drop the navigational property that corresponds to the Deferred FK in your database.

Now the database maintains integrity, you can push data into it, and the awkward circular problem on your model is removed, even though its still allowed in the database. Even though the circular reference is gone from your model, the database is still going to stop you from pushing incomplete data to it.

Clearly this is inconvenient for navigation, but if you've got a circular model to begin with, then you can use an alternate route to navigate the model or provide other mechanisms.

If this isn't an option, then you'll have to do the multiple SaveChanges invocations as other suggested.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
@harry-hathorn
Copy link

I have a similar issue with a table that has a unique index that covers 3 columns and it can have a relationship with another item in the same table, so I can't SaveChanges beforehand because that will try to create non-unique records.

I have decided to use GUID instead of integer here for the primary key and I just created the Guid with Guid.NewGuid();

that works for me here.

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

No branches or pull requests

6 participants