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: incorrect rendering of security schemas #995

Merged
merged 6 commits into from
Apr 25, 2024

Conversation

RobinTail
Copy link
Contributor

@RobinTail RobinTail commented Apr 22, 2024

Fixes #993
Incorrect rendering of security schemas in AsyncAPI 3.0, by reference and inline.
See asyncapi/studio#1066

Changes proposed in this pull request:

  • The security property supplied to Security component seems to be enough to fix all issues:
    • Correctly pick security schema by reference
    • Supporting schemas written inline
  • I also added support for multiple schemas (both by reference and inline).

Perhaps I didn't understand something, but without all those complications it seem to work well on all cases and the tests are passing. Anyway, consider it a proposal, feel free to make or request changes.

Cases 1 and 2 Case 3 Also: multiple schemas
2024-04-22_21-52-22 2024-04-22_22-04-09 2024-04-22_22-26-09

Related issue(s)

asyncapi/studio#1066

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@RobinTail RobinTail changed the title Issue 993 security refs Fix incorrect rendering of security schemas Apr 22, 2024
@RobinTail RobinTail changed the title Fix incorrect rendering of security schemas fix: incorrect rendering of security schemas Apr 22, 2024
@RobinTail RobinTail marked this pull request as ready for review April 22, 2024 20:41
magicmatatjahu
magicmatatjahu previously approved these changes Apr 24, 2024
Copy link
Member

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

LGTM! :)

Copy link
Member

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

cc @derberg Do you wanna add something?

@derberg
Copy link
Member

derberg commented Apr 24, 2024

@RobinTail due to some refactor from #996 you need to solve merge conflicts. Sorry about that.

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@RobinTail
Copy link
Contributor Author

RobinTail commented Apr 24, 2024

@derberg && @magicmatatjahu ,
conflicts resolved, please review again

@derberg
Copy link
Member

derberg commented Apr 25, 2024

Thanks @RobinTail

/rtm

@asyncapi-bot asyncapi-bot merged commit 3be2456 into asyncapi:master Apr 25, 2024
10 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.4.10 🎉

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.

Incorrect rendering of security schemas: by reference and inline
4 participants