From 7430bbee3b3090df62b59e5ed763738ede9cbf74 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Wed, 28 Mar 2018 18:10:40 -0700 Subject: [PATCH] Don't enable IDENTITY_INSERT when it is not needed Add seed data to some test fixtures Fixes #11115 Fixes #10653 --- .../BuiltInDataTypesTestBase.cs | 69 +++++++- .../Query/OwnedQueryTestBase.cs | 150 ++++++++++++------ .../SqlServerMigrationsSqlGenerator.cs | 42 ++--- .../Query/OwnedQueryInMemoryTest.cs | 57 ++++++- .../Query/OwnedQuerySqlServerTest.cs | 1 - .../SqlServerMigrationSqlGeneratorTest.cs | 4 +- 6 files changed, 242 insertions(+), 81 deletions(-) diff --git a/src/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs b/src/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs index d737681983d..bef802ca5d5 100644 --- a/src/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs +++ b/src/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs @@ -1144,9 +1144,74 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con { modelBuilder.Entity(); modelBuilder.Entity(); - modelBuilder.Entity().Property(e => e.Id).ValueGeneratedNever(); + modelBuilder.Entity(eb => + { + eb.HasData( + new BuiltInDataTypes + { + Id = 13, + PartitionId = 1, + TestInt16 = -1234, + TestInt32 = -123456789, + TestInt64 = -1234567890123456789L, + TestDouble = -1.23456789, + TestDecimal = -1234567890.01M, + TestDateTime = DateTime.Parse("01/01/2000 12:34:56"), + TestDateTimeOffset = new DateTimeOffset(DateTime.Parse("01/01/2000 12:34:56"), TimeSpan.FromHours(-8.0)), + TestTimeSpan = new TimeSpan(0, 10, 9, 8, 7), + TestSingle = -1.234F, + TestBoolean = true, + TestByte = 255, + TestUnsignedInt16 = 1234, + TestUnsignedInt32 = 1234565789U, + TestUnsignedInt64 = 1234567890123456789UL, + TestCharacter = 'a', + TestSignedByte = -128, + Enum64 = Enum64.SomeValue, + Enum32 = Enum32.SomeValue, + Enum16 = Enum16.SomeValue, + Enum8 = Enum8.SomeValue, + EnumU64 = EnumU64.SomeValue, + EnumU32 = EnumU32.SomeValue, + EnumU16 = EnumU16.SomeValue, + EnumS8 = EnumS8.SomeValue + }); + eb.Property(e => e.Id).ValueGeneratedNever(); + }); modelBuilder.Entity().Property(e => e.Id).ValueGeneratedNever(); - modelBuilder.Entity().Property(e => e.Id).ValueGeneratedNever(); + modelBuilder.Entity(eb => + { + eb.HasData( + new BuiltInNullableDataTypes + { + Id = 13, + PartitionId = 1, + TestNullableInt16 = -1234, + TestNullableInt32 = -123456789, + TestNullableInt64 = -1234567890123456789L, + TestNullableDouble = -1.23456789, + TestNullableDecimal = -1234567890.01M, + TestNullableDateTimeOffset = new DateTimeOffset(new DateTime(), TimeSpan.FromHours(-8.0)), + TestNullableTimeSpan = new TimeSpan(0, 10, 9, 8, 7), + TestNullableSingle = -1.234F, + TestNullableBoolean = true, + TestNullableByte = 255, + TestNullableUnsignedInt16 = 1234, + TestNullableUnsignedInt32 = 1234565789U, + TestNullableUnsignedInt64 = 1234567890123456789UL, + TestNullableCharacter = 'a', + TestNullableSignedByte = -128, + Enum64 = Enum64.SomeValue, + Enum32 = Enum32.SomeValue, + Enum16 = Enum16.SomeValue, + Enum8 = Enum8.SomeValue, + EnumU64 = EnumU64.SomeValue, + EnumU32 = EnumU32.SomeValue, + EnumU16 = EnumU16.SomeValue, + EnumS8 = EnumS8.SomeValue + }); + eb.Property(e => e.Id).ValueGeneratedNever(); + }); modelBuilder.Entity().Property(e => e.Id).ValueGeneratedNever(); modelBuilder.Entity().Property(e => e.Id).ValueGeneratedNever(); modelBuilder.Entity().Property(e => e.Id).ValueGeneratedNever(); diff --git a/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs b/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs index 3aa46ead8d9..28645b5364f 100644 --- a/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs +++ b/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs @@ -112,68 +112,126 @@ public abstract class OwnedQueryFixtureBase : SharedStoreFixtureBase protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context) { - modelBuilder.Entity().OwnsOne(p => p.PersonAddress).OwnsOne(a => a.Country); - modelBuilder.Entity().OwnsOne(p => p.BranchAddress).OwnsOne(a => a.Country); - modelBuilder.Entity().OwnsOne(p => p.LeafAAddress).OwnsOne(a => a.Country); - modelBuilder.Entity().OwnsOne(p => p.LeafBAddress).OwnsOne(a => a.Country); - } - - public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder) - { - return base.AddOptions(builder).ConfigureWarnings(wcb => wcb.Throw()); - } + modelBuilder.Entity(eb => + { + eb.HasData(new OwnedPerson + { + Id = 1 + }); - protected override void Seed(DbContext context) - { - context.Set().AddRange( - new OwnedPerson + eb.OwnsOne(p => p.PersonAddress, ab => { - PersonAddress = new OwnedAddress + ab.HasData(new { - Country = new OwnedCountry { Name = "USA" } - } - }, - new Branch - { - PersonAddress = new OwnedAddress + OwnedPersonId = 1 + }, new + { + OwnedPersonId = 2 + }, new { - Country = new OwnedCountry { Name = "USA" } - }, - BranchAddress = new OwnedAddress + OwnedPersonId = 3 + }, new { - Country = new OwnedCountry { Name = "Canada" } - } - }, - new LeafA + OwnedPersonId = 4 + }); + + ab.OwnsOne(a => a.Country).HasData(new + { + OwnedAddressOwnedPersonId = 1, + Name = "USA" + }, new + { + OwnedAddressOwnedPersonId = 2, + Name = "USA" + }, new + { + OwnedAddressOwnedPersonId = 3, + Name = "USA" + }, new + { + OwnedAddressOwnedPersonId = 4, + Name = "USA" + }); + }); + }); + + modelBuilder.Entity(eb => + { + eb.HasData(new Branch + { + Id = 2 + }); + + eb.OwnsOne(p => p.BranchAddress, ab => { - PersonAddress = new OwnedAddress + ab.HasData(new + { + BranchId = 2 + },new { - Country = new OwnedCountry { Name = "USA" } - }, - BranchAddress = new OwnedAddress + BranchId = 3 + }); + + ab.OwnsOne(a => a.Country).HasData(new { - Country = new OwnedCountry { Name = "Canada" } - }, - LeafAAddress = new OwnedAddress + OwnedAddressBranchId = 2, + Name = "Canada" + },new { - Country = new OwnedCountry { Name = "Mexico" } - } - }, - new LeafB + OwnedAddressBranchId = 3, + Name = "Canada" + }); + }); + }); + + modelBuilder.Entity(eb => + { + eb.HasData(new LeafA { - PersonAddress = new OwnedAddress + Id = 3 + }); + + eb.OwnsOne(p => p.LeafAAddress, ab => + { + ab.HasData(new { - Country = new OwnedCountry { Name = "USA" } - }, - LeafBAddress = new OwnedAddress + LeafAId = 3 + }); + + ab.OwnsOne(a => a.Country).HasData(new { - Country = new OwnedCountry { Name = "Panama" } - } + OwnedAddressLeafAId = 3, + Name = "Mexico" + }); }); + }); - context.SaveChanges(); + modelBuilder.Entity(eb => + { + eb.HasData(new LeafB + { + Id = 4 + }); + + eb.OwnsOne(p => p.LeafBAddress, ab => + { + ab.HasData(new + { + LeafBId = 4 + }); + + ab.OwnsOne(a => a.Country).HasData(new + { + OwnedAddressLeafBId = 4, + Name = "Panama" + }); + }); + }); } + public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder) + => base.AddOptions(builder).ConfigureWarnings(wcb => wcb.Throw()); + public override DbContext CreateContext() { var context = base.CreateContext(); diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index 26639eee9eb..6a208b27667 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -9,7 +9,6 @@ using System.Text.RegularExpressions; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Infrastructure; -using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Migrations.Operations; using Microsoft.EntityFrameworkCore.SqlServer.Internal; @@ -1045,23 +1044,7 @@ protected override void Generate( Check.NotNull(operation, nameof(operation)); Check.NotNull(builder, nameof(builder)); - var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); - - builder - .Append("IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(") - .Append( - stringTypeMapping.GenerateSqlLiteral( - Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))) - .AppendLine("))"); - - using (builder.Indent()) - { - builder - .Append("SET IDENTITY_INSERT ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) - .Append(" ON") - .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); - } + GenerateIdentityInsert(builder, operation, on: true); var sqlBuilder = new StringBuilder(); ((SqlServerUpdateSqlGenerator)Dependencies.UpdateSqlGenerator).AppendBulkInsertOperation( @@ -1071,11 +1054,22 @@ protected override void Generate( builder.Append(sqlBuilder.ToString()); + GenerateIdentityInsert(builder, operation, on: false); + + builder.EndCommand(); + } + + private void GenerateIdentityInsert(MigrationCommandListBuilder builder, InsertDataOperation operation, bool on) + { + var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); + builder - .Append("IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(") - .Append( - stringTypeMapping.GenerateSqlLiteral( - Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))) + .Append("IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE") + .Append(" [name] IN (") + .Append(string.Join(", ", operation.Columns.Select(stringTypeMapping.GenerateSqlLiteral))) + .Append(") AND [object_id] = OBJECT_ID(") + .Append(stringTypeMapping.GenerateSqlLiteral( + Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))) .AppendLine("))"); using (builder.Indent()) @@ -1083,11 +1077,9 @@ protected override void Generate( builder .Append("SET IDENTITY_INSERT ") .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) - .Append(" OFF") + .Append(on ? " ON" : " OFF") .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); } - - builder.EndCommand(); } /// diff --git a/test/EFCore.InMemory.FunctionalTests/Query/OwnedQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/OwnedQueryInMemoryTest.cs index e25e4007356..409b7da8168 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/OwnedQueryInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/OwnedQueryInMemoryTest.cs @@ -14,14 +14,61 @@ public OwnedQueryInMemoryTest(OwnedQueryInMemoryFixture fixture, ITestOutputHelp //TestLoggerFactory.TestOutputHelper = testOutputHelper; } - public override void No_ignored_include_warning_when_implicit_load() - { - base.No_ignored_include_warning_when_implicit_load(); - } - public class OwnedQueryInMemoryFixture : OwnedQueryFixtureBase { protected override ITestStoreFactory TestStoreFactory => InMemoryTestStoreFactory.Instance; + + // #11474 + protected override void Seed(DbContext context) + { + context.Set().AddRange( + new OwnedPerson + { + PersonAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "USA" } + } + }, + new Branch + { + PersonAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "USA" } + }, + BranchAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "Canada" } + } + }, + new LeafA + { + PersonAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "USA" } + }, + BranchAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "Canada" } + }, + LeafAAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "Mexico" } + } + }, + new LeafB + { + PersonAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "USA" } + }, + LeafBAddress = new OwnedAddress + { + Country = new OwnedCountry { Name = "Panama" } + } + }); + + context.SaveChanges(); + } } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs index 86c7243ed91..101964ceec4 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs @@ -18,7 +18,6 @@ public override void Query_for_base_type_loads_all_owned_navs() { base.Query_for_base_type_loads_all_owned_navs(); - // See issue #10067 AssertSql( @"SELECT [o].[Id], [o].[Discriminator], [t].[Id], [t0].[Id], [t0].[LeafBAddress_Country_Name], [t1].[Id], [t2].[Id], [t2].[LeafAAddress_Country_Name], [t3].[Id], [t4].[Id], [t4].[BranchAddress_Country_Name], [t5].[Id], [t6].[Id], [t6].[PersonAddress_Country_Name] diff --git a/test/EFCore.SqlServer.FunctionalTests/SqlServerMigrationSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/SqlServerMigrationSqlGeneratorTest.cs index c9d7860a190..a77deca842e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/SqlServerMigrationSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/SqlServerMigrationSqlGeneratorTest.cs @@ -1140,7 +1140,7 @@ public override void InsertDataOperation() base.InsertDataOperation(); Assert.Equal( - "IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(N'[People]'))" + EOL + + "IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'Id', N'Full Name') AND [object_id] = OBJECT_ID(N'[People]'))" + EOL + " SET IDENTITY_INSERT [People] ON;" + EOL + "INSERT INTO [People] ([Id], [Full Name])" + EOL + "VALUES (0, NULL)," + EOL + @@ -1148,7 +1148,7 @@ public override void InsertDataOperation() "(2, N'John Snow')," + EOL + "(3, N'Arya Stark')," + EOL + "(4, N'Harry Strickland');" + EOL + - "IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [object_id] = OBJECT_ID(N'[People]'))" + EOL + + "IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'Id', N'Full Name') AND [object_id] = OBJECT_ID(N'[People]'))" + EOL + " SET IDENTITY_INSERT [People] OFF;" + EOL, Sql); }