Merge pull request #747 from OneSignal/feat/update_sw_if_filename_or_…

…scope_changes

Update ServiceWorker if its href or scope changes

src/helpers/ServiceWorkerHelper.ts

@@ -12,24 +12,43 @@ import { OutcomesConfig } from "../models/Outcomes";
 import OutcomesHelper from './shared/OutcomesHelper';
 import { cancelableTimeout, CancelableTimeoutPromise } from './sw/CancelableTimeout';
 import { OSServiceWorkerFields } from "../service-worker/types";
+import Utils from "../context/shared/utils/Utils";
 
 declare var self: ServiceWorkerGlobalScope & OSServiceWorkerFields;
 
 export default class ServiceWorkerHelper {
-  public static getServiceWorkerHref(
+
+  // Get the href of the OneSiganl ServiceWorker that should be installed
+  // If a OneSignal ServiceWorker is already installed we will use an alternating name
+  //   to force an update to the worker.
+  public static getAlternatingServiceWorkerHref(
     workerState: ServiceWorkerActiveState,
-    config: ServiceWorkerManagerConfig): string {
-    let workerFullPath = "";
+    config: ServiceWorkerManagerConfig,
+    appId: string
+    ): string {
+    let workerFullPath: string;
 
     // Determine which worker to install
     if (workerState === ServiceWorkerActiveState.WorkerA)
       workerFullPath = config.workerBPath.getFullPath();
-    else if (workerState === ServiceWorkerActiveState.WorkerB ||
-      workerState === ServiceWorkerActiveState.ThirdParty ||
-      workerState === ServiceWorkerActiveState.None)
+    else
       workerFullPath = config.workerAPath.getFullPath();
 
-    return new URL(workerFullPath, OneSignalUtils.getBaseUrl()).href;
+    return ServiceWorkerHelper.appendServiceWorkerParams(workerFullPath, appId);
+  }
+
+  public static getPossibleServiceWorkerHrefs(
+    config: ServiceWorkerManagerConfig,
+    appId: string
+    ): string[] {
+    const workerFullPaths = [config.workerAPath.getFullPath(), config.workerBPath.getFullPath()];
+    return workerFullPaths.map((href) => ServiceWorkerHelper.appendServiceWorkerParams(href, appId));
+  }
+
+  private static appendServiceWorkerParams(workerFullPath: string, appId: string): string {
+    const fullPath = new URL(workerFullPath, OneSignalUtils.getBaseUrl()).href;
+    const appIdHasQueryParam = Utils.encodeHashAsUriComponent({appId});
+    return `${fullPath}?${appIdHasQueryParam}`;
   }
 
   public static async upsertSession(

src/managers/ServiceWorkerManager.ts

@@ -136,37 +136,95 @@ export class ServiceWorkerManager {
   }
 
   private async shouldInstallWorker(): Promise<boolean> {
+    // 1. Does the browser support ServiceWorkers?
     if (!Environment.supportsServiceWorkers())
       return false;
 
+    // 2. Is OneSignal initialized?
     if (!OneSignal.config)
       return false;
 
+    // 3. Will the service worker be installed on os.tc instead of the current domain?
     if (OneSignal.config.subdomain) {
       // No, if configured to use our subdomain (AKA HTTP setup) AND this is on their page (HTTP or HTTPS).
       // But since safari does not need subscription workaround, installing SW for session tracking.
       if (
-        OneSignal.environmentInfo.browserType !== "safari" && 
+        OneSignal.environmentInfo.browserType !== "safari" &&
           SdkEnvironment.getWindowEnv() === WindowEnvironmentKind.Host
       ) {
         return false;
       }
     }
 
+    // 4. Is a OneSignal ServiceWorker not installed now?
+    // If not and notification permissions are enabled we should install.
+    // This prevents an unnecessary install of the OneSignal worker which saves bandwidth
     const workerState = await this.getActiveState();
-    // If there isn't a SW or it isn't OneSignal's only install our SW if notification permissions are enabled
-    // This prevents an unnessary install which saves bandwidth
+    Log.debug("[shouldInstallWorker] workerState", workerState);
     if (workerState === ServiceWorkerActiveState.None || workerState === ServiceWorkerActiveState.ThirdParty) {
       const permission = await OneSignal.context.permissionManager.getNotificationPermission(
         OneSignal.config!.safariWebId
       );
+      const notificationsEnabled = permission === "granted";
+      if (notificationsEnabled) {
+        Log.info("[shouldInstallWorker] Notification Permissions enabled, will install ServiceWorker");
+      }
+      return notificationsEnabled;
+    }
 
-      return permission === "granted";
+    // 5. We have a OneSignal ServiceWorker installed, but did the path or scope of the ServiceWorker change?
+    if (await this.haveParamsChanged()) {
+      return true;
     }
 
+    // 6. We have a OneSignal ServiceWorker installed, is there an update?
     return this.workerNeedsUpdate();
   }
 
+  private async haveParamsChanged(): Promise<boolean> {
+    // 1. No workerRegistration
+    const workerRegistration = await this.context.serviceWorkerManager.getRegistration();
+    if (!workerRegistration) {
+      Log.info(
+        "[changedServiceWorkerParams] workerRegistration not found at scope",
+        this.config.registrationOptions.scope
+      );
+      return true;
+    }
+
+    // 2. Different scope
+    const existingSwScope = new URL(workerRegistration.scope).pathname;
+    const configuredSwScope = this.config.registrationOptions.scope;
+    if (existingSwScope != configuredSwScope) {
+      Log.info(
+        "[changedServiceWorkerParams] ServiceWorker scope changing",
+        { a_old: existingSwScope, b_new: configuredSwScope }
+      );
+      return true;
+    }
+
+    // 3. Different href?, asking if (path + filename [A or B] + queryParams) is different
+    const availableWorker = ServiceWorkerUtilHelper.getAvailableServiceWorker(workerRegistration);
+    const serviceWorkerHrefs = ServiceWorkerHelper.getPossibleServiceWorkerHrefs(
+      this.config,
+      this.context.appConfig.appId
+    );
+    // 3.1 If we can't get a scriptURL assume it is different
+    if (!availableWorker?.scriptURL) {
+      return true;
+    }
+    // 3.2 We don't care if the only differences is between OneSignal's A(Worker) vs B(WorkerUpdater) filename.
+    if (serviceWorkerHrefs.indexOf(availableWorker.scriptURL) === -1) {
+      Log.info(
+        "[changedServiceWorkerParams] ServiceWorker href changing:",
+        { a_old: availableWorker?.scriptURL, b_new: serviceWorkerHrefs }
+      );
+      return true;
+    }
+
+    return false;
+  }
+
   /**
    * Performs a service worker update by swapping out the current service worker
    * with a content-identical but differently named alternate service worker
@@ -251,6 +309,7 @@ export class ServiceWorkerManager {
   }
 
   public async establishServiceWorkerChannel() {
+    Log.debug('establishServiceWorkerChannel');
     const workerMessenger = this.context.workerMessenger;
     workerMessenger.off();
 
@@ -362,15 +421,16 @@ export class ServiceWorkerManager {
       Log.info(`[Service Worker Installation] 3rd party service worker detected.`);
     }
 
-    const workerFullPath = ServiceWorkerHelper.getServiceWorkerHref(workerState, this.config);
-    const installUrlQueryParams = Utils.encodeHashAsUriComponent({
-      appId: this.context.appConfig.appId
-    });
-    const fullWorkerPath = `${workerFullPath}?${installUrlQueryParams}`;
+    const workerHref = ServiceWorkerHelper.getAlternatingServiceWorkerHref(
+      workerState,
+      this.config,
+      this.context.appConfig.appId
+    );
+
     const scope = `${OneSignalUtils.getBaseUrl()}${this.config.registrationOptions.scope}`;
-    Log.info(`[Service Worker Installation] Installing service worker ${fullWorkerPath} ${scope}.`);
+    Log.info(`[Service Worker Installation] Installing service worker ${workerHref} ${scope}.`);
     try {
-      await navigator.serviceWorker.register(fullWorkerPath, { scope });
+      await navigator.serviceWorker.register(workerHref, { scope });
     } catch (error) {
       Log.error(`[Service Worker Installation] Installing service worker failed ${error}`);
       // Try accessing the service worker path directly to find out what the problem is and report it to OneSignal api.
@@ -381,7 +441,7 @@ export class ServiceWorkerManager {
       if (env === WindowEnvironmentKind.OneSignalSubscriptionPopup)
         throw error;
 
-      const response = await fetch(fullWorkerPath);
+      const response = await fetch(workerHref);
       if (response.status === 403 || response.status === 404)
         throw new ServiceWorkerRegistrationError(response.status, response.statusText);
 

test/support/mocks/service-workers/models/MockServiceWorkerContainer.ts

@@ -22,10 +22,10 @@ export abstract class MockServiceWorkerContainer implements ServiceWorkerContain
   onmessageerror: ((this: ServiceWorkerContainer, ev: MessageEvent) => any) | null;
 
   private dispatchEventUtil: DispatchEventUtil = new DispatchEventUtil();
-  public serviceWorkerRegistration: ServiceWorkerRegistration | null;
+  private serviceWorkerRegistrations: Map<string, ServiceWorkerRegistration>;
 
   constructor() {
-    this.serviceWorkerRegistration = null;
+    this.serviceWorkerRegistrations = new Map();
     this._controller = null;
     this.onmessage = null;
     this.onmessageerror = null;
@@ -40,17 +40,31 @@ export abstract class MockServiceWorkerContainer implements ServiceWorkerContain
     return this.dispatchEventUtil.dispatchEvent(evt);
   }
 
-  async getRegistration(_clientURL?: string): Promise<ServiceWorkerRegistration | undefined> {
-    return this.serviceWorkerRegistration || undefined;
+  async getRegistration(clientURL?: string): Promise<ServiceWorkerRegistration | undefined> {
+    const scope = clientURL || "/";
+
+    // 1. If we find an exact path match
+    let registration = this.serviceWorkerRegistrations.get(scope);
+    if (registration) {
+      return registration;
+    }
+
+    // 2. Match any SW's that are at a higher scope than the one we are querying for as they are under it's control.
+    // WARNING: This mock implementation does not consider which one is correct if more than one applies the scope.
+    this.serviceWorkerRegistrations.forEach((value, key) => {
+      if (scope.startsWith(key)) {
+        registration = value;
+      }
+    });
+
+    return registration;
   }
 
   async getRegistrations(): Promise<ServiceWorkerRegistration[]> {
-    if (this.serviceWorkerRegistration)
-      return [this.serviceWorkerRegistration];
-    return [];
+    return Array.from(this.serviceWorkerRegistrations.values());
   }
 
-  async register(scriptURL: string, _options?: RegistrationOptions): Promise<ServiceWorkerRegistration> {
+  async register(scriptURL: string, options?: RegistrationOptions): Promise<ServiceWorkerRegistration> {
     if (scriptURL.startsWith('/')) {
       const fakeScriptUrl = new URL(window.location.toString());
       scriptURL = fakeScriptUrl.origin + scriptURL;
@@ -59,14 +73,31 @@ export abstract class MockServiceWorkerContainer implements ServiceWorkerContain
     const mockSw = new MockServiceWorker();
     mockSw.scriptURL = scriptURL;
     mockSw.state = 'activated';
- 
+
     this._controller = mockSw;
 
     const swReg = new MockServiceWorkerRegistration();
     swReg.active = this._controller;
-    this.serviceWorkerRegistration = swReg;
 
-    return this.serviceWorkerRegistration;
+    const scope = MockServiceWorkerContainer.getScopeAsPathname(options);
+    this.serviceWorkerRegistrations.set(scope, swReg);
+
+    return swReg;
+  }
+
+  // RegistrationOptions.scope could be falsely or a string in a URL or pathname format.
+  // This will always give us a pathname, defaulting to "/" if falsely.
+  private static getScopeAsPathname(options?: RegistrationOptions): string {
+    if (!options?.scope) {
+      return "/";
+    }
+
+    try {
+      return new URL(options.scope).pathname;
+    } catch(_e) {
+      // Not a valid URL, assuming it's a path
+      return options.scope;
+    }
   }
 
   removeEventListener<K extends keyof ServiceWorkerContainerEventMap>(type: K, listener: (this: ServiceWorkerContainer, ev: ServiceWorkerContainerEventMap[K]) => any, options?: boolean | EventListenerOptions): void;
@@ -79,4 +110,8 @@ export abstract class MockServiceWorkerContainer implements ServiceWorkerContain
   startMessages(): void {
   }
 
+  mockUnregister(scope: string) {
+    this.serviceWorkerRegistrations.delete(scope);
+  }
+
 }

test/support/mocks/service-workers/models/MockServiceWorkerRegistration.ts

@@ -48,7 +48,7 @@ export class MockServiceWorkerRegistration implements ServiceWorkerRegistration
 
   async unregister(): Promise<boolean> {
     const container = navigator.serviceWorker as MockServiceWorkerContainer;
-    container.serviceWorkerRegistration = null;
+    container.mockUnregister(new URL(this.scope).pathname);
     this.active = null;
     return true;
   }

test/unit/managers/ServiceWorkerManager.ts

@@ -264,6 +264,45 @@ test('installWorker() installs Worker B and then A when Worker A is out of date'
   t.is(spy.callCount, 4);
 });
 
+test('installWorker() installs Worker new scope when it changes', async t => {
+  await TestEnvironment.initialize({
+    httpOrHttps: HttpHttpsEnvironment.Https
+  });
+  sandbox.stub(Notification, "permission").value("granted");
+  // We don't want the version number check from "workerNeedsUpdate" interfering with this test.
+  sandbox.stub(ServiceWorkerManager.prototype, <any>"workerNeedsUpdate").resolves(false);
+
+  const serviceWorkerConfig = {
+    workerAPath: new Path('/Worker-A.js'),
+    workerBPath: new Path('/Worker-B.js'),
+    registrationOptions: { scope: '/' }
+  };
+  const manager = new ServiceWorkerManager(OneSignal.context, serviceWorkerConfig);
+
+  // 1. Install ServiceWorker A and assert it was ServiceWorker A
+  await manager.installWorker();
+
+  // 2. Attempt to install again, but with a different scope
+  serviceWorkerConfig.registrationOptions.scope = '/push/onesignal/';
+  const spyRegister = sandbox.spy(navigator.serviceWorker, 'register');
+  await manager.installWorker();
+
+  // 3. Assert we did register our worker under the new scope.
+  const appId = OneSignal.context.appConfig.appId;
+  t.deepEqual(spyRegister.getCalls().map(call => call.args), [
+    [
+      `https://localhost:3001/Worker-B.js?appId=${appId}`,
+      { scope: 'https://localhost:3001/push/onesignal/' }
+    ]
+  ]);
+
+  // 4. Ensure we kept the original ServiceWorker.
+  //   A. Original could contain more than just OneSignal code
+  //   B. New ServiceWorker instance will have it's own pushToken, this may have not been sent onesignal.com yet.
+  const orgRegistration = await navigator.serviceWorker.getRegistration("/");
+  t.is(new URL(orgRegistration!.scope).pathname, "/");
+});
+
 test('Server worker register URL correct when service worker path is a absolute URL', async t => {
   await TestEnvironment.initialize({
     httpOrHttps: HttpHttpsEnvironment.Https

test/unit/meta/mockServiceWorker.ts

@@ -48,12 +48,33 @@ test('mock service worker registration should return the registered worker', asy
   t.deepEqual(registrations, [registration]);
 });
 
+test('mock service worker getRegistrations should return multiple registered workers', async t => {
+  const expectedRegistrations = [] as ServiceWorkerRegistration[];
+  expectedRegistrations.push(await navigator.serviceWorker.register('/workerA.js', { scope: '/' }));
+  expectedRegistrations.push(await navigator.serviceWorker.register('/workerB.js', { scope: '/mypath/' }));
+
+  const registrations = await navigator.serviceWorker.getRegistrations();
+  t.deepEqual(registrations, expectedRegistrations);
+});
+
+test('mock service worker getRegistration should return higher path worker', async t => {
+  const expected = await navigator.serviceWorker.register('/workerA.js', { scope: '/' });
+  const actual = await navigator.serviceWorker.getRegistration("/some/scope/");
+  t.deepEqual(actual, expected);
+});
+
+test('mock service worker getRegistration should return specific path if a higher path worker exists too', async t => {
+  const expected = await navigator.serviceWorker.register('/workerB.js', { scope: '/mypath/' });
+  await navigator.serviceWorker.register('/workerA.js', { scope: '/' });
+  const actual = await navigator.serviceWorker.getRegistration("/mypath/");
+  t.deepEqual(actual, expected);
+});
 
 test('mock service worker unregistration should return no registered workers', async t => {
   await navigator.serviceWorker.register('/worker.js', { scope: '/' });
 
   const initialRegistration = await navigator.serviceWorker.getRegistration();
-  await initialRegistration.unregister();
+  await initialRegistration!.unregister();
 
   const postUnsubscribeRegistration = await navigator.serviceWorker.getRegistration();
   t.is(postUnsubscribeRegistration, undefined);