From 199a13bd46f78f19f219fb925fc4f0b305ede1b4 Mon Sep 17 00:00:00 2001 From: Shinigami92 Date: Fri, 8 Apr 2022 13:06:50 +0200 Subject: [PATCH 1/5] refactor: improve unique --- src/unique.ts | 12 ++++++------ src/utils/unique.ts | 27 ++++++++++++++++++++++----- test/unique.spec.ts | 36 ++++++++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/unique.ts b/src/unique.ts index b19f6132dd5..69ba24df30e 100644 --- a/src/unique.ts +++ b/src/unique.ts @@ -32,12 +32,12 @@ export class Unique { * @param method The method used to generate the values. * @param args The arguments used to call the method. * @param opts The optional options used to configure this method. - * @param opts.startTime The time this execution stared. This will be ignored/overwritten. - * @param opts.maxTime The time this method may take before throwing an error. - * @param opts.maxRetries The total number of attempts to try before throwing an error. - * @param opts.currentIterations The current attempt. This will be ignored/overwritten. - * @param opts.exclude The value or values that should be excluded/skipped. - * @param opts.compare The function used to determine whether a value was already returned. + * @param opts.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. + * @param opts.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. + * @param opts.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. + * @param opts.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. + * @param opts.exclude The value or values that should be excluded/skipped. Defaults to `[]`. + * @param opts.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. * * @example * faker.unique(faker.name.firstName) // 'Corbin' diff --git a/src/utils/unique.ts b/src/utils/unique.ts index 2679b3d86d5..cb4d24a6a19 100644 --- a/src/utils/unique.ts +++ b/src/utils/unique.ts @@ -44,27 +44,44 @@ function errorMessage( 'ms' ); throw new FakerError( - code + - ' for uniqueness check \n\nMay not be able to generate any more unique values with current settings. \nTry adjusting maxTime or maxRetries parameters for faker.unique()' + `${code} for uniqueness check. + +May not be able to generate any more unique values with current settings. +Try adjusting maxTime or maxRetries parameters for faker.unique().` ); } +/** + * Generates a unique result using the results of the given method. + * Used unique entries will be stored internally and filtered from subsequent calls. + * + * @template Method The type of the method to execute. + * @param method The method used to generate the values. + * @param args The arguments used to call the method. + * @param opts The optional options used to configure this method. + * @param opts.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. + * @param opts.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. + * @param opts.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. + * @param opts.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. + * @param opts.exclude The value or values that should be excluded/skipped. Defaults to `[]`. + * @param opts.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. + */ export function exec RecordKey>( method: Method, args: Parameters, opts: { + startTime?: number; maxTime?: number; maxRetries?: number; + currentIterations?: number; exclude?: RecordKey | RecordKey[]; compare?: (obj: Record, key: RecordKey) => 0 | -1; - currentIterations?: number; - startTime?: number; } ): ReturnType { const now = new Date().getTime(); opts = opts || {}; - opts.maxTime = opts.maxTime || 3; + opts.maxTime = opts.maxTime || 50; opts.maxRetries = opts.maxRetries || 50; opts.exclude = opts.exclude || exclude; opts.compare = opts.compare || defaultCompare; diff --git a/test/unique.spec.ts b/test/unique.spec.ts index fab12ba7002..f38bc5c9566 100644 --- a/test/unique.spec.ts +++ b/test/unique.spec.ts @@ -48,14 +48,14 @@ describe('unique', () => { for (const { seed, expectations } of seededRuns) { describe(`seed: ${seed}`, () => { - it(`unique(customMethod)`, () => { + it('unique(customMethod)', () => { faker.seed(seed); const actual = faker.unique(customMethod); expect(actual).toEqual(expectations.withCustomMethod); }); - it(`unique(customMethod, args)`, () => { + it('unique(customMethod, args)', () => { faker.seed(seed); const prefix = 'prefix-1-'; @@ -64,14 +64,14 @@ describe('unique', () => { expect(actual).toEqual(prefix + expectations.withCustomMethod); }); - it(`unique(() => number)`, () => { + it('unique(() => number)', () => { faker.seed(seed); const actual = faker.unique(faker.datatype.number); expect(actual).toEqual(expectations.withNumberMethod); }); - it(`unique(() => number), args)`, () => { + it('unique(() => number), args)', () => { faker.seed(seed); const actual = faker.unique(faker.datatype.number, [50]); @@ -109,7 +109,12 @@ describe('unique', () => { maxRetries: 9999, exclude: ['https', 'http'], }); - }).toThrowError(/^Exceeded maxTime:/); + }).toThrowError( + new FakerError(`Exceeded maxTime: 1 for uniqueness check. + +May not be able to generate any more unique values with current settings. +Try adjusting maxTime or maxRetries parameters for faker.unique().`) + ); }); it('should be possible to limit unique call by maxRetries', () => { @@ -119,7 +124,12 @@ describe('unique', () => { maxRetries: 5, exclude: ['https', 'http'], }); - }).toThrowError(/^Exceeded maxRetries:/); + }).toThrowError( + new FakerError(`Exceeded maxRetries: 5 for uniqueness check. + +May not be able to generate any more unique values with current settings. +Try adjusting maxTime or maxRetries parameters for faker.unique().`) + ); }); it('should throw a FakerError instance on error', () => { @@ -129,7 +139,12 @@ describe('unique', () => { maxRetries: 5, exclude: ['https', 'http'], }); - }).toThrowError(FakerError); + }).toThrowError( + new FakerError(`Exceeded maxRetries: 5 for uniqueness check. + +May not be able to generate any more unique values with current settings. +Try adjusting maxTime or maxRetries parameters for faker.unique().`) + ); }); }); } @@ -159,6 +174,11 @@ describe('unique', () => { faker.unique(method, [], { maxRetries: 1, }) - ).toThrow(); + ).toThrowError( + new FakerError(`Exceeded maxRetries: 1 for uniqueness check. + +May not be able to generate any more unique values with current settings. +Try adjusting maxTime or maxRetries parameters for faker.unique().`) + ); }); }); From ce1d7a089c256bee141e5dfb4272131ba0d36485 Mon Sep 17 00:00:00 2001 From: Shinigami92 Date: Fri, 8 Apr 2022 13:12:19 +0200 Subject: [PATCH 2/5] chore: deprecate unique member fields --- src/unique.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/unique.ts b/src/unique.ts index 69ba24df30e..fc310fc7067 100644 --- a/src/unique.ts +++ b/src/unique.ts @@ -5,15 +5,20 @@ import * as uniqueExec from './utils/unique'; * Module to generate unique entries. */ export class Unique { - // maximum time unique.exec will attempt to run before aborting + /** + * Maximum time `unique.exec` will attempt to run before aborting. + * + * @deprecated Use options instead. + */ maxTime = 10; - // maximum retries unique.exec will recurse before aborting ( max loop depth ) + /** + * Maximum retries `unique.exec` will recurse before aborting (max loop depth). + * + * @deprecated Use options instead. + */ maxRetries = 10; - // time the script started - // startTime: number = 0; - constructor() { // Bind `this` so namespaced is working correctly for (const name of Object.getOwnPropertyNames(Unique.prototype)) { From 5604986184dc5a39728cf05d1f8a1cd97e9c6e92 Mon Sep 17 00:00:00 2001 From: Shinigami92 Date: Fri, 8 Apr 2022 14:03:46 +0200 Subject: [PATCH 3/5] refactor: improve unique --- src/unique.ts | 41 ++++++------ src/utils/unique.ts | 152 +++++++++++++++++++++----------------------- test/unique.spec.ts | 29 +++++++++ 3 files changed, 125 insertions(+), 97 deletions(-) diff --git a/src/unique.ts b/src/unique.ts index fc310fc7067..40cd0668006 100644 --- a/src/unique.ts +++ b/src/unique.ts @@ -36,13 +36,13 @@ export class Unique { * @template Method The type of the method to execute. * @param method The method used to generate the values. * @param args The arguments used to call the method. - * @param opts The optional options used to configure this method. - * @param opts.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. - * @param opts.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. - * @param opts.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. - * @param opts.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. - * @param opts.exclude The value or values that should be excluded/skipped. Defaults to `[]`. - * @param opts.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. + * @param options The optional options used to configure this method. + * @param options.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. + * @param options.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. + * @param options.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. + * @param options.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. + * @param options.exclude The value or values that should be excluded/skipped. Defaults to `[]`. + * @param options.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. * * @example * faker.unique(faker.name.firstName) // 'Corbin' @@ -50,24 +50,27 @@ export class Unique { unique RecordKey>( method: Method, args?: Parameters, - opts?: { + options: { startTime?: number; maxTime?: number; maxRetries?: number; currentIterations?: number; exclude?: RecordKey | RecordKey[]; compare?: (obj: Record, key: RecordKey) => 0 | -1; - } + } = {} ): ReturnType { - opts = opts || {}; - opts.startTime = new Date().getTime(); - if (typeof opts.maxTime !== 'number') { - opts.maxTime = this.maxTime; - } - if (typeof opts.maxRetries !== 'number') { - opts.maxRetries = this.maxRetries; - } - opts.currentIterations = 0; - return uniqueExec.exec(method, args, opts); + const { + startTime = new Date().getTime(), + maxTime = this.maxTime, + maxRetries = this.maxRetries, + currentIterations = 0, + } = options; + return uniqueExec.exec(method, args, { + ...options, + startTime, + maxTime, + maxRetries, + currentIterations, + }); } } diff --git a/src/utils/unique.ts b/src/utils/unique.ts index cb4d24a6a19..0ff4cbb3f93 100644 --- a/src/utils/unique.ts +++ b/src/utils/unique.ts @@ -2,21 +2,31 @@ import { FakerError } from '../errors/faker-error'; export type RecordKey = string | number | symbol; -// global results store -// currently uniqueness is global to entire faker instance -// this means that faker should currently *never* return duplicate values across all API methods when using `Faker.unique` -// it's possible in the future that some users may want to scope found per function call instead of faker instance -const found: Record = {}; +/** + * Global store of unique values. + * Uniqueness for entire faker instance. + * This means that faker should *never* return duplicate values across all API methods when using `Faker.unique` without passing `options.store`. + */ +const GLOBAL_UNIQUE_STORE: Record = {}; -// global exclude list of results -// defaults to nothing excluded -const exclude: RecordKey[] = []; +/** + * Global exclude list of results. + * Defaults to nothing excluded. + */ +const GLOBAL_UNIQUE_EXCLUDE: RecordKey[] = []; -// current iteration or retries of unique.exec ( current loop depth ) +/** + * Current iteration or retries of `unique.exec` (current loop depth). + */ const currentIterations = 0; -// uniqueness compare function -// default behavior is to check value as key against object hash +/** + * Uniqueness compare function. + * Default behavior is to check value as key against object hash. + * + * @param obj The object to check. + * @param key The key to check. + */ function defaultCompare( obj: Record, key: RecordKey @@ -27,21 +37,24 @@ function defaultCompare( return 0; } -// common error handler for messages -function errorMessage( - now: number, - code: string, - opts: { startTime: number } -): never { - console.error('error', code); +/** + * Logs the given code as an error and throws it. + * Also logs a message for helping the user. + * + * @param startTime The time the execution started. + * @param now The current time. + * @param code The error code. + * + * @throws The given error code with additional text. + */ +function errorMessage(startTime: number, now: number, code: string): never { + console.error('Error', code); console.log( - 'found', - Object.keys(found).length, - 'unique entries before throwing error. \nretried:', - currentIterations, - '\ntotal time:', - now - opts.startTime, - 'ms' + `Found ${ + Object.keys(GLOBAL_UNIQUE_STORE).length + } unique entries before throwing error. +retried: ${currentIterations} +total time: ${now - startTime}ms` ); throw new FakerError( `${code} for uniqueness check. @@ -58,86 +71,69 @@ Try adjusting maxTime or maxRetries parameters for faker.unique().` * @template Method The type of the method to execute. * @param method The method used to generate the values. * @param args The arguments used to call the method. - * @param opts The optional options used to configure this method. - * @param opts.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. - * @param opts.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. - * @param opts.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. - * @param opts.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. - * @param opts.exclude The value or values that should be excluded/skipped. Defaults to `[]`. - * @param opts.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. + * @param options The optional options used to configure this method. + * @param options.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. + * @param options.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. + * @param options.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. + * @param options.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. + * @param options.exclude The value or values that should be excluded/skipped. Defaults to `[]`. + * @param options.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. */ export function exec RecordKey>( method: Method, args: Parameters, - opts: { + options: { startTime?: number; maxTime?: number; maxRetries?: number; currentIterations?: number; exclude?: RecordKey | RecordKey[]; compare?: (obj: Record, key: RecordKey) => 0 | -1; - } + } = {} ): ReturnType { const now = new Date().getTime(); - opts = opts || {}; - opts.maxTime = opts.maxTime || 50; - opts.maxRetries = opts.maxRetries || 50; - opts.exclude = opts.exclude || exclude; - opts.compare = opts.compare || defaultCompare; - - if (typeof opts.currentIterations !== 'number') { - opts.currentIterations = 0; - } - - if (opts.startTime == null) { - opts.startTime = new Date().getTime(); + const { + startTime = new Date().getTime(), + maxTime = 50, + maxRetries = 50, + compare = defaultCompare, + } = options; + let { exclude = GLOBAL_UNIQUE_EXCLUDE } = options; + options.currentIterations ||= 0; + + // Support single exclude argument as string + if (!Array.isArray(exclude)) { + exclude = [exclude]; } - const startTime = opts.startTime; - - // support single exclude argument as string - if (!Array.isArray(opts.exclude)) { - opts.exclude = [opts.exclude]; - } - - if (opts.currentIterations > 0) { - // console.log('iterating', currentIterations) - } + // if (options.currentIterations > 0) { + // console.log('iterating', options.currentIterations) + // } // console.log(now - startTime) - if (now - startTime >= opts.maxTime) { - return errorMessage( - now, - `Exceeded maxTime: ${opts.maxTime}`, - // @ts-expect-error: we know that opts.startTime is defined - opts - ); + if (now - startTime >= maxTime) { + return errorMessage(startTime, now, `Exceeded maxTime: ${maxTime}`); } - if (opts.currentIterations >= opts.maxRetries) { - return errorMessage( - now, - `Exceeded maxRetries: ${opts.maxRetries}`, - // @ts-expect-error: we know that opts.startTime is defined - opts - ); + if (options.currentIterations >= maxRetries) { + return errorMessage(startTime, now, `Exceeded maxRetries: ${maxRetries}`); } - // execute the provided method to find a potential satisfied value + // Execute the provided method to find a potential satisfied value. const result: ReturnType = method.apply(this, args); - // if the result has not been previously found, add it to the found array and return the value as it's unique + // If the result has not been previously found, add it to the found array and return the value as it's unique. if ( - opts.compare(found, result) === -1 && - opts.exclude.indexOf(result) === -1 + compare(GLOBAL_UNIQUE_STORE, result) === -1 && + exclude.indexOf(result) === -1 ) { - found[result] = result; - opts.currentIterations = 0; + GLOBAL_UNIQUE_STORE[result] = result; + options.currentIterations = 0; return result; } else { // console.log('conflict', result); - opts.currentIterations++; - return exec(method, args, opts); + options.currentIterations++; + return exec(method, args, options); } } diff --git a/test/unique.spec.ts b/test/unique.spec.ts index f38bc5c9566..6f26a0c5067 100644 --- a/test/unique.spec.ts +++ b/test/unique.spec.ts @@ -181,4 +181,33 @@ May not be able to generate any more unique values with current settings. Try adjusting maxTime or maxRetries parameters for faker.unique().`) ); }); + + it('should not mutate most of the input option properties', () => { + const method = () => 'options-mutate-test'; + + const startTime = new Date().getTime(); + const maxTime = 49; + const maxRetries = 49; + const currentIterations = 0; + const exclude = []; + const compare = (obj, key) => (obj[key] === undefined ? -1 : 0); + + const options = { + startTime, + maxTime, + maxRetries, + currentIterations, + exclude, + compare, + }; + + faker.unique(method, [], options); + + expect(options.startTime).toBe(startTime); + expect(options.maxTime).toBe(maxTime); + expect(options.maxRetries).toBe(maxRetries); + // `options.currentIterations` is incremented in the `faker.unique` function. + expect(options.exclude).toBe(exclude); + expect(options.compare).toBe(compare); + }); }); From a9f8376556316742743ed6ec1a32f713ad1898d7 Mon Sep 17 00:00:00 2001 From: Shinigami92 Date: Fri, 8 Apr 2022 14:12:44 +0200 Subject: [PATCH 4/5] refactor: use node 14 supported syntax --- src/utils/unique.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/unique.ts b/src/utils/unique.ts index 0ff4cbb3f93..28eed85d796 100644 --- a/src/utils/unique.ts +++ b/src/utils/unique.ts @@ -100,7 +100,7 @@ export function exec RecordKey>( compare = defaultCompare, } = options; let { exclude = GLOBAL_UNIQUE_EXCLUDE } = options; - options.currentIterations ||= 0; + options.currentIterations = options.currentIterations ?? 0; // Support single exclude argument as string if (!Array.isArray(exclude)) { From 7db20632b86bf09f461c4c359b525641eed881b9 Mon Sep 17 00:00:00 2001 From: Shinigami92 Date: Fri, 8 Apr 2022 18:25:02 +0200 Subject: [PATCH 5/5] chore: apply suggestions --- src/unique.ts | 15 +++++---------- src/utils/unique.ts | 16 +++++++++++----- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/unique.ts b/src/unique.ts index 40cd0668006..7577357192e 100644 --- a/src/unique.ts +++ b/src/unique.ts @@ -37,10 +37,10 @@ export class Unique { * @param method The method used to generate the values. * @param args The arguments used to call the method. * @param options The optional options used to configure this method. - * @param options.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. + * @param options.startTime This parameter does nothing. * @param options.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. * @param options.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. - * @param options.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. + * @param options.currentIterations This parameter does nothing. * @param options.exclude The value or values that should be excluded/skipped. Defaults to `[]`. * @param options.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. * @@ -59,18 +59,13 @@ export class Unique { compare?: (obj: Record, key: RecordKey) => 0 | -1; } = {} ): ReturnType { - const { - startTime = new Date().getTime(), - maxTime = this.maxTime, - maxRetries = this.maxRetries, - currentIterations = 0, - } = options; + const { maxTime = this.maxTime, maxRetries = this.maxRetries } = options; return uniqueExec.exec(method, args, { ...options, - startTime, + startTime: new Date().getTime(), maxTime, maxRetries, - currentIterations, + currentIterations: 0, }); } } diff --git a/src/utils/unique.ts b/src/utils/unique.ts index 28eed85d796..4e7fadcecbb 100644 --- a/src/utils/unique.ts +++ b/src/utils/unique.ts @@ -4,8 +4,7 @@ export type RecordKey = string | number | symbol; /** * Global store of unique values. - * Uniqueness for entire faker instance. - * This means that faker should *never* return duplicate values across all API methods when using `Faker.unique` without passing `options.store`. + * This means that faker should *never* return duplicate values across all API methods when using `Faker.unique`. */ const GLOBAL_UNIQUE_STORE: Record = {}; @@ -72,10 +71,10 @@ Try adjusting maxTime or maxRetries parameters for faker.unique().` * @param method The method used to generate the values. * @param args The arguments used to call the method. * @param options The optional options used to configure this method. - * @param options.startTime The time this execution stared. This will be ignored/overwritten. Defaults to `new Date().getTime()`. + * @param options.startTime The time this execution stared. Defaults to `new Date().getTime()`. * @param options.maxTime The time in milliseconds this method may take before throwing an error. Defaults to `50`. * @param options.maxRetries The total number of attempts to try before throwing an error. Defaults to `50`. - * @param options.currentIterations The current attempt. This will be ignored/overwritten. Defaults to `0`. + * @param options.currentIterations The current attempt. Defaults to `0`. * @param options.exclude The value or values that should be excluded/skipped. Defaults to `[]`. * @param options.compare The function used to determine whether a value was already returned. Defaults to check the existence of the key. */ @@ -134,6 +133,13 @@ export function exec RecordKey>( } else { // console.log('conflict', result); options.currentIterations++; - return exec(method, args, options); + return exec(method, args, { + ...options, + startTime, + maxTime, + maxRetries, + compare, + exclude, + }); } }