Fix nested pydantic.v1 model detection in pydantic integration#4246
Fix nested pydantic.v1 model detection in pydantic integration#4246
Conversation
Reviewer's GuideUpdates the Strawberry pydantic integration to detect both top-level and nested pydantic.v1 BaseModel types when running under Pydantic v2, centralizing model detection helpers and using them in type replacement, default handling, and error-type generation, plus adds a regression test for nested v1 models. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for adding the Here's a preview of the changelog: Fix Here's the tweet text: |
Greptile SummaryThis PR fixes a bug where nested pydantic v1 models (via Changes:
Architecture fit: The fix is well-scoped, centralizing model detection in Minor observation: The new regression test only validates schema SDL generation and does not exercise the actual query execution / Confidence Score: 5/5
Important Files Changed
Flowchartflowchart TD
A["Model type/instance check needed"] --> B{"is_model_class / is_model_instance"}
B --> C["BASE_MODEL_TYPES = _get_base_model_types()"]
C --> D["pydantic.BaseModel (always included)"]
C --> E{"pydantic.v1 available?"}
E -- "Yes (Pydantic v2 installed)" --> F["pydantic.v1.BaseModel added"]
E -- "No (Pydantic v1 only)" --> G["Only pydantic.BaseModel"]
F --> H["lenient_issubclass(type_, BASE_MODEL_TYPES)"]
G --> H
D --> H
H --> I["fields.py: replace_pydantic_types"]
H --> J["error_type.py: field_type_to_type"]
H --> K["utils.py: get_default_factory_for_field"]
Last reviewed commit: 18ef09a |
| def test_can_use_nested_pydantic_v1_models(): | ||
| from pydantic import v1 as pydantic_v1 | ||
|
|
||
| class Book(pydantic_v1.BaseModel): | ||
| title: str | ||
|
|
||
| class Library(pydantic_v1.BaseModel): | ||
| books: list[Book] | ||
|
|
||
| @strawberry.experimental.pydantic.type(model=Book, all_fields=True) | ||
| class BookType: | ||
| pass | ||
|
|
||
| @strawberry.experimental.pydantic.type(model=Library, all_fields=True) | ||
| class LibraryType: | ||
| pass | ||
|
|
||
| @strawberry.type | ||
| class Query: | ||
| library: LibraryType | ||
|
|
||
| schema = strawberry.Schema(query=Query) | ||
|
|
||
| expected_schema = """ | ||
| type BookType { | ||
| title: String! | ||
| } | ||
|
|
||
| type LibraryType { | ||
| books: [BookType!]! | ||
| } | ||
|
|
||
| type Query { | ||
| library: LibraryType! | ||
| } | ||
| """ | ||
|
|
||
| assert str(schema) == textwrap.dedent(expected_schema).strip() |
There was a problem hiding this comment.
Test only validates schema SDL, not query execution
The test verifies that the schema SDL is generated correctly for nested pydantic v1 models, which is the core regression being fixed. However, it does not exercise the conversion path (e.g., from_pydantic, resolving a query with a real Library instance containing Book objects). The existing test_can_use_both_pydantic_1_and_2 test includes both schema string validation and query execution.
Consider adding an execution assertion to also cover the replace_types_recursively path at runtime, e.g.:
@strawberry.type
class Query:
@strawberry.field
def library(self) -> LibraryType:
return LibraryType.from_pydantic(Library(books=[Book(title="Test")]))
schema = strawberry.Schema(query=Query)
result = schema.execute_sync("{ library { books { title } } }")
assert not result.errors
assert result.data == {"library": {"books": [{"title": "Test"}]}}This would validate the full conversion pipeline for nested v1 models, not just schema generation.
Context Used: Context from dashboard - In tests involving Pydantic models, ensure that the tests accurately reflect the intended behavior b... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Summary
pydantic.BaseModelandpydantic.v1.BaseModelunder Pydantic 2pydantic.v1models withall_fields=TrueReproduction
Before this change, the following fails with:
TypeError: LibraryType fields cannot be resolved. Unexpected type '<class ...Book>'After this change, schema creation succeeds and the generated GraphQL type for
booksis[BookType!]!.Tests
uv run --python 3.12 pytest tests/experimental/pydantic/schema/test_1_and_2.py -quv run --python 3.12 pytest tests/experimental/pydantic/test_error_type.py tests/experimental/pydantic/schema/test_defaults.py tests/experimental/pydantic/test_basic.py -quv run --python 3.12 ruff check strawberry/experimental/pydantic/_compat.py strawberry/experimental/pydantic/fields.py strawberry/experimental/pydantic/error_type.py strawberry/experimental/pydantic/utils.py tests/experimental/pydantic/schema/test_1_and_2.pyCloses #3584