docs: intent API

[jonaslagoni]

Description
This PR suggests the new intent API for the parser.

It is implicit that all properties will have their own intents in the end but are not added for simplicity.

Looking for feedback

• Do you have any extra intents we should add (initially)?
  • Don't expect all intents to be included in the first iteration, you should "easily" see where other intents could be added. But remember this is just an initial version that shows the general approach.
• Do you like the setup/dislike the setup? Why? What can be improved?
• Any intents that are completely off?
• Can you come up with better naming formats?
• Other things?

Next steps

• To validate that this API can handle the emulated breaking changes.
• Once validated, start mocking the methods to integrate the API immediately with the current tooling and validate the UX.

Related issue(s)

• Define Parser(s) API shape-up-process#90

[jonaslagoni]

Emulated breaking changes

We needed to test the intents based on different changes from asyncapi/shape-up-process#93

3.0.0-fake

Assumptions:

• Only one operation type (publish/subscribe) per channel

Intents:

• (GOOD) Channel.messagesSubscribes('<role>') : Message[]
• (GOOD) Channel.messagesPublishes('<role>') : Message[]
• (GOOD) Channel.summaryForPublishing('<role>') : string
• (GOOD) Channel.summaryForSubscribing('<role>') : string

3.0.0-fake2

Because this makes it possible to define multiple schema formats per message, it is no longer a 1:1 (message:schemaFormat) but a 1:* and breaks the return values

• (QUESTIONABLE) Message.schemaFormat() : string (this intent is not part of the initial suggestion, but added to support the breaking change example)
  • This could return multiple schema formats

3.0.0-fake3

Because this makes it possible to define multiple operations per channel, it is no longer 1:1 (channel:operation) but a 1:* and breaks the return values

• (GOOD) Channel.messagesSubscribes('<role>') : Message[]
• (GOOD) Channel.messagesPublishes('<role>') : Message[]
• (QUESTIONABLE) Channel.summaryForPublishing('<role>') : string
  • This could return multiple summaries.
• (QUESTIONABLE) Channel.summaryForSubscribing('<role>') : string
  • This could return multiple summaries.
• (QUESTIONABLE) Channel.operationIdForPublishing('<role>') : string
  • This could return multiple ids.
• (QUESTIONABLE) Channel.operationIdForSubscribing('<role>') : string
  • This could return multiple ids.

3.0.0-fake4

Because this makes it possible to define multiple messages under a channel, and those messages can contain operations we no longer have a 1:1 (channel:operation) but a 1:* and breaks the return values

• (GOOD) Channel.messagesSubscribes('<role>') : Message[]
• (GOOD) Channel.messagesPublishes('<role>') : Message[]
• (QUESTIONABLE) Channel.summaryForPublishing('<role>') : string
  • This could return multiple summaries.
• (QUESTIONABLE) Channel.summaryForSubscribing('<role>') : string
  • This could return multiple summaries.
• (QUESTIONABLE) Channel.operationIdForPublishing('<role>') : string
  • This could return multiple ids.
• (QUESTIONABLE) Channel.operationIdForSubscribing('<role>') : string
  • This could return multiple ids.

Conclusion

As you can see we have a problem with the cardinality of return values in general..

The only apparent solution is to return an array of values, always, to be safe. But this is affecting the toolings using the parser making it more difficult...

[smoya]

Taking this as an example of what would happen in case we return an array of strings:

(QUESTIONABLE) Channel.summaryForPublishing('') : string
This could return multiple summaries.
(QUESTIONABLE) Channel.operationIdForPublishing('') : string
This could return multiple ids.

The issue about returning an array of strings here is that the user won't be able to link the data it gets from each of those methods ever.
For example, it won't be able to answer: "To what operation this summary belongs to?"

I think that, if we end up having such a change on our spec, then the Operation term will be stronger than what it is now and will become part of the entities (models in https://github.com/asyncapi/parser-js/pull/263/files#diff-b916c5a0628af4b4bdb634e7612d93408a30119addea9aab98fe5ac7c28adeafR13 list) we care of.
That will mean we could introduce a new method for getting the Operations of a message or channel instead.

TL;DR. At this point, considering having a good balance between API usability and resiliency, I would consider those breaking changes as unavoidable.

[magicmatatjahu]

Awesome works guys! Sorry for late response. I read docs in PR and issue description/comments and I have problem with:

It is implicit that all properties will have their own intents in the end but are not added for simplicity.

As I understand intents like tagsForPublishing('<role>') will be also available? If yes, in my opinion it can create a lot of boilerplate in code maintenance and API usage. I think that publishFor(<role>) and few others (mainly related to operations/channels) is great, but making API for every intents like summaryFor..., tagsFor..., externalDocsFor... are unnecessary. I very like the current API, because knowing what the specification looks like, I know where to look for it. I also know that other people may not like this solution and I know that in some situation you must do some for ... loop to collect some necessary information but these are single situations (at least for me).

So: Intents like publishFor(<role>) is awesome, but then we can leave current api like publishFor(<role>).message(), publishFor(<role>).bindings() etc.

Currently I render spec in React component and I see that the additional API proposed in PR for the React component itself is redundant, but as I wrote, to someone/for some template it may be useful, but guys :P please try to create as fewer intents as possible that are created due to the way you look at the spec rather than the data the spec has (byt this I like publishFor(<role>) but other not), because then you will create an extra 200 methods that can quickly become deprecated in next version of spec. I hope that my comment will be helpful for you

[derberg]

I don't want to be a party pooper here but overall, it looks that to use the parser I will not only need to know the spec by also some additional vocabulary (roles) that is not in the spec.

this is my first impression, which is by default pushing me away.

The initial intent of new API was to make parser API not reflect the spec but the intents of the user. I think it directly means there will be many methods. So I guess my point of view is totally different too what @magicmatatjahu shared. It is better to have "200" methods here rather than have "200" helpers spread in different tools. As long as the names are consistent and you thanks to it you can easily navigate through them with auto-suggestions from IDE, this is ok imho.

I think that next to the proposal we need to see some code samples how it is used, as I did with this comment:

current state that clearly shows I need to get all the channels and and to "if" statements to get what I need

{% for channelName, channel in asyncapi.channels() -%}
{%- if channel.hasSubscribe() %}
  {%- if channel.subscribe().summary() %}
   some logic
  {%- endif %}
{%- endif %}
{%- endfor %}

and what we could have to make it much easier to consume the spec through the parser (a bit modified comparing to my initial comment from the issue. Extremely useful helper like getOperationsIds that by default returns all, but if you provide the name of the operation, it limits the return array.

{% for operationId in asyncapi.getOperationsIds('subscribe') %}
  {% if asyncapi.hasOperationSummary(operationId) %}
     {{ asyncapi.getOperationSummary(operationId) }}
  {% endif %}
{% endfor %}

doesn't matter where operationId will be. And even if we change subscribe to something else, easy for the user is just to change the name, code is left without changes.

To conclude - parser should be resilient to spec changes as much as possible + speed the work on templates, make it nicer, less complicated.

But yeah, would be great to see your proposal used on some existing code from templates, to see what will change, and if for good or bad

[jonaslagoni]

Thanks for the feedback @derberg, @magicmatatjahu!

.... operations/channels) is great, but making API for every intents like summaryFor..., tagsFor..., externalDocsFor... are unnecessary.

In my opinion, we have to @magicmatatjahu, otherwise what is the point of an intent API if it requires the parsers a major version update when the spec updates? If we only go halfway then there is no point in doing it, it would create more confusion than good.

I don't want to be a party pooper here but overall, it looks that to use the parser I will not only need to know the spec by also some additional vocabulary (roles) that is not in the spec.

@derberg correct, but IMO you would need to anyway since the parser and the spec no longer follow 1:1. So to understand the parser you would need to read the documentation anyway.

To conclude - parser should be resilient to spec changes as much as possible + speed the work on templates, make it nicer, less complicated.
But yeah, would be great to see your proposal used on some existing code from templates, to see what will change, and if for good or bad

@derberg I am working on actually integrating this suggested API so we can see the changes it would have.

[fmvilas]

I think it's looking great and you're making great progress here. It's never easy to design an API and you're scratching your head a lot 😄 That's really good 👏

My advice is that you forget about AsyncAPI for a moment and design the API with generic concepts that may or may not map directly to the AsyncAPI spec. For instance, think of someone creating an EDA. The main concepts that come to the top of my head are:

• My application
• Other applications
• The broker
• The topics on the broker
• The messages on those topics
• The messages my app publish to these topics
• The messages my app subscribe to on these topics
• The messages other apps can publish to these topics
• The messages other apps can subscribe to on these topics

Now try to map these to AsyncAPI:

• My application => AsyncAPI file
• Other applications => Other apps consuming my AsyncAPI file
• The broker => Server
• The topics on the broker => Channels (without publish/subscribe info)
• The messages on those topics => Channel.*.message (where * is either publish/subscribe but we hide it at this level)
• The messages my app publish to these topics => Channel.subscribe.message
• The messages my app subscribe to on these topics => Channel.publish.message
• The messages other apps can publish to these topics => Channel.publish.message
• The messages other apps can subscribe to on these topics => Channel.subscribe.message

Hope it helps! Keep up the great work! 💪

[fmvilas]

Suggested change

	- **(HELP NEEDED)** `AsyncAPIDocument.channelsPublishes('<role>')` : Channel[] (outcome of comment(s): [800282407](https://github.com/asyncapi/shape-up-process/issues/84#issuecomment-800282407), [800935961](https://github.com/asyncapi/shape-up-process/issues/84#issuecomment-800935961))
	- **(HELP NEEDED)** `AsyncAPIDocument.clientPublishableChannels()` : Channel[] (outcome of comment(s): [800282407](https://github.com/asyncapi/shape-up-process/issues/84#issuecomment-800282407), [800935961](https://github.com/asyncapi/shape-up-process/issues/84#issuecomment-800935961))
	- **(HELP NEEDED)** `AsyncAPIDocument.publishingChannels()` : Channel[] (outcome of comment(s): [800282407](https://github.com/asyncapi/shape-up-process/issues/84#issuecomment-800282407), [800935961](https://github.com/asyncapi/shape-up-process/issues/84#issuecomment-800935961))

[jonaslagoni]

Same as above comment.

[jonaslagoni]

@fmvilas thanks for the feedback!

My advice is that you forget about AsyncAPI for a moment and design the API with generic concepts that may or may not map directly to the AsyncAPI spec. For instance, think of someone creating an EDA. The main concepts that come to the top of my head are:

That is basically also what we did, but I guess the description does not clarify this well enough 🤔 The idea was to separate the entities completely from the spec and simply think about what concepts make sense and which relationships they have with each other.

I'll see if I can change the description to state this separation better then it is.

[jonaslagoni]

After playing around a bit with the API these are some of the thoughts I had.

Improvements to the API

• Server is built around the notion of being global for all channels. I think we should change this to be associated with Operations, Channels, and Messages instead of being connected only to the root of the document.
• Message.contentType() should be added and automatically use the global default content type if none is specified for the message.
• A way to check what an operation is, such as Operation.isClientSubscribing.

Breaking changes

It seems like we solved the problems of the breaking changes we had in the last suggestion 👏

3.0.0-fake

Since we already return an array of operations, this will not break the parser API.

3.0.0-fake2

Does not break the parser API since we can add support function such as getAllSchemaFormats to the standard getSchemaFormat so we deprecate the old function as time goes on.

3.0.0-fake3

Still works since we have a bidirectional flow of channels, messages, and operations already.

3.0.0-fake4

Still works since we have a bidirectional flow of channels, messages, and operations already.

[jonaslagoni]

* `Server` is built around the notion of being global for all channels. I think we should change this to be associated with `Operations`, `Channels`, and `Messages` instead of being connected only to the root of the document.

@smoya and I discussed it and would like to suggest making the association between servers and operations such as:

• Server.operations()
• Operation.servers()

Thoughts: Will there be any problems with the generator in terms of the template parameter server?

* `Message.contentType()` should be added and automatically use the global default content type if none is specified for the message.

@smoya and I agreed we should add this.

• A way to check what an operation is, such as Operation.isClientSubscribing.

@smoya and I discussed this and came up with a suggestion for adding the following:

Operation.isClientSubscribing : boolean
Operation.isClientPublishing : boolean
Operation.isApplicationSubscribing : boolean
Operation.isApplicationPublishing : boolean
Operation.type : ClientSubscribing, ClientPublishing, ApplicationSubscribing, ApplicationPublishing

Why add both type and the other functions? Because they solve different use cases, adding something specific based on a specific operation such as if(operation.isClientSubscribing()) return generateClientSubscribe();. The second use case is for example wanting to get the type of operation for something like documentation.

Thoughts?

[jonaslagoni]

Bindings are a bit weird to use, especially if you want to get nested properties such as channel.binding('nats', 'requestReply').is if you do want the is a property you have no way of checking if the binding (is) exist.

We have two choices at the moment, either channel.binding('nats').requestReply.is or channel.binding('nats', 'requestReply', 'is'). Me and @smoya suggest using channel.binding('nats').requestReply.is since bindings are the wild west.

Would it make sense to have a binding intent such as channel.binding('nats').isClientRequester()?

[smoya]

@jonaslagoni and I are gonna move with the mocking of the API based on the previous suggestions.
Then, validating them through the following templates by rewriting them in order to adopt the new API:

• https://github.com/asyncapi/nodejs-ws-template
• https://github.com/asyncapi/markdown-template

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication