Avoid cascade cycles on SQL Server for derived-type referencing many-to-many

src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs

@@ -13,7 +13,9 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
 /// <see href="https://aka.ms/efcore-docs-sqlserver">Accessing SQL Server and SQL Azure databases with EF Core</see>
 /// for more information and examples.
 /// </remarks>
-public class SqlServerOnDeleteConvention : CascadeDeleteConvention, ISkipNavigationForeignKeyChangedConvention
+public class SqlServerOnDeleteConvention : CascadeDeleteConvention,
+ ISkipNavigationForeignKeyChangedConvention,
+ IEntityTypeAnnotationChangedConvention
 {
  /// <summary>
  /// Creates a new instance of <see cref="SqlServerOnDeleteConvention" />.
@@ -60,23 +62,77 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey
  return DeleteBehavior.ClientCascade;
  }
 
- var selfReferencingSkipNavigation = foreignKey.GetReferencingSkipNavigations()
- .FirstOrDefault(s => s.Inverse != null && s.TargetEntityType == s.DeclaringEntityType);
- if (selfReferencingSkipNavigation == null)
+ return ProcessSkipNavigations(foreignKey.GetReferencingSkipNavigations()) ?? deleteBehavior;
+ }
+
+ private DeleteBehavior? ProcessSkipNavigations(IEnumerable<IConventionSkipNavigation> skipNavigations)
+ {
+ var skipNavigation = skipNavigations
+ .FirstOrDefault(
+ s => s.Inverse != null
+ && IsMappedToSameTable(s.DeclaringEntityType, s.TargetEntityType));
+
+ if (skipNavigation != null)
  {
- return deleteBehavior;
+ var isFirstSkipNavigation = IsFirstSkipNavigation(skipNavigation);
+ if (!isFirstSkipNavigation)
+ {
+ skipNavigation = skipNavigation.Inverse!;
+ }
+
+ var inverseSkipNavigation = skipNavigation.Inverse!;
+
+ var deleteBehavior = DefaultDeleteBehavior(skipNavigation);
+ var inverseDeleteBehavior = DefaultDeleteBehavior(inverseSkipNavigation);
+
+ if (deleteBehavior == DeleteBehavior.Cascade
+ && inverseDeleteBehavior == DeleteBehavior.Cascade
+ && !(inverseSkipNavigation.ForeignKey!.GetDeleteBehaviorConfigurationSource() == ConfigurationSource.Explicit
+ && inverseSkipNavigation.ForeignKey!.DeleteBehavior != DeleteBehavior.Cascade))
+ {
+ deleteBehavior = DeleteBehavior.ClientCascade;
+ }
+
+ skipNavigation.ForeignKey!.Builder.OnDelete(deleteBehavior);
+ inverseSkipNavigation.ForeignKey!.Builder.OnDelete(inverseDeleteBehavior);
+
+ return isFirstSkipNavigation ? deleteBehavior : inverseDeleteBehavior;
  }
 
- if (selfReferencingSkipNavigation
- == selfReferencingSkipNavigation.DeclaringEntityType.GetDeclaredSkipNavigations()
- .First(s => s == selfReferencingSkipNavigation || s == selfReferencingSkipNavigation.Inverse)
- && selfReferencingSkipNavigation != selfReferencingSkipNavigation.Inverse)
+ return null;
+
+ DeleteBehavior DefaultDeleteBehavior(IConventionSkipNavigation conventionSkipNavigation)
+ => conventionSkipNavigation.ForeignKey!.IsRequired ? DeleteBehavior.Cascade : DeleteBehavior.ClientSetNull;
+
+ bool IsMappedToSameTable(IConventionEntityType entityType1, IConventionEntityType entityType2)
  {
- selfReferencingSkipNavigation.Inverse!.ForeignKey?.Builder.OnDelete(
- GetTargetDeleteBehavior(selfReferencingSkipNavigation.Inverse.ForeignKey));
- return DeleteBehavior.ClientCascade;
+ var tableName1 = entityType1.GetTableName();
+ var tableName2 = entityType2.GetTableName();
+
+ return tableName1 != null
+ && tableName2 != null
+ && tableName1 == tableName2
+ && entityType1.GetSchema() == entityType2.GetSchema();
  }
 
- return deleteBehavior;
+ bool IsFirstSkipNavigation(IConventionSkipNavigation navigation)
+ => navigation.DeclaringEntityType != navigation.TargetEntityType
+ ? string.Compare(navigation.DeclaringEntityType.Name, navigation.TargetEntityType.Name, StringComparison.Ordinal) < 0
+ : string.Compare(navigation.Name, navigation.Inverse!.Name, StringComparison.Ordinal) < 0;
+ }
+
+ /// <inheritdoc />
+ public virtual void ProcessEntityTypeAnnotationChanged(
+ IConventionEntityTypeBuilder entityTypeBuilder,
+ string name,
+ IConventionAnnotation? annotation,
+ IConventionAnnotation? oldAnnotation,
+ IConventionContext<IConventionAnnotation> context)
+ {
+ if (name == RelationalAnnotationNames.TableName
+ || name == RelationalAnnotationNames.Schema)
+ {
+ ProcessSkipNavigations(entityTypeBuilder.Metadata.GetDeclaredSkipNavigations());
+ }
  }
 }

