Skip to content

Commit

Permalink
Addressing API Review feedback for temporal tables feature:
Browse files Browse the repository at this point in the history
- converted QueryRootCreator into general purpose helper service,
- renamed WithHistoryTable to UseHistoryTable,
- removed Temporal from entity type extensions,
- added dependency objects to temporal convention.
  • Loading branch information
maumar committed Aug 6, 2021
1 parent 8460d0f commit ccd7d07
Show file tree
Hide file tree
Showing 23 changed files with 172 additions and 154 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,19 @@ public SqlServerAnnotationCodeGenerator(AnnotationCodeGeneratorDependencies depe
? 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 class SqlServerEntityTypeBuilderExtensions
/// 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 class SqlServerEntityTypeBuilderExtensions
/// 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 class SqlServerEntityTypeBuilderExtensions
{
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 class SqlServerEntityTypeBuilderExtensions
{
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 @@ public override void Validate(IModel model, IDiagnosticsLogger<DbLoggerCategory.
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

0 comments on commit ccd7d07

Please sign in to comment.