diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs index 363dd025014..a4e73e6e39f 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs @@ -76,7 +76,23 @@ public virtual void ProcessEntityTypeAnnotationChanged( IConventionAnnotation? oldAnnotation, IConventionContext context) { - if (name == RelationalAnnotationNames.TableName) + if (name == RelationalAnnotationNames.ViewName + || name == RelationalAnnotationNames.FunctionName + || name == RelationalAnnotationNames.SqlQuery) + { + if (annotation?.Value != null + && oldAnnotation?.Value == null + && entityTypeBuilder.Metadata.GetTableName() == null) + { + ProcessTableChanged( + entityTypeBuilder, + entityTypeBuilder.Metadata.GetDefaultTableName(), + entityTypeBuilder.Metadata.GetDefaultSchema(), + null, + null); + } + } + else if (name == RelationalAnnotationNames.TableName) { var schema = entityTypeBuilder.Metadata.GetSchema(); ProcessTableChanged( @@ -105,29 +121,43 @@ private static void ProcessTableChanged( string? newTable, string? newSchema) { + if (newTable == null) + { + foreach (var property in entityTypeBuilder.Metadata.GetProperties()) + { + property.Builder.ValueGenerated(null); + } + + return; + } + else if (oldTable == null) + { + foreach (var property in entityTypeBuilder.Metadata.GetProperties()) + { + property.Builder.ValueGenerated(GetValueGenerated(property, StoreObjectIdentifier.Table(newTable, newSchema))); + } + + return; + } + var primaryKey = entityTypeBuilder.Metadata.FindPrimaryKey(); if (primaryKey == null) { return; } - var oldLink = oldTable != null - ? entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(oldTable, oldSchema)) - : null; - var newLink = newTable != null - ? entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(newTable, newSchema)) - : null; + var oldLink = entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(oldTable, oldSchema)); + var newLink = entityTypeBuilder.Metadata.FindRowInternalForeignKeys(StoreObjectIdentifier.Table(newTable, newSchema)); - if ((oldLink?.Any() != true - && newLink?.Any() != true) - || newLink == null) + if (!oldLink.Any() + && !newLink.Any()) { return; } foreach (var property in primaryKey.Properties) { - property.Builder.ValueGenerated(GetValueGenerated(property, StoreObjectIdentifier.Table(newTable!, newSchema))); + property.Builder.ValueGenerated(GetValueGenerated(property, StoreObjectIdentifier.Table(newTable, newSchema))); } } diff --git a/src/EFCore.Relational/Metadata/Internal/ColumnMappingBaseComparer.cs b/src/EFCore.Relational/Metadata/Internal/ColumnMappingBaseComparer.cs index 32bd80aa5e6..53923f46e0b 100644 --- a/src/EFCore.Relational/Metadata/Internal/ColumnMappingBaseComparer.cs +++ b/src/EFCore.Relational/Metadata/Internal/ColumnMappingBaseComparer.cs @@ -52,19 +52,19 @@ public int Compare(IColumnMappingBase? x, IColumnMappingBase? y) return result; } - result = StringComparer.Ordinal.Compare(x.Property.Name, y.Property.Name); + result = TableMappingBaseComparer.Instance.Compare(x.TableMapping, y.TableMapping); if (result != 0) { return result; } - result = StringComparer.Ordinal.Compare(x.Column.Name, y.Column.Name); + result = StringComparer.Ordinal.Compare(x.Property.Name, y.Property.Name); if (result != 0) { return result; } - return TableMappingBaseComparer.Instance.Compare(x.TableMapping, y.TableMapping); + return StringComparer.Ordinal.Compare(x.Column.Name, y.Column.Name); } /// diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs index 1ef190f855d..07dc3e9d6b8 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs @@ -3,7 +3,6 @@ using System.Collections; using System.Globalization; -using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Update.Internal; diff --git a/src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs index 3dca0c289fc..2cfa3f3d202 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs @@ -406,31 +406,53 @@ internal static SqlServerValueGenerationStrategy GetValueGenerationStrategy( ITypeMappingSource? typeMappingSource) { var annotation = property.FindAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy); - if (annotation != null) + if (annotation?.Value != null + && StoreObjectIdentifier.Create(property.DeclaringEntityType, storeObject.StoreObjectType) == storeObject) { - return (SqlServerValueGenerationStrategy?)annotation.Value ?? SqlServerValueGenerationStrategy.None; + return (SqlServerValueGenerationStrategy)annotation.Value; } + var table = storeObject; var sharedTableRootProperty = property.FindSharedStoreObjectRootProperty(storeObject); if (sharedTableRootProperty != null) { - return sharedTableRootProperty.GetValueGenerationStrategy(storeObject) + return sharedTableRootProperty.GetValueGenerationStrategy(storeObject, typeMappingSource) == SqlServerValueGenerationStrategy.IdentityColumn - && property.GetContainingForeignKeys().All(fk => fk.IsBaseLinking()) + && table.StoreObjectType == StoreObjectType.Table + && !property.GetContainingForeignKeys().Any(fk => + !fk.IsBaseLinking() + || (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table) + is StoreObjectIdentifier principal + && fk.GetConstraintName(table, principal) != null)) ? SqlServerValueGenerationStrategy.IdentityColumn : SqlServerValueGenerationStrategy.None; } if (property.ValueGenerated != ValueGenerated.OnAdd - || property.GetContainingForeignKeys().Any(fk => !fk.IsBaseLinking()) + || table.StoreObjectType != StoreObjectType.Table || property.TryGetDefaultValue(storeObject, out _) || property.GetDefaultValueSql(storeObject) != null - || property.GetComputedColumnSql(storeObject) != null) + || property.GetComputedColumnSql(storeObject) != null + || property.GetContainingForeignKeys() + .Any(fk => + !fk.IsBaseLinking() + || (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table) + is StoreObjectIdentifier principal + && fk.GetConstraintName(table, principal) != null))) { return SqlServerValueGenerationStrategy.None; } - return GetDefaultValueGenerationStrategy(property, storeObject, typeMappingSource); + var defaultStategy = GetDefaultValueGenerationStrategy(property, storeObject, typeMappingSource); + if (defaultStategy != SqlServerValueGenerationStrategy.None) + { + if (annotation != null) + { + return (SqlServerValueGenerationStrategy?)annotation.Value ?? SqlServerValueGenerationStrategy.None; + } + } + + return defaultStategy; } private static SqlServerValueGenerationStrategy GetDefaultValueGenerationStrategy(IReadOnlyProperty property) @@ -455,7 +477,6 @@ private static SqlServerValueGenerationStrategy GetDefaultValueGenerationStrateg ITypeMappingSource? typeMappingSource) { var modelStrategy = property.DeclaringEntityType.Model.GetValueGenerationStrategy(); - if (modelStrategy == SqlServerValueGenerationStrategy.SequenceHiLo && IsCompatibleWithValueGeneration(property, storeObject, typeMappingSource)) { diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs index 8e890a7f710..15f5ffc05ee 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationConvention.cs @@ -102,6 +102,8 @@ public override void ProcessEntityTypeAnnotationChanged( return null; } + // If the first mapping can be value generated then we'll consider all mappings to be value generated + // as this is a client-side configuration and can't be specified per-table. return GetValueGenerated(property, declaringTable, Dependencies.TypeMappingSource); } diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationStrategyConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationStrategyConvention.cs index c1350de2664..7c491d53e7e 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationStrategyConvention.cs +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerValueGenerationStrategyConvention.cs @@ -1,8 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. - - // ReSharper disable once CheckNamespace namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; diff --git a/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs b/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs index fd9f2604c1f..5cb7491fdf0 100644 --- a/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs +++ b/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs @@ -219,8 +219,7 @@ public override IEnumerable For(IColumn column, bool designTime) var table = StoreObjectIdentifier.Table(column.Table.Name, column.Table.Schema); var identityProperty = column.PropertyMappings.Where( - m => (m.TableMapping.IsSharedTablePrincipal ?? true) - && m.TableMapping.EntityType == m.Property.DeclaringEntityType) + m => m.TableMapping.EntityType == m.Property.DeclaringEntityType) .Select(m => m.Property) .FirstOrDefault( p => p.GetValueGenerationStrategy(table) diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index 16a5e3212c2..1385e3b7c32 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -574,8 +574,6 @@ public void Views_are_stored_in_the_model_snapshot() b.Property(""Id"") .HasColumnType(""int""); - SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property(""Id""), 1L, 1); - b.HasKey(""Id""); b.ToView(""EntityWithOneProperty"", (string)null); @@ -596,8 +594,6 @@ public void Views_with_schemas_are_stored_in_the_model_snapshot() b.Property(""Id"") .HasColumnType(""int""); - SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property(""Id""), 1L, 1); - b.HasKey(""Id""); b.ToView(""EntityWithOneProperty"", ""ViewSchema""); @@ -947,8 +943,6 @@ public virtual void Entity_splitting_is_stored_in_snapshot_with_views() b.Property(""Id"") .HasColumnType(""int""); - SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property(""Id""), 1L, 1); - b.Property(""Shadow"") .HasColumnType(""int""); @@ -1040,7 +1034,6 @@ public void Unmapped_entity_types_are_stored_in_the_model_snapshot() modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", b => { b.Property(""Id"") - .ValueGeneratedOnAdd() .HasColumnType(""int""); b.HasKey(""Id""); @@ -1122,7 +1115,6 @@ public void Entity_types_mapped_to_queries_are_stored_in_the_model_snapshot() modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithOneProperty"", b => { b.Property(""Id"") - .ValueGeneratedOnAdd() .HasColumnType(""int""); b.HasKey(""Id""); @@ -3329,8 +3321,6 @@ protected override void BuildModel(ModelBuilder modelBuilder) b1.Property(""Id"") .HasColumnType(""int""); - SqlServerPropertyBuilderExtensions.UseIdentityColumn(b1.Property(""Id""), 1L, 1); - b1.Property(""TestEnum"") .HasColumnType(""int""); diff --git a/test/EFCore.Relational.Specification.Tests/EntitySplittingTestBase.cs b/test/EFCore.Relational.Specification.Tests/EntitySplittingTestBase.cs new file mode 100644 index 00000000000..8bb4e5698c6 --- /dev/null +++ b/test/EFCore.Relational.Specification.Tests/EntitySplittingTestBase.cs @@ -0,0 +1,111 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// ReSharper disable InconsistentNaming +namespace Microsoft.EntityFrameworkCore; + +public abstract class EntitySplittingTestBase : NonSharedModelTestBase +{ + protected EntitySplittingTestBase(ITestOutputHelper testOutputHelper) + { + //TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper); + } + + [ConditionalFact(Skip = "Entity splitting query Issue #620")] + public virtual async Task Can_roundtrip() + { + await InitializeAsync(OnModelCreating, sensitiveLogEnabled: false); + + await using (var context = CreateContext()) + { + var meterReading = new MeterReading { ReadingStatus = MeterReadingStatus.NotAccesible, CurrentRead = "100" }; + + context.Add(meterReading); + + TestSqlLoggerFactory.Clear(); + + await context.SaveChangesAsync(); + + Assert.Empty(TestSqlLoggerFactory.Log.Where(l => l.Level == LogLevel.Warning)); + } + + await using (var context = CreateContext()) + { + var reading = await context.MeterReadings.SingleAsync(); + + Assert.Equal(MeterReadingStatus.NotAccesible, reading.ReadingStatus); + Assert.Equal("100", reading.CurrentRead); + } + } + + protected override string StoreName { get; } = "EntitySplittingTest"; + + protected TestSqlLoggerFactory TestSqlLoggerFactory + => (TestSqlLoggerFactory)ListLoggerFactory; + + protected ContextFactory ContextFactory { get; private set; } + + protected void AssertSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected); + + protected virtual void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity( + ob => + { + ob.ToTable("MeterReadings"); + ob.SplitToTable( + "MeterReadingDetails", t => + { + t.Property(o => o.PreviousRead); + t.Property(o => o.CurrentRead); + }); + }); + } + + protected async Task InitializeAsync(Action onModelCreating, bool sensitiveLogEnabled = true) + => ContextFactory = await InitializeAsync( + onModelCreating, + shouldLogCategory: _ => true, + onConfiguring: options => + { + options.ConfigureWarnings(w => w.Log(RelationalEventId.OptionalDependentWithAllNullPropertiesWarning)) + .ConfigureWarnings(w => w.Log(RelationalEventId.OptionalDependentWithoutIdentifyingPropertyWarning)) + .EnableSensitiveDataLogging(sensitiveLogEnabled); + } + ); + + protected virtual EntitySplittingContext CreateContext() + => ContextFactory.CreateContext(); + + public override void Dispose() + { + base.Dispose(); + + ContextFactory = null; + } + + protected class EntitySplittingContext : PoolableDbContext + { + public EntitySplittingContext(DbContextOptions options) + : base(options) + { + } + + public DbSet MeterReadings { get; set; } + } + + protected class MeterReading + { + public int Id { get; set; } + public MeterReadingStatus? ReadingStatus { get; set; } + public string CurrentRead { get; set; } + public string PreviousRead { get; set; } + } + + protected enum MeterReadingStatus + { + Running = 0, + NotAccesible = 2 + } +} diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs index d7b88032f36..d68920617b0 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs @@ -1376,6 +1376,91 @@ public void Can_split_entity_in_two_using_shared_table_with_seed_data() upOps => Assert.Equal(0, upOps.Count), downOps => Assert.Equal(0, downOps.Count)); + [ConditionalFact] + public void Can_add_tables_with_entity_splitting_with_seed_data() + => Execute( + _ => { }, + _ => { }, + modelBuilder => + { + modelBuilder.Entity( + "Animal", + x => + { + x.Property("Id"); + x.Property("MouseId"); + x.Property("BoneId"); + x.HasData( + new + { + Id = 42, + MouseId = "1", + BoneId = "2" + }); + x.SplitToTable("AnimalDetails", t => + { + t.Property("BoneId"); + }); + }); + }, + upOps => Assert.Collection( + upOps, + o => + { + var m = Assert.IsType(o); + Assert.Equal("Animal", m.Name); + Assert.Equal("Id", m.PrimaryKey.Columns.Single()); + Assert.Equal(new[] { "Id", "MouseId" }, m.Columns.Select(c => c.Name)); + Assert.Empty(m.ForeignKeys); + }, + o => + { + var m = Assert.IsType(o); + Assert.Equal("AnimalDetails", m.Name); + Assert.Equal("Id", m.PrimaryKey.Columns.Single()); + Assert.Equal(new[] { "Id", "BoneId" }, m.Columns.Select(c => c.Name)); + var fk = m.ForeignKeys.Single(); + Assert.Equal("Animal", fk.PrincipalTable); + }, + o => + { + var m = Assert.IsType(o); + Assert.Equal("Animal", m.Table); + AssertMultidimensionalArray( + m.Values, + v => Assert.Equal(42, v), + v => Assert.Equal("1", v)); + Assert.Collection( + m.Columns, + v => Assert.Equal("Id", v), + v => Assert.Equal("MouseId", v)); + }, + o => + { + var m = Assert.IsType(o); + Assert.Equal("AnimalDetails", m.Table); + AssertMultidimensionalArray( + m.Values, + v => Assert.Equal(42, v), + v => Assert.Equal("2", v)); + Assert.Collection( + m.Columns, + v => Assert.Equal("Id", v), + v => Assert.Equal("BoneId", v)); + }), + downOps => Assert.Collection( + downOps, + o => + { + var m = Assert.IsType(o); + Assert.Equal("AnimalDetails", m.Name); + }, + o => + { + var m = Assert.IsType(o); + Assert.Equal("Animal", m.Name); + })); + [ConditionalFact] public void Add_owned_type_with_seed_data() => Execute( diff --git a/test/EFCore.Specification.Tests/NonSharedModelTestBase.cs b/test/EFCore.Specification.Tests/NonSharedModelTestBase.cs index 800d7b8ab69..12c523dafe9 100644 --- a/test/EFCore.Specification.Tests/NonSharedModelTestBase.cs +++ b/test/EFCore.Specification.Tests/NonSharedModelTestBase.cs @@ -16,14 +16,14 @@ public abstract class NonSharedModelTestBase : IDisposable, IAsyncLifetime protected IServiceProvider ServiceProvider => _serviceProvider ?? throw new InvalidOperationException( - $"You must call `await {nameof(InitializeAsync)}(\"DatabaseName\");` at the beggining of the test."); + $"You must call `await {nameof(InitializeAsync)}(\"DatabaseName\");` at the beginning of the test."); private TestStore _testStore; protected TestStore TestStore => _testStore ?? throw new InvalidOperationException( - $"You must call `await {nameof(InitializeAsync)}(\"DatabaseName\");` at the beggining of the test."); + $"You must call `await {nameof(InitializeAsync)}(\"DatabaseName\");` at the beginning of the test."); private ListLoggerFactory _listLoggerFactory; diff --git a/test/EFCore.SqlServer.FunctionalTests/EntitySplittingSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/EntitySplittingSqlServerTest.cs new file mode 100644 index 00000000000..881e49d40ea --- /dev/null +++ b/test/EFCore.SqlServer.FunctionalTests/EntitySplittingSqlServerTest.cs @@ -0,0 +1,20 @@ +// 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; + +public class EntitySplittingSqlServerTest : EntitySplittingTestBase +{ + public EntitySplittingSqlServerTest(ITestOutputHelper testOutputHelper) + : base(testOutputHelper) + { + } + + protected override ITestStoreFactory TestStoreFactory + => SqlServerTestStoreFactory.Instance; + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + base.OnModelCreating(modelBuilder); + } +} diff --git a/test/EFCore.Sqlite.FunctionalTests/EntitySplittingSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/EntitySplittingSqliteTest.cs new file mode 100644 index 00000000000..d1d102de2c1 --- /dev/null +++ b/test/EFCore.Sqlite.FunctionalTests/EntitySplittingSqliteTest.cs @@ -0,0 +1,20 @@ +// 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; + +public class EntitySplittingSqliteTest : EntitySplittingTestBase +{ + public EntitySplittingSqliteTest(ITestOutputHelper testOutputHelper) + : base(testOutputHelper) + { + } + + protected override ITestStoreFactory TestStoreFactory + => SqliteTestStoreFactory.Instance; + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + base.OnModelCreating(modelBuilder); + } +}