diff --git a/backend/app/downloads/links.py b/backend/app/downloads/links.py index 08123c7..6b18cbd 100644 --- a/backend/app/downloads/links.py +++ b/backend/app/downloads/links.py @@ -75,13 +75,7 @@ def public_meta(link: DownloadLink, job, asset, file_url: str, poster_url: str | auto-extracted values, and surfaces the source + any extra reference links as clickable URLs.""" from app.downloads import service - source_url = None - if job is not None and job.job_kind != "edit": - source_url = asset.source_webpage_url or ( - service.source_url(job.source_kind, job.source_ref) - if job.source_kind in ("youtube", "url") - else None - ) + source_url = service.reference_url(job, asset) return { "title": (job and job.display_name) or asset.title, "uploader": (job and job.display_uploader) or asset.uploader, diff --git a/backend/app/downloads/service.py b/backend/app/downloads/service.py index b20304b..fedf8a9 100644 --- a/backend/app/downloads/service.py +++ b/backend/app/downloads/service.py @@ -32,6 +32,20 @@ def source_url(source_kind: str, source_ref: str) -> str: return source_ref +def reference_url(job, asset) -> str | None: + """The clean "downloaded from" URL for a job — shared by the private Downloads page and the public + watch page so both surfaces always agree. None for edit derivatives (a clip's source is the user's + own earlier download, not a web page); else the canonical page URL yt-dlp recorded; else one derived + from source_kind+source_ref (covers queued/older rows before the worker fills it).""" + if job is None or job.job_kind == "edit": + return None + if asset is not None and asset.source_webpage_url: + return asset.source_webpage_url + if job.source_kind in ("youtube", "url"): + return source_url(job.source_kind, job.source_ref) + return None + + def get_or_create_asset( db: Session, source_kind: str, source_ref: str, format_sig: str ) -> MediaAsset: diff --git a/backend/app/downloads/storage.py b/backend/app/downloads/storage.py index 7697c7e..1aee7b8 100644 --- a/backend/app/downloads/storage.py +++ b/backend/app/downloads/storage.py @@ -96,6 +96,17 @@ def edit_rel_path(user_id: int, asset_id: int, ext: str) -> str: return f".edits/{user_id}/clip_{asset_id}.{ext}" +def download_filename(display_name: str, container: str | None, path: Path) -> str: + """The Content-Disposition filename for a served download: the cleaned display name carrying a single + correct extension — the asset container, else the file's own suffix — without doubling it when the + name already ends in that extension. Shared by the private download and the public watch endpoints.""" + ext = container or path.suffix.lstrip(".") + base = display_filename(display_name) + if base.lower().endswith(f".{ext.lower()}"): + base = base[: -(len(ext) + 1)] + return f"{base}.{ext}" + + def abs_path(download_root: str, rel: str) -> Path: return Path(download_root) / rel diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index f2075e0..702f313 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -67,20 +67,6 @@ def resolve_source(raw: str) -> tuple[str, str]: raise HTTPException(status_code=400, detail="That doesn't look like a valid video link.") -def _reference_url(job: DownloadJob, asset: MediaAsset | None) -> str | None: - """The clean "downloaded from" URL for the Downloads page: the canonical page URL yt-dlp - recorded, else one derived from source_kind+source_ref (covers queued/older rows before the - worker fills it). Returns None for edit derivatives — a clip's source is the user's own earlier - download, not a web page, so there's no meaningful external URL to show.""" - if job.job_kind == "edit": - return None - if asset is not None and asset.source_webpage_url: - return asset.source_webpage_url - if job.source_kind in ("youtube", "url"): - return service.source_url(job.source_kind, job.source_ref) - return None - - def _serialize(job: DownloadJob, asset: MediaAsset | None) -> dict: ready = asset is not None and asset.status == "ready" and bool(asset.rel_path) return { @@ -95,7 +81,7 @@ def _serialize(job: DownloadJob, asset: MediaAsset | None) -> dict: "created_at": job.created_at.isoformat() if job.created_at else None, "source_kind": job.source_kind, "source_ref": job.source_ref, - "source_url": _reference_url(job, asset), + "source_url": service.reference_url(job, asset), # A locally-generated poster to fall back to when the source had no thumbnail. "poster_url": f"/api/downloads/{job.id}/poster.jpg" if (asset is not None and asset.poster_path) @@ -139,6 +125,13 @@ def _assets_for(db: Session, jobs: list[DownloadJob]) -> dict[int, MediaAsset]: return {a.id: a for a in rows} +def _serialize_job(db: Session, job: DownloadJob) -> dict: + """Resolve a job's cache asset (if any) and serialize the pair — the resolve-then-serialize step + every single-job handler shares.""" + asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None + return _serialize(job, asset) + + def _own_job(db: Session, user: User, job_id: int) -> DownloadJob: job = db.get(DownloadJob, job_id) if job is None or job.user_id != user.id: @@ -272,8 +265,7 @@ def enqueue_download( ) except quota.QuotaExceeded as e: raise HTTPException(status_code=422, detail=_quota_message(e)) - asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None - return _serialize(job, asset) + return _serialize_job(db, job) @router.post("/edit") @@ -298,8 +290,7 @@ def enqueue_edit( raise HTTPException(status_code=422, detail=_quota_message(e)) except service.EditError as e: raise HTTPException(status_code=400, detail=_edit_message(e)) - asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None - return _serialize(job, asset) + return _serialize_job(db, job) def _edit_message(e: service.EditError) -> str: @@ -429,8 +420,7 @@ def update_download_meta( cleaned = [u for u in (_clean_url(x) for x in raw) if u][:_MAX_EXTRA_LINKS] job.extra_links = cleaned or None db.commit() - asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None - return _serialize(job, asset) + return _serialize_job(db, job) def _set_status(db: Session, job: DownloadJob, status: str) -> None: @@ -445,8 +435,7 @@ def pause_download( job = _own_job(db, user, job_id) if job.status in ("queued", "running"): _set_status(db, job, "paused") # a running worker aborts cooperatively via the hook - asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None - return _serialize(job, asset) + return _serialize_job(db, job) @router.post("/{job_id}/resume") @@ -466,8 +455,7 @@ def resume_download( asset.status = "pending" asset.error = None _set_status(db, job, "queued") - asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None - return _serialize(job, asset) + return _serialize_job(db, job) @router.post("/{job_id}/cancel") @@ -479,8 +467,7 @@ def cancel_download( _release_asset(db, job) # release while the job still counts as holding job.status = "canceled" db.commit() - asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None - return _serialize(job, asset) + return _serialize_job(db, job) @router.delete("/{job_id}") @@ -514,11 +501,6 @@ def _release_asset(db: Session, job: DownloadJob) -> None: # --- file download (range-aware, custom display name) -------------------------------------- -def _clean_basename(name: str) -> str: - # 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: job = db.get(DownloadJob, job_id) if job is None: @@ -550,11 +532,9 @@ def download_file( if path is None: raise HTTPException(status_code=404, detail="File is no longer available.") - ext = asset.container or path.suffix.lstrip(".") - base = _clean_basename(job.display_name or asset.title or asset.source_ref) - if base.lower().endswith(f".{ext.lower()}"): - base = base[: -(len(ext) + 1)] - filename = f"{base}.{ext}" + filename = storage.download_filename( + job.display_name or asset.title or asset.source_ref, asset.container, path + ) asset.last_access_at = datetime.now(timezone.utc) db.commit() diff --git a/backend/app/routes/public.py b/backend/app/routes/public.py index 53952a6..7c5b9ea 100644 --- a/backend/app/routes/public.py +++ b/backend/app/routes/public.py @@ -118,11 +118,7 @@ def watch_file(token: str, g: str | None = None, db: Session = Depends(get_db)): if path is None: raise HTTPException(status_code=404, detail="This video is no longer available.") - ext = asset.container or path.suffix.lstrip(".") - base = storage.display_filename(asset.title or "video") - if base.lower().endswith(f".{ext.lower()}"): - base = base[: -(len(ext) + 1)] - filename = f"{base}.{ext}" + filename = storage.download_filename(asset.title or "video", asset.container, path) disposition = "attachment" if link.allow_download else "inline" # Starlette's FileResponse honours the Range header (206) for seeking/scrubbing. return FileResponse(path, filename=filename, content_disposition_type=disposition)