From 0cc5d0e77ad38285aee5e6241ca7b59ba9219e3a Mon Sep 17 00:00:00 2001 From: Jeanmarcos Juarez Date: Sun, 12 Jul 2026 23:10:28 -0400 Subject: [PATCH] feat(inventory-sales): add reservation invariant guards and reconciliation --- .../etc/adminhtml/system.xml | 26 +- InventoryReservations/etc/config.xml | 3 + .../Command/ReconcileReservationsCommand.php | 84 ++++++ InventorySales/Cron/ReconcileReservations.php | 61 +++++ .../GetOrderReservationBalance.php | 71 +++++ .../GetOrderReservationLedger.php | 75 ++++++ ...TerminalOrdersWithResidualReservations.php | 79 ++++++ .../ReservationClampLock.php | 95 +++++++ .../ReconcileOrderReservations.php | 97 +++++++ .../ReconcileReservationsSweep.php | 71 +++++ .../ReconciliationConfig.php | 48 ++++ ...ReservationsOnOrderStateChangeObserver.php | 63 +++++ .../ClampCompensationReservationsPlugin.php | 255 ++++++++++++++++++ .../RejectOversellingReservationsPlugin.php | 90 +++++++ .../Unit/Cron/ReconcileReservationsTest.php | 61 +++++ .../ReservationClampLockTest.php | 79 ++++++ .../ReconcileOrderReservationsTest.php | 128 +++++++++ .../ReconcileReservationsSweepTest.php | 98 +++++++ ...rvationsOnOrderStateChangeObserverTest.php | 94 +++++++ ...lampCompensationReservationsPluginTest.php | 200 ++++++++++++++ ...ejectOversellingReservationsPluginTest.php | 181 +++++++++++++ InventorySales/etc/crontab.xml | 14 + InventorySales/etc/di.xml | 9 + InventorySales/etc/events.xml | 3 + 24 files changed, 1983 insertions(+), 2 deletions(-) create mode 100644 InventorySales/Console/Command/ReconcileReservationsCommand.php create mode 100644 InventorySales/Cron/ReconcileReservations.php create mode 100644 InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationBalance.php create mode 100644 InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationLedger.php create mode 100644 InventorySales/Model/ResourceModel/SourceReservation/GetTerminalOrdersWithResidualReservations.php create mode 100644 InventorySales/Model/ResourceModel/SourceReservation/ReservationClampLock.php create mode 100644 InventorySales/Model/SourceReservation/ReconcileOrderReservations.php create mode 100644 InventorySales/Model/SourceReservation/ReconcileReservationsSweep.php create mode 100644 InventorySales/Model/SourceReservation/ReconciliationConfig.php create mode 100644 InventorySales/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserver.php create mode 100644 InventorySales/Plugin/InventoryReservationsApi/ClampCompensationReservationsPlugin.php create mode 100644 InventorySales/Plugin/InventoryReservationsApi/RejectOversellingReservationsPlugin.php create mode 100644 InventorySales/Test/Unit/Cron/ReconcileReservationsTest.php create mode 100644 InventorySales/Test/Unit/Model/ResourceModel/SourceReservation/ReservationClampLockTest.php create mode 100644 InventorySales/Test/Unit/Model/SourceReservation/ReconcileOrderReservationsTest.php create mode 100644 InventorySales/Test/Unit/Model/SourceReservation/ReconcileReservationsSweepTest.php create mode 100644 InventorySales/Test/Unit/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserverTest.php create mode 100644 InventorySales/Test/Unit/Plugin/InventoryReservationsApi/ClampCompensationReservationsPluginTest.php create mode 100644 InventorySales/Test/Unit/Plugin/InventoryReservationsApi/RejectOversellingReservationsPluginTest.php create mode 100644 InventorySales/etc/crontab.xml diff --git a/InventoryCatalogAdminUi/etc/adminhtml/system.xml b/InventoryCatalogAdminUi/etc/adminhtml/system.xml index 182295774f2b..dbba4b1b4215 100755 --- a/InventoryCatalogAdminUi/etc/adminhtml/system.xml +++ b/InventoryCatalogAdminUi/etc/adminhtml/system.xml @@ -1,8 +1,8 @@ Split sales reservations per source so they affect every stock sharing that source. Changing this value requires a full inventory reindex. + + + Magento\Config\Model\Config\Source\Yesno + When an order reaches a final state, append any release its cancel/credit-memo + observer failed to write. Bounded so it can never over-release. + + + + Magento\Config\Model\Config\Source\Yesno + Periodically reconcile terminal orders whose reservations were never released, + including drift from direct database edits or third-party state changes. + + + + + 1 + + Cron expression controlling how often the reconciliation sweep runs. + diff --git a/InventoryReservations/etc/config.xml b/InventoryReservations/etc/config.xml index d9a0dbe6cb92..83e9828e5c13 100644 --- a/InventoryReservations/etc/config.xml +++ b/InventoryReservations/etc/config.xml @@ -11,6 +11,9 @@ 0 + 0 + 0 + 0 * * * * diff --git a/InventorySales/Console/Command/ReconcileReservationsCommand.php b/InventorySales/Console/Command/ReconcileReservationsCommand.php new file mode 100644 index 000000000000..228e4945dbd4 --- /dev/null +++ b/InventorySales/Console/Command/ReconcileReservationsCommand.php @@ -0,0 +1,84 @@ +setName('inventory:reservation:reconcile'); + $this->setDescription('Reconcile terminal orders whose reservations were never released.'); + $this->addOption( + self::OPTION_DRY_RUN, + null, + InputOption::VALUE_NONE, + 'Report the residues without writing any compensation.' + ); + $this->addOption( + self::OPTION_LIMIT, + null, + InputOption::VALUE_REQUIRED, + 'Maximum number of orders to process.', + (string)self::DEFAULT_LIMIT + ); + parent::configure(); + } + + /** + * @inheritdoc + */ + protected function execute(InputInterface $input, OutputInterface $output): int + { + $dryRun = (bool)$input->getOption(self::OPTION_DRY_RUN); + $limit = max(1, (int)$input->getOption(self::OPTION_LIMIT)); + + $result = $this->reconcileReservationsSweep->execute($limit, $dryRun); + + $output->writeln(sprintf( + '%s %d order(s), %d compensation(s)%s.', + $dryRun ? 'Would reconcile' : 'Reconciled', + $result['orders'], + $result['compensations'], + $result['stock_ids'] ? ' across stock(s) ' . implode(', ', $result['stock_ids']) : '' + )); + if ($result['limit_reached']) { + $output->writeln('Batch limit reached; run again to process the remaining residues.'); + } + + return Command::SUCCESS; + } +} diff --git a/InventorySales/Cron/ReconcileReservations.php b/InventorySales/Cron/ReconcileReservations.php new file mode 100644 index 000000000000..eeac73a6551a --- /dev/null +++ b/InventorySales/Cron/ReconcileReservations.php @@ -0,0 +1,61 @@ +reconciliationConfig->isSweepEnabled()) { + return; + } + + $result = $this->reconcileReservationsSweep->execute(self::BATCH_LIMIT); + if ($result['orders'] > 0) { + $this->logger->info( + 'Source-level reservations: reconciliation sweep healed residual reservations.', + ['orders' => $result['orders'], 'compensations' => $result['compensations']] + ); + } + if ($result['limit_reached']) { + $this->logger->warning( + 'Source-level reservations: reconciliation sweep hit its batch limit; residues remain.', + ['limit' => self::BATCH_LIMIT] + ); + } + } +} diff --git a/InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationBalance.php b/InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationBalance.php new file mode 100644 index 000000000000..fba9c7513beb --- /dev/null +++ b/InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationBalance.php @@ -0,0 +1,71 @@ +> [sku][source_code|''] => SUM(quantity) + */ + public function execute(string $objectIncrementId, array $skus, int $stockId): array + { + if (empty($skus) || $objectIncrementId === '') { + return []; + } + + $connection = $this->resourceConnection->getConnection(); + $incrementIdExpr = sprintf( + "COALESCE(%s, JSON_UNQUOTE(JSON_EXTRACT(metadata, '$.object_increment_id')))", + ReservationInterface::OBJECT_INCREMENT_ID + ); + $select = $connection->select() + ->from( + $this->resourceConnection->getTableName('inventory_reservation'), + [ + ReservationInterface::SKU, + ReservationInterface::SOURCE_CODE, + 'quantity' => 'SUM(' . ReservationInterface::QUANTITY . ')', + ] + ) + ->where(ReservationInterface::STOCK_ID . ' = ?', $stockId) + ->where(ReservationInterface::SKU . ' IN (?)', $skus) + ->where($incrementIdExpr . ' = ?', $objectIncrementId) + ->group([ReservationInterface::SKU, ReservationInterface::SOURCE_CODE]); + + $result = []; + foreach ($connection->fetchAll($select) as $row) { + $result[$row[ReservationInterface::SKU]][(string)$row[ReservationInterface::SOURCE_CODE]] = + (float)$row['quantity']; + } + + return $result; + } +} diff --git a/InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationLedger.php b/InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationLedger.php new file mode 100644 index 000000000000..a3342ac7bdeb --- /dev/null +++ b/InventorySales/Model/ResourceModel/SourceReservation/GetOrderReservationLedger.php @@ -0,0 +1,75 @@ + + */ + public function execute(string $objectIncrementId): array + { + if ($objectIncrementId === '') { + return []; + } + + $connection = $this->resourceConnection->getConnection(); + $incrementIdExpr = sprintf( + "COALESCE(%s, JSON_UNQUOTE(JSON_EXTRACT(metadata, '$.object_increment_id')))", + ReservationInterface::OBJECT_INCREMENT_ID + ); + $select = $connection->select() + ->from( + $this->resourceConnection->getTableName('inventory_reservation'), + [ + ReservationInterface::STOCK_ID, + ReservationInterface::SKU, + ReservationInterface::SOURCE_CODE, + 'balance' => 'SUM(' . ReservationInterface::QUANTITY . ')', + ] + ) + ->where($incrementIdExpr . ' = ?', $objectIncrementId) + ->group([ReservationInterface::STOCK_ID, ReservationInterface::SKU, ReservationInterface::SOURCE_CODE]) + ->having('SUM(' . ReservationInterface::QUANTITY . ') < ?', -self::EPSILON); + + $rows = []; + foreach ($connection->fetchAll($select) as $row) { + $rows[] = [ + 'stock_id' => (int)$row[ReservationInterface::STOCK_ID], + 'sku' => (string)$row[ReservationInterface::SKU], + 'source_code' => $row[ReservationInterface::SOURCE_CODE] !== null + ? (string)$row[ReservationInterface::SOURCE_CODE] + : null, + 'balance' => (float)$row['balance'], + ]; + } + + return $rows; + } +} diff --git a/InventorySales/Model/ResourceModel/SourceReservation/GetTerminalOrdersWithResidualReservations.php b/InventorySales/Model/ResourceModel/SourceReservation/GetTerminalOrdersWithResidualReservations.php new file mode 100644 index 000000000000..606c3294fb03 --- /dev/null +++ b/InventorySales/Model/ResourceModel/SourceReservation/GetTerminalOrdersWithResidualReservations.php @@ -0,0 +1,79 @@ + + */ + public function execute(int $limit): array + { + $connection = $this->resourceConnection->getConnection(); + $incrementIdExpr = sprintf( + "COALESCE(%s, JSON_UNQUOTE(JSON_EXTRACT(metadata, '$.object_increment_id')))", + ReservationInterface::OBJECT_INCREMENT_ID + ); + $residualSelect = $connection->select() + ->from( + $this->resourceConnection->getTableName('inventory_reservation'), + ['increment_id' => $incrementIdExpr] + ) + ->group('increment_id') + ->having('SUM(' . ReservationInterface::QUANTITY . ') < ?', -self::EPSILON); + + $select = $connection->select() + ->from( + ['so' => $this->resourceConnection->getTableName('sales_order')], + ['object_id' => 'entity_id', 'increment_id' => 'increment_id', 'state' => 'state'] + ) + ->join( + ['residual' => $residualSelect], + 'residual.increment_id = so.increment_id', + [] + ) + ->where('so.state IN (?)', self::TERMINAL_STATES) + ->limit($limit); + + $rows = []; + foreach ($connection->fetchAll($select) as $row) { + $rows[] = [ + 'object_id' => (int)$row['object_id'], + 'increment_id' => (string)$row['increment_id'], + 'state' => (string)$row['state'], + ]; + } + + return $rows; + } +} diff --git a/InventorySales/Model/ResourceModel/SourceReservation/ReservationClampLock.php b/InventorySales/Model/ResourceModel/SourceReservation/ReservationClampLock.php new file mode 100644 index 000000000000..9840d5a0bbda --- /dev/null +++ b/InventorySales/Model/ResourceModel/SourceReservation/ReservationClampLock.php @@ -0,0 +1,95 @@ +lockNames($items); + if (empty($names)) { + return []; + } + + $connection = $this->resourceConnection->getConnection(); + $acquired = []; + foreach ($names as $name) { + $connection->fetchOne('SELECT GET_LOCK(?, ?)', [$name, self::LOCK_TIMEOUT]); + $acquired[] = $name; + } + + return $acquired; + } + + /** + * Release the given lock names. + * + * @param string[] $names + * @return void + */ + public function release(array $names): void + { + if (empty($names)) { + return; + } + + $connection = $this->resourceConnection->getConnection(); + foreach ($names as $name) { + try { + $connection->fetchOne('SELECT RELEASE_LOCK(?)', [$name]); + } catch (\Throwable $e) { //phpcs:ignore Magento2.CodeAnalysis.EmptyBlock.DetectedCatch + // Locks are released by MySQL when the connection closes. + } + } + } + + /** + * Build the globally-ordered, de-duplicated lock names for the items. + * + * @param array $items + * @return string[] + */ + private function lockNames(array $items): array + { + $names = []; + foreach ($items as $item) { + // phpcs:ignore Magento2.Security.InsecureFunction + $names[] = sprintf('inv_rsv_clamp_%d_%s', $item['stock_id'], md5($item['sku'])); + } + $names = array_values(array_unique($names)); + sort($names, SORT_STRING); + + return $names; + } +} diff --git a/InventorySales/Model/SourceReservation/ReconcileOrderReservations.php b/InventorySales/Model/SourceReservation/ReconcileOrderReservations.php new file mode 100644 index 000000000000..87e24e61ee7a --- /dev/null +++ b/InventorySales/Model/SourceReservation/ReconcileOrderReservations.php @@ -0,0 +1,97 @@ + + */ + public function execute(int $objectId, string $objectIncrementId, string $orderState, bool $dryRun = false): array + { + if ($objectIncrementId === '' || !in_array($orderState, self::TERMINAL_STATES, true)) { + return []; + } + + $ledger = $this->getOrderReservationLedger->execute($objectIncrementId); + if (empty($ledger)) { + return []; + } + + $metadata = $this->serializer->serialize([ + 'event_type' => 'reconciliation', + 'object_type' => 'order', + 'object_id' => (string)$objectId, + 'object_increment_id' => $objectIncrementId, + ]); + + $compensations = []; + $reservations = []; + foreach ($ledger as $row) { + $quantity = -$row['balance']; + $compensations[] = [ + 'stock_id' => $row['stock_id'], + 'sku' => $row['sku'], + 'source_code' => $row['source_code'], + 'quantity' => $quantity, + ]; + $reservations[] = $this->reservationBuilder + ->setSku($row['sku']) + ->setQuantity($quantity) + ->setStockId($row['stock_id']) + ->setMetadata($metadata) + ->setSourceCode($row['source_code']) + ->setObjectIncrementId($objectIncrementId) + ->build(); + } + + if (!$dryRun && !empty($reservations)) { + $this->appendReservations->execute($reservations); + } + + return $compensations; + } +} diff --git a/InventorySales/Model/SourceReservation/ReconcileReservationsSweep.php b/InventorySales/Model/SourceReservation/ReconcileReservationsSweep.php new file mode 100644 index 000000000000..3d3b9ee52f0c --- /dev/null +++ b/InventorySales/Model/SourceReservation/ReconcileReservationsSweep.php @@ -0,0 +1,71 @@ +getTerminalOrdersWithResidualReservations->execute($limit); + + $compensations = 0; + $stockIds = []; + foreach ($orders as $order) { + $made = $this->reconcileOrderReservations->execute( + $order['object_id'], + $order['increment_id'], + $order['state'], + $dryRun + ); + $compensations += count($made); + foreach ($made as $entry) { + $stockIds[$entry['stock_id']] = $entry['stock_id']; + } + } + + $stockIds = array_values($stockIds); + if (!$dryRun && !empty($stockIds)) { + $this->stockIndexer->executeList($stockIds); + } + + return [ + 'orders' => count($orders), + 'compensations' => $compensations, + 'stock_ids' => $stockIds, + 'limit_reached' => count($orders) >= $limit, + ]; + } +} diff --git a/InventorySales/Model/SourceReservation/ReconciliationConfig.php b/InventorySales/Model/SourceReservation/ReconciliationConfig.php new file mode 100644 index 000000000000..722dda89aee0 --- /dev/null +++ b/InventorySales/Model/SourceReservation/ReconciliationConfig.php @@ -0,0 +1,48 @@ +scopeConfig->isSetFlag(self::XML_PATH_CANCEL_REFUND); + } + + /** + * Whether the periodic out-of-band reconciliation sweep is enabled. + * + * @return bool + */ + public function isSweepEnabled(): bool + { + return $this->scopeConfig->isSetFlag(self::XML_PATH_SWEEP_ENABLED); + } +} diff --git a/InventorySales/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserver.php b/InventorySales/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserver.php new file mode 100644 index 000000000000..baaca3954aa7 --- /dev/null +++ b/InventorySales/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserver.php @@ -0,0 +1,63 @@ +reconciliationConfig->isCancelRefundReconciliationEnabled()) { + return; + } + + $order = $observer->getEvent()->getOrder(); + if (!$order instanceof Order) { + return; + } + + $state = (string)$order->getState(); + if ($state === (string)$order->getOrigData('state') || !in_array($state, self::TERMINAL_STATES, true)) { + return; + } + + $incrementId = (string)$order->getIncrementId(); + if ($incrementId === '') { + return; + } + + $this->reconcileOrderReservations->execute((int)$order->getId(), $incrementId, $state); + } +} diff --git a/InventorySales/Plugin/InventoryReservationsApi/ClampCompensationReservationsPlugin.php b/InventorySales/Plugin/InventoryReservationsApi/ClampCompensationReservationsPlugin.php new file mode 100644 index 000000000000..54d97febca0b --- /dev/null +++ b/InventorySales/Plugin/InventoryReservationsApi/ClampCompensationReservationsPlugin.php @@ -0,0 +1,255 @@ +releaseItems($reservations); + if (empty($lockItems)) { + return $proceed($reservations); + } + + $locks = $this->reservationClampLock->acquire($lockItems); + try { + $balances = $this->loadBalances($reservations); + $consumed = []; + + $result = []; + foreach ($reservations as $reservation) { + $clamped = $this->clamp($reservation, $balances, $consumed); + if ($clamped !== null) { + $result[] = $clamped; + } + } + + if (empty($result)) { + return; + } + + return $proceed($result); + } finally { + $this->reservationClampLock->release($locks); + } + } + + /** + * Collect the distinct (stock, sku) of the compensation (positive) rows to lock. + * + * @param ReservationInterface[] $reservations + * @return array + */ + private function releaseItems(array $reservations): array + { + $items = []; + foreach ($reservations as $reservation) { + if ($reservation->getQuantity() <= 0) { + continue; + } + $items[$reservation->getStockId() . '|' . $reservation->getSku()] = [ + 'stock_id' => $reservation->getStockId(), + 'sku' => $reservation->getSku(), + ]; + } + + return array_values($items); + } + + /** + * Load the pre-existing balance for every order group referenced by a positive row. + * + * @param ReservationInterface[] $reservations + * @return array>> groupKey => [sku][source|''] => balance + */ + private function loadBalances(array $reservations): array + { + $skusByGroup = []; + $groupContext = []; + foreach ($reservations as $reservation) { + if ($reservation->getQuantity() <= 0) { + continue; + } + $incrementId = $this->orderIncrementId($reservation); + if ($incrementId === null) { + continue; + } + $groupKey = $incrementId . '|' . $reservation->getStockId(); + $skusByGroup[$groupKey][$reservation->getSku()] = true; + $groupContext[$groupKey] = [$incrementId, $reservation->getStockId()]; + } + + $balances = []; + foreach ($skusByGroup as $groupKey => $skuSet) { + [$incrementId, $stockId] = $groupContext[$groupKey]; + $balances[$groupKey] = $this->getOrderReservationBalance->execute( + $incrementId, + array_keys($skuSet), + $stockId + ); + } + + return $balances; + } + + /** + * Return the reservation as-is, clamped, or null when it must be dropped. + * + * @param ReservationInterface $reservation + * @param array $balances + * @param array $consumed + * @return ReservationInterface|null + */ + private function clamp( + ReservationInterface $reservation, + array $balances, + array &$consumed + ): ?ReservationInterface { + $requested = $reservation->getQuantity(); + if ($requested <= 0) { + return $reservation; + } + $incrementId = $this->orderIncrementId($reservation); + if ($incrementId === null) { + return $reservation; + } + + $groupKey = $incrementId . '|' . $reservation->getStockId(); + $sku = $reservation->getSku(); + $sourceKey = (string)$reservation->getSourceCode(); + + $existing = $balances[$groupKey][$sku][$sourceKey] ?? 0.0; + $allowed = max(0.0, -$existing) - ($consumed[$groupKey][$sku][$sourceKey] ?? 0.0); + $take = min($requested, max(0.0, $allowed)); + + if ($take + self::EPSILON < $requested) { + $this->traceClamp($incrementId, $sku, $sourceKey, $requested, $take); + } + if ($take <= self::EPSILON) { + return null; + } + $consumed[$groupKey][$sku][$sourceKey] = ($consumed[$groupKey][$sku][$sourceKey] ?? 0.0) + $take; + + if (abs($take - $requested) <= self::EPSILON) { + return $reservation; + } + + return $this->reservationBuilder + ->setSku($sku) + ->setQuantity($take) + ->setStockId($reservation->getStockId()) + ->setMetadata($reservation->getMetadata()) + ->setSourceCode($reservation->getSourceCode()) + ->setObjectIncrementId($reservation->getObjectIncrementId()) + ->build(); + } + + /** + * Resolve the order increment id from the column or the metadata, or null when absent. + * + * @param ReservationInterface $reservation + * @return string|null + */ + private function orderIncrementId(ReservationInterface $reservation): ?string + { + $incrementId = (string)($reservation->getObjectIncrementId() ?? ''); + if ($incrementId !== '') { + return $incrementId; + } + + $metadata = $reservation->getMetadata(); + if ($metadata === null || $metadata === '') { + return null; + } + try { + $data = $this->serializer->unserialize($metadata); + } catch (\Throwable $e) { + return null; + } + if (!is_array($data)) { + return null; + } + $incrementId = (string)($data['object_increment_id'] ?? ''); + + return $incrementId === '' ? null : $incrementId; + } + + /** + * Log a clamped compensation. + * + * @param string $incrementId + * @param string $sku + * @param string $sourceKey + * @param float $requested + * @param float $granted + * @return void + */ + private function traceClamp( + string $incrementId, + string $sku, + string $sourceKey, + float $requested, + float $granted + ): void { + $this->logger->warning( + 'Source-level reservations: clamped compensation exceeding the outstanding balance.', + [ + 'object_increment_id' => $incrementId, + 'sku' => $sku, + 'source_code' => $sourceKey === '' ? null : $sourceKey, + 'requested' => $requested, + 'granted' => $granted, + ] + ); + } +} diff --git a/InventorySales/Plugin/InventoryReservationsApi/RejectOversellingReservationsPlugin.php b/InventorySales/Plugin/InventoryReservationsApi/RejectOversellingReservationsPlugin.php new file mode 100644 index 000000000000..e4d914331a25 --- /dev/null +++ b/InventorySales/Plugin/InventoryReservationsApi/RejectOversellingReservationsPlugin.php @@ -0,0 +1,90 @@ +getQuantity(); + if ($quantity >= 0) { + continue; + } + $stockId = $reservation->getStockId(); + $sku = $reservation->getSku(); + $demandByStock[$stockId][$sku] = ($demandByStock[$stockId][$sku] ?? 0.0) - $quantity; + } + + foreach ($demandByStock as $stockId => $demandBySku) { + $this->assertSalable((int)$stockId, $demandBySku); + } + + return $proceed($reservations); + } + + /** + * Throw when any SKU is not salable for its requested demand on the stock. + * + * @param int $stockId + * @param array $demandBySku + * @return void + * @throws CouldNotSaveException + */ + private function assertSalable(int $stockId, array $demandBySku): void + { + $requests = []; + foreach ($demandBySku as $sku => $qty) { + $requests[] = $this->requestFactory->create(['sku' => (string)$sku, 'qty' => $qty]); + } + + foreach ($this->areProductsSalableForRequestedQty->execute($requests, $stockId) as $result) { + if (!$result->isSalable()) { + throw new CouldNotSaveException( + __( + 'Not enough salable quantity to reserve "%sku" in stock %stock.', + ['sku' => $result->getSku(), 'stock' => $stockId] + ) + ); + } + } + } +} diff --git a/InventorySales/Test/Unit/Cron/ReconcileReservationsTest.php b/InventorySales/Test/Unit/Cron/ReconcileReservationsTest.php new file mode 100644 index 000000000000..e68b3e9f6d56 --- /dev/null +++ b/InventorySales/Test/Unit/Cron/ReconcileReservationsTest.php @@ -0,0 +1,61 @@ +config = $this->createMock(ReconciliationConfig::class); + $this->sweep = $this->createMock(ReconcileReservationsSweep::class); + $this->cron = new ReconcileReservations( + $this->config, + $this->sweep, + $this->createMock(LoggerInterface::class) + ); + } + + public function testSkipsWhenSweepDisabled(): void + { + $this->config->method('isSweepEnabled')->willReturn(false); + $this->sweep->expects(self::never())->method('execute'); + + $this->cron->execute(); + } + + public function testRunsSweepWhenEnabled(): void + { + $this->config->method('isSweepEnabled')->willReturn(true); + $this->sweep->expects(self::once())->method('execute') + ->willReturn(['orders' => 0, 'compensations' => 0, 'stock_ids' => [], 'limit_reached' => false]); + + $this->cron->execute(); + } +} diff --git a/InventorySales/Test/Unit/Model/ResourceModel/SourceReservation/ReservationClampLockTest.php b/InventorySales/Test/Unit/Model/ResourceModel/SourceReservation/ReservationClampLockTest.php new file mode 100644 index 000000000000..080c1ab8ab50 --- /dev/null +++ b/InventorySales/Test/Unit/Model/ResourceModel/SourceReservation/ReservationClampLockTest.php @@ -0,0 +1,79 @@ +connection = $this->createMock(AdapterInterface::class); + $resourceConnection = $this->createMock(ResourceConnection::class); + $resourceConnection->method('getConnection')->willReturn($this->connection); + $this->lock = new ReservationClampLock($resourceConnection); + } + + public function testAcquiresOneLockPerDistinctStockSku(): void + { + $calls = []; + $this->connection->method('fetchOne')->willReturnCallback( + function (string $sql, array $bind) use (&$calls) { + $calls[] = $bind[0]; + return '1'; + } + ); + + $names = $this->lock->acquire([ + ['stock_id' => 2, 'sku' => 'A'], + ['stock_id' => 2, 'sku' => 'A'], + ['stock_id' => 2, 'sku' => 'B'], + ]); + + self::assertCount(2, $names); + self::assertSame($names, array_values($names)); + self::assertSame(array_values(array_unique($names)), $names); + self::assertCount(2, $calls); + } + + public function testReleaseReleasesEachName(): void + { + $released = 0; + $this->connection->method('fetchOne')->willReturnCallback( + function () use (&$released) { + $released++; + return '1'; + } + ); + + $this->lock->release(['inv_rsv_clamp_2_x', 'inv_rsv_clamp_2_y']); + + self::assertSame(2, $released); + } + + public function testAcquireEmptyReturnsEmpty(): void + { + $this->connection->expects(self::never())->method('fetchOne'); + + self::assertSame([], $this->lock->acquire([])); + } +} diff --git a/InventorySales/Test/Unit/Model/SourceReservation/ReconcileOrderReservationsTest.php b/InventorySales/Test/Unit/Model/SourceReservation/ReconcileOrderReservationsTest.php new file mode 100644 index 000000000000..82f7febb34d3 --- /dev/null +++ b/InventorySales/Test/Unit/Model/SourceReservation/ReconcileOrderReservationsTest.php @@ -0,0 +1,128 @@ +getOrderReservationLedger = $this->createMock(GetOrderReservationLedger::class); + $this->appendReservations = $this->createMock(AppendReservationsInterface::class); + + $reservationBuilder = $this->createMock(ReservationBuilderInterface::class); + $reservationBuilder->method('setSku')->willReturnSelf(); + $reservationBuilder->method('setQuantity')->willReturnSelf(); + $reservationBuilder->method('setStockId')->willReturnSelf(); + $reservationBuilder->method('setMetadata')->willReturnSelf(); + $reservationBuilder->method('setSourceCode')->willReturnSelf(); + $reservationBuilder->method('setObjectIncrementId')->willReturnSelf(); + $reservationBuilder->method('build')->willReturnCallback( + fn () => $this->createMock(ReservationInterface::class) + ); + + $serializer = $this->createMock(SerializerInterface::class); + $serializer->method('serialize')->willReturnCallback(static fn ($value) => json_encode($value)); + + $this->model = new ReconcileOrderReservations( + $this->getOrderReservationLedger, + $this->appendReservations, + $reservationBuilder, + $serializer + ); + } + + public function testSkipsNonTerminalOrder(): void + { + $this->getOrderReservationLedger->expects(self::never())->method('execute'); + $this->appendReservations->expects(self::never())->method('execute'); + + $result = $this->model->execute(self::OBJECT_ID, self::INCREMENT_ID, 'processing'); + + self::assertSame([], $result); + } + + public function testSkipsWhenNoNegativeBalance(): void + { + $this->getOrderReservationLedger->method('execute')->willReturn([]); + $this->appendReservations->expects(self::never())->method('execute'); + + $result = $this->model->execute(self::OBJECT_ID, self::INCREMENT_ID, Order::STATE_COMPLETE); + + self::assertSame([], $result); + } + + public function testReleasesNegativeBalancePerSource(): void + { + $this->getOrderReservationLedger->method('execute')->willReturn([ + ['stock_id' => 2, 'sku' => 'SLR-1', 'source_code' => 'slr_a', 'balance' => -3.0], + ['stock_id' => 2, 'sku' => 'SLR-1', 'source_code' => 'slr_b', 'balance' => -2.0], + ]); + $this->appendReservations->expects(self::once())->method('execute') + ->with(self::countOf(2)); + + $result = $this->model->execute(self::OBJECT_ID, self::INCREMENT_ID, Order::STATE_CANCELED); + + self::assertSame( + [ + ['stock_id' => 2, 'sku' => 'SLR-1', 'source_code' => 'slr_a', 'quantity' => 3.0], + ['stock_id' => 2, 'sku' => 'SLR-1', 'source_code' => 'slr_b', 'quantity' => 2.0], + ], + $result + ); + } + + public function testDryRunPlansWithoutAppending(): void + { + $this->getOrderReservationLedger->method('execute')->willReturn([ + ['stock_id' => 2, 'sku' => 'SLR-1', 'source_code' => null, 'balance' => -4.0], + ]); + $this->appendReservations->expects(self::never())->method('execute'); + + $result = $this->model->execute(self::OBJECT_ID, self::INCREMENT_ID, Order::STATE_CLOSED, true); + + self::assertSame( + [['stock_id' => 2, 'sku' => 'SLR-1', 'source_code' => null, 'quantity' => 4.0]], + $result + ); + } + + public function testSkipsEmptyIncrementId(): void + { + $this->getOrderReservationLedger->expects(self::never())->method('execute'); + + self::assertSame([], $this->model->execute(self::OBJECT_ID, '', Order::STATE_COMPLETE)); + } +} diff --git a/InventorySales/Test/Unit/Model/SourceReservation/ReconcileReservationsSweepTest.php b/InventorySales/Test/Unit/Model/SourceReservation/ReconcileReservationsSweepTest.php new file mode 100644 index 000000000000..730cc6c37bcb --- /dev/null +++ b/InventorySales/Test/Unit/Model/SourceReservation/ReconcileReservationsSweepTest.php @@ -0,0 +1,98 @@ +getTerminalOrders = $this->createMock(GetTerminalOrdersWithResidualReservations::class); + $this->engine = $this->createMock(ReconcileOrderReservations::class); + $this->stockIndexer = $this->createMock(StockIndexer::class); + $this->sweep = new ReconcileReservationsSweep( + $this->getTerminalOrders, + $this->engine, + $this->stockIndexer + ); + } + + public function testReconcilesEachOrderAndReindexesAffectedStocks(): void + { + $this->getTerminalOrders->method('execute')->willReturn([ + ['object_id' => 1, 'increment_id' => '000000001', 'state' => Order::STATE_COMPLETE], + ['object_id' => 2, 'increment_id' => '000000002', 'state' => Order::STATE_CANCELED], + ]); + $this->engine->method('execute')->willReturnOnConsecutiveCalls( + [['stock_id' => 2, 'sku' => 'A', 'source_code' => 'slr_a', 'quantity' => 3.0]], + [['stock_id' => 3, 'sku' => 'A', 'source_code' => 'slr_a', 'quantity' => 1.0]] + ); + $this->stockIndexer->expects(self::once())->method('executeList')->with([2, 3]); + + $result = $this->sweep->execute(500); + + self::assertSame(2, $result['orders']); + self::assertSame(2, $result['compensations']); + self::assertSame([2, 3], $result['stock_ids']); + self::assertFalse($result['limit_reached']); + } + + public function testDryRunDoesNotReindex(): void + { + $this->getTerminalOrders->method('execute')->willReturn([ + ['object_id' => 1, 'increment_id' => '000000001', 'state' => Order::STATE_COMPLETE], + ]); + $this->engine->expects(self::once())->method('execute') + ->with(1, '000000001', Order::STATE_COMPLETE, true) + ->willReturn([['stock_id' => 2, 'sku' => 'A', 'source_code' => null, 'quantity' => 4.0]]); + $this->stockIndexer->expects(self::never())->method('executeList'); + + $result = $this->sweep->execute(500, true); + + self::assertSame(1, $result['compensations']); + } + + public function testLimitReachedWhenBatchIsFull(): void + { + $this->getTerminalOrders->method('execute')->willReturn([ + ['object_id' => 1, 'increment_id' => '000000001', 'state' => Order::STATE_COMPLETE], + ]); + $this->engine->method('execute')->willReturn([]); + + $result = $this->sweep->execute(1); + + self::assertTrue($result['limit_reached']); + } +} diff --git a/InventorySales/Test/Unit/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserverTest.php b/InventorySales/Test/Unit/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserverTest.php new file mode 100644 index 000000000000..c7ebad6b35a8 --- /dev/null +++ b/InventorySales/Test/Unit/Observer/CatalogInventory/ReconcileReservationsOnOrderStateChangeObserverTest.php @@ -0,0 +1,94 @@ +config = $this->createMock(ReconciliationConfig::class); + $this->engine = $this->createMock(ReconcileOrderReservations::class); + $this->observer = new ReconcileReservationsOnOrderStateChangeObserver($this->config, $this->engine); + } + + public function testSkipsWhenDisabled(): void + { + $this->config->method('isCancelRefundReconciliationEnabled')->willReturn(false); + $this->engine->expects(self::never())->method('execute'); + + $this->observer->execute($this->observerFor($this->order('complete', 'processing'))); + } + + public function testSkipsWhenNoTransition(): void + { + $this->config->method('isCancelRefundReconciliationEnabled')->willReturn(true); + $this->engine->expects(self::never())->method('execute'); + + $this->observer->execute($this->observerFor($this->order('complete', 'complete'))); + } + + public function testSkipsWhenTransitionIsNotTerminal(): void + { + $this->config->method('isCancelRefundReconciliationEnabled')->willReturn(true); + $this->engine->expects(self::never())->method('execute'); + + $this->observer->execute($this->observerFor($this->order('processing', 'pending'))); + } + + public function testReconcilesOnTerminalTransition(): void + { + $this->config->method('isCancelRefundReconciliationEnabled')->willReturn(true); + $this->engine->expects(self::once())->method('execute')->with(7, '000000007', Order::STATE_CANCELED); + + $this->observer->execute($this->observerFor($this->order(Order::STATE_CANCELED, 'processing'))); + } + + private function order(string $state, ?string $origState): Order + { + $order = $this->createMock(Order::class); + $order->method('getState')->willReturn($state); + $order->method('getOrigData')->with('state')->willReturn($origState); + $order->method('getIncrementId')->willReturn('000000007'); + $order->method('getId')->willReturn(7); + + return $order; + } + + private function observerFor(Order $order): Observer + { + $event = new Event(['order' => $order]); + $observer = new Observer(); + $observer->setEvent($event); + + return $observer; + } +} diff --git a/InventorySales/Test/Unit/Plugin/InventoryReservationsApi/ClampCompensationReservationsPluginTest.php b/InventorySales/Test/Unit/Plugin/InventoryReservationsApi/ClampCompensationReservationsPluginTest.php new file mode 100644 index 000000000000..f9c4cfcd06be --- /dev/null +++ b/InventorySales/Test/Unit/Plugin/InventoryReservationsApi/ClampCompensationReservationsPluginTest.php @@ -0,0 +1,200 @@ +getOrderReservationBalance = $this->createMock(GetOrderReservationBalance::class); + $this->reservationBuilder = $this->createMock(ReservationBuilderInterface::class); + $this->reservationBuilder->method('setSku')->willReturnSelf(); + $this->reservationBuilder->method('setQuantity')->willReturnSelf(); + $this->reservationBuilder->method('setStockId')->willReturnSelf(); + $this->reservationBuilder->method('setMetadata')->willReturnSelf(); + $this->reservationBuilder->method('setSourceCode')->willReturnSelf(); + $this->reservationBuilder->method('setObjectIncrementId')->willReturnSelf(); + $this->reservationBuilder->method('build') + ->willReturn($this->createMock(ReservationInterface::class)); + + $serializer = $this->createMock(SerializerInterface::class); + $serializer->method('unserialize') + ->willReturnCallback(static fn (string $value) => json_decode($value, true) ?? []); + + $this->subject = $this->createMock(AppendReservationsInterface::class); + $this->appended = null; + $this->proceedCalled = false; + + $clampLock = $this->createMock(ReservationClampLock::class); + $clampLock->method('acquire')->willReturn([]); + + $this->plugin = new ClampCompensationReservationsPlugin( + $this->getOrderReservationBalance, + $this->reservationBuilder, + $serializer, + $this->createMock(LoggerInterface::class), + $clampLock + ); + } + + public function testPassesThroughDemandReservations(): void + { + $this->getOrderReservationBalance->expects(self::never())->method('execute'); + $this->reservationBuilder->expects(self::never())->method('build'); + + $reservations = [$this->reservation(-5.0, 'sku-1', 'source-a')]; + $this->invokePlugin($reservations); + + self::assertSame($reservations, $this->appended); + } + + public function testAllowsReleaseWithinOutstandingBalance(): void + { + $this->givenBalance(['sku-1' => ['source-a' => -5.0]]); + $this->reservationBuilder->expects(self::never())->method('build'); + + $reservations = [$this->reservation(5.0, 'sku-1', 'source-a')]; + $this->invokePlugin($reservations); + + self::assertSame($reservations, $this->appended); + } + + public function testClampsReleaseExceedingOutstandingBalance(): void + { + $this->givenBalance(['sku-1' => ['source-a' => -5.0]]); + $this->reservationBuilder->expects(self::once())->method('setQuantity')->with(5.0)->willReturnSelf(); + + $this->invokePlugin([$this->reservation(8.0, 'sku-1', 'source-a')]); + + self::assertCount(1, $this->appended); + } + + public function testDropsReleaseWithNoOutstandingBalance(): void + { + $this->givenBalance(['sku-1' => ['source-a' => 0.0]]); + $this->reservationBuilder->expects(self::never())->method('build'); + + $this->invokePlugin([$this->reservation(5.0, 'sku-1', 'source-a')]); + + self::assertFalse($this->proceedCalled); + } + + public function testMultipleReleasesShareTheOutstandingBalance(): void + { + $this->givenBalance(['sku-1' => ['source-a' => -5.0]]); + $this->reservationBuilder->expects(self::once())->method('setQuantity')->with(2.0)->willReturnSelf(); + + $this->invokePlugin([ + $this->reservation(3.0, 'sku-1', 'source-a'), + $this->reservation(3.0, 'sku-1', 'source-a'), + ]); + + self::assertCount(2, $this->appended); + } + + public function testPassesThroughWhenMetadataHasNoOrderContext(): void + { + $this->getOrderReservationBalance->expects(self::never())->method('execute'); + + $reservation = $this->createMock(ReservationInterface::class); + $reservation->method('getQuantity')->willReturn(5.0); + $reservation->method('getMetadata')->willReturn(''); + $reservations = [$reservation]; + $this->invokePlugin($reservations); + + self::assertSame($reservations, $this->appended); + } + + public function testClampsAgainstStockScopedBalanceForNullSource(): void + { + $this->givenBalance(['sku-1' => ['' => -4.0]]); + $this->reservationBuilder->expects(self::once())->method('setQuantity')->with(4.0)->willReturnSelf(); + + $this->invokePlugin([$this->reservation(6.0, 'sku-1', null)]); + + self::assertCount(1, $this->appended); + } + + /** + * @param array> $balance + */ + private function givenBalance(array $balance): void + { + $this->getOrderReservationBalance->method('execute')->willReturn($balance); + } + + /** + * @param ReservationInterface[] $reservations + */ + private function invokePlugin(array $reservations): void + { + $proceed = function (array $appended) { + $this->proceedCalled = true; + $this->appended = $appended; + }; + $this->plugin->aroundExecute($this->subject, $proceed, $reservations); + } + + private function reservation(float $qty, string $sku, ?string $source): ReservationInterface + { + $reservation = $this->createMock(ReservationInterface::class); + $reservation->method('getQuantity')->willReturn($qty); + $reservation->method('getSku')->willReturn($sku); + $reservation->method('getStockId')->willReturn(self::STOCK_ID); + $reservation->method('getSourceCode')->willReturn($source); + $reservation->method('getObjectIncrementId')->willReturn('000000123'); + $reservation->method('getMetadata')->willReturn( + json_encode(['object_type' => 'order', 'object_id' => '123', 'object_increment_id' => '000000123']) + ); + + return $reservation; + } +} diff --git a/InventorySales/Test/Unit/Plugin/InventoryReservationsApi/RejectOversellingReservationsPluginTest.php b/InventorySales/Test/Unit/Plugin/InventoryReservationsApi/RejectOversellingReservationsPluginTest.php new file mode 100644 index 000000000000..357c07633bb6 --- /dev/null +++ b/InventorySales/Test/Unit/Plugin/InventoryReservationsApi/RejectOversellingReservationsPluginTest.php @@ -0,0 +1,181 @@ + + */ + private $requestedQties; + + protected function setUp(): void + { + $this->areProductsSalableForRequestedQty = $this->createMock( + AreProductsSalableForRequestedQtyInterface::class + ); + $this->requestFactory = $this->createMock( + IsProductSalableForRequestedQtyRequestInterfaceFactory::class + ); + $this->requestedQties = []; + $this->requestFactory->method('create')->willReturnCallback( + function (array $data): IsProductSalableForRequestedQtyRequestInterface { + $this->requestedQties[] = $data['qty']; + $request = $this->createMock(IsProductSalableForRequestedQtyRequestInterface::class); + $request->method('getSku')->willReturn((string)$data['sku']); + $request->method('getQty')->willReturn((float)$data['qty']); + + return $request; + } + ); + + $this->subject = $this->createMock(AppendReservationsInterface::class); + $this->proceedCalled = false; + + $this->plugin = new RejectOversellingReservationsPlugin( + $this->areProductsSalableForRequestedQty, + $this->requestFactory + ); + } + + public function testAllowsSalableDemand(): void + { + $this->givenSalability(['sku-1' => true]); + + $this->invokePlugin([$this->reservation(-5.0, 'sku-1')]); + + self::assertTrue($this->proceedCalled); + } + + public function testRejectsNonSalableDemand(): void + { + $this->givenSalability(['sku-1' => false]); + + $this->expectException(CouldNotSaveException::class); + try { + $this->invokePlugin([$this->reservation(-5.0, 'sku-1')]); + } finally { + self::assertFalse($this->proceedCalled); + } + } + + public function testIgnoresCompensationReservations(): void + { + $this->areProductsSalableForRequestedQty->expects(self::never())->method('execute'); + + $this->invokePlugin([$this->reservation(5.0, 'sku-1')]); + + self::assertTrue($this->proceedCalled); + } + + public function testAggregatesDemandPerSku(): void + { + $this->givenSalability(['sku-1' => true]); + + $this->invokePlugin([ + $this->reservation(-3.0, 'sku-1'), + $this->reservation(-2.0, 'sku-1'), + ]); + + self::assertSame([5.0], $this->requestedQties); + } + + public function testChecksEachStockSeparately(): void + { + $this->areProductsSalableForRequestedQty->expects(self::exactly(2)) + ->method('execute') + ->willReturn([$this->salableResult('sku-1', true)]); + + $this->invokePlugin([ + $this->reservation(-1.0, 'sku-1', 10), + $this->reservation(-1.0, 'sku-1', 20), + ]); + + self::assertTrue($this->proceedCalled); + } + + /** + * @param array $salableBySku + */ + private function givenSalability(array $salableBySku): void + { + $results = []; + foreach ($salableBySku as $sku => $isSalable) { + $results[] = $this->salableResult((string)$sku, $isSalable); + } + $this->areProductsSalableForRequestedQty->method('execute')->willReturn($results); + } + + private function salableResult(string $sku, bool $isSalable): IsProductSalableForRequestedQtyResultInterface + { + $result = $this->createMock(IsProductSalableForRequestedQtyResultInterface::class); + $result->method('getSku')->willReturn($sku); + $result->method('isSalable')->willReturn($isSalable); + + return $result; + } + + private function reservation(float $qty, string $sku, int $stockId = self::STOCK_ID): ReservationInterface + { + $reservation = $this->createMock(ReservationInterface::class); + $reservation->method('getQuantity')->willReturn($qty); + $reservation->method('getSku')->willReturn($sku); + $reservation->method('getStockId')->willReturn($stockId); + + return $reservation; + } + + /** + * @param ReservationInterface[] $reservations + */ + private function invokePlugin(array $reservations): void + { + $proceed = function () { + $this->proceedCalled = true; + }; + $this->plugin->aroundExecute($this->subject, $proceed, $reservations); + } +} diff --git a/InventorySales/etc/crontab.xml b/InventorySales/etc/crontab.xml new file mode 100644 index 000000000000..8fcdebce3b46 --- /dev/null +++ b/InventorySales/etc/crontab.xml @@ -0,0 +1,14 @@ + + + + + + cataloginventory/source_reservations/reconcile_sweep_cron + + + diff --git a/InventorySales/etc/di.xml b/InventorySales/etc/di.xml index ef11c45a756b..a92d118dbf5f 100644 --- a/InventorySales/etc/di.xml +++ b/InventorySales/etc/di.xml @@ -30,6 +30,8 @@ + + @@ -206,4 +208,11 @@ + + + + Magento\InventorySales\Console\Command\ReconcileReservationsCommand + + + diff --git a/InventorySales/etc/events.xml b/InventorySales/etc/events.xml index 568e90310f81..5a09793c3d08 100644 --- a/InventorySales/etc/events.xml +++ b/InventorySales/etc/events.xml @@ -19,4 +19,7 @@ + + +