Add operationId for the reply

[Tenischev]

During check of what changes should be done in template to support Spec v3, I struggling with reply definition.
At least in Java world it's preferable to have a separate method definition for send and receive Message.
So, for example we have following:

operations:
  pingRequest:
    action: send
    channel:
      $ref: '#/channels/ping'
    reply:
      channel:
        $ref: '#/channels/pong'

From this definition of API I could generate method to send message and name it pingRequest based on the name of operation.
But I also should create a method to receive the reply and here I have a problem, because the reply has no name and Operation Reply Object has no field where user could provide the name.

As hardcoded solution, I could generate method with name like pingRequestReply ( + "Reply"), but this approach has a drawback, since occasionally name generated in this way may overlap with name of another operation defined by user in an API.
Thus, I would prefer to have something like operationId in fields of Operation Reply Object where user could define a desired name for generated method.

[smoya]

occasionally name generated in this way may overlap with name of another operation defined by user in an API.

This is, to me, the important issue here.
Adding a new field, like operationId as you suggest, makes the reply object being very close to what an Operation is.

Wondering if then would make sense to suggest we reference an operation in the reply field instead of that?

operations:
  pingRequest:
    action: send
    channel:
      $ref: '#/channels/ping'
    reply:
      $ref: '#/operations/pingReply'
  pingReply:
    action: receive
    channel:
      $ref: '#/channels/pong'

I'm not sure this has been considered in the original issue 🤔

[Tenischev]

The $ref from reply to operation would work for generation, but I see a drawback in use case here, because now pingReply has defined action = receive. Thus it couldn't be reused for the Replier, because for Replier action should be send
When in current design:

• Requestor:

operations:
  pingRequest:
    action: send
    channel: 
      $ref: '#/channels/ping'
    reply:
      channel: 
        $ref: '#/channels/pong'

• Replier:

operations:
  pongReply:
    action: receive
    channel: 
      $ref: '#/channels/ping'
    reply:
      channel: 
        $ref: '#/channels/pong'

The reply part here is common

    reply:
      channel: 
        $ref: '#/channels/pong'

and could be reused by the reference.

[derberg]

operation reply is not the same as operation, we should not enable referencing there. Also adding operation for just reply should not happen, as reply is not a default operation of the app, it is just an operation that happens as a result of the core operation it is located it.

for me it sounds like reply simply needs it's own unique operationId 🤔

[Tenischev]

could we allow design like:

operations:
  pingRequest:
    action: send
    channel: 
      $ref: '#/channels/ping'
    reply:
      pongReply:
        channel: 
          $ref: '#/channels/pong'

?

Consequently

operations:
  pingRequest:
    action: send
    channel: 
      $ref: '#/channels/ping'
    reply:
      $ref: '#/reply/pongReply'
      
components:
  reply:
    pongReply:
      channel: 
        $ref: '#/channels/pong'

[smoya]

The $ref from reply to operation would work for generation, but I see a drawback in use case here, because now pingReply has defined action = receive. Thus it couldn't be reused for the Replier, because for Replier action should be send

That's a remarcable caveat indeed @Tenischev. With the current dereference behaviour, we can't overwrite properties of the referenced object like in the following example:

# requestor.yaml
operations:
  pingRequest:
    $ref: './common.yaml#/operations/pingRequest'
  pongReply:
    $ref: './common.yaml#/operations/pongRequest'

# replier.yaml
operations:
  pingRequest:
    $ref: './common.yaml#/operations/pingRequest'
    action: receive
  pongReply:
    $ref: './common.yaml#/operations/pongRequest'
    action: send

# common.yaml
# ... 
operations:
  pingRequest:
    action: send
    channel:
      $ref: '#/channels/ping'
    reply:
      $ref: '#/operations/pongReply'
  pongReply:
    action: receive
    channel:
      $ref: '#/channels/pong'

Unfortunately, this is not probably gonna happen in the near future. So nothing is viable on that side atm unless we finally give priority to that.

Also, I didn't think about recursivity in the case of allowing referencing an Operation here that has a circular ref on the reply... So it's definitely not trivial.

