asyncapi / bundler

Combine multiple AsyncAPI specification files into one.
Apache License 2.0
30 stars 15 forks source link

Refine AsyncAPI Bundling with Origin Tracing and Component Naming #141

Closed KhudaDad414 closed 7 months ago

KhudaDad414 commented 1 year ago

Context

https://github.com/asyncapi/bundler/issues/97

Summary:

Integrating the optimizer and bundler to enhance user experience requires a few modifications. These include removing redundant features, tracing the origins of references, and improving naming conventions within the bundled output.

Detailed Steps:

1) Simplify the Bundler by Removing referenceIntoComponents:

The referenceIntoComponents feature in the bundler seems to belong to the optimizer. Its removal should be straightforward:

2) Traceability of $ref Origins Post-Bundling:

To maintain a clear lineage of $ref components post-bundling, it's necessary to include an x-origin property. This will annotate where the original $ref was located in the source files.

For example, transform this:

...
  message:
    $ref: ./messages.yaml#/messages/UserSignedUp
...

Into this:

...
  message:
    x-origin: ./messages.yaml#/messages/UserSignedUp
    payload:
      type: object
      properties:
        displayName:
          type: string
          description: Name of the user
...

3) Introduction of a New Flag in the optimizer:

We propose adding a flag to the optimizer to centralize all components under the components section of an AsyncAPI document. The proposed flag is moveAllComponents. Alternative suggestions for the flag name are welcome for better intuitiveness.

4) Intelligent Component Naming in the optimizer:

With the x-origin in place, the optimizer should leverage it to assign meaningful names to components, falling back to a standard naming convention (e.g., message-1) only when a better name isn't discernible from the context.

For instance, utilizing UserSignedUp derived from x-origin instead of a generic message-1.

github-actions[bot] commented 1 year ago

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.

aeworxet commented 1 year ago

@thake Does point 2 presume carrying a satisfying amount of semantic meaning to serve your needs?

@derberg Do you have any remarks concerning the task?

thake commented 1 year ago

@aeworxet, thanks for pinging me on this issue again!

The solution in point 2 (x-origin) does not cover my use case. I want refs to be bundled like they were defined in the common components of the AsyncAPI. By doing so, all tools that display AsyncAPIs would automatically display the name of the previously external component without additional work. By adding a new property x-origin, the same can't be achieved.

I've created a small example repo to demonstrate how it works with OpenAPI tooling: https://github.com/thake/openapi_bundling/tree/main

aeworxet commented 1 year ago

@thake Is this what you meant?

Transform this

...
  message:
    $ref: './messages.yaml#/messages/UserSignedUp'
...

Into this

...
  message:
    $ref: '#/components/messages/UserSignedUp'
...
...
  components:
    messages:
      UserSignedUp:
        x-origin: './messages.yaml#/messages/UserSignedUp'
        payload:
          type: object
          properties:
            displayName:
              type: string
              description: Name of the user
...

?

thake commented 1 year ago

Correct, thanks for the asyncAPI example

aeworxet commented 1 year ago

@KhudaDad414 Does @thake's request imply a rewriting of point 2 of the task and provide the direction in which point 4 should be solved?

KhudaDad414 commented 1 year ago

@thake I see what you mean.

so basically we have two options here: As we have discussed in the previous issue, the point is to have a separation of concern between optimizer and bundler. I think this issue can be resolved by having a flag called moveComponentsWithOrigin and we only move the components that have the x-origin extension. the resulting document would be the same as your example but the flow would still be bundler --> optimizer.

@aeworxet what do you think?

cc: @derberg

aeworxet commented 1 year ago

I have taken a look at the way Optimizer works currently and it seems to be doing just what @thake wants. So, in my opinion, Bundler should dereference all $refs and create a raw merged AsyncAPI specification file, and Optimizer should move components to components, assigning them meaningful names and creating local $refs in the place where they were taken from. So the resulting YAML will be exactly what @thake needs, but in two steps, not in one. And x-origin, well, it will just be there; as an FYI, it doesn't impact anything (except the quantity of traffic on a large number of connections to the server.)

Original

...
  message:
    $ref: './messages.yaml#/messages/UserSignedUp'
...

After Bundler

...
  message:
    x-origin: './messages.yaml#/messages/UserSignedUp'
    payload:
      type: object
      properties:
        displayName:
          type: string
          description: Name of the user
