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(rulesets): support 2.1.0, 2.2.0, 2.3.0 AsyncAPI versions #2067

Merged
merged 3 commits into from
Feb 24, 2022

Conversation

magicmatatjahu
Copy link
Contributor

@magicmatatjahu magicmatatjahu commented Feb 19, 2022

Fixes #1759.

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

There're not a breaking changes but are some backward compatible named exports.

Additional context

This is my first contribution to this project (btw. awesome project!), so if I forgot something please let me know. I don't know if I should describe somewhere in the documentation the support for the latest versions of AsyncAPI, if so please let me know where.

Changes:

  • remove JSON Schema for AsyncAPI 2.0.0 from source code - use @asyncapi/specs package where there are always up-to-date JSON Schemas.
  • adding additional formats like aas2_0, aas2_1 etc.
  • adding/updating tests for new formats.
  • adding asyncApi2DocumentSchema rule identical as for OpenAPI to validate the document using the appropriate JSON Schema version - I know about duplicated code so please let me know if I should create something like common.ts file with reusable functions for AsyncAPI and OpenAPI in the rulesets package.

NOTE:

I cannot run all tests locally, because I have several failed tests related to the spectral-functions package like:

  ● Linter service › when loading specification files from web › outputs warnings

    expect(received).resolves.toEqual()

    Received promise rejected instead of resolved
    Rejected to value: [Error: Cannot resolve module '@stoplight/spectral-functions' from paths ['/Users/-/Documents/Github/spectral/packages/cli/src/services/__tests__/__fixtures__', '/Users/-/Documents/Github/spectral/packages/cli/src/services/linter/utils'] from /Users/-/Documents/Github/spectral/packages/cli/src/services/__tests__/__fixtures__/spectral.js]

      299 |       });
      300 |
    > 301 |       return expect(run(`lint http://foo.local/openapi`)).resolves.toEqual([
          |              ^
      302 |         {
      303 |           code: 'info-matches-stoplight',
      304 |           message: 'Info must contain Stoplight',

      at expect (node_modules/expect/build/index.js:178:15)
      at Object.<anonymous> (packages/cli/src/services/__tests__/linter.test.ts:301:14)

Is a known issue, or perhaps it's related to my environment? 🤷🏼

@P0lip
Copy link
Contributor

P0lip commented Feb 21, 2022

Thanks a bunch for the PR. I truly appreciate it.
I'll take a look at it today or tomorrow. Having a peek at it, it looks good to me, but I want to have a more in-depth take on it.

Rejected to value: [Error: Cannot resolve module '@stoplight/spectral-functions' from paths ['/Users/-/Documents/Github/spectral/packages/cli/src/services/tests/fixtures', '/Users/-/Documents/Github/spectral/packages/cli/src/services/linter/utils'] from /Users/-/Documents/Github/spectral/packages/cli/src/services/tests/fixtures/spectral.js]

Hmmm, that is odd. It did work on the CI though, so everything's good.
Perhaps try running yarn --force to see if that changes anything.

@P0lip P0lip added the enhancement New feature or request label Feb 21, 2022
@@ -17,6 +17,13 @@ __metadata:
languageName: node
linkType: hard

"@asyncapi/specs@npm:^2.13.0":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so good to see that schema.asyncapi2.json disappear ❤️

@magicmatatjahu
Copy link
Contributor Author

@P0lip Thanks for quick response!

I'll take a look at it today or tomorrow. Having a peek at it, it looks good to me, but I want to have a more in-depth take on it.

No problem, no rush :) If something will be wrong (like duplicated functions) please comment and I will handle that.

Hmmm, that is odd. It did work on the CI though, so everything's good. Perhaps try running yarn --force to see if that changes anything.

I see that on CI it pass so probably it's env problems.

@muenchhausen
Copy link

How to test that? It wasn't working for me. I tried:

A rules file:

extends: "spectral:asyncapi"

I just copied the sample from https://studio.asyncapi.com/:

asyncapi: '2.3.0'
info:
  title: Streetlights Kafka API
  version: '1.0.0'
  description: |
    The Smartylighting Streetlights API allows you to remotely manage the city lights.
...

Then I get the error:

  1:11    error  asyncapi-schema                 "asyncapi" property must be equal to one of the allowed values: "2.0.0"

A simple solution is to correct the file packages/rulesets/src/asyncapi/schemas/schema.asyncapi2.json and add version 2.x to be valid.

@magicmatatjahu
Copy link
Contributor Author

magicmatatjahu commented Feb 21, 2022

@muenchhausen Hello! I tested in this way (you need to checkout to this branch):

  • go to packages/cli folder

  • create simple spectral config file like in Readme:

    printf '{\n  "extends": ["spectral:oas", "spectral:asyncapi"]\n}\n' > .spectral.json
  • create AsyncAPI file (like sample.yaml)

  • run command (in the packages/cli folder):

    yarn cli lint {AsyncAPI file}

I have these errors for simple AsyncAPI file (based on 2.3.0 version):

/Users/---/spectral/sample.yaml
  1:1  warning  asyncapi-servers                AsyncAPI object must have non-empty "servers" object.
  1:1  warning  asyncapi-tags                   AsyncAPI object must have non-empty "tags" array.
  2:6  warning  asyncapi-info-contact           Info object must have "contact" object.                        info
  2:6  warning  asyncapi-info-license           Info object must have "license" object.                        info
 8:15  warning  asyncapi-operation-description  Operation "description" must be present and non-empty string.  channels.user/signedup.subscribe
 8:15    error  asyncapi-operation-operationId  Operation must have "operationId".                             channels.user/signedup.subscribe

I also tested this https://raw.githubusercontent.com/asyncapi/spec/master/examples/streetlights-kafka.yml and it also works :)

Copy link
Contributor

@P0lip P0lip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks a lot.
I'll merge it tomorrow, there's one change I'll need to introduce to make sure the release goes smoothly, but I'll handle that.

Comment on lines +14 to +55
function shouldIgnoreError(error: ErrorObject): boolean {
return (
// oneOf is a fairly error as we have 2 options to choose from for most of the time.
error.keyword === 'oneOf' ||
// the required $ref is entirely useless, since aas-schema rules operate on resolved content, so there won't be any $refs in the document
(error.keyword === 'required' && error.params.missingProperty === '$ref')
);
}

// this is supposed to cover edge cases we need to cover manually, when it's impossible to detect the most appropriate error, i.e. oneOf consisting of more than 3 members, etc.
// note, more errors can be included if certain messages reported by AJV are not quite meaningful
const ERROR_MAP = [
{
path: /^components\/securitySchemes\/[^/]+$/,
message: 'Invalid security scheme',
},
];

// The function removes irrelevant (aka misleading, confusing, useless, whatever you call it) errors.
// There are a few exceptions, i.e. security components I covered manually,
// yet apart from them we usually deal with a relatively simple scenario that can be literally expressed as: "either proper value of $ref property".
// The $ref part is never going to be interesting for us, because both aas-schema rules operate on resolved content, so we won't have any $refs left.
// As you can see, what we deal here wit is actually not really oneOf anymore - it's always the first member of oneOf we match against.
// That being said, we always strip both oneOf and $ref, since we are always interested in the first error.
export function prepareResults(errors: ErrorObject[]): void {
// Update additionalProperties errors to make them more precise and prevent them from being treated as duplicates
for (const error of errors) {
if (error.keyword === 'additionalProperties') {
error.instancePath = `${error.instancePath}/${String(error.params['additionalProperty'])}`;
}
}

for (let i = 0; i < errors.length; i++) {
const error = errors[i];

if (i + 1 < errors.length && errors[i + 1].instancePath === error.instancePath) {
errors.splice(i + 1, 1);
i--;
} else if (i > 0 && shouldIgnoreError(error) && errors[i - 1].instancePath.startsWith(error.instancePath)) {
errors.splice(i, 1);
i--;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of that stuff could be indeed shared with the OAS ruleset, but we can do it later.
It's not a big deal for now.

@magicmatatjahu
Copy link
Contributor Author

@P0lip Thanks a lot! If there will be problems/bugs, please ping me in this thread :)

We at AsyncAPI are thinking about moving (if you don't mind) all the rules related to AsyncAPI to our organization on github. That way, people could use the up-to-date ruleset for AsyncAPI and you wouldn't have to worry about maintaning AsyncAPI on your side. Here is the issue I created asyncapi/parser-js#477. Of course I don't give a word that we will do such a thing, we have to discuss it at our place first, but if we decided to do such a thing, would Stoplight have something against it?

By the way, I'm currently testing a custom rules for more complex validations like binding validations, extensions etc and I'm delighted. Super project!

@P0lip
Copy link
Contributor

P0lip commented Feb 24, 2022

@magicmatatjahu I'll let you know soon. I reached out to folks internally to see what their take on it is and will try to provide an update as soon as I learn anything.

By the way, I'm currently testing a custom rules for more complex validations like binding validations, extensions etc and I'm delighted. Super project!

Thanks! ❤️ We're really glad you like the project.

@P0lip P0lip merged commit 969e4d6 into stoplightio:develop Feb 24, 2022
stoplight-bot pushed a commit that referenced this pull request Feb 24, 2022
# [@stoplight/spectral-formats-v1.1.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-formats-v1.0.2...@stoplight/spectral-formats-v1.1.0) (2022-02-24)

### Features

* **formats:** support 2.1.0, 2.2.0, 2.3.0 AsyncAPI versions ([#2067](#2067)) ([b0b008d](b0b008d))
stoplight-bot pushed a commit that referenced this pull request Feb 24, 2022
# [@stoplight/spectral-rulesets-v1.5.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-rulesets-v1.4.3...@stoplight/spectral-rulesets-v1.5.0) (2022-02-24)

### Features

* **rulesets:** support 2.1.0, 2.2.0, 2.3.0 AsyncAPI versions ([#2067](#2067)) ([2f1d7bf](2f1d7bf))
@magicmatatjahu magicmatatjahu deleted the support-latest-asyncapi branch February 25, 2022 10:03
jonaslagoni added a commit to jonaslagoni/spectral that referenced this pull request Mar 7, 2022
After stoplightio#2067 I think this change was missed 🙂
@jonaslagoni jonaslagoni mentioned this pull request Mar 7, 2022
4 tasks
@jharmn jharmn added the AsyncAPI Issues related to the AsyncAPI ruleset label Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AsyncAPI Issues related to the AsyncAPI ruleset enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to AsyncAPI Spec 2.1.0 & 2.2.0 & 2.3.0
4 participants