Update Pipeline: Implement cycle breaking for optional relationships

[lukewaters]

The following model currently throws saying it hit a circular dependency

Test 'Microsoft.Data.Entity.SqlServer.FunctionalTests.NavigationTest.Duplicate_entries_are_not_created_for_navigations_E2E' failed:
    System.InvalidOperationException : A circular dependency was detected: 'Person' {'SiblingReverseId'} -> 'Person' {'Id'}, 'Person' {'LoverId'} -> 'Person' {'Id'}.
    Utilities\Multigraph.cs(338,0): at Microsoft.Data.Entity.Utilities.Multigraph`2.BatchingTopologicalSort(Func`2 formatCycle)
    Update\CommandBatchPreparer.cs(108,0): at Microsoft.Data.Entity.Relational.Update.CommandBatchPreparer.TopologicalSort(IEnumerable`1 commands)
    Update\CommandBatchPreparer.cs(50,0): at Microsoft.Data.Entity.Relational.Update.CommandBatchPreparer.<BatchCommands>d__1.MoveNext()
    Update\BatchExecutor.cs(66,0): at Microsoft.Data.Entity.Relational.Update.BatchExecutor.Execute(IEnumerable`1 commandBatches, RelationalConnection connection)
    RelationalDataStore.cs(82,0): at Microsoft.Data.Entity.Relational.RelationalDataStore.SaveChanges(IReadOnlyList`1 entries)
    ChangeTracking\Internal\StateManager.cs(353,0): at Microsoft.Data.Entity.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
    ChangeTracking\Internal\StateManager.cs(308,0): at Microsoft.Data.Entity.ChangeTracking.Internal.StateManager.SaveChanges()
    DbContext.cs(312,0): at Microsoft.Data.Entity.DbContext.SaveChanges()
    NavigationTest.cs(73,0): at Microsoft.Data.Entity.SqlServer.FunctionalTests.NavigationTest.Duplicate_entries_are_not_created_for_navigations_E2E()

        [Fact]
        public void Duplicate_entries_are_not_created_for_navigations_E2E()
        {
            using (var ctx = new GoTContext())
            {
                ctx.Database.EnsureDeleted();
                ctx.Database.EnsureCreated();
                var cersei = new Person { Name = "Cersei" };
                var jamie = new Person { Name = "Jamie" };

                cersei.Lover = jamie;
                cersei.Siblings = new List<Person> { jamie, };

                ctx.People.AddRange(new[] { cersei, jamie, });
                ctx.SaveChanges();
            }
        }

    public class Person
    {
        public int Id { get; set; }
        public string Name { get; set; }

        public List<Person> Siblings { get; set; }
        public Person Lover { get; set; }
        public Person LoverReverse { get; set; }
        public Person SiblingReverse { get; set; }
    }

    public class GoTContext : DbContext
    {
        public DbSet<Person> People { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Person>().HasMany(p => p.Siblings)
                .WithOne(p => p.SiblingReverse).Required(false);
            modelBuilder.Entity<Person>().HasOne(p => p.Lover)
                .WithOne(p => p.LoverReverse).Required(false);
        }

        protected override void OnConfiguring(DbContextOptions options)
        {
            options.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=StateManagerBug;Trusted_Connection=True;MultipleActiveResultSets=true");
        }
    }

[mikary]

Spoke with @ajcvickers and @divega, and it looks like this will require some refactoring of the update pipeline to accomplish. It isn't a feature we supported in EF6 (Circular references required multiple calls to SaveChanges), so I'm putting it back out for triage.

[maumar]

we should make sure not to expose PK values in the exception messages that will be thrown for unbreakable cycles

[AndriySvyryd]

Note to implementor: Also handle unique indexes and allow a custom cycle breaker for non-nullalbe properties and for filtered unique indexes.

[gokhanabatay]

Hi @ajcvickers, We have been converted our banking solution which has 1500 entities from nhibernate to ef7. I think its a major issue our current domain model has this kind of relationship. Do we have any workaround to break cycle for optional/not optional relationships until this implementation is done ?

[gokhanabatay]

@ajcvickers @AndriySvyryd To better understanding;

Current exception: Unable to save changes because a circular dependency was detected in the data to be saved:
'MrcMerchant { 'Guid': 3111111112203472 } [Added] <-
ForeignKeyConstraint { 'merchant_guid': 3111111112203472 } MrcMerchantAddress { 'Guid': 3111111112203474 } [Added] <-
ForeignKeyConstraint { 'default_address_guid': 3111111112203474 } MrcMerchant { 'Guid': 3111111112203472 } [Added]'.

Why circular its the same address instance with the same uniq id. "3111111112203474"

public class MrcMerchant
{

    public long Guid {get;set;}
    public long DefaultAddressGuid {get;set;}
    public MrcMerchantAddress MrcMerchantAddress {get;set;}
    public List<MrcMerchantAddress> MrcMerchantAddresses {get;set;}
}

public class MrcMerchantAddress
{
  public long Guid {get;set;}
  public MrcMerchant MrcMerchant {get;set;}
  public string AddressLine {get;set;}
}

[lucaslorentz]

While this feature is not ready, you can make one side of the relationship optional and override DB context SaveChanges to:

• detect circular dependencies that are also optional and unset the value
• save changes
• restore the values that were unset
• save changes

Here is a basic code that handles simple cases of circular reference:

Source code

        public override int SaveChanges(bool acceptAllChangesOnSuccess)
        {
            var setCircularReferences = PostponeCircularReferences();
            if (setCircularReferences == null)
            {
                return base.SaveChanges(acceptAllChangesOnSuccess);
            }

            var numAffected = 0;
            using (var transaction = Database.CurrentTransaction == null ? Database.BeginTransaction() : null)
            {
                numAffected += base.SaveChanges(true);

                setCircularReferences();

                numAffected += base.SaveChanges(acceptAllChangesOnSuccess);

                if (transaction != null)
                    transaction.Commit();
            }

            return numAffected;
        }

        public override async Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = default)
        {
            var setCircularReferences = PostponeCircularReferences();
            if (setCircularReferences == null)
            {
                return await base.SaveChangesAsync(acceptAllChangesOnSuccess, cancellationToken);
            }

            var numAffected = 0;
            using (var transaction = Database.CurrentTransaction == null ? Database.BeginTransaction() : null)
            {
                numAffected += await base.SaveChangesAsync(true, cancellationToken);

                setCircularReferences();

                numAffected += await base.SaveChangesAsync(acceptAllChangesOnSuccess, cancellationToken);

                if (transaction != null)
                    await transaction.CommitAsync();
            }

            return numAffected;
        }

        private Action PostponeCircularReferences()
        {
            Action setCircularReferences = null;

            foreach (var entry in ChangeTracker.Entries())
            {
                if (entry.State != EntityState.Added)
                    continue;

                foreach (var reference in entry.References)
                {
                    if (!(reference.Metadata is INavigation navigation))
                        continue;

                    if (navigation.ForeignKey.IsRequired)
                        continue;

                    var referenceTarget = reference.TargetEntry;
                    if (referenceTarget == null)
                        continue;

                    var hasCircularReference = referenceTarget.References.Any(targetRef => targetRef.CurrentValue == entry.Entity);
                    if (hasCircularReference)
                    {
                        var oldValue = reference.CurrentValue;
                        reference.CurrentValue = null;
                        setCircularReferences += () =>
                        {
                            reference.CurrentValue = oldValue;
                        };
                    }
                }
            }

            return setCircularReferences;
        }