...

After Optimizer

...
  message:
    $ref: '#/components/messages/UserSignedUp'
...
...
  components:
    messages:
      UserSignedUp:
        x-origin: './messages.yaml#/messages/UserSignedUp'
        payload:
          type: object
          properties:
            displayName:
              type: string
              description: Name of the user
...

@thake Does this clarification still reflect what you meant?

aeworxet commented 12 months ago

About the new flag.

Given that there are already flags reuseComponents removeComponents moveToComponents

I have two variants: either moveAllToComponents (semantic naming) or moveToComponentsAll (code-consistent naming.)

Which one would everyone choose?

aeworxet commented 11 months ago

In light of the release of the AsyncAPI Specification 3.0.0, should points 3 and 4 regarding Optimizer distinguish between v2 and v3 and work with them differently?

KhudaDad414 commented 11 months ago

@aeworxet what about changing moveToComponents to moveDuplicatesToComponents and naming the new flag moveAllToComponents?

In light of the release of the AsyncAPI Specification 3.0.0, should points 3 and 4 regarding Optimizer distinguish between v2 and v3 and work with them differently?

I don't see why would points 3 and 4 would treat v2 and v3 differently. 🤔

aeworxet commented 11 months ago

I don't see why would points 3 and 4 would treat v2 and v3 differently.

I am concerned about https://www.asyncapi.com/docs/migration/migrating-to-v3#operation-keywords https://www.asyncapi.com/docs/migration/migrating-to-v3#messages-instead-of-message

@Souvikns already had to introduce changes to the Bundler's codebase to support v3: https://github.com/asyncapi/bundler/pull/140/files#diff-8c55dd6a23f93625d2343439d276efb2d1a9cfe097b5e124a80653995ca5b0e9R41 https://github.com/asyncapi/bundler/pull/140/files#diff-8c55dd6a23f93625d2343439d276efb2d1a9cfe097b5e124a80653995ca5b0e9R66

Might it be necessary to implement something similar for the Optimizer?

I would of course do a recursive search by mask so as not to depend on version changes, but still.

@jonaslagoni @derberg

derberg commented 11 months ago

yup, best if flow is bundler -> optimizer also, these are just repos and libraries. How we do it in CLI is up to us, and do it best for users. If it is really needed and makes lots of sense, in CLI we can have a command that allows you to use both libraries in one run

@aeworxet not sure why are you concerned about v2 vs v3 optimizer uses JS Parser and afaik it implements the API that at least in theory should abstract the structure of AsyncAPI document from you - so getting operations from v2 or v3 should actually work the same way

sorry if my answer do not address the question, I might not get it. I also forgot a lot from initial discussion and focused on other stuff, sorry

derberg commented 11 months ago

@aeworxet you added it to bounty, but tbh it is not medium imho, as take into account complexity of the topic - how long discussion is taking place. I'm not maintainer here, but imho this is really and advanced issue

KhudaDad414 commented 11 months ago

@aeworxet your point is valid for the master branch. but https://github.com/asyncapi/optimizer/pull/193 is going to change the implementation so it uses parser-js. no issues there.

aeworxet commented 11 months ago

Bounty Issue's service comment

Text labels: bounty/2024-Q1, level/advanced, bounty/coding First assignment to third-party contributors: 2023-12-23 00:00:00 UTC+12:00 End Of Life: 2024-05-31 23:59:59 UTC-12:00

aeworxet commented 11 months ago

As an AsyncAPI Maintainer, I fall under the first category in the prioritization list, so I assign this issue to myself.

aeworxet commented 11 months ago

Bounty Issue's Timeline

Complexity Level Assignment date (by GitHub) Start date (by BP rules) End date (by BP rules) Draft PR submission Final PR submission Final PR merge
Advanced 2023-12-18 2024-01-01 2024-02-23 2024-01-19 2024-02-09 2024-02-23
Please note that the dates given represent deadlines, not specific dates, so if the goal is reached sooner, it's better.
aeworxet commented 11 months ago

Is there a date when https://github.com/asyncapi/optimizer/pull/193 is expected to be merged? I would prefer to first add functionality to the Optimizer and only then delete it from the Bundler.

Would renaming moveToComponents -> moveDuplicatesToComponents be a breaking change (since moveToComponents is in the API.md)?

KhudaDad414 commented 11 months ago

Is there a date when asyncapi/optimizer#193 is expected to be merged? I would prefer to first add functionality to the Optimizer and only then delete it from the Bundler.

The work is done. it will be merged after I get a green light from @derberg

Would renaming moveToComponents -> moveDuplicatesToComponents be a breaking change (since moveToComponents is in the API.md)?

yes, it is a breaking change. but since we haven't released v1 yet. I am not worried about breaking stuff. 🤷

aeworxet commented 11 months ago

@asyncapi/bounty_team

aeworxet commented 10 months ago

I need clarification on the task. I can reach with 'standard' methods $refs of the AsyncAPI Document provided as input, but not $refs of $refed AsyncAPI Documents in it. In case of input

main.yaml

channels:
  commentLiked:
    address: comment/liked
    messages:
      commentLiked:
        $ref: '../common/messages.yaml#/commentLiked'

../common/messages.yaml

commentLiked:
  description: Message that is being sent when a comment has been liked by someone.
  payload: 
    $ref: './schemas.yaml#/commentLikedPayload'

./schemas.yaml

commentLikedPayload: 
  type: object
  title: commentLikedPayload
  additionalProperties: false
  properties:
    commentId: 
      allOf: 
        - $ref: '#/commentId'
        - description: Id of the comment that was liked

the result is

channels:
  commentLiked:
    address: comment/liked
    messages:
      commentLiked:
        x-origin: ../common/messages.yaml#/commentLiked
        description: Message that is being sent when a comment has been liked by someone.
        payload:
          # here is supposed to be the 'x-origin: ./schemas.yaml#/commentLikedPayload'
          type: object
          title: commentLikedPayload
          additionalProperties: false
          properties:
            commentId:
              allOf:
                - type: string
                - description: Id of the comment that was liked

From one hand, the task is about adding x-origin to COMPONENTS. It's being added to components, so I could call it a day. But what's the sense of adding x-origin properties at all, if they are not added through ALL AsyncAPI Document?

And here comes the tricky part.

The easiest solution would be to alter copies of original AsyncAPI Documents on the filesystem. I can make copies of $refed AsyncAPI Documents on the filesystem (even if I request one from the network, I can still save it, load from the filesystem, iterate through it, make modifications, and save again.) But then Bundler starts to be engaged in a too much filesystem activity.

Or I can find paths to all $refed AsyncAPI Documents load them into memory, iterate through them, make modifications and merge copies of them into the main AsyncAPI Document. But more code introduces more bugs (talking about temptation of some engine to save reference to the original object even in case of deep cloning, due to their own bug.)

So, what should be the golden mean here?

KhudaDad414 commented 10 months ago

@aeworxet I don't know how bundler works and what would be the best solution here. maybe @Souvikns can shed some light here.

aeworxet commented 9 months ago

I decided to go with the memory version and multi-level recursion, to find $refs in the $refed files.

aeworxet commented 9 months ago

Due to circumstances that are beyond the control of the AsyncAPI Maintainer, who is responsible for the resolution of this Bounty Issue from the AsyncAPI's side (@KhudaDad414), the Bounty Issue's Timeline is frozen for an indefinite amount of time.

aeworxet commented 9 months ago

AsyncAPI Maintainer, who is responsible for the resolution of this Bounty Issue from the AsyncAPI's side (@KhudaDad414) was absent online in Slack for six periods of three working days in a row, so all remaining target dates of the Bounty Issue's Timeline are extended by seven calendar weeks.

Bounty Issue's Timeline Extended

Complexity Level Assignment date (by GitHub) Start date (by BP rules) End date (by BP rules) Draft PR submission Final PR submission Final PR merge
Advanced 2023-12-18 2024-01-01 2024-04-12 2024-01-19 2024-03-29 2024-04-12
Please note that the dates given represent deadlines, not specific dates, so if the goal is reached sooner, it's better.
KhudaDad414 commented 8 months ago

thanks for updating the timeline @aeworxet

aeworxet commented 8 months ago

I tried the modified Optimizer (moveAllToComponents: true) on

