[FSSDK-8950] change identifiers to be required for sendOdpEvent (#823)

* Add event hasNecessaryIdentifiers check during enqueue

* Add unit tests for verifying identifier per context

* Fix requested PR changes

* Change Map initialization

Author: mikechu-optimizely

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

@@ -243,6 +243,11 @@ export abstract class OdpEventManager implements IOdpEventManager {
       return;
     }
 
+    if (!this.hasNecessaryIdentifiers(event)) {
+      this.logger.log(LogLevel.ERROR, 'ODP events should have at least one key-value pair in identifiers.');
+      return;
+    }
+
     if (this.queue.length >= this.queueSize) {
       this.logger.log(
         LogLevel.WARNING,
@@ -257,6 +262,8 @@ export abstract class OdpEventManager implements IOdpEventManager {
     this.processQueue();
   }
 
+  protected abstract hasNecessaryIdentifiers(event: OdpEvent): boolean;
+
   /**
    * Process events in the main queue
    * @param shouldFlush Flush all events regardless of available queue event count

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

@@ -1,5 +1,6 @@
 import { IOdpEventManager, OdpEventManager } from '../../../../lib/core/odp/odp_event_manager';
 import { LogLevel } from '../../../modules/logging';
+import { OdpEvent } from "../../../core/odp/odp_event";
 
 const DEFAULT_BROWSER_QUEUE_SIZE = 100;
 
@@ -28,4 +29,6 @@ export class BrowserOdpEventManager extends OdpEventManager implements IOdpEvent
     // in Browser/client-side context, give debug message but leave events in queue
     this.getLogger().log(LogLevel.DEBUG, 'ODPConfig not ready. Leaving events in queue.');
   }
+
+  protected hasNecessaryIdentifiers = (event: OdpEvent): boolean => event.identifiers.size >= 0;
 }

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

@@ -29,4 +29,6 @@ export class NodeOdpEventManager extends OdpEventManager implements IOdpEventMan
     this.getLogger().log(LogLevel.WARNING, 'ODPConfig not ready. Discarding events in queue.');
     this.queue = new Array<OdpEvent>();
   }
+
+  protected hasNecessaryIdentifiers = (event: OdpEvent): boolean => event.identifiers.size >= 1;
 }

packages/optimizely-sdk/tests/odpEventManager.spec.ts

@@ -14,11 +14,11 @@
  * limitations under the License.
  */
 
-import { ODP_EVENT_ACTION, ODP_DEFAULT_EVENT_TYPE } from './../lib/utils/enums/index';
-
+import { ODP_EVENT_ACTION, ODP_DEFAULT_EVENT_TYPE } from '../lib/utils/enums';
 import { OdpConfig } from '../lib/core/odp/odp_config';
 import { STATE } from '../lib/core/odp/odp_event_manager';
-import { NodeOdpEventManager as OdpEventManager } from '../lib/plugins/odp/event_manager/index.node';
+import { BrowserOdpEventManager } from "../lib/plugins/odp/event_manager/index.browser";
+import { NodeOdpEventManager, NodeOdpEventManager as OdpEventManager } from '../lib/plugins/odp/event_manager/index.node';
 import { anything, capture, instance, mock, resetCalls, spy, verify, when } from 'ts-mockito';
 import { NodeOdpEventApiManager as OdpEventApiManager } from '../lib/plugins/odp/event_api_manager/index.node';
 import { LogHandler, LogLevel } from '../lib/modules/logging';
@@ -33,14 +33,12 @@ const EVENTS: OdpEvent[] = [
     't1',
     'a1',
     new Map([['id-key-1', 'id-value-1']]),
-    new Map(
-      Object.entries({
-        'key-1': 'value1',
-        'key-2': null,
-        'key-3': 3.3,
-        'key-4': true,
-      })
-    )
+    new Map<string, unknown>([
+        ['key-1', 'value1'],
+        ['key-2', null],
+        ['key-3', 3.3],
+        ['key-4', true],
+    ]),
   ),
   new OdpEvent(
     't2',
@@ -90,6 +88,26 @@ const PROCESSED_EVENTS: OdpEvent[] = [
     )
   ),
 ];
+const EVENT_WITH_EMPTY_IDENTIFIER  = new OdpEvent(
+    't4',
+    'a4',
+    new Map(),
+    new Map<string, unknown>([
+      ['key-53f3', true],
+      ['key-a04a', 123],
+      ['key-2ab4', 'Linus Torvalds'],
+    ]),
+);
+const EVENT_WITH_UNDEFINED_IDENTIFIER  = new OdpEvent(
+    't4',
+    'a4',
+    undefined,
+    new Map<string, unknown>([
+      ['key-53f3', false],
+      ['key-a04a', 456],
+      ['key-2ab4', 'Bill Gates']
+    ]),
+);
 const makeEvent = (id: number) => {
   const identifiers = new Map<string, string>();
   identifiers.set('identifier1', 'value1-' + id);
@@ -185,12 +203,10 @@ describe('OdpEventManager', () => {
       't3',
       'a3',
       new Map([['id-key-3', 'id-value-3']]),
-      new Map(
-        Object.entries({
-          'key-1': false,
-          'key-2': { random: 'object', whichShouldFail: true },
-        })
-      )
+      new Map<string, unknown>([
+          ['key-1', false],
+          ['key-2', { random: 'object', whichShouldFail: true }],
+      ]),
     );
     eventManager.sendEvent(badEvent);
 
@@ -455,4 +471,38 @@ describe('OdpEventManager', () => {
     expect(eventManager['odpConfig'].segmentsToCheck).toContain(Array.from(segmentsToCheck)[0]);
     expect(eventManager['odpConfig'].segmentsToCheck).toContain(Array.from(segmentsToCheck)[1]);
   });
+
+  it('should error when no identifiers are provided in Node', () => {
+    const eventManager = new NodeOdpEventManager({
+      odpConfig,
+      apiManager,
+      logger,
+      clientEngine,
+      clientVersion,
+    });
+
+    eventManager.start();
+    eventManager.sendEvent(EVENT_WITH_EMPTY_IDENTIFIER);
+    eventManager.sendEvent(EVENT_WITH_UNDEFINED_IDENTIFIER);
+    eventManager.stop();
+
+    verify(mockLogger.log(LogLevel.ERROR, 'ODP events should have at least one key-value pair in identifiers.')).twice();
+  });
+
+  it('should never error when no identifiers are provided in Browser', () => {
+    const eventManager = new BrowserOdpEventManager({
+      odpConfig,
+      apiManager,
+      logger,
+      clientEngine,
+      clientVersion,
+    });
+
+    eventManager.start();
+    eventManager.sendEvent(EVENT_WITH_EMPTY_IDENTIFIER);
+    eventManager.sendEvent(EVENT_WITH_UNDEFINED_IDENTIFIER);
+    eventManager.stop();
+
+    verify(mockLogger.log(LogLevel.ERROR, 'ODP events should have at least one key-value pair in identifiers.')).never();
+  });
 });