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

docs: update Message Example payload type #1082

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

According to JSON Schema headers and payload properties may be of any type
Copy link
Member

@fmvilas fmvilas 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

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Changed my mind after thinking a bit more.

spec/asyncapi.md Outdated
@@ -1453,8 +1453,8 @@ Message Example Object represents an example of a [Message Object](#messageObjec

Field Name | Type | Description
---|:---:|---
<a name="messageExampleObjectHeaders"></a>headers | `Map[string, any]` | The value of this field MUST validate against the [Message Object's headers](#messageObjectHeaders) field.
<a name="messageExampleObjectPayload"></a>payload | `Map[string, any]` | The value of this field MUST validate against the [Message Object's payload](#messageObjectPayload) field.
<a name="messageExampleObjectHeaders"></a>headers | `any` | The value of this field MUST validate against the [Message Object's headers](#messageObjectHeaders) field.
Copy link
Member

Choose a reason for hiding this comment

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

I think the change is justified for payload as it can be an array, for instance. However, I'm not sure in which cases headers could be anything different than a map.

Copy link
Member Author

Choose a reason for hiding this comment

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

@fmvilas Yep, I thought same way, but after this I re-read definitions and it may by of any type

In Message we are defining schema for payload and headers. This two properties are JSON Schema or Multi format Schema

So they can be of any defined and valid JSON Schema type - string, number, object, etc. Or Avro type

This is main reason to define headers and payload in MessageExample as any type and validate them trough it's schemas in Message object

Copy link
Member

Choose a reason for hiding this comment

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

The definition of headers explicitly says:

Schema MUST be a map of key-value pairs.

So it can't be any, right?

Copy link
Member

Choose a reason for hiding this comment

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

headers

IMHO headers should stay as it is. It was always like this, in previous versions too, key/value as this is who headers look like.

So they can be of any defined and valid JSON Schema type - string, number, object, etc. Or Avro type

spec is more specific in case of headers if you compare with payload

Schema MUST be a map of key-value pairs

payload

In case of payload. This is what it was before v3 which makes sense as payload is any

<a name="messageExampleObjectHeaders"></a>headers | `Map[string, any]` | The value of this field MUST validate against the [Message Object's headers](#messageObjectHeaders) field.

<a name="messageExampleObjectPayload"></a>payload | `any` | The value of this field MUST validate against the [Message Object's payload](#messageObjectPayload) field.

in v3 we introduced Multi Format Schema Object, so payload can also be payload.schema and payload.schemaFormat and definition of payload was changed from any to Multi Format Schema Object | Schema Object | Reference Object but yeah, because of Schema Object it is still basically any.

which means message payload example should also be any.

I went through #910 but I see that we were 100% focused there on the message.payload definition and Multi Format Schema Object that the change in Message Example Object Payload type was overlooked.

Copy link
Member Author

Choose a reason for hiding this comment

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

@fmvilas @derberg I have returned Map[string, any] to headers

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, @Pakisan! Approved ✅

Rollback headers to Map[string, any]
Copy link

sonarqubecloud bot commented Jan 9, 2025

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@derberg
Copy link
Member

derberg commented Jan 9, 2025

@Pakisan please adjust the description and PR title to reflect the final scope

@Pakisan Pakisan changed the title docs: update Message Example headers and payload docs: update Message Example payload type Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants