Merge pull request #25 from YieldStudio/bugfix/avoid-sending-more-tha…

…n-push-notif-limit-no-batched-notifications bugfix: adds tests and fix bug sending more than 100 not batched notifications
YieldStudio · Feb 16, 2024 · b642b60 · b642b60
2 parents 0dff9af + ec45977
commit b642b60
Show file tree

Hide file tree

Showing 12 changed files with 471 additions and 58 deletions.
diff --git a/composer.json b/composer.json
@@ -28,11 +28,13 @@
   "require": {
     "php": "^8.1",
     "illuminate/database": "^9|^10",
-    "illuminate/support": "^9|^10"
+    "illuminate/support": "^9|^10",
+    "nesbot/carbon": ">=2.62.1"
   },
   "require-dev": {
     "ciareis/bypass": "^1.0",
     "dg/bypass-finals": "^1.4",
+    "guzzlehttp/guzzle": "^7.8",
     "laravel/pint": "^1.3",
     "orchestra/testbench": "7.*|8.*",
     "pestphp/pest": "^1.21",

diff --git a/phpunit.xml b/phpunit.xml
@@ -19,4 +19,9 @@
             <directory>src/</directory>
         </whitelist>
     </filter>
+    <php>
+        <env name="APP_KEY" value="AckfSECXIvnK5r28GVIWUAxmbBSjTsmF"/>
+        <env name="DB_CONNECTION" value="testing"/>
+        <env name="ENABLE_HTTPS_SUPPORT" value="false"/>
+    </php>
 </phpunit>
diff --git a/src/Contracts/ExpoNotificationsServiceInterface.php b/src/Contracts/ExpoNotificationsServiceInterface.php
@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace YieldStudio\LaravelExpoNotifier\Contracts;
+
+use Illuminate\Support\Collection;
+use YieldStudio\LaravelExpoNotifier\Dto\ExpoMessage;
+
+interface ExpoNotificationsServiceInterface
+{
+    public function __construct(
+        string $apiUrl,
+        string $host,
+        ExpoPendingNotificationStorageInterface $notificationStorage,
+        ExpoTicketStorageInterface $ticketStorage
+    );
+
+    public function notify(ExpoMessage|Collection|array $expoMessages): Collection;
+
+    public function receipts(Collection|array $tokenIds): Collection;
+
+    public function getNotificationChunks(): Collection;
+}
diff --git a/src/ExpoNotificationsChannel.php b/src/ExpoNotificationsChannel.php
@@ -5,13 +5,14 @@
 namespace YieldStudio\LaravelExpoNotifier;
 
 use Illuminate\Notifications\Notification;
+use YieldStudio\LaravelExpoNotifier\Contracts\ExpoNotificationsServiceInterface;
 use YieldStudio\LaravelExpoNotifier\Dto\ExpoMessage;
 use YieldStudio\LaravelExpoNotifier\Exceptions\ExpoNotificationsException;
 
 final class ExpoNotificationsChannel
 {
     public function __construct(
-        protected readonly ExpoNotificationsService $expoNotificationsService,
+        protected readonly ExpoNotificationsServiceInterface $expoNotificationsService,
     ) {
     }
 

diff --git a/src/ExpoNotificationsService.php b/src/ExpoNotificationsService.php
@@ -5,9 +5,11 @@
 namespace YieldStudio\LaravelExpoNotifier;
 
 use Illuminate\Http\Client\PendingRequest;
+use Illuminate\Http\Client\Response;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Http;
+use YieldStudio\LaravelExpoNotifier\Contracts\ExpoNotificationsServiceInterface;
 use YieldStudio\LaravelExpoNotifier\Contracts\ExpoPendingNotificationStorageInterface;
 use YieldStudio\LaravelExpoNotifier\Contracts\ExpoTicketStorageInterface;
 use YieldStudio\LaravelExpoNotifier\Dto\ExpoMessage;
@@ -17,10 +19,22 @@
 use YieldStudio\LaravelExpoNotifier\Events\InvalidExpoToken;
 use YieldStudio\LaravelExpoNotifier\Exceptions\ExpoNotificationsException;
 
-final class ExpoNotificationsService
+final class ExpoNotificationsService implements ExpoNotificationsServiceInterface
 {
+    public const PUSH_NOTIFICATIONS_PER_REQUEST_LIMIT = 100;
+
+    public const SEND_NOTIFICATION_ENDPOINT = '/send';
+
     private PendingRequest $http;
 
+    private ?Collection $expoMessages;
+
+    private ?Collection $notificationsToSend;
+
+    private ?Collection $notificationChunks;
+
+    private Collection $tickets;
+
     public function __construct(
         string $apiUrl,
         string $host,
@@ -33,62 +47,22 @@ public function __construct(
             'accept-encoding' => 'gzip, deflate',
             'content-type' => 'application/json',
         ])->baseUrl($apiUrl);
+
+        $this->tickets = collect();
     }
 
     /**
      * @param  ExpoMessage|ExpoMessage[]|Collection<int, ExpoMessage>  $expoMessages
      * @return Collection<int, PushTicketResponse>
-     *
-     * @throws ExpoNotificationsException
      */
     public function notify(ExpoMessage|Collection|array $expoMessages): Collection
     {
         /** @var Collection<int, ExpoMessage> $expoMessages */
-        $expoMessages = $expoMessages instanceof Collection ? $expoMessages : collect(Arr::wrap($expoMessages));
-
-        $shouldBatchFilter = fn (ExpoMessage $message) => $message->shouldBatch;
-
-        // Store notifications to send in the next batch
-        $expoMessages
-            ->filter($shouldBatchFilter)
-            ->each(fn (ExpoMessage $message) => $this->notificationStorage->store($message));
-
-        // Filter notifications to send now
-        $toSend = $expoMessages
-            ->reject($shouldBatchFilter)
-            ->map(fn (ExpoMessage $message) => $message->toExpoData())
-            ->values();
-
-        if ($toSend->isEmpty()) {
-            return collect();
-        }
-
-        $response = $this->http->post('/send', $toSend->toArray());
-        if (! $response->successful()) {
-            throw new ExpoNotificationsException($response->toPsrResponse());
-        }
-
-        $data = json_decode($response->body(), true);
-        if (! empty($data['errors'])) {
-            throw new ExpoNotificationsException($response->toPsrResponse());
-        }
-
-        $tickets = collect($data['data'])->map(function ($responseItem) {
-            if ($responseItem['status'] === ExpoResponseStatus::ERROR->value) {
-                return (new PushTicketResponse())
-                    ->status($responseItem['status'])
-                    ->message($responseItem['message'])
-                    ->details($responseItem['details']);
-            }
-
-            return (new PushTicketResponse())
-                ->status($responseItem['status'])
-                ->ticketId($responseItem['id']);
-        });
+        $this->expoMessages = $expoMessages instanceof Collection ? $expoMessages : collect(Arr::wrap($expoMessages));
 
-        $this->checkAndStoreTickets($toSend->pluck('to')->flatten(), $tickets);
-
-        return $tickets;
+        return $this->storeNotificationsToSendInTheNextBatch()
+            ->prepareNotificationsToSendNow()
+            ->sendNotifications();
     }
 
     /**
@@ -130,13 +104,17 @@ public function receipts(Collection|array $tokenIds): Collection
         });
     }
 
+    public function getNotificationChunks(): Collection
+    {
+        return $this->notificationChunks ?? collect();
+    }
+
     /**
      * @param  Collection<int, string>  $tokens
-     * @param  Collection<int, PushTicketResponse>  $tickets
      */
-    private function checkAndStoreTickets(Collection $tokens, Collection $tickets): void
+    private function checkAndStoreTickets(Collection $tokens): void
     {
-        $tickets
+        $this->tickets
             ->intersectByKeys($tokens)
             ->each(function (PushTicketResponse $ticket, $index) use ($tokens) {
                 if ($ticket->status === ExpoResponseStatus::ERROR->value) {
@@ -152,4 +130,86 @@ private function checkAndStoreTickets(Collection $tokens, Collection $tickets):
                 }
             });
     }
+
+    private function storeNotificationsToSendInTheNextBatch(): ExpoNotificationsService
+    {
+        $this->expoMessages
+            ->filter(fn (ExpoMessage $message) => $message->shouldBatch)
+            ->each(fn (ExpoMessage $message) => $this->notificationStorage->store($message));
+
+        return $this;
+    }
+
+    private function prepareNotificationsToSendNow(): ExpoNotificationsService
+    {
+        $this->notificationsToSend = $this->expoMessages
+            ->reject(fn (ExpoMessage $message) => $message->shouldBatch)
+            ->map(fn (ExpoMessage $message) => $message->toExpoData())
+            ->values();
+
+        // Splits into multiples chunks of max limitation
+        $this->notificationChunks = $this->notificationsToSend->chunk(self::PUSH_NOTIFICATIONS_PER_REQUEST_LIMIT);
+
+        return $this;
+    }
+
+    private function sendNotifications(): Collection
+    {
+        if ($this->notificationsToSend->isEmpty()) {
+            return collect();
+        }
+
+        $this->notificationChunks
+            ->each(
+                fn ($chunk, $index) => $this->sendNotificationsChunk($chunk->toArray())
+            );
+
+        $this->checkAndStoreTickets($this->notificationsToSend->pluck('to')->flatten());
+
+        return $this->tickets;
+    }
+
+    private function handleSendNotificationsResponse(Response $response): void
+    {
+        $data = json_decode($response->body(), true, 512, JSON_THROW_ON_ERROR);
+        if (! empty($data['errors'])) {
+            throw new ExpoNotificationsException($response->toPsrResponse());
+        }
+
+        $this->setTicketsFromData($data);
+    }
+
+    private function setTicketsFromData(array $data): ExpoNotificationsService
+    {
+        collect($data['data'])
+            ->each(function ($responseItem) {
+                if ($responseItem['status'] === ExpoResponseStatus::ERROR->value) {
+                    $this->tickets->push(
+                        (new PushTicketResponse())
+                            ->status($responseItem['status'])
+                            ->message($responseItem['message'])
+                            ->details($responseItem['details'])
+                    );
+                } else {
+                    $this->tickets->push(
+                        (new PushTicketResponse())
+                            ->status($responseItem['status'])
+                            ->ticketId($responseItem['id'])
+                    );
+                }
+            });
+
+        return $this;
+    }
+
+    private function sendNotificationsChunk(array $chunk)
+    {
+        $response = $this->http->post(self::SEND_NOTIFICATION_ENDPOINT, $chunk);
+
+        if (! $response->successful()) {
+            throw new ExpoNotificationsException($response->toPsrResponse());
+        }
+
+        $this->handleSendNotificationsResponse($response);
+    }
 }
diff --git a/src/ExpoNotificationsServiceProvider.php b/src/ExpoNotificationsServiceProvider.php
@@ -8,6 +8,7 @@
 use Illuminate\Support\ServiceProvider;
 use YieldStudio\LaravelExpoNotifier\Commands\CheckTickets;
 use YieldStudio\LaravelExpoNotifier\Commands\SendPendingNotifications;
+use YieldStudio\LaravelExpoNotifier\Contracts\ExpoNotificationsServiceInterface;
 use YieldStudio\LaravelExpoNotifier\Contracts\ExpoPendingNotificationStorageInterface;
 use YieldStudio\LaravelExpoNotifier\Contracts\ExpoTicketStorageInterface;
 use YieldStudio\LaravelExpoNotifier\Contracts\ExpoTokenStorageInterface;
@@ -22,7 +23,7 @@ public function register(): void
         $this->app->bind(ExpoTicketStorageInterface::class, config('expo-notifications.drivers.ticket'));
         $this->app->bind(ExpoPendingNotificationStorageInterface::class, config('expo-notifications.drivers.notification'));
 
-        $this->app->bind(ExpoNotificationsService::class, function ($app) {
+        $this->app->bind(ExpoNotificationsServiceInterface::class, function ($app) {
             $apiUrl = config('expo-notifications.service.api_url');
             $host = config('expo-notifications.service.host');