feat(order): lifecycle gate with in-TX ports and idempotency

[Ibochkarev]

Описание

OrderStatusService — единый gate для non-draft смены статуса: опциональный allow-list переходов, DI-порты lifecycle (пока no-op, точка входа для #603), режим idempotent / ensure() для интеграций.

Контракт change() (согласован с #603)

1. Validate + msOnBeforeChangeOrderStatus (может отменить до любых записей).
2. DB-транзакция: in-TX lifecycle ports (могут запретить) → status_id → commit. Откат только rollback транзакции. Компенсирующего rollbackStatus() нет.
3. После commit: msOnChangeOrderStatus → журнал → уведомления. Ошибка плагина возвращается наружу, статус не откатывается (согласованность со складом).
4. Порты — pre-persist domain hooks, не post-save() side-effects.

Manager update не пишет status_id в общий save() до вызова сервиса.

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

• Новая функциональность (non-breaking change)
• Исправление бага (non-breaking change)
• Breaking change — поведение после ошибки msOnChangeOrderStatus: статус остаётся (как смысл beta), без compensating save из ранней редакции feat(order): lifecycle gate with in-TX ports and idempotency #596

Связанные Issues

Closes #592

Refs #603 (склад сядет на in-TX ports), #604 / #605

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

cd core/components/minishop3
php -l src/Services/Order/OrderStatusService.php
php -l src/Services/Order/OrderLifecyclePortsInterface.php
./vendor/bin/phpunit tests/Unit/Services/Order/OrderStatusServiceLifecycleTest.php \
  tests/Unit/Services/Order/OrderStatusTransitionPolicyTest.php
# exit 0 — 13 tests

• Автоматические тесты
• Ручное тестирование на живом MODX

Конфигурация: ветка feat/issue-592-order-lifecycle (rebase/merge beta + контракт PR comment)

Чеклист

• Код соответствует стилю проекта
• Лексиконы ru/en (удалён неиспользуемый ms3_err_status_rollback)
• Не ломает default final/fixed; ports no-op
• PHPStan — CI
• CHANGELOG — на релизе

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

Порядок вливания линии: #605 → #604 → #596 → #603.
См. #596 (comment)

[AgelxNash]

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

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

Как вошёл в сборку: Слился чисто.

[AgelxNash]

Привет! Просто пожелание: удачи с этим PR 🚀 Работа нужная — пусть рассмотрят и смержат как можно скорее. Успехов!

[biz87]

Проверил. Сам по себе PR корректен: с текущей beta сливается чисто, smoke, PHPUnit и PHPStan зелёные. Битый формат ms3_order_status_transitions запрещает переходы, а не пропускает их. Но вливать его отдельно от #603 не предлагаю — ниже почему.

Порты не подходят тем, для кого сделаны

Докблок OrderLifecyclePortsInterface обещает реализации в #589–#591, но ни #603, ни #604, ни #605 интерфейс не используют. И #603 не может использовать его в текущем виде:

• порты вызываются после $msOrder->save() и вне транзакции, а резерв из feat(core): add opt-in inventory reserve, commit and release #603 должен выполняться до сохранения, в одной транзакции со сменой status_id, и при нехватке остатка запрещать переход;
• портов три — оплачен, отменён, отправлен. Для «Нового», где feat(core): add opt-in inventory reserve, commit and release #603 резервирует остаток, порта нет.

Два разных отката

#596 при ошибке порта или msOnChangeOrderStatus возвращает прежний статус повторным save() (rollbackStatus()). #603 оборачивает резерв и save() в транзакцию, а при ошибке msOnChangeOrderStatus откатывает только переход в «Новый» (undoUncommittedNewStatus() с освобождением резерва).

Если объединить как есть: остаток зафиксирован или освобождён и закоммичен, затем плагин в msOnChangeOrderStatus возвращает ошибку, rollbackStatus() ставит прежний статус — а остаток остаётся изменённым.

Что предлагаю

Свести #596 и #603 к одной модели смены статуса в OrderStatusService::change():

1. Транзакция: доменные шаги до сохранения (резерв, фиксация, освобождение остатка — могут запретить переход) → status_id → commit.
2. После commit — msOnChangeOrderStatus, журнал, уведомления.
3. Откат только через транзакцию, без компенсирующего save(). Отдельно решить, может ли msOnChangeOrderStatus отменить переход. В beta не может: статус остаётся, возвращается ошибка. Если должен мочь — событие придётся вызывать до commit.
4. Один механизм «заказ уже в этом статусе — успех». Сейчас их два: $options['idempotent'] здесь и ensure() в feat(core): add payment attempt lifecycle and webhook callback #604.

Порты тогда либо убрать, либо превратить в шаг из п. 1, через который работает склад из #603.

После вливания любого PR линии остальные три конфликтуют, так что вливать придётся по одному с ребейзом. Предлагаю порядок #605 → #604 → #596 → #603: у #604 и #605 зависимость от OrderStatusService ограничивается публичным change(), и к моменту переделки #596 и #603 у модели будут реальные потребители.

Мелочи

• Для всех сайтов меняется поведение: ошибка плагина в msOnChangeOrderStatus теперь откатывает статус, а запись в журнал заказа идёт после события. Это стоит вынести в описание PR и CHANGELOG.
• testInvalidJsonIsNotDenyAll проверяет MODE_INVALID, который сервис трактует как запрет всех переходов, — название вводит в заблуждение.

[Ibochkarev]

Rebased onto current beta (shipment lifecycle merged). Conflicts resolved: kept shipment settings + lexicon and added ms3_order_status_transitions. Lifecycle 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]

@biz87 Спасибо. По модели OrderStatusService::change() — решение такое (сводим #596 и #603 к одному контракту).

Контракт change()

1. Одна DB-транзакция (до commit): доменные шаги, которые могут запретить переход (склад: reserve / commit / release) → запись status_id → commit. Откат только через rollback транзакции. Компенсирующего rollbackStatus() / undoUncommittedNewStatus() после commit не будет.
2. После commit: msOnChangeOrderStatus → журнал → уведомления.
3. msOnChangeOrderStatus не отменяет уже закоммиченный переход. Как в текущей beta по смыслу «статус уже применён»: ошибка плагина возвращается наружу, но status_id и склад остаются согласованными. Откат статуса повторным save() из feat(order): lifecycle gate with in-TX ports and idempotency #596 убираем — он как раз и даёт рассинхрон со складом. Если когда-нибудь понадобится отмена плагином — это отдельное решение: событие до commit, не компенсирующие save().
4. Один idempotent-контракт: повторный вызов «уже в этом статусе» → успех без событий/уведомлений (свести $options['idempotent'] из feat(order): lifecycle gate with in-TX ports and idempotency #596 и ensure() из feat(core): add payment attempt lifecycle and webhook callback #604 к одному публичному API).
5. Порты: не post-save() side-effects. Либо убрать, либо сделать in-TX domain hooks (через них же ходит склад feat(core): add opt-in inventory reserve, commit and release #603), с возможностью запретить переход до commit. Порта «стал Новым» для резерва либо нет как отдельной семантики — резерв вызывается явно как шаг политики статуса / координатора склада внутри п.1.

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

Согласен с предложенным: #605 → #604 → #596 (переделка под контракт выше) → #603 (склад садится на in-TX шаги, без своего компенсирующего undo после события).

#603 после модели

После переделки change() отдельно закроем:

• условный переход журнала (UPDATE … WHERE state = 'reserved') + MySQL-тест на два соединения;
• предупреждение в setting_ms3_inventory_enabled_desc (ru/en) про NULL→0 и общий остаток на товар.

Могу следующим шагом переписать #596 под этот контракт (без compensating rollback, порты/склад как pre-commit steps) и обновить описание PR.

[Ibochkarev]

@biz87 Переписал #596 под зафиксированный контракт:

• порты до persist status_id, внутри optional DB TX;
• убран compensating rollbackStatus();
• ошибка msOnChangeOrderStatus оставляет закоммиченный статус, без журнала/notify;
• удалён неиспользуемый ms3_err_status_rollback.

Ветка смержена с актуальной beta. Unit: OrderStatusServiceLifecycleTest + policy — 13 tests, exit 0.

Дальше по плану: #603 садится на in-TX hooks + conditional journal / lexicon.

[biz87]

Спасибо, контракт в коде виден: порты до записи status_id внутри транзакции, компенсирующего save() нет, после commit — событие, журнал, уведомления.

Слил с актуальной beta (ac8e267e), конфликтов нет. Гейт зелёный: PHPUnit 651, PHPStan версии CI (2.2.5) без ошибок.

Проверил на dev (MariaDB 10.6) вызовами OrderStatusService на временных заказах:

Сценарий	Результат
2 → 3	true, статус 3, запись в журнале
повтор того же статуса	ошибка «уже установлен»; idempotent и ensure() — true
порт запрещает переход	возвращается сообщение порта, статус 2, журнал пуст
allow-list пустой	переход разрешён
2:3, переход 2 → 5	«Такой переход статуса не разрешён», статус не изменён
2:3,2:5, переход 2 → 5	разрешён
мусор в настройке	«Некорректный allow-list», переход запрещён

Два места стоит поправить.

1. Вызов внутри уже открытой транзакции падает

persistStatusWithPorts() зовёт beginTransaction() безусловно и вне try. xPDO::beginTransaction() просто делегирует в PDO::beginTransaction(), а тот при активной транзакции бросает исключение. Проверил на dev: $modx->beginTransaction(); $service->change($id, 3, true); → PDOException: There is already an active transaction, исключение уходит наружу из change().

Сейчас оба вызывающих — PaymentLifecycleService и ShipmentLifecycleService — зовут change() / ensure() уже после своего commit, так что в штатном пути этого не случится. Но после #603 порты начнут писать склад, а плагин или интеграция может позвать change() из своей транзакции. В проекте уже есть нужный паттерн — PdoShipmentStore::beginTransaction() и PdoPaymentAttemptStore::writeWithEvent(): если транзакция уже открыта, работать в ней, свою не начинать и не коммитить.

Там же: is_callable([$this->modx, 'beginTransaction']) всегда истинно — проверять нужно inTransaction(), а не наличие метода.

2. Ошибка плагина на msOnChangeOrderStatus теряет запись в журнале

Журнал теперь пишется после события, поэтому при ошибке плагина статус закоммичен, а записи о смене нет. Проверил на dev временным плагином, который возвращает сообщение об ошибке:

change='тестовая ошибка плагина #596' status_id=3 status_logs=0

По контракту статус и не должен откатываться — но тогда журнал обязан отражать то, что уже произошло, иначе в истории заказа перехода не видно и разобраться, почему заказ оплачен, невозможно. Предлагаю писать журнал сразу после commit, до события (как в beta), а результат события возвращать наружу как сейчас.

Заодно

ShipmentLifecycleService::syncOrderStatus() зовёт change(), а PaymentLifecycleService — ensure(). С новым контрактом ошибка плагина оставляет статус применённым и возвращает ошибку наружу — вебхук отгрузки повторит запрос и получит «Этот статус уже установлен». Для повторов ensure() (или idempotent) логичнее, стоит выровнять оба вызова.

[Ibochkarev]

@biz87 Ветка перебазирована на beta (ac8e267e), конфликтов не было. Правки из ревью в 88d8671f.

1. persistStatusWithPorts() смотрит inTransaction(). Если транзакция уже открыта, работаем в ней и не вызываем beginTransaction() / commit() / rollback(). Свою транзакцию начинаем и закрываем только когда её не было. Повтор beginTransaction() внутри чужой транзакции больше не бросает PDOException.
2. Журнал пишется сразу после commit и до msOnChangeOrderStatus. Ошибка плагина по-прежнему не откатывает статус, но запись о переходе остаётся. Результат события возвращается наружу как раньше.
3. ShipmentLifecycleService::syncOrderStatus() зовёт ensure(), как оплата. Повтор вебхука при уже применённом статусе не получает «Этот статус уже установлен».

PHPUnit: OrderStatusServiceLifecycleTest и ShipmentLifecycleServiceTest, 28 тестов.

[Ibochkarev]

@biz87 Уточнение к 88d8671f: у xPDO нет inTransaction(), поэтому первая проверка транзакцию на MODX не открывала. В следующем коммите активная транзакция читается через $modx->pdo->inTransaction().