Fix to #26676 - EF Core 6 - IsTemporal adds an unnecessary migration …

…when DefaultSchema is configured at the DbContext Level

Problem was a discrepancy between RelationalModel build in runtime and the current model (from OnModelCreating) - relational model would set history table schema annotation using the table schema or default schema, however current model wouldn't set those annotations.
Model differ would then pick up on those differences and create migration to set the schema for history table. When building actual migration sql we already compensated for the difference and not generate actual sql code, but the problem persists.

Fix is to add missing default annotations in the model finalization step. This is a temporary measure until we implement default value conventions (#9329)

Fixes #26676

src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs

@@ -252,8 +252,7 @@ public static void SetHistoryTableName(this IMutableEntityType entityType, strin
     public static string? GetHistoryTableSchema(this IReadOnlyEntityType entityType)
         => (entityType is RuntimeEntityType)
             ? throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData)
-            : entityType[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
-            ?? entityType[RelationalAnnotationNames.Schema] as string;
+            : entityType[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string ?? entityType.GetSchema();
 
     /// <summary>
     ///     Sets a value representing the schema of the history table associated with the entity mapped to a temporal table.

src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs

@@ -118,6 +118,7 @@ public override ConventionSet CreateConventionSet()
             conventionSet.ModelFinalizingConventions,
             (SharedTableConvention)new SqlServerSharedTableConvention(Dependencies, RelationalDependencies));
         conventionSet.ModelFinalizingConventions.Add(new SqlServerDbFunctionConvention(Dependencies, RelationalDependencies));
+        conventionSet.ModelFinalizingConventions.Add(sqlServerTemporalConvention);
 
         ReplaceConvention(
             conventionSet.ModelFinalizedConventions,

src/EFCore.SqlServer/Metadata/Conventions/SqlServerTemporalConvention.cs

@@ -13,10 +13,10 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
 ///     <see href="https://aka.ms/efcore-docs-sqlserver">Accessing SQL Server and SQL Azure databases with EF Core</see>
 ///     for more information and examples.
 /// </remarks>
-public class SqlServerTemporalConvention : IEntityTypeAnnotationChangedConvention, ISkipNavigationForeignKeyChangedConvention
+public class SqlServerTemporalConvention : IEntityTypeAnnotationChangedConvention, ISkipNavigationForeignKeyChangedConvention, IModelFinalizingConvention
 {
-    private const string PeriodStartDefaultName = "PeriodStart";
-    private const string PeriodEndDefaultName = "PeriodEnd";
+    private const string DefaultPeriodStartName = "PeriodStart";
+    private const string DefaultPeriodEndName = "PeriodEnd";
 
     /// <summary>
     ///     Creates a new instance of <see cref="SqlServerTemporalConvention" />.
@@ -56,12 +56,12 @@ public virtual void ProcessEntityTypeAnnotationChanged(
             {
                 if (entityTypeBuilder.Metadata.GetPeriodStartPropertyName() == null)
                 {
-                    entityTypeBuilder.HasPeriodStart(PeriodStartDefaultName);
+                    entityTypeBuilder.HasPeriodStart(DefaultPeriodStartName);
                 }
 
                 if (entityTypeBuilder.Metadata.GetPeriodEndPropertyName() == null)
                 {
-                    entityTypeBuilder.HasPeriodEnd(PeriodEndDefaultName);
+                    entityTypeBuilder.HasPeriodEnd(DefaultPeriodEndName);
                 }
 
                 foreach (var skipLevelNavigation in entityTypeBuilder.Metadata.GetSkipNavigations())
@@ -138,4 +138,18 @@ public virtual void ProcessSkipNavigationForeignKeyChanged(
             joinEntityType.SetIsTemporal(true);
         }
     }
+
+    /// <inheritdoc />
+    public virtual void ProcessModelFinalizing(
+        IConventionModelBuilder modelBuilder,
+        IConventionContext<IConventionModelBuilder> context)
+    {
+        foreach (var entityType in modelBuilder.Metadata.GetEntityTypes().Where(e => e.IsTemporal()))
+        {
+            // Needed for the annotation to show up in the model snapshot - issue #9329
+            // history table name will always be non-null for temporal table case
+            entityType.Builder.UseHistoryTableName(entityType.GetHistoryTableName()!);
+            entityType.Builder.UseHistoryTableSchema(entityType.GetHistoryTableSchema());
+        }
+    }
 }

src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs

@@ -533,7 +533,8 @@ protected override void Generate(
         if (operation[SqlServerAnnotationNames.IsTemporal] as bool? == true)
         {
             var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
-                ?? model?.GetDefaultSchema();
+                ?? operation.Schema;
+
             var needsExec = historyTableSchema == null;
             var subBuilder = needsExec
                 ? new MigrationCommandListBuilder(Dependencies)
@@ -1261,7 +1262,7 @@ protected override void Generate(
         {
             var historyTableName = operation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
             var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
-                ?? model?.GetDefaultSchema();
+                ?? operation.Schema ?? model?.GetDefaultSchema();
             var periodStartColumnName = operation[SqlServerAnnotationNames.TemporalPeriodStartColumnName] as string;
             var periodEndColumnName = operation[SqlServerAnnotationNames.TemporalPeriodEndColumnName] as string;
 
@@ -2261,7 +2262,7 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
                 schema ??= model?.GetDefaultSchema();
                 var historyTableName = operation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
                 var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
-                    ?? model?.GetDefaultSchema();
+                    ?? schema;
                 var periodStartColumnName = operation[SqlServerAnnotationNames.TemporalPeriodStartColumnName] as string;
                 var periodEndColumnName = operation[SqlServerAnnotationNames.TemporalPeriodEndColumnName] as string;
 

test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs

@@ -2123,6 +2123,7 @@ public virtual void Temporal_table_information_is_stored_in_snapshot_minimal_set
 
                     b.ToTable(tb => tb.IsTemporal(ttb =>
                         {
+                            ttb.UseHistoryTable(""EntityWithStringPropertyHistory"");
                             ttb
                                 .HasPeriodStart(""PeriodStart"")
                                 .HasColumnName(""PeriodStart"");
@@ -2138,7 +2139,7 @@ public virtual void Temporal_table_information_is_stored_in_snapshot_minimal_set
                     "Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithStringProperty");
                 var annotations = temporalEntity.GetAnnotations().ToList();
 
-                Assert.Equal(5, annotations.Count);
+                Assert.Equal(6, annotations.Count);
                 Assert.Contains(annotations, a => a.Name == SqlServerAnnotationNames.IsTemporal && a.Value as bool? == true);
                 Assert.Contains(
                     annotations,

test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs

@@ -1062,6 +1062,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
             {
                 entity.ToTable(tb => tb.IsTemporal(ttb =>
     {
+        ttb.UseHistoryTable(""CustomerHistory"");
         ttb
             .HasPeriodStart(""PeriodStart"")
             .HasColumnName(""PeriodStart"");