From 328310b946ed74b213d616dad48bbe81b6f5b5b0 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Tue, 3 Sep 2024 14:01:44 -0400 Subject: [PATCH 1/7] Add new records state Used to keep track of creation of users and subscriptions inorder to delay operations afterwards. The delay is needed because after the server creates the user/subscription, there is a small amount of time where the data is not up to date. --- src/core/CoreModule.ts | 8 +++++- src/core/CoreModuleDirector.ts | 3 +++ src/core/executors/ExecutorBase.ts | 25 +++++++++++++++++-- src/core/executors/ExecutorFactory.ts | 12 ++++++--- src/core/executors/ExecutorStore.ts | 5 ++-- src/core/executors/IdentityExecutor.ts | 8 ++++-- src/core/executors/PropertiesExecutor.ts | 8 ++++-- src/core/executors/SubscriptionExecutor.ts | 8 ++++-- src/core/operationRepo/Operation.ts | 1 + src/core/operationRepo/OperationRepo.ts | 10 ++++++-- src/shared/models/NewRecordsState.ts | 29 ++++++++++++++++++++++ 11 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 src/shared/models/NewRecordsState.ts diff --git a/src/core/CoreModule.ts b/src/core/CoreModule.ts index fc134da81..ebeb0e120 100644 --- a/src/core/CoreModule.ts +++ b/src/core/CoreModule.ts @@ -5,11 +5,13 @@ import { OSModelStoreFactory } from './modelRepo/OSModelStoreFactory'; import Log from '../shared/libraries/Log'; import { logMethodCall } from '../shared/utils/utils'; import { SupportedModel } from './models/SupportedModels'; +import { NewRecordsState } from '../shared/models/NewRecordsState'; export default class CoreModule { public modelRepo?: ModelRepo; public operationRepo?: OperationRepo; public initPromise: Promise; + public newRecordsState?: NewRecordsState; private modelCache: ModelCache; private initResolver: () => void = () => null; @@ -25,7 +27,11 @@ export default class CoreModule { .then((allCachedOSModels) => { const modelStores = OSModelStoreFactory.build(allCachedOSModels); this.modelRepo = new ModelRepo(this.modelCache, modelStores); - this.operationRepo = new OperationRepo(this.modelRepo); + this.newRecordsState = new NewRecordsState(); + this.operationRepo = new OperationRepo( + this.modelRepo, + this.newRecordsState, + ); this.initResolver(); }) .catch((e) => { diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index a858564c4..eb877cc34 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -175,6 +175,9 @@ export class CoreModuleDirector { } /* G E T T E R S */ + public getNewRecordsState(): NewRecordsState { + return this.core.newRecordsState; + } public getModelByTypeAndId( modelName: ModelName, diff --git a/src/core/executors/ExecutorBase.ts b/src/core/executors/ExecutorBase.ts index 0adeb368a..d9f11c061 100644 --- a/src/core/executors/ExecutorBase.ts +++ b/src/core/executors/ExecutorBase.ts @@ -9,12 +9,14 @@ import { ExecutorResult } from './ExecutorResult'; import Log from '../../shared/libraries/Log'; import Database from '../../shared/services/Database'; import LocalStorage from '../../shared/utils/LocalStorage'; +import { NewRecordsState } from '../../shared/models/NewRecordsState'; const RETRY_AFTER = 5_000; export default abstract class ExecutorBase { protected _deltaQueue: CoreDelta[] = []; protected _operationQueue: Operation[] = []; + protected _newRecordsState: NewRecordsState; protected _executeAdd?: ( operation: Operation, @@ -31,7 +33,10 @@ export default abstract class ExecutorBase { static OPERATIONS_BATCH_PROCESSING_TIME = 5; static RETRY_COUNT = 5; - constructor(executorConfig: ExecutorConfig) { + constructor( + executorConfig: ExecutorConfig, + newRecordsState: NewRecordsState, + ) { setInterval(() => { Log.debug('OneSignal: checking for operations to process from cache'); const cachedOperations = this.getOperationsFromCache(); @@ -48,6 +53,8 @@ export default abstract class ExecutorBase { this._executeAdd = executorConfig.add; this._executeUpdate = executorConfig.update; this._executeRemove = executorConfig.remove; + + this._newRecordsState = newRecordsState; } abstract processDeltaQueue(): void; @@ -124,7 +131,7 @@ export default abstract class ExecutorBase { if (operation) { OperationCache.enqueue(operation); - if (this.onlineStatus) { + if (this._canExecute(operation)) { this._processOperation(operation, ExecutorBase.RETRY_COUNT).catch( (err) => { Log.error(err); @@ -187,4 +194,18 @@ export default abstract class ExecutorBase { this._processOperationQueue.call(this); } } + + private _canExecute(operation: Operation): boolean { + if (!this.onlineStatus) { + return false; + } + + if (operation.applyToRecordId) { + if (!this._newRecordsState.canAccess(operation.applyToRecordId)) { + return false; + } + } + + return true; + } } diff --git a/src/core/executors/ExecutorFactory.ts b/src/core/executors/ExecutorFactory.ts index daaf47801..f1cdefa4e 100644 --- a/src/core/executors/ExecutorFactory.ts +++ b/src/core/executors/ExecutorFactory.ts @@ -1,3 +1,4 @@ +import { NewRecordsState } from '../../shared/models/NewRecordsState'; import { Executor } from '../models/Executor'; import { ExecutorConfig } from '../models/ExecutorConfig'; import { ModelName, SupportedModel } from '../models/SupportedModels'; @@ -6,14 +7,17 @@ import { PropertiesExecutor } from './PropertiesExecutor'; import { SubscriptionExecutor } from './SubscriptionExecutor'; export class ExecutorFactory { - static build(executorConfig: ExecutorConfig): Executor { + static build( + executorConfig: ExecutorConfig, + newRecordsState: NewRecordsState, + ): Executor { switch (executorConfig.modelName) { case ModelName.Identity: - return new IdentityExecutor(executorConfig); + return new IdentityExecutor(executorConfig, newRecordsState); case ModelName.Properties: - return new PropertiesExecutor(executorConfig); + return new PropertiesExecutor(executorConfig, newRecordsState); case ModelName.Subscriptions: - return new SubscriptionExecutor(executorConfig); + return new SubscriptionExecutor(executorConfig, newRecordsState); } } } diff --git a/src/core/executors/ExecutorStore.ts b/src/core/executors/ExecutorStore.ts index 09a25e594..aa2eede6d 100644 --- a/src/core/executors/ExecutorStore.ts +++ b/src/core/executors/ExecutorStore.ts @@ -1,3 +1,4 @@ +import { NewRecordsState } from '../../shared/models/NewRecordsState'; import { ModelName } from '../models/SupportedModels'; import OSExecutor from './ExecutorBase'; import { EXECUTOR_CONFIG_MAP } from './ExecutorConfigMap'; @@ -10,10 +11,10 @@ type ExecutorStoreInterface = { export class ExecutorStore { store: ExecutorStoreInterface = {}; - constructor() { + constructor(newRecordsState: NewRecordsState) { Object.values(ModelName).forEach((modelName) => { const config = EXECUTOR_CONFIG_MAP[modelName as ModelName]; - this.store[modelName] = ExecutorFactory.build(config); + this.store[modelName] = ExecutorFactory.build(config, newRecordsState); }); } diff --git a/src/core/executors/IdentityExecutor.ts b/src/core/executors/IdentityExecutor.ts index dce4b0c8f..6c8d479e5 100644 --- a/src/core/executors/IdentityExecutor.ts +++ b/src/core/executors/IdentityExecutor.ts @@ -1,3 +1,4 @@ +import { NewRecordsState } from '../../shared/models/NewRecordsState'; import OperationCache from '../caching/OperationCache'; import { CoreChangeType } from '../models/CoreChangeType'; import { PropertyDelta } from '../models/CoreDeltas'; @@ -8,8 +9,11 @@ import { isPropertyDelta } from '../utils/typePredicates'; import ExecutorBase from './ExecutorBase'; export class IdentityExecutor extends ExecutorBase { - constructor(executorConfig: ExecutorConfig) { - super(executorConfig); + constructor( + executorConfig: ExecutorConfig, + newRecordsState: NewRecordsState, + ) { + super(executorConfig, newRecordsState); } processDeltaQueue(): void { diff --git a/src/core/executors/PropertiesExecutor.ts b/src/core/executors/PropertiesExecutor.ts index a8b343bb7..71d8808db 100644 --- a/src/core/executors/PropertiesExecutor.ts +++ b/src/core/executors/PropertiesExecutor.ts @@ -1,3 +1,4 @@ +import { NewRecordsState } from '../../shared/models/NewRecordsState'; import ExecutorBase from './ExecutorBase'; import { Operation } from '../operationRepo/Operation'; import { CoreChangeType } from '../models/CoreChangeType'; @@ -6,8 +7,11 @@ import { ModelName, SupportedModel } from '../models/SupportedModels'; import OperationCache from '../caching/OperationCache'; export class PropertiesExecutor extends ExecutorBase { - constructor(executorConfig: ExecutorConfig) { - super(executorConfig); + constructor( + executorConfig: ExecutorConfig, + newRecordsState: NewRecordsState, + ) { + super(executorConfig, newRecordsState); } processDeltaQueue(): void { diff --git a/src/core/executors/SubscriptionExecutor.ts b/src/core/executors/SubscriptionExecutor.ts index 19919aaa9..6476a2202 100644 --- a/src/core/executors/SubscriptionExecutor.ts +++ b/src/core/executors/SubscriptionExecutor.ts @@ -1,3 +1,4 @@ +import { NewRecordsState } from '../../shared/models/NewRecordsState'; import OperationCache from '../caching/OperationCache'; import { CoreChangeType } from '../models/CoreChangeType'; import { CoreDelta } from '../models/CoreDeltas'; @@ -7,8 +8,11 @@ import { Operation } from '../operationRepo/Operation'; import ExecutorBase from './ExecutorBase'; export class SubscriptionExecutor extends ExecutorBase { - constructor(executorConfig: ExecutorConfig) { - super(executorConfig); + constructor( + executorConfig: ExecutorConfig, + newRecordsState: NewRecordsState, + ) { + super(executorConfig, newRecordsState); } processDeltaQueue(): void { diff --git a/src/core/operationRepo/Operation.ts b/src/core/operationRepo/Operation.ts index 67f5120f4..97a562c52 100644 --- a/src/core/operationRepo/Operation.ts +++ b/src/core/operationRepo/Operation.ts @@ -13,6 +13,7 @@ export class Operation { model?: OSModel; jwtTokenAvailable: Promise; jwtToken?: string | null; + applyToRecordId: string | undefined; constructor( readonly changeType: CoreChangeType, diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index 41e6f9f09..3fb2478e3 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -3,15 +3,21 @@ import { ExecutorStore } from '../executors/ExecutorStore'; import { CoreDelta } from '../models/CoreDeltas'; import { SupportedModel } from '../models/SupportedModels'; import { logMethodCall } from '../../shared/utils/utils'; +import { NewRecordsState } from '../../shared/models/NewRecordsState'; export class OperationRepo { public executorStore: ExecutorStore; + public newRecordsState: NewRecordsState; private _unsubscribeFromModelRepo: () => void; private _deltaQueue: CoreDelta[] = []; static DELTAS_BATCH_PROCESSING_TIME = 1; - constructor(private modelRepo: ModelRepo) { - this.executorStore = new ExecutorStore(); + constructor( + private modelRepo: ModelRepo, + newRecordsState: NewRecordsState, + ) { + this.newRecordsState = newRecordsState; + this.executorStore = new ExecutorStore(this.newRecordsState); this._unsubscribeFromModelRepo = this.modelRepo.subscribe( (delta: CoreDelta) => { diff --git a/src/shared/models/NewRecordsState.ts b/src/shared/models/NewRecordsState.ts new file mode 100644 index 000000000..bdecc619c --- /dev/null +++ b/src/shared/models/NewRecordsState.ts @@ -0,0 +1,29 @@ +/** + * Purpose: Keeps track of IDs that were just created on the backend. + * This list gets used to delay network calls to ensure upcoming + * requests are ready to be accepted by the backend. + */ +export class NewRecordsState { + // time in ms + OP_REPO_POST_CREATE_DELAY: number; + // Key = a string id + // Value = A Timestamp in ms of when the id was created + private records: Map = new Map(); + + constructor(time = 3000) { + this.OP_REPO_POST_CREATE_DELAY = time; + } + + public add(key: string, overwrite = false): void { + if (overwrite || this.records.get(key)) { + this.records.set(key, Date.now()); + } + } + + public canAccess(key: string): boolean { + const timeLastMovedOrCreated = this.records.get(key); + return timeLastMovedOrCreated + ? Date.now() - timeLastMovedOrCreated > this.OP_REPO_POST_CREATE_DELAY + : true; + } +} From 67a906393bcf8ec3ba9afa8fea3794e27ee29289 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Tue, 3 Sep 2024 18:07:17 -0400 Subject: [PATCH 2/7] Add OneSignal and push sub ids to new records --- src/core/CoreModuleDirector.ts | 2 +- .../requestService/SubscriptionRequests.ts | 9 ++++- src/onesignal/UserDirector.ts | 25 +++++++++++++- src/page/managers/LoginManager.ts | 33 ++++++++++++++++++- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index eb877cc34..eafa028e7 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -175,7 +175,7 @@ export class CoreModuleDirector { } /* G E T T E R S */ - public getNewRecordsState(): NewRecordsState { + public getNewRecordsState(): NewRecordsState | undefined { return this.core.newRecordsState; } diff --git a/src/core/requestService/SubscriptionRequests.ts b/src/core/requestService/SubscriptionRequests.ts index a4caa1fb9..4d6ff13a0 100644 --- a/src/core/requestService/SubscriptionRequests.ts +++ b/src/core/requestService/SubscriptionRequests.ts @@ -25,13 +25,20 @@ export default class SubscriptionRequests { logMethodCall('SubscriptionRequests.addSubscription', operation); const appId = await MainHelper.getAppId(); - const { subscription, aliasPair } = processSubscriptionOperation(operation); + const { subscription, aliasPair, subscriptionId } = + processSubscriptionOperation(operation); const response = await RequestService.createSubscription( { appId }, aliasPair, { subscription }, ); + + const status = response.status; + if (status >= 200 && status < 300) { + OneSignal.coreDirector.getNewRecordsState().add(subscriptionId); + } + return SubscriptionRequests._processSubscriptionResponse(response); } diff --git a/src/onesignal/UserDirector.ts b/src/onesignal/UserDirector.ts index 4df8e56ef..f4adc0be7 100644 --- a/src/onesignal/UserDirector.ts +++ b/src/onesignal/UserDirector.ts @@ -110,7 +110,30 @@ export default class UserDirector { userData, ); user.isCreatingUser = false; - return response.result as UserData; + const status = response.status; + const result = response.result as UserData; + + if (status >= 200 && status < 300) { + const onesignalId = userData.identity?.onesignal_id; + + if (onesignalId) { + OneSignal.coreDirector.getNewRecordsState().add(onesignalId); + } + + const payloadSubcriptionToken = userData.subscriptions[0].token; + const resultSubscription = result.subscriptions.find( + (sub) => sub.token === payloadSubcriptionToken, + ); + + if (resultSubscription) { + if (isCompleteSubscriptionObject(resultSubscription)) { + OneSignal.coreDirector + .getNewRecordsState() + .add(resultSubscription.id); + } + } + } + return result; } catch (e) { Log.error(e); } diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index 3450205b5..71e3bac5b 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -224,10 +224,32 @@ export default class LoginManager { { appId, subscriptionId }, userData, ); - const result = response?.result; + const result = response?.result as UserData; const status = response?.status; if (status >= 200 && status < 300) { + const onesignalId = userData.identity?.onesignal_id; + + const newRecordsState = OneSignal.coreDirector.getNewRecordsState(); + + if (!newRecordsState) { + Log.error(`UpsertUser: NewRecordsState is undefined`); + } + + if (onesignalId) { + newRecordsState?.add(onesignalId); + } + + const payloadSubcriptionToken = userData.subscriptions?.[0]?.token; + const resultSubscription = result.subscriptions?.find( + (sub) => sub.token === payloadSubcriptionToken, + ); + + if (resultSubscription) { + if (isCompleteSubscriptionObject(resultSubscription)) { + newRecordsState?.add(resultSubscription.id); + } + } Log.info('Successfully created user', result); } else if (status >= 400 && status < 500) { Log.error('Malformed request', result); @@ -288,6 +310,15 @@ export default class LoginManager { if (identifyResponseStatus >= 200 && identifyResponseStatus < 300) { Log.info('identifyUser succeeded'); + + const newRecordsState = OneSignal.coreDirector.getNewRecordsState(); + + if (!newRecordsState) { + Log.error(`IdentifyUser: NewRecordsState is undefined`); + } + + // External id takes time to update on server. Include as new record with current time + newRecordsState?.add(onesignalId, true); } else if (identifyResponseStatus === 409 && pushSubscriptionId) { return await this.transferSubscription( appId, From 0219417b5970a27cec23e54bb2c75791ecfee885 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Fri, 6 Sep 2024 09:49:48 -0400 Subject: [PATCH 3/7] Add applyToRecordId Used to delay operations if the user was just created. Applied to addTag(s), removeTag(s), addAlias(es), removeAlias(es) --- src/core/modelRepo/ModelRepo.ts | 3 +++ src/core/modelRepo/OSModel.ts | 7 ++++++- src/core/models/CoreDeltas.ts | 1 + src/core/operationRepo/Operation.ts | 3 ++- src/onesignal/User.ts | 9 +++++++-- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/core/modelRepo/ModelRepo.ts b/src/core/modelRepo/ModelRepo.ts index 8a1949f74..b8734993d 100644 --- a/src/core/modelRepo/ModelRepo.ts +++ b/src/core/modelRepo/ModelRepo.ts @@ -60,6 +60,7 @@ export class ModelRepo extends Subscribable> { this.broadcast({ model: payload, changeType: CoreChangeType.Add, + applyToRecordId: payload?.applyToRecordId, }); } @@ -77,6 +78,7 @@ export class ModelRepo extends Subscribable> { this.broadcast({ model: payload, changeType: CoreChangeType.Remove, + applyToRecordId: payload?.applyToRecordId, }); } @@ -103,6 +105,7 @@ export class ModelRepo extends Subscribable> { property: payload.property, oldValue: payload.oldValue, newValue: payload.newValue, + applyToRecordId: payload.model?.applyToRecordId, }; this.broadcast(delta); } diff --git a/src/core/modelRepo/OSModel.ts b/src/core/modelRepo/OSModel.ts index 3536324f1..5185b0c3c 100644 --- a/src/core/modelRepo/OSModel.ts +++ b/src/core/modelRepo/OSModel.ts @@ -13,11 +13,11 @@ import { logMethodCall } from '../../shared/utils/utils'; export class OSModel extends Subscribable> { data: Model; modelId: string; - onesignalId?: string; awaitOneSignalIdAvailable: Promise; onesignalIdAvailableCallback?: (onesignalId: string) => void; externalId?: string; + applyToRecordId?: string; constructor( readonly modelName: ModelName, @@ -55,6 +55,11 @@ export class OSModel extends Subscribable> { this.externalId = externalId; } + public setApplyToRecordId(applyToRecordId: string): void { + logMethodCall('setapplyToRecordId', { applyToRecordId }); + this.applyToRecordId = applyToRecordId; + } + /** * We use this method to update the model data. * Results in a broadcasted update event. diff --git a/src/core/models/CoreDeltas.ts b/src/core/models/CoreDeltas.ts index d99a46af3..ef1cc6953 100644 --- a/src/core/models/CoreDeltas.ts +++ b/src/core/models/CoreDeltas.ts @@ -5,6 +5,7 @@ import { StringKeys } from './StringKeys'; export type ModelDelta = { model: OSModel; changeType: CoreChangeType; + applyToRecordId?: string; }; export interface PropertyDelta extends ModelDelta { diff --git a/src/core/operationRepo/Operation.ts b/src/core/operationRepo/Operation.ts index 97a562c52..06c18d202 100644 --- a/src/core/operationRepo/Operation.ts +++ b/src/core/operationRepo/Operation.ts @@ -11,9 +11,9 @@ export class Operation { timestamp: number; payload?: Partial; model?: OSModel; + applyToRecordId?: string; jwtTokenAvailable: Promise; jwtToken?: string | null; - applyToRecordId: string | undefined; constructor( readonly changeType: CoreChangeType, @@ -23,6 +23,7 @@ export class Operation { this.operationId = Math.random().toString(36).substring(2); this.payload = deltas ? this.getPayload(deltas) : undefined; this.model = deltas ? deltas[deltas.length - 1].model : undefined; + this.applyToRecordId = deltas?.[deltas.length - 1]?.applyToRecordId; this.timestamp = Date.now(); // eslint-disable-next-line no-async-promise-executor this.jwtTokenAvailable = new Promise(async (resolve) => { diff --git a/src/onesignal/User.ts b/src/onesignal/User.ts index 3a16c78c2..0c86f51c9 100644 --- a/src/onesignal/User.ts +++ b/src/onesignal/User.ts @@ -82,8 +82,10 @@ export default class User { } }); + const identityModel = OneSignal.coreDirector.getIdentityModel(); + identityModel?.setApplyToRecordId(identityModel?.onesignalId); + Object.keys(aliases).forEach(async (label) => { - const identityModel = OneSignal.coreDirector.getIdentityModel(); identityModel?.set(label, aliases[label]); }); } @@ -109,8 +111,10 @@ export default class User { throw new InvalidArgumentError('aliases', InvalidArgumentReason.Empty); } + const identityModel = OneSignal.coreDirector.getIdentityModel(); + identityModel?.setApplyToRecordId(identityModel?.onesignalId); + aliases.forEach(async (alias) => { - const identityModel = OneSignal.coreDirector.getIdentityModel(); identityModel?.set(alias, undefined); }); } @@ -349,6 +353,7 @@ export default class User { tagKeys.forEach((tagKey) => { tagsCopy[tagKey] = ''; }); + propertiesModel?.setApplyToRecordId(propertiesModel?.onesignalId); propertiesModel?.set('tags', tagsCopy); } } From bd6383b0d8d491088d100310f0ebb3050a112a2b Mon Sep 17 00:00:00 2001 From: Shepherd Date: Mon, 23 Sep 2024 11:58:10 -0400 Subject: [PATCH 4/7] Avoid casting --- src/page/managers/LoginManager.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index 71e3bac5b..51f9f2c36 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -224,7 +224,7 @@ export default class LoginManager { { appId, subscriptionId }, userData, ); - const result = response?.result as UserData; + const result = response?.result; const status = response?.status; if (status >= 200 && status < 300) { @@ -242,7 +242,8 @@ export default class LoginManager { const payloadSubcriptionToken = userData.subscriptions?.[0]?.token; const resultSubscription = result.subscriptions?.find( - (sub) => sub.token === payloadSubcriptionToken, + (sub: { token: string | undefined }) => + sub.token === payloadSubcriptionToken, ); if (resultSubscription) { From d6e61c4e70c31ee77952afb28eb76f23ae3b99a4 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Mon, 23 Sep 2024 12:38:48 -0400 Subject: [PATCH 5/7] Use local external Id in user hydration The server may still be updating and can return an outdated external Id from the getUser request. Instead of waiting for the updated external Id, we can hydrate the user with the local external Id as getting a response from the server means that the login was successful. The external Id param from login() is used as the local external Id instead of the Identity Model because the Identity Model gets reset from resetModelRepoAndCache() on L89 and returns undefined. --- src/core/CoreModuleDirector.ts | 6 +++--- src/page/managers/LoginManager.ts | 15 +++++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index eafa028e7..1d6c37b3c 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -63,14 +63,13 @@ export class CoreModuleDirector { await this.core.resetModelRepoAndCache(); } - public hydrateUser(user: UserData): void { + public hydrateUser(user: UserData, externalId?: string): void { logMethodCall('CoreModuleDirector.hydrateUser', { user }); try { const identity = this.getIdentityModel(); const properties = this.getPropertiesModel(); - const { onesignal_id: onesignalId, external_id: externalId } = - user.identity; + const { onesignal_id: onesignalId } = user.identity; if (!onesignalId) { throw new OneSignalError('OneSignal ID is missing from user data'); @@ -83,6 +82,7 @@ export class CoreModuleDirector { if (externalId) { identity?.setExternalId(externalId); properties?.setExternalId(externalId); + user.identity.external_id = externalId; } // identity and properties models are always single, so we hydrate immediately (i.e. replace existing data) diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index 51f9f2c36..862fb6989 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -101,7 +101,8 @@ export default class LoginManager { ); return; } - await LoginManager.fetchAndHydrate(onesignalId); + // hydrating with local externalId as server could still be updating + await LoginManager.fetchAndHydrate(onesignalId, externalId); } catch (e) { Log.error( `Login: Error while identifying/upserting user: ${e.message}`, @@ -111,7 +112,10 @@ export default class LoginManager { Log.debug('Login: Restoring old user data'); try { - await LoginManager.fetchAndHydrate(onesignalIdBackup); + await LoginManager.fetchAndHydrate( + onesignalIdBackup, + currentExternalId, + ); } catch (e) { Log.error( `Login: Error while restoring old user data: ${e.message}`, @@ -342,7 +346,10 @@ export default class LoginManager { return { identity: identityResult }; } - static async fetchAndHydrate(onesignalId: string): Promise { + static async fetchAndHydrate( + onesignalId: string, + externalId?: string, + ): Promise { logMethodCall('LoginManager.fetchAndHydrate', { onesignalId }); const fetchUserResponse = await RequestService.getUser( @@ -350,7 +357,7 @@ export default class LoginManager { new AliasPair(AliasPair.ONESIGNAL_ID, onesignalId), ); - OneSignal.coreDirector.hydrateUser(fetchUserResponse?.result); + OneSignal.coreDirector.hydrateUser(fetchUserResponse?.result, externalId); } /** From 3f07130568cae4213d4c4cba72fd675ab925965a Mon Sep 17 00:00:00 2001 From: Shepherd Date: Mon, 23 Sep 2024 12:44:11 -0400 Subject: [PATCH 6/7] Fix login tests timing out Change operationsFromCache stub to remove promise as it is not an async function and finish running timers after each test --- __test__/unit/user/login.test.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/__test__/unit/user/login.test.ts b/__test__/unit/user/login.test.ts index b6412db50..696ba20d2 100644 --- a/__test__/unit/user/login.test.ts +++ b/__test__/unit/user/login.test.ts @@ -22,24 +22,13 @@ jest.mock('../../../src/shared/libraries/Log'); describe('Login tests', () => { beforeEach(() => { jest.useFakeTimers(); - test.stub( - PropertiesExecutor.prototype, - 'getOperationsFromCache', - Promise.resolve([]), - ); - test.stub( - IdentityExecutor.prototype, - 'getOperationsFromCache', - Promise.resolve([]), - ); - test.stub( - SubscriptionExecutor.prototype, - 'getOperationsFromCache', - Promise.resolve([]), - ); + test.stub(PropertiesExecutor.prototype, 'getOperationsFromCache', []); + test.stub(IdentityExecutor.prototype, 'getOperationsFromCache', []); + test.stub(SubscriptionExecutor.prototype, 'getOperationsFromCache', []); }); afterEach(() => { + jest.runOnlyPendingTimers(); jest.resetAllMocks(); }); From 67b4d5be3cb08fdf567b679517659bd03730c655 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Mon, 23 Sep 2024 14:29:36 -0400 Subject: [PATCH 7/7] Update logs to include additional params --- src/core/CoreModuleDirector.ts | 4 +++- src/page/managers/LoginManager.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index 1d6c37b3c..8909979d5 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -64,7 +64,7 @@ export class CoreModuleDirector { } public hydrateUser(user: UserData, externalId?: string): void { - logMethodCall('CoreModuleDirector.hydrateUser', { user }); + logMethodCall('CoreModuleDirector.hydrateUser', { user, externalId }); try { const identity = this.getIdentityModel(); const properties = this.getPropertiesModel(); @@ -109,6 +109,8 @@ export class CoreModuleDirector { ): void { logMethodCall('CoreModuleDirector._hydrateSubscriptions', { subscriptions, + onesignalId, + externalId, }); if (!subscriptions) { diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index 862fb6989..81720a169 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -350,7 +350,7 @@ export default class LoginManager { onesignalId: string, externalId?: string, ): Promise { - logMethodCall('LoginManager.fetchAndHydrate', { onesignalId }); + logMethodCall('LoginManager.fetchAndHydrate', { onesignalId, externalId }); const fetchUserResponse = await RequestService.getUser( { appId: await MainHelper.getAppId() },