Skip to content

feat: Support for recursive and circular references #582

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 3 commits into from

Conversation

maz808
Copy link
Contributor

@maz808 maz808 commented Jan 30, 2022

This PR adds support for recursive and circular references in properties and additionalProperties.

Related To/May Close

Known Issues

  • Recursive/circular reference paths that are required all the way through would be impossible to use in generated clients
class Example:
    # This would be impossible to create as the `example` attribute would require infinitely
    # recursive creation of Example objects 
    example: Example

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
Previous changes prevented models defined in array schemas from being
built properly. This commit fixes that issue and adds support for
recursive and circular references in array schema item objects, a
behavior that was previously expected but not functioning correctly. This
does not add support for recursive and circular references directly in an
array schema's 'items' section. However, this feature would be
functionally useless, so a warning is logged on detection.
@codecov
Copy link

codecov bot commented Apr 24, 2022

Codecov Report

Merging #582 (43c58d1) into main (d8d9cec) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##              main     #582      +/-   ##
===========================================
- Coverage   100.00%   99.94%   -0.06%     
===========================================
  Files           49       49              
  Lines         1713     1816     +103     
===========================================
+ Hits          1713     1815     +102     
- Misses           0        1       +1     
Impacted Files Coverage Δ
...penapi_python_client/parser/properties/__init__.py 100.00% <100.00%> (ø)
..._python_client/parser/properties/model_property.py 99.45% <100.00%> (-0.55%) ⬇️
...penapi_python_client/parser/properties/property.py 100.00% <100.00%> (ø)
openapi_python_client/parser/properties/schemas.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8d9cec...43c58d1. Read the comment docs.

@mtovt
Copy link
Contributor

mtovt commented Jun 15, 2022

@maz808 Hello! Thanks for your work and PR! Seems like the generated client doesn’t work due to circular import exceptions.

For example:

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'

When I trying to use Sister model

from example_client.models.sister import Sister

if __name__ == '__main__':
    Sister()
File “…/example_client/models/__init__.py", line 3, in <module>
    from .brother import Brother
  File "…/example_client/models/brother.py", line 5, in <module>
    from ..models.sister import Sister
  File "…/example_client/models/sister.py", line 5, in <module>
    from ..models.brother import Brother
ImportError: cannot import name 'Brother' from partially initialized module 'example_client.models.brother' (most likely due to a circular import)

Seems like the first problem should be solved in the way that supports circular references. It has several workarounds. But I like the approach to use just one module for storing all models, like module models.py with

from __future__ import annotations

@attr.s(auto_attribs=True)
class Brother:
	…
    sister: Union[Unset, Sister] = UNSET@classmethod
    def from_dict(cls: Brother, src_dict: Dict[str, Any]) -> Brother:
        …


@attr.s(auto_attribs=True)
class Sister:
	…

@dbanty What do you think about it?

@dbanty
Copy link
Collaborator

dbanty commented Sep 18, 2022

This PR lives on through @mtovt's work in #670 🥳

@dbanty dbanty closed this Sep 18, 2022
@maz808
Copy link
Contributor Author

maz808 commented Nov 12, 2022

@dbanty I noticed the release page doesn't acknowledge my contribution to the recursive and nested schema issue. Could I be added to the list of thanks?

@dbanty
Copy link
Collaborator

dbanty commented Nov 12, 2022

Weird... I remember explicitly adding you. Sorry about that, I'll fix it!!

@maz808
Copy link
Contributor Author

maz808 commented Nov 12, 2022

@dbanty I thought you had too 🤔 I appreciate it though! Thank you!

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.

3 participants