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: add simplified channel parameter #816

Conversation

jonaslagoni
Copy link
Sponsor Member

Description
This PR adds the support for the simplified channel parameters by utilizing a custom implementation of Schema that just looks in the parent parameter object for the values instead of under a .schema property that no longer exist for v3.

Related issue(s)
Fixes #815

@smoya
Copy link
Member

smoya commented Jul 25, 2023

Would you consider throwing warning log lines for those methods that users are unexpected to use?

@jonaslagoni
Copy link
Sponsor Member Author

jonaslagoni commented Jul 25, 2023

@smoya I considered it, but I could not justify it with any reason. Cause it's not like the functions are deprecated or anything 🤷 They just wont return anything if paired with v3 documents, but will with v2.

So not sure who the logging is for? Owners of tools that use it? No cause it's not until the users provide a document those things are caught.
For the users of the tool? What exactly do they need that information for? 🤔

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Jul 25, 2023

@jonaslagoni I don't know if it was discussed but you can reuse v3.SchemaModel but passing as json object the serialized schema object from new parameter shape, I mean map fields like type etc:

{
  type: "string"
  description: parameter.description
}

etc. - all data will be handled in ChannelParameter model, inside schema() method.

Creating new model for this is not a good idea.

@jonaslagoni
Copy link
Sponsor Member Author

@magicmatatjahu you right, thats way cleaner, let me refactor it.

@magicmatatjahu
Copy link
Member

Something like:

  schema(): SchemaInterface | undefined {
    if (!this.hasSchema()) return undefined;
    return this.createModel(Schema, {
      type: 'string',
      description: this._json.description,
      examples: this._json.examples,
      // etc...
    }, { pointer: `${this._meta.pointer}` });
  }

@jonaslagoni
Copy link
Sponsor Member Author

Done 👍

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.

Only one comment, rest is good :)

src/models/v3/channel-parameter.ts Outdated Show resolved Hide resolved
jonaslagoni and others added 2 commits July 25, 2023 15:14
Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>
Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>
@jonaslagoni
Copy link
Sponsor Member Author

@magicmatatjahu do you have a bit of OCD by any chance? 😄

@sonarcloud
Copy link

sonarcloud bot commented Jul 25, 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
No Duplication information No Duplication information

@jonaslagoni
Copy link
Sponsor Member Author

/rtm

@jonaslagoni jonaslagoni merged commit c9f56b1 into asyncapi:next-major-spec Jul 26, 2023
11 checks passed
@jonaslagoni jonaslagoni deleted the add_simplified_channel_parameter branch July 26, 2023 10:12
@asyncapi-bot
Copy link
Contributor

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

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.2.0-next-major-spec.1 🎉

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.

None yet

4 participants