peter/siftlode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(reader): the Takeout import holds up under abuse, races and a run it cannot see

Browse Source 
- Uploads: one at a time per user and ten an hour, each refused only after its body arrived;
  the history read an entry at a time (32 MiB cap), a Watch later list capped at YouTube's 5000,
  a user's kept videos at 200k; any unreadable shape is a 422, not a 500.
- Quota: Watch later lookups stop at the backfill reserve and at a per-user daily share; what is
  not asked about waits, known videos still go in, and the shorts job retries the rest.
- Staging and settling share one per-user advisory lock (no deadlock); a run settles only the
  rows it read, so a row staged meanwhile is never called gone.
- A failed run's notice names a translated failure, never the upstream's text.
- The status says when an import is queued; the page watches it to its end even when left, then
  reads the feed, its counts and the playlists again.
- Settings: one live region, the zone kept (unavailable) while a run goes, focus handed back,
  a missed drop kept from the browser, an oversize file refused before it is sent.
- Scheduler and run lists show Takeout runs by the importing user.
This commit is contained in:
peter
2026-10-23 20:48:23 +02:00
parent e0b02a0c14
commit 9f580fd8c8
29 changed files with 1235 additions and 213 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/test_takeout.py
+335 -11
View File
@@ -3,16 +3,20 @@ import job makes of them — watched here (never a Short, never over a state the
a video the catalog does not know yet, and the Watch later list joined to the user's own."""
import json
import tracemalloc
from datetime import datetime, timedelta, timezone
import pytest
from sqlalchemy import select
from sqlalchemy import select, text
from sqlalchemy.exc import OperationalError
from app import jobruns, scheduler
from app import jobruns, quota, scheduler
from app.db import SessionLocal
from app.catalog.models import Channel, Video
from app.models import JobRun, Notification
from app.models import JobRun, Notification, User
from app.runstatus import RunStatus, RunTrigger
from app.reader import service, takeout
from app.reader import queries, service, takeout
from app.routes import feed as feed_routes
from app.reader.models import (
Playlist,
PlaylistItem,
@@ -107,6 +111,18 @@ def test_the_watch_later_csv_is_read_with_a_header_in_any_language_or_none():
json.dumps([_view("aaaaaaaaaaa", details=[{"name": "From Google Ads"}])]).encode(),
"no_videos",
),
# Read an entry at a time, the list must still be one: nothing after it, nothing missing.
(_history(_view("aaaaaaaaaaa")) + b" []", "not_takeout"),
(_history(_view("aaaaaaaaaaa"))[:-1], "not_takeout"),
(b"[" + json.dumps(_view("aaaaaaaaaaa")).encode() + b" x]", "not_takeout"),
# However a hostile file fails, it is one the import cannot read, not a server error.
(b"[" * 100_000, "not_takeout"),
(b"[" + b"1" * 5000 + b"]", "not_takeout"),
(b"Ngb04xUK8uU," + b"x" * 200_000 + b"\n", "not_takeout"),
(
"".join(f"v{n:010d},2019-02-07T15:23:30Z\n" for n in range(5001)).encode(),
"too_many",
),
],
)
def test_a_file_the_import_cannot_use_is_refused_with_its_reason(body, reason):
@@ -115,6 +131,42 @@ def test_a_file_the_import_cannot_use_is_refused_with_its_reason(body, reason):
assert caught.value.reason == reason
def test_the_history_is_read_an_entry_at_a_time_not_as_one_tree():
"""A file of a million tiny entries must cost about its own size, not a million dicts at once:
the whole tree built first measured ~25 times the file."""
body = b"[" + b",".join([b"{}"] * 1_000_000) + b"]" # 3 MB
tracemalloc.start()
try:
with pytest.raises(takeout.TakeoutError):
takeout.parse(body)
_, peak = tracemalloc.get_traced_memory()
finally:
tracemalloc.stop()
assert peak < 5 * len(body), f"parsing held {peak / len(body):.1f} times the file"
def test_the_id_lists_are_asked_in_chunks(db, make_user, monkeypatch):
user = make_user("chunks@example.com")
ids = [f"chunk{n:06d}" for n in range(5)]
for vid in ids[:3]:
_video(db, vid)
db.commit()
service.stage_takeout(db, user, TakeoutKind.WATCHED, {vid: (T0, None) for vid in ids[:4]})
db.commit()
monkeypatch.setattr(queries, "_IN_CHUNK", 2)
assert queries.known_video_ids(db, ids) == set(ids[:3])
assert queries.takeout_rows_besides(db, user, TakeoutKind.WATCHED, ids) == 0
assert queries.takeout_rows_besides(db, user, TakeoutKind.WATCHED, ids[:1]) == 3
def test_the_history_is_read_whatever_its_whitespace():
body = b" \n[ \n" + json.dumps(_view("aaaaaaaaaaa")).encode() + b" ,\n\t"
body += json.dumps(_view("bbbbbbbbbbb")).encode() + b"\n] \n"
assert sorted(takeout.parse(body).videos) == ["aaaaaaaaaaa", "bbbbbbbbbbb"]
full = "".join(f"v{n:010d},2019-02-07T15:23:30Z\n" for n in range(5000))
assert len(takeout.parse(full.encode()).videos) == 5000 # YouTube's own cap still fits
# --- keeping and applying ----------------------------------------------------------------------
@@ -234,10 +286,15 @@ def test_the_watch_later_list_joins_the_users_own_oldest_first_after_what_it_hol
},
)
counts = service.apply_takeout_watch_later(db, user)
service.stage_takeout(db, user, TakeoutKind.WATCH_LATER, {"unasked0000": (T0, None)})
rows = queries.takeout_waiting(db, user, TakeoutKind.WATCH_LATER)
counts = service.apply_takeout_watch_later(db, user, rows, asked={"gone0000000"})
db.commit()
assert counts == {"watch_later_added": 2, "watch_later_present": 1, "watch_later_missing": 1}
# Unknown and not asked about: it waits, it is not called gone.
assert _rows(db, user)["WL:unasked0000"].outcome is None
items = db.execute(
select(PlaylistItem.video_id)
.where(PlaylistItem.playlist_id == pl.id)
@@ -265,16 +322,17 @@ def test_the_job_fetches_the_unknown_watch_later_videos_and_says_what_waits(
asked = []
def fake_ensure(session, yt, ids):
asked.append(ids)
# The lookup is the importing user's spend, on the import's own action.
asked.append((ids, quota._actor_id.get(), quota._action.get()))
_video(session, "fetched0000")
session.commit()
session.flush()
monkeypatch.setattr(takeout, "ensure_videos", fake_ensure)
monkeypatch.setattr(takeout, "get_service_user", lambda session: user)
result = takeout.run_import(user.id)
assert asked == [["fetched0000"]]
assert asked == [(["fetched0000"], user.id, quota.QuotaAction.VIDEOS_LOOKUP)]
assert result == {
"watched": 1,
"kept": 0,
@@ -286,13 +344,152 @@ def test_the_job_fetches_the_unknown_watch_later_videos_and_says_what_waits(
}
def _wl_job(db, make_user, monkeypatch, email, *, fetcher=True, reserve=False):
user = make_user(email)
_video(db, "known000000")
db.commit()
service.stage_takeout(
db,
user,
TakeoutKind.WATCH_LATER,
{"known000000": (T0, None), "unknown0000": (T0 + timedelta(minutes=1), None)},
)
db.commit()
asked = []
monkeypatch.setattr(takeout, "ensure_videos", lambda session, yt, ids: asked.append(ids))
monkeypatch.setattr(takeout, "get_service_user", lambda session: user if fetcher else None)
monkeypatch.setattr(takeout.quota, "backfill_reserve_reached", lambda: reserve)
return user, asked
@pytest.mark.parametrize(
"why", ["no account to ask with", "the budget's reserve", "the user's share"]
)
def test_a_watch_later_video_not_asked_about_waits_and_the_known_ones_go_in(
db, make_user, monkeypatch, why
):
user, asked = _wl_job(
db,
make_user,
monkeypatch,
"wait@example.com",
fetcher=why != "no account to ask with",
reserve=why == "the budget's reserve",
)
if why == "the user's share":
monkeypatch.setattr(takeout, "_lookups", takeout.RateLimiter(0, 3600))
result = takeout.run_import(user.id)
assert asked == []
assert (result["watch_later_added"], result["watch_later_missing"]) == (1, 0)
assert result["watch_later_waiting"] == 1
assert _rows(db, user)["WL:unknown0000"].outcome is None
def test_the_shorts_job_puts_in_the_watch_later_videos_a_run_could_not(db, make_user, monkeypatch):
user, asked = _wl_job(db, make_user, monkeypatch, "retry@example.com", fetcher=False)
takeout.run_import(user.id)
assert asked == []
# A YouTube-connected account exists now; the next probe run asks for what waits.
monkeypatch.setattr(takeout, "get_service_user", lambda session: user)
def fetched(session, yt, ids):
asked.append(ids)
_video(session, "unknown0000")
session.flush()
monkeypatch.setattr(takeout, "ensure_videos", fetched)
assert takeout.apply_waiting(db) == {"takeout_watch_later_added": 1}
assert asked == [["unknown0000"]]
assert _rows(db, user)["WL:unknown0000"].outcome == "added"
assert takeout.apply_waiting(db) == {}
def test_a_watch_later_row_staged_while_youtube_is_asked_is_left_for_the_next_run(
db, make_user, monkeypatch
):
user, _ = _wl_job(db, make_user, monkeypatch, "race@example.com")
def meanwhile(session, yt, ids):
# A second upload lands while the lookup runs: its video was never asked about.
with SessionLocal() as other:
service.stage_takeout(
other,
other.get(User, user.id),
TakeoutKind.WATCH_LATER,
{"second00000": (T0, None)},
)
other.commit()
monkeypatch.setattr(takeout, "ensure_videos", meanwhile)
takeout.run_import(user.id)
rows = _rows(db, user)
assert rows["WL:unknown0000"].outcome == "missing" # asked, and YouTube has it no longer
assert rows["WL:second00000"].outcome is None
def test_a_youtube_failure_keeps_what_the_watched_part_already_marked(db, make_user, monkeypatch):
user = make_user("fails@example.com")
_video(db, "plain000000")
db.commit()
service.stage_takeout(db, user, TakeoutKind.WATCHED, {"plain000000": (T0, CHANNEL)})
service.stage_takeout(db, user, TakeoutKind.WATCH_LATER, {"unknown0000": (T0, None)})
db.commit()
def refused(session, yt, ids):
raise RuntimeError("quota exceeded")
monkeypatch.setattr(takeout, "ensure_videos", refused)
monkeypatch.setattr(takeout, "get_service_user", lambda session: user)
with pytest.raises(RuntimeError):
takeout.run_import(user.id)
with SessionLocal() as fresh:
state = fresh.execute(select(VideoState).where(VideoState.user_id == user.id)).scalar_one()
assert (state.video_id, state.status) == ("plain000000", "watched")
outcome = fresh.execute(
select(TakeoutItem.outcome).where(TakeoutItem.video_id == "plain000000")
).scalar_one()
assert outcome == "watched"
def test_staging_and_settling_take_the_users_one_lock(db, make_user):
"""An upload's staging and an import's settling lock the same rows in no common order: one lock
over both is what keeps the two from deadlocking."""
user = make_user("lock@example.com")
db.commit()
steps = {
"stage": lambda s, u: service.stage_takeout(
s, u, TakeoutKind.WATCHED, {"aaaaaaaaaaa": (T0, None)}
),
"watched": lambda s, u: service.apply_takeout_watched(s, u),
"watch later": lambda s, u: service.apply_takeout_watch_later(
s, u, [(1, "aaaaaaaaaaa")], set()
),
}
with SessionLocal() as holder:
service.lock_takeout(holder, holder.get(User, user.id))
for name, step in steps.items():
with SessionLocal() as other:
other.execute(text("SET lock_timeout = '200ms'"))
with pytest.raises(OperationalError, match="lock timeout"):
step(other, other.get(User, user.id))
pytest.fail(f"{name} did not wait for the user's lock")
holder.rollback()
# --- the upload --------------------------------------------------------------------------------
@pytest.fixture
def queued(monkeypatch):
runs = []
monkeypatch.setattr(scheduler, "run_ad_hoc", lambda job, fn, **kw: runs.append((job, kw)))
monkeypatch.setattr(
scheduler, "run_ad_hoc", lambda job, fn, **kw: runs.append((job, fn.func, fn.args, kw))
)
return runs
@@ -318,6 +515,8 @@ def test_an_upload_keeps_its_videos_and_queues_the_users_import_with_a_notice(
assert queued == [
(
"takeout_import",
takeout.run_import,
(user.id,),
{"actor_id": user.id, "target": f"user:{user.id}", "notify": True},
)
]
@@ -345,8 +544,87 @@ def test_an_upload_over_the_cap_is_refused_while_it_arrives(db, make_user, as_us
assert _rows(db, user) == {}
def test_an_anonymous_upload_is_refused(client):
assert client.post("/api/takeout", content=_history(_view("aaaaaaaaaaa"))).status_code == 401
def _counting_body(chunks: int = 4, size: int = 1024):
"""A body that records how much of itself the server pulled (test_upload.py's probe)."""
pulled: list[int] = []
def body():
for _ in range(chunks):
pulled.append(size)
yield b"x" * size
return body(), pulled
def test_an_anonymous_upload_is_refused_before_a_byte_of_it_is_read(client):
body, pulled = _counting_body()
assert client.post("/api/takeout", content=body).status_code == 401
assert pulled == [], f"the server read {sum(pulled)} bytes from an anonymous caller"
def test_a_signed_in_upload_has_its_body_read(db, make_user, as_user, queued):
"""The mutation check for the test above: a route reading nobody's body would pass it too."""
body, pulled = _counting_body()
c = as_user(make_user("read@example.com"))
assert c.post("/api/takeout", content=body).status_code == 422
assert len(pulled) == 4
def test_uploads_past_the_hourly_limit_are_refused_after_their_body_arrived(
db, make_user, as_user, queued
):
c = as_user(make_user("many@example.com"))
for _ in range(10):
assert c.post("/api/takeout", content=_history(_view("aaaaaaaaaaa"))).status_code == 200
body, pulled = _counting_body()
resp = c.post("/api/takeout", content=body)
assert resp.status_code == 429
assert resp.json()["detail"] == {"code": "refused", "reason": "takeout_cooldown"}
# Read to the end first: refused mid-upload, the browser would say "network error" instead.
assert len(pulled) == 4
assert len(queued) == 10
def test_a_second_upload_while_one_of_the_users_is_still_arriving_is_refused(
db, make_user, as_user, queued, monkeypatch
):
user = make_user("busy@example.com")
monkeypatch.setattr(feed_routes, "_takeout_uploading", {user.id})
resp = as_user(user).post("/api/takeout", content=_history(_view("aaaaaaaaaaa")))
assert resp.status_code == 429
assert resp.json()["detail"]["reason"] == "takeout_busy"
assert queued == []
# Another user's upload is not held up by it, and lets go of its own place when it is done.
other = as_user(make_user("free@example.com"))
assert other.post("/api/takeout", content=_history(_view("aaaaaaaaaaa"))).status_code == 200
assert feed_routes._takeout_uploading == {user.id}
def test_an_upload_that_would_keep_too_many_videos_is_refused(
db, make_user, as_user, queued, monkeypatch
):
user = make_user("cap@example.com")
c = as_user(user)
monkeypatch.setattr(takeout, "ROWS_MAX", 3)
two = _history(_view("aaaaaaaaaaa"), _view("bbbbbbbbbbb"))
assert c.post("/api/takeout", content=two).status_code == 200
assert c.post("/api/takeout", content=two).status_code == 200 # the same videos again: no more
resp = c.post("/api/takeout", content=_history(_view("ccccccccccc"), _view("ddddddddddd")))
assert resp.status_code == 422
assert resp.json()["detail"] == {"code": "refused", "reason": "takeout_too_many", "limit": 3}
assert sorted(_rows(db, user)) == ["aaaaaaaaaaa", "bbbbbbbbbbb"]
assert len(queued) == 2
def test_a_watch_later_file_longer_than_youtube_allows_is_refused(db, make_user, as_user, queued):
body = "".join(f"v{n:010d},2019-02-07T15:23:30Z\n" for n in range(5001)).encode()
resp = as_user(make_user("long@example.com")).post("/api/takeout", content=body)
assert resp.status_code == 422
assert resp.json()["detail"] == {
"code": "refused",
"reason": "takeout_list_too_long",
"limit": 5000,
}
# --- the notice --------------------------------------------------------------------------------
@@ -363,6 +641,50 @@ def test_an_ad_hoc_run_that_asks_for_it_tells_its_user_how_it_went(db, make_user
]
def test_a_failed_run_tells_its_user_the_failure_never_the_upstreams_text(db, make_user):
user = make_user("failnotice@example.com")
def fails():
raise RuntimeError("(psycopg.errors.Something) [SQL: SELECT secret]")
scheduler._ad_hoc("takeout_import", fails, user.id, "user:1", True)
scheduler._ad_hoc("channel_backfill", fails, user.id, "UC_x")
db.expire_all()
(notice,) = db.execute(select(Notification).where(Notification.user_id == user.id)).scalars()
assert notice.body is None
assert notice.data == {
"job_id": "takeout_import",
"status": "error",
"summary": None,
"result": {"failure": "unexpected"},
}
# The admin's run row keeps the diagnostic.
run = db.execute(select(JobRun).where(JobRun.job == "takeout_import")).scalar_one()
assert "SELECT secret" in run.error
def test_a_queued_import_is_told_apart_from_the_last_one_until_its_run_is_recorded(
db, make_user, as_user, monkeypatch
):
user = make_user("queued@example.com")
added = []
class Held:
def add_job(self, *args, **kwargs):
added.append(kwargs["id"])
monkeypatch.setattr(scheduler, "_scheduler", Held())
target = takeout.run_target(user.id)
scheduler.run_ad_hoc("takeout_import", dict, actor_id=user.id, target=target)
assert scheduler.ad_hoc_queued("takeout_import", target)
assert as_user(user).get("/api/takeout").json()["queued"] is True
scheduler._ad_hoc("takeout_import", dict, user.id, target)
assert not scheduler.ad_hoc_queued("takeout_import", target)
assert as_user(user).get("/api/takeout").json()["queued"] is False
assert added == [f"takeout_import@{target}"]
# --- the videos that wait ----------------------------------------------------------------------
@@ -410,6 +732,8 @@ def test_the_shorts_job_settles_the_waiting_rows_its_probe_made_ready(db, make_u
def test_the_status_is_the_users_own_last_run_and_counts(db, make_user, as_user):
user, other = make_user("status@example.com"), make_user("other@example.com")
assert as_user(user).get("/api/takeout").json() == {
"queued": False,
"max_bytes": takeout.MAX_BYTES,
"run": None,
"watched": {},
"watch_later": {},