Derive the asset id lazily: SQLite opens no longer hash the song (#108)

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 -----------------------------------------------------------
 

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,15 +214,17 @@ 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
+        ),
     )
 
 
 @contextmanager
 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:

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}")

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
 
 

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