Skip to content

Commit ee82765

Browse files
committed
Derive asset preview URLs from the file path
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. Derive the URL from where the file actually sits instead. That covers every root /api/view serves, temp included, and no longer depends on tags or on a filename in user_metadata. A file outside those roots, or content no client can render from its own bytes, gets no preview URL rather than one that cannot work. Nominated previews are resolved a page at a time rather than per row, so a list costs one extra query however long it is. A preview that is soft-deleted or not visible to the caller drops out of that lookup and is no longer advertised.
1 parent bd34f33 commit ee82765

8 files changed

Lines changed: 616 additions & 35 deletions

File tree

app/assets/api/routes.py

Lines changed: 56 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import functools
33
import json
44
import logging
5+
import mimetypes
56
import os
67
import urllib.parse
78
import uuid
@@ -32,6 +33,7 @@
3233
create_from_hash,
3334
delete_asset_reference,
3435
get_asset_detail,
36+
get_preview_file_paths,
3537
list_assets_page,
3638
list_tags,
3739
remove_tags,
@@ -40,7 +42,7 @@
4042
upload_from_temp_path,
4143
)
4244
from app.assets.services.cursor import InvalidCursorError
43-
from app.assets.services.path_utils import compute_display_name
45+
from app.assets.services.path_utils import compute_asset_response_paths
4446
from app.assets.services.tagging import list_tag_histogram
4547

4648
ROUTES = web.RouteTableDef()
@@ -207,44 +209,63 @@ def _validate_sort_field(requested: str | None) -> str:
207209
return "created_at"
208210

209211

210-
def _build_preview_url_from_view(tags: list[str], user_metadata: dict[str, Any] | None) -> str | None:
211-
"""Build a /api/view preview URL from asset tags and user_metadata filename."""
212-
if not user_metadata:
212+
# What a client can render from the bytes themselves; anything else needs a nominated preview.
213+
PREVIEWABLE_MIME_PREFIXES = ("image/", "video/", "audio/", "text/")
214+
215+
# models is deliberately absent: /api/view has no directory type for it.
216+
VIEWABLE_NAMESPACES = frozenset({"input", "output", "temp"})
217+
218+
219+
def _has_previewable_content(asset: schemas.AssetData | None, file_path: str | None) -> bool:
220+
if asset is None:
221+
return False
222+
# Resolved from the path, not the caller-editable name, so a rename cannot change what previews.
223+
raw = asset.mime_type or mimetypes.guess_type(file_path or "")[0] or ""
224+
return raw.split(";", 1)[0].strip().lower().startswith(PREVIEWABLE_MIME_PREFIXES)
225+
226+
227+
def _build_preview_url(file_path: str | None) -> str | None:
228+
# /api/view is a FileResponse: byte-range seeking, no user header, no access write.
229+
if not file_path:
213230
return None
214-
filename = user_metadata.get("filename")
215-
if not filename:
231+
paths = compute_asset_response_paths(file_path)
232+
if not paths:
216233
return None
217-
218-
if "input" in tags:
219-
view_type = "input"
220-
elif "output" in tags:
221-
view_type = "output"
222-
else:
234+
logical_path, relative_path = paths
235+
namespace = logical_path.split("/", 1)[0]
236+
if namespace not in VIEWABLE_NAMESPACES or not relative_path:
223237
return None
224238

225-
subfolder = ""
226-
if "/" in filename:
227-
subfolder, filename = filename.rsplit("/", 1)
228-
229-
encoded_filename = urllib.parse.quote(filename, safe="")
230-
url = f"/api/view?type={view_type}&filename={encoded_filename}"
239+
subfolder, _, filename = relative_path.rpartition("/")
240+
url = f"/api/view?type={namespace}&filename={urllib.parse.quote(filename, safe='')}"
231241
if subfolder:
232242
url += f"&subfolder={urllib.parse.quote(subfolder, safe='')}"
233243
return url
234244

235245

236-
def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResult) -> schemas_out.Asset:
237-
"""Build an Asset response from a service result."""
246+
def _resolve_preview_paths(
247+
results: "list[schemas.AssetDetailResult] | list[schemas.AssetSummaryData]",
248+
owner_id: str,
249+
) -> dict[str, str]:
250+
# A miss means no live, owner-visible preview - that is what keeps a soft-deleted one quiet.
251+
preview_ids = {r.ref.preview_id for r in results if r.ref.preview_id}
252+
return get_preview_file_paths(sorted(preview_ids), owner_id=owner_id)
253+
254+
255+
def _build_asset_response(
256+
result: schemas.AssetDetailResult | schemas.UploadResult,
257+
preview_paths: dict[str, str],
258+
) -> schemas_out.Asset:
238259
if result.ref.preview_id:
239-
preview_detail = get_asset_detail(result.ref.preview_id)
240-
if preview_detail:
241-
preview_url = _build_preview_url_from_view(preview_detail.tags, preview_detail.ref.user_metadata)
242-
else:
243-
preview_url = None
260+
# A nominated preview is one whatever it holds, so no media check here.
261+
preview_url = _build_preview_url(preview_paths.get(result.ref.preview_id))
262+
elif _has_previewable_content(result.asset, result.ref.file_path):
263+
preview_url = _build_preview_url(result.ref.file_path)
244264
else:
245-
preview_url = _build_preview_url_from_view(result.tags, result.ref.user_metadata)
265+
preview_url = None
246266
if result.ref.file_path:
247-
display_name = compute_display_name(result.ref.file_path)
267+
paths = compute_asset_response_paths(result.ref.file_path)
268+
display_name = paths[1] if paths else None
248269
# In-root loader path (model category dropped): what model loaders consume.
249270
loader_path = result.ref.loader_path
250271
else:
@@ -324,7 +345,10 @@ async def list_assets_route(request: web.Request) -> web.Response:
324345
except InvalidCursorError as e:
325346
return _build_error_response(400, "INVALID_CURSOR", str(e))
326347

327-
summaries = [_build_asset_response(item) for item in result.items]
348+
preview_paths = _resolve_preview_paths(
349+
result.items, USER_MANAGER.get_request_user_id(request)
350+
)
351+
summaries = [_build_asset_response(item, preview_paths) for item in result.items]
328352

329353
# has_more semantics differ by mode:
330354
# - cursor mode: a non-empty next_cursor means there are more results.
@@ -363,7 +387,7 @@ async def get_asset_route(request: web.Request) -> web.Response:
363387
{"id": reference_id},
364388
)
365389

366-
payload = _build_asset_response(result)
390+
payload = _build_asset_response(result, _resolve_preview_paths([result], USER_MANAGER.get_request_user_id(request)))
367391
except ValueError as e:
368392
return _build_error_response(
369393
404, "ASSET_NOT_FOUND", str(e), {"id": reference_id}
@@ -494,7 +518,7 @@ async def create_asset_from_hash_route(request: web.Request) -> web.Response:
494518
404, "ASSET_NOT_FOUND", f"Asset content {body.hash} does not exist"
495519
)
496520

497-
asset = _build_asset_response(result)
521+
asset = _build_asset_response(result, _resolve_preview_paths([result], USER_MANAGER.get_request_user_id(request)))
498522
payload_out = schemas_out.AssetCreated(
499523
**asset.model_dump(),
500524
created_new=result.created_new,
@@ -585,7 +609,7 @@ async def upload_asset(request: web.Request) -> web.Response:
585609
logging.exception("upload_asset failed for owner_id=%s", owner_id)
586610
return _build_error_response(500, "INTERNAL", "Unexpected server error.")
587611

588-
asset = _build_asset_response(result)
612+
asset = _build_asset_response(result, _resolve_preview_paths([result], owner_id))
589613
payload_out = schemas_out.AssetCreated(
590614
**asset.model_dump(),
591615
created_new=result.created_new,
@@ -615,7 +639,7 @@ async def update_asset_route(request: web.Request) -> web.Response:
615639
owner_id=USER_MANAGER.get_request_user_id(request),
616640
preview_id=body.preview_id,
617641
)
618-
payload = _build_asset_response(result)
642+
payload = _build_asset_response(result, _resolve_preview_paths([result], USER_MANAGER.get_request_user_id(request)))
619643
except PermissionError as pe:
620644
return _build_error_response(403, "FORBIDDEN", str(pe), {"id": reference_id})
621645
except ValueError as ve:

app/assets/database/queries/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
get_reference_by_id,
2929
get_reference_with_owner_check,
3030
get_reference_ids_by_ids,
31+
get_reference_paths_by_ids,
3132
get_references_by_paths_and_asset_ids,
3233
get_references_for_prefixes,
3334
get_unenriched_references,
@@ -101,6 +102,7 @@
101102
"get_reference_by_id",
102103
"get_reference_with_owner_check",
103104
"get_reference_ids_by_ids",
105+
"get_reference_paths_by_ids",
104106
"get_reference_tags",
105107
"get_references_by_paths_and_asset_ids",
106108
"get_references_for_prefixes",

app/assets/database/queries/asset_reference.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,6 +1064,29 @@ def get_references_by_paths_and_asset_ids(
10641064
return winners
10651065

10661066

1067+
def get_reference_paths_by_ids(
1068+
session: Session,
1069+
reference_ids: list[str],
1070+
owner_id: str = "",
1071+
) -> dict[str, str]:
1072+
"""Map reference id -> file_path for live, owner-visible, file-backed references."""
1073+
if not reference_ids:
1074+
return {}
1075+
1076+
paths: dict[str, str] = {}
1077+
for chunk in iter_chunks(reference_ids, MAX_BIND_PARAMS):
1078+
rows = session.execute(
1079+
select(AssetReference.id, AssetReference.file_path).where(
1080+
AssetReference.id.in_(chunk),
1081+
AssetReference.file_path.is_not(None),
1082+
AssetReference.deleted_at.is_(None),
1083+
build_visible_owner_clause(owner_id),
1084+
)
1085+
)
1086+
paths.update({rid: fp for rid, fp in rows})
1087+
return paths
1088+
1089+
10671090
def get_reference_ids_by_ids(
10681091
session: Session,
10691092
reference_ids: list[str],

app/assets/services/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
get_asset_by_hash,
55
get_asset_detail,
66
list_assets_page,
7+
get_preview_file_paths,
78
resolve_asset_for_download,
89
set_asset_preview,
910
update_asset_metadata,
@@ -83,6 +84,7 @@
8384
"list_tags",
8485
"cleanup_unreferenced_assets",
8586
"remove_tags",
87+
"get_preview_file_paths",
8688
"resolve_asset_for_download",
8789
"set_asset_preview",
8890
"update_asset_metadata",

app/assets/services/asset_management.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
reference_exists_for_asset_id,
2222
delete_reference_by_id,
2323
fetch_reference_and_asset,
24+
get_reference_paths_by_ids,
2425
soft_delete_reference_by_id,
2526
fetch_reference_asset_and_tags,
2627
get_asset_by_hash as queries_get_asset_by_hash,
@@ -424,6 +425,19 @@ def resolve_hash_to_path(
424425
)
425426

426427

428+
def get_preview_file_paths(
429+
preview_ids: list[str],
430+
owner_id: str = "",
431+
) -> dict[str, str]:
432+
"""Map preview reference id -> file_path, in one query for the whole page."""
433+
if not preview_ids:
434+
return {}
435+
with create_session() as session:
436+
return get_reference_paths_by_ids(
437+
session, reference_ids=preview_ids, owner_id=owner_id
438+
)
439+
440+
427441
def resolve_asset_for_download(
428442
reference_id: str,
429443
owner_id: str = "",

tests-unit/assets_test/services/test_asset_response_loader_path.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def test_uses_persisted_loader_path_without_recomputing():
4343
loader_path="SENTINEL/stored.safetensors",
4444
)
4545

46-
resp = _build_asset_response(result)
46+
resp = _build_asset_response(result, {})
4747

4848
assert resp.loader_path == "SENTINEL/stored.safetensors"
4949

@@ -67,7 +67,7 @@ def test_null_stored_loader_path_is_served_as_null(tmp_path: Path):
6767
mock_fp.models_dir = str(models)
6868

6969
result = _make_result(file_path=str(f), loader_path=None)
70-
resp = _build_asset_response(result)
70+
resp = _build_asset_response(result, {})
7171

7272
assert resp.loader_path is None
7373
assert resp.display_name == "checkpoints/bar.safetensors"
@@ -77,7 +77,7 @@ def test_all_path_fields_null_without_file_path():
7777
"""API-created / hash-only references (no file_path) expose no paths."""
7878
result = _make_result(file_path=None, loader_path=None)
7979

80-
resp = _build_asset_response(result)
80+
resp = _build_asset_response(result, {})
8181

8282
assert resp.loader_path is None
8383
assert resp.display_name is None

0 commit comments

Comments
 (0)