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

why we do not use message name in v3 conversion #195

Closed
derberg opened this issue Oct 9, 2023 · 3 comments
Closed

why we do not use message name in v3 conversion #195

derberg opened this issue Oct 9, 2023 · 3 comments

Comments

@derberg
Copy link
Member

derberg commented Oct 9, 2023

as described in https://github.com/asyncapi/converter-js#conversion-2xx-to-3xx

If the operation does not have an operationId field defined, the unique identifier of the operation will be defined as a combination of the identifier of the channel on which the operation was defined + the type of operation, publish or subscribe. Identical situation is with messages. However, here the priority is the messageId field and then the concatenation {publish|subscribe}.messages.{optional index of oneOf messages}.

Why we do not take into account name of the message, because it can be duplicated across operations (not unique like messageId)? If we see duplication, we can have the same approach as described above, with index.

Or am I something missing

Currently in case of this 2.x example:

channels:
  smartylighting/streetlights/1/0/event/{streetlightId}/lighting/measured:
    parameters:
      streetlightId:
        $ref: '#/components/parameters/streetlightId'
    publish:
      summary: Inform about environmental lighting conditions of a particular streetlight.
      operationId: receiveLightMeasurement
      message:
        $ref: '#/components/messages/lightMeasured'

components:
  messages:
    lightMeasured:
      name: lightMeasured
      title: Light measured
      summary: Inform about environmental lighting conditions of a particular streetlight.

resulting message name in a new v3 channel is:

messages:
      receiveLightMeasurement.message:
        $ref: '#/components/messages/lightMeasured'

and better would be

messages:
      lightMeasured:
        $ref: '#/components/messages/lightMeasured'

or in some cases:

messages:
      lightMeasured.1:
        $ref: '#/components/messages/lightMeasured'

cc @jonaslagoni @magicmatatjahu

@derberg
Copy link
Member Author

derberg commented Oct 9, 2023

also in case of:

    subscribe:
      message:
        $ref: '#/components/messages/book'      
    publish:
      message:
        $ref: '#/components/messages/book'

instead of

      publish.message:
        $ref: '#/components/messages/book'
      subscribe.message:
        $ref: '#/components/messages/book'

can't we convert to

      oneMessageAsWeSeeTheSameRef:
        $ref: '#/components/messages/book'

@magicmatatjahu
Copy link
Member

Why we do not take into account name of the message, because it can be duplicated across operations (not unique like messageId)?

Yes, name in v2 isn't defined as unique field across all messages inside spec so converter takes messageId for that or create new identity based on the channel name + usually some suffix to prevent duplication. About case with duplicated refs, good 👁️ - converter can support that, it shouldn't be a problem to implement that.

Copy link
Member Author

derberg commented Oct 10, 2023

it is combining operationId with suffix index atm, or operation name if id is missing. My suggestion is that message.name should have higher priority, so messageId -> message.name -> what we already have

I had to rework many examples, as these message names from converter were really not readable - not to say useless

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

No branches or pull requests

2 participants