[BUG] The bundle command does not bundle referenced files

[mfroberg]

Describe the bug.

The CLI tool now accepts asyncapi bundle for v3 documents (thanks to fix of issue #1137). However, it does not work as I expected, but of course I could be missing something.

I created a new default document with asyncapi new and moved the single message to a file in the same directory. Added a $ref in the main file. When I ran asyncapi bundle main.yaml on the new file, I expected it to merge the “message file”. But it didn’t.

main.yaml

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        $ref: '#/components/messages/UserSignedUp'
operations:
  onUserSignUp:
    action: receive
    channel:
      $ref: '#/channels/userSignedUp'
    messages:
      - $ref: '#/channels/userSignedUp/messages/UserSignedUp'
components:
  messages:
    UserSignedUp:
      $ref: './messages.yaml#/UserSignedUp'

messages.yaml

UserSignedUp:
  payload:
    type: object
    properties:
      displayName:
        type: string
        description: Name of the user
      email:
        type: string
        format: email
        description: Email of the user

Expected behavior

One yaml file containing the whole specification.

expected_bundle.yaml

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        $ref: '#/components/messages/UserSignedUp'
operations:
  onUserSignUp:
    action: receive
    channel:
      $ref: '#/channels/userSignedUp'
    messages:
      - $ref: '#/channels/userSignedUp/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

Screenshots

How to Reproduce

1. Extract main.yaml and messages.yaml to the same directory
2. In that directory, execute asyncapi bundle main.yaml

🥦 Browser

None

👀 Have you checked for similar open issues?

• I checked and didn't find similar issue

🏢 Have you read the Contributing Guidelines?

• I have read the Contributing Guidelines

Are you willing to work on this issue ?

None

[github-actions]

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.

[francocm]

I know this is not too helpful, but wanted to share that I am observing the same problem here as well, using asyncapi/cli:1.8.4. The referenced files remain referenced, and not bundled up.

[peter-rr]

@mfroberg @francocm
Thanks for reporting, I'm having a look at this 👀

[francocm]

@Amzani and @peter-rr I've raised a draft PR that replicates this problem: #1389

I can try giving a look later at the root cause too, but in the meantime I hope that's helpful.

Also, existing assertions that validated the output spec were not actually asserting for to.be.true.

[peter-rr]

@Souvikns @Shurtu-gal
The issue occurs just with v3 documents. I've been doing some research but haven't managed to find the root cause. Would you have any clue about it? 🤔

[francocm]

@peter-rr I've updated the related PR slightly regarding the way assertions are done, and also which referenced files etc are used for comparison.

I spent some time trying to get to the root cause, but I am suspecting it's originating from https://github.com/asyncapi/bundler. This repo uses 0.4.0. I noticed that 0.5.0 is available, but the upgrade to 0.5.0 is not straightforward. A lot of tests started failing after upgrading, as well as I noticed that 0.5.0 changed the signature and configuration options.

So this requires deeper investigation and also a plan for upgrade.

[peter-rr]

Hey @francocm , thanks for the updates on the PR 🙌 I'm having a look at them!

Yeah, the root cause must be definitely at the @asyncapi/bundler repo. Regarding the latest available version 0.5.0, it should be correctly updated at CLI repo since last week but there might be an issue related to the bot in charge of bumping to latest versions. Here is the info I have about it so you can take a look 👀 @Souvikns @Shurtu-gal @Amzani
I can also create a PR to update the version manually in case the auto-update is not working properly.

[Shurtu-gal]

Hey @peter-rr I am the one who reverted it. The bundler has to be migrated separately as there were some changes made in 0.5.0 due to which the bundle.command was not working properly.

Ref: #1372

[peter-rr]

Cool @Shurtu-gal 👍 Thanks for the info!
Let me know if you need some help in order to solve the issue :)

[francocm]

