Validate impressionsDisabled type

Author: ZamoraEmmanuel

src/sdkClient/__tests__/clientInputValidation.spec.ts

@@ -107,4 +107,24 @@ describe('clientInputValidationDecorator', () => {
 
     expect(logSpy).not.toBeCalled();
   });
+
+  test('should ignore the properties in the 4th argument if an empty object is passed', () => {
+    expect(clientWithValidation.getTreatment('key', 'ff', undefined, { impressionsDisabled: true })).toBe(EVALUATION_RESULT);
+    expect(client.getTreatment).toHaveBeenLastCalledWith('key', 'ff', undefined, { impressionsDisabled: true });
+
+    expect(clientWithValidation.getTreatment('key', 'ff', undefined, { impressionsDisabled: {} })).toBe(EVALUATION_RESULT);
+    expect(client.getTreatment).toHaveBeenLastCalledWith('key', 'ff', undefined, undefined);
+
+    expect(clientWithValidation.getTreatment('key', 'ff', undefined, { impressionsDisabled: 'true' })).toBe(EVALUATION_RESULT);
+    expect(client.getTreatment).toHaveBeenLastCalledWith('key', 'ff', undefined, undefined);
+
+    expect(clientWithValidation.getTreatment('key', 'ff', undefined, { impressionsDisabled: null })).toBe(EVALUATION_RESULT);
+    expect(client.getTreatment).toHaveBeenLastCalledWith('key', 'ff', undefined, undefined);
+
+    expect(clientWithValidation.getTreatment('key', 'ff', undefined, { impressionsDisabled: false })).toBe(EVALUATION_RESULT);
+    expect(client.getTreatment).toHaveBeenLastCalledWith('key', 'ff', undefined, undefined); // impressionsDisabled false is the default behavior, so we don't pass it along
+
+    expect(clientWithValidation.getTreatment('key', 'ff', undefined, { properties: undefined })).toBe(EVALUATION_RESULT);
+    expect(client.getTreatment).toHaveBeenLastCalledWith('key', 'ff', undefined, undefined);
+  });
 });

src/utils/inputValidation/__tests__/eventProperties.spec.ts

@@ -1,7 +1,7 @@
-import { ERROR_NOT_PLAIN_OBJECT, ERROR_SIZE_EXCEEDED, WARN_SETTING_NULL, WARN_TRIMMING_PROPERTIES } from '../../../logger/constants';
+import { ERROR_NOT_BOOLEAN, ERROR_NOT_PLAIN_OBJECT, ERROR_SIZE_EXCEEDED, WARN_SETTING_NULL, WARN_TRIMMING_PROPERTIES } from '../../../logger/constants';
 import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
 
-import { validateEventProperties } from '../eventProperties';
+import { validateEventProperties, validateImpressionsDisabled } from '../eventProperties';
 
 function calculateSize(obj: any) {
   // we calculate the expected size.
@@ -193,3 +193,47 @@ describe('INPUT VALIDATION for Event Properties', () => {
     expect(loggerMock.error).toBeCalledWith(ERROR_SIZE_EXCEEDED, ['some_method_eventProps']); // Should log an error.
   });
 });
+
+describe('INPUT VALIDATION for Impressions disabled', () => {
+
+  afterEach(() => { loggerMock.mockClear(); });
+
+  const impressionsDisabledInvalidValues = [
+    [],
+    () => { },
+    'something',
+    NaN,
+    -Infinity,
+    Infinity,
+    new Promise(res => res),
+    Symbol('asd'),
+    new Map()
+  ];
+
+  test('Not setting impressionsDisabled is acceptable', () => {
+    expect(validateImpressionsDisabled(loggerMock, undefined, 'some_method_eventProps')).toBeUndefined();
+
+    expect(loggerMock.error).not.toBeCalled(); // Should not log any errors.
+    expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings.
+  });
+
+  test('When setting a value for impressionsDisabled, only booleans are acceptable', () => {
+
+    impressionsDisabledInvalidValues.forEach(val => {
+      expect(validateImpressionsDisabled(loggerMock, val, 'some_method_eventProps')).toBeUndefined();
+      expect(loggerMock.error).toBeCalledWith(ERROR_NOT_BOOLEAN, ['some_method_eventProps', 'impressionsDisabled']); // Should log an error.
+      loggerMock.error.mockClear();
+    });
+
+    expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings.
+  });
+
+  test('It should return the impressionsDisabled value when valid', () => {
+    expect(validateImpressionsDisabled(loggerMock, true, 'some_method_eventProps')).toBeTruthy();
+    expect(validateImpressionsDisabled(loggerMock, false, 'some_method_eventProps')).toBeFalsy();
+
+    expect(loggerMock.error).not.toBeCalled(); // Should not log any errors.
+    expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings.
+  });
+
+});

src/utils/inputValidation/eventProperties.ts

@@ -2,7 +2,7 @@ import { isObject, isString, isFiniteNumber, isBoolean } from '../lang';
 import { objectAssign } from '../lang/objectAssign';
 import SplitIO from '../../../types/splitio';
 import { ILogger } from '../../logger/types';
-import { ERROR_NOT_PLAIN_OBJECT, ERROR_SIZE_EXCEEDED, WARN_SETTING_NULL, WARN_TRIMMING_PROPERTIES } from '../../logger/constants';
+import { ERROR_NOT_PLAIN_OBJECT, ERROR_NOT_BOOLEAN, ERROR_SIZE_EXCEEDED, WARN_SETTING_NULL, WARN_TRIMMING_PROPERTIES } from '../../logger/constants';
 
 const ECMA_SIZES = {
   NULL: 0, // While on the JSON it's going to occupy more space, we'll take it as 0 for the approximation.
@@ -14,6 +14,17 @@ const MAX_PROPERTIES_AMOUNT = 300;
 const MAX_EVENT_SIZE = 1024 * 32;
 const BASE_EVENT_SIZE = 1024; // We assume 1kb events without properties (avg measured)
 
+export function validateImpressionsDisabled(log: ILogger, maybeImpressionsDisabled: any, method: string): boolean | undefined {
+  if (maybeImpressionsDisabled === undefined) return;
+
+  if (maybeImpressionsDisabled === null || !isBoolean(maybeImpressionsDisabled)) {
+    log.error(ERROR_NOT_BOOLEAN, [method, 'impressionsDisabled']);
+    return;
+  }
+
+  return maybeImpressionsDisabled;
+}
+
 export function validateEventProperties(log: ILogger, maybeProperties: any, method: string): { properties: SplitIO.Properties | null | false, size: number } {
   if (maybeProperties == undefined) return { properties: null, size: BASE_EVENT_SIZE }; // eslint-disable-line eqeqeq
 
@@ -72,7 +83,7 @@ export function validateEvaluationOptions(log: ILogger, maybeOptions: any, metho
     const properties = validateEventProperties(log, maybeOptions.properties, method).properties;
     let options =  properties && Object.keys(properties).length > 0 ? { properties } : undefined;
 
-    const impressionsDisabled = maybeOptions.impressionsDisabled;
+    const impressionsDisabled = validateImpressionsDisabled(log, maybeOptions.impressionsDisabled, method);
     if (!impressionsDisabled) return options;
 
     return options ? { ...options, impressionsDisabled } : { impressionsDisabled };