From fa910c3da369f1f39565f0be1df98f180d40d177 Mon Sep 17 00:00:00 2001 From: "E.Gavrilov" Date: Fri, 4 Sep 2026 00:04:42 +0300 Subject: [PATCH] Editorial: refine Bitrix legacy refactoring article 326 --- editorial/agent-rewrites/326.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editorial/agent-rewrites/326.json b/editorial/agent-rewrites/326.json index 547cdef..c547d0d 100644 --- a/editorial/agent-rewrites/326.json +++ b/editorial/agent-rewrites/326.json @@ -1,7 +1,7 @@ { "index": 326, "slug": "editorial-2018-12-mechanism-legacy-refactoring", - "title": "Bitrix: как безопасно вынести запись из смешанного save.php", + "title": "Bitrix: как отделить запись от смешанного save.php", "excerpt": "Старый обработчик формы может изменить инфоблок, отправить уведомление и показать HTML в одном проходе. Разбираем, как отделить запись элемента, увидеть отрицательный путь и проверить локальный рефакторинг без ложного сообщения об успехе.", - "contentHtml": "

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

\n

Так происходит, когда старый save.php читает $_POST, вызывает CIBlockElement::Update, отправляет уведомление и выводит HTML в одной функции. Первый рефакторинг такого файла должен разделить не все слои системы, а один наблюдаемый переход. Сначала отделяем запись элемента от формы и последующих действий. Затем проверяем фактическое значение повторным чтением.

\n

Симптом начинается раньше сообщения

\n

Один HTTP-запрос может оставить несколько следов. Входные поля приходят из формы. Инфоблок хранит изменённое поле. Почтовый или сетевой вызов оставляет след во внешней системе. Браузер получает только последний ответ обработчика. Если поздний вызов падает, этот ответ не рассказывает, что произошло с инфоблоком несколькими строками выше.

\n

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

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

Механизм: Update и ответ формы живут на разных границах

\n

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

\n

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

\n
СимптомПричинаПроверкаДействие
Браузер показал ошибку, но поле изменилосьСбой произошёл после UpdateСнова прочитать элемент по ID и сравнить полеНе повторять POST; разобрать поздний вызов
Пустой ID выглядит как ошибка BitrixГлобальный вход привели к целому числу поздноПроверить ID и обязательные поля до записиВернуть ошибку валидации без записи
Значение отличается от расчётаОбработчик изменил вход или другой процесс записал полеПроверить обработчики и повторную выборкуОстановить расширение шва
Повтор формы отправил два уведомленияВнешний эффект не имеет отдельного статусаРазвести результат записи и уведомленияОпределить политику повтора
Затронуты лишние свойстваВ Update передали широкий массивСравнить ключи с контрактомПередавать только нужное поле
\n

Учебный пример смешанного обработчика

\n

Следующий фрагмент — учебный пример, ограниченный объяснением механизма. Он не взят из конкретного production-проекта и не доказывает наличие функции sendPartnerNotice в вашем сайте. В нём намеренно оставлены типичные границы старого файла.

\n
<?php
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
    $elementId = (int) $_POST['ID'];
    $name = trim((string) $_POST['NAME']);

    if ($elementId <= 0 || $name === '') {
        echo 'Заполните ID и название.';
        return;
    }

    CModule::IncludeModule('iblock');
    $element = new CIBlockElement();
    if (!$element->Update($elementId, array('NAME' => $name))) {
        echo $element->LAST_ERROR;
        return;
    }

    sendPartnerNotice($elementId, $name);
    echo 'Сохранено';
}
\n

У примера три отрицательных пути. Валидация останавливает запрос до записи. Bitrix может вернуть отказ. Внешний вызов может завершиться ошибкой после успешной записи. Последний путь нельзя исправить строкой «повторить Update»: она не делает внешнюю операцию безопасной и скрывает, что поле уже изменилось.

\n

Первый шов: функция возвращает только результат записи

\n

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

\n
<?php
function saveProductName($elementId, $name)
{
    if (!CModule::IncludeModule('iblock')) {
        throw new RuntimeException('Module iblock is unavailable');
    }

    $elementId = (int) $elementId;
    $name = trim((string) $name);
    if ($elementId <= 0 || $name === '') {
        throw new InvalidArgumentException('ID and NAME are required');
    }

    $element = new CIBlockElement();
    if (!$element->Update($elementId, array('NAME' => $name))) {
        throw new RuntimeException($element->LAST_ERROR ?: 'Element update failed');
    }

    return $elementId;
}

try {
    $savedId = saveProductName($_POST['ID'], $_POST['NAME']);
    $message = 'Карточка сохранена: ' . $savedId;
} catch (InvalidArgumentException $error) {
    $message = $error->getMessage();
} catch (RuntimeException $error) {
    $message = $error->getMessage();
}
\n

Фрагмент остаётся учебным. Он не заменяет правила доступа, CSRF-защиту, журналирование и обработку конкретной версии Bitrix. Перед переносом сохраните контракт формы и проверьте зарегистрированные обработчики.

\n

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

\n

Обработчики и набор полей

\n

OnBeforeIBlockElementUpdate получает параметры до изменения и может их переопределить или отменить операцию. Обработчик после попытки обновления может сработать и при неудаче, поэтому в нём нужно смотреть результат, а не только факт вызова. Эти события объясняют расхождение между переданным массивом и состоянием элемента.

\n

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

\n

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

\n

Порядок локальной переделки

\n
  1. Запишите один симптом: например, ошибка формы не отвечает, изменилось ли имя.
  2. Перечислите побочные эффекты старого обработчика в фактическом порядке: запись, уведомление, кеш, HTML и внешние вызовы.
  3. Выберите один эффект для первого шва и зафиксируйте ID, инфоблок, поле, успех и отказ.
  4. Проверьте зарегистрированные обработчики Bitrix и все места вызова участка.
  5. Вынесите запись в функцию с явными аргументами. Уберите из неё $_POST, echo и сторонние вызовы.
  6. Подключите функцию одним вызовом, оставив неисследованные действия в прежнем порядке.
  7. На разрешённом тестовом элементе сохраните исходное значение, выполните один вызов и прочитайте поле обратно.
  8. Проверьте пустой вход, неверный ID, отказ обработчика и ошибку позднего эффекта.
  9. Если значение расходится с ожидаемым, остановите расширение. Не добавляйте повторную запись наугад.
\n

Отрицательный путь важнее зелёного сообщения

\n

Нужно различать четыре состояния: вход отклонён, запись отклонена, запись подтверждена, поздний эффект завершился отдельно. Первые два не должны менять элемент. Третье подтверждается повторным чтением. Четвёртое сообщает о частичном результате и не заставляет пользователя повторять весь запрос.

\n

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

\n

Ограничения метода

\n

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

\n

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

\n

Критерий готовности

\n

Локальный рефакторинг готов, когда для одного выбранного элемента выполнены все условия: вход проходит отдельную проверку; функция меняет только заявленное поле; результат Update обработан явно; обработчики просмотрены; фактическое значение подтверждено повторной выборкой; поздний эффект имеет отдельный результат; отрицательный путь не запускает повторную запись; при расхождении есть действие остановки или возврата.

\n

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

\n

Проверяемые источники

" + "contentHtml": "

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

\n

Так происходит, когда старый save.php читает $_POST, вызывает CIBlockElement::Update, отправляет уведомление и выводит HTML в одной функции. Первый рефакторинг такого файла должен отделить один наблюдаемый переход, а не сразу переделать весь проект. Сначала отделяем запись элемента от формы и последующих действий. Затем проверяем фактическое значение повторным чтением.

\n

Симптом начинается раньше сообщения

\n

Один HTTP-запрос может оставить несколько следов. Входные поля приходят из формы. Инфоблок хранит изменённое поле. Почтовый или сетевой вызов оставляет след во внешней системе. Браузер получает только последний ответ обработчика. Если поздний вызов падает, этот ответ не рассказывает, что произошло с инфоблоком несколькими строками выше.

\n

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

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

Механизм: Update и ответ формы живут на разных границах

\n

CIBlockElement::Update принимает ID и массив полей. Официальная документация фиксирует его контракт: метод возвращает true при успехе и false при ошибке, а текст ошибки доступен в LAST_ERROR. До записи обработчик OnBeforeIBlockElementUpdate может изменить параметры или отменить операцию. После попытки обновления может сработать обработчик другого этапа.

\n

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

\n
От симптома к проверке границы записи и следующему действию
СимптомПричинаПроверкаДействие
Браузер показал ошибку, но поле изменилосьСбой произошёл после UpdateСнова прочитать элемент по ID и сравнить полеНе повторять POST; разобрать поздний вызов
Пустой ID выглядит как ошибка BitrixГлобальный вход привели к целому числу поздноПроверить ID и обязательные поля до записиВернуть ошибку валидации без записи
Значение отличается от расчётаОбработчик изменил вход или другой процесс записал полеПроверить обработчики и повторную выборкуОстановить расширение шва
Повтор формы отправил два уведомленияВнешний эффект не имеет отдельного статусаРазвести результат записи и уведомленияОпределить политику повтора
Затронуты лишние свойстваВ Update передали широкий массивСравнить ключи с контрактомПередавать только нужное поле
\n

Учебный пример смешанного обработчика

\n

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

\n
<?php\nif ($_SERVER['REQUEST_METHOD'] === 'POST') {\n    $elementId = (int) $_POST['ID'];\n    $name = trim((string) $_POST['NAME']);\n\n    if ($elementId <= 0 || $name === '') {\n        echo 'Заполните ID и название.';\n        return;\n    }\n\n    CModule::IncludeModule('iblock');\n    $element = new CIBlockElement();\n    if (!$element->Update($elementId, array('NAME' => $name))) {\n        echo $element->LAST_ERROR;\n        return;\n    }\n\n    sendPartnerNotice($elementId, $name);\n    echo 'Сохранено';\n}
\n

У примера три отрицательных пути. Валидация останавливает запрос до записи. Bitrix может вернуть отказ. Внешний вызов может завершиться ошибкой после успешной записи. Последний путь нельзя исправить строкой «повторить Update»: она не делает внешнюю операцию безопасной и скрывает, что поле уже изменилось.

\n

Первый шов: функция возвращает только результат записи

\n

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

\n
<?php\nfunction saveProductName($elementId, $name)\n{\n    if (!CModule::IncludeModule('iblock')) {\n        throw new RuntimeException('Module iblock is unavailable');\n    }\n\n    $elementId = (int) $elementId;\n    $name = trim((string) $name);\n    if ($elementId <= 0 || $name === '') {\n        throw new InvalidArgumentException('ID and NAME are required');\n    }\n\n    $element = new CIBlockElement();\n    if (!$element->Update($elementId, array('NAME' => $name))) {\n        throw new RuntimeException($element->LAST_ERROR ?: 'Element update failed');\n    }\n\n    return $elementId;\n}\n\ntry {\n    $savedId = saveProductName($_POST['ID'], $_POST['NAME']);\n    $message = 'Карточка сохранена: ' . $savedId;\n} catch (InvalidArgumentException $error) {\n    $message = $error->getMessage();\n} catch (RuntimeException $error) {\n    $message = $error->getMessage();\n}
\n

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

\n

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

\n

Обработчики и набор полей

\n

OnBeforeIBlockElementUpdate получает параметры до изменения и может их переопределить или отменить операцию. OnAfterIBlockElementUpdate вызывается после попытки обновления даже при неудаче; в его массиве доступны RESULT и, при ошибке, RESULT_MESSAGE. Поэтому в нём нужно смотреть результат, а не только факт вызова.

\n

Эти события объясняют расхождение между переданным массивом и состоянием элемента. Передавайте в Update только поля текущей задачи. Широкий массив повышает цену ошибки: обработчик видит лишнее, а свойства могут получить нежелательное значение. Узкий массив не гарантирует атомарность, но сокращает поверхность изменения.

\n

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

\n

Порядок локальной переделки

\n
  1. Запишите один симптом: например, ошибка формы не отвечает, изменилось ли имя.
  2. Перечислите побочные эффекты старого обработчика в фактическом порядке: запись, уведомление, кеш, HTML и внешние вызовы.
  3. Выберите один эффект для первого шва и зафиксируйте ID, инфоблок, поле, успех и отказ.
  4. Проверьте зарегистрированные обработчики Bitrix и все места вызова участка.
  5. Вынесите запись в функцию с явными аргументами. Уберите из неё $_POST, echo и сторонние вызовы.
  6. Подключите функцию одним вызовом, оставив неисследованные действия в прежнем порядке.
  7. На разрешённом тестовом элементе сохраните исходное значение, выполните один вызов и прочитайте поле обратно.
  8. Проверьте пустой вход, неверный ID, отказ обработчика и ошибку позднего эффекта.
  9. Если значение расходится с ожидаемым, остановите расширение. Не добавляйте повторную запись наугад.
\n

Отрицательный путь важнее зелёного сообщения

\n

Нужно различать четыре состояния: вход отклонён, запись отклонена, запись подтверждена, поздний эффект завершился отдельно. Первые два не должны менять элемент. Третье подтверждается повторным чтением. Четвёртое сообщает о частичном результате и не заставляет пользователя повторять весь запрос.

\n

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

\n

Ограничения метода

\n

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

\n

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

\n

Критерий готовности

\n

Локальный рефакторинг готов, когда для одного выбранного элемента выполнены все условия: вход проходит отдельную проверку; функция меняет только заявленное поле; результат Update обработан явно; обработчики просмотрены; фактическое значение подтверждено повторной выборкой; поздний эффект имеет отдельный результат; отрицательный путь не запускает повторную запись; при расхождении есть действие остановки или возврата.

\n

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

\n

Проверяемые источники

" }