Derive asset preview URLs from the content endpoint

preview_url was assembled from a /api/view link whose type was chosen by
matching the asset's tags against "input" then "output". Anything written
anywhere else - temp above all, where preview nodes put their images - fell
off the end of that chain and came back with no preview at all. Tags are
user-editable, so removing one also silently destroyed the URL.

Point the preview at the asset's own content endpoint instead, which
resolves a reference wherever its file lives. Model weights and other
content a browser cannot render get no preview URL, since pointing one at
their bytes would only make the client download the whole file. Which
content is previewable is resolved the same way /content resolves the type
it serves - the stored mime type, falling back to the reference name - so a
scan that recorded no mime type does not cost an image its preview.

Author: synap5e

app/assets/api/routes.py

@@ -2,6 +2,7 @@
 import functools
 import json
 import logging
+import mimetypes
 import os
 import urllib.parse
 import uuid
@@ -207,42 +208,32 @@ def _validate_sort_field(requested: str | None) -> str:
     return "created_at"
 
 
-def _build_preview_url_from_view(tags: list[str], user_metadata: dict[str, Any] | None) -> str | None:
-    """Build a /api/view preview URL from asset tags and user_metadata filename."""
-    if not user_metadata:
-        return None
-    filename = user_metadata.get("filename")
-    if not filename:
-        return None
+# Anything else has no visual form: a preview of its own bytes would just make the client download it all.
+PREVIEWABLE_MIME_PREFIXES = ("image/", "video/", "audio/")
 
-    if "input" in tags:
-        view_type = "input"
-    elif "output" in tags:
-        view_type = "output"
-    else:
-        return None
 
-    subfolder = ""
-    if "/" in filename:
-        subfolder, filename = filename.rsplit("/", 1)
+def _has_previewable_content(asset: schemas.AssetData | None, name: str | None) -> bool:
+    if asset is None:
+        return False
+    # A scan run without metadata extraction leaves mime NULL; resolve from the name as /content does.
+    raw = asset.mime_type or mimetypes.guess_type(name or "")[0] or ""
+    return raw.split(";", 1)[0].strip().lower().startswith(PREVIEWABLE_MIME_PREFIXES)
+
 
-    encoded_filename = urllib.parse.quote(filename, safe="")
-    url = f"/api/view?type={view_type}&filename={encoded_filename}"
-    if subfolder:
-        url += f"&subfolder={urllib.parse.quote(subfolder, safe='')}"
-    return url
+def _build_preview_url(reference_id: str) -> str:
+    # Asking for inline does not weaken /content: it still forces dangerous types to download.
+    return f"/api/assets/{reference_id}/content?disposition=inline"
 
 
 def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResult) -> schemas_out.Asset:
     """Build an Asset response from a service result."""
     if result.ref.preview_id:
-        preview_detail = get_asset_detail(result.ref.preview_id)
-        if preview_detail:
-            preview_url = _build_preview_url_from_view(preview_detail.tags, preview_detail.ref.user_metadata)
-        else:
-            preview_url = None
+        # A nominated preview is one whatever it holds, so no media check here.
+        preview_url = _build_preview_url(result.ref.preview_id)
+    elif _has_previewable_content(result.asset, result.ref.name):
+        preview_url = _build_preview_url(result.ref.id)
     else:
-        preview_url = _build_preview_url_from_view(result.tags, result.ref.user_metadata)
+        preview_url = None
     if result.ref.file_path:
         display_name = compute_display_name(result.ref.file_path)
         # In-root loader path (model category dropped): what model loaders consume.

tests-unit/assets_test/services/test_asset_response_preview_url.py

