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

Entity deleted when required principal is swapped instead of an exception. #22391

Closed
stephaneDjoumbou opened this issue Sep 3, 2020 · 2 comments · Fixed by #25821
Closed
Labels
area-change-tracking 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

@stephaneDjoumbou
Copy link

We have two entities, cars and persons. Every car belongs to 0 or 1 person and every person owns 0 or 1 car.
So it is a 0..1 to 0..1 relationship between the two entities.

Let's load two cars that have each an owner:

var car1 = context.Cars.Find(c1Guid); 
var car2 = context.Cars.Find(c2Guid); 

Then swap the owners of the two cars:

var otherOwner = car2.Owner;
car2.Owner = car1.Owner;
car1.Owner = otherOwner;
// update owner.vehicle
car1.Owner.Vehicle = car1;
car2.Owner.Vehicle = car2;

and save it back to the database with context.SaveChanges();. This throws the following exception:

System.InvalidOperationException
Unable to save changes because a circular dependency was detected in the data to be saved: 'Car [Modified] <- Index { 'fk_PersonId' } Car [Modified] <- Index { 'fk_PersonId' } Car [Modified]'.
   bei Microsoft.EntityFrameworkCore.Internal.Multigraph`2.ThrowCycle(List`1 cycle, Func`2 formatCycle)
   bei Microsoft.EntityFrameworkCore.Internal.Multigraph`2.BatchingTopologicalSort(Func`2 formatCycle)
   bei Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.<BatchCommands>d__11.MoveNext()
   bei Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   bei Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
   bei Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(DbContext _, Boolean acceptAllChangesOnSuccess)
   bei Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   bei Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   bei DDDTest.TestClass.CreateEntities() in C:\Users\stephane\Desktop\Work\Projects\DemoProjects\C_Sharp\DDDTest\DDDTest\TestClass.cs:Zeile 111.

NOTE: Everything works fine if the relationship is not required (isRequired(false)).

Steps to reproduce

Here is the example code that reproduces the issue:

using System;
using Microsoft.EntityFrameworkCore;
using Xunit;

namespace DDDTest
{
    public class Car
    {
        protected Car() { }

        public Guid Id { get; private set; }
        public virtual Person Owner { get; set; }

        internal Car(Person owner)
        {
            Id = Guid.NewGuid();
            Owner = owner;
        }
    }

    public class Person
    {
        public Guid Id { get; private set; }
        public virtual Car Vehicle { get; set; }

        public Person()
        {
            Id = Guid.NewGuid();
        }

    }

    public class Context : DbContext
    {
        public DbSet<Person> TestPersons { get; set; }
        public DbSet<Car> Cars { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            base.OnConfiguring(optionsBuilder);
            optionsBuilder.UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=MyDatabase;Trusted_Connection=True;");
            optionsBuilder.UseLazyLoadingProxies();
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {

            modelBuilder.Entity<Person>(p =>
            {
                p.HasKey(tp => tp.Id);
            });


            modelBuilder.Entity<Car>(c =>
            {
                c.HasKey(tc => tc.Id);
                c.HasOne(tc => tc.Owner)
                    .WithOne(tp => tp.Vehicle)
                    .HasForeignKey<Car>("fk_PersonId")
                    .IsRequired(false); // Everything works if we put `true` here!
            });
        }

    }

    public class TestClass
    {
        [Fact]
        public void CreateEntities()
        {
            // setup: Create two TestPersons
            Guid p1Guid, p2Guid, c1Guid, c2Guid ;

            using (var context = new Context())
            {
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();

                var c1 = new Car(new Person());
                var c2 = new Car(new Person());
                p1Guid = c1.Owner.Id;
                p2Guid = c2.Owner.Id;

                c1Guid = c1.Id;
                c2Guid = c2.Id;
                context.Cars.Add(c1);
                context.Cars.Add(c2);

                context.SaveChanges();
            }
            using(var context = new Context()) {
                var car1 = context.Cars.Find(c1Guid); 
                var car2 = context.Cars.Find(c2Guid); 

                // assert
                Assert.Equal(car1.Owner.Id, p1Guid);
                Assert.Equal(car2.Owner.Id, p2Guid);
                 
                // swap owners
                var otherOwner = car2.Owner;
                car2.Owner = car1.Owner;
                car1.Owner = otherOwner;
                // update owner.vehicle
                car1.Owner.Vehicle = car1;
                car2.Owner.Vehicle = car2;

                // assert
                Assert.Equal(car1.Owner.Id, p2Guid);
                Assert.Equal(car2.Owner.Id, p1Guid);

                context.SaveChanges();
            }
        }
    }
}

Further technical details

EF Core version: 3.1.7
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .Net 4.7.2
Operating system: Windows 10
IDE: Visual Studio 2019 16.4.4

@AndriySvyryd
Copy link
Member

Since "fk_PersonId" is unique one of the cars needs to have the Owner set to null temporarily and saved, then the correct owners can be set.

This will be done automatically when #1699 is implemented

When you call IsRequired(true) one of the cars is deleted due to a bug.

@AndriySvyryd AndriySvyryd changed the title Circular Reference exception for 0..1 to 0..1 relationship Entity deleted when required principal is swapped instead of an exception. Sep 3, 2020
@ajcvickers ajcvickers self-assigned this Sep 8, 2020
@ajcvickers ajcvickers added this to the Backlog milestone Sep 8, 2020
@ajcvickers ajcvickers removed this from the Backlog milestone Sep 11, 2020
@ajcvickers ajcvickers added this to the 5.0.0-rc2 milestone Sep 11, 2020
@ajcvickers
Copy link
Member

I investigated this and tracked it down to an automatic delete-orphans behavior that happens while working through the re-parenting. The workaround is to use:

context.ChangeTracker.DeleteOrphansTiming = CascadeTiming.OnSaveChanges;

@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking 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.

3 participants