Refactoring Messaging

packages/messaging/src/controllers/controller-interface.ts

@@ -77,15 +77,11 @@ export abstract class ControllerInterface implements FirebaseMessaging {
   async getToken(): Promise<string | null> {
     // Check with permissions
     const currentPermission = this.getNotificationPermission_();
-    if (currentPermission !== 'granted') {
-      if (currentPermission === 'denied') {
-        return Promise.reject(
-          errorFactory.create(ERROR_CODES.NOTIFICATIONS_BLOCKED)
-        );
-      }
-
+    if (currentPermission === 'denied') {
+      throw errorFactory.create(ERROR_CODES.NOTIFICATIONS_BLOCKED);
+    } else if (currentPermission !== 'granted') {
       // We must wait for permission to be granted
-      return Promise.resolve(null);
+      return null;
     }
 
     const swReg = await this.getSWRegistration_();
@@ -223,7 +219,6 @@ export abstract class ControllerInterface implements FirebaseMessaging {
    * unsubscribes the token from FCM  and then unregisters the push
    * subscription if it exists. It returns a promise that indicates
    * whether or not the unsubscribe request was processed successfully.
-   * @export
    */
   async deleteToken(token: string): Promise<boolean> {
     // Delete the token details from the database.

packages/messaging/src/controllers/window-controller.ts

@@ -36,6 +36,10 @@ import {
 } from '../models/worker-page-message';
 import { ControllerInterface } from './controller-interface';
 
+interface ManifestContent {
+  gcm_sender_id: string;
+}
+
 export class WindowController extends ControllerInterface {
   private registrationToUse: ServiceWorkerRegistration | null = null;
   private publicVapidKeyToUse: Uint8Array | null = null;
@@ -75,55 +79,12 @@ export class WindowController extends ControllerInterface {
    * @return Returns a promise that resolves to an FCM token or null if
    * permission isn't granted.
    */
-  getToken(): Promise<string | null> {
-    return this.manifestCheck_().then(() => {
-      return super.getToken();
-    });
-  }
-
-  /**
-   * The method checks that a manifest is defined and has the correct GCM
-   * sender ID.
-   * @return Returns a promise that resolves if the manifest matches
-   * our required sender ID
-   */
-  // Visible for testing
-  // TODO: make private
-  manifestCheck_(): Promise<void> {
-    if (this.manifestCheckPromise) {
-      return this.manifestCheckPromise;
-    }
-
-    const manifestTag = document.querySelector<HTMLAnchorElement>(
-      'link[rel="manifest"]'
-    );
-    if (!manifestTag) {
-      this.manifestCheckPromise = Promise.resolve();
-    } else {
-      this.manifestCheckPromise = fetch(manifestTag.href)
-        .then(response => {
-          return response.json();
-        })
-        .catch(() => {
-          // If the download or parsing fails allow check.
-          // We only want to error if we KNOW that the gcm_sender_id is incorrect.
-        })
-        .then(manifestContent => {
-          if (!manifestContent) {
-            return;
-          }
-
-          if (!manifestContent['gcm_sender_id']) {
-            return;
-          }
-
-          if (manifestContent['gcm_sender_id'] !== '103953800507') {
-            throw errorFactory.create(ERROR_CODES.INCORRECT_GCM_SENDER_ID);
-          }
-        });
+  async getToken(): Promise<string | null> {
+    if (!this.manifestCheckPromise) {
+      this.manifestCheckPromise = manifestCheck();
     }
-
-    return this.manifestCheckPromise;
+    await this.manifestCheckPromise;
+    return super.getToken();
   }
 
   /**
@@ -132,37 +93,18 @@ export class WindowController extends ControllerInterface {
    * @return Resolves if the permission was granted, otherwise rejects
    */
   async requestPermission(): Promise<void> {
-    if (
-      // TODO: Remove the cast when this issue is fixed:
-      // https://github.com/Microsoft/TypeScript/issues/14701
-      // tslint:disable-next-line no-any
-      ((Notification as any).permission as NotificationPermission) === 'granted'
-    ) {
+    if (this.getNotificationPermission_() === 'granted') {
       return;
     }
 
-    return new Promise<void>((resolve, reject) => {
-      const managePermissionResult = (result: NotificationPermission) => {
-        if (result === 'granted') {
-          return resolve();
-        } else if (result === 'denied') {
-          return reject(errorFactory.create(ERROR_CODES.PERMISSION_BLOCKED));
-        } else {
-          return reject(errorFactory.create(ERROR_CODES.PERMISSION_DEFAULT));
-        }
-      };
-
-      // The Notification.requestPermission API was changed to
-      // return a promise so now have to handle both in case
-      // browsers stop support callbacks for promised version
-      const permissionPromise = Notification.requestPermission(
-        managePermissionResult
-      );
-      if (permissionPromise) {
-        // Prefer the promise version as it's the future API.
-        permissionPromise.then(managePermissionResult);
-      }
-    });
+    const permissionResult = await Notification.requestPermission();
+    if (permissionResult === 'granted') {
+      return;
+    } else if (permissionResult === 'denied') {
+      throw errorFactory.create(ERROR_CODES.PERMISSION_BLOCKED);
+    } else {
+      throw errorFactory.create(ERROR_CODES.PERMISSION_DEFAULT);
+    }
   }
 
   /**
@@ -334,12 +276,12 @@ export class WindowController extends ControllerInterface {
    * This will return the default VAPID key or the uint8array version of the public VAPID key
    * provided by the developer.
    */
-  getPublicVapidKey_(): Promise<Uint8Array> {
+  async getPublicVapidKey_(): Promise<Uint8Array> {
     if (this.publicVapidKeyToUse) {
-      return Promise.resolve(this.publicVapidKeyToUse);
+      return this.publicVapidKeyToUse;
     }
 
-    return Promise.resolve(DEFAULT_PUBLIC_VAPID_KEY);
+    return DEFAULT_PUBLIC_VAPID_KEY;
   }
 
   /**
@@ -376,3 +318,38 @@ export class WindowController extends ControllerInterface {
     );
   }
 }
+
+/**
+ * The method checks that a manifest is defined and has the correct GCM
+ * sender ID.
+ * @return Returns a promise that resolves if the manifest matches
+ * our required sender ID
+ */
+// Exported for testing
+export async function manifestCheck(): Promise<void> {
+  const manifestTag = document.querySelector<HTMLAnchorElement>(
+    'link[rel="manifest"]'
+  );
+
+  if (!manifestTag) {
+    return;
+  }
+
+  let manifestContent: ManifestContent;
+  try {
+    const response = await fetch(manifestTag.href);
+    manifestContent = await response.json();
+  } catch (e) {
+    // If the download or parsing fails allow check.
+    // We only want to error if we KNOW that the gcm_sender_id is incorrect.
+    return;
+  }
+
+  if (!manifestContent || !manifestContent.gcm_sender_id) {
+    return;
+  }
+
+  if (manifestContent.gcm_sender_id !== '103953800507') {
+    throw errorFactory.create(ERROR_CODES.INCORRECT_GCM_SENDER_ID);
+  }
+}

packages/messaging/test/window-controller.test.ts

@@ -19,7 +19,10 @@ import * as sinon from 'sinon';
 import { makeFakeApp } from './testing-utils/make-fake-app';
 import { makeFakeSWReg } from './testing-utils/make-fake-sw-reg';
 
-import { WindowController } from '../src/controllers/window-controller';
+import {
+  manifestCheck,
+  WindowController
+} from '../src/controllers/window-controller';
 import { base64ToArrayBuffer } from '../src/helpers/base64-to-array-buffer';
 import { DEFAULT_PUBLIC_VAPID_KEY } from '../src/models/fcm-details';
 
@@ -46,15 +49,14 @@ describe('Firebase Messaging > *WindowController', () => {
     return cleanup();
   });
 
-  describe('manifestCheck_()', () => {
+  describe('manifestCheck()', () => {
     it("should resolve when the tag isn't defined", () => {
       sandbox
         .stub(document, 'querySelector')
         .withArgs('link[rel="manifest"]')
         .returns(null);
 
-      const controller = new WindowController(app);
-      return controller.manifestCheck_();
+      return manifestCheck();
     });
 
     it('should fetch the manifest if defined and resolve when no gcm_sender_id', () => {
@@ -76,8 +78,7 @@ describe('Firebase Messaging > *WindowController', () => {
           })
         );
 
-      const controller = new WindowController(app);
-      return controller.manifestCheck_();
+      return manifestCheck();
     });
 
     it('should fetch the manifest if defined and resolve with expected gcm_sender_id', () => {
@@ -101,8 +102,7 @@ describe('Firebase Messaging > *WindowController', () => {
           })
         );
 
-      const controller = new WindowController(app);
-      return controller.manifestCheck_();
+      return manifestCheck();
     });
 
     it('should fetch the manifest if defined and reject when using wrong gcm_sender_id', () => {
@@ -126,8 +126,7 @@ describe('Firebase Messaging > *WindowController', () => {
           })
         );
 
-      const controller = new WindowController(app);
-      return controller.manifestCheck_().then(
+      return manifestCheck().then(
         () => {
           throw new Error('Expected error to be thrown.');
         },
@@ -150,8 +149,7 @@ describe('Firebase Messaging > *WindowController', () => {
         .withArgs('https://firebase.io/messaging/example')
         .returns(Promise.reject(new Error('Injected Failure.')));
 
-      const controller = new WindowController(app);
-      return controller.manifestCheck_();
+      return manifestCheck();
     });
   });
 
@@ -206,16 +204,6 @@ describe('Firebase Messaging > *WindowController', () => {
       const controller = new WindowController(app);
       return controller.requestPermission();
     });
-
-    it('should resolve if the requestPermission() is granted using old callback API', () => {
-      sandbox.stub(Notification as any, 'permission').value('default');
-      sandbox.stub(Notification, 'requestPermission').callsFake(cb => {
-        cb('granted');
-      });
-
-      const controller = new WindowController(app);
-      return controller.requestPermission();
-    });
   });
 
   describe('useServiceWorker()', () => {