Anyway, in that same path, adding operationId would confirm indeed that a reply is considered an operation, isn't? Otherwise, you would call it replyId or similar, which we could go for it atm since the lack of support of the overwrite properties when dereferencing.

operation reply is not the same as operation, we should not enable referencing there. Also adding operation for just reply should not happen, as reply is not a default operation of the app, it is just an operation that happens as a result of the core operation it is located it.

Even though you can read my answer to @Tenischev above, I still don't really see such a difference. A reply to a request it's an operation that is triggered by another operation. I see it in that way.

[Tenischev]

@derberg @smoya
What do you think about my suggestion here #981 (comment) ?
In this way structure under reply will have a name either by direct definition or by name of ref object which could be referenced.

[GreenRover]

@Tenischev I dont agree with your argumentation that you need a dedicated reply method for responses.
Eighter you are async and there you have 2 methods. Or your do request reply where you have a single functional interface.

Please checkout this interface/lib:
https://github.com/solacecommunity/spring-cloud-stream-request-reply

[Tenischev]

@GreenRover You are right, that reply not mandatory to be handled asynchronously and some libraries provide default methods to handle it synchronously.
But here we have the spec, not a vendor-lock solution. If we would refer to what spec says, then

Request/Reply is a communication pattern where one entity, the 'requestor,' sends a message to another, the 'replier', and waits for a response. Such a pattern is used when a component (or service) needs to initiate an action and receive a specific response in return, either synchronously or asynchronously. In Event-Driven Architectures, this communication pattern is asynchronous, meaning that the requester does not block and wait for an immediate response. Instead, it can continue processing other tasks or even send out other requests while waiting for the reply to arrive.
https://www.asyncapi.com/docs/concepts/asyncapi-document/reply-info

[GreenRover]

Yes i know and understand that.
I not like to prevent a vender lock solution. The sample i provided is to show an implementation of an design pattern.
A single operation is not necessary's blocking. It cloud also response an Fututre(Java), Promise(ECMA-Script), Mono(Reactor)

As soon as you have to operations it is not a classic request reply pattern.

Than you end up with something like this:

operations:
  sendPing:
    action: send
    channel: 
      $ref: '#/channels/ping'
  receivePong:
    action: receive
    channel: 
      $ref: '#/channels/pong'

This is also a very common design pattern. But here you don`t have a machine readable correlation between ping and pong.
What i intended with the current solution is that you can generate code out of the spec that full fills the classic blocking request reply.

What is the target of your change? What is the requirement you currently can not solve?

[smoya]

As soon as you have to operations it is not a classic request reply pattern.

That's actually a good point @GreenRover.

[smoya]

In order to not pollute this issue about the topic "Should reply object become an operation object?", I created #1009

[Tenischev]

What i intended with the current solution is that you can generate code out of the spec that full fills the classic blocking request reply.

Looks like we have a "something" (or a "classic") what was implicitly put into the specification. Which is at least for me - a problem. Who is define what is the "classic" and what's not?
cc @derberg @fmvilas

What is the target of your change? What is the requirement you currently can not solve?

I want to give to an user as much flexibility in the template as I can. Solution mentioned by you to use an async types like the Fututre form Java could be the good solution for the "send and reply", whereas for the "receive and reply" it's a bit more tricky.
But again, it's only one of possible solution, another mentioned by me in first comment of the issue is to generate a separate methods to "request" and to "reply".

[GreenRover]

Can you provide examples that currently can not be solved. May this would helps me to understand your point a little better.

[Tenischev]

@GreenRover it's described in first message, but let me give a more detailed example and sorry for java oriented pseudo-code.
Assume we have this very simple spec where reply is receive:

operations:
  pingRequest:
    action: send
    channel:
      $ref: '#/channels/ping'
    reply:
      channel:
        $ref: '#/channels/pong'

Approach 1. Use asynchronous object like Future (or another protocol specific) to handle reply

 public FutureMessage pingRequest(PingMessage ping) {
   return broker.sendAndGetReply(ping, pongChannel);
}

Simple code, but require from underling broker library support of methods like sendAndGetReply. Useful in scenario when we need validate a reply and e.g. if it's error make a retry.

Approach 2. Use callback object to handle reply

public void pingRequest(PingMessage ping) {
  broker.sendAndReceive(ping, pingCallback);
}

CallbackOperation pingCallback = new Callback() {
public void onSuccess(ReplyMessage message) {
}

public void onError(Error e) {
}
}

Callback implementation could be unified for some operations and write once. Here I used a pingCallback name which is just variation of how we could modify original operation name to avoid naming error, but the problem of such naming was described in first comment.
Approach 3. Split send and receive to separate code blocks.

class MessageSender {
  public void pingRequest(PingMessage ping) {
    broker.send(ping);
  }
}
class MessageHandler {
  public Message pingReply() {
    return broker.receiveFromTopic(pong);
  }
}

This solution may be sufficient when user need to make a complicated logic on a reply which are not directly connected with sending itself. In addition we could scale instances of Sender and Handler independently. Here I used a pingReply name which is just variation of how we could modify original operation name to avoid naming error, but the problem of such naming was described in first comment.

Of course these 3 implementation are not exhaustive and depending on broker, architecture approach e.t.c. user may want something else, but I could name them "common" that you'll find in most guides\books.
Also, I want to mention that only "send and receive" is present here. For "receive and reply" the same scenarios are applicable.

[GreenRover]

I guess i understood what you wanted to say. But still can not aggress.
1.) RequestReply is always a feature of your library. I know not a single broker providing it as special protocoll.
Here some examples:
If you watch: https://www.youtube.com/watch?v=-OsMet9h_dg
We find 2 Kinds of implementations (Variant 2 and 3)

• Variant 2: Was implemented by solace in java JCSMP lib
• Variant 3: Was implemented by tibco in java TibRv lib
• Variant 2: Was implemented by me for SpringCloud Stream in: https://github.com/solacecommunity/spring-cloud-stream-request-reply. This is used with multiple different broker protocols under the hood.

Recap: There is never an broker.sendAndGetReply those are sometimes part of a library provided by the vendor, but it dont have to be like this.

2.) Your approach 3 is not working this easy.
This is simple pub sub with out correlation of request and reply. The matching using unique topic or correlationId is missing.
This is not a trivial thing to do. You need to handle message lost to avoid message lost. Synchronization between threads is a problem as well. The attempt of providing requestReply in AsyncApi is to documented how this correlation is/should be implemented.

3.) You pointed out a general problem of AsyncApi: Is the documentation in perspective of client or server. Is not only given for RequestReply. The same problem exists for operation.action:
https://www.asyncapi.com/docs/reference/specification/v3.0.0#operationObject
My "easy" solution is to write apis always from the viw point of api owner what is always the data owner aka server.
There for code generator should provide an "invert" flag to generator operations the opposite way. But this is imho not a problem of spec.

But in #1009 your brough up a good point security and bindings are missing. I personally not use this features what make me missing this fact.

[Tenischev]

1. I wrote

Simple code, but require from underling broker library support of methods like sendAndGetReply.

I think it's clear. I do not plan to reinvent application-broker communication protocol, I have libraries for this.

• for Spring-Kafka sendAndReceive
• for Spring-AMQP convertSendAndReceive
• for case when could use spring-integration IntegrationFlow and protocol-broker specific library do not provide sendAndGetReply

Yes, they are a library functions, but I didn’t say otherwise.

2. I couldn't write an entire template in a comment, I showed approach and explain why it could be interested for a user to use it.
   Maybe somewhere in some template it would be a problem for a thread synchronization in the third approach, but not in java-spring template.

The attempt of providing requestReply in AsyncApi is to documented how this correlation is/should be implemented.

The Spec, the AsyncAPI file should show how messages are goes between applications.
"The AsyncAPI specification does not assume any kind of software topology, architecture or pattern."
Architecture may vary as I tried to show and as I wrote "I want to give to an user as much flexibility in the template as I can."

3. Yes, I stepped in this pit at first time when tried to understood reply "direction", this article from @jonaslagoni helps a lot and this "pit" is a reason why I open Provide definition for the reply #980

[github-actions]

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

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 ❤️