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

fix(open-api): support generic types for OpenAPI schema generation #2463

Merged
merged 42 commits into from Oct 23, 2023

Conversation

guacs
Copy link
Member

@guacs guacs commented Oct 17, 2023

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR
  • Pre-Commit Checks were ran and passed
  • Tests were ran and passed

Description

  • This PR adds support for generating the OpenAPI schema for generic types.

Close Issue(s)

@guacs guacs requested review from a team as code owners October 17, 2023 04:37
@guacs
Copy link
Member Author

guacs commented Oct 17, 2023

The reason the generic types were not working was because get_type_hints does not resolve the generic types as seen here. So a helper function was created to get the type hints after resolving for the generic types as much as possible. The relevant tests should make the expected behavior clear.

For the following handlers, the resulting schema will be as follows:

from typing import Generic, TypeVar
from dataclasses import dataclass
from litestar import get, post
from litestar.app import Litestar

T = TypeVar("T")


@dataclass
class Foo(Generic[T]):
    foo: T


@get("/")
async def get_foo() -> Foo[int]:
    return Foo(1)


@get("/no-type")
async def get_foo_no_type() -> Foo:
    return Foo(1)


@post("/generic-type")
async def get_foo_generic(foo: Foo[T]) -> Foo[T]:
    return foo


app = Litestar([get_foo, get_foo_no_type, get_foo_generic])
image

@guacs
Copy link
Member Author

guacs commented Oct 17, 2023

Generic Pydantic models do not work currently due to this issue. I just realized (as I was typing this :P) a simple workaround for this would be to just get the __parameters__ and __args__ using the pydantic API and then use the _substitute_typevars to get the correct type hints.

I'll add this in along with tests to this PR itself.

@guacs guacs force-pushed the generic-response-openapi branch 3 times, most recently from 878f443 to edab314 Compare October 17, 2023 05:46
Copy link
Contributor

@peterschutt peterschutt left a comment

Choose a reason for hiding this comment

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

Sorry, ran out of time to complete my review, but a couple of comments so far. I'll pick up again later. Cheers!

litestar/_openapi/schema_generation/schema.py Outdated Show resolved Hide resolved
litestar/utils/predicates.py Show resolved Hide resolved
@guacs
Copy link
Member Author

guacs commented Oct 19, 2023

@peterschutt first of all, thank you for such a fantastic analysis!

Yeah so the reason for this is that in _substitute_typevars, the check for whether there are any values for __parameters__ was incorrect. Essentially the substitution should only happen if there are any __parameters__. The current _substitute_typevars implementation should be changed to:

def _substitute_typevars(obj: Any, typevar_map: Mapping[Any, Any]) -> Any:
    params = getattr(obj, "__parameters__", None)

    if params: # this was `if params is None` before but list[str].__parameters__ returns an empty tuple instead
        args = tuple(_substitute_typevars(typevar_map.get(p), typevar_map) for p in params)
        return obj[args]

    ....

This also resolves the issue related to __slots__ being returned in get_type_hints! It doesn't matter even if __slots__ is included in the type hints since the OpenAPI generation will be using the respective APIs to get the field names and then use that to get the annotation from the resolved type hints dictionary we get from get_type_hints_with_generics_resolved.

The fix for handling empty parameters tuples fixed the underlying issue.
@peterschutt
Copy link
Contributor

This also resolves the issue related to slots being returned in get_type_hints! It doesn't matter even if slots is included in the type hints since the OpenAPI generation will be using the respective APIs to get the field names and then use that to get the annotation from the resolved type hints dictionary we get from get_type_hints_with_generics_resolved

Oh, of course! Good stuff:)

Copy link
Contributor

@peterschutt peterschutt left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

litestar/_openapi/schema_generation/schema.py Outdated Show resolved Hide resolved
tests/unit/test_openapi/test_schema.py Show resolved Hide resolved
peterschutt and others added 2 commits October 23, 2023 19:17
* refactor: _get_type_schema_name receive FieldDefinition

Removes the need to call `get_args()` on the annotation by passing the field def. directly through.

* Removes an unneeded call to `get_origin_or_inner_type()`

* Fix type error.
@guacs guacs enabled auto-merge (squash) October 23, 2023 13:51
@guacs guacs merged commit bb13abb into main Oct 23, 2023
19 checks passed
@guacs guacs deleted the generic-response-openapi branch October 23, 2023 13:56
@sonarcloud
Copy link

sonarcloud bot commented Oct 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

95.1% 95.1% Coverage
0.0% 0.0% Duplication

@github-actions
Copy link

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/2463

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.

Bug: Missing API Schema for generic response type annotations
3 participants