Skip to content

Commit

Permalink
Review changes and remove duplicate code for logging event.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ajcvickers
ajcvickers committed Jun 24, 2022
1 parent 21cdf12 commit 898e146
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 129 deletions.
6 changes: 4 additions & 2 deletions src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ public override int SaveChanges(IList<IUpdateEntry> entries)
var exception = WrapUpdateException(ex, errorEntries);

if (exception is not DbUpdateConcurrencyException
|| !Dependencies.Logger.OptimisticConcurrencyException(entry.Context, errorEntries, exception).IsSuppressed)
|| !Dependencies.Logger.OptimisticConcurrencyException(
entry.Context, errorEntries, (DbUpdateConcurrencyException)exception, null).IsSuppressed)
{
throw exception;
}
Expand Down Expand Up @@ -170,7 +171,8 @@ public override async Task<int> SaveChangesAsync(

if (exception is not DbUpdateConcurrencyException
|| !(await Dependencies.Logger.OptimisticConcurrencyExceptionAsync(
entry.Context, errorEntries, exception, cancellationToken).ConfigureAwait(false)).IsSuppressed)
entry.Context, errorEntries, (DbUpdateConcurrencyException)exception, null, cancellationToken)
.ConfigureAwait(false)).IsSuppressed)
{
throw exception;
}
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public virtual void Delete(IUpdateEntry entry, IDiagnosticsLogger<DbLoggerCatego
{
var entries = new[] { entry };
var exception = new DbUpdateConcurrencyException(InMemoryStrings.UpdateConcurrencyException, entries);
if (!updateLogger.OptimisticConcurrencyException(entry.Context, entries, exception).IsSuppressed)
if (!updateLogger.OptimisticConcurrencyException(entry.Context, entries, exception, null).IsSuppressed)
{
throw exception;
}
Expand Down Expand Up @@ -315,7 +315,7 @@ public virtual void Update(IUpdateEntry entry, IDiagnosticsLogger<DbLoggerCatego
{
var entries = new[] { entry };
var exception = new DbUpdateConcurrencyException(InMemoryStrings.UpdateConcurrencyException, entries);
if (!updateLogger.OptimisticConcurrencyException(entry.Context, entries, exception).IsSuppressed)
if (!updateLogger.OptimisticConcurrencyException(entry.Context, entries, exception, null).IsSuppressed)
{
throw exception;
}
Expand Down Expand Up @@ -437,7 +437,7 @@ protected virtual void ThrowUpdateConcurrencyException(
entries);


if (!updateLogger.OptimisticConcurrencyException(entry.Context, entries, exception).IsSuppressed)
if (!updateLogger.OptimisticConcurrencyException(entry.Context, entries, exception, null).IsSuppressed)
{
throw exception;
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Diagnostics/RelationalConcurrencyExceptionEventData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public RelationalConcurrencyExceptionEventData(
Guid commandId,
Guid connectionId,
IReadOnlyList<IUpdateEntry> entries,
Exception exception)
DbUpdateConcurrencyException exception)
: base(eventDefinition, messageGenerator, context, entries, exception)
{
Connection = connection;
Expand Down
111 changes: 0 additions & 111 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3207,115 +3207,4 @@ private static string ColumnOrderIgnoredWarning(EventDefinitionBase definition,
var p = (MigrationColumnOperationEventData)payload;
return d.GenerateMessage((p.ColumnOperation.Table, p.ColumnOperation.Schema).FormatTable(), p.ColumnOperation.Name);
}

/// <summary>
/// Logs for the <see cref="CoreEventId.OptimisticConcurrencyException" /> event.
/// </summary>
/// <param name="diagnostics">The diagnostics logger to use.</param>
/// <param name="context">The context in use.</param>
/// <param name="reader">The data reader.</param>
/// <param name="entries">The entries that were involved in the concurrency violation.</param>
/// <param name="exception">The exception that caused this event.</param>
public static InterceptionResult OptimisticConcurrencyException(
this IDiagnosticsLogger<DbLoggerCategory.Update> diagnostics,
DbContext context,
RelationalDataReader reader,
IReadOnlyList<IUpdateEntry> entries,
Exception exception)
{
#pragma warning disable EF1001
var definition = CoreResources.LogOptimisticConcurrencyException(diagnostics);
#pragma warning restore EF1001

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, exception);
}

if (diagnostics.NeedsEventData<ISaveChangesInterceptor>(
definition,
out var interceptor, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = CreateConcurrencyExceptionEventData(context, reader, exception, entries, definition);

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);

if (interceptor != null)
{
return interceptor.ThrowingConcurrencyException(eventData, default);
}
}

return default;
}

/// <summary>
/// Logs for the <see cref="CoreEventId.OptimisticConcurrencyException" /> event.
/// </summary>
/// <param name="diagnostics">The diagnostics logger to use.</param>
/// <param name="context">The context in use.</param>
/// <param name="reader">The data reader.</param>
/// <param name="entries">The entries that were involved in the concurrency violation.</param>
/// <param name="exception">The exception that caused this event.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
/// <returns>A <see cref="Task" /> for the async result.</returns>
/// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
public static ValueTask<InterceptionResult> OptimisticConcurrencyExceptionAsync(
this IDiagnosticsLogger<DbLoggerCategory.Update> diagnostics,
DbContext context,
RelationalDataReader reader,
IReadOnlyList<IUpdateEntry> entries,
Exception exception,
CancellationToken cancellationToken = default)
{
#pragma warning disable EF1001
var definition = CoreResources.LogOptimisticConcurrencyException(diagnostics);
#pragma warning restore EF1001

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, exception);
}

if (diagnostics.NeedsEventData<ISaveChangesInterceptor>(
definition,
out var interceptor, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = CreateConcurrencyExceptionEventData(context, reader, exception, entries, definition);

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);

if (interceptor != null)
{
return interceptor.ThrowingConcurrencyExceptionAsync(eventData, default, cancellationToken);
}
}

return default;
}

private static RelationalConcurrencyExceptionEventData CreateConcurrencyExceptionEventData(
DbContext context,
RelationalDataReader reader,
Exception exception,
IReadOnlyList<IUpdateEntry> entries,
EventDefinition<Exception> definition)
=> new(
definition,
OptimisticConcurrencyException,
context,
reader.RelationalConnection.DbConnection,
reader.DbCommand,
reader.DbDataReader,
reader.CommandId,
reader.RelationalConnection.ConnectionId,
entries,
exception);

private static string OptimisticConcurrencyException(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<Exception>)definition;
var p = (RelationalConcurrencyExceptionEventData)payload;
return d.GenerateMessage(p.Exception);
}
}
35 changes: 29 additions & 6 deletions src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ private IReadOnlyList<IUpdateEntry> AggregateEntries(int endIndex, int commandCo
/// <param name="expectedRowsAffected">The expected number of rows affected.</param>
/// <param name="rowsAffected">The actual number of rows affected.</param>
protected virtual void ThrowAggregateUpdateConcurrencyException(
RelationalDataReader reader,
RelationalDataReader reader,
int commandIndex,
int expectedRowsAffected,
int rowsAffected)
Expand All @@ -359,9 +359,9 @@ protected virtual void ThrowAggregateUpdateConcurrencyException(

if (!Dependencies.UpdateLogger.OptimisticConcurrencyException(
Dependencies.CurrentContext.Context,
reader,
entries,
exception).IsSuppressed)
entries,
exception,
(c, ex, e, d) => CreateConcurrencyExceptionEventData(c, reader, ex, e, d)).IsSuppressed)
{
throw exception;
}
Expand All @@ -378,7 +378,7 @@ protected virtual void ThrowAggregateUpdateConcurrencyException(
/// <returns> A task that represents the asynchronous operation.</returns>
/// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
protected virtual async Task ThrowAggregateUpdateConcurrencyExceptionAsync(
RelationalDataReader reader,
RelationalDataReader reader,
int commandIndex,
int expectedRowsAffected,
int rowsAffected,
Expand All @@ -390,10 +390,33 @@ protected virtual async Task ThrowAggregateUpdateConcurrencyExceptionAsync(
entries);

if (!(await Dependencies.UpdateLogger.OptimisticConcurrencyExceptionAsync(
Dependencies.CurrentContext.Context, reader, entries, exception, cancellationToken: cancellationToken)
Dependencies.CurrentContext.Context,
entries,
exception,
(c, ex, e, d) => CreateConcurrencyExceptionEventData(c, reader, ex, e, d),
cancellationToken: cancellationToken)
.ConfigureAwait(false)).IsSuppressed)
{
throw exception;
}
}

private static RelationalConcurrencyExceptionEventData CreateConcurrencyExceptionEventData(
DbContext context,
RelationalDataReader reader,
DbUpdateConcurrencyException exception,
IReadOnlyList<IUpdateEntry> entries,
EventDefinition<Exception> definition)
=> new(
definition,
(definition1, payload)
=> ((EventDefinition<Exception>)definition1).GenerateMessage(((ConcurrencyExceptionEventData)payload).Exception),
context,
reader.RelationalConnection.DbConnection,
reader.DbCommand,
reader.DbDataReader,
reader.CommandId,
reader.RelationalConnection.ConnectionId,
entries,
exception);
}
8 changes: 7 additions & 1 deletion src/EFCore/Diagnostics/ConcurrencyExceptionEventData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,18 @@ public ConcurrencyExceptionEventData(
Func<EventDefinitionBase, EventData, string> messageGenerator,
DbContext context,
IReadOnlyList<IUpdateEntry> entries,
Exception exception)
DbUpdateConcurrencyException exception)
: base(eventDefinition, messageGenerator, context, exception)
{
_internalEntries = entries;
}

/// <summary>
/// The exception that will be thrown, unless throwing is suppressed.
/// </summary>
public new virtual DbUpdateConcurrencyException Exception
=> (DbUpdateConcurrencyException)base.Exception;

