Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

concurrency issue when updating items in cart #640

Open
bindermuehle opened this issue Mar 9, 2020 · 0 comments
Open

concurrency issue when updating items in cart #640

bindermuehle opened this issue Mar 9, 2020 · 0 comments
Labels

Comments

@bindermuehle
Copy link

issue

calling POST /shop-api/carts/:token/items/:identifier concurrently will bring the system into a state that successive calls will return a 500 error

    "code": 500,
    "message": "Notice: Undefined offset: 9"

This error in terms is caused in the file
OrderItemsTaxesApplicator.php on line 74
$item->getQuantity() and $item->getUnits() do not return an array of the same size.

File: OrderItemsTaxesApplicator.php
57:     public function apply(OrderInterface $order, ZoneInterface $zone): void
58:     {
59:         foreach ($order->getItems() as $item) {
60:             $quantity = $item->getQuantity();
61:             Assert::notSame($quantity, 0, 'Cannot apply tax to order item with 0 quantity.');
62: 
63:             $taxRate = $this->taxRateResolver->resolve($item->getVariant(), ['zone' => $zone]);
64: 
65:             if (null === $taxRate) {
66:                 continue;
67:             }
68: 
69:             $totalTaxAmount = $this->calculator->calculate($item->getTotal(), $taxRate);
70:             $splitTaxes = $this->distributor->distribute($totalTaxAmount, $quantity);
71: 
72:             $i = 0;
73:             foreach ($item->getUnits() as $unit) {
74:                 if (0 === $splitTaxes[$i]) {
75:                     continue;
76:                 }
77: 
78:                 $this->addAdjustment($unit, $splitTaxes[$i], $taxRate->getLabel(), $taxRate->isIncludedInPrice());
79:                 ++$i;
80:             }
81:         }
82:     }

The reason for that is that the columns sylius_order_items.quantity and the amount of entries in the table sylius_order_item_unit don't match.

MySQL [sylius_dev]> select quantity,id from sylius_order_item where order_id=22;
+----------+----+
| quantity | id |
+----------+----+
|       11 | 67 |
+----------+----+
1 row in set (0.008 sec)

MySQL [sylius_dev]> select count(*) from sylius_order_item_unit where order_item_id=67;
+----------+
| count(*) |
+----------+
|       16 |
+----------+

cause

When two transaction add lines to the sylius_order_item_unit table at the same time to increase the number of entires, both transactions will succeed. The later transaction will overwrite the first transactions quantity but the both transaction will have added a row to sylius_order_item_unit.

how to reproduce:

  • install vanilla sylius
  • add shop-api-plugin
  • run symfony serve
  • POST /carts
  • add an item to the cart
  • create a few calls to the POST /shop-api/carts/:token/items/:identifier concurrently
    you can use a basic shell script to do that
for ((i=1;i<=20;i++));
do
rand=$((1 + RANDOM % 40))
curl 'http://localhost:8000/shop-api/carts/89d5c92a-f560-4029-81ed-b9d7c79e0483/items/67' -X PUT -H 'Content-Type: application/json'  --data-binary "{\"quantity\":$rand}" --compressed &
done

Ideas how to resolve this

order unit items could be numbered which number of item is it and there could be a uniq index over order_item_id and item_number which would prevent to transactions to add the same rows.
Otherwise item_units should be recalculated in every transaction before they are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants