fix(downloads): a suspended user's queued jobs wait, and a clip too big for the space left says so

A suspension now blocks the user's downloads as quota.resolve already advised: the worker leaves
their queued jobs unclaimed until it is lifted (the confirm says so). A cut clip larger than the
remaining storage is refused with its own message (not "storage is full"), and a clip's quota
refusal reads as a clip's in both languages. The cancel poll compares to JobStatus.RUNNING.
This commit is contained in:
2026-10-10 20:28:52 +02:00
parent 4d4eea2c54
commit a867145bcb
14 changed files with 73 additions and 11 deletions
+8 -2
View File
@@ -37,8 +37,8 @@ from app.downloads.credentials import host_key_for_url
# Worker-authored strings that this module owns because each is coupled to `_RULES`. # Worker-authored strings that this module owns because each is coupled to `_RULES`.
# #
# QUOTA_FULL_MESSAGE is raised by the worker and must classify back to `quota_exceeded`; its rule # QUOTA_FULL_MESSAGE and CLIP_TOO_BIG_MESSAGE are raised by the worker and must classify back to
# below matches a fragment of it, and test_download_errors pins the pair, because a message and its # `quota_exceeded`; their rules below match a fragment of each, and test_download_errors pins the pair, because a message and its
# rule drifting apart is exactly how a category silently becomes unreachable. # rule drifting apart is exactly how a category silently becomes unreachable.
# #
# NETWORK_REDACTED_MESSAGE REPLACES a message after classification (`user_error_text`). It is written # NETWORK_REDACTED_MESSAGE REPLACES a message after classification (`user_error_text`). It is written
@@ -53,6 +53,11 @@ from app.downloads.credentials import host_key_for_url
# constants, imported above; each has a rule below that gives its category back, and a catalog entry # constants, imported above; each has a rule below that gives its category back, and a catalog entry
# that gives the site's sentence. # that gives the site's sentence.
QUOTA_FULL_MESSAGE = "Your download storage is full — free up space, then try again." QUOTA_FULL_MESSAGE = "Your download storage is full — free up space, then try again."
# A cut clip held to the space left, which is not zero: "full" would be untrue.
CLIP_TOO_BIG_MESSAGE = (
"The clip is larger than the download storage you have left — "
"free up space or cut a shorter clip, then try again."
)
UPLOAD_MISSING_MESSAGE = "The uploaded file is missing." UPLOAD_MISSING_MESSAGE = "The uploaded file is missing."
NETWORK_REDACTED_MESSAGE = "The server could not reach that address." NETWORK_REDACTED_MESSAGE = "The server could not reach that address."
@@ -62,6 +67,7 @@ _RULES: tuple[tuple[str, str], ...] = (
# Our own refusal: the worker found the user already at their storage cap (SEC-13). Placed # Our own refusal: the worker found the user already at their storage cap (SEC-13). Placed
# first — it is our wording, so it can never be a false match on an upstream message. # first — it is our wording, so it can never be a false match on an upstream message.
("download storage is full", "quota_exceeded"), ("download storage is full", "quota_exceeded"),
("larger than the download storage you have left", "quota_exceeded"),
# Our own redaction of a transport failure (see NETWORK_REDACTED_MESSAGE) — our wording too. # Our own redaction of a transport failure (see NETWORK_REDACTED_MESSAGE) — our wording too.
(NETWORK_REDACTED_MESSAGE.lower(), "network"), (NETWORK_REDACTED_MESSAGE.lower(), "network"),
# Our own verdicts on a post link (adapters.py) — our wording too: its page holds no single video, # Our own verdicts on a post link (adapters.py) — our wording too: its page holds no single video,
+2 -2
View File
@@ -226,7 +226,7 @@ def _cancel_poll(job_id: int, interval_s: float = _CANCEL_POLL_INTERVAL_S, *, no
def cancelled() -> bool: def cancelled() -> bool:
if not state["cancelled"] and allow(): if not state["cancelled"] and allow():
state["cancelled"] = _job_status(job_id) not in ("running", None) state["cancelled"] = _job_status(job_id) not in (JobStatus.RUNNING, None)
return state["cancelled"] return state["cancelled"]
return cancelled return cancelled
@@ -1061,7 +1061,7 @@ def _process_clip(job_id: int, asset_id: int) -> None:
# again, since other jobs of this owner may have landed during the encode. # again, since other jobs of this owner may have landed during the encode.
remaining = _clip_room(user_id) remaining = _clip_room(user_id)
if remaining is not None and out.stat().st_size > remaining: if remaining is not None and out.stat().st_size > remaining:
raise _OwnerRefused(dlerrors.QUOTA_FULL_MESSAGE) raise _OwnerRefused(dlerrors.CLIP_TOO_BIG_MESSAGE)
ext = out.suffix.lstrip(".").lower() or "mp4" ext = out.suffix.lstrip(".").lower() or "mp4"
rel = storage.edit_rel_path(user_id, asset_id, ext) rel = storage.edit_rel_path(user_id, asset_id, ext)
storage.place_file(out, settings.download_root, rel) storage.place_file(out, settings.download_root, rel)
+2 -1
View File
@@ -67,7 +67,8 @@ def resolve(db: Session, user_id: int) -> QuotaLimits:
# "at limit" checks — the worker's claim-window exclusion vs at_concurrency_limit — since a # "at limit" checks — the worker's claim-window exclusion vs at_concurrency_limit — since a
# 0-limit user is "capped" with zero running jobs, which the SQL exclusion can't see. Keep # 0-limit user is "capped" with zero running jobs, which the SQL exclusion can't see. Keep
# them consistent by never letting the effective limit drop below 1. (To BLOCK a user's # them consistent by never letting the effective limit drop below 1. (To BLOCK a user's
# downloads, suspend them or set max_jobs=0, not max_concurrent=0.) # downloads, suspend them — the worker leaves a suspended user's queued jobs unclaimed — or
# set max_jobs=0, not max_concurrent=0.)
return QuotaLimits(row.max_bytes, max(1, row.max_concurrent), row.max_jobs, row.unlimited) return QuotaLimits(row.max_bytes, max(1, row.max_concurrent), row.max_jobs, row.unlimited)
return QuotaLimits( return QuotaLimits(
max_bytes=sysconfig.get_int(db, "download_default_max_bytes"), max_bytes=sysconfig.get_int(db, "download_default_max_bytes"),
+2 -1
View File
@@ -123,7 +123,8 @@ class User(Base, TimestampMixin, CanonicalEmailMixin):
is_active: Mapped[bool] = mapped_column(Boolean, default=True, server_default="true") is_active: Mapped[bool] = mapped_column(Boolean, default=True, server_default="true")
# Admin-imposed access block, distinct from is_active (approval lifecycle). A suspended # Admin-imposed access block, distinct from is_active (approval lifecycle). A suspended
# account can't sign in by any method and its live sessions are rejected; the user is told # account can't sign in by any method and its live sessions are rejected; the user is told
# why and pointed at the operator's contact. # why and pointed at the operator's contact. The worker leaves its queued downloads unclaimed
# until the block is lifted (`worker._claim_job`).
is_suspended: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false") is_suspended: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false")
# Server-side session revocation (SA4): a monotonic counter bumped on password reset, password # Server-side session revocation (SA4): a monotonic counter bumped on password reset, password
# change, or "log out everywhere". A signed session cookie records the epoch it was minted under; # change, or "log out everywhere". A signed session cookie records the epoch it was minted under;
+2 -1
View File
@@ -484,7 +484,8 @@ def set_user_suspended(
db: Session = Depends(get_db), db: Session = Depends(get_db),
) -> dict: ) -> dict:
"""Suspend or un-suspend an account. A suspended user can't sign in by any method and any """Suspend or un-suspend an account. A suspended user can't sign in by any method and any
live session is dropped on its next request. Guards: can't suspend yourself, and can't suspend the last active admin (that would lock admin out). On live session is dropped on its next request; their queued downloads wait (the worker skips them)
until they are un-suspended. Guards: can't suspend yourself, and can't suspend the last active admin (that would lock admin out). On
un-suspend the user is emailed they can return (suspend is announced reactively, on a blocked un-suspend the user is emailed they can return (suspend is announced reactively, on a blocked
sign-in).""" sign-in)."""
suspended = body.suspended suspended = body.suspended
+9 -1
View File
@@ -163,7 +163,11 @@ def _claim_job() -> int | None:
seen.) The per-user limit is the quota row's `max_concurrent` (else the sysconfig default); seen.) The per-user limit is the quota row's `max_concurrent` (else the sysconfig default);
`unlimited` users are never capped. A final Python `at_concurrency_limit` re-check still guards `unlimited` users are never capped. A final Python `at_concurrency_limit` re-check still guards
the tiny race between the exclusion subquery and the claim (it can only ever reject, never the tiny race between the exclusion subquery and the claim (it can only ever reject, never
over-claim), so correctness never depends on the two observations agreeing.""" over-claim), so correctness never depends on the two observations agreeing.
A suspended user's queued jobs are left where they are: suspension blocks their downloads (not
only their sign-in), and lifting it lets them run in their old place. A job already running
when the user is suspended finishes."""
with SessionLocal() as db: with SessionLocal() as db:
default_mc = sysconfig.get_int(db, "download_default_max_concurrent") default_mc = sysconfig.get_int(db, "download_default_max_concurrent")
rows = db.execute( rows = db.execute(
@@ -171,6 +175,10 @@ def _claim_job() -> int | None:
""" """
SELECT id, user_id FROM download_jobs SELECT id, user_id FROM download_jobs
WHERE status='queued' WHERE status='queued'
AND NOT EXISTS (
SELECT 1 FROM users u
WHERE u.id = download_jobs.user_id AND u.is_suspended
)
AND NOT EXISTS ( AND NOT EXISTS (
SELECT 1 FROM media_assets a SELECT 1 FROM media_assets a
WHERE a.id = download_jobs.asset_id AND a.status = 'downloading' WHERE a.id = download_jobs.asset_id AND a.status = 'downloading'
+2 -1
View File
@@ -696,9 +696,10 @@ def test_a_clip_larger_than_what_its_owner_has_left_is_refused_and_not_placed(
job = _clip(as_user(make_user()), source=source, ref=ref).json() job = _clip(as_user(make_user()), source=source, ref=ref).json()
monkeypatch.setattr(quota, "remaining_bytes", lambda _db, _uid: 3) # the cut is 4 bytes monkeypatch.setattr(quota, "remaining_bytes", lambda _db, _uid: 3) # the cut is 4 bytes
job, asset = worker["run"](job) job, asset = worker["run"](job)
# Not "full": there is room left, just not this much.
assert (job.status, job.error, asset.status, asset.rel_path) == ( assert (job.status, job.error, asset.status, asset.rel_path) == (
"error", "error",
dlerrors.QUOTA_FULL_MESSAGE, dlerrors.CLIP_TOO_BIG_MESSAGE,
"pending", "pending",
None, None,
) )
+1
View File
@@ -63,6 +63,7 @@ def test_our_own_worker_refusals_classify_as_themselves():
# fragment of the text. Pinning message and rule together is what stops an edit to the wording # fragment of the text. Pinning message and rule together is what stops an edit to the wording
# from silently orphaning the category — the UI would then fall back to "Download failed". # from silently orphaning the category — the UI would then fall back to "Download failed".
assert classify(dlerrors.QUOTA_FULL_MESSAGE) == "quota_exceeded" assert classify(dlerrors.QUOTA_FULL_MESSAGE) == "quota_exceeded"
assert classify(dlerrors.CLIP_TOO_BIG_MESSAGE) == "quota_exceeded"
def test_the_redacted_network_text_classifies_back_to_network(): def test_the_redacted_network_text_classifies_back_to_network():
+19
View File
@@ -155,6 +155,25 @@ def test_unlimited_user_is_never_excluded(db):
assert _claim_job() == q.id assert _claim_job() == q.id
def test_a_suspended_users_queued_jobs_wait(db):
# Suspending a user blocks their downloads (quota.resolve's advice): a job they queued before
# stays queued, unclaimed, and runs once the suspension is lifted.
s = User(email="suspended@example.com", is_suspended=True)
o = User(email="other@example.com")
db.add_all([s, o])
db.flush()
waits = _job(db, s.id, queue_pos=1)
served = _job(db, o.id, queue_pos=2)
db.commit()
assert _claim_job() == served.id
assert _claim_job() is None
db.expire_all()
assert db.get(DownloadJob, waits.id).status == "queued"
db.get(User, s.id).is_suspended = False
db.commit()
assert _claim_job() == waits.id
# --- #3 · list cap -------------------------------------------------------------------------- # --- #3 · list cap --------------------------------------------------------------------------
@@ -247,6 +247,10 @@
} }
}, },
"edit": { "edit": {
"quota_exceeded": {
"short": "Not enough storage for the clip",
"body": "This clip doesn't fit in the storage you have left. Delete something from your library, or cut a shorter clip, then try again."
},
"unknown": { "unknown": {
"short": "Couldn't make the clip", "short": "Couldn't make the clip",
"body": "Something went wrong while making this clip. The technical details below may help." "body": "Something went wrong while making this clip. The technical details below may help."
+1 -1
View File
@@ -45,7 +45,7 @@
"undoAction": "Unsuspend", "undoAction": "Unsuspend",
"title": "Suspend account?", "title": "Suspend account?",
"undoTitle": "Unsuspend account?", "undoTitle": "Unsuspend account?",
"confirm": "Suspend {{email}}? They won't be able to sign in (by password or Google) until you unsuspend them, and any active session ends.", "confirm": "Suspend {{email}}? They won't be able to sign in (by password or Google) until you unsuspend them, and any active session ends. Their queued downloads wait until then.",
"undoConfirm": "Unsuspend {{email}}? They'll be able to sign in again.", "undoConfirm": "Unsuspend {{email}}? They'll be able to sign in again.",
"done": "{{email}} suspended", "done": "{{email}} suspended",
"undone": "{{email}} unsuspended", "undone": "{{email}} unsuspended",
@@ -247,6 +247,10 @@
} }
}, },
"edit": { "edit": {
"quota_exceeded": {
"short": "Nincs elég hely a klipnek",
"body": "Ez a klip nem fér bele a megmaradt tárhelyedbe. Törölj valamit a könyvtáradból, vagy vágj rövidebb klipet, majd próbáld újra."
},
"unknown": { "unknown": {
"short": "Nem sikerült elkészíteni a klipet", "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." "body": "Hiba történt a klip készítése közben. Az alábbi technikai részletek segíthetnek."
+1 -1
View File
@@ -45,7 +45,7 @@
"undoAction": "Feloldás", "undoAction": "Feloldás",
"title": "Felfüggeszted a fiókot?", "title": "Felfüggeszted a fiókot?",
"undoTitle": "Feloldod a felfüggesztést?", "undoTitle": "Feloldod a felfüggesztést?",
"confirm": "Felfüggeszted: {{email}}? Nem fog tudni belépni (sem jelszóval, sem Google-lel), amíg fel nem oldod, és az aktív munkamenete megszűnik.", "confirm": "Felfüggeszted: {{email}}? Nem fog tudni belépni (sem jelszóval, sem Google-lel), amíg fel nem oldod, és az aktív munkamenete megszűnik. A sorban álló letöltései addig várnak.",
"undoConfirm": "Feloldod a felfüggesztést: {{email}}? Újra be tud majd lépni.", "undoConfirm": "Feloldod a felfüggesztést: {{email}}? Újra be tud majd lépni.",
"done": "{{email}} felfüggesztve", "done": "{{email}} felfüggesztve",
"undone": "{{email}} felfüggesztése feloldva", "undone": "{{email}} felfüggesztése feloldva",
+16
View File
@@ -84,6 +84,22 @@ describe("failTitleKey", () => {
} }
}); });
it("a clip refused for storage says so of a clip, and not that the quota is used up", async () => {
// Refused before it is cut (nothing left) or after (the cut is bigger than what is left): one
// sentence true of both, worded for a clip and naming the clip-side way out.
for (const job_kind of ["clip", "edit"] as const) {
const clip = { error_code: "quota_exceeded", error_detail: null, job_kind } 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(`downloads.errors.fail.cat.quota_exceeded.${part}`));
}
}
}
});
it("a clip that fails for no known reason says so of a clip, in both languages", async () => { 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. // 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 clip = { error_code: "unknown", error_detail: null, job_kind: "edit" } as const;