diff --git a/app/assets/api/routes.py b/app/assets/api/routes.py index f40211f6c..2a3f4a858 100644 --- a/app/assets/api/routes.py +++ b/app/assets/api/routes.py @@ -39,6 +39,7 @@ from app.assets.services import ( upload_from_temp_path, ) from app.assets.services.cursor import InvalidCursorError +from app.assets.services.path_utils import compute_asset_response_paths from app.assets.services.tagging import list_tag_histogram ROUTES = web.RouteTableDef() @@ -160,11 +161,18 @@ def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResu preview_url = None else: preview_url = _build_preview_url_from_view(result.tags, result.ref.user_metadata) + if result.ref.file_path: + paths = compute_asset_response_paths(result.ref.file_path) + file_path, display_name = paths if paths else (None, None) + else: + file_path, display_name = None, None asset_content_hash = result.asset.hash if result.asset else None return schemas_out.Asset( id=result.ref.id, name=result.ref.name, hash=asset_content_hash, + file_path=file_path, + display_name=display_name, 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, diff --git a/app/assets/api/schemas_out.py b/app/assets/api/schemas_out.py index 4e38e19d1..d4d2c699d 100644 --- a/app/assets/api/schemas_out.py +++ b/app/assets/api/schemas_out.py @@ -9,8 +9,20 @@ class Asset(BaseModel): ``id`` here is the AssetReference id, not the content-addressed Asset id.""" id: str - name: str + name: str = Field( + ..., + deprecated=True, + description="Reference label, often caller-provided or derived from the filename. Deprecated for storage path/display semantics; use `file_path` and `display_name` when present.", + ) hash: str | None = None + file_path: str | None = Field( + default=None, + description="Runtime storage locator for filesystem-backed assets, using Comfy storage namespaces such as `input/`, `output/`, `temp/`, or `models/`. Not an absolute filesystem path, unique identity, or model loader path.", + ) + display_name: str | None = Field( + default=None, + description="Human-facing label derived from `file_path`, usually the path below the top-level storage namespace. Not unique.", + ) asset_hash: str | None = None size: int | None = None mime_type: str | None = None diff --git a/app/assets/services/path_utils.py b/app/assets/services/path_utils.py index 3c64f0bef..8b2eba37f 100644 --- a/app/assets/services/path_utils.py +++ b/app/assets/services/path_utils.py @@ -88,6 +88,62 @@ def validate_path_within_base(candidate: str, base: str) -> None: raise ValueError("destination escapes base directory") +def _compute_relative_path(child: str, parent: str) -> str: + rel = os.path.relpath(os.path.abspath(child), os.path.abspath(parent)) + if rel == ".": + return "" + return rel.replace(os.sep, "/") + + +def _is_relative_to(child: str, parent: str) -> bool: + return Path(os.path.abspath(child)).is_relative_to(os.path.abspath(parent)) + + +def compute_asset_response_paths(file_path: str) -> tuple[str, str | None] | None: + """Return public (file_path, display_name) response fields for a file path. + + These fields are storage locators, not model-loader namespaces. Registered + model-folder membership is represented by backend tags such as + ``model_type:``; response paths only use known storage roots. + """ + fp_abs = os.path.abspath(file_path) + candidates: list[tuple[int, int, str, str]] = [] + + for order, (namespace, base) in enumerate( + ( + ("input", folder_paths.get_input_directory()), + ("output", folder_paths.get_output_directory()), + ("temp", folder_paths.get_temp_directory()), + ("models", getattr(folder_paths, "models_dir", "")), + ) + ): + if not base: + continue + base_abs = os.path.abspath(base) + if _is_relative_to(fp_abs, base_abs): + candidates.append((len(base_abs), -order, namespace, base_abs)) + + if not candidates: + return None + + _base_len, _order, namespace, base = max(candidates) + rel = _compute_relative_path(fp_abs, base) + public_path = f"{namespace}/{rel}" if rel else namespace + return public_path, rel or None + + +def compute_display_name(file_path: str) -> str | None: + """Return the asset's `display_name`, or None for unknown paths.""" + result = compute_asset_response_paths(file_path) + return result[1] if result else None + + +def compute_file_path(file_path: str) -> str | None: + """Return the asset's logical storage `file_path`, or None for unknown paths.""" + result = compute_asset_response_paths(file_path) + return result[0] if result else None + + def compute_relative_filename(file_path: str) -> str | None: """ Return the model's path relative to the last well-known folder (the model category), @@ -95,6 +151,9 @@ def compute_relative_filename(file_path: str) -> str | None: /.../models/checkpoints/flux/123/flux.safetensors -> "flux/123/flux.safetensors" /.../models/text_encoders/clip_g.safetensors -> "clip_g.safetensors" + This is legacy metadata/view filename logic, not the public Asset response + `display_name`. Response fields should use compute_asset_response_paths(). + For non-model paths, returns None. """ try: @@ -139,9 +198,10 @@ def get_asset_category_and_relative_path( def _compute_relative(child: str, parent: str) -> str: # Normalize relative path, stripping any leading ".." components # by anchoring to root (os.sep) then computing relpath back from it. - return os.path.relpath( + rel = os.path.relpath( os.path.join(os.sep, os.path.relpath(child, parent)), os.sep ) + return "" if rel == "." else rel.replace(os.sep, "/") # 1) input input_base = os.path.abspath(folder_paths.get_input_directory()) @@ -172,7 +232,8 @@ def get_asset_category_and_relative_path( if best is not None: _, bucket, rel_inside = best combined = os.path.join(bucket, rel_inside) - return "models", os.path.relpath(os.path.join(os.sep, combined), os.sep) + normalized = os.path.relpath(os.path.join(os.sep, combined), os.sep) + return "models", normalized.replace(os.sep, "/") raise ValueError( f"Path is not within input, output, temp, or configured model bases: {file_path}" diff --git a/tests-unit/assets_test/services/test_path_utils.py b/tests-unit/assets_test/services/test_path_utils.py index fe92896a8..5a1e1df38 100644 --- a/tests-unit/assets_test/services/test_path_utils.py +++ b/tests-unit/assets_test/services/test_path_utils.py @@ -7,6 +7,8 @@ from unittest.mock import patch import pytest from app.assets.services.path_utils import ( + compute_display_name, + compute_file_path, get_asset_category_and_relative_path, get_name_and_tags_from_asset_path, resolve_destination_from_tags, @@ -21,7 +23,8 @@ def fake_dirs(): input_dir = root_path / "input" output_dir = root_path / "output" temp_dir = root_path / "temp" - models_dir = root_path / "models" / "checkpoints" + models_root = root_path / "models" + models_dir = models_root / "checkpoints" for d in (input_dir, output_dir, temp_dir, models_dir): d.mkdir(parents=True) @@ -29,6 +32,7 @@ def fake_dirs(): mock_fp.get_input_directory.return_value = str(input_dir) mock_fp.get_output_directory.return_value = str(output_dir) mock_fp.get_temp_directory.return_value = str(temp_dir) + mock_fp.models_dir = str(models_root) with patch( "app.assets.services.path_utils.get_comfy_models_folders", @@ -38,6 +42,7 @@ def fake_dirs(): "input": input_dir, "output": output_dir, "temp": temp_dir, + "models_root": models_root, "models": models_dir, } @@ -170,6 +175,230 @@ class TestGetAssetCategoryAndRelativePath: get_asset_category_and_relative_path("/some/random/path.png") +class TestResponseStoragePaths: + def test_input_file_path_and_display_name_include_subfolder(self, fake_dirs): + sub = fake_dirs["input"] / "some" / "folder" + sub.mkdir(parents=True) + f = sub / "image.png" + f.touch() + + assert compute_file_path(str(f)) == "input/some/folder/image.png" + assert compute_display_name(str(f)) == "some/folder/image.png" + + def test_output_file_path_and_display_name_include_subfolder(self, fake_dirs): + sub = fake_dirs["output"] / "renders" + sub.mkdir() + f = sub / "ComfyUI_00001_.png" + f.touch() + + assert compute_file_path(str(f)) == "output/renders/ComfyUI_00001_.png" + assert compute_display_name(str(f)) == "renders/ComfyUI_00001_.png" + + def test_temp_file_path_and_display_name(self, fake_dirs): + f = fake_dirs["temp"] / "preview.png" + f.touch() + + assert compute_file_path(str(f)) == "temp/preview.png" + assert compute_display_name(str(f)) == "preview.png" + + def test_exact_storage_root_has_no_display_name(self, fake_dirs): + assert compute_file_path(str(fake_dirs["input"])) == "input" + assert compute_display_name(str(fake_dirs["input"])) is None + + def test_longest_matching_builtin_root_wins(self, fake_dirs, tmp_path: Path): + nested_output = fake_dirs["input"] / "nested-output" + nested_output.mkdir() + f = nested_output / "image.png" + f.touch() + + with patch("app.assets.services.path_utils.folder_paths") as mock_fp: + mock_fp.get_input_directory.return_value = str(fake_dirs["input"]) + mock_fp.get_output_directory.return_value = str(nested_output) + mock_fp.get_temp_directory.return_value = str(tmp_path / "temp") + mock_fp.models_dir = str(fake_dirs["models_root"]) + + assert compute_file_path(str(f)) == "output/image.png" + assert compute_display_name(str(f)) == "image.png" + + def test_model_file_path_is_relative_to_physical_models_root(self, fake_dirs): + sub = fake_dirs["models"] / "flux" + sub.mkdir() + f = sub / "model.safetensors" + f.touch() + + assert compute_file_path(str(f)) == "models/checkpoints/flux/model.safetensors" + assert compute_display_name(str(f)) == "checkpoints/flux/model.safetensors" + + name, tags = get_name_and_tags_from_asset_path(str(f)) + assert name == "model.safetensors" + assert "models" in tags + assert "model_type:checkpoints" in tags + assert "checkpoints" not in tags + assert "flux" not in tags + + @pytest.mark.parametrize( + "folder_name", + ["checkpoints", "clip", "vae", "diffusion_models", "loras"], + ) + def test_output_model_folder_uses_output_storage_file_path(self, fake_dirs, folder_name): + output_model_dir = fake_dirs["output"] / folder_name + output_model_dir.mkdir(exist_ok=True) + default_model_dir = fake_dirs["models_root"] / folder_name + default_model_dir.mkdir(exist_ok=True) + f = output_model_dir / "saved.safetensors" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[(folder_name, [str(default_model_dir), str(output_model_dir)])], + ): + assert compute_file_path(str(f)) == f"output/{folder_name}/saved.safetensors" + assert compute_display_name(str(f)) == f"{folder_name}/saved.safetensors" + + name, tags = get_name_and_tags_from_asset_path(str(f)) + assert name == "saved.safetensors" + assert "output" in tags + assert "models" in tags + assert f"model_type:{folder_name}" in tags + assert folder_name not in tags + + def test_output_model_subfolder_uses_output_storage_file_path(self, fake_dirs): + folder_name = "loras" + output_model_dir = fake_dirs["output"] / folder_name + subdir = output_model_dir / "experiments" + subdir.mkdir(parents=True) + f = subdir / "my_lora.safetensors" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[(folder_name, [str(output_model_dir)])], + ): + assert ( + compute_file_path(str(f)) + == "output/loras/experiments/my_lora.safetensors" + ) + assert compute_display_name(str(f)) == "loras/experiments/my_lora.safetensors" + + name, tags = get_name_and_tags_from_asset_path(str(f)) + assert name == "my_lora.safetensors" + assert "output" in tags + assert "models" in tags + assert "model_type:loras" in tags + assert "loras" not in tags + assert "experiments" not in tags + + def test_external_model_folder_without_provenance_has_no_file_path(self, tmp_path: Path): + external_checkpoints_dir = tmp_path / "external" / "not_named_like_category" + external_checkpoints_dir.mkdir(parents=True) + f = external_checkpoints_dir / "external.safetensors" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("checkpoints", [str(external_checkpoints_dir)])], + ): + assert compute_file_path(str(f)) is None + assert compute_display_name(str(f)) is None + + name, tags = get_name_and_tags_from_asset_path(str(f)) + assert name == "external.safetensors" + assert "models" in tags + assert "model_type:checkpoints" in tags + + def test_same_relative_model_file_under_multiple_external_roots_has_no_storage_file_path( + self, tmp_path: Path + ): + foo_dir = tmp_path / "foo" + bar_dir = tmp_path / "bar" + foo_dir.mkdir() + bar_dir.mkdir() + foo_file = foo_dir / "baz.safetensors" + bar_file = bar_dir / "baz.safetensors" + foo_file.touch() + bar_file.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("checkpoints", [str(foo_dir), str(bar_dir)])], + ): + assert compute_file_path(str(foo_file)) is None + assert compute_file_path(str(bar_file)) is None + assert compute_display_name(str(foo_file)) is None + assert compute_display_name(str(bar_file)) is None + + def test_output_clip_folder_uses_output_storage_and_text_encoder_tag(self, fake_dirs): + output_clip_dir = fake_dirs["output"] / "clip" + output_clip_dir.mkdir() + f = output_clip_dir / "clip_l.safetensors" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("text_encoders", [str(output_clip_dir)])], + ): + assert compute_file_path(str(f)) == "output/clip/clip_l.safetensors" + assert compute_display_name(str(f)) == "clip/clip_l.safetensors" + + name, tags = get_name_and_tags_from_asset_path(str(f)) + assert name == "clip_l.safetensors" + assert "output" in tags + assert "models" in tags + assert "model_type:text_encoders" in tags + assert "clip" not in tags + + def test_physical_unet_folder_uses_storage_path_and_diffusion_models_tag(self, fake_dirs): + unet_dir = fake_dirs["models_root"] / "unet" + diffusion_models_dir = fake_dirs["models_root"] / "diffusion_models" + unet_dir.mkdir() + diffusion_models_dir.mkdir() + f = unet_dir / "wan.safetensors" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("diffusion_models", [str(unet_dir), str(diffusion_models_dir)])], + ): + assert compute_file_path(str(f)) == "models/unet/wan.safetensors" + assert compute_display_name(str(f)) == "unet/wan.safetensors" + + name, tags = get_name_and_tags_from_asset_path(str(f)) + assert name == "wan.safetensors" + assert "models" in tags + assert "model_type:diffusion_models" in tags + assert "unet" not in tags + + def test_unregistered_file_under_physical_models_root_still_has_storage_file_path(self, fake_dirs): + f = fake_dirs["models_root"] / "not_registered" / "orphan.bin" + f.parent.mkdir() + f.touch() + + assert compute_file_path(str(f)) == "models/not_registered/orphan.bin" + assert compute_display_name(str(f)) == "not_registered/orphan.bin" + + def test_output_checkpoint_folder_without_registration_has_only_output_tag(self, fake_dirs): + f = fake_dirs["output"] / "checkpoints" / "saved.safetensors" + f.parent.mkdir(exist_ok=True) + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[], + ): + assert compute_file_path(str(f)) == "output/checkpoints/saved.safetensors" + assert compute_display_name(str(f)) == "checkpoints/saved.safetensors" + + name, tags = get_name_and_tags_from_asset_path(str(f)) + assert name == "saved.safetensors" + assert "output" in tags + assert "models" not in tags + assert not any(tag.startswith("model_type:") for tag in tags) + + def test_unknown_path_returns_none(self): + assert compute_file_path("/some/random/path.png") is None + assert compute_display_name("/some/random/path.png") is None + + class TestResolveDestinationFromTags: def test_explicit_subfolder_is_path_component(self, fake_dirs): base_dir, subdirs = resolve_destination_from_tags( diff --git a/tests-unit/assets_test/test_uploads.py b/tests-unit/assets_test/test_uploads.py index cb7f6cd30..a0bd73527 100644 --- a/tests-unit/assets_test/test_uploads.py +++ b/tests-unit/assets_test/test_uploads.py @@ -45,6 +45,8 @@ def test_upload_ok_duplicate_reference(http: requests.Session, api_base: str, ma assert a2["asset_hash"] == a1["asset_hash"] assert a2["hash"] == a1["hash"] assert a2["id"] != a1["id"] # new reference with same content + assert a2.get("file_path") is None + assert a2.get("display_name") is None # Third upload with the same data but different name also creates new AssetReference files = {"file": (name, data, "application/octet-stream")} @@ -55,6 +57,8 @@ def test_upload_ok_duplicate_reference(http: requests.Session, api_base: str, ma assert a3["asset_hash"] == a1["asset_hash"] assert a3["id"] != a1["id"] assert a3["id"] != a2["id"] + assert a3.get("file_path") is None + assert a3.get("display_name") is None def test_upload_fastpath_from_existing_hash_no_file(http: requests.Session, api_base: str): @@ -88,6 +92,49 @@ def test_upload_fastpath_from_existing_hash_no_file(http: requests.Session, api_ assert "checkpoints" in b2["tags"] assert "uploaded" not in b2["tags"] assert not any(tag.startswith("model_type:") for tag in b2["tags"]) + assert b2.get("file_path") is None + assert b2.get("display_name") is None + + rg = http.get(f"{api_base}/api/assets/{b2['id']}", timeout=120) + detail = rg.json() + assert rg.status_code == 200, detail + assert detail.get("file_path") is None + assert detail.get("display_name") is None + + +def test_create_from_hash_with_model_tags_does_not_synthesize_file_path( + http: requests.Session, api_base: str +): + seed_name = "from_hash_seed.safetensors" + seed_tags = ["models", "model_type:checkpoints", "unit-tests"] + files = {"file": (seed_name, b"D" * 1024, "application/octet-stream")} + form = { + "tags": json.dumps(seed_tags), + "name": seed_name, + "user_metadata": json.dumps({}), + } + seed_r = http.post(api_base + "/api/assets", data=form, files=files, timeout=120) + seed = seed_r.json() + assert seed_r.status_code == 201, seed + + payload = { + "hash": seed["asset_hash"], + "name": "from_hash_copy.safetensors", + "tags": ["models", "model_type:checkpoints", "unit-tests", "spoofed"], + } + created_r = http.post(api_base + "/api/assets/from-hash", json=payload, timeout=120) + created = created_r.json() + assert created_r.status_code == 201, created + assert created["created_new"] is False + assert created["asset_hash"] == seed["asset_hash"] + assert created.get("file_path") is None + assert created.get("display_name") is None + + detail_r = http.get(f"{api_base}/api/assets/{created['id']}", timeout=120) + detail = detail_r.json() + assert detail_r.status_code == 200, detail + assert detail.get("file_path") is None + assert detail.get("display_name") is None def test_upload_fastpath_with_known_hash_and_file( @@ -147,6 +194,8 @@ def test_duplicate_byte_upload_is_reference_only_and_does_not_need_destination( assert "not-a-destination" in duplicate["tags"] assert "uploaded" not in duplicate["tags"] assert "input" not in duplicate["tags"] + assert duplicate.get("file_path") is None + assert duplicate.get("display_name") is None def test_upload_multiple_tags_fields_are_merged(http: requests.Session, api_base: str): @@ -170,6 +219,69 @@ def test_upload_multiple_tags_fields_are_merged(http: requests.Session, api_base assert {"models", "model_type:checkpoints", "unit-tests", "alpha"}.issubset(tags) +@pytest.mark.parametrize( + ("tags", "extension", "subfolder", "expected_prefix", "expected_display_prefix"), + [ + (["input", "unit-tests"], ".png", "uploads", "input", ""), + ( + ["models", "model_type:checkpoints", "unit-tests"], + ".safetensors", + "flux", + "models/checkpoints", + "checkpoints/", + ), + ], +) +def test_upload_response_includes_file_path_and_display_name( + tags: list[str], + extension: str, + subfolder: str, + expected_prefix: str, + expected_display_prefix: str, + http: requests.Session, + api_base: str, + make_asset_bytes, +): + scope = f"response-paths-{uuid.uuid4().hex[:6]}" + scoped_tags = [*tags, scope] + name = f"asset_response_path{extension}" + + files = {"file": (name, make_asset_bytes(name, 1024), "application/octet-stream")} + form = { + "tags": json.dumps(scoped_tags), + "name": name, + "user_metadata": json.dumps({}), + "subfolder": subfolder, + } + created_r = http.post(api_base + "/api/assets", data=form, files=files, timeout=120) + created = created_r.json() + assert created_r.status_code in (200, 201), created + stored_filename = get_asset_filename(created["asset_hash"], extension) + expected_suffix = f"{subfolder}/{stored_filename}" + expected_file_path = f"{expected_prefix}/{expected_suffix}" + expected_display_name = f"{expected_display_prefix}{expected_suffix}" + + assert created["file_path"] == expected_file_path + assert created["display_name"] == expected_display_name + + detail_r = http.get(f"{api_base}/api/assets/{created['id']}", timeout=120) + detail = detail_r.json() + assert detail_r.status_code == 200, detail + assert detail["file_path"] == expected_file_path + assert detail["display_name"] == expected_display_name + + list_r = http.get( + api_base + "/api/assets", + params={"include_tags": f"unit-tests,{scope}", "limit": "50"}, + timeout=120, + ) + listed = list_r.json() + assert list_r.status_code == 200, listed + match = next(a for a in listed["assets"] if a["id"] == created["id"]) + assert match["file_path"] == expected_file_path + assert match["display_name"] == expected_display_name + + @pytest.mark.parametrize("root", ["input", "output"]) def test_concurrent_upload_identical_bytes_different_names( root: str,