[Ibochkarev]

Описание

Поле stock было только для витрины и импорта. Корзина и оформление его не читали, списания не было, два покупателя могли взять последний экземпляр.

В Core появился контракт InventoryServiceInterface (DI ms3_inventory) и реализация на ms3_products.stock плюс журнал ms3_inventory_reservations. Учёт включается настройкой ms3_inventory_enabled (по умолчанию выключена). Тогда:

• до выдачи номера заказа проверяется доступность
• статус New (ms3_status_new) резервирует остаток атомарным UPDATE … WHERE stock >= qty в одной транзакции со сменой status_id
• Paid (ms3_status_paid) фиксирует резерв
• Canceled (ms3_status_canceled) до commit возвращает остаток
• неуспешный payment->send() (в том числе false вместо массива) переводит заказ в отмену, чтобы не оставлять New без холда
• ошибка после выдачи номера, но до New, снимает номер с черновика (при включённом учёте)
• ошибка msOnChangeOrderStatus после перехода в New отпускает резерв и откатывает status_id

Внешний склад подменяется через ms3_inventory. События msOnBeforeInventory* / msOnInventory* (Reserve, Commit, Release). release не восстанавливает stock, если резерв уже committed (обход OrderStatusService у кастомных провайдеров). Мультисклад, партии и ERP-sync в этот PR не входят.

Документация вызовов: setting_ms3_inventory_enabled_desc (ru/en) и phpdoc InventoryServiceInterface.

Тип изменений

• Новая функциональность (non-breaking change)

Связанные Issues

Closes #589

Как это было протестировано?

GitHub Actions на HEAD 0edad252 (run 32212167139):

cd core/components/minishop3
composer ci:php
# php -l 644 files, exit 0
# smoke 90, exit 0
# PHPUnit 274 tests, 706 assertions, exit 0 (CI с MySQL, @group mysql включён)

composer stan
# [OK] No errors, exit 0

cd ../../../vueManager
npm run lint:ci
# pass (файлы Vue этим PR не менялись)

• Автоматические тесты (composer ci:php, composer stan, npm run lint:ci в CI)
• Ручное тестирование checkout с ms3_inventory_enabled=1
• Тестирование на разных версиях PHP/MODX — CI PHP 8.2.33, локально PHP 8.4.17

Конфигурация тестирования:

• MiniShop3: ветка feat/issue-589-inventory-abstraction от beta (коммиты ec5b1bbd, 0edad252)
• MODX: не поднимался
• PHP: 8.2.33 (CI), 8.4.17 (локально)

Скриншоты (если применимо)

N/A, backend.

Чеклист

• Код соответствует стилю проекта
• Добавлены/обновлены комментарии в сложных местах
• Изменения не ломают существующую функциональность (default off)
• Лексиконы добавлены на двух языках (ru/en)
• PHPStan проходит без новых ошибок (composer stan)
• ESLint (npm run lint:ci) — CI pass, Vue не тронут
• CHANGELOG.md — не трогал, запись на релиз

Дополнительные заметки

Второй коммит закрыл разрывы ревью: send() === false больше не оставляет New с холдом, after-event на New отпускает сток, номер черновика снимается при фейле до New, ledger reserve/commit/release пишется в одной TX со сменой статуса, mysql-тест гонки идёт на двух соединениях, InMemoryInventoryStockStore лежит в tests/support/.

Номер заказа и reserve по-прежнему в разных транзакциях. Assert до номера, reserve в TX со сменой статуса на New. Второй checkout последнего qty может получить номер и упасть на reserve. Номер тогда снимается, дырявого hold нет.

[AgelxNash]

Этот PR включён в тестовую интеграционную сборку всех открытых PR MiniShop3: AgelxNash/MiniShop3, ветка integration/open-prs-20260906 (28/28 открытых).

Сборка нужна, чтобы проверить совместимость взаимозависимых серий PR до их мержа — при последовательном слиянии они конфликтуют друг с другом. Это не ревью и не конкурирующий PR: авторство сохранено (1 PR = 1 коммит с исходным автором), ветка пересобирается по мере обновления PR.

Как вошёл в сборку: Конфликт со слитым #596 (lifecycle gate) разрешён: фабрика ms3_order_status теперь принимает и lifecycle ports, и OrderInventoryCoordinator; карта зависимостей объединена (ms3_order_log, ms3_order_lifecycle_ports, ms3_inventory); persistStatusWithInventory() заменил inline-персист статуса; на провал msOnChangeOrderStatus вызываются undoUncommittedNewStatus() и rollbackStatus().

[AgelxNash]

Желаю скорейшего ревью и мержа! Красивый PR 👏

[biz87]

Проверил. С текущей beta сливается чисто, smoke, PHPUnit и PHPStan зелёные. При ms3_inventory_enabled=false поведение действительно не меняется во всех точках встраивания, включая отмену заказа при неуспешном send(). Атомарный UPDATE ... WHERE stock >= :qty_check правильно не даёт продать последний экземпляр двум разным заказам.

