channel.servers() should return a list of all the servers by default

[fmvilas]

Describe the bug

By default, channel.servers() returns an empty array. When a servers property is not present in a channel definition, the meaning is that this channel is present in all servers described in the servers section of the document.

Expected behavior

This line should return an array with all the server names in the document.

[jonaslagoni]

Well, there are two views on this.

As it currently stand with the parser, all it does (and nothing else) is give you the exact information that is available for that property. This is why it only gives you the information about whether someone defined a server reference in the channel or not. This is also why contentType does not return defaultContentType from the root object - https://github.com/asyncapi/parser-js/blob/08f425cb034c1a1d29f33399fcf78e16924cb28f/lib/models/message-traitable.js#L61

If we go with the other approach, and being knowledable about other properties, and try to interpret what your intent is, you won't be able to only get the channel servers. i.e. where you have a use-case that requires you to not lookup the root servers as well.

Might need two functions for this? 🤔

[fmvilas]

I don't think it's the same. channel.servers() doesn't give you exactly what is on the AsyncAPI document. If you don't specify any servers property at channel level, it gives you an empty array instead of undefined. So there you are already interpreting the intent. I just think the interpretation is incorrect.

On the other hand, contentType() returns exactly what's found in the AsyncAPI document. That's different.

[derberg]

Logically your point makes a lot of sense. You want parser to give you servers that channel is available at.

But, are there other parts of the parser that work this way. Thing is that the way you work with the parser atm, I would be confused to see all the servers in returned array. I would first check if channel hasServers and in case of false read all servers from servers root.

[fmvilas]

Cool, time for a rewrite of the parser it seems 😄 Good to take this into account for the intent-driven one.

[smoya]

Cool, time for a rewrite of the parser it seems 😄 Good to take this into account for the intent-driven one.

I was about to say this. From an intent point of view, I think it as "Just give me the servers where the channel is present". So I agree with @fmvilas suggestion here.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

[smoya]

@fmvilas this is already happening in the new development of the Parser-JS planned for 3.0.0 https://github.com/asyncapi/parser-js/blob/next-major/src/models/v2/channel.ts#L41-L50

Do you think it makes sense to close this issue and considered done? Or rather wait until we release it?

cc @magicmatatjahu

[fmvilas]

Cool! I'd just leave this open until the new version is released. It can still change so it's not yet done.

[magicmatatjahu]

Issue is resolved in v2 ParserJS - now in https://github.com/asyncapi/parser-js/tree/next-major branch