Общая оценка

PR решает реальную проблему (Issue #68): $modx->services->get('ms3') бросает Exception если сервис не зарегистрирован, а проверка if ($ms3) после get() бесполезна — выполнение до неё не доходит. Подход has() + get() — правильный.

Проблемы

1. api.php: удалена fallback-регистрация сервиса

Текущий код имеет fallback:

if (!$modx->services->has('ms3')) {
    $modx->services->add('ms3', function() use ($modx) {
        return new \MiniShop3\MiniShop3($modx);
    });
}

PR заменяет это на 503-ошибку. api.php — самостоятельная точка входа, сервис регистрируется через bootstrap.php при $modx->initialize('web'). Если bootstrap отработал — fallback не нужен. Если не отработал — 503 оправдан.

Но это изменение поведения, стоит упомянуть в описании PR.

2. Response::error() + поле code — глобальный побочный эффект

Добавление 'code' => $statusCode в Response::error() влияет на ВСЕ API-ошибки, не только на 503.

Router::normalizeResponse() уже имеет $statusCode = $result['code'] ?? 400. До PR: Response::error('Token is required', 401)->getData() не содержал code → HTTP всегда 400. После PR: содержит 'code' => 401 → HTTP 401.

Это правильное исправление, но затрагивает всю систему. Стоит вынести в отдельный коммит или документировать как отдельный fix.

3. Исправлены баги в CartController (не упомянуто)

CartController::change() и CartController::remove() возвращают Response::error(...) (объект Response) вместо Response::error(...)->getData() (массив), нарушая тип : array. PR молча исправляет это — стоит отметить как отдельный fix.

4. Бойлерплейт: 4 строки × 15+ методов

$ms3 = $this->getMs3OrFail();
if ($ms3 === null) {
    return $this->serviceUnavailableResponse();
}

Повторяется ~15 раз. Альтернатива — middleware на уровне роутера, который проверяет has('ms3') до вызова контроллера. Одна проверка вместо пятнадцати.

5. Два паттерна для одной проверки

• Контроллеры: GetMs3OrFailTrait
• Роуты (web.php): замыкание $ensureMs3Service

Middleware решил бы оба случая единообразно.

6. code попадает в JSON-ответ клиенту

Поле code теперь будет в каждом error-ответе. Это изменение API-контракта — не критично, но стоит зафиксировать.

Что хорошо

• Сниппеты и плагин — чистые проверки
• Trait с понятным API
• Логирование при отсутствии сервиса
• connector.php — корректный Content-Type

Рекомендация

PR решает реальную проблему, но нужны доработки:

1. Описать изменение поведения api.php
2. Выделить fix Response::error code и CartController bugs — они самоценны вне issue #68
3. Рассмотреть middleware вместо бойлерплейта в контроллерах

Дополнение к ревью: middleware вместо бойлерплейта в контроллерах

Конкретное предложение по пункту 4-5. Все контроллеры и inline closures в web.php проходят через роутер, который уже поддерживает middleware (см. AuthMiddleware, RateLimitMiddleware).

Middleware:

<?php
namespace MiniShop3\Router\Middleware;

use MiniShop3\Router\Response;
use MODX\Revolution\modX;

class ServiceCheckMiddleware implements MiddlewareInterface
{
    protected modX $modx;

    public function __construct(modX $modx)
    {
        $this->modx = $modx;
    }

    public function handle(array $params)
    {
        if (!$this->modx->services->has('ms3')) {
            $this->modx->log(modX::LOG_LEVEL_ERROR, '[MiniShop3] Service not registered');
            return Response::error('Service unavailable', 503);
        }
        return null;
    }
}

Применение в web.php — одна строка:

$serviceCheckMiddleware = new ServiceCheckMiddleware($modx);

$router->group('/api/v1', function($router) use (...) {
    // все роуты защищены
}, [$corsMiddleware, $rateLimitMiddleware, $serviceCheckMiddleware]);

Что это убирает:

• GetMs3OrFailTrait — не нужен
• $ensureMs3Service лямбда в web.php — не нужна
• 4 строки × 15 методов = ~60 строк бойлерплейта в контроллерах — не нужны

Контроллеры остаются чистыми — services->get('ms3') безопасен, т.к. middleware уже гарантировал наличие сервиса. Как $tokenMiddleware гарантирует авторизацию.

Проверки в api.php, connector.php, плагине и сниппетах остаются как есть — они не проходят через роутер.