From 332ee5a7f8e5cd466fecc70c7d20fae2d674d1ce Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 16 Aug 2022 15:08:32 -0700 Subject: [PATCH] ExecuteDelete: Generate correct Contains method when query has unsupported operators Resolves #28745 --- ...yableMethodTranslatingExpressionVisitor.cs | 2 +- .../NonSharedModelBulkUpdatesTestBase.cs | 144 ++++++++++++++++++ .../NorthwindBulkUpdatesTestBase.cs | 11 +- .../TestUtilities/BulkUpdatesAsserter.cs | 10 +- .../NonSharedModelBulkUpdatesSqlServerTest.cs | 30 ++++ .../NorthwindBulkUpdatesSqlServerTest.cs | 12 ++ .../NonSharedModelBulkUpdatesSqliteTest.cs | 26 ++++ .../NorthwindBulkUpdatesSqliteTest.cs | 14 ++ 8 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs create mode 100644 test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs create mode 100644 test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 918bc3b0cc5..4997eef762d 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -1082,7 +1082,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp if (pk.Properties.Count == 1) { predicateBody = Expression.Call( - QueryableMethods.Contains.MakeGenericMethod(clrType), source, entityParameter); + EnumerableMethods.Contains.MakeGenericMethod(clrType), source, entityParameter); } else { diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs new file mode 100644 index 00000000000..96259bc98c3 --- /dev/null +++ b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs @@ -0,0 +1,144 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.BulkUpdates; + +public abstract class NonSharedModelBulkUpdatesTestBase : NonSharedModelTestBase +{ + protected override string StoreName => "NonSharedModelBulkUpdatesTests"; + +#nullable enable + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task Delete_predicate_based_on_optional_navigation(bool async) + { + var contextFactory = await InitializeAsync(); + await AssertDelete(async, contextFactory.CreateContext, + context => context.Posts.Where(p => p.Blog!.Title!.StartsWith("Arthur")), rowsAffectedCount: 1); + } + + protected class Context28745 : DbContext + { + public Context28745(DbContextOptions options) + : base(options) + { + } + + public DbSet Blogs => Set(); + public DbSet Posts => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity() + .HasData(new Blog { Id = 1, Title = "Arthur" }, new Blog { Id = 2, Title = "Brice" }); + + modelBuilder.Entity() + .HasData( + new { Id = 1, BlogId = 1 }, + new { Id = 2, BlogId = 2 }, + new { Id = 3, BlogId = 2 }); + } + } + + public class Blog + { + public int Id { get; set; } + public string? Title { get; set; } + + public virtual ICollection Posts { get; } = new List(); + } + + public class Post + { + public int Id { get; set; } + public virtual Blog? Blog { get; set; } + } + +#nullable disable + + + #region HelperMethods + + public async Task AssertDelete( + bool async, + Func contextCreator, + Func> query, + int rowsAffectedCount) + where TContext : DbContext + { + if (async) + { + await TestHelpers.ExecuteWithStrategyInTransactionAsync( + contextCreator, UseTransaction, + async context => + { + var processedQuery = query(context); + + var result = await processedQuery.ExecuteDeleteAsync(); + + Assert.Equal(rowsAffectedCount, result); + }); + } + else + { + TestHelpers.ExecuteWithStrategyInTransaction( + contextCreator, UseTransaction, + context => + { + var processedQuery = query(context); + + var result = processedQuery.ExecuteDelete(); + + Assert.Equal(rowsAffectedCount, result); + }); + } + } + + public async Task AssertUpdate( + bool async, + Func contextCreator, + Func> query, + Expression, SetPropertyStatements>> setPropertyStatements, + int rowsAffectedCount) + where TResult : class + where TContext : DbContext + { + if (async) + { + await TestHelpers.ExecuteWithStrategyInTransactionAsync( + contextCreator, UseTransaction, + async context => + { + var processedQuery = query(context); + + var result = await processedQuery.ExecuteUpdateAsync(setPropertyStatements); + + Assert.Equal(rowsAffectedCount, result); + }); + } + else + { + TestHelpers.ExecuteWithStrategyInTransaction( + contextCreator, UseTransaction, + context => + { + var processedQuery = query(context); + + var result = processedQuery.ExecuteUpdate(setPropertyStatements); + + Assert.Equal(rowsAffectedCount, result); + }); + } + } + + public void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction) + => facade.UseTransaction(transaction.GetDbTransaction()); + + protected TestSqlLoggerFactory TestSqlLoggerFactory + => (TestSqlLoggerFactory)ListLoggerFactory; + + protected void ClearLog() + => TestSqlLoggerFactory.Clear(); + + #endregion +} diff --git a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NorthwindBulkUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NorthwindBulkUpdatesTestBase.cs index e51da687cf8..dc8d5cca556 100644 --- a/test/EFCore.Relational.Specification.Tests/BulkUpdates/NorthwindBulkUpdatesTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/BulkUpdates/NorthwindBulkUpdatesTestBase.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.EntityFrameworkCore.TestModels.Northwind; -using Microsoft.VisualBasic; namespace Microsoft.EntityFrameworkCore.BulkUpdates; @@ -279,6 +278,16 @@ FROM [Order Details] } } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Delete_Where_optional_navigation_predicate(bool async) + => AssertDelete( + async, + ss => from od in ss.Set() + where od.Order.Customer.City.StartsWith("Se") + select od, + rowsAffectedCount: 66); + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Delete_with_join(bool async) diff --git a/test/EFCore.Relational.Specification.Tests/TestUtilities/BulkUpdatesAsserter.cs b/test/EFCore.Relational.Specification.Tests/TestUtilities/BulkUpdatesAsserter.cs index 351dc0b37b7..5331d024055 100644 --- a/test/EFCore.Relational.Specification.Tests/TestUtilities/BulkUpdatesAsserter.cs +++ b/test/EFCore.Relational.Specification.Tests/TestUtilities/BulkUpdatesAsserter.cs @@ -82,10 +82,7 @@ await TestHelpers.ExecuteWithStrategyInTransactionAsync( var after = processedQuery.AsNoTracking().Select(entitySelector).OrderBy(elementSorter).ToList(); - if (asserter != null) - { - asserter(before, after); - } + asserter?.Invoke(before, after); }); } else @@ -104,10 +101,7 @@ await TestHelpers.ExecuteWithStrategyInTransactionAsync( var after = processedQuery.AsNoTracking().Select(entitySelector).OrderBy(elementSorter).ToList(); - if (asserter != null) - { - asserter(before, after); - } + asserter?.Invoke(before, after); }); } } diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs new file mode 100644 index 00000000000..160ccc428ac --- /dev/null +++ b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.BulkUpdates; + +public class NonSharedModelBulkUpdatesSqlServerTest : NonSharedModelBulkUpdatesTestBase +{ + protected override ITestStoreFactory TestStoreFactory => SqlServerTestStoreFactory.Instance; + + [ConditionalFact] + public virtual void Check_all_tests_overridden() + => TestHelpers.AssertAllMethodsOverridden(GetType()); + + public override async Task Delete_predicate_based_on_optional_navigation(bool async) + { + await base.Delete_predicate_based_on_optional_navigation(async); + + AssertSql( + @"DELETE FROM [p] +FROM [Posts] AS [p] +LEFT JOIN [Blogs] AS [b] ON [p].[BlogId] = [b].[Id] +WHERE [b].[Title] IS NOT NULL AND ([b].[Title] LIKE N'Arthur%')"); + } + + private void AssertSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected); + + private void AssertExecuteUpdateSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected, forUpdate: true); +} diff --git a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs index d65a64fc885..3a95f571010 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs @@ -412,6 +412,18 @@ SELECT 1 WHERE [m].[OrderID] = [o].[OrderID] AND [m].[ProductID] = [o].[ProductID])"); } + public override async Task Delete_Where_optional_navigation_predicate(bool async) + { + await base.Delete_Where_optional_navigation_predicate(async); + + AssertSql( + @"DELETE FROM [o] +FROM [Order Details] AS [o] +INNER JOIN [Orders] AS [o0] ON [o].[OrderID] = [o0].[OrderID] +LEFT JOIN [Customers] AS [c] ON [o0].[CustomerID] = [c].[CustomerID] +WHERE [c].[City] IS NOT NULL AND ([c].[City] LIKE N'Se%')"); + } + public override async Task Delete_with_join(bool async) { await base.Delete_with_join(async); diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs new file mode 100644 index 00000000000..b43eca820d2 --- /dev/null +++ b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs @@ -0,0 +1,26 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.BulkUpdates; + +public class NonSharedModelBulkUpdatesSqliteTest : NonSharedModelBulkUpdatesTestBase +{ + protected override ITestStoreFactory TestStoreFactory => SqliteTestStoreFactory.Instance; + + [ConditionalFact] + public virtual void Check_all_tests_overridden() + => TestHelpers.AssertAllMethodsOverridden(GetType()); + + public override async Task Delete_predicate_based_on_optional_navigation(bool async) + { + await base.Delete_predicate_based_on_optional_navigation(async); + + AssertSql(); + } + + private void AssertSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected); + + private void AssertExecuteUpdateSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected, forUpdate: true); +} diff --git a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqliteTest.cs index 05b89063b28..dd85bb0a8b2 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqliteTest.cs @@ -408,6 +408,20 @@ SELECT 1 WHERE ""m"".""OrderID"" = ""o"".""OrderID"" AND ""m"".""ProductID"" = ""o"".""ProductID"")"); } + public override async Task Delete_Where_optional_navigation_predicate(bool async) + { + await base.Delete_Where_optional_navigation_predicate(async); + + AssertSql( + @"DELETE FROM ""Order Details"" AS ""o"" +WHERE EXISTS ( + SELECT 1 + FROM ""Order Details"" AS ""o0"" + INNER JOIN ""Orders"" AS ""o1"" ON ""o0"".""OrderID"" = ""o1"".""OrderID"" + LEFT JOIN ""Customers"" AS ""c"" ON ""o1"".""CustomerID"" = ""c"".""CustomerID"" + WHERE ""c"".""City"" IS NOT NULL AND (""c"".""City"" LIKE 'Se%') AND ""o0"".""OrderID"" = ""o"".""OrderID"" AND ""o0"".""ProductID"" = ""o"".""ProductID"")"); + } + public override async Task Delete_with_join(bool async) { await base.Delete_with_join(async);