I have pulled from upstream into https://github.com/francocm/asyncapi-cli/tree/fix/1323/bundleNotBundlingExternalFiles

and went from 2 failing tests under test/integration/bundle/bundle.test.ts to 20, with the errors now including:

  20) validate
       with no context file
         throws error message if no context file exists:

      AssertionError: expected 'ContextError: No context is set as cu…' to equal 'error locating AsyncAPI document: The…'
      + expected - actual

      -ContextError: No context is set as current, please set a current context.
      +error locating AsyncAPI document: These are your options to specify in the CLI what AsyncAPI file should be used:
      + - You can provide a path to the AsyncAPI file: asyncapi <command> path/to/file/asyncapi.yml
      + - You can provide URL to the AsyncAPI file: asyncapi <command> https://example.com/path/to/file/asyncapi.yml
      + - You can also pass a saved context that points to your AsyncAPI file: asyncapi <command> context-name
      + - In case you did not specify a context that you want to use, the CLI checks if there is a default context and uses it. To set default context run: asyncapi config context use mycontext
      + - In case you did not provide any reference to AsyncAPI file and there is no default context, the CLI detects if in your current working directory you have files like asyncapi.json, asyncapi.yaml, asyncapi.yml. Just rename your file accordingly.
      +
      
      at Context.<anonymous> (test/integration/validate.test.ts:164:31)
      at Object.run (node_modules/fancy-test/lib/base.js:44:38)
      at Context.run (node_modules/fancy-test/lib/base.js:68:36)

It looks like this is a result of some error messaging/handling changes.

Nonetheless, with the state of 'broken' tests currently in master, these were not detected. I can refactor the tests to align with the updated output, but:

1. I have not spent enough time in the recent changes to know for sure I'm not simply working around the problem rather than solving it.
2. We would still end up with 2 broken tests (this bug), that the updated test/integration/bundle/bundle.test.ts captures.

I can't spend too much time on it right now, but happy to do any small changes in my branch if you wish.

Any thoughts @peter-rr @Shurtu-gal

[francocm]

Thanks @aeworxet this issue seems to be resolved through your change.

I've noticed a new potential gap, which I've detailed in a comment in my tests PR here: #1389 (comment)

[peter-rr]

@francocm @mfroberg
After the @asyncapi/bundler dependency upgrade, the file we are getting when running asyncapi bundle main.yaml looks like:

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        payload:
          type: object
          properties:
            displayName:
              type: string
              description: Name of the user
            email:
              type: string
              format: email
              description: Email of the user
operations:
  onUserSignUp:
    action: receive
    channel:
      $ref: '#/channels/userSignedUp'
    messages:
      - $ref: '#/channels/userSignedUp/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

Could you confirm this is the expected bundled file? I'm not sure since the messages.yaml content is shown duplicated, meaning the referenced object has been dereferenced twice after bundle process 🤔

[aeworxet]

@peter-rr
Hey. Me again. 👋
I modified Optimizer (v1.0.0+) some time ago (simultaneously with Bundler v0.5.0) exactly for these cases.
After Optimizer v1.0.2 (current master) with

{
  output: 'YAML',
  rules: {
    reuseComponents: true,
    removeComponents: true,
    moveAllToComponents: true,
    moveDuplicatesToComponents: false,
  },
  disableOptimizationFor: {
    schema: true,
  },
}

the output file looks like this:

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  userSignedUp:
    $ref: '#/components/channels/userSignedUp'
operations:
  onUserSignUp:
    $ref: '#/components/operations/onUserSignUp'
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
  operations:
    onUserSignUp:
      action: receive
      channel:
        $ref: '#/channels/userSignedUp'
      messages:
        - $ref: '#/channels/userSignedUp/messages/UserSignedUp'
  channels:
    userSignedUp:
      address: user/signedup
      messages:
        UserSignedUp:
          $ref: '#/components/messages/UserSignedUp'

Does this suit your needs better?

@KhudaDad414
How would the above option object look in the form of a command for asyncapi optimize? I haven't delved into that code yet.

[KhudaDad414]

@aeworxet currently there is no way to ignore schemas in CLI. I have opened an issue here: #1432

btw, I don't think it's a good solution for the issue that @peter-rr reported. #1323 (comment)

The problem is that the bundler de-references the local reference in channels/userSignedUp/messages/UserSignedUp. not sure If this is an expected behaviour.

[aeworxet]

@KhudaDad414

The problem is that the bundler de-references the local reference in channels/userSignedUp/messages/UserSignedUp. not sure If this is an expected behaviour.

I was going from the v3.0.0 Spec: channels/channel/messages/message is supposed to be dereferenced. 
Bundler does this and $ref: '#/channels/userSignedUp/messages/UserSignedUp' is pointing to the correct location at that time.
Then Optimizer moves channels/userSignedUp to components but doesn't add components to the $ref, which now should be $ref: '#/components/channels/userSignedUp/messages/UserSignedUp'.

What else should be changed after this issue is fixed?

currently there is no way to ignore schemas in CLI.

Maybe format of

disableOptimizationFor: {
  schema: true,
},

should be rethought in a way that makes both options (in standalone and CLI) consistent?

Like

{ disableOptimizationForSchema: true } // in the standalone Optimizer

--disableOptimizationForSchema // in CLI's command line, as the very presence of the cmd switch already
                               // means 'true' and its absence - 'false'

{ disableOptimizationForSchema: flags.disableOptimizationForSchema } // in CLI's 'optimize.ts'

?

[aeworxet]

The issue with updating the $refs of the AsyncAPI Document after moving the AsyncAPI components to the components section by prefixing the corresponding JSON Pointers with the characters components/ will be solved by asyncapi/optimizer#263.

[KhudaDad414]

I was going from the v3.0.0 Spec: channels/channel/messages/message is supposed to be dereferenced.

@aeworxet if we leave the optimizer out of the picture for now, which of these options should be the output of the bundler?

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        payload:
          type: object
          properties:
            displayName:
              type: string
              description: Name of the user
            email:
              type: string
              format: email
              description: Email of the user
operations:
  onUserSignUp:
    action: receive
    channel:
      $ref: '#/channels/userSignedUp'
    messages:
      - $ref: '#/channels/userSignedUp/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

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        $ref: '#/components/messages/UserSignedUp'
operations:
  onUserSignUp:
    action: receive
    channel:
      $ref: '#/channels/userSignedUp'
    messages:
      - $ref: '#/channels/userSignedUp/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

I think option 2 should be the result. as a user of bundler I don't expect bundler to de-reference my local references. If there is disagreement here, I think we should discuss this in a separate issue and see what the community wants.

should be rethought in a way that makes both options (in standalone and CLI) consistent?

Agreed.

[aeworxet]

I'll ask here since this issue already has the intended audience.

Should there be a separate mode in Bundler that will dereference only external $refs, leaving internal $refs intact (previous behavior?)

[KhudaDad414]

Should there be a separate mode in Bundler that will dereference only external $refs, leaving internal $refs intact (previous behavior?)

cc: @Souvikns @derberg @peter-rr @mfroberg @francocm

[mfroberg]

I think it is reasonable use case to just dereference external refs and leave local ones intact. My original need is to get rid of the external refs (by bundling that content) so that I am able to visualize the file using Async Studio web tool.

[peter-rr]

Should there be a separate mode in Bundler that will dereference only external $refs, leaving internal $refs intact (previous behavior?)

IMO, that should be the default behaviour. In case we want to de-reference also the local refs, then we could add an option to bundle command for that purpose.

[aeworxet]

I will be able to implement bundle after the onBundle callback with the similar functionality as onDereference is introduced in @apidevtools/json-schema-ref-parser (I still need to do stuff after bundling and keep cmd switches consistent across versions.)