asyncapi / bundler

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

Ref's not resolved in v3 documents #151

Closed SnakeDoc closed 7 months ago

SnakeDoc commented 7 months ago

https://github.com/asyncapi/bundler/blob/f4cc6d91820a1e672f7b05df9e8f9473fbc576b9/src/v3/parser.ts#L101

Referencing the linked line - is there a reason the bundler is not being used? As-is, resolving a v3 document only resolves references found within channels and operations. I might be missing something?

asyncapi: 3.0.0
info:
  $ref: './info.yaml#/info'
...

ends up outputting $ref: './info.yaml#/info' instead of resolving the value of the reference into the final document. The same goes for other parts of the v3 document, including tags, etc.

Switching the spec version to 2.6.0 does result in correctly resolved references (ignoring differences between 2.x and 3.x specs).

main.yaml:

asyncapi: 3.0.0
info:
  $ref: './info.yaml#/info'
channels:
  userSignedup:
    address: 'user/signedup'
    messages:
      userSignedUpMessage:
        $ref: './messages.yaml#/messages/UserSignedUp'
  test:
    address: '/test'
    messages:
      testMessage:
        $ref: '#/components/messages/TestMessage'
operations:
  UserSignedUp:
    action: send
    channel:
      $ref: '#/channels/userSignedup'
    messages:
      - $ref: './messages.yaml#/messages/UserSignedUp'
  TestOpp:
    action: send
    channel:
      $ref: '#/channels/test'
    messages:
      - $ref: '#/components/messages/TestMessage'
components:
  messages:
    TestMessage:
      payload:
        type: string

messages.yaml:

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

info.yaml:

info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signupsA

expected output:

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signupsA
channels:
  userSignedup:
    address: user/signedup
    messages:
      userSignedUpMessage:
        $ref: '#/components/messages/UserSignedUp'
  test:
    address: /test
    messages:
      testMessage:
        $ref: '#/components/messages/TestMessage'
operations:
  UserSignedUp:
    action: send
    channel:
      $ref: '#/channels/userSignedup'
    messages:
      - $ref: '#/components/messages/UserSignedUp'
  TestOpp:
    action: send
    channel:
      $ref: '#/channels/test'
    messages:
      - $ref: '#/components/messages/TestMessage'
components:
  messages:
    TestMessage:
      payload:
        type: string
    UserSignedUp:
      payload:
        type: object
        properties:
          displayName:
            type: string
            description: Name of the user
          email:
            type: string
            format: email
            description: Email of the user

actual output:

asyncapi: 3.0.0
info:
  $ref: ./info.yaml#/info    # <-- unresolved ref
channels:
  userSignedup:
    address: user/signedup
    messages:
      userSignedUpMessage:
        $ref: '#/components/messages/UserSignedUp'
  test:
    address: /test
    messages:
      testMessage:
        $ref: '#/components/messages/TestMessage'
operations:
  UserSignedUp:
    action: send
    channel:
      $ref: '#/channels/userSignedup'
    messages:
      - $ref: '#/components/messages/UserSignedUp'
  TestOpp:
    action: send
    channel:
      $ref: '#/channels/test'
    messages:
      - $ref: '#/components/messages/TestMessage'
components:
  messages:
    TestMessage:
      payload:
        type: string
    UserSignedUp:
      payload:
        type: object
        properties:
          displayName:
            type: string
            description: Name of the user
          email:
            type: string
            format: email
            description: Email of the user

the following test fails when run against the above files:

test('should be able to bundle v3 files', async () => {
    const files = ['./main.yaml']
    const response = await bundle(
        files.map(file => fs.readFileSync(path.resolve(process.cwd(), file), 'utf-8'))
    )
    const expected = fs.readFileSync(path.resolve(process.cwd(), './bundled.yaml'), 'utf-8');
    expect(response.yml()).toBe(expected);
  });
github-actions[bot] commented 7 months 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.

SnakeDoc commented 7 months ago

97, #141 and #147 seem related. can we allow resolving all refs in the meantime, even if x-origin support isn't quite there yet? we do have this behavior in 2.x, and it's surprising 3.x does not have it too.

aeworxet commented 7 months ago

I am on this issue, but at the same time, the validate() function of parser-js gives on validation of both 'expected' and 'actual' output error:

[
  {
    code: 'asyncapi3-operation-messages-from-referred-channel',
    message: 'Operation message does not belong to the specified channel.',
    path: [ 'operations', 'UserSignedUp', 'messages', '0' ],
    severity: 0,
    range: { start: [Object], end: [Object] }
  },
  {
    code: 'asyncapi3-operation-messages-from-referred-channel',
    message: 'Operation message does not belong to the specified channel.',
    path: [ 'operations', 'TestOpp', 'messages', '0' ],
    severity: 0,
    range: { start: [Object], end: [Object] }
  }
]

So, while I'm checking from my side, I would please ask you to simultaneously check the AsyncAPI Document's format from your side.

aeworxet commented 7 months ago

@SnakeDoc

Can I please ask you to be a beta-tester of my PR a bit?

Do

$ mkdir test
$ cd test/
$ git clone https://github.com/aeworxet/asyncapi-bundler.git .
$ git switch feat-add-x-origin-property      #  <-- essential, because "@apidevtools/json-schema-ref-parser" has another version in this branch
$ npm install
$ tsc
$ cd example/
$ npm install

# add your testing YAMLs

$ node bundle-cjs.cjs

and tell if the PR's code outputs to asyncapi.yaml what you need.

If the resulting (dereferenced) AsyncAPI Document doesn't pass validation, it outputs an error, similar to

Validation of the resulting AsyncAPI Document failed.
List of remarks:
 [
  {
    code: 'asyncapi3-operation-messages-from-referred-channel',
    message: 'Operation message does not belong to the specified channel.',
    path: [ 'operations', 'UserSignedUp', 'messages', '0' ],
    severity: 0,
    range: { start: [Object], end: [Object] }
  },
  {
    code: 'asyncapi3-operation-messages-from-referred-channel',
    message: 'Operation message does not belong to the specified channel.',
    path: [ 'operations', 'TestOpp', 'messages', '0' ],
    severity: 0,
    range: { start: [Object], end: [Object] }
  }
]

and exits without writing to asyncapi.yaml.

Souvikns commented 7 months ago

Currently, bundler is only resolving the references in channels and operations, I think that is why info references are not getting resolved.

SnakeDoc commented 7 months ago

Thank you @aeworxet - great work on this feature so far. I've been playing with your fork and find it works well for me.

I did have a few errors in my original files that caused the validation issues you pointed out.

I'm attaching new files that pass validation, and resolve correctly using your beta bundler.

main.yaml:

asyncapi: 3.0.0
info:
  $ref: './info.yaml#/info'
channels:
  userSignedup:
    address: 'user/signedup'
    messages:
      userSignedUpMessage:
        $ref: './messages.yaml#/messages/UserSignedUp'
  test:
    address: '/test'
    messages:
      testMessage:
        $ref: '#/components/messages/TestMessage'
operations:
  UserSignedUp:
    action: send
    channel:
      $ref: '#/channels/userSignedup'
    messages:
      - $ref: '#/channels/userSignedup/messages/userSignedUpMessage'
  TestOpp:
    action: send
    channel:
      $ref: '#/channels/test'
    messages:
      - $ref: '#/channels/test/messages/testMessage'
components:
  messages:
    TestMessage:
      payload:
        type: string

info.yaml:

info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups

messages.yaml:

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

I have only been validating my files after bundling, which is why I didn't notice the validation errors before posting. My view has been the multi-file spec is treated as "source" and the bundler produces the final output. I can see flaws in this logic though, and do agree it is better to have validation at each step. I assume asyncapi is promoting a src -> bundler -> optimizer workflow, with validation each step of the way.

I, however, was not able to resolve references to entire channel and operation objects, due to how the bundler resolves objects in-line instead of moving them to the components section for reuse. It would be very convenient to be able to do something like:

