diff --git a/api/modules/v1/controllers/OrderController.php b/api/modules/v1/controllers/OrderController.php index a21519310..cdb4a4ba4 100644 --- a/api/modules/v1/controllers/OrderController.php +++ b/api/modules/v1/controllers/OrderController.php @@ -159,8 +159,9 @@ public function actionPlaceAnOrder($id) { } } - - $response = []; + $response = null; + $orderAssemblyCommitted = false; + $transaction = Yii::$app->db->beginTransaction(); if ($order->save()) { @@ -207,6 +208,7 @@ public function actionPlaceAnOrder($id) { 'operation' => 'error', 'message' => $orderItemExtraOption->errors, ]; + break 2; } } } @@ -217,6 +219,7 @@ public function actionPlaceAnOrder($id) { 'operation' => 'error', 'message' => $orderItem->getErrors() ]; + break; } } } else { @@ -243,13 +246,13 @@ public function actionPlaceAnOrder($id) { if ($response == null) { if (!$order->updateOrderTotalPrice()) { - return [ + $response = [ 'operation' => 'error', 'message' => $order->getErrors() ]; } - if ($order->order_mode == Order::ORDER_MODE_DELIVERY && $order->subtotal < $order->restaurantDelivery->min_charge) { + if ($response == null && $order->order_mode == Order::ORDER_MODE_DELIVERY && $order->subtotal < $order->restaurantDelivery->min_charge) { $response = [ 'operation' => 'error', 'message' => 'Minimum order amount ' . Yii::$app->formatter->asCurrency( @@ -260,6 +263,12 @@ public function actionPlaceAnOrder($id) { ]; } + if ($response == null) { + $transaction->commit(); + $orderAssemblyCommitted = true; + } else { + $transaction->rollBack(); + } //if payment method not cash redirect customer to payment gateway @@ -488,8 +497,11 @@ public function actionPlaceAnOrder($id) { } } + if (!$orderAssemblyCommitted && $transaction->getIsActive()) { + $transaction->rollBack(); + } - if (array_key_exists('operation', $response) && $response['operation'] == 'error') { + if ($orderAssemblyCommitted && is_array($response) && array_key_exists('operation', $response) && $response['operation'] == 'error') { $order->delete(); } } else { @@ -501,7 +513,9 @@ public function actionPlaceAnOrder($id) { //for https://pogi.sentry.io/issues/3889482226/?project=5220572&query=is%3Aunresolved&referrer=issue-stream&stream_index=0 - $restaurant->updateStats(); + if ($restaurant) { + $restaurant->updateStats(); + } return $response; } diff --git a/tests/check-api-v1-order-atomicity.sh b/tests/check-api-v1-order-atomicity.sh new file mode 100755 index 000000000..02629e0d0 --- /dev/null +++ b/tests/check-api-v1-order-atomicity.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env sh +set -eu + +controller="api/modules/v1/controllers/OrderController.php" + +grep -q 'Yii::\$app->db->beginTransaction()' "$controller" +grep -q '\$transaction->commit();' "$controller" +grep -q '\$transaction->rollBack();' "$controller" +grep -q '\$orderAssemblyCommitted = true;' "$controller" +grep -q '!\$orderAssemblyCommitted && \$transaction->getIsActive()' "$controller" +grep -q '\$orderAssemblyCommitted && is_array(\$response)' "$controller" +grep -q 'if (\$restaurant) {' "$controller" + +if grep -n 'if (!\$order->updateOrderTotalPrice())' "$controller" | grep -q .; then + start_line=$(grep -n 'if (!\$order->updateOrderTotalPrice())' "$controller" | head -1 | cut -d: -f1) + end_line=$((start_line + 8)) + if sed -n "${start_line},${end_line}p" "$controller" | grep -q 'return \['; then + echo "v1 order total failure still returns before transaction cleanup" >&2 + exit 1 + fi +fi + +echo "api v1 order atomicity guard passed"