Рефакторинг легаси на Yii2: что ревьюить в первую очередь

Типичная картина проекта на Yii2 (2.0.x), который живёт пять-семь лет: контроллеры по 800–1200 строк, бизнес-логика размазана между actionUpdate() и beforeSave(), а половина запросов к базе — это Model::find()->all() с последующей фильтрацией в PHP. Формально всё работает, но каждое изменение в заказах ломает отчёты, а новый разработчик тратит день, чтобы понять, где считается итоговая цена.

Это разбор для того, кто отвечает за такой проект и должен решить, что ревьюить и переписывать в первую очередь, а что не трогать. Речь не про переход на другой фреймворк и не про полное переписывание — только про точечный рефакторинг внутри Yii2 с фокусом на то, что даёт максимум пользы при минимуме риска.

Смежные темы вроде миграции на Yii3 или перехода на другую ORM здесь не рассматриваются — это отдельный разговор с другой ценой.

С чего начинать ревью: не с красоты, а с боли

Порядок разбора легаси задаётся не эстетикой кода, а тем, что чаще ломается и дороже стоит в отладке. В проектах на Yii2 болевые точки повторяются от заказа к заказу.

• Толстые контроллеры. Бизнес-логика в экшенах — источник дублирования: один и тот же расчёт скидки живёт в трёх контроллерах с расхождениями.
• Fat models с логикой в событиях. beforeSave() на 150 строк, который дёргает внешний API и шлёт письма, — тестировать это невозможно, а падает оно в самый неподходящий момент.
• Запросы в цикле. Классический N+1: перебор коллекции с обращением к связи внутри цикла без with().
• Прямой доступ к $_POST и $_GET. В обход Yii::$app->request и валидации через модель.

Начинайте с того, что чаще всего фигурирует в задачах на баги. Если 30% тикетов касаются расчёта цены заказа — рефакторить надо расчёт цены, а не переименовывать переменные в модуле, который никто не трогает.

Рефакторинг легаси — это не уборка ради чистоты, а снижение стоимости следующего изменения. Если после недели работы правка бага стала занимать не два часа, а двадцать минут, рефакторинг окупился.

Что конкретно писать в ревью

Абстрактное «улучшите качество кода» в комментарии к пул-реквесту бесполезно. Формулировки должны быть проверяемыми.

Толстый контроллер

Вместо «слишком много логики» пишите конкретику: «Расчёт итоговой суммы (строки 40–95) дублирует логику из CartController::actionCheckout. Вынести в отдельный класс OrderPriceCalculator, оба контроллера дёргают его метод». Так вы указываете и проблему, и куда девать код.

В Yii2 естественное место для такой логики — не модель ActiveRecord и не контроллер, а отдельный сервисный класс. Регистрируется он в контейнере через Yii::$container или в конфиге components, а в контроллер приходит через конструктор — Yii2 умеет внедрять зависимости в экшены с версии 2.0.36.

<?php
// services/OrderPriceCalculator.php — сервис расчёта цены заказа
namespace app\services;

use app\models\Order;

class OrderPriceCalculator
{
    private float $vatRate;

    public function __construct(float $vatRate = 0.20)
    {
        $this->vatRate = $vatRate;
    }

    // Возвращает итоговую сумму заказа с учётом скидки и НДС
    public function total(Order $order): float
    {
        $sum = 0.0;
        foreach ($order->items as $item) {
            $sum += $item->price * $item->quantity;
        }
        $sum -= $this->discount($order, $sum);
        return round($sum * (1 + $this->vatRate), 2);
    }

    private function discount(Order $order, float $sum): float
    {
        return $order->isVip ? $sum * 0.1 : 0.0;
    }
}

Контроллер после этого тощает до вызова $this->calculator->total($order), а расчёт можно покрыть юнит-тестом без поднятия всего приложения. Замечание в ревью: «Логика цены не должна зависеть от Yii::$app->request — сервис принимает готовую модель, а не разбирает POST».

