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

Message ID doesn't exist on reply level #873

Closed
AceTheCreator opened this issue Oct 10, 2023 · 16 comments
Closed

Message ID doesn't exist on reply level #873

AceTheCreator opened this issue Oct 10, 2023 · 16 comments
Labels
bug Something isn't working

Comments

@AceTheCreator
Copy link
Member

When using the request-reply pattern, and you're trying to retrieve the reply() messages, the parser is returning the wrong message ID for some reason.

Here's what I mean by wrong messages. In the example below, I expect the parser to return the messageID of the messages available in the reply object, but instead, all I get is the index(integer) of the message as ID

operations:
    requestCosting:
      action: receive
      channel: 
        $ref: '#/channels/costingRequest'
      reply:
        channel: 
          $ref: '#/channels/costingResponse'
        messages:
          - $ref: "#/components/messages/costingResponse"

cc @smoya @fmvilas

@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup 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.

@smoya
Copy link
Member

smoya commented Oct 10, 2023

@AceTheCreator could you please provide a code snippet of what you are executing and the output? Thank you!

@AceTheCreator
Copy link
Member Author

@smoya, here's a sample code snippet

AsyncAPI document

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  user/signedup:
    address: user/signedup
    messages:
      subscribe.message:
        $ref: '#/components/messages/UserSignedUp'
operations:
  user/signedup.subscribe:
    action: send
    channel:
      $ref: '#/channels/user~1signedup'
    messages:
      - $ref: '#/components/messages/UserSignedUp'
    reply:
      channel:
        $ref: '#/channels/user~1signedup'
      messages:
        - $ref: '#/components/messages/UserSignedUp'
components:
  messages:
    UserSignedUp:
      payload:
        type: object
        properties:
          displayName:
            type: string
            description: Name of the user
          email:
            type: string
            format: email
            description: Email of the user

Here's the code I'm trying to retrieve the ID with

    document?.operations().filterBy((operation) => {
      if (operation.reply !== undefined) {
        const replyObj = operation.reply()
		     const messages = replyObj.messages().all();
		     messages.forEach(message => {
		       console.log(message.messageId())
                        console.log(message.id())
		     });
		   }
		});

Here's what I get in the console
undefined
0

@smoya
Copy link
Member

smoya commented Oct 23, 2023

Can this be somehow related to the issue under #856 @jonaslagoni ?

@jonaslagoni
Copy link
Sponsor Member

messageId is a specific value in the spec, where id comes from a more generic approach based on this.messageId() || this._meta.id || this.extensions().get(xParserMessageName)?.value<string>(). We should probably have better JSDoc for each function 👍

The 0 returned by id is because you in this case have an array of messages where this one is the first index.
undefined is returned because, well, the message object does not contain a messageId property.

Can this be somehow related to the issue under #856 @jonaslagoni ?

Not related, no 🙂

@jonaslagoni
Copy link
Sponsor Member

jonaslagoni commented Oct 23, 2023

The 0 returned by id is because you in this case have an array of messages where this one is the first index.
undefined is returned because, well, the message object does not contain a messageId property.

Whether this behavior is the right and expected behavior, we will have to chime in @magicmatatjahu 😄 Or @fmvilas 🤔

@fmvilas
Copy link
Member

fmvilas commented Nov 6, 2023

Whether this behavior is the right and expected behavior, we will have to chime in @magicmatatjahu 😄 Or @fmvilas 🤔

I don't think it's the expected one. IMHO it should return the id of the message, most likely the one coming from the key under components/messages in this case. Or the key under a channel's messages, if $ref is pointing there.

@jonaslagoni
Copy link
Sponsor Member

jonaslagoni commented Nov 6, 2023

The problem then is the way references are currently being handled, is NOT to replace the reference in operations to channels and messages with the already resolved object that we created for the instance in the channel.

We don't have access to the key in the key/value pair that is being referenced. Therefore the ONLY possible solution we can do is the current approach, at least with the current tooling we have at hand.

Otherwise, we have to create our own reference tooling that can resolve partial references and manually control what instance should be returned.

@derberg
Copy link
Member

derberg commented Nov 7, 2023

Just kind reminder that we knew that using $ref everywhere will have a side effect that it will he harder to get the id of the element and that we will have to hook into dereferencing process. So instead of waiting for or working on completely new tool that handles references, parser should grab id from the reference, and put in some extension like x-parser-refId or whatever

@smoya
Copy link
Member

smoya commented Nov 7, 2023

So instead of waiting for or working on completely new tool that handles references, parser should grab id from the reference, and put in some extension like x-parser-refId or whatever

It is not a trivial thing. Dereference is not handled by the parser directly but by @stoplight/json-ref-resolver (nothing new, we already knew). The fact we would need to get access to the surroundings of the referenced object in order to get the key of the map if exists makes it even harder.

Copy link
Member

derberg commented Nov 7, 2023

@smoya you have the key of referenced object in the reference itself, as part of $ref string

@smoya
Copy link
Member

smoya commented Nov 10, 2023

Made a working POC about getting the ID from the JSON Pointer in the reference for reply.chanel and reply.messages[]: #895

Do you think it makes sense in that way? Is that what you meant @derberg ?

@derberg
Copy link
Member

derberg commented Nov 15, 2023

@smoya I think objectIdFromJSONPointer is complex enough that it deserves unit tests that can clearly explain what works and what doesn't work 😓

@smoya
Copy link
Member

smoya commented Nov 15, 2023

@smoya I think objectIdFromJSONPointer is complex enough that it deserves unit tests that can clearly explain what works and what doesn't work 😓

It is just a draft, a proof of concept. It takes the last part of the pointer as the Id.

@derberg
Copy link
Member

derberg commented Nov 16, 2023

sounds good to me

@derberg derberg added the bug Something isn't working label Nov 16, 2023
@jonaslagoni
Copy link
Sponsor Member

Should be solved, feel free to re-open if you still encounter it 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants