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

Prevent crash in createExample when a multi variant type is being parsed. All variants are concatinated with '|'. Also added a "null" type wich is not the """null""" type, made sure to extra check against this. #267

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

brymko
Copy link

@brymko brymko commented Dec 5, 2023

While running this project on our API generated OAPI spec i found it to be crashing when running into variant types. For instance incase there is an optional field:

"properties": {
  "is_active": {
    "type": [
      "boolean",
      "null"
    ]
  }

This merge is forwarding my local fix

…sed. All variants are concatinated with '|'. Also added a "null" type wich is not the """null""" type, made sure to extra check against this.
Copy link

netlify bot commented Dec 5, 2023

Deploy Preview for docusaurus-openapi ready!

Name Link
🔨 Latest commit a604750
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-openapi/deploys/656eabb102fd86000874c6d7
😎 Deploy Preview https://deploy-preview-267--docusaurus-openapi.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@brymko
Copy link
Author

brymko commented Dec 5, 2023

Im not sure if my default assumption of concating everyting with "|" together holds. Because this is only an example maybe we only want the first element ?|

As an example, this would be generated with the proposed changes:

curl -L -X GET 'some_path'' \
-H 'Content-Type: application/json' \
-H 'Accept: text/plain; charset=utf-8' \
-d '{
  "is_active": "true | null",
 }

Now this would obviously be rejected by the server, but so would all context aware data. And given that this should only server as a default i think it might be fine. Open for thoughts.

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.

None yet

1 participant