Skip to content

Обнаружение рассинхрона версии пакета и файлов на диске - #624

Open
Ibochkarev wants to merge 3 commits into
betafrom
fix/issue-622-version-mismatch-check
Open

Обнаружение рассинхрона версии пакета и файлов на диске#624
Ibochkarev wants to merge 3 commits into
betafrom
fix/issue-622-version-mismatch-check

Conversation

@Ibochkarev

Copy link
Copy Markdown
Member

Описание

Обновление пакета может завершиться «успешно» в MODX, даже если файлы не скопировались на диск. В БД оказывается новая версия и миграции, а core/components/minishop3/ остаётся старым. Пользователь об этом не узнаёт.

Этот PR:

  1. При install/upgrade записывает системную настройку ms3_version из signature транспортного пакета (resolver_09_version.php).
  2. В плагине на OnManagerPageBeforeRender сравнивает $ms3->version (диск) с ms3_version (пакет) и показывает заметный баннер при расхождении.
  3. Логика сравнения и парсинг signature в плагине/резолвере — inline, без autoload новых классов из src/, чтобы проверка работала даже когда файлы на диске не обновились (плагины не static → код в БД).

Health-эндпоинты /api/mgr/health и /api/v1/health уже читают ms3_version и начнут отдавать реальную версию после установки.

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

  • Исправление бага (non-breaking change)
  • Новая функциональность (non-breaking change)
  • Breaking change (изменение, ломающее обратную совместимость)
  • Рефакторинг (без изменения функциональности)
  • Документация
  • Другое (опишите):

Связанные Issues

Closes #622

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

php -l _build/resolvers/resolver_09_version.php
php -l core/components/minishop3/elements/plugins/minishop3.php
# … остальные затронутые PHP — exit 0

cd core/components/minishop3
./vendor/bin/phpunit tests/Unit/Utils/VersionSyncContractTest.php
# OK (11 tests, 21 assertions), exit 0
  • Ручное тестирование
  • Автоматические тесты (composer ci:php / composer test, npm run lint:ci, composer stan / GitHub Actions CI)
  • Тестирование на разных версиях PHP/MODX

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

  • MiniShop3: ветка fix/issue-622-version-mismatch-check
  • MODX: n/a (unit/contract)
  • PHP: 8.4.17

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

До После

Чеклист

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

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

  • Уже сломанные сайты получат баннер только после установки пакета с этим плагином (код плагина приедет в БД независимо от копирования файлов).
  • ms3_version — обычный textfield в ms3_main; значение выставляет резолвер. Ручное редактирование может скрыть или вызвать ложное предупреждение.
  • Gate F: code-reviewer (Sol) — без BLOCK; thermo — убран неиспользуемый PackageVersion класс, оставлен contract-тест диск-независимости.

Persist ms3_version from the transport signature and warn in the manager
when disk MiniShop3::$version drifts from the installed package.
@Ibochkarev
Ibochkarev requested a review from biz87 August 21, 2026 03:39
@biz87

biz87 commented Aug 29, 2026

Copy link
Copy Markdown
Member

Спасибо, механика верная — проверил по коду, не по описанию:

  • static.plugins => false + update.plugins => true (_build/config.inc.php:33,24) — код плагина живёт в БД и перезаписывается при обновлении, то есть проверка действительно доедет до сломанных сайтов, где файлы не скопировались.
  • Резолвер подхватится без правки build.php — там scandir (_build/build.php:98-107).
  • Порядок верный: vehicle настроек кладётся в манифест раньше главного category-vehicle, значит ms3_version='' создаётся, а резолвер потом пишет реальную версию.
  • File-резолверы регистрируются раньше php-резолверов, а xPDOVehicle::resolve() не прерывает цикл при сбое — resolver_09 отработает при любом исходе копирования.
  • Кейс из Обновление может пройти «успешно» без доставки файлов — нужна проверка рассинхрона версий #622 сверил по тегу 1.4.0-beta1: bootstrap.php там регистрирует ms3, $version публичный — баннер бы сработал.
  • Нагрузки нет: getOption('ms3_version') читает $modx->config, запросов к БД ноль.

Тесты локально на ветке, смерженной с актуальной beta: smoke 89/89, PHPUnit 265 тестов / 623 assertions, PHPStan по изменённым файлам чисто.

Три вещи прошу доработать.

1. Не покрыт сценарий, где каталога на диске нет вообще

Блок проверки стоит после гварда if (!$modx->services->has('ms3')) break; (elements/plugins/minishop3.php:37-40). Сервис регистрируется из bootstrap.php, который MODX грузит под is_readable (modX::_initNamespaces(), modX.php:623).

Кейс из #622 это не ломает — там каталог существовал со старыми файлами 1.4.0, сервис регистрировался. Но при первой установке с полным провалом копирования (в логе из issue не скопировалось 855 файлов src/ и 6809 vendor/) каталога нет, сервиса нет, плагин делает break до проверки — и остаётся одна строчка в логе. Сценарий тише и хуже исходного: пользователь видит менеджер вообще без следов MiniShop3.

Показательно, что твой же тест утверждает isMismatch('', '1.13.0-beta1') === true (VersionSyncContractTest.php:70) — до этого состояния код физически не доходит, тест описывает контракт, которого в плагине нет.

Предлагаю поднять блок выше гварда:

case 'OnManagerPageBeforeRender':
    $packageVersion = (string)$modx->getOption('ms3_version', null, '');
    $diskVersion = $modx->services->has('ms3')
        ? (string)$modx->services->get('ms3')->version
        : '';
    // проверка здесь, с отдельным текстом для $diskVersion === ''
    // («файлы компонента не найдены на диске»)
    if (!$modx->services->has('ms3')) {
        $modx->log(modX::LOG_LEVEL_ERROR, '[MiniShop3] Service not registered');
        break;
    }

2. Ложные срабатывания при git/rsync-деплое

Сравнение строгое в обе стороны (:50). Сайты, где файлы обновляются не транспортным пакетом (dev/staging, CI-деплой), получат постоянный неубираемый баннер, как только MiniShop3::$version уйдёт вперёд последнего установленного пакета. Предлагаю version_compare($disk, $package, '<') — предупреждать только когда диск отстаёт.

Заодно просьба добавить в новый тест-файл ассерт, что MiniShop3::$version === config['version'] . '-' . config['release']. Сейчас они синхронизируются руками и ничем не проверяются (на данный момент совпадают — 1.13.0-beta1). Раньше расхождение было косметикой, после этого PR оно даст ложный красный баннер всем пользователям релиза.

3. LOG_LEVEL_ERROR на каждый рендер страницы менеджера

:69-72 — пока держится рассинхрон, это дисковый I/O на каждый запрос. Воспроизводит ровно ту проблему с гигантским логом, от которой #622 и отталкивался. Нужен throttle (раз на сессию) или понижение до WARN.

Мелочи, на усмотрение

  • regClientStartupHTMLBlock кладёт <div> в <head> (header.tpl:31, до </head>); корректнее regClientHTMLBlock — попадёт в footer.tpl:7, внутри body.
  • У баннера нет кнопки закрытия и нет компенсирующего отступа у body — он навсегда перекрывает верхние ~45px каждой страницы, включая #modx-action-buttons-container с кнопкой «Сохранить».
  • Стоит ограничить показ по $modx->user && $modx->user->hasSessionContext('mgr') — редакторы всё равно ничего не сделают, а этот паттерн двумя строками ниже уже используется.
  • $modx->lexicon() может вернуть null (modX.php:2237-2245), две проверки через === этого не ловят → htmlspecialchars(null) deprecated с PHP 8.1.
  • Есть готовый xPDOTransport::parseSignature(). Обоснование «inline ради независимости от диска» к xPDO не относится — это vendor MODX, он есть всегда; инлайн-версия к тому же хуже разбирает имена пакетов с дефисом.
  • Возврат $setting->save() в резолвере не проверяется.
  • В health'ах getOption(..., '1.0.0') без $skipEmpty: если настройка есть, но пустая (резолвер не смог, почистили руками), вернётся "", а не дефолт.

К сведению, не к правке: /api/v1/health публичный (config/routes/web.php:52,340 — CORS + rate limit, без TokenMiddleware), и PR меняет там константу 1.0.0 на реальную версию компонента для анонимных запросов. Это осознанное решение, вопрос не к тебе. Баннер на странице логина не показывается — security/login.tpl не выводит $cssjs, это проверил отдельно.

Warn only when disk lags or is missing, surface total copy failure before
the ms3 guard, throttle WARN logs, and harden resolver/health edge cases.
@Ibochkarev

Copy link
Copy Markdown
Member Author

Ответ на ревью

Учтены три обязательных пункта и мелочи из комментария:

  1. Нет каталога / нет сервиса — проверка версии до гварда ms3; при пустом диске отдельный ключ ms3_version_files_missing.
  2. git/rsync — предупреждение только при version_compare($disk, $package, '<') (и при пустом диске). Добавлен ассерт MiniShop3::$version === config version-release.
  3. ЛогLOG_LEVEL_WARN + один раз за mgr-сессию.

Также: regClientHTMLBlock, dismiss + динамический padding-top, gate mgr-сессии, null-safe lexicon, xPDOTransport::parseSignature, проверка save(), health getOption(..., true).

Gate E: php -l на затронутых PHP — OK; VersionSyncContractTest — 14 tests, 32 assertions, exit 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants