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

fix: upgrade @asyncapi/specs #423

Merged

Conversation

jonaslagoni
Copy link
Member

@jonaslagoni jonaslagoni commented Dec 9, 2021

Description
This PR upgrades the @asyncapi/specs library to the new v3.

One important note is, that with the new @asyncapi/specs package, the schema for validating JSON Schema's has to be removed in validation libraries. Why?

Long story short, because the AsyncAPI JSON Schema documents are written with a specific meta schema. That meta schema, is already loaded by Ajv, and when you try to load the bundled AsyncAPI JSON Schema, it now contains that same meta schema, because we use it for payload validation. And Ajv does NOT like to load duplicate schemas and simply throw an error and cannot validate documents.

Related issue(s)
Blocked by asyncapi/spec-json-schemas#128

@sonarcloud
Copy link

sonarcloud bot commented Dec 10, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jonaslagoni jonaslagoni marked this pull request as ready for review March 30, 2022 17:52
lib/asyncapiSchemaFormatParser.js Outdated Show resolved Hide resolved
* @returns {Object} valid JSON Schema document describing format of AsyncAPI-valid schema for message payload
*/
function preparePayloadSchema(asyncapiSchema) {
function preparePayloadSchema(asyncapiSchema, version) {
const payloadSchema = `http://asyncapi.com/definitions/${version}/schema.json`;
Copy link
Member

Choose a reason for hiding this comment

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

why can't we grab it from @asyncapi/specs instead of remote?

sorry if that was answered somewhere already. Also, I do not remember really much from times when I created asyncapiSchemaFormatParser.js

just dereferencing something in the parser from remote is going to slow down parsing, or?

Copy link
Member Author

Choose a reason for hiding this comment

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

We are using the local one from @asyncapi/specs 🙂

It is only what the reference is called. Ajv automatically matches it with the schema inside definitions 🙂

Copy link
Member

Choose a reason for hiding this comment

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

do we need http://asyncapi.com/definitions/ published before we merge?
tbh I'm not 100% sure how these $id work...to complicated for me 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Not at all, the $ids when bundled together like so, do not need external access to the schemas 🙂 Because validation tools must look for the reference within before reaching externally.

We don't need to expose them at all if everyone uses the bundled schemas, but say you want to use this one: https://github.com/asyncapi/spec-json-schemas/blob/master/definitions/2.4.0/asyncapi.json, it would need the schemas exposed on http://asyncapi.com/definitions/ as the schemas are not located within.

Does that make sense?

@sonarcloud
Copy link

sonarcloud bot commented Apr 6, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jonaslagoni
Copy link
Member Author

PR is ready

Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
@sonarcloud
Copy link

sonarcloud bot commented Jun 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@derberg
Copy link
Member

derberg commented Aug 18, 2022

@jonaslagoni sorry for being so late 😓 please solve conflicts and I can accept

@jonaslagoni
Copy link
Member Author

@jonaslagoni sorry for being so late 😓 please solve conflicts and I can accept

Done 👍

@sonarcloud
Copy link

sonarcloud bot commented Sep 13, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jonaslagoni jonaslagoni merged commit a8d6283 into asyncapi:master Sep 13, 2022
@jonaslagoni jonaslagoni deleted the feature/update_spec_json_schema branch September 13, 2022 19:47
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.15.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.0.0-next-major.18 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants