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

Add support for recursively defined schemas #338

Closed
sp-schoen opened this issue Feb 17, 2021 · 13 comments · Fixed by #670
Closed

Add support for recursively defined schemas #338

sp-schoen opened this issue Feb 17, 2021 · 13 comments · Fixed by #670
Labels
✨ enhancement New feature or improvement 🐲 here there be dragons This is a very hard issue to solve. Beware!
Milestone

Comments

@sp-schoen
Copy link

Describe the bug
I tried to create a python client from an OpenApi-Spec with the command openapi-python-client generate --path secret_server_openapi3.json. Then I got multple warnings:

  • invalid data in items of array settings
  • Could not find reference in parsed models or enums
  • Cannot parse response for status code 200, response will be ommitted from generated client
    I searched the schemas, which are responsible for the errors and they all had in common, that they either reference another schema which is recursively defined or reference themself.
    For example one of the problematic schemas:
{
  "SecretDependencySetting": {
        "type": "object",
        "properties": {
          "active": {
            "type": "boolean",
            "description": "Indicates the setting is active."
          },
          "childSettings": {
            "type": "array",
            "description": "The Child Settings that would be used  for list of options.",
            "items": {
              "$ref": "#/components/schemas/SecretDependencySetting"
            }
          }
        },
        "description": "Secret Dependency Settings - Mostly used internally"
      }
}

To Reproduce
Define a schema recursively and then try to create a client from it.

Expected behavior
The software can also parse a recursively defined schema.

OpenAPI Spec File
It's 66000 Lines, so I'm not sure if this will help you, or github will allow me to post it 😄
Just ask if you need specific parts of the spec, aside from what I already provided above.

Desktop (please complete the following information):

  • OS: Linux Manjaro
  • Python Version: 3.9.1
  • openapi-python-client version 0.7.3
@sp-schoen sp-schoen added the 🐞bug Something isn't working label Feb 17, 2021
@emann
Copy link
Collaborator

emann commented Feb 17, 2021

Thanks for reporting @sp-schoen! This should be fixed by #329 which all being well should make it into the 0.8 release we have coming up.

@sp-schoen
Copy link
Author

I checked out the PR you suggested and tried it, but I still get the same error.

@emann
Copy link
Collaborator

emann commented Feb 17, 2021

Ah, looking it over again you're totally right. The stuff in there will be needed to make recursive schemas work however, so whenever that is reviewed/merged work can begin on adding support for recursive definitions.

@emann
Copy link
Collaborator

emann commented Feb 19, 2021

Just FYI @sp-schoen, @p1-ra has graciously begun working on supporting recursive definitions in the aforementioned PR!

@emann emann added ✨ enhancement New feature or improvement and removed 🐞bug Something isn't working labels Feb 25, 2021
@emann emann changed the title ERROR when parsing recursively defined schema Add support for recursively defined schemas Feb 25, 2021
@jkinkead
Copy link

jkinkead commented May 5, 2021

Was this fixed by #366 ?

@dbanty
Copy link
Collaborator

dbanty commented May 5, 2021

I'm not sure 😅 probably not though. #366 was just about fixing reference resolution in general, my guess is that generation will still fail for recursive models, we'll need to add a special case to those.

@dbanty dbanty added this to To do in OpenAPI 3.0 Compliance via automation May 5, 2021
@jkinkead
Copy link

jkinkead commented May 5, 2021

Cool, thanks! It wasn't clear given that #329 was closed in favor of #366. :)

@emann
Copy link
Collaborator

emann commented May 5, 2021

Based on @p1-ra's feedback in #366, it seems like recursive references do in fact work - @jkinkead if you could try using your schema with 0.9.0 and let us know how it shakes out that'd be awesome!

@jkinkead
Copy link

jkinkead commented May 5, 2021

Hmm, I still get errors with 0.9.0. :/

The repro file at the top of the issue still produces errors:

$ openapi-python-client --version
openapi-python-client version: 0.9.0
$ openapi-python-client generate --path repro.json 
Error(s) encountered while generating, client was not created

Failed to parse OpenAPI document

3 validation errors for OpenAPI
info
  field required (type=value_error.missing)
paths
  field required (type=value_error.missing)
openapi
  field required (type=value_error.missing)


If you believe this was a mistake or this tool is missing a feature you need, please open an issue at https://github.com/triaxtec/openapi-python-client/issues/new/choose

@emann
Copy link
Collaborator

emann commented May 5, 2021

Does that repro.json that you made just contain the block at the top of the issue? You'll need to provide a full OpenAPI document with all of the required fields in it - try one of the samples @p1-ra provided in #366 (assuming they match what your use case is).

@sp-schoen
Copy link
Author

@emann
I also tried it again with 0.9.0 and still get the same errors.

@jkinkead
Copy link

jkinkead commented May 7, 2021

Sorry, yes - I was using just that json file, but my local repro is much too large to paste in.

I'll add a shorter repro if I have time!

@p1-ra
Copy link
Contributor

p1-ra commented May 7, 2021

My bad, It was to early on the morning it looks I've messed up with my tests; probably did them on the wrong branch ....; I've redo the tests using today main branch, I still have the issues.

All sample define here are NOK

@dbanty dbanty added the 🐲 here there be dragons This is a very hard issue to solve. Beware! label Jan 9, 2022
maz808 referenced this issue in maz808/openapi-python-client Jan 29, 2022
This commit adds support for recursive and circular references in object
properties and additionalProperties, but not in allOf. The changes
include:

-Delayed processing of schema model properties
-Cascading removal of invalid schema reference dependencies
-Prevention of self import in ModelProperty relative imports
-Prevention of forward recursive type reference errors in generated modules
-Logging for detection of recursive references in allOf
maz808 referenced this issue in maz808/openapi-python-client Jan 29, 2022
This commit adds support for recursive and circular references in object
properties and additionalProperties, but not in allOf. The changes
include:

-Delayed processing of schema model properties
-Cascading removal of invalid schema reference dependencies
-Prevention of self import in ModelProperty relative imports
-Prevention of forward recursive type reference errors in generated modules
-Logging for detection of recursive references in allOf
@dbanty dbanty added this to the 0.12.0 milestone Sep 26, 2022
dbanty added a commit that referenced this issue Nov 12, 2022
#670, #338, #466]. Thanks @mtovt!

Co-authored-by: maz808 <mazamora808@gmail.com>
Co-authored-by: Dylan Anthony <dbanty@users.noreply.github.com>
OpenAPI 3.0 Compliance automation moved this from To do to Done Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or improvement 🐲 here there be dragons This is a very hard issue to solve. Beware!
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants