From 6a0e78debf4640504fafb371c3186263a9cfe883 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Tue, 4 Jun 2024 12:47:23 -0400 Subject: [PATCH 01/15] Add delta queue to operation repo Added a delta queue to the operation repo to flush all executor deltas at the same time. The operation repo's delta queue enqueues all deltas and then will sort and pass the delta to its correct executor. Previously, each executor was in charge of enqueuing and flushing their own delta queue. This change also aligns the Web SDK more with the iOS SDK's operation repo implementation. --- __test__/support/constants.ts | 2 +- src/core/executors/ExecutorBase.ts | 7 ----- src/core/operationRepo/OperationRepo.ts | 36 ++++++++++++++++++++++--- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/__test__/support/constants.ts b/__test__/support/constants.ts index 2df75902e..aee1eee94 100644 --- a/__test__/support/constants.ts +++ b/__test__/support/constants.ts @@ -14,7 +14,7 @@ * @constant {number} DELTA_QUEUE_TIME_ADVANCE - The time advance for the delta queue. */ export const OPERATION_QUEUE_TIME_ADVANCE = 5001; -export const DELTA_QUEUE_TIME_ADVANCE = 1001; +export const DELTA_QUEUE_TIME_ADVANCE = 5001; /* S T R I N G C O N S T A N T S */ export const APP_ID = '34fcbe85-278d-4fd2-a4ec-0f80e95072c5'; diff --git a/src/core/executors/ExecutorBase.ts b/src/core/executors/ExecutorBase.ts index e794b9d9d..e68284cf6 100644 --- a/src/core/executors/ExecutorBase.ts +++ b/src/core/executors/ExecutorBase.ts @@ -28,17 +28,10 @@ export default abstract class ExecutorBase { private onlineStatus = true; - static DELTAS_BATCH_PROCESSING_TIME = 1; static OPERATIONS_BATCH_PROCESSING_TIME = 5; static RETRY_COUNT = 5; constructor(executorConfig: ExecutorConfig) { - setInterval(() => { - if (this._deltaQueue.length > 0) { - this.processDeltaQueue.call(this); - } - }, ExecutorBase.DELTAS_BATCH_PROCESSING_TIME * 1_000); - setInterval(() => { Log.debug('OneSignal: checking for operations to process from cache'); const cachedOperations = this.getOperationsFromCache(); diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index 2fdf8a120..745db9052 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -7,6 +7,8 @@ import { logMethodCall } from '../../shared/utils/utils'; export class OperationRepo { public executorStore: ExecutorStore; private _unsubscribeFromModelRepo: () => void; + private _deltaQueue: CoreDelta[] = []; + static DELTAS_BATCH_PROCESSING_TIME = 5; constructor(private modelRepo: ModelRepo) { this.executorStore = new ExecutorStore(); @@ -16,6 +18,12 @@ export class OperationRepo { this._processDelta(delta); }, ); + + setInterval(() => { + if (this._deltaQueue.length > 0) { + this._processDeltaQueue(); + } + }, OperationRepo.DELTAS_BATCH_PROCESSING_TIME * 1_000); } setModelRepoAndResubscribe(modelRepo: ModelRepo) { @@ -33,9 +41,31 @@ export class OperationRepo { this.executorStore.forceDeltaQueueProcessingOnAllExecutors(); } + private _flushDeltas(): void { + logMethodCall('OperationRepo._flushDeltas'); + this._deltaQueue = []; + } + private _processDelta(delta: CoreDelta): void { - logMethodCall('processDelta', { delta }); - const { modelName } = delta.model; - this.executorStore.store[modelName]?.enqueueDelta(delta); + logMethodCall('OperationRepo._processDelta', { delta }); + const deltaCopy = JSON.parse(JSON.stringify(delta)); + this._deltaQueue.push(deltaCopy); + } + + private _processDeltaQueue(): void { + logMethodCall('OperationRepo._processDeltaQueue'); + + this._deltaQueue.forEach((delta) => { + const { modelName } = delta.model; + this.executorStore.store[modelName]?.enqueueDelta(delta); + }); + + // for each executor + // TODO: fires SubscriptionExecutor.processDeltaQueue and SubscriptionExecutor._flushDeltas 3 times + // ExecutorStore has 3 ModelName for Subscriptions: smsSubscription, emailSubscription, pushSubscription + this.forceDeltaQueueProcessingOnAllExecutors(); + // executors flush is in the above method + + this._flushDeltas(); } } From f19842238df9caea688b63dc0d571ec332815f2b Mon Sep 17 00:00:00 2001 From: Shepherd Date: Wed, 5 Jun 2024 19:34:40 -0400 Subject: [PATCH 02/15] Change push, sms, email to use the same executor Change pushSubscription, smsSubscription, and emailSubscription to point to the same SubscriptionExecutor. This way the SubscriptionExecutor only fires once instead of 3 times. --- src/core/executors/ExecutorFactory.ts | 9 ++++++++- src/core/executors/ExecutorStore.ts | 13 +++++++++++++ src/core/operationRepo/OperationRepo.ts | 5 +---- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/core/executors/ExecutorFactory.ts b/src/core/executors/ExecutorFactory.ts index 32ed9c1c9..5703e41ec 100644 --- a/src/core/executors/ExecutorFactory.ts +++ b/src/core/executors/ExecutorFactory.ts @@ -6,6 +6,8 @@ import { PropertiesExecutor } from './PropertiesExecutor'; import { SubscriptionExecutor } from './SubscriptionExecutor'; export class ExecutorFactory { + static subscriptionExecutor?: SubscriptionExecutor = undefined; + static build(executorConfig: ExecutorConfig): Executor { switch (executorConfig.modelName) { case ModelName.Identity: @@ -15,7 +17,12 @@ export class ExecutorFactory { case ModelName.PushSubscriptions: case ModelName.EmailSubscriptions: case ModelName.SmsSubscriptions: - return new SubscriptionExecutor(executorConfig); + if (!ExecutorFactory.subscriptionExecutor) { + ExecutorFactory.subscriptionExecutor = new SubscriptionExecutor( + executorConfig, + ); + } + return ExecutorFactory.subscriptionExecutor; } } } diff --git a/src/core/executors/ExecutorStore.ts b/src/core/executors/ExecutorStore.ts index 09a25e594..8458aacdd 100644 --- a/src/core/executors/ExecutorStore.ts +++ b/src/core/executors/ExecutorStore.ts @@ -19,8 +19,21 @@ export class ExecutorStore { // call processDeltaQueue on all executors immediately public forceDeltaQueueProcessingOnAllExecutors(): void { + let didSubscriptionExecutorProcessDeltaQueue = false; + Object.values(this.store).forEach((executor) => { + if ( + executor instanceof SubscriptionExecutor && + didSubscriptionExecutorProcessDeltaQueue + ) { + return; + } + executor.processDeltaQueue(); + + if (executor instanceof SubscriptionExecutor) { + didSubscriptionExecutorProcessDeltaQueue = true; + } }); } } diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index 745db9052..7fed3b9a8 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -60,11 +60,8 @@ export class OperationRepo { this.executorStore.store[modelName]?.enqueueDelta(delta); }); - // for each executor - // TODO: fires SubscriptionExecutor.processDeltaQueue and SubscriptionExecutor._flushDeltas 3 times - // ExecutorStore has 3 ModelName for Subscriptions: smsSubscription, emailSubscription, pushSubscription + // for each executor: processDeltaQueue and flush this.forceDeltaQueueProcessingOnAllExecutors(); - // executors flush is in the above method this._flushDeltas(); } From 1861c31779d9e11337400de203eab03dbf586444 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Wed, 5 Jun 2024 20:54:25 -0400 Subject: [PATCH 03/15] Clear operation queue for operation repo tests Running tests consecutively that enqueue operations to the SubscriptionExecutor will fail due to the operationQueue length not matching the expected value. pushSubscription, smsSubscription, emailSubscription now point to the same SubscriptionExecutor and the operationQueue was not being cleared for the next test. The other tests don't fail but I also cleared the operation queue there too. --- __test__/unit/core/operationRepo.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/__test__/unit/core/operationRepo.test.ts b/__test__/unit/core/operationRepo.test.ts index c197930f9..9c920e223 100644 --- a/__test__/unit/core/operationRepo.test.ts +++ b/__test__/unit/core/operationRepo.test.ts @@ -79,6 +79,8 @@ describe('OperationRepo tests', () => { const executor = operationRepo?.executorStore.store[ModelName.EmailSubscriptions]; + executor._operationQueue = []; + modelRepo?.subscribe(() => { broadcastCount += 1; passIfBroadcastNTimes(1, broadcastCount, done); @@ -108,6 +110,8 @@ describe('OperationRepo tests', () => { const executor = operationRepo?.executorStore.store[ModelName.EmailSubscriptions]; + executor._operationQueue = []; + const processDeltaSpy = jest.spyOn( OperationRepo.prototype as any, '_processDelta', @@ -144,6 +148,8 @@ describe('OperationRepo tests', () => { const { modelRepo, operationRepo } = OneSignal.coreDirector.core; const executor = operationRepo?.executorStore.store[ModelName.Identity]; + executor._operationQueue = []; + const processDeltaSpy = jest.spyOn( OperationRepo.prototype as any, '_processDelta', @@ -211,6 +217,8 @@ describe('OperationRepo tests', () => { const { modelRepo, operationRepo } = OneSignal.coreDirector.core; const executor = operationRepo?.executorStore.store[ModelName.Properties]; + executor._operationQueue = []; + const processDeltaSpy = jest.spyOn( OperationRepo.prototype as any, '_processDelta', From 57211dc234ee0358e7dd940143a16aa080527081 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Wed, 10 Jul 2024 23:00:17 -0400 Subject: [PATCH 04/15] Add external id property to operations Will be used to retry operations for users with a valid Jwt token. Accessed through Operation.model.externalId --- src/core/CoreModuleDirector.ts | 23 ++++++++++++++++++++++- src/core/caching/EncodedModel.ts | 1 + src/core/modelRepo/OSModel.ts | 14 ++++++++++++-- src/onesignal/User.ts | 16 ++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index 837d2c45d..b513600fc 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -41,6 +41,13 @@ export class CoreModuleDirector { if (user.hasOneSignalId) { pushModel.setOneSignalId(user.onesignalId); } + + const identity = this.getIdentityModel(); + const externalId = identity?.data.external_id; + if (externalId) { + pushModel.setExternalId(externalId); + } + // don't propagate since we will be including the subscription in the user create call OneSignal.coreDirector.add( ModelName.PushSubscriptions, @@ -59,7 +66,8 @@ export class CoreModuleDirector { const identity = this.getIdentityModel(); const properties = this.getPropertiesModel(); - const { onesignal_id: onesignalId } = user.identity; + const { onesignal_id: onesignalId, external_id: externalId } = + user.identity; if (!onesignalId) { throw new OneSignalError('OneSignal ID is missing from user data'); @@ -69,6 +77,11 @@ export class CoreModuleDirector { identity?.setOneSignalId(onesignalId); properties?.setOneSignalId(onesignalId); + if (externalId) { + identity?.setExternalId(externalId); + properties?.setExternalId(externalId); + } + // identity and properties models are always single, so we hydrate immediately (i.e. replace existing data) identity?.hydrate(user.identity); properties?.hydrate(user.properties); @@ -78,6 +91,7 @@ export class CoreModuleDirector { this._hydrateSubscriptions( user.subscriptions as SubscriptionModel[], onesignalId, + externalId, ); EventHelper.checkAndTriggerUserChanged(); } catch (e) { @@ -88,6 +102,7 @@ export class CoreModuleDirector { private _hydrateSubscriptions( subscriptions: SubscriptionModel[], onesignalId: string, + externalId?: string, ): void { logMethodCall('CoreModuleDirector._hydrateSubscriptions', { subscriptions, @@ -122,10 +137,16 @@ export class CoreModuleDirector { if (existingSubscription) { // set onesignalId on existing subscription *before* hydrating so that the onesignalId is updated in model cache existingSubscription.setOneSignalId(onesignalId); + if (externalId) { + existingSubscription?.setExternalId(externalId); + } existingSubscription.hydrate(subscription); } else { const model = new OSModel(modelName, subscription); model.setOneSignalId(onesignalId); + if (externalId) { + model?.setExternalId(externalId); + } modelStores[modelName].add(model, false); // don't propagate to server } }); diff --git a/src/core/caching/EncodedModel.ts b/src/core/caching/EncodedModel.ts index 0fba54c77..e8daebf11 100644 --- a/src/core/caching/EncodedModel.ts +++ b/src/core/caching/EncodedModel.ts @@ -8,5 +8,6 @@ export default interface EncodedModel { modelId: string; modelName: ModelName; onesignalId?: string; + externalId?: string; [key: string]: unknown; } diff --git a/src/core/modelRepo/OSModel.ts b/src/core/modelRepo/OSModel.ts index 598361d04..3536324f1 100644 --- a/src/core/modelRepo/OSModel.ts +++ b/src/core/modelRepo/OSModel.ts @@ -17,6 +17,7 @@ export class OSModel extends Subscribable> { onesignalId?: string; awaitOneSignalIdAvailable: Promise; onesignalIdAvailableCallback?: (onesignalId: string) => void; + externalId?: string; constructor( readonly modelName: ModelName, @@ -28,6 +29,7 @@ export class OSModel extends Subscribable> { this.modelName = modelName; this.data = data; this.onesignalId = undefined; + this.externalId = undefined; this.awaitOneSignalIdAvailable = new Promise((resolve) => { this.onesignalIdAvailableCallback = resolve; @@ -48,6 +50,11 @@ export class OSModel extends Subscribable> { } } + public setExternalId(externalId?: string): void { + logMethodCall('setExternalId', { externalId }); + this.externalId = externalId; + } + /** * We use this method to update the model data. * Results in a broadcasted update event. @@ -92,7 +99,8 @@ export class OSModel extends Subscribable> { const modelId = this.modelId as string; const modelName = this.modelName; const onesignalId = this.onesignalId; - return { modelId, modelName, onesignalId, ...this.data }; + const externalId = this.externalId; + return { modelId, modelName, onesignalId, externalId, ...this.data }; } /** @@ -102,7 +110,8 @@ export class OSModel extends Subscribable> { */ static decode(encodedModel: EncodedModel): OSModel { logMethodCall('decode', { encodedModel }); - const { modelId, modelName, onesignalId, ...data } = encodedModel; + const { modelId, modelName, onesignalId, externalId, ...data } = + encodedModel; const decodedModel = new OSModel( modelName as ModelName, @@ -111,6 +120,7 @@ export class OSModel extends Subscribable> { ); decodedModel.setOneSignalId(onesignalId); + decodedModel.setExternalId(externalId); return decodedModel; } } diff --git a/src/onesignal/User.ts b/src/onesignal/User.ts index 695aa8668..b7d8feaf2 100644 --- a/src/onesignal/User.ts +++ b/src/onesignal/User.ts @@ -145,6 +145,10 @@ export default class User { ) { // existing user newSubscription.setOneSignalId(User.singletonInstance?.onesignalId); + const identityModel = OneSignal.coreDirector.getIdentityModel(); + if (identityModel.data.external_id) { + newSubscription.setExternalId(identityModel.data.external_id); + } OneSignal.coreDirector.add( ModelName.EmailSubscriptions, newSubscription, @@ -165,6 +169,10 @@ export default class User { throw e; }, ); + const identityModel = OneSignal.coreDirector.getIdentityModel(); + if (identityModel.data.external_id) { + newSubscription.setExternalId(identityModel.data.external_id); + } } public async addSms(sms: string): Promise { @@ -194,6 +202,10 @@ export default class User { ) { // existing user newSubscription.setOneSignalId(User.singletonInstance?.onesignalId); + const identityModel = OneSignal.coreDirector.getIdentityModel(); + if (identityModel.data.external_id) { + newSubscription.setExternalId(identityModel.data.external_id); + } OneSignal.coreDirector.add( ModelName.SmsSubscriptions, newSubscription, @@ -214,6 +226,10 @@ export default class User { throw e; }, ); + const identityModel = OneSignal.coreDirector.getIdentityModel(); + if (identityModel.data.external_id) { + newSubscription.setExternalId(identityModel.data.external_id); + } } public removeEmail(email: string): void { From 552c7aff580885e400559f282b83bf4f365785e2 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Thu, 18 Jul 2024 12:13:14 -0400 Subject: [PATCH 05/15] Reset external id on logout External id from previously logged in user was kept on anonymous user's push subscription model when logged out --- src/page/managers/LoginManager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index c40443f75..0b3bc3e5f 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -142,6 +142,7 @@ export default class LoginManager { UserDirector.resetUserMetaProperties(); const pushSubModel = await OneSignal.coreDirector.getPushSubscriptionModel(); + pushSubModel?.setExternalId(undefined); await OneSignal.coreDirector.resetModelRepoAndCache(); // Initialize as a local User, as we don't have a push subscription to create a remote anonymous user. From 6746365aa7b33af7e1b36e523bc668622687b61a Mon Sep 17 00:00:00 2001 From: Shepherd Date: Thu, 18 Jul 2024 12:17:28 -0400 Subject: [PATCH 06/15] Change delta queue time to be less than op queue Change delta queue time back to 1 second. Was creating misalignment bugs due to the time being the same --- __test__/support/constants.ts | 2 +- src/core/operationRepo/OperationRepo.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/__test__/support/constants.ts b/__test__/support/constants.ts index aee1eee94..2df75902e 100644 --- a/__test__/support/constants.ts +++ b/__test__/support/constants.ts @@ -14,7 +14,7 @@ * @constant {number} DELTA_QUEUE_TIME_ADVANCE - The time advance for the delta queue. */ export const OPERATION_QUEUE_TIME_ADVANCE = 5001; -export const DELTA_QUEUE_TIME_ADVANCE = 5001; +export const DELTA_QUEUE_TIME_ADVANCE = 1001; /* S T R I N G C O N S T A N T S */ export const APP_ID = '34fcbe85-278d-4fd2-a4ec-0f80e95072c5'; diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index 7fed3b9a8..74c30409e 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -8,7 +8,7 @@ export class OperationRepo { public executorStore: ExecutorStore; private _unsubscribeFromModelRepo: () => void; private _deltaQueue: CoreDelta[] = []; - static DELTAS_BATCH_PROCESSING_TIME = 5; + static DELTAS_BATCH_PROCESSING_TIME = 1; constructor(private modelRepo: ModelRepo) { this.executorStore = new ExecutorStore(); From 1bb4bb0692c606c09285c944e376cb59c14c8a14 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Thu, 1 Aug 2024 10:29:57 -0400 Subject: [PATCH 07/15] Move all subscriptions under 1 model name Refactors ModelName enum for SmsSubscription, EmailSubscription, PushSubscription to 1 name called Subscriptions. This omits adding multiple subscription executors for sms, email, push and only creates 1 executor because all subscriptions are grouped together. --- src/core/CoreModuleDirector.ts | 100 ++++++++++++++------- src/core/executors/ExecutorConfigMap.ts | 12 +-- src/core/executors/ExecutorFactory.ts | 13 +-- src/core/executors/ExecutorStore.ts | 13 --- src/core/executors/SubscriptionExecutor.ts | 12 +-- src/core/models/SupportedModels.ts | 5 +- src/onesignal/User.ts | 17 ++-- src/page/managers/LoginManager.ts | 2 +- src/shared/services/IndexedDb.ts | 68 ++++++++++++-- src/sw/helpers/ModelCacheDirectAccess.ts | 2 +- 10 files changed, 152 insertions(+), 92 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index b513600fc..f94520361 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -6,6 +6,7 @@ import { SupportedIdentity } from './models/IdentityModel'; import { ModelStoresMap } from './models/ModelStoresMap'; import { SubscriptionModel, + SubscriptionType, SupportedSubscription, } from './models/SubscriptionModels'; import { ModelName, SupportedModel } from './models/SupportedModels'; @@ -33,7 +34,7 @@ export class CoreModuleDirector { }); // new subscription const pushModel = new OSModel( - ModelName.PushSubscriptions, + ModelName.Subscriptions, new FuturePushSubscriptionRecord(rawPushSubscription).serialize(), ); @@ -50,7 +51,7 @@ export class CoreModuleDirector { // don't propagate since we will be including the subscription in the user create call OneSignal.coreDirector.add( - ModelName.PushSubscriptions, + ModelName.Subscriptions, pushModel as OSModel, false, ); @@ -114,24 +115,16 @@ export class CoreModuleDirector { const modelStores = this.getModelStores(); - const getModelName = (subscription: SupportedSubscription) => { - if (subscription.type === 'Email') { - return ModelName.EmailSubscriptions; - } else if (subscription.type === 'SMS') { - return ModelName.SmsSubscriptions; - } else { - return ModelName.PushSubscriptions; - } - }; - subscriptions.forEach(async (subscription) => { - const modelName = getModelName(subscription); /* We use the token to identify the model because the subscription ID is not set until the server responds. * So when we initially hydrate after init, we may already have a push model with a token, but no ID. * We don't want to create a new model in this case, so we use the token to identify the model. */ const existingSubscription = !!subscription.token - ? this.getSubscriptionOfTypeWithToken(modelName, subscription.token) + ? this.getSubscriptionOfTypeWithToken( + subscription.type, + subscription.token, + ) : undefined; if (existingSubscription) { @@ -142,12 +135,15 @@ export class CoreModuleDirector { } existingSubscription.hydrate(subscription); } else { - const model = new OSModel(modelName, subscription); + const model = new OSModel( + ModelName.Subscriptions, + subscription, + ); model.setOneSignalId(onesignalId); if (externalId) { model?.setExternalId(externalId); } - modelStores[modelName].add(model, false); // don't propagate to server + modelStores[ModelName.Subscriptions].add(model, false); // don't propagate to server } }); } @@ -195,13 +191,21 @@ export class CoreModuleDirector { } { logMethodCall('CoreModuleDirector.getEmailSubscriptionModels'); const modelStores = this.getModelStores(); - return modelStores.emailSubscriptions.models as { + const subscriptionModels = modelStores.subscriptions.models as { [key: string]: OSModel; }; + + const emailSubscriptions = Object.fromEntries( + Object.entries(subscriptionModels).filter( + ([, model]) => model.data?.type === SubscriptionType.Email, + ), + ); + + return emailSubscriptions; } public async hasEmail(): Promise { - const emails = await this.getEmailSubscriptionModels(); + const emails = this.getEmailSubscriptionModels(); return Object.keys(emails).length > 0; } @@ -210,13 +214,21 @@ export class CoreModuleDirector { } { logMethodCall('CoreModuleDirector.getSmsSubscriptionModels'); const modelStores = this.getModelStores(); - return modelStores.smsSubscriptions.models as { + const subscriptionModels = modelStores.subscriptions.models as { [key: string]: OSModel; }; + + const smsSubscriptions = Object.fromEntries( + Object.entries(subscriptionModels).filter( + ([, model]) => model.data?.type === SubscriptionType.SMS, + ), + ); + + return smsSubscriptions; } public async hasSms(): Promise { - const smsModels = await this.getSmsSubscriptionModels(); + const smsModels = this.getSmsSubscriptionModels(); return Object.keys(smsModels).length > 0; } @@ -228,9 +240,17 @@ export class CoreModuleDirector { } { logMethodCall('CoreModuleDirector.getAllPushSubscriptionModels'); const modelStores = this.getModelStores(); - return modelStores.pushSubscriptions.models as { + const subscriptionModels = modelStores.subscriptions.models as { [key: string]: OSModel; }; + + const pushSubscriptions = Object.fromEntries( + Object.entries(subscriptionModels).filter(([, model]) => + this.isPushSubscriptionType(model.data?.type), + ), + ); + + return pushSubscriptions; } private async getPushSubscriptionModelByCurrentToken(): Promise< @@ -239,9 +259,9 @@ export class CoreModuleDirector { logMethodCall('CoreModuleDirector.getPushSubscriptionModelByCurrentToken'); const pushToken = await MainHelper.getCurrentPushToken(); if (pushToken) { - return this.getSubscriptionOfTypeWithToken( - ModelName.PushSubscriptions, - pushToken, + const pushSubscriptions = this.getAllPushSubscriptionModels(); + return Object.values(pushSubscriptions).find( + (subscription) => subscription.data.token === pushToken, ); } return undefined; @@ -257,9 +277,9 @@ export class CoreModuleDirector { ); const { lastKnownPushToken } = await Database.getAppState(); if (lastKnownPushToken) { - return this.getSubscriptionOfTypeWithToken( - ModelName.PushSubscriptions, - lastKnownPushToken, + const pushSubscriptions = this.getAllPushSubscriptionModels(); + return Object.values(pushSubscriptions).find( + (subscription) => subscription.data.token === lastKnownPushToken, ); } return undefined; @@ -312,7 +332,7 @@ export class CoreModuleDirector { } public getSubscriptionOfTypeWithToken( - type: ModelName, + type: SubscriptionType, token: string, ): OSModel | undefined { logMethodCall('CoreModuleDirector.getSubscriptionOfTypeWithToken', { @@ -320,19 +340,22 @@ export class CoreModuleDirector { token, }); switch (type) { - case ModelName.EmailSubscriptions: { + case SubscriptionType.Email: { const emailSubscriptions = this.getEmailSubscriptionModels(); return Object.values(emailSubscriptions).find( (subscription) => subscription.data.token === token, ); } - case ModelName.SmsSubscriptions: { + case SubscriptionType.SMS: { const smsSubscriptions = this.getSmsSubscriptionModels(); return Object.values(smsSubscriptions).find( (subscription) => subscription.data.token === token, ); } - case ModelName.PushSubscriptions: { + case SubscriptionType.ChromePush: + case SubscriptionType.SafariPush: + case SubscriptionType.SafariLegacyPush: + case SubscriptionType.FirefoxPush: { const pushSubscriptions = this.getAllPushSubscriptionModels(); return Object.values(pushSubscriptions).find( (subscription) => subscription.data.token === token, @@ -348,4 +371,19 @@ export class CoreModuleDirector { private getModelStores(): ModelStoresMap { return this.core.modelRepo?.modelStores as ModelStoresMap; } + + /** + * Helper that checks if a given SubscriptionType is a push subscription. + */ + public isPushSubscriptionType(type: SubscriptionType): boolean { + switch (type) { + case SubscriptionType.ChromePush: + case SubscriptionType.SafariPush: + case SubscriptionType.SafariLegacyPush: + case SubscriptionType.FirefoxPush: + return true; + default: + return false; + } + } } diff --git a/src/core/executors/ExecutorConfigMap.ts b/src/core/executors/ExecutorConfigMap.ts index 7fbabd160..1e64d67cf 100644 --- a/src/core/executors/ExecutorConfigMap.ts +++ b/src/core/executors/ExecutorConfigMap.ts @@ -20,16 +20,8 @@ export const EXECUTOR_CONFIG_MAP: ExecutorConfigMap = { modelName: ModelName.Properties, update: UserPropertyRequests.updateUserProperties, }, - [ModelName.PushSubscriptions]: { - modelName: ModelName.PushSubscriptions, - ...subscriptionConfig, - }, - [ModelName.EmailSubscriptions]: { - modelName: ModelName.EmailSubscriptions, - ...subscriptionConfig, - }, - [ModelName.SmsSubscriptions]: { - modelName: ModelName.SmsSubscriptions, + [ModelName.Subscriptions]: { + modelName: ModelName.Subscriptions, ...subscriptionConfig, }, }; diff --git a/src/core/executors/ExecutorFactory.ts b/src/core/executors/ExecutorFactory.ts index 5703e41ec..daaf47801 100644 --- a/src/core/executors/ExecutorFactory.ts +++ b/src/core/executors/ExecutorFactory.ts @@ -6,23 +6,14 @@ import { PropertiesExecutor } from './PropertiesExecutor'; import { SubscriptionExecutor } from './SubscriptionExecutor'; export class ExecutorFactory { - static subscriptionExecutor?: SubscriptionExecutor = undefined; - static build(executorConfig: ExecutorConfig): Executor { switch (executorConfig.modelName) { case ModelName.Identity: return new IdentityExecutor(executorConfig); case ModelName.Properties: return new PropertiesExecutor(executorConfig); - case ModelName.PushSubscriptions: - case ModelName.EmailSubscriptions: - case ModelName.SmsSubscriptions: - if (!ExecutorFactory.subscriptionExecutor) { - ExecutorFactory.subscriptionExecutor = new SubscriptionExecutor( - executorConfig, - ); - } - return ExecutorFactory.subscriptionExecutor; + case ModelName.Subscriptions: + return new SubscriptionExecutor(executorConfig); } } } diff --git a/src/core/executors/ExecutorStore.ts b/src/core/executors/ExecutorStore.ts index 8458aacdd..09a25e594 100644 --- a/src/core/executors/ExecutorStore.ts +++ b/src/core/executors/ExecutorStore.ts @@ -19,21 +19,8 @@ export class ExecutorStore { // call processDeltaQueue on all executors immediately public forceDeltaQueueProcessingOnAllExecutors(): void { - let didSubscriptionExecutorProcessDeltaQueue = false; - Object.values(this.store).forEach((executor) => { - if ( - executor instanceof SubscriptionExecutor && - didSubscriptionExecutorProcessDeltaQueue - ) { - return; - } - executor.processDeltaQueue(); - - if (executor instanceof SubscriptionExecutor) { - didSubscriptionExecutorProcessDeltaQueue = true; - } }); } } diff --git a/src/core/executors/SubscriptionExecutor.ts b/src/core/executors/SubscriptionExecutor.ts index e770e3b01..19919aaa9 100644 --- a/src/core/executors/SubscriptionExecutor.ts +++ b/src/core/executors/SubscriptionExecutor.ts @@ -35,17 +35,7 @@ export class SubscriptionExecutor extends ExecutorBase { } getOperationsFromCache(): Operation[] { - const smsOperations = OperationCache.getOperationsWithModelName( - ModelName.SmsSubscriptions, - ); - const emailOperations = OperationCache.getOperationsWithModelName( - ModelName.EmailSubscriptions, - ); - const pushSubOperations = OperationCache.getOperationsWithModelName( - ModelName.PushSubscriptions, - ); - - return [...smsOperations, ...emailOperations, ...pushSubOperations]; + return OperationCache.getOperationsWithModelName(ModelName.Subscriptions); } private separateDeltasByChangeType(deltas: CoreDelta[]): { diff --git a/src/core/models/SupportedModels.ts b/src/core/models/SupportedModels.ts index 1806f0528..cb4470b6b 100644 --- a/src/core/models/SupportedModels.ts +++ b/src/core/models/SupportedModels.ts @@ -5,7 +5,10 @@ import { UserPropertiesModel } from './UserPropertiesModel'; export enum ModelName { Identity = 'identity', Properties = 'properties', - // TO DO: make singular + Subscriptions = 'subscriptions', +} + +export enum LegacyModelName { PushSubscriptions = 'pushSubscriptions', EmailSubscriptions = 'emailSubscriptions', SmsSubscriptions = 'smsSubscriptions', diff --git a/src/onesignal/User.ts b/src/onesignal/User.ts index b7d8feaf2..3a16c78c2 100644 --- a/src/onesignal/User.ts +++ b/src/onesignal/User.ts @@ -135,7 +135,7 @@ export default class User { token: email, }; const newSubscription = new OSModel( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, subscription, ); @@ -150,14 +150,14 @@ export default class User { newSubscription.setExternalId(identityModel.data.external_id); } OneSignal.coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSubscription, true, ); } else { // new user OneSignal.coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSubscription, false, ); @@ -192,7 +192,7 @@ export default class User { }; const newSubscription = new OSModel( - ModelName.SmsSubscriptions, + ModelName.Subscriptions, subscription, ); @@ -207,14 +207,14 @@ export default class User { newSubscription.setExternalId(identityModel.data.external_id); } OneSignal.coreDirector.add( - ModelName.SmsSubscriptions, + ModelName.Subscriptions, newSubscription, true, ); } else { // new user OneSignal.coreDirector.add( - ModelName.SmsSubscriptions, + ModelName.Subscriptions, newSubscription, false, ); @@ -226,6 +226,7 @@ export default class User { throw e; }, ); + const identityModel = OneSignal.coreDirector.getIdentityModel(); if (identityModel.data.external_id) { newSubscription.setExternalId(identityModel.data.external_id); @@ -249,7 +250,7 @@ export default class User { modelIds.forEach(async (modelId) => { const model = emailSubscriptions[modelId]; if (model.data?.token === email) { - OneSignal.coreDirector.remove(ModelName.EmailSubscriptions, modelId); + OneSignal.coreDirector.remove(ModelName.Subscriptions, modelId); } }); } @@ -273,7 +274,7 @@ export default class User { modelIds.forEach(async (modelId) => { const model = smsSubscriptions[modelId]; if (model.data?.token === smsNumber) { - OneSignal.coreDirector.remove(ModelName.SmsSubscriptions, modelId); + OneSignal.coreDirector.remove(ModelName.Subscriptions, modelId); } }); } diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index 0b3bc3e5f..3450205b5 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -153,7 +153,7 @@ export default class LoginManager { // add the push subscription model back to the repo since we need at least 1 sub to create a new user OneSignal.coreDirector.add( - ModelName.PushSubscriptions, + ModelName.Subscriptions, pushSubModel as OSModel, false, ); diff --git a/src/shared/services/IndexedDb.ts b/src/shared/services/IndexedDb.ts index 1c5398e79..59a0724a3 100644 --- a/src/shared/services/IndexedDb.ts +++ b/src/shared/services/IndexedDb.ts @@ -1,9 +1,9 @@ -import { ModelName } from '../../core/models/SupportedModels'; +import { ModelName, LegacyModelName } from '../../core/models/SupportedModels'; import Utils from '../context/Utils'; import Emitter from '../libraries/Emitter'; import Log from '../libraries/Log'; -const DATABASE_VERSION = 5; +const DATABASE_VERSION = 6; export default class IndexedDb { public emitter: Emitter; @@ -155,9 +155,13 @@ export default class IndexedDb { if (newDbVersion >= 4 && event.oldVersion < 4) { db.createObjectStore(ModelName.Identity, { keyPath: 'modelId' }); db.createObjectStore(ModelName.Properties, { keyPath: 'modelId' }); - db.createObjectStore(ModelName.PushSubscriptions, { keyPath: 'modelId' }); - db.createObjectStore(ModelName.SmsSubscriptions, { keyPath: 'modelId' }); - db.createObjectStore(ModelName.EmailSubscriptions, { + db.createObjectStore(LegacyModelName.PushSubscriptions, { + keyPath: 'modelId', + }); + db.createObjectStore(LegacyModelName.SmsSubscriptions, { + keyPath: 'modelId', + }); + db.createObjectStore(LegacyModelName.EmailSubscriptions, { keyPath: 'modelId', }); } @@ -166,6 +170,9 @@ export default class IndexedDb { this.migrateOutcomesNotificationReceivedTableForV5(db, transaction); } if (newDbVersion >= 6 && event.oldVersion < 6) { + this.migrateModelNameSubscriptionsTableForV6(db, transaction); + } + if (newDbVersion >= 7 && event.oldVersion < 7) { // Make sure to update the database version at the top of the file } // Wrap in conditional for tests @@ -251,6 +258,57 @@ export default class IndexedDb { ); }; } + private migrateModelNameSubscriptionsTableForV6( + db: IDBDatabase, + transaction: IDBTransaction, + ) { + const newTableName = ModelName.Subscriptions; + db.createObjectStore(newTableName, { keyPath: 'modelId' }); + + let currentExternalId: string; + const identityCursor = transaction + .objectStore(ModelName.Identity) + .openCursor(); + identityCursor.onsuccess = () => { + if (identityCursor.result) { + currentExternalId = identityCursor.result.value.externalId; + } + }; + identityCursor.onerror = () => { + console.error( + 'Could not find ' + ModelName.Identity + ' records', + identityCursor.error, + ); + }; + + Object.values(LegacyModelName).forEach((oldTableName) => { + const legacyCursor = transaction.objectStore(oldTableName).openCursor(); + legacyCursor.onsuccess = () => { + if (!legacyCursor.result) { + // Delete old table once we have gone through all records + db.deleteObjectStore(oldTableName); + return; + } + const oldValue = legacyCursor.result.value; + + transaction.objectStore(newTableName).put({ + ...oldValue, + modelName: ModelName.Subscriptions, + externalId: currentExternalId, + }); + legacyCursor.result.continue(); + }; + legacyCursor.onerror = () => { + // If there is an error getting old records nothing we can do but + // move on. Old table will stay around so an attempt could be made + // later. + console.error( + 'Could not migrate ' + oldTableName + ' records', + legacyCursor.error, + ); + }; + }); + } /** * Asynchronously retrieves the value of the key at the table (if key is specified), or the entire table diff --git a/src/sw/helpers/ModelCacheDirectAccess.ts b/src/sw/helpers/ModelCacheDirectAccess.ts index a659446be..2478245d6 100644 --- a/src/sw/helpers/ModelCacheDirectAccess.ts +++ b/src/sw/helpers/ModelCacheDirectAccess.ts @@ -12,7 +12,7 @@ export class ModelCacheDirectAccess { token: string, ): Promise { const pushSubscriptions = await Database.getAll( - ModelName.PushSubscriptions, + ModelName.Subscriptions, ); for (const pushSubscription of pushSubscriptions) { if (pushSubscription['token'] === token) { From 0f0fa5a273bf628228a5be52ff5c6d878d89a0e0 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Thu, 1 Aug 2024 10:35:30 -0400 Subject: [PATCH 08/15] Update tests to use subscription model name --- .../support/environment/TestEnvironment.ts | 2 +- __test__/support/helpers/core.ts | 4 +- __test__/support/helpers/pushSubscription.ts | 2 +- __test__/unit/core/executors.test.ts | 2 +- __test__/unit/core/modelCache.test.ts | 4 +- __test__/unit/core/modelRepo.test.ts | 37 +++++++++++-------- __test__/unit/core/operationRepo.test.ts | 4 +- __test__/unit/core/osModel.test.ts | 8 ++-- 8 files changed, 34 insertions(+), 29 deletions(-) diff --git a/__test__/support/environment/TestEnvironment.ts b/__test__/support/environment/TestEnvironment.ts index badf3a2c2..03b6ae786 100644 --- a/__test__/support/environment/TestEnvironment.ts +++ b/__test__/support/environment/TestEnvironment.ts @@ -59,7 +59,7 @@ export class TestEnvironment { if (config.useMockPushSubscriptionModel) { OneSignal.coreDirector.add( - ModelName.PushSubscriptions, + ModelName.Subscriptions, getDummyPushSubscriptionOSModel(), false, ); diff --git a/__test__/support/helpers/core.ts b/__test__/support/helpers/core.ts index e2fa10ba5..1ae038a17 100644 --- a/__test__/support/helpers/core.ts +++ b/__test__/support/helpers/core.ts @@ -18,7 +18,7 @@ import { UserPropertiesModel } from '../../../src/core/models/UserPropertiesMode export function generateNewSubscription(modelId = '0000000000') { return new OSModel( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, { type: SubscriptionType.Email, id: '123', // subscription id @@ -51,7 +51,7 @@ export function getDummyPropertyOSModel( export function getDummyPushSubscriptionOSModel(): OSModel { return new OSModel( - ModelName.PushSubscriptions, + ModelName.Subscriptions, { type: SubscriptionType.ChromePush, id: DUMMY_SUBSCRIPTION_ID, diff --git a/__test__/support/helpers/pushSubscription.ts b/__test__/support/helpers/pushSubscription.ts index 2b31c8132..ecdff2031 100644 --- a/__test__/support/helpers/pushSubscription.ts +++ b/__test__/support/helpers/pushSubscription.ts @@ -22,7 +22,7 @@ export async function initializeWithPermission( const pushModel = getDummyPushSubscriptionOSModel(); OneSignal.coreDirector.add( - ModelName.PushSubscriptions, + ModelName.Subscriptions, pushModel as OSModel, false, ); diff --git a/__test__/unit/core/executors.test.ts b/__test__/unit/core/executors.test.ts index 55bd5ebb2..accc28cb7 100644 --- a/__test__/unit/core/executors.test.ts +++ b/__test__/unit/core/executors.test.ts @@ -64,7 +64,7 @@ describe('Executor tests', () => { core.modelRepo?.broadcast({ changeType: CoreChangeType.Add, - model: new OSModel(ModelName.EmailSubscriptions, {}), + model: new OSModel(ModelName.Subscriptions, {}), }); jest.runOnlyPendingTimers(); diff --git a/__test__/unit/core/modelCache.test.ts b/__test__/unit/core/modelCache.test.ts index 7c9ef01da..127df9e78 100644 --- a/__test__/unit/core/modelCache.test.ts +++ b/__test__/unit/core/modelCache.test.ts @@ -119,10 +119,10 @@ describe('ModelCache tests', () => { const modelCache = new ModelCache(); const model1 = new OSModel(ModelName.Identity, { id: 'test1' }); const model2 = new OSModel(ModelName.Identity, { id: 'test2' }); - const model3 = new OSModel(ModelName.EmailSubscriptions, { id: 'test3' }); + const model3 = new OSModel(ModelName.Subscriptions, { id: 'test3' }); await modelCache.add(ModelName.Identity, model1); await modelCache.add(ModelName.Identity, model2); - await modelCache.add(ModelName.EmailSubscriptions, model3); + await modelCache.add(ModelName.Subscriptions, model3); const cachedModels = await modelCache.getAndDecodeModelsWithModelName( ModelName.Identity, ); diff --git a/__test__/unit/core/modelRepo.test.ts b/__test__/unit/core/modelRepo.test.ts index b8ea7a36a..798dc39f3 100644 --- a/__test__/unit/core/modelRepo.test.ts +++ b/__test__/unit/core/modelRepo.test.ts @@ -47,15 +47,18 @@ describe('ModelRepo tests', () => { 'processModelAdded', ); const newSub = generateNewSubscription(); - const emailSubModels = coreDirector.getEmailSubscriptionModels(); + let emailSubModels = coreDirector.getEmailSubscriptionModels(); expect(Object.keys(emailSubModels).length).toBe(0); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); + + emailSubModels = coreDirector.getEmailSubscriptionModels(); + expect(processModelAddedSpy).toHaveBeenCalledTimes(1); expect(Object.keys(emailSubModels).length).toBe(1); }); @@ -66,16 +69,20 @@ describe('ModelRepo tests', () => { 'processModelRemoved', ); const newSub = generateNewSubscription(); - const emailSubModels = coreDirector.getEmailSubscriptionModels(); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); + + let emailSubModels = coreDirector.getEmailSubscriptionModels(); + expect(Object.keys(emailSubModels).length).toBe(1); - coreDirector.remove(ModelName.EmailSubscriptions, newSub.modelId); + coreDirector.remove(ModelName.Subscriptions, newSub.modelId); + emailSubModels = coreDirector.getEmailSubscriptionModels(); + expect(processModelRemovedSpy).toHaveBeenCalledTimes(1); expect(Object.keys(emailSubModels).length).toBe(0); }); @@ -88,7 +95,7 @@ describe('ModelRepo tests', () => { ); const newSub = generateNewSubscription(); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); @@ -109,7 +116,7 @@ describe('ModelRepo tests', () => { }); const newSub = generateNewSubscription(); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); @@ -124,11 +131,11 @@ describe('ModelRepo tests', () => { }); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); - coreDirector.remove(ModelName.EmailSubscriptions, newSub.modelId); + coreDirector.remove(ModelName.Subscriptions, newSub.modelId); }); test('ModelRepo update subscription -> delta is broadcasted twice', (done: jest.DoneCallback) => { @@ -140,7 +147,7 @@ describe('ModelRepo tests', () => { }); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); @@ -156,7 +163,7 @@ describe('ModelRepo tests', () => { done(); }); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); @@ -179,11 +186,11 @@ describe('ModelRepo tests', () => { }); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); - coreDirector.remove(ModelName.EmailSubscriptions, newSub.modelId); + coreDirector.remove(ModelName.Subscriptions, newSub.modelId); }); test('ModelRepo update subscription -> delta is broadcasted with correct change type and payload', (done: jest.DoneCallback) => { @@ -201,7 +208,7 @@ describe('ModelRepo tests', () => { }); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); @@ -218,7 +225,7 @@ describe('ModelRepo tests', () => { const modelCacheAddSpy = jest.spyOn(ModelCache.prototype as any, 'add'); coreDirector.add( - ModelName.EmailSubscriptions, + ModelName.Subscriptions, newSub as OSModel, true, ); diff --git a/__test__/unit/core/operationRepo.test.ts b/__test__/unit/core/operationRepo.test.ts index 9c920e223..8b9b9df9c 100644 --- a/__test__/unit/core/operationRepo.test.ts +++ b/__test__/unit/core/operationRepo.test.ts @@ -77,7 +77,7 @@ describe('OperationRepo tests', () => { test('Model repo delta broadcast is received and processed by operation repo', (done: jest.DoneCallback) => { const { modelRepo, operationRepo } = OneSignal.coreDirector.core; const executor = - operationRepo?.executorStore.store[ModelName.EmailSubscriptions]; + operationRepo?.executorStore.store[ModelName.Subscriptions]; executor._operationQueue = []; @@ -108,7 +108,7 @@ describe('OperationRepo tests', () => { test('Add Subscriptions: multiple delta broadcasts -> two operations of change type: add', (done: jest.DoneCallback) => { const { modelRepo, operationRepo } = OneSignal.coreDirector.core; const executor = - operationRepo?.executorStore.store[ModelName.EmailSubscriptions]; + operationRepo?.executorStore.store[ModelName.Subscriptions]; executor._operationQueue = []; diff --git a/__test__/unit/core/osModel.test.ts b/__test__/unit/core/osModel.test.ts index 61b187ec0..6dc821bcb 100644 --- a/__test__/unit/core/osModel.test.ts +++ b/__test__/unit/core/osModel.test.ts @@ -39,7 +39,7 @@ describe('OSModel tests', () => { const encodedSub = newSub.encode(); expect(encodedSub).toEqual({ modelId: '0000000000', - modelName: ModelName.EmailSubscriptions, + modelName: ModelName.Subscriptions, type: SubscriptionType.Email, id: '123', token: 'myToken', @@ -49,7 +49,7 @@ describe('OSModel tests', () => { test('Decode function returns decoded model', async () => { const encodedSub = { modelId: '0000000000', - modelName: ModelName.EmailSubscriptions, + modelName: ModelName.Subscriptions, type: SubscriptionType.Email, id: '123', token: 'myToken', @@ -64,9 +64,7 @@ describe('OSModel tests', () => { // upstream bug workaround https://github.com/facebook/jest/issues/8475 expect(JSON.stringify(decodedSub)).toEqual( - JSON.stringify( - new OSModel(ModelName.EmailSubscriptions, model, '0000000000'), - ), + JSON.stringify(new OSModel(ModelName.Subscriptions, model, '0000000000')), ); }); }); From 4e7e4655f297ae00a6e320dfcb0e5fc768c8a436 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Thu, 1 Aug 2024 10:37:04 -0400 Subject: [PATCH 09/15] Add database migration test for new model name --- __test__/unit/services/indexedDb.test.ts | 145 +++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/__test__/unit/services/indexedDb.test.ts b/__test__/unit/services/indexedDb.test.ts index 65f30a5ac..7beec5d21 100644 --- a/__test__/unit/services/indexedDb.test.ts +++ b/__test__/unit/services/indexedDb.test.ts @@ -1,5 +1,11 @@ +import { DUMMY_EXTERNAL_ID, DUMMY_ONESIGNAL_ID } from '../../support/constants'; +import { + LegacyModelName, + ModelName, +} from '../../../src/core/models/SupportedModels'; import IndexedDb from '../../../src/shared/services/IndexedDb'; import Random from '../../support/utils/Random'; +import { SubscriptionType } from '../../../src/core/models/SubscriptionModels'; require('fake-indexeddb/auto'); @@ -115,4 +121,143 @@ describe('migrations', () => { ]); }); }); + describe('v6', () => { + test('can write to new subscriptions table', async () => { + const db = newOSIndexedDb('testDbv6', 6); + const result = await db.put(ModelName.Subscriptions, { + modelId: '1', + }); + expect(result).toEqual({ modelId: '1' }); + }); + + test('migrates v5 email, push, sms subscriptions records to v6 subscriptions record', async () => { + const dbName = 'testDbV5upgradeToV6' + Random.getRandomString(10); + const db = newOSIndexedDb(dbName, 5); + await db.put(LegacyModelName.EmailSubscriptions, { + modelId: '1', + modelName: LegacyModelName.EmailSubscriptions, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.Email, + }); + await db.put(LegacyModelName.PushSubscriptions, { + modelId: '2', + modelName: LegacyModelName.PushSubscriptions, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.ChromePush, + }); + await db.put(LegacyModelName.SmsSubscriptions, { + modelId: '3', + modelName: LegacyModelName.SmsSubscriptions, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.SMS, + }); + db.close(); + + const db2 = newOSIndexedDb(dbName, 6); + const result = await db2.getAll(ModelName.Subscriptions); + expect(result).toEqual([ + { + modelId: '1', + modelName: ModelName.Subscriptions, + externalId: undefined, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.Email, + }, + { + modelId: '2', + modelName: ModelName.Subscriptions, + externalId: undefined, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.ChromePush, + }, + { + modelId: '3', + modelName: ModelName.Subscriptions, + externalId: undefined, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.SMS, + }, + ]); + }); + + test('removes v5 email, push, sms subscriptions tables', async () => { + const dbName = 'testDbV5upgradeToV6' + Random.getRandomString(10); + const db = newOSIndexedDb(dbName, 5); + db.close(); + + const db2 = newOSIndexedDb(dbName, 6); + await expect( + db2.getAll(LegacyModelName.EmailSubscriptions), + ).rejects.toThrow( + 'No objectStore named emailSubscriptions in this database', + ); + await expect( + db2.getAll(LegacyModelName.SmsSubscriptions), + ).rejects.toThrow( + 'No objectStore named smsSubscriptions in this database', + ); + await expect( + db2.getAll(LegacyModelName.PushSubscriptions), + ).rejects.toThrow( + 'No objectStore named pushSubscriptions in this database', + ); + }); + + test('migrates v5 email, push, sms subscriptions records of logged in user to v6 subscriptions record with external id', async () => { + const dbName = 'testDbV5upgradeToV6' + Random.getRandomString(10); + const db = newOSIndexedDb(dbName, 5); + await db.put(LegacyModelName.EmailSubscriptions, { + modelId: '1', + modelName: LegacyModelName.EmailSubscriptions, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.Email, + }); + await db.put(LegacyModelName.PushSubscriptions, { + modelId: '2', + modelName: LegacyModelName.PushSubscriptions, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.ChromePush, + }); + await db.put(LegacyModelName.SmsSubscriptions, { + modelId: '3', + modelName: LegacyModelName.SmsSubscriptions, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.SMS, + }); + // user is logged in + await db.put(ModelName.Identity, { + modelId: '4', + modelName: ModelName.Identity, + onesignalId: DUMMY_ONESIGNAL_ID, + externalId: DUMMY_EXTERNAL_ID, + }); + db.close(); + + const db2 = newOSIndexedDb(dbName, 6); + const result = await db2.getAll(ModelName.Subscriptions); + expect(result).toEqual([ + { + modelId: '1', + modelName: ModelName.Subscriptions, + externalId: DUMMY_EXTERNAL_ID, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.Email, + }, + { + modelId: '2', + modelName: ModelName.Subscriptions, + externalId: DUMMY_EXTERNAL_ID, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.ChromePush, + }, + { + modelId: '3', + modelName: ModelName.Subscriptions, + externalId: DUMMY_EXTERNAL_ID, + onesignalId: DUMMY_ONESIGNAL_ID, + type: SubscriptionType.SMS, + }, + ]); + }); + }); }); From c2cbfe1c9013193f8b977b7a9dca1b1c9bbc75dd Mon Sep 17 00:00:00 2001 From: Shepherd Date: Thu, 1 Aug 2024 11:03:07 -0400 Subject: [PATCH 10/15] Clear operation queue before each --- __test__/unit/core/operationRepo.test.ts | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/__test__/unit/core/operationRepo.test.ts b/__test__/unit/core/operationRepo.test.ts index 8b9b9df9c..88ac6494a 100644 --- a/__test__/unit/core/operationRepo.test.ts +++ b/__test__/unit/core/operationRepo.test.ts @@ -45,8 +45,21 @@ describe('OperationRepo tests', () => { onesignal_id: '123', }); jest.useFakeTimers(); - TestEnvironment.initialize(); + await TestEnvironment.initialize(); broadcastCount = 0; + + const { operationRepo } = OneSignal.coreDirector.core; + + const identityExecutor = + operationRepo?.executorStore.store[ModelName.Identity]; + const propertiesExecutor = + operationRepo?.executorStore.store[ModelName.Properties]; + const subscriptionExecutor = + operationRepo?.executorStore.store[ModelName.Subscriptions]; + + identityExecutor._operationQueue = []; + propertiesExecutor._operationQueue = []; + subscriptionExecutor._operationQueue = []; }); afterEach(async () => { @@ -79,8 +92,6 @@ describe('OperationRepo tests', () => { const executor = operationRepo?.executorStore.store[ModelName.Subscriptions]; - executor._operationQueue = []; - modelRepo?.subscribe(() => { broadcastCount += 1; passIfBroadcastNTimes(1, broadcastCount, done); @@ -110,8 +121,6 @@ describe('OperationRepo tests', () => { const executor = operationRepo?.executorStore.store[ModelName.Subscriptions]; - executor._operationQueue = []; - const processDeltaSpy = jest.spyOn( OperationRepo.prototype as any, '_processDelta', @@ -148,8 +157,6 @@ describe('OperationRepo tests', () => { const { modelRepo, operationRepo } = OneSignal.coreDirector.core; const executor = operationRepo?.executorStore.store[ModelName.Identity]; - executor._operationQueue = []; - const processDeltaSpy = jest.spyOn( OperationRepo.prototype as any, '_processDelta', @@ -217,8 +224,6 @@ describe('OperationRepo tests', () => { const { modelRepo, operationRepo } = OneSignal.coreDirector.core; const executor = operationRepo?.executorStore.store[ModelName.Properties]; - executor._operationQueue = []; - const processDeltaSpy = jest.spyOn( OperationRepo.prototype as any, '_processDelta', From fb3dc39bbad78ac1e74aac500e1a0062e6e3281c Mon Sep 17 00:00:00 2001 From: Shepherd Date: Fri, 2 Aug 2024 15:19:58 -0400 Subject: [PATCH 11/15] Remove unnecessary check and fix broken tests Fixes failing executor tests that result in removing the check. Tests were changed to: await the async TestEnvironment initialize function, remove additional initialization of core module because it was already created in TestEnvironment initialization, fix timers for tests that check the delta queue to check after 1 flush. --- __test__/unit/core/executors.test.ts | 43 ++++++++++--------------- src/core/operationRepo/OperationRepo.ts | 4 +-- 2 files changed, 18 insertions(+), 29 deletions(-) diff --git a/__test__/unit/core/executors.test.ts b/__test__/unit/core/executors.test.ts index accc28cb7..452b1e733 100644 --- a/__test__/unit/core/executors.test.ts +++ b/__test__/unit/core/executors.test.ts @@ -1,6 +1,5 @@ import { TestEnvironment } from '../../support/environment/TestEnvironment'; import ModelCache from '../../../src/core/caching/ModelCache'; -import CoreModule from '../../../src/core/CoreModule'; import { IdentityExecutor } from '../../../src/core/executors/IdentityExecutor'; import { PropertiesExecutor } from '../../../src/core/executors/PropertiesExecutor'; import { SubscriptionExecutor } from '../../../src/core/executors/SubscriptionExecutor'; @@ -10,6 +9,7 @@ import { ModelName, SupportedModel, } from '../../../src/core/models/SupportedModels'; +import { DELTA_QUEUE_TIME_ADVANCE } from '../../support/constants'; import { CoreDelta } from '../../../src/core/models/CoreDeltas'; import { generateNewSubscription } from '../../support/helpers/core'; import 'jest-localstorage-mock'; @@ -23,18 +23,19 @@ describe('Executor tests', () => { | jest.SpyInstance Promise], any> | jest.SpyInstance; - beforeEach(() => { + beforeEach(async () => { spyProcessOperationQueue = jest.spyOn( ExecutorBase.prototype as any, '_processOperationQueue', ); jest.useFakeTimers(); + test.stub(ModelCache.prototype, 'load', Promise.resolve({})); test.stub(PropertiesExecutor.prototype, 'getOperationsFromCache', []); test.stub(IdentityExecutor.prototype, 'getOperationsFromCache', []); test.stub(SubscriptionExecutor.prototype, 'getOperationsFromCache', []); - TestEnvironment.initialize(); + await TestEnvironment.initialize(); }); afterAll(async () => { @@ -50,9 +51,6 @@ describe('Executor tests', () => { /* F L U S H */ test('Subscriptions executor flushes deltas at end of `processDeltaQueue`', async () => { - const core = new CoreModule(); - await core.init(); - const processDeltaQueueSpy = jest.spyOn( SubscriptionExecutor.prototype, 'processDeltaQueue', @@ -62,21 +60,19 @@ describe('Executor tests', () => { '_flushDeltas', ); - core.modelRepo?.broadcast({ + const { modelRepo } = OneSignal.coreDirector.core; + modelRepo?.broadcast({ changeType: CoreChangeType.Add, model: new OSModel(ModelName.Subscriptions, {}), }); - jest.runOnlyPendingTimers(); + jest.advanceTimersByTime(DELTA_QUEUE_TIME_ADVANCE); expect(processDeltaQueueSpy).toHaveBeenCalledTimes(1); expect(flushDeltasSpy).toHaveBeenCalledTimes(1); }); test('Identity executor flushes deltas at end of `processDeltaQueue`', async () => { - const core = new CoreModule(); - await core.init(); - const processDeltaQueueSpy = jest.spyOn( IdentityExecutor.prototype, 'processDeltaQueue', @@ -86,21 +82,19 @@ describe('Executor tests', () => { '_flushDeltas', ); - core.modelRepo?.broadcast({ + const { modelRepo } = OneSignal.coreDirector.core; + modelRepo?.broadcast({ changeType: CoreChangeType.Update, model: new OSModel(ModelName.Identity, {}), }); - jest.runOnlyPendingTimers(); + jest.advanceTimersByTime(DELTA_QUEUE_TIME_ADVANCE); expect(processDeltaQueueSpy).toHaveBeenCalledTimes(1); expect(flushDeltasSpy).toHaveBeenCalledTimes(1); }); test('User Properties executor flushes deltas at end of `processDeltaQueue`', async () => { - const core = new CoreModule(); - await core.init(); - const processDeltaQueueSpy = jest.spyOn( PropertiesExecutor.prototype, 'processDeltaQueue', @@ -110,12 +104,13 @@ describe('Executor tests', () => { '_flushDeltas', ); - core.modelRepo?.broadcast({ + const { modelRepo } = OneSignal.coreDirector.core; + modelRepo?.broadcast({ changeType: CoreChangeType.Update, model: new OSModel(ModelName.Properties, {}), }); - jest.runOnlyPendingTimers(); + jest.advanceTimersByTime(DELTA_QUEUE_TIME_ADVANCE); expect(processDeltaQueueSpy).toHaveBeenCalledTimes(1); expect(flushDeltasSpy).toHaveBeenCalledTimes(1); @@ -123,9 +118,6 @@ describe('Executor tests', () => { /* S U B S C R I P T I O N E X E C U T O R H E L P E R S */ test('separateDeltasByModelId returns correct delta matrix', async () => { - const core = new CoreModule(); - await core.init(); - const model = generateNewSubscription(); const separateDeltasByModelIdSpy = jest.spyOn( SubscriptionExecutor.prototype as any, @@ -137,7 +129,8 @@ describe('Executor tests', () => { model: model as OSModel, }; - core.modelRepo?.broadcast(delta); + const { modelRepo } = OneSignal.coreDirector.core; + modelRepo?.broadcast(delta); jest.runOnlyPendingTimers(); @@ -148,9 +141,6 @@ describe('Executor tests', () => { }); test('separateDeltasByChangeType returns correct delta array map', async () => { - const core = new CoreModule(); - await core.init(); - const model = generateNewSubscription(); const separateDeltasByChangeTypeSpy = jest.spyOn( SubscriptionExecutor.prototype as any, @@ -161,7 +151,8 @@ describe('Executor tests', () => { changeType: CoreChangeType.Add, model: model as OSModel, }; - core.modelRepo?.broadcast(delta); + const { modelRepo } = OneSignal.coreDirector.core; + modelRepo?.broadcast(delta); jest.runOnlyPendingTimers(); diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index 74c30409e..fb376be5e 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -20,9 +20,7 @@ export class OperationRepo { ); setInterval(() => { - if (this._deltaQueue.length > 0) { - this._processDeltaQueue(); - } + this._processDeltaQueue(); }, OperationRepo.DELTAS_BATCH_PROCESSING_TIME * 1_000); } From 5eab7f9afb938c3af55b09d7ef08d2515e4765c6 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Mon, 9 Sep 2024 13:21:47 -0400 Subject: [PATCH 12/15] Nit remove logs These logs clutter the console due to being called every second/5 seconds --- src/core/executors/ExecutorBase.ts | 1 - src/core/operationRepo/OperationRepo.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/core/executors/ExecutorBase.ts b/src/core/executors/ExecutorBase.ts index e68284cf6..0adeb368a 100644 --- a/src/core/executors/ExecutorBase.ts +++ b/src/core/executors/ExecutorBase.ts @@ -78,7 +78,6 @@ export default abstract class ExecutorBase { } protected _flushDeltas(): void { - logMethodCall('ExecutorBase._flushDeltas'); this._deltaQueue = []; } diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index fb376be5e..41e6f9f09 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -40,7 +40,6 @@ export class OperationRepo { } private _flushDeltas(): void { - logMethodCall('OperationRepo._flushDeltas'); this._deltaQueue = []; } From 4d38448be4275ab0df828ffa868d772ce572cc0b Mon Sep 17 00:00:00 2001 From: Shepherd Date: Fri, 27 Sep 2024 12:16:58 -0400 Subject: [PATCH 13/15] Add new subscription channel type Create new type to make it clear what subscription channel is being filtered for in getSubscriptionOfTypeWithToken --- src/core/CoreModuleDirector.ts | 69 ++++++++++++++++----------- src/core/models/SubscriptionModels.ts | 6 +++ 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index f94520361..7ef1d0cd6 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -5,6 +5,7 @@ import { OSModel } from './modelRepo/OSModel'; import { SupportedIdentity } from './models/IdentityModel'; import { ModelStoresMap } from './models/ModelStoresMap'; import { + SubscriptionChannel, SubscriptionModel, SubscriptionType, SupportedSubscription, @@ -122,7 +123,7 @@ export class CoreModuleDirector { */ const existingSubscription = !!subscription.token ? this.getSubscriptionOfTypeWithToken( - subscription.type, + this.toSubscriptionChannel(subscription.type), subscription.token, ) : undefined; @@ -259,9 +260,9 @@ export class CoreModuleDirector { logMethodCall('CoreModuleDirector.getPushSubscriptionModelByCurrentToken'); const pushToken = await MainHelper.getCurrentPushToken(); if (pushToken) { - const pushSubscriptions = this.getAllPushSubscriptionModels(); - return Object.values(pushSubscriptions).find( - (subscription) => subscription.data.token === pushToken, + return this.getSubscriptionOfTypeWithToken( + SubscriptionChannel.Push, + pushToken, ); } return undefined; @@ -277,9 +278,9 @@ export class CoreModuleDirector { ); const { lastKnownPushToken } = await Database.getAppState(); if (lastKnownPushToken) { - const pushSubscriptions = this.getAllPushSubscriptionModels(); - return Object.values(pushSubscriptions).find( - (subscription) => subscription.data.token === lastKnownPushToken, + return this.getSubscriptionOfTypeWithToken( + SubscriptionChannel.Push, + lastKnownPushToken, ); } return undefined; @@ -332,38 +333,33 @@ export class CoreModuleDirector { } public getSubscriptionOfTypeWithToken( - type: SubscriptionType, + type: SubscriptionChannel | undefined, token: string, ): OSModel | undefined { logMethodCall('CoreModuleDirector.getSubscriptionOfTypeWithToken', { type, token, }); + + let subscriptions: Record>; + switch (type) { - case SubscriptionType.Email: { - const emailSubscriptions = this.getEmailSubscriptionModels(); - return Object.values(emailSubscriptions).find( - (subscription) => subscription.data.token === token, - ); - } - case SubscriptionType.SMS: { - const smsSubscriptions = this.getSmsSubscriptionModels(); - return Object.values(smsSubscriptions).find( - (subscription) => subscription.data.token === token, - ); - } - case SubscriptionType.ChromePush: - case SubscriptionType.SafariPush: - case SubscriptionType.SafariLegacyPush: - case SubscriptionType.FirefoxPush: { - const pushSubscriptions = this.getAllPushSubscriptionModels(); - return Object.values(pushSubscriptions).find( - (subscription) => subscription.data.token === token, - ); - } + case SubscriptionChannel.Email: + subscriptions = this.getEmailSubscriptionModels(); + break; + case SubscriptionChannel.SMS: + subscriptions = this.getSmsSubscriptionModels(); + break; + case SubscriptionChannel.Push: + subscriptions = this.getAllPushSubscriptionModels(); + break; default: return undefined; } + + return Object.values(subscriptions).find( + (subscription) => subscription.data.token === token, + ); } /* P R I V A T E */ @@ -386,4 +382,19 @@ export class CoreModuleDirector { return false; } } + + public toSubscriptionChannel(type: SubscriptionType) { + switch (type) { + case SubscriptionType.Email: + return SubscriptionChannel.Email; + case SubscriptionType.SMS: + return SubscriptionChannel.SMS; + default: + if (this.isPushSubscriptionType(type)) { + return SubscriptionChannel.Push; + } + + return undefined; + } + } } diff --git a/src/core/models/SubscriptionModels.ts b/src/core/models/SubscriptionModels.ts index be9c0e3c2..595d02c1a 100644 --- a/src/core/models/SubscriptionModels.ts +++ b/src/core/models/SubscriptionModels.ts @@ -12,6 +12,12 @@ export enum SubscriptionType { // There are other OneSignal types, but only including ones used here. } +export enum SubscriptionChannel { + Email = 'Email', + SMS = 'SMS', + Push = 'Push', +} + export interface FutureSubscriptionModel { type: SubscriptionType; token?: string; // maps to legacy player.identifier From 1c2b29791cfd81aefce239c2b15c2dfd943088d8 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Fri, 27 Sep 2024 12:18:02 -0400 Subject: [PATCH 14/15] Add login tests for external Id For externalId on OSModel --- __test__/support/constants.ts | 1 + __test__/unit/user/login.test.ts | 112 +++++++++++++++++++++++++------ 2 files changed, 93 insertions(+), 20 deletions(-) diff --git a/__test__/support/constants.ts b/__test__/support/constants.ts index 2df75902e..9ff6569ac 100644 --- a/__test__/support/constants.ts +++ b/__test__/support/constants.ts @@ -22,6 +22,7 @@ export const APP_ID = '34fcbe85-278d-4fd2-a4ec-0f80e95072c5'; export const DUMMY_PUSH_TOKEN = 'https://fcm.googleapis.com/fcm/send/01010101010101'; export const DUMMY_ONESIGNAL_ID = '1111111111-2222222222-3333333333'; +export const DUMMY_ONESIGNAL_ID_2 = '2222222222-3333333333-4444444444'; export const DUMMY_EXTERNAL_ID = 'rodrigo'; export const DUMMY_EXTERNAL_ID_2 = 'iryna'; export const DUMMY_SUBSCRIPTION_ID = '4444444444-5555555555-6666666666'; diff --git a/__test__/unit/user/login.test.ts b/__test__/unit/user/login.test.ts index 503bdf168..b6412db50 100644 --- a/__test__/unit/user/login.test.ts +++ b/__test__/unit/user/login.test.ts @@ -6,7 +6,11 @@ import { setupLoginStubs } from '../../support/helpers/login'; import { RequestService } from '../../../src/core/requestService/RequestService'; import { getDummyIdentityOSModel } from '../../support/helpers/core'; import { ModelName } from '../../../src/core/models/SupportedModels'; -import { DUMMY_EXTERNAL_ID, DUMMY_ONESIGNAL_ID } from '../../support/constants'; +import { + DUMMY_EXTERNAL_ID, + DUMMY_EXTERNAL_ID_2, + DUMMY_ONESIGNAL_ID, +} from '../../support/constants'; import { IdentityExecutor } from '../../../src/core/executors/IdentityExecutor'; import { PropertiesExecutor } from '../../../src/core/executors/PropertiesExecutor'; import { SubscriptionExecutor } from '../../../src/core/executors/SubscriptionExecutor'; @@ -88,12 +92,6 @@ describe('Login tests', () => { const identifyOrUpsertUserSpy = test.stub( LoginManager, 'identifyOrUpsertUser', - Promise.resolve({ - identity: { - external_id: DUMMY_EXTERNAL_ID, - onesignal_id: '1234567890', - }, - }), ); await LoginManager.login(DUMMY_EXTERNAL_ID); @@ -230,23 +228,97 @@ describe('Login tests', () => { }); test('If login with JWT token, save it to the database', async () => {}); -}); -test('Login called before any Subscriptions, should save external_id but not create User', async () => { - setupLoginStubs(); - await TestEnvironment.initialize(); - test.nock({}); + test('Login called before any Subscriptions, should save external_id but not create User', async () => { + setupLoginStubs(); + await TestEnvironment.initialize(); + test.nock({}); + + OneSignal.coreDirector.add(ModelName.Identity, getDummyIdentityOSModel()); + + const createUserSpy = jest.spyOn(RequestService, 'createUser'); + + await LoginManager.login(DUMMY_EXTERNAL_ID); + + expect(OneSignal.coreDirector.getIdentityModel().data.external_id).toBe( + DUMMY_EXTERNAL_ID, + ); + + // User should NOT be created, as we have no subscriptions yet. + expect(createUserSpy.mock.calls.length).toBe(0); + }); + + test('Login updates externalId on all models', async () => { + setupLoginStubs(); + await TestEnvironment.initialize({ + useMockIdentityModel: true, + useMockPushSubscriptionModel: true, + }); + + test.stub( + LoginManager, + 'identifyOrUpsertUser', + Promise.resolve({ + identity: { + external_id: DUMMY_EXTERNAL_ID, + onesignal_id: DUMMY_ONESIGNAL_ID, + }, + }), + ); + + await LoginManager.login(DUMMY_EXTERNAL_ID); + + expect(OneSignal.coreDirector.getIdentityModel().externalId).toBe( + DUMMY_EXTERNAL_ID, + ); + + expect(OneSignal.coreDirector.getPropertiesModel().externalId).toBe( + DUMMY_EXTERNAL_ID, + ); + + const subs = await OneSignal.coreDirector.getAllSubscriptionsModels(); + + expect( + subs.every( + (sub: { externalId: string }) => sub.externalId === DUMMY_EXTERNAL_ID, + ), + ).toBe(true); + }); + + test('Login twice updates current externalId on all models', async () => { + setupLoginStubs(); + await TestEnvironment.initialize({ + useMockIdentityModel: true, + }); + + test.stub( + LoginManager, + 'identifyOrUpsertUser', + Promise.resolve({ + identity: { + external_id: DUMMY_EXTERNAL_ID, + onesignal_id: DUMMY_ONESIGNAL_ID, + }, + }), + ); - OneSignal.coreDirector.add(ModelName.Identity, getDummyIdentityOSModel()); + await LoginManager.login(DUMMY_EXTERNAL_ID); + await LoginManager.login(DUMMY_EXTERNAL_ID_2); - const createUserSpy = jest.spyOn(RequestService, 'createUser'); + expect(OneSignal.coreDirector.getIdentityModel().externalId).toBe( + DUMMY_EXTERNAL_ID_2, + ); - await LoginManager.login(DUMMY_EXTERNAL_ID); + expect(OneSignal.coreDirector.getPropertiesModel().externalId).toBe( + DUMMY_EXTERNAL_ID_2, + ); - expect(OneSignal.coreDirector.getIdentityModel().data.external_id).toBe( - DUMMY_EXTERNAL_ID, - ); + const subs = await OneSignal.coreDirector.getAllSubscriptionsModels(); - // User should NOT be created, as we have no subscriptions yet. - expect(createUserSpy.mock.calls.length).toBe(0); + expect( + subs.every( + (sub: { externalId: string }) => sub.externalId === DUMMY_EXTERNAL_ID_2, + ), + ).toBe(true); + }); }); From d4bbe89a7513745d8bf7bb0c9efb1464bd31c361 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Fri, 27 Sep 2024 12:32:50 -0400 Subject: [PATCH 15/15] Move helper method into helper class --- src/core/CoreModuleDirector.ts | 35 ++---------------------- src/shared/helpers/SubscriptionHelper.ts | 35 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index 7ef1d0cd6..a858564c4 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -21,6 +21,7 @@ import User from '../onesignal/User'; import OneSignal from '../onesignal/OneSignal'; import Database from '../shared/services/Database'; import EventHelper from '../shared/helpers/EventHelper'; +import SubscriptionHelper from '../../src/shared/helpers/SubscriptionHelper'; /* Contains OneSignal User-Model-specific logic*/ @@ -123,7 +124,7 @@ export class CoreModuleDirector { */ const existingSubscription = !!subscription.token ? this.getSubscriptionOfTypeWithToken( - this.toSubscriptionChannel(subscription.type), + SubscriptionHelper.toSubscriptionChannel(subscription.type), subscription.token, ) : undefined; @@ -247,7 +248,7 @@ export class CoreModuleDirector { const pushSubscriptions = Object.fromEntries( Object.entries(subscriptionModels).filter(([, model]) => - this.isPushSubscriptionType(model.data?.type), + SubscriptionHelper.isPushSubscriptionType(model.data?.type), ), ); @@ -367,34 +368,4 @@ export class CoreModuleDirector { private getModelStores(): ModelStoresMap { return this.core.modelRepo?.modelStores as ModelStoresMap; } - - /** - * Helper that checks if a given SubscriptionType is a push subscription. - */ - public isPushSubscriptionType(type: SubscriptionType): boolean { - switch (type) { - case SubscriptionType.ChromePush: - case SubscriptionType.SafariPush: - case SubscriptionType.SafariLegacyPush: - case SubscriptionType.FirefoxPush: - return true; - default: - return false; - } - } - - public toSubscriptionChannel(type: SubscriptionType) { - switch (type) { - case SubscriptionType.Email: - return SubscriptionChannel.Email; - case SubscriptionType.SMS: - return SubscriptionChannel.SMS; - default: - if (this.isPushSubscriptionType(type)) { - return SubscriptionChannel.Push; - } - - return undefined; - } - } } diff --git a/src/shared/helpers/SubscriptionHelper.ts b/src/shared/helpers/SubscriptionHelper.ts index e23cae3c3..80b47f38b 100755 --- a/src/shared/helpers/SubscriptionHelper.ts +++ b/src/shared/helpers/SubscriptionHelper.ts @@ -10,6 +10,10 @@ import Log from '../libraries/Log'; import { ContextSWInterface } from '../models/ContextSW'; import SdkEnvironment from '../managers/SdkEnvironment'; import { PermissionUtils } from '../utils/PermissionUtils'; +import { + SubscriptionChannel, + SubscriptionType, +} from '../../../src/core/models/SubscriptionModels'; export default class SubscriptionHelper { public static async registerForPush(): Promise { @@ -43,4 +47,35 @@ export default class SubscriptionHelper { return subscription; } + + /** + * Helper that checks if a given SubscriptionType is a push subscription. + */ + public static isPushSubscriptionType(type: SubscriptionType): boolean { + switch (type) { + case SubscriptionType.ChromePush: + case SubscriptionType.SafariPush: + case SubscriptionType.SafariLegacyPush: + case SubscriptionType.FirefoxPush: + return true; + default: + return false; + } + } + + public static toSubscriptionChannel( + type: SubscriptionType, + ): SubscriptionChannel | undefined { + switch (type) { + case SubscriptionType.Email: + return SubscriptionChannel.Email; + case SubscriptionType.SMS: + return SubscriptionChannel.SMS; + default: + if (this.isPushSubscriptionType(type)) { + return SubscriptionChannel.Push; + } + return undefined; + } + } }