From bd7d95bdb40d4c3aec7453bd52c97f6426d8cbb6 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Thu, 2 Jul 2026 08:23:55 +1200 Subject: [PATCH] test(assets): lock loader_path matrix (asymmetry, null, persist/read) Cover the behaviour that has no production change but is easy to regress: the extra-path asymmetry (loadable but no storage namespace), null loader_path persistence for orphan files, and the response reading the stored column with a compute fallback for un-backfilled rows. --- .../test_asset_response_loader_path.py | 84 +++++++++++++++++++ .../assets_test/services/test_bulk_ingest.py | 30 +++++++ .../assets_test/services/test_path_utils.py | 26 ++++++ 3 files changed, 140 insertions(+) create mode 100644 tests-unit/assets_test/services/test_asset_response_loader_path.py diff --git a/tests-unit/assets_test/services/test_asset_response_loader_path.py b/tests-unit/assets_test/services/test_asset_response_loader_path.py new file mode 100644 index 000000000..fbe864152 --- /dev/null +++ b/tests-unit/assets_test/services/test_asset_response_loader_path.py @@ -0,0 +1,84 @@ +"""Tests for how _build_asset_response derives the response `loader_path`. + +Guards the persist-and-read contract: the response reads the stored +`loader_path` directly, and only recomputes when the column is NULL (rows +written before the column existed). +""" + +from datetime import datetime +from pathlib import Path +from unittest.mock import patch + +from app.assets.api.routes import _build_asset_response +from app.assets.services.schemas import AssetDetailResult, ReferenceData + +_TS = datetime(2024, 1, 1, 0, 0, 0) + + +def _make_result( + *, file_path: str | None, loader_path: str | None +) -> AssetDetailResult: + ref = ReferenceData( + id="ref-1", + name="model.safetensors", + file_path=file_path, + loader_path=loader_path, + user_metadata=None, + preview_id=None, + created_at=_TS, + updated_at=_TS, + last_access_time=_TS, + ) + return AssetDetailResult(ref=ref, asset=None, tags=[]) + + +def test_uses_persisted_loader_path_without_recomputing(): + """A stored loader_path is returned verbatim, not re-derived from file_path. + + The sentinel value could never be produced by compute_loader_path for this + file_path, so seeing it in the response proves the stored column is read. + """ + result = _make_result( + file_path="/unmatched/root/model.safetensors", + loader_path="SENTINEL/stored.safetensors", + ) + + resp = _build_asset_response(result) + + assert resp.loader_path == "SENTINEL/stored.safetensors" + + +def test_falls_back_to_compute_when_stored_loader_path_is_null(tmp_path: Path): + """A NULL column (pre-migration row) is backfilled at read time.""" + models = tmp_path / "models" + ckpt = models / "checkpoints" + ckpt.mkdir(parents=True) + f = ckpt / "bar.safetensors" + f.touch() + + with patch("app.assets.services.path_utils.folder_paths") as mock_fp, patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("checkpoints", [str(ckpt)], {".safetensors"})], + ): + mock_fp.get_input_directory.return_value = str(tmp_path / "in") + mock_fp.get_output_directory.return_value = str(tmp_path / "out") + mock_fp.get_temp_directory.return_value = str(tmp_path / "tmp") + mock_fp.models_dir = str(models) + + result = _make_result(file_path=str(f), loader_path=None) + resp = _build_asset_response(result) + + assert resp.loader_path == "bar.safetensors" + assert resp.logical_path == "models/checkpoints/bar.safetensors" + assert resp.display_name == "checkpoints/bar.safetensors" + + +def test_all_path_fields_null_without_file_path(): + """API-created / hash-only references (no file_path) expose no paths.""" + result = _make_result(file_path=None, loader_path=None) + + resp = _build_asset_response(result) + + assert resp.loader_path is None + assert resp.logical_path is None + assert resp.display_name is None diff --git a/tests-unit/assets_test/services/test_bulk_ingest.py b/tests-unit/assets_test/services/test_bulk_ingest.py index c174fce44..a3889f235 100644 --- a/tests-unit/assets_test/services/test_bulk_ingest.py +++ b/tests-unit/assets_test/services/test_bulk_ingest.py @@ -253,6 +253,36 @@ class TestBatchInsertSeedAssets: "model_type:diffusion_models", } + def test_loader_path_persisted_as_null_when_fname_is_none( + self, session: Session, temp_dir: Path + ): + """A file with no in-root loader path (fname=None, e.g. an orphan under + models_root) persists loader_path as NULL rather than a synthesized value.""" + file_path = temp_dir / "orphan.bin" + file_path.write_bytes(b"x") + + specs: list[SeedAssetSpec] = [ + { + "abs_path": str(file_path), + "size_bytes": 1, + "mtime_ns": 1234567890000000000, + "info_name": "orphan.bin", + "tags": [], + "fname": None, + "metadata": None, + "hash": None, + "mime_type": None, + } + ] + + result = batch_insert_seed_assets(session, specs=specs, owner_id="") + + assert result.inserted_refs == 1 + refs = session.query(AssetReference).all() + assert len(refs) == 1 + assert refs[0].file_path == str(file_path) + assert refs[0].loader_path is None + class TestMetadataExtraction: def test_extracts_mime_type_for_model_files(self, temp_dir: Path): diff --git a/tests-unit/assets_test/services/test_path_utils.py b/tests-unit/assets_test/services/test_path_utils.py index a14487193..ad769d209 100644 --- a/tests-unit/assets_test/services/test_path_utils.py +++ b/tests-unit/assets_test/services/test_path_utils.py @@ -523,6 +523,32 @@ class TestLoaderPath: assert compute_logical_path(str(f)) == "models/not_registered/orphan.bin" assert compute_loader_path(str(f)) is None + def test_extra_path_model_has_loader_path_but_no_logical_path(self, tmp_path: Path): + """Registered category base outside models_dir (extra_model_paths style). + + Loadable, so loader_path resolves; but it is not under any canonical + storage root, so logical_path/display_name are None. This asymmetry is + intentional: loader_path resolves every registered model-folder base, + logical_path only resolves the canonical storage roots. + """ + extra = tmp_path / "extra_ckpts" + extra.mkdir() + f = extra / "foo.safetensors" + f.touch() + + with patch("app.assets.services.path_utils.folder_paths") as mock_fp, patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("checkpoints", [str(extra)], {".safetensors"})], + ): + mock_fp.get_input_directory.return_value = str(tmp_path / "in") + mock_fp.get_output_directory.return_value = str(tmp_path / "out") + mock_fp.get_temp_directory.return_value = str(tmp_path / "tmp") + mock_fp.models_dir = str(tmp_path / "models") # extra is NOT under this + + assert compute_loader_path(str(f)) == "foo.safetensors" + assert compute_logical_path(str(f)) is None + assert compute_display_name(str(f)) is None + def test_unknown_path_returns_none(self): assert compute_loader_path("/some/random/path.png") is None