Skip to content

Fix(parser): reference type resolution (fix #371) #418

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

Conversation

p1-ra
Copy link
Contributor

@p1-ra p1-ra commented May 7, 2021

This PR resolve #371
The parser assumed that a reference was always pointing to an enum (EnumProperty) or to an object (ModelProperty).
It was defaulting to a ModelProperty, thus when a reference was pointing to an other kind of property (StringProperty, DateProperty, FileProperty, etc) a model was generated.

It now correctly resolve the Property type in function of the schema type.

BR.

@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #418 (0321209) into main (a731477) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 0321209 differs from pull request most recent head 612384c. Consider uploading reports for the commit 612384c to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##              main      #418   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           47        47           
  Lines         1549      1546    -3     
=========================================
- Hits          1549      1546    -3     
Impacted Files Coverage Δ
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 a731477...612384c. Read the comment docs.

@p1-ra p1-ra force-pushed the fix/parser-reference-type-resolution branch from 0321209 to 5ac6881 Compare May 7, 2021 19:31
@p1-ra p1-ra force-pushed the fix/parser-reference-type-resolution branch from 5ac6881 to 612384c Compare May 7, 2021 19:51
Copy link
Collaborator

@dbanty dbanty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is great! I figured it would be a lot more work to implement which is why I put it off so long. Thanks for contributing this! 🥳

@dbanty
Copy link
Collaborator

dbanty commented May 13, 2021

There was an error with the e2e test once I merged in main so I recreated in #425 and simplified the e2e generated test by making all properties required (so there will be less churn).

@dbanty dbanty closed this May 13, 2021
dbanty added a commit that referenced this pull request May 13, 2021
][#425]. Thanks @p1-ra!

* parser / properties / do not restrict reference pointer type to `enum` or `object`

* e2e / update `openapi.json` and golden-record

* tpl / model / add missing typing import: `Tuple,Optional,BinaryIO,TextIO`

* test: Make big ref test model properties required to lessen generated code

Co-authored-by: Nementon <[email protected]>
p1-ra pushed a commit to P1sec/openapi-python-client that referenced this pull request May 14, 2021
…-generators#371][openapi-generators#418][openapi-generators#425]. Thanks @p1-ra!

* parser / properties / do not restrict reference pointer type to `enum` or `object`

* e2e / update `openapi.json` and golden-record

* tpl / model / add missing typing import: `Tuple,Optional,BinaryIO,TextIO`

* test: Make big ref test model properties required to lessen generated code

Co-authored-by: Nementon <[email protected]>
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.

Support references to non-object types
3 participants