pb33f / libopenapi

libopenapi is a fully featured, high performance OpenAPI 3.1, 3.0 and Swagger parser, library, validator and toolkit for golang applications.
https://pb33f.io/libopenapi/
Other
362 stars 46 forks source link

render empty oauth scopes #290

Closed khart-twilio closed 3 weeks ago

khart-twilio commented 3 weeks ago

see https://github.com/pb33f/libopenapi/issues/289

This is my naive attempt to fix the issue. It works but it may have negative side-effects I'm not aware of.

khart-twilio commented 3 weeks ago

@daveshanley One edge case I noticed when testing:

I had:

...
      requestBody:
        content:
          application/json:
            schema:
              type: array
              items:
                type: object
                properties:
                  value:
                    type: null # null isn't allowed here I don't think according to the spec

And with these changes in libopenapi it is now being processed and spit out as

...
                properties:
                  value:
                    type: "ObjectNode"

It doesn't matter for this case since using null was invalid and I changed it to something else, but I'm wondering if it might matter in other places. I'm not familiar with how things like null, true, false come through as yaml.Node object when deserialized and if I'd need to handle these cases in the changes.

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.62%. Comparing base (0e74598) to head (2ba004f).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #290 +/- ## ======================================= Coverage 99.62% 99.62% ======================================= Files 162 162 Lines 16075 16077 +2 ======================================= + Hits 16014 16016 +2 Misses 56 56 Partials 5 5 ``` | [Flag](https://app.codecov.io/gh/pb33f/libopenapi/pull/290/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pb33f) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/pb33f/libopenapi/pull/290/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pb33f) | `99.62% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pb33f#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

daveshanley commented 3 weeks ago

@daveshanley One edge case I noticed when testing:

I had:

...
      requestBody:
        content:
          application/json:
            schema:
              type: array
              items:
                type: object
                properties:
                  value:
                    type: null # null isn't allowed here I don't think according to the spec

And with these changes in libopenapi it is now being processed and spit out as

...
                properties:
                  value:
                    type: "ObjectNode"

It doesn't matter for this case since using null was invalid and I changed it to something else, but I'm wondering if it might matter in other places. I'm not familiar with how things like null, true, false come through as yaml.Node object when deserialized and if I'd need to handle these cases in the changes.

I have no idea where ObjectNode is coming from. It's not part of libopenapi

Because I have no idea, let's just proceed, if someone else finds this issue then we can investigate further.