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

Index.GetAllSchemas() regression in values returned #280

Closed
TristanSpeakEasy opened this issue Apr 30, 2024 · 6 comments · Fixed by #281
Closed

Index.GetAllSchemas() regression in values returned #280

TristanSpeakEasy opened this issue Apr 30, 2024 · 6 comments · Fixed by #281

Comments

@TristanSpeakEasy
Copy link
Contributor

Between versions v0.15.14 and v0.16.2 there seems to have been a regression in what GetAllSchemas() from the index returns.

Basically we are seeing "schema" like objects under examples being returned which shouldn't be as they aren't schemas but just examples

Here is a spec that shows the issue https://raw.githubusercontent.com/codatio/oas/main/yaml/Codat-Lending.yaml

For example line 69399 is a object that is being returned in the list from GetAllSchemas()

@TristanSpeakEasy
Copy link
Contributor Author

Looks to have been introduced in v0.16.1

@TristanSpeakEasy
Copy link
Contributor Author

Specifically this commit 1e22e02

@TristanSpeakEasy
Copy link
Contributor Author

An example spec that returns more than the 1 schema actually included in it from GetAllSchemas()

openapi: 3.1.0
paths:
  '/test':
    get:
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object
              examples:
                test example:
                  value:
                    type: Object
                    description: test
                    properties:
                      lineItems:
                        type: Array
                        description: test
                        properties:
                          description:
                            required: false
                          taxRateRef:
                            type: Object
                            description: test
                            properties:
                              effectiveTaxRate:
                                type: Number
                                description: test
                                required: false
                            required: true
                      paymentAllocations:
                        type: Array
                        description: test
                        properties:
                          payment:
                            type: Object
                            description: test
                            properties:
                              accountRef:
                                type: Object

@TristanSpeakEasy
Copy link
Contributor Author

Looks like this change introduced the issue
1e22e02#diff-d9bde18e87a806403ee7c4564cad7bc2fcbc9835d3730f3f550c2b414ce09f89R447

@TristanSpeakEasy
Copy link
Contributor Author

PR to fix this here: #281

I don't fully understand the root cause but the issue seems to have been introduced by not continue any further logic in this file when examples was detected.

This change ensures that the original behaviour is retained while also handling the case the change in the commit that regressed this is also addressed

@daveshanley
Copy link
Member

Strange that this caused a regression, but thank you for fixing it!

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 a pull request may close this issue.

2 participants