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

Pack of API review changes #28490

Merged
merged 4 commits into from
Jul 22, 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 @@ -1093,20 +1093,20 @@ public static bool CanSetComment(
/// See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
/// </remarks>
/// <param name="entityTypeBuilder">The entity type builder.</param>
/// <param name="name">The name of the trigger.</param>
/// <param name="modelName">The name of the trigger.</param>
/// <param name="tableName">The name of the table on which this trigger is defined.</param>
/// <param name="tableSchema">The schema of the table on which this trigger is defined.</param>
/// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
/// <returns>The same builder instance if the check constraint was configured, <see langword="null" /> otherwise.</returns>
public static IConventionTriggerBuilder? HasTrigger(
this IConventionEntityTypeBuilder entityTypeBuilder,
string name,
string modelName,
string? tableName,
string? tableSchema,
bool fromDataAnnotation = false)
=> InternalTriggerBuilder.HasTrigger(
entityTypeBuilder.Metadata,
name,
modelName,
tableName,
tableSchema,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention)
Expand Down
10 changes: 5 additions & 5 deletions src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1700,15 +1700,15 @@ public static IEnumerable<StoreObjectIdentifier> GetMappedStoreObjects(
Check.DebugAssert(previousDetailedErrorsEnabled == detailedErrorsEnabled, "Differing values of DetailedErrorsEnabled");
#endif

var getReadFieldValue = property.GetOrAddRuntimeAnnotationValue(
RelationalAnnotationNames.GetReaderFieldValue,
static x => CreateGetValueDelegate(x.property, x.detailedErrorsEnabled),
var fieldValueGetter = property.GetOrAddRuntimeAnnotationValue(
RelationalAnnotationNames.FieldValueGetter,
static x => CreateFieldValueGetter(x.property, x.detailedErrorsEnabled),
(property, detailedErrorsEnabled));

return getReadFieldValue(relationalReader.DbDataReader, ordinal);
return fieldValueGetter(relationalReader.DbDataReader, ordinal);
}

private static Func<DbDataReader, int, object?> CreateGetValueDelegate(IProperty property, bool detailedErrorsEnabled)
private static Func<DbDataReader, int, object?> CreateFieldValueGetter(IProperty property, bool detailedErrorsEnabled)
{
var readerParameter = Expression.Parameter(typeof(DbDataReader), "reader");
var indexParameter = Expression.Parameter(typeof(int), "index");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ public virtual OwnedNavigationSplitTableBuilder ExcludeFromMigrations(bool exclu
/// <summary>
/// Configures a database trigger on the table.
/// </summary>
/// <param name="name">The name of the trigger.</param>
/// <param name="modelName">The name of the trigger.</param>
/// <returns>A builder that can be used to configure the database trigger.</returns>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
/// </remarks>
public virtual TriggerBuilder HasTrigger(string name)
public virtual TriggerBuilder HasTrigger(string modelName)
=> new((Trigger)InternalTriggerBuilder.HasTrigger(
(IConventionEntityType)MappingFragment.EntityType,
name,
modelName,
Name,
Schema,
ConfigurationSource.Explicit)!);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ public virtual OwnedNavigationTableBuilder ExcludeFromMigrations(bool excluded =
/// <summary>
/// Configures a database trigger on the table.
/// </summary>
/// <param name="name">The name of the trigger.</param>
/// <param name="modelName">The name of the trigger.</param>
/// <returns>A builder that can be used to configure the database trigger.</returns>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
/// </remarks>
public virtual TriggerBuilder HasTrigger(string name)
public virtual TriggerBuilder HasTrigger(string modelName)
=> new((Trigger)InternalTriggerBuilder.HasTrigger(
(IConventionEntityType)Metadata,
name,
modelName,
Name,
Schema,
ConfigurationSource.Explicit)!);
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore.Relational/Metadata/Builders/SplitTableBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ public virtual SplitTableBuilder ExcludeFromMigrations(bool excluded = true)
/// <summary>
/// Configures a database trigger on the table.
/// </summary>
/// <param name="name">The name of the trigger.</param>
/// <param name="modelName">The name of the trigger.</param>
/// <returns>A builder that can be used to configure the database trigger.</returns>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
/// </remarks>
public virtual TriggerBuilder HasTrigger(string name)
public virtual TriggerBuilder HasTrigger(string modelName)
=> new((Trigger)InternalTriggerBuilder.HasTrigger(
(IConventionEntityType)MappingFragment.EntityType,
name,
modelName,
Name,
Schema,
ConfigurationSource.Explicit)!);
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore.Relational/Metadata/Builders/TableBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ public virtual TableBuilder ExcludeFromMigrations(bool excluded = true)
/// <summary>
/// Configures a database trigger on the table.
/// </summary>
/// <param name="name">The name of the trigger.</param>
/// <param name="modelName">The name of the trigger.</param>
/// <returns>A builder that can be used to configure the database trigger.</returns>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-triggers">Database triggers</see> for more information and examples.
/// </remarks>
public virtual TriggerBuilder HasTrigger(string name)
public virtual TriggerBuilder HasTrigger(string modelName)
=> new((Trigger)InternalTriggerBuilder.HasTrigger(
(IConventionEntityType)Metadata,
name,
modelName,
Name,
Schema,
ConfigurationSource.Explicit)!);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ public virtual bool CanSetName(string? name, ConfigurationSource configurationSo
/// </summary>
public static IConventionTrigger? HasTrigger(
IConventionEntityType entityType,
string name,
string modelName,
string? tableName,
string? tableSchema,
ConfigurationSource configurationSource)
{
List<IConventionTrigger>? triggersToBeDetached = null;
var trigger = entityType.FindTrigger(name);
var trigger = entityType.FindTrigger(modelName);
if (trigger != null)
{
if ((tableName == null && tableSchema == null)
Expand All @@ -78,13 +78,13 @@ public virtual bool CanSetName(string? name, ConfigurationSource configurationSo
return null;
}

entityType.RemoveTrigger(name);
entityType.RemoveTrigger(modelName);
}
else
{
foreach (var derivedType in entityType.GetDerivedTypes())
{
var derivedTrigger = (IConventionTrigger?)Trigger.FindDeclaredTrigger(derivedType, name);
var derivedTrigger = (IConventionTrigger?)Trigger.FindDeclaredTrigger(derivedType, modelName);
if (derivedTrigger == null)
{
continue;
Expand Down Expand Up @@ -114,7 +114,7 @@ public virtual bool CanSetName(string? name, ConfigurationSource configurationSo
}
}

trigger = new Trigger((IMutableEntityType)entityType, name, tableName, tableSchema, configurationSource);
trigger = new Trigger((IMutableEntityType)entityType, modelName, tableName, tableSchema, configurationSource);

if (detachedTriggers != null)
{
Expand Down
34 changes: 17 additions & 17 deletions src/EFCore.Relational/Metadata/Internal/Trigger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ public class Trigger : ConventionAnnotatable, IMutableTrigger, IConventionTrigge
/// </summary>
public Trigger(
IMutableEntityType entityType,
string name,
string modelName,
string? tableName,
string? tableSchema,
ConfigurationSource configurationSource)
{
EntityType = entityType;
ModelName = name;
ModelName = modelName;
_tableName = tableName;
_tableSchema = tableSchema;
_configurationSource = configurationSource;
Expand All @@ -47,40 +47,40 @@ public Trigger(
entityType.SetOrRemoveAnnotation(RelationalAnnotationNames.Triggers, triggers);
}

if (triggers.ContainsKey(name))
if (triggers.ContainsKey(modelName))
{
throw new InvalidOperationException(
RelationalStrings.DuplicateTrigger(
name, entityType.DisplayName(), entityType.DisplayName()));
modelName, entityType.DisplayName(), entityType.DisplayName()));
}

var baseTrigger = entityType.BaseType?.FindTrigger(name);
var baseTrigger = entityType.BaseType?.FindTrigger(modelName);
if (baseTrigger != null)
{
throw new InvalidOperationException(
RelationalStrings.DuplicateTrigger(
name, entityType.DisplayName(), baseTrigger.EntityType.DisplayName()));
modelName, entityType.DisplayName(), baseTrigger.EntityType.DisplayName()));
}

foreach (var derivedType in entityType.GetDerivedTypes())
{
var derivedTrigger = FindTrigger(derivedType, name);
var derivedTrigger = FindTrigger(derivedType, modelName);
if (derivedTrigger != null)
{
throw new InvalidOperationException(
RelationalStrings.DuplicateTrigger(
name, entityType.DisplayName(), derivedTrigger.EntityType.DisplayName()));
modelName, entityType.DisplayName(), derivedTrigger.EntityType.DisplayName()));
}
}

if (entityType.GetTableName() is null)
{
throw new InvalidOperationException(RelationalStrings.TriggerOnUnmappedEntityType(name, entityType.DisplayName()));
throw new InvalidOperationException(RelationalStrings.TriggerOnUnmappedEntityType(modelName, entityType.DisplayName()));
}

EnsureMutable();

triggers.Add(name, this);
triggers.Add(modelName, this);

_builder = new InternalTriggerBuilder(this, ((IConventionModel)entityType.Model).Builder);
}
Expand Down Expand Up @@ -113,20 +113,20 @@ public static IEnumerable<IReadOnlyTrigger> GetTriggers(IReadOnlyEntityType enti
/// </summary>
public static IReadOnlyTrigger? FindTrigger(
IReadOnlyEntityType entityType,
string name)
=> entityType.BaseType?.FindTrigger(name) ?? FindDeclaredTrigger(entityType, name);
string modelName)
=> entityType.BaseType?.FindTrigger(modelName) ?? FindDeclaredTrigger(entityType, modelName);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static IReadOnlyTrigger? FindDeclaredTrigger(IReadOnlyEntityType entityType, string name)
public static IReadOnlyTrigger? FindDeclaredTrigger(IReadOnlyEntityType entityType, string modelName)
{
var triggers = (SortedDictionary<string, ITrigger>?)entityType[RelationalAnnotationNames.Triggers];

return triggers is not null && triggers.TryGetValue(name, out var trigger)
return triggers is not null && triggers.TryGetValue(modelName, out var trigger)
? trigger
: null;
}
Expand All @@ -137,17 +137,17 @@ public static IEnumerable<IReadOnlyTrigger> GetTriggers(IReadOnlyEntityType enti
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static Trigger? RemoveTrigger(IMutableEntityType entityType, string name)
public static Trigger? RemoveTrigger(IMutableEntityType entityType, string modelName)
{
var triggers = (SortedDictionary<string, ITrigger>?)entityType[RelationalAnnotationNames.Triggers];
if (triggers == null
|| !triggers.TryGetValue(name, out var trigger))
|| !triggers.TryGetValue(modelName, out var trigger))
{
return null;
}

var mutableTrigger = (Trigger)trigger;
triggers.Remove(name);
triggers.Remove(modelName);
mutableTrigger.SetRemovedFromModel();

return mutableTrigger;
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public static class RelationalAnnotationNames
public const string ModelDependencies = Prefix + "ModelDependencies";

/// <summary>
/// The name for the reader get value delegate annotations.
/// The name for the reader field value getter delegate annotation.
/// </summary>
public const string GetReaderFieldValue = Prefix + "GetReaderFieldValue";
public const string FieldValueGetter = Prefix + "FieldValueGetter";
}
14 changes: 4 additions & 10 deletions src/EFCore.Relational/Update/IBatchExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,15 @@ public interface IBatchExecutor
/// <summary>
/// Executes the commands in the batches against the given database connection.
/// </summary>
/// <param name="commandBatches">
/// A list of value tuples, each of which contains a batch to execute, and whether more batches are available.
/// </param>
/// <param name="commandBatches">The batches to execute.</param>
/// <param name="connection">The database connection to use.</param>
/// <returns>The total number of rows affected.</returns>
int Execute(
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IRelationalConnection connection);
int Execute(IEnumerable<ModificationCommandBatch> commandBatches, IRelationalConnection connection);

/// <summary>
/// Executes the commands in the batches against the given database connection.
/// </summary>
/// <param name="commandBatches">
/// A list of value tuples, each of which contains a batch to execute, and whether more batches are available.
/// </param>
/// <param name="commandBatches">The batches to execute.</param>
/// <param name="connection">The database connection to use.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
/// <returns>
Expand All @@ -51,7 +45,7 @@ int Execute(
/// </returns>
/// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
Task<int> ExecuteAsync(
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IEnumerable<ModificationCommandBatch> commandBatches,
IRelationalConnection connection,
CancellationToken cancellationToken = default);
}
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Update/ICommandBatchPreparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ public interface ICommandBatchPreparer
/// </summary>
/// <param name="entries">The entries that represent the entities to be modified.</param>
/// <param name="updateAdapter">The model data.</param>
/// <returns>A list of value tuples, each of which contains a batch to execute, and whether more batches are available.</returns>
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> BatchCommands(IList<IUpdateEntry> entries, IUpdateAdapter updateAdapter);
/// <returns>The list of batches to execute.</returns>
IEnumerable<ModificationCommandBatch> BatchCommands(IList<IUpdateEntry> entries, IUpdateAdapter updateAdapter);
}
18 changes: 8 additions & 10 deletions src/EFCore.Relational/Update/Internal/BatchExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ public BatchExecutor(
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual int Execute(
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IRelationalConnection connection)
public virtual int Execute(IEnumerable<ModificationCommandBatch> commandBatches, IRelationalConnection connection)
{
using var batchEnumerator = commandBatches.GetEnumerator();

Expand All @@ -59,7 +57,7 @@ public virtual int Execute(
return 0;
}

var (batch, hasMoreBatches) = batchEnumerator.Current;
var batch = batchEnumerator.Current;

var rowsAffected = 0;
var transaction = connection.CurrentTransaction;
Expand All @@ -73,7 +71,7 @@ public virtual int Execute(
&& transactionEnlistManager?.CurrentAmbientTransaction is null
&& CurrentContext.Context.Database.AutoTransactionsEnabled
// Don't start a transaction if we have a single batch which doesn't require a transaction (single command), for perf.
&& (hasMoreBatches || batch.RequiresTransaction))
&& (batch.AreMoreBatchesExpected || batch.RequiresTransaction))
{
transaction = connection.BeginTransaction();
beganTransaction = true;
Expand All @@ -92,7 +90,7 @@ public virtual int Execute(

do
{
batch = batchEnumerator.Current.Batch;
batch = batchEnumerator.Current;
batch.Execute(connection);
rowsAffected += batch.ModificationCommands.Count;
}
Expand Down Expand Up @@ -156,7 +154,7 @@ public virtual int Execute(
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual async Task<int> ExecuteAsync(
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IEnumerable<ModificationCommandBatch> commandBatches,
IRelationalConnection connection,
CancellationToken cancellationToken = default)
{
Expand All @@ -167,7 +165,7 @@ public virtual async Task<int> ExecuteAsync(
return 0;
}

var (batch, hasMoreBatches) = batchEnumerator.Current;
var batch = batchEnumerator.Current;

var rowsAffected = 0;
var transaction = connection.CurrentTransaction;
Expand All @@ -181,7 +179,7 @@ public virtual async Task<int> ExecuteAsync(
&& transactionEnlistManager?.CurrentAmbientTransaction is null
&& CurrentContext.Context.Database.AutoTransactionsEnabled
// Don't start a transaction if we have a single batch which doesn't require a transaction (single command), for perf.
&& (hasMoreBatches || batch.RequiresTransaction))
&& (batch.AreMoreBatchesExpected || batch.RequiresTransaction))
{
transaction = await connection.BeginTransactionAsync(cancellationToken).ConfigureAwait(false);
beganTransaction = true;
Expand All @@ -200,7 +198,7 @@ public virtual async Task<int> ExecuteAsync(

do
{
batch = batchEnumerator.Current.Batch;
batch = batchEnumerator.Current;
await batch.ExecuteAsync(connection, cancellationToken).ConfigureAwait(false);
rowsAffected += batch.ModificationCommands.Count;
}
Expand Down
Loading