From 5b81e226775effe0f2dd6de2c2e28736e36dfd9e Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 26 Jun 2026 03:15:53 +0200 Subject: [PATCH] refactor(backend): quota.measured helper, consistent admin gating, messageable predicate - quota.measured() context manager folds the before/after units diff that the manual sync routes each re-spelled (also makes quota_remaining_today consistent). - sync pause/resume now use Depends(admin_user) instead of inline role checks. - is_messageable_user() unifies the 'real, active, non-suspended human' rule that was encoded three ways (WS auth, send recipient, and the SQL _messageable()). --- backend/app/quota.py | 20 ++++++++++++++++++++ backend/app/routes/messages.py | 11 ++++++++--- backend/app/routes/sync.py | 34 ++++++++++------------------------ 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/backend/app/quota.py b/backend/app/quota.py index be699a1..1a45011 100644 --- a/backend/app/quota.py +++ b/backend/app/quota.py @@ -66,6 +66,26 @@ def attribute(actor_id: int | None, action: str): _action.reset(t2) +class Measure: + """Result handle from `measured()` — read .used / .remaining after the block exits.""" + + used = 0 + remaining = 0 + + +@contextlib.contextmanager +def measured(db: Session, actor_id: int | None, action: str): + """Like `attribute()`, but also reports how much quota the block spent. Yields a `Measure` + whose `.used` (units charged inside the block) and `.remaining` (today's remaining after) + are populated on exit — so manual sync routes don't each re-spell the before/after diff.""" + m = Measure() + before = units_used_today(db) + with attribute(actor_id, action): + yield m + m.used = units_used_today(db) - before + m.remaining = remaining_today(db) + + def pacific_today(): return datetime.now(_PACIFIC).date() diff --git a/backend/app/routes/messages.py b/backend/app/routes/messages.py index 59998a4..9718e1d 100644 --- a/backend/app/routes/messages.py +++ b/backend/app/routes/messages.py @@ -103,6 +103,7 @@ def _serialize_msg(m: Message) -> dict: def _messageable() -> list: + """SQL clauses for "a real, active, non-suspended human" — for directory/recipient queries.""" return [ User.is_demo.is_(False), User.is_active.is_(True), @@ -110,6 +111,11 @@ def _messageable() -> list: ] +def is_messageable_user(u: User | None) -> bool: + """Python mirror of `_messageable()` for an already-loaded row (WS auth, send recipient).""" + return bool(u and not u.is_demo and u.is_active and not u.is_suspended) + + def ensure_welcome(db: Session, user: User) -> None: """Create the one-time system welcome message for a (human) user the first time it's needed. Idempotent: guarded on whether any system message already exists for them.""" @@ -315,7 +321,7 @@ def send_message( if payload.recipient_id == user.id: raise HTTPException(status_code=400, detail="You can't message yourself.") recipient = db.get(User, payload.recipient_id) - if recipient is None or recipient.is_demo or not recipient.is_active or recipient.is_suspended: + if not is_messageable_user(recipient): raise HTTPException(status_code=404, detail="Recipient not available.") if db.get(MessageKey, recipient.id) is None: raise HTTPException(status_code=404, detail="That member hasn't set up messaging yet.") @@ -363,8 +369,7 @@ async def messages_ws(ws: WebSocket) -> None: return db = SessionLocal() try: - u = db.get(User, uid) - ok = bool(u and not u.is_demo and not u.is_suspended and u.is_active) + ok = is_messageable_user(db.get(User, uid)) finally: db.close() if not ok: diff --git a/backend/app/routes/sync.py b/backend/app/routes/sync.py index e6fdcc2..0ee9f4e 100644 --- a/backend/app/routes/sync.py +++ b/backend/app/routes/sync.py @@ -3,7 +3,7 @@ from sqlalchemy import and_, case, func, select, update from sqlalchemy.orm import Session from app import quota, state -from app.auth import current_user, has_read_scope, require_human +from app.auth import admin_user, current_user, has_read_scope, require_human from app.db import get_db from app.models import Channel, Subscription, User, Video from app.sync.runner import ( @@ -40,11 +40,10 @@ def sync_subscriptions( status_code=403, detail="Connect your YouTube account (read access) to import subscriptions.", ) - before = quota.units_used_today(db) - with quota.attribute(user.id, quota.QuotaAction.SUBSCRIPTIONS_PULL): + with quota.measured(db, user.id, quota.QuotaAction.SUBSCRIPTIONS_PULL) as m: result = import_subscriptions(db, user) - result["quota_used_estimate"] = quota.units_used_today(db) - before - result["quota_remaining_today"] = quota.remaining_today(db) + result["quota_used_estimate"] = m.used + result["quota_remaining_today"] = m.remaining return result @@ -62,10 +61,9 @@ def sync_backfill( db: Session = Depends(get_db), max_channels: int = 25, ) -> dict: - before = quota.units_used_today(db) - with quota.attribute(user.id, quota.QuotaAction.VIDEOS_BACKFILL_RECENT): + with quota.measured(db, user.id, quota.QuotaAction.VIDEOS_BACKFILL_RECENT) as m: result = run_recent_backfill(db, _user_channels(db, user), max_channels=max_channels) - result["quota_used_estimate"] = quota.units_used_today(db) - before + result["quota_used_estimate"] = m.used return result @@ -73,13 +71,9 @@ def sync_backfill( def sync_enrich( user: User = Depends(require_human), db: Session = Depends(get_db) ) -> dict: - before = quota.units_used_today(db) - with quota.attribute(user.id, quota.QuotaAction.VIDEOS_ENRICH): + with quota.measured(db, user.id, quota.QuotaAction.VIDEOS_ENRICH) as m: enriched = run_enrich(db) - return { - "enriched": enriched, - "quota_used_estimate": quota.units_used_today(db) - before, - } + return {"enriched": enriched, "quota_used_estimate": m.used} @router.get("/status") @@ -194,20 +188,12 @@ def deep_all( @router.post("/pause") -def pause_sync( - user: User = Depends(current_user), db: Session = Depends(get_db) -) -> dict: - if user.role != "admin": - raise HTTPException(status_code=403, detail="Admin only") +def pause_sync(_: User = Depends(admin_user), db: Session = Depends(get_db)) -> dict: state.set_sync_paused(db, True) return {"paused": True} @router.post("/resume") -def resume_sync( - user: User = Depends(current_user), db: Session = Depends(get_db) -) -> dict: - if user.role != "admin": - raise HTTPException(status_code=403, detail="Admin only") +def resume_sync(_: User = Depends(admin_user), db: Session = Depends(get_db)) -> dict: state.set_sync_paused(db, False) return {"paused": False}