Deep-review round 2, on round 1's own fix commit. It found MORE than round 1 did (28 raw, against 13), and most of it was mine -- a new piece of UI attracts findings a backend-only diff never does. The pattern in almost all of them is the same: a principle applied to the site that prompted it and not to its siblings. * `display_text` reached three of the FIVE writers of a user-supplied name. `POST /api/downloads` and `POST /api/downloads/edit` still handed Postgres an unbounded string, so a 300-character name pasted into the Add-download dialog or the editor was a bare 500 (main.py re-raises every DataError but the NUL case). Four of the seven reviewers found it independently. The bound now lives in `models.py`, the three columns are DECLARED with it, and the guard for the next writer is parsed rather than remembered: every `display_name` / `display_uploader` assignment under `app/routes/` must go through the helper. That guard immediately found two more -- the Google sign-in callback writes the profile name into the same 255-wide column -- so it is six, not five. * The connection release reached one of the THREE routes that read their own body. The screenshot route (10 MiB, from a phone) and the client log sink both still held a pooled connection idle-in-transaction for the length of the upload, which is the invariant db.py sizes the pool on. All three now call `db.release_read_connection`, whose precondition -- nothing pending on the shared session -- is CHECKED rather than described, because a whole-session rollback discarding a caller's staged write is a mistake this repo has already made once. `test_body_limit.py` holds the rule for the next such route. * The AST drift registry keyed its sites by (module, limit expression), so a second route copied from the first collapsed onto the same entry and went uncovered -- the exact hole the registry exists to close. Keyed per call site now, and it carries the release requirement too. * The dialog's own refusal was announced once and never again: two over-cap picks leave an identical node with identical text, and an untouched live region says nothing. It is keyed per pick now, the drop zone reads as invalid and points at the sentence, and the reason is reachable from the control rather than only from a one-shot announcement. * Two tests rebuilt the blind spot they were written to remove: the dialog's cap test sat nowhere near the boundary (a `>` loosened to `>=` survived it, which is the mutation round 1 fixed on the server side), and the header-precedence test only covered an exact-case duplicate while a differently-cased one would have been COMBINED onto the wire rather than replaced. Both fixed, and the merge now dedupes case-insensitively because header names are. Also: the upload dialog's unstubbed usage query was firing a real request from jsdom in a neighbouring suite and leaking a delayed "connection lost" toast across test boundaries; `GET /api/downloads/usage` had no test at all though the whole client-side check hangs off it; and the comment claiming the edge 413 is readable now says what was measured -- readable to 200 MiB over loopback, a bare `onerror` through a proxy -- so the pre-send check is named as the mechanism and that branch as the backstop it is. Every test added or changed here was mutation-checked. One mutation SURVIVED and is recorded rather than papered over: the `tooBig` term inside `submit()` cannot be reached, because `ui/Button` swallows the activation itself, so it is marked as belt and braces instead of being described as the guard.
123 lines
5.9 KiB
Python
123 lines
5.9 KiB
Python
"""Local file ingest for the download center (E7 S5) — the non-yt-dlp path into the media library.
|
|
|
|
A user-uploaded file joins the SAME library as a downloaded one, so it inherits the whole function
|
|
set already built (metadata chips, share links, and — for media — in-app play/edit). Unlike a
|
|
download it bypasses yt-dlp AND the worker entirely: the file is already on the host, so ingest
|
|
probes it, places it on disk, and creates an already-`ready` MediaAsset + `done` DownloadJob in one
|
|
step — the same "the file exists, mark it done now" shortcut `service.enqueue` takes on a cache hit.
|
|
|
|
Kinds (the invariant lives in `mediakind`): a probe with a real video stream -> video, audio-only
|
|
-> audio, neither -> a generic 'file' (pdf/zip/docx/…) that is download-only and never rendered
|
|
inline (see the force-download serving in the file/watch routes). This module is the ingest seam a
|
|
future sub-function (e.g. convert) plugs a new adapter into, mirroring how it sidesteps the worker.
|
|
|
|
An upload does NOT get an `expires_at`: it's user-provided and may be irreplaceable (there's no
|
|
source to re-download), so it doesn't age out on the retention clock the way a re-fetchable download
|
|
does — it lives until the user deletes it.
|
|
"""
|
|
|
|
import secrets
|
|
from datetime import datetime, timezone
|
|
from pathlib import Path
|
|
|
|
from sqlalchemy.orm import Session
|
|
|
|
from app.config import settings
|
|
from app.downloads import edit as editmod
|
|
from app.downloads import mediakind, probe, quota, service, storage
|
|
from app.models import DownloadJob, MediaAsset, display_text
|
|
|
|
|
|
def _title_from_filename(filename: str) -> str:
|
|
"""A human display title from the original upload name: the stem with separators tidied to
|
|
spaces (kept as-is otherwise — this is display metadata, not the shell-safe on-disk name)."""
|
|
stem = Path(filename).stem.replace("_", " ").replace(".", " ").strip()
|
|
return (stem or "Upload")[:255]
|
|
|
|
|
|
def ingest_upload(
|
|
db: Session,
|
|
user_id: int,
|
|
*,
|
|
filename: str,
|
|
src_path: Path,
|
|
size: int,
|
|
display_name: str | None = None,
|
|
) -> DownloadJob:
|
|
"""Ingest an already-uploaded file at `src_path` into `user_id`'s library and return the `done`
|
|
job. `src_path` is MOVED into place on success (the caller stages it to a temp file first).
|
|
|
|
Raises quota.QuotaExceeded (holdings/footprint cap) — checked BEFORE any disk write, so a
|
|
rejected upload leaves nothing behind and the caller just cleans up its temp file."""
|
|
quota.check_upload(db, user_id, size) # the file's size is known → charge it up front
|
|
|
|
vcodec, acodec, width, height, duration = probe.probe_media(src_path)
|
|
kind = mediakind.derive(vcodec, acodec)
|
|
|
|
# An upload never dedups (each file is its own asset): a random ref makes the cache key unique,
|
|
# so `format_sig` is a constant marker rather than a format hash.
|
|
ref = f"{user_id}:{secrets.token_hex(8)}"[:512]
|
|
asset = MediaAsset(
|
|
source_kind="upload", source_ref=ref, format_sig="upload", status="pending", ref_count=0
|
|
)
|
|
db.add(asset)
|
|
db.flush() # need asset.id for the on-disk path
|
|
|
|
rel = storage.upload_rel_path(user_id, asset.id, filename)
|
|
storage.place_file(src_path, settings.download_root, rel)
|
|
# From here the file is on disk under `.uploads/`. If the transaction fails before it commits, the
|
|
# asset/job rows roll back but the placed file would be ORPHANED — nothing references it, the
|
|
# per-user delete path keys off asset.rel_path (which is gone), and the GC orphan sweep skips the
|
|
# dot-prefixed `.uploads/` tree. So roll back the on-disk side too on any failure.
|
|
try:
|
|
final = storage.abs_path(settings.download_root, rel)
|
|
actual_size = final.stat().st_size if final.exists() else size
|
|
|
|
asset.status = "ready"
|
|
asset.rel_path = rel
|
|
asset.size_bytes = actual_size
|
|
asset.container = storage.upload_ext(filename)
|
|
asset.vcodec = vcodec
|
|
asset.acodec = acodec
|
|
asset.width = width
|
|
asset.height = height
|
|
asset.duration_s = duration
|
|
asset.media_kind = kind
|
|
asset.title = (display_name or "").strip() or _title_from_filename(filename)
|
|
asset.last_access_at = datetime.now(timezone.utc)
|
|
# No expires_at — see the module docstring (an upload is irreplaceable, so it doesn't age out).
|
|
if kind == mediakind.VIDEO:
|
|
# A frame poster gives the library row a thumbnail + the share link an og:image. Audio has
|
|
# no frame and a generic file no player, so neither gets one. Best-effort (ffmpeg decodes
|
|
# server side even for a codec the browser can't play).
|
|
asset.poster_path = editmod.ensure_poster(asset, settings.download_root)
|
|
|
|
job = DownloadJob(
|
|
user_id=user_id,
|
|
asset_id=asset.id,
|
|
job_kind="upload",
|
|
source_kind="upload",
|
|
source_ref=ref,
|
|
profile_snapshot={},
|
|
display_name=display_text(display_name),
|
|
status="done",
|
|
progress=100,
|
|
queue_pos=service.next_queue_pos(db),
|
|
)
|
|
db.add(job)
|
|
service.incr_ref_count(db, asset.id)
|
|
db.commit()
|
|
except Exception:
|
|
db.rollback()
|
|
# The rows are gone; remove the now-unreferenced file so no bytes leak. Clean the poster by its
|
|
# DETERMINISTIC path (rel with a .jpg suffix, the same path ensure_poster writes to) rather than
|
|
# ensure_poster's return value: it writes the frame directly to that path and returns None on an
|
|
# ffmpeg failure, so a partial poster can be on disk while the return is None — gating on the
|
|
# return would skip that orphan. unlink is missing_ok, so the poster path is a harmless no-op
|
|
# when none was generated (audio/file, or a video whose poster never started).
|
|
for leaked in (rel, str(Path(rel).with_suffix(".jpg"))):
|
|
storage.abs_path(settings.download_root, leaked).unlink(missing_ok=True)
|
|
raise
|
|
db.refresh(job)
|
|
return job
|