Skip to content

Commit

Permalink
Make DbUpdateException.Entries empty when we can't populate it
Browse files Browse the repository at this point in the history 
Closes #29379
  • Loading branch information
@roji
roji committed Nov 15, 2022
1 parent 3dc1fdf commit 004a855
Show file tree
Hide file tree
Showing 5 changed files with 226 additions and 54 deletions.
126 changes: 75 additions & 51 deletions src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,43 +270,55 @@ await ThrowAggregateUpdateConcurrencyExceptionAsync(
/// <returns>The ordinal of the next result set that must be consumed.</returns>
protected virtual int ConsumeResultSet(int startCommandIndex, RelationalDataReader reader)
{
var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
{
if (!reader.Read())
{
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
{
expectedRowsAffected++;
}
IReadOnlyModificationCommand? command = null;

ThrowAggregateUpdateConcurrencyException(reader, commandIndex, expectedRowsAffected, rowsAffected);
}
else
try
{
var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
{
var resultSetMapping = ResultSetMappings[commandIndex];

var command = ModificationCommands[
command = ModificationCommands[
resultSetMapping.HasFlag(ResultSetMapping.IsPositionalResultMappingEnabled)
? startCommandIndex + reader.DbDataReader.GetInt32(reader.DbDataReader.FieldCount - 1)
: commandIndex];

Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");
if (!reader.Read())
{
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
{
expectedRowsAffected++;
}

ThrowAggregateUpdateConcurrencyException(reader, commandIndex, expectedRowsAffected, rowsAffected);
}
else
{
Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");

command.PropagateResults(reader);
}

command.PropagateResults(reader);
rowsAffected++;
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

rowsAffected++;
return commandIndex - 1;
}
catch (Exception ex) when (ex is not DbUpdateException and not OperationCanceledException)
{
throw new DbUpdateException(
RelationalStrings.UpdateStoreException,
ex,
command?.Entries ?? Array.Empty<IUpdateEntry>());
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

return commandIndex - 1;
}

/// <summary>
Expand All @@ -326,44 +338,56 @@ protected virtual async Task<int> ConsumeResultSetAsync(
RelationalDataReader reader,
CancellationToken cancellationToken)
{
var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
{
if (!await reader.ReadAsync(cancellationToken).ConfigureAwait(false))
{
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
{
expectedRowsAffected++;
}
IReadOnlyModificationCommand? command = null;

await ThrowAggregateUpdateConcurrencyExceptionAsync(
reader, commandIndex, expectedRowsAffected, rowsAffected, cancellationToken).ConfigureAwait(false);
}
else
try
{
var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
{
var resultSetMapping = ResultSetMappings[commandIndex];

var command = ModificationCommands[
command = ModificationCommands[
resultSetMapping.HasFlag(ResultSetMapping.IsPositionalResultMappingEnabled)
? startCommandIndex + reader.DbDataReader.GetInt32(reader.DbDataReader.FieldCount - 1)
: commandIndex];

Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");
if (!await reader.ReadAsync(cancellationToken).ConfigureAwait(false))
{
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
{
expectedRowsAffected++;
}

command.PropagateResults(reader);
await ThrowAggregateUpdateConcurrencyExceptionAsync(
reader, commandIndex, expectedRowsAffected, rowsAffected, cancellationToken).ConfigureAwait(false);
}
else
{
Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");

command.PropagateResults(reader);
}

rowsAffected++;
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

rowsAffected++;
return commandIndex - 1;
}
catch (Exception ex) when (ex is not DbUpdateException and not OperationCanceledException)
{
throw new DbUpdateException(
RelationalStrings.UpdateStoreException,
ex,
command?.Entries ?? Array.Empty<IUpdateEntry>());
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

return commandIndex - 1;
}

/// <summary>
Expand Down
38 changes: 36 additions & 2 deletions test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ protected override string StoreName
=> "NonSharedModelUpdatesTestBase";

[ConditionalTheory] // Issue #29356
[InlineData(false)]
[InlineData(true)]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async)
{
var contextFactory = await InitializeAsync<DbContext>(
Expand Down Expand Up @@ -111,6 +110,41 @@ private class Book
public Author? Author { get; set; }
}
[ConditionalTheory] // Issue #29379
[MemberData(nameof(IsAsyncData))]
public virtual async Task DbUpdateException_Entries_is_correct_with_multiple_inserts(bool async)
{
var contextFactory = await InitializeAsync<DbContext>(onModelCreating: mb => mb.Entity<Blog>().HasIndex(b => b.Name).IsUnique());
await ExecuteWithStrategyInTransactionAsync(
contextFactory,
async context =>
{
context.Add(new Blog { Name = "Blog2" });
await context.SaveChangesAsync();
},
async context =>
{
context.Add(new Blog { Name = "Blog1" });
context.Add(new Blog { Name = "Blog2" });
context.Add(new Blog { Name = "Blog3" });
var exception = async
? await Assert.ThrowsAsync<DbUpdateException>(() => context.SaveChangesAsync())
: Assert.Throws<DbUpdateException>(() => context.SaveChanges());
var entry = Assert.Single(exception.Entries);
Assert.Equal("Blog2", ((Blog)entry.Entity).Name);
});
}
public class Blog
{
public int Id { get; set; }
public string? Name { get; set; }
}
protected virtual void ExecuteWithStrategyInTransaction(
ContextFactory<DbContext> contextFactory,
Action<DbContext> testOperation,
Expand Down
28 changes: 28 additions & 0 deletions test/EFCore.Relational.Tests/Update/ReaderModificationCommandBatchTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,34 @@ public async Task OperationCanceledException_is_not_wrapped_with_DbUpdateExcepti
Assert.Same(originalException, actualException);
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public async Task DbUpdateException(bool async)
{
var entry = CreateEntry(EntityState.Added, generateKeyValues: true);

var command = CreateModificationCommand(entry, new ParameterNameGenerator().GenerateNext, true, null);
command.AddEntry(entry, true);

var originalException = new FakeDbException();

var connection = CreateConnection(
new FakeCommandExecutor(
executeReaderAsync: (c, b, ct) => throw originalException,
executeReader: (c, b) => throw originalException));

var batch = new ModificationCommandBatchFake();
batch.TryAddCommand(command);
batch.Complete(moreBatchesExpected: false);

var actualException = async
? await Assert.ThrowsAsync<DbUpdateException>(() => batch.ExecuteAsync(connection))
: Assert.Throws<DbUpdateException>(() => batch.Execute(connection));

Assert.Same(originalException, actualException.InnerException);
}

[ConditionalFact]
public void CreateStoreCommand_creates_parameters_for_each_ModificationCommand()
{
Expand Down
56 changes: 56 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Update/NonSharedModelUpdatesSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,62 @@ OUTPUT 1
""");
}

public override async Task DbUpdateException_Entries_is_correct_with_multiple_inserts(bool async)
{
// SQL Server's bulk insert support makes it impossible to populate the entry which caused the exception, since the position
// used to find the entry is returned as an output column, but the row is never received in case of an exception.
// Instead we make sure Entries is empty.
var contextFactory = await InitializeAsync<DbContext>(onModelCreating: mb => mb.Entity<Blog>().HasIndex(b => b.Name).IsUnique());

await ExecuteWithStrategyInTransactionAsync(
contextFactory,
async context =>
{
context.Add(new Blog { Name = "Blog2" });
await context.SaveChangesAsync();
},
async context =>
{
context.Add(new Blog { Name = "Blog1" });
context.Add(new Blog { Name = "Blog2" });
context.Add(new Blog { Name = "Blog3" });
var exception = async
? await Assert.ThrowsAsync<DbUpdateException>(() => context.SaveChangesAsync())
: Assert.Throws<DbUpdateException>(() => context.SaveChanges());
Assert.Empty(exception.Entries);
});

AssertSql(
"""
@p0='Blog2' (Size = 450)

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
INSERT INTO [Blog] ([Name])
OUTPUT INSERTED.[Id]
VALUES (@p0);
""",
//
"""
@p0='Blog1' (Size = 450)
@p1='Blog2' (Size = 450)
@p2='Blog3' (Size = 450)

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
MERGE [Blog] USING (
VALUES (@p0, 0),
(@p1, 1),
(@p2, 2)) AS i ([Name], _Position) ON 1=0
WHEN NOT MATCHED THEN
INSERT ([Name])
VALUES (i.[Name])
OUTPUT INSERTED.[Id], i._Position;
""");
}

private void AssertSql(params string[] expected)
=> TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
32 changes: 31 additions & 1 deletion test/EFCore.Sqlite.FunctionalTests/Update/NonSharedModelUpdatesSqliteTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace Microsoft.EntityFrameworkCore.Update;

public class NonSharedModelUpdatesSqlServerTest : NonSharedModelUpdatesTestBase
public class NonSharedModelUpdatesSqliteTest : NonSharedModelUpdatesTestBase
{
public override async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async)
{
Expand Down Expand Up @@ -78,6 +78,36 @@ DELETE FROM "Author"
""");
}

public override async Task DbUpdateException_Entries_is_correct_with_multiple_inserts(bool async)
{
await base.DbUpdateException_Entries_is_correct_with_multiple_inserts(async);

AssertSql(
"""
@p0='Blog2' (Size = 5)

INSERT INTO "Blog" ("Name")
VALUES (@p0)
RETURNING "Id";
""",
//
"""
@p0='Blog1' (Size = 5)

INSERT INTO "Blog" ("Name")
VALUES (@p0)
RETURNING "Id";
""",
//
"""
@p0='Blog2' (Size = 5)

INSERT INTO "Blog" ("Name")
VALUES (@p0)
RETURNING "Id";
""");
}

private void AssertSql(params string[] expected)
=> TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down

0 comments on commit 004a855

Please sign in to comment.