Reputation: 731
I have the following scenario:
At the moment, when creating a new order, this requirement is implemented as follows:
var newOrder = /* logic for creating the new order */;
var orders = _ordersRepository.GetAllBy(userId, date); // get the orders from the db
var totalAmount = orders.Sum(o => o.Amount);
if(totalAmount < MaximumAmount) {
newOrder.IsApproved = true;
}
else {
newOrder.IsApproved = false;
}
_ordersRepository.Add(newOrder);
_ordersRepository.SaveChanges(); // insert into the db
The problem with this approach is that it doesn't handle concurrent insertions properly in scenarios like:
The requests are handled concurrently and because of the short timeframe, the currently implemented check is executed before the new orders are saved in the database and therefore allows creating all of them. In the end, the user ends exceeding the maximum limit.
How could I solve this issue, ideally without having to call SaveChanges
multiple times? I'm using Entity Framework Core 5 and SQL Server.
Upvotes: 5
Views: 4282
Reputation: 406
you can use lock statement to prevent user parallel requests.
https://github.com/Darkseal/LockProvider
public static async Task CreateOrder(currentUserId)
{
// lock for currentUserId only
await LockProvider.WaitAsync(currentUserId);
try
{
var newOrder = /* logic for creating the new order */;
var orders = _ordersRepository.GetAllBy(userId, date); // get the orders from the db
var totalAmount = orders.Sum(o => o.Amount);
if(totalAmount < MaximumAmount) {
newOrder.IsApproved = true;
}
else {
newOrder.IsApproved = false;
}
_ordersRepository.Add(newOrder);
_ordersRepository.SaveChanges(); // insert into the db
}
finally
{
// release the lock
LockProvider.Release(currentUserId);
}
}
Upvotes: 2
Reputation: 731
Thank you for the suggestions from the comments!
I've tried using transactions with the IsolationLevel
set to Serializable
, but then I've realized this would lock too many tables (the example from the question is a dummy one, the actual implementation is more complex).
I agree that sometimes it might be easier to have this kind of logic in the database, but adding a stored procedure for this will kind of break the current consistency and most probably leave the door open for other stored procedures. I'm not saying stored procedures are bad, just that in my current situation, even if it is a bit harder/more complex to achieve this without them, I believe it's worth it for consistency reasons.
I've ended splitting the flow in 2 steps as follows:
// step 1
var newOrder = /* logic for creating the new order */;
_ordersRepository.Add(newOrder);
_ordersRepository.SaveChanges(); // insert into the db
// step 2
var orders = _ordersRepository.GetAllBy(userId, date); // get the orders from the db
var totalAmount = orders.Sum(o => o.Amount);
if(totalAmount < MaximumAmount) {
newOrder.IsApproved = true;
}
_ordersRepository.Update(newOrder);
_ordersRepository.SaveChanges(); // update the new order
IsApproved
flag being left to the default which is false
.IsApproved
flag to true
.I know it's not an actual solution, but a workaround. Locking a table might have a too big performance impact, especially if the given table is used by multiple app features. With this solution, even if there was an issue in the Step 2, the order will be left with IsApproved=false
so it won't have any impact and the user can either try again later, or somebody from support can handle it.
Upvotes: 2