-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
[CI] Lint and validate JSON Schema docs #436
Comments
Yes, that would be lovely. Since spectral has some build it rules for AsyncAPI, maybe they have them for JSON schema, and it would make it much easier to use. |
@smoya if we put it up under bounty, would you be available as maintainer to onboard and review with higher priority than other PRs (because of bounty constraints)? |
Sure! Let's do it 🙌 |
@derberg i think removing the spectral rules from the scope of this issue will help ensure the accomplishment for the bounty program. Wdyt? |
I would like to give this issue a try. Although I don't have much knowledge about linting and JSON Schemas and how these things work, so it may take some time to learn about it. Also, I'm having some experience with ci/cd as I've been working on this PR. So if this approach falls under the bounty program's timeline, I'm happy to get started with it. 😄 Also, any reference/docs in your opinion as experienced developers to help me get started with this issue? 👀 Thank you! |
Bounty Issue's End of Life (EOL): 2024-02-29 23:59:59 UTC-12:00 |
As per the After carefully evaluating each individual case, I determine @AnimeshKumar923 becomes the participant for this issue in the Bounty Program 2023 Q4. Again, it is hard decision because you @princerajpoot20 are also a very potential candidate for this and someone I would also trust to accomplish this. Thanks for your understanding! Congrats @AnimeshKumar923 🎉 |
@smoya Thank you very much for giving me this opportunity! 🙇 I'm much eager to get started with a new challenge and take it to completion. I'll give the best from my side to the resolution of this issue. 🚀 |
What will be the timeline for this issue? 👀 @aeworxet |
@smoya please make the assignment action for @AnimeshKumar923 so I can determine and post the timeline. |
Issue assigned |
Bounty Issue's Timeline
Please note that the dates given represent deadlines, not specific dates, so if the goal is reached sooner, it's better. |
@smoya Thank you for the assignment. I'll start working on it right away. |
The approach that I've understood is that we can use this Spectral GH Action to lint JSON files.
We have the following code example on how can we implement the GH Action 👇
If we have a custom rule-set, we can provide that in the last Is my approach correct? 👀 |
Based on the following part found in the description of this issue:
Ideally we should be able to run such a validation through Also, I would tell you to discard Spectral, at least by now, since you will need to deal with rulesets, something we don't want to do atm since we do not have any rule defined. We just want to lint the JSON Schemas ensuring they are proper JSON Schema files and don't contain typos and/or errors.
In practice, by validating the files are JSON Schema files you are already validating they are proper JSON files, so you just need to focus on the second. |
Okay, I see...
Got it. Won't proceed forward with the Spectral approach.
Okay, I'll take AJV an proceed forward with it.
Okay, understood the difference. Thank you for your detailed response 😄 |
I tried installing AJV and ajv-cli on my local system and tinkered around using it. I followed your recommendations and read about AJV from the docs link you provided. I assume we will validate the incoming schemas using the existing ones present in the I tried validating it with same it says that There was some different versions of drafts mentioned here What version are we using here at the the I'm new to these topics and learning these so I will require a bit more guidance on how these works. Therefore, I hope you'll guide me through it, I may ask a lot of questions. Thank you! 😅 |
The The JSON Schema doc. This will be the one AJV expects in the {
"type": "object",
"properties": {
"name": {
"type": "string"
}
}
} The JSON doc. This will be the one AJV expects in the {
"name": "Sergio"
} However, since we want to validate the JSON Schema documents themselves, we do not have data to test against.
The reason for the missing schema for draft-04 is that ajv doesn't support draft-04 by default anymore, instead you need to install an extra package, as mentioned in https://github.com/ajv-validator/ajv#ajv-json-schema-validator.
This is a known issue for us in fact. The point is that AJV includes by default several JSON Schema drafts, including the 07. Since our JSON Schema files under In order to do all of this, I would say we would need to create our own script (in ts/javascript) that executes the
Depends on the version of the spec. Mainly Draft-07, but 1.x.x.json files use the Draft-04. But afaik, you can validate with Draft-07 as well since those meta schemas are included into our final schema files. |
Okay, now it's more clear to me that we only have to validate the documents itself and not against any data input. Got it!
So this can be solved by integrating the
Yes, we can follow this approach. It's getting a bit complex for me as I'm new to all these technologies but I'll manage somehow along the way... 😁
So, I guess using the
I assume it will be solved if we validate using the method you mentioned... 👉 |
Hey @smoya 👋 I did some more research and I have following updates... I tried tinkering and developing the following script:
I'm still learning javascript at the moment and at an early stage right now, so the majority of the above code I've generated through referencing docs and using ChatGPT in the process. Referenced this example from parser-js docs for the below code portion 👇
This is the output 👇
Additional info:
|
Let me clarify concepts and hopefully help you to get more context:
Now, getting back to the technical side of this issue, this is some more context you might need to know as well:
|
After my previous comment trying to clarify some concepts, I'm gonna answer some of your previous questions. Most of them should be answered by my previous explanation, but if not, please do not hesitate to come back again.
No. We can't use the Parser-JS because that parser only parses AsyncAPI documents. Examples of AsyncAPI documents can be found here.
You are heading in the right direction. I believe you are able to make it 🙌 . I'm here with you.
I'm not purely convinced about that. I think we should compile all with Draft-07 except for
If you open any of our JSON Schema documents under As AJV has already those metaschemas preloaded, it will collide with that one. The hacky solution for this? To remove our own definition from the document prior to send the schema to validate (compile in our case). |
Thank you for your in-depth explanations. I'll be asking more questions later on as I try to understand and figure out the problem and it's corresponding solution. Thank you for having patience with me 😅 🙇
Understood!
Thank you for believing in me. Sometimes it gets overwhelming while learning but it's good to be able to help and contribute here because of people like you supporting. 😀
I used the search button and found out that files from
Okay, I see... 👀
Yes, I think that's why this error is show
So, we have to somehow incorporate this method into the script that will be written so that it will remove the duplicates? |
Makes sense 👍
That's correct!
Exactly! |
Okay, I think I'm going in the right direction. |
UPDATETried developing the following script while getting familiarized with TypeScript and used the function that you mentioned above: /*
* We can run this script using ts-node which is installed .nvm/versions/node/v20.9.0/bin/ts-node
*
* The tsconfig.json file is required to run this script in TypeScript itself.
*
* This script can be directly executed using the following command by going into scripts folder and then execute --> ```ts-node validate-schemas.ts```
*/
import Ajv, { ErrorObject } from 'ajv';
import specs from '@asyncapi/specs';
type AsyncAPIVersions = keyof typeof specs.schemas;
/************ Code from Asyncapi-parser-js/src/ruleset/functions/documentStructure.ts *************/
function getCopyOfSchema(version: AsyncAPIVersions): Record<string, unknown> {
return JSON.parse(JSON.stringify(specs.schemas[version])) as Record<string, unknown>;
}
const serializedSchemas = new Map<AsyncAPIVersions, Record<string, unknown>>();
function getSerializedSchema(version: AsyncAPIVersions): Record<string, unknown> {
const schema = serializedSchemas.get(version);
if (schema) {
return schema;
}
// Copy to not operate on the original json schema - between imports (in different modules) we operate on this same schema.
const copied = getCopyOfSchema(version) as { definitions: Record<string, unknown> };
// Remove the meta schemas because they are already present within Ajv, and it's not possible to add duplicated schemas.
delete copied.definitions['http://json-schema.org/draft-07/schema'];
delete copied.definitions['http://json-schema.org/draft-04/schema'];
serializedSchemas.set(version, copied);
return copied;
}
/***************************************************************************************************/
const asyncApiVersionToUse: AsyncAPIVersions = '2.0.0'; // Replace with the actual version
// Get the serialized schema for the specified AsyncAPI version
const serializedSchema = getSerializedSchema(asyncApiVersionToUse);
// Create an instance of Ajv
const ajv: Ajv = new Ajv();
// Compile the schema
const validate = ajv.compile(serializedSchema);
// Check if the schema is valid
const isSchemaValid: boolean | PromiseLike<boolean> | undefined = validate(serializedSchema);
if (isSchemaValid) {
console.log('JSON Schema is valid!');
} else {
const validationErrors: ErrorObject[] | null | undefined = validate.errors;
console.error('JSON Schema is not valid:', validationErrors);
} Tried executing this script and getting the this error:
I think it's referring to this line in the I don't know how to fix this. Can you please help me out? |
@smoya Bringing your kind attention to this issue |
Hi @AnimeshKumar923! Sorry for the delay. I've reviewed your script and these are the list of issues I found:
I would say the code you have prior to the AJV instantiation should be removed and maybe replaced with the logic that loads the files and, in a loop, call the ajv.compile. |
@AnimeshKumar923 I added a simple mermaid flow chart in the description of this issue. I tried to simplify the CI flow that surrounds this script. |
Thank you for your hard-work! 🙏 Will do. |
For the record and for any other future mentee who reads this, @AnimeshKumar923 and I kept chatting via Slack DMs and videocalls. Sometimes synchronous communication works better when things are blocked. :) |
Sometimes I prefer Slack because it makes the work and discussions a bit more organized in my opinion. 😅
It's working superbly I would say 🤩 |
Please provide an update to the PR. |
Bounty Issue Completed 🎉Please go to the AsyncAPI's OpenCollective page and submit an invoice for |
So well deserved! 🚀 |
Grateful! 🙇 |
🎉 This issue has been resolved in version 6.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@asyncapi/bounty_team |
Reason/Context
In order to follow an standard, but also ensuring any change won't break the JSON Schema documents, I suggest we add a new lint + validate action in our GH CI workflows that run on each PR and block the PR if JSON Schema files are not valid or following our requirements.
Description
Consider adding a new script to the package.json scripts section, that lints and validate JSON Schema files. Something like
npm run lint
or similar. Also could be included innpm test
call. In that way, developers could run that command before pushing code but also during the execution of CI tests.Run the following CI tasks on the JSON Schema files on each PR. It doesn't mean all of the following should run in different actions since most of them could be done just with one tool like Spectral.
[ ] Add some rules (Spectral?) for validating things like checking all definitions come with theirNot included for the bounty programexamples
anddescription
fields. Any others are welcome as well.Detailed context
parser-js
, for instance) to validate AsyncAPI documents./schemas
dir./definitions.
Each of those small schemas usually refers to the validation related to just one object from the AsyncAPI spec, I.e., Message Object, Channel Object, etc./schemas
but rather modify a particular (sometimes several of them) file under/definitions
. That's done this way to help developers focus on what they want to change and not deal with a big file containing the whole JSON Schema doc for the entire specification.Technical details that concern this issue:
Run linter
is not about lining JSON Schemas but linting Javascript code. So, it has nothing to do with the work we want to do here./definitions
into the bundled big ones located in/schemas
./schemas
), meaning our JSON Schema linting script should run after the generation of assets (the actual last step).To illustrate the details above, here is a simple flow chart. Note that the script we should create is represented in the
Validate schemas from /schemas
step.The text was updated successfully, but these errors were encountered: