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

Addressing API Review feedback for temporal tables feature: #25430

Merged
merged 1 commit into from
Aug 6, 2021
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 @@ -100,19 +100,19 @@ public override IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
? annotations[SqlServerAnnotationNames.TemporalHistoryTableSchema].Value as string
: null;

var periodStartProperty = entityType.GetProperty(entityType.GetTemporalPeriodStartPropertyName()!);
var periodEndProperty = entityType.GetProperty(entityType.GetTemporalPeriodEndPropertyName()!);
var periodStartProperty = entityType.GetProperty(entityType.GetPeriodStartPropertyName()!);
var periodEndProperty = entityType.GetProperty(entityType.GetPeriodEndPropertyName()!);
var periodStartColumnName = periodStartProperty[RelationalAnnotationNames.ColumnName] as string;
var periodEndColumnName = periodEndProperty[RelationalAnnotationNames.ColumnName] as string;

// ttb => ttb.WithHistoryTable("HistoryTable", "schema")
// ttb => ttb.UseHistoryTable("HistoryTable", "schema")
var temporalTableBuilderCalls = new List<MethodCallCodeFragment>();
if (historyTableName != null)
{
temporalTableBuilderCalls.Add(
historyTableSchema != null
? new MethodCallCodeFragment(nameof(TemporalTableBuilder.WithHistoryTable), historyTableName, historyTableSchema)
: new MethodCallCodeFragment(nameof(TemporalTableBuilder.WithHistoryTable), historyTableName));
? new MethodCallCodeFragment(nameof(TemporalTableBuilder.UseHistoryTable), historyTableName, historyTableSchema)
: new MethodCallCodeFragment(nameof(TemporalTableBuilder.UseHistoryTable), historyTableName));
}

// ttb => ttb.HasPeriodStart("Start").HasColumnName("ColumnStart")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,14 @@ public static bool CanSetIsTemporal(
/// The same builder instance if the configuration was applied,
/// <see langword="null" /> otherwise.
/// </returns>
public static IConventionEntityTypeBuilder? WithHistoryTableName(
public static IConventionEntityTypeBuilder? UseHistoryTableName(
this IConventionEntityTypeBuilder entityTypeBuilder,
string name,
bool fromDataAnnotation = false)
{
if (entityTypeBuilder.CanSetHistoryTableName(name, fromDataAnnotation))
{
entityTypeBuilder.Metadata.SetTemporalHistoryTableName(name, fromDataAnnotation);
entityTypeBuilder.Metadata.SetHistoryTableName(name, fromDataAnnotation);

return entityTypeBuilder;
}
Expand Down Expand Up @@ -212,14 +212,14 @@ public static bool CanSetHistoryTableName(
/// The same builder instance if the configuration was applied,
/// <see langword="null" /> otherwise.
/// </returns>
public static IConventionEntityTypeBuilder? WithHistoryTableSchema(
public static IConventionEntityTypeBuilder? UseHistoryTableSchema(
this IConventionEntityTypeBuilder entityTypeBuilder,
string? schema,
bool fromDataAnnotation = false)
{
if (entityTypeBuilder.CanSetHistoryTableSchema(schema, fromDataAnnotation))
{
entityTypeBuilder.Metadata.SetTemporalHistoryTableSchema(schema, fromDataAnnotation);
entityTypeBuilder.Metadata.SetHistoryTableSchema(schema, fromDataAnnotation);

return entityTypeBuilder;
}
Expand Down Expand Up @@ -261,7 +261,7 @@ public static bool CanSetHistoryTableSchema(
{
if (entityTypeBuilder.CanSetPeriodStart(propertyName, fromDataAnnotation))
{
entityTypeBuilder.Metadata.SetTemporalPeriodStartPropertyName(propertyName, fromDataAnnotation);
entityTypeBuilder.Metadata.SetPeriodStartPropertyName(propertyName, fromDataAnnotation);

return entityTypeBuilder;
}
Expand Down Expand Up @@ -303,7 +303,7 @@ public static bool CanSetPeriodStart(
{
if (entityTypeBuilder.CanSetPeriodEnd(propertyName, fromDataAnnotation))
{
entityTypeBuilder.Metadata.SetTemporalPeriodEndPropertyName(propertyName, fromDataAnnotation);
entityTypeBuilder.Metadata.SetPeriodEndPropertyName(propertyName, fromDataAnnotation);

return entityTypeBuilder;
}
Expand Down
32 changes: 16 additions & 16 deletions src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ public static void SetIsTemporal(this IMutableEntityType entityType, bool tempor
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> Name of the period start property. </returns>
public static string? GetTemporalPeriodStartPropertyName(this IReadOnlyEntityType entityType)
public static string? GetPeriodStartPropertyName(this IReadOnlyEntityType entityType)
=> entityType[SqlServerAnnotationNames.TemporalPeriodStartPropertyName] as string;

/// <summary>
/// Sets a value representing the name of the period start property of the entity mapped to a temporal table.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="periodStartPropertyName"> The value to set. </param>
public static void SetTemporalPeriodStartPropertyName(this IMutableEntityType entityType, string? periodStartPropertyName)
public static void SetPeriodStartPropertyName(this IMutableEntityType entityType, string? periodStartPropertyName)
=> entityType.SetAnnotation(SqlServerAnnotationNames.TemporalPeriodStartPropertyName, periodStartPropertyName);

/// <summary>
Expand All @@ -119,7 +119,7 @@ public static void SetTemporalPeriodStartPropertyName(this IMutableEntityType en
/// <param name="periodStartPropertyName"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
public static string? SetTemporalPeriodStartPropertyName(
public static string? SetPeriodStartPropertyName(
this IConventionEntityType entityType,
string? periodStartPropertyName,
bool fromDataAnnotation = false)
Expand All @@ -137,23 +137,23 @@ public static void SetTemporalPeriodStartPropertyName(this IMutableEntityType en
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> The configuration source for the temporal table period start property name setting. </returns>
public static ConfigurationSource? GetTemporalPeriodStartPropertyNameConfigurationSource(this IConventionEntityType entityType)
public static ConfigurationSource? GetPeriodStartPropertyNameConfigurationSource(this IConventionEntityType entityType)
=> entityType.FindAnnotation(SqlServerAnnotationNames.TemporalPeriodStartPropertyName)?.GetConfigurationSource();

/// <summary>
/// Returns a value representing the name of the period end property of the entity mapped to a temporal table.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> Name of the period start property. </returns>
public static string? GetTemporalPeriodEndPropertyName(this IReadOnlyEntityType entityType)
public static string? GetPeriodEndPropertyName(this IReadOnlyEntityType entityType)
=> entityType[SqlServerAnnotationNames.TemporalPeriodEndPropertyName] as string;

/// <summary>
/// Sets a value representing the name of the period end property of the entity mapped to a temporal table.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="periodEndPropertyName"> The value to set. </param>
public static void SetTemporalPeriodEndPropertyName(this IMutableEntityType entityType, string? periodEndPropertyName)
public static void SetPeriodEndPropertyName(this IMutableEntityType entityType, string? periodEndPropertyName)
=> entityType.SetAnnotation(SqlServerAnnotationNames.TemporalPeriodEndPropertyName, periodEndPropertyName);

/// <summary>
Expand All @@ -163,7 +163,7 @@ public static void SetTemporalPeriodEndPropertyName(this IMutableEntityType enti
/// <param name="periodEndPropertyName"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
public static string? SetTemporalPeriodEndPropertyName(
public static string? SetPeriodEndPropertyName(
this IConventionEntityType entityType,
string? periodEndPropertyName,
bool fromDataAnnotation = false)
Expand All @@ -181,15 +181,15 @@ public static void SetTemporalPeriodEndPropertyName(this IMutableEntityType enti
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> The configuration source for the temporal table period end property name setting. </returns>
public static ConfigurationSource? GetTemporalPeriodEndPropertyNameConfigurationSource(this IConventionEntityType entityType)
public static ConfigurationSource? GetPeriodEndPropertyNameConfigurationSource(this IConventionEntityType entityType)
=> entityType.FindAnnotation(SqlServerAnnotationNames.TemporalPeriodEndPropertyName)?.GetConfigurationSource();

/// <summary>
/// Returns a value representing the name of the history table associated with the entity mapped to a temporal table.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> Name of the history table. </returns>
public static string? GetTemporalHistoryTableName(this IReadOnlyEntityType entityType)
public static string? GetHistoryTableName(this IReadOnlyEntityType entityType)
=> entityType[SqlServerAnnotationNames.TemporalHistoryTableName] is string historyTableName
? historyTableName
: entityType[SqlServerAnnotationNames.IsTemporal] as bool? == true
Expand All @@ -201,7 +201,7 @@ public static void SetTemporalPeriodEndPropertyName(this IMutableEntityType enti
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="historyTableName"> The value to set. </param>
public static void SetTemporalHistoryTableName(this IMutableEntityType entityType, string? historyTableName)
public static void SetHistoryTableName(this IMutableEntityType entityType, string? historyTableName)
=> entityType.SetAnnotation(SqlServerAnnotationNames.TemporalHistoryTableName, historyTableName);

/// <summary>
Expand All @@ -211,7 +211,7 @@ public static void SetTemporalHistoryTableName(this IMutableEntityType entityTyp
/// <param name="historyTableName"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
public static string? SetTemporalHistoryTableName(
public static string? SetHistoryTableName(
this IConventionEntityType entityType,
string? historyTableName,
bool fromDataAnnotation = false)
Expand All @@ -229,15 +229,15 @@ public static void SetTemporalHistoryTableName(this IMutableEntityType entityTyp
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> The configuration source for the temporal history table name setting. </returns>
public static ConfigurationSource? GetTemporalHistoryTableNameConfigurationSource(this IConventionEntityType entityType)
public static ConfigurationSource? GetHistoryTableNameConfigurationSource(this IConventionEntityType entityType)
=> entityType.FindAnnotation(SqlServerAnnotationNames.TemporalHistoryTableName)?.GetConfigurationSource();

/// <summary>
/// Returns a value representing the schema of the history table associated with the entity mapped to a temporal table.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> Name of the history table. </returns>
public static string? GetTemporalHistoryTableSchema(this IReadOnlyEntityType entityType)
public static string? GetHistoryTableSchema(this IReadOnlyEntityType entityType)
=> entityType[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
?? entityType[RelationalAnnotationNames.Schema] as string;

Expand All @@ -246,7 +246,7 @@ public static void SetTemporalHistoryTableName(this IMutableEntityType entityTyp
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="historyTableSchema"> The value to set. </param>
public static void SetTemporalHistoryTableSchema(this IMutableEntityType entityType, string? historyTableSchema)
public static void SetHistoryTableSchema(this IMutableEntityType entityType, string? historyTableSchema)
=> entityType.SetAnnotation(SqlServerAnnotationNames.TemporalHistoryTableSchema, historyTableSchema);

/// <summary>
Expand All @@ -256,7 +256,7 @@ public static void SetTemporalHistoryTableSchema(this IMutableEntityType entityT
/// <param name="historyTableSchema"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
public static string? SetTemporalHistoryTableSchema(
public static string? SetHistoryTableSchema(
this IConventionEntityType entityType,
string? historyTableSchema,
bool fromDataAnnotation = false)
Expand All @@ -274,7 +274,7 @@ public static void SetTemporalHistoryTableSchema(this IMutableEntityType entityT
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <returns> The configuration source for the temporal history table schema setting. </returns>
public static ConfigurationSource? GetTemporalHistoryTableSchemaConfigurationSource(this IConventionEntityType entityType)
public static ConfigurationSource? GetHistoryTableSchemaConfigurationSource(this IConventionEntityType entityType)
=> entityType.FindAnnotation(SqlServerAnnotationNames.TemporalHistoryTableSchema)?.GetConfigurationSource();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public static IServiceCollection AddEntityFrameworkSqlServer(this IServiceCollec
.TryAdd<IQuerySqlGeneratorFactory, SqlServerQuerySqlGeneratorFactory>()
.TryAdd<IRelationalSqlTranslatingExpressionVisitorFactory, SqlServerSqlTranslatingExpressionVisitorFactory>()
.TryAdd<IRelationalParameterBasedSqlProcessorFactory, SqlServerParameterBasedSqlProcessorFactory>()
.TryAdd<IQueryRootCreator, SqlServerQueryRootCreator>()
.TryAdd<INavigationExpansionExtensibilityHelper, SqlServerNavigationExpansionExtensibilityHelper>()
.TryAdd<IQueryableMethodTranslatingExpressionVisitorFactory, SqlServerQueryableMethodTranslatingExpressionVisitorFactory>()
.TryAddProviderSpecificServices(
b => b
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,8 @@ protected virtual void ValidateTemporalTables(
private void ValidateTemporalPeriodProperty(IEntityType temporalEntityType, bool periodStart)
{
var annotationPropertyName = periodStart
? temporalEntityType.GetTemporalPeriodStartPropertyName()
: temporalEntityType.GetTemporalPeriodEndPropertyName();
? temporalEntityType.GetPeriodStartPropertyName()
: temporalEntityType.GetPeriodEndPropertyName();

if (annotationPropertyName == null)
{
Expand Down
14 changes: 7 additions & 7 deletions src/EFCore.SqlServer/Metadata/Builders/TemporalTableBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ public TemporalTableBuilder(IMutableEntityType entityType)
/// </summary>
/// <param name="name"> The name of the history table. </param>
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
public virtual TemporalTableBuilder WithHistoryTable(string name)
public virtual TemporalTableBuilder UseHistoryTable(string name)
{
_entityType.SetTemporalHistoryTableName(name);
_entityType.SetHistoryTableName(name);

return this;
}
Expand All @@ -46,10 +46,10 @@ public virtual TemporalTableBuilder WithHistoryTable(string name)
/// <param name="name"> The name of the history table. </param>
/// <param name="schema"> The schema of the history table. </param>
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
public virtual TemporalTableBuilder WithHistoryTable(string name, string? schema)
public virtual TemporalTableBuilder UseHistoryTable(string name, string? schema)
{
_entityType.SetTemporalHistoryTableName(name);
_entityType.SetTemporalHistoryTableSchema(schema);
_entityType.SetHistoryTableName(name);
_entityType.SetHistoryTableSchema(schema);

return this;
}
Expand All @@ -61,7 +61,7 @@ public virtual TemporalTableBuilder WithHistoryTable(string name, string? schema
/// <returns> An object that can be used to configure the period start property. </returns>
public virtual TemporalPeriodPropertyBuilder HasPeriodStart(string propertyName)
{
_entityType.SetTemporalPeriodStartPropertyName(propertyName);
_entityType.SetPeriodStartPropertyName(propertyName);

return new TemporalPeriodPropertyBuilder(_entityType, propertyName);
}
Expand All @@ -73,7 +73,7 @@ public virtual TemporalPeriodPropertyBuilder HasPeriodStart(string propertyName)
/// <returns> An object that can be used to configure the period end property. </returns>
public virtual TemporalPeriodPropertyBuilder HasPeriodEnd(string propertyName)
{
_entityType.SetTemporalPeriodEndPropertyName(propertyName);
_entityType.SetPeriodEndPropertyName(propertyName);

return new TemporalPeriodPropertyBuilder(_entityType, propertyName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public TemporalTableBuilder(IMutableEntityType entityType)
/// <param name="name"> The name of the history table. </param>
/// <param name="schema"> The schema of the history table. </param>
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
public new virtual TemporalTableBuilder<TEntity> WithHistoryTable(string name, string? schema = null)
=> (TemporalTableBuilder<TEntity>)base.WithHistoryTable(name, schema);
public new virtual TemporalTableBuilder<TEntity> UseHistoryTable(string name, string? schema = null)
=> (TemporalTableBuilder<TEntity>)base.UseHistoryTable(name, schema);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public override ConventionSet CreateConventionSet()
ReplaceConvention(
conventionSet.EntityTypeAnnotationChangedConventions, (RelationalValueGenerationConvention)valueGenerationConvention);

var sqlServerTemporalConvention = new SqlServerTemporalConvention();
var sqlServerTemporalConvention = new SqlServerTemporalConvention(Dependencies, RelationalDependencies);
ConventionSet.AddBefore(
conventionSet.EntityTypeAnnotationChangedConventions,
sqlServerTemporalConvention,
Expand Down
Loading