Skip to content

Fully resolve external $ref tree #9294

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

Closed
wants to merge 11 commits into from
Closed

Fully resolve external $ref tree #9294

wants to merge 11 commits into from

Conversation

jweisman
Copy link

@jweisman jweisman commented Mar 24, 2019

Fully resolve references when loading OpenAPI specification. This is required to fully traverse the external $ref tags

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.

Description of the PR

Call ParseOptions.setResolveFully to fully walk the reference tree for responseBodies and schemas.

Fully resolve references when loading OpenAPI specification. This is required to fully traverse the external $ref tags
1. Load spec as URL
2. $ref in requestBody
3. Internal $ref within referenced document throws NPE
@jweisman
Copy link
Author

jweisman commented Jun 3, 2019

Hi there. Any chance this can be merged? Is there something else I can do on this PR? Thanks!

@daemonfire300
Copy link

Came here because we probably also need such a fix.
Saw the failing test, but I am no java/library expert so I could not fix it.

Fyi: But I tried skipping it and it turns out that it is not just this test that is failing. If you skip this test two more tests within the same suite fail.

@daemonfire300
Copy link

@jweisman any plans on resolving those failed tests?

@jweisman
Copy link
Author

Since this PR has been open for 9 months, the upstream repository changed considerably. I will need to rebase and see why the tests are failing.

@daemonfire300
Copy link

@jweisman Thank you 👍

@jweisman
Copy link
Author

Hi @daemonfire300 . I decided to add resolveFully as an option to avoid breaking any tests or previous behavior. I opened a new PR for that- #10200. Would be grateful if you could take a look there and provide any comments. Thanks!

@jweisman jweisman closed this Apr 23, 2020
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.

4 participants