asyncapi / optimizer

AsyncAPI offers many different ways to reuse certain parts of the document like messages or schemas definitions or references to external files, not to even mention the traits. There is a need for a tool that can be plugged into any workflows and optimize documents that are generated from code, but not only.
Apache License 2.0
14 stars 9 forks source link

[BUG] moveAllComponents is not working as expected. #261

Open KhudaDad414 opened 5 months ago

KhudaDad414 commented 5 months ago

Describe the bug.

I tried to run the optimizer on this file:

asyncapi: 3.0.0
info:
  title: Account Service
  version: "1.0"
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        $ref: '#/components/messages/UserSignedUp'
components:
  messages:
    UserSignedUp:
      payload:
        type: object
        properties:
          email:
            type: string

It generated the following invalid document.

asyncapi: 3.0.0
info:
  title: Account Service
  version: '1.0'
channels:
  userSignedUp:
    $ref: '#/components/channels/userSignedUp'
components:
  messages:
    UserSignedUp:
      payload:
        type: object
        properties:
          email:
            type: string
  schemas: {}
  channels:
    userSignedUp:
      address: user/signedup
      messages:
        UserSignedUp:
          $ref: '#/components/messages/UserSignedUp'
          payload:
            properties:
              email:
                $ref: '#/components/schemas/email'

Expected behavior

It should have generated:

asyncapi: 3.0.0
info:
  title: Account Service
  version: "1.0"
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        $ref: '#/components/messages/UserSignedUp'
components:
  schemas:
    email:
      type: string
  messages:
    UserSignedUp:
      payload:
        type: object
        properties:
          email: "$ref: #/components/schemas/email"

Screenshots

...

How to Reproduce

Run the optimiser on the given AsyncAPI file.

🥦 Browser

None

👀 Have you checked for similar open issues?

🏢 Have you read the Contributing Guidelines?

Are you willing to work on this issue ?

No, someone else can work on it

KhudaDad414 commented 5 months ago

@aeworxet any idea why this is happening?

aeworxet commented 5 months ago

Investigating.

If disableOptimizationFor: { schema: true, }, is set, is this output correct?

asyncapi: 3.0.0
info:
  title: Account Service
  version: '1.0'
channels:
  userSignedUp:
    $ref: '#/components/channels/userSignedUp'
components:
  messages:
    UserSignedUp:
      payload:
        type: object
        properties:
          email:
            type: string
  channels:
    userSignedUp:
      address: user/signedup
      messages:
        UserSignedUp:
          $ref: '#/components/messages/UserSignedUp'
KhudaDad414 commented 5 months ago

@aeworxet yes.

aeworxet commented 4 months ago

File (the one that issue was opened on)

asyncapi: 3.0.0
info:
  title: Account Service
  version: '1.0'
channels:
  userSignedUp:
    $ref: '#/components/channels/userSignedUp'
components:
  messages:
    UserSignedUp:
      payload:
        type: object
        properties:
          email:
            type: string
  schemas: {}
  channels:
    userSignedUp:
      address: user/signedup
      messages:
        UserSignedUp:
          $ref: '#/components/messages/UserSignedUp'
          payload:
            properties:
              email:
                $ref: '#/components/schemas/email'

passed validation with Parser.

Original file (everything that was in components was moved to the root of JSON)

asyncapi: 3.0.0
info:
  title: Account Service
  version: "1.0"
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        payload:
          type: object
          properties:
            email:
              type: string

was optimized as

asyncapi: 3.0.0
info:
  title: Account Service
  version: '1.0'
channels:
  userSignedUp:
    $ref: '#/components/channels/userSignedUp'
components:
  schemas:
    email:
      type: string
    payload:
      type: object
      properties:
        email:
          $ref: '#/components/schemas/email'
  messages:
    UserSignedUp:
      payload:
        $ref: '#/components/schemas/payload'
  channels:
    userSignedUp:
      address: user/signedup
      messages:
        UserSignedUp:
          $ref: '#/components/messages/UserSignedUp'

Thus, the issue is related to parsing components that are ALREADY in components.

KhudaDad414 commented 4 months ago

@aeworxet

File (the one that the issue was opened on) passed validation with Parser.

It's still invalid, right? I see two issues with it. 1) components/channels/userSignedUp/messages/userSignedUp contains both a reference and an object. 2) $ref: '#/components/schemas/email' is wrong since there is not components/schemas/email in the file.

aeworxet commented 4 months ago

Then the list of issues in https://github.com/asyncapi/optimizer/issues/261#issuecomment-2135024356 is a reason for @jonaslagoni and @smoya to review the code of parser-js once again and find out why Parser validates obviously invalid things, along with fixing.

KhudaDad414 commented 4 months ago

@aeworxet we can open an issue and discuss it on the respective repo but ultimately that is a different discussion and it's up to the parser-js maintainers to decide. For us, we have to make sure optimizer doesn't produce incorrect output.

jonaslagoni commented 4 months ago

Opening separate issues is definitely a good way of doing it ✌️