this AsyncAPI Document, which is the result from the modified `Bundler` ```yaml asyncapi: 3.0.0 info: title: Comments Service version: 1.0.0 description: This service is in charge of processing all the events related to comments. servers: mosquitto: host: test.mosquitto.org protocol: mqtt tags: - name: env:production description: This environment is meant for production use case - name: kind:remote description: This server is a remote server. Not exposed by the application - name: visibility:public description: This resource is public and available to everyone bindings: mqtt: clientId: comment-service channels: commentLiked: address: comment/liked messages: commentLiked: x-origin: ./messages.yaml#/commentLiked description: Message that is being sent when a comment has been liked by someone. payload: x-origin: ./schemas.yaml#/commentLikedPayload type: object title: commentLikedPayload additionalProperties: false properties: commentId: allOf: - type: string - description: Id of the comment that was liked description: >- Updates the likes count in the database and sends the new count to the broker. commentCountChange: address: comment/{commentId}/changed messages: commentChanged: x-origin: ./messages.yaml#/commentChanged description: Message that is being sent when a comment have been updated. payload: x-origin: ./schemas.yaml#/commentChangedPayload type: object title: commentChangedPayload additionalProperties: false properties: commentId: allOf: - type: string - description: >- Id of the comment that was changed, such as when someone liked it. likeCount: type: integer description: The new like count of how many have liked the comment. description: >- Sends the new count to the broker after it has been updated in the database. parameters: commentId: x-origin: ./parameters.yaml#/commentId description: ID of the comment operations: receiveCommentLiked: action: receive channel: $ref: '#/channels/commentLiked' messages: - $ref: '#/channels/commentLiked/messages/commentLiked' sendCommentChange: action: send channel: $ref: '#/channels/commentCountChange' messages: - $ref: '#/channels/commentCountChange/messages/commentChanged' ```

and received

this ```yaml asyncapi: 3.0.0 info: title: Comments Service version: 1.0.0 description: This service is in charge of processing all the events related to comments. servers: mosquitto: $ref: '#/components/servers/mosquitto' channels: commentLiked: $ref: '#/components/channels/commentLiked' commentCountChange: $ref: '#/components/channels/commentCountChange' operations: receiveCommentLiked: $ref: '#/components/operations/receiveCommentLiked' sendCommentChange: $ref: '#/components/operations/sendCommentChange' components: schemas: commentCountChange: x-origin: ./parameters.yaml#/commentId description: ID of the comment commentLiked: allOf: - $ref: '#/components/schemas/commentLiked' - $ref: '#/components/schemas/commentLiked' commentChangedPayload: x-origin: ./schemas.yaml#/commentChangedPayload type: object title: commentChangedPayload additionalProperties: false properties: commentId: $ref: '#/components/schemas/commentCountChange' likeCount: $ref: '#/components/schemas/commentCountChange' commentLikedPayload: x-origin: ./schemas.yaml#/commentLikedPayload type: object title: commentLikedPayload additionalProperties: false properties: commentId: $ref: '#/components/schemas/commentLiked' messages: commentChanged: x-origin: ./messages.yaml#/commentChanged description: Message that is being sent when a comment have been updated. payload: $ref: '#/components/schemas/commentChangedPayload' commentLiked: x-origin: ./messages.yaml#/commentLiked description: Message that is being sent when a comment has been liked by someone. payload: $ref: '#/components/schemas/commentLikedPayload' operations: receiveCommentLiked: action: receive channel: $ref: '#/channels/commentLiked' messages: - $ref: '#/channels/commentLiked/messages/commentLiked' sendCommentChange: action: send channel: $ref: '#/channels/commentCountChange' messages: - $ref: '#/channels/commentCountChange/messages/commentChanged' channels: commentCountChange: address: comment/{commentId}/changed messages: commentChanged: $ref: '#/components/messages/commentChanged' description: >- Sends the new count to the broker after it has been updated in the database. parameters: commentId: $ref: '#/components/schemas/commentCountChange' commentLiked: address: comment/liked messages: commentLiked: $ref: '#/components/messages/commentLiked' description: >- Updates the likes count in the database and sends the new count to the broker. servers: mosquitto: host: test.mosquitto.org protocol: mqtt tags: - name: env:production description: This environment is meant for production use case - name: kind:remote description: This server is a remote server. Not exposed by the application - name: visibility:public description: This resource is public and available to everyone bindings: mqtt: clientId: comment-service ```

