From db704cf296d497ab2b3dc458f7e73762ea46b69e Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:21:16 +0000 Subject: [PATCH] Derive the asset id lazily: opening a SQLite project no longer hashes its song (#108) open_graph_store / iter_scope_stores accept a zero-argument callable for asset_id, called only by the Postgres backend; FolderStorage.open_stores, open_project_graph and ProjectGraph.asset_id use it. A plain string still works. Co-Authored-By: Claude Sonnet 5.5 --- nw/graph.py | 8 ++++- nw/graph_backend.py | 28 ++++++++++----- nw/migrate.py | 4 +-- nw/storage.py | 6 +++- tests/test_lazy_asset_id.py | 70 +++++++++++++++++++++++++++++++++++++ 5 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 tests/test_lazy_asset_id.py diff --git a/nw/graph.py b/nw/graph.py index 336ecaac..cf3536f3 100644 --- a/nw/graph.py +++ b/nw/graph.py @@ -30,6 +30,7 @@ import logging from contextlib import contextmanager from dataclasses import dataclass +from functools import cached_property from pathlib import Path from typing import Iterable, Iterator, Optional from uuid import UUID, uuid4 @@ -137,7 +138,12 @@ class ProjectGraph: def __init__(self, project: ProjectLike) -> None: self.storage = as_project_storage(project) self.project_root = self.storage.root - self.asset_id = self.storage.asset_id + + @cached_property + def asset_id(self) -> str: + # Lazy: deriving it hashes the song file, and only writes (and the + # Postgres tenant key) need it. Computed once per graph, as before. + return self.storage.asset_id # -- lifecycle ----------------------------------------------------------- diff --git a/nw/graph_backend.py b/nw/graph_backend.py index 7c5f2fd6..df1f7b01 100644 --- a/nw/graph_backend.py +++ b/nw/graph_backend.py @@ -52,10 +52,10 @@ import logging import os -from collections.abc import Mapping +from collections.abc import Callable, Mapping from contextlib import contextmanager from pathlib import Path -from typing import Iterator, Optional +from typing import Iterator, Optional, Union from lacing import SqliteStore from lacing.store import IntervalAnnotationStore @@ -77,6 +77,12 @@ ENV_OWNER_ID = "NW_GRAPH_OWNER_ID" GraphBackend = str # "sqlite" | "postgres" at the boundary; str for forgiveness + +#: A tenant anchor, or a zero-argument callable that produces one. Only the +#: Postgres backend needs it, and deriving it can be costly (it may hash the +#: project's song file), so callers on a hot path pass the callable and SQLite +#: never calls it. +AssetIdLike = Union[str, Callable[[], str]] _SQLITE = "sqlite" _POSTGRES = "postgres" @@ -131,7 +137,7 @@ def _postgres_project_id(asset_id: str, scope: str) -> str: def open_graph_store( db_path: Path | str, *, - asset_id: str, + asset_id: AssetIdLike, scope: Optional[str] = None, rate: Optional[int] = None, env: Mapping[str, str] | None = None, @@ -147,7 +153,9 @@ def open_graph_store( db_path: The per-scope SQLite path (used directly in SQLite mode; in Postgres mode only its filename is used to derive the scope). asset_id: The project's stable ``project_asset_id`` — the Postgres - tenant anchor. Ignored in SQLite mode. + tenant anchor — or a zero-argument callable returning it. Ignored + in SQLite mode, where a callable is never called (so a hot path + can pass ``lambda: storage.asset_id`` without paying to derive it). scope: The logical scope name (``"graph"`` / ``"storyboard"`` / ``"alignment"`` / …). Defaults to deriving it from ``db_path``. rate: Project-wide rate for the Postgres store. Defaults to lacing's @@ -182,7 +190,7 @@ def open_graph_store( def _open_postgres( *, - asset_id: str, + asset_id: AssetIdLike, scope: str, rate: Optional[int], env: Mapping[str, str], @@ -206,7 +214,9 @@ def _open_postgres( db_url, rate=rate if rate is not None else DEFAULT_RATE, owner_id=owner_id, - project_id=_postgres_project_id(asset_id, scope), + project_id=_postgres_project_id( + asset_id() if callable(asset_id) else asset_id, scope + ), ) @@ -214,7 +224,7 @@ def _open_postgres( def iter_scope_stores( scope_paths: Mapping[str, Path], *, - asset_id: str, + asset_id: AssetIdLike, env: Mapping[str, str] | None = None, ) -> Iterator[Iterator[IntervalAnnotationStore]]: """Yield an iterator of open stores, one per scope, backend chosen by env. @@ -230,7 +240,9 @@ def iter_scope_stores( *exist* on disk are visited in SQLite mode; in Postgres mode every listed scope is visited (existence is a DB question, not a file one). - asset_id: The project's ``project_asset_id`` (Postgres tenant anchor). + asset_id: The project's ``project_asset_id`` (Postgres tenant anchor), + or a zero-argument callable returning it (never called in SQLite + mode). env: Environment mapping. Defaults to ``os.environ``. Yields: diff --git a/nw/migrate.py b/nw/migrate.py index 4640975e..4f0d207c 100644 --- a/nw/migrate.py +++ b/nw/migrate.py @@ -101,7 +101,7 @@ def open_project_graph(project_root: Path) -> IntervalAnnotationStore: """ db = project_graph_db_path(project_root) store = open_graph_store( - db, asset_id=project_asset_id(project_root), scope=SCOPE_GRAPH + db, asset_id=lambda: project_asset_id(project_root), scope=SCOPE_GRAPH ) _ensure_tiers(store) return store @@ -157,7 +157,7 @@ def open_project_graph_readonly(project_root: Path) -> IntervalAnnotationStore: db = project_graph_db_path(project_root) if selected_backend(os.environ) == "postgres": return open_graph_store( - db, asset_id=project_asset_id(project_root), scope=SCOPE_GRAPH + db, asset_id=lambda: project_asset_id(project_root), scope=SCOPE_GRAPH ) if not Path(db).exists(): raise FileNotFoundError(f"no graph database for project: {project_root}") diff --git a/nw/storage.py b/nw/storage.py index 4b696c98..65171109 100644 --- a/nw/storage.py +++ b/nw/storage.py @@ -224,7 +224,11 @@ def open_graph_readonly(self) -> IntervalAnnotationStore: @contextmanager def open_stores(self) -> Iterator[Iterator[IntervalAnnotationStore]]: - with iter_scope_stores(self.scope_paths(), asset_id=self.asset_id) as stores: + # Lazy: only the Postgres backend keys a tenant on the asset id, and + # deriving it hashes the song file. + with iter_scope_stores( + self.scope_paths(), asset_id=lambda: self.asset_id + ) as stores: yield stores diff --git a/tests/test_lazy_asset_id.py b/tests/test_lazy_asset_id.py new file mode 100644 index 00000000..43592e00 --- /dev/null +++ b/tests/test_lazy_asset_id.py @@ -0,0 +1,70 @@ +"""Opening a SQLite-backed project must not hash its song file (nw#108). + +``asset_id`` is the Postgres tenant anchor and the media reference of new +annotations; deriving it SHA-256s the whole song. A host that opens a project's +stores on a hot path (reelee reads ``project-meta`` rows on every call that +targets a shared project) must not pay that on a backend that never uses it. +""" + +from __future__ import annotations + +import pytest + +import lacing +import nw +from nw.graph import ProjectGraph, iter_all_annotations +from nw.graph_backend import iter_scope_stores, open_graph_store + + +@pytest.fixture +def project_with_song(tmp_path, monkeypatch): + monkeypatch.delenv("NW_GRAPH_BACKEND", raising=False) + song = tmp_path / "song.wav" + song.write_bytes(b"not really audio" * 100) + project = nw.Project.init(tmp_path / "p", title="t", song=song) + project.storage.open_graph().close() # the graph exists before hashing is forbidden + return project + + +@pytest.fixture +def forbid_song_hash(monkeypatch): + def boom(*a, **k): + raise AssertionError("the song file was hashed") + + monkeypatch.setattr(lacing, "hash_file", boom) + + +def test_opening_stores_does_not_hash_the_song(project_with_song, forbid_song_hash): + storage = project_with_song.storage + with storage.open_stores() as stores: + assert len(list(stores)) >= 1 + assert list(iter_all_annotations(storage.root)) is not None + storage.open_graph().close() + storage.open_graph_readonly().close() + + +def test_constructing_a_graph_does_not_hash_the_song( + project_with_song, forbid_song_hash +): + graph = ProjectGraph(project_with_song.root) + list(graph.shots()) + + +def test_asset_id_is_still_the_song_hash_when_asked(project_with_song): + graph = ProjectGraph(project_with_song.root) + assert graph.asset_id == lacing.hash_file(project_with_song.song_path()) + + +def test_callable_asset_id_is_never_called_under_sqlite(tmp_path): + def boom(): + raise AssertionError("called") + + store = open_graph_store( + tmp_path / "project.annot.sqlite", asset_id=boom, env={} + ) + store.close() + (tmp_path / "project.annot.sqlite").exists() + with iter_scope_stores( + {"graph": tmp_path / "project.annot.sqlite"}, asset_id=boom, env={} + ) as stores: + assert len(list(stores)) == 1