Ревью модуля Drupal: чек-лист для версий 10 и 11

Модуль Drupal может пройти функциональное тестирование и всё равно сломать кеш, открыть служебные данные или заблокировать обновление ядра. На ревью модуля для Drupal 10 и 11 смотрите не только на результат в браузере, но и на то, как код получает сервисы, проверяет доступ, хранит настройки и реагирует на пустые или неожиданные данные.

С чего начать ревью

Сначала зафиксируйте контекст: версию Drupal, версию PHP из проекта, способ установки зависимостей и назначение модуля. Один и тот же фрагмент может быть допустимым в узком внутреннем модуле, но неприемлемым для модуля, который публикуется для других сайтов.

• Откройте composer.json и проверьте ограничения для Drupal core, PHP и сторонних пакетов.
• Сопоставьте структуру модуля с его назначением: сервисы, плагины, формы, маршруты, конфигурация и тесты должны быть разделены по ответственности.
• Посмотрите историю изменений: незавершённый рефакторинг и временные обходы чаще всего видны именно там.
• Соберите модуль на чистой копии проекта и включите проверку Drupal Coding Standards, статический анализ и автоматические тесты, если они есть.

Не начинайте с форматирования. Если в модуле неверно задано разрешение доступа, идеальные отступы не меняют риск.

Хорошее ревью Drupal-модуля отвечает на три вопроса: кто может вызвать код, какие данные он изменит и что произойдёт при повторном вызове.

Пять проходов по коду

1. Архитектура и внедрение зависимостей

Проверьте сервисы в *.services.yml. Класс не должен доставать сервисы через глобальный контейнер в рабочей логике, если зависимость можно передать через конструктор. Так проще увидеть контракт класса, заменить зависимость в тесте и понять, что именно нужно для запуска операции.

Особенно внимательно смотрите на контроллеры, формы и плагины. Если один класс одновременно читает конфигурацию, сохраняет сущности, отправляет запрос во внешний API и строит разметку, это несколько зон ответственности, а не один удобный класс.

Отдельно проверьте порядок вызовов. Сначала должна происходить проверка входных данных и доступа, затем изменение состояния. Логирование, уведомления и внешние запросы не должны маскировать ошибку сохранения.

2. Маршруты, права и CSRF

Откройте каждый файл маршрутов и сопоставьте его с методом контроллера или формой. Для административной операции должно быть явно указано требуемое разрешение. Проверка «пользователь вошёл в систему» не заменяет permission: обычный авторизованный пользователь тоже может не иметь права менять настройки сайта.

Проверьте, что разрешение объявлено в .permissions.yml, имеет понятное имя и действительно ограничивает нужное действие. Опасные действия по маршруту GET — повод остановиться: просмотр и изменение данных не должны смешиваться без веской причины.

Для операций, которые меняют состояние, проверьте защиту от межсайтовой подделки запроса и корректное использование форм Drupal. Не принимайте токен из произвольного поля только потому, что запрос «идёт из админки».

В замечании лучше писать конкретно: «Маршрут удаляет сущность через GET и не требует отдельного разрешения. Нужен безопасный сценарий изменения состояния с проверкой доступа и защитой запроса», а не «добавить безопасность».

3. Ввод, вывод и работа с сущностями

Проследите путь пользовательского значения от формы или параметра маршрута до базы, лога и HTML. Проверка формата, проверка доступа и экранирование — разные операции. Наличие одной из них не отменяет остальные.

• Для сущности проверьте не только существование идентификатора, но и право текущего пользователя на конкретное действие.
• Не доверяйте значениям из URL, скрытым полям формы и данным из JavaScript: клиент может изменить их до отправки.
• При выводе убедитесь, что используется подходящий безопасный контекст Drupal, а не склейка HTML-строк с пользовательскими данными.
• При запросах к базе проверьте параметризацию и отсутствие конкатенации значений из запроса.
• В логах не должны оказаться пароли, токены, персональные данные и полные ответы внешних сервисов.

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

Врезка. Проверка доступа должна отвечать не на вопрос «залогинен ли пользователь», а на вопрос «может ли именно этот пользователь выполнить именно это действие над именно этим объектом».

4. Конфигурация, состояние и секреты

Разделите настройки модуля и техническое состояние. Конфигурация предназначена для переносимых параметров сайта: её можно экспортировать и применить в другом окружении. State хранит локальное состояние конкретной установки, например отметку о последнем запуске или результат обнаружения возможностей сервера.

Секреты нельзя помещать в обычную конфигурацию, которая попадёт в репозиторий или экспорт. Для ключей внешних сервисов заранее определите способ хранения, который принят в вашем проекте: переменные окружения, секретное хранилище или настройка вне репозитория. В коде допустима только заглушка вроде YOUR_API_KEY.

Проверьте схему конфигурации и обновления. Новое поле должно корректно появляться на чистой установке и после обновления существующего сайта. Удаление или переименование настройки без update-пути оставляет старые данные и создаёт расхождение между кодом и конфигурацией.

5. Кеширование и побочные эффекты

Кеш — самая частая зона, где «рабочий» модуль начинает отдавать неправильный результат. Для каждого кешируемого ответа выясните, от чего он зависит: от маршрута, языка, роли, пользователя, сущности, query-параметров или внешнего ответа.

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

Проверьте и обратную проблему: отключение кеша «для надёжности» может оставить страницу рабочей, но увеличить число запросов к базе или внешнему API. На ревью попросите измерение на тестовых данных, а не обещание, что нагрузка будет небольшой.

Что проверять в тестах

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

• Есть ли тест на пользователя без нужного permission.
• Проверяется ли доступ к объекту, который принадлежит другому пользователю или группе.
• Что произойдёт при пустом значении, неверном идентификаторе и удалённой сущности.
• Не создаёт ли повторная отправка формы дубликаты.
• Можно ли применить конфигурацию на чистой базе и обновить старую установку.
• Проверяется ли поведение при недоступном внешнем сервисе.

Не требуйте от каждого маленького модуля одинакового набора тестов. Но отсутствие тестов для разрешений, изменения данных и миграций должно быть осознанным решением, а не результатом нехватки времени.

Формулировки замечаний, которые помогают исправить код

Ревью не должно превращаться в список вкусовых предпочтений. Каждое замечание привязывайте к риску, месту и проверяемому действию.

• Вместо: «Вынести в сервис». Лучше: «Контроллер сам формирует запрос и отправляет уведомление. Вынесите операцию в сервис, чтобы повторно использовать её из очереди и проверить без HTTP-контекста».
• Вместо: «Нужна проверка прав». Лучше: «Проверяется только авторизация, но permission не ограничивает изменение настроек. Добавьте отдельное разрешение и тест пользователя без него».
• Вместо: «Добавить кеш». Лучше: «Ответ зависит от языка и роли, но метаданные этого не отражают. Опишите обе зависимости или не кешируйте этот ответ».
• Вместо: «Не использовать deprecated API». Лучше: «Вызов помечен устаревшим в целевой ветке Drupal. Проверьте замену по документации этой версии и добавьте отдельный коммит миграции».

Где подход ломается

Чек-лист не заменяет запуск модуля в окружении, похожем на продакшен. Ошибки часто появляются только при включённом reverse proxy, нескольких языках, большом числе ролей, неполной конфигурации или медленном внешнем API.

Не ограничивайтесь проверкой файлов модуля. Посмотрите, как он взаимодействует с настройками сайта, очередями, cron, импортом конфигурации и обновлением ядра. Модуль может быть аккуратным сам по себе, но опасным из-за неверного предположения о том, что другая часть проекта всегда включена.

Если в коде есть совместимость одновременно с Drupal 10 и 11, не угадывайте поведение по названию класса. Сверьте поддерживаемые API, ограничения в composer.json и предупреждения статического анализа. При сомнении зафиксируйте версию, на которой проходило ревью, и отдельно заведите задачу на проверку следующей версии.

Что сделать после ревью

Соберите результаты в три списка: блокирующие риски, исправления до релиза и улучшения без влияния на безопасность или данные. Затем прогоните чистую установку, обновление с предыдущей версии и сценарий отката. Для каждого исправления добавьте тест или воспроизводимый шаг проверки.

После выкладки наблюдайте не только ошибки PHP. Смотрите на отказы доступа, время ответа маршрутов, количество запросов к внешним сервисам, промахи кеша и сообщения очереди. Для модулей с конфигурацией отдельно проверьте следующий экспорт и импорт: именно там быстро обнаруживаются настройки, которые код использует, но схема или обновление не поддерживают.

Термин: dependency injection, или внедрение зависимостей, — передача сервисов классу извне, обычно через конструктор, вместо самостоятельного поиска этих сервисов внутри класса.