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: parse message payload of any type other than map of String to Object #208

Conversation

yogeshnikam671
Copy link

Currently the asyncAPI parser expects the example payload to be an object and hence if it is an array, it does not parse the message associated with such array message.

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.

@yogeshnikam671 yogeshnikam671 force-pushed the message_of_array_type_with_an_example_parse_issue_fix branch from ade6db2 to fa7c8e3 Compare December 27, 2024 09:59
…o fix the failure where the spec with an array payload was not getting parsed properly
@yogeshnikam671 yogeshnikam671 force-pushed the message_of_array_type_with_an_example_parse_issue_fix branch from fa7c8e3 to 3dfe458 Compare December 27, 2024 10:12
@Pakisan
Copy link
Member

Pakisan commented Dec 27, 2024

@yogeshnikam671 hi!

Let's remember for what MessageExample was introduced. We can use it to show example of message with headers, payload

That's why payload and headers have Map<String, Object> type, to allow users to attach structured examples, like JSON

But here is a clash between MessageExample JSON Schema and MessageExample definition in our docs

It's main reason, why it's take a while to review and analyze proposed change

Right now, I'm figuring out which type is valid and how it can be changed in lib

Will be awesome, if you provide as much of different examples, as it possible

@harikrishnan83
Copy link

@Pakisan Thanks a lot for the clarification. Primary we foresee below schema types as payloads.

  • Plain old objects
  • Array of objects (we often have messages where payload is a list)
  • Simple String (which this is not something we encourage, sometimes existing systems do have such cases)

The priority / blocker at the moment is the "Array of objects" use case for which @yogeshnikam671 has added sample spec in this PR and we have also made code changes to achieve this capability.

Kindly review.

cc @yogeshnikam671

@Pakisan
Copy link
Member

Pakisan commented Dec 27, 2024

@harikrishnan83 I'm working on PR to our JSON Schemas and documentation repositories

Yep, looks like payload and headers can be of any type, so 'Object' is a valid type for this fields in class

And they must be validated through schema in Message class

Once I open PR, I'll attach them to this PR

@yogeshnikam671 yogeshnikam671 changed the title Add a failing test to demonstrate the failure when the spec contains an example where the payload is an array instead of an object Fix: Parse message payload of any type other than map of String to Object Dec 28, 2024
@yogeshnikam671 yogeshnikam671 changed the title Fix: Parse message payload of any type other than map of String to Object fix: Parse message payload of any type other than map of String to Object Dec 28, 2024
@yogeshnikam671 yogeshnikam671 changed the title fix: Parse message payload of any type other than map of String to Object fix: parse message payload of any type other than map of String to Object Dec 28, 2024
@Pakisan Pakisan changed the base branch from master to release/1.0.0-RC3 January 9, 2025 12:04
@Pakisan Pakisan self-assigned this Jan 9, 2025
Copy link
Member

@Pakisan Pakisan left a comment

Choose a reason for hiding this comment

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

lgtm

@Pakisan Pakisan merged commit 0a4242f into asyncapi:release/1.0.0-RC3 Jan 9, 2025
14 of 17 checks passed
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.

4 participants