From dbd65fd46b23b5d369ef98f7a199f437a2cceba6 Mon Sep 17 00:00:00 2001 From: Ryan Kelly Date: Mon, 5 Aug 2024 13:16:14 +0100 Subject: [PATCH] ConcurrencyFix - Added back in RowCount checking I added an additional test to ensure that the usecase I'm looking for is working --- EFCore/src/Update/MySQLUpdateSqlGenerator.cs | 30 ++++++++++-- .../ConcurrencyTests.cs | 47 +++++++++++++++++-- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/EFCore/src/Update/MySQLUpdateSqlGenerator.cs b/EFCore/src/Update/MySQLUpdateSqlGenerator.cs index 7350729a8..bb0f528ac 100644 --- a/EFCore/src/Update/MySQLUpdateSqlGenerator.cs +++ b/EFCore/src/Update/MySQLUpdateSqlGenerator.cs @@ -355,7 +355,7 @@ public virtual ResultSetMapping AppendInsertOperation( return AppendSelectAffectedCommand(commandStringBuilder, name, schema, readOperations, keyOperations); } - return readOperations.Count > 0 ? ResultSetMapping.LastInResultSet : ResultSetMapping.NoResults; + return ResultSetMapping.NoResults; } protected virtual void AppendInsertCommand( @@ -399,7 +399,7 @@ public override ResultSetMapping AppendUpdateOperation( return AppendSelectAffectedCommand(commandStringBuilder, name, schema, readOperations, keyOperations); } - return ResultSetMapping.NoResults; + return AppendSelectModificationCommand(commandStringBuilder, name); } public override ResultSetMapping AppendDeleteOperation( @@ -408,8 +408,6 @@ public override ResultSetMapping AppendDeleteOperation( int commandPosition, out bool requiresTransaction) { - // The default implementation adds RETURNING 1 to do concurrency check (was the row actually deleted), but in PostgreSQL we check - // the per-statement row-affected value exposed by Npgsql in the batch; so no need for RETURNING 1. var name = command.TableName; var schema = command.Schema; var conditionOperations = command.ColumnModifications.Where(o => o.IsCondition).ToList(); @@ -418,7 +416,7 @@ public override ResultSetMapping AppendDeleteOperation( AppendDeleteCommand(commandStringBuilder, name, schema, Array.Empty(), conditionOperations); - return ResultSetMapping.NoResults; + return AppendSelectModificationCommand(commandStringBuilder, name); } protected override void AppendValues( @@ -434,6 +432,28 @@ protected override void AppendValues( commandStringBuilder.Append("()"); } } + + /// + /// Appends a SQL command for selecting affected data. + /// + /// The builder to which the SQL should be appended. + /// The name of the table. + /// The for this command. + + private ResultSetMapping AppendSelectModificationCommand( + StringBuilder commandStringBuilder, + string name) + { + Check.NotNull(commandStringBuilder, "commandStringBuilder"); + Check.NotEmpty(name, "name"); + + commandStringBuilder + .Append("SELECT ROW_COUNT()") + .Append(SqlGenerationHelper.StatementTerminator).AppendLine() + .AppendLine(); + + return ResultSetMapping.ResultSetWithRowsAffectedOnly; + } /// /// Appends a SQL command for selecting affected data. diff --git a/EFCore/tests/MySql.EFCore.Basic.Tests/ConcurrencyTests.cs b/EFCore/tests/MySql.EFCore.Basic.Tests/ConcurrencyTests.cs index 7591baea0..9c17c7f81 100644 --- a/EFCore/tests/MySql.EFCore.Basic.Tests/ConcurrencyTests.cs +++ b/EFCore/tests/MySql.EFCore.Basic.Tests/ConcurrencyTests.cs @@ -39,7 +39,7 @@ namespace MySql.EntityFrameworkCore.Basic.Tests public class ConcurrencyTests { [Test] - public void CanHandleConcurrencyConflicts() + public void CanHandleDeleteConcurrencyConflicts() { var serviceCollection = new ServiceCollection(); serviceCollection.AddEntityFrameworkMySQL() @@ -52,7 +52,8 @@ public void CanHandleConcurrencyConflicts() context.Database.EnsureDeleted(); context.Database.EnsureCreated(); - context.People.Add(new Person { Name = "Mike Redman", SocialSecurityNumber = "SSS1229932", PhoneNumber = "565665656" }); + context.People.Add(new Person + { Name = "Mike Redman", SocialSecurityNumber = "SSS1229932", PhoneNumber = "565665656" }); context.SaveChanges(); var person = context.People.Single(p => p.PersonId == 1); @@ -72,7 +73,8 @@ public void CanHandleConcurrencyConflicts() { // Using a NoTracking query means we get the entity but it is not tracked by the context // and will not be merged with existing entities in the context. - var databaseEntity = context.People.AsNoTracking().Single(p => p.PersonId == ((Person)entry.Entity).PersonId); + var databaseEntity = context.People.AsNoTracking() + .Single(p => p.PersonId == ((Person)entry.Entity).PersonId); var databaseEntry = context.Entry(databaseEntity); foreach (var property in entry.Metadata.GetProperties()) @@ -89,7 +91,8 @@ public void CanHandleConcurrencyConflicts() } else { - throw new NotSupportedException("Don't know how to handle concurrency conflicts for " + entry.Metadata.Name); + throw new NotSupportedException("Don't know how to handle concurrency conflicts for " + + entry.Metadata.Name); } } @@ -102,5 +105,41 @@ public void CanHandleConcurrencyConflicts() } } } + + [Test] + public void CanHandleUpdateConcurrencyConflicts() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddEntityFrameworkMySQL() + .AddDbContext(); + + var serviceProvider = serviceCollection.BuildServiceProvider(); + + using (var context = serviceProvider.GetRequiredService()) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.People.Add(new Person + { Name = "Mike Redman", SocialSecurityNumber = "SSS1229932", PhoneNumber = "565665656" }); + context.SaveChanges(); + + var updatePerson1 = context.People.Single(p => p.PersonId == 1); + var updatePerson2 = context.People.Single(p => p.PersonId == 1); + + updatePerson1.SocialSecurityNumber = "SS15555"; + updatePerson1.PhoneNumber = "555555555"; + + context.SaveChanges(); + + var item = context.ChangeTracker.Entries().First(x => Object.ReferenceEquals(x.Entity, updatePerson2)); + item.OriginalValues["SocialSecurityNumber"] = "SSS1229932"; + updatePerson2.SocialSecurityNumber = "SS16666"; + updatePerson2.PhoneNumber = "666666666"; + + TestDelegate action = () => context.SaveChanges(); + Assert.Throws(action); + } + } } }