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

feat: dedupe results by unique fingerprint #856

Merged
merged 9 commits into from
Dec 20, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
46 changes: 37 additions & 9 deletions docs/guides/javascript.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ spectral.registerFormat('oas3', isOpenApiv3);
const {
Spectral,
isJSONSchema,
isJSONSchemaDraft4,
isJSONSchemaDraft4,
isJSONSchemaDraft6,
isJSONSchemaDraft7,
isJSONSchemaDraft2019_09,
isJSONSchemaDraft7,
isJSONSchemaDraft2019_09,
isJSONSchemaLoose,
} = require('@stoplight/spectral');

Expand All @@ -96,7 +96,7 @@ Learn more about predefined formats in the (ruleset documentation)[../getting-st

## Loading Rules

Spectral comes with some rulesets that are very specific to OpenAPI v2/v3, and they can be loaded using `Spectral.loadRuleset()`.
Spectral comes with some rulesets that are very specific to OpenAPI v2/v3, and they can be loaded using `Spectral.loadRuleset()`.

```js
const { Spectral, isOpenApiv2, isOpenApiv3 } = require('@stoplight/spectral');
Expand All @@ -114,7 +114,7 @@ spectral.loadRuleset('spectral:oas')
.then(results => {
console.log('here are the results', results);
});
```
```

The OpenAPI rules are opinionated. There might be some rules that you prefer to change, or disable. We encourage you to create your rules to fit your use case, and we welcome additions to the existing rulesets as well!

Expand Down Expand Up @@ -153,17 +153,20 @@ spectral
.then(result => {
expect(result).toEqual([
expect.objectContaining({
code: 'rule1',
code: 'rule1',
}),
]);
});
```

### Using custom resolver

Spectral lets you provide any custom $ref resolver. By default, http(s) and file protocols are resolved, relatively to the document Spectral lints against.
If you'd like support any additional protocol or adjust the resolution, you are absolutely fine to do it.
In order to achieve that, you need to create a custom json-ref-resolver instance.
Spectral lets you provide any custom $ref resolver. By default, http(s) and file protocols are resolved, relatively to
the document Spectral lints against. If you'd like support any additional protocol or adjust the resolution, you are
absolutely fine to do it. In order to achieve that, you need to create a custom json-ref-resolver instance.

You can find more information about how to create custom resolvers in
the [@stoplight/json-ref-resolver](https://github.com/stoplightio/json-ref-resolver) repository.

```js
const path = require('path');
Expand Down Expand Up @@ -199,3 +202,28 @@ const spectral = new Spectral({ resolver: customFileResolver });
The custom resolver we've just created will resolve all remote file refs relatively to the current working directory.

More on that can be found in the [json-ref-resolver repo](https://github.com/stoplightio/json-ref-resolver).

### Using custom de-duplication strategy

By default, Spectral will de-duplicate results based on the result code and document location. You can customize this
marbemac marked this conversation as resolved.
Show resolved Hide resolved
behavior with the `computeFingerprint` option. For example, here is the default fingerprint implementation:

The final reported results are de-duplicated based on their computed fingerprint.

```ts
const spectral = new Spectral({
computeFingerprint: (rule: IRuleResult, hash) => {
let id = String(rule.code);

if (rule.path && rule.path.length) {
id += JSON.stringify(rule.path);
} else if (rule.range) {
id += JSON.stringify(rule.range);
}

if (rule.source) id += rule.source;

return hash(id);
},
});
```
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"ajv": "^6.10",
"ajv-oai": "^1.1.5",
"better-ajv-errors": "^0.6.7",
"blueimp-md5": "^2.12.0",
"chalk": "^3.0.0",
"eol": "^0.9.1",
"fast-glob": "^3.1.0",
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/linter.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('Linter', () => {
]);
});

describe('evaluate {{value}} in validation messages', () => {
describe('evaluate "value" in validation messages', () => {
test('should print correct values for referenced files', async () => {
spectral = new Spectral({ resolver: httpAndFileResolver });

Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('linter', () => {
});

expect(result).toEqual([
{
expect.objectContaining({
code: 'rule1',
message,
severity: DiagnosticSeverity.Warning,
Expand All @@ -111,7 +111,7 @@ describe('linter', () => {
line: 5,
},
},
},
}),
]);
});

Expand Down
106 changes: 3 additions & 103 deletions src/__tests__/spectral.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ describe('Spectral', () => {
);
});

test('should recognize the source of remote $refs', async () => {
test('should recognize the source of remote $refs, and de-dupe results by fingerprint', async () => {
const s = new Spectral({ resolver: httpAndFileResolver });
const documentUri = path.join(__dirname, './__fixtures__/gh-658/URIError.yaml');

Expand All @@ -214,6 +214,8 @@ describe('Spectral', () => {

const results = await s.run(fs.readFileSync(documentUri, 'utf8'), { resolve: { documentUri } });

expect(results.length).toEqual(3);

return expect(results).toEqual([
expect.objectContaining({
path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'],
Expand Down Expand Up @@ -263,108 +265,6 @@ describe('Spectral', () => {
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 21,
},
start: {
character: 20,
line: 20,
},
},
}),
expect.objectContaining({
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 21,
},
start: {
character: 20,
line: 20,
},
},
}),
expect.objectContaining({
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 21,
},
start: {
character: 20,
line: 20,
},
},
}),
expect.objectContaining({
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),
]);
});
});
33 changes: 0 additions & 33 deletions src/rulesets/oas/__tests__/contact-properties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,39 +33,6 @@ describe('contact-properties', () => {
info: { contact: {} },
});
expect(results).toEqual([
{
code: 'contact-properties',
message: 'Contact object should have `name`, `url` and `email`.',
path: ['info', 'contact'],
range: {
end: {
character: 17,
line: 4,
},
start: {
character: 14,
line: 4,
},
},
severity: 1,
source: undefined,
},
{
code: 'contact-properties',
message: 'Contact object should have `name`, `url` and `email`.',
path: ['info', 'contact'],
range: {
end: {
character: 17,
line: 4,
},
start: {
character: 14,
line: 4,
},
},
severity: DiagnosticSeverity.Warning,
},
{
code: 'contact-properties',
message: 'Contact object should have `name`, `url` and `email`.',
Expand Down
2 changes: 1 addition & 1 deletion src/rulesets/oas/functions/__tests__/oasOpParams.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ describe('oasOpParams', () => {
},
},
});
expect(results.length).toEqual(2);
expect(results.length).toEqual(1);
});

test('Error if multiple in:body', async () => {
Expand Down
30 changes: 23 additions & 7 deletions src/spectral.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
const md5 = require('blueimp-md5');

import { getLocationForJsonPath as getLocationForJsonPathJson, JsonParserResult, safeStringify } from '@stoplight/json';
import { Resolver } from '@stoplight/json-ref-resolver';
import { ICache, IUriParser } from '@stoplight/json-ref-resolver/types';
import { extname, normalize } from '@stoplight/path';
import { Dictionary, IDiagnostic, Optional } from '@stoplight/types';
import { getLocationForJsonPath as getLocationForJsonPathYaml, YamlParserResult } from '@stoplight/yaml';
import { merge } from 'lodash';
import { memoize, merge, uniqBy } from 'lodash';

import { STATIC_ASSETS } from './assets';
import { formatParserDiagnostics, formatResolverErrors } from './error-messages';
Expand All @@ -31,20 +33,24 @@ import {
RunRuleCollection,
} from './types';
import { IRuleset } from './types/ruleset';
import { empty } from './utils';
import { ComputeFingerprintFunc, defaultComputeResultFingerprint, empty } from './utils';

memoize.Cache = WeakMap;

export * from './types';

export class Spectral {
private readonly _resolver: IResolver;
private readonly _parsedRefs: Dictionary<IParsedResult>;
private static readonly _parsedCache = new WeakMap<ICache | IResolver, Dictionary<IParsedResult>>();
public functions: FunctionCollection = { ...defaultFunctions };
public rules: RunRuleCollection = {};

public formats: RegisteredFormats;

private readonly _computeFingerprint: ComputeFingerprintFunc;
private readonly _resolver: IResolver;
private readonly _parsedRefs: Dictionary<IParsedResult>;
private static readonly _parsedCache = new WeakMap<ICache | IResolver, Dictionary<IParsedResult>>();

constructor(opts?: IConstructorOpts) {
this._computeFingerprint = memoize(opts?.computeFingerprint || defaultComputeResultFingerprint);
this._resolver = opts && opts.resolver ? opts.resolver : new Resolver();
this.formats = {};

Expand Down Expand Up @@ -108,9 +114,19 @@ export class Spectral {
...runRules(resolved, this.rules, this.functions),
];

// add fingerprint func to each result, to uniquely identify and group them
const computeFingerprint = this._computeFingerprint;
for (const r of validationResults) {
Object.defineProperty(r, 'fingerprint', {
get() {
return computeFingerprint(r, md5);
},
});
}

return {
resolved: resolved.resolved,
results: validationResults,
results: uniqBy(validationResults, 'fingerprint'),
};
}

Expand Down
2 changes: 2 additions & 0 deletions src/types/spectral.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '@stoplight/types';
import { JSONSchema4, JSONSchema6, JSONSchema7 } from 'json-schema';
import { IFunction, IRule, Rule } from '.';
import { ComputeFingerprintFunc } from '../utils';

export type FunctionCollection = Dictionary<IFunction, string>;
export type RuleCollection = Dictionary<Rule, string>;
Expand All @@ -31,6 +32,7 @@ export type RuleDeclarationCollection = Dictionary<boolean, string>;

export interface IConstructorOpts {
resolver?: IResolver;
computeFingerprint?: ComputeFingerprintFunc;
}

export interface IRunOpts {
Expand Down
17 changes: 17 additions & 0 deletions src/utils/fingerprint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { IRuleResult } from '../types';

export type ComputeFingerprintFunc = (rule: IRuleResult, hash: (val: string) => string) => string;

export const defaultComputeResultFingerprint: ComputeFingerprintFunc = (rule, hash) => {
let id = String(rule.code);

if (rule.path && rule.path.length) {
id += JSON.stringify(rule.path);
} else if (rule.range) {
id += JSON.stringify(rule.range);
}

if (rule.source) id += rule.source;

return hash(id);
};
Loading