From e3c4fb5bab71cfcb95ccc785c00fb27d00868865 Mon Sep 17 00:00:00 2001 From: Mertcan Mermerkaya Date: Thu, 26 Apr 2018 12:58:14 +0100 Subject: [PATCH 1/3] Some more async/await --- .../src/controllers/controller-interface.ts | 13 ++-- .../src/controllers/window-controller.ts | 69 +++++++++---------- 2 files changed, 37 insertions(+), 45 deletions(-) diff --git a/packages/messaging/src/controllers/controller-interface.ts b/packages/messaging/src/controllers/controller-interface.ts index 66cb3317582..907e8dc09e7 100644 --- a/packages/messaging/src/controllers/controller-interface.ts +++ b/packages/messaging/src/controllers/controller-interface.ts @@ -77,15 +77,11 @@ export abstract class ControllerInterface implements FirebaseMessaging { async getToken(): Promise { // 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 { // Delete the token details from the database. diff --git a/packages/messaging/src/controllers/window-controller.ts b/packages/messaging/src/controllers/window-controller.ts index fa2cd6c8fbb..5920db015be 100644 --- a/packages/messaging/src/controllers/window-controller.ts +++ b/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,10 +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 { - return this.manifestCheck_().then(() => { - return super.getToken(); - }); + async getToken(): Promise { + if (!this.manifestCheckPromise) { + this.manifestCheckPromise = this.manifestCheck_(); + } + await this.manifestCheckPromise; + return super.getToken(); } /** @@ -89,41 +95,32 @@ export class WindowController extends ControllerInterface { */ // Visible for testing // TODO: make private - manifestCheck_(): Promise { - if (this.manifestCheckPromise) { - return this.manifestCheckPromise; - } - + async manifestCheck_(): Promise { const manifestTag = document.querySelector( '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); - } - }); + 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; } - return this.manifestCheckPromise; + if (!manifestContent || !manifestContent.gcm_sender_id) { + return; + } + + if (manifestContent.gcm_sender_id !== '103953800507') { + throw errorFactory.create(ERROR_CODES.INCORRECT_GCM_SENDER_ID); + } } /** @@ -334,12 +331,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 { + async getPublicVapidKey_(): Promise { if (this.publicVapidKeyToUse) { - return Promise.resolve(this.publicVapidKeyToUse); + return this.publicVapidKeyToUse; } - return Promise.resolve(DEFAULT_PUBLIC_VAPID_KEY); + return DEFAULT_PUBLIC_VAPID_KEY; } /** From 68e2827c616b1cc9d720407eb309772033213bbb Mon Sep 17 00:00:00 2001 From: Mertcan Mermerkaya Date: Thu, 26 Apr 2018 13:10:40 +0100 Subject: [PATCH 2/3] Remove deprecated callback API --- .../src/controllers/window-controller.ts | 37 +++++-------------- .../messaging/test/window-controller.test.ts | 10 ----- 2 files changed, 9 insertions(+), 38 deletions(-) diff --git a/packages/messaging/src/controllers/window-controller.ts b/packages/messaging/src/controllers/window-controller.ts index 5920db015be..5a2bf0d481a 100644 --- a/packages/messaging/src/controllers/window-controller.ts +++ b/packages/messaging/src/controllers/window-controller.ts @@ -129,37 +129,18 @@ export class WindowController extends ControllerInterface { * @return Resolves if the permission was granted, otherwise rejects */ async requestPermission(): Promise { - 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((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); + } } /** diff --git a/packages/messaging/test/window-controller.test.ts b/packages/messaging/test/window-controller.test.ts index 3490655b435..3925f0b9b2c 100644 --- a/packages/messaging/test/window-controller.test.ts +++ b/packages/messaging/test/window-controller.test.ts @@ -206,16 +206,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()', () => { From 374cf9fd655567e36baa02294b8317c63e8de4c7 Mon Sep 17 00:00:00 2001 From: Mertcan Mermerkaya Date: Wed, 9 May 2018 17:38:22 +0100 Subject: [PATCH 3/3] Make manifestCheck a function --- .../src/controllers/window-controller.ts | 73 +++++++++---------- .../messaging/test/window-controller.test.ts | 22 +++--- 2 files changed, 46 insertions(+), 49 deletions(-) diff --git a/packages/messaging/src/controllers/window-controller.ts b/packages/messaging/src/controllers/window-controller.ts index 5a2bf0d481a..1e049fead69 100644 --- a/packages/messaging/src/controllers/window-controller.ts +++ b/packages/messaging/src/controllers/window-controller.ts @@ -81,48 +81,12 @@ export class WindowController extends ControllerInterface { */ async getToken(): Promise { if (!this.manifestCheckPromise) { - this.manifestCheckPromise = this.manifestCheck_(); + this.manifestCheckPromise = manifestCheck(); } await this.manifestCheckPromise; 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 - async manifestCheck_(): Promise { - const manifestTag = document.querySelector( - '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); - } - } - /** * Request permission if it is not currently granted * @@ -354,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 { + const manifestTag = document.querySelector( + '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); + } +} diff --git a/packages/messaging/test/window-controller.test.ts b/packages/messaging/test/window-controller.test.ts index 3925f0b9b2c..6913d738b79 100644 --- a/packages/messaging/test/window-controller.test.ts +++ b/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(); }); });