Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(pattern): improve general perf #1300

Merged
merged 2 commits into from
Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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