Архитектурный вопрос общий с #596 — описал там: #596 (comment). Коротко: порты из #596 складу не подходят, а откаты двух PR при объединении дают рассинхрон статуса и остатка. Нужна одна модель смены статуса с транзакцией, в которой резерв может запретить переход. Ниже — то, что относится только к складу.

1. Гонка внутри одного заказа

ProductStockInventory::reserve() и release() сначала читают журнал через findReservation() — обычный SELECT без блокировки, — затем меняют stock, затем пишут журнал через INSERT ... ON DUPLICATE KEY UPDATE.

Два одновременных вызова для одного заказа (двойной клик «Отменить», повтор запроса) оба увидят reserved, и оба вернут остаток. Два одновременных резерва оба спишут. Уникальный индекс (order_id, product_id) защищает только от второй строки журнала. InventoryStockConcurrentMysqlTest проверяет разные заказы, этот случай не покрыт.

Переход состояния журнала нужно сделать условным — UPDATE ... SET state = ... WHERE order_id = ? AND product_id = ? AND state = <ожидаемое> или вставка с проверкой уникальности — и менять stock только если переход выполнил именно этот запрос. Нужен MySQL-тест на два соединения для одного заказа.

2. Незаполненный остаток

PdoInventoryStockStore::getAvailable() возвращает 0 для NULL, признака «остаток не учитывается» нет. Во многих магазинах stock до сих пор был витринным полем и не заполнялся — после включения настройки все такие товары станут недоступны к заказу. Об этом нужно написать в setting_ms3_inventory_enabled_desc (ru и en): перед включением заполнить остатки.

Там же стоит указать, что остаток общий на товар: OrderInventoryCoordinator::qtyByProduct() суммирует строки заказа по product_id, и варианты с разными опциями списываются с одного stock.

Порядок вливания

После вливания любого PR линии остальные конфликтуют. Предлагаю порядок #605 → #604 → #596 → #603, то есть этот PR — последним, после ребейза на beta с уже влитым #596.

[Ibochkarev]

Rebased onto current beta (shipment lifecycle already merged). Conflicts resolved in events/settings/lexicon and tests/bootstrap.php (kept both InMemoryShipmentStore and InMemoryInventoryStockStore). Inventory unit tests green locally.

[biz87]

@Ibochkarev после ревью 14.09 ветки #596 и #603 только перебазированы на beta — сверил текущие головы (f4c35553, 32b2abb5), обе сливаются с beta без конфликтов, но между собой конфликтуют в OrderStatusService.php, ServiceRegistry*.php, настройках и лексиконах.

Открытыми остаются:

Модель смены статуса (#596 и #603 вместе) — #596 (comment). В #596 по-прежнему порты после save() и компенсирующий rollbackStatus(), в #603 — своя транзакция и undoUncommittedNewStatus(). При объединении ошибка плагина в msOnChangeOrderStatus откатит статус, а остаток останется изменённым. Нужно решение, как будет устроен OrderStatusService::change(), — от него зависит, как переделывать оба PR.

#603, гонка внутри одного заказа. ProductStockInventory::reserve() и release() читают журнал через findReservation() без блокировки и только потом меняют stock. В release() транзакция охватывает increment() и запись журнала, но проверка состояния остаётся снаружи — два одновременных release() для одного заказа оба увидят reserved и оба вернут остаток. Нужен условный переход состояния журнала (UPDATE … WHERE state = 'reserved' и изменение stock только если запрос затронул строку) и MySQL-тест на два соединения для одного заказа.

#603, незаполненный остаток. setting_ms3_inventory_enabled_desc (ru/en) всё ещё не предупреждает, что NULL в stock считается нулём — после включения товары без заполненного остатка станут недоступны, — и что остаток общий на товар для всех вариантов опций.

Прежде чем смотреть дальше, нужен ответ по модели смены статуса.

[Ibochkarev]

Ветка на текущей beta, #596 уже влит. OrderStatusService::change() сохраняет статус в persistStatusWithInventory: одна транзакция, сначала applyStatusChange, потом lifecycle ports, потом status_id. Открытую транзакцию не начинаем заново.

Если msOnChangeOrderStatus падает, оплаченный, отменённый и отправленный статусы остаются как есть. Для «Новый» при включённом учёте undoUncommittedNewStatus() снимает резерв и пишет прежний status_id. Остаток и статус в этом случае меняются вместе.

reserve(), release() и commit() меняют stock только когда этот вызов выиграл строку журнала. Новый резерв пишется INSERT. Смена состояния: UPDATE ... WHERE state = 'reserved' (повторный резерв после release идёт из released). Если строка не обновилась, stock не трогаем. Юнит-тест проверяет, что проигранный release не возвращает остаток. InventoryStockConcurrentMysqlTest гоняет два соединения на один order_id: второе не забирает уже удерживаемый release, остаток восстанавливается один раз. Локально этот тест не запускал, нет MS3_TEST_MYSQL_DSN.

В setting_ms3_inventory_enabled_desc (ru и en) добавлено, что NULL в stock считается нулём и перед включением остатки нужно заполнить. Остаток один на товар: строки с одним product_id, включая варианты с разными опциями, списываются с него.

Коммит 38e162e3. Локально tests/Unit/Services/Inventory и OrderStatusServiceLifecycleTest: 29 тестов, 72 проверки.

[biz87]

Спасибо за переделку — условный переход журнала и предупреждение в описании настройки на месте, замечания от 14.09 закрыты.

Ветка сливается с актуальной beta (f0dfe593) без конфликтов, гейт зелёный: PHPUnit 675, PHPStan версии CI без ошибок. MySQL-группу прогнал на живой MariaDB 10.6 (MS3_TEST_MYSQL_DSN на отдельную базу) — 24 теста проходят, включая конкурентные.

Но при включённой настройке на живом MODX складской учёт не работает вообще.

1. Склад ломает любую смену статуса

Проверил на dev: ms3_inventory_enabled = 1, заказ с одной позицией, остаток 10.

Переход	Результат
New	«Неизвестная ошибка», статус не изменился, журнал резерва пуст, остаток 10
Paid	то же
Canceled	то же

В журнале MODX: [OrderStatusService] persistStatusWithInventory: There is already an active transaction.

Причина в проводке. ServiceRegistryFactories создаёт хранилище с $modx, а не с $modx->pdo — рядом, для PdoShipmentStore и PdoPaymentAttemptStore, передаётся именно $modx->pdo. Дальше PdoInventoryStockStore::runInTransaction() определяет открытую транзакцию через method_exists($this->db, 'inTransaction'), а у xPDO такого метода нет — это вы сами отметили в комментарии к OrderStatusService::hasOpenTransaction(). Проверка всегда ложна, хранилище открывает вложенную транзакцию внутри той, что уже открыл persistStatusWithInventory(), и PDO::beginTransaction() бросает исключение. Оно вылетает до tryDecrement() и записи в журнал, поэтому остаток не меняется, а наружу уходит ms3_err_unknown.

Проверил обе ветки напрямую на dev:

Хранилище получило	Вызов внутри открытой транзакции
modX (как в фабрике)	PDOException: There is already an active transaction
$modx->pdo	проходит

Почему не поймали тесты: юнит-тесты используют InMemoryInventoryStockStore, где runInTransaction() — пустышка, а InventoryStockConcurrentMysqlTest создаёт хранилище поверх голого PDO, у которого inTransaction() есть. Через реальную фабрику ms3_inventory не проходит ни один тест. В чек-листе PR ручная проверка checkout с включённой настройкой отмечена как невыполненная — проблема ровно там.

Правка на строку: передавать $modx->pdo и проверять instanceof \PDO, как у соседних фабрик. Плюс тест, который идёт через ms3_order_status и ms3_inventory из контейнера с включённой настройкой, а не через изолированный PdoInventoryStockStore($pdo, …). Докблок InventoryStockStoreInterface::runInTransaction() про «Nested calls join an already open transaction» тоже стоит поправить.

2. undoUncommittedNewStatus() расходится с контрактом #596 и содержит гонку

Докблок OrderStatusService говорит «no compensating status save after commit… does not revert status_id», а метод именно это и делает для «Нового». Это мы явно согласовали в #596: компенсирующих записей после commit не будет.

Помимо принципиального расхождения:

• $msOrder — тот же объект, загруженный в начале change(), из базы он не перечитывается. Если между провалом msOnChangeOrderStatus и компенсацией параллельный процесс (например, вебхук оплаты) успел перевести заказ в «Оплачен», set('status_id', $previousId) и save() затрут этот переход. Журнал резерва от повторного списания защищён условным переходом состояния, а status_id — нет;
• результат save() здесь не проверяется, в отличие от persistStatusWithInventory(). При тихом сбое получится «Новый» без живого резерва.

Сейчас этот путь недостижим: из-за пункта 1 до события дело не доходит. Но как только транзакция будет исправлена, он оживёт, поэтому чинить лучше оба пункта сразу.

Стоит исправить

• persistStatusWithInventory(), ветка if (!$ownsTx): перед проверкой save() в объекте уже выставлен новый status_id, поэтому releaseAll() видит «Оплачен» и выходит, ничего не освободив. При переходе в «Оплачен» внутри чужой транзакции фиксация в журнале останется, а статус — нет.
• OrderInventoryCoordinator::reserveAll() при частичном отказе вручную освобождает уже зарезервированное. На уровне SQL это лишнее — внешняя транзакция и так откатится, — но release() по пути дёргает события msOnBeforeInventoryRelease и msOnInventoryRelease. Внешняя интеграция получит уведомление об освобождении резерва, которого в базе никогда не было. Либо не вызывать события до commit, либо положиться на откат транзакции.
• OrderSubmitHandler::revertAllocatedNumber() — save() без проверки результата.

Мелочь

OrderInventoryCoordinator::releaseAll() дублирует проверку состояния, которая уже есть в ProductStockInventory::release(). Не ошибка, но стоит пометить комментарием как быстрый выход, иначе читается как единственная защита.