test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs

@@ -1566,7 +1566,7 @@ public static RuntimeForeignKey CreateForeignKey2(RuntimeEntityType declaringEnt
  var runtimeForeignKey = declaringEntityType.AddForeignKey(new[] { declaringEntityType.FindProperty(""PrincipalsId"")!, declaringEntityType.FindProperty(""PrincipalsAlternateId"")! },
  principalEntityType.FindKey(new[] { principalEntityType.FindProperty(""Id"")!, principalEntityType.FindProperty(""AlternateId"")! })!,
  principalEntityType,
- deleteBehavior: DeleteBehavior.Cascade,
+ deleteBehavior: DeleteBehavior.ClientCascade,
  required: true);
 
  return runtimeForeignKey;

test/EFCore.Relational.Specification.Tests/ManyToManyTrackingRelationalTestBase.cs

@@ -0,0 +1,64 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel;
+
+namespace Microsoft.EntityFrameworkCore;
+
+public abstract class ManyToManyTrackingRelationalTestBase<TFixture> : ManyToManyTrackingTestBase<TFixture>
+ where TFixture : ManyToManyTrackingRelationalTestBase<TFixture>.ManyToManyTrackingRelationalFixture
+{
+ protected ManyToManyTrackingRelationalTestBase(TFixture fixture)
+ : base(fixture)
+ {
+ }
+
+ [ConditionalFact]
+ public void Many_to_many_delete_behaviors_are_set()
+ {
+ using var context = CreateContext();
+ var model = context.Model;
+
+ var navigations = model.GetEntityTypes().SelectMany(e => e.GetDeclaredSkipNavigations())
+ .Where(e => e.ForeignKey.DeleteBehavior != DeleteBehavior.Cascade).ToList();
+
+ var builder = new StringBuilder();
+ foreach (var navigation in navigations)
+ {
+ builder.AppendLine($"{{ \"{navigation.DeclaringEntityType.ShortName()}.{navigation.Name}\", DeleteBehavior.ClientCascade }},");
+ }
+
+ var x = builder.ToString();
+
+ foreach (var skipNavigation in model.GetEntityTypes().SelectMany(e => e.GetSkipNavigations()))
+ {
+ Assert.Equal(
+ CustomDeleteBehaviors.TryGetValue(
+ $"{skipNavigation.DeclaringEntityType.ShortName()}.{skipNavigation.Name}", out var deleteBehavior)
+ ? deleteBehavior
+ : DeleteBehavior.Cascade,
+ skipNavigation.ForeignKey.DeleteBehavior);
+ }
+ }
+
+ protected virtual Dictionary<string, DeleteBehavior> CustomDeleteBehaviors { get; } = new();
+
+ protected override void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
+ => facade.UseTransaction(transaction.GetDbTransaction());
+
+ public abstract class ManyToManyTrackingRelationalFixture : ManyToManyTrackingFixtureBase
+ {
+ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+ {
+ base.OnModelCreating(modelBuilder, context);
+
+ modelBuilder.Entity<EntityTableSharing1>().ToTable("TableSharing");
+ modelBuilder.Entity<EntityTableSharing2>(
+ b =>
+ {
+ b.HasOne<EntityTableSharing1>().WithOne().HasForeignKey<EntityTableSharing2>(e => e.Id);
+ b.ToTable("TableSharing");
+ });
+ }
+ }
+}

test/EFCore.Relational.Specification.Tests/Query/ManyToManyQueryRelationalFixture.cs

@@ -1,10 +1,25 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel;
+
 namespace Microsoft.EntityFrameworkCore.Query;
 
 public abstract class ManyToManyQueryRelationalFixture : ManyToManyQueryFixtureBase
 {
  public TestSqlLoggerFactory TestSqlLoggerFactory
  => (TestSqlLoggerFactory)ListLoggerFactory;
+
+ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+ {
+ base.OnModelCreating(modelBuilder, context);
+
+ modelBuilder.Entity<EntityTableSharing1>().ToTable("TableSharing");
+ modelBuilder.Entity<EntityTableSharing2>(
+ b =>
+ {
+ b.HasOne<EntityTableSharing1>().WithOne().HasForeignKey<EntityTableSharing2>(e => e.Id);
+ b.ToTable("TableSharing");
+ });
+ }
 }

test/EFCore.Relational.Specification.Tests/Query/TPCManyToManyQueryRelationalFixture.cs

@@ -23,6 +23,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
  modelBuilder.Entity<EntityRoot>().ToTable("Roots");
  modelBuilder.Entity<EntityBranch>().ToTable("Branches");
  modelBuilder.Entity<EntityLeaf>().ToTable("Leaves");
+ modelBuilder.Entity<EntityBranch2>().ToTable("Branch2s");
+ modelBuilder.Entity<EntityLeaf2>().ToTable("Leaf2s");
 
  modelBuilder.Entity<UnidirectionalEntityRoot>().UseTpcMappingStrategy();
  modelBuilder.Entity<UnidirectionalEntityRoot>().ToTable("UnidirectionalRoots");

test/EFCore.Relational.Specification.Tests/Query/TPTManyToManyQueryRelationalFixture.cs

@@ -17,6 +17,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
  modelBuilder.Entity<EntityRoot>().ToTable("Roots");
  modelBuilder.Entity<EntityBranch>().ToTable("Branches");
  modelBuilder.Entity<EntityLeaf>().ToTable("Leaves");
+ modelBuilder.Entity<EntityBranch2>().ToTable("Branch2s");
+ modelBuilder.Entity<EntityLeaf2>().ToTable("Leaf2s");
 
  modelBuilder.Entity<UnidirectionalEntityRoot>().ToTable("UnidirectionalRoots");
  modelBuilder.Entity<UnidirectionalEntityBranch>().ToTable("UnidirectionalBranches");

test/EFCore.Specification.Tests/Query/ManyToManyFieldsQueryFixtureBase.cs

@@ -269,6 +269,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
  .HasMany(e => e.TwoSkipShared)
  .WithMany(e => e.OneSkipShared);
 
+ modelBuilder.Entity<EntityRoot>()
+ .HasMany(e => e.BranchSkipShared)
+ .WithMany(e => e.RootSkipShared);
+
  // Nav:2 Payload:No Join:Concrete Extra:None
  modelBuilder.Entity<EntityOne>()
  .HasMany(e => e.TwoSkip)

test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs

@@ -40,6 +40,10 @@ public ISetSource GetExpectedData()
  { typeof(EntityRoot), e => ((EntityRoot)e)?.Id },
  { typeof(EntityBranch), e => ((EntityBranch)e)?.Id },
  { typeof(EntityLeaf), e => ((EntityLeaf)e)?.Id },
+ { typeof(EntityBranch2), e => ((EntityBranch2)e)?.Id },
+ { typeof(EntityLeaf2), e => ((EntityLeaf2)e)?.Id },
+ { typeof(EntityTableSharing1), e => ((EntityTableSharing1)e)?.Id },
+ { typeof(EntityTableSharing2), e => ((EntityTableSharing2)e)?.Id },
  { typeof(UnidirectionalEntityOne), e => ((UnidirectionalEntityOne)e)?.Id },
  { typeof(UnidirectionalEntityTwo), e => ((UnidirectionalEntityTwo)e)?.Id },
  { typeof(UnidirectionalEntityThree), e => ((UnidirectionalEntityThree)e)?.Id },
@@ -165,6 +169,69 @@ public ISetSource GetExpectedData()
  }
  }
  },
