From bf62eaf95aebfde1cb1084889fbb1dbbcfe96f2f Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Sat, 3 Sep 2022 00:18:41 +0100 Subject: [PATCH] Don't skip creating cascade delete on SQL Server when types are base-linking across tables (#28965) --- .../SqlServerOnDeleteConvention.cs | 14 ++++++++----- .../Migrations/ModelSnapshotSqlServerTest.cs | 10 +++++----- .../CSharpRuntimeModelCodeGeneratorTest.cs | 6 +++--- ...PTFiltersInheritanceBulkUpdatesTestBase.cs | 12 ----------- .../TPTGearsOfWarQueryRelationalFixture.cs | 6 ++++++ .../Metadata/RelationalModelTest.cs | 2 ++ ...tersInheritanceBulkUpdatesSqlServerTest.cs | 8 +++++++- .../Migrations/SqlServerModelDifferTest.cs | 6 +++--- ...FiltersInheritanceBulkUpdatesSqliteTest.cs | 20 +++++++++++-------- 9 files changed, 47 insertions(+), 37 deletions(-) diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs index 297c9e489c4..a1797ed017a 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerOnDeleteConvention.cs @@ -57,11 +57,6 @@ protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey return deleteBehavior; } - if (foreignKey.IsBaseLinking()) - { - return DeleteBehavior.ClientCascade; - } - return ProcessSkipNavigations(foreignKey.GetReferencingSkipNavigations()) ?? deleteBehavior; } @@ -133,6 +128,15 @@ public virtual void ProcessEntityTypeAnnotationChanged( || name == RelationalAnnotationNames.Schema) { ProcessSkipNavigations(entityTypeBuilder.Metadata.GetDeclaredSkipNavigations()); + + foreach (var foreignKey in entityTypeBuilder.Metadata.GetDeclaredForeignKeys()) + { + var deleteBehavior = GetTargetDeleteBehavior(foreignKey); + if (foreignKey.DeleteBehavior != deleteBehavior) + { + foreignKey.Builder.OnDelete(deleteBehavior); + } + } } } } diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index 8df411c52b3..c977a53806b 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -538,7 +538,7 @@ public virtual void Entities_are_stored_in_model_snapshot_for_TPT() b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+BaseEntity"", null) .WithOne() .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+DerivedEntity"", ""Id"") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); });"), model => @@ -606,7 +606,7 @@ public virtual void Entities_are_stored_in_model_snapshot_for_TPT_with_one_exclu b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+BaseEntity"", null) .WithOne() .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+DerivedEntity"", ""Id"") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); });"), o => @@ -832,7 +832,7 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_tables() b.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order"", null) .WithOne() .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order"", ""Id"") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); b.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderBillingDetails"", b1 => @@ -863,7 +863,7 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_tables() b1.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderBillingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", null) .WithOne() .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderBillingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderId"") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); b1.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+StreetAddress"", ""StreetAddress"", b2 => @@ -912,7 +912,7 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_tables() b1.HasOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderShippingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", null) .WithOne() .HasForeignKey(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+Order.OrderShippingDetails#Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderId"") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); b1.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+StreetAddress"", ""StreetAddress"", b2 => diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs index 3d3951627f0..9d781135057 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs @@ -1340,7 +1340,7 @@ public static RuntimeForeignKey CreateForeignKey2(RuntimeEntityType declaringEnt var runtimeForeignKey = declaringEntityType.AddForeignKey(new[] { declaringEntityType.FindProperty(""PrincipalBaseId"")!, declaringEntityType.FindProperty(""PrincipalBaseAlternateId"")! }, principalEntityType.FindKey(new[] { principalEntityType.FindProperty(""PrincipalBaseId"")!, principalEntityType.FindProperty(""PrincipalBaseAlternateId"")! })!, principalEntityType, - deleteBehavior: DeleteBehavior.ClientCascade, + deleteBehavior: DeleteBehavior.Cascade, unique: true, required: true, requiredDependent: true); @@ -1681,7 +1681,7 @@ public static RuntimeForeignKey CreateForeignKey1(RuntimeEntityType declaringEnt var runtimeForeignKey = declaringEntityType.AddForeignKey(new[] { declaringEntityType.FindProperty(""Id"")!, declaringEntityType.FindProperty(""AlternateId"")! }, principalEntityType.FindKey(new[] { principalEntityType.FindProperty(""Id"")!, principalEntityType.FindProperty(""AlternateId"")! })!, principalEntityType, - deleteBehavior: DeleteBehavior.ClientCascade, + deleteBehavior: DeleteBehavior.Cascade, unique: true, required: true); @@ -1967,7 +1967,7 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) Assert.True(tptForeignKey.IsUnique); Assert.Null(tptForeignKey.DependentToPrincipal); Assert.Null(tptForeignKey.PrincipalToDependent); - Assert.Equal(DeleteBehavior.ClientCascade, tptForeignKey.DeleteBehavior); + Assert.Equal(DeleteBehavior.Cascade, tptForeignKey.DeleteBehavior); Assert.Equal(principalKey.Properties, tptForeignKey.Properties); Assert.Same(principalKey, tptForeignKey.PrincipalKey); diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesTestBase.cs index 8eb6cc01c08..0d4cc7d378b 100644 --- a/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesTestBase.cs @@ -34,18 +34,6 @@ public override Task Delete_GroupBy_Where_Select_First_3(bool async) RelationalStrings.ExecuteOperationOnTPT("ExecuteDelete", "Animal"), () => base.Delete_GroupBy_Where_Select_First_3(async)); - [ConditionalTheory(Skip = "Issue#28532")] - public override Task Delete_where_using_hierarchy(bool async) - { - return base.Delete_where_using_hierarchy(async); - } - - [ConditionalTheory(Skip = "Issue#28532")] - public override Task Delete_where_using_hierarchy_derived(bool async) - { - return base.Delete_where_using_hierarchy_derived(async); - } - // Keyless entities are mapped as TPH only public override Task Update_where_keyless_entity_mapped_to_sql_query(bool async) => Task.CompletedTask; diff --git a/test/EFCore.Relational.Specification.Tests/Query/TPTGearsOfWarQueryRelationalFixture.cs b/test/EFCore.Relational.Specification.Tests/Query/TPTGearsOfWarQueryRelationalFixture.cs index ab5638b8add..80665734a22 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/TPTGearsOfWarQueryRelationalFixture.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/TPTGearsOfWarQueryRelationalFixture.cs @@ -28,5 +28,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con modelBuilder.Entity().ToTable("LocustHordes"); modelBuilder.Entity().ToTable("LocustCommanders"); + + modelBuilder.Entity() + .HasMany(s => s.Members) + .WithOne(g => g.Squad) + .HasForeignKey(g => g.SquadId) + .OnDelete(DeleteBehavior.ClientCascade); } } diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs index 1c615fb1559..024a777aec7 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs @@ -857,11 +857,13 @@ private static void AssertTables(IRelationalModel model, Mapping mapping) Assert.Equal("FK_SpecialCustomer_Customer_Id", specialCustomerTptFkConstraint.Name); Assert.NotNull(specialCustomerTptFkConstraint.MappedForeignKeys.Single()); Assert.Same(customerTable, specialCustomerTptFkConstraint.PrincipalTable); + Assert.Equal(ReferentialAction.Cascade, specialCustomerTptFkConstraint.OnDeleteAction); var anotherSpecialCustomerFkConstraint = foreignKeys[2]; Assert.Equal("FK_SpecialCustomer_SpecialCustomer_AnotherRelatedCustomerId", anotherSpecialCustomerFkConstraint.Name); Assert.NotNull(anotherSpecialCustomerFkConstraint.MappedForeignKeys.Single()); Assert.Same(specialCustomerTable, anotherSpecialCustomerFkConstraint.PrincipalTable); + Assert.Equal(ReferentialAction.Cascade, specialCustomerTptFkConstraint.OnDeleteAction); Assert.Equal(new[] { orderCustomerFkConstraint, specialCustomerTptFkConstraint }, customerTable.ReferencingForeignKeyConstraints); diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqlServerTest.cs index c01a9822937..8a1fe8c4562 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqlServerTest.cs @@ -39,6 +39,9 @@ FROM [Countries] AS [c] WHERE ( SELECT COUNT(*) FROM [Animals] AS [a] + LEFT JOIN [Birds] AS [b] ON [a].[Id] = [b].[Id] + LEFT JOIN [Eagle] AS [e] ON [a].[Id] = [e].[Id] + LEFT JOIN [Kiwi] AS [k] ON [a].[Id] = [k].[Id] WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[CountryId] > 0) > 0"); } @@ -52,7 +55,10 @@ FROM [Countries] AS [c] WHERE ( SELECT COUNT(*) FROM [Animals] AS [a] - WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[Discriminator] = N'Kiwi' AND [a].[CountryId] > 0) > 0"); + LEFT JOIN [Birds] AS [b] ON [a].[Id] = [b].[Id] + LEFT JOIN [Eagle] AS [e] ON [a].[Id] = [e].[Id] + LEFT JOIN [Kiwi] AS [k] ON [a].[Id] = [k].[Id] + WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [k].[Id] IS NOT NULL AND [a].[CountryId] > 0) > 0"); } public override async Task Delete_where_keyless_entity_mapped_to_sql_query(bool async) diff --git a/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs b/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs index bb3992c9ab7..52b288987f0 100644 --- a/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs +++ b/test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs @@ -1024,7 +1024,7 @@ public void Noop_TPT_with_FKs_and_seed_data() b.HasOne("Animal", null) .WithOne() .HasForeignKey("Cat", "Id") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); b.HasOne("Animal", null) @@ -1038,7 +1038,7 @@ public void Noop_TPT_with_FKs_and_seed_data() b.HasOne("Animal", null) .WithOne() .HasForeignKey("Dog", "Id") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); b.HasOne("Animal", null) @@ -1052,7 +1052,7 @@ public void Noop_TPT_with_FKs_and_seed_data() b.HasOne("Animal", null) .WithOne() .HasForeignKey("Mouse", "Id") - .OnDelete(DeleteBehavior.ClientCascade) + .OnDelete(DeleteBehavior.Cascade) .IsRequired(); }); }, diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqliteTest.cs index b15a43d4429..a0da9740a2e 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/TPTFiltersInheritanceBulkUpdatesSqliteTest.cs @@ -34,12 +34,14 @@ public override async Task Delete_where_using_hierarchy(bool async) await base.Delete_where_using_hierarchy(async); AssertSql( - @"DELETE FROM [c] -FROM [Countries] AS [c] + @"DELETE FROM ""Countries"" AS ""c"" WHERE ( SELECT COUNT(*) - FROM [Animals] AS [a] - WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[CountryId] > 0) > 0"); + FROM ""Animals"" AS ""a"" + LEFT JOIN ""Birds"" AS ""b"" ON ""a"".""Id"" = ""b"".""Id"" + LEFT JOIN ""Eagle"" AS ""e"" ON ""a"".""Id"" = ""e"".""Id"" + LEFT JOIN ""Kiwi"" AS ""k"" ON ""a"".""Id"" = ""k"".""Id"" + WHERE ""a"".""CountryId"" = 1 AND ""c"".""Id"" = ""a"".""CountryId"" AND ""a"".""CountryId"" > 0) > 0"); } public override async Task Delete_where_using_hierarchy_derived(bool async) @@ -47,12 +49,14 @@ public override async Task Delete_where_using_hierarchy_derived(bool async) await base.Delete_where_using_hierarchy_derived(async); AssertSql( - @"DELETE FROM [c] -FROM [Countries] AS [c] + @"DELETE FROM ""Countries"" AS ""c"" WHERE ( SELECT COUNT(*) - FROM [Animals] AS [a] - WHERE [a].[CountryId] = 1 AND [c].[Id] = [a].[CountryId] AND [a].[Discriminator] = N'Kiwi' AND [a].[CountryId] > 0) > 0"); + FROM ""Animals"" AS ""a"" + LEFT JOIN ""Birds"" AS ""b"" ON ""a"".""Id"" = ""b"".""Id"" + LEFT JOIN ""Eagle"" AS ""e"" ON ""a"".""Id"" = ""e"".""Id"" + LEFT JOIN ""Kiwi"" AS ""k"" ON ""a"".""Id"" = ""k"".""Id"" + WHERE ""a"".""CountryId"" = 1 AND ""c"".""Id"" = ""a"".""CountryId"" AND ""k"".""Id"" IS NOT NULL AND ""a"".""CountryId"" > 0) > 0"); } public override async Task Delete_where_keyless_entity_mapped_to_sql_query(bool async)