then I turned off schemas, received

this ```yaml asyncapi: 3.0.0 info: title: Comments Service version: 1.0.0 description: This service is in charge of processing all the events related to comments. servers: mosquitto: $ref: '#/components/servers/mosquitto' channels: commentLiked: $ref: '#/components/channels/commentLiked' commentCountChange: $ref: '#/components/channels/commentCountChange' operations: receiveCommentLiked: $ref: '#/components/operations/receiveCommentLiked' sendCommentChange: $ref: '#/components/operations/sendCommentChange' components: messages: commentChanged: x-origin: ./messages.yaml#/commentChanged description: Message that is being sent when a comment have been updated. payload: x-origin: ./schemas.yaml#/commentChangedPayload type: object title: commentChangedPayload additionalProperties: false properties: commentId: allOf: - type: string - description: >- Id of the comment that was changed, such as when someone liked it. likeCount: type: integer description: The new like count of how many have liked the comment. commentLiked: x-origin: ./messages.yaml#/commentLiked description: Message that is being sent when a comment has been liked by someone. payload: x-origin: ./schemas.yaml#/commentLikedPayload type: object title: commentLikedPayload additionalProperties: false properties: commentId: allOf: - type: string - description: Id of the comment that was liked operations: receiveCommentLiked: action: receive channel: $ref: '#/channels/commentLiked' messages: - $ref: '#/channels/commentLiked/messages/commentLiked' sendCommentChange: action: send channel: $ref: '#/channels/commentCountChange' messages: - $ref: '#/channels/commentCountChange/messages/commentChanged' channels: commentCountChange: address: comment/{commentId}/changed messages: commentChanged: $ref: '#/components/messages/commentChanged' description: >- Sends the new count to the broker after it has been updated in the database. parameters: commentId: x-origin: ./parameters.yaml#/commentId description: ID of the comment commentLiked: address: comment/liked messages: commentLiked: $ref: '#/components/messages/commentLiked' description: >- Updates the likes count in the database and sends the new count to the broker. servers: mosquitto: host: test.mosquitto.org protocol: mqtt tags: - name: env:production description: This environment is meant for production use case - name: kind:remote description: This server is a remote server. Not exposed by the application - name: visibility:public description: This resource is public and available to everyone bindings: mqtt: clientId: comment-service ```

and I have a feeling that users would like the latter more.

Thus, should schemas be turned off by default, and users would need to specifically ENABLE them, for example, with the command-line switch --include-schemas? It may be the opposite way around, but I believe that the possibility of changing the appearance/invisibility of schemas should definitely exist.

KhudaDad414 commented 8 months ago

@aeworxet I think this is a separate issue that we need to discuss for Optimiser. can you open an issue on the Optimizer repo and let's discuss it there?

aeworxet commented 7 months ago

Bounty Issue bundler#141 will be closed through:

  1. The merging of https://github.com/asyncapi/optimizer/pull/216 and the release of Optimizer v1.0.0
  2. AFTER the release of Optimizer v1.0.0, the merging of https://github.com/asyncapi/bundler/pull/147
KhudaDad414 commented 7 months ago

@asyncapi/bounty_team

With the merging of https://github.com/asyncapi/optimizer/pull/216 and https://github.com/asyncapi/bundler/pull/147 this issue has been solved and the features is working as expected, thanks @aeworxet.

aeworxet commented 7 months ago

According to the user's real-world testing, the code in PRs https://github.com/asyncapi/bundler/pull/147 and https://github.com/asyncapi/optimizer/pull/216 works flawlessly.

aeworxet commented 7 months ago

There are no immediate bugs opened on the newly added functionality, so this issue is completed.

aeworxet commented 7 months ago

After a technical delay with merges of the PRs, which was beyond the control of the Bounty Program Participant https://github.com/asyncapi/bundler/issues/141#issuecomment-2049268061 https://github.com/asyncapi/optimizer/pull/216#issuecomment-2047390804 https://github.com/asyncapi/bundler/pull/147#issuecomment-2049210720

Bounty Issue Completed 🎉

@aeworxet, please go to the AsyncAPI page on Open Collective and submit an invoice for USD 400.00 with the expense title Bounty bundler#141, tag bounty, and full URL of this Bounty Issue in the description.