Логика в событиях модели

Отправка письма или вызов внешнего API внутри afterSave() — частая находка. Проблема в том, что сохранение модели и побочный эффект оказываются в одной транзакции по смыслу, хотя технически ей не являются: письмо ушло, а коммит откатился. Формулировка замечания: «Отправку уведомления из afterSave() вынести в отдельный вызов после успешного сохранения или в очередь. Событие модели не место для сетевых операций».

N+1 в списках

Самое дешёвое по трудозатратам и самое заметное по эффекту. Ищите foreach по результату find()->all(), где внутри тела дёргается связь.

<?php
// Было: N+1 — на каждый заказ отдельный запрос за клиентом и товарами
$orders = Order::find()->all();
foreach ($orders as $order) {
    echo $order->customer->name;   // +1 запрос
    echo count($order->items);     // +1 запрос
}

// Стало: жадная загрузка связей одним пакетом запросов
$orders = Order::find()
    ->with(['customer', 'items'])
    ->all();
foreach ($orders as $order) {
    echo $order->customer->name;   // уже загружено
    echo count($order->items);
}

Проверить эффект можно через панель отладки Yii (yii2-debug): вкладка Database покажет число запросов до и после. На списке из 200 заказов разница обычно с 400+ запросов до трёх-четырёх.

Как не сломать всё за один заход

Легаси без тестов рефакторить страшно, потому что нечем подтвердить, что поведение не изменилось. Порядок, который снижает риск:

1. Сначала характеризующий тест. Перед тем как трогать расчёт цены, напишите тест, фиксирующий текущий результат на реальных данных — даже если он «неправильный». Ваша задача на этом шаге — сохранить поведение, а не исправить его.
2. Выделите код, не меняя логику. Перенос метода в сервис без изменения тела — самый безопасный вид рефакторинга. Тест из шага 1 должен остаться зелёным.
3. Только потом исправляйте. Когда логика изолирована и покрыта тестом, багфикс становится дешёвым и наблюдаемым.
4. Мелкими пул-реквестами. PR на 40 файлов не ревьюится — он аппрувится по доверию. Один класс за раз.

Для Yii2 тесты пишутся через Codeception (он идёт в стандартном шаблоне приложения) либо чистым PHPUnit. Для сервисных классов без обращения к базе хватит PHPUnit — они не тянут за собой Yii::$app.

Сравнение: куда девать бизнес-логику

Где рефакторинг Yii2 ломается

Несколько мест, где типовой подход даёт осечку именно на этом фреймворке.

• Поведения (behaviors) и события. Прежде чем выносить логику из beforeSave(), проверьте, не навешаны ли на модель behaviors через конфиг или другой код — TimestampBehavior, BlameableBehavior. Вынесете не то — потеряете заполнение created_at.
• Скоупы и defaultScope. Если в модели переопределён метод построения запроса, ваш with() может конфликтовать с уже применённым условием. Смотрите, что реально уходит в SQL.
• Гидратация в контейнере. Внедрение зависимостей в экшены работает с 2.0.36. На более старых версиях сервис придётся получать через Yii::$container->get() вручную — проверьте версию в composer.lock.
• Кеш схемы. После изменения структуры таблиц ActiveRecord может держать старую схему в кеше. Если после миграции модель «не видит» новое поле — сбросьте кеш схемы.

Что делать дальше

Заведите на проекте статический анализатор — PHPStan или Psalm — и запустите его на уровне 0, чтобы получить срез самых грубых проблем без потока шума. Дальше поднимайте уровень по одному, фиксируя достигнутый в CI, чтобы новый код не откатывал прогресс.

Параллельно измеряйте главный показатель — время от заведения тикета до мержа фикса. Если оно снижается на модулях, которые вы уже причесали, рефакторинг идёт в правильную сторону. Если нет — вы полируете код, который никого не беспокоит, и это трата бюджета.