Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to #26676 - EF Core 6 - IsTemporal adds an unnecessary migration when DefaultSchema is configured at the DbContext Level #27198

Merged
merged 1 commit into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />.
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ protected override void Generate(
{
var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
?? model?.GetDefaultSchema();

var needsExec = historyTableSchema == null;
var subBuilder = needsExec
? new MigrationCommandListBuilder(Dependencies)
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"");
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
{
entity.ToTable(tb => tb.IsTemporal(ttb =>
{
ttb.UseHistoryTable(""CustomerHistory"");
ttb
.HasPeriodStart(""PeriodStart"")
.HasColumnName(""PeriodStart"");
Expand Down
Loading