Ревью кода на Битрикс: что проверять в первую очередь

Код на Битрикс может выглядеть аккуратно и при этом замедлять каталог, обходить права доступа или сбрасывать кеш после каждого запроса. На ревью проекта под 1С-Битрикс: Управление сайтом 24.x сначала проверяйте не форматирование, а путь данных: от входных параметров до SQL-запроса, вывода и кеша. Такой порядок помогает быстро найти дефекты, которые превращаются в 500, утечку данных или часы отладки после релиза.

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

Не открывайте pull request и не читайте его построчно с первой строки. Сначала определите границы изменения. Один и тот же фрагмент опасен по-разному в компоненте каталога, обработчике события и административной странице.

• Где выполняется код: публичная страница, AJAX-обработчик, CLI-скрипт, агент или событие.
• Кто может его вызвать: любой посетитель, авторизованный пользователь, менеджер или администратор.
• Какие данные он читает и изменяет: инфоблоки, highload-блоки, заказы, файлы, пользовательские поля.
• Сколько раз он вызывается за один запрос и может ли работать в цикле.
• Что произойдёт с кешем после изменения данных.

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

Главный вопрос ревью на Битрикс: «Может ли этот код выполнить лишнюю работу или дать пользователю чужие данные?»

Термин: D7 — современный слой API Битрикс с пространствами имён вроде Bitrix\Main и ORM. Старый процедурный API не становится автоматически ошибкой, но новый код стоит оценивать с учётом поддержки, типизации и поведения запросов.

Пять проверок в правильном порядке

1. Входные данные и права

Проверьте все значения из GET, POST, URL, cookies, AJAX-запросов и скрытых полей формы. Фильтрация строки не заменяет проверку права на объект: пользователь может передать корректный идентификатор чужого заказа или элемента инфоблока.

В ревью полезны конкретные формулировки:

• «Где проверяется, что текущий пользователь имеет право читать этот объект?»
• «Почему идентификатор берётся из запроса без проверки принадлежности пользователю?»
• «Есть ли CSRF-защита для действия, которое изменяет данные?»
• «Что произойдёт, если параметр пустой, отрицательный или содержит несколько значений?»

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

2. Запросы к базе и цикл

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

Ищите:

• повторный вызов выборки внутри foreach;
• получение всех полей и свойств, когда нужны два поля;
• сортировку и фильтрацию в PHP вместо базы данных;
• выборку без ограничения количества записей;
• запросы из шаблона компонента;
• загрузку файлов, изображений и связанных сущностей по одному объекту.

В новом коде на D7 проверьте, какие поля выбирает ORM-запрос и какие связи он раскрывает. В старом API смотрите параметры выборки и фильтра: сам вызов CIBlockElement::GetList не доказывает, что запрос оптимален.

3. Кеш и его границы

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

Проверьте три вещи:

1. Все ли параметры, влияющие на ответ, входят в ключ кеша.
2. Есть ли понятный срок жизни и сценарий очистки после записи.
3. Не кешируется ли персональный или административный результат на публичном уровне.

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

4. События, агенты и побочные эффекты

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

Опасные признаки:

• сетевой запрос к CRM или службе доставки внутри пользовательского запроса;
• отправка письма при каждом повторном сохранении без защиты от дублей;
• изменение того же объекта внутри обработчика его сохранения;
• тяжёлая обработка в агенте без контроля времени выполнения;
• исключение внешнего сервиса, которое ломает оформление заказа.

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

5. Ошибки, логирование и совместимость

Пустой catch, вывод ошибки пользователю и запись полного ответа внешнего сервиса в лог — три разных проблемы. В публичном ответе не должны появляться путь к файлу, SQL-текст, токен, персональные данные или внутреннее исключение.

Проверьте также:

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

Если версия ядра или модуля неизвестна, не принимайте утверждение «метод есть в Битрикс» на веру. Зафиксируйте целевую версию 1С-Битрикс и сверьте вызов с официальной документацией этой версии.

Что проверять первым в зависимости от места кода

Что ломается на практике

«Проверка есть, но данные всё равно чужие»

Разработчик проверяет, что пользователь вошёл в систему, но не проверяет доступ к конкретному элементу. Авторизация отвечает на вопрос «кто это», а не «что ему разрешено». В ревью просите показать источник права и тест с идентификатором объекта другого пользователя.

«Кеш включили, а страница стала неправильной»

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

«На тесте быстро, на каталоге медленно»

Тестовые данные редко показывают проблему с запросами. Попросите замер на объёме, близком к рабочему, и зафиксируйте число запросов, время ответа, объём памяти и размер ответа. Если замеров нет, не объявляйте оптимизацию успешной: сначала добавьте измерение.

«После сохранения действие выполняется дважды»

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

Как формулировать замечания, чтобы их исправили

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

Хорошее замечание содержит четыре части:

1. место проблемы;
2. условие, при котором она проявится;
3. последствие для пользователя или сервера;
4. способ проверить исправление.

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

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

Соберите для проекта короткий чек-лист и применяйте его к каждому изменению: входные данные и права, запросы, кеш, события, ошибки, версия ядра. Затем добавьте два обязательных теста к наиболее дорогим сценариям — просмотр каталога и изменение заказа. В проде наблюдайте время ответа, количество SQL-запросов, ошибки PHP, очистки кеша и повторные вызовы интеграций. Именно эти показатели покажут, прошло ли ревью дальше формального согласования.