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

feat!: resolve $refs relative to document #179

Closed
wants to merge 3 commits into from

Conversation

Laupetin
Copy link

@Laupetin Laupetin commented Jul 17, 2024

Description

This implements the behaviour describes in #178 as a proposal for an implementation, in case the related change is accepted.

This does change the default behaviour to resolve $refs relative to the document which is what the CLI would do as well.
For details look into the linked issue.

Alternatively it would also be possible to do this in a non-breaking way by only applying the changed behaviour when an option like --path-relative-to-document is specified, if that would be prefered.

Related issue(s)
Resolves #178

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 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.

@Laupetin Laupetin changed the title feat: resolve $refs relative to document feat!: resolve $refs relative to document Jul 17, 2024
@aeworxet
Copy link
Collaborator

@Laupetin
Please share files, structured the way you mentioned in #178 as an archive, a comment, or a separate branch in https://github.com/Laupetin/asyncapi-bundler
Anonymized, but as close to real as possible (or use one of the examples to keep the structure completely generic,) before we move further with this PR.

@Laupetin
Copy link
Author

Laupetin commented Jul 22, 2024

@aeworxet Hey there :) I did add a new branch with an approach similar to what i've tried (just using the streetlights demo documentation): https://github.com/Laupetin/asyncapi-bundler/tree/folder-structure-example

The example setup can:

  • Validate the individual asyncapi documents.
  • Generate markdown based on the individual asyncapi documents.
  • It cannot bundle them because the bundler does path resolving differently than the other commands.
  • It could generate html documentation based on the bundled document, if that worked.

I did use scripts to run the asyncapi CLI due to a lack of support for globbing in paths and the need to run some of these commands for each file individually.

As you will be able to notice: The bundler does work differently than all other tools when resolving paths which makes things especially weird when it is part of the CLI since that makes different subcommands of the CLI work differently when it comes to paths.

You could make the whole thing work for the bundler by for example changing the path of the ref in docs/asyncapi/receive/lightingMeasured/asyncapi.yaml from ./schema.json to docs/asyncapi/receive/lightingMeasured/asyncapi.yaml (which imho is a less intuitive way to think of the paths) to make it work there.
This does however break all other commands then.
Even when working with the baseDir option for the bundler there is no way to make this work for all tools.

@aeworxet
Copy link
Collaborator

#178 (comment)

@aeworxet
Copy link
Collaborator

Setup at https://github.com/Laupetin/asyncapi-bundler/tree/folder-structure-example was used for review.

Result of the code from this PR
'0': d
'1': o
'2': c
'3': s
'4': /
'5': a
'6': s
'7': 'y'
'8': 'n'
'9': c
'10': a
'11': p
'12': i
'13': /
'14': i
'15': 'n'
'16': d
'17': e
'18': x
'19': .
'20': 'y'
'21': a
'22': m
'23': l
asyncapi: 3.0.0
info:
  title: Light Turn On
  version: 1.0.0
channels:
  lightingMeasured:
    address: smartylighting/streetlights/1/0/event/{streetlightId}/lighting/measured
    messages:
      lightMeasured:
        name: lightMeasured
        title: Light measured
        summary: >-
          Inform about environmental lighting conditions of a particular
          streetlight.
        contentType: application/json
        payload:
          $schema: https://json-schema.org/draft/2020-12/schema
          type: object
          properties:
            lumens:
              type: integer
              minimum: 0
              description: Light intensity measured in lumens.
            sentAt:
              type: string
              format: date-time
              description: Date and time when the message was sent.
    description: The topic on which measured values may be produced and consumed.
    parameters:
      streetlightId:
        description: The ID of the streetlight.
  lightTurnOff:
    address: smartylighting/streetlights/1/0/action/{streetlightId}/turn/off
    messages:
      turnOff:
        name: turnOnOff
        title: Turn on/off
        summary: Command a particular streetlight to turn the lights on or off.
        payload:
          $schema: https://json-schema.org/draft/2020-12/schema
          type: object
          properties:
            command:
              type: string
              enum:
                - 'on'
                - 'off'
              description: Whether to turn on or off the light.
            sentAt:
              type: string
              format: date-time
              description: Date and time when the message was sent.
  lightTurnOn:
    address: smartylighting/streetlights/1/0/action/{streetlightId}/turn/on
    messages:
      turnOn:
        name: turnOnOff
        title: Turn on/off
        summary: Command a particular streetlight to turn the lights on or off.
        payload:
          $schema: https://json-schema.org/draft/2020-12/schema
          type: object
          properties:
            command:
              type: string
              enum:
                - 'on'
                - 'off'
              description: Whether to turn on or off the light.
            sentAt:
              type: string
              format: date-time
              description: Date and time when the message was sent.
operations:
  receiveLightMeasurement:
    action: receive
    channel:
      $ref: '#/channels/lightingMeasured'
    summary: >-
      Inform about environmental lighting conditions of a particular
      streetlight.
    messages:
      - $ref: '#/channels/lightingMeasured/messages/lightMeasured'
  turnOff:
    action: send
    channel:
      $ref: '#/channels/lightTurnOff'
    messages:
      - $ref: '#/channels/lightTurnOff/messages/turnOff'
  turnOn:
    action: send
    channel:
      $ref: '#/channels/lightTurnOn'
    messages:
      - $ref: '#/channels/lightTurnOn/messages/turnOn'
components:
  messages:
    lightMeasured:
      name: lightMeasured
      title: Light measured
      summary: >-
        Inform about environmental lighting conditions of a particular
        streetlight.
      contentType: application/json
      payload:
        $schema: https://json-schema.org/draft/2020-12/schema
        type: object
        properties:
          lumens:
            type: integer
            minimum: 0
            description: Light intensity measured in lumens.
          sentAt:
            type: string
            format: date-time
            description: Date and time when the message was sent.
    turnOnOff:
      name: turnOnOff
      title: Turn on/off
      summary: Command a particular streetlight to turn the lights on or off.
      payload:
        $schema: https://json-schema.org/draft/2020-12/schema
        type: object
        properties:
          command:
            type: string
            enum:
              - 'on'
              - 'off'
            description: Whether to turn on or off the light.
          sentAt:
            type: string
            format: date-time
            description: Date and time when the message was sent.
Result of the code from https://github.com//issues/178#issuecomment-2257552102
asyncapi: 3.0.0
info:
  title: Streetlights MQTT API
  version: 1.0.0
  description: >
    The Smartylighting Streetlights API allows you to remotely manage the city
    lights.


    ### Check out its awesome features:


    * Turn a specific streetlight on/off 🌃

    * Dim a specific streetlight 😎

    * Receive real-time information about environmental lighting conditions 📈
  license:
    name: Apache 2.0
    url: https://www.apache.org/licenses/LICENSE-2.0
channels:
  lightingMeasured:
    address: smartylighting/streetlights/1/0/event/{streetlightId}/lighting/measured
    messages:
      lightMeasured:
        name: lightMeasured
        title: Light measured
        summary: >-
          Inform about environmental lighting conditions of a particular
          streetlight.
        contentType: application/json
        payload:
          $schema: https://json-schema.org/draft/2020-12/schema
          type: object
          properties:
            lumens:
              type: integer
              minimum: 0
              description: Light intensity measured in lumens.
            sentAt:
              type: string
              format: date-time
              description: Date and time when the message was sent.
    description: The topic on which measured values may be produced and consumed.
    parameters:
      streetlightId:
        description: The ID of the streetlight.
  lightTurnOff:
    address: smartylighting/streetlights/1/0/action/{streetlightId}/turn/off
    messages:
      turnOff:
        name: turnOnOff
        title: Turn on/off
        summary: Command a particular streetlight to turn the lights on or off.
        payload:
          $schema: https://json-schema.org/draft/2020-12/schema
          type: object
          properties:
            command:
              type: string
              enum:
                - 'on'
                - 'off'
              description: Whether to turn on or off the light.
            sentAt:
              type: string
              format: date-time
              description: Date and time when the message was sent.
  lightTurnOn:
    address: smartylighting/streetlights/1/0/action/{streetlightId}/turn/on
    messages:
      turnOn:
        name: turnOnOff
        title: Turn on/off
        summary: Command a particular streetlight to turn the lights on or off.
        payload:
          $schema: https://json-schema.org/draft/2020-12/schema
          type: object
          properties:
            command:
              type: string
              enum:
                - 'on'
                - 'off'
              description: Whether to turn on or off the light.
            sentAt:
              type: string
              format: date-time
              description: Date and time when the message was sent.
operations:
  receiveLightMeasurement:
    action: receive
    channel:
      $ref: '#/channels/lightingMeasured'
    summary: >-
      Inform about environmental lighting conditions of a particular
      streetlight.
    messages:
      - $ref: '#/channels/lightingMeasured/messages/lightMeasured'
  turnOff:
    action: send
    channel:
      $ref: '#/channels/lightTurnOff'
    messages:
      - $ref: '#/channels/lightTurnOff/messages/turnOff'
  turnOn:
    action: send
    channel:
      $ref: '#/channels/lightTurnOn'
    messages:
      - $ref: '#/channels/lightTurnOn/messages/turnOn'
components:
  messages:
    lightMeasured:
      name: lightMeasured
      title: Light measured
      summary: >-
        Inform about environmental lighting conditions of a particular
        streetlight.
      contentType: application/json
      payload:
        $schema: https://json-schema.org/draft/2020-12/schema
        type: object
        properties:
          lumens:
            type: integer
            minimum: 0
            description: Light intensity measured in lumens.
          sentAt:
            type: string
            format: date-time
            description: Date and time when the message was sent.
    turnOnOff:
      name: turnOnOff
      title: Turn on/off
      summary: Command a particular streetlight to turn the lights on or off.
      payload:
        $schema: https://json-schema.org/draft/2020-12/schema
        type: object
        properties:
          command:
            type: string
            enum:
              - 'on'
              - 'off'
            description: Whether to turn on or off the light.
          sentAt:
            type: string
            format: date-time
            description: Date and time when the message was sent.
defaultContentType: application/json
servers:
  production:
    host: test.mosquitto.org:{port}
    protocol: mqtt
    description: Test broker
    variables:
      port:
        description: Secure connection (TLS) is available through port 8883.
        default: '1883'
        enum:
          - '1883'
          - '8883'

The code from this PR also loses properties from original files

image

image

and introduces unnecessary breaking changes.

@Laupetin, would you like to replace the code in this PR with the code from #178 (comment) ?

Copy link

sonarcloud bot commented Jul 31, 2024

@Laupetin
Copy link
Author

Laupetin commented Aug 5, 2024

@aeworxet hey there, i was not available for a bit, now i had some time to test your suggestion.

I tested both your suggestion and the code from this PR, they result in identical files.
I am not entirely sure how the first result you posted came to be, it works perfectly fine for me.
The result also does not really look like something that could come from the changes i made, maybe there were some changes from other branches you had? 😅

Anyway, from reading the code you posted it does a similar thing, you prefer changing working directory on the top level instead of within the parse method.
I guess that works too, I'm not sure why you consider this an unnecessary breaking change, as far as I understood, the method is not available outside of bundler anyway, right?

I noticed two things that behave differently in your version:

  • The "base" file does not change directory in your version of the code. This works fine for the exact example I did in this case, because the index.yaml I used in my example does not have any $refs. In theory though resolving would still behave like it did before this PR for any file specified as base. So $refs in a base file would be resolved incorrectly.
  • Parsing is now done twice for all specified files (Once in line 115: readFile = await parse(readFile, getSpecVersion(readFile), options) and another time in line 138: const resolvedJsons: AsyncAPIObject[] = await resolve(). I'm not sure if that is necessarily introducing a bad behaviour, it is probably still unwanted.

I'd fix both of these things by moving the changing directory to the parse method but as far as I understood you don't want the behaviour to be in there?

@aeworxet
Copy link
Collaborator

aeworxet commented Aug 6, 2024

@Laupetin

Cloned clean, and yes, now the resulting AsyncAPI Document looks the same as the one merged manually (with a couple of technical differences,) my apologies.

I have performed strategic refactoring of the Bundler's code in #181, getting the codebase ready for several future implementations, and along the way added resolution of $refs in subdirectories.

So $refs in a base file would be resolved incorrectly.

Addressed this issue in #181. Now the base file can be dereferenced even if its path is specified outside of baseDir ({ base: '../index.yaml', baseDir: 'example-with-nested-dirs/asyncapi', }).

Parsing is now done twice for all specified files

Optimized process of parsing and bundling in #181.

Please, check if the code in #181 does what you need and whether you will have more remarks?

@Laupetin
Copy link
Author

Laupetin commented Aug 6, 2024

@aeworxet I tested the version of your PR and it works fine on my use case, the problem I had with paths previously is solved like that, thanks :)

@Laupetin
Copy link
Author

Laupetin commented Aug 8, 2024

solved in #181 -> closing this

@Laupetin Laupetin closed this Aug 8, 2024
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.

[FEATURE] Resolve $refs relative to the parsed file's directory by default
3 participants