diff --git a/backend/app/downloads/edit.py b/backend/app/downloads/edit.py
index f340ff04..21b82983 100644
--- a/backend/app/downloads/edit.py
+++ b/backend/app/downloads/edit.py
@@ -338,13 +338,16 @@ def build_concat_plan(src: Path, dest: Path, spec: dict, out_ext: str, staging:
]
else:
listp = staging / "concat.txt"
- lines = []
+ lines = ["ffconcat version 1.0"]
for s in segs:
lines.append(f"file '{_concat_quote(src)}'")
+ # ffmpeg hands the list's whitelist (which must name `concat` to read the list) down to
+ # the file it names, so a library file whose bytes are a concat list would be read as one
+ # and open the paths inside it. Each entry's own whitelist replaces the inherited one.
+ lines.append(f"option format_whitelist {INPUT_GUARD[1]}")
lines.append(f"inpoint {s['start_s']}")
lines.append(f"outpoint {s['end_s']}")
prep[listp] = "\n".join(lines) + "\n"
- # The list is ours; the whitelist reaches the file it names too (ffmpeg hands it down).
guard = ["-format_whitelist", "concat," + INPUT_GUARD[1]]
cmd = head + ["-f", "concat", "-safe", "0", *guard, "-i", str(listp), "-c", "copy"]
diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py
index 853a3b5d..b8ac2324 100644
--- a/backend/app/downloads/pipeline.py
+++ b/backend/app/downloads/pipeline.py
@@ -914,14 +914,59 @@ def _unplace(placed: Path | None) -> None:
placed.unlink(missing_ok=True)
+def _conversion_landed(asset_id: int, new_rel: str) -> bool | None:
+ """After the commit that makes a converted upload ready RAISED: did it land anyway? A commit can
+ reach the server and lose its answer (the connection drops after COMMIT). True/False by what the
+ row now says; None when the row cannot be read either."""
+ try:
+ with SessionLocal() as db:
+ asset = db.get(MediaAsset, asset_id)
+ return asset is not None and asset.status == "ready" and asset.rel_path == new_rel
+ except Exception: # noqa: BLE001 — "cannot tell" is the answer, the caller acts on it
+ # silent-ok: the caller re-raises the commit's own error, which `_fail_edit` reports.
+ return None
+
+
+def _drop_original(job_id: int, src: Path) -> None:
+ """Remove an upload's original once the converted file is committed in its place. Best effort:
+ the job is done and its file is ready, so a failure here must not fail it — the original is
+ named by no row now, and the GC's `.uploads` sweep takes it."""
+ try:
+ src.unlink(missing_ok=True)
+ except OSError as exc:
+ log.warning(
+ "upload job %s: original not removed, left to the GC sweep: %s",
+ job_id,
+ exc,
+ extra=logs.event("upload.original.unlink_failed", job_id=job_id),
+ )
+
+
+def _free_unheld_upload(asset_id: int) -> bool:
+ """Free an upload no job holds any more — cancelled or deleted while its conversion ran, when
+ `release_asset` had to leave it because the conversion still read its file. Once that has
+ stopped, free it as the release would have; left alone, its file would sit on disk counted by no
+ owner's footprint until the GC's upload pass. False when a job still holds it (paused)."""
+ with SessionLocal() as db:
+ asset = db.get(MediaAsset, asset_id)
+ if asset is None or asset.ref_count != 0:
+ return False
+ if asset.rel_path:
+ storage.delete_asset_files(settings.download_root, asset.rel_path)
+ db.delete(asset)
+ db.commit()
+ return True
+
+
def _convert_upload(job_id: int, asset_id: int) -> None:
"""Convert a queued upload into the phone-safe shape (`playable`) and make it ready.
The original sits at the asset's `rel_path` (see `upload`); the converted file replaces it there
under the new extension, and only then is the original removed — so a failed or stopped
conversion leaves the original in place for Retry, and a delete still finds it by `rel_path`.
- A converted file placed but never committed is removed again on the way out; a stop in the
- instant between the commit and the original's unlink is left to the GC's `.uploads` sweep."""
+ A converted file placed but never committed is removed again on the way out. When the commit's
+ own outcome is unknown neither file is removed: the row names one of them, and the GC's `.uploads`
+ sweep takes the one it does not — as it does an original whose unlink after the commit failed."""
with SessionLocal() as db:
asset = db.get(MediaAsset, asset_id)
old_rel = asset.rel_path if asset else None
@@ -965,12 +1010,25 @@ def _convert_upload(job_id: int, asset_id: int) -> None:
finalized = service.finalize_if_running(
db, job_id, status=JobStatus.DONE, progress=100, phase=None
)
- db.commit()
+ try:
+ db.commit()
+ except Exception:
+ landed = _conversion_landed(asset_id, new_rel)
+ if landed is False:
+ raise # rolled back: the original is still the asset's file — fail as any step
+ # Landed, or cannot tell: the converted file may be the asset's file now, so it is
+ # never removed blind (the sweep takes whichever file no row names).
+ placed = None
+ if landed is None:
+ raise
+ db.rollback()
placed = None
if finalized:
_notify_download_done(db, owner_id, title, job_id, kind="upload")
if new_rel != old_rel:
- src.unlink(missing_ok=True)
+ _drop_original(job_id, src)
+ if not finalized:
+ _free_unheld_upload(asset_id) # finished after its job was cancelled or deleted
if finalized:
log.info(
"upload job %s converted: asset %s",
@@ -979,9 +1037,11 @@ def _convert_upload(job_id: int, asset_id: int) -> None:
extra=logs.event("upload.job.done", job_id=job_id, asset_id=asset_id, ext=ext),
)
except editmod.EditAborted:
- # Paused/canceled mid-encode: the original is untouched; free the asset for a later retry.
+ # Paused/canceled mid-encode: the original is untouched. A paused job keeps it for its
+ # resume; one cancelled or deleted holds it no more, so it is freed now.
_unplace(placed)
- _reset_asset_pending(asset_id)
+ if not _free_unheld_upload(asset_id):
+ _reset_asset_pending(asset_id)
log.info(
"upload job %s conversion aborted (pause/cancel)",
job_id,
diff --git a/backend/app/downloads/probe.py b/backend/app/downloads/probe.py
index 3e29d157..eaf2a650 100644
--- a/backend/app/downloads/probe.py
+++ b/backend/app/downloads/probe.py
@@ -121,18 +121,18 @@ def resolve_media_facts(
is authoritative — INCLUDING a deliberate None (cover-art-only audio, a still image). A per-field
`probe or info` fallback would let `info`'s stale vcodec resurrect a video track the file doesn't
have (yt-dlp reports a progressive `vcodec` for an audio-mode download whose video it then strips),
- mislabelling an audio download as a broken in-app 'video'. Only when ffprobe could NOT read the
- file at all do we fall back to `info` (the pre-probe behaviour, a safety net); the cosmetic
- dimensions and the duration still gap-fill from `info` in either case."""
+ mislabelling an audio download as a broken in-app 'video'. When ffprobe could NOT read the file
+ at all, no codec is reported: the probe runs with the media demuxer whitelist, so a file it
+ cannot open is one the player and the editor cannot open either — and taking `info`'s codecs
+ for it would make it an editable 'video' whose bytes are whatever the remote site served. The
+ cosmetic dimensions and the duration still gap-fill from `info` in either case."""
data = editmod.run_ffprobe_json(path, show_streams=True, show_format=True)
- info_v = info.get("vcodec") if info.get("vcodec") not in (None, "none") else None
- info_a = info.get("acodec") if info.get("acodec") not in (None, "none") else None
info_w = int(info["width"]) if info.get("width") else None
info_h = int(info["height"]) if info.get("height") else None
info_d = int(info["duration"]) if info.get("duration") else None
if data is None:
- # ffprobe could not read the file — fall back to the extractor's info entirely.
- return info_v, info_a, info_w, info_h, info_d
+ # Unreadable to the guarded probe: a download-only 'file' (no codecs → media_kind FILE).
+ return None, None, info_w, info_h, info_d
pv, pa, pw, ph, pd = _facts_from_probe(data)
# Probe ran → its codecs win outright (a None means "no such track", not "unknown"); info only
# gap-fills the width/height/duration the probe couldn't read.
diff --git a/backend/app/downloads/upload.py b/backend/app/downloads/upload.py
index 7a491ea7..1629a05e 100644
--- a/backend/app/downloads/upload.py
+++ b/backend/app/downloads/upload.py
@@ -83,9 +83,10 @@ def ingest_upload(
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.
+ # asset/job rows roll back and nothing names the placed file: the per-user delete path keys off
+ # asset.rel_path (which is gone), and the GC's `.uploads` sweep reaches it only an hour later. So
+ # roll back the on-disk side too — unless the commit itself raised after it landed (see below).
+ asset_id = asset.id
try:
final = storage.abs_path(settings.download_root, rel)
actual_size = final.stat().st_size if final.exists() else size
@@ -126,6 +127,11 @@ def ingest_upload(
db.commit()
except Exception:
db.rollback()
+ if _row_landed(db, asset_id) is not False:
+ # The commit reached the server and lost its answer, or the row cannot be read: the file
+ # may be the asset's now, so it is never removed blind — the sweep takes it if no row
+ # names it.
+ raise
# 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
@@ -137,3 +143,13 @@ def ingest_upload(
raise
db.refresh(job)
return job
+
+
+def _row_landed(db: Session, asset_id: int) -> bool | None:
+ """After the ingest transaction raised: is the asset row there anyway? A commit can reach the
+ server and lose its answer (the connection drops after COMMIT). None when it cannot be read."""
+ try:
+ return db.get(MediaAsset, asset_id) is not None
+ except Exception: # noqa: BLE001 — "cannot tell" is the answer, the caller acts on it
+ # silent-ok: the caller re-raises the ingest's own error.
+ return None
diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py
index d9a81c19..a6508e4b 100644
--- a/backend/app/routes/downloads.py
+++ b/backend/app/routes/downloads.py
@@ -1144,7 +1144,9 @@ def admin_list_downloads(
@admin_router.get("/storage")
def admin_storage(admin: User = Depends(admin_user), db: Session = Depends(get_db)) -> dict:
- ready = db.execute(
+ # Every file on disk (`quota.ON_DISK`): ready ones, and uploads waiting for or failed in their
+ # conversion — the same disk the footprint and the GC cap count.
+ on_disk = db.execute(
select(func.count(), func.coalesce(func.sum(MediaAsset.size_bytes), 0)).where(quota.ON_DISK)
).one()
total_assets = db.execute(select(func.count()).select_from(MediaAsset)).scalar()
@@ -1158,8 +1160,8 @@ def admin_storage(admin: User = Depends(admin_user), db: Session = Depends(get_d
for u in db.execute(select(User).where(User.id.in_([uid for uid, _ in per_user]))).scalars()
}
return {
- "ready_files": ready[0],
- "total_bytes": int(ready[1]),
+ "files_on_disk": on_disk[0],
+ "total_bytes": int(on_disk[1]),
"total_assets": total_assets,
"total_cap_bytes": sysconfig.get_int(db, "download_total_max_bytes"),
"per_user": [
diff --git a/backend/app/routes/public.py b/backend/app/routes/public.py
index d8402e32..fcf0d575 100644
--- a/backend/app/routes/public.py
+++ b/backend/app/routes/public.py
@@ -16,6 +16,7 @@ from app.config import settings
from app.db import get_db
from app.downloads import links as linksmod
from app.downloads import storage
+from app.downloads.status import JobStatus
from app.models import DownloadJob, DownloadLink, MediaAsset
from app.ratelimit import RateLimiter, client_key
from app.security import verify_password
@@ -46,9 +47,16 @@ def _link_or_404(db: Session, token: str) -> DownloadLink:
def _ready_target(db: Session, link: DownloadLink) -> tuple[DownloadJob, MediaAsset]:
job = db.get(DownloadJob, link.job_id)
asset = db.get(MediaAsset, job.asset_id) if job and job.asset_id else None
- if asset is not None and asset.status in ("pending", "downloading"):
+ if (
+ job is not None
+ and asset is not None
+ and asset.status in ("pending", "downloading")
+ and job.status in (JobStatus.QUEUED, JobStatus.RUNNING)
+ ):
# Being (re)made — a re-download, or a conversion — so the link works again once it is ready:
# its own reason, so the page can say "try again shortly" rather than "this link is gone".
+ # Only while the job is still going: a cancelled or paused rebuild leaves the asset pending
+ # with nothing working on it, and "try again in a few minutes" would never come true.
raise refused(404, "video_preparing")
if job is None or asset is None or asset.status != "ready" or not asset.rel_path:
raise refused(404, "video_gone")
diff --git a/backend/tests/test_ffmpeg_input_guard.py b/backend/tests/test_ffmpeg_input_guard.py
index d87cbf2e..0d73299d 100644
--- a/backend/tests/test_ffmpeg_input_guard.py
+++ b/backend/tests/test_ffmpeg_input_guard.py
@@ -75,10 +75,45 @@ def test_a_join_is_guarded(accurate, tmp_path):
segs = [{"start_s": 0, "end_s": 1}, {"start_s": 2, "end_s": 3}]
spec = edit.normalize_edit_spec({"segments": segs, "accurate": accurate})
cmd, _ = edit.build_concat_plan(Path("in.mp4"), Path("out.mp4"), spec, "mp4", tmp_path)
- # The fast join reads a concat list WE write; the whitelist rides down to the file it names.
+ # The fast join reads a concat list WE write, so the list itself may be opened as concat.
_assert_media_guarded(cmd, extra=set() if accurate else {"concat"})
+def test_the_fast_join_list_gives_each_entry_the_media_list_without_concat(tmp_path):
+ # ffmpeg hands the list's whitelist (with `concat`) down to the library file each entry names;
+ # the entry's own `option format_whitelist` must replace it, or that file may open as a list.
+ segs = [{"start_s": 0, "end_s": 1}, {"start_s": 2, "end_s": 3}]
+ spec = edit.normalize_edit_spec({"segments": segs, "accurate": False})
+ _, prep = edit.build_concat_plan(Path("in.mp4"), Path("out.mp4"), spec, "mp4", tmp_path)
+ (text,) = prep.values()
+ lines = text.splitlines()
+ entries = [i for i, ln in enumerate(lines) if ln.startswith("file ")]
+ assert len(entries) == 2
+ for i in entries:
+ key, _, allowed = lines[i + 1].partition(" format_whitelist ")
+ assert key == "option", lines[i + 1]
+ assert set(allowed.split(",")) == MEDIA
+
+
+@pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available")
+def test_the_fast_join_still_joins_a_real_file(tmp_path):
+ src = tmp_path / "in.mp4"
+ subprocess.run(
+ ["ffmpeg", "-y", "-loglevel", "error", "-f", "lavfi", "-i", "testsrc=size=64x48:rate=10",
+ "-f", "lavfi", "-i", "sine=frequency=440", "-t", "4", "-c:v", "libx264",
+ "-pix_fmt", "yuv420p", "-c:a", "aac", str(src)],
+ check=True,
+ ) # fmt: skip
+ segs = [{"start_s": 0, "end_s": 1}, {"start_s": 2, "end_s": 3}]
+ spec = edit.normalize_edit_spec({"segments": segs, "accurate": False})
+ out = tmp_path / "out.mp4"
+ cmd, prep = edit.build_concat_plan(src, out, spec, "mp4", tmp_path)
+ for p, content in prep.items():
+ p.write_text(content, encoding="utf-8")
+ subprocess.run(cmd, check=True, capture_output=True)
+ assert (edit.probe_duration(out) or 0) > 0
+
+
def test_the_poster_and_the_filmstrip_are_guarded():
_assert_media_guarded(edit.build_poster_cmd(Path("in.mp4"), Path("p.jpg"), 2.0))
geom = edit.storyboard_geometry(60)
diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py
index db0699fd..f7360911 100644
--- a/backend/tests/test_upload.py
+++ b/backend/tests/test_upload.py
@@ -803,10 +803,10 @@ def test_ingest_rejects_over_max_jobs(db, make_user, _root, monkeypatch):
def test_ingest_cleans_up_placed_files_on_commit_failure(db, make_user, _root, monkeypatch):
"""A failure AFTER storage.place_file has moved the file onto disk must roll back AND unlink the
- placed file + any (even partial) poster — otherwise the orphan is permanent (the per-user delete
- keys off the rolled-back asset.rel_path, and the GC orphan sweep skips the dot-prefixed .uploads/
- tree). Exercises the rollback/cleanup branch that IS the leak remediation, incl. the poster case
- where ensure_poster left a partial file but returned None."""
+ placed file + any (even partial) poster — otherwise nothing names it (the per-user delete keys
+ off the rolled-back asset.rel_path) and it sits on disk until the GC's `.uploads` sweep. Exercises
+ the rollback/cleanup branch, incl. the poster case where ensure_poster left a partial file but
+ returned None."""
_probe(monkeypatch, ("h264", "aac", 320, 240, 5)) # video → the poster path runs
u = make_user()
@@ -837,6 +837,28 @@ def test_ingest_cleans_up_placed_files_on_commit_failure(db, make_user, _root, m
assert db.query(MediaAsset).count() == 0 # the flushed asset rolled back — no dangling row
+def test_ingest_keeps_the_file_when_its_commit_landed_but_raised(db, make_user, _root, monkeypatch):
+ # The commit reached the server and lost its answer: the row names the file now, so removing it
+ # as "rolled back" would leave a ready upload pointing at nothing.
+ _probe(monkeypatch, ("h264", "aac", 320, 240, 5))
+ monkeypatch.setattr(uploadmod.editmod, "ensure_poster", lambda *_a: None)
+ u = make_user()
+ real_commit = db.commit
+
+ def _landed_then_lost():
+ real_commit()
+ raise RuntimeError("connection lost after COMMIT")
+
+ monkeypatch.setattr(db, "commit", _landed_then_lost)
+ src = _root / "clip.mp4"
+ src.write_bytes(b"fake-video-bytes")
+ with pytest.raises(RuntimeError):
+ uploadmod.ingest_upload(db, u.id, filename="clip.mp4", src_path=src, size=16)
+
+ (asset,) = db.query(MediaAsset).all()
+ assert storage.abs_path(str(_root), asset.rel_path).exists()
+
+
# --- classifier: the REAL probe_media parsing (stubs ffprobe one level down) ------------------
# The ingest/route tests above stub probe_media wholesale; these exercise its OWN logic — image
# detection, the cover-art skip, dimension/duration parsing — by faking only run_ffprobe_json, so a
@@ -1022,22 +1044,16 @@ class TestResolveMediaFacts:
)
assert v is None and mediakind.derive(v, a) == mediakind.FILE
- def test_info_fills_in_only_when_ffprobe_could_not_read_the_file(self, monkeypatch):
- # ffprobe failed entirely (None) → fall back to info so we never regress the working path.
+ def test_a_file_the_guarded_probe_cannot_read_is_a_download_only_file(self, monkeypatch):
+ # ffprobe (media demuxers only) failed entirely: the player and the editor cannot open it
+ # either, so info's codecs must not make it an editable 'video'. Dimensions still gap-fill.
self._ffprobe(monkeypatch, None)
v, a, w, h, _d = probemod.resolve_media_facts(
- Path("x"), {"vcodec": "vp9", "acodec": "opus", "width": 1920, "height": 1080}
+ Path("x"),
+ {"vcodec": "avc1.64001f", "acodec": "mp4a.40.2", "width": 1920, "height": 1080},
)
- assert (v, a, w, h) == ("vp9", "opus", 1920, 1080)
-
- def test_info_none_codec_string_is_treated_as_absent(self, monkeypatch):
- # yt-dlp writes the literal "none" for a missing stream — it must not become a codec name.
- self._ffprobe(monkeypatch, None) # probe failed → info path, where the "none" scrub applies
- v, a, w, h, _d = probemod.resolve_media_facts(
- Path("x"), {"vcodec": "none", "acodec": "aac"}
- )
- assert (v, a) == (None, "aac")
- assert mediakind.derive(v, a) == mediakind.AUDIO
+ assert (v, a, w, h) == (None, None, 1920, 1080)
+ assert mediakind.derive(v, a) == mediakind.FILE
# --- the phone-safe conversion (an iPhone's HEVC, a webm, an mkv) ------------------------------
diff --git a/backend/tests/test_upload_conversion.py b/backend/tests/test_upload_conversion.py
index e7452881..2a33bea5 100644
--- a/backend/tests/test_upload_conversion.py
+++ b/backend/tests/test_upload_conversion.py
@@ -21,7 +21,9 @@ from app.downloads import probe as probemod
from app.downloads import upload as uploadmod
from app.downloads.stated import StatedFailure
from app.models import DownloadJob, DownloadQuota, MediaAsset, Notification
-from sqlalchemy import select
+from sqlalchemy import select, update
+from sqlalchemy.exc import OperationalError
+from sqlalchemy.orm import Session
@pytest.fixture
@@ -164,6 +166,33 @@ def test_two_dotted_bare_names_keep_a_file_each(db, make_user, _root, monkeypatc
assert all(storage.abs_path(str(_root), r).exists() for r in finals)
+def test_a_bare_names_stem_holds_no_dot_so_a_swapped_suffix_keeps_its_id():
+ # Each of the two name rules is held on its own: a new name's stem has no dot, so a sibling
+ # derived by `with_suffix` (the poster, the converted file) keeps `_a{id}`.
+ rel = storage.upload_rel_path(1, 5, "Holiday.2024.ä")
+ assert rel == ".uploads/1/Holiday_2024_a5"
+ assert Path(rel).with_suffix(".jpg").name == "Holiday_2024_a5.jpg"
+
+
+def test_an_upload_placed_with_a_dotted_bare_name_keeps_its_id_when_converted(
+ db, make_user, _root, monkeypatch
+):
+ # A row from before the stems lost their dots: its stem's tail is no extension.
+ _probe(monkeypatch)
+ u = make_user()
+ _converts_to(monkeypatch, size=5)
+ job, asset = _ingest(db, _root, u.id, name="Holiday.2024.ä")
+ legacy = f".uploads/{u.id}/Holiday.2024_a{asset.id}"
+ storage.abs_path(str(_root), asset.rel_path).rename(storage.abs_path(str(_root), legacy))
+ asset.rel_path, asset.container = legacy, None
+ _claim(db, job, asset)
+
+ pipeline._convert_upload(job.id, asset.id)
+
+ db.expire_all()
+ assert db.get(MediaAsset, asset.id).rel_path == f"{legacy}.mp4"
+
+
def test_a_conversion_failing_after_it_placed_its_file_removes_it(
db, make_user, _root, monkeypatch
):
@@ -187,6 +216,94 @@ def test_a_conversion_failing_after_it_placed_its_file_removes_it(
assert [p.name for p in _uploads_dir(_root, u.id).iterdir()] == [original.name]
+def _finishing_commit_raises(monkeypatch, *, lands: bool) -> None:
+ """The commit that makes the converted upload ready raises — after it reached the server
+ (`lands`: the connection dropped before its answer) or before (it rolled back)."""
+ real_finalize, real_commit = pipeline.service.finalize_if_running, Session.commit
+ state = {"session": None, "fired": False}
+
+ def _finalize(db, *a, **k):
+ if not state["fired"] and state["session"] is None:
+ state["session"] = db
+ return real_finalize(db, *a, **k)
+
+ def _commit(self):
+ if self is state["session"] and not state["fired"]:
+ state["fired"] = True
+ if lands:
+ real_commit(self)
+ raise OperationalError("COMMIT", {}, Exception("connection lost"))
+ return real_commit(self)
+
+ monkeypatch.setattr(pipeline.service, "finalize_if_running", _finalize)
+ monkeypatch.setattr(Session, "commit", _commit)
+
+
+def test_a_finishing_commit_that_landed_but_raised_keeps_the_converted_file(
+ db, make_user, _root, monkeypatch
+):
+ # The row names the converted file now: removing it as "never committed" would leave the asset
+ # pointing at nothing and its original named by no row, for the sweep to take — both copies gone.
+ _probe(monkeypatch)
+ u = make_user()
+ job, asset = _ingest(db, _root, u.id)
+ _claim(db, job, asset)
+ _converts_to(monkeypatch, size=5)
+ _finishing_commit_raises(monkeypatch, lands=True)
+
+ pipeline._convert_upload(job.id, asset.id)
+
+ db.expire_all()
+ asset = db.get(MediaAsset, asset.id)
+ assert (db.get(DownloadJob, job.id).status, asset.status) == ("done", "ready")
+ assert storage.abs_path(str(_root), asset.rel_path).exists()
+
+
+def test_a_finishing_commit_that_rolled_back_fails_and_keeps_the_original(
+ db, make_user, _root, monkeypatch
+):
+ _probe(monkeypatch)
+ u = make_user()
+ job, asset = _ingest(db, _root, u.id)
+ original = storage.abs_path(str(_root), asset.rel_path)
+ _claim(db, job, asset)
+ _converts_to(monkeypatch, size=5)
+ _finishing_commit_raises(monkeypatch, lands=False)
+
+ pipeline._convert_upload(job.id, asset.id)
+
+ db.expire_all()
+ assert db.get(DownloadJob, job.id).status == "error"
+ assert [p.name for p in _uploads_dir(_root, u.id).iterdir()] == [original.name]
+
+
+def test_an_original_that_cannot_be_removed_leaves_the_upload_ready(
+ db, make_user, _root, monkeypatch
+):
+ # The converted file is committed and the job done: the original's unlink failing must not flip
+ # a working asset to an error no Retry is offered for. The sweep takes the original later.
+ _probe(monkeypatch)
+ u = make_user()
+ job, asset = _ingest(db, _root, u.id)
+ original = storage.abs_path(str(_root), asset.rel_path)
+ _claim(db, job, asset)
+ _converts_to(monkeypatch, size=5)
+ real_unlink = Path.unlink
+
+ def _unlink(self, missing_ok=False):
+ if self == original:
+ raise PermissionError("read-only")
+ return real_unlink(self, missing_ok=missing_ok)
+
+ monkeypatch.setattr(Path, "unlink", _unlink)
+ pipeline._convert_upload(job.id, asset.id)
+
+ db.expire_all()
+ asset = db.get(MediaAsset, asset.id)
+ assert (db.get(DownloadJob, job.id).status, asset.status) == ("done", "ready")
+ assert original.exists()
+
+
# --- what the owner is told ------------------------------------------------------------------
@@ -272,6 +389,116 @@ def test_the_sweep_takes_a_stranded_upload_file_and_keeps_a_waiting_original(
assert not stranded.exists() # no row names it
+@pytest.mark.parametrize(
+ ("job_status", "asset_status"),
+ [
+ ("queued", "pending"), # waiting for the worker
+ ("running", "downloading"), # being converted, for longer than the sweep's age floor
+ ("error", "error"), # failed, kept for Retry
+ ("done", "ready"), # converted
+ ],
+)
+def test_the_sweep_keeps_an_aged_upload_file_its_row_names_in_any_state(
+ job_status, asset_status, db, make_user, _root, monkeypatch
+):
+ _probe(monkeypatch)
+ job, asset = _ingest(db, _root, make_user().id)
+ path = storage.abs_path(str(_root), asset.rel_path)
+ job.status, asset.status = job_status, asset_status
+ db.commit()
+ _aged(path)
+
+ gcmod._sweep_orphan_files(db, str(_root))
+
+ assert path.exists()
+
+
+def _row(db, asset_id: int) -> MediaAsset | None:
+ """The asset as the database has it now (a `get` would refresh the session's own copy, and
+ raise for one deleted under it)."""
+ db.expire_all()
+ return db.execute(select(MediaAsset).where(MediaAsset.id == asset_id)).scalar_one_or_none()
+
+
+def _route_moved(db, job, asset, job_status: str, ref_count: int) -> None:
+ """What the cancel/pause/delete route leaves behind, written as SQL: the session's copy of
+ `ref_count` is stale (the routes count it in SQL), so an ORM assignment of the same value would
+ write nothing."""
+ db.execute(update(DownloadJob).where(DownloadJob.id == job.id).values(status=job_status))
+ db.execute(update(MediaAsset).where(MediaAsset.id == asset.id).values(ref_count=ref_count))
+ db.commit()
+
+
+def _stopped_by(monkeypatch, db, job, asset, job_status: str, ref_count: int) -> None:
+ """The conversion is stopped mid-encode after the route moved the job to `job_status` and its
+ hold on the asset to `ref_count` (cancel and delete release it; pause keeps it)."""
+
+ def _encode(*_a, **_k):
+ _route_moved(db, job, asset, job_status, ref_count)
+ raise pipeline.editmod.EditAborted()
+
+ monkeypatch.setattr(pipeline, "_ensure_phone_safe", _encode)
+
+
+def test_an_upload_cancelled_mid_conversion_is_freed_once_the_conversion_stops(
+ db, make_user, _root, monkeypatch
+):
+ # `release_asset` leaves it while the conversion reads the file; once it stopped, nothing holds it
+ # — kept, it would sit on disk outside every footprint until the GC's upload pass.
+ _probe(monkeypatch)
+ u = make_user()
+ job, asset = _ingest(db, _root, u.id, size=10)
+ original = storage.abs_path(str(_root), asset.rel_path)
+ _claim(db, job, asset)
+ _stopped_by(monkeypatch, db, job, asset, "canceled", 0)
+
+ pipeline._convert_upload(job.id, asset.id)
+
+ assert _row(db, asset.id) is None
+ assert not original.exists()
+ assert quota.footprint(db, u.id) == 0
+
+
+def test_an_upload_paused_mid_conversion_keeps_its_original_for_the_resume(
+ db, make_user, _root, monkeypatch
+):
+ _probe(monkeypatch)
+ u = make_user()
+ job, asset = _ingest(db, _root, u.id, size=10)
+ original = storage.abs_path(str(_root), asset.rel_path)
+ _claim(db, job, asset)
+ _stopped_by(monkeypatch, db, job, asset, "paused", 1)
+
+ pipeline._convert_upload(job.id, asset.id)
+
+ db.expire_all()
+ assert db.get(MediaAsset, asset.id).status == "pending"
+ assert original.exists()
+
+
+def test_an_upload_whose_conversion_finished_after_its_job_was_cancelled_is_freed(
+ db, make_user, _root, monkeypatch
+):
+ _probe(monkeypatch)
+ u = make_user()
+ job, asset = _ingest(db, _root, u.id, size=10)
+ _claim(db, job, asset)
+ _converts_to(monkeypatch, size=5)
+ real_encode = pipeline._ensure_phone_safe
+
+ def _encode_then_cancelled(*a, **k):
+ out = real_encode(*a, **k)
+ _route_moved(db, job, asset, "canceled", 0)
+ return out
+
+ monkeypatch.setattr(pipeline, "_ensure_phone_safe", _encode_then_cancelled)
+ pipeline._convert_upload(job.id, asset.id)
+
+ assert _row(db, asset.id) is None
+ left = _uploads_dir(_root, u.id)
+ assert not left.exists() or list(left.iterdir()) == []
+
+
def test_the_cache_cap_never_evicts_an_upload(db, make_user, _root, monkeypatch):
_probe(monkeypatch, vcodec="h264")
u = make_user()
@@ -301,12 +528,21 @@ def test_the_cache_cap_never_evicts_an_upload(db, make_user, _root, monkeypatch)
# --- a share link ----------------------------------------------------------------------------
-@pytest.mark.parametrize("state", ["pending", "downloading"])
-def test_a_link_whose_video_is_being_made_again_says_preparing_not_gone(
- state, db, as_user, make_user, _root, monkeypatch, upload_headers
+@pytest.mark.parametrize(
+ ("job_status", "asset_status", "reason"),
+ [
+ ("queued", "pending", "video_preparing"), # a re-download waiting for the worker
+ ("running", "downloading", "video_preparing"), # being made now
+ ("canceled", "pending", "video_gone"), # a rebuild cancelled: nothing will finish it
+ ("paused", "pending", "video_gone"), # held until its owner resumes it, maybe never
+ ],
+)
+def test_a_link_says_preparing_only_while_its_video_is_being_made(
+ job_status, asset_status, reason, db, as_user, make_user, _root, monkeypatch, upload_headers
):
# A link is made on a finished job; its asset can be (re)made under it later — a shared cache row
- # re-fetched, a conversion retried. Until it is ready the page says "try again shortly".
+ # re-fetched, a conversion retried. While that runs the page says "try again shortly"; once
+ # nothing is making it, it says the video is gone rather than promising it forever.
_probe(monkeypatch, vcodec="h264")
c = as_user(make_user())
job = c.post(
@@ -314,11 +550,12 @@ def test_a_link_whose_video_is_being_made_again_says_preparing_not_gone(
).json()
link = c.post(f"/api/downloads/{job['id']}/links", json={"allow_download": False}).json()
assert c.get(f"/api/public/watch/{link['token']}").status_code == 200
- db.get(MediaAsset, job["asset"]["id"]).status = state
+ db.get(MediaAsset, job["asset"]["id"]).status = asset_status
+ db.get(DownloadJob, job["id"]).status = job_status
db.commit()
r = c.get(f"/api/public/watch/{link['token']}")
assert r.status_code == 404
- assert r.json()["detail"]["reason"] == "video_preparing"
+ assert r.json()["detail"]["reason"] == reason
def test_the_admin_storage_view_counts_a_waiting_upload(db, as_user, make_user, _root, monkeypatch):
@@ -327,7 +564,7 @@ def test_the_admin_storage_view_counts_a_waiting_upload(db, as_user, make_user,
u = make_user()
_ingest(db, _root, u.id, size=10)
r = as_user(make_user(role="admin")).get("/api/admin/downloads/storage").json()
- assert (r["ready_files"], r["total_bytes"]) == (1, 10)
+ assert (r["files_on_disk"], r["total_bytes"]) == (1, 10)
assert {p["user_id"]: p["footprint_bytes"] for p in r["per_user"]} == {u.id: 10}
@@ -345,11 +582,11 @@ def test_the_upload_reap_leaves_one_still_held_or_still_read(
_probe(monkeypatch)
job, asset = _ingest(db, _root, make_user().id)
path = storage.abs_path(str(_root), asset.rel_path)
- job.status, asset.status, asset.ref_count = job_status, asset_status, ref_count
+ asset.status = asset_status
db.commit()
+ _route_moved(db, job, asset, job_status, ref_count)
gcmod.run_download_gc(db)
- db.expire_all()
- assert db.get(MediaAsset, asset.id) is not None
+ assert _row(db, asset.id) is not None
assert path.exists()
diff --git a/frontend/src/components/DownloadCenter.confirm.dom.test.tsx b/frontend/src/components/DownloadCenter.confirm.dom.test.tsx
index 46deb263..30833de2 100644
--- a/frontend/src/components/DownloadCenter.confirm.dom.test.tsx
+++ b/frontend/src/components/DownloadCenter.confirm.dom.test.tsx
@@ -10,7 +10,9 @@ import type { DownloadJob } from "../lib/api";
const mocks = vi.hoisted(() => ({
jobs: [] as unknown[],
- confirm: vi.fn(async (_opts: { title: string; confirmLabel?: string }) => false),
+ confirm: vi.fn(
+ async (_opts: { title: string; message?: string; confirmLabel?: string }) => false,
+ ),
}));
vi.mock("react-i18next", async (importOriginal) => {
@@ -141,4 +143,41 @@ describe("DownloadCenter confirm dialogs", () => {
"downloads.confirm.cancelUploadAction",
]);
});
+
+ it("cancelling a clip speaks of the clip", async () => {
+ mocks.jobs = [{ ...job(4, "running"), job_kind: "edit" }];
+ render(
+
+
+ ,
+ );
+ await pick("Clip 4", "downloads.actions.cancel");
+ await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1));
+ const [opts] = mocks.confirm.mock.calls[0]!;
+ expect([opts.title, opts.message, opts.confirmLabel]).toEqual([
+ "downloads.confirm.cancelEditTitle",
+ "downloads.confirm.cancelEditBody",
+ "downloads.confirm.cancelEditAction",
+ ]);
+ });
+
+ it.each([
+ ["upload", "downloads.confirm.deleteUploadTitle", "downloads.confirm.deleteUploadBody"],
+ ["edit", "downloads.confirm.deleteEditTitle", "downloads.confirm.deleteEditBody"],
+ ["download", "downloads.confirm.deleteTitle", "downloads.confirm.deleteBody"],
+ ] as const)("deleting a finished %s says what happens to its file", async (kind, title, body) => {
+ // Only a download's file may stay cached for other users; an upload's or a clip's is theirs alone.
+ mocks.jobs = [{ ...job(5, "done"), job_kind: kind }];
+ render(
+
+
+ ,
+ );
+ fireEvent.click(screen.getByRole("tab", { name: /downloads\.tabs\.done/ }));
+ await screen.findByText("Clip 5");
+ await pick("Clip 5", "downloads.actions.delete");
+ await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1));
+ const [opts] = mocks.confirm.mock.calls[0]!;
+ expect([opts.title, opts.message]).toEqual([title, body]);
+ });
});
diff --git a/frontend/src/components/WatchPage.dom.test.tsx b/frontend/src/components/WatchPage.dom.test.tsx
index 9bcab3db..5ae96f85 100644
--- a/frontend/src/components/WatchPage.dom.test.tsx
+++ b/frontend/src/components/WatchPage.dom.test.tsx
@@ -224,13 +224,33 @@ describe("WatchPage honest failures", () => {
);
const page = render();
const retry = await page.findByRole("button", { name: "Try again" });
- expect(page.getByRole("status").textContent).toContain("This video is being prepared.");
+ expect(page.getByRole("alert").textContent).toContain("This video is being prepared.");
expect(page.queryByText(GONE)).toBeNull();
fireEvent.click(retry);
await waitFor(() => expect(page.container.querySelector("video")).not.toBeNull());
});
+ it("a Retry that is answered 'still preparing' says so and keeps the focus on Retry", async () => {
+ // The answer replaces the button the viewer was on: announced (an alert, inserted filled), and
+ // the new Retry takes the focus instead of leaving it on .
+ window.history.replaceState({}, "", "/watch/tok-preparing");
+ const preparing = () => res(404, { detail: { code: 404, reason: "video_preparing" } });
+ vi.stubGlobal(
+ "fetch",
+ vi.fn().mockImplementation(() => Promise.resolve(preparing())),
+ );
+ const page = render();
+ const first = await page.findByRole("button", { name: "Try again" });
+ expect(document.activeElement).not.toBe(first); // the first load moves no focus
+
+ fireEvent.click(first);
+ await waitFor(() =>
+ expect(document.activeElement).toBe(page.getByRole("button", { name: "Try again" })),
+ );
+ expect(page.getByRole("alert").textContent).toContain("This video is being prepared.");
+ });
+
async function tryPassword(second: () => Promise) {
vi.stubGlobal(
"fetch",
diff --git a/frontend/src/components/WatchPage.tsx b/frontend/src/components/WatchPage.tsx
index cb1a0023..a42e4f9e 100644
--- a/frontend/src/components/WatchPage.tsx
+++ b/frontend/src/components/WatchPage.tsx
@@ -114,6 +114,10 @@ export default function WatchPage() {
// Bumped by Retry, which asks again.
const [attempt, setAttempt] = useState(0);
+ // Set once the viewer acted (Retry, the password): the state that answers replaces the button they
+ // were on, so its own Retry takes the focus rather than leaving it on . Not on the first load,
+ // where nothing had the focus to lose.
+ const [acted, setActed] = useState(false);
useEffect(() => {
if (!token) return; // the seed above already says "gone"
send(`/api/public/watch/${encodeURIComponent(token)}`)
@@ -136,6 +140,7 @@ export default function WatchPage() {
.catch(() => setStatus("error"));
}, [token, attempt]);
const retry = () => {
+ setActed(true);
setStatus("loading");
setAttempt((n) => n + 1);
};
@@ -167,6 +172,7 @@ export default function WatchPage() {
};
const unlock = async () => {
+ setActed(true);
setUnlocking(true);
setError(null);
try {
@@ -203,11 +209,19 @@ export default function WatchPage() {
{status === "preparing" && (
- {/* Announced like the failure below: it replaces the loading line. */}
-
+ {/* An alert, like the failure below: it is inserted already filled, which a polite status
+ region is not reliably announced for — and a Retry that answers "still preparing"
+ would otherwise say nothing at all. */}
+
{T.preparing}
{T.preparingHint}
-
@@ -220,7 +234,13 @@ export default function WatchPage() {
{[
- [t("downloads.admin.readyFiles"), String(s.ready_files)],
+ [t("downloads.admin.filesOnDisk"), String(s.files_on_disk)],
[t("downloads.admin.totalSize"), formatBytes(s.total_bytes, lang)],
[
t("downloads.admin.cap"),
diff --git a/frontend/src/components/useDownloadActions.tsx b/frontend/src/components/useDownloadActions.tsx
index 1731c101..8f1ac72a 100644
--- a/frontend/src/components/useDownloadActions.tsx
+++ b/frontend/src/components/useDownloadActions.tsx
@@ -16,7 +16,7 @@ import {
Trash2,
} from "lucide-react";
import { qk } from "../lib/queryKeys";
-import { api, type DownloadJob } from "../lib/api";
+import { api, type DownloadJob, type DownloadJobKind } from "../lib/api";
import { notifyActionError } from "../lib/actionErrors";
import { invalidateDownloads, quickShareLink } from "../lib/downloads";
import { libraryRowActions, sharedRowActions } from "../lib/downloadActions";
@@ -46,6 +46,29 @@ const CANCEL_UPLOAD = {
body: "downloads.confirm.cancelUploadBody",
action: "downloads.confirm.cancelUploadAction",
};
+const CANCEL_EDIT = {
+ title: "downloads.confirm.cancelEditTitle",
+ body: "downloads.confirm.cancelEditBody",
+ action: "downloads.confirm.cancelEditAction",
+};
+// Cancelling an upload deletes the file it uploaded (there is nothing to fetch again), so its
+// confirmation says so — "stop the download" would read as safe to undo. A clip is being cut, not
+// downloaded.
+const CANCEL_COPY: Record = {
+ download: CANCEL_DOWNLOAD,
+ upload: CANCEL_UPLOAD,
+ edit: CANCEL_EDIT,
+};
+// Deleting a download may leave its shared cache file for other holders. An upload and a clip are
+// their owner's alone (a clip's asset is keyed by its user), so deleting one removes the file for good.
+const DELETE_COPY: Record = {
+ download: { title: "downloads.confirm.deleteTitle", body: "downloads.confirm.deleteBody" },
+ upload: {
+ title: "downloads.confirm.deleteUploadTitle",
+ body: "downloads.confirm.deleteUploadBody",
+ },
+ edit: { title: "downloads.confirm.deleteEditTitle", body: "downloads.confirm.deleteEditBody" },
+};
export function useDownloadActions({
onPlay,
@@ -159,10 +182,8 @@ export function useDownloadActions({
label: t("downloads.actions.cancel"),
icon: icon(Ban),
danger: true,
- // Cancelling an upload deletes the file it uploaded (there is nothing to fetch again), so
- // its confirmation says so — "stop the download" would read as safe to undo.
onSelect: () => {
- const copy = job.job_kind === "upload" ? CANCEL_UPLOAD : CANCEL_DOWNLOAD;
+ const copy = CANCEL_COPY[job.job_kind ?? "download"];
void confirmThen(t(copy.title), t(copy.body), t(copy.action), () =>
act.mutate({ id: job.id, op: "cancel" }),
);
@@ -233,13 +254,12 @@ export function useDownloadActions({
label: t("downloads.actions.delete"),
icon: icon(Trash2),
danger: true,
- onSelect: () =>
- void confirmThen(
- t("downloads.confirm.deleteTitle"),
- t("downloads.confirm.deleteBody"),
- t("downloads.actions.delete"),
- () => act.mutate({ id: job.id, op: "delete" }),
- ),
+ onSelect: () => {
+ const copy = DELETE_COPY[job.job_kind ?? "download"];
+ void confirmThen(t(copy.title), t(copy.body), t("downloads.actions.delete"), () =>
+ act.mutate({ id: job.id, op: "delete" }),
+ );
+ },
},
],
);
diff --git a/frontend/src/i18n/locales/en/downloads.json b/frontend/src/i18n/locales/en/downloads.json
index 3130bb73..93d65bf6 100644
--- a/frontend/src/i18n/locales/en/downloads.json
+++ b/frontend/src/i18n/locales/en/downloads.json
@@ -155,8 +155,15 @@
"cancelUploadTitle": "Delete upload?",
"cancelUploadBody": "The file you uploaded is removed from the server along with it.",
"cancelUploadAction": "Delete upload",
+ "cancelEditTitle": "Cancel clip?",
+ "cancelEditBody": "This stops making the clip and removes it from your queue.",
+ "cancelEditAction": "Stop making this clip",
"deleteTitle": "Delete download?",
"deleteBody": "This deletes it from your downloads. The file may stay cached for other users until it expires.",
+ "deleteUploadTitle": "Delete upload?",
+ "deleteUploadBody": "The uploaded file is removed from the server. This can't be undone.",
+ "deleteEditTitle": "Delete clip?",
+ "deleteEditBody": "The clip is removed from the server. This can't be undone.",
"removeSharedTitle": "Remove from your list?",
"removeSharedBody": "This removes it from your shared list only — it won't delete the owner's file.",
"redownloadTitle": "Re-download this file?",
@@ -187,7 +194,7 @@
"deleteConfirm": "Delete the format “{{name}}”?"
},
"admin": {
- "readyFiles": "Ready files",
+ "filesOnDisk": "Files on disk",
"totalSize": "Total size",
"cap": "Cache cap",
"noCap": "no cap",
@@ -231,8 +238,20 @@
"fail": {
"title": "Couldn't download",
"titleUpload": "The upload isn't ready",
+ "titleEdit": "The clip isn't ready",
"detailsLink": "details",
"detailsToggle": "Technical details",
+ "upload": {
+ "quota_exceeded": {
+ "body": "The phone-ready version of this upload would not fit in your storage quota. Your original is kept – delete something from your library, then try again."
+ }
+ },
+ "edit": {
+ "unknown": {
+ "short": "Couldn't make the clip",
+ "body": "Something went wrong while making this clip. The technical details below may help."
+ }
+ },
"cat": {
"unavailable": {
"short": "Video unavailable",
diff --git a/frontend/src/i18n/locales/hu/downloads.json b/frontend/src/i18n/locales/hu/downloads.json
index 7dd76372..3b371083 100644
--- a/frontend/src/i18n/locales/hu/downloads.json
+++ b/frontend/src/i18n/locales/hu/downloads.json
@@ -155,8 +155,15 @@
"cancelUploadTitle": "Törlöd a feltöltést?",
"cancelUploadBody": "A feltöltött fájl is törlődik a szerverről.",
"cancelUploadAction": "Feltöltés törlése",
+ "cancelEditTitle": "Megszakítod a klipet?",
+ "cancelEditBody": "Ez leállítja a klip készítését és eltávolítja a sorból.",
+ "cancelEditAction": "Klip készítésének leállítása",
"deleteTitle": "Törlöd a letöltést?",
"deleteBody": "Ez törli a letöltéseid közül. A fájl a lejáratáig még a gyorsítótárban maradhat mások számára.",
+ "deleteUploadTitle": "Törlöd a feltöltést?",
+ "deleteUploadBody": "A feltöltött fájl törlődik a szerverről. Ez nem vonható vissza.",
+ "deleteEditTitle": "Törlöd a klipet?",
+ "deleteEditBody": "A klip törlődik a szerverről. Ez nem vonható vissza.",
"removeSharedTitle": "Eltávolítod a listádból?",
"removeSharedBody": "Csak a te megosztott listádból tűnik el — a tulajdonos fájlját nem törli.",
"redownloadTitle": "Újraletöltöd ezt a fájlt?",
@@ -187,7 +194,7 @@
"deleteConfirm": "Törlöd a(z) „{{name}}” formátumot?"
},
"admin": {
- "readyFiles": "Kész fájlok",
+ "filesOnDisk": "Fájlok a lemezen",
"totalSize": "Teljes méret",
"cap": "Gyorsítótár-korlát",
"noCap": "nincs korlát",
@@ -231,8 +238,20 @@
"fail": {
"title": "Nem sikerült letölteni",
"titleUpload": "A feltöltés nem készült el",
+ "titleEdit": "A klip nem készült el",
"detailsLink": "részletek",
"detailsToggle": "Technikai részletek",
+ "upload": {
+ "quota_exceeded": {
+ "body": "A feltöltés telefonra alakított változata nem fér bele a tárhelykeretedbe. Az eredeti megmaradt – törölj valamit a könyvtáradból, majd próbáld újra."
+ }
+ },
+ "edit": {
+ "unknown": {
+ "short": "Nem sikerült elkészíteni a klipet",
+ "body": "Hiba történt a klip készítése közben. Az alábbi technikai részletek segíthetnek."
+ }
+ },
"cat": {
"unavailable": {
"short": "A videó nem elérhető",
diff --git a/frontend/src/lib/api/downloads.ts b/frontend/src/lib/api/downloads.ts
index 231b66a8..33266730 100644
--- a/frontend/src/lib/api/downloads.ts
+++ b/frontend/src/lib/api/downloads.ts
@@ -47,7 +47,7 @@ export type DownloadStatus = "queued" | "running" | "paused" | "done" | "error"
// "download" (default), "edit" (a per-user trim/crop derivative cut from another job), or "upload"
// (a locally-uploaded file — no re-fetchable source, so re-download is rejected backend-side).
// Local (not exported): consumers compare `job.job_kind` against literals, never name the type.
-type DownloadJobKind = "download" | "edit" | "upload";
+export type DownloadJobKind = "download" | "edit" | "upload";
// Phase-2 editor recipe. A single `trim` (one output file) or a `segments` cut-list joined into
// one file. `accurate` re-encodes for a frame-accurate cut (a crop always re-encodes).
@@ -137,7 +137,7 @@ export interface DownloadUsage {
}
export interface AdminStorage {
- ready_files: number;
+ files_on_disk: number;
total_bytes: number;
total_assets: number;
total_cap_bytes: number;
diff --git a/frontend/src/lib/downloadFailure.test.ts b/frontend/src/lib/downloadFailure.test.ts
index 579c333c..8eba0bc3 100644
--- a/frontend/src/lib/downloadFailure.test.ts
+++ b/frontend/src/lib/downloadFailure.test.ts
@@ -49,13 +49,61 @@ describe("failCopyKeys", () => {
});
describe("failTitleKey", () => {
- it("titles an upload's failure as a conversion, anything else as a download", async () => {
+ it("titles a failure by what the job was making", async () => {
await i18n.changeLanguage("en");
- // the locale has the copy (a missing key would come back as the key itself)
- expect(i18n.t(failTitleKey({ job_kind: "upload" }))).not.toMatch(/^downloads\./);
expect(failTitleKey({ job_kind: "upload" })).toBe("downloads.errors.fail.titleUpload");
- expect(failTitleKey({ job_kind: "edit" })).toBe("downloads.errors.fail.title");
+ expect(failTitleKey({ job_kind: "edit" })).toBe("downloads.errors.fail.titleEdit");
+ expect(failTitleKey({ job_kind: "download" })).toBe("downloads.errors.fail.title");
expect(failTitleKey({})).toBe("downloads.errors.fail.title");
+ for (const lng of ["en", "hu"]) {
+ await i18n.changeLanguage(lng);
+ // the locale has the copy (a missing key would come back as the key itself)
+ for (const job_kind of ["upload", "edit"] as const)
+ expect(i18n.t(failTitleKey({ job_kind }))).not.toMatch(/^downloads\./);
+ }
+ });
+
+ it("an upload's quota refusal is worded as its conversion's, in both languages", async () => {
+ // The converted file outgrew the cap; the original stays. Not "this download was stopped because
+ // your quota is already used up" — the download sentence stays for a download.
+ const upload = {
+ error_code: "quota_exceeded",
+ error_detail: null,
+ job_kind: "upload",
+ } as const;
+ const download = { ...upload, job_kind: "download" } as const;
+ for (const lng of ["en", "hu"]) {
+ await i18n.changeLanguage(lng);
+ const body = i18n.t(failCopyKeys(upload, "body"));
+ expect(body).not.toMatch(/^downloads\./);
+ expect(body).not.toBe(i18n.t(failCopyKeys(download, "body")));
+ expect(i18n.t(failCopyKeys(download, "body"))).toBe(
+ i18n.t("downloads.errors.fail.cat.quota_exceeded.body"),
+ );
+ }
+ });
+
+ it("a clip that fails for no known reason says so of a clip, in both languages", async () => {
+ // Not "Couldn't download / Something went wrong with this download" — nothing was downloaded.
+ const clip = { error_code: "unknown", error_detail: null, job_kind: "edit" } as const;
+ const download = { ...clip, job_kind: "download" } as const;
+ for (const lng of ["en", "hu"]) {
+ await i18n.changeLanguage(lng);
+ for (const part of ["short", "body"] as const) {
+ const own = i18n.t(failCopyKeys(clip, part));
+ expect(own).not.toMatch(/^downloads\./);
+ expect(own).not.toBe(i18n.t(failCopyKeys(download, part)));
+ }
+ }
+ // a category with no copy of its own falls back to the clip's unknown sentence, not the download's
+ expect(
+ failCopyKeys({ error_code: "brand_new", error_detail: null, job_kind: "edit" }, "body"),
+ ).toEqual([
+ "downloads.errors.fail.edit.brand_new.body",
+ "downloads.errors.fail.cat.brand_new.body",
+ "downloads.errors.fail.edit.unknown.body",
+ "downloads.errors.fail.cat.unknown.body",
+ ]);
});
it("an upload's own failures read as an upload's", async () => {
diff --git a/frontend/src/lib/downloadFailure.ts b/frontend/src/lib/downloadFailure.ts
index 612f4918..72f3a4d5 100644
--- a/frontend/src/lib/downloadFailure.ts
+++ b/frontend/src/lib/downloadFailure.ts
@@ -1,27 +1,38 @@
-import type { DownloadJob } from "./api";
+import type { DownloadJob, DownloadJobKind } from "./api";
// The i18n keys that explain a failed download, most specific first — the list form `t()` takes, so
// the first key a locale has wins. A failure-catalog entry (`error_detail`, e.g. `facebook.login_wall`)
// names what went wrong on that site and what to do; the category (`error_code`) is the generic
// sentence, and the fallback when an older client meets a detail it has no copy for; a category it has
-// no copy for falls back to the unknown one, so the row never prints a key. One helper for the queue
-// row's headline and the failure window, so the two can never explain the same job differently.
+// no copy for falls back to the unknown one, so the row never prints a key. A job that is not a
+// download (an upload's conversion, a clip) may word a category its own way, ahead of the shared
+// sentence — "this download was stopped" is not what happened to it — and its own unknown sentence
+// comes before the download's. One helper for the queue row's headline and the failure window, so
+// the two can never explain the same job differently.
export function failCopyKeys(
- job: Pick,
+ job: Pick,
part: "short" | "body",
): string[] {
- const category = (code: string) => `downloads.errors.fail.cat.${code}.${part}`;
+ const kind = job.job_kind ?? "download";
+ const category = (code: string) => [
+ ...(kind === "download" ? [] : [`downloads.errors.fail.${kind}.${code}.${part}`]),
+ `downloads.errors.fail.cat.${code}.${part}`,
+ ];
const code = job.error_code || "unknown";
return [
...(job.error_detail ? [`downloads.errors.fail.catalog.${job.error_detail}.${part}`] : []),
- category(code),
- ...(code === "unknown" ? [] : [category("unknown")]),
+ ...category(code),
+ ...(code === "unknown" ? [] : category("unknown")),
];
}
-/** The title over a failed job's explanation: an upload's failure is a conversion, not a download. */
+const TITLE_KEYS: Record = {
+ download: "downloads.errors.fail.title",
+ upload: "downloads.errors.fail.titleUpload", // its failure is a conversion, not a download
+ edit: "downloads.errors.fail.titleEdit", // a clip being cut
+};
+
+/** The title over a failed job's explanation, worded for what the job was making. */
export function failTitleKey(job: Pick): string {
- return job.job_kind === "upload"
- ? "downloads.errors.fail.titleUpload"
- : "downloads.errors.fail.title";
+ return TITLE_KEYS[job.job_kind ?? "download"];
}