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(pydantic): Check for unused strawberry.auto fields when converting BaseModel #1551

18 changes: 18 additions & 0 deletions RELEASE.MD
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Release type: patch

This release checks for AutoFieldsNotInBaseModelError when converting from pydantic models.
It is raised when strawberry.auto is used, but the pydantic model does not have
the particular field defined.
thejaminator marked this conversation as resolved.
Show resolved Hide resolved

```{python}
thejaminator marked this conversation as resolved.
Show resolved Hide resolved
class User(BaseModel):
age: int

@strawberry.experimental.pydantic.type(User)
class UserType:
age: strawberry.auto
password: strawberry.auto
```

Previously no errors would be raised, and the password field would not appear on graphql schema.
Such mistakes could be common during refactoring. Now, AutoFieldsNotInBaseModelError is raised.
12 changes: 11 additions & 1 deletion strawberry/experimental/pydantic/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any, Type
from typing import Any, List, Type

from pydantic import BaseModel
from pydantic.typing import NoArgAnyCallable
Expand Down Expand Up @@ -35,3 +35,13 @@ def __init__(self, default: Any, default_factory: NoArgAnyCallable):
)

super().__init__(message)


class AutoFieldsNotInBaseModelError(Exception):
def __init__(self, fields: List[str], cls_name: str, model: Type[BaseModel]):
message = (
f"{cls_name} defines {fields} with strawberry.auto. "
f"Field(s) not present in {model.__name__} BaseModel."
)

super().__init__(message)
5 changes: 5 additions & 0 deletions strawberry/experimental/pydantic/object_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from strawberry.experimental.pydantic.fields import get_basic_type
from strawberry.experimental.pydantic.utils import (
DataclassCreationFields,
ensure_all_auto_fields_in_pydantic,
get_default_factory_for_field,
get_private_fields,
sort_creation_fields,
Expand Down Expand Up @@ -111,6 +112,10 @@ def wrap(cls):
if not fields_set:
raise MissingFieldsListError(cls)

ensure_all_auto_fields_in_pydantic(
model=model, auto_fields=fields_set, cls_name=cls.__name__
)

all_model_fields: List[DataclassCreationFields] = [
DataclassCreationFields(
name=field_name,
Expand Down
20 changes: 19 additions & 1 deletion strawberry/experimental/pydantic/utils.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import dataclasses
from typing import Any, List, NamedTuple, Tuple, Type, Union, cast
from typing import Any, List, NamedTuple, NoReturn, Set, Tuple, Type, Union, cast

from pydantic import BaseModel
from pydantic.fields import ModelField
from pydantic.typing import NoArgAnyCallable
from pydantic.utils import smart_deepcopy

from strawberry.arguments import UNSET, _Unset, is_unset # type: ignore
from strawberry.experimental.pydantic.exceptions import (
AutoFieldsNotInBaseModelError,
BothDefaultAndDefaultFactoryDefinedError,
UnregisteredTypeException,
)
Expand Down Expand Up @@ -118,3 +120,19 @@ def get_default_factory_for_field(field: ModelField) -> Union[NoArgAnyCallable,
return lambda: None

return UNSET


def ensure_all_auto_fields_in_pydantic(
model: Type[BaseModel], auto_fields: Set[str], cls_name: str
) -> Union[NoReturn, None]:
# Raise error if user defined a strawberry.auto field not present in the model
pydantic_fields = set(field_name for field_name, _ in model.__fields__.items())
undefined = [
field_name for field_name in auto_fields if field_name not in pydantic_fields
]
thejaminator marked this conversation as resolved.
Show resolved Hide resolved
if len(undefined) > 0:
raise AutoFieldsNotInBaseModelError(
fields=undefined, cls_name=cls_name, model=model
patrick91 marked this conversation as resolved.
Show resolved Hide resolved
)
else:
return None
6 changes: 3 additions & 3 deletions tests/experimental/pydantic/schema/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ class BranchAType:
class BranchBType:
pass

@strawberry.experimental.pydantic.type(User, fields=["age", "union_field"])
@strawberry.experimental.pydantic.type(User, fields=["union_field"])
class UserType:
pass

Expand Down Expand Up @@ -370,7 +370,7 @@ class BranchAType:
class BranchBType:
pass

@strawberry.experimental.pydantic.type(User, fields=["age", "union_field"])
@strawberry.experimental.pydantic.type(User, fields=["union_field"])
class UserType:
pass

Expand Down Expand Up @@ -446,7 +446,7 @@ class BranchAType(BaseType):
class BranchBType(BaseType):
pass

@strawberry.experimental.pydantic.type(User, fields=["age", "interface_field"])
@strawberry.experimental.pydantic.type(User, fields=["interface_field"])
class UserType:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mistakes caught in our tests :D

pass

Expand Down
62 changes: 62 additions & 0 deletions tests/experimental/pydantic/test_conversion.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import re
from enum import Enum
from typing import Any, List, Optional, Union, cast

Expand All @@ -10,6 +11,7 @@
import strawberry
from strawberry.arguments import UNSET
from strawberry.experimental.pydantic.exceptions import (
AutoFieldsNotInBaseModelError,
BothDefaultAndDefaultFactoryDefinedError,
)
from strawberry.experimental.pydantic.utils import (
Expand Down Expand Up @@ -55,6 +57,66 @@ class UserType:
assert user.password == "abc"


def test_cannot_convert_pydantic_type_to_strawberry_missing_field():
class User(BaseModel):
age: int

with pytest.raises(
AutoFieldsNotInBaseModelError,
match=re.escape(
"UserType defines ['password'] with strawberry.auto."
" Field(s) not present in User BaseModel."
),
):

@strawberry.experimental.pydantic.type(User)
class UserType:
age: strawberry.auto
password: strawberry.auto


def test_cannot_convert_pydantic_type_to_strawberry_property_auto():
class User(BaseModel):
age: int

@property
def password(self) -> str:
return "hunter2"

with pytest.raises(
AutoFieldsNotInBaseModelError,
match=re.escape(
"UserType defines ['password'] with strawberry.auto."
" Field(s) not present in User BaseModel."
),
):

@strawberry.experimental.pydantic.type(User)
class UserType:
age: strawberry.auto
password: strawberry.auto


def test_can_convert_pydantic_type_to_strawberry_property():
class User(BaseModel):
age: int

@property
def password(self) -> str:
return "hunter2"

@strawberry.experimental.pydantic.type(User)
class UserType:
age: strawberry.auto
password: str

origin_user = User(age=1)
user = UserType.from_pydantic(origin_user)

assert user.age == 1
assert user.password == "hunter2"
patrick91 marked this conversation as resolved.
Show resolved Hide resolved


def test_can_convert_alias_pydantic_field_to_strawberry():
class UserModel(BaseModel):
age_: int = Field(..., alias="age")
Expand Down