From 618864af0d951a9c151ffc0b38d988783c2e3fe8 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Tue, 30 Jun 2026 16:55:35 -0400 Subject: [PATCH 1/4] feat: fixture refresh DB-generated values onto returned instances --- src/fastapi_toolsets/fixtures/utils.py | 100 ++++++++++++++++-- src/fastapi_toolsets/pytest/plugin.py | 138 +++++-------------------- tests/test_fixtures.py | 40 +++++++ tests/test_pytest.py | 2 +- 4 files changed, 161 insertions(+), 119 deletions(-) diff --git a/src/fastapi_toolsets/fixtures/utils.py b/src/fastapi_toolsets/fixtures/utils.py index d25a978f..939a481b 100644 --- a/src/fastapi_toolsets/fixtures/utils.py +++ b/src/fastapi_toolsets/fixtures/utils.py @@ -2,12 +2,14 @@ from collections.abc import Callable, Sequence from enum import Enum -from typing import Any +from typing import Any, cast +from sqlalchemy import Table, select from sqlalchemy import inspect as sa_inspect from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import DeclarativeBase +from sqlalchemy.orm import DeclarativeBase, selectinload +from sqlalchemy.orm.interfaces import ExecutableOption, ORMOption from ..db import transaction from ..logger import get_logger @@ -98,12 +100,28 @@ async def _batch_insert( model_cls: type[DeclarativeBase], instances: list[DeclarativeBase], ) -> None: - """INSERT all instances — raises on conflict (no duplicate handling).""" + """INSERT all instances, raises on conflict.""" for cls in _get_table_chain(model_cls): + table = cast(Table, cls.__table__) dicts = [_instance_to_dict_for_cls(i, cls) for i in instances] - for group_dicts, _ in _group_by_column_set(dicts, instances): - if group_dicts and group_dicts[0]: # pragma: no branch - await session.execute(pg_insert(cls).values(group_dicts)) + for group_dicts, group_instances in _group_by_column_set(dicts, instances): + if not group_dicts or not group_dicts[0]: # pragma: no cover + continue + missing_pk_cols = [ + col + for col in table.primary_key.columns + if col.key not in group_dicts[0] + ] + if not missing_pk_cols: + await session.execute(pg_insert(table), group_dicts) + continue + stmt = pg_insert(table).returning( + *missing_pk_cols, sort_by_parameter_order=True + ) + result = await session.execute(stmt, group_dicts) + for inst, row in zip(group_instances, result): + for col, val in zip(missing_pk_cols, row): + setattr(inst, col.key, val) async def _batch_merge( @@ -169,8 +187,14 @@ async def _batch_skip_existing( loaded = list(no_pk) if no_pk: no_pk_dicts = [_instance_to_dict(i) for i in no_pk] - for group_dicts, _ in _group_by_column_set(no_pk_dicts, no_pk): - await session.execute(pg_insert(model_cls).values(group_dicts)) + for group_dicts, group_instances in _group_by_column_set(no_pk_dicts, no_pk): + stmt = pg_insert(cast(Table, model_cls.__table__)).returning( + *mapper.primary_key, sort_by_parameter_order=True + ) + result = await session.execute(stmt, group_dicts) + for inst, row in zip(group_instances, result): + for col, val in zip(mapper.primary_key, row): + setattr(inst, col.key, val) if with_pk_pairs: with_pk = [i for i, _ in with_pk_pairs] @@ -196,6 +220,64 @@ async def _batch_skip_existing( return loaded +def _relationship_load_options(model: type[DeclarativeBase]) -> list[ExecutableOption]: + """Build selectinload options for all direct relationships on a model.""" + return [ + selectinload(getattr(model, rel.key)) for rel in model.__mapper__.relationships + ] + + +async def _reload_with_relationships( + session: AsyncSession, + instances: list[DeclarativeBase], + load_options: list[ExecutableOption], +) -> list[DeclarativeBase]: + """Reload instances in a single bulk query with relationship eager-loading.""" + model = type(instances[0]) + mapper = model.__mapper__ + pk_cols = mapper.primary_key + + if len(pk_cols) == 1: + pk_attr = getattr(model, pk_cols[0].key) + pks = [getattr(inst, pk_cols[0].key) for inst in instances] + result = await session.execute( + select(model).where(pk_attr.in_(pks)).options(*load_options) + ) + by_pk = {getattr(row, pk_cols[0].key): row for row in result.unique().scalars()} + return [by_pk[pk] for pk in pks] + + # Composite PK: fall back to per-instance reload + reloaded: list[DeclarativeBase] = [] + for instance in instances: + pk = _get_primary_key(instance) + refreshed = await session.get( + model, + pk, + options=cast(list[ORMOption], load_options), + populate_existing=True, + ) + if refreshed is not None: # pragma: no branch + reloaded.append(refreshed) + return reloaded + + +async def _refresh_loaded( + session: AsyncSession, instances: list[DeclarativeBase] +) -> list[DeclarativeBase]: + """Re-select freshly written rows, eager-loading relationships.""" + if not instances: + return [] + refreshed: list[DeclarativeBase | None] = [None] * len(instances) + for model_cls, group in _group_by_type(instances): + positions = [i for i, inst in enumerate(instances) if type(inst) is model_cls] + load_options = _relationship_load_options(model_cls) + for pos, new in zip( + positions, await _reload_with_relationships(session, group, load_options) + ): + refreshed[pos] = new + return cast(list[DeclarativeBase], refreshed) + + async def _load_ordered( session: AsyncSession, registry: FixtureRegistry, @@ -244,6 +326,8 @@ async def _load_ordered( case _: # pragma: no cover pass + loaded = await _refresh_loaded(session, loaded) + results[name] = loaded logger.info(f"Loaded fixture '{name}': {len(loaded)} {model_name}(s)") diff --git a/src/fastapi_toolsets/pytest/plugin.py b/src/fastapi_toolsets/pytest/plugin.py index 1b2b5eb1..56f05562 100644 --- a/src/fastapi_toolsets/pytest/plugin.py +++ b/src/fastapi_toolsets/pytest/plugin.py @@ -1,16 +1,14 @@ """Pytest plugin for using FixtureRegistry fixtures in tests.""" -from collections.abc import Callable, Sequence -from typing import Any, cast +from collections.abc import Sequence +from typing import Any import pytest -from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import DeclarativeBase, selectinload -from sqlalchemy.orm.interfaces import ExecutableOption, ORMOption +from sqlalchemy.orm import DeclarativeBase -from ..db import transaction from ..fixtures import FixtureRegistry, LoadStrategy +from ..fixtures.utils import _get_primary_key, _load_ordered, _refresh_loaded def register_fixtures( @@ -83,56 +81,38 @@ def _create_fixture_function( fixture_name: str, dependencies: list[str], strategy: LoadStrategy, -) -> Callable[..., Any]: +) -> Any: """Create a fixture function with the correct signature. The function signature must include all dependencies as parameters - for pytest to resolve them correctly. + for pytest (and pytest-anyio's fixture chaining) to resolve them + correctly — dynamic resolution via ``request.getfixturevalue`` deadlocks + when called from inside an already-running async fixture. """ - # Get the fixture definition fixture_def = registry.get(fixture_name) - # Build the function dynamically with correct parameters - # We need the session as first param, then all dependencies async def fixture_func(**kwargs: Any) -> Sequence[DeclarativeBase]: - # Get session from kwargs (first dependency) session: AsyncSession = kwargs[dependencies[0]] - - # Load the fixture data - instances = list(fixture_def.func()) - - if not instances: - return [] - - loaded: list[DeclarativeBase] = [] - - async with transaction(session): - for instance in instances: - if strategy == LoadStrategy.INSERT: - session.add(instance) - loaded.append(instance) - elif strategy == LoadStrategy.MERGE: - merged = await session.merge(instance) - loaded.append(merged) - elif strategy == LoadStrategy.SKIP_EXISTING: # pragma: no branch - pk = _get_primary_key(instance) - if pk is not None: - existing = await session.get(type(instance), pk) - if existing is None: - session.add(instance) - loaded.append(instance) - else: - loaded.append(existing) - else: - session.add(instance) - loaded.append(instance) - - if loaded: # pragma: no branch - load_options = _relationship_load_options(type(loaded[0])) - if load_options: - return await _reload_with_relationships(session, loaded, load_options) - - return loaded + result = (await _load_ordered(session, registry, [fixture_name], strategy))[ + fixture_name + ] + + if strategy is LoadStrategy.SKIP_EXISTING: + # _load_ordered only returns newly-inserted rows for this + # strategy (the CLI seeding contract). A test fixture should + # still hand back the full, usable set including rows that + # were already present, so top up with those. + declared = list(fixture_def.func()) + result_pks = {_get_primary_key(r) for r in result} + missing = [ + d + for d in declared + if (pk := _get_primary_key(d)) is not None and pk not in result_pks + ] + if missing: + result = result + await _refresh_loaded(session, missing) + + return result # Update function signature to include dependencies # This is needed for pytest to inject the right fixtures @@ -146,65 +126,3 @@ async def fixture_func(**kwargs: Any) -> Sequence[DeclarativeBase]: created_func.__doc__ = f"Load {fixture_name} fixture data." return created_func - - -def _relationship_load_options(model: type[DeclarativeBase]) -> list[ExecutableOption]: - """Build selectinload options for all direct relationships on a model.""" - return [ - selectinload(getattr(model, rel.key)) for rel in model.__mapper__.relationships - ] - - -async def _reload_with_relationships( - session: AsyncSession, - instances: list[DeclarativeBase], - load_options: list[ExecutableOption], -) -> list[DeclarativeBase]: - """Reload instances in a single bulk query with relationship eager-loading. - - Uses one SELECT … WHERE pk IN (…) so selectinload can batch all relationship - queries — 1 + N_relationships round-trips regardless of how many instances - there are, instead of one session.get() per instance. - - Preserves the original insertion order. - """ - model = type(instances[0]) - mapper = model.__mapper__ - pk_cols = mapper.primary_key - - if len(pk_cols) == 1: - pk_attr = getattr(model, pk_cols[0].key) - pks = [getattr(inst, pk_cols[0].key) for inst in instances] - result = await session.execute( - select(model).where(pk_attr.in_(pks)).options(*load_options) - ) - by_pk = {getattr(row, pk_cols[0].key): row for row in result.unique().scalars()} - return [by_pk[pk] for pk in pks] - - # Composite PK: fall back to per-instance reload - reloaded: list[DeclarativeBase] = [] - for instance in instances: - pk = _get_primary_key(instance) - refreshed = await session.get( - model, - pk, - options=cast(list[ORMOption], load_options), - populate_existing=True, - ) - if refreshed is not None: # pragma: no branch - reloaded.append(refreshed) - return reloaded - - -def _get_primary_key(instance: DeclarativeBase) -> Any | None: - """Get the primary key value of a model instance.""" - mapper = instance.__class__.__mapper__ - pk_cols = mapper.primary_key - - if len(pk_cols) == 1: - return getattr(instance, pk_cols[0].name, None) - - pk_values = tuple(getattr(instance, col.name, None) for col in pk_cols) - if all(v is not None for v in pk_values): - return pk_values - return None diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py index ab8ab2c1..623fe9f6 100644 --- a/tests/test_fixtures.py +++ b/tests/test_fixtures.py @@ -2,6 +2,7 @@ import uuid from enum import Enum +from typing import cast import pytest from sqlalchemy.ext.asyncio import AsyncSession @@ -812,6 +813,45 @@ def int_roles(): db_session, registry, "int_roles", strategy=LoadStrategy.SKIP_EXISTING ) assert len(result["int_roles"]) == 1 + # The generated autoincrement PK must be written back onto the + # returned instance, not just visible via a fresh DB query. + assert cast(IntRole, result["int_roles"][0]).id is not None + + @pytest.mark.anyio + async def test_insert_refreshes_autoincrement_pk_on_returned_instance( + self, db_session: AsyncSession + ): + """INSERT strategy writes the generated PK back onto the returned instance.""" + registry = FixtureRegistry() + + @registry.register + def int_roles(): + return [IntRole(name="auto")] + + result = await load_fixtures( + db_session, registry, "int_roles", strategy=LoadStrategy.INSERT + ) + assert cast(IntRole, result["int_roles"][0]).id is not None + + @pytest.mark.anyio + async def test_merge_refreshes_server_default_on_returned_instance( + self, db_session: AsyncSession + ): + """MERGE strategy refreshes the returned instance with server-generated values.""" + registry = FixtureRegistry() + + @registry.register + def challenges(): + return [ + Challenge(id=uuid.uuid4(), title="Solo", challenge_type="challenge") + ] + + result = await load_fixtures( + db_session, registry, "challenges", strategy=LoadStrategy.MERGE + ) + # `points` has a column default of 0 applied by the DB, never set on + # the in-memory instance — the returned object must reflect it. + assert cast(Challenge, result["challenges"][0]).points == 0 class TestLoadFixturesByContext: diff --git a/tests/test_pytest.py b/tests/test_pytest.py index 2063024e..77af6285 100644 --- a/tests/test_pytest.py +++ b/tests/test_pytest.py @@ -20,7 +20,7 @@ register_fixtures, worker_database_url, ) -from fastapi_toolsets.pytest.plugin import ( +from fastapi_toolsets.fixtures.utils import ( _get_primary_key, _relationship_load_options, _reload_with_relationships, From 9f6e10871754f4df108c6393061503ad28330913 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Wed, 1 Jul 2026 12:24:29 -0400 Subject: [PATCH 2/4] refactor: replace utils.get_obj_by_attr/get_field_by_attr with registry.obj/field lookups --- docs/module/fixtures.md | 18 ++++++-- docs/reference/fixtures.md | 3 -- src/fastapi_toolsets/fixtures/__init__.py | 9 +--- src/fastapi_toolsets/fixtures/registry.py | 43 +++++++++++++++++ src/fastapi_toolsets/fixtures/utils.py | 52 --------------------- tests/test_fixtures.py | 56 ++++++++++++++--------- 6 files changed, 92 insertions(+), 89 deletions(-) diff --git a/docs/module/fixtures.md b/docs/module/fixtures.md index dd2df82a..da0823d8 100644 --- a/docs/module/fixtures.md +++ b/docs/module/fixtures.md @@ -147,18 +147,26 @@ Fixtures with the same name are allowed as long as their context sets do not ove ## Looking up fixture instances -[`get_obj_by_attr`](../reference/fixtures.md#fastapi_toolsets.fixtures.utils.get_obj_by_attr) retrieves a specific instance from a fixture function by attribute value — useful when building cross-fixture `depends_on` relationships: +[`FixtureRegistry.obj`](../reference/fixtures.md#fastapi_toolsets.fixtures.registry.FixtureRegistry.obj) retrieves a specific instance from a registered fixture by attribute value, looked up by name on the registry — useful when building cross-fixture `depends_on` relationships: ```python -from fastapi_toolsets.fixtures import get_obj_by_attr - @fixtures.register(depends_on=["roles"]) def users(): - admin_role = get_obj_by_attr(roles, "name", "admin") + admin_role = fixtures.obj("roles", "name", "admin") return [User(id=1, username="alice", role_id=admin_role.id)] ``` -Raises `StopIteration` if no matching instance is found. +Looking the fixture up by name (instead of importing the `roles` function directly) means fixture modules never need to import each other, which avoids circular imports in larger projects split across multiple files — the same reason `depends_on` takes fixture names rather than the functions themselves. The registry passed in must be the one that actually contains the fixture by load time; with a single shared registry this is automatic, but if you merge registries with `include_registry`, call `obj`/`field` on the merged registry. + +[`FixtureRegistry.field`](../reference/fixtures.md#fastapi_toolsets.fixtures.registry.FixtureRegistry.field) is shorthand for pulling a single attribute (`id` by default): + +```python +@fixtures.register(depends_on=["roles"]) +def users(): + return [User(id=1, username="alice", role_id=fixtures.field("roles", "name", "admin"))] +``` + +Both raise `StopIteration` if no matching instance is found, and `KeyError` if the fixture name isn't registered. ## Pytest integration diff --git a/docs/reference/fixtures.md b/docs/reference/fixtures.md index 9239c5de..038b6044 100644 --- a/docs/reference/fixtures.md +++ b/docs/reference/fixtures.md @@ -12,7 +12,6 @@ from fastapi_toolsets.fixtures import ( FixtureRegistry, load_fixtures, load_fixtures_by_context, - get_obj_by_attr, ) ``` @@ -27,5 +26,3 @@ from fastapi_toolsets.fixtures import ( ## ::: fastapi_toolsets.fixtures.utils.load_fixtures ## ::: fastapi_toolsets.fixtures.utils.load_fixtures_by_context - -## ::: fastapi_toolsets.fixtures.utils.get_obj_by_attr diff --git a/src/fastapi_toolsets/fixtures/__init__.py b/src/fastapi_toolsets/fixtures/__init__.py index 66a2e7da..faa21af2 100644 --- a/src/fastapi_toolsets/fixtures/__init__.py +++ b/src/fastapi_toolsets/fixtures/__init__.py @@ -2,19 +2,12 @@ from .enum import LoadStrategy from .registry import Context, FixtureRegistry -from .utils import ( - get_field_by_attr, - get_obj_by_attr, - load_fixtures, - load_fixtures_by_context, -) +from .utils import load_fixtures, load_fixtures_by_context __all__ = [ "Context", "FixtureRegistry", "LoadStrategy", - "get_field_by_attr", - "get_obj_by_attr", "load_fixtures", "load_fixtures_by_context", "register_fixtures", diff --git a/src/fastapi_toolsets/fixtures/registry.py b/src/fastapi_toolsets/fixtures/registry.py index 9dabe83c..f42a9663 100644 --- a/src/fastapi_toolsets/fixtures/registry.py +++ b/src/fastapi_toolsets/fixtures/registry.py @@ -227,6 +227,49 @@ def get_all(self) -> list[Fixture]: """Get all registered fixtures (all variants of all names).""" return [f for variants in self._fixtures.values() for f in variants] + def obj(self, name: str, attr_name: str, value: Any) -> DeclarativeBase: + """Get a model instance from a registered fixture by attribute value. + + Args: + name: Fixture name to look up. + attr_name: Name of the attribute to match against. + value: Value to match. + + Returns: + The first model instance where the attribute matches the given value. + + Raises: + KeyError: If no fixture named *name* is registered. + StopIteration: If no matching object is found. + """ + instances = ( + obj for variant in self.get_variants(name) for obj in variant.func() + ) + try: + return next(obj for obj in instances if getattr(obj, attr_name) == value) + except StopIteration: + raise StopIteration( + f"No object with {attr_name}={value} found in fixture '{name}'" + ) from None + + def field(self, name: str, attr_name: str, value: Any, *, field: str = "id") -> Any: + """Get a single field value from a fixture object matched by an attribute. + + Args: + name: Fixture name to look up. + attr_name: Name of the attribute to match against. + value: Value to match. + field: Attribute name to return from the matched object (default: ``"id"``). + + Returns: + The value of ``field`` on the first matching model instance. + + Raises: + KeyError: If no fixture named *name* is registered. + StopIteration: If no matching object is found. + """ + return getattr(self.obj(name, attr_name, value), field) + def get_by_context(self, *contexts: str | Enum) -> list[Fixture]: """Get fixtures for specific contexts.""" context_values = set(_normalize_contexts(contexts)) diff --git a/src/fastapi_toolsets/fixtures/utils.py b/src/fastapi_toolsets/fixtures/utils.py index 939a481b..a76d0ef9 100644 --- a/src/fastapi_toolsets/fixtures/utils.py +++ b/src/fastapi_toolsets/fixtures/utils.py @@ -1,6 +1,5 @@ """Fixture loading utilities for database seeding.""" -from collections.abc import Callable, Sequence from enum import Enum from typing import Any, cast @@ -13,7 +12,6 @@ from ..db import transaction from ..logger import get_logger -from ..types import ModelType from .enum import LoadStrategy from .registry import FixtureRegistry, _normalize_contexts @@ -348,56 +346,6 @@ def _get_primary_key(instance: DeclarativeBase) -> Any | None: return None -def get_obj_by_attr( - fixtures: Callable[[], Sequence[ModelType]], attr_name: str, value: Any -) -> ModelType: - """Get a SQLAlchemy model instance by matching an attribute value. - - Args: - fixtures: A fixture function registered via ``@registry.register`` - that returns a sequence of SQLAlchemy model instances. - attr_name: Name of the attribute to match against. - value: Value to match. - - Returns: - The first model instance where the attribute matches the given value. - - Raises: - StopIteration: If no matching object is found in the fixture group. - """ - try: - return next(obj for obj in fixtures() if getattr(obj, attr_name) == value) - except StopIteration: - raise StopIteration( - f"No object with {attr_name}={value} found in fixture '{getattr(fixtures, '__name__', repr(fixtures))}'" - ) from None - - -def get_field_by_attr( - fixtures: Callable[[], Sequence[ModelType]], - attr_name: str, - value: Any, - *, - field: str = "id", -) -> Any: - """Get a single field value from a fixture object matched by an attribute. - - Args: - fixtures: A fixture function registered via ``@registry.register`` - that returns a sequence of SQLAlchemy model instances. - attr_name: Name of the attribute to match against. - value: Value to match. - field: Attribute name to return from the matched object (default: ``"id"``). - - Returns: - The value of ``field`` on the first matching model instance. - - Raises: - StopIteration: If no matching object is found in the fixture group. - """ - return getattr(get_obj_by_attr(fixtures, attr_name, value), field) - - async def load_fixtures( session: AsyncSession, registry: FixtureRegistry, diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py index 623fe9f6..2122aa7f 100644 --- a/tests/test_fixtures.py +++ b/tests/test_fixtures.py @@ -11,8 +11,6 @@ Context, FixtureRegistry, LoadStrategy, - get_field_by_attr, - get_obj_by_attr, load_fixtures, load_fixtures_by_context, ) @@ -931,8 +929,8 @@ def test_users(): assert await UserCrud.count(db_session) == 1 -class TestGetObjByAttr: - """Tests for get_obj_by_attr helper function.""" +class TestRegistryObj: + """Tests for FixtureRegistry.obj.""" def setup_method(self): """Set up test fixtures for each test.""" @@ -974,23 +972,20 @@ def users() -> list[User]: ), ] - self.roles = roles - self.users = users - def test_get_by_id(self): """Get an object by its id attribute.""" - role = get_obj_by_attr(self.roles, "id", self.role_id_1) - assert role.name == "admin" + role = self.registry.obj("roles", "id", self.role_id_1) + assert cast(Role, role).name == "admin" def test_get_user_by_username(self): """Get a user by username.""" - user = get_obj_by_attr(self.users, "username", "bob") + user = cast(User, self.registry.obj("users", "username", "bob")) assert user.id == self.user_id_2 assert user.email == "bob@example.com" def test_returns_first_match(self): """Returns the first matching object when multiple could match.""" - user = get_obj_by_attr(self.users, "role_id", self.role_id_1) + user = cast(User, self.registry.obj("users", "role_id", self.role_id_1)) assert user.username == "alice" def test_no_match_raises_stop_iteration(self): @@ -999,16 +994,37 @@ def test_no_match_raises_stop_iteration(self): StopIteration, match="No object with name=nonexistent found in fixture 'roles'", ): - get_obj_by_attr(self.roles, "name", "nonexistent") + self.registry.obj("roles", "name", "nonexistent") def test_no_match_on_wrong_value_type(self): """Raises StopIteration when value type doesn't match.""" with pytest.raises(StopIteration): - get_obj_by_attr(self.roles, "id", "not-a-uuid") + self.registry.obj("roles", "id", "not-a-uuid") + + def test_unknown_fixture_raises_key_error(self): + """Raises KeyError when the fixture name isn't registered.""" + with pytest.raises(KeyError): + self.registry.obj("unknown", "id", self.role_id_1) + + def test_searches_across_context_variants(self): + """obj() finds matches across all context variants of a fixture name, not just one.""" + registry = FixtureRegistry() + tester_id = uuid.uuid4() + + @registry.register(contexts=[Context.BASE]) + def variant_users() -> list[User]: + return [User(id=uuid.uuid4(), username="admin", email="admin@x.com")] + + @registry.register(contexts=[Context.TESTING]) + def variant_users() -> list[User]: # noqa: F811 + return [User(id=tester_id, username="tester", email="tester@x.com")] + user = cast(User, registry.obj("variant_users", "username", "tester")) + assert user.id == tester_id -class TestGetFieldByAttr: - """Tests for get_field_by_attr helper function.""" + +class TestRegistryField: + """Tests for FixtureRegistry.field.""" def setup_method(self): self.registry = FixtureRegistry() @@ -1024,22 +1040,20 @@ def roles() -> list[Role]: Role(id=role_id_2, name="user"), ] - self.roles = roles - def test_returns_id_by_default(self): """Returns the id field when no field is specified.""" - result = get_field_by_attr(self.roles, "name", "admin") + result = self.registry.field("roles", "name", "admin") assert result == self.role_id_1 def test_returns_specified_field(self): """Returns the requested field instead of id.""" - result = get_field_by_attr(self.roles, "id", self.role_id_2, field="name") + result = self.registry.field("roles", "id", self.role_id_2, field="name") assert result == "user" def test_no_match_raises_stop_iteration(self): - """Propagates StopIteration from get_obj_by_attr when no match found.""" + """Propagates StopIteration from obj() when no match found.""" with pytest.raises(StopIteration, match="No object with name=missing"): - get_field_by_attr(self.roles, "name", "missing") + self.registry.field("roles", "name", "missing") class TestGetPrimaryKey: From aa8fb9752335ba5499cb6f4c0393375dc48cb084 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Wed, 1 Jul 2026 12:31:16 -0400 Subject: [PATCH 3/4] refactor: log fixture command output instead of print --- src/fastapi_toolsets/cli/commands/fixtures.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/fastapi_toolsets/cli/commands/fixtures.py b/src/fastapi_toolsets/cli/commands/fixtures.py index 02b5a0a6..4dbeabda 100644 --- a/src/fastapi_toolsets/cli/commands/fixtures.py +++ b/src/fastapi_toolsets/cli/commands/fixtures.py @@ -7,6 +7,7 @@ from rich.table import Table from ...fixtures import Context, LoadStrategy, load_fixtures_by_context +from ...logger import get_logger from ..config import get_db_context, get_fixtures_registry from ..utils import async_command @@ -16,6 +17,7 @@ no_args_is_help=True, ) console = Console() +logger = get_logger() @fixture_cli.command("list") @@ -35,7 +37,7 @@ def list_fixtures( fixtures = registry.get_by_context(context.value) if context else registry.get_all() if not fixtures: - print("No fixtures found.") + logger.info("No fixtures found.") return table = Table("Name", "Contexts", "Dependencies") @@ -46,7 +48,7 @@ def list_fixtures( table.add_row(fixture.name, contexts, deps) console.print(table) - print(f"\nTotal: {len(fixtures)} fixture(s)") + logger.info("Total: %d fixture(s)", len(fixtures)) @fixture_cli.command("load") @@ -77,18 +79,18 @@ async def load( ordered = registry.resolve_context_dependencies(*context_list) if not ordered: - print("No fixtures to load for the specified context(s).") + logger.info("No fixtures to load for the specified context(s).") return - print(f"\nFixtures to load ({strategy.value} strategy):") + logger.info("Fixtures to load (%s strategy):", strategy.value) for name in ordered: fixture = registry.get(name) instances = list(fixture.func()) model_name = type(instances[0]).__name__ if instances else "?" - print(f" - {name}: {len(instances)} {model_name}(s)") + logger.info(" - %s: %d %s(s)", name, len(instances), model_name) if dry_run: - print("\n[Dry run - no changes made]") + logger.info("Dry run - no changes made") return async with db_context() as session: @@ -97,4 +99,4 @@ async def load( ) total = sum(len(items) for items in result.values()) - print(f"\nLoaded {total} record(s) successfully.") + logger.info("Loaded %d record(s) successfully.", total) From 156f7e73fd8b7e69dd99fa1238bef19e6c5e0ec2 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Wed, 1 Jul 2026 13:13:06 -0400 Subject: [PATCH 4/4] chore: clean up fixture module --- src/fastapi_toolsets/cli/commands/fixtures.py | 19 ++--- src/fastapi_toolsets/cli/config.py | 53 +++++++----- src/fastapi_toolsets/fixtures/__init__.py | 1 - src/fastapi_toolsets/fixtures/registry.py | 61 +++++++------ src/fastapi_toolsets/fixtures/utils.py | 85 ++++++++++--------- src/fastapi_toolsets/pytest/plugin.py | 2 +- 6 files changed, 119 insertions(+), 102 deletions(-) diff --git a/src/fastapi_toolsets/cli/commands/fixtures.py b/src/fastapi_toolsets/cli/commands/fixtures.py index 4dbeabda..c3411796 100644 --- a/src/fastapi_toolsets/cli/commands/fixtures.py +++ b/src/fastapi_toolsets/cli/commands/fixtures.py @@ -34,7 +34,7 @@ def list_fixtures( ) -> None: """List all registered fixtures.""" registry = get_fixtures_registry() - fixtures = registry.get_by_context(context.value) if context else registry.get_all() + fixtures = registry.get_by_context(context) if context else registry.get_all() if not fixtures: logger.info("No fixtures found.") @@ -74,7 +74,7 @@ async def load( registry = get_fixtures_registry() db_context = get_db_context() - context_list = list(contexts) if contexts else [Context.BASE] + context_list = contexts or [Context.BASE] ordered = registry.resolve_context_dependencies(*context_list) @@ -82,15 +82,14 @@ async def load( logger.info("No fixtures to load for the specified context(s).") return - logger.info("Fixtures to load (%s strategy):", strategy.value) - for name in ordered: - fixture = registry.get(name) - instances = list(fixture.func()) - model_name = type(instances[0]).__name__ if instances else "?" - logger.info(" - %s: %d %s(s)", name, len(instances), model_name) - if dry_run: - logger.info("Dry run - no changes made") + logger.info("Fixtures to load (%s strategy):", strategy.value) + for name in ordered: + variants = registry.get_load_variants(name, *context_list) + instances = [inst for v in variants for inst in v.func()] + model_name = type(instances[0]).__name__ if instances else "?" + logger.info(" - %s: %d %s(s)", name, len(instances), model_name) + logger.info("[Dry run - no changes made]") return async with db_context() as session: diff --git a/src/fastapi_toolsets/cli/config.py b/src/fastapi_toolsets/cli/config.py index 13d74779..70a265c9 100644 --- a/src/fastapi_toolsets/cli/config.py +++ b/src/fastapi_toolsets/cli/config.py @@ -4,7 +4,7 @@ import importlib import sys -from typing import TYPE_CHECKING, Any, Literal, overload +from typing import TYPE_CHECKING, Any, Literal, TypeVar, overload import typer @@ -13,6 +13,8 @@ if TYPE_CHECKING: from ..fixtures import FixtureRegistry +T = TypeVar("T") + def _ensure_project_in_path(): """Add project root to sys.path if not installed in editable mode.""" @@ -88,19 +90,39 @@ def get_config_value(key: str, required: bool = False) -> Any | None: return value -def get_fixtures_registry() -> FixtureRegistry: - """Import and return the fixtures registry from config.""" - from ..fixtures import FixtureRegistry +@overload +def _import_typed( + key: str, expected_type: type[T], *, required: Literal[True] +) -> T: ... # pragma: no cover +@overload +def _import_typed( + key: str, expected_type: type[T], *, required: bool +) -> T | None: ... # pragma: no cover +def _import_typed(key: str, expected_type: type[T], *, required: bool) -> T | None: + """Import a config value by key and validate its type. - import_path = get_config_value("fixtures", required=True) - registry = import_from_string(import_path) + Raises: + typer.BadParameter: If required and missing, or if the imported + value isn't an instance of *expected_type*. + """ + import_path = get_config_value(key, required=required) + if not import_path: + return None - if not isinstance(registry, FixtureRegistry): + obj = import_from_string(import_path) + if not isinstance(obj, expected_type): raise typer.BadParameter( - f"'fixtures' must be a FixtureRegistry instance, got {type(registry).__name__}" + f"'{key}' must be a {expected_type.__name__} instance, got {type(obj).__name__}" ) - return registry + return obj + + +def get_fixtures_registry() -> FixtureRegistry: + """Import and return the fixtures registry from config.""" + from ..fixtures import FixtureRegistry + + return _import_typed("fixtures", FixtureRegistry, required=True) def get_db_context() -> Any: @@ -111,15 +133,4 @@ def get_db_context() -> Any: def get_custom_cli() -> typer.Typer | None: """Import and return the custom CLI Typer instance from config.""" - import_path = get_config_value("custom_cli") - if not import_path: - return None - - custom = import_from_string(import_path) - - if not isinstance(custom, typer.Typer): - raise typer.BadParameter( - f"'custom_cli' must be a Typer instance, got {type(custom).__name__}" - ) - - return custom + return _import_typed("custom_cli", typer.Typer, required=False) diff --git a/src/fastapi_toolsets/fixtures/__init__.py b/src/fastapi_toolsets/fixtures/__init__.py index faa21af2..df5302e0 100644 --- a/src/fastapi_toolsets/fixtures/__init__.py +++ b/src/fastapi_toolsets/fixtures/__init__.py @@ -10,5 +10,4 @@ "LoadStrategy", "load_fixtures", "load_fixtures_by_context", - "register_fixtures", ] diff --git a/src/fastapi_toolsets/fixtures/registry.py b/src/fastapi_toolsets/fixtures/registry.py index f42a9663..c94a3b61 100644 --- a/src/fastapi_toolsets/fixtures/registry.py +++ b/src/fastapi_toolsets/fixtures/registry.py @@ -7,11 +7,8 @@ from sqlalchemy.orm import DeclarativeBase -from ..logger import get_logger from .enum import Context -logger = get_logger() - def _normalize_contexts( contexts: list[str | Enum] | tuple[str | Enum, ...], @@ -189,9 +186,7 @@ def get(self, name: str) -> Fixture: ValueError: If the fixture has multiple context variants — use :meth:`get_variants` in that case. """ - if name not in self._fixtures: - raise KeyError(f"Fixture '{name}' not found") - variants = self._fixtures[name] + variants = self.get_variants(name) if len(variants) > 1: raise ValueError( f"Fixture '{name}' has {len(variants)} context variants. " @@ -223,10 +218,40 @@ def get_variants(self, name: str, *contexts: str | Enum) -> list[Fixture]: context_values = set(_normalize_contexts(contexts)) return [v for v in variants if set(v.contexts) & context_values] + def get_load_variants(self, name: str, *contexts: str | Enum) -> list[Fixture]: + """Return variants for *name* filtered by *contexts*. + + Raises: + KeyError: If no fixture with *name* is registered. + """ + variants = self.get_variants(name, *contexts) + if contexts and not variants: + return self.get_variants(name) + return variants + def get_all(self) -> list[Fixture]: """Get all registered fixtures (all variants of all names).""" return [f for variants in self._fixtures.values() for f in variants] + def get_dependencies(self, name: str) -> list[str]: + """Get the union of ``depends_on`` across all variants of *name*. + + Raises: + KeyError: If no fixture named *name* is registered. + """ + variants = self._fixtures.get(name) + if variants is None: + raise KeyError(f"Fixture '{name}' not found") + + seen: set[str] = set() + deps: list[str] = [] + for variant in variants: + for dep in variant.depends_on: + if dep not in seen: + deps.append(dep) + seen.add(dep) + return deps + def obj(self, name: str, attr_name: str, value: Any) -> DeclarativeBase: """Get a model instance from a registered fixture by attribute value. @@ -297,7 +322,6 @@ def resolve_dependencies(self, *names: str) -> list[str]: ValueError: If circular dependency detected """ resolved: list[str] = [] - seen: set[str] = set() visiting: set[str] = set() def visit(name: str) -> None: @@ -307,25 +331,11 @@ def visit(name: str) -> None: raise ValueError(f"Circular dependency detected: {name}") visiting.add(name) - variants = self._fixtures.get(name) - if variants is None: - raise KeyError(f"Fixture '{name}' not found") - - # Union of depends_on across all variants, preserving first-seen order. - seen_deps: set[str] = set() - all_deps: list[str] = [] - for variant in variants: - for dep in variant.depends_on: - if dep not in seen_deps: - all_deps.append(dep) - seen_deps.add(dep) - - for dep in all_deps: + for dep in self.get_dependencies(name): visit(dep) visiting.remove(name) resolved.append(name) - seen.add(name) for name in names: visit(name) @@ -346,9 +356,4 @@ def resolve_context_dependencies(self, *contexts: str | Enum) -> list[str]: # appear multiple times if it has variants in different contexts). names = list(dict.fromkeys(f.name for f in context_fixtures)) - all_deps: set[str] = set() - for name in names: - deps = self.resolve_dependencies(name) - all_deps.update(deps) - - return self.resolve_dependencies(*all_deps) + return self.resolve_dependencies(*names) diff --git a/src/fastapi_toolsets/fixtures/utils.py b/src/fastapi_toolsets/fixtures/utils.py index a76d0ef9..a034d228 100644 --- a/src/fastapi_toolsets/fixtures/utils.py +++ b/src/fastapi_toolsets/fixtures/utils.py @@ -1,5 +1,6 @@ """Fixture loading utilities for database seeding.""" +from collections.abc import Iterator from enum import Enum from typing import Any, cast @@ -93,33 +94,42 @@ def _group_by_column_set( return list(groups.values()) +def _grouped_table_dicts( + model_cls: type[DeclarativeBase], instances: list[DeclarativeBase] +) -> Iterator[ + tuple[type[DeclarativeBase], list[dict[str, Any]], list[DeclarativeBase]] +]: + """Yield (cls, group_dicts, group_instances) per table in the inheritance + chain and per column-set group, skipping empty groups. + """ + for cls in _get_table_chain(model_cls): + dicts = [_instance_to_dict_for_cls(i, cls) for i in instances] + for group_dicts, group_instances in _group_by_column_set(dicts, instances): + if group_dicts and group_dicts[0]: # pragma: no branch + yield cls, group_dicts, group_instances + + async def _batch_insert( session: AsyncSession, model_cls: type[DeclarativeBase], instances: list[DeclarativeBase], ) -> None: """INSERT all instances, raises on conflict.""" - for cls in _get_table_chain(model_cls): + for cls, group_dicts, group_instances in _grouped_table_dicts(model_cls, instances): table = cast(Table, cls.__table__) - dicts = [_instance_to_dict_for_cls(i, cls) for i in instances] - for group_dicts, group_instances in _group_by_column_set(dicts, instances): - if not group_dicts or not group_dicts[0]: # pragma: no cover - continue - missing_pk_cols = [ - col - for col in table.primary_key.columns - if col.key not in group_dicts[0] - ] - if not missing_pk_cols: - await session.execute(pg_insert(table), group_dicts) - continue - stmt = pg_insert(table).returning( - *missing_pk_cols, sort_by_parameter_order=True - ) - result = await session.execute(stmt, group_dicts) - for inst, row in zip(group_instances, result): - for col, val in zip(missing_pk_cols, row): - setattr(inst, col.key, val) + missing_pk_cols = [ + col for col in table.primary_key.columns if col.key not in group_dicts[0] + ] + if not missing_pk_cols: + await session.execute(pg_insert(table), group_dicts) + continue + stmt = pg_insert(table).returning( + *missing_pk_cols, sort_by_parameter_order=True + ) + result = await session.execute(stmt, group_dicts) + for inst, row in zip(group_instances, result): + for col, val in zip(missing_pk_cols, row): + setattr(inst, col.key, val) async def _batch_merge( @@ -128,30 +138,26 @@ async def _batch_merge( instances: list[DeclarativeBase], ) -> None: """UPSERT: insert new rows, update existing ones with the provided values.""" - for cls in _get_table_chain(model_cls): + for cls, group_dicts, _ in _grouped_table_dicts(model_cls, instances): pk_names = [col.name for col in cls.__table__.primary_key] pk_names_set = set(pk_names) own_col_keys = {col.key for col in cls.__table__.columns} non_pk_cols = [k for k in own_col_keys if k not in pk_names_set] - dicts = [_instance_to_dict_for_cls(i, cls) for i in instances] - for group_dicts, _ in _group_by_column_set(dicts, instances): - if not group_dicts or not group_dicts[0]: # pragma: no cover - continue - stmt = pg_insert(cls).values(group_dicts) + stmt = pg_insert(cls).values(group_dicts) - inserted_keys = set(group_dicts[0]) - update_cols = [col for col in non_pk_cols if col in inserted_keys] + inserted_keys = set(group_dicts[0]) + update_cols = [col for col in non_pk_cols if col in inserted_keys] - if update_cols: - stmt = stmt.on_conflict_do_update( - index_elements=pk_names, - set_={col: stmt.excluded[col] for col in update_cols}, - ) - else: - stmt = stmt.on_conflict_do_nothing(index_elements=pk_names) + if update_cols: + stmt = stmt.on_conflict_do_update( + index_elements=pk_names, + set_={col: stmt.excluded[col] for col in update_cols}, + ) + else: + stmt = stmt.on_conflict_do_nothing(index_elements=pk_names) - await session.execute(stmt) + await session.execute(stmt) async def _batch_skip_existing( @@ -288,14 +294,11 @@ async def _load_ordered( for name in ordered_names: variants = ( - registry.get_variants(name, *contexts) + registry.get_load_variants(name, *contexts) if contexts is not None else registry.get_variants(name) ) - if contexts is not None and not variants: - variants = registry.get_variants(name) - if not variants: # pragma: no cover results[name] = [] continue @@ -327,7 +330,7 @@ async def _load_ordered( loaded = await _refresh_loaded(session, loaded) results[name] = loaded - logger.info(f"Loaded fixture '{name}': {len(loaded)} {model_name}(s)") + logger.info("Loaded fixture '%s': %d %s(s)", name, len(loaded), model_name) return results diff --git a/src/fastapi_toolsets/pytest/plugin.py b/src/fastapi_toolsets/pytest/plugin.py index 56f05562..3ae9b767 100644 --- a/src/fastapi_toolsets/pytest/plugin.py +++ b/src/fastapi_toolsets/pytest/plugin.py @@ -55,7 +55,7 @@ def register_fixtures( # Build list of pytest fixture dependencies pytest_deps = [session_fixture] - for dep in fixture.depends_on: + for dep in registry.get_dependencies(fixture.name): pytest_deps.append(f"{prefix}{dep}") # Create the fixture function