Merge pull request #1179 from OneSignal/fix/operationRepoRefactor

Operation repo refactor
OneSignal · Sep 30, 2024 · 3cdf8a1 · 3cdf8a1
2 parents 5868c04 + d4bbe89
commit 3cdf8a1
Show file tree

Hide file tree

Showing 26 changed files with 567 additions and 172 deletions.
diff --git 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__/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
@@ -18,7 +18,7 @@ import { UserPropertiesModel } from '../../../src/core/models/UserPropertiesMode
 
 export function generateNewSubscription(modelId = '0000000000') {
   return new OSModel<SupportedSubscription>(
-    ModelName.EmailSubscriptions,
+    ModelName.Subscriptions,
     {
       type: SubscriptionType.Email,
       id: '123', // subscription id
@@ -51,7 +51,7 @@ export function getDummyPropertyOSModel(
 
 export function getDummyPushSubscriptionOSModel(): OSModel<SupportedSubscription> {
   return new OSModel<SupportedSubscription>(
-    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
@@ -22,7 +22,7 @@ export async function initializeWithPermission(
 
   const pushModel = getDummyPushSubscriptionOSModel();
   OneSignal.coreDirector.add(
-    ModelName.PushSubscriptions,
+    ModelName.Subscriptions,
     pushModel as OSModel<SupportedModel>,
     false,
   );

diff --git 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<void, [() => Promise<void>], any>
     | jest.SpyInstance<void>;
 
-  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.EmailSubscriptions, {}),
+      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,22 +104,20 @@ 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);
   });
 
   /* 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<SupportedModel>,
     };
 
-    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<SupportedModel>,
     };
-    core.modelRepo?.broadcast(delta);
+    const { modelRepo } = OneSignal.coreDirector.core;
+    modelRepo?.broadcast(delta);
 
     jest.runOnlyPendingTimers();
 

diff --git 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
@@ -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<SupportedModel>,
       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<SupportedModel>,
       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<SupportedModel>,
       true,
     );
@@ -109,7 +116,7 @@ describe('ModelRepo tests', () => {
     });
     const newSub = generateNewSubscription();
     coreDirector.add(
-      ModelName.EmailSubscriptions,
+      ModelName.Subscriptions,
       newSub as OSModel<SupportedModel>,
       true,
     );
@@ -124,11 +131,11 @@ describe('ModelRepo tests', () => {
     });
 
     coreDirector.add(
-      ModelName.EmailSubscriptions,
+      ModelName.Subscriptions,
       newSub as OSModel<SupportedModel>,
       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<SupportedModel>,
       true,
     );
@@ -156,7 +163,7 @@ describe('ModelRepo tests', () => {
       done();
     });
     coreDirector.add(
-      ModelName.EmailSubscriptions,
+      ModelName.Subscriptions,
       newSub as OSModel<SupportedModel>,
       true,
     );
@@ -179,11 +186,11 @@ describe('ModelRepo tests', () => {
     });
 
     coreDirector.add(
-      ModelName.EmailSubscriptions,
+      ModelName.Subscriptions,
       newSub as OSModel<SupportedModel>,
       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<SupportedModel>,
       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<SupportedModel>,
       true,
     );

diff --git 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 () => {
@@ -77,7 +90,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];
 
     modelRepo?.subscribe(() => {
       broadcastCount += 1;
@@ -106,7 +119,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];
 
     const processDeltaSpy = jest.spyOn(
       OperationRepo.prototype as any,