Problem Statement

The SmsEvents::MESSAGE_QUEUE_POST_PROCESS event will still get dispatched after SmsEvents::MESSAGE_OUTGOING_POST_PROCESS and SmsEvents::MESSAGE_POST_PROCESS are dispatched if the gateway happens to have skip_queue TRUE.

Even though it is documented that skip_queue is not recommended in production, this is still a WTF for development and testing where the post process event handlers would behave funny in a test (for instance).

Proposed resolution

Filter out messages that have passed through a gateway with skip_queue on before passing them through the MESSAGE_QUEUE_POST_PROCESS event dispatch.

Comments

almaudoh created an issue. See original summary.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented

These events are not mutually exclusive.

All MESSAGE_QUEUE* events are guaranteed to run if you use ::queue. Likewise MESSAGE_*_*_PROCESS are guaranteed to run regardless of entry point (queue, incoming, outgoing). I think the \Drupal\Tests\sms\Kernel\SmsFrameworkProviderTest::testEvents* test outline it pretty well. Specifically ::testEventsQueueOutgoing and ::testEventsQueueOutgoingSkipQueue

SmsEvents::MESSAGE_QUEUE_POST_PROCESS should always be run in the original ::queue request regardless of skip_queue value.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Status: Active » Postponed (maintainer needs more info)

Not sure if this is a misunderstanding on either part.

almaudoh’s picture

almaudoh CreditAttribution: almaudoh commented

All MESSAGE_QUEUE* events are guaranteed to run if you use ::queue. Likewise MESSAGE_*_*_PROCESS are guaranteed to run regardless of entry point (queue, incoming, outgoing)

Ok. That makes sense. But then the order of the events change, which may be problematic for consuming code.

Event sequence

skip_queue: true

  • MESSAGE_PRE_PROCESS
  • MESSAGE_QUEUE_PRE_PROCESS
  • MESSAGE_OUTGOING_PRE_PROCESS or MESSAGE_INCOMING_PRE_PROCESS
  • MESSAGE_OUTGOING_POST_PROCESS or MESSAGE_INCOMING_POST_PROCESS
  • MESSAGE_POST_PROCESS
  • MESSAGE_QUEUE_POST_PROCESS***

skip_queue: false

  • MESSAGE_PRE_PROCESS
  • MESSAGE_QUEUE_PRE_PROCESS
  • MESSAGE_QUEUE_POST_PROCESS***
  • MESSAGE_OUTGOING_PRE_PROCESS or MESSAGE_INCOMING_PRE_PROCESS
  • MESSAGE_OUTGOING_POST_PROCESS or MESSAGE_INCOMING_POST_PROCESS
  • MESSAGE_POST_PROCESS
dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented

I think thats okay.

MESSAGE_QUEUE_POST_PROCESS is the only special case, i don't really expect many people to consume this event.

almaudoh’s picture

almaudoh CreditAttribution: almaudoh commented

MESSAGE_QUEUE_POST_PROCESS is the only special case, i don't really expect many people to consume this event.

Well, I was consuming it. That is why I wrote the bug report. And I needed the same behavior whether in test code (skip_queue == true) or in production (skip_queue == false).

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented

Most importantly the event is consistent. It is the last thing to execute after attempting to queue. Whether it actually was queued could be determined by:

// in a MESSAGE_QUEUE_POST_PROCESS subscriber:
$messages = $event->getMessages();
$message = $message[0]; //normally you would iterate over $messages. Test only code.
$was_queued = $message instanceof SmsMessageEntityInterface && !$message->isNew();

Does that work for you?