Ruleset: AsyncAPI by nulltoken · Pull Request #974 · stoplightio/spectral

nulltoken · 2020-02-19T06:52:50Z

Fixes #965
Fixes #789

Checklist

Tests added / updated
Docs added / updated
Log issue to add a rule to ensure all variables used in server url are defined
Log issue to add a rule to ensure all defined server variables are used
Log issue to add a rule to ensure all parameters used in channel path are defined
Log issue to add a rule to ensure all defined channel parameters are used
Log issue to add a rule to identify orphaned components (messageTraits, ...)
Migrate asyncApi2PayloadValidation to a custom function of the ruleset

Does this PR introduce a breaking change?

Yes
No

If indicated yes above, please describe the breaking change(s).

Remove this quote before creating the PR.

Screenshots

If applicable, add screenshots or gifs to help demonstrate the changes. If not applicable, remove this screenshots section before creating the PR.

Additional context

Add any other context about the pull request here. Remove this section if there is no additional context.

nulltoken · 2020-03-11T22:42:47Z

@philsturgeon Few questions:

In the original document, some rules were commented out. I didn't consider them. Should I try and re-integrate them or were they left out on purpose?
Some rules are identical between oas and aas. Should this stay as is or should we try to somehow reuse a common set of shared rules between the two rulesets?

@P0lip @philsturgeon Next steps:

I believe I've now got the overall skeleton in place (@P0lip Would you have some time for a first review pass?). I'm now going to create tests for each of those new rules (at least one passing and one failing per rule).
Then some documentation for the new ruleset (@philsturgeon I might need your help on that)

philsturgeon · 2020-03-12T08:11:08Z

@nulltoken the commented out ones were things I didn't delete or make work. I deleted some things which were definitely not worth trying to port over, and left in some things I hadn't researched. If those rules are relevant lets get them working, if they are not we can delete them.

should we try to somehow reuse a common set of shared rules between the two rulesets?

A little bit of duplication is ok for now, I'd rather get it working than spend a bunch of time on DRYing it prematurely.

philsturgeon · 2020-03-12T08:11:30Z

@nulltoken the commented out ones were things I didn't delete or make work. I deleted some things which were definitely not worth trying to port over, and left in some things I hadn't researched. If those rules are relevant lets get them working, if they are not we can delete them.

should we try to somehow reuse a common set of shared rules between the two rulesets?

A little bit of duplication is ok for now, I'd rather get it working than spend a bunch of time on DRYing it prematurely.

nulltoken · 2020-03-21T13:43:09Z

Dear Diary,

At last! Back in the world of CI green. Pretty relieved!

Suffered a lot in Karma land and weird execution of jest BeforeAll() test hooks. Had to switch over to BeforeEach(). A bit of a waste as we keep on rebuilding something which is supposed to be immutable, but, at least... it works.

philsturgeon · 2020-04-17T13:20:35Z

+
+### asyncapi2-channel-no-query-nor-fragment
+
+Query parameters and fragments shouldn't be used in channel names. Instead, use bindings to define them.


Can we get some examples?

@philsturgeon I'm afraid I won't be able to come up with a good/bad example that would make sense in an AsyncAPI context.

@fmvilas Could you please help me here? What's the known bad practices regarding query and fragments and how should they be converted to bindings?

philsturgeon · 2020-04-17T21:25:09Z

+Spectral currently only supports payload validation against the default AsyncAPI schema.
+
+This rule will notify you when a payload is decorated with a `schemaFormat` property. Spectral won't be able to validate it and potential issues in this payload will go undetected by the `asyncapi-payload-*` rules.
+


Let's go with something like....

AsyncAPI can support various schemaFormat values, with the default being one of the following:

application/vnd.aai.asyncapi;version=2.0.0 application/vnd.aai.asyncapi+json;version=2.0.0 application/vnd.aai.asyncapi+yaml;version=2.0.0

At this point no other schemaFormat's are supported by Spectral, so if you use them this rule will emit an info message and skip validating the payload.

Other formats such as OpenAPI Schema Object, JSON Schema Draft 07 and Avro will be added in various upcoming versions.

@philsturgeon Awesome! I've slightly reworded your proposal (as we don't support setting the schemaFormat at all).

philsturgeon

@nulltoken with those few changes done this is all thumbs from me! When you and @P0lip are happy merge away and let's get a beta out for people to play with! 🥳

P0lip · 2020-04-17T21:36:15Z

+import { IFunction, IFunctionContext } from '../types';
+import { IFunctionResult } from '../types/function';
+
+import * as asyncApi2Schema from '../rulesets/asyncapi/schemas/schema.asyncapi2.json';


This could be provided in a ruleset as a function option?

nulltoken · 2020-04-18T10:50:26Z

@nulltoken with those few changes done this is all thumbs from me! When you and @P0lip are happy merge away and let's get a beta out for people to play with! 🥳

@philsturgeon Thanks a lot for the feedback.

@P0lip What's your thinking Sir?

P0lip · 2020-04-18T14:10:38Z

+
+import { IFunction, IFunctionResult } from '../types';
+import { getLintTargets } from '../utils';
+import { schema } from './schema';


We don't need to import it.

P0lip · 2020-04-18T14:11:03Z

+  return validator;
+};
+
+export const asyncApi2PayloadValidation: IFunction<IPayloadValidationOptions> = (


Suggested change

export const asyncApi2PayloadValidation: IFunction<IPayloadValidationOptions> = (

export function asyncApi2PayloadValidation = (

this: IFunctionContext,

// ..rest of args

P0lip · 2020-04-18T14:11:18Z

+  paths,
+  otherValues,
+) => {
+  const ajvValidationFn = buildAsyncApi2SchemaObjectValidator(schema);


Suggested change

const ajvValidationFn = buildAsyncApi2SchemaObjectValidator(schema);

const ajvValidationFn = buildAsyncApi2SchemaObjectValidator(this.functions.schema);

P0lip · 2020-04-18T14:11:41Z

+  const results: IFunctionResult[] = [];
+
+  for (const relevantItem of relevantItems) {
+    const path = rootPath.slice(0);


How about?

Suggested change

const path = rootPath.slice(0);

const path = [...rootPath, ...relevantItem.path]

P0lip · 2020-04-18T14:11:48Z

+    const path = rootPath.slice(0);
+    Array.prototype.push.apply(path, relevantItem.path);
+
+    const result = schema(


Suggested change

const result = schema(

const result = this.functions.schema(

P0lip · 2020-04-18T14:31:08Z

+) => {
+  const ajvValidationFn = buildAsyncApi2SchemaObjectValidator(schema);
+
+  const relevantItems = getLintTargets(targetVal, opts.field);


Just for the clarity, we've discussed this one on slack - this should not be needed, since runner already handles it for us.

P0lip · 2020-04-18T14:31:48Z

Dang, hit the button too late. 😆

P0lip

One last comment.
Looks good from my point of view.

P0lip · 2020-04-18T15:01:28Z

-            // happens
-          }
-          return opts;
+          return { result: parse(opts.result) };


If I am not mistaken, we should return original options.
I suggest not touch it unless it's needed?

nulltoken force-pushed the ntk/asyncapi branch from ccac9a2 to 3ddd35a Compare February 20, 2020 07:07

nulltoken mentioned this pull request Feb 20, 2020

fix: Teach isOpenApivX() to be a bit more strict #976

Closed

4 tasks

nulltoken force-pushed the ntk/asyncapi branch from 83cdc4a to d8006df Compare February 25, 2020 07:36

nulltoken commented Feb 25, 2020

View reviewed changes

Comment thread scripts/generate-assets.js Outdated

nulltoken force-pushed the ntk/asyncapi branch from e685572 to d80da0a Compare March 1, 2020 07:12

nulltoken mentioned this pull request Mar 1, 2020

JSON Schema Validation struggles with circular references #789

Closed

nulltoken force-pushed the ntk/asyncapi branch 6 times, most recently from c351478 to 40234f9 Compare March 11, 2020 22:33

philsturgeon reviewed Mar 12, 2020

View reviewed changes

Comment thread rollup.config.js Outdated

nulltoken mentioned this pull request Mar 12, 2020

Little repro case of asyncapi schema related issues APIDevTools/json-schema-ref-parser#158

Closed

nulltoken force-pushed the ntk/asyncapi branch 3 times, most recently from a1e8cba to 6429157 Compare March 13, 2020 05:53

This was referenced Mar 13, 2020

fix #915: Ignore examples with circular schemas #923

Closed

chore: prepare assets generation for future changes #1013

Merged

nulltoken force-pushed the ntk/asyncapi branch 5 times, most recently from 05e9e50 to be2be40 Compare March 21, 2020 13:33

nulltoken mentioned this pull request Mar 22, 2020

chore(karma): Provide actionable developer feedback #1024

Merged

4 tasks

nulltoken force-pushed the ntk/asyncapi branch from be2be40 to a463568 Compare March 22, 2020 11:01

nulltoken requested a review from P0lip April 17, 2020 13:24

philsturgeon reviewed Apr 17, 2020

View reviewed changes

nulltoken mentioned this pull request Apr 17, 2020

chore(ruleset): use human friendly severity levels #1099

Closed

4 tasks

philsturgeon reviewed Apr 17, 2020

View reviewed changes

Comment thread docs/reference/asyncapi-rules.md Outdated

philsturgeon reviewed Apr 17, 2020

View reviewed changes

Comment thread docs/reference/asyncapi-rules.md Outdated

philsturgeon reviewed Apr 17, 2020

View reviewed changes

P0lip reviewed Apr 17, 2020

View reviewed changes

nulltoken force-pushed the ntk/asyncapi branch 2 times, most recently from 40cb464 to ab947a0 Compare April 18, 2020 10:34

nulltoken requested review from P0lip and philsturgeon April 18, 2020 10:47

P0lip reviewed Apr 18, 2020

View reviewed changes

nulltoken force-pushed the ntk/asyncapi branch from ab947a0 to f3f8399 Compare April 18, 2020 14:56

P0lip previously approved these changes Apr 18, 2020

View reviewed changes

feat(ruleset): add asyncapi2 ruleset

4dd3b46

nulltoken dismissed P0lip’s stale review via 4dd3b46 April 18, 2020 15:11

nulltoken force-pushed the ntk/asyncapi branch from f3f8399 to 4dd3b46 Compare April 18, 2020 15:11

P0lip approved these changes Apr 18, 2020

View reviewed changes

P0lip merged commit 6b45165 into develop Apr 18, 2020

P0lip deleted the ntk/asyncapi branch April 18, 2020 15:19

P0lip assigned nulltoken Apr 18, 2020

P0lip added the enhancement New feature or request label Apr 18, 2020

This was referenced Apr 18, 2020

[AsyncAPI] Enhance parameters and variables linting #1102

Closed

[AsyncAPI] Identify more orphaned components besides schemas #1103

Open

[WIP] feat(asyncapi): make asyncapi-payload-* rules support asyncapi schemaFormats #1120

Closed

nulltoken mentioned this pull request May 18, 2020

chore(schema): swallow ajv missing reference #1147

Merged

4 tasks


		### asyncapi2-channel-no-query-nor-fragment

		Query parameters and fragments shouldn't be used in channel names. Instead, use bindings to define them.

		Spectral currently only supports payload validation against the default AsyncAPI schema.

		This rule will notify you when a payload is decorated with a `schemaFormat` property. Spectral won't be able to validate it and potential issues in this payload will go undetected by the `asyncapi-payload-*` rules.

-export const asyncApi2PayloadValidation: IFunction<IPayloadValidationOptions> = (
+export function asyncApi2PayloadValidation = (
+this: IFunctionContext,
+// ..rest of args

	const ajvValidationFn = buildAsyncApi2SchemaObjectValidator(schema);
	const ajvValidationFn = buildAsyncApi2SchemaObjectValidator(this.functions.schema);

	const path = rootPath.slice(0);
	const path = [...rootPath, ...relevantItem.path]

Uh oh!

Conversation

nulltoken commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nulltoken commented Mar 11, 2020

Uh oh!

philsturgeon commented Mar 12, 2020

Uh oh!

Uh oh!

philsturgeon commented Mar 12, 2020

Uh oh!

nulltoken commented Mar 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philsturgeon Apr 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philsturgeon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nulltoken commented Apr 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

P0lip commented Apr 18, 2020

Uh oh!

P0lip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nulltoken commented Feb 19, 2020 •

edited

Loading

nulltoken commented Mar 21, 2020 •

edited

Loading

philsturgeon Apr 17, 2020 •

edited

Loading