Always use multicast and send events for each failed send report

[ahawlitschek]

Regarding to #138, @Ben-Ro and I changed the sending to always use multicast and fire a single event for each failure send report.

Currently we would provide the single token that is associated with the failure. The previous version used the array key "token", but provided an array of tokens (Since it only throew the exception on single sending, the array always holds only 1 token - kind of missleading here).

Our change in this change would we a breaking change. Therefore, we could just wrap it into an array, but wanted to start a discussion before just wrapping it.

Looking forwarding talking about the PR Adrian & Ben

[dwightwatson]

Thanks for taking the time to put this together. I think this is a great improvement even if it is a breaking change.

I'll put this to you - should we ditch the automatic chunking and instead throw an error if you're trying to hit 500 tokens at once? I'd assume 99% of people using this package aren't sending to 500+ tokens at once, and it would let us greatly simplify the notification handling here.

[ahawlitschek]

Thanks for taking the time to put this together. I think this is a great improvement even if it is a breaking change.

I'll put this to you - should we ditch the automatic chunking and instead throw an error if you're trying to hit 500 tokens at once? I'd assume 99% of people using this package aren't sending to 500+ tokens at once, and it would let us greatly simplify the notification handling here.

Always happy to help.

I think using always multicast is the right way. Since usage of devices might increase over application lifetime, a sudden exception prompting the user to perform splitting by himself might cause whole application outages.

If a breaking release is no problem, I would also consider changing the event data. Currently there is the following data:

That leed to Listeners like this:

Since we will ditch the Exception, I would propose including the SendReport instead of the extracted entries in the data array. This would preserve the proper typing and also the helper methods that are already defined on SendReport:

Would be nice to even have the SendReport as property of the event class, but since Laravel does not listen on parent classes, previous Listener implementations of package users won't work. I am currently thinking what's worth more. Completely typed Event vs. only one single Listener Registration. But having it inside the data array would still be a big win in my opinion.

If you are fine with this, I would adjust the PR accordingly

[dwightwatson]

I wonder also if we can simply this all using a collection, something like this...

collect($tokens)
  ->chunk(self::MAX_TOKENS_PER_REQUEST)
  ->map(fn ($tokens) => $this->send($message, $tokens))
  ->map(function (MulticastSendReport $report) {
    collect($report->getItems())
      ->filter(fn (SendReport $report) => $report->isFailure())
      ->each(function (SendReport $report) {
        $this->failedNotification(...)
      })
  })

[dwightwatson]

Thanks again for taking the lead on this.

I've merged this in to master for anyone that would like to use it now. I've opened up #168 where I'm iterating on it and working towards a new major release.