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

docs(spec): added name proposal to channel item object #613

Closed
wants to merge 1 commit into from

Conversation

bali182
Copy link

@bali182 bali182 commented Aug 25, 2021


title: "Nameable Channel Item Object"


Related issue(s):

#614

This proposal adds an optional name field to the Channel Item Object type. I'm trying to generate WebSocket code using AsyncAPI document. Subscribe and publish operations are nameable/identifiable using operationId, but that's not how sockets work, as a single channel is capable for both. It makes no sense to open a connection for both.

Example:

Let's assume we have the following schema:

{
  "asyncapi": "2.0",
  "info": {
    "title": "",
    "version": ""
  },
  "components": {
    "schemas": {
      "NamedSimpleObject": {
        "type": "object",
        "required": [
          "stringProperty",
        ],
        "properties": {
          "stringProperty": {
            "type": "string"
          }
        }
      }
  },
  "channels": {
    "/test/{pathParam}": {
      "name": "TestChannel",
      "parameters": {
        "pathParam": {
          "description": "test param",
          "schema": {
            "type": "string"
          }
        }
      },
      "subscribe": {
        "operationId": "subToTest",
        "summary": "Sub to test",
        "message": {
          "payload": {
            "$ref": "#/components/schemas/NamedSimpleObject"
          }
        }
      },
      "publish": {
        "operationId": "pubToTest",
        "summary": "Pub to test",
        "message": {
          "payload": {
            "$ref": "#/components/schemas/NamedSimpleObject"
          }
        }
      }
    }
  }
}

This could be translated roughly to something like this in code

// Name based on my proposal
class TestChannel {
  private socket: WebSocket
  
  // Parameter from the channels ParameterObject
  constructor(pathParam: string) {
    this.socket = new WebSocket(`/test/${pathParam}`)
  }

  // subToTest is the operationId, NamedSimpleObject the message's schema, all good here
  subToTest(listener: (data: NamedSimpleObject) => void): void {
    this.socket.addEventListener('message', ({ data }) => {
      listener(JSON.parse(data))
    })
  }
  
  // pubToTest is the operationId, NamedSimpleObject the message's schema, all good here too
  pubToTest(data: NamedSimpleObject): void {
    this.socket.send(JSON.stringify(data))
  }
}

As you can see without the name field, it's not possible to name whatever concept generated code is using to encapsulate both subscribe and publish. It could be channelId too in case it needs to be inline with the operationId.

@sonarcloud
Copy link

sonarcloud bot commented Aug 25, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. 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.

@derberg
Copy link
Member

derberg commented Dec 21, 2021

@smoya I know you are now involved in work towards a possible 3.0 that is related to #618

Proposal 618 is pretty solid if it comes to the change from grouping channels by pathname to an actual name, and the current path goes into address property. So in theory, if we add name in 2.3 then converting this to 3.0 format will be super easy. Right?

@fmvilas thoughts?

@bali182 what is your take on this one? you created it as docs edit but it is actually a change in the spec, so has to go through contribution stages. Do you want to champion it and are you ready to contribute changes in JSON Schema for the spec and also the JavaScript parser?

@smoya
Copy link
Member

smoya commented Dec 22, 2021

if we add name in 2.3 then converting this to 3.0 format will be super easy

As far as we ensure names are unique across channels. In #618 comes by default because the name is the key on the channels map.

@bali182
Copy link
Author

bali182 commented Dec 29, 2021

@derberg I had a quick look at #618, if I understand it correctly, that would solve what I proposed in an even more generic and elegant way. What is the status of this proposal? Does it need more input/work/decision?

@smoya
Copy link
Member

smoya commented Dec 29, 2021

@derberg I had a quick look at #618, if I understand it correctly, that would solve what I proposed in an even more generic and elegant way. What is the status of this proposal? Does it need more input/work/decision?

I started working on it in little chunks #618 (comment)

I created several issues on spec repository so anyone can pick it up.
It would be awesome if anyone else wants to join the efforts! (Which are not so trivial)

@bali182
Copy link
Author

bali182 commented Jan 1, 2022

Can't promise anything, but gonna check it out if I have time. Gonna close this PR.

@bali182 bali182 closed this Jan 1, 2022
@derberg
Copy link
Member

derberg commented Jan 3, 2022

@bali182 at least please be involved in review and share your opinion. More different opinions = better features along the way

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

Successfully merging this pull request may close these issues.

3 participants