Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Move ready queue processing after identity request #933

Open
wants to merge 8 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from './utils';
import { hasMPIDAndUserLoginChanged, hasMPIDChanged } from './user-utils';
import { getNewIdentitiesByName } from './type-utils';
import { processReadyQueue } from './pre-init-utils';

export default function Identity(mpInstance) {
const { getFeatureFlag, extend } = mpInstance._Helpers;
Expand Down Expand Up @@ -1679,6 +1680,11 @@ export default function Identity(mpInstance) {
'Error parsing JSON response from Identity server: ' + e
);
}
mpInstance._Store.isInitialized = true;

mpInstance._preInit.readyQueue = processReadyQueue(
mpInstance._preInit.readyQueue
);
};

// send a user identity change request on identify, login, logout, modify when any values change.
Expand Down
51 changes: 9 additions & 42 deletions src/mp-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ import Consent from './consent';
import KitBlocker from './kitBlocking';
import ConfigAPIClient from './configAPIClient';
import IdentityAPIClient from './identityApiClient';
import { isEmpty, isFunction } from './utils';
import { isFunction } from './utils';
import { LocalStorageVault } from './vault';
import { removeExpiredIdentityCacheDates } from './identity-utils';
import IntegrationCapture from './integrationCapture';
import { processReadyQueue } from './pre-init-utils';

const { Messages, HTTPCodes, FeatureFlags } = Constants;
const { ReportBatching, CaptureIntegrationSpecificIds } = FeatureFlags;
Expand Down Expand Up @@ -1361,15 +1362,17 @@ function completeSDKInitialization(apiKey, config, mpInstance) {
);
}

mpInstance._Store.isInitialized = true;
// If for some reason the identity call inside of Session Manager does not run
// we should clear out the ready queue.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your PR description appropriately states when this is the case, rather than "for some reason", so I'd turn that into a comment.

"Will also continue to clear out the ready queue as part of the initial init flow if an identify request is unnecessary, such as if there is an existing session".

if (
(mpInstance._Store.mpid && !mpInstance._Store.identifyCalled) ||
mpInstance._Store.webviewBridgeEnabled
) {
mpInstance._Store.isInitialized = true;

// Call any functions that are waiting for the library to be initialized
try {
mpInstance._preInit.readyQueue = processReadyQueue(
mpInstance._preInit.readyQueue
);
} catch (error) {
mpInstance.Logger.error(error);
}

// https://go.mparticle.com/work/SQDSDKS-6040
Expand Down Expand Up @@ -1508,42 +1511,6 @@ function processIdentityCallback(
}
}

function processPreloadedItem(readyQueueItem) {
const args = readyQueueItem;
const method = args.splice(0, 1)[0];
// if the first argument is a method on the base mParticle object, run it
if (mParticle[args[0]]) {
mParticle[method].apply(this, args);
// otherwise, the method is on either eCommerce or Identity objects, ie. "eCommerce.setCurrencyCode", "Identity.login"
} else {
const methodArray = method.split('.');
try {
var computedMPFunction = mParticle;
for (let i = 0; i < methodArray.length; i++) {
const currentMethod = methodArray[i];
computedMPFunction = computedMPFunction[currentMethod];
}
computedMPFunction.apply(this, args);
} catch (e) {
throw new Error('Unable to compute proper mParticle function ' + e);
}
}
}

function processReadyQueue(readyQueue) {
if (!isEmpty(readyQueue)) {
readyQueue.forEach(function(readyQueueItem) {
if (isFunction(readyQueueItem)) {
readyQueueItem();
} else if (Array.isArray(readyQueueItem)) {
processPreloadedItem(readyQueueItem);
}
});
}
// https://go.mparticle.com/work/SQDSDKS-6835
return [];
}

function queueIfNotInitialized(func, self) {
if (!self.isInitialized()) {
self.ready(function() {
Expand Down
38 changes: 38 additions & 0 deletions src/pre-init-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { isEmpty, isFunction } from './utils';

export const processReadyQueue = (readyQueue): Function[] => {
if (!isEmpty(readyQueue)) {
readyQueue.forEach(readyQueueItem => {
if (isFunction(readyQueueItem)) {
// debugger;
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved
readyQueueItem();
} else if (Array.isArray(readyQueueItem)) {
// debugger;
alexs-mparticle marked this conversation as resolved.
Show resolved Hide resolved
processPreloadedItem(readyQueueItem);
}
});
}
return [];
};

const processPreloadedItem = (readyQueueItem): void => {
const args = readyQueueItem;
const method = args.splice(0, 1)[0];

// if the first argument is a method on the base mParticle object, run it
if (typeof window !== 'undefined' && window.mParticle && window.mParticle[args[0]]) {
window.mParticle[method].apply(this, args);
// otherwise, the method is on either eCommerce or Identity objects, ie. "eCommerce.setCurrencyCode", "Identity.login"
} else {
const methodArray = method.split('.');
try {
let computedMPFunction = window.mParticle;
for (const currentMethod of methodArray) {
computedMPFunction = computedMPFunction[currentMethod];
}
((computedMPFunction as unknown) as Function).apply(this, args);
} catch (e) {
throw new Error('Unable to compute proper mParticle function ' + e);
}
}
};
56 changes: 56 additions & 0 deletions test/jest/pre-init-utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { processReadyQueue } from '../../src/pre-init-utils';

describe('pre-init-utils', () => {
describe('#processReadyQueue', () => {
it('should return an empty array if readyQueue is empty', () => {
const result = processReadyQueue([]);
expect(result).toEqual([]);
});

it('should process functions passed as arguments', () => {
const functionSpy = jest.fn();
const readyQueue: Function[] = [functionSpy, functionSpy, functionSpy];
const result = processReadyQueue(readyQueue);
expect(functionSpy).toHaveBeenCalledTimes(3);
expect(result).toEqual([]);
});

it('should process functions passed as arrays', () => {
const functionSpy = jest.fn();
(window.mParticle as any) = {
fakeFunction: functionSpy,
};
const readyQueue = [['fakeFunction']];
processReadyQueue(readyQueue);
expect(functionSpy).toHaveBeenCalled();
});

it('should process functions passed as arrays with arguments', () => {
const functionSpy = jest.fn();
(window.mParticle as any) = {
fakeFunction: functionSpy,
args: () => {},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this args function necessary? I haven't pulled this code down, but as I read through the processReadyQueue function, the only times I see args is where it references the item in the array, as opposed to it being a key on window.mParticle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to discussing this further. Adding args here was the only way I could get it to hit that specific codepath in my tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's hop on a zoom about this to walk through

Copy link
Member

@rmi22186 rmi22186 Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm. when i comment out line 32, it passes for me. see screenshot below:
image

};
const readyQueue = [['fakeFunction', 'args']];
processReadyQueue(readyQueue);
expect(functionSpy).toHaveBeenCalledWith('args');
});

it('should process arrays passed as arguments with multiple methods', () => {
const functionSpy = jest.fn();
(window.mParticle as any) = {
fakeFunction: {
anotherFakeFunction: functionSpy,
},
};
const readyQueue = [['fakeFunction.anotherFakeFunction', 'foo']];
processReadyQueue(readyQueue);
expect(functionSpy).toHaveBeenCalledWith('foo');
});
rmi22186 marked this conversation as resolved.
Show resolved Hide resolved

it('should throw an error if it cannot compute the proper mParticle function', () => {
const readyQueue = [['Identity.login']];
expect(() => processReadyQueue(readyQueue)).toThrowError("Unable to compute proper mParticle function TypeError: Cannot read properties of undefined (reading 'login')");
});
});
});
2 changes: 1 addition & 1 deletion test/src/_test.index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import './tests-kit-blocking';
import './tests-event-logging';
import './tests-eCommerce';
import './tests-persistence';
import './tests-forwarders';
import './tests-helpers';
import './tests-forwarders';
import './tests-cookie-syncing';
import './tests-identities-attributes';
import './tests-native-sdk';
Expand Down
4 changes: 3 additions & 1 deletion test/src/config/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,8 @@ var pluses = /\+/g,
},
hasIdentifyReturned = () => {
return window.mParticle.Identity.getCurrentUser()?.getMPID() === testMPID;
};
},
hasIdentityCallInflightReturned = () => !mParticle.getInstance()?._Store?.identityCallInFlight;

var TestsCore = {
getLocalStorageProducts: getLocalStorageProducts,
Expand Down Expand Up @@ -658,6 +659,7 @@ var TestsCore = {
waitForCondition: waitForCondition,
fetchMockSuccess: fetchMockSuccess,
hasIdentifyReturned: hasIdentifyReturned,
hasIdentityCallInflightReturned,
};

export default TestsCore;
38 changes: 16 additions & 22 deletions test/src/tests-core-sdk.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { expect } from 'chai';
import Utils from './config/utils';
import Store from '../../src/store';
import Constants, { HTTP_ACCEPTED, HTTP_OK } from '../../src/constants';
Expand All @@ -11,7 +12,7 @@ const DefaultConfig = Constants.DefaultConfig,
findEventFromRequest = Utils.findEventFromRequest,
findBatch = Utils.findBatch;

const { waitForCondition, fetchMockSuccess, hasIdentifyReturned } = Utils;
const { waitForCondition, fetchMockSuccess, hasIdentifyReturned, hasIdentityCallInflightReturned } = Utils;

describe('core SDK', function() {
beforeEach(function() {
Expand Down Expand Up @@ -121,7 +122,7 @@ describe('core SDK', function() {
});
});

it('should process ready queue when initialized', function(done) {
it('should process ready queue when initialized', async function() {
let readyFuncCalled = false;

mParticle._resetForTests(MPConfig);
Expand All @@ -130,10 +131,9 @@ describe('core SDK', function() {
readyFuncCalled = true;
});
mParticle.init(apiKey, window.mParticle.config);
await waitForCondition(hasIdentityCallInflightReturned);

Should(readyFuncCalled).equal(true);

done();
expect(readyFuncCalled).equal(true);
});

it('should set app version on the payload', function(done) {
Expand All @@ -150,14 +150,12 @@ describe('core SDK', function() {
})
});

it('should get app version', function(done) {
it('should get app version', async function() {
await waitForCondition(hasIdentityCallInflightReturned);
mParticle.setAppVersion('2.0');

const appVersion = mParticle.getAppVersion();

appVersion.should.equal('2.0');

done();
expect(appVersion).to.equal('2.0');
});

it('should get environment setting when set to `production`', function(done) {
Expand Down Expand Up @@ -1191,15 +1189,14 @@ describe('core SDK', function() {
});
});

it('should initialize without a config object passed to init', function(done) {
it('should initialize without a config object passed to init', async function() {
// this instance occurs when self hosting and the user only passes an object into init
mParticle._resetForTests(MPConfig);

mParticle.init(apiKey);
await waitForCondition(hasIdentityCallInflightReturned);

mParticle.getInstance()._Store.isInitialized.should.equal(true);

done();
});

it('should generate hash both on the mparticle instance and the mparticle instance manager', function(done) {
Expand Down Expand Up @@ -1262,18 +1259,17 @@ describe('core SDK', function() {
done();
});

it('should set a device id when calling setDeviceId', function(done) {
it('should set a device id when calling setDeviceId', async function() {
mParticle._resetForTests(MPConfig);

mParticle.init(apiKey, window.mParticle.config);
await waitForCondition(hasIdentityCallInflightReturned);
// this das should be the SDK auto generated one, which is 36 characters long
mParticle.getDeviceId().length.should.equal(36);

mParticle.setDeviceId('foo-guid');

mParticle.getDeviceId().should.equal('foo-guid');

done();
});

it('should set a device id when set on mParticle.config', function(done) {
Expand Down Expand Up @@ -1310,34 +1306,32 @@ describe('core SDK', function() {
done();
});

it('should set the wrapper sdk info in Store when mParticle._setWrapperSDKInfo() method is called after init is called', function(done) {
it('should set the wrapper sdk info in Store when mParticle._setWrapperSDKInfo() method is called after init is called', async function() {
mParticle._resetForTests(MPConfig);

mParticle._setWrapperSDKInfo('flutter', '1.0.3');

mParticle.init(apiKey, window.mParticle.config);
await waitForCondition(hasIdentityCallInflightReturned);

mParticle.getInstance()._Store.wrapperSDKInfo.name.should.equal('flutter');
mParticle.getInstance()._Store.wrapperSDKInfo.version.should.equal('1.0.3');
mParticle.getInstance()._Store.wrapperSDKInfo.isInfoSet.should.equal(true);

done();
});

it('should not set the wrapper sdk info in Store after it has previously been set', function(done) {
it('should not set the wrapper sdk info in Store after it has previously been set', async function() {
mParticle._resetForTests(MPConfig);

mParticle._setWrapperSDKInfo('flutter', '1.0.3');

mParticle.init(apiKey, window.mParticle.config);
await waitForCondition(hasIdentityCallInflightReturned);

mParticle._setWrapperSDKInfo('none', '2.0.5');

mParticle.getInstance()._Store.wrapperSDKInfo.name.should.equal('flutter');
mParticle.getInstance()._Store.wrapperSDKInfo.version.should.equal('1.0.3');
mParticle.getInstance()._Store.wrapperSDKInfo.isInfoSet.should.equal(true);

done();
});

describe('pod feature flag', function() {
Expand Down
Loading
Loading