From fd3e70dce4ca86e8429485e440c5604982606c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Fri, 8 May 2020 00:16:38 +0200 Subject: [PATCH] perf: minor tweaks to core functions --- src/functions/__tests__/alphabetical.test.ts | 16 ++++---- .../unreferencedReusableObject.test.ts | 4 +- src/functions/alphabetical.ts | 37 +++++++++---------- src/functions/enumeration.ts | 6 +-- src/functions/length.ts | 8 ++-- src/functions/pattern.ts | 4 +- src/functions/schema.ts | 4 +- src/functions/unreferencedReusableObject.ts | 8 +--- src/functions/xor.ts | 6 +-- src/meta/rule.schema.json | 4 +- src/rulesets/__tests__/validation.test.ts | 6 +-- 11 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/functions/__tests__/alphabetical.test.ts b/src/functions/__tests__/alphabetical.test.ts index e1c996508b..bb9e1d5a19 100644 --- a/src/functions/__tests__/alphabetical.test.ts +++ b/src/functions/__tests__/alphabetical.test.ts @@ -20,12 +20,12 @@ function runAlphabetical(target: any, keyedBy?: string) { } describe('alphabetical', () => { - test('given falsy target should return empty array', () => { - expect(runAlphabetical(false)).toEqual([]); + test('given falsy target should return nothing', () => { + expect(runAlphabetical(false)).toBeUndefined(); }); - test('given single element target should return empty array', () => { - expect(runAlphabetical(['a'])).toEqual([]); + test('given single element target should return nothing', () => { + expect(runAlphabetical(['a'])).toBeUndefined(); }); test('given an object and keys not in order return an error message', () => { @@ -68,7 +68,7 @@ describe('alphabetical', () => { }); test('given a sorted array of strings should NOT return error', () => { - expect(runAlphabetical(['a', 'ab'])).toEqual([]); + expect(runAlphabetical(['a', 'ab'])).toBeUndefined(); }); test('given an unsorted array of numbers should return error', () => { @@ -81,7 +81,7 @@ describe('alphabetical', () => { }); test('given an array of objects should NOT return an error', () => { - expect(runAlphabetical([{ a: '10' }, { b: '1' }])).toEqual([]); + expect(runAlphabetical([{ a: '10' }, { b: '1' }])).toBeUndefined(); }); }); @@ -95,11 +95,11 @@ describe('alphabetical', () => { }); test('given an array of objects with sorted prop values to NOT return an error', () => { - expect(runAlphabetical([{ a: '1' }, { a: '2' }, { a: '2' }], 'a')).toEqual([]); + expect(runAlphabetical([{ a: '1' }, { a: '2' }, { a: '2' }], 'a')).toBeUndefined(); }); test('given an array primitives should not return error', () => { - expect(runAlphabetical([100, 1], 'a')).toEqual([]); + expect(runAlphabetical([100, 1], 'a')).toBeUndefined(); }); }); diff --git a/src/functions/__tests__/unreferencedReusableObject.test.ts b/src/functions/__tests__/unreferencedReusableObject.test.ts index c9892f1278..7d8dfc4428 100644 --- a/src/functions/__tests__/unreferencedReusableObject.test.ts +++ b/src/functions/__tests__/unreferencedReusableObject.test.ts @@ -12,7 +12,7 @@ describe('unreferencedReusableObject', () => { expect(() => runUnreferencedReusableObject({}, 'Nope')).toThrow(); }); - test('given a non object data should return empty array', () => { - expect(runUnreferencedReusableObject('Nope', '#')).toEqual([]); + test('given a non object data should return nothing', () => { + expect(runUnreferencedReusableObject('Nope', '#')).toBeUndefined(); }); }); diff --git a/src/functions/alphabetical.ts b/src/functions/alphabetical.ts index 81a2e412dc..d82230bec9 100644 --- a/src/functions/alphabetical.ts +++ b/src/functions/alphabetical.ts @@ -1,5 +1,5 @@ import { isObject } from 'lodash'; -import { IFunction, IFunctionResult } from '../types'; +import { IFunction } from '../types'; export interface IAlphaRuleOptions { /** if sorting array of objects, which key to use for comparison */ @@ -29,13 +29,9 @@ const getUnsortedItems = (arr: T[], compareFn: (a: T, B: T) => number): null }; export const alphabetical: IFunction = (targetVal, opts, paths, { documentInventory }) => { - const results: IFunctionResult[] = []; + if (!isObject(targetVal)) return; - if (!isObject(targetVal)) { - return results; - } - - let targetArray: any[] | string[] = []; + let targetArray: any[] | string[]; if (Array.isArray(targetVal)) { targetArray = targetVal; @@ -46,14 +42,14 @@ export const alphabetical: IFunction = (targetVal, opt } if (targetArray.length < 2) { - return results; + return; } const keyedBy = opts?.keyedBy; const unsortedItems = getUnsortedItems( targetArray, - keyedBy + keyedBy !== void 0 ? (a, b) => { if (!isObject(a) || !isObject(b)) return 0; @@ -66,16 +62,19 @@ export const alphabetical: IFunction = (targetVal, opt if (unsortedItems != null) { const path = paths.target || paths.given; - - results.push({ - ...(!keyedBy && { path: [...path, Array.isArray(targetVal) ? unsortedItems[0] : targetArray[unsortedItems[0]]] }), - message: keyedBy - ? 'properties are not in alphabetical order' - : `at least 2 properties are not in alphabetical order: "${ - targetArray[unsortedItems[0]] - }" should be placed after "${targetArray[unsortedItems[1]]}"`, - }); + return [ + { + ...(!keyedBy && { + path: [...path, Array.isArray(targetVal) ? unsortedItems[0] : targetArray[unsortedItems[0]]], + }), + message: keyedBy + ? 'properties are not in alphabetical order' + : `at least 2 properties are not in alphabetical order: "${ + targetArray[unsortedItems[0]] + }" should be placed after "${targetArray[unsortedItems[1]]}"`, + }, + ]; } - return results; + return; }; diff --git a/src/functions/enumeration.ts b/src/functions/enumeration.ts index b467f52852..9110597eb7 100644 --- a/src/functions/enumeration.ts +++ b/src/functions/enumeration.ts @@ -5,11 +5,11 @@ export interface IEnumRuleOptions { } export const enumeration: IFunction = (targetVal, opts) => { - const results: IFunctionResult[] = []; + if (targetVal === void 0) return; - const { values } = opts; + const { values } = opts!; - if (!targetVal) return results; + const results: IFunctionResult[] = []; if (!values.includes(targetVal)) { results.push({ diff --git a/src/functions/length.ts b/src/functions/length.ts index 054b84810a..0b096c50af 100644 --- a/src/functions/length.ts +++ b/src/functions/length.ts @@ -6,12 +6,10 @@ export interface ILengthRuleOptions { } export const length: IFunction = (targetVal, opts) => { - const results: IFunctionResult[] = []; + if (targetVal === void 0 || targetVal === null) return; const { min, max } = opts; - if (!targetVal) return results; - let value; const valueType = typeof targetVal; if (valueType === 'object') { @@ -24,7 +22,9 @@ export const length: IFunction = (targetVal, opts) => { value = targetVal.length; } - if (typeof value === 'undefined') return results; + if (typeof value === 'undefined') return; + + const results: IFunctionResult[] = []; if (typeof min !== 'undefined' && value < min) { results.push({ diff --git a/src/functions/pattern.ts b/src/functions/pattern.ts index de69c31972..d8da21fcdd 100644 --- a/src/functions/pattern.ts +++ b/src/functions/pattern.ts @@ -29,9 +29,9 @@ function test(value: string, regex: RegExp | string) { } export const pattern: IFunction = (targetVal, opts) => { - const results: IFunctionResult[] = []; + if (typeof targetVal !== 'string') return; - if (!targetVal || typeof targetVal !== 'string') return results; + const results: IFunctionResult[] = []; const { match, notMatch } = opts; diff --git a/src/functions/schema.ts b/src/functions/schema.ts index e4864f575e..3916ef365e 100644 --- a/src/functions/schema.ts +++ b/src/functions/schema.ts @@ -144,8 +144,6 @@ const cleanAJVErrorMessage = (message: string, path: Optional, suggestio }; export const schema: ISchemaFunction = (targetVal, opts, paths, { rule }) => { - const results: IFunctionResult[] = []; - const path = paths.target || paths.given; if (targetVal === void 0) { @@ -157,6 +155,8 @@ export const schema: ISchemaFunction = (targetVal, opts, paths, { rule }) => { ]; } + const results: IFunctionResult[] = []; + // we already access a resolved object in src/functions/schema-path.ts const { schema: schemaObj } = opts; diff --git a/src/functions/unreferencedReusableObject.ts b/src/functions/unreferencedReusableObject.ts index a8c1f7a46a..b168ebad51 100644 --- a/src/functions/unreferencedReusableObject.ts +++ b/src/functions/unreferencedReusableObject.ts @@ -8,13 +8,7 @@ export const unreferencedReusableObject: IFunction<{ reusableObjectsLocation: st _paths, otherValues, ) => { - if (!isObject(data)) return []; - - if (!opts.reusableObjectsLocation.startsWith('#')) { - throw new Error( - "Function option 'reusableObjectsLocation' doesn't look like containing a valid local json pointer.", - ); - } + if (!isObject(data)) return; const normalizedSource = otherValues.documentInventory.source ?? ''; diff --git a/src/functions/xor.ts b/src/functions/xor.ts index b7928b8cd2..ac4d52763f 100644 --- a/src/functions/xor.ts +++ b/src/functions/xor.ts @@ -6,11 +6,11 @@ export interface IXorRuleOptions { } export const xor: IFunction = (targetVal, opts) => { - const results: IFunctionResult[] = []; - const { properties } = opts; - if (!targetVal || typeof targetVal !== 'object' || properties.length !== 2) return results; + if (targetVal === null || typeof targetVal !== 'object' || properties.length !== 2) return; + + const results: IFunctionResult[] = []; const intersection = Object.keys(targetVal).filter(value => -1 !== properties.indexOf(value)); if (intersection.length !== 1) { diff --git a/src/meta/rule.schema.json b/src/meta/rule.schema.json index de6a1537b5..5afe8355ec 100644 --- a/src/meta/rule.schema.json +++ b/src/meta/rule.schema.json @@ -256,7 +256,9 @@ "additionalProperties": false, "properties": { "reusableObjectsLocation": { - "type": "string" + "type": "string", + "minLength": 1, + "pattern": "^#" } }, "required": [ diff --git a/src/rulesets/__tests__/validation.test.ts b/src/rulesets/__tests__/validation.test.ts index 3e3a051ebe..1bf0316ccc 100644 --- a/src/rulesets/__tests__/validation.test.ts +++ b/src/rulesets/__tests__/validation.test.ts @@ -860,7 +860,7 @@ describe('Ruleset Validation', () => { then: { function: 'unreferencedReusableObject', functionOptions: { - reusableObjectsLocation: 'foo', + reusableObjectsLocation: '#', }, }, }, @@ -888,7 +888,7 @@ describe('Ruleset Validation', () => { ).toThrow(ValidationError); }); - it('complains about invalid options', () => { + it.each([2, '', 'd'])('complains about invalid options %s', reusableObjectsLocation => { expect( assertValidRuleset.bind(null, { rules: { @@ -897,7 +897,7 @@ describe('Ruleset Validation', () => { then: { function: 'unreferencedReusableObject', functionOptions: { - reusableObjectsLocation: 2, + reusableObjectsLocation, }, }, },