Merge pull request #63 from laravel-notification-channels/refactoring

Refactoring

Author: Kovah

.github/workflows/test.yml

@@ -1,50 +1,47 @@
 name: Tests
 
-on: [push, pull_request]
+on:
+  - push
+  - pull_request
 
 jobs:
-    test:
-        name: PHP ${{ matrix.php }} - Laravel ${{ matrix.laravel }}
-        runs-on: ubuntu-latest
-        strategy:
-            fail-fast: true
-            matrix:
-                php: [ 8.2, 8.1, 8.0 ]
-                laravel: [ 10.*, 9.*, 8.* ]
-                include:
-                    - laravel: 10.*
-                      testbench: 8.*
-                    - laravel: 9.*
-                      testbench: 7.*
-                    - laravel: 8.*
-                      testbench: 6.*
-                exclude:
-                    - laravel: 10.*
-                      php: 8.0
-                    - laravel: 10.*
-                      php: 7.4
-                    - laravel: 9.*
-                      php: 7.4
-
-        steps:
-            - name: Checkout code
-              uses: actions/checkout@v2
-
-            - name: Set correct PHP version
-              uses: shivammathur/setup-php@v2
-              with:
-                  php-version: ${{ matrix.php }}
-                  coverage: pcov
-
-            - name: Install dependencies
-              run: |
-                  composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update
-                  composer update --prefer-stable --prefer-dist --no-interaction --no-suggest
-
-            - name: Execute tests
-              run: vendor/bin/phpunit
-
-            - name: Upload coverage to Scrutinizer
-              run: |
-                  wget https://scrutinizer-ci.com/ocular.phar
-                  php ocular.phar code-coverage:upload --format=php-clover coverage.clover
+  test:
+    name: PHP ${{ matrix.php }} - Laravel ${{ matrix.laravel }}
+
+    runs-on: ubuntu-latest
+
+    strategy:
+      fail-fast: true
+      matrix:
+        php: [8.3, 8.2, 8.1]
+        laravel: ['8.*', '9.*', '10.*', '11.*']
+        include:
+          - laravel: 10.*
+            testbench: 8.*
+          - laravel: 9.*
+            testbench: 7.*
+          - laravel: 8.*
+            testbench: 6.*
+          - laravel: 11.*
+            testbench: 9.*
+        exclude:
+          - laravel: 11.*
+            php: 8.1
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v2
+
+      - name: Set correct PHP version
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php }}
+          coverage: pcov
+
+      - name: Install dependencies
+        run: |
+          composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update
+          composer update --prefer-stable --prefer-dist --no-interaction --no-suggest
+
+      - name: Execute tests
+        run: vendor/bin/phpunit

.gitignore

@@ -1,4 +1,5 @@
 /vendor
 build
+.phpunit.result.cache
 composer.phar
 composer.lock

composer.json

@@ -12,17 +12,20 @@
         }
     ],
     "require": {
-        "php": "^8.0",
+        "php": "^8.1",
         "guzzlehttp/guzzle": "^7.0.1",
-        "illuminate/notifications": "^8.0 || ^9.0 || ^10.0",
-        "illuminate/support": "^8.0 || ^9.0 || ^10.0"
+        "illuminate/notifications": "^8.0 || ^9.0 || ^10.0 || ^11.0",
+        "illuminate/support": "^8.0 || ^9.0 || ^10.0 || ^11.0"
     },
     "require-dev": {
         "mockery/mockery": "^1.3.1",
-        "phpunit/phpunit": "^9.3",
-        "orchestra/testbench": "^8.0",
+        "phpunit/phpunit": "^9.3 || ^10.5",
+        "orchestra/testbench": "^8.0 || ^9.0",
         "dms/phpunit-arraysubset-asserts": ">=0.1.0"
     },
+    "suggest": {
+        "ext-exif": "Required for image attachment support"
+    },
     "autoload": {
         "psr-4": {
             "NotificationChannels\\Pushover\\": "src"

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'",
@@ -29,4 +30,19 @@ public static function serviceRespondedWithAnError(ResponseInterface $response,
 
         return new static($exceptionMessage, $statusCode);
     }
+
+    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),
+            $notifiable->getKey()
+        );
+
+        return new static($exceptionMessage);
+    }
 }

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()}`");
     }

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
      */
-    public function __construct(HttpClient $http, $token)
+    public function __construct(HttpClient $http, string $token)
     {
         $this->http = $http;
 
@@ -55,19 +57,22 @@ public function __construct(HttpClient $http, $token)
      * @link  https://pushover.net/api
      *
      * @param  array  $params
-     * @return \Psr\Http\Message\ResponseInterface
+     * @param  mixed  $notifiable
+     * @return ResponseInterface
      *
      * @throws CouldNotSendNotification
+     * @throws ServiceCommunicationError
+     * @throws GuzzleException
      */
-    public function send($params)
+    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 {
@@ -79,6 +84,7 @@ public function send($params)
                 }
             }
 
+            //dd($multipart);
             return $this->http->post(
                 $this->pushoverApiUrl,
                 [
@@ -103,7 +109,7 @@ public function send($params)
      * @param  array  $params
      * @return array
      */
-    protected function paramsWithToken($params)
+    protected function paramsWithToken(array $params): array
     {
         return array_merge([
             'token' => $this->token,
@@ -116,11 +122,17 @@ protected function paramsWithToken($params)
      * If there is any error (problem with reading the file, file size exceeds the limit, the file is not an image),
      * silently returns null and sends the message without image attachment.
      *
-     * @param $file
+     * @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)) {
@@ -161,10 +173,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,
             ],
         ];

src/PushoverChannel.php

@@ -2,23 +2,24 @@
 
 namespace NotificationChannels\Pushover;
 
+use GuzzleHttp\Exception\GuzzleException;
 use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Notifications\Events\NotificationFailed;
 use Illuminate\Notifications\Notification;
+use NotificationChannels\Pushover\Exceptions\CouldNotSendNotification;
 use NotificationChannels\Pushover\Exceptions\ServiceCommunicationError;
 
 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  Dispatcher  $events
      */
     public function __construct(Pushover $pushover, Dispatcher $events)
     {
@@ -30,32 +31,42 @@ public function __construct(Pushover $pushover, Dispatcher $events)
      * Send the given notification.
      *
      * @param  mixed  $notifiable
-     * @param  \Illuminate\Notifications\Notification  $notification
+     * @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;
         }
 
         if (is_string($pushoverReceiver)) {
+            // From https://pushover.net/api:
+            // "User and group identifiers are 30 characters long, ..."
+            if (strlen($pushoverReceiver) !== 30) {
+                throw CouldNotSendNotification::pushoverKeyHasWrongLength($notifiable);
+            }
+
             $pushoverReceiver = PushoverReceiver::withUserKey($pushoverReceiver);
         }
 
         $message = $notification->toPushover($notifiable);
 
         try {
-            $this->pushover->send(array_merge($message->toArray(), $pushoverReceiver->toArray()));
+            $this->pushover->send(
+                array_merge($message->toArray(), $pushoverReceiver->toArray()),
+                $notifiable
+            );
         } catch (ServiceCommunicationError $serviceCommunicationError) {
             $this->fireFailedEvent($notifiable, $notification, $serviceCommunicationError->getMessage());
         }
     }
 
-    protected function fireFailedEvent($notifiable, $notification, $message)
+    protected function fireFailedEvent($notifiable, $notification, $message): void
     {
-        $this->events->fire(
+        $this->events->dispatch(
             new NotificationFailed($notifiable, $notification, 'pushover', [$message])
         );
     }