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: examples of messages should not follow json schema #434

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

derberg
Copy link
Member

@derberg derberg commented Oct 10, 2023

#370 introduced bug in Message Object JSON schema

examples headers and payload should not follow json schema spec

just like in previous versions these fields need to be flexible as payload or headers are no necessarily json, could be avro for example

noticed with https://github.com/asyncapi/spec/blob/next-major-spec/examples/websocket-gemini.yml
Screenshot 2023-10-10 at 17 52 12

I checked the change locally and works like a charm

@sonarcloud
Copy link

sonarcloud bot commented Oct 10, 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

@smoya
Copy link
Member

smoya commented Oct 10, 2023

There is something I should be missing because when I read the following in the spec:

Google Chrome_TZUvXJc2@2x
  • For headers field in message example object:

    The value of this field MUST validate against the Message Object's headers field.

  • For payload field in message example object:

    The value of this field MUST validate against the Message Object's payload field.

With that statement, I understand that it should stay as it is, with the anySchema, allowing then any schema (JSON Schema, Avro, etc).
Not sure what I'm missing!

@derberg
Copy link
Member Author

derberg commented Oct 11, 2023

but the spec says map of string and any, so no, it should not stay as it is. As not we expect that in example of the payload, people should provide schema, while it should be an example of payload that is valid with with the schema provided in payload

now, if my payload is

type: string

my example must be

type: string

which makes no sense as I want my example to be

'my example'

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.

You are right, examples should not have those references 😄

@smoya
Copy link
Member

smoya commented Oct 11, 2023

while it should be an example of payload that is valid with with the schema provided in payload

Yeah, that totally makes sense from the usability point of view. I completely misunderstood those statements by thinking they were the same in terms of validation than the object they represent.
If I'm the only one, nothing else to add there, otherwise we could clarify it a bit more on the spec so no one else understands it wrongly as I did.

Copy link
Member

@smoya smoya left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀🌔 Thanks for this catch and thanks for clarifying something might sound obvious 🙌

@derberg
Copy link
Member Author

derberg commented Oct 11, 2023

Thanks @jonaslagoni and @smoya for quick review 🍺

/rtm

@derberg derberg changed the title fix: examples of messages should not follow json schems fix: examples of messages should not follow json schema Oct 11, 2023
@asyncapi-bot asyncapi-bot merged commit f80e22e into asyncapi:next-major-spec Oct 11, 2023
16 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 6.0.0-next-major-spec.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.

4 participants