From 14266fe78937e6d94716cb7279a36cab6de776e5 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Fri, 19 Jun 2026 15:42:33 +1200 Subject: [PATCH] Defer public asset file paths Amp-Thread-ID: https://ampcode.com/threads/T-019ecf39-2e6f-747d-ae80-addba6b8e4f5 Co-authored-by: Amp --- app/assets/api/routes.py | 2 -- app/assets/api/schemas_out.py | 1 - app/assets/services/path_utils.py | 19 ------------------- openapi.yaml | 16 ---------------- .../assets_test/test_assets_missing_sync.py | 19 ++++++------------- tests-unit/assets_test/test_crud.py | 6 +----- .../assets_test/test_prune_orphaned_assets.py | 17 ++++++----------- tests-unit/assets_test/test_uploads.py | 2 -- 8 files changed, 13 insertions(+), 69 deletions(-) diff --git a/app/assets/api/routes.py b/app/assets/api/routes.py index cdcb39c79..f40211f6c 100644 --- a/app/assets/api/routes.py +++ b/app/assets/api/routes.py @@ -39,7 +39,6 @@ from app.assets.services import ( upload_from_temp_path, ) from app.assets.services.cursor import InvalidCursorError -from app.assets.services.path_utils import compute_api_file_path from app.assets.services.tagging import list_tag_histogram ROUTES = web.RouteTableDef() @@ -169,7 +168,6 @@ def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResu asset_hash=asset_content_hash, size=int(result.asset.size_bytes) if result.asset else None, mime_type=result.asset.mime_type if result.asset else None, - file_path=compute_api_file_path(result.ref.file_path), tags=result.tags, preview_url=preview_url, preview_id=result.ref.preview_id, diff --git a/app/assets/api/schemas_out.py b/app/assets/api/schemas_out.py index 4214aeb0e..4e38e19d1 100644 --- a/app/assets/api/schemas_out.py +++ b/app/assets/api/schemas_out.py @@ -14,7 +14,6 @@ class Asset(BaseModel): asset_hash: str | None = None size: int | None = None mime_type: str | None = None - file_path: str | None = None tags: list[str] = Field(default_factory=list) preview_url: str | None = None preview_id: str | None = None # references an asset_reference id, not an asset id diff --git a/app/assets/services/path_utils.py b/app/assets/services/path_utils.py index ac9ce152a..3c64f0bef 100644 --- a/app/assets/services/path_utils.py +++ b/app/assets/services/path_utils.py @@ -114,25 +114,6 @@ def compute_relative_filename(file_path: str) -> str | None: return "/".join(parts) # input/output: keep all parts -def compute_api_file_path(file_path: str | None) -> str | None: - """Return a stable API-visible path relative to a known asset root. - - Examples: - /.../input/foo.png -> "input/foo.png" - /.../models/checkpoints/foo.safetensors -> "models/checkpoints/foo.safetensors" - - Returns None for references without a filesystem path or paths outside - known asset roots. - """ - if not file_path: - return None - try: - root_category, rel_path = get_asset_category_and_relative_path(file_path) - except ValueError: - return None - return "/".join([root_category, *Path(rel_path).parts]) - - def get_asset_category_and_relative_path( file_path: str, ) -> tuple[Literal["input", "output", "temp", "models"], str]: diff --git a/openapi.yaml b/openapi.yaml index 2446e64e4..8c10305ee 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -7,14 +7,6 @@ components: description: Timestamp when the asset was created format: date-time type: string - display_name: - description: Display name of the asset. Mirrors name for backwards compatibility. - nullable: true - type: string - file_path: - description: Relative path in global-namespace-root form (e.g. "models/checkpoints/flux.safetensors") - nullable: true - type: string hash: description: Blake3 hash of the asset content. pattern: ^blake3:[a-f0-9]{64}$ @@ -138,14 +130,6 @@ components: AssetUpdated: description: Response returned when an existing asset is successfully updated. properties: - display_name: - description: Display name of the asset. Mirrors name for backwards compatibility. - nullable: true - type: string - file_path: - description: Relative path in global-namespace-root form (e.g. "models/checkpoints/flux.safetensors") - nullable: true - type: string hash: description: Blake3 hash of the asset content. pattern: ^blake3:[a-f0-9]{64}$ diff --git a/tests-unit/assets_test/test_assets_missing_sync.py b/tests-unit/assets_test/test_assets_missing_sync.py index e9c3d94b5..205723650 100644 --- a/tests-unit/assets_test/test_assets_missing_sync.py +++ b/tests-unit/assets_test/test_assets_missing_sync.py @@ -19,8 +19,8 @@ def test_seed_asset_removed_when_file_is_deleted( """Asset without hash (seed) whose file disappears: after triggering sync_seed_assets, Asset + AssetInfo disappear. """ - # Create a file directly under input/unit-tests/. Path components are - # exposed through file_path; backend tags only classify the root. + # Create a file directly under input/unit-tests/. Backend tags only + # classify the root; nested path components are not exposed as tags. case_dir = comfy_tmp_base_dir / root / "unit-tests" / "syncseed" case_dir.mkdir(parents=True, exist_ok=True) name = f"seed_{uuid.uuid4().hex[:8]}.bin" @@ -39,11 +39,7 @@ def test_seed_asset_removed_when_file_is_deleted( body1 = r1.json() assert r1.status_code == 200 # there should be exactly one with that name - expected_suffix = f"{root}/unit-tests/syncseed/{name}" - matches = [ - a for a in body1.get("assets", []) - if a.get("name") == name and a.get("file_path") == expected_suffix - ] + matches = [a for a in body1.get("assets", []) if a.get("name") == name] assert matches # Seed assets have no hash; exclude_none drops both keys from the response assert "asset_hash" not in matches[0] @@ -64,10 +60,7 @@ def test_seed_asset_removed_when_file_is_deleted( ) body2 = r2.json() assert r2.status_code == 200 - matches2 = [ - a for a in body2.get("assets", []) - if a.get("name") == name and a.get("file_path") == expected_suffix - ] + matches2 = [a for a in body2.get("assets", []) if a.get("name") == name] assert not matches2, f"Seed asset {asset_info_id} should be gone after sync" @@ -140,7 +133,7 @@ def test_hashed_asset_two_asset_infos_both_get_missing( second_id = b2["id"] # Remove the single underlying file - p = comfy_tmp_base_dir / created["file_path"] + p = comfy_tmp_base_dir / "input" / get_asset_filename(created["asset_hash"], ".png") assert p.exists() p.unlink() @@ -258,7 +251,7 @@ def test_missing_tag_clears_on_fastpass_when_mtime_and_size_match( a = asset_factory(name, [root, "unit-tests", scope], {}, data) aid = a["id"] - p = comfy_tmp_base_dir / a["file_path"] + p = comfy_tmp_base_dir / root / get_asset_filename(a["asset_hash"], ".bin") st0 = p.stat() orig_mtime_ns = getattr(st0, "st_mtime_ns", int(st0.st_mtime * 1_000_000_000)) diff --git a/tests-unit/assets_test/test_crud.py b/tests-unit/assets_test/test_crud.py index 1e2a5d7ae..9a965bcdf 100644 --- a/tests-unit/assets_test/test_crud.py +++ b/tests-unit/assets_test/test_crud.py @@ -295,11 +295,7 @@ def test_metadata_filename_is_set_for_seed_asset_without_hash( ) body = r1.json() assert r1.status_code == 200, body - expected_file_path = f"{root}/unit-tests/{scope}/a/b/{name}" - matches = [ - a for a in body.get("assets", []) - if a.get("name") == name and a.get("file_path") == expected_file_path - ] + matches = [a for a in body.get("assets", []) if a.get("name") == name] assert matches, "Seed asset should be visible after sync" # Seed assets have no hash; exclude_none drops both keys from the response assert "asset_hash" not in matches[0] diff --git a/tests-unit/assets_test/test_prune_orphaned_assets.py b/tests-unit/assets_test/test_prune_orphaned_assets.py index f797b5732..7ab74ce2d 100644 --- a/tests-unit/assets_test/test_prune_orphaned_assets.py +++ b/tests-unit/assets_test/test_prune_orphaned_assets.py @@ -3,7 +3,7 @@ from pathlib import Path import pytest import requests -from helpers import trigger_sync_seed_assets +from helpers import get_asset_filename, trigger_sync_seed_assets @pytest.fixture @@ -35,11 +35,6 @@ def find_asset(http: requests.Session, api_base: str): r = http.get(f"{api_base}/api/assets", params=params, timeout=120) assert r.status_code == 200 assets = r.json().get("assets", []) - expected_path_fragment = f"/unit-tests/{scope}/" - assets = [ - a for a in assets - if expected_path_fragment in f"/{a.get('file_path', '')}" - ] if name: return [a for a in assets if a.get("name") == name] return assets @@ -96,7 +91,7 @@ def test_hashed_asset_not_pruned_when_file_missing( data = make_asset_bytes("test", 2048) a = asset_factory("test.bin", ["input", "unit-tests", scope], {}, data) - path = comfy_tmp_base_dir / a["file_path"] + path = comfy_tmp_base_dir / "input" / get_asset_filename(a["asset_hash"], ".bin") path.unlink() trigger_sync_seed_assets(http, api_base) @@ -117,14 +112,14 @@ def test_prune_across_multiple_roots( create_seed_file("output", scope, "output.bin") trigger_sync_seed_assets(http, api_base) - assert len(find_asset(scope)) == 2 + assert find_asset(scope, input_fp.name) + assert find_asset(scope, "output.bin") input_fp.unlink() trigger_sync_seed_assets(http, api_base) - remaining = find_asset(scope) - assert len(remaining) == 1 - assert remaining[0]["name"] == "output.bin" + assert not find_asset(scope, input_fp.name) + assert find_asset(scope, "output.bin") @pytest.mark.parametrize("dirname", ["100%_done", "my_folder_name", "has spaces"]) diff --git a/tests-unit/assets_test/test_uploads.py b/tests-unit/assets_test/test_uploads.py index 7bdab6499..cb7f6cd30 100644 --- a/tests-unit/assets_test/test_uploads.py +++ b/tests-unit/assets_test/test_uploads.py @@ -381,7 +381,6 @@ def test_upload_subfolder_is_explicit_path_component( assert r.status_code == 201, body stored_name = get_asset_filename(body["asset_hash"], ".bin") assert (comfy_tmp_base_dir / "input" / "foo" / "bar" / stored_name).exists() - assert body["file_path"] == f"input/foo/bar/{stored_name}" assert "foo" in body["tags"] @@ -498,7 +497,6 @@ def test_multipart_upload_role_selects_write_location( stored_name = get_asset_filename(body["asset_hash"], extension) expected_disk_path = comfy_tmp_base_dir / expected_root / stored_name assert expected_disk_path.exists() - assert body["file_path"] == f"{expected_root}/{stored_name}" def test_upload_empty_tags_rejected(http: requests.Session, api_base: str):