feat(identify): make identify API accept deep object as value (#90)

* feat(identify): make identify API accept deep object as value

* move isValidateProperties to utils and add unit test

* make type recursively tightened

* validateProperties fixes

* add warn when type is invalid

packages/identify/src/identify.ts

@@ -5,7 +5,7 @@ import {
   ValidPropertyType,
   SpecialEventType,
 } from '@amplitude/types';
-import { logger, generateBase36Id } from '@amplitude/utils';
+import { logger, generateBase36Id, isValidProperties } from '@amplitude/utils';
 
 import { UNSET_VALUE, USER_IDENTIFY_OPERATIONS, GROUP_IDENTIFY_OPERATIONS } from './constants';
 
@@ -154,11 +154,6 @@ export class Identify {
       return false;
     }
 
-    if (typeof property !== 'string') {
-      identifyWarn(operation, 'expected string for property but got: ', typeof property, '. Skipping operation');
-      return false;
-    }
-
     if (this._propertySet.has(property)) {
       identifyWarn(operation, 'property ', property, ' already used. Skipping operation');
       return false;
@@ -167,19 +162,8 @@ export class Identify {
     if (operation === IdentifyOperation.ADD) {
       return typeof value === 'number';
     } else if (operation !== IdentifyOperation.UNSET && operation !== IdentifyOperation.REMOVE) {
-      if (Array.isArray(value)) {
-        for (const valueElement of value) {
-          if (!(typeof valueElement === 'number' || typeof valueElement === 'string')) {
-            identifyWarn(operation, 'invalid array element type ', typeof valueElement, '. Skipping operation');
-            return false;
-          }
-        }
-      } else if (!(typeof value === 'number' || typeof value === 'string')) {
-        identifyWarn(operation, 'invalid value type ', typeof value, '. Skipping operation');
-        return false;
-      }
+      return isValidProperties(property, value);
     }
-
     return true;
   }
 }

packages/types/src/identify.ts

@@ -20,7 +20,7 @@ export enum IdentifyOperation {
   CLEAR_ALL = '$clearAll',
 }
 
-export type ValidPropertyType = number | string | Array<string | number>;
+export type ValidPropertyType = number | string | Array<string | number> | { [key: string]: ValidPropertyType };
 
 interface BaseOperationConfig {
   [key: string]: ValidPropertyType;

packages/utils/src/index.ts

@@ -5,3 +5,4 @@ export { AsyncQueue } from './queue';
 export { collectInvalidEventIndices, mapHttpMessageToResponse, mapJSONToResponse } from './response';
 export { mapHttpCodeToStatus } from './status';
 export { isValidEvent } from './validate';
+export { isValidProperties } from './validateProperties';

packages/utils/src/validateProperties.ts

@@ -0,0 +1,42 @@
+import { logger } from './logger';
+const MAX_PROPERTY_KEYS = 1000;
+
+const _isValidObject = (properties: { [key: string]: any }): boolean => {
+  if (Object.keys(properties).length > MAX_PROPERTY_KEYS) {
+    logger.warn('too many properties. Skipping operation');
+    return false;
+  }
+  for (const key in properties) {
+    if (typeof key !== 'string') {
+      logger.warn('invalid properties format. Skipping operation');
+      return false;
+    }
+    let value = properties[key];
+    if (!isValidProperties(key, value)) return false;
+  }
+  return true;
+};
+
+const isValidProperties = (property: string, value: any): boolean => {
+  if (typeof property != 'string') return false;
+  if (Array.isArray(value)) {
+    for (const valueElement of value) {
+      if (Array.isArray(valueElement)) {
+        logger.warn('invalid array element type ', typeof valueElement);
+        return false;
+      } else if (typeof valueElement === 'object') {
+        return _isValidObject(value);
+      } else if (!(typeof valueElement === 'number' || typeof valueElement === 'string')) {
+        logger.warn('invalid array element type ', typeof valueElement);
+        return false;
+      }
+    }
+  } else if (typeof value === 'object') {
+    return _isValidObject(value);
+  } else if (!(typeof value === 'number' || typeof value === 'string')) {
+    logger.warn('invalid value type ', typeof value);
+    return false;
+  }
+  return true;
+};
+export { isValidProperties };

packages/utils/test/validateProperties.test.ts

@@ -0,0 +1,43 @@
+import { isValidProperties } from '../src/validateProperties';
+describe('isValidateProperties', () => {
+  it('should pass on valid properties', () => {
+    const validProperties = {
+      keyForString: 'stringValue',
+      keyForNumber: 123,
+      keyForArray: ['test', 456, { arrayObjKey1: 'arrayObjValue1' }],
+      keyForObj: {
+        objKey1: 'objValue1',
+        objKey2: 'objValue2',
+      },
+    };
+    expect(isValidProperties('property', validProperties)).toBe(true);
+  });
+
+  it('should fail on invalid properties with function as value', () => {
+    const testFunc = () => {
+      return 'test';
+    };
+    const inValidProperties = {
+      keyForFunct: testFunc,
+    };
+    expect(isValidProperties('property', inValidProperties)).toBe(false);
+  });
+
+  it('should fail on invalid properties with array nested in array', () => {
+    const inValidProperties = ['item1', 123, ['subItem1', 'subItem2']];
+    expect(isValidProperties('property', inValidProperties)).toBe(false);
+  });
+
+  it('should fail when any key is not string', () => {
+    const validProperties = {
+      keyForString: 'stringValue',
+      keyForNumber: 123,
+      keyForArray: ['test', 456],
+      keyForObj: {
+        objKey1: 'objValue1',
+        objKey2: 'objValue2',
+      },
+    };
+    expect(isValidProperties(1 as any, validProperties)).toBe(false);
+  });
+});