main.yaml:

asyncapi: 3.0.0
info:
  $ref: './info.yaml#/info'
channels:
  userLoggedIn:
    $ref: './channels.yaml#/channels/userLoggedIn'
operations:
  UserLoggedIn:
    $ref: './operations.yaml#/operations/UserLoggedIn'

info.yaml:

info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups

messages.yaml:

messages:
  UserLoggedIn:
    payload:
      type: object
      properties:
        id:
          type: string

channels.yaml:

channels:
  userLoggedIn:
    address: 'user/loggedin'
    messages:
      userLoggedInMessage:
        $ref: './messages.yaml#/messages/UserLoggedIn'

operations.yaml

operations:
  UserLoggedIn:
    action: send
    channel:
      $ref: './channels.yaml#/channels/userLoggedIn'
    messages:
      - $ref: './messages.yaml#/messages/UserLoggedIn'

The above gets a validation error:

Validation of the resulting AsyncAPI Document failed.
List of remarks:
 [
  {
    code: 'asyncapi3-operation-messages-from-referred-channel',
    message: 'Operation message does not belong to the specified channel.',
    path: [ 'operations', 'UserLoggedIn', 'messages', '0' ],
    severity: 0,
    range: { start: [Object], end: [Object] }
  }
]

Failed Document:

{
  "asyncapi": "3.0.0",
  "info": {
    "x-origin": "./info.yaml#/info",
    "title": "Account Service",
    "version": "1.0.0",
    "description": "This service is in charge of processing user signups"
  },
  "channels": {
    "userLoggedIn": {
      "x-origin": "./channels.yaml#/channels/userLoggedIn",
      "address": "user/loggedin",
      "messages": {
        "userLoggedInMessage": {
          "payload": {
            "type": "object",
            "properties": {
              "id": {
                "type": "string"
              }
            }
          }
        }
      }
    }
  },
  "operations": {
    "UserLoggedIn": {
      "x-origin": "./operations.yaml#/operations/UserLoggedIn",
      "action": "send",
      "channel": {
        "$ref": "./channels.yaml#/channels/userLoggedIn"
      },
      "messages": [
        {
          "$ref": "./messages.yaml#/messages/UserLoggedIn"
        }
      ]
    }
  }
}

This error seems to be due to bundler resolving objects in-line instead of moving them to the components section for reuse. You can see how the channel message gets resolved in-line, and the operation fails to resolve entirely.

Maybe optimizer takes care of this, but it would be nice to have it happen at bundling time so the source documents can be written more efficiently. Maybe I missed something though?

Might be wishful thinking, but it would be even more convenient to bulk reference, like:

main.yaml:

asyncapi: 3.0.0
info:
  $ref: './info.yaml#/info'
channels:
  $ref: './channels.yaml#/channels'
operations:
  $ref: './operations.yaml#/operations'

Overall, the beta bundler clearly seems headed in a great direction.

aeworxet commented 7 months ago

After completion of https://github.com/asyncapi/bundler/issues/141, the process Bundler->Optimizer, to obtain an AsyncAPI Document with components moved to components, will become mandatory in CI/code (about May 2024.)

Meanwhile, I'm gathering feedback during the development.

@SnakeDoc Can you please take a look at https://github.com/asyncapi/bundler/issues/141#issuecomment-2017469779 and tell which version of the AsyncAPI Document after Optimizer would serve your needs better, what behavior of Optimizer on schemas do you think should be default, and in what way would you prefer to toggle it? (I'm leaning to a command-line switch that will be parsed in code, assigned to a property of some options object, which then will be passed around the code to different functions)

SnakeDoc commented 7 months ago

@aeworxet Thanks for the follow-up. I've reviewed #141 and can offer a few thoughts:

1) Perhaps an issue for Optimizer, but I do not see value in retaining the x-origin property post optimization. It seems to be an artifact left over from previous "stages" of generating the final document. From my observation and limited perspective, it appears the output of bundler will not be readily useful for most people, and they will want to run their bundled document through the optimizer. x-origin may leak internal details about how the source of the document is structured, and doesn't seem provide useful information for codegen/end-users later on. This also probably means the CLI should automatically do bundle -> optimize unless the user opts-out with a flag or something, ie. --disable-optimization. Therefore my opinion is optimizer should consume/strip x-origin before output of the final document.

2) I must preface that my experience comes from the OpenAPI side, which may bias my opinion - but I actually like the components/schema option better. In my opinion, schemas are the smallest, reusable component within the document, and should be used to construct more complex objects, such as messages, with channels/operations built on top of messages, etc.

Two different messages can share the same payload (schema), but have different headers, etc, or even be exactly the same. Duplicating and in-lining the payload definition seems unnecessary, and perhaps incorrectly hints that the two definitions are not the same object.

I think the primary audience for bundled & optimized documents is mostly code/documentation generators, and to a much lesser extent, actual people. Therefore, the verbosity and complex references produced using the schema option probably doesn't matter much in my opinion.

Further - from a document writer perspective (document source, pre-bundler and pre-optimizer), I want to clearly communicate my intentions, and re-use as much as possible wherever possible (DRY). Modularity of various components also greatly enhances the maintainability of larger documents as the api grows.

I would love to do something like:

main.yaml ```yaml asyncapi: 3.0.0 info: title: Example Service version: 1.0.0 description: Example Service. channels: commentLikedChannel: $ref: './channels.yaml#/channels/commentLikedChannel' ```
channels.yaml ```yaml channels: commentLikedChannel: address: comment/liked messages: commentLikedMessage: $ref: './messages.yaml#/messages/commentLikedMessage' ```
messages.yaml ```yaml messages: commentLikedMessage: description: Message that is being sent when a comment has been liked by someone. payload: $ref: './schemas.yaml#/schemas/commentLikedSchema' ```
schemas.yaml ```yaml schemas: commentLikedSchema: type: object title: commentLikedPayload properties: commentId: $ref: './schemas.yaml#/schemas/idSchema' idSchema: type: string description: an id object ```

and produce this after bundler -> optimizer:

asyncapi.yaml ```yaml asyncapi: 3.0.0 info: title: Example Service version: 1.0.0 description: Example Service. channels: commentLikedChannel: $ref: '#/components/channels/commentLikedChannel' components: channels: commentLikedChannel: address: comment/liked messages: commentLikedMessage: $ref: '#/components/messages/commentLikedMessage' messages: commentLikedMessage: description: Message that is being sent when a comment has been liked by someone. payload: $ref: '#/components/schemas/commentLikedSchema' schemas: commentLikedSchema: type: object title: commentLikedPayload properties: commentId: $ref: '#/components/schemas/idSchema' idSchema: type: string description: an id object ```
aeworxet commented 6 months ago

@SnakeDoc, thank you for the test case! I had run this scenario through Bundler -> Optimizer process with newly introduced switches

disableOptimizationFor: { schema: false } image

disableOptimizationFor: { schema: true } image

and your expectations meet everything except schemas (this behavior is yet to be changed in the future.)

@KhudaDad414 Can https://github.com/asyncapi/bundler/issues/151#issuecomment-2030436419 be a hint for https://github.com/asyncapi/optimizer/issues/88 ?

aeworxet commented 6 months ago

@SnakeDoc A bit improved mechanism meets your expectations even better. image

aeworxet commented 6 months ago

@SnakeDoc

x-origin property originated from this comment in a year-long discussion

The $ref usually also carries a semantical meaning to understand easier what it is (example "$ref : financial-system.yaml#/components/schemas/bankAccountIdentifier"). If the bundling just resolves this ref inline, the semantical meaning of the $ref pointer gets lost and cannot be recovered in later steps.

and is essential to mindful properties' naming during optimization with Optimizer

The optimizer would need to invent an artificial component name for the "bankAccountIdentifier" when moving it to the components section.

So x-origin needs to be there.

A switch might be introduced to remove it after the optimization or not to add it at all during bundling if a user doesn't need it, though; this point is completely valid.

aeworxet commented 6 months ago

@SnakeDoc, can you please check if it is understandable how to perform this process to achieve this result, using code from PRs https://github.com/asyncapi/bundler/pull/147 and https://github.com/asyncapi/optimizer/pull/216 ? (@apidevtools/json-schema-ref-parser's version was changed in Bundler again)

SnakeDoc commented 6 months ago

@aeworxet excellent work on both #147 and asyncapi/optimizer#216!

With latest aeworxet/asyncapi-bundler:feat-add-x-origin-property, I was able to produce the following using the above example:

asyncapi.yaml ```yaml asyncapi: 3.0.0 info: title: Example Service version: 1.0.0 description: Example Service. channels: commentLikedChannel: address: comment/liked messages: commentLikedMessage: description: Message that is being sent when a comment has been liked by someone. payload: type: object title: commentLikedPayload properties: commentId: type: string description: an id object x-origin: ./schemas.yaml#/schemas/idSchema x-origin: ./schemas.yaml#/schemas/commentLikedSchema x-origin: ./messages.yaml#/messages/commentLikedMessage x-origin: ./channels.yaml#/channels/commentLikedChannel ```

With latest aeworxet/asyncapi-optimizer:feat-add-flag-moveAllToComponents, and the below configuration, the following optimized document was produced:

optimizer config ```json { "output": "YAML", "rules": { "reuseComponents": true, "removeComponents": true, "moveAllToComponents": true, "moveDuplicatesToComponents": false }, "disableOptimizationFor": { "schema": false } } ```
optimized.yaml ```yaml asyncapi: 3.0.0 info: title: Example Service version: 1.0.0 description: Example Service. channels: commentLikedChannel: $ref: '#/components/channels/commentLikedChannel' components: schemas: idSchema: type: string description: an id object x-origin: ./schemas.yaml#/schemas/idSchema commentLikedSchema: type: object title: commentLikedPayload properties: commentId: $ref: '#/components/schemas/idSchema' x-origin: ./schemas.yaml#/schemas/commentLikedSchema messages: commentLikedMessage: description: Message that is being sent when a comment has been liked by someone. payload: $ref: '#/components/schemas/commentLikedSchema' x-origin: ./messages.yaml#/messages/commentLikedMessage channels: commentLikedChannel: address: comment/liked messages: commentLikedMessage: $ref: '#/components/messages/commentLikedMessage' x-origin: ./channels.yaml#/channels/commentLikedChannel ```

Which makes me quite happy! :+1:

Disabling the schema optimization with "disableOptimizationFor": { "schema": true } yields:

schema-false.yaml ```yaml asyncapi: 3.0.0 info: title: Example Service version: 1.0.0 description: Example Service. channels: commentLikedChannel: $ref: '#/components/channels/commentLikedChannel' components: messages: commentLikedMessage: description: Message that is being sent when a comment has been liked by someone. payload: type: object title: commentLikedPayload properties: commentId: type: string description: an id object x-origin: ./schemas.yaml#/schemas/idSchema x-origin: ./schemas.yaml#/schemas/commentLikedSchema x-origin: ./messages.yaml#/messages/commentLikedMessage channels: commentLikedChannel: address: comment/liked messages: commentLikedMessage: $ref: '#/components/messages/commentLikedMessage' x-origin: ./channels.yaml#/channels/commentLikedChannel ```

All of the above validates with @asyncapi/cli@1.8.3.

Coupled with the other newly available optimizer configurations, I imagine this covers just about it all. Very good work!

aeworxet commented 6 months ago

@SnakeDoc Thanks for the testing! The PRs https://github.com/asyncapi/bundler/pull/147 and https://github.com/asyncapi/optimizer/pull/216 are now known as Bundler v0.5.0 and Optimizer v1.0.0, respectively.

aeworxet commented 6 months ago

There was a baseDir option reimplemented in the last minute. You can read about how it works now at https://github.com/asyncapi/bundler/blob/master/README.md#option-basedir and observe it in action in ./example/bundle-cjs.cjs.