Skip to content

Commit 15f99da

Browse files
committed
Catch invalid type & action for sendOdpEvent + patch tests
2 parents 81041d9 + d79df98 commit 15f99da

File tree

4 files changed

+105
-14
lines changed

4 files changed

+105
-14
lines changed

packages/optimizely-sdk/lib/core/odp/odp_manager.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,12 @@ export abstract class OdpManager implements IOdpManager {
166166
* @param {OdpEvent} > ODP Event to send to event manager
167167
*/
168168
sendEvent({ type, action, identifiers, data }: OdpEvent): void {
169+
let mType = type;
170+
171+
if (typeof mType !== 'string' || mType === '') {
172+
mType = 'fullstack';
173+
}
174+
169175
if (!this.enabled) {
170176
throw new Error(ERROR_MESSAGES.ODP_NOT_ENABLED);
171177
}
@@ -182,7 +188,10 @@ export abstract class OdpManager implements IOdpManager {
182188
throw new Error(ERROR_MESSAGES.ODP_SEND_EVENT_FAILED_EVENT_MANAGER_MISSING);
183189
}
184190

185-
this.eventManager.sendEvent(new OdpEvent(type, action, identifiers, data));
191+
if (typeof action !== 'string' || action === '') {
192+
throw new Error('ODP action is not valid (cannot be empty).');
193+
}
194+
186195
}
187196

188197
abstract isVuidEnabled(): boolean;

packages/optimizely-sdk/lib/index.browser.tests.js

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { BrowserOdpManager } from './plugins/odp_manager/index.browser';
3030
import { OdpConfig } from './core/odp/odp_config';
3131
import { BrowserOdpEventManager } from './plugins/odp/event_manager/index.browser';
3232
import { BrowserOdpEventApiManager } from './plugins/odp/event_api_manager/index.browser';
33+
import { OdpEvent } from './core/odp/odp_event';
3334

3435
var LocalStoragePendingEventsDispatcher = eventProcessor.LocalStoragePendingEventsDispatcher;
3536

@@ -771,7 +772,7 @@ describe('javascript-sdk (Browser)', function() {
771772
sinon.assert.called(fakeEventManager.start);
772773
});
773774

774-
it('should send an odp event with sendOdpEvent', async () => {
775+
it('should send an odp event when calling sendOdpEvent with valid parameters', async () => {
775776
const fakeEventManager = {
776777
updateSettings: sinon.spy(),
777778
start: sinon.spy(),
@@ -803,6 +804,80 @@ describe('javascript-sdk (Browser)', function() {
803804
sinon.assert.called(fakeEventManager.sendEvent);
804805
});
805806

807+
it('should throw an error and not send an odp event when calling sendOdpEvent with an invalid action input', async () => {
808+
const fakeEventManager = {
809+
updateSettings: sinon.spy(),
810+
start: sinon.spy(),
811+
stop: sinon.spy(),
812+
registerVuid: sinon.spy(),
813+
identifyUser: sinon.spy(),
814+
sendEvent: sinon.spy(),
815+
flush: sinon.spy(),
816+
};
817+
818+
const client = optimizelyFactory.createInstance({
819+
datafile: testData.getOdpIntegratedConfigWithSegments(),
820+
errorHandler: fakeErrorHandler,
821+
eventDispatcher: fakeEventDispatcher,
822+
eventBatchSize: null,
823+
logger,
824+
odpOptions: {
825+
eventManager: fakeEventManager,
826+
},
827+
});
828+
829+
const readyData = await client.onReady();
830+
assert.equal(readyData.success, true);
831+
assert.isUndefined(readyData.reason);
832+
833+
client.sendOdpEvent('');
834+
sinon.assert.called(logger.error);
835+
836+
client.sendOdpEvent(null);
837+
sinon.assert.calledTwice(logger.error);
838+
839+
client.sendOdpEvent(undefined);
840+
sinon.assert.calledThrice(logger.error);
841+
842+
sinon.assert.notCalled(fakeEventManager.sendEvent);
843+
});
844+
845+
it('should use fullstack as a fallback value for the odp event when calling sendOdpEvent with an empty type input', async () => {
846+
const fakeEventManager = {
847+
updateSettings: sinon.spy(),
848+
start: sinon.spy(),
849+
stop: sinon.spy(),
850+
registerVuid: sinon.spy(),
851+
identifyUser: sinon.spy(),
852+
sendEvent: sinon.spy(),
853+
flush: sinon.spy(),
854+
};
855+
856+
const client = optimizelyFactory.createInstance({
857+
datafile: testData.getOdpIntegratedConfigWithSegments(),
858+
errorHandler: fakeErrorHandler,
859+
eventDispatcher: fakeEventDispatcher,
860+
eventBatchSize: null,
861+
logger,
862+
odpOptions: {
863+
eventManager: fakeEventManager,
864+
},
865+
});
866+
867+
const readyData = await client.onReady();
868+
assert.equal(readyData.success, true);
869+
assert.isUndefined(readyData.reason);
870+
871+
client.sendOdpEvent('dummy-action', '');
872+
873+
sinon.assert.calledWith(fakeEventManager.sendEvent, {
874+
action: 'dummy-action',
875+
type: 'fullstack',
876+
identifiers: new Map(),
877+
data: new Map(),
878+
});
879+
});
880+
806881
it('should log an error when attempting to send an odp event when odp is disabled', async () => {
807882
const client = optimizelyFactory.createInstance({
808883
datafile: testData.getTestProjectConfigWithFeatures(),

packages/optimizely-sdk/lib/plugins/odp/event_manager/index.browser.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
1-
import { OdpEventManager } from "../../../../lib/core/odp/odp_event_manager";
1+
import { IOdpEventManager, OdpEventManager } from '../../../../lib/core/odp/odp_event_manager';
22
import { LogLevel } from '../../../modules/logging';
33

44
const DEFAULT_BROWSER_QUEUE_SIZE = 100;
55

6-
export class BrowserOdpEventManager extends OdpEventManager {
7-
protected initParams(batchSize: number | undefined, queueSize: number | undefined, flushInterval: number | undefined): void {
6+
export class BrowserOdpEventManager extends OdpEventManager implements IOdpEventManager {
7+
protected initParams(
8+
batchSize: number | undefined,
9+
queueSize: number | undefined,
10+
flushInterval: number | undefined
11+
): void {
812
this.queueSize = queueSize || DEFAULT_BROWSER_QUEUE_SIZE;
913

1014
// disable event batching for browser
1115
this.batchSize = 1;
1216
this.flushInterval = 0;
13-
17+
1418
if (typeof batchSize !== 'undefined' && batchSize !== 1) {
1519
this.getLogger().log(LogLevel.WARNING, 'ODP event batch size must be 1 in the browser.');
1620
}
@@ -24,4 +28,4 @@ export class BrowserOdpEventManager extends OdpEventManager {
2428
// in Browser/client-side context, give debug message but leave events in queue
2529
this.getLogger().log(LogLevel.DEBUG, 'ODPConfig not ready. Leaving events in queue.');
2630
}
27-
}
31+
}

packages/optimizely-sdk/lib/plugins/odp/event_manager/index.node.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1-
2-
import { OdpEvent } from "../../../../lib/core/odp/odp_event";
3-
import { OdpEventManager } from "../../../../lib/core/odp/odp_event_manager";
4-
import { LogLevel } from "../../../../lib/modules/logging";
1+
import { OdpEvent } from '../../../../lib/core/odp/odp_event';
2+
import { IOdpEventManager, OdpEventManager } from '../../../../lib/core/odp/odp_event_manager';
3+
import { LogLevel } from '../../../../lib/modules/logging';
54

65
const DEFAULT_BATCH_SIZE = 10;
76
const DEFAULT_FLUSH_INTERVAL_MSECS = 1000;
87
const DEFAULT_SERVER_QUEUE_SIZE = 10000;
98

10-
export class NodeOdpEventManager extends OdpEventManager {
11-
protected initParams(batchSize: number | undefined, queueSize: number | undefined, flushInterval: number | undefined): void {
9+
export class NodeOdpEventManager extends OdpEventManager implements IOdpEventManager {
10+
protected initParams(
11+
batchSize: number | undefined,
12+
queueSize: number | undefined,
13+
flushInterval: number | undefined
14+
): void {
1215
this.queueSize = queueSize || DEFAULT_SERVER_QUEUE_SIZE;
1316
this.batchSize = batchSize || DEFAULT_BATCH_SIZE;
1417

@@ -26,4 +29,4 @@ export class NodeOdpEventManager extends OdpEventManager {
2629
this.getLogger().log(LogLevel.WARNING, 'ODPConfig not ready. Discarding events in queue.');
2730
this.queue = new Array<OdpEvent>();
2831
}
29-
}
32+
}

0 commit comments

Comments
 (0)