Downgrade Firebase (#59)

* Use a lower firebase package verison * Prevent firebase from sending when no firebase settings has been set * Don't do anything if the settings is empty * Remove debug * Add better exception handling * Fix error handling * Remove expired token * CS fixes * Clean up logging * Improve json validation * Prettier * CS fixes
askvortsov1 · Jun 15, 2024 · 7f312cd · 7f312cd
1 parent 23704aa
commit 7f312cd
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 22 deletions.
diff --git a/composer.json b/composer.json
@@ -15,7 +15,7 @@
         "flarum/core": "^1.7",
         "minishlink/web-push": "^7.0",
         "spomky-labs/base64url" : "^2.0",
-        "kreait/firebase-php": "^7.9"
+        "kreait/firebase-php": "^5.0"
     },
     "suggest": {
         "ext-gmp": "This can improve push notification performance."

diff --git a/js/src/admin/components/PWAPage.js b/js/src/admin/components/PWAPage.js
@@ -172,8 +172,13 @@ export default class PWAPage extends ExtensionPage {
                   setting: 'askvortsov-pwa.windowControlsOverlay',
                   label: app.translator.trans('askvortsov-pwa.admin.pwa.other.window_controls_overlay_label'),
                   help: app.translator.trans('askvortsov-pwa.admin.pwa.other.window_controls_overlay_text', {
-                    compatibilitylink: <a href="https://caniuse.com/mdn-api_windowcontrolsoverlay" tabindex="-1"/>,
-                    learnlink: <a href="https://learn.microsoft.com/en-us/microsoft-edge/progressive-web-apps-chromium/how-to/window-controls-overlay" tabindex="-1"/>
+                    compatibilitylink: <a href="https://caniuse.com/mdn-api_windowcontrolsoverlay" tabindex="-1" />,
+                    learnlink: (
+                      <a
+                        href="https://learn.microsoft.com/en-us/microsoft-edge/progressive-web-apps-chromium/how-to/window-controls-overlay"
+                        tabindex="-1"
+                      />
+                    ),
                   }),
                   type: 'bool',
                 })}

diff --git a/src/FirebasePushSender.php b/src/FirebasePushSender.php
@@ -12,8 +12,11 @@
 namespace Askvortsov\FlarumPWA;
 
 use Flarum\Notification\Blueprint\BlueprintInterface;
+use Flarum\Settings\SettingsRepositoryInterface;
 use Illuminate\Container\Container;
 use Kreait\Firebase\Contract\Messaging;
+use Kreait\Firebase\Exception\Messaging\AuthenticationError;
+use Kreait\Firebase\Exception\Messaging\NotFound;
 use Kreait\Firebase\Messaging\CloudMessage;
 use Kreait\Firebase\Messaging\Notification;
 use Psr\Log\LoggerInterface;
@@ -26,23 +29,33 @@ class FirebasePushSender
 
     protected LoggerInterface $logger;
 
+    protected SettingsRepositoryInterface $settings;
+
     public function __construct(
         Container $container,
         NotificationBuilder $notifications,
         LoggerInterface $logger,
+        SettingsRepositoryInterface $settings,
     ) {
         $this->container = $container;
         $this->notifications = $notifications;
         $this->logger = $logger;
+        $this->settings = $settings;
     }
 
     public function notify(BlueprintInterface $blueprint, array $userIds = []): void
     {
+        if (! $this->hasValidFirebaseSettings()) {
+            return;
+        }
+
         try {
             // We're using the container to resolve the FirebaseMessagingContract here so we have more
             // control on when and where to log the error. Having it passed on the constructor will mean
             // we'll have to throw an exception and log the error for the user in the exception handler
             // rather than directly in the class that consumes the contract.
+
+            /** @throws FirebaseConfigInvalid */
             $messaging = $this->container->make(Messaging::class);
         } catch (FirebaseConfigInvalid) {
             $this->logger->error('Firebase config invalid');
@@ -51,9 +64,13 @@ public function notify(BlueprintInterface $blueprint, array $userIds = []): void
         }
 
         FirebasePushSubscription::whereIn('user_id', $userIds)->each(function (FirebasePushSubscription $subscription) use ($messaging, $blueprint) {
-            $messaging->send(
-                $this->newFirebaseCloudMessage($subscription, $blueprint)
-            );
+            try {
+                $messaging->send($this->newFirebaseCloudMessage($subscription, $blueprint));
+            } catch (AuthenticationError $e) {
+                $this->logger->error($e->getMessage());
+            } catch (NotFound) {
+                $subscription->delete();
+            }
         });
     }
 
@@ -70,4 +87,17 @@ private function newFirebaseCloudMessage(FirebasePushSubscription $subscription,
                 ])
             );
     }
+
+    private function hasValidFirebaseSettings(): bool
+    {
+        $config = $this->settings->get('askvortsov-pwa.firebaseConfig');
+
+        if (! $config) {
+            return false;
+        }
+
+        json_decode($config);
+
+        return json_last_error() === JSON_ERROR_NONE;
+    }
 }
diff --git a/src/Job/SendPushNotificationsJob.php b/src/Job/SendPushNotificationsJob.php
@@ -11,13 +11,11 @@
 
 namespace Askvortsov\FlarumPWA\Job;
 
-use Askvortsov\FlarumPWA\FirebaseConfigInvalid;
 use Askvortsov\FlarumPWA\FirebasePushSender;
 use Askvortsov\FlarumPWA\PushSender;
 use ErrorException;
 use Flarum\Notification\Blueprint\BlueprintInterface;
 use Flarum\Queue\AbstractJob;
-use Flarum\Settings\SettingsRepositoryInterface;
 
 class SendPushNotificationsJob extends AbstractJob
 {
@@ -37,22 +35,10 @@ public function __construct(BlueprintInterface $blueprint, array $recipientIds =
     /**
      * @throws ErrorException
      */
-    public function handle(PushSender $native, FirebasePushSender $firebase, SettingsRepositoryInterface $settings): void
+    public function handle(PushSender $native, FirebasePushSender $firebase): void
     {
         $native->notify($this->blueprint, $this->recipientIds);
 
-        try {
-            $firebase->notify($this->blueprint, $this->recipientIds);
-        } catch (FirebaseConfigInvalid) {
-        }
-    }
-
-    private function hasValidFirebaseSettings(SettingsRepositoryInterface $settings): bool
-    {
-        try {
-            return (bool) json_encode($settings->get('askvortsov-pwa.firebaseConfig'));
-        } catch (\Throwable) {
-            return false;
-        }
+        $firebase->notify($this->blueprint, $this->recipientIds);
     }
 }