fix(search): every related and live-search request takes a slot, and a failed one gives it back
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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 ---------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user