Skip to content

Support of recursive and circular references #466

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
mtovts opened this issue Aug 13, 2021 · 11 comments
Closed

Support of recursive and circular references #466

mtovts opened this issue Aug 13, 2021 · 11 comments
Labels
✨ enhancement New feature or improvement 🐲 here there be dragons This is a very hard issue to solve. Beware!
Milestone

Comments

@mtovts
Copy link
Contributor

mtovts commented Aug 13, 2021

Is your feature request related to a problem? Please describe.
References to schemas that have not been processed yet are not supported now. See the example.

Describe the solution you'd like
I suggest to add models that are referenced without properties. And fill in the properties later. This adds the ability to refer to schemas that have not been processed yet.

Describe alternatives you've considered
Also it may also be easier to create all models without properties first, then add properties with second pass.

Additional context
Example OpenAPI:

openapi: 3.0.3
info:
  title: 'Example'
  version: 0.1.0
servers:
  - url: 'http://example.com'
paths:
  '/foo':
    delete:
      responses:
        '200':
          description: OK
components:
  schemas:
    Brother:
      type: object
      properties:
        sister:
          $ref: '#/components/schemas/Sister'
    Sister:
      type: object
      properties:
        brother:
          $ref: '#/components/schemas/Brother'

Generator produces warnings and models package is empty.

Could not find reference in parsed models or enums
Reference(ref='#/components/schemas/Sister')

Could not find reference in parsed models or enums
Reference(ref='#/components/schemas/Brother')

I expect the generated models for Brother and Sister. Something like:

# models/brother.py
class Brother:
    sister: Union[Unset, Sister] = UNSET

# models/sister.py
class Sister:
    brother: Union[Unset, Brother] = UNSET
@mtovts mtovts added the ✨ enhancement New feature or improvement label Aug 13, 2021
@mtovts mtovts changed the title Support of references to unprocessed schemas Support of recursive and circular references Aug 15, 2021
@barakalon
Copy link

This would be great!

I want to use openapi-python-client, but this issue is a blocker.

I'll keep an eye on #467...

@mtovts
Copy link
Contributor Author

mtovts commented Aug 31, 2021

Hello, @barakalon! The branch #467 does not work and I do not plan to finish it. Instead, I wrote another one that allows to generate a client that supports circular, recursive references and inheritance in allOf cases . But it is of low quality, unfinished branch. Please PM if you need such a code. I don’t want to publish it because it’s unfinished. I don’t have time to refactor it. I think it’s the right idea, some things in model generation have to be redesigned to make it clear and high quality with support of circular references and inheritance.

By the way, I think it is better to use pydantic in models generation as it is done in datamodel-code-generator it looks cleaner. Also
I don’t really like to put the classes in one module, but it also lets not write a bunch of ugly local try ... except ... in case with circular references.
For my OpenAPI, I got a lot of local pieces like,

        try:
            from ..models import user as user_m
            # ... 15 lines 
        except ImportError:
            import sys

            user_m = sys.modules[__package__ + "user"]
            # ... 15 lines 

@dbanty
Copy link
Collaborator

dbanty commented Oct 17, 2021

A note for the eventual fix of this, I've added an explicit check and warning to reduce user confusion when this issue presents itself in #519 which should be removed once this is supported.

@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
@penske-file
Copy link

penske-file commented Feb 25, 2022

What release is this scheduled to go into? when will that release go out? and what can I do to help? as this is a feature I need.

@dbanty
Copy link
Collaborator

dbanty commented Feb 27, 2022

There's currently no schedule for it. There is a new attempt at implementing it in #582 but I have to have time to test & review it before it can be merged. If you're able to test out that branch and give feedback on any problems you encounter, that would definitely be valuable.

@penske-file
Copy link

I am not sure how you usually work but is it possible to create a branch with the code changes that have been made then I could then add some test schemas and code along with any other changes that may be required

@maz808
Copy link
Contributor

maz808 commented Feb 27, 2022

I am not sure how you usually work but is it possible to create a branch with the code changes that have been made then I could then add some test schemas and code along with any other changes that may be required

@penske-file If you have GitHub cli and have cloned the openapi-generators/openapi-python-client repo, you can use this command within the repo directory on your machine to checkout the PR branch:
gh pr checkout 582

I worked on this branch so please let me know if you have questions or run into any issues with your use cases.

@ayemiller
Copy link

@maz808 @penske-file @dbanty

openapi: 3.0.3
info:
  title: 'Example'
  version: 0.1.0
servers:
  - url: 'http://example.com'
paths:
  '/foo':
    delete:
      responses:
        '200':
          description: OK
components:
  schemas:
    Brother:
      type: object
      properties:
        sister:
          $ref: '#/components/schemas/Sister'
    Sister:
      type: object
      properties:
        brother:
          $ref: '#/components/schemas/Brother'

I think this is related to an issue that I ran into. If it is helpful to anybody, I found that modifying the example as follows results in an error-free generation-

openapi: 3.0.3
info:
  title: 'WorkingExample'
  version: 0.1.0
servers:
  - url: 'http://example.com'
paths:
  '/foo':
    delete:
      responses:
        '200':
          description: OK
components:
  schemas:
    Brother:
      type: object
      properties:
        sister:
          schema:
            $ref: '#/components/schemas/Sister'
    Sister:
      type: object
      properties:
        brother:
          schema:
            $ref: '#/components/schemas/Brother'

@maz808
Copy link
Contributor

maz808 commented Apr 9, 2022

@ayemiller As far as I'm aware, the second example you've given isn't valid OpenAPI spec. The schema keyword isn't meant to be used in that context. If it's not throwing errors, that may be indicative of another issue.

@hhoeflin
Copy link

Here I just wanted to add that instead of pydantic, the use of attrs (modern interface) and especially cattrs (for structuring/unstructuring) could be really good. There is a lot of stuff now in attrs and cattrs that overall is more flexible than pydantic.

@dbanty
Copy link
Collaborator

dbanty commented Nov 12, 2022

Closed by #670 🥳

@dbanty dbanty closed this as completed 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
None yet
Development

No branches or pull requests

7 participants