diff --git a/backend/app/downloads/formats.py b/backend/app/downloads/formats.py index 8d79266..28b7f45 100644 --- a/backend/app/downloads/formats.py +++ b/backend/app/downloads/formats.py @@ -85,14 +85,6 @@ def format_sig(spec: dict) -> str: return hashlib.sha256(payload.encode()).hexdigest()[:24] -def target_ext(spec: dict) -> str: - """Best-effort final extension for the produced file (also used to name the staging output).""" - s = normalize(spec) - if s["mode"] == "a": - return s["audio_format"] or "m4a" - return s["container"] or "mp4" - - def build_ydl_opts( spec: dict, outtmpl: str, 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 9436c14..e2bf16f 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 d15bb3a..1aee7b8 100644 --- a/backend/app/downloads/storage.py +++ b/backend/app/downloads/storage.py @@ -96,10 +96,32 @@ 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 +def safe_abs_path(download_root: str, rel: str) -> Path | None: + """The resolved absolute path for `rel`, but ONLY if it stays inside DOWNLOAD_ROOT and exists on + disk — otherwise None. Centralizes the path-traversal + existence guard every file-serving endpoint + shares, so the containment check lives in exactly one place.""" + root = Path(download_root).resolve() + path = abs_path(download_root, rel).resolve() + if root not in path.parents or not path.exists(): + return None + return path + + def place_file(staging_file: Path, download_root: str, rel: str) -> None: """Move a completed staging file into its final location under DOWNLOAD_ROOT.""" dest = abs_path(download_root, rel) diff --git a/backend/app/models.py b/backend/app/models.py index 5e0c915..d65ee87 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -232,6 +232,11 @@ class Subscription(Base, TimestampMixin): channel: Mapped["Channel"] = relationship(back_populates="subscriptions") +# Video.live_status values that mean "currently a live/upcoming broadcast" — transient states +# hidden from feeds/search and revisited by the refresh job until they settle into a VOD. +LIVE_OR_UPCOMING = ("live", "upcoming") + + class Video(Base, TimestampMixin): """A YouTube video. Shared across users; per-user state lives elsewhere.""" diff --git a/backend/app/routes/channels.py b/backend/app/routes/channels.py index 4c47b71..adfa4b8 100644 --- a/backend/app/routes/channels.py +++ b/backend/app/routes/channels.py @@ -5,13 +5,14 @@ import logging from datetime import datetime, timezone from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import and_, func, select, update +from sqlalchemy import func, select, update from sqlalchemy.orm import Session from app import quota, sysconfig from app.auth import current_user, has_write_scope, require_human from app.db import get_db from app.models import ( + LIVE_OR_UPCOMING, BlockedChannel, Channel, ChannelTag, @@ -59,7 +60,7 @@ def list_channels( func.max(Video.published_at), func.coalesce(func.sum(Video.duration_seconds), 0), func.count(Video.id).filter(Video.is_short.is_(True)), - func.count(Video.id).filter(Video.live_status.in_(("live", "upcoming"))), + func.count(Video.id).filter(Video.live_status.in_(LIVE_OR_UPCOMING)), ) .where(Video.channel_id.in_(channel_ids)) .group_by(Video.channel_id) diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index 066249c..28a8bb6 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -7,7 +7,6 @@ serves finished files (range-aware, with the user's custom display name), and sh """ import re from datetime import datetime, timedelta, timezone -from pathlib import Path from urllib.parse import parse_qs, urlparse from fastapi import APIRouter, Depends, HTTPException, Request @@ -69,20 +68,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 { @@ -97,7 +82,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) @@ -141,6 +126,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: @@ -274,8 +266,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") @@ -300,8 +291,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: @@ -431,8 +421,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: @@ -447,8 +436,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") @@ -468,8 +456,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") @@ -481,8 +468,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}") @@ -525,11 +511,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: @@ -557,16 +538,13 @@ def download_file( asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None if asset is None or asset.status != "ready" or not asset.rel_path: raise HTTPException(status_code=409, detail="This download isn't ready.") - path = storage.abs_path(settings.download_root, asset.rel_path).resolve() - root = Path(settings.download_root).resolve() - if root not in path.parents or not path.exists(): + path = storage.safe_abs_path(settings.download_root, asset.rel_path) + 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() @@ -611,9 +589,8 @@ def poster_image( asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None if asset is None or not asset.poster_path: raise HTTPException(status_code=404, detail="No poster.") - path = storage.abs_path(settings.download_root, asset.poster_path).resolve() - root = Path(settings.download_root).resolve() - if root not in path.parents or not path.exists(): + path = storage.safe_abs_path(settings.download_root, asset.poster_path) + if path is None: raise HTTPException(status_code=404, detail="No poster.") return FileResponse(path, media_type="image/jpeg") @@ -626,9 +603,8 @@ def storyboard_image( asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None if asset is None or not asset.storyboard_path: raise HTTPException(status_code=404, detail="No filmstrip.") - path = storage.abs_path(settings.download_root, asset.storyboard_path).resolve() - root = Path(settings.download_root).resolve() - if root not in path.parents or not path.exists(): + path = storage.safe_abs_path(settings.download_root, asset.storyboard_path) + if path is None: raise HTTPException(status_code=404, detail="No filmstrip.") return FileResponse(path, media_type="image/jpeg") @@ -685,7 +661,7 @@ def unshare_download( user: User = Depends(require_human), db: Session = Depends(get_db), ) -> dict: - job = _own_job(db, user, job_id) + _own_job(db, user, job_id) # ownership guard (404s if not the caller's job) recipient = db.execute( select(User).where(func.lower(User.email) == email.strip().lower()) ).scalar_one_or_none() diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 7b76021..54f6b49 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -14,6 +14,7 @@ from app import quota from app.auth import current_user from app.db import get_db from app.models import ( + LIVE_OR_UPCOMING, BlockedChannel, Channel, ChannelTag, @@ -34,7 +35,6 @@ router = APIRouter(prefix="/api", tags=["feed"]) # "saved" used to be a status; it's now membership in the built-in Watch later playlist. VALID_STATES = {"new", "watched", "hidden"} -HIDDEN_LIVE = ("live", "upcoming") # Resume-position thresholds (mirror the client): positions below this are "didn't # really start" and within this of the end are "basically finished" — neither is worth @@ -132,7 +132,7 @@ def _filtered_query( exclude_tag_category: str | None = None, ) -> tuple[Select, object]: """Build the feed query (joins + all WHERE filters), shared by /feed and /feed/count. - Returns the column-bearing select plus the watch-status expression for sorting. + Returns the column-bearing select plus the priority/relevance rank expression for sorting. `scope="my"` (default) restricts the feed to the user's own non-hidden subscriptions. `scope="all"` shows every video in the shared catalog (any user's ingested channels); @@ -313,12 +313,12 @@ def _filtered_query( type_clauses = [] if show_normal: type_clauses.append( - and_(Video.is_short.is_(False), Video.live_status.notin_(HIDDEN_LIVE)) + and_(Video.is_short.is_(False), Video.live_status.notin_(LIVE_OR_UPCOMING)) ) if include_shorts: type_clauses.append(Video.is_short.is_(True)) if include_live: - type_clauses.append(Video.live_status.in_(HIDDEN_LIVE)) + type_clauses.append(Video.live_status.in_(LIVE_OR_UPCOMING)) query = query.where(or_(*type_clauses) if type_clauses else false()) if show == "unwatched": @@ -335,7 +335,7 @@ def _filtered_query( else: # all query = query.where(status_expr != "hidden") - return query, status_expr, rank_expr + return query, rank_expr def _to_tsquery_str(q: str) -> str | None: @@ -492,7 +492,7 @@ def get_feed( user: User = Depends(current_user), db: Session = Depends(get_db), ) -> dict: - query, _status, rank_expr = _filtered_query(db, user, **params) + query, rank_expr = _filtered_query(db, user, **params) keys = _sort_keys(sort, seed, rank_expr) # Expose each key column on the row so we can build the next cursor from the last item. @@ -522,7 +522,7 @@ def get_feed_count( user: User = Depends(current_user), db: Session = Depends(get_db), ) -> dict: - query, _status, _rank = _filtered_query(db, user, **params) + query, _rank = _filtered_query(db, user, **params) total = db.scalar(select(func.count()).select_from(query.subquery())) return {"count": total or 0} @@ -550,7 +550,7 @@ def get_facets( # keeps them applied, so each remaining chip narrows to channels that ALSO have all # already-selected topics — and tags that can't co-occur drop to zero (hidden). conjunctive = category == "topic" and params.get("tag_mode") == "and" - base, _status, _rank = _filtered_query( + base, _rank = _filtered_query( db, user, **{**params, "exclude_tag_category": None if conjunctive else category}, diff --git a/backend/app/routes/public.py b/backend/app/routes/public.py index b36f1e9..7c5b9ea 100644 --- a/backend/app/routes/public.py +++ b/backend/app/routes/public.py @@ -5,7 +5,6 @@ credential (a capability URL). They only ever expose one file's stream + minimal the app or any account data. Password-protected links exchange the password once at `/unlock` for a short-lived signed grant that the media URL carries (a `