Skip to content

Commit 20a1f71

Browse files
authored
feat(service): allow delete_many to accept model instances (#654) (#752)
## Summary `delete_many` previously accepted only primary-key values; passing model instances raised `invalid input syntax for type uuid: "<Model object ...>"`. It now also accepts model instances, with **per-item detection** so a single call may mix raw IDs and instances. ## Change - `SQLAlchemyAsyncRepositoryService.delete_many` (+ unasyncd-generated sync twin) normalizes each item before delegating: - model instance → PK extracted via existing repo helpers (`get_id_attribute_value`, or `get_primary_key_value` for composite keys) - raw value → passed through unchanged - Purely additive; no existing caller behavior changes. ## Tests - `test_service_delete_many_accepts_instances` (list of instances) - `test_service_delete_many_accepts_mixed_instances_and_ids` (mixed) - Verified failing before / passing after on sqlite (async + sync, uuid + bigint); existing delete + composite-PK tests still green. Closes #654
1 parent bbf14ba commit 20a1f71

5 files changed

Lines changed: 159 additions & 7 deletions

File tree

advanced_alchemy/service/_async.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
)
2828
from advanced_alchemy.repository._util import LoadSpec, model_from_dict
2929
from advanced_alchemy.repository.typing import MISSING, ModelT, OrderingPair, PrimaryKeyType, SQLAlchemyAsyncRepositoryT
30-
from advanced_alchemy.service._util import ResultConverter
30+
from advanced_alchemy.service._util import ResultConverter, resolve_item_ids
3131
from advanced_alchemy.utils.dataclass import Empty, EmptyType
3232
from advanced_alchemy.utils.deprecation import warn_deprecation
3333
from advanced_alchemy.utils.serialization import (
@@ -1337,6 +1337,8 @@ async def delete_many(
13371337
item_ids: List of identifiers of instances to be deleted.
13381338
For single primary key models, pass a list of scalar values.
13391339
For composite primary key models, pass a list of tuples or dicts.
1340+
Model instances may also be passed in place of identifiers, and the list
1341+
may mix instances with raw identifier values.
13401342
auto_expunge: Remove object from session before returning.
13411343
auto_commit: Commit objects before returning.
13421344
id_attribute: Allows customization of the unique identifier to use for model fetching.
@@ -1377,7 +1379,12 @@ async def delete_many(
13771379
return cast(
13781380
"Sequence[ModelT]",
13791381
await self.repository.delete_many(
1380-
item_ids=item_ids,
1382+
item_ids=resolve_item_ids(
1383+
item_ids,
1384+
model_type=self.model_type,
1385+
repository=self.repository,
1386+
id_attribute=id_attribute,
1387+
),
13811388
auto_commit=auto_commit,
13821389
auto_expunge=auto_expunge,
13831390
id_attribute=id_attribute,

advanced_alchemy/service/_sync.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
from advanced_alchemy.repository import SQLAlchemySyncQueryRepository
2727
from advanced_alchemy.repository._util import LoadSpec, model_from_dict
2828
from advanced_alchemy.repository.typing import MISSING, ModelT, OrderingPair, PrimaryKeyType, SQLAlchemySyncRepositoryT
29-
from advanced_alchemy.service._util import ResultConverter
29+
from advanced_alchemy.service._util import ResultConverter, resolve_item_ids
3030
from advanced_alchemy.utils.dataclass import Empty, EmptyType
3131
from advanced_alchemy.utils.deprecation import warn_deprecation
3232
from advanced_alchemy.utils.serialization import (
@@ -1336,6 +1336,8 @@ def delete_many(
13361336
item_ids: List of identifiers of instances to be deleted.
13371337
For single primary key models, pass a list of scalar values.
13381338
For composite primary key models, pass a list of tuples or dicts.
1339+
Model instances may also be passed in place of identifiers, and the list
1340+
may mix instances with raw identifier values.
13391341
auto_expunge: Remove object from session before returning.
13401342
auto_commit: Commit objects before returning.
13411343
id_attribute: Allows customization of the unique identifier to use for model fetching.
@@ -1376,7 +1378,12 @@ def delete_many(
13761378
return cast(
13771379
"Sequence[ModelT]",
13781380
self.repository.delete_many(
1379-
item_ids=item_ids,
1381+
item_ids=resolve_item_ids(
1382+
item_ids,
1383+
model_type=self.model_type,
1384+
repository=self.repository,
1385+
id_attribute=id_attribute,
1386+
),
13801387
auto_commit=auto_commit,
13811388
auto_expunge=auto_expunge,
13821389
id_attribute=id_attribute,

advanced_alchemy/service/_util.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
from advanced_alchemy.exceptions import AdvancedAlchemyError
1616
from advanced_alchemy.filters import LimitOffset, StatementFilter
17+
from advanced_alchemy.repository.typing import PrimaryKeyType
1718
from advanced_alchemy.service.pagination import OffsetPagination
1819
from advanced_alchemy.typing import (
1920
ATTRS_INSTALLED,
@@ -41,7 +42,7 @@
4142
from advanced_alchemy.base import ModelProtocol
4243
from advanced_alchemy.repository.typing import ModelOrRowMappingT
4344

44-
__all__ = ("ResultConverter", "find_filter")
45+
__all__ = ("ResultConverter", "find_filter", "resolve_item_ids")
4546

4647
DEFAULT_TYPE_DECODERS = [ # pyright: ignore[reportUnknownVariableType]
4748
(lambda x: x is UUID, lambda t, v: t(v.hex)), # pyright: ignore[reportUnknownLambdaType,reportUnknownMemberType]
@@ -71,6 +72,28 @@ def find_filter(
7172
)
7273

7374

75+
def resolve_item_ids(
76+
item_ids: list[PrimaryKeyType],
77+
*,
78+
model_type: type[Any],
79+
repository: Any,
80+
id_attribute: Any = None,
81+
) -> list[PrimaryKeyType]:
82+
"""Resolve model instances in ``item_ids`` without copying id-only input."""
83+
resolved_item_ids: Optional[list[PrimaryKeyType]] = None
84+
for index, item in enumerate(item_ids):
85+
if isinstance(item, model_type):
86+
if resolved_item_ids is None:
87+
resolved_item_ids = item_ids[:index]
88+
if repository.has_composite_pk:
89+
resolved_item_ids.append(repository.get_primary_key_value(item))
90+
else:
91+
resolved_item_ids.append(repository.get_id_attribute_value(item=item, id_attribute=id_attribute))
92+
elif resolved_item_ids is not None:
93+
resolved_item_ids.append(item)
94+
return item_ids if resolved_item_ids is None else resolved_item_ids
95+
96+
7497
class ResultConverter:
7598
"""Simple mixin to help convert to a paginated response model.
7699

tests/integration/test_repository.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,41 @@ async def test_service_delete_method(seeded_test_session_async: "tuple[AsyncSess
562562
assert len(remaining_authors) == 1
563563

564564

565+
async def test_service_delete_many_accepts_instances(
566+
seeded_test_session_async: "tuple[AsyncSession, dict[str, type]]",
567+
) -> None:
568+
"""Test service delete_many accepts model instances as well as raw primary keys."""
569+
author_service = get_service_from_session(seeded_test_session_async, "author")
570+
571+
authors = await maybe_async(author_service.get_many())
572+
assert len(authors) == 2
573+
574+
deleted = await maybe_async(author_service.delete_many(list(authors)))
575+
assert len(deleted) == 2
576+
assert {a.id for a in deleted} == {a.id for a in authors}
577+
578+
remaining = await maybe_async(author_service.get_many())
579+
assert len(remaining) == 0
580+
581+
582+
async def test_service_delete_many_accepts_mixed_instances_and_ids(
583+
seeded_test_session_async: "tuple[AsyncSession, dict[str, type]]",
584+
) -> None:
585+
"""Test service delete_many accepts a mixed list of model instances and raw primary keys."""
586+
author_service = get_service_from_session(seeded_test_session_async, "author")
587+
588+
authors = await maybe_async(author_service.get_many())
589+
assert len(authors) == 2
590+
591+
mixed = [authors[0], authors[1].id]
592+
deleted = await maybe_async(author_service.delete_many(mixed))
593+
assert len(deleted) == 2
594+
assert {a.id for a in deleted} == {a.id for a in authors}
595+
596+
remaining = await maybe_async(author_service.get_many())
597+
assert len(remaining) == 0
598+
599+
565600
# Additional filter tests
566601
async def test_repo_filter_before_after(seeded_test_session_async: "tuple[AsyncSession, dict[str, type]]") -> None:
567602
"""Test repository with BeforeAfter filter."""
@@ -1174,6 +1209,29 @@ async def test_composite_pk_delete_many_by_dicts(
11741209
await maybe_async(user_role_repo.get((2, 10)))
11751210

11761211

1212+
async def test_composite_pk_service_delete_many_accepts_mixed_instances_and_ids(
1213+
seeded_test_session_async: "tuple[AsyncSession, dict[str, type]]",
1214+
) -> None:
1215+
"""Test service delete_many accepts mixed model instances and raw composite keys."""
1216+
session, models = seeded_test_session_async
1217+
if "user_role" not in models:
1218+
pytest.skip("user_role model not available")
1219+
1220+
user_role_service = create_service(session, models["user_role"])
1221+
1222+
roles = await maybe_async(user_role_service.get_many())
1223+
assert len(roles) == 3
1224+
1225+
mixed = [roles[0], (roles[1].user_id, roles[1].role_id)]
1226+
deleted = await maybe_async(user_role_service.delete_many(mixed))
1227+
1228+
assert len(deleted) == 2
1229+
assert {(role.user_id, role.role_id) for role in deleted} == {
1230+
(roles[0].user_id, roles[0].role_id),
1231+
(roles[1].user_id, roles[1].role_id),
1232+
}
1233+
1234+
11771235
async def test_composite_pk_count(
11781236
seeded_test_session_async: "tuple[AsyncSession, dict[str, type]]",
11791237
) -> None:

tests/unit/test_service_to_model_flow.py

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
"""Unit tests for service.update() model conversion flow.
1+
"""Unit tests for service model conversion flow.
22
33
These tests verify that update input types are routed through to_model(data, "update")
4-
and the update lifecycle hook before persistence.
4+
and the update lifecycle hook before persistence. They also cover service-level
5+
input normalization before repository delegation.
56
"""
67

78
from __future__ import annotations
@@ -17,6 +18,7 @@
1718
from advanced_alchemy.base import UUIDAuditBase
1819
from advanced_alchemy.repository import SQLAlchemyAsyncRepository, SQLAlchemySyncRepository
1920
from advanced_alchemy.repository._util import get_primary_key_info
21+
from advanced_alchemy.repository.typing import PrimaryKeyType
2022
from advanced_alchemy.service import SchemaDumpConfig, SQLAlchemyAsyncRepositoryService, SQLAlchemySyncRepositoryService
2123
from advanced_alchemy.utils.serialization import ATTRS_INSTALLED, MSGSPEC_INSTALLED, PYDANTIC_INSTALLED, ModelDictT
2224

@@ -141,6 +143,24 @@ def to_model_on_update(self, data: ModelDictT[MockModel]) -> ModelDictT[MockMode
141143
# Tests for async service
142144

143145

146+
@pytest.mark.asyncio
147+
async def test_delete_many_passes_id_only_input_through_without_copying() -> None:
148+
"""Id-only delete_many() input should keep the existing bulk-delete fast path."""
149+
service = TrackingService()
150+
captured: dict[str, Any] = {}
151+
152+
async def delete_many(item_ids: list[Any], **_: Any) -> list[MockModel]:
153+
captured["item_ids"] = item_ids
154+
return []
155+
156+
service.repository.delete_many = AsyncMock(side_effect=delete_many) # type: ignore[method-assign]
157+
158+
item_ids: list[PrimaryKeyType] = ["first-id", "second-id"]
159+
await service.delete_many(item_ids)
160+
161+
assert captured["item_ids"] is item_ids
162+
163+
144164
@pytest.mark.asyncio
145165
async def test_update_dict_calls_to_model_with_operation() -> None:
146166
"""Test that update() with dict data calls to_model(data, 'update')."""
@@ -358,6 +378,43 @@ async def test_update_propagates_with_for_update_flag() -> None:
358378
# Tests for sync service
359379

360380

381+
def test_sync_delete_many_passes_id_only_input_through_without_copying() -> None:
382+
"""Sync id-only delete_many() input should keep the existing bulk-delete fast path."""
383+
service = TrackingSyncService()
384+
captured: dict[str, Any] = {}
385+
386+
def delete_many(item_ids: list[Any], **_: Any) -> list[MockModel]:
387+
captured["item_ids"] = item_ids
388+
return []
389+
390+
service.repository.delete_many = MagicMock(side_effect=delete_many) # type: ignore[method-assign]
391+
392+
item_ids: list[PrimaryKeyType] = ["first-id", "second-id"]
393+
service.delete_many(item_ids)
394+
395+
assert captured["item_ids"] is item_ids
396+
397+
398+
def test_sync_delete_many_resolves_model_instances_before_delegating() -> None:
399+
"""Sync delete_many() should resolve model instances to primary key values."""
400+
service = TrackingSyncService()
401+
captured: dict[str, Any] = {}
402+
403+
def delete_many(item_ids: list[Any], **_: Any) -> list[MockModel]:
404+
captured["item_ids"] = item_ids
405+
return []
406+
407+
service.repository.delete_many = MagicMock(side_effect=delete_many) # type: ignore[method-assign]
408+
409+
model = MockModel()
410+
model.id = "model-id" # type: ignore[assignment]
411+
item_ids = cast("list[PrimaryKeyType]", [model, "raw-id"])
412+
service.delete_many(item_ids)
413+
414+
assert captured["item_ids"] == ["model-id", "raw-id"]
415+
assert captured["item_ids"] is not item_ids
416+
417+
361418
def test_sync_update_dict_calls_to_model_with_operation() -> None:
362419
"""Test that sync update() with dict data calls to_model(data, 'update')."""
363420
service = TrackingSyncService()

0 commit comments

Comments
 (0)