+ {
+ typeof(EntityBranch2), (e, a) =>
+ {
+ Assert.Equal(e == null, a == null);
+
+ if (a != null)
+ {
+ var ee = (EntityBranch2)e;
+ var aa = (EntityBranch2)a;
+
+ Assert.Equal(ee.Id, aa.Id);
+ Assert.Equal(ee.Name, aa.Name);
+ Assert.Equal(ee.Slumber, aa.Slumber);
+ }
+ }
+ },
+ {
+ typeof(EntityLeaf2), (e, a) =>
+ {
+ Assert.Equal(e == null, a == null);
+
+ if (a != null)
+ {
+ var ee = (EntityLeaf2)e;
+ var aa = (EntityLeaf2)a;
+
+ Assert.Equal(ee.Id, aa.Id);
+ Assert.Equal(ee.Name, aa.Name);
+ Assert.Equal(ee.Slumber, aa.Slumber);
+ Assert.Equal(ee.IsBrown, aa.IsBrown);
+ }
+ }
+ },
+ {
+ typeof(EntityTableSharing1), (e, a) =>
+ {
+ Assert.Equal(e == null, a == null);
+
+ if (a != null)
+ {
+ var ee = (EntityTableSharing1)e;
+ var aa = (EntityTableSharing1)a;
+
+ Assert.Equal(ee.Id, aa.Id);
+ Assert.Equal(ee.Name, aa.Name);
+ }
+ }
+ },
+ {
+ typeof(EntityTableSharing2), (e, a) =>
+ {
+ Assert.Equal(e == null, a == null);
+
+ if (a != null)
+ {
+ var ee = (EntityTableSharing2)e;
+ var aa = (EntityTableSharing2)a;
+
+ Assert.Equal(ee.Id, aa.Id);
+ Assert.Equal(ee.Cucumber, aa.Cucumber);
+ }
+ }
+ },
  {
  typeof(UnidirectionalEntityOne), (e, a) =>
  {
@@ -292,6 +359,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
  modelBuilder.Entity<EntityRoot>().Property(e => e.Id).ValueGeneratedNever();
  modelBuilder.Entity<EntityBranch>().HasBaseType<EntityRoot>();
  modelBuilder.Entity<EntityLeaf>().HasBaseType<EntityBranch>();
+ modelBuilder.Entity<EntityBranch2>().HasBaseType<EntityRoot>();
+ modelBuilder.Entity<EntityLeaf2>().HasBaseType<EntityBranch2>();
+ modelBuilder.Entity<EntityTableSharing1>().Property(e => e.Id).ValueGeneratedNever();
+ modelBuilder.Entity<EntityTableSharing2>().Property(e => e.Id).ValueGeneratedNever();
 
  modelBuilder.Entity<UnidirectionalEntityOne>().Property(e => e.Id).ValueGeneratedNever();
  modelBuilder.Entity<UnidirectionalEntityTwo>().Property(e => e.Id).ValueGeneratedNever();
@@ -321,6 +392,22 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
  .HasMany(e => e.TwoSkipShared)
  .WithMany(e => e.OneSkipShared);
 
+ modelBuilder.Entity<EntityRoot>()
+ .HasMany(e => e.BranchSkipShared)
+ .WithMany(e => e.RootSkipShared);
+
+ modelBuilder.Entity<EntityBranch2>()
+ .HasMany(e => e.SelfSkipSharedLeft)
+ .WithMany(e => e.SelfSkipSharedRight);
+
+ modelBuilder.Entity<EntityBranch2>()
+ .HasMany(e => e.Leaf2SkipShared)
+ .WithMany(e => e.Branch2SkipShared);
+
+ modelBuilder.Entity<EntityTableSharing1>()
+ .HasMany(e => e.TableSharing2Shared)
+ .WithMany(e => e.TableSharing1Shared);
+
  // Nav:2 Payload:No Join:Concrete Extra:None
  modelBuilder.Entity<EntityOne>()
  .HasMany(e => e.TwoSkip)
@@ -439,6 +526,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
  .HasMany(e => e.TwoSkipShared)
  .WithMany();
 
+ modelBuilder.Entity<UnidirectionalEntityBranch>()
+ .HasMany<UnidirectionalEntityRoot>()
+ .WithMany(e => e.BranchSkipShared);
+
  // Nav:2 Payload:No Join:Concrete Extra:None
  modelBuilder.Entity<UnidirectionalEntityOne>()
  .HasMany(e => e.TwoSkip)

test/EFCore.Specification.Tests/TestModels/ManyToManyFieldsModel/EntityBranch.cs

@@ -7,4 +7,5 @@ public class EntityBranch : EntityRoot
 {
  public long Number;
  public ICollection<EntityOne> OneSkip;
+ public ICollection<EntityRoot> RootSkipShared;
 }

test/EFCore.Specification.Tests/TestModels/ManyToManyFieldsModel/EntityRoot.cs

@@ -9,4 +9,5 @@ public class EntityRoot
  public string Name;
  public ICollection<EntityThree> ThreeSkipShared;
  public ICollection<EntityCompositeKey> CompositeKeySkipShared;
+ public ICollection<EntityBranch> BranchSkipShared;
 }