/// <summary>
/// The entries that were involved in the concurrency violation.
/// </summary>
Expand Down
18 changes: 13 additions & 5 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,14 @@ private static string OldModelVersion(EventDefinitionBase definition, EventData
/// <param name="context">The context in use.</param>
/// <param name="entries">The entries that were involved in the concurrency violation.</param>
/// <param name="exception">The exception that caused this event.</param>
/// <param name="createEventData">Optional delegate to override event data creation.</param>
public static InterceptionResult OptimisticConcurrencyException(
this IDiagnosticsLogger<DbLoggerCategory.Update> diagnostics,
DbContext context,
IReadOnlyList<IUpdateEntry> entries,
Exception exception)
DbUpdateConcurrencyException exception,
Func<DbContext, DbUpdateConcurrencyException, IReadOnlyList<IUpdateEntry>, EventDefinition<Exception>,
ConcurrencyExceptionEventData>? createEventData)
{
var definition = CoreResources.LogOptimisticConcurrencyException(diagnostics);

Expand All @@ -254,7 +257,8 @@ public static InterceptionResult OptimisticConcurrencyException(
definition,
out var interceptor, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = CreateConcurrencyExceptionEventData(context, exception, entries, definition);
var eventData = createEventData?.Invoke(context, exception, entries, definition)
?? CreateConcurrencyExceptionEventData(context, exception, entries, definition);

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);

Expand All @@ -274,14 +278,17 @@ public static InterceptionResult OptimisticConcurrencyException(
/// <param name="context">The context in use.</param>
/// <param name="entries">The entries that were involved in the concurrency violation.</param>
/// <param name="exception">The exception that caused this event.</param>
/// <param name="createEventData">Optional delegate to override event data creation.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
/// <returns>A <see cref="Task" /> for the async result.</returns>
/// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
public static ValueTask<InterceptionResult> OptimisticConcurrencyExceptionAsync(
this IDiagnosticsLogger<DbLoggerCategory.Update> diagnostics,
DbContext context,
IReadOnlyList<IUpdateEntry> entries,
Exception exception,
DbUpdateConcurrencyException exception,
Func<DbContext, DbUpdateConcurrencyException, IReadOnlyList<IUpdateEntry>, EventDefinition<Exception>,
ConcurrencyExceptionEventData>? createEventData,
CancellationToken cancellationToken = default)
{
var definition = CoreResources.LogOptimisticConcurrencyException(diagnostics);
Expand All @@ -295,7 +302,8 @@ public static ValueTask<InterceptionResult> OptimisticConcurrencyExceptionAsync(
definition,
out var interceptor, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = CreateConcurrencyExceptionEventData(context, exception, entries, definition);
var eventData = createEventData?.Invoke(context, exception, entries, definition)
?? CreateConcurrencyExceptionEventData(context, exception, entries, definition);

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);

Expand All @@ -310,7 +318,7 @@ public static ValueTask<InterceptionResult> OptimisticConcurrencyExceptionAsync(

private static ConcurrencyExceptionEventData CreateConcurrencyExceptionEventData(
DbContext context,
Exception exception,
DbUpdateConcurrencyException exception,
IReadOnlyList<IUpdateEntry> entries,
EventDefinition<Exception> definition)
=> new(
Expand Down
1 change: 1 addition & 0 deletions test/EFCore.Tests/Infrastructure/CoreEventIdTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public void Every_eventId_has_a_logger_method_and_logs_when_level_enabled()
{ typeof(ICollection<IServiceProvider>), () => new List<IServiceProvider>() },
{ typeof(IReadOnlyList<IPropertyBase>), () => new[] { property } },
{ typeof(IReadOnlyList<IUpdateEntry>), () => Array.Empty<IUpdateEntry>() },
{ typeof(Func<DbContext, DbUpdateConcurrencyException, IReadOnlyList<IUpdateEntry>, EventDefinition<Exception>, ConcurrencyExceptionEventData>), () => null },
{ typeof(IReadOnlyList<IReadOnlyPropertyBase>), () => new[] { property } },
{ typeof(IEnumerable<Tuple<MemberInfo, Type>>), () => new[] { new Tuple<MemberInfo, Type>(propertyInfo, typeof(object)) } },
{ typeof(MemberInfo), () => propertyInfo },
Expand Down

0 comments on commit 898e146

Please sign in to comment.