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

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

Open
2 tasks done
KhudaDad414 opened this issue May 1, 2024 · 8 comments
Open
2 tasks done

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

KhudaDad414 opened this issue May 1, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@KhudaDad414
Copy link
Member

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?

  • I checked and didn't find similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to work on this issue ?

No, someone else can work on it

@KhudaDad414 KhudaDad414 added the bug Something isn't working label May 1, 2024
@KhudaDad414
Copy link
Member Author

@aeworxet any idea why this is happening?

@aeworxet
Copy link
Collaborator

aeworxet commented May 1, 2024

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

@aeworxet yes.

@aeworxet
Copy link
Collaborator

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

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

Then the list of issues in #261 (comment) 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
Copy link
Member Author

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants