From e5ab9d0c73906ed1f17caedcd9336597d4de9266 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 7 Feb 2022 13:05:56 +0200 Subject: [PATCH] Allow migration tests to specify migrations options (#27331) Specifically to actually run idempotent migrations against the database. Closes #23811 --- .../Migrations/MigrationsTestBase.cs | 32 ++- .../Migrations/MigrationsSqlServerTest.cs | 213 ++++++++++++++++++ .../SqlServerMigrationsSqlGeneratorTest.cs | 155 ------------- 3 files changed, 233 insertions(+), 167 deletions(-) diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs index 8e8604dd05f..439686def68 100644 --- a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs @@ -1816,7 +1816,7 @@ await Test( @"-- I <3 DDL"); } - private class Person + protected class Person { public int Id { get; set; } public int AnotherId { get; set; } @@ -1861,15 +1861,17 @@ protected virtual Task Test( Action buildSourceAction, Action buildTargetAction, Action asserter, - bool withConventions = true) - => Test(_ => { }, buildSourceAction, buildTargetAction, asserter, withConventions); + bool withConventions = true, + MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default) + => Test(_ => { }, buildSourceAction, buildTargetAction, asserter, withConventions, migrationsSqlGenerationOptions); protected virtual Task Test( Action buildCommonAction, Action buildSourceAction, Action buildTargetAction, Action asserter, - bool withConventions = true) + bool withConventions = true, + MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default) { var context = CreateContext(); var modelDiffer = context.GetService(); @@ -1899,21 +1901,23 @@ protected virtual Task Test( // Get the migration operations between the two models and test var operations = modelDiffer.GetDifferences(sourceModel.GetRelationalModel(), targetModel.GetRelationalModel()); - return Test(sourceModel, targetModel, operations, asserter); + return Test(sourceModel, targetModel, operations, asserter, migrationsSqlGenerationOptions); } protected virtual Task Test( Action buildSourceAction, MigrationOperation operation, Action asserter, - bool withConventions = true) - => Test(buildSourceAction, new[] { operation }, asserter, withConventions); + bool withConventions = true, + MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default) + => Test(buildSourceAction, new[] { operation }, asserter, withConventions, migrationsSqlGenerationOptions); protected virtual Task Test( Action buildSourceAction, IReadOnlyList operations, Action asserter, - bool withConventions = true) + bool withConventions = true, + MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default) { var sourceModelBuilder = CreateModelBuilder(withConventions); buildSourceAction(sourceModelBuilder); @@ -1934,14 +1938,15 @@ protected virtual Task Test( modelSnapshotNamespace: null, typeof(DbContext), "MigrationsTestSnapshot", preSnapshotSourceModel); var sourceModel = BuildModelFromSnapshotSource(sourceModelSnapshot); - return Test(sourceModel, targetModel: null, operations, asserter); + return Test(sourceModel, targetModel: null, operations, asserter, migrationsSqlGenerationOptions); } protected virtual async Task Test( IModel sourceModel, IModel targetModel, IReadOnlyList operations, - Action asserter) + Action asserter, + MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default) { var context = CreateContext(); var serviceProvider = ((IInfrastructure)context).Instance; @@ -1958,14 +1963,17 @@ protected virtual async Task Test( using (Fixture.TestSqlLoggerFactory.SuspendRecordingEvents()) { await migrationsCommandExecutor.ExecuteNonQueryAsync( - migrationsSqlGenerator.Generate(modelDiffer.GetDifferences(null, sourceModel.GetRelationalModel()), sourceModel), + migrationsSqlGenerator.Generate( + modelDiffer.GetDifferences(null, sourceModel.GetRelationalModel()), + sourceModel, + migrationsSqlGenerationOptions), connection); } // Apply migrations to get from source to target, then reverse-engineer and execute the // test-provided assertions on the resulting database model await migrationsCommandExecutor.ExecuteNonQueryAsync( - migrationsSqlGenerator.Generate(operations, targetModel), connection); + migrationsSqlGenerator.Generate(operations, targetModel, migrationsSqlGenerationOptions), connection); var scaffoldedModel = databaseModelFactory.Create( context.Database.GetDbConnection(), diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs index 466bd42ce95..a9ecf303e50 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs @@ -344,6 +344,7 @@ await Test( AssertSql($@"ALTER TABLE [People] ADD [Birthday] datetime2({precision}) NOT NULL DEFAULT '2015-04-12T17:05:00{fractionalSeconds}';"); } + [ConditionalTheory] [InlineData(0, "", 1234567)] [InlineData(1, ".1", 1234567)] @@ -498,6 +499,26 @@ public override async Task Add_column_with_computedSql(bool? stored) @$"ALTER TABLE [People] ADD [Sum] AS [X] + [Y]{computedColumnTypeSql};"); } + [ConditionalFact] + public virtual async Task Add_column_generates_exec_when_computed_and_idempotent() + { + await Test( + builder => builder.Entity("People").Property("Id"), + builder => { }, + builder => builder.Entity("People").Property("IdPlusOne").HasComputedColumnSql("[Id] + 1"), + model => + { + var table = Assert.Single(model.Tables); + Assert.Equal(2, table.Columns.Count); + var column = Assert.Single(table.Columns, c => c.Name == "IdPlusOne"); + Assert.Equal("([Id]+(1))", column.ComputedColumnSql); + }, + migrationsSqlGenerationOptions: MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + @"EXEC(N'ALTER TABLE [People] ADD [IdPlusOne] AS [Id] + 1');"); + } + public override async Task Add_column_with_required() { await base.Add_column_with_required(); @@ -1298,6 +1319,39 @@ FROM [sys].[default_constraints] [d] @"CREATE INDEX [IX_People_Name] ON [People] ([Name]) WHERE [Name] IS NOT NULL;"); } + [ConditionalFact] + public virtual async Task CreateIndex_generates_exec_when_filter_and_idempotent() + { + await Test( + builder => builder.Entity( + "People", e => + { + e.Property("Id"); + e.Property("Name"); + }), + builder => { }, + builder => builder.Entity("People").HasIndex("Name").HasFilter("[Name] IS NOT NULL"), + model => + { + var table = Assert.Single(model.Tables); + var index = Assert.Single(table.Indexes); + Assert.Same(table.Columns.Single(c => c.Name == "Name"), Assert.Single(index.Columns)); + Assert.Contains("Name", index.Filter); + }, + migrationsSqlGenerationOptions: MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + @"DECLARE @var0 sysname; +SELECT @var0 = [d].[name] +FROM [sys].[default_constraints] [d] +INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id] +WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'Name'); +IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];'); +ALTER TABLE [People] ALTER COLUMN [Name] nvarchar(450) NULL;", + // + @"EXEC(N'CREATE INDEX [IX_People_Name] ON [People] ([Name]) WHERE [Name] IS NOT NULL');"); + } + public override async Task Create_unique_index_with_filter() { await base.Create_unique_index_with_filter(); @@ -1925,6 +1979,28 @@ public override async Task Add_check_constraint_with_name() @"ALTER TABLE [People] ADD CONSTRAINT [CK_People_Foo] CHECK ([DriverLicense] > 0);"); } + [ConditionalFact] + public virtual async Task Add_check_constraint_generates_exec_when_idempotent() + { + await Test( + builder => builder.Entity( + "People", e => + { + e.Property("Id"); + e.Property("DriverLicense"); + }), + builder => { }, + builder => builder.Entity("People").HasCheckConstraint("CK_People_Foo", "[DriverLicense] > 0"), + model => + { + // TODO: no scaffolding support for check constraints, https://github.com/aspnet/EntityFrameworkCore/issues/15408 + }, + migrationsSqlGenerationOptions: MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + @"EXEC(N'ALTER TABLE [People] ADD CONSTRAINT [CK_People_Foo] CHECK ([DriverLicense] > 0)');"); + } + public override async Task Alter_check_constraint() { await base.Alter_check_constraint(); @@ -2142,6 +2218,87 @@ public override async Task UpdateDataOperation_multiple_columns() SELECT @@ROWCOUNT;"); } + [ConditionalFact] + public virtual async Task InsertDataOperation_generates_exec_when_idempotent() + { + await Test( + builder => builder.Entity( + "Person", e => + { + e.Property("Id"); + e.Property("Name"); + e.HasKey("Id"); + }), + builder => { }, + builder => builder.Entity("Person") + .HasData( + new Person { Id = 1, Name = "Daenerys Targaryen" }, + new Person { Id = 2, Name = "John Snow" }, + new Person { Id = 3, Name = "Arya Stark" }, + new Person { Id = 4, Name = "Harry Strickland" }, + new Person { Id = 5, Name = null }), + model => { }, + migrationsSqlGenerationOptions: MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + @"IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'Id', N'Name') AND [object_id] = OBJECT_ID(N'[Person]')) + SET IDENTITY_INSERT [Person] ON; +EXEC(N'INSERT INTO [Person] ([Id], [Name]) +VALUES (1, N''Daenerys Targaryen''), +(2, N''John Snow''), +(3, N''Arya Stark''), +(4, N''Harry Strickland''), +(5, NULL)'); +IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'Id', N'Name') AND [object_id] = OBJECT_ID(N'[Person]')) + SET IDENTITY_INSERT [Person] OFF;"); + } + + [ConditionalFact] + public virtual async Task DeleteDataOperation_generates_exec_when_idempotent() + { + await Test( + builder => builder.Entity( + "Person", e => + { + e.Property("Id"); + e.Property("Name"); + e.HasKey("Id"); + e.HasData(new Person { Id = 1, Name = "Daenerys Targaryen" }); + }), + builder => builder.Entity("Person").HasData(new Person { Id = 2, Name = "John Snow" }), + builder => { }, + model => { }, + migrationsSqlGenerationOptions: MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + @"EXEC(N'DELETE FROM [Person] +WHERE [Id] = 2; +SELECT @@ROWCOUNT');"); + } + + [ConditionalFact] + public virtual async Task UpdateDataOperation_generates_exec_when_idempotent() + { + await Test( + builder => builder.Entity( + "Person", e => + { + e.Property("Id"); + e.Property("Name"); + e.HasKey("Id"); + e.HasData(new Person { Id = 1, Name = "Daenerys Targaryen" }); + }), + builder => builder.Entity("Person").HasData(new Person { Id = 2, Name = "John Snow" }), + builder => builder.Entity("Person").HasData(new Person { Id = 2, Name = "Another John Snow" }), + model => { }, + migrationsSqlGenerationOptions: MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + @"EXEC(N'UPDATE [Person] SET [Name] = N''Another John Snow'' +WHERE [Id] = 2; +SELECT @@ROWCOUNT');"); + } + [ConditionalFact] public virtual async Task Create_temporal_table_default_column_mappings_and_default_history_table() { @@ -4442,6 +4599,62 @@ await Test( EXEC(N'ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].[CustomerHistory]))')"); } + [ConditionalFact] + public virtual async Task Convert_normal_table_to_temporal_generates_exec_when_idempotent() + { + await Test( + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("PeriodStart").ValueGeneratedOnAddOrUpdate(); + e.Property("PeriodEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + e.ToTable(tb => tb.IsTemporal()); + + e.Metadata[SqlServerAnnotationNames.TemporalPeriodStartPropertyName] = "PeriodStart"; + e.Metadata[SqlServerAnnotationNames.TemporalPeriodEndPropertyName] = "PeriodEnd"; + }), + model => + { + var table = Assert.Single(model.Tables); + Assert.Equal("Customer", table.Name); + Assert.Equal(true, table[SqlServerAnnotationNames.IsTemporal]); + Assert.Equal("CustomerHistory", table[SqlServerAnnotationNames.TemporalHistoryTableName]); + + Assert.Collection( + table.Columns, + c => Assert.Equal("Id", c.Name), + c => Assert.Equal("Name", c.Name)); + Assert.Same( + table.Columns.Single(c => c.Name == "Id"), + Assert.Single(table.PrimaryKey!.Columns)); + }, + migrationsSqlGenerationOptions: MigrationsSqlGenerationOptions.Idempotent); + + AssertSql( + @"ALTER TABLE [Customer] ADD [PeriodEnd] datetime2 NOT NULL DEFAULT '9999-12-31T23:59:59.9999999';", + // + @"ALTER TABLE [Customer] ADD [PeriodStart] datetime2 NOT NULL DEFAULT '0001-01-01T00:00:00.0000000';", + // + @"EXEC(N'ALTER TABLE [Customer] ADD PERIOD FOR SYSTEM_TIME ([PeriodStart], [PeriodEnd])')", + // + @"ALTER TABLE [Customer] ALTER COLUMN [PeriodStart] ADD HIDDEN", + // + @"ALTER TABLE [Customer] ALTER COLUMN [PeriodEnd] ADD HIDDEN", + // + @"DECLARE @historyTableSchema sysname = SCHEMA_NAME() +EXEC(N'ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].[CustomerHistory]))')"); + } + [ConditionalFact] public virtual async Task Convert_normal_table_with_period_columns_to_temporal_table_default_column_mappings_and_default_history_table() diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs index 37801b99558..cfbce687f24 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -1009,55 +1009,6 @@ public override void DefaultValue_with_line_breaks(bool isUnicode) AssertSql(expectedSql); } - [ConditionalFact] - public virtual void AddColumn_generates_exec_when_computed_and_idempotent() - { - Generate( - modelBuilder => { }, - migrationBuilder => migrationBuilder.AddColumn( - name: "Column2", - table: "Table1", - computedColumnSql: "[Column1] + 1"), - MigrationsSqlGenerationOptions.Idempotent); - - AssertSql( - @"EXEC(N'ALTER TABLE [Table1] ADD [Column2] AS [Column1] + 1'); -"); - } - - [ConditionalFact] - public virtual void AddCheckConstraint_generates_exec_when_idempotent() - { - Generate( - modelBuilder => { }, - migrationBuilder => migrationBuilder.AddCheckConstraint( - name: "CK_Table1", - table: "Table1", - "[Column1] BETWEEN 0 AND 100"), - MigrationsSqlGenerationOptions.Idempotent); - - AssertSql( - @"EXEC(N'ALTER TABLE [Table1] ADD CONSTRAINT [CK_Table1] CHECK ([Column1] BETWEEN 0 AND 100)'); -"); - } - - [ConditionalFact] - public virtual void CreateIndex_generates_exec_when_filter_and_idempotent() - { - Generate( - modelBuilder => { }, - migrationBuilder => migrationBuilder.CreateIndex( - name: "IX_Table1_Column1", - table: "Table1", - column: "Column1", - filter: "[Column1] IS NOT NULL"), - MigrationsSqlGenerationOptions.Idempotent); - - AssertSql( - @"EXEC(N'CREATE INDEX [IX_Table1_Column1] ON [Table1] ([Column1]) WHERE [Column1] IS NOT NULL'); -"); - } - [ConditionalFact] public virtual void CreateIndex_generates_exec_when_legacy_filter_and_idempotent() { @@ -1077,112 +1028,6 @@ public virtual void CreateIndex_generates_exec_when_legacy_filter_and_idempotent AssertSql( @"EXEC(N'CREATE UNIQUE INDEX [IX_Table1_Column1] ON [Table1] ([Column1]) WHERE [Column1] IS NOT NULL'); -"); - } - - [ConditionalFact] - public virtual void DeleteData_generates_exec_when_idempotent() - { - Generate( - modelBuilder => { }, - migrationBuilder => migrationBuilder - .DeleteData( - table: "Table1", - keyColumn: "Id", - keyColumnType: "int", - keyValue: 1), - MigrationsSqlGenerationOptions.Idempotent); - - AssertSql( - @"EXEC(N'DELETE FROM [Table1] -WHERE [Id] = 1; -SELECT @@ROWCOUNT'); -"); - } - - [ConditionalFact] - public virtual void InsertData_generates_exec_when_idempotent() - { - Generate( - modelBuilder => { }, - migrationBuilder => migrationBuilder - .InsertData( - table: "Table1", - column: "Id", - columnType: "int", - value: 1), - MigrationsSqlGenerationOptions.Idempotent); - - AssertSql( - @"IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'Id') AND [object_id] = OBJECT_ID(N'[Table1]')) - SET IDENTITY_INSERT [Table1] ON; -EXEC(N'INSERT INTO [Table1] ([Id]) -VALUES (1)'); -IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'Id') AND [object_id] = OBJECT_ID(N'[Table1]')) - SET IDENTITY_INSERT [Table1] OFF; -"); - } - - [ConditionalFact] - public virtual void UpdateData_generates_exec_when_idempotent() - { - Generate( - modelBuilder => { }, - migrationBuilder => - { - var operation = migrationBuilder - .UpdateData( - table: "Table1", - keyColumnTypes: new[] { "int" }, - keyColumns: new[] { "Id" }, - keyValues: new object[] { 1 }, - columns: new[] { "Column1" }, - columnTypes: new[] { "int" }, - values: new object[] { 2 }); - }, - MigrationsSqlGenerationOptions.Idempotent); - - AssertSql( - @"EXEC(N'UPDATE [Table1] SET [Column1] = 2 -WHERE [Id] = 1; -SELECT @@ROWCOUNT'); -"); - } - - [ConditionalFact] - public virtual void Converting_table_to_temporal_idempotent() - { - Generate( - builder => builder.Entity( - "Customer", e => - { - e.Property("Id").ValueGeneratedOnAdd(); - e.Property("Name"); - e.Property("PeriodStart").ValueGeneratedOnAddOrUpdate(); - e.Property("PeriodEnd").ValueGeneratedOnAddOrUpdate(); - e.HasKey("Id"); - }).HasAnnotation(RelationalAnnotationNames.Schema, "dbo"), - migrationBuilder => migrationBuilder - .AlterTable("Customer") - .Annotation(SqlServerAnnotationNames.IsTemporal, true) - .Annotation(SqlServerAnnotationNames.TemporalHistoryTableName, "CustomerHistory") - .Annotation(SqlServerAnnotationNames.TemporalPeriodStartColumnName, "PeriodStart") - .Annotation(SqlServerAnnotationNames.TemporalPeriodEndColumnName, "PeriodEnd"), - MigrationsSqlGenerationOptions.Idempotent); - - AssertSql( - @"EXEC(N'ALTER TABLE [Customer] ADD PERIOD FOR SYSTEM_TIME ([PeriodStart], [PeriodEnd])') -GO - -ALTER TABLE [Customer] ALTER COLUMN [PeriodStart] ADD HIDDEN -GO - -ALTER TABLE [Customer] ALTER COLUMN [PeriodEnd] ADD HIDDEN -GO - -DECLARE @historyTableSchema sysname = SCHEMA_NAME() -EXEC(N'ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].[CustomerHistory]))') - "); }