Major code cleanup, formatting fixes and upgrades like typed properti…

…es and params This also adjusts the test according to the latest changes.
laravel-notification-channels · Mar 11, 2024 · 0698f1a · 0698f1a
1 parent bee31a7
commit 0698f1a
Show file tree

Hide file tree

Showing 18 changed files with 233 additions and 204 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -13,7 +13,7 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        php: [8.2, 8.1, 8.0]
+        php: [8.3, 8.2, 8.1]
         laravel: ['8.*', '9.*', '10.*', '11.*']
         include:
           - laravel: 10.*
@@ -25,16 +25,8 @@ jobs:
           - laravel: 11.*
             testbench: 9.*
         exclude:
-          - laravel: 10.*
-            php: 8.0
-          - laravel: 10.*
-            php: 7.4
-          - laravel: 9.*
-            php: 7.4
           - laravel: 11.*
             php: 8.1
-          - laravel: 11.*
-            php: 8.0
 
     steps:
       - name: Checkout code

diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,5 @@
 /vendor
 build
+.phpunit.result.cache
 composer.phar
 composer.lock
diff --git a/composer.json b/composer.json
@@ -12,7 +12,7 @@
         }
     ],
     "require": {
-        "php": "^8.0",
+        "php": "^8.1",
         "guzzlehttp/guzzle": "^7.0.1",
         "illuminate/notifications": "^8.0 || ^9.0 || ^10.0 || ^11.0",
         "illuminate/support": "^8.0 || ^9.0 || ^10.0 || ^11.0"

diff --git a/src/Exceptions/CouldNotSendNotification.php b/src/Exceptions/CouldNotSendNotification.php
@@ -3,15 +3,16 @@
 namespace NotificationChannels\Pushover\Exceptions;
 
 use Exception;
+use Illuminate\Notifications\AnonymousNotifiable;
 use Psr\Http\Message\ResponseInterface;
 
 class CouldNotSendNotification extends Exception
 {
-    public static function serviceRespondedWithAnError(ResponseInterface $response, $notifiable)
+    public static function serviceRespondedWithAnError(ResponseInterface $response, $notifiable): static
     {
         $statusCode = $response->getStatusCode();
 
-        $result = json_decode($response->getBody());
+        $result = json_decode($response->getBody()->getContents());
 
         $exceptionMessage = sprintf(
             "Pushover responded with an error (%s) for notifiable '%s' with id '%s'",
@@ -30,8 +31,12 @@ public static function serviceRespondedWithAnError(ResponseInterface $response,
         return new static($exceptionMessage, $statusCode);
     }
 
-    public static function pushoverKeyHasWrongLength($notifiable)
+    public static function pushoverKeyHasWrongLength($notifiable): static
     {
+        if ($notifiable instanceof AnonymousNotifiable) {
+            return new static('Pushover key has wrong length. It needs to be 30 characters long.');
+        }
+
         $exceptionMessage = sprintf(
             "Pushover key has wrong length for notifiable '%s' with id '%s'. It needs to be 30 characters long.",
             get_class($notifiable),

diff --git a/src/Exceptions/ServiceCommunicationError.php b/src/Exceptions/ServiceCommunicationError.php
@@ -6,7 +6,7 @@
 
 class ServiceCommunicationError extends Exception
 {
-    public static function communicationFailed(Exception $exception)
+    public static function communicationFailed(Exception $exception): static
     {
         return new static("The communication with Pushover failed because `{$exception->getCode()} - {$exception->getMessage()}`");
     }

diff --git a/src/Pushover.php b/src/Pushover.php
@@ -4,9 +4,11 @@
 
 use Exception;
 use GuzzleHttp\Client as HttpClient;
+use GuzzleHttp\Exception\GuzzleException;
 use GuzzleHttp\Exception\RequestException;
 use NotificationChannels\Pushover\Exceptions\CouldNotSendNotification;
 use NotificationChannels\Pushover\Exceptions\ServiceCommunicationError;
+use Psr\Http\Message\ResponseInterface;
 
 class Pushover
 {
@@ -22,27 +24,27 @@ class Pushover
      *
      * @var string
      */
-    protected $pushoverApiUrl = 'https://api.pushover.net/1/messages.json';
+    protected string $pushoverApiUrl = 'https://api.pushover.net/1/messages.json';
 
     /**
      * The HTTP client instance.
      *
-     * @var \GuzzleHttp\Client
+     * @var HttpClient
      */
-    protected $http;
+    protected HttpClient $http;
 
     /**
      * Pushover App Token.
      *
      * @var string
      */
-    protected $token;
+    protected string $token;
 
     /**
-     * @param  HttpClient  $http
-     * @param  string  $token
+     * @param HttpClient $http
+     * @param string     $token
      */
-    public function __construct(HttpClient $http, $token)
+    public function __construct(HttpClient $http, string $token)
     {
         $this->http = $http;
 
@@ -54,21 +56,23 @@ public function __construct(HttpClient $http, $token)
      *
      * @link  https://pushover.net/api
      *
-     * @param  array  $params
+     * @param array $params
      * @param mixed $notifiable
-     * @return \Psr\Http\Message\ResponseInterface
+     * @return ResponseInterface
      *
      * @throws CouldNotSendNotification
+     * @throws ServiceCommunicationError
+     * @throws GuzzleException
      */
-    public function send($params, $notifiable)
+    public function send(array $params, mixed $notifiable): ResponseInterface
     {
         try {
             $multipart = [];
 
             foreach ($this->paramsWithToken($params) as $name => $contents) {
                 if ($name !== 'image') {
                     $multipart[] = [
-                        'name'     => $name,
+                        'name' => $name,
                         'contents' => $contents,
                     ];
                 } else {
@@ -80,6 +84,7 @@ public function send($params, $notifiable)
                 }
             }
 
+            //dd($multipart);
             return $this->http->post(
                 $this->pushoverApiUrl,
                 [
@@ -101,10 +106,10 @@ public function send($params, $notifiable)
     /**
      * Merge token into parameters array, unless it has been set on the PushoverReceiver.
      *
-     * @param  array  $params
+     * @param array $params
      * @return array
      */
-    protected function paramsWithToken($params)
+    protected function paramsWithToken(array $params): array
     {
         return array_merge([
             'token' => $this->token,
@@ -119,9 +124,14 @@ protected function paramsWithToken($params)
      *
      * @param $file
      * @return array|null
+     * @throws GuzzleException
      */
     private function getImageData($file): ?array
     {
+        if (empty($file)) {
+            return null;
+        }
+
         try {
             // check if $file is not too big
             if (is_file($file) && is_readable($file)) {
@@ -141,7 +151,7 @@ private function getImageData($file): ?array
                 }
 
                 // some servers may not return the "Content-Length" header
-                $fileSizeChecked = (bool) $contentLength;
+                $fileSizeChecked = (bool)$contentLength;
             }
 
             // check if $file is an image
@@ -153,7 +163,7 @@ private function getImageData($file): ?array
 
             $contents = file_get_contents($file);
             // if not checked before, finally check the file size after reading it
-            if (! $fileSizeChecked && strlen($contents) > self::IMAGE_SIZE_LIMIT) {
+            if (!$fileSizeChecked && strlen($contents) > self::IMAGE_SIZE_LIMIT) {
                 return null;
             }
         } catch (Exception $exception) {
@@ -162,10 +172,10 @@ private function getImageData($file): ?array
 
         return [
             // name of the field holding the image must be 'attachment' (https://pushover.net/api#attachments)
-            'name'     => 'attachment',
+            'name' => 'attachment',
             'contents' => $contents,
             'filename' => basename($file),
-            'headers'  => [
+            'headers' => [
                 'Content-Type' => $contentType,
             ],
         ];

diff --git a/src/PushoverChannel.php b/src/PushoverChannel.php
@@ -2,6 +2,7 @@
 
 namespace NotificationChannels\Pushover;
 
+use GuzzleHttp\Exception\GuzzleException;
 use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Notifications\Events\NotificationFailed;
 use Illuminate\Notifications\Notification;
@@ -10,16 +11,15 @@
 
 class PushoverChannel
 {
-    /** @var Pushover */
-    protected $pushover;
+    protected Pushover $pushover;
 
-    /** @var Dispatcher */
-    protected $events;
+    protected Dispatcher $events;
 
     /**
      * Create a new Pushover channel instance.
      *
-     * @param  Pushover  $pushover
+     * @param Pushover   $pushover
+     * @param Dispatcher $events
      */
     public function __construct(Pushover $pushover, Dispatcher $events)
     {
@@ -30,12 +30,13 @@ public function __construct(Pushover $pushover, Dispatcher $events)
     /**
      * Send the given notification.
      *
-     * @param  mixed  $notifiable
-     * @param  \Illuminate\Notifications\Notification  $notification
+     * @param mixed        $notifiable
+     * @param Notification $notification
      *
-     * @throws \NotificationChannels\Pushover\Exceptions\CouldNotSendNotification
+     * @throws CouldNotSendNotification
+     * @throws GuzzleException
      */
-    public function send($notifiable, Notification $notification)
+    public function send(mixed $notifiable, Notification $notification): void
     {
         if (! $pushoverReceiver = $notifiable->routeNotificationFor('pushover')) {
             return;
@@ -63,7 +64,7 @@ public function send($notifiable, Notification $notification)
         }
     }
 
-    protected function fireFailedEvent($notifiable, $notification, $message)
+    protected function fireFailedEvent($notifiable, $notification, $message): void
     {
         $this->events->dispatch(
             new NotificationFailed($notifiable, $notification, 'pushover', [$message])