Rencom16 мар 2016 в 05:52

Работа с событиями в Laravel. Рассылка push уведомлений при публикации статьи

10 мин

21K

Laravel *

Туториал

Комментарии 13

difiso 16 мар 2016 в 06:36

Недостатки решаются обработкой событий в асинхронном режиме.

Для этого надо чтобы Listener реализовывал пустой интерфейс ShouldQueue. В этом случае все события обрабатываются очередью в фоне (ну да, надо запустить php artisan queue:listen или php artisan queue:work, в доках по очередям это есть).

Метод Post::saving выполняется ДО сохранения модели в базу. В этом методе вы не можете быть уверены в том, что модель сохранится в базу. Отправлять уведомления надо только в том случае, когда пост сохранился, т.е. использовать метод Post::saved

И, соответственно, очень опрометчиво менять $notify_status до отправки уведомлений. В чем проблема менять, когда точно известно, что уведомление ушло — после получения ответа от сервера?

Rencom 16 мар 2016 в 06:41

Если Вы не заметили, я все это указал конце, очереди буду рассматривать отдельно. Вместе с расширением функционала (не только push уведомления планируются)
Уровень статьи — для тех, кто делает первые (или вторые) шаги. Много информации сразу сбивает с толку.

difiso 16 мар 2016 в 06:46

Про очереди не заметил, виноват. Но логику-то зачем воротить? Сначала пометить сделанным, а потом надеяться что внешний сервер ответит успехом?

Rencom 16 мар 2016 в 06:50

Чтобы был лишний повод для рефакторинга. Показать, как делают обычно :)) и как это исправить

Sway 16 мар 2016 в 12:23

Вот не стоит так делать в этом случае. Одно дело — когда в одной статье рассматривается и проблема и ее решение, а совсем другое, когда в 1й статье умышленно делается ошибки/недочет, которые исправляются в другой статье. К моменту выхода 2й статьи уже будет поздно что-либо исправлять, а многие ее и не прочитают, особенно если найдут такие ляпы.

Rencom 16 мар 2016 в 12:32

В статье не просто молча допущен недочет (не смертельный кстати).
В ней отдельным параграфом это указано. Лишний повод продумать решение самому, как это сделал difiso выше

Sway 16 мар 2016 в 13:41

На мой взгляд от этого легче не становится. Ошибки есть, они не исправлены. Заставлять читателя думать там, где это необязательно, не всегда уместно. Я вот зашел почитать как другой человек работает с событиями и для чего их использует.
Кстати, не вижу особого смысла использовать для этой задачи события. Этим вы только усложняете понимание происходящего т.к. вызов события никак не передает суть того, что оно будет делать. К тому же это событие всегда вызывается из одного и того же места и имеет один обработчик (даже если их будет и несколько, то все равно смысла мало).
Что мешает сделать вот так?

protected  function onBeforeSave()
{
    //Мы проверяем статус статьи – если он «Опубликован», смотрим на статус оповещения, если он еще не «Опубликован»
    if ($this->status == self::STATUS_PUBLISHED 
        && $this->notify_status < self::STATUS_PUBLISHED){

        //то устанавливаемый статус оповещения в «опубикован»
        $this->notify_status = self::STATUS_PUBLISHED;

        //и  «выстреливаем» событие PostPublishedEvent, передавая в него собственный инстанс.
        (new OneSignalHandler())->sendNotify($event->post);

    }
}

И при необходимости добавлять сюда действия другого типа одной строчкой.
Ведь подумайте — onBeforeSave() — это по сути обработчик события (префикс "on" на это толсто намекает). А вы пихаете вызов другого события внутрь обработчика события. Жуть просто.

А теперь давайте рассмотрим почему мой вариант лучше на примере того как человек читает код.

Ваш вариант глазами другого человека, который знает Laravel:

Заходим в модель, видим onBeforeSave
Смотрим что метод делает:
2.1. Проверка статусов поста и нотификации (А предполагается ли такая ситуация что пост был изменен после публикации и рассылки?)
2.2. Устанавливаем новый статус нотификации (стоп! какого? мы ж ничего не сделали!)
2.3. Отправляем событие PostPublishedEvent
Так. А что делает это событие и где?
3.1. Открываем класс события (ну как обычно — ctrl+click, привыкли уже). Хм… В нем нет никакой инфы о его деятельности.
3.2. Ааа! Точно! У него ж должен быть listener. Таак и где среди этой сотни обработчиков нужный нам (я это к тому что с таким подходом в более-менее большом проекте обработчиков будет навалом)?
3.3. Нашли! Ура! Итак, наконец-то я узнаю что он делает! (new OneSignalHandler())->sendNotify($event->post);
3.4. Что????? И это все???? И я перелопатил хренову тучу классов/файлов ради этого????
Я утрирую, дальше ведь там будут еще другие действия, но все-равно путь длинноват.
Так… А с чем я там вообще изначально разбирался и где? (потеря концентрации и времени)

Мой вариант:

Заходим в модель, видим onBeforeSave
Смотрим что метод делает:
2.1. Проверка статусов поста и нотификации (А предполагается ли такая ситуация что пост был изменен после публикации и рассылки?)
2.2. Устанавливаем новый статус нотификации (стоп! какого? мы ж ничего не сделали!)
2.3. Отправляем нотификацию через OneSignalHandler (опа! да тут же косяк! статус меняется независимо от того отправилась нотификация или, да еще и отправка происходит до сохранения данных, что вообще-то совершенно неправильно — вдруг не сохраниться?)
Всё понятно. Потенциальный баг найден и будет исправлен.

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

События — хорошая штука, но применять их нужно не бездумно везде где захочется — будет только хуже.
Я пока нашел всего 1 применение для очень специфического случая, где без них был бы полнейший кошмар. Больше нигде не применял — не нашлось места. Знаю только что они применяются в коде Laravel, и там это обосновано — не нужно делать 100500 методов onSomeAction(). События удобны для библиотек, когда неизвестно какие действия захочет выполнить использующий либу программист.

Еще:

Почему бы не сделать OneSignalHandler статическим классом? По сути он не хранит состояния. Его задаче — предоставить обертку для отправки пуша. Тестовую отправку можно сделать через аргумент метода sendNotify, хранить этот флаг не вижу смысла. Если же нужно глобально включать/выключать тестовую отправку — это можно организовать через поле static public $testMode или через setter для него.
Разберитесь с очередью (queue) — она для данной задачи необходима намного больше чем событие т.к. значительно увеличит скорость обработки процесса публикации поста (естественно, если будет выполняться в фоне)

Rencom 16 мар 2016 в 14:09

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

Метод OnBeforeSave тут правда лишний, мой косяк, тк в оригинале я наследуюсь от своего CrudModel, в котором находится boot(), получается что дернул из разных мест: получил в итоге масло масляное

В следующей статье расскажу про очереди. Я это изначально планировал.