From 0845f338e6d1b22689ca6aa99ff05b6849529223 Mon Sep 17 00:00:00 2001 From: Robert Rosca <32569096+RobertRosca@users.noreply.github.com> Date: Wed, 8 Jul 2026 08:58:08 +0200 Subject: [PATCH 1/6] chore(api/deps): add advanced-alchemy dependency --- api/pyproject.toml | 1 + uv.lock | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/api/pyproject.toml b/api/pyproject.toml index d3848627..78aa0a6b 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -38,6 +38,7 @@ dependencies = [ "sqlmodel>=0.0.31", "pyyaml>=6.0.3", "litestar~=2.24.0", + "advanced-alchemy>=1.11.0", ] [dependency-groups] diff --git a/uv.lock b/uv.lock index 2476189f..dba008a3 100644 --- a/uv.lock +++ b/uv.lock @@ -8,6 +8,21 @@ members = [ "damnit-web", ] +[[package]] +name = "advanced-alchemy" +version = "1.11.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "alembic" }, + { name = "greenlet" }, + { name = "sqlalchemy" }, + { name = "typing-extensions" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/54/6d/1e3299a213281454d5b7e4e9242fda255a5d82947b7befdeaf361f398eb2/advanced_alchemy-1.11.0.tar.gz", hash = "sha256:3e42d854b9be6ee37867c490c74d0fdb3894b13e009f5b515e8c24d113011e08", size = 1260879, upload-time = "2026-05-31T18:14:26.664Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/db/1d/fdeb83451261ff933fbea56cae9769c8a3d4908a8d6c4983f1346817fcab/advanced_alchemy-1.11.0-py3-none-any.whl", hash = "sha256:5e9d92e3df5c073565c06f0a401f4b3c24ed1a450834b8dd467239136cd86b5c", size = 342338, upload-time = "2026-05-31T18:14:24.718Z" }, +] + [[package]] name = "aiofiles" version = "25.1.0" @@ -81,6 +96,20 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/00/b7/e3bf5133d697a08128598c8d0abc5e16377b51465a33756de24fa7dee953/aiosqlite-0.22.1-py3-none-any.whl", hash = "sha256:21c002eb13823fad740196c5a2e9d8e62f6243bd9e7e4a1f87fb5e44ecb4fceb", size = 17405, upload-time = "2025-12-23T19:25:42.139Z" }, ] +[[package]] +name = "alembic" +version = "1.18.5" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "mako" }, + { name = "sqlalchemy" }, + { name = "typing-extensions" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/1a/cc/ac0bed8e562e7407fe55c3ba85a4dce86e6dbd8730887bd1e406a6c5c18a/alembic-1.18.5.tar.gz", hash = "sha256:1554982221dd17e9a749b53902407578eb305e453f71999e8c7f0a48389fff8e", size = 2060480, upload-time = "2026-06-25T15:20:54.888Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/96/78/5fe6dc3a3a5b2f5a2a4faef8bfe336d5fa049a38884ab3172e0098160c01/alembic-1.18.5-py3-none-any.whl", hash = "sha256:06d8ba9d04558022f5395e9317de03d270f3dced49cee01f89fe7a13c26f14bc", size = 264664, upload-time = "2026-06-25T15:20:56.673Z" }, +] + [[package]] name = "annotated-types" version = "0.7.0" @@ -378,6 +407,7 @@ name = "damnit-api" version = "0.1.1" source = { editable = "api" } dependencies = [ + { name = "advanced-alchemy" }, { name = "aiofiles" }, { name = "aiohttp" }, { name = "aiosqlite" }, @@ -454,6 +484,7 @@ test = [ [package.metadata] requires-dist = [ + { name = "advanced-alchemy", specifier = ">=1.11.0" }, { name = "aiofiles", specifier = ">=24.1.0" }, { name = "aiohttp", specifier = "~=3.11" }, { name = "aiosqlite", specifier = "~=0.19" }, @@ -947,6 +978,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f2/24/8d99982f0aa9c1cd82073c6232b54a0dbe6797c7d63c0583a6c68ee3ddf2/litestar_htmx-0.5.0-py3-none-any.whl", hash = "sha256:92833aa47e0d0e868d2a7dbfab75261f124f4b83d4f9ad12b57b9a68f86c50e6", size = 9970, upload-time = "2025-06-11T21:19:44.465Z" }, ] +[[package]] +name = "mako" +version = "1.3.12" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "markupsafe" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/00/62/791b31e69ae182791ec67f04850f2f062716bbd205483d63a215f3e062d3/mako-1.3.12.tar.gz", hash = "sha256:9f778e93289bd410bb35daadeb4fc66d95a746f0b75777b942088b7fd7af550a", size = 400219, upload-time = "2026-04-28T19:01:08.512Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/bc/b1/a0ec7a5a9db730a08daef1fdfb8090435b82465abbf758a596f0ea88727e/mako-1.3.12-py3-none-any.whl", hash = "sha256:8f61569480282dbf557145ce441e4ba888be453c30989f879f0d652e39f53ea9", size = 78521, upload-time = "2026-04-28T19:01:10.393Z" }, +] + [[package]] name = "markdown" version = "3.10.2" From f30527bf1133292f3746d1bc527b2a5cc4b1d6be Mon Sep 17 00:00:00 2001 From: Robert Rosca <32569096+RobertRosca@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:03:49 +0200 Subject: [PATCH 2/6] feat(api/appdb): adopt Advanced Alchemy SQLAlchemyPlugin --- api/src/damnit_api/_db/dependencies.py | 18 +++----------- api/src/damnit_api/auth/dependencies.py | 2 +- api/src/damnit_api/main.py | 33 +++++++++++++++++-------- api/src/damnit_api/metadata/routers.py | 2 +- api/src/damnit_api/state.py | 22 +++++++---------- 5 files changed, 38 insertions(+), 39 deletions(-) diff --git a/api/src/damnit_api/_db/dependencies.py b/api/src/damnit_api/_db/dependencies.py index 0a7f7873..8bf92b06 100644 --- a/api/src/damnit_api/_db/dependencies.py +++ b/api/src/damnit_api/_db/dependencies.py @@ -1,17 +1,7 @@ -"""Litestar dependency helpers for database sessions.""" +"""Database session type alias for Litestar handlers.""" -from collections.abc import AsyncIterator +from sqlalchemy.ext.asyncio import AsyncSession -from litestar.datastructures import State -from sqlmodel.ext.asyncio.session import AsyncSession - - -async def get_session(state: State) -> AsyncIterator[AsyncSession]: - """Provide a database session from the application state.""" - - async with state.app_state.db_sessionmaker() as session: # type: ignore[attr-defined] - yield session - - -# Plain type alias; Litestar injects by the parameter name `session`. +# Type alias used for annotations in other modules; the session itself is +# provided by the Advanced Alchemy plugin (session_dependency_key="session"). DBSession = AsyncSession diff --git a/api/src/damnit_api/auth/dependencies.py b/api/src/damnit_api/auth/dependencies.py index e3ff02e5..5306179b 100644 --- a/api/src/damnit_api/auth/dependencies.py +++ b/api/src/damnit_api/auth/dependencies.py @@ -6,7 +6,7 @@ from authlib.integrations.httpx_client import AsyncOAuth2Client from litestar import Request from litestar.datastructures import State -from sqlmodel.ext.asyncio.session import AsyncSession +from sqlalchemy.ext.asyncio import AsyncSession from .._mymdc.dependencies import MyMdCClient from .models import OAuthUserInfo as _OAuthUserInfo diff --git a/api/src/damnit_api/main.py b/api/src/damnit_api/main.py index 0025e86f..d7b83554 100644 --- a/api/src/damnit_api/main.py +++ b/api/src/damnit_api/main.py @@ -5,7 +5,6 @@ from litestar.exceptions import HTTPException from . import contextfile, metadata -from ._db.dependencies import get_session from ._mymdc.dependencies import get_mymdc_client from .auth.dependencies import get_oauth_user_info, get_user @@ -14,6 +13,11 @@ def create_app(): + from advanced_alchemy.extensions.litestar import ( + AsyncSessionConfig, + SQLAlchemyAsyncConfig, + SQLAlchemyPlugin, + ) from litestar import Request, Response from litestar.channels import ChannelsPlugin from litestar.channels.backends.memory import MemoryChannelsBackend @@ -23,6 +27,7 @@ def create_app(): from litestar.stores.file import FileStore from litestar.stores.memory import MemoryStore from litestar.stores.registry import StoreRegistry + from sqlmodel import SQLModel from . import _logging, auth, get_logger from .auth.oauth import SESSION_COOKIE_KEY, create_oauth_client @@ -34,8 +39,6 @@ def create_app(): from .shared.settings import settings from .state import ( AppState, - create_db_engine, - create_db_sessionmaker, create_mymdc_client, create_repositories, provide_app_state, @@ -50,6 +53,20 @@ def create_app(): # (in-memory locally, file-backed otherwise) and selected below. session_config = ServerSideSessionConfig(key=SESSION_COOKIE_KEY) + # ── App database (ADR-010: `appdb`, dw_api.sqlite) ─────────────────────── + # Advanced Alchemy owns engine/session lifecycle and provides the + # per-request `session` dependency (commit-on-success). Distinct + # dependency/state keys so a second config (e.g. a future DAMNIT + # Postgres) can coexist. + alchemy_config = SQLAlchemyAsyncConfig( + connection_string=f"sqlite+aiosqlite:///{settings.db_path}", + session_config=AsyncSessionConfig(expire_on_commit=False), + session_dependency_key="session", + engine_dependency_key="appdb_engine", + metadata=SQLModel.metadata, + create_all=True, + ) + # ── Channels (run-update pub/sub) ───────────────────────────────────────── # Subscribers consume per-proposal channels; the composition-selected # publisher (built in the lifespan) produces the events. The in-memory @@ -114,8 +131,6 @@ async def lifespan(app: Litestar): logger.info("Starting application lifespan") - engine = create_db_engine(settings) - oauth_client = create_oauth_client(settings) if oauth_client is not None: await oauth_client.load_server_metadata() @@ -127,8 +142,7 @@ async def lifespan(app: Litestar): ) app.state.app_state = AppState( - db_engine=engine, - db_sessionmaker=create_db_sessionmaker(engine), + db_sessionmaker=alchemy_config.create_session_maker(), mymdc_client=create_mymdc_client(settings), oauth_client=oauth_client, repositories=repositories, @@ -140,7 +154,6 @@ async def lifespan(app: Litestar): yield finally: await run_update_publisher.aclose() - await engine.dispose() def _file_store(name: str) -> FileStore: # FileStore does not create its directory on write; the session read @@ -176,7 +189,7 @@ def _file_store(name: str) -> FileStore: "oauth_config": Provide( auth.dependencies.get_oauth_client, sync_to_thread=False ), - "session": Provide(get_session), + # The `session` dependency comes from the Advanced Alchemy plugin. "mymdc": Provide(get_mymdc_client, sync_to_thread=False), "user": Provide(get_user), "oauth_user": Provide(get_oauth_user_info, sync_to_thread=False), @@ -186,7 +199,7 @@ def _file_store(name: str) -> FileStore: ), "repositories": Provide(get_repositories, sync_to_thread=False), }, - plugins=[channels_plugin], + plugins=[channels_plugin, SQLAlchemyPlugin(config=alchemy_config)], stores=stores, middleware=[ session_config.middleware, diff --git a/api/src/damnit_api/metadata/routers.py b/api/src/damnit_api/metadata/routers.py index 3f006b50..28891ebd 100644 --- a/api/src/damnit_api/metadata/routers.py +++ b/api/src/damnit_api/metadata/routers.py @@ -2,7 +2,7 @@ from litestar import Router, get from litestar.di import Provide -from sqlmodel.ext.asyncio.session import AsyncSession +from sqlalchemy.ext.asyncio import AsyncSession from .._mymdc.dependencies import MyMdCClient from ..auth.models import User diff --git a/api/src/damnit_api/state.py b/api/src/damnit_api/state.py index 38112cfe..0ea331f9 100644 --- a/api/src/damnit_api/state.py +++ b/api/src/damnit_api/state.py @@ -14,11 +14,12 @@ from litestar.datastructures import ( State as LitestarState, # noqa: TC002 - Litestar inspects annotations at runtime via get_type_hints ) -from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine -from sqlmodel.ext.asyncio.session import AsyncSession if TYPE_CHECKING: + from collections.abc import Callable + from litestar.channels import ChannelsPlugin + from sqlalchemy.ext.asyncio import AsyncSession from ._mymdc.clients import MyMdCClient from .auth.oauth import OAuthClient @@ -29,8 +30,12 @@ @dataclass(frozen=True) class AppState: - db_engine: AsyncEngine - db_sessionmaker: async_sessionmaker[AsyncSession] + # Session factory from the Advanced Alchemy config (main.py); held here + # for non-request contexts (e.g. the proposal-membership guard). + # Advanced Alchemy's create_session_maker() is typed as this Callable, + # not as async_sessionmaker[AsyncSession] (its actual runtime type in + # the non-routing case) - match its declared type here. + db_sessionmaker: Callable[[], AsyncSession] mymdc_client: MyMdCClient oauth_client: OAuthClient | None # None when auth is disabled repositories: DamnitRepositoryRegistry @@ -38,15 +43,6 @@ class AppState: run_update_publisher: RunUpdatePublisher -def create_db_engine(settings: Settings) -> AsyncEngine: - db_url = f"sqlite+aiosqlite:///{settings.db_path}" - return create_async_engine(db_url, echo=False, future=True) - - -def create_db_sessionmaker(engine: AsyncEngine) -> async_sessionmaker[AsyncSession]: - return async_sessionmaker(bind=engine, class_=AsyncSession, expire_on_commit=False) - - def create_mymdc_client(settings: Settings) -> MyMdCClient: from ._mymdc import clients from ._mymdc.settings import MyMdCHTTPSettings, MyMdCMockSettings From d3a4e1a8531b6d4fe9c4178d7d2557136a618c52 Mon Sep 17 00:00:00 2001 From: Robert Rosca <32569096+RobertRosca@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:06:58 +0200 Subject: [PATCH 3/6] refactor(api/metadata): advanced alchemy repository for proposals --- api/src/damnit_api/metadata/repository.py | 22 ++++++++++ api/src/damnit_api/metadata/services.py | 49 ++++++++--------------- 2 files changed, 38 insertions(+), 33 deletions(-) create mode 100644 api/src/damnit_api/metadata/repository.py diff --git a/api/src/damnit_api/metadata/repository.py b/api/src/damnit_api/metadata/repository.py new file mode 100644 index 00000000..0e1ec209 --- /dev/null +++ b/api/src/damnit_api/metadata/repository.py @@ -0,0 +1,22 @@ +"""App-DB repository for cached proposal metadata (Advanced Alchemy). + +This is the app database's data access (ADR-010: `dw_api.sqlite`, writable) - +distinct from the read-only `DamnitRepository` over per-proposal DAMNIT files. +""" + +from advanced_alchemy.repository import SQLAlchemyAsyncRepository + +from .models import ProposalMeta + + +class ProposalMetaRepository( + SQLAlchemyAsyncRepository[ProposalMeta] # ty: ignore[invalid-type-arguments] +): + """CRUD/upsert access to the proposal-metadata cache. + + SQLModel `table=True` models satisfy Advanced Alchemy's `ModelProtocol` at + runtime (they have `__table__`/`__mapper__`), but ty cannot verify that + structurally. + """ + + model_type = ProposalMeta diff --git a/api/src/damnit_api/metadata/services.py b/api/src/damnit_api/metadata/services.py index d76b45d2..acbc017d 100644 --- a/api/src/damnit_api/metadata/services.py +++ b/api/src/damnit_api/metadata/services.py @@ -5,13 +5,14 @@ from pathlib import Path from typing import TYPE_CHECKING +from advanced_alchemy.filters import CollectionFilter from anyio import Path as APath -from sqlmodel import col, select from .. import get_logger from ..shared.errors import ForbiddenError from ..shared.models import ProposalNumber from .models import ProposalMeta, ProposalMetaBase +from .repository import ProposalMetaRepository logger = get_logger() @@ -192,10 +193,9 @@ async def _get_proposal_meta( ) -> ProposalMeta: """Get proposal metadata by proposal number, using the repository and/or provided MyMdC Client.""" + repo = ProposalMetaRepository(session=session, auto_commit=True) - statement = select(ProposalMeta).where(ProposalMeta.number == proposal_number) - result = (await session.exec(statement)).one_or_none() - + result = await repo.get_one_or_none(number=proposal_number) if result: await logger.ainfo( "Loaded proposal metadata from repository", proposal=proposal_number @@ -203,10 +203,7 @@ async def _get_proposal_meta( return result fetched = await _fetch_proposal_meta(client, proposal_number) - result = ProposalMeta(**fetched.model_dump()) - session.add(result) - await session.commit() - return result + return await repo.add(ProposalMeta(**fetched.model_dump())) def _chunks(list_, n=10): @@ -221,9 +218,7 @@ async def _get_proposal_meta_many( only_with_damnit: bool = True, start_after: datetime | None = None, ) -> list[ProposalMeta]: - statement = select(ProposalMeta).where( - col(ProposalMeta.number).in_(proposal_numbers) - ) + repo = ProposalMetaRepository(session=session, auto_commit=True) filters = [] if only_with_damnit: @@ -232,7 +227,11 @@ async def _get_proposal_meta_many( if start_after: filters.append(lambda p: p.start_date and p.start_date >= start_after) - results = list((await session.exec(statement)).all()) + results = list( + await repo.get_many( + CollectionFilter(field_name="number", values=proposal_numbers) + ) + ) missing = set(proposal_numbers) - {p.number for p in results} if not missing: @@ -248,9 +247,7 @@ async def _get_proposal_meta_many( for p in new_fetched if not isinstance(p, BaseException) ] - session.add_all(new) - await session.commit() - results.extend(new) + results.extend(await repo.add_many(new)) return [p for p in results if all(f(p) for f in filters)] @@ -280,24 +277,10 @@ async def _update_proposal_meta( ) -> ProposalMeta: fetched = await _fetch_proposal_meta(client, proposal_number) - # Upsert into DB - statement = select(ProposalMeta).where(ProposalMeta.number == proposal_number) - result = (await session.exec(statement)).one_or_none() - - if not result: - new = ProposalMeta(**fetched.model_dump()) - session.add(new) - await session.commit() - return new - - for key, value in fetched.model_dump().items(): - if getattr(result, key) != value: - setattr(result, key, value) - - await session.commit() - await session.refresh(result) - - return result + repo = ProposalMetaRepository(session=session, auto_commit=True) + return await repo.upsert( + ProposalMeta(**fetched.model_dump()), match_fields=["number"] + ) async def update_proposal_meta( From 30be87f420ef75f247ae2ce8aa68274f196634cd Mon Sep 17 00:00:00 2001 From: Robert Rosca <32569096+RobertRosca@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:13:12 +0200 Subject: [PATCH 4/6] fix(api/runs): restore event loop after test_session's asyncio.run --- api/tests/runs/sqlite/test_session.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/api/tests/runs/sqlite/test_session.py b/api/tests/runs/sqlite/test_session.py index 6ecbad5a..26dcd473 100644 --- a/api/tests/runs/sqlite/test_session.py +++ b/api/tests/runs/sqlite/test_session.py @@ -110,5 +110,12 @@ async def do_read(): await manager.close() assert _open_file_descriptors_to(db_file) == [] - asyncio.run(do_read()) + # asyncio.run() clears the thread's "current" event loop on exit, which + # breaks pytest-asyncio's session-scoped loop for any async test that + # runs afterwards; restore it so later tests aren't affected. + current_loop = asyncio.get_event_loop() + try: + asyncio.run(do_read()) + finally: + asyncio.set_event_loop(current_loop) assert _open_file_descriptors_to(db_file) == [] From 3aa4654ae94cb69f2a7164d8d66a761832b1f48b Mon Sep 17 00:00:00 2001 From: Robert Rosca <32569096+RobertRosca@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:13:30 +0200 Subject: [PATCH 5/6] test(api/metadata): cover the app-DB repository against a real engine --- api/tests/test_metadata_services.py | 139 ++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 api/tests/test_metadata_services.py diff --git a/api/tests/test_metadata_services.py b/api/tests/test_metadata_services.py new file mode 100644 index 00000000..c1b15471 --- /dev/null +++ b/api/tests/test_metadata_services.py @@ -0,0 +1,139 @@ +"""Tests for damnit_api.metadata.services against a real app-DB session. + +Exercises the Advanced Alchemy repository-backed code paths (cache hit, +fetch-and-add on miss, upsert) with a real in-memory engine rather than a +mocked session. +""" + +from datetime import UTC, datetime +from unittest.mock import AsyncMock, MagicMock + +import pytest +import pytest_asyncio + +from damnit_api.metadata.models import ProposalMetaBase +from damnit_api.shared.models import ProposalNumber + + +@pytest_asyncio.fixture +async def appdb_session(): + """A real (in-memory) app-DB session, tables created from SQLModel metadata.""" + from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + from sqlmodel import SQLModel + + import damnit_api.metadata.models # noqa: F401 - registers the tables + + engine = create_async_engine("sqlite+aiosqlite://") + async with engine.begin() as conn: + await conn.run_sync(SQLModel.metadata.create_all) + maker = async_sessionmaker(engine, expire_on_commit=False) + async with maker() as session: + yield session + await engine.dispose() + + +def _fetched_meta(number: int, damnit_path: str | None = "/some/path"): + return ProposalMetaBase( + id=number, + number=ProposalNumber(number), + cycle="202401", + instrument="TST", + path=f"/gpfs/exfel/exp/TST/202401/p{number:06d}", + title=f"Proposal {number}", + principal_investigator="Test User", + start_date=datetime(2024, 1, 1, tzinfo=UTC), + end_date=None, + damnit_path=damnit_path, + ) + + +@pytest.mark.asyncio +async def test_get_proposal_meta_fetches_and_caches_on_miss(mocker, appdb_session): + """A cache miss fetches from MyMdC and adds the result via the repository.""" + from damnit_api.metadata.services import _get_proposal_meta + + proposal_number = ProposalNumber(100001) + mocker.patch( + "damnit_api.metadata.services._fetch_proposal_meta", + AsyncMock(return_value=_fetched_meta(int(proposal_number))), + ) + + result = await _get_proposal_meta(MagicMock(), proposal_number, appdb_session) + + assert result.number == proposal_number + assert result.id is not None + + +@pytest.mark.asyncio +async def test_get_proposal_meta_returns_cached_row_without_fetching( + mocker, appdb_session +): + """A cache hit returns the stored row and does not call MyMdC again.""" + from damnit_api.metadata.services import _get_proposal_meta + + proposal_number = ProposalNumber(100002) + fetch = mocker.patch( + "damnit_api.metadata.services._fetch_proposal_meta", + AsyncMock(return_value=_fetched_meta(int(proposal_number))), + ) + + await _get_proposal_meta(MagicMock(), proposal_number, appdb_session) + fetch.reset_mock() + + result = await _get_proposal_meta(MagicMock(), proposal_number, appdb_session) + + assert result.number == proposal_number + fetch.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_get_proposal_meta_many_fetches_missing_and_keeps_cached( + mocker, appdb_session +): + """Cached proposals are returned from the repository; missing ones are + fetched and persisted via `add_many`, then included in the result.""" + from damnit_api.metadata.services import _get_proposal_meta, _get_proposal_meta_many + + cached = ProposalNumber(100003) + missing = ProposalNumber(100004) + + mocker.patch( + "damnit_api.metadata.services._fetch_proposal_meta", + AsyncMock(side_effect=lambda client, no: _fetched_meta(int(no))), + ) + await _get_proposal_meta(MagicMock(), cached, appdb_session) + + results = await _get_proposal_meta_many( + MagicMock(), + [cached, missing], + appdb_session, + only_with_damnit=False, + ) + + result_numbers = {r.number for r in results} + assert result_numbers == {cached, missing} + + +@pytest.mark.asyncio +async def test_update_proposal_meta_upserts_existing_row(mocker, appdb_session): + """Refreshing an already-cached proposal updates its fields in place + (matched on `number`), rather than creating a duplicate row.""" + from damnit_api.metadata.services import _get_proposal_meta, _update_proposal_meta + + proposal_number = ProposalNumber(100005) + mocker.patch( + "damnit_api.metadata.services._fetch_proposal_meta", + AsyncMock(return_value=_fetched_meta(int(proposal_number))), + ) + original = await _get_proposal_meta(MagicMock(), proposal_number, appdb_session) + + mocker.patch( + "damnit_api.metadata.services._fetch_proposal_meta", + AsyncMock( + return_value=_fetched_meta(int(proposal_number), damnit_path="/new/path") + ), + ) + updated = await _update_proposal_meta(MagicMock(), proposal_number, appdb_session) + + assert updated.id == original.id + assert updated.damnit_path == "/new/path" From 59fdfbd09bc70915db9f6e2d56f45b6311aa5c95 Mon Sep 17 00:00:00 2001 From: Robert Rosca <32569096+RobertRosca@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:18:21 +0200 Subject: [PATCH 6/6] docs(api/adr): add ADR-010 two databases, sweep back-references --- .../adr/000-vertical-slice-architecture.md | 2 +- api/docs/adr/009-channels-subscriptions.md | 2 +- api/docs/adr/010-two-databases.md | 37 +++++++++++++++++++ api/docs/architecture.md | 2 +- 4 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 api/docs/adr/010-two-databases.md diff --git a/api/docs/adr/000-vertical-slice-architecture.md b/api/docs/adr/000-vertical-slice-architecture.md index e4bde4d6..6ceeb6be 100644 --- a/api/docs/adr/000-vertical-slice-architecture.md +++ b/api/docs/adr/000-vertical-slice-architecture.md @@ -86,7 +86,7 @@ damnit_api/ ├── auth/ # OIDC flow, sessions, tokens, users, authz policy ├── contextfile/ # context-file (REST) endpoints ├── mymdc/ # MyMdC port -├── appdb/ # application-DB engine/session/models +├── appdb/ # application-DB engine/session/models; see ADR-010 │ └── graphql/ # transport composition only (ADR-007): ├── schema.py # assemble Query/Subscription from feature gql modules diff --git a/api/docs/adr/009-channels-subscriptions.md b/api/docs/adr/009-channels-subscriptions.md index 9cf300d9..38fd0306 100644 --- a/api/docs/adr/009-channels-subscriptions.md +++ b/api/docs/adr/009-channels-subscriptions.md @@ -8,7 +8,7 @@ date: 2026-07-08 The frontend's live table updates use a GraphQL subscription (`latest_data`) over `graphql-transport-ws`. DAMNIT proposal databases are plain SQLite files written by an external process. There is no change feed to subscribe to today, so the server must produce one by polling. -That will not stay true. Candidate event sources exist on the horizon: the facility Kafka bus, or Postgres `LISTEN/NOTIFY` should the application database move to Postgres. The subscription design must not weld resolvers to any one change-detection mechanism. +That will not stay true. Candidate event sources exist on the horizon: the facility Kafka bus, or Postgres `LISTEN/NOTIFY` should the application database ([ADR-010](010-two-databases.md)) move to Postgres. The subscription design must not weld resolvers to any one change-detection mechanism. Two forces shape the design. Naive per-client polling multiplies identical reads: N subscribers to one proposal would issue N queries per tick against a GPFS-hosted SQLite file. And any process-local coordination state is a deployment constraint that must be a recorded decision with a retirement path, not an accident. diff --git a/api/docs/adr/010-two-databases.md b/api/docs/adr/010-two-databases.md new file mode 100644 index 00000000..c5344800 --- /dev/null +++ b/api/docs/adr/010-two-databases.md @@ -0,0 +1,37 @@ +--- +date: 2026-07-08 +--- + +# ADR-010 - Two databases: application DB vs per-proposal DAMNIT DBs + +## Context and Problem Statement + +The API touches two databases with different owners and lifecycles. + +Per-proposal DAMNIT databases (`{proposal_dir}/{damnit_subdir}/runs.sqlite`) are external. One file exists per proposal, on GPFS, written by the DAMNIT listener. The API reflects their schema at runtime and treats them as read-only. + +The application database (`dw_api.sqlite`) is owned by this service. Its schema and lifecycle are ours to control, and it is the only database this service writes to. + +Nothing structurally prevents application code from wiring the two together through a shared, generic `db.py`/`get_session` pair, which would let a query meant for one database silently run against the other. + +## Considered Options + +- Structural separation: disjoint packages, entry points, and names per database. +- A generic shared DB layer (one `db.py`, one `get_session`) serving both. +- Convention and documentation only. + +## Decision Outcome + +Chosen option: "structural separation", because the two stacks get disjoint entry points and distinct types, so the wrong-database mistake cannot compile. + +`appdb`-owned code (`state.py`, `main.py`'s `SQLAlchemyAsyncConfig`, `metadata/repository.py`) is the only writer of `dw_api.sqlite`. `runs/` repositories ([ADR-005](005-repository-pattern.md)) remain the only reader of DAMNIT proposal databases. The naming convention is fixed: "app DB" always means `dw_api.sqlite`; "DAMNIT DB" or "proposal DB" always means a `runs.sqlite`. + +### Consequences + +- Good: the wrong-database failure mode disappears structurally instead of by discipline. +- Good: the app DB's engine/session lifecycle has one home (`main.py`'s `SQLAlchemyPlugin`), so a future second config (a shared or Postgres backend) has a clear place to slot in without disturbing `runs/`. +- Bad: there are two parallel data-access stacks, with no shared session helper by design. + +## Details + +Possible future: a move to Postgres for the app DB would also make `LISTEN/NOTIFY` available as a push publisher for run-update subscriptions - see [ADR-009](009-channels-subscriptions.md)'s publisher selection. diff --git a/api/docs/architecture.md b/api/docs/architecture.md index c6a05626..4fd640cb 100644 --- a/api/docs/architecture.md +++ b/api/docs/architecture.md @@ -23,7 +23,7 @@ For more information, see [ADR-000](adr/000-vertical-slice-architecture.md). | `auth/` | Authentication and authorisation | OAuth flow, sessions, token store, `User`, permission classes, the membership policy | Partial | Policy still in `metadata/services.py` | | `contextfile/` | Context-file viewing | File reading, watching, its routes | Done | As-is | | `graphql/` | GraphQL transport only | Schema assembly, context, directives, controller binding - no resolvers, no domain logic (see [ADR-007](adr/007-graphql-transport-only.md)) | Partial | Assembly still in `shared/gql.py`; resolvers still here | -| `appdb/` | The app's own database (infrastructure) | Models, engine/session plumbing for `dw_api.sqlite` | Planned | `_db/` | +| `appdb/` | The app's own database (infrastructure) | Models, engine/session plumbing for `dw_api.sqlite` (see [ADR-010](adr/010-two-databases.md)) | Partial | `_db/` (Advanced Alchemy `SQLAlchemyPlugin`); `metadata/repository.py` | | `mymdc/` | MyMdC client (infrastructure) | Ports, clients, vendored models | Planned | `_mymdc/` | | `core/` | Cross-cutting, framework-free | Shared error classes (see [ADR-001](adr/001-error-classes.md)), `DamnitType`, value types (see [ADR-004](adr/004-proposal-path-locator.md)), converters | Planned | `shared/` + `utils.py` | | `main.py` / `app.py` / `state.py` | Composition root | `AppState`, `create_*` factories (see [ADR-002](adr/002-no-global-mutable-state.md)), `create_app()` - the only place that may import everything and read settings | Partial | `main.py` + `state.py` |