From f61ca96b6c17d0a94d6ad2dfee67e21625f01273 Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Fri, 3 Jul 2026 02:16:36 +0200
Subject: [PATCH] fix(downloads): strip emoji from device filename + free disk
 when a download is deleted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two UAT findings:
1. The device download filename (Content-Disposition) kept emoji/symbols from the video title.
   Add storage.display_filename (drops emoji/symbol/control unicode, keeps spaces + accents)
   and use it for the download name — readable and clean ("…alapján!.mp4", no emoji).
2. Deleting/canceling a download removed the job but the shared MediaAsset (and its file) lingered
   as cache, so 'Ready files' stayed inflated and disk wasn't freed. Rework: _release_asset drops
   the hold and, once no job holds the asset, deletes the file + row immediately (the cache only
   needs to span overlapping holders). Also fixes cancel never decrementing (it flipped status to
   'canceled' before releasing, tripping the holding-state guard).

Verified: filename emoji-stripped; enqueue→delete removes the asset row + file from disk.
---
 backend/app/downloads/storage.py | 11 +++++++++++
 backend/app/routes/downloads.py  | 31 ++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/backend/app/downloads/storage.py b/backend/app/downloads/storage.py
index cf94a28..1c4f166 100644
--- a/backend/app/downloads/storage.py
+++ b/backend/app/downloads/storage.py
@@ -63,6 +63,17 @@ def sanitize(name: str, limit: int = _MAX_TITLE) -> str:
     return text or "untitled"
 
 
+def display_filename(name: str) -> str:
+    """User-facing download filename (Content-Disposition): strip emoji/symbols/control +
+    filesystem-illegal chars, but KEEP spaces, accents, and ordinary punctuation — unlike the
+    underscore-joined on-disk name, this is the readable name the user saves to their device."""
+    text = unicodedata.normalize("NFKC", name or "")
+    text = "".join(ch for ch in text if unicodedata.category(ch) not in _DROP_CATEGORIES)
+    text = _ILLEGAL.sub("", text)
+    text = re.sub(r"\s+", " ", text).strip(" .")
+    return text or "download"
+
+
 def rel_path(meta: MediaMeta, ext: str, layout: str = "plex") -> str:
     """Path of the media file relative to DOWNLOAD_ROOT (forward slashes)."""
     title = sanitize(meta.title)
diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py
index 83bb496..a5e3358 100644
--- a/backend/app/routes/downloads.py
+++ b/backend/app/routes/downloads.py
@@ -32,7 +32,6 @@ router = APIRouter(prefix="/api/downloads", tags=["downloads"])
 admin_router = APIRouter(prefix="/api/admin/downloads", tags=["admin-downloads"])
 
 _VIDEO_ID = re.compile(r"^[A-Za-z0-9_-]{11}$")
-_BASENAME_ILLEGAL = re.compile(r'[<>:"/\\|?*\x00-\x1f]')
 
 
 # --- source + serialization ----------------------------------------------------------------
@@ -376,8 +375,8 @@ def cancel_download(
 ) -> dict:
     job = _own_job(db, user, job_id)
     if job.status not in ("done", "canceled"):
+        _release_asset(db, job)  # release while the job still counts as holding
         job.status = "canceled"
-        _decrement_ref(db, job)
         db.commit()
     asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None
     return _serialize(job, asset)
@@ -388,25 +387,35 @@ def delete_download(
     job_id: int, user: User = Depends(require_human), db: Session = Depends(get_db)
 ) -> dict:
     job = _own_job(db, user, job_id)
-    _decrement_ref(db, job)
+    _release_asset(db, job)
     db.delete(job)
     db.commit()
     return {"deleted": job_id}
 
 
-def _decrement_ref(db: Session, job: DownloadJob) -> None:
-    """Drop this job's hold on its asset. The asset itself lingers as cache until its TTL /
-    eviction (so a re-add is still a cache hit); only the reference count changes here."""
-    if job.asset_id and job.status not in ("canceled", "error"):
-        asset = db.get(MediaAsset, job.asset_id)
-        if asset and asset.ref_count > 0:
-            asset.ref_count -= 1
+def _release_asset(db: Session, job: DownloadJob) -> None:
+    """Drop this job's hold on its asset when it leaves a holding state. Once NO job holds the
+    asset anymore, delete the file + row immediately — a deleted download should free its disk,
+    and the shared cache only needs to span *overlapping* holders (a later re-add just downloads
+    again). Idempotent: a job that's already left holding (canceled/error) is a no-op, so delete
+    after cancel doesn't double-release."""
+    if not job.asset_id or job.status not in ("queued", "running", "paused", "done"):
+        return
+    asset = db.get(MediaAsset, job.asset_id)
+    if asset is None:
+        return
+    asset.ref_count = max(0, (asset.ref_count or 0) - 1)
+    if asset.ref_count == 0 and asset.status == "ready":
+        if asset.rel_path:
+            storage.delete_asset_files(settings.download_root, asset.rel_path)
+        db.delete(asset)
 
 
 # --- file download (range-aware, custom display name) --------------------------------------
 
 def _clean_basename(name: str) -> str:
-    return _BASENAME_ILLEGAL.sub("", name).strip() or "download"
+    # Emoji/symbol-free but keeps spaces + accents (a readable device filename).
+    return storage.display_filename(name)
 
 
 def _accessible_job(db: Session, user: User, job_id: int) -> DownloadJob: