fix(search): the API-sourced live search takes its slot under the lock too
Checked first and logged after search.list, requests sent together at the last free slot all passed, and each spent 100 units of the shared budget. Every live search now claims its slot (quota.claim_action) whatever the source, and gives it back when it showed nothing. The cap counts the zero-unit claims only, so search.list's own spend event is accounting, not a second slot. log_action, the unlocked writer, goes. Tests: the API source reads one page per request, is refused before the call when the budget cannot pay, counts one slot per search and refunds a failed one; the lock test now proves the count happens after the lock.
This commit is contained in:
@@ -75,9 +75,9 @@ def test_trimming_read_notifications_trims_only_mine(db, make_user):
|
||||
|
||||
def test_todays_actions_count_only_mine(db, make_user):
|
||||
me, other = make_user(), make_user()
|
||||
quota.log_action(me.id, quota.QuotaAction.VIDEOS_SEARCH)
|
||||
quota.log_action(other.id, quota.QuotaAction.VIDEOS_SEARCH)
|
||||
quota.log_action(other.id, quota.QuotaAction.VIDEOS_SEARCH)
|
||||
quota.claim_action(me.id, quota.QuotaAction.VIDEOS_SEARCH, 5)
|
||||
quota.claim_action(other.id, quota.QuotaAction.VIDEOS_SEARCH, 5)
|
||||
quota.claim_action(other.id, quota.QuotaAction.VIDEOS_SEARCH, 5)
|
||||
assert quota.actions_today(db, me.id, quota.QuotaAction.VIDEOS_SEARCH) == 1
|
||||
|
||||
|
||||
|
||||
@@ -44,15 +44,15 @@ def test_reads_see_separate_session_spend_mid_transaction(db):
|
||||
assert after - before == 7
|
||||
|
||||
|
||||
def test_log_action_survives_caller_rollback(db):
|
||||
def test_a_claimed_slot_survives_caller_rollback(db):
|
||||
user = User(email="searcher@example.com")
|
||||
db.add(user)
|
||||
db.commit() # a committed user to attribute the (free) action to
|
||||
db.add(User(email="pending@example.com")) # uncommitted caller work
|
||||
quota.log_action(user.id, quota.QuotaAction.VIDEOS_SEARCH)
|
||||
assert quota.claim_action(user.id, quota.QuotaAction.VIDEOS_SEARCH, 5) is not None
|
||||
db.rollback()
|
||||
|
||||
# The zero-cost action event survived (per-user daily caps must still count it) ...
|
||||
# The zero-cost claim event survived (per-user daily caps must still count it) ...
|
||||
events = db.execute(select(QuotaEvent)).scalars().all()
|
||||
assert len(events) == 1
|
||||
assert events[0].user_id == user.id
|
||||
|
||||
@@ -336,10 +336,11 @@ def test_the_last_slot_is_taken_under_a_lock(db, make_user):
|
||||
t.start()
|
||||
t.join(timeout=1)
|
||||
assert t.is_alive() # it waits for the one holding the lock
|
||||
# ...and counts only once it has the lock: the one holding it takes the last slot meanwhile.
|
||||
db.add(QuotaEvent(user_id=user.id, action=action, units=0))
|
||||
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
|
||||
assert got == [None]
|
||||
|
||||
|
||||
def _videos_list_fails(monkeypatch, *, on_call: int = 1, reason: str = "quotaExceeded") -> None:
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
"""`GET /api/search/youtube` — the per-user daily cap of the live search (scrape source).
|
||||
"""`GET /api/search/youtube` — the per-user daily cap of the live search, both sources.
|
||||
|
||||
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"."""
|
||||
The scrape and search.list are 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"; a lookup that fails says why instead of answering "no results"; and the API source
|
||||
— 100 units a page — reads one page per request, is refused before the call when the shared budget
|
||||
cannot pay for it, and counts one slot per search though search.list logs its own event."""
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -42,6 +44,19 @@ class _FakeYT:
|
||||
def get_channels(self, ids):
|
||||
return []
|
||||
|
||||
# search.list: the pages the test hands out, each logged as the real client does (100 units,
|
||||
# under the action the route attributes it to).
|
||||
api_pages: list = []
|
||||
api_calls = 0
|
||||
|
||||
def search_videos(self, q, page_token=None, order="relevance"):
|
||||
_FakeYT.api_calls += 1
|
||||
quota.record_usage(100)
|
||||
page = _FakeYT.api_pages.pop(0)
|
||||
if isinstance(page, Exception):
|
||||
raise page
|
||||
return page
|
||||
|
||||
def get_videos(self, ids):
|
||||
if _FakeYT.fail_videos:
|
||||
raise YouTubeError("videos -> 403", reason="quotaExceeded", path="videos")
|
||||
@@ -74,6 +89,8 @@ def pages(monkeypatch, db):
|
||||
monkeypatch.setattr(search_routes.search_scrape, "search", search)
|
||||
monkeypatch.setattr(search_routes, "YouTubeClient", _FakeYT)
|
||||
monkeypatch.setattr(_FakeYT, "fail_videos", False)
|
||||
monkeypatch.setattr(_FakeYT, "api_pages", [])
|
||||
monkeypatch.setattr(_FakeYT, "api_calls", 0)
|
||||
return state
|
||||
|
||||
|
||||
@@ -130,3 +147,78 @@ def test_a_lookup_that_fails_says_why_not_no_results(db, make_user, as_user, pag
|
||||
assert resp.status_code == 422
|
||||
assert resp.json()["detail"] == {"code": "youtube", "reason": "quota"}
|
||||
assert quota.actions_today(db, user.id, ACTION) == 0
|
||||
|
||||
|
||||
def _api(db) -> None:
|
||||
sysconfig.set_value(db, "search_source", "api")
|
||||
sysconfig.set_value(db, "quota_daily_budget", 10000)
|
||||
|
||||
|
||||
def test_the_api_source_reads_one_page_per_request(db, make_user, as_user, pages):
|
||||
# Each page is 100 units: a short page is answered as it is, with its cursor, never paged on.
|
||||
_api(db)
|
||||
_FakeYT.api_pages = [{"items": [_item(1)], "next_page_token": "nextpage"}, _page(2)]
|
||||
resp = as_user(make_user()).get("/api/search/youtube?q=cats&limit=20")
|
||||
assert resp.status_code == 200
|
||||
assert _FakeYT.api_calls == 1
|
||||
assert resp.json()["next_cursor"] == "nextpage"
|
||||
|
||||
|
||||
def test_the_api_source_counts_one_slot_per_search(db, make_user, as_user, pages):
|
||||
# search.list logs its own event under the action (record_usage): accounting, not a second slot.
|
||||
user = make_user()
|
||||
_api(db)
|
||||
sysconfig.set_value(db, "search_daily_limit_per_user", 2)
|
||||
_FakeYT.api_pages = [_page(1), _page(2), _page(3)]
|
||||
client = as_user(user)
|
||||
assert client.get("/api/search/youtube?q=cats").status_code == 200
|
||||
assert quota.actions_today(db, user.id, ACTION) == 1
|
||||
assert client.get("/api/search/youtube?q=dogs").status_code == 200
|
||||
refused = client.get("/api/search/youtube?q=birds")
|
||||
assert refused.status_code == 429
|
||||
assert refused.json()["detail"]["reason"] == "search_daily_limit"
|
||||
assert _FakeYT.api_calls == 2
|
||||
|
||||
|
||||
def test_the_api_source_takes_its_slot_under_the_lock(db, make_user, as_user, pages, monkeypatch):
|
||||
user = make_user()
|
||||
_api(db)
|
||||
claimed: list = []
|
||||
real = quota.claim_action
|
||||
|
||||
def claim(uid, action, limit):
|
||||
claimed.append(action)
|
||||
return real(uid, action, limit)
|
||||
|
||||
monkeypatch.setattr(search_routes.quota, "claim_action", claim)
|
||||
_FakeYT.api_pages = [_page(1)]
|
||||
assert as_user(user).get("/api/search/youtube?q=cats").status_code == 200
|
||||
assert claimed == [ACTION]
|
||||
|
||||
|
||||
def test_the_api_source_is_refused_before_the_call_when_the_budget_cannot_pay(
|
||||
db, make_user, as_user, pages
|
||||
):
|
||||
user = make_user()
|
||||
_api(db)
|
||||
sysconfig.set_value(db, "quota_daily_budget", 50)
|
||||
_FakeYT.api_pages = [_page(1)]
|
||||
resp = as_user(user).get("/api/search/youtube?q=cats")
|
||||
assert resp.status_code == 429
|
||||
assert resp.json()["detail"]["reason"] == "quota_spent"
|
||||
assert _FakeYT.api_calls == 0
|
||||
assert quota.actions_today(db, user.id, ACTION) == 0
|
||||
|
||||
|
||||
def test_an_api_search_that_failed_gives_its_slot_back(db, make_user, as_user, pages):
|
||||
user = make_user()
|
||||
_api(db)
|
||||
sysconfig.set_value(db, "search_daily_limit_per_user", 1)
|
||||
_FakeYT.api_pages = [
|
||||
YouTubeError("search -> 403", reason="quotaExceeded", path="search"),
|
||||
_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
|
||||
|
||||
Reference in New Issue
Block a user