@@ -0,0 +1,148 @@
+from datetime import datetime
+
+import pytest
+
+from app.assets.api.routes import _build_asset_response
+from app.assets.services.schemas import AssetData, AssetDetailResult, ReferenceData
+
+_TS = datetime(2024, 1, 1, 0, 0, 0)
+
+
+def _make_result(
+    *,
+    ref_id: str = "ref-1",
+    name: str = "ComfyUI_temp_abcde_00001_.png",
+    file_path: str | None = "/base/temp/ComfyUI_temp_abcde_00001_.png",
+    mime_type: str | None = "image/png",
+    preview_id: str | None = None,
+    tags: list[str] | None = None,
+    user_metadata: dict | None = None,
+    with_asset: bool = True,
+) -> AssetDetailResult:
+    ref = ReferenceData(
+        id=ref_id,
+        name=name,
+        file_path=file_path,
+        loader_path=None,
+        user_metadata=user_metadata,
+        preview_id=preview_id,
+        created_at=_TS,
+        updated_at=_TS,
+        last_access_time=_TS,
+    )
+    asset = (
+        AssetData(hash="blake3:abc", size_bytes=1024, mime_type=mime_type)
+        if with_asset
+        else None
+    )
+    return AssetDetailResult(ref=ref, asset=asset, tags=tags or [])
+
+
+def test_temp_image_gets_a_preview_url():
+    resp = _build_asset_response(_make_result())
+
+    assert resp.preview_url == "/api/assets/ref-1/content?disposition=inline", (
+        "a reference outside input and output must still resolve a preview"
+    )
+
+
+@pytest.mark.parametrize(
+    "tags",
+    [[], ["input"], ["output"], ["models", "model_type:checkpoints"]],
+)
+def test_preview_url_does_not_depend_on_tags(tags: list[str]):
+    resp = _build_asset_response(_make_result(tags=tags))
+
+    assert resp.preview_url == "/api/assets/ref-1/content?disposition=inline", (
+        "tags are user-editable; removing one must not destroy the preview"
+    )
+
+
+def test_preview_url_does_not_depend_on_the_metadata_filename():
+    resp = _build_asset_response(_make_result(user_metadata=None))
+
+    assert resp.preview_url == "/api/assets/ref-1/content?disposition=inline", (
+        "a reference carrying no metadata filename must still get a preview"
+    )
+
+
+def test_preview_id_wins_over_the_asset_itself():
+    result = _make_result(preview_id="preview-ref")
+
+    resp = _build_asset_response(result)
+
+    assert resp.preview_url == "/api/assets/preview-ref/content?disposition=inline"
+    assert resp.preview_id == "preview-ref"
+
+
+def test_preview_id_is_used_whatever_the_asset_content_is():
+    result = _make_result(
+        mime_type="application/safetensors", preview_id="preview-ref"
+    )
+
+    resp = _build_asset_response(result)
+
+    assert resp.preview_url == "/api/assets/preview-ref/content?disposition=inline", (
+        "a nominated preview stands in for content with no visual form"
+    )
+
+
+@pytest.mark.parametrize(
+    "mime_type",
+    ["application/safetensors", "application/gguf", "application/octet-stream"],
+)
+def test_no_preview_url_for_content_a_browser_cannot_render(mime_type: str):
+    resp = _build_asset_response(
+        _make_result(name="model.safetensors", mime_type=mime_type)
+    )
+
+    assert resp.preview_url is None, (
+        "content a browser cannot render must not advertise its own bytes as a preview"
+    )
+
+
+@pytest.mark.parametrize(
+    ("name", "expected"),
+    [
+        ("shot.png", "/api/assets/ref-1/content?disposition=inline"),
+        ("clip.mp4", "/api/assets/ref-1/content?disposition=inline"),
+        ("model.safetensors", None),
+    ],
+)
+def test_missing_mime_type_falls_back_to_the_name(name: str, expected: str | None):
+    resp = _build_asset_response(_make_result(name=name, mime_type=None))
+
+    assert resp.preview_url == expected, (
+        "a previewable file must not lose its preview just because the scan "
+        "that found it recorded no mime type"
+    )
+
+
+def test_unrecognised_name_and_no_mime_yields_no_preview():
+    resp = _build_asset_response(_make_result(name="blob", mime_type=None))
+
+    assert resp.preview_url is None, (
+        "nothing identifies this as media, so do not advertise a preview"
+    )
+
+
+def test_mime_type_parameters_do_not_defeat_the_media_check():
+    resp = _build_asset_response(_make_result(mime_type="IMAGE/PNG; charset=binary"))
+
+    assert resp.preview_url == "/api/assets/ref-1/content?disposition=inline"
+
+
+def test_no_preview_url_without_content():
+    resp = _build_asset_response(_make_result(with_asset=False))
+
+    assert resp.preview_url is None, (
+        "no asset row means the content endpoint has nothing to serve"
+    )
+
+
+def test_reference_without_file_path_still_gets_a_preview_url():
+    resp = _build_asset_response(_make_result(file_path=None))
+
+    assert resp.preview_url == "/api/assets/ref-1/content?disposition=inline", (
+        "a NULL file_path still resolves through a sibling reference"
+    )

tests-unit/assets_test/test_preview_url.py

@@ -0,0 +1,113 @@
+import contextlib
+import json
+import uuid
+
+import requests
+
+
+def test_preview_url_outside_input_and_output(
+    http: requests.Session, api_base: str, asset_factory, make_asset_bytes
+):
+    scope = f"preview-url-{uuid.uuid4().hex[:6]}"
+    name = f"{scope}.png"
+    data = make_asset_bytes(name, 2048)
+
+    body = asset_factory(
+        name, ["models", "model_type:checkpoints", "unit-tests", scope], {}, data
+    )
+
+    assert body["preview_url"] == f"/api/assets/{body['id']}/content?disposition=inline", (
+        "an image in a directory /api/view never covered must still get a preview"
+    )
+
+    r = http.get(api_base + body["preview_url"], timeout=120)
+    assert r.status_code == 200, r.text
+    assert r.content == data, "the preview URL must serve the asset's own bytes"
+    assert "inline" in r.headers.get("Content-Disposition", "")
+    assert r.headers.get("Content-Type", "").startswith("image/png")
+
+
+def test_preview_url_survives_tag_removal(
+    http: requests.Session, api_base: str, asset_factory, make_asset_bytes
+):
+    scope = f"preview-tags-{uuid.uuid4().hex[:6]}"
+    name = f"{scope}.png"
+    data = make_asset_bytes(name, 2048)
+
+    body = asset_factory(name, ["input", "unit-tests", scope], {}, data)
+    aid = body["id"]
+    preview_url = body["preview_url"]
+    assert preview_url, "an uploaded image starts out with a preview"
+
+    r = http.delete(
+        f"{api_base}/api/assets/{aid}/tags", json={"tags": ["input"]}, timeout=120
+    )
+    assert r.status_code == 200, r.text
+
+    after = http.get(f"{api_base}/api/assets/{aid}", timeout=120).json()
+    assert "input" not in after["tags"]
+    assert after["preview_url"] == preview_url, (
+        "dropping the tag that used to select the view type must not take the "
+        "preview with it"
+    )
+    assert http.get(api_base + preview_url, timeout=120).status_code == 200
+
+
+def test_preview_url_is_the_nominated_preview_when_one_is_set(
+    http: requests.Session, api_base: str, asset_factory, make_asset_bytes
+):
+    scope = f"preview-id-{uuid.uuid4().hex[:6]}"
+    thumb_name = f"{scope}_thumb.png"
+    thumb_data = make_asset_bytes(thumb_name, 1024)
+    thumb = asset_factory(thumb_name, ["input", "unit-tests", scope], {}, thumb_data)
+
+    model_name = f"{scope}.safetensors"
+    model_data = make_asset_bytes(model_name, 2048)
+    files = {"file": (model_name, model_data, "application/octet-stream")}
+    form_data = {
+        "tags": json.dumps(["models", "model_type:checkpoints", "unit-tests", scope]),
+        "name": model_name,
+        "preview_id": thumb["id"],
+    }
+    r = http.post(api_base + "/api/assets", files=files, data=form_data, timeout=120)
+    model = r.json()
+    assert r.status_code in (200, 201), model
+
+    try:
+        assert model["preview_id"] == thumb["id"]
+        assert (
+            model["preview_url"]
+            == f"/api/assets/{thumb['id']}/content?disposition=inline"
+        ), "a nominated preview stands in for content with no visual form"
+        got = http.get(api_base + model["preview_url"], timeout=120)
+        assert got.status_code == 200, got.text
+        assert got.content == thumb_data
+    finally:
+        with contextlib.suppress(Exception):
+            http.delete(f"{api_base}/api/assets/{model['id']}", timeout=30)
+
+
+def test_no_preview_url_for_content_a_browser_cannot_render(
+    http: requests.Session, api_base: str, asset_factory, make_asset_bytes
+):
+    scope = f"preview-model-{uuid.uuid4().hex[:6]}"
+    name = f"{scope}.safetensors"
+
+    body = asset_factory(
+        name,
+        ["models", "model_type:checkpoints", "unit-tests", scope],
+        {},
+        make_asset_bytes(name, 2048),
+    )
+
+    assert body.get("preview_url") is None, (
+        "model weights have no preview; advertising one would only make the "
+        "client download the whole file to discover that"
+    )
+
+    listed = http.get(
+        api_base + "/api/assets", params={"include_tags": scope}, timeout=120
+    ).json()["assets"]
+    assert [a.get("preview_url") for a in listed] == [None], (
+        "the list route must withhold it too, not just the detail route"
+    )