Skip to content

Commit fad4eff

Browse files
authored
fix: nullable relationship detection and FileObject nested metadata (#679)
* fix(dto): detect nullable one-to-one relationships on inverse side (#227) `detect_nullable_relationship()` only checked MANYTOONE direction, missing ONETOMANY+uselist=False (inverse side of one-to-one). Also fixed `parse_type_from_element()` to not treat scalar one-to-one relationships as collections. * fix(file-object): serialize nested metadata for obstore attributes (#676) Obstore attributes must be strings. Non-string metadata values (lists, dicts, numbers) are now serialized via encode_json before passing to obstore's put(). Fixes TypeError when FileObject has nested metadata.
1 parent 9488dd7 commit fad4eff

4 files changed

Lines changed: 106 additions & 6 deletions

File tree

advanced_alchemy/extensions/litestar/dto.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,10 @@ def parse_type_from_element(elem: ElementType, orm_descriptor: InspectionAttr) -
513513
return FieldDefinition.from_annotation(elem.type.python_type)
514514

515515
if isinstance(elem, RelationshipProperty):
516-
if elem.direction in {RelationshipDirection.ONETOMANY, RelationshipDirection.MANYTOMANY}:
516+
if (
517+
elem.direction in {RelationshipDirection.ONETOMANY, RelationshipDirection.MANYTOMANY}
518+
and elem.uselist is not False
519+
):
517520
collection_type = FieldDefinition.from_annotation(elem.collection_class or list) # pyright: ignore[reportUnknownMemberType]
518521
return FieldDefinition.from_annotation(collection_type.safe_generic_origin[elem.mapper.class_])
519522

@@ -535,13 +538,16 @@ def parse_type_from_element(elem: ElementType, orm_descriptor: InspectionAttr) -
535538
def detect_nullable_relationship(elem: RelationshipProperty[Any]) -> bool:
536539
"""Detects if a relationship is nullable.
537540
538-
This attempts to decide if we should allow a ``None`` default value for a relationship by looking at the
539-
foreign key fields. If all foreign key fields are nullable, then we allow a ``None`` default value.
541+
For MANYTOONE relationships (FK on this side), checks if all FK columns are nullable.
542+
For ONETOMANY with uselist=False (inverse side of one-to-one), the related object may not
543+
exist since no local FK enforces it, so these are always treated as nullable.
540544
541545
Args:
542546
elem: The relationship to check.
543547
544548
Returns:
545549
bool: ``True`` if the relationship is nullable, ``False`` otherwise.
546550
"""
547-
return elem.direction == RelationshipDirection.MANYTOONE and all(c.nullable for c in elem.local_columns)
551+
if elem.direction == RelationshipDirection.MANYTOONE:
552+
return all(c.nullable for c in elem.local_columns)
553+
return elem.direction == RelationshipDirection.ONETOMANY and elem.uselist is False

advanced_alchemy/types/file_object/backends/obstore.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from pathlib import Path
66
from typing import TYPE_CHECKING, Any, Optional, Union, cast
77

8+
from advanced_alchemy._serialization import encode_json
89
from advanced_alchemy.exceptions import MissingDependencyError
910
from advanced_alchemy.types.file_object._utils import get_mtime_equivalent, get_or_generate_etag
1011
from advanced_alchemy.types.file_object.base import (
@@ -54,6 +55,28 @@ def schema_from_type(obj: Any) -> str: # noqa: PLR0911
5455
return "file"
5556

5657

58+
def _serialize_metadata_for_attributes(metadata: "dict[str, Any]") -> "dict[str, str]":
59+
"""Serialize metadata values to strings for obstore attributes.
60+
61+
Obstore attributes must be strings. Non-string values (lists, dicts, etc.)
62+
are serialized using the project's ``encode_json`` to avoid TypeError when
63+
passed to obstore's ``put()``.
64+
65+
Args:
66+
metadata: The metadata dict from a FileObject.
67+
68+
Returns:
69+
A dict with all values as strings.
70+
"""
71+
result: dict[str, str] = {}
72+
for key, value in metadata.items():
73+
if isinstance(value, str):
74+
result[key] = value
75+
else:
76+
result[key] = encode_json(value)
77+
return result
78+
79+
5780
class ObstoreBackend(StorageBackend):
5881
"""Obstore-backed storage backend implementing both sync and async operations."""
5982

@@ -132,8 +155,9 @@ def save_object(
132155
attributes["Content-Type"] = file_object.content_type
133156

134157
# Add any custom metadata from file_object.metadata
158+
# Obstore attributes must be strings, so serialize non-string values to JSON
135159
if file_object.metadata:
136-
attributes.update(file_object.metadata)
160+
attributes.update(_serialize_metadata_for_attributes(file_object.metadata))
137161

138162
# LocalStore doesn't support attributes parameter - skip it for local filesystem
139163
put_params: dict[str, Any] = {
@@ -188,8 +212,9 @@ async def save_object_async(
188212
attributes["Content-Type"] = file_object.content_type
189213

190214
# Add any custom metadata from file_object.metadata
215+
# Obstore attributes must be strings, so serialize non-string values to JSON
191216
if file_object.metadata:
192-
attributes.update(file_object.metadata)
217+
attributes.update(_serialize_metadata_for_attributes(file_object.metadata))
193218

194219
# LocalStore doesn't support attributes parameter - skip it for local filesystem
195220
put_params: dict[str, Any] = {

tests/integration/test_file_object.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1831,6 +1831,34 @@ async def test_obstore_content_type_and_metadata_passing(storage_registry: Stora
18311831
assert updated_obj_sync.metadata == custom_metadata
18321832

18331833

1834+
@pytest.mark.xdist_group("file_object")
1835+
async def test_obstore_nested_metadata_serialization(storage_registry: StorageRegistry) -> None:
1836+
"""Test that nested metadata (lists, dicts) doesn't crash obstore's put().
1837+
1838+
Regression test for https://github.com/jolt-org/advanced-alchemy/issues/676.
1839+
Obstore attributes must be strings; non-string values are JSON-serialized.
1840+
"""
1841+
remove_listeners()
1842+
backend = storage_registry.get_backend("memory")
1843+
1844+
nested_metadata = {
1845+
"tags": ["sample", "test"],
1846+
"nested": {"key": "value", "count": 42},
1847+
"flat_string": "just-a-string",
1848+
"number": 123,
1849+
}
1850+
1851+
# Async path
1852+
obj = FileObject(backend=backend, filename="nested_async.txt", metadata=nested_metadata)
1853+
updated_obj = await backend.save_object_async(obj, b"async content")
1854+
assert updated_obj.metadata == nested_metadata
1855+
1856+
# Sync path
1857+
obj_sync = FileObject(backend=backend, filename="nested_sync.txt", metadata=nested_metadata)
1858+
updated_obj_sync = backend.save_object(obj_sync, b"sync content")
1859+
assert updated_obj_sync.metadata == nested_metadata
1860+
1861+
18341862
@pytest.mark.xdist_group("file_object")
18351863
async def test_obstore_content_type_guessing(storage_registry: StorageRegistry) -> None:
18361864
"""Test that content_type is properly guessed when not explicitly set."""

tests/unit/test_extensions/test_litestar/test_dto.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,47 @@ class B(Base):
540540
assert model.a is None
541541

542542

543+
async def test_dto_nullable_one_to_one_inverse_relationship(
544+
create_module: Callable[[str], ModuleType],
545+
asgi_connection: Request[Any, Any, Any],
546+
) -> None:
547+
"""Test that nullable one-to-one relationships on the inverse side (no FK) are handled correctly.
548+
549+
Regression test for https://github.com/jolt-org/advanced-alchemy/issues/227.
550+
When the FK is on the other model and uselist=False, the DTO should allow None.
551+
"""
552+
module = create_module(
553+
"""
554+
from __future__ import annotations
555+
556+
from typing import Optional
557+
558+
from sqlalchemy import ForeignKey
559+
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column, relationship
560+
from typing_extensions import Annotated
561+
562+
from advanced_alchemy.extensions.litestar.dto import SQLAlchemyDTO, SQLAlchemyDTOConfig
563+
564+
class Base(DeclarativeBase):
565+
id: Mapped[int] = mapped_column(primary_key=True)
566+
567+
class B(Base):
568+
__tablename__ = "b"
569+
a_id: Mapped[int] = mapped_column(ForeignKey("a.id"))
570+
a: Mapped["A"] = relationship(back_populates="b")
571+
572+
class A(Base):
573+
__tablename__ = "a"
574+
b: Mapped[Optional[B]] = relationship(back_populates="a")
575+
576+
dto_type = SQLAlchemyDTO[Annotated[A, SQLAlchemyDTOConfig()]]
577+
""",
578+
)
579+
model = await get_model_from_dto(module.dto_type, module.A, asgi_connection, b'{"id": 1, "b": null}')
580+
assert isinstance(model, module.A)
581+
assert model.b is None
582+
583+
543584
async def test_forward_ref_relationship_resolution(
544585
create_module: Callable[[str], ModuleType],
545586
asgi_connection: Request[Any, Any, Any],

0 commit comments

Comments
 (0)