Skip to content

Event mutation silently dropped: unified audit of invokeEvent callers that expect by-ref mutation #219

@biz87

Description

@biz87

TL;DR

В пяти местах ядра плагин, подписанный на событие, молча не может изменить данные, которые должен был изменить. Ядро вызывает `$modx->invokeEvent(...)` напрямую (минуя `Utils::invokeEvent()`) и даже передаёт параметры по ссылке — но мутация из плагина не распространяется в scope вызова. Все эти события задокументированы / предполагают мутацию, но архитектурно нет способа её донести.

Конкретный случай `msOnBeforeSendNotification` разобран в #218 по логам с продакшна. Здесь — полный список аналогичных мест.

Фон

Единственный корректный путь передать мутацию из плагина обратно в ядро — `Utils::invokeEvent()` (`src/Utils/Utils.php`). Он читает `$modx->event->returnedValues` и мержит их обратно в `$params`. Большинство событий MiniShop3 ходят через эту обёртку и работают правильно (cart, order fields, cost, customer — 48 мест).

Но 5 мест используют прямой `$this->modx->invokeEvent(...)` с `&`-параметрами в ожидании, что by-ref донесёт мутацию. MODX внутри `invokeEvent()` делает `extract()` в scope плагина — by-ref в массиве params не действует на этапе extract, и ничего обратно не попадает. `returnedValues` здесь тоже не читается.

Реальные места (5)

# Файл Строка Событие Параметры с `&` Что должен мутировать плагин
1 `src/Notifications/NotificationManager.php` 72 `msOnBeforeSendNotification` `recipient`, `channels` подменить получателя / список каналов (см. #218)
2 `src/Utils/ImportCSV.php` 124 `msOnBeforeImport` `params` изменить параметры импорта до старта
3 `src/Utils/ImportCSV.php` 347 `msOnImportRow` `data`, `tvData`, `optionData`, `gallery` обогатить / нормализовать строку перед сохранением
4 `elements/snippets/ms3_products.php` 370 `msOnProductsLoad` `rows` batch-preload (например, загрузить опции товаров одним запросом и расширить $rows)
5 `elements/snippets/ms3_products.php` 442 `msOnProductPrepare` `row` обогатить конкретный row перед рендером чанка

Все пять — legitimate extensibility points (так документация и семантика названий обещает), но фактически плагинная мутация теряется.

Предложение

Во всех пяти местах:

  1. Убрать декоративное `&` в params — оно только вводит в заблуждение.
  2. После `$this->modx->invokeEvent(...)` читать `$this->modx->event->returnedValues` и мержить в соответствующие локальные переменные.
  3. Где уместно (cancel notification / abort import) — сохранить работу через `$modx->event->output === false` как есть.

Пример патча (NotificationManager):

```php
$this->modx->invokeEvent('msOnBeforeSendNotification', [
'notification' => $notification,
'recipient' => $recipient,
'recipientType' => $recipientType,
'channels' => $channels,
]);

if (isset($this->modx->event->returnedValues) && is_array($this->modx->event->returnedValues)) {
$out = $this->modx->event->returnedValues;
if (isset($out['recipient']) && is_array($out['recipient'])) {
$recipient = array_merge($recipient, $out['recipient']);
}
if (isset($out['channels']) && is_array($out['channels'])) {
$channels = $out['channels'];
}
}
```

Плагин пишет:

```php
$modx->event->returnedValues = ['recipient' => ['email' => $newEmail]];
```

Альтернатива — перевести все 5 мест на `Utils::invokeEvent()` и единый формат. Но у неё есть нюансы (она возвращает ответ в формате `{success, message, data}`, обрывает на первом `success: false` — не всегда желаемо; например для `msOnImportRow` «фейл одной строки» не должен ронять импорт). Поэтому точечный патч с `returnedValues` более консервативен.

Обратная совместимость

Плагины, которые сейчас пытаются мутировать by-ref — и так не работают (см. #218 как доказательство). Патч их не ломает, а начинает работать в ожидаемом виде, если плагин научится использовать `returnedValues`. Для совместимости можно параллельно читать и мутацию через `&` (если плагин вдруг вызывается в scope где by-ref ещё имеет смысл) — но на практике `extract()` это обнуляет, так что безопасно не заморачиваться.

Связанные

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions