1

I have an app + website that uses an API where you can purchase goods for virtual currency.

When you've bought an item you can do a refund and get your money back.

Below is some pseudo code to explain.

api/order/refund/{orderid}

if(order.status == accepted)
{
    order.status = refunded;
    user.AddMoney(order.Amount);
    dbcontext.saveChanges();
}

the problem is, if the API gets called like api/order/refund/21 multiple times quite fast, it will run before saveChanges(); is successful and will AddMoney(); to the user more than once for the same order.

Does anybody know a solution for this?

Thanks

que
  • 188
  • 1
  • 11
  • 1
    This may help. http://stackoverflow.com/questions/13404061/how-can-i-lock-a-table-on-read-using-entity-framework You need to lock the table for the duration of your transaction. – CollinD Sep 26 '15 at 18:52

3 Answers3

0

Use a lock like in the sample below

private Object thisLock = new Object();
if(order.status == accepted)
{
 lock(thisLock)
{

 if(order.status == accepted)
 {
  order.status = refunded;
  user.AddMoney(order.Amount);
  dbcontext.saveChanges();

 }
}    
}
shat90
  • 405
  • 3
  • 8
  • I used the solution I posted above. Your solution is more simple but I think it would prevent other users from refunding their products at the same time as it locks the statement to only use one thread? – que Sep 26 '15 at 20:06
  • the lock should wrap the check `order.status == accepted`, otherwise it has no meaning at all. After passing the check, sooner or later the inside code will be executed more than once. When wrapping the check, once the first refunding has been done, the next check will be failed (order.status != accepted) to let execution go inside. – Hopeless Sep 26 '15 at 22:35
  • @Hopeless, thanks made an edit to accommodate that. -- to que, I do not understand how multiple users can have the same product? My understanding would be that each productId would be uniquely assigned to a user who bought it. please correct my understanding – shat90 Sep 27 '15 at 08:37
  • @shat90 the scenario here may be that the same user may log in via multiple pages/interfaces/devices ... So it's better to prevent such a case. BTW your edit is still wrong (redundant), we don't need 2 checks `order.status == accepted` (the outer one should be removed) – Hopeless Sep 27 '15 at 08:43
  • I dont want to lock unless I am sure the order.status is accepted. I put the lock outside the first one, it locks everytime instead of only on the appropriate status, hence not redundant! – shat90 Sep 27 '15 at 09:02
0

I ended up doing like this:

 var vctx = new VenueContext();
        using(var dbtransaction = vctx.Database.BeginTransaction(IsolationLevel.RepeatableRead))
        {
            try
            {
                var order = vctx.Orders.Include("Venue").Include("Transactions.MenuItem").FirstOrDefault(x => x.Id == orderid);
                if (order.StatusId != 1)
                {
                    return false;
                }
                if (status == 3 || status == 4)
                {
                    vctx = MoneyRepository.AddMoney(order.UserId, order.Total * -1, vctx);
                }
                order.StatusId = status;
                vctx.SaveChanges();
                dbtransaction.Commit();
            }
            catch (Exception)
            {
                dbtransaction.Rollback();
                return false;
            } 
        }
        return true;
    }
que
  • 188
  • 1
  • 11
0

I ended using https://github.com/stefanprodan/WebApiThrottle which you can use to limit request amount on selected endpoint.

behruz kosari
  • 24
  • 1