[Fix] Outcomes IndexedDB schema

src/shared/helpers/OutcomesHelper.ts

@@ -1,6 +1,6 @@
 import { OutcomesConfig, OutcomeAttribution, OutcomeAttributionType, SentUniqueOutcome } from '../models/Outcomes';
 import { OutcomesNotificationReceived } from "../models/OutcomesNotificationEvents";
-import Database from "../services/Database";
+import Database, { TABLE_OUTCOMES_NOTIFICATION_RECEIVED } from "../services/Database";
 import Log from '../libraries/Log';
 import { Utils } from "../../shared/context/Utils";
 import { logMethodCall, awaitOneSignalInitAndSupported } from '../utils/utils';
@@ -235,7 +235,7 @@ export default class OutcomesHelper {
         const notificationIdsToDelete = allReceivedNotificationSorted
           .filter(notif => matchingNotificationIds.indexOf(notif.notificationId) === -1)
           .map(notif => notif.notificationId);
-        notificationIdsToDelete.forEach(id => Database.remove("NotificationReceived", id));
+        notificationIdsToDelete.forEach(id => Database.remove(TABLE_OUTCOMES_NOTIFICATION_RECEIVED, id));
         Log.debug(`\t${notificationIdsToDelete.length} received notifications will be deleted.`);
 
         if (matchingNotificationIds.length > 0) {

src/shared/services/Database.ts

@@ -40,12 +40,13 @@ interface DatabaseResult {
 
 /**
  * "NotificationOpened" = Pending Notification Click events that haven't fired yet
- * "NotificationClicked" = Outcomes only, notifications part of it's session
- * "NotificationReceived" = Outcomes only, notifications part of it's session
  */
 
+export const TABLE_OUTCOMES_NOTIFICATION_CLICKED = "Outcomes.NotificationClicked";
+export const TABLE_OUTCOMES_NOTIFICATION_RECEIVED = "Outcomes.NotificationReceived";
+
 export type OneSignalDbTable = "Options" | "Ids" | "NotificationOpened" | "Sessions" |
-  "NotificationOpened" | "NotificationReceived" | "NotificationClicked" | "SentUniqueOutcome" |  ModelName;
+  "NotificationOpened" | typeof TABLE_OUTCOMES_NOTIFICATION_RECEIVED | typeof TABLE_OUTCOMES_NOTIFICATION_CLICKED | "SentUniqueOutcome" |  ModelName;
 
 export default class Database {
 
@@ -443,13 +444,13 @@ export default class Database {
   }
 
   async getAllNotificationClickedForOutcomes(): Promise<OutcomesNotificationClicked[]> {
-    const notifications = await this.getAll<NotificationReceivedForOutcomesSchema>("NotificationClicked");
+    const notifications = await this.getAll<NotificationReceivedForOutcomesSchema>(TABLE_OUTCOMES_NOTIFICATION_CLICKED);
     return notifications.map(notification => NotificationClickedForOutcomesSerializer.fromDatabase(notification));
   }
 
   async putNotificationClickedForOutcomes(appId: string, event: NotificationClickEventInternal): Promise<void> {
     await this.put(
-      "NotificationClicked",
+      TABLE_OUTCOMES_NOTIFICATION_CLICKED,
       NotificationClickedForOutcomesSerializer.toDatabase(appId, event)
     );
   }
@@ -476,17 +477,17 @@ export default class Database {
   }
 
   async removeAllNotificationClickedForOutcomes(): Promise<void> {
-    await this.remove("NotificationClicked");
+    await this.remove(TABLE_OUTCOMES_NOTIFICATION_CLICKED);
   }
 
   async getAllNotificationReceivedForOutcomes(): Promise<OutcomesNotificationReceived[]> {
-    const notifications = await this.getAll<NotificationReceivedForOutcomesSchema>("NotificationReceived");
+    const notifications = await this.getAll<NotificationReceivedForOutcomesSchema>(TABLE_OUTCOMES_NOTIFICATION_RECEIVED);
     return notifications.map(notification => NotificationReceivedForOutcomesSerializer.fromDatabase(notification));
   }
 
   async putNotificationReceivedForOutcomes(appId: string, notification: IOSNotification): Promise<void> {
     await this.put(
-      "NotificationReceived",
+      TABLE_OUTCOMES_NOTIFICATION_RECEIVED,
       NotificationReceivedForOutcomesSerializer.toDatabase(appId, notification, new Date().getTime())
     );
   }
@@ -510,8 +511,8 @@ export default class Database {
       Database.singletonInstance.remove("Ids"),
       Database.singletonInstance.remove("NotificationOpened"),
       Database.singletonInstance.remove("Options"),
-      Database.singletonInstance.remove("NotificationReceived"),
-      Database.singletonInstance.remove("NotificationClicked"),
+      Database.singletonInstance.remove(TABLE_OUTCOMES_NOTIFICATION_RECEIVED),
+      Database.singletonInstance.remove(TABLE_OUTCOMES_NOTIFICATION_CLICKED),
       Database.singletonInstance.remove("SentUniqueOutcome")
     ]);
   }

src/shared/services/IndexedDb.ts

@@ -4,7 +4,7 @@ import Utils from '../context/Utils';
 import Emitter from '../libraries/Emitter';
 import Log from '../libraries/Log';
 
-const DATABASE_VERSION = 4;
+const DATABASE_VERSION = 5;
 
 export default class IndexedDb {
 
@@ -29,9 +29,9 @@ export default class IndexedDb {
       if (!request) {
         return null;
       }
-      request.onerror = this.onDatabaseOpenError;
-      request.onblocked = this.onDatabaseOpenBlocked;
-      request.onupgradeneeded = this.onDatabaseUpgradeNeeded;
+      request.onerror = this.onDatabaseOpenError.bind(this);
+      request.onblocked = this.onDatabaseOpenBlocked.bind(this);
+      request.onupgradeneeded = this.onDatabaseUpgradeNeeded.bind(this);
       request.onsuccess = () => {
         this.database = request.result;
         this.database.onerror = this.onDatabaseError;
@@ -102,7 +102,12 @@ export default class IndexedDb {
    */
   private onDatabaseUpgradeNeeded(event: IDBVersionChangeEvent): void {
     Log.debug('IndexedDb: Database is being rebuilt or upgraded (upgradeneeded event).');
-    const db = (event.target as IDBOpenDBRequest).result;
+    const target = event.target as IDBOpenDBRequest;
+    const transaction = target.transaction;
+    if (!transaction) {
+      throw Error("Can't migrate DB without a transaction");
+    }
+    const db = target.result;
     if (event.oldVersion < 1) {
       db.createObjectStore("Ids", { keyPath: "type" });
       db.createObjectStore("NotificationOpened", { keyPath: "url" });
@@ -111,7 +116,12 @@ export default class IndexedDb {
     if (event.oldVersion < 2) {
       db.createObjectStore("Sessions", { keyPath: "sessionKey" });
       db.createObjectStore("NotificationReceived", { keyPath: "notificationId" });
-      db.createObjectStore("NotificationClicked", { keyPath: "notification.id" });
+      // NOTE: 160000.beta4 to 160000 releases modified this line below as
+      // "{ keyPath: "notification.id" }". This resulted in DB v4 either
+      // having "notificationId" or "notification.id" depending if the visitor
+      // was new while this version was live.
+      // DB v5 was create to trigger a migration to fix this bug.
+      db.createObjectStore("NotificationClicked", { keyPath: "notificationId" });
     }
     if (event.oldVersion < 3) {
       db.createObjectStore("SentUniqueOutcome", { keyPath: "outcomeName" });
@@ -124,6 +134,10 @@ export default class IndexedDb {
       db.createObjectStore(ModelName.EmailSubscriptions, { keyPath: "modelId" });
     }
     if (event.oldVersion < 5) {
+      this.migrateOutcomesNotificationClickedTableForV5(db, transaction);
+      this.migrateOutcomesNotificationReceivedTableForV5(db, transaction);
+    }
+    if (event.oldVersion < 6) {
       // Make sure to update the database version at the top of the file
     }
     // Wrap in conditional for tests
@@ -132,6 +146,74 @@ export default class IndexedDb {
     }
   }
 
+  // Table rename "NotificationClicked" -> "Outcomes.NotificationClicked"
+  // and migrate existing records.
+  // Motivation: This is done to correct the keyPath, you can't change it
+  // so a new table must be created.
+  // Background: Table was created with wrong keyPath of "notification.id"
+  // for new visitors for versions 160000.beta4 to 160000. Writes were
+  // attempted as "notificationId" in released 160000 however they may
+  // have failed if the visitor was new when those releases were in the wild.
+  // However those new on 160000.beta4 to 160000.beta8 will have records
+  // saved as "notification.id" that will be converted here.
+  private migrateOutcomesNotificationClickedTableForV5(
+    db: IDBDatabase,
+    transaction: IDBTransaction,
+  ) {
+    const newTableName = "Outcomes.NotificationClicked";
+    db.createObjectStore(newTableName, { keyPath: "notificationId" });
+
+    const oldTableName = "NotificationClicked"
+    const cursor = transaction.objectStore(oldTableName).openCursor();
+    cursor.onsuccess = (event: any) => { // Using any here as the TypeScript definition is wrong
+      const cursorResult = event.target.result as IDBCursorWithValue;
+      if (!cursorResult) {
+        // Delete old table once we have gone through all records
+        db.deleteObjectStore(oldTableName);
+        return;
+      }
+      const oldValue = cursorResult.value;
+      transaction
+        .objectStore(newTableName)
+        .put({
+          // notification.id was possible from 160000.beta4 to 160000.beta8
+          notificationId: oldValue.notificationId || oldValue.notification.id,
+          appId: oldValue.appId,
+          timestamp: oldValue.timestamp,
+        });
+      cursorResult.continue();
+    };
+    cursor.onerror = () => { throw cursor.error; };
+  }
+
+  // Table rename "NotificationReceived" -> "Outcomes.NotificationReceived"
+  // and migrate existing records.
+  // Motivation: Consistency of using pre-fix "Outcomes." like we have for
+  // the "Outcomes.NotificationClicked" table.
+  private migrateOutcomesNotificationReceivedTableForV5(
+    db: IDBDatabase,
+    transaction: IDBTransaction,
+  ) {
+    const newTableName = "Outcomes.NotificationReceived";
+    db.createObjectStore(newTableName, { keyPath: "notificationId" });
+
+    const oldTableName = "NotificationReceived"
+    const cursor = transaction.objectStore(oldTableName).openCursor();
+    cursor.onsuccess = (event: any) => { // Using any here as the TypeScript definition is wrong
+      const cursorResult = event.target.result as IDBCursorWithValue;
+      if (!cursorResult) {
+        // Delete old table once we have gone through all records
+        db.deleteObjectStore(oldTableName);
+        return;
+      }
+      transaction
+        .objectStore(newTableName)
+        .put(cursorResult.value);
+      cursorResult.continue();
+    };
+    cursor.onerror = () => { throw cursor.error; };
+  }
+
   /**
    * Asynchronously retrieves the value of the key at the table (if key is specified), or the entire table
    * (if key is not specified).

test/support/tester/OutcomeTestHelper.ts

@@ -1,5 +1,5 @@
 import Random from '../../support/tester/Random';
-import Database from '../../../src/shared/services/Database';
+import Database, { TABLE_OUTCOMES_NOTIFICATION_RECEIVED } from '../../../src/shared/services/Database';
 import { initializeNewSession, Session } from '../../../src/shared/models/Session';
 import { OutcomesNotificationClicked, OutcomesNotificationReceived } from "src/shared/models/OutcomesNotificationEvents";
 
@@ -24,7 +24,7 @@ export default class OutcomeTestHelper {
       if (notificationReceived.timestamp >= maxTimestamp && receivedNotificationIdsWithinTimeframe.length < limit) {
         receivedNotificationIdsWithinTimeframe.push(notificationReceived.notificationId);
       }
-      await Database.put("NotificationReceived", notificationReceived);
+      await Database.put(TABLE_OUTCOMES_NOTIFICATION_RECEIVED, notificationReceived);
     }
 
     return receivedNotificationIdsWithinTimeframe;

test/unit/public-sdk-apis/sendOutcome.ts

@@ -5,7 +5,7 @@ import { OutcomeRequestData } from "../../../src/page/models/OutcomeRequestData"
 import { DeliveryPlatformKind } from "../../../src/shared/models/DeliveryPlatformKind";
 import { SubscriptionStateKind } from "../../../src/shared/models/SubscriptionStateKind";
 import Log from "../../../src/shared/libraries/Log";
-import Database from "../../../src/shared/services/Database";
+import Database, { TABLE_OUTCOMES_NOTIFICATION_CLICKED } from "../../../src/shared/services/Database";
 import timemachine from "timemachine";
 import OutcomeTestHelper from '../../support/tester/OutcomeTestHelper';
 import OneSignalApiShared from "../../../src/shared/api/OneSignalApiShared";
@@ -111,7 +111,7 @@ test("when outcome is unattributed and feature enabled and has weight it sends a
 
 test("when outcome is direct and feature enabled it sends an api call", async t => {
   const notificationClicked = OutcomeTestHelper.generateNotification();
-  await Database.put("NotificationClicked", notificationClicked);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_CLICKED, notificationClicked);
   const apiSpy = sinonSandbox.stub(OneSignalApiShared, "sendOutcome").resolves();
   sinonSandbox.stub(OneSignal, "privateIsPushNotificationsEnabled").resolves(true);
   sinonSandbox.stub(MainHelper, "getCurrentNotificationType").resolves(SubscriptionStateKind.Subscribed);
@@ -134,7 +134,7 @@ test("when outcome is direct and feature disabled there are no api calls", async
   OneSignal.config!.userConfig.outcomes!.unattributed.enabled = false;
 
   const notificationClicked = OutcomeTestHelper.generateNotification();
-  await Database.put("NotificationClicked", notificationClicked);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_CLICKED, notificationClicked);
   const apiSpy = sinonSandbox.stub(OneSignalApiShared, "sendOutcome").resolves();
   sinonSandbox.stub(OneSignal, "privateIsPushNotificationsEnabled").resolves(true);
   sinonSandbox.stub(MainHelper, "getCurrentNotificationType").resolves(SubscriptionStateKind.Subscribed);
@@ -145,7 +145,7 @@ test("when outcome is direct and feature disabled there are no api calls", async
 
 test("when outcome is direct and feature enabled and has weight it sends an api call", async t => {
   const notificationClicked = OutcomeTestHelper.generateNotification();
-  await Database.put("NotificationClicked", notificationClicked);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_CLICKED, notificationClicked);
   const apiSpy = sinonSandbox.stub(OneSignalApiShared, "sendOutcome").resolves();
   sinonSandbox.stub(OneSignal, "privateIsPushNotificationsEnabled").resolves(true);
   sinonSandbox.stub(MainHelper, "getCurrentNotificationType").resolves(SubscriptionStateKind.Subscribed);

test/unit/public-sdk-apis/sendUniqueOutcome.ts

@@ -6,7 +6,7 @@ import { OutcomeRequestData } from "../../../src/page/models/OutcomeRequestData"
 import { DeliveryPlatformKind } from "../../../src/shared/models/DeliveryPlatformKind";
 import { SubscriptionStateKind } from "../../../src/shared/models/SubscriptionStateKind";
 import Log from "../../../src/shared/libraries/Log";
-import Database from "../../../src/shared/services/Database";
+import Database, { TABLE_OUTCOMES_NOTIFICATION_CLICKED, TABLE_OUTCOMES_NOTIFICATION_RECEIVED } from "../../../src/shared/services/Database";
 import timemachine from "timemachine";
 import { SentUniqueOutcome } from '../../../src/shared/models/Outcomes';
 import OutcomeTestHelper from '../../support/tester/OutcomeTestHelper';
@@ -87,7 +87,7 @@ test("when outcome is unattributed and feature disabled there are no api calls",
 
 test("when outcome is direct and feature enabled it sends an api call", async t => {
   const notificationClicked = OutcomeTestHelper.generateNotification();
-  await Database.put("NotificationClicked", notificationClicked);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_CLICKED, notificationClicked);
   const apiSpy = sinonSandbox.stub(OneSignalApiShared, "sendOutcome").resolves();
   sinonSandbox.stub(OneSignal, "privateIsPushNotificationsEnabled").resolves(true);
   sinonSandbox.stub(MainHelper, "getCurrentNotificationType").resolves(SubscriptionStateKind.Subscribed);
@@ -109,7 +109,7 @@ test("when outcome is direct and feature disabled there are no api calls", async
   OneSignal.config!.userConfig.outcomes!.unattributed.enabled = false;
 
   const notificationClicked = OutcomeTestHelper.generateNotification();
-  await Database.put("NotificationClicked", notificationClicked);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_CLICKED, notificationClicked);
   const apiSpy = sinonSandbox.stub(OneSignalApiShared, "sendOutcome").resolves();
   sinonSandbox.stub(OneSignal, "privateIsPushNotificationsEnabled").resolves(true);
   sinonSandbox.stub(MainHelper, "getCurrentNotificationType").resolves(SubscriptionStateKind.Subscribed);
@@ -155,7 +155,7 @@ test("when outcome is indirect and feature disabled there are no api calls", asy
 
 test("when direct outcome is sent twice, there is only one api call", async t => {
   const notificationClicked = OutcomeTestHelper.generateNotification();
-  await Database.put("NotificationClicked", notificationClicked);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_CLICKED, notificationClicked);
   const apiSpy = sinonSandbox.stub(OneSignalApiShared, "sendOutcome").resolves();
   const logSpy = sinonSandbox.stub(Log, "warn");
   sinonSandbox.stub(OneSignal, "privateIsPushNotificationsEnabled").resolves(true);
@@ -228,11 +228,11 @@ test("attribution of an outcome with multiple notifications happens correctly",
   const notificationArr = [];
   let notificationReceived = OutcomeTestHelper.generateNotification();
   notificationArr.push(notificationReceived.notificationId);
-  await Database.put("NotificationReceived", notificationReceived);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_RECEIVED, notificationReceived);
   await OneSignal.sendUniqueOutcome(OUTCOME_NAME);
   notificationReceived = OutcomeTestHelper.generateNotification();
   notificationArr.push(notificationReceived.notificationId);
-  await Database.put("NotificationReceived", notificationReceived);
+  await Database.put(TABLE_OUTCOMES_NOTIFICATION_RECEIVED, notificationReceived);
   await OneSignal.sendUniqueOutcome(OUTCOME_NAME);
 
   const sentOutcome = await Database.get<SentUniqueOutcome>("SentUniqueOutcome", OUTCOME_NAME);