search

OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
869 stars 436 forks source link

Issue with one use limited coupon #1408

Open diogoceribelli opened 3 years ago

diogoceribelli commented 3 years ago

Magento 1.9

Steps to reproduce (*)

  1. Administrator creates a new discount coupon, limited to one use;
  2. Two or more users insert products into their shopping cart and apply the discount using the coupon;
  3. Two or more users click on checkout at the same time.

Expected result (*)

  1. Even if pressing at the same time, some order must have been placed first;
  2. This order will receive the coupon discount, subsequent orders will receive some type of error saying that the coupon has already been used;
  3. Now the coupon that had only one use, can no longer be used.

Actual result (*)

  1. All users who pressed the checkout button at the same time will receive the coupon discount even with the one use limit;
  2. Only after simultaneous orders will the coupon be deactivated.

The problem then is that purchases made simultaneously will disregard the limit defined in the discount coupon rule. This should not happen, only one of those purchases should receive the discount.

colinmollenhour commented 3 years ago

Magento is actually rife with possible race conditions.. The only place it uses real locking (SELECT ... FOR UPDATE) is around the inventory. Everything else is anything goes. It may be possible to reorganize the code so that coupon loading and updates happen inside the transaction after the inventory is locked which would probably fix the issue. I personally am not in a position to tackle this myself right now, though.

kiatng commented 3 years ago

I can confirm @colinmollenhour that real locking is SELECT ... FOR UPDATE, an example:

https://github.com/OpenMage/magento-lts/blob/bbd05bee0f8b07f4abd5f9555265e65ea42f3496/app/code/core/Mage/Eav/Model/Entity/Type.php#L163-L223

I had used similar method above in a custom module to resolve a race condition.

I hope this comment is useful for tackling this issue.

[edit] I think this is the code @colinmollenhour is referring:

https://github.com/OpenMage/magento-lts/blob/bbd05bee0f8b07f4abd5f9555265e65ea42f3496/app/code/core/Mage/CatalogInventory/Model/Resource/Stock.php#L154-L192

lucasfpiovesan12 commented 3 years ago

@kiatng But how to implement this in the shopping cart rule?