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.

Sign up for GitHub

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

Jump to bottom

bug: openapi schema generation for DTO with generic wrapper #2929

Closed
byte-bot-app bot opened this issue Dec 24, 2023 · 2 comments · Fixed by #3371
Closed

bug: openapi schema generation for DTO with generic wrapper #2929

byte-bot-app bot opened this issue Dec 24, 2023 · 2 comments · Fixed by #3371

Comments

@byte-bot-app
Copy link

byte-bot-app bot commented Dec 24, 2023 •
edited by peterschutt

Handler such as:

    @get('/all-not-working', return_dto=QuotesSQLAlchemyDTO)
    async def get_all_quotes_not_working(self, sqlite_session: AsyncSession) -> GetAllQuotesResponse[QuotesTable]:
        res = GetAllQuotesResponse(quotes=await get_all_quotes(sqlite_session))
        LOGGER.critical(f"sherlock {res}")
        return res

Renders such as (only generates schema for the model type, not the wrapper type):

image

Issue is that we don't pass the full annotation from the DTO to the backend, we only pass the model type that is extracted from the annotation and the name of the attribute that it exists upon on the wrapper class. So, when the DTO is called upon to generate the openapi schema for the type, it doesn't incorporate the wrapper type, only the model.

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@peterschutt peterschutt changed the title Issue from Discord big: openapi schema generation for DTO with generic wrapper Dec 24, 2023
@peterschutt peterschutt changed the title big: openapi schema generation for DTO with generic wrapper bug: openapi schema generation for DTO with generic wrapper Dec 24, 2023
peterschutt added a commit that referenced this issue Dec 24, 2023
@peterschutt
fix: OpenAPI schema for DTO supported types with generic wrapper
c8dee30 
This PR proposes a method to repair schema generation when a DTO supported type is wrapped with a generic wrapper. Prior to this PR, the generic wrapper type was omitted from the generated OpenAPI schema for the type.

WIP.

Closes #2929
@peterschutt peterschutt mentioned this issue Dec 24, 2023
Closed
provinzkraut pushed a commit that referenced this issue Feb 3, 2024
@peterschutt @provinzkraut
fix: OpenAPI schema for DTO supported types with generic wrapper
b92282a 
This PR proposes a method to repair schema generation when a DTO supported type is wrapped with a generic wrapper. Prior to this PR, the generic wrapper type was omitted from the generated OpenAPI schema for the type.

WIP.

Closes #2929
@peterschutt peterschutt mentioned this issue Apr 10, 2024
Closed
4 tasks
peterschutt added a commit that referenced this issue Apr 10, 2024
@peterschutt
fix: Fix schema for generic wrapped return types with DTO
7d0c33d 
Adds failing test.

Closes #2929
@peterschutt peterschutt mentioned this issue Apr 10, 2024
Merged
peterschutt added a commit that referenced this issue Apr 11, 2024
@peterschutt
fix: Fix schema for generic wrapped return types with DTO
1fc9f8d 
Adds failing test.

Closes #2929
peterschutt added a commit that referenced this issue Apr 11, 2024
@peterschutt
fix: schema for generic wrapped return types with DTO (#3371)
40a5685 
* fix: Fix schema for generic wrapped return types with DTO

Adds failing test.

Closes #2929

* fix: remove redundant type var

* fix: fix openapi schema for generic return types with dto

Prior behavior of using the `backend.annotation` as the basis for generating the openapi schema for the represented type is not applicable for the case where the DTO supported type is wrapped in a generic outer object.

This PR detects the case where we unwrap an outer generic type, and rebuilds the generic annotation in a manner appropriate for schema generation, before generating the schema for the annotation. It does this by substituting the DTOs transfer model for the original model in the original annotations type arguments.

This is really treating a symptom of the generic outer type being removed from the annotation before the DTO backend receives it, which is a design flaw that adds quite a bit of complexity. The DTOs should treat annotations at a whole, rebuild the complete annotation substituting out any supported types found within with transfer models.

Also, we don't really have a need for a separate backend anymore, now that we are only supporting one serialization library (backend were introduced when we were trying to support either pydantic or msgspec (and attrs?) throughout the whole application.

* test: add another test.
@github-actions GitHub Actions
Copy link

This issue has been closed in #3371. The change will be included in the upcoming patch release.

peterschutt added a commit that referenced this issue Apr 11, 2024
@peterschutt
fix: schema for generic wrapped return types with DTO (#3371)
0d3dc87 
* fix: Fix schema for generic wrapped return types with DTO

Adds failing test.

Closes #2929

* fix: remove redundant type var

* fix: fix openapi schema for generic return types with dto

Prior behavior of using the `backend.annotation` as the basis for generating the openapi schema for the represented type is not applicable for the case where the DTO supported type is wrapped in a generic outer object.

This PR detects the case where we unwrap an outer generic type, and rebuilds the generic annotation in a manner appropriate for schema generation, before generating the schema for the annotation. It does this by substituting the DTOs transfer model for the original model in the original annotations type arguments.

This is really treating a symptom of the generic outer type being removed from the annotation before the DTO backend receives it, which is a design flaw that adds quite a bit of complexity. The DTOs should treat annotations at a whole, rebuild the complete annotation substituting out any supported types found within with transfer models.

Also, we don't really have a need for a separate backend anymore, now that we are only supporting one serialization library (backend were introduced when we were trying to support either pydantic or msgspec (and attrs?) throughout the whole application.

* test: add another test.

(cherry picked from commit 40a5685)
peterschutt added a commit that referenced this issue Apr 11, 2024
@peterschutt
fix: schema for generic wrapped return types with DTO (#3371)
4edcfa3 
* fix: Fix schema for generic wrapped return types with DTO

Adds failing test.

Closes #2929

* fix: remove redundant type var

* fix: fix openapi schema for generic return types with dto

Prior behavior of using the `backend.annotation` as the basis for generating the openapi schema for the represented type is not applicable for the case where the DTO supported type is wrapped in a generic outer object.

This PR detects the case where we unwrap an outer generic type, and rebuilds the generic annotation in a manner appropriate for schema generation, before generating the schema for the annotation. It does this by substituting the DTOs transfer model for the original model in the original annotations type arguments.

This is really treating a symptom of the generic outer type being removed from the annotation before the DTO backend receives it, which is a design flaw that adds quite a bit of complexity. The DTOs should treat annotations at a whole, rebuild the complete annotation substituting out any supported types found within with transfer models.

Also, we don't really have a need for a separate backend anymore, now that we are only supporting one serialization library (backend were introduced when we were trying to support either pydantic or msgspec (and attrs?) throughout the whole application.

* test: add another test.

(cherry picked from commit 40a5685)
peterschutt added a commit that referenced this issue May 1, 2024
@peterschutt
fix: schema for generic wrapped return types with DTO (#3371)
ada4fde 
* fix: Fix schema for generic wrapped return types with DTO

Adds failing test.

Closes #2929

* fix: remove redundant type var

* fix: fix openapi schema for generic return types with dto

Prior behavior of using the `backend.annotation` as the basis for generating the openapi schema for the represented type is not applicable for the case where the DTO supported type is wrapped in a generic outer object.

This PR detects the case where we unwrap an outer generic type, and rebuilds the generic annotation in a manner appropriate for schema generation, before generating the schema for the annotation. It does this by substituting the DTOs transfer model for the original model in the original annotations type arguments.

This is really treating a symptom of the generic outer type being removed from the annotation before the DTO backend receives it, which is a design flaw that adds quite a bit of complexity. The DTOs should treat annotations at a whole, rebuild the complete annotation substituting out any supported types found within with transfer models.

Also, we don't really have a need for a separate backend anymore, now that we are only supporting one serialization library (backend were introduced when we were trying to support either pydantic or msgspec (and attrs?) throughout the whole application.

* test: add another test.

(cherry picked from commit 40a5685)
peterschutt added a commit that referenced this issue May 2, 2024
@peterschutt
fix: schema for generic wrapped return types with DTO (#3371)
deddcfb 
* fix: Fix schema for generic wrapped return types with DTO

Adds failing test.

Closes #2929

* fix: remove redundant type var

* fix: fix openapi schema for generic return types with dto

Prior behavior of using the `backend.annotation` as the basis for generating the openapi schema for the represented type is not applicable for the case where the DTO supported type is wrapped in a generic outer object.

This PR detects the case where we unwrap an outer generic type, and rebuilds the generic annotation in a manner appropriate for schema generation, before generating the schema for the annotation. It does this by substituting the DTOs transfer model for the original model in the original annotations type arguments.

This is really treating a symptom of the generic outer type being removed from the annotation before the DTO backend receives it, which is a design flaw that adds quite a bit of complexity. The DTOs should treat annotations at a whole, rebuild the complete annotation substituting out any supported types found within with transfer models.

Also, we don't really have a need for a separate backend anymore, now that we are only supporting one serialization library (backend were introduced when we were trying to support either pydantic or msgspec (and attrs?) throughout the whole application.

* test: add another test.

(cherry picked from commit 40a5685)
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 2, 2024

A fix for this issue has been released in v2.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
0 participants