Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,6 @@ exclude = [
"dist",
"node_modules",
"venv",
"tests/codegen/snapshots",
"alembic/* ",
]
src = ["src/strawchemy", "tests"]

Expand Down
113 changes: 4 additions & 109 deletions src/strawchemy/dto/strawberry.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
from strawchemy.utils.text import camel_to_snake

if TYPE_CHECKING:
from collections.abc import Callable, Hashable, Iterable, Iterator, Sequence
from collections.abc import Callable, Hashable, Iterator

from sqlalchemy import ColumnElement

Expand Down Expand Up @@ -112,33 +112,12 @@ def is_transform(self) -> bool:
class StrawchemyDefinition:
description: str = "GraphQL type"
is_root_aggregation_type: bool = False
field_map: dict[DTOKey, GraphQLFieldDefinition] = dataclasses.field(default_factory=dict)
query_hook: QueryHook[Any] | list[QueryHook[Any]] | None = None
filter: type[Any] | None = None
order_by: type[Any] | None = None
distinct_on: type[Any] | None = None
purpose: GraphQLPurpose | None = None

def __copy__(self) -> StrawchemyDefinition:
return dataclasses.replace(self, field_map=dict(self.field_map))

def populate_fields(
self,
key_source: type[Any] | DTOKey,
fields: Iterable[GraphQLFieldDefinition],
) -> Self:
key = key_source if isinstance(key_source, DTOKey) else DTOKey([key_source])
self.field_map = {key + f.name: f for f in fields}
return self

def get_field(self, key: DTOKey, name: str | None = None) -> GraphQLFieldDefinition:
full_key = key + name if name else key
return self.field_map[full_key]

def get_field_or_none(self, key: DTOKey, name: str | None = None) -> GraphQLFieldDefinition | None:
full_key = key + name if name else key
return self.field_map.get(full_key)

@property
def query_hooks(self) -> list[QueryHook[Any]]:
if self.query_hook is None:
Expand All @@ -154,6 +133,7 @@ def is_update_purpose(self) -> bool:

class StrawchemyObject:
__strawchemy_definition__: ClassVar[StrawchemyDefinition]
__dto_field_definitions__: ClassVar[dict[str, GraphQLFieldDefinition]]

def __init_subclass__(cls, **kwargs: Any) -> None:
super().__init_subclass__(**kwargs)
Expand All @@ -164,89 +144,6 @@ def __init_subclass__(cls, **kwargs: Any) -> None:
cls.__strawchemy_definition__ = copy(existing)


class _Key(Generic[T]):
"""A class to represent a key with multiple components.

The key is a sequence of components joined by a separator (default: ":").
It can be constructed from a sequence of components or a single string.
Components can be of any type, but must be convertible to a string.

The key can be extended with additional components using the `extend` or
`append` methods. The key can also be concatenated with another key or a
string using the `+` operator.

The key can be converted to a string using the `str` function or the
`to_str` method.

Subclasses should implement the `to_str` method to convert a component to a
string.
"""

__slots__ = ("_key",)

separator: ClassVar[str] = ":"

def __init__(self, components: Sequence[T | str] | str | None = None) -> None:
self._key: str = ""
if isinstance(components, str):
self._key = components
elif components:
self._key = str(self.extend(components))

def _components_to_str(self, objects: Sequence[T | str]) -> Sequence[str]:
return [obj if isinstance(obj, str) else self.to_str(obj) for obj in objects]

def to_str(self, obj: T) -> str:
raise NotImplementedError

def append(self, component: T | str) -> Self:
return self.extend([component])

def extend(self, components: Sequence[T | str]) -> Self:
str_components = self._components_to_str(components)
self._key = self.separator.join([self._key, *str_components] if self._key else str_components)
return self

def __add__(self, other: Self | str) -> Self:
if isinstance(other, str):
return self.__class__((self._key, other))
return self.__class__((self._key, other._key))

@override
def __str__(self) -> str:
return self._key

@override
def __hash__(self) -> int:
return hash(str(self))

@override
def __eq__(self, other: object) -> bool:
return hash(self) == hash(other)

@override
def __ne__(self, other: object) -> bool:
return hash(self) != hash(other)


class DTOKey(_Key[type[Any]]):
@override
def to_str(self, obj: type[Any]) -> str:
return obj.__name__

@classmethod
def from_dto_node(cls, node: Node[Any, Any]) -> Self:
return cls([node.value.model])

@classmethod
def from_query_node(cls, node: QueryNodeType) -> Self:
if node.is_root:
return cls([node.value.model])
if node.value.related_model:
return cls([node.value.related_model])
return cls([node.value.model])


class OrderByRelationFilterDTO(RelationFilterDTO, Generic[OrderByDTOT], frozen=True):
order_by: tuple[OrderByDTOT, ...] = field(default_factory=tuple)

Expand Down Expand Up @@ -604,11 +501,10 @@ class AggregationFunctionFilterDTO(UnmappedStrawberryGraphQLDTO[DeclarativeBase]
class OrderByDTO(GraphQLFilterDTO):
def tree(self, _node: QueryNodeType | None = None) -> QueryNodeType:
node = _node or QueryNode.root_node(self.__dto_model__)
key = DTOKey.from_query_node(node)

for name in self.dto_set_fields:
value: OrderByDTO | OrderByEnum = getattr(self, name)
field = self.__strawchemy_definition__.get_field(key, name)
field = self.__dto_field_definitions__[name]
if isinstance(field, FunctionFieldDefinition) and not field.has_model_field:
field.model_field = node.value.model_field
Comment on lines +507 to 509

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Copy the field before adding node-specific model context.

__dto_field_definitions__ is a ClassVar, so field.model_field = node.value.model_field mutates shared DTO metadata. After the first traversal sets it, later OrderByDTO instances can reuse a stale parent model_field and build incorrect order-by nodes.

🐛 Proposed fix
-            field = self.__dto_field_definitions__[name]
+            field = self.__dto_field_definitions__[name]
             if isinstance(field, FunctionFieldDefinition) and not field.has_model_field:
+                field = copy(field)
                 field.model_field = node.value.model_field
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/strawchemy/dto/strawberry.py` around lines 507 - 509, The issue is that
field.model_field assignment in the FunctionFieldDefinition handling directly
mutates the shared __dto_field_definitions__ ClassVar, causing stale model_field
values to persist across subsequent OrderByDTO instances. Instead of mutating
the shared field definition, create a copy of the field object before assigning
node.value.model_field to it. This ensures each instance has its own independent
copy of the field with the correct node-specific model context rather than
reusing a mutated shared definition.

if isinstance(value, OrderByDTO):
Expand All @@ -627,15 +523,14 @@ class BooleanFilterDTO(GraphQLFilterDTO):

def filters_tree(self, _node: QueryNodeType | None = None) -> tuple[QueryNodeType, Filter]:
node = _node or QueryNode.root_node(self.__dto_model__)
key = DTOKey.from_query_node(node)
query = Filter(
and_=[and_val.filters_tree(node)[1] for and_val in self.and_],
or_=[or_val.filters_tree(node)[1] for or_val in self.or_],
not_=self.not_.filters_tree(node)[1] if self.not_ else None,
)
for name in self.dto_set_fields:
value: EqualityComparison[Any] | BooleanFilterDTO | AggregateFilterDTO = getattr(self, name)
field = self.__strawchemy_definition__.get_field(key, name)
field = self.__dto_field_definitions__[name]
if isinstance(value, BooleanFilterDTO):
child, _ = node.upsert_child(field, match_on="value_equality")
_, sub_query = value.filters_tree(child)
Expand Down
14 changes: 2 additions & 12 deletions src/strawchemy/repository/strawberry/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,7 @@

from strawchemy.constants import JSON_PATH_KEY, ORDER_BY_KEY
from strawchemy.dto.base import ModelT
from strawchemy.dto.strawberry import (
DTOKey,
OrderByRelationFilterDTO,
QueryNode,
QueryNodeMetadata,
RelationFilterDTO,
StrawchemyObject,
)
from strawchemy.dto.strawberry import OrderByRelationFilterDTO, QueryNodeMetadata, RelationFilterDTO, StrawchemyObject
from strawchemy.exceptions import StrawchemyError
from strawchemy.repository.strawberry._node import StrawberryQueryNode
from strawchemy.schema.mutation import error_type_names
Expand Down Expand Up @@ -231,7 +224,6 @@ def _build(
model_field_name = camel_to_snake(selection.name) if self.auto_snake_case else selection.name
strawberry_field = next(field for field in strawberry_definition.fields if field.name == model_field_name)
strawberry_field_type = strawberry_contained_user_type(strawberry_field.type)
dto_model = dto_model_from_type(selection_type)

if (hooks := self._get_field_hooks(strawberry_field)) is not None:
self._add_query_hooks([hooks] if isinstance(hooks, QueryHook) else hooks, node)
Expand All @@ -243,9 +235,7 @@ def _build(
raise StrawchemyError(msg)
assert issubclass(dto, StrawchemyObject)

key = DTOKey.from_query_node(QueryNode.root_node(dto_model)) + strawberry_field.name

field_definition = dto.__strawchemy_definition__.get_field_or_none(key)
field_definition = dto.__dto_field_definitions__.get(strawberry_field.name)
if field_definition is None:
continue

Expand Down
3 changes: 1 addition & 2 deletions src/strawchemy/schema/factories/_kwargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from sqlalchemy.orm import QueryableAttribute

from strawchemy.dto.base import DTOFieldDefinition
from strawchemy.dto.strawberry import BooleanFilterDTO, DTOKey, GraphQLFieldDefinition, OrderByDTO
from strawchemy.dto.strawberry import BooleanFilterDTO, OrderByDTO
from strawchemy.dto.types import FieldSpec
from strawchemy.schema.pagination import DefaultOffsetPagination
from strawchemy.typing import GraphQLPurpose
Expand Down Expand Up @@ -102,7 +102,6 @@ class ForwardedFactoryKwargs(TypedDict, total=False):
tags: set[str] | None
backend_kwargs: dict[str, Any] | None
no_cache: bool
field_map: dict[DTOKey, GraphQLFieldDefinition] | None
register_type: bool
user_defined: bool

Expand Down
7 changes: 1 addition & 6 deletions src/strawchemy/schema/factories/aggregations.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@

from strawchemy.dto.backend.strawberry import StrawberrryDTOBackend
from strawchemy.dto.strawberry import (
DTOKey,
EnumDTO,
FilterFunctionInfo,
FunctionArgFieldDefinition,
GraphQLFieldDefinition,
OutputFunctionInfo,
UnmappedStrawberryGraphQLDTO,
)
Expand Down Expand Up @@ -99,13 +97,10 @@ def iter_field_definitions(
node: Node[Relation[DeclarativeBase, UnmappedStrawberryGraphQLDTO[DeclarativeBase]], None],
if_no_fields: Literal["raise", "skip"] = "skip",
*,
field_map: dict[DTOKey, GraphQLFieldDefinition] | None = None,
function: FunctionInfo | None = None,
**kwargs: Any,
) -> Generator[DTOFieldDefinition[DeclarativeBase, QueryableAttribute[Any]]]:
for field_def in super().iter_field_definitions(
name, model, dto_config, base, node, if_no_fields, field_map=field_map, **kwargs
):
for field_def in super().iter_field_definitions(name, model, dto_config, base, node, if_no_fields, **kwargs):
yield (
FunctionArgFieldDefinition.from_field(field_def, function=function)
if function is not None
Expand Down
13 changes: 1 addition & 12 deletions src/strawchemy/schema/factories/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
from strawchemy.dto.base import DTOBackend, DTOBase, DTOFactory, DTOFieldDefinition, Relation
from strawchemy.dto.strawberry import (
BooleanFilterDTO,
DTOKey,
EnumDTO,
GraphQLFieldDefinition,
MappedStrawberryGraphQLDTO,
Expand Down Expand Up @@ -488,16 +487,10 @@ def iter_field_definitions(
base: type[DTOBase[DeclarativeBase]] | None,
node: Node[Relation[DeclarativeBase, GraphQLDTOT], None],
if_no_fields: Literal["raise", "skip"] = "skip",
*,
field_map: dict[DTOKey, GraphQLFieldDefinition] | None = None,
**kwargs: Any,
) -> Generator[DTOFieldDefinition[DeclarativeBase, QueryableAttribute[Any]]]:
field_map = field_map if field_map is not None else {}
for field in super().iter_field_definitions(name, model, dto_config, base, node, if_no_fields, **kwargs):
key = DTOKey.from_dto_node(node)
graphql_field = GraphQLFieldDefinition.from_field(field)
yield graphql_field
field_map[key + field.name] = graphql_field
yield GraphQLFieldDefinition.from_field(field)

@override
def factory(
Expand All @@ -520,7 +513,6 @@ def factory(
user_defined: bool = False,
**kwargs: Any,
) -> type[GraphQLDTOT]:
field_map: dict[DTOKey, GraphQLFieldDefinition] = {}
if not user_defined and no_cache:
name = self.root_dto_name(model, dto_config, current_node) if name is None else name
name = self._mapper.registry.uniquify_name(self.graphql_type(dto_config), name)
Expand All @@ -539,11 +531,8 @@ def factory(
tags=tags,
backend_kwargs=backend_kwargs,
no_cache=no_cache,
field_map=field_map,
**kwargs,
)
if not dto.__strawchemy_definition__.field_map:
dto.__strawchemy_definition__.field_map = field_map
dto.__strawchemy_definition__.description = self.type_description()

if register_type:
Expand Down
28 changes: 4 additions & 24 deletions src/strawchemy/schema/factories/inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
AggregateFilterDTO,
AggregationFunctionFilterDTO,
BooleanFilterDTO,
DTOKey,
FilterFunctionInfo,
FunctionArgFieldDefinition,
FunctionFieldDefinition,
Expand Down Expand Up @@ -107,24 +106,15 @@ def iter_field_definitions(
if_no_fields: Literal["raise", "skip"] = "skip",
*,
aggregate_filters: bool = False,
field_map: dict[DTOKey, GraphQLFieldDefinition] | None = None,
**kwargs: Any,
) -> Generator[DTOFieldDefinition[DeclarativeBase, QueryableAttribute[Any]]]:
field_map = field_map if field_map is not None else {}
for field in super().iter_field_definitions(
name, model, dto_config, base, node, if_no_fields, field_map=field_map, **kwargs
):
key = DTOKey.from_dto_node(node)
for field in super().iter_field_definitions(name, model, dto_config, base, node, if_no_fields, **kwargs):
if field.is_relation:
field.type_ = Union[field.type_, None]
if field.uselist and field.related_dto:
field.type_ = Union[field.related_dto, None] # ty: ignore[invalid-type-form]
if aggregate_filters:
aggregation_field = self._aggregation_field(
field, dto_config.copy_with(partial_default=UNSET, partial=True)
)
field_map[key + aggregation_field.name] = aggregation_field
yield aggregation_field
yield self._aggregation_field(field, dto_config.copy_with(partial_default=UNSET, partial=True))
else:
comparison_type = self._filter_type(field)
field.type_ = Optional[comparison_type] # ty: ignore[invalid-type-form]
Expand Down Expand Up @@ -247,11 +237,6 @@ def _aggregate_function_type(
),
],
)
fields = [
FunctionArgFieldDefinition.from_field(field, function=aggregation)
for _, field in self.inspector.field_definitions(model, dto_config)
]
dto.__strawchemy_definition__.populate_fields(model, fields)
dto.__strawchemy_definition__.description = "Field filtering information"
dto.__dto_function_info__ = aggregation
return self._mapper.registry.register_type(
Expand Down Expand Up @@ -304,7 +289,6 @@ def _factory(
dto.__strawchemy_definition__.description = (
"Boolean expression to compare field aggregations. All fields are combined with logical 'AND'."
)
dto.__strawchemy_definition__.populate_fields(model, field_defs)
return dto


Expand Down Expand Up @@ -336,24 +320,21 @@ def _filter_type(self, field: DTOFieldDefinition[T, ModelFieldT]) -> type[OrderB
def _order_by_aggregation_fields(
self, aggregation: FilterFunctionInfo, model: type[Any], dto_config: DTOConfig
) -> type[OrderByDTO]:
model_fields = {field.name: field for _, field in self.inspector.field_definitions(model, dto_config)}
field_defs = [
FunctionArgFieldDefinition(
dto_config=dto_config,
model=model,
model_field_name=name.field_definition.name,
type_hint=OrderByEnum,
_function=aggregation,
_model_field=model_fields[name.field_definition.name].model_field,
)
for name in aggregation.enum_fields
]

name = f"{model.__name__}Aggregate{snake_to_camel(aggregation.aggregation_type)}FieldsOrderBy"
dto = self.backend.build(name, model, field_defs)
fields = [
FunctionArgFieldDefinition.from_field(field, function=aggregation)
for _, field in self.inspector.field_definitions(model, dto_config)
]
dto.__strawchemy_definition__.populate_fields(model, fields)
return self._mapper.registry.register_type(
dto,
dto_config=dto_config,
Expand Down Expand Up @@ -386,7 +367,6 @@ def _order_by_aggregation(self, model: type[DeclarativeBase], dto_config: DTOCon
)

dto = self.backend.build(f"{model.__name__}AggregateOrderBy", model, field_definitions)
dto.__strawchemy_definition__.populate_fields(model, field_definitions)
return self._mapper.registry.register_type(
dto,
dto_config=dto_config,
Expand Down
Loading
Loading