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

Refine AsyncAPI Bundling with Origin Tracing and Component Naming #141

Closed
KhudaDad414 opened this issue Nov 8, 2023 · 34 comments · Fixed by #147 or asyncapi/optimizer#213
Closed
Assignees
Labels
bounty enhancement New feature or request

Comments

@KhudaDad414
Copy link
Member

KhudaDad414 commented Nov 8, 2023

Context

#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:

  • Identify the areas of code where the referenceIntoComponents flag is utilized.
  • Ensure that the feature is decoupled without affecting other functionalities.
  • Remove the feature and its flag entirely from the codebase.

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.

@KhudaDad414 KhudaDad414 added the enhancement New feature or request label Nov 8, 2023
Copy link

github-actions bot commented Nov 8, 2023

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.

@KhudaDad414 KhudaDad414 changed the title remove the referenceIntoComponents feature Refine AsyncAPI Bundling with Origin Tracing and Component Naming Nov 8, 2023
@aeworxet
Copy link
Collaborator

@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
Copy link

thake commented Nov 28, 2023

@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
Copy link
Collaborator

@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
Copy link

thake commented Nov 29, 2023

Correct, thanks for the asyncAPI example

@aeworxet
Copy link
Collaborator

@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
Copy link
Member Author

@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
Copy link
Collaborator

aeworxet commented Dec 1, 2023

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
Copy link
Collaborator

aeworxet commented Dec 6, 2023

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
Copy link
Collaborator

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
Copy link
Member Author

@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
Copy link
Collaborator

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
Copy link
Member

derberg commented Dec 14, 2023

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
Copy link
Member

derberg commented Dec 18, 2023

@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
Copy link
Member Author

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

@aeworxet
Copy link
Collaborator

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
Copy link
Collaborator

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

@aeworxet aeworxet self-assigned this Dec 18, 2023
@aeworxet
Copy link
Collaborator

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
Copy link
Collaborator

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.

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

@KhudaDad414
Copy link
Member Author

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
Copy link
Collaborator

@asyncapi/bounty_team

@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@asyncapi asyncapi deleted a comment from asyncapi-bot Apr 22, 2024
@aeworxet
Copy link
Collaborator

According to the user's real-world testing, the code in PRs #147 and asyncapi/optimizer#216 works flawlessly.

@aeworxet
Copy link
Collaborator

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

@aeworxet
Copy link
Collaborator

After a technical delay with merges of the PRs, which was beyond the control of the Bounty Program Participant
#141 (comment)
asyncapi/optimizer#216 (comment)
#147 (comment)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty enhancement New feature or request
Projects
Status: Completed
4 participants