From bca3fd45f9207ba3e287556dec6ba00dc554ec91 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 17 Mar 2021 16:32:21 -0300 Subject: [PATCH 1/2] polishing and test updates --- src/logger/constants.ts | 46 ++++----- src/logger/messages/error.ts | 36 +++---- src/logger/messages/warn.ts | 18 ++-- .../__tests__/attributes.spec.ts | 4 +- .../inputValidation/__tests__/event.spec.ts | 60 ++++++------ .../__tests__/eventProperties.spec.ts | 35 +++---- .../__tests__/eventValue.spec.ts | 17 ++-- .../inputValidation/__tests__/key.spec.ts | 94 +++++++++---------- .../inputValidation/__tests__/split.spec.ts | 44 ++++----- .../__tests__/splitExistance.spec.ts | 7 +- .../inputValidation/__tests__/splits.spec.ts | 3 +- .../__tests__/trafficType.spec.ts | 42 ++++----- .../__tests__/trafficTypeExistance.spec.ts | 41 ++++---- src/utils/inputValidation/attributes.ts | 4 +- src/utils/inputValidation/event.ts | 10 +- src/utils/inputValidation/eventProperties.ts | 10 +- src/utils/inputValidation/eventValue.ts | 4 +- src/utils/inputValidation/key.ts | 14 +-- src/utils/inputValidation/split.ts | 10 +- src/utils/inputValidation/splitExistance.ts | 4 +- src/utils/inputValidation/splits.ts | 4 +- src/utils/inputValidation/trafficType.ts | 10 +- .../inputValidation/trafficTypeExistance.ts | 4 +- 23 files changed, 238 insertions(+), 283 deletions(-) diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 77ef7339..3e4904be 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -67,29 +67,19 @@ export const ERROR_9 = 309; export const ERROR_10 = 310; export const ERROR_11 = 311; export const ERROR_12 = 312; -export const ERROR_13 = 313; -export const ERROR_14 = 314; -export const ERROR_15 = 315; -export const ERROR_16 = 316; -export const ERROR_17 = 317; -export const ERROR_18 = 318; -export const ERROR_19 = 319; -export const ERROR_20 = 320; +export const ERROR_EVENT_TYPE_FORMAT = 314; +export const ERROR_NOT_PLAIN_OBJECT = 318; +export const ERROR_SIZE_EXCEEDED = 319; +export const ERROR_NOT_FINITE = 320; export const ERROR_21 = 321; -export const ERROR_22 = 322; -export const ERROR_23 = 323; -export const ERROR_24 = 324; -export const ERROR_25 = 325; -export const ERROR_26 = 326; -export const ERROR_32 = 332; -export const ERROR_33 = 333; -export const ERROR_34 = 334; -export const ERROR_35 = 335; -export const ERROR_36 = 336; -export const ERROR_37 = 337; -export const ERROR_38 = 338; +export const ERROR_NULL = 322; +export const ERROR_TOO_LONG = 323; +export const ERROR_INVALID_KEY_OBJECT = 326; +export const ERROR_INVALID = 332; +export const ERROR_EMPTY = 333; +export const ERROR_EMPTY_ARRAY = 334; +export const ERROR_INVALID_ENUM = 338; export const ERROR_39 = 339; -export const ERROR_API_KEY = 340; // node export const ERROR_1 = 301; @@ -132,14 +122,14 @@ export const WARN_8 = 208; export const WARN_9 = 209; export const WARN_10 = 210; export const WARN_11 = 211; -export const WARN_12 = 212; -export const WARN_13 = 213; +export const WARN_SETTING_NULL = 212; +export const WARN_TRIMMING_PROPERTIES = 213; export const WARN_14 = 214; -export const WARN_15 = 215; -export const WARN_17 = 217; -export const WARN_18 = 218; -export const WARN_19 = 219; -export const WARN_20 = 220; +export const WARN_CONVERTING = 215; +export const WARN_TRIMMING = 217; +export const WARN_NOT_EXISTENT_SPLIT = 218; +export const WARN_LOWERCASE_TRAFFIC_TYPE = 219; +export const WARN_NOT_EXISTENT_TT = 220; export const WARN_21 = 221; export const WARN_22 = 222; export const WARN_23 = 223; diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index 9c2cbf1f..eb99b714 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -1,4 +1,4 @@ -import { ERROR_0, ERROR_2, ERROR_3, ERROR_4, ERROR_5, ERROR_7, ERROR_9, ERROR_10, ERROR_11, ERROR_12, ERROR_13, ERROR_14, ERROR_15, ERROR_16, ERROR_17, ERROR_18, ERROR_19, ERROR_20, ERROR_21, ERROR_22, ERROR_23, ERROR_24, ERROR_25, ERROR_26, ERROR_32, ERROR_33, ERROR_34, ERROR_35, ERROR_36, ERROR_37, ERROR_38, ERROR_39, ERROR_API_KEY } from '../constants'; +import { ERROR_0, ERROR_2, ERROR_3, ERROR_4, ERROR_5, ERROR_7, ERROR_9, ERROR_10, ERROR_11, ERROR_12, ERROR_EVENT_TYPE_FORMAT, ERROR_NOT_PLAIN_OBJECT, ERROR_SIZE_EXCEEDED, ERROR_NOT_FINITE, ERROR_21, ERROR_NULL, ERROR_TOO_LONG, ERROR_INVALID_KEY_OBJECT, ERROR_INVALID, ERROR_EMPTY, ERROR_EMPTY_ARRAY, ERROR_INVALID_ENUM, ERROR_39 } from '../constants'; export const codesError: [number, string][] = [ [ERROR_0, 'splitio-engine:combiner => Invalid Split provided, no valid conditions found'], @@ -11,27 +11,19 @@ export const codesError: [number, string][] = [ [ERROR_10, 'splitio-sync:push-manager => Failed to authenticate for streaming. Error: "%s".'], [ERROR_11, 'splitio-client:impressions-tracker => Could not store impressions bulk with %s impression%s. Error: %s'], [ERROR_12, 'splitio-client:impressions-tracker => Impression listener logImpression method threw: %s.'], - [ERROR_13, '%s: attributes must be a plain object.'], - [ERROR_14, '%s: you passed "%s", event_type must adhere to the regular expression /^[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}$/g. This means an event_type must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, underscore, period, or colon as separators of alphanumeric characters.'], - [ERROR_15, '%s: you passed a null or undefined event_type, event_type must be a non-empty string.'], - [ERROR_16, '%s: you passed an invalid event_type, event_type must be a non-empty string.'], - [ERROR_17, '%s: you passed an empty event_type, event_type must be a non-empty string.'], - [ERROR_18, '%s: properties must be a plain object.'], - [ERROR_19, '%s: The maximum size allowed for the properties is 32768 bytes, which was exceeded. Event not queued.'], - [ERROR_20, '%s: value must be a finite number.'], [ERROR_21, 'Client has already been destroyed - no calls possible.'], - [ERROR_22, '%s: you passed a null or undefined %s, %s must be a non-empty string.'], - [ERROR_23, '%s: %s too long, %s must be 250 characters or less.'], - [ERROR_24, '%s: you passed an invalid %s type, %s must be a non-empty string.'], - [ERROR_25, '%s: you passed an empty string, %s must be a non-empty string.'], - [ERROR_26, '%s: Key must be an object with bucketingKey and matchingKey with valid string properties.'], - [ERROR_32, '%s: you passed an invalid %s, %s must be a non-empty string.'], - [ERROR_33, '%s: you passed an empty %s, %s must be a non-empty string.'], - [ERROR_34, '%s: %s must be a non-empty array.'], - [ERROR_35, '%s: you passed a null or undefined traffic_type_name, traffic_type_name must be a non-empty string.'], - [ERROR_36, '%s: you passed an invalid traffic_type_name, traffic_type_name must be a non-empty string.'], - [ERROR_37, '%s: you passed an empty traffic_type_name, traffic_type_name must be a non-empty string.'], - [ERROR_38, 'splitio-settings => You passed an invalid impressionsMode, impressionsMode should be one of the following values: "%s" or "%s". Defaulting to "%s" mode.'], [ERROR_39, 'Response status is not OK. Status: %s. URL: %s. Message: %s'], - [ERROR_API_KEY, 'Factory instantiation: %s, api_key must be a non-empty string'] + // input validation + [ERROR_EVENT_TYPE_FORMAT, '%s: you passed "%s", event_type must adhere to the regular expression /^[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}$/g. This means an event_type must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, underscore, period, or colon as separators of alphanumeric characters.'], + [ERROR_NOT_PLAIN_OBJECT, '%s: %s must be a plain object.'], + [ERROR_SIZE_EXCEEDED, '%s: the maximum size allowed for the properties is 32768 bytes, which was exceeded. Event not queued.'], + [ERROR_NOT_FINITE, '%s: value must be a finite number.'], + [ERROR_NULL, '%s: you passed a null or undefined %s. It must be a non-empty string.'], + [ERROR_TOO_LONG, '%s: %s too long. It must have 250 characters or less.'], + [ERROR_INVALID_KEY_OBJECT, '%s: Key must be an object with bucketingKey and matchingKey with valid string properties.'], + [ERROR_INVALID, '%s: you passed an invalid %s. It must be a non-empty string.'], + [ERROR_EMPTY, '%s: you passed an empty %s. It must be a non-empty string.'], + [ERROR_EMPTY_ARRAY, '%s: %s must be a non-empty array.'], + // initialization / settings validation + [ERROR_INVALID_ENUM, '%s: you passed an invalid %s. It should be one of the following values: %s. Defaulting to "%s" mode.'], ]; diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index 8e4bca72..9a9e1328 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -1,4 +1,4 @@ -import { WARN_0, WARN_1, WARN_2, WARN_4, WARN_5, WARN_6, WARN_7, WARN_8, WARN_9, WARN_10, WARN_11, WARN_12, WARN_13, WARN_14, WARN_15, WARN_17, WARN_18, WARN_19, WARN_20, WARN_21, WARN_22, WARN_23, WARN_24, WARN_25, WARN_API_KEY } from '../constants'; +import { WARN_0, WARN_1, WARN_2, WARN_4, WARN_5, WARN_6, WARN_7, WARN_8, WARN_9, WARN_10, WARN_11, WARN_SETTING_NULL, WARN_TRIMMING_PROPERTIES, WARN_14, WARN_CONVERTING, WARN_TRIMMING, WARN_NOT_EXISTENT_SPLIT, WARN_LOWERCASE_TRAFFIC_TYPE, WARN_NOT_EXISTENT_TT, WARN_21, WARN_22, WARN_23, WARN_24, WARN_25, WARN_API_KEY } from '../constants'; export const codesWarn: [number, string][] = [ [WARN_0, 'splitio-engine:value => Value %s %sdoesn\'t match with expected type.'], @@ -12,14 +12,16 @@ export const codesWarn: [number, string][] = [ [WARN_9, 'splitio-sync:submitters => Droping %s %s after retry. Reason %s.'], [WARN_10, 'splitio-sync:submitters => Failed to push %s %s, keeping data to retry on next iteration. Reason %s.'], [WARN_11, 'splitio-client:event-tracker => Failed to queue %s'], - [WARN_12, '%s: Property %s is of invalid type. Setting value to null.'], - [WARN_13, '%s: Event has more than 300 properties. Some of them will be trimmed when processed.'], + // input validation + [WARN_SETTING_NULL, '%s: Property "%s" is of invalid type. Setting value to null.'], + [WARN_TRIMMING_PROPERTIES, '%s: Event has more than 300 properties. Some of them will be trimmed when processed.'], [WARN_14, '%s: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method.'], - [WARN_15, '%s: %s "%s" is not of type string, converting.'], - [WARN_17, '%s: %s "%s" has extra whitespace, trimming.'], - [WARN_18, '%s: you passed "%s" that does not exist in this environment, please double check what Splits exist in the web console.'], - [WARN_19, '%s: traffic_type_name should be all lowercase - converting string to lowercase.'], - [WARN_20, '%s: Traffic Type %s does not have any corresponding Splits in this environment, make sure you\'re tracking your events to a valid traffic type defined in the Split console.'], + [WARN_CONVERTING, '%s: %s "%s" is not of type string, converting.'], + [WARN_TRIMMING, '%s: %s "%s" has extra whitespace, trimming.'], + [WARN_NOT_EXISTENT_SPLIT, '%s: split "%s" does not exist in this environment, please double check what splits exist in the web console.'], + [WARN_LOWERCASE_TRAFFIC_TYPE, '%s: traffic_type_name should be all lowercase - converting string to lowercase.'], + [WARN_NOT_EXISTENT_TT, '%s: traffic type "%s" does not have any corresponding split in this environment, make sure you\'re tracking your events to a valid traffic type defined in the web console.'], + // initialization / settings validation [WARN_21, 'splitio-settings => %s integration %s at settings %s invalid. %s'], [WARN_22, 'Factory instantiation: split filters have been configured but will have no effect if mode is not "%s", since synchronization is being deferred to an external tool.'], [WARN_23, 'Factory instantiation: split filter at position %s is invalid. It must be an object with a valid filter type ("byName" or "byPrefix") and a list of "values".'], diff --git a/src/utils/inputValidation/__tests__/attributes.spec.ts b/src/utils/inputValidation/__tests__/attributes.spec.ts index e7ae5e0d..ae7b2c9b 100644 --- a/src/utils/inputValidation/__tests__/attributes.spec.ts +++ b/src/utils/inputValidation/__tests__/attributes.spec.ts @@ -1,4 +1,4 @@ -import { ERROR_13 } from '../../../logger/constants'; +import { ERROR_NOT_PLAIN_OBJECT } from '../../../logger/constants'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { validateAttributes } from '../attributes'; @@ -41,7 +41,7 @@ describe('INPUT VALIDATION for Attributes', () => { const invalidAttribute = invalidAttributes[i]; expect(validateAttributes(loggerMock, invalidAttribute, 'test_method')).toBe(false); // Invalid attribute objects should return false. - expect(loggerMock.error).lastCalledWith(ERROR_13, ['test_method']); // The error should be logged for the invalid attributes map. + expect(loggerMock.error).lastCalledWith(ERROR_NOT_PLAIN_OBJECT, ['test_method', 'attributes']); // The error should be logged for the invalid attributes map. loggerMock.error.mockClear(); } diff --git a/src/utils/inputValidation/__tests__/event.spec.ts b/src/utils/inputValidation/__tests__/event.spec.ts index 9abfdff1..2e54136b 100644 --- a/src/utils/inputValidation/__tests__/event.spec.ts +++ b/src/utils/inputValidation/__tests__/event.spec.ts @@ -1,34 +1,28 @@ +import { ERROR_INVALID, ERROR_EMPTY, ERROR_NULL, ERROR_EVENT_TYPE_FORMAT } from '../../../logger/constants'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { validateEvent } from '../event'; -const errorMsgs = { - NULL_EVENT: () => 'you passed a null or undefined event_type, event_type must be a non-empty string.', - WRONG_TYPE_EVENT: () => 'you passed an invalid event_type, event_type must be a non-empty string.', - EMPTY_EVENT: () => 'you passed an empty event_type, event_type must be a non-empty string.', - WRONG_FORMAT_EVENT: (invalidEvent: any) => `you passed "${invalidEvent}", event_type must adhere to the regular expression /^[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}$/g. This means an event_type must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, underscore, period, or colon as separators of alphanumeric characters.` -}; - const invalidEvents = [ - { event: [], msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: () => { }, msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: false, msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: true, msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: {}, msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: Object.create({}), msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: 'something+withInvalidchars', msg: errorMsgs.WRONG_FORMAT_EVENT }, - { event: 'with spaces', msg: errorMsgs.WRONG_FORMAT_EVENT }, - { event: ' asd', msg: errorMsgs.WRONG_FORMAT_EVENT }, - { event: 'asd ', msg: errorMsgs.WRONG_FORMAT_EVENT }, - { event: '?', msg: errorMsgs.WRONG_FORMAT_EVENT }, - { event: '', msg: errorMsgs.EMPTY_EVENT }, - { event: NaN, msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: -Infinity, msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: Infinity, msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: new Promise(res => res), msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: Symbol('asd'), msg: errorMsgs.WRONG_TYPE_EVENT }, - { event: null, msg: errorMsgs.NULL_EVENT }, - { event: undefined, msg: errorMsgs.NULL_EVENT } + { event: [], msg: ERROR_INVALID }, + { event: () => { }, msg: ERROR_INVALID }, + { event: false, msg: ERROR_INVALID }, + { event: true, msg: ERROR_INVALID }, + { event: {}, msg: ERROR_INVALID }, + { event: Object.create({}), msg: ERROR_INVALID }, + { event: 'something+withInvalidchars', msg: ERROR_EVENT_TYPE_FORMAT }, + { event: 'with spaces', msg: ERROR_EVENT_TYPE_FORMAT }, + { event: ' asd', msg: ERROR_EVENT_TYPE_FORMAT }, + { event: 'asd ', msg: ERROR_EVENT_TYPE_FORMAT }, + { event: '?', msg: ERROR_EVENT_TYPE_FORMAT }, + { event: '', msg: ERROR_EMPTY }, + { event: NaN, msg: ERROR_INVALID }, + { event: -Infinity, msg: ERROR_INVALID }, + { event: Infinity, msg: ERROR_INVALID }, + { event: new Promise(res => res), msg: ERROR_INVALID }, + { event: Symbol('asd'), msg: ERROR_INVALID }, + { event: null, msg: ERROR_NULL }, + { event: undefined, msg: ERROR_NULL } ]; describe('INPUT VALIDATION for Event types', () => { @@ -38,26 +32,26 @@ describe('INPUT VALIDATION for Event types', () => { test('Should return the provided event type if it is a valid string without logging any errors', () => { expect(validateEvent(loggerMock, 'valid:Too', 'some_method_eventType')).toBe('valid:Too'); // It should return the provided string if it is valid. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. expect(validateEvent(loggerMock, 'I.am.valid-string_ValUe', 'some_method_eventType')).toBe('I.am.valid-string_ValUe'); // It should return the provided string if it is valid. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. expect(validateEvent(loggerMock, 'a', 'some_method_eventType')).toBe('a'); // It should return the provided string if it is valid. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('Should return false and log error if event type is not a valid string', () => { for (let i = 0; i < invalidEvents.length; i++) { const invalidValue = invalidEvents[i]['event']; - const expectedLog = invalidEvents[i]['msg'](invalidValue); + const expectedLog = invalidEvents[i]['msg']; expect(validateEvent(loggerMock, invalidValue, 'test_method')).toBe(false); // Invalid event types should always return false. - expect(loggerMock.error.mock.calls[0][0]).toEqual(`test_method: ${expectedLog}`); // Should log the error for the invalid event type. + expect(loggerMock.error).toBeCalledWith(expectedLog, expectedLog === ERROR_EVENT_TYPE_FORMAT ? ['test_method', invalidValue] : ['test_method']); // Should log the error for the invalid event type. loggerMock.error.mockClear(); } - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); }); diff --git a/src/utils/inputValidation/__tests__/eventProperties.spec.ts b/src/utils/inputValidation/__tests__/eventProperties.spec.ts index 3f2caa9b..d17f08bd 100644 --- a/src/utils/inputValidation/__tests__/eventProperties.spec.ts +++ b/src/utils/inputValidation/__tests__/eventProperties.spec.ts @@ -1,3 +1,4 @@ +import { 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'; @@ -48,8 +49,8 @@ describe('INPUT VALIDATION for Event Properties', () => { }); // It should return the passed null since it is valid with default event size.'); - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + 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 properties, only objects are acceptable', () => { @@ -59,12 +60,12 @@ describe('INPUT VALIDATION for Event Properties', () => { properties: false, size: 1024 }); // It should return default size and properties false if the properties value is not an object or null/undefined.'); - expect(loggerMock.error.mock.calls).toEqual([['some_method_eventProps: properties must be a plain object.']]); // Should log an error. + expect(loggerMock.error).toBeCalledWith(ERROR_NOT_PLAIN_OBJECT, ['some_method_eventProps', 'properties']); // Should log an error. loggerMock.error.mockClear(); }); - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('It should return the properties object when valid and also the correct event size', () => { @@ -87,8 +88,8 @@ describe('INPUT VALIDATION for Event Properties', () => { expect(validProperties).not.toBe(output.properties); // Returned properties should be a clone. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('It should return the properties object when valid and also the correct event size, nulling any invalid prop', () => { @@ -117,11 +118,11 @@ describe('INPUT VALIDATION for Event Properties', () => { expect(providedProperties).not.toBe(output.properties); // Returned properties should be a clone. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(4); // It should have logged one warning per each property of the invalid type. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. + expect(loggerMock.warn).toBeCalledTimes(4); // It should have logged one warning per each property of the invalid type. ['willBeNulled1', 'willBeNulled2', 'willBeNulled3', 'willBeNulled4'].forEach((key, index) => { - expect(loggerMock.warn.mock.calls[index][0]).toBe(`some_method_eventProps: Property ${key} is of invalid type. Setting value to null.`); + expect(loggerMock.warn.mock.calls[index]).toEqual([WARN_SETTING_NULL, ['some_method_eventProps', key]]); }); }); @@ -139,8 +140,8 @@ describe('INPUT VALIDATION for Event Properties', () => { }); // It should return the properties and the event size.'); - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. // @ts-ignore validProperties.a = null; // Adding one prop to exceed the limit. @@ -152,8 +153,8 @@ describe('INPUT VALIDATION for Event Properties', () => { }); // It should return the properties and the event size.'); - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. - expect(loggerMock.warn.mock.calls).toEqual([['some_method_eventProps: Event has more than 300 properties. Some of them will be trimmed when processed.']]); // It should have logged a warning. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. + expect(loggerMock.warn).toBeCalledWith(WARN_TRIMMING_PROPERTIES, ['some_method_eventProps']); // It should have logged a warning. }); const fiveHundredChars = 'JKHSAKFJASHFJKASHSFKHAKJSGJKASGH1234567890JASHGJHASGJKAHSJKGHAJKSHGJKAHGJKASHajksghkjahsgjkhsakjghjkashgjkhagjkhajksghjkahsgjksahgjkahsgjkhasgjkhsagjkabsgjhaenjkrnjkwnqrkjnqwekjrnkjweqntkjnjkenasdjkngjksdajkghkjdasgkjnadsjgn asdkjgnkjsadngkjnasdjkngjknasdkjgnasdlgnsdakgnlkasndugbuoewqoitnwlkgadsgjdnsagubadisugboisdngklasdgndsgbjasdbgjkasbdgubuiqwetoiqhweiojtioweqhtiohqweiohtiowqehtoihewqiobtgoiqwengiowqnegionwqeogiqwneoignqiowegnioqewgnwqoiegnoiqwengiowqnegoinqwgionqwegionwqeoignqwegoinoiadnfaosignoiansgk'; @@ -174,8 +175,8 @@ describe('INPUT VALIDATION for Event Properties', () => { // It should return the properties and the event size.'); expect(validProperties).not.toBe(output.properties); // Returned properties should be a clone. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(0); // Should not log any warnings. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. + expect(loggerMock.warn).not.toBeCalled(); // Should not log any warnings. // @ts-ignore validProperties.a = null; // exceed by two bytes (1 char string key which is two bytes, null value which we count as 0 to match other SDKs) @@ -188,7 +189,7 @@ describe('INPUT VALIDATION for Event Properties', () => { }); // It should return false instead of the properties and the event size.'); - expect(loggerMock.warn.mock.calls.length).toBe(0); // Should not log any warnings. - expect(loggerMock.error.mock.calls).toEqual([['some_method_eventProps: The maximum size allowed for the properties is 32768 bytes, which was exceeded. Event not queued.']]); // Should log an error. + expect(loggerMock.warn).not.toBeCalled(); // Should not log any warnings. + expect(loggerMock.error).toBeCalledWith(ERROR_SIZE_EXCEEDED, ['some_method_eventProps']); // Should log an error. }); }); diff --git a/src/utils/inputValidation/__tests__/eventValue.spec.ts b/src/utils/inputValidation/__tests__/eventValue.spec.ts index b98ca3c0..dc7d0f33 100644 --- a/src/utils/inputValidation/__tests__/eventValue.spec.ts +++ b/src/utils/inputValidation/__tests__/eventValue.spec.ts @@ -1,3 +1,4 @@ +import { ERROR_NOT_FINITE } from '../../../logger/constants'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { validateEventValue } from '../eventValue'; @@ -23,20 +24,20 @@ describe('INPUT VALIDATION for Event Values', () => { test('Should return the passed value if it is a valid finite number without logging any errors', () => { expect(validateEventValue(loggerMock, 50, 'some_method_eventValue')).toBe(50); // It should return the passed number if it is valid. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. expect(validateEventValue(loggerMock, -50, 'some_method_eventValue')).toBe(-50); // It should return the passed number if it is valid. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('Should return the passed value if it is a null or undefined (since it is optional) without logging any errors', () => { expect(validateEventValue(loggerMock, null, 'some_method_eventValue')).toBe(null); // It should return the passed number if it is valid. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. expect(validateEventValue(loggerMock, undefined, 'some_method_eventValue')).toBe(undefined); // It should return the passed number if it is valid. - expect(loggerMock.error.mock.calls.length).toBe(0); // Should not log any errors. + expect(loggerMock.error).not.toBeCalled(); // Should not log any errors. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('Should return false and log error if event value is not a valid finite number', () => { @@ -44,11 +45,11 @@ describe('INPUT VALIDATION for Event Values', () => { const invalidValue = invalidValues[i]; expect(validateEventValue(loggerMock, invalidValue, 'test_method')).toBe(false); // Invalid event values should always return false. - expect(loggerMock.error.mock.calls[0][0]).toEqual('test_method: value must be a finite number.'); // Should log the error for the invalid event value. + expect(loggerMock.error).toBeCalledWith(ERROR_NOT_FINITE, ['test_method']); // Should log the error for the invalid event value. loggerMock.error.mockClear(); } - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); }); diff --git a/src/utils/inputValidation/__tests__/key.spec.ts b/src/utils/inputValidation/__tests__/key.spec.ts index 62600870..74bdb62e 100644 --- a/src/utils/inputValidation/__tests__/key.spec.ts +++ b/src/utils/inputValidation/__tests__/key.spec.ts @@ -1,35 +1,27 @@ +import { ERROR_EMPTY, ERROR_INVALID, ERROR_INVALID_KEY_OBJECT, ERROR_NULL, ERROR_TOO_LONG, WARN_CONVERTING } from '../../../logger/constants'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { validateKey } from '../key'; -const errorMsgs = { - EMPTY_KEY: (keyType: string) => `you passed an empty string, ${keyType} must be a non-empty string.`, - LONG_KEY: (keyType: string) => `${keyType} too long, ${keyType} must be 250 characters or less.`, - NULL_KEY: (keyType: string) => `you passed a null or undefined ${keyType}, ${keyType} must be a non-empty string.`, - WRONG_TYPE_KEY: (keyType: string) => `you passed an invalid ${keyType} type, ${keyType} must be a non-empty string.`, - NUMERIC_KEY: (keyType: string, value: any) => `${keyType} "${value}" is not of type string, converting.`, - WRONG_KEY_PROPS: 'Key must be an object with bucketingKey and matchingKey with valid string properties.' -}; - const invalidKeys = [ - { key: '', msg: errorMsgs.EMPTY_KEY }, - { key: 'a'.repeat(251), msg: errorMsgs.LONG_KEY }, - { key: null, msg: errorMsgs.NULL_KEY }, - { key: undefined, msg: errorMsgs.NULL_KEY }, - { key: () => { }, msg: errorMsgs.WRONG_TYPE_KEY }, - { key: new Promise(r => r()), msg: errorMsgs.WRONG_TYPE_KEY }, - { key: Symbol('asd'), msg: errorMsgs.WRONG_TYPE_KEY }, - { key: [], msg: errorMsgs.WRONG_TYPE_KEY }, - { key: true, msg: errorMsgs.WRONG_TYPE_KEY }, - { key: NaN, msg: errorMsgs.WRONG_TYPE_KEY }, - { key: Infinity, msg: errorMsgs.WRONG_TYPE_KEY }, - { key: -Infinity, msg: errorMsgs.WRONG_TYPE_KEY }, + { key: '', msg: ERROR_EMPTY }, + { key: 'a'.repeat(251), msg: ERROR_TOO_LONG }, + { key: null, msg: ERROR_NULL }, + { key: undefined, msg: ERROR_NULL }, + { key: () => { }, msg: ERROR_INVALID }, + { key: new Promise(r => r()), msg: ERROR_INVALID }, + { key: Symbol('asd'), msg: ERROR_INVALID }, + { key: [], msg: ERROR_INVALID }, + { key: true, msg: ERROR_INVALID }, + { key: NaN, msg: ERROR_INVALID }, + { key: Infinity, msg: ERROR_INVALID }, + { key: -Infinity, msg: ERROR_INVALID }, ]; const stringifyableKeys = [ - { key: 200, msg: errorMsgs.NUMERIC_KEY }, - { key: 1235891238571295, msg: errorMsgs.NUMERIC_KEY }, - { key: 0, msg: errorMsgs.NUMERIC_KEY } + { key: 200, msg: WARN_CONVERTING }, + { key: 1235891238571295, msg: WARN_CONVERTING }, + { key: 0, msg: WARN_CONVERTING } ]; const invalidKeyObjects = [ @@ -51,52 +43,52 @@ describe('INPUT VALIDATION for Key', () => { }; expect(validateKey(loggerMock, validKey, 'some_method_keys')).toEqual(validKey); // It will return the valid key. - expect(loggerMock.error.mock.calls.length).toBe(0); // No errors should be logged. + expect(loggerMock.error).not.toBeCalled(); // No errors should be logged. expect(validateKey(loggerMock, validObjKey, 'some_method_keys')).toEqual(validObjKey); // It will return the valid key. - expect(loggerMock.error.mock.calls.length).toBe(0); // No errors should be logged. + expect(loggerMock.error).not.toBeCalled(); // No errors should be logged. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('String key / Should return false and log error if key is invalid', () => { for (let i = 0; i < invalidKeys.length; i++) { const invalidKey = invalidKeys[i]['key']; - const expectedLog = invalidKeys[i]['msg']('key'); + const expectedLog = invalidKeys[i]['msg']; expect(validateKey(loggerMock, invalidKey, 'test_method')).toBe(false); // Invalid keys should return false. - expect(loggerMock.error.mock.calls[0][0]).toEqual(`test_method: ${expectedLog}`); // The error should be logged for the invalid key. + expect(loggerMock.error).toBeCalledWith(expectedLog, ['test_method', 'key']); // The error should be logged for the invalid key. loggerMock.error.mockClear(); } - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('String key / Should return stringified version of the key if it is convertible to one and log a warning.', () => { for (let i = 0; i < stringifyableKeys.length; i++) { const invalidKey = stringifyableKeys[i]['key']; - const expectedLog = stringifyableKeys[i]['msg']('key', invalidKey); + const expectedLog = stringifyableKeys[i]['msg']; validateKey(loggerMock, invalidKey, 'test_method'); - expect(loggerMock.warn.mock.calls[0][0]).toEqual(`test_method: ${expectedLog}`); // But if the logger allows for warnings, it should be logged. + expect(loggerMock.warn).toBeCalledWith(expectedLog, ['test_method', 'key', invalidKey]); // But if the logger allows for warnings, it should be logged. loggerMock.warn.mockClear(); } - expect(loggerMock.error.mock.calls.length).toBe(0); // It should have not logged any errors. + expect(loggerMock.error).not.toBeCalled(); // It should have not logged any errors. }); test('Object key / Should return false and log error if a part of the key is invalid', () => { // Test invalid object format for (let i = 0; i < invalidKeyObjects.length; i++) { expect(validateKey(loggerMock, invalidKeyObjects[i], 'test_method')).toBe(false); // Invalid key objects should return false. - expect(loggerMock.error.mock.calls[loggerMock.error.mock.calls.length - 1][0]).toEqual(`test_method: ${errorMsgs.WRONG_KEY_PROPS}`); // The error should be logged for the invalid key. + expect(loggerMock.error).lastCalledWith(ERROR_INVALID_KEY_OBJECT, ['test_method']); // The error should be logged for the invalid key. loggerMock.error.mockClear(); } - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. loggerMock.mockClear(); // Test invalid matchingKey @@ -105,15 +97,15 @@ describe('INPUT VALIDATION for Key', () => { matchingKey: invalidKeys[i]['key'], bucketingKey: 'thisIsValid' }; - const expectedLog = invalidKeys[i]['msg']('matchingKey'); + const expectedLog = invalidKeys[i]['msg']; expect(validateKey(loggerMock, invalidKey, 'test_method')).toBe(false); // Invalid keys should return false. - expect(loggerMock.error.mock.calls[0][0]).toEqual(`test_method: ${expectedLog}`); // The error should be logged for the invalid key. + expect(loggerMock.error).toBeCalledWith(expectedLog, ['test_method', 'matchingKey']); // The error should be logged for the invalid key. loggerMock.error.mockClear(); } - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. loggerMock.mockClear(); @@ -123,15 +115,15 @@ describe('INPUT VALIDATION for Key', () => { matchingKey: 'thisIsValidToo', bucketingKey: invalidKeys[i]['key'] }; - const expectedLog = invalidKeys[i]['msg']('bucketingKey'); + const expectedLog = invalidKeys[i]['msg']; expect(validateKey(loggerMock, invalidKey, 'test_method')).toBe(false); // Invalid keys should return false. - expect(loggerMock.error.mock.calls[0][0]).toEqual(`test_method: ${expectedLog}`); // The error should be logged for the invalid key. + expect(loggerMock.error).toBeCalledWith(expectedLog, ['test_method', 'bucketingKey']); // The error should be logged for the invalid key. loggerMock.error.mockClear(); } - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. loggerMock.mockClear(); @@ -140,14 +132,14 @@ describe('INPUT VALIDATION for Key', () => { matchingKey: invalidKeys[0]['key'], bucketingKey: invalidKeys[1]['key'] }; - let expectedLogMK = invalidKeys[0]['msg']('matchingKey'); - let expectedLogBK = invalidKeys[1]['msg']('bucketingKey'); + let expectedLogMK = invalidKeys[0]['msg']; + let expectedLogBK = invalidKeys[1]['msg']; expect(validateKey(loggerMock, invalidKey, 'test_method')).toBe(false); // Invalid keys should return false. - expect(loggerMock.error.mock.calls[0][0]).toEqual(`test_method: ${expectedLogMK}`); // The error should be logged for the invalid key property. - expect(loggerMock.error.mock.calls[1][0]).toEqual(`test_method: ${expectedLogBK}`); // The error should be logged for the invalid key property. + expect(loggerMock.error).nthCalledWith(1, expectedLogMK, ['test_method', 'matchingKey']); // The error should be logged for the invalid key property. + expect(loggerMock.error).nthCalledWith(2, expectedLogBK, ['test_method', 'bucketingKey']); // The error should be logged for the invalid key property. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. + expect(loggerMock.warn).not.toBeCalled(); // It should have not logged any warnings. }); test('Object key / Should return stringified version of the key props if those are convertible and log the corresponding warnings', () => { @@ -159,13 +151,13 @@ describe('INPUT VALIDATION for Key', () => { matchingKey: String(invalidKey.matchingKey), bucketingKey: String(invalidKey.bucketingKey), }; - let expectedLogMK = stringifyableKeys[0]['msg']('matchingKey', invalidKey.matchingKey); - let expectedLogBK = stringifyableKeys[1]['msg']('bucketingKey', invalidKey.bucketingKey); + let expectedLogMK = stringifyableKeys[0]['msg']; + let expectedLogBK = stringifyableKeys[1]['msg']; expect(validateKey(loggerMock, invalidKey, 'test_method')).toEqual(expectedKey); // If a key object had stringifyable values, those will be stringified and Key returned. - expect(loggerMock.warn.mock.calls[0][0]).toEqual(`test_method: ${expectedLogMK}`); // The warning should be logged for the stringified prop if warnings are enabled. - expect(loggerMock.warn.mock.calls[1][0]).toEqual(`test_method: ${expectedLogBK}`); // The warning should be logged for the stringified prop if warnings are enabled. + expect(loggerMock.warn).nthCalledWith(1, expectedLogMK, ['test_method', 'matchingKey', invalidKey.matchingKey]); // The warning should be logged for the stringified prop if warnings are enabled. + expect(loggerMock.warn).nthCalledWith(2, expectedLogBK, ['test_method', 'bucketingKey', invalidKey.bucketingKey]); // The warning should be logged for the stringified prop if warnings are enabled. - expect(loggerMock.error.mock.calls.length).toBe(0); // It should have not logged any errors. + expect(loggerMock.error).not.toBeCalled(); // It should have not logged any errors. }); }); diff --git a/src/utils/inputValidation/__tests__/split.spec.ts b/src/utils/inputValidation/__tests__/split.spec.ts index 1a6417b9..13ccfaa3 100644 --- a/src/utils/inputValidation/__tests__/split.spec.ts +++ b/src/utils/inputValidation/__tests__/split.spec.ts @@ -1,30 +1,24 @@ +import { ERROR_INVALID, ERROR_NULL, ERROR_EMPTY, WARN_TRIMMING } from '../../../logger/constants'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { validateSplit } from '../split'; -const errorMsgs = { - NULL_SPLIT: () => 'you passed a null or undefined split name, split name must be a non-empty string.', - WRONG_TYPE_SPLIT: () => 'you passed an invalid split name, split name must be a non-empty string.', - EMPTY_SPLIT: () => 'you passed an empty split name, split name must be a non-empty string.', - TRIMMABLE_SPLIT: (splitName: string) => `split name "${splitName}" has extra whitespace, trimming.` -}; - const invalidSplits = [ - { split: [], msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: () => { }, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: Object.create({}), msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: {}, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: true, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: false, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: 10, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: 0, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: NaN, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: Infinity, msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: null, msg: errorMsgs.NULL_SPLIT }, - { split: undefined, msg: errorMsgs.NULL_SPLIT }, - { split: new Promise(res => res), msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: Symbol('asd'), msg: errorMsgs.WRONG_TYPE_SPLIT }, - { split: '', msg: errorMsgs.EMPTY_SPLIT } + { split: [], msg: ERROR_INVALID }, + { split: () => { }, msg: ERROR_INVALID }, + { split: Object.create({}), msg: ERROR_INVALID }, + { split: {}, msg: ERROR_INVALID }, + { split: true, msg: ERROR_INVALID }, + { split: false, msg: ERROR_INVALID }, + { split: 10, msg: ERROR_INVALID }, + { split: 0, msg: ERROR_INVALID }, + { split: NaN, msg: ERROR_INVALID }, + { split: Infinity, msg: ERROR_INVALID }, + { split: null, msg: ERROR_NULL }, + { split: undefined, msg: ERROR_NULL }, + { split: new Promise(res => res), msg: ERROR_INVALID }, + { split: Symbol('asd'), msg: ERROR_INVALID }, + { split: '', msg: ERROR_EMPTY } ]; const trimmableSplits = [ @@ -52,7 +46,7 @@ describe('INPUT VALIDATION for Split name', () => { for (let i = 0; i < trimmableSplits.length; i++) { const trimmableSplit = trimmableSplits[i]; expect(validateSplit(loggerMock, trimmableSplit, 'some_method_splitName')).toBe(trimmableSplit.trim()); // It should return the trimmed version of the split name received. - expect(loggerMock.warn.mock.calls[0][0]).toEqual(`some_method_splitName: ${errorMsgs.TRIMMABLE_SPLIT(trimmableSplit)}`); // Should log a warning if those are enabled. + expect(loggerMock.warn).toBeCalledWith(WARN_TRIMMING, ['some_method_splitName', 'split name', trimmableSplit]); // Should log a warning if those are enabled. loggerMock.warn.mockClear(); } @@ -64,10 +58,10 @@ describe('INPUT VALIDATION for Split name', () => { for (let i = 0; i < invalidSplits.length; i++) { const invalidValue = invalidSplits[i]['split']; // @ts-ignore - const expectedLog = invalidSplits[i]['msg'](invalidValue); + const expectedLog = invalidSplits[i]['msg']; expect(validateSplit(loggerMock, invalidValue, 'test_method')).toBe(false); // Invalid event types should always return false. - expect(loggerMock.error.mock.calls[0][0]).toEqual(`test_method: ${expectedLog}`); // Should log the error for the invalid event type. + expect(loggerMock.error).toBeCalledWith(expectedLog, ['test_method', 'split name']); // Should log the error for the invalid event type. loggerMock.error.mockClear(); } diff --git a/src/utils/inputValidation/__tests__/splitExistance.spec.ts b/src/utils/inputValidation/__tests__/splitExistance.spec.ts index ae1cfd5f..6b72ad87 100644 --- a/src/utils/inputValidation/__tests__/splitExistance.spec.ts +++ b/src/utils/inputValidation/__tests__/splitExistance.spec.ts @@ -5,10 +5,7 @@ import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { validateSplitExistance } from '../splitExistance'; import { IReadinessManager } from '../../../readiness/types'; - -const errorMsgs = { - NOT_EXISTENT_SPLIT: (splitName: string) => `you passed "${splitName}" that does not exist in this environment, please double check what Splits exist in the web console.` -}; +import { WARN_NOT_EXISTENT_SPLIT } from '../../../logger/constants'; describe('Split existance (special case)', () => { @@ -43,7 +40,7 @@ describe('Split existance (special case)', () => { expect(validateSplitExistance(loggerMock, readinessManagerMock, 'other_split', LabelConstants.SPLIT_NOT_FOUND, 'other_method')).toBe(false); // Should return false if it receives a label but it is the split not found one. expect(loggerMock.warn.mock.calls.length).toBe(3); // It should have logged 3 warnings, one per each time we called it - loggerMock.warn.mock.calls.forEach(call => expect(call[0]).toBe(`other_method: ${errorMsgs.NOT_EXISTENT_SPLIT('other_split')}`)); // Warning logs should have the correct message. + loggerMock.warn.mock.calls.forEach(call => expect(call).toEqual([WARN_NOT_EXISTENT_SPLIT, ['other_method', 'other_split']])); // Warning logs should have the correct message. expect(loggerMock.error.mock.calls.length).toBe(0); // We log warnings, not errors. }); diff --git a/src/utils/inputValidation/__tests__/splits.spec.ts b/src/utils/inputValidation/__tests__/splits.spec.ts index 1bbe14d0..a4d51b71 100644 --- a/src/utils/inputValidation/__tests__/splits.spec.ts +++ b/src/utils/inputValidation/__tests__/splits.spec.ts @@ -3,6 +3,7 @@ import startsWith from 'lodash/startsWith'; // mocks sdkLogger import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import { ERROR_EMPTY_ARRAY } from '../../../logger/constants'; // mocks validateSplit jest.mock('../split'); @@ -61,7 +62,7 @@ describe('INPUT VALIDATION for Split names', () => { test('Should return false and log an error for the array if it is invalid', () => { for (let i = 0; i < invalidSplits.length; i++) { expect(validateSplits(loggerMock, invalidSplits[i], 'test_method')).toBe(false); // It will return false as the array is of an incorrect type. - expect(loggerMock.error.mock.calls).toEqual([['test_method: split_names must be a non-empty array.']]); // Should log the error for the collection. + expect(loggerMock.error).toBeCalledWith(ERROR_EMPTY_ARRAY, ['test_method', 'split_names']); // Should log the error for the collection. expect(validateSplitMock.mock.calls.length).toBe(0); // Should not try to validate any inner value if there is no valid array. loggerMock.error.mockClear(); diff --git a/src/utils/inputValidation/__tests__/trafficType.spec.ts b/src/utils/inputValidation/__tests__/trafficType.spec.ts index 2b5af848..43355676 100644 --- a/src/utils/inputValidation/__tests__/trafficType.spec.ts +++ b/src/utils/inputValidation/__tests__/trafficType.spec.ts @@ -1,30 +1,24 @@ +import { ERROR_NULL, ERROR_INVALID, ERROR_EMPTY, WARN_LOWERCASE_TRAFFIC_TYPE } from '../../../logger/constants'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; import { validateTrafficType } from '../trafficType'; -const errorMsgs = { - NULL_TRAFFIC_TYPE: 'you passed a null or undefined traffic_type_name, traffic_type_name must be a non-empty string.', - WRONG_TYPE_TRAFFIC_TYPE: 'you passed an invalid traffic_type_name, traffic_type_name must be a non-empty string.', - EMPTY_TRAFFIC_TYPE: 'you passed an empty traffic_type_name, traffic_type_name must be a non-empty string.', - LOWERCASE_TRAFFIC_TYPE: 'traffic_type_name should be all lowercase - converting string to lowercase.' -}; - const invalidTrafficTypes = [ - { tt: [], msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: () => { }, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: Object.create({}), msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: {}, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: true, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: false, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: 10, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: 0, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: NaN, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: Infinity, msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: null, msg: errorMsgs.NULL_TRAFFIC_TYPE }, - { tt: undefined, msg: errorMsgs.NULL_TRAFFIC_TYPE }, - { tt: new Promise(res => res), msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: Symbol('asd'), msg: errorMsgs.WRONG_TYPE_TRAFFIC_TYPE }, - { tt: '', msg: errorMsgs.EMPTY_TRAFFIC_TYPE } + { tt: [], msg: ERROR_INVALID }, + { tt: () => { }, msg: ERROR_INVALID }, + { tt: Object.create({}), msg: ERROR_INVALID }, + { tt: {}, msg: ERROR_INVALID }, + { tt: true, msg: ERROR_INVALID }, + { tt: false, msg: ERROR_INVALID }, + { tt: 10, msg: ERROR_INVALID }, + { tt: 0, msg: ERROR_INVALID }, + { tt: NaN, msg: ERROR_INVALID }, + { tt: Infinity, msg: ERROR_INVALID }, + { tt: null, msg: ERROR_NULL }, + { tt: undefined, msg: ERROR_NULL }, + { tt: new Promise(res => res), msg: ERROR_INVALID }, + { tt: Symbol('asd'), msg: ERROR_INVALID }, + { tt: '', msg: ERROR_EMPTY } ]; const convertibleTrafficTypes = [ @@ -53,7 +47,7 @@ describe('INPUT VALIDATION for Traffic Types', () => { const convertibleTrafficType = convertibleTrafficTypes[i]; expect(validateTrafficType(loggerMock, convertibleTrafficType, 'some_method_trafficType')).toBe(convertibleTrafficType.toLowerCase()); // It should return the lowercase version of the traffic type received. - expect(loggerMock.warn.mock.calls[i][0]).toEqual(`some_method_trafficType: ${errorMsgs.LOWERCASE_TRAFFIC_TYPE}`); // Should log a warning. + expect(loggerMock.warn.mock.calls[i]).toEqual([WARN_LOWERCASE_TRAFFIC_TYPE, ['some_method_trafficType']]); // Should log a warning. } expect(loggerMock.error.mock.calls.length).toBe(0); // It should have not logged any errors. @@ -65,7 +59,7 @@ describe('INPUT VALIDATION for Traffic Types', () => { const expectedLog = invalidTrafficTypes[i]['msg']; expect(validateTrafficType(loggerMock, invalidValue, 'test_method')).toBe(false); // Invalid traffic types should always return false. - expect(loggerMock.error.mock.calls[i][0]).toEqual(`test_method: ${expectedLog}`); // Should log the error for the invalid traffic type. + expect(loggerMock.error.mock.calls[i]).toEqual([expectedLog, ['test_method']]); // Should log the error for the invalid traffic type. } expect(loggerMock.warn.mock.calls.length).toBe(0); // It should have not logged any warnings. diff --git a/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts b/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts index f8e30bd7..ba2f1666 100644 --- a/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts +++ b/src/utils/inputValidation/__tests__/trafficTypeExistance.spec.ts @@ -2,6 +2,7 @@ import { IReadinessManager } from '../../../readiness/types'; import { ISplitsCacheBase } from '../../../storages/types'; import { LOCALHOST_MODE, STANDALONE_MODE } from '../../constants'; import thenable from '../../promise/thenable'; +import { WARN_NOT_EXISTENT_TT } from '../../../logger/constants'; /** Mocks */ import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; @@ -29,10 +30,6 @@ const splitsCacheMock = { /** Test target */ import { validateTrafficTypeExistance } from '../trafficTypeExistance'; -const errorMsgs = { - NOT_EXISTENT_TT: (ttName: string) => `Traffic Type ${ttName} does not have any corresponding Splits in this environment, make sure you're tracking your events to a valid traffic type defined in the Split console.` -}; - describe('validateTrafficTypeExistance', () => { afterEach(() => { @@ -43,16 +40,16 @@ describe('validateTrafficTypeExistance', () => { test('Should return true without going to the storage and log nothing if the SDK is not ready or in localhost mode', () => { // Not ready and not localstorage expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, 'test_tt', 'test_method')).toBe(true); // If the SDK is not ready yet, it will return true. - expect(splitsCacheMock.trafficTypeExists.mock.calls.length).toBe(0); // If the SDK is not ready yet, it does not try to go to the storage. - expect(loggerMock.error.mock.calls.length).toBe(0); // If the SDK is not ready yet, it will not log any errors. - expect(loggerMock.error.mock.calls.length).toBe(0); // If the SDK is not ready yet, it will not log any errors. + expect(splitsCacheMock.trafficTypeExists).not.toBeCalled(); // If the SDK is not ready yet, it does not try to go to the storage. + expect(loggerMock.error).not.toBeCalled(); // If the SDK is not ready yet, it will not log any errors. + expect(loggerMock.error).not.toBeCalled(); // If the SDK is not ready yet, it will not log any errors. // Ready and in localstorage mode. readinessManagerMock.isReady.mockImplementation(() => true); expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, LOCALHOST_MODE, 'test_tt', 'test_method')).toBe(true); // If the SDK is in localhost mode, it will return true. - expect(splitsCacheMock.trafficTypeExists.mock.calls.length).toBe(0); // If the SDK is in localhost mode, it does not try to go to the storage. - expect(loggerMock.warn.mock.calls.length).toBe(0); // If the SDK is in localhost mode, it will not log any warnings. - expect(loggerMock.error.mock.calls.length).toBe(0); // If the SDK is in localhost mode, it will not log any errors. + expect(splitsCacheMock.trafficTypeExists).not.toBeCalled(); // If the SDK is in localhost mode, it does not try to go to the storage. + expect(loggerMock.warn).not.toBeCalled(); // If the SDK is in localhost mode, it will not log any warnings. + expect(loggerMock.error).not.toBeCalled(); // If the SDK is in localhost mode, it will not log any errors. }); test('Should return true and log nothing if SDK Ready, not localhost mode and the traffic type exists in the storage', () => { @@ -61,16 +58,16 @@ describe('validateTrafficTypeExistance', () => { expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_TT, 'test_method')).toBe(true); // If the SDK is in condition to validate but the TT exists, it will return true. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the storage. - expect(loggerMock.warn.mock.calls.length).toBe(0); // If the SDK is in condition to validate but the TT exists, it will not log any warnings. - expect(loggerMock.error.mock.calls.length).toBe(0); // If the SDK is in condition to validate but the TT exists, it will not log any errors. + expect(loggerMock.warn).not.toBeCalled(); // If the SDK is in condition to validate but the TT exists, it will not log any warnings. + expect(loggerMock.error).not.toBeCalled(); // If the SDK is in condition to validate but the TT exists, it will not log any errors. }); test('Should return false and log warning if SDK Ready, not localhost mode and the traffic type does NOT exist in the storage', () => { // Ready, standalone, and the TT not exists in the storage. expect(validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_TT, 'test_method_y')).toBe(false); // If the SDK is in condition to validate but the TT does not exist in the storage, it will return false. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the storage. - expect(loggerMock.warn.mock.calls).toEqual([[`test_method_y: ${errorMsgs.NOT_EXISTENT_TT(TEST_NOT_EXISTENT_TT)}`]]); // If the SDK is in condition to validate but the TT does not exist in the storage, it will log the expected warning. - expect(loggerMock.error.mock.calls.length).toBe(0); // It logged a warning so no errors should be logged. + expect(loggerMock.warn).toBeCalledWith(WARN_NOT_EXISTENT_TT, ['test_method_y', TEST_NOT_EXISTENT_TT]); // If the SDK is in condition to validate but the TT does not exist in the storage, it will log the expected warning. + expect(loggerMock.error).not.toBeCalled(); // It logged a warning so no errors should be logged. }); test('validateTrafficTypeExistance w/async storage - If the storage is async but the SDK is in condition to validate, it will validate that the TT exists on the storage', async () => { @@ -79,14 +76,14 @@ describe('validateTrafficTypeExistance', () => { const validationPromise = validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_EXISTENT_ASYNC_TT, 'test_method_z'); expect(thenable(validationPromise)).toBe(true); // If the storage is async, it should also return a promise. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the async storage. - expect(loggerMock.warn.mock.calls.length).toBe(0); // We are still fetching the data from the storage, no logs yet. - expect(loggerMock.error.mock.calls.length).toBe(0); // We are still fetching the data from the storage, no logs yet. + expect(loggerMock.warn).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. + expect(loggerMock.error).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. const isValid = await validationPromise; expect(isValid).toBe(true); // As the split existed, it will resolve to true. - expect(loggerMock.warn.mock.calls.length).toBe(0); // It was valid so no logs. - expect(loggerMock.error.mock.calls.length).toBe(0); // It was valid so no logs. + expect(loggerMock.warn).not.toBeCalled(); // It was valid so no logs. + expect(loggerMock.error).not.toBeCalled(); // It was valid so no logs. // Second round, a TT that does not exist on the asnyc storage splitsCacheMock.trafficTypeExists.mockClear(); @@ -94,12 +91,12 @@ describe('validateTrafficTypeExistance', () => { const validationPromise2 = validateTrafficTypeExistance(loggerMock, readinessManagerMock, splitsCacheMock, STANDALONE_MODE, TEST_NOT_EXISTENT_ASYNC_TT, 'test_method_z'); expect(thenable(validationPromise2)).toBe(true); // If the storage is async, it should also return a promise. expect(splitsCacheMock.trafficTypeExists.mock.calls).toEqual([[TEST_NOT_EXISTENT_ASYNC_TT]]); // If the SDK is in condition to validate, it checks that TT existance with the async storage. - expect(loggerMock.warn.mock.calls.length).toBe(0); // We are still fetching the data from the storage, no logs yet. - expect(loggerMock.error.mock.calls.length).toBe(0); // We are still fetching the data from the storage, no logs yet. + expect(loggerMock.warn).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. + expect(loggerMock.error).not.toBeCalled(); // We are still fetching the data from the storage, no logs yet. const isValid2 = await validationPromise2; expect(isValid2).toBe(false); // As the split is not on the storage, it will resolve to false, failing the validation.. - expect(loggerMock.warn.mock.calls).toEqual([[`test_method_z: ${errorMsgs.NOT_EXISTENT_TT(TEST_NOT_EXISTENT_ASYNC_TT)}`]]); // If the SDK is in condition to validate but the TT does not exist in the storage, it will log the expected warning. - expect(loggerMock.error.mock.calls.length).toBe(0); // It logged a warning so no errors should be logged. + expect(loggerMock.warn).toBeCalledWith(WARN_NOT_EXISTENT_TT, ['test_method_z', TEST_NOT_EXISTENT_ASYNC_TT]); // If the SDK is in condition to validate but the TT does not exist in the storage, it will log the expected warning. + expect(loggerMock.error).not.toBeCalled(); // It logged a warning so no errors should be logged. }); }); diff --git a/src/utils/inputValidation/attributes.ts b/src/utils/inputValidation/attributes.ts index d40f21db..e45f67c0 100644 --- a/src/utils/inputValidation/attributes.ts +++ b/src/utils/inputValidation/attributes.ts @@ -1,13 +1,13 @@ import { isObject } from '../lang'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; -import { ERROR_13 } from '../../logger/constants'; +import { ERROR_NOT_PLAIN_OBJECT } from '../../logger/constants'; export function validateAttributes(log: ILogger, maybeAttrs: any, method: string): SplitIO.Attributes | undefined | false { // Attributes are optional if (isObject(maybeAttrs) || maybeAttrs == undefined) // eslint-disable-line eqeqeq return maybeAttrs; - log.error(ERROR_13, [method]); + log.error(ERROR_NOT_PLAIN_OBJECT, [method, 'attributes']); return false; } diff --git a/src/utils/inputValidation/event.ts b/src/utils/inputValidation/event.ts index f067b23e..03e79c9e 100644 --- a/src/utils/inputValidation/event.ts +++ b/src/utils/inputValidation/event.ts @@ -1,4 +1,4 @@ -import { ERROR_14, ERROR_15, ERROR_16, ERROR_17 } from '../../logger/constants'; +import { ERROR_EVENT_TYPE_FORMAT, ERROR_NULL, ERROR_INVALID, ERROR_EMPTY } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { isString } from '../lang'; @@ -6,14 +6,14 @@ const EVENT_TYPE_REGEX = /^[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}$/; export function validateEvent(log: ILogger, maybeEvent: any, method: string): string | false { if (maybeEvent == undefined) { // eslint-disable-line eqeqeq - log.error(ERROR_15, [method]); + log.error(ERROR_NULL, [method]); } else if (!isString(maybeEvent)) { - log.error(ERROR_16, [method]); + log.error(ERROR_INVALID, [method]); } else { // It is a string. if (maybeEvent.length === 0) { - log.error(ERROR_17, [method]); + log.error(ERROR_EMPTY, [method]); } else if (!EVENT_TYPE_REGEX.test(maybeEvent)) { - log.error(ERROR_14, [method, maybeEvent]); + log.error(ERROR_EVENT_TYPE_FORMAT, [method, maybeEvent]); } else { return maybeEvent; } diff --git a/src/utils/inputValidation/eventProperties.ts b/src/utils/inputValidation/eventProperties.ts index 6f195e09..8a599a97 100644 --- a/src/utils/inputValidation/eventProperties.ts +++ b/src/utils/inputValidation/eventProperties.ts @@ -1,7 +1,7 @@ import { isObject, shallowClone, isString, isFiniteNumber, isBoolean } from '../lang'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; -import { ERROR_18, ERROR_19, WARN_12, WARN_13 } from '../../logger/constants'; +import { ERROR_NOT_PLAIN_OBJECT, 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. @@ -17,7 +17,7 @@ export function validateEventProperties(log: ILogger, maybeProperties: any, meth if (maybeProperties == undefined) return { properties: null, size: BASE_EVENT_SIZE }; // eslint-disable-line eqeqeq if (!isObject(maybeProperties)) { - log.error(ERROR_18, [method]); + log.error(ERROR_NOT_PLAIN_OBJECT, [method, 'properties']); return { properties: false, size: BASE_EVENT_SIZE }; } @@ -30,7 +30,7 @@ export function validateEventProperties(log: ILogger, maybeProperties: any, meth }; if (keys.length > MAX_PROPERTIES_AMOUNT) { - log.warn(WARN_13, [method]); + log.warn(WARN_TRIMMING_PROPERTIES, [method]); } for (let i = 0; i < keys.length; i++) { @@ -47,7 +47,7 @@ export function validateEventProperties(log: ILogger, maybeProperties: any, meth clone[keys[i]] = null; val = null; isNullVal = true; - log.warn(WARN_12, [method, keys[i]]); + log.warn(WARN_SETTING_NULL, [method, keys[i]]); } if (isNullVal) output.size += ECMA_SIZES.NULL; @@ -56,7 +56,7 @@ export function validateEventProperties(log: ILogger, maybeProperties: any, meth else if (isStringVal) output.size += val.length * ECMA_SIZES.STRING; if (output.size > MAX_PROPERTIES_SIZE) { - log.error(ERROR_19, [method]); + log.error(ERROR_SIZE_EXCEEDED, [method]); output.properties = false; break; } diff --git a/src/utils/inputValidation/eventValue.ts b/src/utils/inputValidation/eventValue.ts index ef9f5c36..9b7bdd16 100644 --- a/src/utils/inputValidation/eventValue.ts +++ b/src/utils/inputValidation/eventValue.ts @@ -1,4 +1,4 @@ -import { ERROR_20 } from '../../logger/constants'; +import { ERROR_NOT_FINITE } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { isFiniteNumber } from '../lang'; @@ -6,6 +6,6 @@ export function validateEventValue(log: ILogger, maybeValue: any, method: string if (isFiniteNumber(maybeValue) || maybeValue == undefined) // eslint-disable-line eqeqeq return maybeValue; - log.error(ERROR_20, [method]); + log.error(ERROR_NOT_FINITE, [method]); return false; } diff --git a/src/utils/inputValidation/key.ts b/src/utils/inputValidation/key.ts index f7ec064f..9068bed6 100644 --- a/src/utils/inputValidation/key.ts +++ b/src/utils/inputValidation/key.ts @@ -1,17 +1,17 @@ import { isObject, isString, isFiniteNumber, toString } from '../lang'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; -import { ERROR_22, WARN_15, ERROR_25, ERROR_23, ERROR_24, ERROR_26 } from '../../logger/constants'; +import { ERROR_NULL, WARN_CONVERTING, ERROR_EMPTY, ERROR_TOO_LONG, ERROR_INVALID, ERROR_INVALID_KEY_OBJECT } from '../../logger/constants'; const KEY_MAX_LENGTH = 250; function validateKeyValue(log: ILogger, maybeKey: any, method: string, type: string): string | false { if (maybeKey == undefined) { // eslint-disable-line eqeqeq - log.error(ERROR_22, [method, type, type]); + log.error(ERROR_NULL, [method, type]); return false; } if (isFiniteNumber(maybeKey)) { - log.warn(WARN_15, [method, type, maybeKey]); + log.warn(WARN_CONVERTING, [method, type, maybeKey]); return toString(maybeKey); } if (isString(maybeKey)) { @@ -22,12 +22,12 @@ function validateKeyValue(log: ILogger, maybeKey: any, method: string, type: str if (maybeKey.length > 0 && maybeKey.length <= KEY_MAX_LENGTH) return maybeKey; if (maybeKey.length === 0) { - log.error(ERROR_25, [method, type]); + log.error(ERROR_EMPTY, [method, type]); } else if (maybeKey.length > KEY_MAX_LENGTH) { - log.error(ERROR_23, [method, type, type]); + log.error(ERROR_TOO_LONG, [method, type]); } } else { - log.error(ERROR_24, [method, type, type]); + log.error(ERROR_INVALID, [method, type]); } return false; @@ -43,7 +43,7 @@ export function validateKey(log: ILogger, maybeKey: any, method: string): SplitI matchingKey, bucketingKey }; - log.error(ERROR_26, [method]); + log.error(ERROR_INVALID_KEY_OBJECT, [method]); return false; } else { return validateKeyValue(log, maybeKey, method, 'key'); diff --git a/src/utils/inputValidation/split.ts b/src/utils/inputValidation/split.ts index 12064144..b7261604 100644 --- a/src/utils/inputValidation/split.ts +++ b/src/utils/inputValidation/split.ts @@ -1,4 +1,4 @@ -import { ERROR_22, ERROR_32, WARN_17, ERROR_33 } from '../../logger/constants'; +import { ERROR_NULL, ERROR_INVALID, WARN_TRIMMING, ERROR_EMPTY } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { isString } from '../lang'; @@ -7,19 +7,19 @@ const TRIMMABLE_SPACES_REGEX = /^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/; export function validateSplit(log: ILogger, maybeSplit: any, method: string, item = 'split name'): string | false { if (maybeSplit == undefined) { // eslint-disable-line eqeqeq - log.error(ERROR_22, [method, item, item]); + log.error(ERROR_NULL, [method, item]); } else if (!isString(maybeSplit)) { - log.error(ERROR_32, [method, item, item]); + log.error(ERROR_INVALID, [method, item]); } else { if (TRIMMABLE_SPACES_REGEX.test(maybeSplit)) { - log.warn(WARN_17, [method, item, maybeSplit]); + log.warn(WARN_TRIMMING, [method, item, maybeSplit]); maybeSplit = maybeSplit.trim(); } if (maybeSplit.length > 0) { return maybeSplit; } else { - log.error(ERROR_33, [method, item, item]); + log.error(ERROR_EMPTY, [method, item]); } } diff --git a/src/utils/inputValidation/splitExistance.ts b/src/utils/inputValidation/splitExistance.ts index b8798f84..ddafa767 100644 --- a/src/utils/inputValidation/splitExistance.ts +++ b/src/utils/inputValidation/splitExistance.ts @@ -1,7 +1,7 @@ import { SPLIT_NOT_FOUND } from '../labels'; import { IReadinessManager } from '../../readiness/types'; import { ILogger } from '../../logger/types'; -import { WARN_18 } from '../../logger/constants'; +import { WARN_NOT_EXISTENT_SPLIT } from '../../logger/constants'; /** * This is defined here and in this format mostly because of the logger and the fact that it's considered a validation at product level. @@ -10,7 +10,7 @@ import { WARN_18 } from '../../logger/constants'; export function validateSplitExistance(log: ILogger, readinessManager: IReadinessManager, splitName: string, labelOrSplitObj: any, method: string): boolean { if (readinessManager.isReady()) { // Only if it's ready we validate this, otherwise it may just be that the SDK is not ready yet. if (labelOrSplitObj === SPLIT_NOT_FOUND || labelOrSplitObj == null) { - log.warn(WARN_18, [method, splitName]); + log.warn(WARN_NOT_EXISTENT_SPLIT, [method, splitName]); return false; } } diff --git a/src/utils/inputValidation/splits.ts b/src/utils/inputValidation/splits.ts index 6807d7da..ee381210 100644 --- a/src/utils/inputValidation/splits.ts +++ b/src/utils/inputValidation/splits.ts @@ -1,4 +1,4 @@ -import { ERROR_34 } from '../../logger/constants'; +import { ERROR_EMPTY_ARRAY } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { uniq } from '../lang'; import { validateSplit } from './split'; @@ -16,6 +16,6 @@ export function validateSplits(log: ILogger, maybeSplits: any, method: string, l if (validatedArray.length) return uniq(validatedArray); } - log.error(ERROR_34, [method, listName]); + log.error(ERROR_EMPTY_ARRAY, [method, listName]); return false; } diff --git a/src/utils/inputValidation/trafficType.ts b/src/utils/inputValidation/trafficType.ts index 189560e8..79f1eb02 100644 --- a/src/utils/inputValidation/trafficType.ts +++ b/src/utils/inputValidation/trafficType.ts @@ -1,4 +1,4 @@ -import { ERROR_35, ERROR_36, ERROR_37, WARN_19 } from '../../logger/constants'; +import { ERROR_NULL, ERROR_INVALID, ERROR_EMPTY, WARN_LOWERCASE_TRAFFIC_TYPE } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { isString } from '../lang'; @@ -6,15 +6,15 @@ const CAPITAL_LETTERS_REGEX = /[A-Z]/; export function validateTrafficType(log: ILogger, maybeTT: any, method: string): string | false { if (maybeTT == undefined) { // eslint-disable-line eqeqeq - log.error(ERROR_35, [method]); + log.error(ERROR_NULL, [method]); } else if (!isString(maybeTT)) { - log.error(ERROR_36, [method]); + log.error(ERROR_INVALID, [method]); } else { if (maybeTT.length === 0) { - log.error(ERROR_37, [method]); + log.error(ERROR_EMPTY, [method]); } else { if (CAPITAL_LETTERS_REGEX.test(maybeTT)) { - log.warn(WARN_19, [method]); + log.warn(WARN_LOWERCASE_TRAFFIC_TYPE, [method]); maybeTT = maybeTT.toLowerCase(); } diff --git a/src/utils/inputValidation/trafficTypeExistance.ts b/src/utils/inputValidation/trafficTypeExistance.ts index 79244801..eb64c0d3 100644 --- a/src/utils/inputValidation/trafficTypeExistance.ts +++ b/src/utils/inputValidation/trafficTypeExistance.ts @@ -5,10 +5,10 @@ import { IReadinessManager } from '../../readiness/types'; import { SDKMode } from '../../types'; import { MaybeThenable } from '../../dtos/types'; import { ILogger } from '../../logger/types'; -import { WARN_20 } from '../../logger/constants'; +import { WARN_NOT_EXISTENT_TT } from '../../logger/constants'; function logTTExistanceWarning(log: ILogger, maybeTT: string, method: string) { - log.warn(WARN_20, [method, maybeTT]); + log.warn(WARN_NOT_EXISTENT_TT, [method, maybeTT]); } /** From adfc9812a688740bc9c9f277f67a8b7f67523d7d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 17 Mar 2021 16:41:53 -0300 Subject: [PATCH 2/2] update for review --- src/logger/messages/error.ts | 3 +-- src/logger/messages/warn.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index eb99b714..65461821 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -12,6 +12,7 @@ export const codesError: [number, string][] = [ [ERROR_11, 'splitio-client:impressions-tracker => Could not store impressions bulk with %s impression%s. Error: %s'], [ERROR_12, 'splitio-client:impressions-tracker => Impression listener logImpression method threw: %s.'], [ERROR_21, 'Client has already been destroyed - no calls possible.'], + [ERROR_38, 'splitio-settings => You passed an invalid impressionsMode, impressionsMode should be one of the following values: "%s" or "%s". Defaulting to "%s" mode.'], [ERROR_39, 'Response status is not OK. Status: %s. URL: %s. Message: %s'], // input validation [ERROR_EVENT_TYPE_FORMAT, '%s: you passed "%s", event_type must adhere to the regular expression /^[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}$/g. This means an event_type must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, underscore, period, or colon as separators of alphanumeric characters.'], @@ -24,6 +25,4 @@ export const codesError: [number, string][] = [ [ERROR_INVALID, '%s: you passed an invalid %s. It must be a non-empty string.'], [ERROR_EMPTY, '%s: you passed an empty %s. It must be a non-empty string.'], [ERROR_EMPTY_ARRAY, '%s: %s must be a non-empty array.'], - // initialization / settings validation - [ERROR_INVALID_ENUM, '%s: you passed an invalid %s. It should be one of the following values: %s. Defaulting to "%s" mode.'], ]; diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index 9a9e1328..97aab2f9 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -12,10 +12,10 @@ export const codesWarn: [number, string][] = [ [WARN_9, 'splitio-sync:submitters => Droping %s %s after retry. Reason %s.'], [WARN_10, 'splitio-sync:submitters => Failed to push %s %s, keeping data to retry on next iteration. Reason %s.'], [WARN_11, 'splitio-client:event-tracker => Failed to queue %s'], + [WARN_14, '%s: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method.'], // input validation [WARN_SETTING_NULL, '%s: Property "%s" is of invalid type. Setting value to null.'], [WARN_TRIMMING_PROPERTIES, '%s: Event has more than 300 properties. Some of them will be trimmed when processed.'], - [WARN_14, '%s: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method.'], [WARN_CONVERTING, '%s: %s "%s" is not of type string, converting.'], [WARN_TRIMMING, '%s: %s "%s" has extra whitespace, trimming.'], [WARN_NOT_EXISTENT_SPLIT, '%s: split "%s" does not exist in this environment, please double check what splits exist in the web console.'],