From df07e79519558824db2edb0f7e2a570e1c8cfb03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Tue, 19 Dec 2023 11:20:43 -0300 Subject: [PATCH 1/8] Add validateValueType function to ensure data type integrity --- .../__tests__/createIntegrationEntity.test.ts | 61 ++++++++++++++----- .../src/data/createIntegrationEntity.ts | 46 ++++++++++++-- 2 files changed, 86 insertions(+), 21 deletions(-) diff --git a/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts b/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts index 8061ea6c2..1350ead98 100644 --- a/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts +++ b/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts @@ -5,6 +5,7 @@ import { IntegrationEntityData, schemaWhitelistedPropertyNames, schemaWhitelists, + validateValueType, } from '../createIntegrationEntity'; const networkSourceData = { @@ -13,8 +14,6 @@ const networkSourceData = { CIDR: '255.255.255.0', name: 'My Network', notInDataModel: 'Not In Data Model', - owner: { name: 'Bob' }, - summary: [{ title: 'Summary' }, { description: 'Description' }], }; const networkResourceEntity = { @@ -74,6 +73,50 @@ describe('schemaWhitelistedPropertyNames', () => { }); }); +describe('validateValueType', () => { + test('should accept string type', () => { + expect(() => validateValueType('Sample String', 'testPath')).not.toThrow(); + }); + + test('should accept number type', () => { + expect(() => validateValueType(123, 'testPath')).not.toThrow(); + }); + + test('should accept boolean type', () => { + expect(() => validateValueType(true, 'testPath')).not.toThrow(); + }); + + test('should accept null type', () => { + expect(() => validateValueType(null, 'testPath')).not.toThrow(); + }); + + test('should accept array of supported types', () => { + expect(() => validateValueType([1, 'a', true], 'testPath')).not.toThrow(); + }); + + test('should validate nested arrays with supported types', () => { + expect(() => + validateValueType([1, ['a', 2], [true, false]], 'testPath'), + ).not.toThrow(); + }); + + test('should reject nested arrays with unsupported types', () => { + expect(() => + validateValueType([1, ['a', { key: 'value' }], true], 'testPath'), + ).toThrow(); + }); + + test('should reject object type', () => { + expect(() => validateValueType({ key: 'value' }, 'testPath')).toThrow(); + }); + + test('should reject unsupported type in array', () => { + expect(() => + validateValueType([1, 2, { key: 'value' }], 'testPath'), + ).toThrow(); + }); +}); + describe('createIntegrationEntity', () => { test('combines source with assignments', () => { const entity = createIntegrationEntity({ @@ -118,20 +161,6 @@ describe('createIntegrationEntity', () => { expect('notWhitelisted' in entity).toBe(false); }); - test('ignore source properties that are object types', () => { - const entity = createIntegrationEntity({ - entityData, - }); - expect('owner' in entity).toBe(false); - }); - - test('ignore source properties that are object[] types', () => { - const entity = createIntegrationEntity({ - entityData, - }); - expect('summary' in entity).toBe(false); - }); - test('handles empty tags in source data', () => { const entity = createIntegrationEntity({ entityData: { diff --git a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts index ce0f821e2..3a5b6b1e8 100644 --- a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts +++ b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts @@ -10,7 +10,7 @@ import { parseTimePropertyValue } from './converters'; import { validateRawData } from './rawData'; import { assignTags, ResourceTagList, ResourceTagMap } from './tagging'; -const SUPPORTED_TYPES = ['string', 'number', 'boolean']; +const SUPPORTED_TYPES = ['string', 'number', 'boolean', 'array', 'undefined']; /** * Properties to be assigned to a generated entity which are declared in code @@ -179,6 +179,40 @@ function generateEntity({ return entity; } +/** + * Validates that the provided value conforms to the supported types. + * If the value is an array, this function is called recursively on each element + * to ensure nested arrays are also validated. This function throws an error + * if it encounters a value type that is not supported. + * + * @param value The value to be validated. It can be a single value or an array. + * For arrays, each element is validated recursively. + * @param path The path to the current property being validated, used for error messaging. + * This is updated with each recursive call to reflect the current context. + */ +export function validateValueType(value: any, path: string): void { + // Explicitly allow null values + if (value === null) { + return; + } + + if (Array.isArray(value)) { + // If the value is an array, validate each element + value.forEach((item, index) => { + validateValueType(item, `${path}[${index}]`); + }); + } else { + // For non-array values, check if the type is supported + const valueType = typeof value; + if (!SUPPORTED_TYPES.includes(valueType)) { + throw new IntegrationError({ + code: 'UNSUPPORTED_TYPE', + message: `Unsupported type found at "${path}": ${valueType}`, + }); + } + } +} + /** * Answers a form of the provider data with only the properties supported by the * data model schema. @@ -192,12 +226,14 @@ function whitelistedProviderData( ): Omit { const whitelistedProviderData: ProviderSourceData = {}; const schemaProperties = schemaWhitelistedPropertyNames(_class); + for (const [key, value] of Object.entries(source)) { + // Ensure the property is part of the schema and not null if (value != null && schemaProperties.includes(key)) { - const valueType = Array.isArray(value) ? typeof value[0] : typeof value; - if (SUPPORTED_TYPES.includes(valueType)) { - whitelistedProviderData[key] = value; - } + if (key != 'tags') validateValueType(value, key); + + // If validation passes, assign the value to the whitelisted data + whitelistedProviderData[key] = value; } } return whitelistedProviderData; From aa054766de755a656b351c0dd6ef6306758fe062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Tue, 19 Dec 2023 11:51:23 -0300 Subject: [PATCH 2/8] Add validateValueType performance tests --- .../__tests__/createIntegrationEntity.test.ts | 45 ------ .../data/__tests__/validateValueType.test.ts | 132 ++++++++++++++++++ 2 files changed, 132 insertions(+), 45 deletions(-) create mode 100644 packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts diff --git a/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts b/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts index 1350ead98..d0fe594b9 100644 --- a/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts +++ b/packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts @@ -5,7 +5,6 @@ import { IntegrationEntityData, schemaWhitelistedPropertyNames, schemaWhitelists, - validateValueType, } from '../createIntegrationEntity'; const networkSourceData = { @@ -73,50 +72,6 @@ describe('schemaWhitelistedPropertyNames', () => { }); }); -describe('validateValueType', () => { - test('should accept string type', () => { - expect(() => validateValueType('Sample String', 'testPath')).not.toThrow(); - }); - - test('should accept number type', () => { - expect(() => validateValueType(123, 'testPath')).not.toThrow(); - }); - - test('should accept boolean type', () => { - expect(() => validateValueType(true, 'testPath')).not.toThrow(); - }); - - test('should accept null type', () => { - expect(() => validateValueType(null, 'testPath')).not.toThrow(); - }); - - test('should accept array of supported types', () => { - expect(() => validateValueType([1, 'a', true], 'testPath')).not.toThrow(); - }); - - test('should validate nested arrays with supported types', () => { - expect(() => - validateValueType([1, ['a', 2], [true, false]], 'testPath'), - ).not.toThrow(); - }); - - test('should reject nested arrays with unsupported types', () => { - expect(() => - validateValueType([1, ['a', { key: 'value' }], true], 'testPath'), - ).toThrow(); - }); - - test('should reject object type', () => { - expect(() => validateValueType({ key: 'value' }, 'testPath')).toThrow(); - }); - - test('should reject unsupported type in array', () => { - expect(() => - validateValueType([1, 2, { key: 'value' }], 'testPath'), - ).toThrow(); - }); -}); - describe('createIntegrationEntity', () => { test('combines source with assignments', () => { const entity = createIntegrationEntity({ diff --git a/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts b/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts new file mode 100644 index 000000000..afaa541bd --- /dev/null +++ b/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts @@ -0,0 +1,132 @@ +/* eslint-disable no-console */ +import { validateValueType } from '../createIntegrationEntity'; + +describe('validateValueType', () => { + test('should accept string type', () => { + expect(() => validateValueType('Sample String', 'testPath')).not.toThrow(); + }); + + test('should accept number type', () => { + expect(() => validateValueType(123, 'testPath')).not.toThrow(); + }); + + test('should accept boolean type', () => { + expect(() => validateValueType(true, 'testPath')).not.toThrow(); + }); + + test('should accept null type', () => { + expect(() => validateValueType(null, 'testPath')).not.toThrow(); + }); + + test('should accept array of supported types', () => { + expect(() => validateValueType([1, 'a', true], 'testPath')).not.toThrow(); + }); + + test('should validate nested arrays with supported types', () => { + expect(() => + validateValueType([1, ['a', 2], [true, false]], 'testPath'), + ).not.toThrow(); + }); + + test('should reject nested arrays with unsupported types', () => { + expect(() => + validateValueType([1, ['a', { key: 'value' }], true], 'testPath'), + ).toThrow(); + }); + + test('should reject object type', () => { + expect(() => validateValueType({ key: 'value' }, 'testPath')).toThrow(); + }); + + test('should reject unsupported type in array', () => { + expect(() => + validateValueType([1, 2, { key: 'value' }], 'testPath'), + ).toThrow(); + }); +}); + +describe('validateValueType - successful performance tests', () => { + const largeDatasetSize = 500000; + + function generateLargeDataset(type) { + switch (type) { + case 'string': + return new Array(largeDatasetSize).fill('testString'); + case 'number': + return Array.from({ length: largeDatasetSize }, (_, i) => i); + case 'boolean': + return new Array(largeDatasetSize).fill(true); + case 'array': + return new Array(largeDatasetSize).fill([1, 2, 3]); + case 'undefined': + return new Array(largeDatasetSize).fill(undefined); + default: + throw new Error(`Unsupported type: ${type}`); + } + } + + ['string', 'number', 'boolean', 'array', 'undefined'].forEach((type) => { + test(`should handle large dataset of type ${type} efficiently`, () => { + const largeDataset = generateLargeDataset(type); + + const start = performance.now(); + largeDataset.forEach((item) => + validateValueType(item, `testPath-${type}`), + ); + const end = performance.now(); + + const executionTime = end - start; + + console.log(`Execution time for ${type} dataset: ${executionTime} ms`); + + const acceptableTimeLimit = 500; + expect(executionTime).toBeLessThan(acceptableTimeLimit); + }); + }); +}); + +describe('validateValueType - error Scenarios', () => { + const largeDatasetSize = 500000; + + function generateErrorDataset() { + const data: any[] = []; + for (let i = 0; i < largeDatasetSize; i++) { + if (i % 50000 === 0) { + data.push({ key: `value-${i}` }); + } else { + data.push(i % 2 === 0 ? `string-${i}` : i); + } + } + return data; + } + + test(`should throw error for large dataset with unsupported types`, () => { + const errorDataset = generateErrorDataset(); + let executionTime = 0; + + const start = performance.now(); + try { + errorDataset.forEach((item) => + validateValueType(item, `testPath-errorCase`), + ); + const end = performance.now(); + executionTime = end - start; + } catch (error) { + const end = performance.now(); + executionTime = end - start; + + console.log(`Execution time until error: ${executionTime} ms`); + } + + expect(executionTime).toBeGreaterThan(0); + + const acceptableErrorDetectionTime = 1000; + expect(executionTime).toBeLessThan(acceptableErrorDetectionTime); + + expect(() => { + errorDataset.forEach((item) => + validateValueType(item, `testPath-errorCase`), + ); + }).toThrow(); + }); +}); From 193fbbd7b63c8126ff07e2eae59fbb5a7e40b94b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Tue, 19 Dec 2023 11:57:21 -0300 Subject: [PATCH 3/8] Increase acceptableTimeLimit to 1 sec for real world machines --- .../src/data/__tests__/validateValueType.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts b/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts index afaa541bd..47f4e7bf5 100644 --- a/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts +++ b/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts @@ -79,7 +79,7 @@ describe('validateValueType - successful performance tests', () => { console.log(`Execution time for ${type} dataset: ${executionTime} ms`); - const acceptableTimeLimit = 500; + const acceptableTimeLimit = 1000; expect(executionTime).toBeLessThan(acceptableTimeLimit); }); }); From ca0433449ff782735bfe46ab8492f7ca7dae5406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Tue, 2 Jan 2024 13:41:21 -0300 Subject: [PATCH 4/8] Reject nested arrays --- .../src/data/__tests__/validateValueType.test.ts | 12 ++++++------ .../src/data/createIntegrationEntity.ts | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts b/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts index 47f4e7bf5..285dc73f7 100644 --- a/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts +++ b/packages/integration-sdk-core/src/data/__tests__/validateValueType.test.ts @@ -22,12 +22,6 @@ describe('validateValueType', () => { expect(() => validateValueType([1, 'a', true], 'testPath')).not.toThrow(); }); - test('should validate nested arrays with supported types', () => { - expect(() => - validateValueType([1, ['a', 2], [true, false]], 'testPath'), - ).not.toThrow(); - }); - test('should reject nested arrays with unsupported types', () => { expect(() => validateValueType([1, ['a', { key: 'value' }], true], 'testPath'), @@ -43,6 +37,12 @@ describe('validateValueType', () => { validateValueType([1, 2, { key: 'value' }], 'testPath'), ).toThrow(); }); + + test('should reject array inside arrays', () => { + expect(() => + validateValueType([1, 2, { key: 'value' }, [1, [4, 5, 6]]], 'testPath'), + ).toThrow(); + }); }); describe('validateValueType - successful performance tests', () => { diff --git a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts index 3a5b6b1e8..7f4ca6d28 100644 --- a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts +++ b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts @@ -190,16 +190,28 @@ function generateEntity({ * @param path The path to the current property being validated, used for error messaging. * This is updated with each recursive call to reflect the current context. */ -export function validateValueType(value: any, path: string): void { +export function validateValueType( + value: any, + path: string, + depth: number = 1, +): void { // Explicitly allow null values if (value === null) { return; } if (Array.isArray(value)) { + // If the depth is > 1 then we won't allow arrays inside arrays. + if (depth > 1) { + throw new IntegrationError({ + code: 'UNSUPPORTED_TYPE', + message: `Unsupported type found at "${path}": Nested arrays are not supported.`, + }); + } + // If the value is an array, validate each element value.forEach((item, index) => { - validateValueType(item, `${path}[${index}]`); + validateValueType(item, `${path}[${index}]`, ++depth); }); } else { // For non-array values, check if the type is supported From cdbe8dd1587cf61fe38c59635e034fc1ddb51640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Tue, 2 Jan 2024 16:11:57 -0300 Subject: [PATCH 5/8] Rename depth to currentValueDepth and add jsdocs for said value --- .../src/data/createIntegrationEntity.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts index 7f4ca6d28..532728e65 100644 --- a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts +++ b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts @@ -193,7 +193,7 @@ function generateEntity({ export function validateValueType( value: any, path: string, - depth: number = 1, + currentValueDepth: number = 1, ): void { // Explicitly allow null values if (value === null) { @@ -202,7 +202,7 @@ export function validateValueType( if (Array.isArray(value)) { // If the depth is > 1 then we won't allow arrays inside arrays. - if (depth > 1) { + if (currentValueDepth > 1) { throw new IntegrationError({ code: 'UNSUPPORTED_TYPE', message: `Unsupported type found at "${path}": Nested arrays are not supported.`, @@ -211,7 +211,7 @@ export function validateValueType( // If the value is an array, validate each element value.forEach((item, index) => { - validateValueType(item, `${path}[${index}]`, ++depth); + validateValueType(item, `${path}[${index}]`, ++currentValueDepth); }); } else { // For non-array values, check if the type is supported From c00e6735fe2ab51c819d26c209a51fa96880ebf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Tue, 2 Jan 2024 16:16:46 -0300 Subject: [PATCH 6/8] Add jsDoc for currentValueDepth --- .../integration-sdk-core/src/data/createIntegrationEntity.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts index 532728e65..5841348a0 100644 --- a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts +++ b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts @@ -189,6 +189,9 @@ function generateEntity({ * For arrays, each element is validated recursively. * @param path The path to the current property being validated, used for error messaging. * This is updated with each recursive call to reflect the current context. + * @param currentValueDepth The current depth of recursion for array validation. It starts at 1 + * and increments with each recursive call into deeper array levels. + * This parameter is used to prevent validation of nested arrays beyond the first level. */ export function validateValueType( value: any, From 3e9c797e8afe7844a1c623616f3020d209230b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Tue, 2 Jan 2024 20:27:10 -0300 Subject: [PATCH 7/8] Rename whitelistedProviderData to allowedProviderData --- .../src/data/createIntegrationEntity.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts index 5841348a0..82576b90c 100644 --- a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts +++ b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts @@ -125,7 +125,7 @@ function generateEntity({ const _class = Array.isArray(assign._class) ? assign._class : [assign._class]; const entity: GeneratedEntity = { - ...whitelistedProviderData(source, _class), + ...allowedProviderData(source, _class), ...assign, _class, _rawData, @@ -229,17 +229,18 @@ export function validateValueType( } /** - * Answers a form of the provider data with only the properties supported by the + * Filters the provider data to include only the properties allowed by the * data model schema. * - * @param source resource data from the resource provider/external system - * @param _class entity `_class: string[]` value + * @param {ProviderSourceData} source - The resource data from the provider or external system. + * @param {string[]} _class - An array representing the entity's class for schema validation. + * @returns {Omit} - The filtered provider data with only allowed properties. */ -function whitelistedProviderData( +function allowedProviderData( source: ProviderSourceData, _class: string[], ): Omit { - const whitelistedProviderData: ProviderSourceData = {}; + const filteredProviderData: ProviderSourceData = {}; const schemaProperties = schemaWhitelistedPropertyNames(_class); for (const [key, value] of Object.entries(source)) { @@ -247,11 +248,11 @@ function whitelistedProviderData( if (value != null && schemaProperties.includes(key)) { if (key != 'tags') validateValueType(value, key); - // If validation passes, assign the value to the whitelisted data - whitelistedProviderData[key] = value; + // If validation passes, assign the value to the filtered data + filteredProviderData[key] = value; } } - return whitelistedProviderData; + return filteredProviderData; } /** From 67272d3f6389908411390a45ee5e4d16999dd0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Thu, 4 Jan 2024 13:46:40 -0300 Subject: [PATCH 8/8] Fix typo --- .../integration-sdk-core/src/data/createIntegrationEntity.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts index 0dbf6a81a..70748e9a1 100644 --- a/packages/integration-sdk-core/src/data/createIntegrationEntity.ts +++ b/packages/integration-sdk-core/src/data/createIntegrationEntity.ts @@ -125,7 +125,7 @@ function generateEntity({ const _class = Array.isArray(assign._class) ? assign._class : [assign._class]; const entity: GeneratedEntity = { - ...(source ? allowedProviderData(source, _class), : {}), + ...(source ? allowedProviderData(source, _class) : {}), ...assign, _class, _rawData,