From d4159289729199b792a9831cf8ecd0aecbc65738 Mon Sep 17 00:00:00 2001 From: Andrew Peters Date: Tue, 23 May 2017 11:11:30 -0700 Subject: [PATCH] Fix #8557 - Adds parameterized interpolated string support to Database.ExecuteSqlCommand. --- .../SqlExecutorTestBase.cs | 72 +++++++++++++++-- .../RelationalDatabaseFacadeExtensions.cs | 39 +++++++-- .../breakingchanges.netcore.json | 5 ++ .../breakingchanges.netframework.json | 5 ++ .../RelationalDatabaseFacadeExtensionsTest.cs | 11 ++- .../SqlExecutorSqlServerTest.cs | 79 ++++++++++++++----- 6 files changed, 172 insertions(+), 39 deletions(-) diff --git a/src/EFCore.Relational.Specification.Tests/SqlExecutorTestBase.cs b/src/EFCore.Relational.Specification.Tests/SqlExecutorTestBase.cs index a30a313b6fc..ca8e9c01df8 100644 --- a/src/EFCore.Relational.Specification.Tests/SqlExecutorTestBase.cs +++ b/src/EFCore.Relational.Specification.Tests/SqlExecutorTestBase.cs @@ -3,15 +3,16 @@ using System; using System.Data.Common; -using System.Threading; using System.Threading.Tasks; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; +using Microsoft.EntityFrameworkCore.Specification.Tests; using Microsoft.EntityFrameworkCore.Specification.Tests.TestModels.Northwind; using Microsoft.Extensions.DependencyInjection; using Xunit; +// ReSharper disable ConvertToConstant.Local -namespace Microsoft.EntityFrameworkCore.Specification.Tests +namespace Microsoft.EntityFrameworkCore.Relational.Specification { public abstract class SqlExecutorTestBase : IClassFixture where TFixture : NorthwindQueryFixtureBase, new() @@ -59,6 +60,38 @@ public virtual void Throws_on_concurrent_command() } } + [Fact] + public virtual void Query_with_parameters() + { + var city = "London"; + var contactTitle = "Sales Representative"; + + using (var context = CreateContext()) + { + var actual = context.Database + .ExecuteSqlCommand( + @"SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = {0} AND ""ContactTitle"" = {1}", city, contactTitle); + + Assert.Equal(-1, actual); + } + } + + [Fact] + public virtual void Query_with_parameters_interpolated() + { + var city = "London"; + var contactTitle = "Sales Representative"; + + using (var context = CreateContext()) + { + var actual = context.Database + .ExecuteSqlCommand( + $@"SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = {city} AND ""ContactTitle"" = {contactTitle}"); + + Assert.Equal(-1, actual); + } + } + [Fact] public virtual async Task Executes_stored_procedure_async() { @@ -102,13 +135,42 @@ public virtual async Task Throws_on_concurrent_command_async() } } - protected NorthwindContext CreateContext() => Fixture.CreateContext(); + [Fact] + public virtual async Task Query_with_parameters_async() + { + var city = "London"; + var contactTitle = "Sales Representative"; + + using (var context = CreateContext()) + { + var actual = await context.Database + .ExecuteSqlCommandAsync( + @"SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = {0} AND ""ContactTitle"" = {1}", city, contactTitle); + + Assert.Equal(-1, actual); + } + } - protected SqlExecutorTestBase(TFixture fixture) + [Fact] + public virtual async Task Query_with_parameters_interpolated_async() { - Fixture = fixture; + var city = "London"; + var contactTitle = "Sales Representative"; + + using (var context = CreateContext()) + { + var actual = await context.Database + .ExecuteSqlCommandAsync( + $@"SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = {city} AND ""ContactTitle"" = {contactTitle}"); + + Assert.Equal(-1, actual); + } } + protected NorthwindContext CreateContext() => Fixture.CreateContext(); + + protected SqlExecutorTestBase(TFixture fixture) => Fixture = fixture; + protected TFixture Fixture { get; } protected abstract DbParameter CreateDbParameter(string name, object value); diff --git a/src/EFCore.Relational/RelationalDatabaseFacadeExtensions.cs b/src/EFCore.Relational/RelationalDatabaseFacadeExtensions.cs index cdabdc4cd6d..bad8dc258d2 100644 --- a/src/EFCore.Relational/RelationalDatabaseFacadeExtensions.cs +++ b/src/EFCore.Relational/RelationalDatabaseFacadeExtensions.cs @@ -102,17 +102,35 @@ public static Task MigrateAsync( => Check.NotNull(databaseFacade, nameof(databaseFacade)).GetRelationalService() .MigrateAsync(cancellationToken: cancellationToken); + /// + /// A SQL format string. This type enables overload resolution between + /// the regular and interpolated ExecuteSqlCommand overloads. + /// + public struct SqlFormat + { + public static implicit operator SqlFormat([NotNull] string s) => new SqlFormat(s); + public static implicit operator SqlFormat([NotNull] FormattableString fs) => default(SqlFormat); + public SqlFormat([NotNull] string s) => Format = s; + public string Format { get; } + } + // Note that this method doesn't start a transaction hence it doesn't use ExecutionStrategy public static int ExecuteSqlCommand( [NotNull] this DatabaseFacade databaseFacade, - [NotNull] string sql, + SqlFormat sql, [NotNull] params object[] parameters) => ExecuteSqlCommand(databaseFacade, sql, (IEnumerable)parameters); + + // Note that this method doesn't start a transaction hence it doesn't use ExecutionStrategy + public static int ExecuteSqlCommand( + [NotNull] this DatabaseFacade databaseFacade, + [NotNull] FormattableString sql) + => ExecuteSqlCommand(databaseFacade, sql.Format, sql.GetArguments()); // Note that this method doesn't start a transaction hence it doesn't use ExecutionStrategy public static int ExecuteSqlCommand( [NotNull] this DatabaseFacade databaseFacade, - [NotNull] string sql, + SqlFormat sql, [NotNull] IEnumerable parameters) { Check.NotNull(databaseFacade, nameof(databaseFacade)); @@ -125,7 +143,7 @@ public static int ExecuteSqlCommand( { var rawSqlCommand = databaseFacade .GetRelationalService() - .Build(sql, parameters); + .Build(sql.Format, parameters); return rawSqlCommand .RelationalCommand @@ -138,21 +156,28 @@ public static int ExecuteSqlCommand( // Note that this method doesn't start a transaction hence it doesn't use ExecutionStrategy public static Task ExecuteSqlCommandAsync( [NotNull] this DatabaseFacade databaseFacade, - [NotNull] string sql, + [NotNull] FormattableString sql, + CancellationToken cancellationToken = default(CancellationToken)) + => ExecuteSqlCommandAsync(databaseFacade, sql.Format, sql.GetArguments(), cancellationToken); + + // Note that this method doesn't start a transaction hence it doesn't use ExecutionStrategy + public static Task ExecuteSqlCommandAsync( + [NotNull] this DatabaseFacade databaseFacade, + SqlFormat sql, CancellationToken cancellationToken = default(CancellationToken)) => ExecuteSqlCommandAsync(databaseFacade, sql, Enumerable.Empty(), cancellationToken); // Note that this method doesn't start a transaction hence it doesn't use ExecutionStrategy public static Task ExecuteSqlCommandAsync( [NotNull] this DatabaseFacade databaseFacade, - [NotNull] string sql, + SqlFormat sql, [NotNull] params object[] parameters) => ExecuteSqlCommandAsync(databaseFacade, sql, (IEnumerable)parameters); // Note that this method doesn't start a transaction hence it doesn't use ExecutionStrategy public static async Task ExecuteSqlCommandAsync( [NotNull] this DatabaseFacade databaseFacade, - [NotNull] string sql, + SqlFormat sql, [NotNull] IEnumerable parameters, CancellationToken cancellationToken = default(CancellationToken)) { @@ -166,7 +191,7 @@ public static async Task ExecuteSqlCommandAsync( { var rawSqlCommand = databaseFacade .GetRelationalService() - .Build(sql, parameters); + .Build(sql.Format, parameters); return await rawSqlCommand .RelationalCommand diff --git a/src/EFCore.Relational/breakingchanges.netcore.json b/src/EFCore.Relational/breakingchanges.netcore.json index e0179a3b066..fa09f10ef3b 100644 --- a/src/EFCore.Relational/breakingchanges.netcore.json +++ b/src/EFCore.Relational/breakingchanges.netcore.json @@ -1107,5 +1107,10 @@ "TypeId": "public static class Microsoft.EntityFrameworkCore.Storage.RelationalTypeMapperExtensions", "MemberId": "public static System.Boolean IsTypeMapped(this Microsoft.EntityFrameworkCore.Storage.IRelationalTypeMapper typeMapper, System.Type clrType)", "Kind": "Removal" + }, + { + "TypeId": "public static class Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions", + "MemberId": "public static System.Int32 ExecuteSqlCommand(this Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade databaseFacade, System.String sql, params System.Object[] parameters)", + "Kind": "Removal" } ] diff --git a/src/EFCore.Relational/breakingchanges.netframework.json b/src/EFCore.Relational/breakingchanges.netframework.json index 43f71bd1a67..dbe93b1f5d9 100644 --- a/src/EFCore.Relational/breakingchanges.netframework.json +++ b/src/EFCore.Relational/breakingchanges.netframework.json @@ -1107,5 +1107,10 @@ "TypeId": "public static class Microsoft.EntityFrameworkCore.Storage.RelationalTypeMapperExtensions", "MemberId": "public static System.Boolean IsTypeMapped(this Microsoft.EntityFrameworkCore.Storage.IRelationalTypeMapper typeMapper, System.Type clrType)", "Kind": "Removal" + }, + { + "TypeId": "public static class Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions", + "MemberId": "public static System.Int32 ExecuteSqlCommand(this Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade databaseFacade, System.String sql, params System.Object[] parameters)", + "Kind": "Removal" } ] diff --git a/test/EFCore.Relational.Tests/Storage/RelationalDatabaseFacadeExtensionsTest.cs b/test/EFCore.Relational.Tests/Storage/RelationalDatabaseFacadeExtensionsTest.cs index 197e0233409..57f195f8db2 100644 --- a/test/EFCore.Relational.Tests/Storage/RelationalDatabaseFacadeExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Storage/RelationalDatabaseFacadeExtensionsTest.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.Extensions.DependencyInjection; using Moq; @@ -276,15 +277,13 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) .UseTransientInMemoryDatabase(); } + [UsedImplicitly] private class TestRawSqlCommandBuilder : IRawSqlCommandBuilder { - public string Sql { get; set; } - public IEnumerable Parameters { get; set; } + public string Sql { get; private set; } + public IEnumerable Parameters { get; private set; } - public IRelationalCommand Build(string sql) - { - throw new NotImplementedException(); - } + public IRelationalCommand Build(string sql) => throw new NotImplementedException(); public RawSqlCommand Build(string sql, IEnumerable parameters) { diff --git a/test/EFCore.SqlServer.FunctionalTests/SqlExecutorSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/SqlExecutorSqlServerTest.cs index 9637c55186c..471751a40e5 100644 --- a/test/EFCore.SqlServer.FunctionalTests/SqlExecutorSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/SqlExecutorSqlServerTest.cs @@ -1,51 +1,92 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Data.Common; using System.Data.SqlClient; -using Microsoft.EntityFrameworkCore.Specification.Tests; -using Xunit; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Relational.Specification; +using Xunit.Abstractions; namespace Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests { public class SqlExecutorSqlServerTest : SqlExecutorTestBase { + public SqlExecutorSqlServerTest(NorthwindQuerySqlServerFixture fixture, ITestOutputHelper testOutputHelper) + : base(fixture) + { + fixture.TestSqlLoggerFactory.Clear(); + //fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); + } + public override void Executes_stored_procedure() { base.Executes_stored_procedure(); - Assert.Equal( - "[dbo].[Ten Most Expensive Products]", - Sql); + AssertSql("[dbo].[Ten Most Expensive Products]"); } public override void Executes_stored_procedure_with_parameter() { base.Executes_stored_procedure_with_parameter(); - Assert.Equal( + AssertSql( @"@CustomerID: ALFKI (Nullable = false) (Size = 5) -[dbo].[CustOrderHist] @CustomerID", - Sql); +[dbo].[CustOrderHist] @CustomerID"); } public override void Executes_stored_procedure_with_generated_parameter() { base.Executes_stored_procedure_with_generated_parameter(); - Assert.Equal( + AssertSql( @"@p0: ALFKI (Size = 4000) -[dbo].[CustOrderHist] @CustomerID = @p0", - Sql); +[dbo].[CustOrderHist] @CustomerID = @p0"); } - public SqlExecutorSqlServerTest(NorthwindQuerySqlServerFixture fixture) - : base(fixture) + public override void Query_with_parameters() { - fixture.TestSqlLoggerFactory.Clear(); + base.Query_with_parameters(); + + AssertSql( + @"@p0: London (Size = 4000) +@p1: Sales Representative (Size = 4000) + +SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = @p0 AND ""ContactTitle"" = @p1"); + } + + public override void Query_with_parameters_interpolated() + { + base.Query_with_parameters_interpolated(); + + AssertSql( + @"@p0: London (Size = 4000) +@p1: Sales Representative (Size = 4000) + +SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = @p0 AND ""ContactTitle"" = @p1"); + } + + public override async Task Query_with_parameters_async() + { + await base.Query_with_parameters_async(); + + AssertSql( + @"@p0: London (Size = 4000) +@p1: Sales Representative (Size = 4000) + +SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = @p0 AND ""ContactTitle"" = @p1"); + } + + public override async Task Query_with_parameters_interpolated_async() + { + await base.Query_with_parameters_interpolated_async(); + + AssertSql( + @"@p0: London (Size = 4000) +@p1: Sales Representative (Size = 4000) + +SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = @p0 AND ""ContactTitle"" = @p1"); } protected override DbParameter CreateDbParameter(string name, object value) @@ -56,14 +97,10 @@ protected override DbParameter CreateDbParameter(string name, object value) }; protected override string TenMostExpensiveProductsSproc => "[dbo].[Ten Most Expensive Products]"; - protected override string CustomerOrderHistorySproc => "[dbo].[CustOrderHist] @CustomerID"; - protected override string CustomerOrderHistoryWithGeneratedParameterSproc => "[dbo].[CustOrderHist] @CustomerID = {0}"; - private const string FileLineEnding = @" -"; - - private string Sql => Fixture.TestSqlLoggerFactory.Sql.Replace(Environment.NewLine, FileLineEnding); + private void AssertSql(params string[] expected) + => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); } }