Raise PHPStan level to max and allow multiple mailers (#23)

* Work towards PHPStan level max * Handle invalid configuration * Fix tests on older pest versions
InnoGE · Apr 22, 2024 · 2336ce2 · 2336ce2
1 parent 0c0e892
commit 2336ce2
Show file tree

Hide file tree

Showing 9 changed files with 183 additions and 87 deletions.
diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml
@@ -11,7 +11,8 @@ jobs:
     name: phpstan
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - name: Checkout code
+        uses: actions/checkout@v4
 
       - name: Setup PHP
         uses: shivammathur/setup-php@v2
@@ -23,4 +24,4 @@ jobs:
         uses: ramsey/composer-install@v3
 
       - name: Run PHPStan
-        run: ./vendor/bin/phpstan --error-format=github
+        run: vendor/bin/phpstan --error-format=github
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -2,12 +2,11 @@ includes:
     - phpstan-baseline.neon
 
 parameters:
-    level: 8
+    level: max
     paths:
         - src
     tmpDir: build/phpstan
     checkOctaneCompatibility: true
     checkModelProperties: true
     checkMissingIterableValueType: false
     checkGenericClassInNonGenericObjectType: false
-
diff --git a/src/Exceptions/ConfigurationInvalid.php b/src/Exceptions/ConfigurationInvalid.php
@@ -0,0 +1,14 @@
+<?php
+
+namespace InnoGE\LaravelMsGraphMail\Exceptions;
+
+use Exception;
+
+class ConfigurationInvalid extends Exception
+{
+    public function __construct(string $key, mixed $value)
+    {
+        $invalidValue = var_export($value, true);
+        parent::__construct("Configuration key {$key} for microsoft-graph mailer has invalid value: {$invalidValue}.");
+    }
+}
diff --git a/src/Exceptions/ConfigurationMissing.php b/src/Exceptions/ConfigurationMissing.php
@@ -6,23 +6,8 @@
 
 class ConfigurationMissing extends Exception
 {
-    public static function tenantId(): self
+    public function __construct(string $key)
     {
-        return new self('The tenant id is missing from the configuration file.');
-    }
-
-    public static function clientId(): self
-    {
-        return new self('The client id is missing from the configuration file.');
-    }
-
-    public static function clientSecret(): self
-    {
-        return new self('The client secret is missing from the configuration file.');
-    }
-
-    public static function fromAddress(): self
-    {
-        return new self('The mail from address is missing from the configuration file.');
+        parent::__construct("Configuration key {$key} for microsoft-graph mailer is missing.");
     }
 }
diff --git a/src/Exceptions/InvalidResponse.php b/src/Exceptions/InvalidResponse.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace InnoGE\LaravelMsGraphMail\Exceptions;
+
+use Exception;
+
+class InvalidResponse extends Exception
+{
+}
diff --git a/src/LaravelMsGraphMailServiceProvider.php b/src/LaravelMsGraphMailServiceProvider.php
@@ -3,6 +3,7 @@
 namespace InnoGE\LaravelMsGraphMail;
 
 use Illuminate\Support\Facades\Mail;
+use InnoGE\LaravelMsGraphMail\Exceptions\ConfigurationInvalid;
 use InnoGE\LaravelMsGraphMail\Exceptions\ConfigurationMissing;
 use InnoGE\LaravelMsGraphMail\Services\MicrosoftGraphApiService;
 use Spatie\LaravelPackageTools\Package;
@@ -23,26 +24,40 @@ public function configurePackage(Package $package): void
 
     public function boot(): void
     {
-        $this->app->bind(MicrosoftGraphApiService::class, function () {
-            //throw exceptions when config is missing
-            throw_unless(filled(config('mail.mailers.microsoft-graph.tenant_id')), ConfigurationMissing::tenantId());
-            throw_unless(filled(config('mail.mailers.microsoft-graph.client_id')), ConfigurationMissing::clientId());
-            throw_unless(filled(config('mail.mailers.microsoft-graph.client_secret')), ConfigurationMissing::clientSecret());
-
-            return new MicrosoftGraphApiService(
-                tenantId: config('mail.mailers.microsoft-graph.tenant_id', ''),
-                clientId: config('mail.mailers.microsoft-graph.client_id', ''),
-                clientSecret: config('mail.mailers.microsoft-graph.client_secret', ''),
-                accessTokenTtl: config('mail.mailers.microsoft-graph.access_token_ttl', 3000),
-            );
-        });
+        Mail::extend('microsoft-graph', function (array $config): MicrosoftGraphTransport {
+            throw_if(blank($config['from']['address'] ?? []), new ConfigurationMissing('from.address'));
 
-        Mail::extend('microsoft-graph', function (array $config) {
-            throw_unless(filled($config['from']['address'] ?? []), ConfigurationMissing::fromAddress());
+            $accessTokenTtl = $config['access_token_ttl'] ?? 3000;
+            if (! is_int($accessTokenTtl)) {
+                throw new ConfigurationInvalid('access_token_ttl', $accessTokenTtl);
+            }
 
             return new MicrosoftGraphTransport(
-                $this->app->make(MicrosoftGraphApiService::class)
+                new MicrosoftGraphApiService(
+                    tenantId: $this->requireConfigString($config, 'tenant_id'),
+                    clientId: $this->requireConfigString($config, 'client_id'),
+                    clientSecret: $this->requireConfigString($config, 'client_secret'),
+                    accessTokenTtl: $accessTokenTtl,
+                ),
             );
         });
     }
+
+    /**
+     * @param  array<string, mixed>  $config
+     * @return non-empty-string
+     */
+    protected function requireConfigString(array $config, string $key): string
+    {
+        if (! array_key_exists($key, $config)) {
+            throw new ConfigurationMissing($key);
+        }
+
+        $value = $config[$key];
+        if (! is_string($value) || $value === '') {
+            throw new ConfigurationInvalid($key, $value);
+        }
+
+        return $value;
+    }
 }
diff --git a/src/MicrosoftGraphTransport.php b/src/MicrosoftGraphTransport.php
@@ -82,7 +82,7 @@ protected function prepareAttachments(Email $email, ?string $html): array
     }
 
     /**
-     * @param  Collection<Address>  $recipients
+     * @param  Collection<array-key, Address>  $recipients
      */
     protected function transformEmailAddresses(Collection $recipients): array
     {
@@ -101,7 +101,7 @@ protected function transformEmailAddress(Address $address): array
     }
 
     /**
-     * @return Collection<Address>
+     * @return Collection<array-key, Address>
      */
     protected function getRecipients(Email $email, Envelope $envelope): Collection
     {

diff --git a/src/Services/MicrosoftGraphApiService.php b/src/Services/MicrosoftGraphApiService.php
@@ -7,6 +7,7 @@
 use Illuminate\Http\Client\Response;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Http;
+use InnoGE\LaravelMsGraphMail\Exceptions\InvalidResponse;
 
 class MicrosoftGraphApiService
 {
@@ -48,7 +49,13 @@ protected function getAccessToken(): string
 
             $response->throw();
 
-            return $response->json('access_token');
+            $accessToken = $response->json('access_token');
+            if (! is_string($accessToken)) {
+                $notString = var_export($accessToken, true);
+                throw new InvalidResponse("Expected response to contain key access_token of type string, got: {$notString}.");
+            }
+
+            return $accessToken;
         });
     }
 }
diff --git a/tests/MicrosoftGraphTransportTest.php b/tests/MicrosoftGraphTransportTest.php
@@ -6,7 +6,9 @@
 use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Mail;
 use Illuminate\Support\Str;
+use InnoGE\LaravelMsGraphMail\Exceptions\ConfigurationInvalid;
 use InnoGE\LaravelMsGraphMail\Exceptions\ConfigurationMissing;
+use InnoGE\LaravelMsGraphMail\Exceptions\InvalidResponse;
 use InnoGE\LaravelMsGraphMail\Tests\Stubs\TestMail;
 use InnoGE\LaravelMsGraphMail\Tests\Stubs\TestMailWithInlineImage;
 
@@ -219,69 +221,133 @@
         ->toBe('foo_access_token');
 });
 
-it('throws exceptions when config is missing', function (array $config, string $exceptionMessage) {
+it('throws exceptions on invalid access token in response', function () {
+    Config::set('mail.mailers.microsoft-graph', [
+        'transport' => 'microsoft-graph',
+        'client_id' => 'foo_client_id',
+        'client_secret' => 'foo_client_secret',
+        'tenant_id' => 'foo_tenant_id',
+        'from' => [
+            'address' => 'taylor@laravel.com',
+            'name' => 'Taylor Otwell',
+        ],
+    ]);
+    Config::set('mail.default', 'microsoft-graph');
+
+    Http::fake([
+        'https://login.microsoftonline.com/foo_tenant_id/oauth2/v2.0/token' => Http::response(['access_token' => 123]),
+    ]);
+
+    expect(fn () => Mail::to('caleb@livewire.com')->send(new TestMail(false)))
+        ->toThrow(InvalidResponse::class, 'Expected response to contain key access_token of type string, got: 123.');
+});
+
+it('throws exceptions when config is invalid', function (array $config, Exception $exception) {
     Config::set('mail.mailers.microsoft-graph', $config);
     Config::set('mail.default', 'microsoft-graph');
 
-    try {
-        Mail::to('caleb@livewire.com')
-            ->send(new TestMail(false));
-    } catch (Exception $e) {
-        expect($e)
-            ->toBeInstanceOf(ConfigurationMissing::class)
-            ->getMessage()->toBe($exceptionMessage);
-    }
-})->with(
+    expect(fn () => Mail::to('caleb@livewire.com')->send(new TestMail(false)))
+        ->toThrow(get_class($exception), $exception->getMessage());
+})->with([
     [
         [
-            [
-                'transport' => 'microsoft-graph',
-                'client_id' => 'foo_client_id',
-                'client_secret' => 'foo_client_secret',
-                'tenant_id' => '',
-                'from' => [
-                    'address' => 'taylor@laravel.com',
-                    'name' => 'Taylor Otwell',
-                ],
+            'transport' => 'microsoft-graph',
+            'client_id' => 'foo_client_id',
+            'client_secret' => 'foo_client_secret',
+            'from' => [
+                'address' => 'taylor@laravel.com',
+                'name' => 'Taylor Otwell',
             ],
-            'The tenant id is missing from the configuration file.',
         ],
+        new ConfigurationMissing('tenant_id'),
+    ],
+    [
         [
-            [
-                'transport' => 'microsoft-graph',
-                'client_id' => '',
-                'client_secret' => 'foo_client_secret',
-                'tenant_id' => 'foo_tenant_id',
-                'from' => [
-                    'address' => 'taylor@laravel.com',
-                    'name' => 'Taylor Otwell',
-                ],
+            'transport' => 'microsoft-graph',
+            'tenant_id' => 123,
+            'client_id' => 'foo_client_id',
+            'client_secret' => 'foo_client_secret',
+            'from' => [
+                'address' => 'taylor@laravel.com',
+                'name' => 'Taylor Otwell',
             ],
-            'The client id is missing from the configuration file.',
         ],
+        new ConfigurationInvalid('tenant_id', 123),
+    ],
+    [
         [
-            [
-                'transport' => 'microsoft-graph',
-                'client_id' => 'foo_client_id',
-                'client_secret' => '',
-                'tenant_id' => 'foo_tenant_id',
-                'from' => [
-                    'address' => 'taylor@laravel.com',
-                    'name' => 'Taylor Otwell',
-                ],
+            'transport' => 'microsoft-graph',
+            'tenant_id' => 'foo_tenant_id',
+            'client_secret' => 'foo_client_secret',
+            'from' => [
+                'address' => 'taylor@laravel.com',
+                'name' => 'Taylor Otwell',
             ],
-            'The client secret is missing from the configuration file.',
         ],
+        new ConfigurationMissing('client_id'),
+    ],
+    [
         [
-            [
-                'transport' => 'microsoft-graph',
-                'client_id' => 'foo_client_id',
-                'client_secret' => 'foo_client_secret',
-                'tenant_id' => 'foo_tenant_id',
+            'transport' => 'microsoft-graph',
+            'tenant_id' => 'foo_tenant_id',
+            'client_id' => '',
+            'client_secret' => 'foo_client_secret',
+            'from' => [
+                'address' => 'taylor@laravel.com',
+                'name' => 'Taylor Otwell',
             ],
-            'The mail from address is missing from the configuration file.',
         ],
-    ]);
+        new ConfigurationInvalid('client_id', ''),
+    ],
+    [
+        [
+            'transport' => 'microsoft-graph',
+            'tenant_id' => 'foo_tenant_id',
+            'client_id' => 'foo_client_id',
+            'from' => [
+                'address' => 'taylor@laravel.com',
+                'name' => 'Taylor Otwell',
+            ],
+        ],
+        new ConfigurationMissing('client_secret'),
+    ],
+    [
+        [
+            'transport' => 'microsoft-graph',
+            'tenant_id' => 'foo_tenant_id',
+            'client_id' => 'foo_client_id',
+            'client_secret' => null,
+            'from' => [
+                'address' => 'taylor@laravel.com',
+                'name' => 'Taylor Otwell',
+            ],
+        ],
+        new ConfigurationInvalid('client_secret', null),
+    ],
+    [
+        [
+            'transport' => 'microsoft-graph',
+            'tenant_id' => 'foo_tenant_id',
+            'client_id' => 'foo_client_id',
+            'client_secret' => 'foo_client_secret',
+        ],
+        new ConfigurationMissing('from.address'),
+    ],
+    [
+        [
+            'transport' => 'microsoft-graph',
+            'tenant_id' => 'foo_tenant_id',
+            'client_id' => 'foo_client_id',
+            'client_secret' => 'foo_client_secret',
+            'access_token_ttl' => false,
+            'from' => [
+                'address' => 'taylor@laravel.com',
+                'name' => 'Taylor Otwell',
+            ],
+        ],
+        new ConfigurationInvalid('access_token_ttl', false),
+    ],
+]);
 
 it('sends html mails with inline images with microsoft graph', function () {
     Config::set('mail.mailers.microsoft-graph', [