From ebdd886f2f8361c535059a4b5254a3af81eb30a3 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 09:43:39 +0600 Subject: [PATCH] fix(order): prevent duplicate order numbers under concurrent submit Allocate and persist msOrder.num under MySQL GET_LOCK, add UNIQUE index with NULL drafts, and centralize generation in OrderNumberGenerator. --- .../minishop3/lexicon/en/default.inc.php | 2 + .../minishop3/lexicon/ru/default.inc.php | 2 + ...16120000_add_unique_index_on_order_num.php | 147 ++++++++++++++ .../schema/minishop3.mysql.schema.xml | 5 +- .../Api/Manager/OrdersController.php | 41 +--- .../minishop3/src/Model/mysql/msOrder.php | 20 +- .../minishop3/src/ServiceRegistry.php | 20 +- .../Services/Order/OrderFinalizeService.php | 78 +++----- .../Services/Order/OrderNumberGenerator.php | 189 ++++++++++++++++++ .../src/Services/Order/OrderSubmitHandler.php | 96 ++++----- .../tests/OrderNumberGeneratorTest.php | 43 ++++ 11 files changed, 497 insertions(+), 146 deletions(-) create mode 100644 core/components/minishop3/migrations/20260716120000_add_unique_index_on_order_num.php create mode 100644 core/components/minishop3/src/Services/Order/OrderNumberGenerator.php create mode 100644 core/components/minishop3/tests/OrderNumberGeneratorTest.php diff --git a/core/components/minishop3/lexicon/en/default.inc.php b/core/components/minishop3/lexicon/en/default.inc.php index 2a79dfe9..100bf103 100644 --- a/core/components/minishop3/lexicon/en/default.inc.php +++ b/core/components/minishop3/lexicon/en/default.inc.php @@ -175,6 +175,8 @@ $_lang['ms3_err_user_nf'] = 'User not found.'; $_lang['ms3_err_order_nf'] = 'Order with this identifier not found.'; $_lang['ms3_err_order_load'] = 'Error loading order.'; +$_lang['ms3_err_order_num_lock'] = 'Could not acquire a lock to generate the order number. Please try again.'; +$_lang['ms3_err_order_num_save'] = 'Could not save the order number. Please try again.'; $_lang['ms3_err_status_nf'] = 'Status with this identifier not found.'; $_lang['ms3_err_delivery_nf'] = 'Delivery method with this identifier not found.'; $_lang['ms3_err_payment_nf'] = 'Payment method with this identifier not found.'; diff --git a/core/components/minishop3/lexicon/ru/default.inc.php b/core/components/minishop3/lexicon/ru/default.inc.php index 1a01983e..a2956ba5 100644 --- a/core/components/minishop3/lexicon/ru/default.inc.php +++ b/core/components/minishop3/lexicon/ru/default.inc.php @@ -175,6 +175,8 @@ $_lang['ms3_err_user_nf'] = 'Пользователь не найден.'; $_lang['ms3_err_order_nf'] = 'Заказ с таким идентификатором не найден.'; $_lang['ms3_err_order_load'] = 'Ошибка при загрузке заказа.'; +$_lang['ms3_err_order_num_lock'] = 'Не удалось получить блокировку для генерации номера заказа. Попробуйте ещё раз.'; +$_lang['ms3_err_order_num_save'] = 'Не удалось сохранить номер заказа. Попробуйте ещё раз.'; $_lang['ms3_err_status_nf'] = 'Статус с таким идентификатором не найден.'; $_lang['ms3_err_delivery_nf'] = 'Способ доставки с таким идентификатором не найден.'; $_lang['ms3_err_payment_nf'] = 'Способ оплаты с таким идентификатором не найден.'; diff --git a/core/components/minishop3/migrations/20260716120000_add_unique_index_on_order_num.php b/core/components/minishop3/migrations/20260716120000_add_unique_index_on_order_num.php new file mode 100644 index 00000000..7b393ef2 --- /dev/null +++ b/core/components/minishop3/migrations/20260716120000_add_unique_index_on_order_num.php @@ -0,0 +1,147 @@ +quoteIdentifier($this->ordersTable()); + + $table = $this->table('ms3_orders'); + $table->changeColumn('num', 'string', [ + 'limit' => 20, + 'null' => true, + 'default' => null, + ])->update(); + + $this->execute("UPDATE {$quoted} SET `num` = NULL WHERE `num` IS NULL OR `num` = ''"); + + $this->disambiguateDuplicateNums($quoted); + $this->assertNoDuplicateNums($quoted); + + if (!$table->hasIndexByName('num')) { + $table->addIndex(['num'], [ + 'unique' => true, + 'name' => 'num', + ])->update(); + } + } + + public function down(): void + { + $table = $this->table('ms3_orders'); + + if ($table->hasIndexByName('num')) { + $table->removeIndexByName('num')->update(); + } + + $quoted = $this->quoteIdentifier($this->ordersTable()); + $this->execute("UPDATE {$quoted} SET `num` = '' WHERE `num` IS NULL"); + } + + private function ordersTable(): string + { + $prefix = (string)($this->getAdapter()->getOption('table_prefix') ?? ''); + + return $prefix . 'ms3_orders'; + } + + private function disambiguateDuplicateNums(string $quotedTable): void + { + $pdo = $this->getAdapter()->getConnection(); + $dupStmt = $pdo->query( + "SELECT `num` FROM {$quotedTable} + WHERE `num` IS NOT NULL AND `num` != '' + GROUP BY `num` + HAVING COUNT(*) > 1" + ); + + if ($dupStmt === false) { + throw new \RuntimeException('Failed to detect duplicate order numbers before unique index'); + } + + $select = $pdo->prepare( + "SELECT `id` FROM {$quotedTable} WHERE `num` = ? ORDER BY `id` ASC" + ); + $update = $pdo->prepare( + "UPDATE {$quotedTable} SET `num` = ? WHERE `id` = ?" + ); + $exists = $pdo->prepare( + "SELECT 1 FROM {$quotedTable} WHERE `num` = ? LIMIT 1" + ); + + foreach ($dupStmt->fetchAll(\PDO::FETCH_ASSOC) as $row) { + $num = (string)$row['num']; + $select->execute([$num]); + $ids = array_column($select->fetchAll(\PDO::FETCH_ASSOC), 'id'); + + foreach (array_slice($ids, 1) as $index => $id) { + $newNum = $this->uniqueDisambiguatedNum($exists, $num, (int)$id, $index + 1); + if (!$update->execute([$newNum, (int)$id])) { + throw new \RuntimeException( + "Failed to disambiguate order id={$id} num={$num}" + ); + } + } + } + } + + /** + * @param \PDOStatement $exists + */ + private function uniqueDisambiguatedNum($exists, string $num, int $id, int $ordinal): string + { + for ($attempt = 0; $attempt < 50; $attempt++) { + $suffix = '-d' . $id . ($attempt > 0 ? 'x' . $attempt : ''); + if ($ordinal > 1 && $attempt === 0) { + $suffix = '-d' . $id; + } + $baseLen = max(1, 20 - strlen($suffix)); + $candidate = substr($num, 0, $baseLen) . $suffix; + if (strlen($candidate) > 20) { + $candidate = substr($candidate, 0, 20); + } + + $exists->execute([$candidate]); + if ($exists->fetchColumn() === false) { + return $candidate; + } + } + + throw new \RuntimeException("Unable to allocate unique num for order id={$id}"); + } + + private function assertNoDuplicateNums(string $quotedTable): void + { + $pdo = $this->getAdapter()->getConnection(); + $stmt = $pdo->query( + "SELECT `num` FROM {$quotedTable} + WHERE `num` IS NOT NULL AND `num` != '' + GROUP BY `num` + HAVING COUNT(*) > 1 + LIMIT 1" + ); + + if ($stmt === false) { + throw new \RuntimeException('Failed to verify unique order numbers before index'); + } + + if ($stmt->fetchColumn() !== false) { + throw new \RuntimeException('Duplicate order numbers remain; refusing to add UNIQUE index'); + } + } + + private function quoteIdentifier(string $name): string + { + return '`' . str_replace('`', '``', $name) . '`'; + } +} diff --git a/core/components/minishop3/schema/minishop3.mysql.schema.xml b/core/components/minishop3/schema/minishop3.mysql.schema.xml index 0173f849..2fbae93e 100644 --- a/core/components/minishop3/schema/minishop3.mysql.schema.xml +++ b/core/components/minishop3/schema/minishop3.mysql.schema.xml @@ -241,7 +241,7 @@ - + @@ -268,6 +268,9 @@ + + + diff --git a/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php b/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php index ce13d719..d131d4be 100644 --- a/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php +++ b/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php @@ -623,8 +623,8 @@ public function create(array $params = []): array $order->set('payment_id', (int) ($params['payment_id'] ?? 0)); $order->set('order_comment', $params['order_comment'] ?? ''); - // Order number will be generated on finalization - $order->set('num', ''); + // Order number will be generated on finalization (NULL so UNIQUE allows many drafts) + $order->set('num', null); // Initial costs (will be recalculated after adding products) $order->set('cart_cost', 0); @@ -748,43 +748,6 @@ public function finalize(array $params = []): array return Response::success($this->formatOrder($orderData), 'ms3_order_finalized')->getData(); } - /** - * Generate new order number - * - * @return string Order number like "2512/1" - */ - protected function generateOrderNum(): string - { - $format = htmlspecialchars($this->modx->getOption('ms3_order_format_num', null, 'ym')); - $separator = trim( - preg_replace( - "/[^,\/\-]/", - '', - $this->modx->getOption('ms3_order_format_num_separator', null, '/') - ) - ); - $separator = $separator ?: '/'; - - $cur = $format ? date($format) : date('ym'); - - $count = 0; - - $c = $this->modx->newQuery(msOrder::class); - $c->where(['num:LIKE' => "{$cur}%"]); - $c->select('num'); - $c->sortby('id', 'DESC'); - $c->limit(1); - if ($c->prepare() && $c->stmt->execute()) { - $num = $c->stmt->fetchColumn(); - if ($num && strpos($num, $separator) !== false) { - [, $count] = explode($separator, $num); - } - } - $count = intval($count) + 1; - - return sprintf('%s%s%d', $cur, $separator, $count); - } - /** * Get status name by ID * diff --git a/core/components/minishop3/src/Model/mysql/msOrder.php b/core/components/minishop3/src/Model/mysql/msOrder.php index 4b564ba0..016e1195 100644 --- a/core/components/minishop3/src/Model/mysql/msOrder.php +++ b/core/components/minishop3/src/Model/mysql/msOrder.php @@ -21,7 +21,7 @@ class msOrder extends \MiniShop3\Model\msOrder 'uuid' => '', 'createdon' => null, 'updatedon' => null, - 'num' => '', + 'num' => null, 'cost' => 0.0, 'cart_cost' => 0.0, 'delivery_cost' => 0.0, @@ -85,7 +85,7 @@ class msOrder extends \MiniShop3\Model\msOrder 'precision' => '20', 'phptype' => 'string', 'null' => true, - 'default' => '', + 'default' => null, ], 'cost' => [ @@ -233,6 +233,22 @@ class msOrder extends \MiniShop3\Model\msOrder ], ], ], + 'num' => + [ + 'alias' => 'num', + 'primary' => false, + 'unique' => true, + 'type' => 'BTREE', + 'columns' => + [ + 'num' => + [ + 'length' => '', + 'collation' => 'A', + 'null' => true, + ], + ], + ], 'status_id' => [ 'alias' => 'status_id', diff --git a/core/components/minishop3/src/ServiceRegistry.php b/core/components/minishop3/src/ServiceRegistry.php index 44840b80..bbb7035d 100644 --- a/core/components/minishop3/src/ServiceRegistry.php +++ b/core/components/minishop3/src/ServiceRegistry.php @@ -109,6 +109,10 @@ class ServiceRegistry 'class' => \MiniShop3\Services\Order\OrderUserResolver::class, 'interface' => null, ], + 'ms3_order_number_generator' => [ + 'class' => \MiniShop3\Services\Order\OrderNumberGenerator::class, + 'interface' => null, + ], 'ms3_order_submit_handler' => [ 'class' => \MiniShop3\Services\Order\OrderSubmitHandler::class, 'interface' => null, @@ -447,7 +451,6 @@ protected function registerService(string $serviceKey, array $config): bool 'ms3_order_cost_calculator', 'ms3_order_user_resolver', 'ms3_order_log', - 'ms3_order_finalize', 'ms3_cart_item_manager', 'ms3_customer_address_manager', ]; @@ -458,6 +461,7 @@ protected function registerService(string $serviceKey, array $config): bool 'ms3_order_address_manager', 'ms3_order_submit_handler', 'ms3_order_status', + 'ms3_order_finalize', ]; if (in_array($serviceKey, $controllersWithMs3Only)) { @@ -517,8 +521,6 @@ protected function registerServiceWithDependencies(string $serviceKey, string $v break; case 'ms3_order_submit_handler': - // OrderSubmitHandler(modX, MiniShop3, OrderDraftManager, OrderCostCalculator, - // OrderFieldManager, OrderAddressManager, OrderUserResolver) $this->modx->services->add($serviceKey, function () use ($validatedClass, $modx) { $ms3 = $modx->getService('MiniShop3', \MiniShop3\MiniShop3::class); $draftManager = $modx->services->get('ms3_order_draft_manager'); @@ -526,6 +528,7 @@ protected function registerServiceWithDependencies(string $serviceKey, string $v $fieldManager = $modx->services->get('ms3_order_field_manager'); $addressManager = $modx->services->get('ms3_order_address_manager'); $userResolver = $modx->services->get('ms3_order_user_resolver'); + $numberGenerator = $modx->services->get('ms3_order_number_generator'); return new $validatedClass( $modx, $ms3, @@ -533,11 +536,20 @@ protected function registerServiceWithDependencies(string $serviceKey, string $v $costCalculator, $fieldManager, $addressManager, - $userResolver + $userResolver, + $numberGenerator ); }); break; + case 'ms3_order_finalize': + $this->modx->services->add($serviceKey, function () use ($validatedClass, $modx) { + $ms3 = $modx->getService('MiniShop3', \MiniShop3\MiniShop3::class); + $numberGenerator = $modx->services->get('ms3_order_number_generator'); + return new $validatedClass($modx, $ms3, $numberGenerator); + }); + break; + case 'ms3_order_status': // OrderStatusService(modX, MiniShop3, OrderLogService) $this->modx->services->add($serviceKey, function () use ($validatedClass, $modx) { diff --git a/core/components/minishop3/src/Services/Order/OrderFinalizeService.php b/core/components/minishop3/src/Services/Order/OrderFinalizeService.php index 893d1433..b6d3dc68 100644 --- a/core/components/minishop3/src/Services/Order/OrderFinalizeService.php +++ b/core/components/minishop3/src/Services/Order/OrderFinalizeService.php @@ -23,11 +23,13 @@ class OrderFinalizeService { protected modX $modx; protected MiniShop3 $ms3; + protected OrderNumberGenerator $numberGenerator; - public function __construct(modX $modx, MiniShop3 $ms3) + public function __construct(modX $modx, MiniShop3 $ms3, ?OrderNumberGenerator $numberGenerator = null) { $this->modx = $modx; $this->ms3 = $ms3; + $this->numberGenerator = $numberGenerator ?? new OrderNumberGenerator($modx); } /** @@ -106,17 +108,37 @@ public function finalize(int $orderId, array $options = []): array return $costResult; } - // Generate order number if not set - if (empty($order->get('num'))) { - $order->set('num', $this->getNewOrderNum()); - } - - // Update order + // Persist costs; allocate num under GET_LOCK when still empty (#380) $order->set('updatedon', time()); $order->set('cost', $costResult['data']['total_cost']); $order->set('cart_cost', $costResult['data']['cart_cost']); $order->set('delivery_cost', $costResult['data']['delivery_cost']); - $order->save(); + + try { + if (empty($order->get('num'))) { + $this->numberGenerator->runWithNextNumber(function (string $num) use ($order): void { + $order->set('num', $num); + if (!$order->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderFinalizeService] Order save failed while assigning num=' . $num + ); + throw new \RuntimeException('ms3_err_order_num_save'); + } + }); + } elseif (!$order->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderFinalizeService] Order save failed after cost update' + ); + return $this->error('ms3_err_order_num_save'); + } + } catch (\RuntimeException $e) { + return $this->error($e->getMessage()); + } catch (\Throwable $e) { + $this->modx->log(modX::LOG_LEVEL_ERROR, '[OrderFinalizeService] ' . $e->getMessage()); + return $this->error('ms3_err_order_num_save'); + } // Event: before create order (same as frontend) $response = $this->ms3->utils->invokeEvent('msOnBeforeCreateOrder', [ @@ -427,46 +449,6 @@ protected function calculateCosts(msOrder $order): array ]); } - /** - * Generate new order number - * - * @return string - */ - protected function getNewOrderNum(): string - { - $format = htmlspecialchars($this->modx->getOption('ms3_order_format_num', null, 'ym')); - $separator = trim( - preg_replace( - "/[^,\/\-]/", - '', - $this->modx->getOption('ms3_order_format_num_separator', null, '/') - ) - ); - $separator = $separator ?: '/'; - - $prefix = $format ? date($format) : date('ym'); - - // Find last order with this prefix - $c = $this->modx->newQuery(msOrder::class); - $c->where(['num:LIKE' => "{$prefix}%"]); - $c->select('num'); - $c->sortby('id', 'DESC'); - $c->limit(1); - - $count = 0; - if ($c->prepare() && $c->stmt->execute()) { - $num = $c->stmt->fetchColumn(); - if (!empty($num)) { - $parts = explode($separator, $num); - $count = (int)($parts[1] ?? 0); - } - } - - $count++; - - return sprintf('%s%s%d', $prefix, $separator, $count); - } - /** * Success response */ diff --git a/core/components/minishop3/src/Services/Order/OrderNumberGenerator.php b/core/components/minishop3/src/Services/Order/OrderNumberGenerator.php new file mode 100644 index 00000000..e8c4e107 --- /dev/null +++ b/core/components/minishop3/src/Services/Order/OrderNumberGenerator.php @@ -0,0 +1,189 @@ +resolveFormat(); + $lockName = $this->lockName($prefix); + $locked = $this->acquireLock($lockName); + + if (!$locked) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderNumberGenerator] Failed to acquire lock: ' . $lockName + ); + throw new RuntimeException('ms3_err_order_num_lock'); + } + + try { + $lastError = null; + for ($attempt = 1; $attempt <= self::MAX_PERSIST_ATTEMPTS; $attempt++) { + $num = self::buildNumber( + $prefix, + $separator, + $this->fetchMaxCounter($prefix, $separator) + 1 + ); + try { + $persist($num); + + return $num; + } catch (RuntimeException $e) { + $lastError = $e; + if ($e->getMessage() !== 'ms3_err_order_num_save') { + throw $e; + } + $this->modx->log( + modX::LOG_LEVEL_WARN, + '[OrderNumberGenerator] Persist failed for ' . $num + . ' (attempt ' . $attempt . '/' . self::MAX_PERSIST_ATTEMPTS . ')' + ); + } + } + + throw $lastError ?? new RuntimeException('ms3_err_order_num_save'); + } finally { + $this->releaseLock($lockName); + } + } + + /** + * Peek next order number under a named lock without persisting. + * + * @deprecated Prefer runWithNextNumber() so allocate and save share one lock. + * @throws RuntimeException when the named lock cannot be acquired or counter query fails + */ + public function generate(): string + { + return $this->runWithNextNumber(static function (string $num): void { + }); + } + + /** + * @return array{0: string, 1: string} [prefix, separator] + */ + public function resolveFormat(): array + { + $format = htmlspecialchars((string)$this->modx->getOption('ms3_order_format_num', null, 'ym')); + $separator = trim( + preg_replace( + "/[^,\/\-]/", + '', + (string)$this->modx->getOption('ms3_order_format_num_separator', null, '/') + ) ?? '' + ); + $separator = $separator !== '' ? $separator : '/'; + $prefix = $format !== '' ? date($format) : date('ym'); + + return [$prefix, $separator]; + } + + /** + * Parse counter from an existing order number. + * + * @internal used by unit tests + */ + public static function parseCounter(string $num, string $separator): int + { + if ($num === '' || !str_contains($num, $separator)) { + return 0; + } + + $parts = explode($separator, $num); + + return (int)($parts[1] ?? 0); + } + + /** + * @internal used by unit tests + */ + public static function buildNumber(string $prefix, string $separator, int $count): string + { + return sprintf('%s%s%d', $prefix, $separator, $count); + } + + /** + * @throws RuntimeException when the counter query fails + */ + private function fetchMaxCounter(string $prefix, string $separator): int + { + $table = $this->modx->getTableName(msOrder::class); + $like = $prefix . $separator . '%'; + + $sql = "SELECT MAX(CAST(SUBSTRING_INDEX(num, ?, -1) AS UNSIGNED)) + FROM {$table} + WHERE num IS NOT NULL AND num != '' AND num LIKE ?"; + + $stmt = $this->modx->prepare($sql); + if (!$stmt || !$stmt->execute([$separator, $like])) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderNumberGenerator] Counter query failed for prefix ' . $prefix + ); + throw new RuntimeException('ms3_err_order_num_lock'); + } + + $max = $stmt->fetchColumn(); + + return $max === false || $max === null || $max === '' ? 0 : (int)$max; + } + + private function lockName(string $prefix): string + { + return 'ms3_ord_num_' . substr(hash('sha256', $prefix), 0, 24); + } + + private function acquireLock(string $lockName): bool + { + $stmt = $this->modx->prepare('SELECT GET_LOCK(?, ?)'); + if (!$stmt || !$stmt->execute([$lockName, self::LOCK_TIMEOUT_SECONDS])) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderNumberGenerator] GET_LOCK prepare/execute failed: ' . $lockName + ); + + return false; + } + + return (int)$stmt->fetchColumn() === 1; + } + + private function releaseLock(string $lockName): void + { + $stmt = $this->modx->prepare('SELECT RELEASE_LOCK(?)'); + if (!$stmt || !$stmt->execute([$lockName])) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderNumberGenerator] RELEASE_LOCK failed: ' . $lockName + ); + } + } +} diff --git a/core/components/minishop3/src/Services/Order/OrderSubmitHandler.php b/core/components/minishop3/src/Services/Order/OrderSubmitHandler.php index fe454034..7381647d 100644 --- a/core/components/minishop3/src/Services/Order/OrderSubmitHandler.php +++ b/core/components/minishop3/src/Services/Order/OrderSubmitHandler.php @@ -22,6 +22,7 @@ class OrderSubmitHandler protected OrderFieldManager $fieldManager; protected OrderAddressManager $addressManager; protected OrderUserResolver $userResolver; + protected OrderNumberGenerator $numberGenerator; public function __construct( modX $modx, @@ -30,7 +31,8 @@ public function __construct( OrderCostCalculator $costCalculator, OrderFieldManager $fieldManager, OrderAddressManager $addressManager, - OrderUserResolver $userResolver + OrderUserResolver $userResolver, + ?OrderNumberGenerator $numberGenerator = null ) { $this->modx = $modx; $this->ms3 = $ms3; @@ -39,6 +41,7 @@ public function __construct( $this->fieldManager = $fieldManager; $this->addressManager = $addressManager; $this->userResolver = $userResolver; + $this->numberGenerator = $numberGenerator ?? new OrderNumberGenerator($modx); } /** @@ -174,23 +177,40 @@ public function submit( $cartCost = $costData['cart_cost']; $totalCost = (float) $costData['cost']; - // Generate order number - $num = $this->getNewOrderNum(); - - // Update draft with final data (cost matches calculator: cart + delivery + payment) - - $draft->fromArray([ - 'customer_id' => $customerId, - 'user_id' => $userId, - 'updatedon' => time(), - 'num' => $num, - 'cart_cost' => $cartCost, - 'delivery_cost' => $deliveryCost, - 'cost' => $totalCost, - ]); - - $draft->Address->set('updatedon', time()); - $draft->save(); + // Allocate order number and persist costs under GET_LOCK (#380) + try { + $this->numberGenerator->runWithNextNumber(function (string $num) use ( + $draft, + $customerId, + $userId, + $cartCost, + $deliveryCost, + $totalCost + ): void { + $draft->fromArray([ + 'customer_id' => $customerId, + 'user_id' => $userId, + 'updatedon' => time(), + 'num' => $num, + 'cart_cost' => $cartCost, + 'delivery_cost' => $deliveryCost, + 'cost' => $totalCost, + ]); + $draft->Address->set('updatedon', time()); + if (!$draft->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderSubmitHandler] Order save failed while assigning num=' . $num + ); + throw new \RuntimeException('ms3_err_order_num_save'); + } + }); + } catch (\RuntimeException $e) { + return $this->error($e->getMessage()); + } catch (\Throwable $e) { + $this->modx->log(modX::LOG_LEVEL_ERROR, '[OrderSubmitHandler] ' . $e->getMessage()); + return $this->error('ms3_err_order_num_save'); + } // Save address to customer's saved addresses if requested $properties = $draft->get('properties') ?? []; @@ -271,46 +291,18 @@ public function submit( } /** - * Generate new order number + * Peek next order number (legacy / plugin API). * - * Format: {date_format}{separator}{counter} - * Example: 2501/1, 2501/2, etc. - * - * @return string Generated order number + * @deprecated Prefer OrderNumberGenerator::runWithNextNumber() so allocate and save share one lock. */ public function getNewOrderNum(): string { - $format = htmlspecialchars($this->modx->getOption('ms3_order_format_num', null, 'ym')); - $separator = trim( - preg_replace( - "/[^,\/\-]/", - '', - $this->modx->getOption('ms3_order_format_num_separator', null, '/') - ) + $this->modx->log( + modX::LOG_LEVEL_WARN, + '[OrderSubmitHandler] getNewOrderNum() is deprecated; use runWithNextNumber() for atomic allocate+save' ); - $separator = $separator ?: '/'; - - $prefix = $format ? date($format) : date('ym'); - - // Find last order with this prefix - $c = $this->modx->newQuery(msOrder::class); - $c->where(['num:LIKE' => "{$prefix}%"]); - $c->select('num'); - $c->sortby('id', 'DESC'); - $c->limit(1); - - $count = 0; - if ($c->prepare() && $c->stmt->execute()) { - $num = $c->stmt->fetchColumn(); - if (!empty($num)) { - $parts = explode($separator, $num); - $count = (int)($parts[1] ?? 0); - } - } - - $count++; - return sprintf('%s%s%d', $prefix, $separator, $count); + return $this->numberGenerator->generate(); } /** diff --git a/core/components/minishop3/tests/OrderNumberGeneratorTest.php b/core/components/minishop3/tests/OrderNumberGeneratorTest.php new file mode 100644 index 00000000..6c964b81 --- /dev/null +++ b/core/components/minishop3/tests/OrderNumberGeneratorTest.php @@ -0,0 +1,43 @@ +