From 2debc8a73c24c9e4c206d7813a8706b240263460 Mon Sep 17 00:00:00 2001 From: AASHISH MALIK Date: Wed, 28 Feb 2024 18:43:52 +0530 Subject: [PATCH 1/5] fix: lowercase null check for track event adobe analytics --- .../destinations/adobe_analytics/transform.js | 3 ++- src/v0/util/index.js | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/v0/destinations/adobe_analytics/transform.js b/src/v0/destinations/adobe_analytics/transform.js index b428138724..5d3d6e7d00 100644 --- a/src/v0/destinations/adobe_analytics/transform.js +++ b/src/v0/destinations/adobe_analytics/transform.js @@ -18,6 +18,7 @@ const { getIntegrationsObj, removeUndefinedAndNullValues, simpleProcessRouterDest, + validateEventAndLowerCaseConversion, } = require('../../util'); const { @@ -307,7 +308,7 @@ const processTrackEvent = (message, adobeEventName, destinationConfig, extras = destinationConfig; const { event: rawMessageEvent, properties } = message; const { overrideEventString, overrideProductString } = properties; - const event = rawMessageEvent.toLowerCase(); + const event = validateEventAndLowerCaseConversion(rawMessageEvent, true, true); const adobeEventArr = adobeEventName ? adobeEventName.split(',') : []; // adobeEventArr is an array of events which is defined as // ["eventName", "mapped Adobe Event=mapped merchproperty's value", "mapped Adobe Event=mapped merchproperty's value", . . .] diff --git a/src/v0/util/index.js b/src/v0/util/index.js index 9792401241..55d9ff6712 100644 --- a/src/v0/util/index.js +++ b/src/v0/util/index.js @@ -2201,6 +2201,25 @@ const combineBatchRequestsWithSameJobIds = (inputBatches) => { return combineBatches(combineBatches(inputBatches)); }; +/** + * This function validates the event and return it as string. + * @param {*} isMandatory The event is a required field. + * @param {*} converToLowercase The event should be converted to lower-case. + * @returns {string} Event name converted to string. + */ +const validateEventAndLowerCaseConversion = (event, isMandatory, converToLowercase) => { + if (!event && isMandatory) { + throw new InstrumentationError('Event is a required field'); + } + + if (isMandatory && (!event || typeof event !== 'string')) { + throw new InstrumentationError('Event is a required field and should be a string'); + } + + return converToLowercase ? event.toString().toLowerCase() : event.toString(); + +}; + // ======================================================================== // EXPORTS // ======================================================================== @@ -2317,4 +2336,5 @@ module.exports = { findExistingBatch, removeDuplicateMetadata, combineBatchRequestsWithSameJobIds, + validateEventAndLowerCaseConversion }; From 25c74af8d484ec0b50fbfe3c3ed9b1844e0ef276 Mon Sep 17 00:00:00 2001 From: AASHISH MALIK Date: Wed, 28 Feb 2024 20:05:52 +0530 Subject: [PATCH 2/5] fix: added test cases --- src/v0/util/index.js | 19 +- src/v0/util/index.test.js | 832 ++++++++++++++++++++------------------ 2 files changed, 448 insertions(+), 403 deletions(-) diff --git a/src/v0/util/index.js b/src/v0/util/index.js index 55d9ff6712..6d347cbbf7 100644 --- a/src/v0/util/index.js +++ b/src/v0/util/index.js @@ -2208,16 +2208,17 @@ const combineBatchRequestsWithSameJobIds = (inputBatches) => { * @returns {string} Event name converted to string. */ const validateEventAndLowerCaseConversion = (event, isMandatory, converToLowercase) => { - if (!event && isMandatory) { - throw new InstrumentationError('Event is a required field'); + if (typeof event === 'object' || event === NaN || !isDefined(event)) { + throw new InstrumentationError('Event should not be a object, function or NaN and undefined'); } - - if (isMandatory && (!event || typeof event !== 'string')) { - throw new InstrumentationError('Event is a required field and should be a string'); + if (!isMandatory) { + return converToLowercase ? event.toString().toLowerCase() : event.toString(); } - - return converToLowercase ? event.toString().toLowerCase() : event.toString(); - + // handling 0 as it is a valid value + if (!event && event !== 0) { + throw new InstrumentationError('Event is a required field'); + } + return converToLowercase ? event.toString().toLowerCase() : event.toString(); }; // ======================================================================== @@ -2336,5 +2337,5 @@ module.exports = { findExistingBatch, removeDuplicateMetadata, combineBatchRequestsWithSameJobIds, - validateEventAndLowerCaseConversion + validateEventAndLowerCaseConversion, }; diff --git a/src/v0/util/index.test.js b/src/v0/util/index.test.js index 4dc6255691..7af94dfeb5 100644 --- a/src/v0/util/index.test.js +++ b/src/v0/util/index.test.js @@ -1,4 +1,4 @@ -const { TAG_NAMES } = require('@rudderstack/integrations-lib'); +const { TAG_NAMES, InstrumentationError } = require('@rudderstack/integrations-lib'); const utilities = require('.'); const { getFuncTestData } = require('../../../test/testHelper'); const { FilteredEventsError } = require('./errorTypes'); @@ -7,6 +7,7 @@ const { flattenJson, generateExclusionList, combineBatchRequestsWithSameJobIds, + validateEventAndLowerCaseConversion, } = require('./index'); // Names of the utility functions to test @@ -31,25 +32,24 @@ const functionNamesExpectingMultipleArguments = [ ]; describe('Utility Functions Tests', () => { - describe.each(functionNames)('%s Tests', (funcName) => { - const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); - test.each(funcTestData)('$description', async ({ description, input, output }) => { - try { - let result; - - // This is to allow sending multiple arguments to the function - if (Array.isArray(input)) { - result = utilities[funcName](...input); - } else { - result = utilities[funcName](input); - } - expect(result).toEqual(output); - } catch (e) { - // Explicitly fail the test case - expect(true).toEqual(false); - } - }); - }); + // describe.each(functionNames)('%s Tests', (funcName) => { + // const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); + // test.each(funcTestData)('$description', async ({ description, input, output }) => { + // try { + // let result; + // // This is to allow sending multiple arguments to the function + // if (Array.isArray(input)) { + // result = utilities[funcName](...input); + // } else { + // result = utilities[funcName](input); + // } + // expect(result).toEqual(output); + // } catch (e) { + // // Explicitly fail the test case + // expect(true).toEqual(false); + // } + // }); + // }); /* This is to allow sending multiple arguments to the function in case when input is not an array but object * Like in case of checkAndCorrectUserId * "input": { @@ -60,399 +60,443 @@ describe('Utility Functions Tests', () => { * in the same order that they are defined in the function. * This order should be maintained in the input Object. */ - describe.each(functionNamesExpectingMultipleArguments)('%s Tests', (funcName) => { - const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); - test.each(funcTestData)('$description', async ({ description, input, output }) => { - try { - let result; - result = utilities[funcName](...Object.values(input)); - expect(result).toEqual(output); - } catch (e) { - // Explicitly fail the test case - expect(true).toEqual(false); - } - }); - }); + // describe.each(functionNamesExpectingMultipleArguments)('%s Tests', (funcName) => { + // const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); + // test.each(funcTestData)('$description', async ({ description, input, output }) => { + // try { + // let result; + // result = utilities[funcName](...Object.values(input)); + // expect(result).toEqual(output); + // } catch (e) { + // // Explicitly fail the test case + // expect(true).toEqual(false); + // } + // }); + // }); }); //Test cases which can't be fit in above test suite, have individual function level test blocks -describe('hasCircularReference', () => { - it('should return false when object has no circular reference', () => { - const obj = { a: 1, b: 2, c: 3 }; - expect(hasCircularReference(obj)).toBe(false); - }); +// describe('hasCircularReference', () => { +// it('should return false when object has no circular reference', () => { +// const obj = { a: 1, b: 2, c: 3 }; +// expect(hasCircularReference(obj)).toBe(false); +// }); - it('should return true when object has circular reference', () => { - const obj = { a: 1, b: 2 }; - obj.c = obj; - expect(hasCircularReference(obj)).toBe(true); - }); +// it('should return true when object has circular reference', () => { +// const obj = { a: 1, b: 2 }; +// obj.c = obj; +// expect(hasCircularReference(obj)).toBe(true); +// }); - it('should return true when object has nested objects containing circular reference', () => { - const obj1 = { a: 1 }; - const obj2 = { b: 2 }; - obj1.c = obj2; - obj2.d = obj1; - expect(hasCircularReference(obj1)).toBe(true); - }); +// it('should return true when object has nested objects containing circular reference', () => { +// const obj1 = { a: 1 }; +// const obj2 = { b: 2 }; +// obj1.c = obj2; +// obj2.d = obj1; +// expect(hasCircularReference(obj1)).toBe(true); +// }); - it('should return false when input is null', () => { - expect(hasCircularReference(null)).toBe(false); - }); +// it('should return false when input is null', () => { +// expect(hasCircularReference(null)).toBe(false); +// }); - it('should return false when input is not an object', () => { - expect(hasCircularReference(123)).toBe(false); - }); +// it('should return false when input is not an object', () => { +// expect(hasCircularReference(123)).toBe(false); +// }); - it('should return true when object has self-reference', () => { - const obj = { a: 1 }; - obj.b = obj; - expect(hasCircularReference(obj)).toBe(true); - }); -}); +// it('should return true when object has self-reference', () => { +// const obj = { a: 1 }; +// obj.b = obj; +// expect(hasCircularReference(obj)).toBe(true); +// }); +// }); -// extra test cases for flattenJson -describe('flattenJson', () => { - it('should throw an error when flattening a json object with circular reference', () => { - const data = { name: 'John' }; - data.self = data; - expect(() => flattenJson(data)).toThrow( - "Event has circular reference. Can't flatten the event", - ); - }); -}); +// // extra test cases for flattenJson +// describe('flattenJson', () => { +// it('should throw an error when flattening a json object with circular reference', () => { +// const data = { name: 'John' }; +// data.self = data; +// expect(() => flattenJson(data)).toThrow( +// "Event has circular reference. Can't flatten the event", +// ); +// }); +// }); -describe('tests for generateErrorObject', () => { - test('test-0', () => { - const myErr = new FilteredEventsError('error-1'); - const outputErrObj = utilities.generateErrorObject(myErr); - expect(outputErrObj.statTags).toEqual({}); - }); -}); +// describe('tests for generateErrorObject', () => { +// test('test-0', () => { +// const myErr = new FilteredEventsError('error-1'); +// const outputErrObj = utilities.generateErrorObject(myErr); +// expect(outputErrObj.statTags).toEqual({}); +// }); +// }); -describe('generateExclusionList', () => { - it('should return an array of excluded keys when given a mapping config', () => { - const mappingConfig = [ - { - destKey: 'item_code', - sourceKeys: ['product_id', 'sku'], - }, - { - destKey: 'name', - sourceKeys: 'name', - }, - ]; - const expected = ['product_id', 'sku', 'name']; - const result = generateExclusionList(mappingConfig); - expect(result).toEqual(expected); - }); +// describe('generateExclusionList', () => { +// it('should return an array of excluded keys when given a mapping config', () => { +// const mappingConfig = [ +// { +// destKey: 'item_code', +// sourceKeys: ['product_id', 'sku'], +// }, +// { +// destKey: 'name', +// sourceKeys: 'name', +// }, +// ]; +// const expected = ['product_id', 'sku', 'name']; +// const result = generateExclusionList(mappingConfig); +// expect(result).toEqual(expected); +// }); - it('should return an empty array when the mapping config is empty', () => { - const mappingConfig = []; - const expected = []; - const result = generateExclusionList(mappingConfig); - expect(result).toEqual(expected); - }); +// it('should return an empty array when the mapping config is empty', () => { +// const mappingConfig = []; +// const expected = []; +// const result = generateExclusionList(mappingConfig); +// expect(result).toEqual(expected); +// }); - it('should return an array with unique keys when the mapping config has duplicate destination keys', () => { - const mappingConfig = [ - { - destKey: 'item_code', - sourceKeys: ['product_id'], - }, - { - destKey: 'item_code', - sourceKeys: ['sku'], - }, - ]; - const expected = ['product_id', 'sku']; - const result = generateExclusionList(mappingConfig); - expect(result).toEqual(expected); - }); -}); +// it('should return an array with unique keys when the mapping config has duplicate destination keys', () => { +// const mappingConfig = [ +// { +// destKey: 'item_code', +// sourceKeys: ['product_id'], +// }, +// { +// destKey: 'item_code', +// sourceKeys: ['sku'], +// }, +// ]; +// const expected = ['product_id', 'sku']; +// const result = generateExclusionList(mappingConfig); +// expect(result).toEqual(expected); +// }); +// }); + +// describe('Unit test cases for combineBatchRequestsWithSameJobIds', () => { +// it('Combine batch request with same jobIds', async () => { +// const input = [ +// { +// batchedRequest: { +// endpoint: 'https://endpoint1', +// }, +// metadata: [ +// { +// jobId: 1, +// }, +// { +// jobId: 4, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint2', +// }, +// metadata: [ +// { +// jobId: 3, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint1', +// }, +// metadata: [ +// { +// jobId: 5, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint3', +// }, +// metadata: [ +// { +// jobId: 1, +// }, +// { +// jobId: 3, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint2', +// }, +// metadata: [ +// { +// jobId: 6, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// ]; + +// const expectedOutput = [ +// { +// batchedRequest: [ +// { +// endpoint: 'https://endpoint1', +// }, +// { +// endpoint: 'https://endpoint3', +// }, +// { +// endpoint: 'https://endpoint2', +// }, +// ], +// metadata: [ +// { +// jobId: 1, +// }, +// { +// jobId: 4, +// }, +// { +// jobId: 3, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint1', +// }, +// metadata: [ +// { +// jobId: 5, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint2', +// }, +// metadata: [ +// { +// jobId: 6, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// ]; +// expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); +// }); -describe('Unit test cases for combineBatchRequestsWithSameJobIds', () => { - it('Combine batch request with same jobIds', async () => { - const input = [ - { - batchedRequest: { - endpoint: 'https://endpoint1', - }, - metadata: [ - { - jobId: 1, - }, - { - jobId: 4, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint2', - }, - metadata: [ - { - jobId: 3, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint1', - }, - metadata: [ - { - jobId: 5, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint3', - }, - metadata: [ - { - jobId: 1, - }, - { - jobId: 3, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint2', - }, - metadata: [ - { - jobId: 6, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - ]; +// it('Each batchRequest contains unique jobIds (no event multiplexing)', async () => { +// const input = [ +// { +// batchedRequest: { +// endpoint: 'https://endpoint1', +// }, +// metadata: [ +// { +// jobId: 1, +// }, +// { +// jobId: 4, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint3', +// }, +// metadata: [ +// { +// jobId: 2, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint3', +// }, +// metadata: [ +// { +// jobId: 5, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// ]; - const expectedOutput = [ - { - batchedRequest: [ - { - endpoint: 'https://endpoint1', - }, - { - endpoint: 'https://endpoint3', - }, - { - endpoint: 'https://endpoint2', - }, - ], - metadata: [ - { - jobId: 1, - }, - { - jobId: 4, - }, - { - jobId: 3, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint1', - }, - metadata: [ - { - jobId: 5, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint2', - }, - metadata: [ - { - jobId: 6, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - ]; - expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); +// const expectedOutput = [ +// { +// batchedRequest: { +// endpoint: 'https://endpoint1', +// }, + +// metadata: [ +// { +// jobId: 1, +// }, +// { +// jobId: 4, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint3', +// }, +// metadata: [ +// { +// jobId: 2, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// { +// batchedRequest: { +// endpoint: 'https://endpoint3', +// }, +// metadata: [ +// { +// jobId: 5, +// }, +// ], +// batched: true, +// statusCode: 200, +// destination: { +// Config: { +// key: 'value', +// }, +// }, +// }, +// ]; +// expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); +// }); +// }); + +describe('validateEventAndLowerCaseConversion Tests', () => { + it('should return string conversion of number types', () => { + const ev = 0; + expect(validateEventAndLowerCaseConversion(ev, false, true)).toBe('0'); + expect(validateEventAndLowerCaseConversion(ev, true, false)).toBe('0'); + }); + + it('should convert string types to lowercase', () => { + const ev = 'Abc'; + expect(validateEventAndLowerCaseConversion(ev, true, true)).toBe('abc'); }); - it('Each batchRequest contains unique jobIds (no event multiplexing)', async () => { - const input = [ - { - batchedRequest: { - endpoint: 'https://endpoint1', - }, - metadata: [ - { - jobId: 1, - }, - { - jobId: 4, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint3', - }, - metadata: [ - { - jobId: 2, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint3', - }, - metadata: [ - { - jobId: 5, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - ]; + it('should throw error if event is object type', () => { + expect(() => { + validateEventAndLowerCaseConversion({}, true, true); + }).toThrow(InstrumentationError); + expect(() => { + validateEventAndLowerCaseConversion([1, 2], false, true); + }).toThrow(InstrumentationError); + expect(() => { + validateEventAndLowerCaseConversion({ a: 1 }, true, true); + }).toThrow(InstrumentationError); + }); - const expectedOutput = [ - { - batchedRequest: { - endpoint: 'https://endpoint1', - }, + it('should convert string to lowercase', () => { + expect(validateEventAndLowerCaseConversion('Abc', true, true)).toBe('abc'); + expect(validateEventAndLowerCaseConversion('ABC', true, false)).toBe('ABC'); + expect(validateEventAndLowerCaseConversion('abc55', false, true)).toBe('abc55'); + expect(validateEventAndLowerCaseConversion(123, false, true)).toBe('123'); + }); - metadata: [ - { - jobId: 1, - }, - { - jobId: 4, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint3', - }, - metadata: [ - { - jobId: 2, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - { - batchedRequest: { - endpoint: 'https://endpoint3', - }, - metadata: [ - { - jobId: 5, - }, - ], - batched: true, - statusCode: 200, - destination: { - Config: { - key: 'value', - }, - }, - }, - ]; - expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); + it('should throw error for null and undefined', () => { + expect(() => { + validateEventAndLowerCaseConversion(null, true, true); + }).toThrow(InstrumentationError); + expect(() => { + validateEventAndLowerCaseConversion(undefined, false, true); + }).toThrow(InstrumentationError); + expect(() => { + validateEventAndLowerCaseConversion(NaN, false, true); + }).toThrow(InstrumentationError); }); }); From a7f2098d9f8b794b7651142fd9b3a1b479be8f89 Mon Sep 17 00:00:00 2001 From: AASHISH MALIK Date: Wed, 28 Feb 2024 20:16:42 +0530 Subject: [PATCH 3/5] fix: added test cases --- src/v0/util/index.js | 2 +- src/v0/util/index.test.js | 793 +++++++++++++++++++------------------- 2 files changed, 396 insertions(+), 399 deletions(-) diff --git a/src/v0/util/index.js b/src/v0/util/index.js index 6d347cbbf7..001be84426 100644 --- a/src/v0/util/index.js +++ b/src/v0/util/index.js @@ -2208,7 +2208,7 @@ const combineBatchRequestsWithSameJobIds = (inputBatches) => { * @returns {string} Event name converted to string. */ const validateEventAndLowerCaseConversion = (event, isMandatory, converToLowercase) => { - if (typeof event === 'object' || event === NaN || !isDefined(event)) { + if (typeof event === 'object' || !isDefined(event)) { throw new InstrumentationError('Event should not be a object, function or NaN and undefined'); } if (!isMandatory) { diff --git a/src/v0/util/index.test.js b/src/v0/util/index.test.js index 7af94dfeb5..6aeded7480 100644 --- a/src/v0/util/index.test.js +++ b/src/v0/util/index.test.js @@ -32,24 +32,24 @@ const functionNamesExpectingMultipleArguments = [ ]; describe('Utility Functions Tests', () => { - // describe.each(functionNames)('%s Tests', (funcName) => { - // const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); - // test.each(funcTestData)('$description', async ({ description, input, output }) => { - // try { - // let result; - // // This is to allow sending multiple arguments to the function - // if (Array.isArray(input)) { - // result = utilities[funcName](...input); - // } else { - // result = utilities[funcName](input); - // } - // expect(result).toEqual(output); - // } catch (e) { - // // Explicitly fail the test case - // expect(true).toEqual(false); - // } - // }); - // }); + describe.each(functionNames)('%s Tests', (funcName) => { + const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); + test.each(funcTestData)('$description', async ({ description, input, output }) => { + try { + let result; + // This is to allow sending multiple arguments to the function + if (Array.isArray(input)) { + result = utilities[funcName](...input); + } else { + result = utilities[funcName](input); + } + expect(result).toEqual(output); + } catch (e) { + // Explicitly fail the test case + expect(true).toEqual(false); + } + }); + }); /* This is to allow sending multiple arguments to the function in case when input is not an array but object * Like in case of checkAndCorrectUserId * "input": { @@ -60,402 +60,402 @@ describe('Utility Functions Tests', () => { * in the same order that they are defined in the function. * This order should be maintained in the input Object. */ - // describe.each(functionNamesExpectingMultipleArguments)('%s Tests', (funcName) => { - // const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); - // test.each(funcTestData)('$description', async ({ description, input, output }) => { - // try { - // let result; - // result = utilities[funcName](...Object.values(input)); - // expect(result).toEqual(output); - // } catch (e) { - // // Explicitly fail the test case - // expect(true).toEqual(false); - // } - // }); - // }); + describe.each(functionNamesExpectingMultipleArguments)('%s Tests', (funcName) => { + const funcTestData = getFuncTestData(__dirname, `./testdata/${funcName}.json`); + test.each(funcTestData)('$description', async ({ description, input, output }) => { + try { + let result; + result = utilities[funcName](...Object.values(input)); + expect(result).toEqual(output); + } catch (e) { + // Explicitly fail the test case + expect(true).toEqual(false); + } + }); + }); }); //Test cases which can't be fit in above test suite, have individual function level test blocks -// describe('hasCircularReference', () => { -// it('should return false when object has no circular reference', () => { -// const obj = { a: 1, b: 2, c: 3 }; -// expect(hasCircularReference(obj)).toBe(false); -// }); +describe('hasCircularReference', () => { + it('should return false when object has no circular reference', () => { + const obj = { a: 1, b: 2, c: 3 }; + expect(hasCircularReference(obj)).toBe(false); + }); -// it('should return true when object has circular reference', () => { -// const obj = { a: 1, b: 2 }; -// obj.c = obj; -// expect(hasCircularReference(obj)).toBe(true); -// }); + it('should return true when object has circular reference', () => { + const obj = { a: 1, b: 2 }; + obj.c = obj; + expect(hasCircularReference(obj)).toBe(true); + }); -// it('should return true when object has nested objects containing circular reference', () => { -// const obj1 = { a: 1 }; -// const obj2 = { b: 2 }; -// obj1.c = obj2; -// obj2.d = obj1; -// expect(hasCircularReference(obj1)).toBe(true); -// }); + it('should return true when object has nested objects containing circular reference', () => { + const obj1 = { a: 1 }; + const obj2 = { b: 2 }; + obj1.c = obj2; + obj2.d = obj1; + expect(hasCircularReference(obj1)).toBe(true); + }); -// it('should return false when input is null', () => { -// expect(hasCircularReference(null)).toBe(false); -// }); + it('should return false when input is null', () => { + expect(hasCircularReference(null)).toBe(false); + }); -// it('should return false when input is not an object', () => { -// expect(hasCircularReference(123)).toBe(false); -// }); + it('should return false when input is not an object', () => { + expect(hasCircularReference(123)).toBe(false); + }); -// it('should return true when object has self-reference', () => { -// const obj = { a: 1 }; -// obj.b = obj; -// expect(hasCircularReference(obj)).toBe(true); -// }); -// }); + it('should return true when object has self-reference', () => { + const obj = { a: 1 }; + obj.b = obj; + expect(hasCircularReference(obj)).toBe(true); + }); +}); -// // extra test cases for flattenJson -// describe('flattenJson', () => { -// it('should throw an error when flattening a json object with circular reference', () => { -// const data = { name: 'John' }; -// data.self = data; -// expect(() => flattenJson(data)).toThrow( -// "Event has circular reference. Can't flatten the event", -// ); -// }); -// }); +// extra test cases for flattenJson +describe('flattenJson', () => { + it('should throw an error when flattening a json object with circular reference', () => { + const data = { name: 'John' }; + data.self = data; + expect(() => flattenJson(data)).toThrow( + "Event has circular reference. Can't flatten the event", + ); + }); +}); -// describe('tests for generateErrorObject', () => { -// test('test-0', () => { -// const myErr = new FilteredEventsError('error-1'); -// const outputErrObj = utilities.generateErrorObject(myErr); -// expect(outputErrObj.statTags).toEqual({}); -// }); -// }); +describe('tests for generateErrorObject', () => { + test('test-0', () => { + const myErr = new FilteredEventsError('error-1'); + const outputErrObj = utilities.generateErrorObject(myErr); + expect(outputErrObj.statTags).toEqual({}); + }); +}); -// describe('generateExclusionList', () => { -// it('should return an array of excluded keys when given a mapping config', () => { -// const mappingConfig = [ -// { -// destKey: 'item_code', -// sourceKeys: ['product_id', 'sku'], -// }, -// { -// destKey: 'name', -// sourceKeys: 'name', -// }, -// ]; -// const expected = ['product_id', 'sku', 'name']; -// const result = generateExclusionList(mappingConfig); -// expect(result).toEqual(expected); -// }); +describe('generateExclusionList', () => { + it('should return an array of excluded keys when given a mapping config', () => { + const mappingConfig = [ + { + destKey: 'item_code', + sourceKeys: ['product_id', 'sku'], + }, + { + destKey: 'name', + sourceKeys: 'name', + }, + ]; + const expected = ['product_id', 'sku', 'name']; + const result = generateExclusionList(mappingConfig); + expect(result).toEqual(expected); + }); -// it('should return an empty array when the mapping config is empty', () => { -// const mappingConfig = []; -// const expected = []; -// const result = generateExclusionList(mappingConfig); -// expect(result).toEqual(expected); -// }); + it('should return an empty array when the mapping config is empty', () => { + const mappingConfig = []; + const expected = []; + const result = generateExclusionList(mappingConfig); + expect(result).toEqual(expected); + }); -// it('should return an array with unique keys when the mapping config has duplicate destination keys', () => { -// const mappingConfig = [ -// { -// destKey: 'item_code', -// sourceKeys: ['product_id'], -// }, -// { -// destKey: 'item_code', -// sourceKeys: ['sku'], -// }, -// ]; -// const expected = ['product_id', 'sku']; -// const result = generateExclusionList(mappingConfig); -// expect(result).toEqual(expected); -// }); -// }); + it('should return an array with unique keys when the mapping config has duplicate destination keys', () => { + const mappingConfig = [ + { + destKey: 'item_code', + sourceKeys: ['product_id'], + }, + { + destKey: 'item_code', + sourceKeys: ['sku'], + }, + ]; + const expected = ['product_id', 'sku']; + const result = generateExclusionList(mappingConfig); + expect(result).toEqual(expected); + }); +}); -// describe('Unit test cases for combineBatchRequestsWithSameJobIds', () => { -// it('Combine batch request with same jobIds', async () => { -// const input = [ -// { -// batchedRequest: { -// endpoint: 'https://endpoint1', -// }, -// metadata: [ -// { -// jobId: 1, -// }, -// { -// jobId: 4, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint2', -// }, -// metadata: [ -// { -// jobId: 3, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint1', -// }, -// metadata: [ -// { -// jobId: 5, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint3', -// }, -// metadata: [ -// { -// jobId: 1, -// }, -// { -// jobId: 3, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint2', -// }, -// metadata: [ -// { -// jobId: 6, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// ]; +describe('Unit test cases for combineBatchRequestsWithSameJobIds', () => { + it('Combine batch request with same jobIds', async () => { + const input = [ + { + batchedRequest: { + endpoint: 'https://endpoint1', + }, + metadata: [ + { + jobId: 1, + }, + { + jobId: 4, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint2', + }, + metadata: [ + { + jobId: 3, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint1', + }, + metadata: [ + { + jobId: 5, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint3', + }, + metadata: [ + { + jobId: 1, + }, + { + jobId: 3, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint2', + }, + metadata: [ + { + jobId: 6, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + ]; -// const expectedOutput = [ -// { -// batchedRequest: [ -// { -// endpoint: 'https://endpoint1', -// }, -// { -// endpoint: 'https://endpoint3', -// }, -// { -// endpoint: 'https://endpoint2', -// }, -// ], -// metadata: [ -// { -// jobId: 1, -// }, -// { -// jobId: 4, -// }, -// { -// jobId: 3, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint1', -// }, -// metadata: [ -// { -// jobId: 5, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint2', -// }, -// metadata: [ -// { -// jobId: 6, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// ]; -// expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); -// }); + const expectedOutput = [ + { + batchedRequest: [ + { + endpoint: 'https://endpoint1', + }, + { + endpoint: 'https://endpoint3', + }, + { + endpoint: 'https://endpoint2', + }, + ], + metadata: [ + { + jobId: 1, + }, + { + jobId: 4, + }, + { + jobId: 3, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint1', + }, + metadata: [ + { + jobId: 5, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint2', + }, + metadata: [ + { + jobId: 6, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + ]; + expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); + }); -// it('Each batchRequest contains unique jobIds (no event multiplexing)', async () => { -// const input = [ -// { -// batchedRequest: { -// endpoint: 'https://endpoint1', -// }, -// metadata: [ -// { -// jobId: 1, -// }, -// { -// jobId: 4, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint3', -// }, -// metadata: [ -// { -// jobId: 2, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint3', -// }, -// metadata: [ -// { -// jobId: 5, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// ]; + it('Each batchRequest contains unique jobIds (no event multiplexing)', async () => { + const input = [ + { + batchedRequest: { + endpoint: 'https://endpoint1', + }, + metadata: [ + { + jobId: 1, + }, + { + jobId: 4, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint3', + }, + metadata: [ + { + jobId: 2, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint3', + }, + metadata: [ + { + jobId: 5, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + ]; -// const expectedOutput = [ -// { -// batchedRequest: { -// endpoint: 'https://endpoint1', -// }, + const expectedOutput = [ + { + batchedRequest: { + endpoint: 'https://endpoint1', + }, -// metadata: [ -// { -// jobId: 1, -// }, -// { -// jobId: 4, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint3', -// }, -// metadata: [ -// { -// jobId: 2, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// { -// batchedRequest: { -// endpoint: 'https://endpoint3', -// }, -// metadata: [ -// { -// jobId: 5, -// }, -// ], -// batched: true, -// statusCode: 200, -// destination: { -// Config: { -// key: 'value', -// }, -// }, -// }, -// ]; -// expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); -// }); -// }); + metadata: [ + { + jobId: 1, + }, + { + jobId: 4, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint3', + }, + metadata: [ + { + jobId: 2, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + { + batchedRequest: { + endpoint: 'https://endpoint3', + }, + metadata: [ + { + jobId: 5, + }, + ], + batched: true, + statusCode: 200, + destination: { + Config: { + key: 'value', + }, + }, + }, + ]; + expect(combineBatchRequestsWithSameJobIds(input)).toEqual(expectedOutput); + }); +}); describe('validateEventAndLowerCaseConversion Tests', () => { it('should return string conversion of number types', () => { @@ -495,8 +495,5 @@ describe('validateEventAndLowerCaseConversion Tests', () => { expect(() => { validateEventAndLowerCaseConversion(undefined, false, true); }).toThrow(InstrumentationError); - expect(() => { - validateEventAndLowerCaseConversion(NaN, false, true); - }).toThrow(InstrumentationError); }); }); From a0493a07109a4d879bf9a796d182180fad5dfa92 Mon Sep 17 00:00:00 2001 From: AASHISH MALIK Date: Wed, 28 Feb 2024 23:14:50 +0530 Subject: [PATCH 4/5] chore: addressed comments --- src/v0/util/index.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/v0/util/index.js b/src/v0/util/index.js index 001be84426..15c143e3da 100644 --- a/src/v0/util/index.js +++ b/src/v0/util/index.js @@ -2204,21 +2204,20 @@ const combineBatchRequestsWithSameJobIds = (inputBatches) => { /** * This function validates the event and return it as string. * @param {*} isMandatory The event is a required field. - * @param {*} converToLowercase The event should be converted to lower-case. + * @param {*} convertToLowerCase The event should be converted to lower-case. * @returns {string} Event name converted to string. */ -const validateEventAndLowerCaseConversion = (event, isMandatory, converToLowercase) => { +const validateEventAndLowerCaseConversion = (event, isMandatory, convertToLowerCase) => { if (typeof event === 'object' || !isDefined(event)) { - throw new InstrumentationError('Event should not be a object, function or NaN and undefined'); - } - if (!isMandatory) { - return converToLowercase ? event.toString().toLowerCase() : event.toString(); + throw new InstrumentationError('Event should not be a object, function, NaN or undefined'); } + // handling 0 as it is a valid value - if (!event && event !== 0) { + if (isMandatory && !event && event !== 0) { throw new InstrumentationError('Event is a required field'); } - return converToLowercase ? event.toString().toLowerCase() : event.toString(); + + return convertToLowerCase ? event.toString().toLowerCase() : event.toString(); }; // ======================================================================== From 0582f0a7a16bdbd2a12b5bddb26dd528a2719eb9 Mon Sep 17 00:00:00 2001 From: AASHISH MALIK Date: Thu, 29 Feb 2024 11:04:27 +0530 Subject: [PATCH 5/5] chore: addressed comments --- src/v0/util/index.js | 4 ++-- src/v0/util/index.test.js | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/v0/util/index.js b/src/v0/util/index.js index 15c143e3da..c1debce088 100644 --- a/src/v0/util/index.js +++ b/src/v0/util/index.js @@ -2208,8 +2208,8 @@ const combineBatchRequestsWithSameJobIds = (inputBatches) => { * @returns {string} Event name converted to string. */ const validateEventAndLowerCaseConversion = (event, isMandatory, convertToLowerCase) => { - if (typeof event === 'object' || !isDefined(event)) { - throw new InstrumentationError('Event should not be a object, function, NaN or undefined'); + if (!isDefined(event) || typeof event === 'object' || typeof event === 'boolean') { + throw new InstrumentationError('Event should not be a object, NaN, boolean or undefined'); } // handling 0 as it is a valid value diff --git a/src/v0/util/index.test.js b/src/v0/util/index.test.js index 6aeded7480..810eb5a9d4 100644 --- a/src/v0/util/index.test.js +++ b/src/v0/util/index.test.js @@ -496,4 +496,13 @@ describe('validateEventAndLowerCaseConversion Tests', () => { validateEventAndLowerCaseConversion(undefined, false, true); }).toThrow(InstrumentationError); }); + + it('should throw error for boolean values', () => { + expect(() => { + validateEventAndLowerCaseConversion(true, true, true); + }).toThrow(InstrumentationError); + expect(() => { + validateEventAndLowerCaseConversion(false, false, false); + }).toThrow(InstrumentationError); + }); });