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

refactor: match Spectral formats based on schemas found in @asyncapi/specs pkg #822

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

smoya
Copy link
Member

@smoya smoya commented Aug 1, 2023

Description

This PR removes the need for creating new Spectral ruleset formats matching new AsyncAPI versions.
This translates to the removal of the requirement of making changes every time we add a new version into @asyncapi/specs package. It might be some exceptions in the case we, for example, add a new major spec version, where rules won't apply, and manual changes (and expected I would say) will need to be done, but will be close to as simple as including that new version in the allowed formats in ruleset.

This PR enables future support for v3 of the spec in a very easy way (will come in a new PR).

Related issue(s)

#817

cc @jonaslagoni @magicmatatjahu @Amzani @fmvilas

@smoya smoya changed the title feat: match Spectral formats based on schemas found in @asyncapi/specs pkg refactor: match Spectral formats based on schemas found in @asyncapi/specs pkg Aug 1, 2023
jonaslagoni
jonaslagoni previously approved these changes Aug 1, 2023
Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

Nothing major, just general comments you can apply or not, up to you 😄

Otherwise it looks good to me 👍

}
}

export const AsyncAPIFormats = new Formats(Object.entries(schemas).reverse().map(([version]) => [version, createFormat(version)])); // reverse is used for giving newer versions a higher priority when matching
Copy link
Member

Choose a reason for hiding this comment

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

Not the biggest fan of using reverse instead of a sorting function, cause what if the spec library changes the order? It implies that the sorted list is a feature and the knowledge of it 😄

if (componentType === 'securitySchemes' && aas2(targetVal, null)) {
// if component type is `securitySchemes` we skip the validation
// security schemes in >=2.x.x are referenced by keys, not by object ref - for this case we have a separate `asyncapi2-unused-securityScheme` rule
if (componentType === 'securitySchemes') {
Copy link
Member

Choose a reason for hiding this comment

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

For v3, we need to remember to let securitySchemes through, and not use asyncapi2-unused-securityScheme.

@@ -334,7 +333,7 @@ export const v2RecommendedRuleset = {
'asyncapi2-message-messageId': {
description: 'Message should have a "messageId" field defined.',
recommended: true,
formats: aas2AllFormats.slice(4), // from 2.4.0
formats: AsyncAPIFormats.filterByMajorVersions(['2']).excludeByVersions(['2.0.0', '2.1.0', '2.2.0', '2.3.0']).formats(), // message.messageId is available starting from v2.4.
Copy link
Member

Choose a reason for hiding this comment

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

AsyncAPIFormats.filterByMajorVersions(['2']).fromVersion(['2.4.0']).formats(), would make it easier to read and use 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah but then I will need to deal with sorting and filtering making it a bit more complicated and I don't want to go in that direction yet if this solves the issue.

@sonarcloud
Copy link

sonarcloud bot commented Aug 1, 2023

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
1.2% 1.2% Duplication

@jonaslagoni
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 0382c65 into asyncapi:master Aug 1, 2023
11 checks passed
@smoya smoya deleted the feat/spectralFormatMatching branch August 2, 2023 09:59
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.1.0-next-major-spec.4 🎉

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.

3 participants