From b04b51c90931dbe0003c26e74d682722d8e6020c Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 25 Oct 2026 00:32:26 +0200 Subject: [PATCH] fix(search): every related and live-search request takes a slot, and a failed one gives it back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A related list's cursor cost nothing, and a cursor is a free string: a replayed one read and enriched without end. Every request now takes a slot of the user's daily cap — taken under a lock before the work (`quota.claim_action`), so requests sent together cannot all pass the last free one, and given back when the request fails outright (`quota.refund_action`). The live search's scrape source takes its slots the same way. A limit of 0 turns the feature off: a distinct 409 `related_disabled` / `search_disabled`, and `/api/me` says `related_enabled` / `search_enabled` so the UI can leave it out. A videos.list that fails no longer reads as an empty page: `ingest_candidates` raises, and the search and the related list answer the refusal (the shared budget spent says "quota") instead of "no results". The two routes' page loops are one (`_gather`). `clear` deletes only what the caller found: a related list's rows carry no find, and any user could name them. --- backend/app/catalog/search.py | 30 +- backend/app/config.py | 6 +- backend/app/quota.py | 24 ++ backend/app/routes/me.py | 4 + backend/app/routes/search.py | 346 ++++++++++++--------- backend/tests/test_me_search_source.py | 24 +- backend/tests/test_related_route.py | 108 ++++++- backend/tests/test_route_commits.py | 17 + backend/tests/test_search_cap.py | 132 ++++++++ backend/tests/test_search_scrape_ingest.py | 22 ++ frontend/src/i18n/locales/en/config.json | 2 +- frontend/src/i18n/locales/en/errors.json | 2 + frontend/src/i18n/locales/hu/config.json | 2 +- frontend/src/i18n/locales/hu/errors.json | 2 + 14 files changed, 556 insertions(+), 165 deletions(-) create mode 100644 backend/tests/test_search_cap.py diff --git a/backend/app/catalog/search.py b/backend/app/catalog/search.py index 95ca4bc7..97a400f4 100644 --- a/backend/app/catalog/search.py +++ b/backend/app/catalog/search.py @@ -75,7 +75,8 @@ def ingest_candidates( ) -> list[str]: """Materialise one page of search candidates into the catalog (channel stubs + enrich, video stubs flagged via_search, enrich, Shorts classification) and return the KEEPABLE - video ids — non-Short, non-live — in the input relevance order.""" + video ids — non-Short, non-live — in the input relevance order. A videos.list that fails + raises its YouTubeError: with nothing enriched, nothing could be kept.""" ids = [it["id"] for it in candidates] if not ids: return [] @@ -132,13 +133,16 @@ def ingest_candidates( with quota.attribute(user.id, quota.QuotaAction.VIDEOS_ENRICH): detail_by_id = {it["id"]: it for it in yt.get_videos(ids)} except YouTubeError: - # Every candidate then stays unenriched, and an unenriched video cannot be judged live or - # Short — so it is dropped. A working search returns nothing and reads as "no results". + # An unenriched video cannot be judged live or Short, so none of this page could be kept: + # swallowed, the failure answered as a page with nothing on it — "no results" for a search + # that never ran (the shared budget spent, no API key). The stubs stay (`insert_stubs` + # committed them), as the channels' do above; the caller answers the failure (its refusal + # says which). log.warning( "the videos found by a search could not be enriched", extra=logs.event("search.video_enrich_failed", candidates=len(ids)), ) - detail_by_id = {} + raise enriched_at = datetime.now( timezone.utc ) # when THIS enrich ran (distinct from the stub placeholder) @@ -212,17 +216,25 @@ def clear(db: Session, user: User, video_ids: list[str]) -> int: """Discard these results "as if never added": drop this user's finds of them, then delete the ones nobody kept — a `via_search` video no reader holds (`reader.queries.kept_by_a_reader`), whose channel nobody subscribes to, and that no other user still has as a find. Returns how - many videos went.""" - db.execute( - delete(SearchFind).where( - owned_by(SearchFind.user_id, user), SearchFind.video_id.in_(video_ids) + many videos went. + + Only the ones this user found: a video in the catalog with no find at all (a related list's, + which records none) is in someone's list right now, and an id any user could name would let + them delete it from under that list.""" + mine = list( + db.scalars( + delete(SearchFind) + .where(owned_by(SearchFind.user_id, user), SearchFind.video_id.in_(video_ids)) + .returning(SearchFind.video_id) ) ) + if not mine: + return 0 subscribed = select(Subscription.id).where(Subscription.channel_id == Video.channel_id).exists() other_find = select(SearchFind.id).where(SearchFind.video_id == Video.id).exists() result = db.execute( delete(Video).where( - Video.id.in_(video_ids), + Video.id.in_(mine), Video.via_search.is_(True), ~reader_queries.kept_by_a_reader(Video.id), ~subscribed, diff --git a/backend/app/config.py b/backend/app/config.py index ffc420b8..eb1ab096 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -113,8 +113,10 @@ class Settings(BaseSettings): # each search.list costs 100 units, so the shared budget only affords ~80-90/day total; with # the zero-quota scrape source it's a pure anti-abuse rate limit. Admin-tunable. search_daily_limit_per_user: int = 70 - # Per-user cap on related-video lists opened per day (the InnerTube watch page, no quota; each - # list's enrich is 1 unit per 50 videos). A pure anti-abuse rate limit. Admin-tunable. + # Per-user cap on related-video requests per day — a list opened and each "Show more" (the + # InnerTube watch page, no quota; its enrich is up to ~8 units: up to 4 pages, each a videos.list + # and a channels.list). A failed one gives its slot back. 0 turns related videos off. + # Admin-tunable. related_daily_limit_per_user: int = 200 # Source for live YouTube search: "scrape" (InnerTube, no API quota) or "api" (search.list, # 100 units/page). Scrape is the default; flip to "api" if scraping is blocked/breaks. diff --git a/backend/app/quota.py b/backend/app/quota.py index 108e45c4..66d4fa4d 100644 --- a/backend/app/quota.py +++ b/backend/app/quota.py @@ -166,6 +166,30 @@ def log_action(user_id: int, action: str) -> None: qs.commit() +def claim_action(user_id: int, action: str, limit: int) -> int | None: + """Take one of this user's `limit` daily `action` slots, or None when they are all taken — the + check and the take in one transaction, under a lock per user and action. Checked first and + logged after the work, N requests sent together at the last free slot all passed the check + (the work in between is seconds of scraping). Returns the event's id, for `refund_action` when + the work then produced nothing. Own session, as `log_action`.""" + with SessionLocal() as qs: + qs.execute(select(func.pg_advisory_xact_lock(func.hashtext(action), user_id))) + if actions_today(qs, user_id, action) >= limit: + return None + event = QuotaEvent(user_id=user_id, action=action, units=0) + qs.add(event) + qs.commit() + return event.id + + +def refund_action(event_id: int) -> None: + """Give a slot `claim_action` took back: the work it was taken for failed outright, and a + request that showed the user nothing must not count against them.""" + with SessionLocal() as qs: + qs.execute(delete(QuotaEvent).where(QuotaEvent.id == event_id)) + qs.commit() + + def actions_today(db: Session, user_id: int, action: str) -> int: """How many quota events of `action` this user has logged so far in the current Pacific day — for per-user, per-action daily caps (e.g. the live-search limit). Counts events, diff --git a/backend/app/routes/me.py b/backend/app/routes/me.py index 0c5101cd..66d9cf02 100644 --- a/backend/app/routes/me.py +++ b/backend/app/routes/me.py @@ -79,6 +79,10 @@ def get_me( # Which live-search source runs, so the search button can say what a search costs: "scrape" # spends no API quota, "api" spends 100 units a page. "search_source": sysconfig.get_str(db, "search_source"), + # Whether live search and related videos are on: an admin turns either off with a daily + # limit of 0, and the UI then leaves out what would only be refused. + "search_enabled": sysconfig.get_int(db, "search_daily_limit_per_user") > 0, + "related_enabled": sysconfig.get_int(db, "related_daily_limit_per_user") > 0, "can_read": has_read_scope(user), "can_write": has_write_scope(user), "pending_invites": pending_invites, diff --git a/backend/app/routes/search.py b/backend/app/routes/search.py index c5386899..06741528 100644 --- a/backend/app/routes/search.py +++ b/backend/app/routes/search.py @@ -20,6 +20,7 @@ cap — with what this user has already seen left out. This is the HTTP side — the paging, the caps and the refusals; what a page of results leaves in the catalog is `catalog/search`.""" +from collections.abc import Callable from typing import Literal from fastapi import APIRouter, Depends, Path @@ -34,6 +35,7 @@ from app.db import get_db from app.models import User from app.reader import queries as reader_queries from app.youtube import related_scrape, search_scrape +from app.youtube.innertube import ScrapeError from app.youtube.client import YouTubeClient, YouTubeError from app.refusal import refused from app.routes._common import youtube_refusal @@ -43,6 +45,8 @@ router = APIRouter(prefix="/api/search", tags=["search"]) # search.list costs this many quota units per page. SEARCH_COST = 100 +# The scrape source reads at most this many pages for one search. +SEARCH_MAX_PAGES = 12 # A related list is filled to this many unseen videos, reading at most this many pages (about 20 # videos each, measured) — the ones the user has seen are left out, so a page alone can fall short. @@ -52,6 +56,66 @@ RELATED_MAX_PAGES = 4 RELATED_REGIONS = {"topical": ("en", "US"), "hu": ("hu", "HU")} +def _gather( + db: Session, + yt: YouTubeClient, + user: User, + *, + cursor: str | None, + max_pages: int, + fetch: Callable[[str | None], dict], + candidates_of: Callable[[list[dict], list[str]], list[dict]], + enough: Callable[[list[str]], bool], + scrape_failed: Callable[[Exception], None], + ingest_failed: Callable[[int, int], None], +) -> tuple[list[str], str | None]: + """Page through a YouTube list into the catalog: fetch a page, keep its `candidates_of`, ingest + them, and go on from its continuation until `enough` or `max_pages`. Returns the kept ids and + the cursor to read further from. A page that fails after earlier ones gathered something ends + the list with what it has; a first page that fails raises the 422 refusal. + + 422, not 502: this is a JSON endpoint, and the FE treats 502/503/504 as a gateway dropout + (auto-retry + "connection lost" banner — lib/api/core RETRIABLE_GATEWAY), which hides the real + message and repeats the work. A Data API failure is logged where it happened; the scraper's is + not, and its text does not ride the answer — so `scrape_failed` says it. An ingest (DB writes + + enrich) fails the same way, guarded for any exception so a write or shape failure is a 422 with + a message too, not a bare 500; `ingest_failed` logs it inside the `except`, with the trace.""" + collected: list[str] = [] + next_cursor = cursor + for _ in range(max_pages): + try: + page = fetch(next_cursor) + except (YouTubeError, ScrapeError) as exc: + if collected: + break # keep what earlier pages already gathered + if isinstance(exc, ScrapeError): + scrape_failed(exc) + raise youtube_refusal(exc) from exc + candidates = candidates_of(page["items"], collected) + try: + collected.extend(catalog_search.ingest_candidates(db, yt, user, candidates)) + except Exception as exc: # any ingest failure, not just YouTubeError + db.rollback() + ingest_failed(len(candidates), len(collected)) + if collected: + break + raise youtube_refusal(exc) from exc + next_cursor = page["next_page_token"] + if enough(collected) or not next_cursor: + break + return collected, next_cursor + + +def _cap_spent(feature: str, limit: int) -> None: + """Say that a request was refused for the per-user daily cap. The 429 is in the request line + either way; which of the caps produced it is not, and they are answered by different things — + this one by tomorrow, the shared budget by the admin.""" + log.info( + "a request was refused: the per-user daily cap is spent", + extra=logs.event(f"{feature}.refused", reason="daily_cap", limit=limit), + ) + + @router.get("/youtube") def search_youtube( q: str | None = None, @@ -72,109 +136,103 @@ def search_youtube( # Source: "scrape" (InnerTube, no API quota) or "api" (search.list, 100 units/page). source = sysconfig.get_str(db, "search_source") - # Per-user daily cap applies to both sources (anti-abuse); with scrape it's the only guard - # since the search itself spends no quota. + # Every request takes a slot of the per-user daily cap, a cursor's too (anti-abuse); with + # scrape it's the only guard since the search itself spends no quota. A limit of 0 turns live + # search off on this instance — a refusal tomorrow will not change. daily_limit = sysconfig.get_int(db, "search_daily_limit_per_user") - used = quota.actions_today(db, user.id, quota.QuotaAction.VIDEOS_SEARCH) - if used >= daily_limit: - # The 429 is in the request line either way; which of the two caps produced it is not, and - # they are answered by different things — this one by tomorrow, the other by the admin. - log.info( - "a live search was refused: the per-user daily cap is spent", - extra=logs.event("search.refused", reason="daily_cap", limit=daily_limit), - ) - raise refused(429, "search_daily_limit", limit=daily_limit) - # The 100-unit budget pre-check only applies to the API source. - if source == "api" and not quota.can_spend(db, SEARCH_COST): - log.info( - "a live search was refused: the shared YouTube budget is spent", - extra=logs.event("search.refused", reason="shared_quota", cost=SEARCH_COST), - ) - raise refused(429, "quota_spent") + if daily_limit <= 0: + raise refused(409, "search_disabled") + claimed: int | None = None + if source == "api": + # The one search.list call logs this action's event itself (record_usage, attributed + # below), so here the cap is only checked: a claimed slot would count it twice. + if quota.actions_today(db, user.id, quota.QuotaAction.VIDEOS_SEARCH) >= daily_limit: + _cap_spent("search", daily_limit) + raise refused(429, "search_daily_limit", limit=daily_limit) + # The 100-unit budget pre-check only applies to the API source. + if not quota.can_spend(db, SEARCH_COST): + log.info( + "a live search was refused: the shared YouTube budget is spent", + extra=logs.event("search.refused", reason="shared_quota", cost=SEARCH_COST), + ) + raise refused(429, "quota_spent") + else: + # The check and the take are one step (`quota.claim_action`); a search that fails + # outright gives the slot back below. + claimed = quota.claim_action(user.id, quota.QuotaAction.VIDEOS_SEARCH, daily_limit) + if claimed is None: + _cap_spent("search", daily_limit) + raise refused(429, "search_daily_limit", limit=daily_limit) # Channels this user has blocked are dropped before ingestion — never shown, never stored. blocked = queries.blocked_ids(db, user) - collected: list[str] = [] - next_cursor = cursor - MAX_PAGES = 12 # scrape safety bound - with YouTubeClient(db, user) as yt: - for _ in range(MAX_PAGES): - try: - if source == "api": - with quota.attribute(user.id, quota.QuotaAction.VIDEOS_SEARCH): - page = yt.search_videos(term, page_token=next_cursor) - else: - page = search_scrape.search(term, continuation=next_cursor) - except (YouTubeError, search_scrape.ScrapeError) as exc: - if collected: - break # keep what we already gathered - # 422, not 502: this is a JSON endpoint, and the FE treats 502/503/504 as a gateway - # dropout (auto-retry + "connection lost" banner — lib/api/core RETRIABLE_GATEWAY), - # which hides the real message and burns a second quota-costed search. Upstream failure - # on a JSON endpoint = 422 + message; reserve 5xx for the raw byte-proxies. (B-core-1) - # A Data API failure is logged where it happened; the scraper's is not, and its text - # no longer rides the answer — so it is said here. - if isinstance(exc, search_scrape.ScrapeError): - log.warning( - "the YouTube search scrape failed", - extra=logs.event("search.scrape_failed", error=str(exc)), - ) - raise youtube_refusal(exc) from exc + def fetch(token: str | None) -> dict: + if source == "api": + with quota.attribute(user.id, quota.QuotaAction.VIDEOS_SEARCH): + return yt.search_videos(term, page_token=token) + return search_scrape.search(term, continuation=token) - # Drop currently-live/upcoming, channel-less, and blocked-channel results. - candidates = [ - it - for it in page["items"] - if it["live_broadcast"] == "none" - and it["channel_id"] - and it["channel_id"] not in blocked - ] - # Ingest (DB writes + enrich) runs outside the page-fetch try above; guard it too so a - # write/shape failure degrades to a 422 with a message on this JSON endpoint instead of a - # bare 500 hidden behind the FE's generic "search failed" (same rationale as B-core-1). - try: - collected.extend(catalog_search.ingest_candidates(db, yt, user, candidates)) - except Exception as exc: # any ingest failure, not just YouTubeError - db.rollback() - # The term is NOT in this line. A search string is exactly what the privacy - # policy keeps out of the log unless the user turned Teljes/TRACE on for - # themselves, and `redact_text` does not cover it — it scrubs addresses and - # credential params, not what someone typed into the search box. What the - # ERROR line carries instead is the shape of the failure, which is what tells - # an ingest bug from an upstream one; the term follows at TRACE, and only - # under this user's own consent — not whenever the process runs at TRACE. - log.exception( - "live search ingest failed", - extra=logs.event( - "search.live.ingest_failed", - source=source, - candidates=len(candidates), - collected=len(collected), - ), - ) - if logs.consented_trace(): - log.log( - logs.TRACE, - "the term the ingest failed on", - # log-pii-ok: only under this user's own Teljes consent - extra=logs.event("search.live.term", term=term), - ) - if collected: - break # keep what earlier pages already gathered - raise youtube_refusal(exc) from exc - next_cursor = page["next_page_token"] - # The API source pages cost 100 units each — never auto-page; one page per request. - if source == "api" or len(collected) >= limit or not next_cursor: - break + def candidates_of(items: list[dict], _collected: list[str]) -> list[dict]: + # Drop currently-live/upcoming, channel-less, and blocked-channel results. + return [ + it + for it in items + if it["live_broadcast"] == "none" + and it["channel_id"] + and it["channel_id"] not in blocked + ] - # Count this as ONE per-user search against the daily cap. The scrape source spends no - # quota, so it never logs an event on its own; the API source already logged exactly one - # (via record_usage — it never auto-pages). Logging once here, after a search that produced - # at least one page (a total failure raises 502 above and never reaches this), keeps the cap - # counting user searches instead of internal continuation pages. - if source != "api": - quota.log_action(user.id, quota.QuotaAction.VIDEOS_SEARCH) + def scrape_failed(exc: Exception) -> None: + log.warning( + "the YouTube search scrape failed", + extra=logs.event("search.scrape_failed", error=str(exc)), + ) + + def ingest_failed(candidates: int, collected: int) -> None: + # The term is NOT in this line. A search string is exactly what the privacy policy keeps + # out of the log unless the user turned Teljes/TRACE on for themselves, and `redact_text` + # does not cover it — it scrubs addresses and credential params, not what someone typed + # into the search box. What the ERROR line carries instead is the shape of the failure, + # which is what tells an ingest bug from an upstream one; the term follows at TRACE, and + # only under this user's own consent — not whenever the process runs at TRACE. + log.exception( + "live search ingest failed", + extra=logs.event( + "search.live.ingest_failed", + source=source, + candidates=candidates, + collected=collected, + ), + ) + if logs.consented_trace(): + log.log( + logs.TRACE, + "the term the ingest failed on", + # log-pii-ok: only under this user's own Teljes consent + extra=logs.event("search.live.term", term=term), + ) + + try: + with YouTubeClient(db, user) as yt: + collected, next_cursor = _gather( + db, + yt, + user, + cursor=cursor, + # The API source pages cost 100 units each — never auto-page; one page per request. + max_pages=1 if source == "api" else SEARCH_MAX_PAGES, + fetch=fetch, + candidates_of=candidates_of, + enough=lambda got: len(got) >= limit, + scrape_failed=scrape_failed, + ingest_failed=ingest_failed, + ) + except Exception: + # A search that showed the user nothing does not count against them. + if claimed is not None: + quota.refund_action(claimed) + raise ordered = collected[:limit] @@ -228,62 +286,58 @@ def related_videos( without the ones this user has watched or hidden here or in their Takeout history, and without blocked channels. Pages on (`cursor`) until `RELATED_TARGET` are gathered; `next_cursor` reads further. `region` picks the locale YouTube fills the list in (`RELATED_REGIONS`).""" + # Every request takes a slot, a cursor's too: each one scrapes pages and enriches what they + # hold, and a cursor is a free string — counting only first pages let a replayed cursor read + # without end. The check and the take are one step (`quota.claim_action`); a request that fails + # outright gives its slot back. A limit of 0 turns related videos off on this instance. daily_limit = sysconfig.get_int(db, "related_daily_limit_per_user") - used = quota.actions_today(db, user.id, quota.QuotaAction.VIDEOS_RELATED) - if used >= daily_limit: - log.info( - "a related list was refused: the per-user daily cap is spent", - extra=logs.event("related.refused", reason="daily_cap", limit=daily_limit), - ) + if daily_limit <= 0: + raise refused(409, "related_disabled") + claimed = quota.claim_action(user.id, quota.QuotaAction.VIDEOS_RELATED, daily_limit) + if claimed is None: + _cap_spent("related", daily_limit) raise refused(429, "related_daily_limit", limit=daily_limit) hl, gl = RELATED_REGIONS[region] blocked = queries.blocked_ids(db, user) - collected: list[str] = [] - next_cursor = cursor - with YouTubeClient(db, user) as yt: - for _ in range(RELATED_MAX_PAGES): - try: - page = related_scrape.related(video_id, continuation=next_cursor, hl=hl, gl=gl) - except related_scrape.ScrapeError as exc: - if collected: - break # keep what earlier pages already gathered - log.warning( - "the related-videos scrape failed", - extra=logs.event("related.scrape_failed", error=str(exc)), - ) - raise youtube_refusal(exc) from exc - ids = [it["id"] for it in page["items"]] - drop = reader_queries.seen_by(db, user, ids) | set(collected) | {video_id} - candidates = [ - it - for it in page["items"] - if it["id"] not in drop and it["channel_id"] not in blocked - ] - # As in the search above: an ingest failure answers 422, not a bare 500. - try: - collected.extend(catalog_search.ingest_candidates(db, yt, user, candidates)) - except Exception as exc: # any ingest failure, not just YouTubeError - db.rollback() - log.exception( - "a related list's ingest failed", - extra=logs.event( - "related.ingest_failed", - candidates=len(candidates), - collected=len(collected), - ), - ) - if collected: - break - raise youtube_refusal(exc) from exc - next_cursor = page["next_page_token"] - if len(collected) >= RELATED_TARGET or not next_cursor: - break - # One opened list is one event against the cap, however many pages it read; reading further - # (a cursor) is the same list. - if cursor is None: - quota.log_action(user.id, quota.QuotaAction.VIDEOS_RELATED) + def candidates_of(items: list[dict], collected: list[str]) -> list[dict]: + ids = [it["id"] for it in items] + drop = reader_queries.seen_by(db, user, ids) | set(collected) | {video_id} + return [it for it in items if it["id"] not in drop and it["channel_id"] not in blocked] + + def scrape_failed(exc: Exception) -> None: + log.warning( + "the related-videos scrape failed", + extra=logs.event("related.scrape_failed", error=str(exc)), + ) + + def ingest_failed(candidates: int, collected: int) -> None: + log.exception( + "a related list's ingest failed", + extra=logs.event("related.ingest_failed", candidates=candidates, collected=collected), + ) + + try: + with YouTubeClient(db, user) as yt: + collected, next_cursor = _gather( + db, + yt, + user, + cursor=cursor, + max_pages=RELATED_MAX_PAGES, + fetch=lambda token: related_scrape.related( + video_id, continuation=token, hl=hl, gl=gl + ), + candidates_of=candidates_of, + enough=lambda got: len(got) >= RELATED_TARGET, + scrape_failed=scrape_failed, + ingest_failed=ingest_failed, + ) + except Exception: + quota.refund_action(claimed) + raise + rows = reader_queries.feed_rows_by_ids(db, user, collected) # A related list mixes channels the user follows with ones they never met: each row says which. followed = queries.followed_among(db, user, {r["channel_id"] for r in rows}) diff --git a/backend/tests/test_me_search_source.py b/backend/tests/test_me_search_source.py index 7f5bb2f3..b3c110a4 100644 --- a/backend/tests/test_me_search_source.py +++ b/backend/tests/test_me_search_source.py @@ -1,5 +1,6 @@ """`/api/me` names the live-search source, so the search button's tooltip can say what a search costs -under the source that actually runs (the default website source spends no API quota).""" +under the source that actually runs (the default website source spends no API quota), and says +whether live search and related videos are on at all.""" from app import sysconfig @@ -24,3 +25,24 @@ def test_me_names_the_running_search_source(client, db, make_user): sysconfig.set_value(db, "search_source", "api") db.commit() assert _me(client)["search_source"] == "api" + + +def test_me_says_whether_live_search_and_related_videos_are_on(client, db, make_user): + # A daily limit of 0 turns the feature off; the UI leaves it out instead of offering a refusal. + user = make_user(password=_PW) + login = client.post("/auth/password-login", json={"email": user.email, "password": _PW}) + assert login.status_code == 200 + + me = _me(client) + assert (me["search_enabled"], me["related_enabled"]) == (True, True) + + sysconfig.set_value(db, "search_daily_limit_per_user", 0) + db.commit() + me = _me(client) + assert (me["search_enabled"], me["related_enabled"]) == (False, True) + + sysconfig.set_value(db, "search_daily_limit_per_user", 5) + sysconfig.set_value(db, "related_daily_limit_per_user", 0) + db.commit() + me = _me(client) + assert (me["search_enabled"], me["related_enabled"]) == (True, False) diff --git a/backend/tests/test_related_route.py b/backend/tests/test_related_route.py index 705ffffa..a0f8aa29 100644 --- a/backend/tests/test_related_route.py +++ b/backend/tests/test_related_route.py @@ -6,16 +6,18 @@ watched or hid — here or in their Takeout history, unless taken back to "new" channels and the video itself; it pages on until it has enough; it lands in the catalog the way search finds do; and a failure, or the day's cap, says so instead of answering an empty list.""" +import threading from datetime import datetime, timedelta, timezone import pytest -from sqlalchemy import select +from sqlalchemy import func, select, text from app import quota, sysconfig from app.catalog.models import BlockedChannel, Channel, Subscription, Video from app.models import QuotaEvent from app.reader.models import TakeoutItem, TakeoutKind, VideoState, VideoUnmark from app.routes import search as search_routes +from app.youtube.client import YouTubeError from app.youtube.innertube import ScrapeError T0 = datetime(2026, 10, 1, tzinfo=timezone.utc) @@ -153,6 +155,8 @@ def test_what_the_takeout_history_shows_watched_is_left_out_unless_taken_back( db.commit() pages["pages"] = [_page([_item(n) for n in range(1, 5)], token=None)] assert _ids(as_user(user).get(f"/api/search/related/{SOURCE}")) == [_vid(2), _vid(3), _vid(4)] + # Left out before the ingest: what the user has seen is not stored for a list that hides it. + assert db.get(Video, _vid(1)) is None def test_an_unmark_older_than_the_history_row_does_not_bring_it_back(db, make_user, as_user, pages): @@ -288,14 +292,108 @@ def test_one_list_is_one_event_against_the_days_cap(db, make_user, as_user, page assert len(pages["calls"]) == 2 # the refused one asked YouTube nothing -def test_reading_further_is_the_same_list_not_another_one(db, make_user, as_user, pages): +def test_reading_further_takes_a_slot_too(db, make_user, as_user, pages): + # Each request scrapes and enriches; a cursor is a free string, so one that cost nothing let a + # replayed cursor read without end. user = make_user() - sysconfig.set_value(db, "related_daily_limit_per_user", 2) + sysconfig.set_value(db, "related_daily_limit_per_user", 1) pages["pages"] = [_page([_item(1)], token=None), _page([_item(2)], token=None)] client = as_user(user) assert client.get(f"/api/search/related/{SOURCE}").status_code == 200 - assert client.get(f"/api/search/related/{SOURCE}?cursor=more").status_code == 200 - assert quota.actions_today(db, user.id, quota.QuotaAction.VIDEOS_RELATED) == 1 + refused = client.get(f"/api/search/related/{SOURCE}?cursor=more") + assert refused.status_code == 429 + assert refused.json()["detail"]["reason"] == "related_daily_limit" + assert len(pages["calls"]) == 1 + + +def test_a_list_that_failed_gives_its_slot_back(db, make_user, as_user, pages): + user = make_user() + sysconfig.set_value(db, "related_daily_limit_per_user", 1) + pages["pages"] = [ScrapeError("blocked"), _page([_item(1)], token=None)] + client = as_user(user) + assert client.get(f"/api/search/related/{SOURCE}").status_code == 422 + assert quota.actions_today(db, user.id, quota.QuotaAction.VIDEOS_RELATED) == 0 + assert _ids(client.get(f"/api/search/related/{SOURCE}")) == [_vid(1)] + + +def test_a_zero_limit_turns_related_videos_off(db, make_user, as_user, pages): + user = make_user() + sysconfig.set_value(db, "related_daily_limit_per_user", 0) + resp = as_user(user).get(f"/api/search/related/{SOURCE}") + assert resp.status_code == 409 + assert resp.json()["detail"] == {"code": "refused", "reason": "related_disabled"} + assert pages["calls"] == [] + + +def test_the_last_slot_is_taken_under_a_lock(db, make_user): + # Checked first and logged after seconds of scraping, requests sent together at the last free + # slot all passed. The claim waits for one already taking a slot for this user and action. + user = make_user() + action = quota.QuotaAction.VIDEOS_RELATED + db.execute(select(func.pg_advisory_xact_lock(func.hashtext(action), user.id))) + got: list[int | None] = [] + t = threading.Thread(target=lambda: got.append(quota.claim_action(user.id, action, 1))) + t.start() + t.join(timeout=1) + assert t.is_alive() # it waits for the one holding the lock + db.commit() + t.join(timeout=10) + assert got and got[0] is not None + assert quota.claim_action(user.id, action, 1) is None # the one slot is taken + + +def _videos_list_fails(monkeypatch, *, on_call: int = 1, reason: str = "quotaExceeded") -> None: + """YouTube's videos.list refuses from its `on_call`-th call on.""" + calls = {"n": 0} + real = _FakeYT.get_videos + + def get_videos(self, ids): + calls["n"] += 1 + if calls["n"] >= on_call: + raise YouTubeError("videos -> 403", reason=reason, path="videos") + return real(self, ids) + + monkeypatch.setattr(_FakeYT, "get_videos", get_videos) + + +def test_a_lookup_that_fails_says_why_not_no_videos(db, make_user, as_user, pages, monkeypatch): + # Nothing it found can be kept unenriched; an empty list would read as "nothing related". + user = make_user() + _videos_list_fails(monkeypatch) + pages["pages"] = [_page([_item(1)], token=None)] + resp = as_user(user).get(f"/api/search/related/{SOURCE}") + assert resp.status_code == 422 + assert resp.json()["detail"] == {"code": "youtube", "reason": "quota"} + assert quota.actions_today(db, user.id, quota.QuotaAction.VIDEOS_RELATED) == 0 + + +def test_a_lookup_failing_on_a_later_page_keeps_what_came_before( + db, make_user, as_user, pages, monkeypatch +): + user = make_user() + _videos_list_fails(monkeypatch, on_call=2) + pages["pages"] = [_page([_item(1)], token="page2"), _page([_item(2)], token=None)] + assert _ids(as_user(user).get(f"/api/search/related/{SOURCE}")) == [_vid(1)] + + +def test_an_ingest_that_breaks_the_session_is_rolled_back( + db, make_user, as_user, pages, monkeypatch +): + # A database error leaves the session unusable until it is rolled back: the rows of the pages + # before could not be read for the answer. + user = make_user() + real = search_routes.catalog_search.ingest_candidates + calls = {"n": 0} + + def ingest(db_, yt, user_, candidates): + calls["n"] += 1 + if calls["n"] == 2: + db_.execute(text("SELECT 1/0")) + return real(db_, yt, user_, candidates) + + monkeypatch.setattr(search_routes.catalog_search, "ingest_candidates", ingest) + pages["pages"] = [_page([_item(1)], token="page2"), _page([_item(2)], token=None)] + assert _ids(as_user(user).get(f"/api/search/related/{SOURCE}")) == [_vid(1)] def test_each_row_says_whether_the_user_follows_its_channel(db, make_user, as_user, pages): diff --git a/backend/tests/test_route_commits.py b/backend/tests/test_route_commits.py index 5a6fc0bb..5dea6860 100644 --- a/backend/tests/test_route_commits.py +++ b/backend/tests/test_route_commits.py @@ -291,6 +291,23 @@ def test_search_results_cleared_through_the_route_are_gone(db, make_user, as_use assert db.execute(select(SearchFind)).first() is None +def test_clearing_leaves_a_video_the_caller_never_found(db, make_user, as_user): + # A related list ingests its rows with no find: they are in someone's list right now, and an id + # any user could name must not delete them from under it. + user = make_user() + _video(db, "s1", via_search=True) + _video(db, "s2", via_search=True) + db.add(SearchFind(user_id=user.id, video_id="s1")) + db.commit() + r = as_user(user).post("/api/search/clear", json={"video_ids": ["s1", "s2"]}) + assert r.json() == {"ok": True, "removed": 1} + db.expire_all() + assert db.get(Video, "s1") is None and db.get(Video, "s2") is not None + # Naming only ids it never found removes nothing. + r = as_user(user).post("/api/search/clear", json={"video_ids": ["s2"]}) + assert r.json() == {"ok": True, "removed": 0} + + # --- Plex show and season marks --------------------------------------------------------------- diff --git a/backend/tests/test_search_cap.py b/backend/tests/test_search_cap.py new file mode 100644 index 00000000..447db142 --- /dev/null +++ b/backend/tests/test_search_cap.py @@ -0,0 +1,132 @@ +"""`GET /api/search/youtube` — the per-user daily cap of the live search (scrape source). + +The scrape is replaced by pages the test hands out, and YouTube's videos.list by an offline enrich. +What is held: every request takes a slot, a cursor's too; a search that failed outright gives its +slot back; a zero limit turns live search off, which says so instead of "try again tomorrow"; and a +lookup that fails says why instead of answering "no results".""" + +import pytest + +from app import quota, sysconfig +from app.routes import search as search_routes +from app.youtube.client import YouTubeError +from app.youtube.innertube import ScrapeError + +ACTION = quota.QuotaAction.VIDEOS_SEARCH + + +def _item(n: int) -> dict: + return { + "id": f"srch{n:07d}", + "channel_id": "UCsearch001", + "channel_title": "Chan", + "title": f"Found {n}", + "published_at": None, + "thumbnail_url": None, + "live_broadcast": "none", + } + + +class _FakeYT: + fail_videos = False + + def __init__(self, *_a, **_kw): + pass + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def get_channels(self, ids): + return [] + + def get_videos(self, ids): + if _FakeYT.fail_videos: + raise YouTubeError("videos -> 403", reason="quotaExceeded", path="videos") + return [ + { + "id": i, + "snippet": {"title": f"T {i}", "publishedAt": "2025-01-01T00:00:00Z"}, + "contentDetails": {"duration": "PT30M"}, + "statistics": {}, + "status": {}, + } + for i in ids + ] + + +@pytest.fixture +def pages(monkeypatch, db): + """The scrape answers with the pages in `state["pages"]`, one per call.""" + sysconfig.set_value(db, "shorts_probe_max_seconds", 60) + sysconfig.set_value(db, "search_source", "scrape") + state: dict = {"pages": [], "calls": 0} + + def search(term, continuation=None): + state["calls"] += 1 + page = state["pages"].pop(0) + if isinstance(page, Exception): + raise page + return page + + monkeypatch.setattr(search_routes.search_scrape, "search", search) + monkeypatch.setattr(search_routes, "YouTubeClient", _FakeYT) + monkeypatch.setattr(_FakeYT, "fail_videos", False) + return state + + +def _page(n: int) -> dict: + return {"items": [_item(n)], "next_page_token": None} + + +def test_every_request_takes_a_slot_a_cursors_too(db, make_user, as_user, pages): + user = make_user() + sysconfig.set_value(db, "search_daily_limit_per_user", 1) + pages["pages"] = [_page(1), _page(2)] + client = as_user(user) + assert client.get("/api/search/youtube?q=cats").status_code == 200 + refused = client.get("/api/search/youtube?q=cats&cursor=more") + assert refused.status_code == 429 + assert refused.json()["detail"]["reason"] == "search_daily_limit" + assert pages["calls"] == 1 + + +def test_a_search_that_failed_gives_its_slot_back(db, make_user, as_user, pages): + user = make_user() + sysconfig.set_value(db, "search_daily_limit_per_user", 1) + pages["pages"] = [ScrapeError("blocked"), _page(1)] + client = as_user(user) + assert client.get("/api/search/youtube?q=cats").status_code == 422 + assert quota.actions_today(db, user.id, ACTION) == 0 + assert client.get("/api/search/youtube?q=cats").status_code == 200 + assert quota.actions_today(db, user.id, ACTION) == 1 + + +def test_a_zero_limit_turns_live_search_off(db, make_user, as_user, pages): + user = make_user() + sysconfig.set_value(db, "search_daily_limit_per_user", 0) + resp = as_user(user).get("/api/search/youtube?q=cats") + assert resp.status_code == 409 + assert resp.json()["detail"] == {"code": "refused", "reason": "search_disabled"} + assert pages["calls"] == 0 + + +def test_a_zero_limit_turns_the_api_source_off_too(db, make_user, as_user, pages): + user = make_user() + sysconfig.set_value(db, "search_source", "api") + sysconfig.set_value(db, "search_daily_limit_per_user", 0) + resp = as_user(user).get("/api/search/youtube?q=cats") + assert resp.status_code == 409 + assert resp.json()["detail"]["reason"] == "search_disabled" + + +def test_a_lookup_that_fails_says_why_not_no_results(db, make_user, as_user, pages, monkeypatch): + user = make_user() + monkeypatch.setattr(_FakeYT, "fail_videos", True) + pages["pages"] = [_page(1)] + resp = as_user(user).get("/api/search/youtube?q=cats") + assert resp.status_code == 422 + assert resp.json()["detail"] == {"code": "youtube", "reason": "quota"} + assert quota.actions_today(db, user.id, ACTION) == 0 diff --git a/backend/tests/test_search_scrape_ingest.py b/backend/tests/test_search_scrape_ingest.py index 8d817880..6c38c3cf 100644 --- a/backend/tests/test_search_scrape_ingest.py +++ b/backend/tests/test_search_scrape_ingest.py @@ -13,10 +13,13 @@ a placeholder never becomes the video's permanent date. from datetime import datetime, timezone +import pytest + from app import sysconfig from app.models import User from app.catalog.models import Video from app.catalog.search import ingest_candidates +from app.youtube.client import YouTubeError class _FakeYT: @@ -97,6 +100,25 @@ def test_scrape_ingest_enrich_miss_keeps_nonnull_placeholder(db): assert c.enriched_at is None +class _FailingYT(_FakeYT): + def get_videos(self, ids): + raise YouTubeError("videos -> 403", reason="quotaExceeded", path="videos") + + +def test_a_videos_list_failure_raises_and_keeps_the_stubs(db): + # Swallowed, it answered as a page with nothing on it — "no results" for a search whose lookup + # never ran. The stubs stay: the FK targets, backfilled later. + user = User(email="searcher3@example.com") + db.add(user) + db.commit() + + with pytest.raises(YouTubeError): + ingest_candidates(db, _FailingYT({}), user, [_candidate("vid_ddddddd", "UC_chan_d")]) + + db.rollback() # what the caller does: the stubs were committed before the lookup + assert db.get(Video, "vid_ddddddd") is not None + + def test_search_youtube_route_returns_422_when_ingest_fails(db, monkeypatch, override_dep): # Route-level guard (B-core-1): an ingest failure with nothing yet collected must surface as a 422 # + message on this JSON endpoint (the FE hides 5xx behind a generic "connection lost" + auto-retry, diff --git a/frontend/src/i18n/locales/en/config.json b/frontend/src/i18n/locales/en/config.json index 976c1d44..71fc61e2 100644 --- a/frontend/src/i18n/locales/en/config.json +++ b/frontend/src/i18n/locales/en/config.json @@ -56,7 +56,7 @@ }, "related_daily_limit_per_user": { "label": "Related videos — daily limit per user", - "hint": "Max related-video lists a user can open per day. They come from YouTube's website and spend no search quota; the videos in a list are looked up for 1 unit per 50. A pure anti-abuse limit. Set 0 to turn related videos off." + "hint": "Max related-video requests a user can make per day — opening a list and each Show more count one. They come from YouTube's website and spend no search quota; looking up the videos in one costs up to about 8 units (up to 4 pages, each a video and a channel lookup). A failed one doesn't count. Set 0 to turn related videos off." }, "search_daily_limit_per_user": { "label": "Live search — daily limit per user", diff --git a/frontend/src/i18n/locales/en/errors.json b/frontend/src/i18n/locales/en/errors.json index 0db7a8df..e601b553 100644 --- a/frontend/src/i18n/locales/en/errors.json +++ b/frontend/src/i18n/locales/en/errors.json @@ -188,7 +188,9 @@ "quota_spent": "Today's shared YouTube budget is spent. Try again tomorrow.", "recipient_required": "A recipient email is required", "registration_closed": "Registration is currently closed", + "related_disabled": "Related videos are turned off on this instance.", "related_daily_limit": "You've opened today's limit of related lists ({{limit}}). Try again tomorrow.", + "search_disabled": "Live YouTube search is turned off on this instance.", "search_daily_limit": "You've reached today's YouTube search limit ({{limit}}). Try again tomorrow.", "search_term_required": "Enter something to search for", "session_gone": "The playback session ended", diff --git a/frontend/src/i18n/locales/hu/config.json b/frontend/src/i18n/locales/hu/config.json index aabc465a..b1bc3eb1 100644 --- a/frontend/src/i18n/locales/hu/config.json +++ b/frontend/src/i18n/locales/hu/config.json @@ -56,7 +56,7 @@ }, "related_daily_limit_per_user": { "label": "Kapcsolódó videók — napi limit / felhasználó", - "hint": "Hány kapcsolódóvideó-listát nyithat meg egy felhasználó naponta. A listák a YouTube weboldaláról jönnek, keresési kvótát nem fogyasztanak; a bennük lévő videók lekérése 50-enként 1 egység. Tiszta visszaélés-elleni limit. 0 = a kapcsolódó videók kikapcsolva." + "hint": "Hányszor kérhet kapcsolódó videókat egy felhasználó naponta — egy lista megnyitása és minden „Mutass többet” egynek számít. A YouTube weboldaláról jönnek, keresési kvótát nem fogyasztanak; a bennük lévő videók lekérése kérésenként legfeljebb kb. 8 egység (legfeljebb 4 oldal, mindegyik egy videó- és egy csatornalekérés). A sikertelen nem számít. 0 = a kapcsolódó videók kikapcsolva." }, "search_daily_limit_per_user": { "label": "Élő keresés — napi limit / felhasználó", diff --git a/frontend/src/i18n/locales/hu/errors.json b/frontend/src/i18n/locales/hu/errors.json index 6ee470d8..4ec16375 100644 --- a/frontend/src/i18n/locales/hu/errors.json +++ b/frontend/src/i18n/locales/hu/errors.json @@ -188,7 +188,9 @@ "quota_spent": "A mai közös YouTube-keret elfogyott. Próbáld holnap.", "recipient_required": "Adj meg egy címzett e-mail címet", "registration_closed": "A regisztráció jelenleg zárva van", + "related_disabled": "A kapcsolódó videók ki vannak kapcsolva ezen a példányon.", "related_daily_limit": "Elérted a mai kapcsolódó listák korlátját ({{limit}}). Próbáld holnap.", + "search_disabled": "Az élő YouTube-keresés ki van kapcsolva ezen a példányon.", "search_daily_limit": "Elérted a mai YouTube-keresési korlátot ({{limit}}). Próbáld holnap.", "search_term_required": "Írj be egy keresőkifejezést", "session_gone": "A lejátszási munkamenet véget ért",