Open derberg opened 11 months ago
That makes sense to me yea 🤔
Yes, that makes sense to me as well.
How should this interact with operation traits that specify a message? Only allowing references to messages in a specific channel for them would limit their reusability.
@buehlefs could you visualize a problem with an example? message trait is used usually when messageTrait
keeps entire message definition, without references, and also messageTrait
is something you use on message
level 🤔 you would anyway add it in channel.message right?
@fmvilas @jonaslagoni be aware that it can affect release date, unless we get help to make sure that is supported in parser-js and converter-js. Probably examples need a review. Nevertheless, better to add this limitation now, and relax with 3.1 or 3.2, cause other way around it will be a breaking change
@derberg I am talking about an operationTrait
object that can "contain any property from the Operation Object, except the action, channel and traits" (as per current pre release spec). I can see this being used to specify a login (or haertbeat) operation with a login message that will be used in multiple locations with different channels. But this reuse would only be possible if the message is referenced from components and not from a specific channel. (but I may be missing something here)
@buehlefs The following sentence located in the description of the Operation Trait object is wrong (nice catch!):
This object MAY contain any property from the Operation Object, except the action, channel and traits ones.
Source: https://www.asyncapi.com/docs/reference/specification/v3.0.0-next-major-spec.16#operationTraitObject
If you take a look at the fields an Operation Trait object has, messages
field is not included.
You can confirm this is by design and expected by checking its corresponding JSON Schema definition at https://github.com/asyncapi/spec-json-schemas/blob/next-major-spec/definitions/3.0.0/operationTrait.json.
We should fix that sentence. (I created the following issue so we work on it asap https://github.com/asyncapi/spec/issues/994)
@derberg @fmvilas I'm gonna start working on the validation for this in Parser-JS. We would need to add an Spectral rule to check that.
Btw, we do not have particular rules for v3 implemented atm besides the standard ruleset. In case any of you think other must-have rules are missing, please mention it in an issue so we can work on it.
Awesome! Thanks, @smoya!
I was wondering today if we shouldn't apply a similar rule to the channel
field in an operation. It should only be allowed to point to a channel in the channels
section and not to a channel in components/channels
or in another file or URL. Does that make sense?
It should be applied to the spec, JSON schemas, and the parser.
Probably a good idea 🤔
Awesome! Thanks, @smoya!
I was wondering today if we shouldn't apply a similar rule to the
channel
field in an operation. It should only be allowed to point to a channel in thechannels
section and not to a channel incomponents/channels
or in another file or URL. Does that make sense?It should be applied to the spec, JSON schemas, and the parser.
Yup, good catch
I was wondering today if we shouldn't apply a similar rule to the
channel
field in an operation. It should only be allowed to point to a channel in thechannels
section and not to a channel incomponents/channels
or in another file or URL. Does that make sense?
I gave this another thought and I think this shouldn't happen. What happens with channels and linked operations declared under components?
Consider the following components in an AsyncAPI v3 doc:
components:
channels:
aChannel:
messages:
aMessage:
$ref: '#/components/messages/aMessage'
operations:
anOperation:
action: send
channel:
$ref: '#/components/channels/aChannel'
messages:
- $ref: '#/components/channels/aChannel/messages/aMessage'
You can see the anOperation
refers to the aChannel
channel, also in components. Same with its message.
If we force those to always refer to channels from #/channels
, the use case above won't be valid.
I believe the current description of the messages
field of an Operation is already telling us what should be the enforcements:
It MUST contain a subset of the messages defined in the channel referenced in this operation
If the channel is located under #/channels
or rather #/components/channels
is not important here if we always keep that consistency in referenced messages under messages
field.
cc @fmvilas @jonaslagoni @derberg
Clarifying the rules we want to add:
operations
), the operation channel must only point to a required channel (under root channels
). components
), the operation channel can point either to a required channel (under root operations
) or to an optional channel (under components
).Are we aligned?
cc @fmvilas @derberg @jonaslagoni and anyone interested
Yes, that's exactly what I mean.
The messages, no matter where they are located, have to point to messages from the channel specified in the field channel.
1/3 rules created: https://github.com/asyncapi/parser-js/pull/911
- If the operation is optional (under
components
), the operation channel can point either to a required channel (under rootoperations
) or to an optional channel (undercomponents
).
Actually this won't be considered as a rule because there is nothing to validate here, right?
If the operation is required (under root operations), the operation channel must only point to a required channel (under root channels).
Second rule created: https://github.com/smoya/parser-js/pull/5 However, it is located in my fork, because all needed scaffolding for enabling Spectral rules for v3 are added into the opened PR https://github.com/asyncapi/parser-js/pull/911. Once is merged, I will close and open a new PR against AsyncAPI repo. But feel free to review anyway please.
we have similar situation with channel.servers
where servers is an array of $ref pointers. Such $ref should always point to the root server
and not components/server
so in general, we need to add one new sentence to servers under channels
, messages under operation and operation reply
, channels under operation and operation reply
suggested edit, for channel:
#current text:
A $ref pointer to the definition of the channel in which this operation is performed
#suggested edit
A $ref pointer to the definition of the channel in which this operation is performed. It MUST point to channel definition located in [Channels Object](#channelsObject) and MUST NOT point to channel definition located in [Components Object](#componentsObject)
#I know first must make it clear that components are not allowed, but I think better to be explicit like this
above example can of course be easily modified and for messages will for example sound
#current
A list of $ref pointers pointing to the supported [Message Objects](https://github.com/asyncapi/spec/blob/next-major-spec/spec/asyncapi.md#messageObject) that can be processed by this operation. It MUST contain a subset of the messages defined in the [channel referenced in this operation](https://github.com/asyncapi/spec/blob/next-major-spec/spec/asyncapi.md#operationObjectChannel).
#new
A list of $ref pointers pointing to the supported [Message Objects](#messageObject) that can be processed by this operation. It MUST contain a subset of the messages defined in the [channel referenced in this operation](#operationObjectChannel). Individual $ref pointer MUST point to message definition located in the [channel referenced in this operation](#operationObjectChannel) and MUST NOT point to message definition located in [Components Object](#componentsObject)
we have similar situation with
channel.servers
where servers is an array of $ref pointers. Such $ref should always point to the rootserver
and notcomponents/server
Good catch again. I will create the proper Spectral rule as well.
I will create the proper Spectral rule as well.
Done. Added into the same previous PR https://github.com/asyncapi/parser-js/pull/913
All parser-js required Spectral rules are now merged and enabled in next-major-spec
above example can of course be easily modified and for messages will for example sound
I don't think we need to change that. It sounds pretty clear to me, specially now that we will update the channel
operation field to be explicit about where should it point to.
It MUST contain a subset of the messages defined in the channel referenced in this operation
@smoya but current text is not clear that $ref pointer must point to the channel - it only says the message must be the same (but it is not explicit - even for us as even our converter do not convert properly)
Let's move this discussion into https://github.com/asyncapi/spec/pull/996
We are missing the Spectral rules for OperationReply.channel
and OperationReply.messages
.
They should validate that:
OperationReply.channel
Does it make sense?
I think it does, yeah 👍
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:
Hi everyone! 👋
I believe this issue is still important despite the inactivity. We're missing Spectral rules for OperationReply.channel and OperationReply.messages. These rules should validate that:
messages point to a subset of messages in the OperationReply.channel. channel should reference: The root channel if OperationReply is under a root operation. The root or a component channel if under components Operation or Replies. To move forward, let's finalize these rules and implement them in Spectral. I'm happy to help with the implementation or testing.
Looking forward to your thoughts!
Thanks! ❤️
Context
Messages in operation
Messages in operation reply
Key of the problem
Confusion
but does that mean that the reference must point to a message through channel location, or can be also
components
?for me this should be enforced:
this part
and people should not be allowed to do
otherwise we'll get into a trouble of mismatch, unless I'm wrong and
messageId
is not considered to be a part of messagetake this v2 example
converter will convert it to
operation points to message
UserSignedUp
but on a channel level we have a messagesubscribe.message
imho the only proper way is to specify that
$ref
pointer must point to the channel - and this needs to be validated with parser of course.Am I missing any possible errors that this could cause?