Skip to content

Commit

Permalink
perf(pattern): improve general perf (#1300)
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip authored Aug 4, 2020
1 parent 72d30fe commit c9d2714
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 81 deletions.
35 changes: 9 additions & 26 deletions src/functions/__tests__/pattern.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { pattern } from '../pattern';

function runPattern(targetVal: any, options?: any) {
return pattern(
return pattern.call(
{
cache: new Map(),
},
targetVal,
options,
{
Expand All @@ -15,37 +18,17 @@ function runPattern(targetVal: any, options?: any) {
}

describe('pattern', () => {
describe('given a true regex', () => {
test('should return empty array when given value matches the given match regex', () => {
expect(runPattern('abc', { match: /[abc]+/ })).toEqual([]);
});

test('should return a message when given value does not match the given notMatch regex', () => {
expect(runPattern('def', { match: /[abc]+/ })).toEqual([{ message: "must match the pattern '/[abc]+/'" }]);
});

test('should return empty array when given value does not match the given notMatch regex', () => {
expect(runPattern('dEf', { notMatch: /[abc]+/i })).toEqual([]);
});

test('should return a message when given value does match the given notMatch regex', () => {
expect(runPattern('aBc', { notMatch: /[abc]+/i })).toEqual([
{ message: "must not match the pattern '/[abc]+/i'" },
]);
});
});

describe('given a string regex', () => {
test('should return empty array when given value matches the given match string regex without slashes', () => {
expect(runPattern('abc', { match: '[abc]+' })).toEqual([]);
expect(runPattern('abc', { match: '[abc]+' })).toBeUndefined();
});

test('should return empty array when given value matches the given match string regex with slashes', () => {
expect(runPattern('abc', { match: '/[abc]+/' })).toEqual([]);
expect(runPattern('abc', { match: '/[abc]+/' })).toBeUndefined();
});

test('should return empty array when given value matches the given match string regex with slashes and modifier', () => {
expect(runPattern('aBc', { match: '/[abc]+/im' })).toEqual([]);
expect(runPattern('aBc', { match: '/[abc]+/im' })).toBeUndefined();
});

test('should throw an exception when given string regex contains invalid flags', () => {
Expand All @@ -55,13 +38,13 @@ describe('pattern', () => {
});

test('should return empty array when given value does not match the given notMatch string regex with slashes and modifier', () => {
expect(runPattern('def', { notMatch: '/[abc]+/i' })).toEqual([]);
expect(runPattern('def', { notMatch: '/[abc]+/i' })).toBeUndefined();
});
});

describe('given match and notMatch regexes', () => {
test('should return empty array when given value match the given match and does not match the given notMatch regexes', () => {
expect(runPattern('def', { match: /[def]+/, notMatch: /[abc]+/ })).toEqual([]);
expect(runPattern('def', { match: /[def]+/, notMatch: /[abc]+/ })).toBeUndefined();
});
});
});
76 changes: 48 additions & 28 deletions src/functions/pattern.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { IFunction, IFunctionResult } from '../types';
import type { IFunction, IFunctionContext, IFunctionResult } from '../types';
import type { Optional } from '@stoplight/types';

export interface IRulePatternOptions {
/** regex that target must match */
Expand All @@ -8,46 +9,65 @@ export interface IRulePatternOptions {
notMatch?: string;
}

function test(value: string, regex: RegExp | string) {
let re;
if (typeof regex === 'string') {
// regex in a string like {"match": "/[a-b]+/im"} or {"match": "[a-b]+"} in a json ruleset
// the available flags are "gimsuy" as described here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp
const splitRegex = /^\/(.+)\/([a-z]*)$/.exec(regex);
if (splitRegex) {
// with slashes like /[a-b]+/ and possibly with flags like /[a-b]+/im
re = new RegExp(splitRegex[1], splitRegex[2]);
} else {
// without slashes like [a-b]+
re = new RegExp(regex);
}
// regex in a string like {"match": "/[a-b]+/im"} or {"match": "[a-b]+"} in a json ruleset
// the available flags are "gimsuy" as described here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp
const REGEXP_PATTERN = /^\/(.+)\/([a-z]*)$/;

function getFromCache(cache: Map<string, RegExp>, pattern: string): RegExp {
const existingPattern = cache.get(pattern);
if (existingPattern !== void 0) {
return existingPattern;
}

const newPattern = createRegex(pattern);
cache.set(pattern, newPattern);
return newPattern;
}

function createRegex(pattern: string): RegExp {
const splitRegex = REGEXP_PATTERN.exec(pattern);
if (splitRegex !== null) {
// with slashes like /[a-b]+/ and possibly with flags like /[a-b]+/im
return new RegExp(splitRegex[1], splitRegex[2]);
} else {
// true regex
re = new RegExp(regex);
// without slashes like [a-b]+
return new RegExp(pattern);
}
return re.test(value);
}

export const pattern: IFunction<IRulePatternOptions> = (targetVal, opts) => {
export const pattern: IFunction<IRulePatternOptions> = function (this: IFunctionContext, targetVal, opts) {
if (typeof targetVal !== 'string') return;

const results: IFunctionResult[] = [];
let results: Optional<IFunctionResult[]>;

const { match, notMatch } = opts;
const cache = this.cache as Map<string, RegExp>;

if (match !== void 0) {
const pattern = getFromCache(cache, match);

if (match) {
if (test(targetVal, match) !== true) {
results.push({
message: `must match the pattern '${match}'`,
});
if (!pattern.test(targetVal)) {
results = [
{
message: `must match the pattern '${match}'`,
},
];
}
}

if (notMatch) {
if (test(targetVal, notMatch) === true) {
results.push({
if (notMatch !== void 0) {
const pattern = getFromCache(cache, notMatch);

if (pattern.test(targetVal)) {
const result = {
message: `must not match the pattern '${notMatch}'`,
});
};

if (results === void 0) {
results = [result];
} else {
results.push(result);
}
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/rulesets/__tests__/evaluators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,18 @@ describe('Code evaluators', () => {
const fn2 = setFunctionContext(context, jest.fn().mockReturnThis());
expect(fn()).not.toBe(fn2());
});

it('copies enumerable properties', () => {
const context = { a: true };
const prop = Object.freeze({});
const fn = function () {
return;
};

fn.foo = prop;

const boundFn = setFunctionContext(context, fn);
expect(boundFn.foo).toBe(prop);
});
});
});
6 changes: 5 additions & 1 deletion src/rulesets/evaluators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,12 @@ export const compileExportedFunction = ({ code, name, source, schema, inject }:
};

export function setFunctionContext(context: unknown, fn: Function) {
return Function.prototype.bind.call(
const boundFn = Function.prototype.bind.call(
fn,
Object.freeze(Object.defineProperties({}, Object.getOwnPropertyDescriptors(context))),
);

Object.assign(boundFn, fn);

return boundFn;
}
54 changes: 28 additions & 26 deletions src/spectral.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ export class Spectral {
private readonly _resolver: IResolver;
private readonly agent: Agent | undefined;

public functions: FunctionCollection & CoreFunctions = { ...coreFunctions };
public rules: RunRuleCollection = {};
public exceptions: RulesetExceptionCollection = {};
public formats: RegisteredFormats;
public readonly functions: FunctionCollection & CoreFunctions = { ...coreFunctions };
public readonly rules: RunRuleCollection = {};
public readonly exceptions: RulesetExceptionCollection = {};
public readonly formats: RegisteredFormats;

protected readonly runtime: RunnerRuntime;

Expand All @@ -70,6 +70,8 @@ export class Spectral {

this.formats = {};
this.runtime = new RunnerRuntime();

this.setFunctions(coreFunctions);
}

public static registerStaticAssets(assets: Dictionary<string, string>) {
Expand Down Expand Up @@ -141,7 +143,16 @@ export class Spectral {
public setFunctions(functions: FunctionCollection): void {
empty(this.functions);

Object.assign(this.functions, { ...coreFunctions, ...functions });
const mergedFunctions = { ...coreFunctions, ...functions };

for (const key of Object.keys(mergedFunctions)) {
const context: IFunctionContext = {
functions: this.functions,
cache: new Map(),
};

this.functions[key] = setFunctionContext(context, mergedFunctions[key]);
}
}

public setRules(rules: RuleCollection): void {
Expand Down Expand Up @@ -198,29 +209,20 @@ export class Spectral {
return fns;
}

const context: IFunctionContext = {
functions: this.functions,
cache: new Map(),
};

fns[key] = setFunctionContext(
context,
compileExportedFunction({
code,
name,
source,
schema,
inject: {
fetch: request,
spectral: this.runtime.spawn(),
},
}),
);
fns[key] = compileExportedFunction({
code,
name,
source,
schema,
inject: {
fetch: request,
spectral: this.runtime.spawn(),
},
});

return fns;
},
{
...coreFunctions,
},
{},
),
);

Expand Down

0 comments on commit c9d2714

Please sign in to comment.