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: support multiple binding versions #312

Conversation

jonaslagoni
Copy link
Member

@jonaslagoni jonaslagoni commented Feb 2, 2023

Description

This PR changes a few things:

  1. Adds support for ONLY allowing a specific range of binding versions
  2. Adds support for defaulting to the latest version of the bindings
  3. Adds support for explicitly only check the appropriate binding schema when bindingVersion is explicitly defined.
  4. Now only validate a binding if needed instead of all bindings, which could cause issues when reporting back errors.
  5. Add missing message binding for AMQP
  6. Add missing server binding for MQTT5
  7. Add missing operation binding for HTTP
  8. Fix the wrong version for MQTT5 server schema
  9. Fix incorrect regex for pattern properties for Kafka 0.4 and MQTT5

Here is how the binding schema changes work (example with mqtt5 bindings):

    "mqtt5": {
      "properties": {
        "bindingVersion": {
          "enum": ["0.2.0"]
        }
      },

Is added to ensure only a specific range of binding versions (if the property bindingVersion is set) are valid i.e. it gives you an error if you use "bindingVersion": "0.3.0",, while still allowing no values.

      "allOf": [
        {
          "description": "If no bindingVersion specified, use the latest binding",
          "if": {
            "not": {
              "required": [
                "bindingVersion"
              ]
            }
          },
          "then": {
            "$ref": "http://asyncapi.com/bindings/mqtt5/0.2.0/server.json"
          }
        },

The first entry in the allOf is for matching the default version, i.e. when no specific bindingVersion has been sat. If so it defaults to the newest, in this case, version 0.2.0.

        { 
          "if": {
            "required": [ "bindingVersion" ],
            "properties": {
              "bindingVersion": {
                "const": "0.2.0"
              }
            }
          }, 
          "then": {
            "$ref": "http://asyncapi.com/bindings/mqtt5/0.2.0/server.json"
          }
        }

The next entry in the allOf includes the validation for when the user explicitly states a binding version, i.e. if the property bindingVersion has been sat, and is of value 0.2.0 we validate against http://asyncapi.com/bindings/mqtt5/0.2.0/server.json.

@jonaslagoni
Copy link
Member Author

Need to double check it validates correctly.

dalelane
dalelane previously approved these changes Feb 5, 2023
char0n
char0n previously approved these changes Feb 5, 2023
Copy link
Collaborator

@char0n char0n left a comment

Choose a reason for hiding this comment

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

LGTM, nice work 🎉

derberg
derberg previously approved these changes Feb 6, 2023
@jonaslagoni jonaslagoni dismissed stale reviews from derberg, char0n, and dalelane via 0d7cfce February 8, 2023 04:34
@jonaslagoni
Copy link
Member Author

jonaslagoni commented Feb 8, 2023

Had to update with the latest changes, just need a single approval again 🙂

@jonaslagoni jonaslagoni requested review from char0n and dalelane and removed request for char0n and dalelane February 8, 2023 04:43
@jonaslagoni jonaslagoni requested review from derberg, char0n and dalelane and removed request for derberg and char0n February 8, 2023 04:43
@jonaslagoni
Copy link
Member Author

Thanks @char0n!

/rtm

@jonaslagoni
Copy link
Member Author

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 8, 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
Member Author

Lets try again, thanks @dalelane 😄
/rtm

@jonaslagoni
Copy link
Member Author

/dnm

@asyncapi-bot asyncapi-bot merged commit a6c905a into asyncapi:next-major-spec Feb 8, 2023
@jonaslagoni
Copy link
Member Author

Ahh well, wanted to have #321 released first.

Guess it's fine 🙄

@jonaslagoni jonaslagoni deleted the feature/add_latest_binding_support branch February 8, 2023 16:48
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 5.0.0-next-major-spec.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 6.0.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.

5 participants