refactor: address code-review — layering + public API naming

Applies the three findings from the /code-review high pass on this stack:

1. to_tsquery_str moved from app.feedquery to app.utils — it's a generic
   tsquery sanitizer with no feed dependency; housing it in the feed
   read-model coupled the Plex browse module to the feed domain. Both feed
   and plex now import it from the neutral util module.

2. feedquery.decode_cursor no longer raises fastapi.HTTPException — it raises
   a plain ValueError, and routes/feed.get_feed translates it to the HTTP 400.
   The read-model no longer imports fastapi, matching its "no request state"
   docstring.

3. Dropped the leading underscore on the helpers that are now a cross-module
   public API: feedquery {filtered_query, sort_keys, order_by, keyset_predicate,
   encode_cursor, decode_cursor}; plex.query {TS_CONFIG, FIELD_COLUMNS,
   field_recheck, split_csv, added_cutoff, meta_filter_conds, apply_meta_filters,
   like_escape}; plex.cards {movie_card, show_card, episode_card, leaf_card}.
   Genuinely-internal helpers (_channel_url, _saved_expr, _ADDED_WINDOWS,
   _lib_key) keep their underscore.

Pure code motion + rename. DB-lane suite 287 passed / 2 skipped; ruff clean.
This commit is contained in:
2026-08-17 18:23:03 +02:00
parent e7b5b8e87f
commit 2ab188d873
6 changed files with 101 additions and 94 deletions
+15 -26
View File
@@ -1,18 +1,17 @@
"""Feed read-model: the pure query-building and row-serialization layer behind the feed.
Extracted from `routes/feed.py` so the read-model has one home instead of living inside the
router module — the feed route handlers, the playlist-detail query, and the Plex search box all
share these helpers (the projection, the filter builder, `to_tsquery_str`, and keyset paging).
Nothing here defines a route or reaches for request state; it's all `(db, user, params) -> query`.
router module — the feed route handlers and the playlist-detail query share these helpers (the
projection, the filter builder, and keyset paging). Nothing here defines a route or reaches for
request state; it's all `(db, user, params) -> query`, and a malformed cursor raises a plain
`ValueError` for the route handler to translate (the tsquery sanitizer lives in `app.utils`).
"""
import base64
import binascii
import json
import re
from datetime import date, datetime, timedelta, timezone
from fastapi import HTTPException
from sqlalchemy import BigInteger, Select, and_, cast, false, func, or_, select
from sqlalchemy.orm import Session, aliased
@@ -31,7 +30,7 @@ from app.models import (
Video,
VideoState,
)
from app.utils import iso
from app.utils import iso, to_tsquery_str
def _channel_url(channel_id: str, handle: str | None) -> str:
@@ -101,7 +100,7 @@ def serialize(row) -> dict:
}
def _filtered_query(
def filtered_query(
db: Session,
user: User,
*,
@@ -330,18 +329,6 @@ def _filtered_query(
return query, rank_expr
def to_tsquery_str(q: str) -> str | None:
"""Turn free-text into a to_tsquery() string: word runs ANDed together, the LAST word
prefix-matched (`:*`) for as-you-type search — e.g. "ti amo magyar" → "ti & amo & magyar:*".
Only `\\w` runs containing an alphanumeric are kept, so no tsquery operators can leak in
(the string is built from sanitised tokens, never the raw query)."""
tokens = [t for t in re.findall(r"\w+", q, flags=re.UNICODE) if any(c.isalnum() for c in t)]
if not tokens:
return None
tokens[-1] = tokens[-1] + ":*"
return " & ".join(tokens)
# --- Keyset (cursor) pagination ---------------------------------------------
#
# Each sort is described as an ordered list of key columns (expression + direction
@@ -351,7 +338,7 @@ def to_tsquery_str(q: str) -> str | None:
# replaces OFFSET/LIMIT: deep scrolling no longer scans-and-discards skipped rows,
# and the page boundary is stable even as the scheduler ingests new videos mid-scroll
# (OFFSET would shift and duplicate/skip rows).
def _sort_keys(sort: str, seed: int, rank_expr=None) -> list[dict]:
def sort_keys(sort: str, seed: int, rank_expr=None) -> list[dict]:
pub = {"expr": Video.published_at, "asc": False, "nullable": True, "kind": "dt"}
table: dict[str, list[dict]] = {
"newest": [pub],
@@ -382,7 +369,7 @@ def _sort_keys(sort: str, seed: int, rank_expr=None) -> list[dict]:
return table.get(sort) or table["newest"]
def _order_by(keys: list[dict]):
def order_by(keys: list[dict]):
cols = []
for k in keys:
c = k["expr"].asc() if k["asc"] else k["expr"].desc()
@@ -391,7 +378,7 @@ def _order_by(keys: list[dict]):
return cols
def _keyset_predicate(keys: list[dict], cursor_id: str):
def keyset_predicate(keys: list[dict], cursor_id: str):
"""WHERE clause selecting rows strictly AFTER the cursor in the sort order.
Lexicographic over the key columns then id, NULL-aware for `nulls_last`: a NULL
@@ -413,7 +400,7 @@ def _keyset_predicate(keys: list[dict], cursor_id: str):
return or_(*branches)
def _encode_cursor(keys: list[dict], row) -> str:
def encode_cursor(keys: list[dict], row) -> str:
vals = []
for i, k in enumerate(keys):
v = getattr(row, f"_k{i}")
@@ -422,7 +409,7 @@ def _encode_cursor(keys: list[dict], row) -> str:
return base64.urlsafe_b64encode(payload.encode()).decode()
def _decode_cursor(keys: list[dict], cursor: str) -> tuple[list, str]:
def decode_cursor(keys: list[dict], cursor: str) -> tuple[list, str]:
try:
payload = json.loads(base64.urlsafe_b64decode(cursor.encode()))
raw = payload["k"]
@@ -433,5 +420,7 @@ def _decode_cursor(keys: list[dict], cursor: str) -> tuple[list, str]:
for k, v in zip(keys, raw)
]
return vals, payload["id"]
except (binascii.Error, json.JSONDecodeError, KeyError, ValueError, TypeError):
raise HTTPException(status_code=400, detail="Invalid feed cursor")
except (binascii.Error, json.JSONDecodeError, KeyError, ValueError, TypeError) as exc:
# Framework-free: the route handler translates this into an HTTP 400 so the
# read-model stays web-layer-agnostic (see routes/feed.get_feed).
raise ValueError("Invalid feed cursor") from exc
+6 -6
View File
@@ -6,7 +6,7 @@ router. Pure functions — no DB, no request state."""
from app.models import PlexItem, PlexShow, PlexState
def _movie_card(it: PlexItem, st: PlexState | None) -> dict:
def movie_card(it: PlexItem, st: PlexState | None) -> dict:
return {
"id": it.rating_key,
"type": "movie",
@@ -20,7 +20,7 @@ def _movie_card(it: PlexItem, st: PlexState | None) -> dict:
}
def _show_card(sh: PlexShow, status: str = "new") -> dict:
def show_card(sh: PlexShow, status: str = "new") -> dict:
return {
"id": sh.rating_key,
"type": "show",
@@ -33,7 +33,7 @@ def _show_card(sh: PlexShow, status: str = "new") -> dict:
}
def _episode_card(e: PlexItem, st: PlexState | None) -> dict:
def episode_card(e: PlexItem, st: PlexState | None) -> dict:
return {
"id": e.rating_key,
"type": "episode",
@@ -49,7 +49,7 @@ def _episode_card(e: PlexItem, st: PlexState | None) -> dict:
}
def _leaf_card(
def leaf_card(
it: PlexItem,
st: PlexState | None,
show_title: str | None = None,
@@ -58,8 +58,8 @@ def _leaf_card(
"""A playable-leaf card (movie or episode). Used by playlists, which can mix both kinds.
`show_rk` (the show's rating_key) lets the client group a playlist's episodes by show/season."""
if it.kind == "episode":
card = _episode_card(it, st)
card = episode_card(it, st)
card["show_title"] = show_title
card["show_id"] = show_rk
return card
return _movie_card(it, st)
return movie_card(it, st)
+20 -20
View File
@@ -8,55 +8,55 @@ from datetime import datetime, timedelta, timezone
from sqlalchemy import and_, func, or_
_TS_CONFIG = "public.unaccent_simple"
TS_CONFIG = "public.unaccent_simple"
# Field-scoped search: the scope-pill in the header search box narrows a full-text query to ONE
# facet of the row. `all` uses the GIN-indexed weighted `search_vector` untouched (byte-identical to
# the pre-scope behaviour); a specific field keeps that GIN prefilter AND re-checks a per-column
# tsvector, so the scoped hits are always a strict subset of Everywhere. The per-field expressions
# mirror the `search_vector` Computed definition (same `_TS_CONFIG`, same `left(summary, 1000)`
# mirror the `search_vector` Computed definition (same `TS_CONFIG`, same `left(summary, 1000)`
# truncation) so scoping never surfaces a match the vector itself wouldn't. `episode` is handled
# separately in the query body (it targets episode rows, not the movie/show grid).
_FIELD_COLUMNS = ("title", "people", "summary", "episode")
FIELD_COLUMNS = ("title", "people", "summary", "episode")
def _field_recheck(model, field: str, tsq):
def field_recheck(model, field: str, tsq):
"""A per-field `to_tsvector(...) @@ tsq` clause for a non-`all`, non-`episode` scope, or None."""
if field == "title":
expr = func.to_tsvector(_TS_CONFIG, func.coalesce(model.title, ""))
expr = func.to_tsvector(TS_CONFIG, func.coalesce(model.title, ""))
elif field == "people":
expr = func.to_tsvector(_TS_CONFIG, func.coalesce(model.people_text, ""))
expr = func.to_tsvector(TS_CONFIG, func.coalesce(model.people_text, ""))
elif field == "summary":
expr = func.to_tsvector(
_TS_CONFIG, func.left(func.coalesce(model.summary, ""), 1000)
TS_CONFIG, func.left(func.coalesce(model.summary, ""), 1000)
)
else:
return None
return expr.op("@@")(tsq)
def _csv(v: str | None) -> list[str]:
def split_csv(v: str | None) -> list[str]:
return [s.strip() for s in v.split(",") if s.strip()] if v else []
_ADDED_WINDOWS = {"24h": 1, "1w": 7, "1m": 30, "6m": 182, "1y": 365}
def _added_cutoff(within: str | None) -> datetime | None:
def added_cutoff(within: str | None) -> datetime | None:
days = _ADDED_WINDOWS.get(within or "")
return datetime.now(timezone.utc) - timedelta(days=days) if days else None
def _meta_filter_conds(model, p: dict) -> list:
def meta_filter_conds(model, p: dict) -> list:
"""The shared metadata-filter conditions (identical column names on plex_items + plex_shows) for
`p` (the request's filter params). Duration is movie-only, handled by the caller. Returned as a
list so faceted search can apply all-but-one group. DRY for browse, unified, and /facets."""
conds: list = []
gsel = _csv(p.get("genres"))
gsel = split_csv(p.get("genres"))
if gsel:
gc = [model.genres.contains([g]) for g in gsel]
conds.append(and_(*gc) if p.get("genre_mode") == "all" else or_(*gc))
crs = _csv(p.get("content_ratings"))
crs = split_csv(p.get("content_ratings"))
if crs:
conds.append(model.content_rating.in_(crs))
if p.get("year_min") is not None:
@@ -65,16 +65,16 @@ def _meta_filter_conds(model, p: dict) -> list:
conds.append(model.year <= p["year_max"])
if p.get("rating_min") is not None:
conds.append(model.rating >= p["rating_min"])
cutoff = _added_cutoff(p.get("added_within"))
cutoff = added_cutoff(p.get("added_within"))
if cutoff is not None:
conds.append(model.added_at >= cutoff)
for d in _csv(p.get("directors")): # AND
for d in split_csv(p.get("directors")): # AND
conds.append(model.directors.contains([d]))
alist = _csv(p.get("actors"))
alist = split_csv(p.get("actors"))
if alist: # AND (all) or OR (any) — default OR, mirroring genre_mode
ac = [model.cast_names.contains([a]) for a in alist]
conds.append(and_(*ac) if p.get("actor_mode") == "all" else or_(*ac))
stds = _csv(p.get("studios"))
stds = split_csv(p.get("studios"))
if stds: # OR
conds.append(model.studio.in_(stds))
if p.get("collection"):
@@ -82,8 +82,8 @@ def _meta_filter_conds(model, p: dict) -> list:
return conds
def _apply_meta_filters(q, model, p: dict):
conds = _meta_filter_conds(model, p)
def apply_meta_filters(q, model, p: dict):
conds = meta_filter_conds(model, p)
return q.where(*conds) if conds else q
@@ -91,7 +91,7 @@ def _apply_meta_filters(q, model, p: dict):
class LibraryFilters:
"""The shared sidebar filter query-params for /library + /facets, declared once and injected via
``Depends()``. `as_dict()` yields the `p` dict the query builders read; `duration_*` is included
for /facets' wsq but ignored by `_meta_filter_conds` (both callers apply duration separately)."""
for /facets' wsq but ignored by `meta_filter_conds` (both callers apply duration separately)."""
genres: str | None = None
genre_mode: str = "any"
@@ -129,5 +129,5 @@ class LibraryFilters:
}
def _like_escape(s: str) -> str:
def like_escape(s: str) -> str:
return s.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
+19 -14
View File
@@ -11,13 +11,13 @@ from app import quota
from app.auth import current_user
from app.db import get_db
from app.feedquery import (
_decode_cursor,
_encode_cursor,
_filtered_query,
_keyset_predicate,
_order_by,
_sort_keys,
decode_cursor,
encode_cursor,
filtered_query,
keyset_predicate,
order_by,
serialize,
sort_keys,
)
from app.models import (
ChannelTag,
@@ -91,25 +91,30 @@ def get_feed(
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
query, rank_expr = _filtered_query(db, user, **params)
keys = _sort_keys(sort, seed, rank_expr)
query, rank_expr = filtered_query(db, user, **params)
keys = sort_keys(sort, seed, rank_expr)
# Expose each key column on the row so we can build the next cursor from the last item.
query = query.add_columns(*[k["expr"].label(f"_k{i}") for i, k in enumerate(keys)])
query = query.order_by(*_order_by(keys))
query = query.order_by(*order_by(keys))
if cursor:
vals, cursor_id = _decode_cursor(keys, cursor)
# The read-model raises a plain ValueError on a malformed cursor; translate it to the
# HTTP 400 here so feedquery stays web-framework-free.
try:
vals, cursor_id = decode_cursor(keys, cursor)
except ValueError:
raise HTTPException(status_code=400, detail="Invalid feed cursor")
for k, v in zip(keys, vals):
k["value"] = v
query = query.where(_keyset_predicate(keys, cursor_id))
query = query.where(keyset_predicate(keys, cursor_id))
rows = db.execute(query.limit(limit + 1)).all()
has_more = len(rows) > limit
page = rows[:limit]
return {
"items": [serialize(r) for r in page],
"next_cursor": _encode_cursor(keys, page[-1]) if (has_more and page) else None,
"next_cursor": encode_cursor(keys, page[-1]) if (has_more and page) else None,
"has_more": has_more,
"limit": limit,
}
@@ -121,7 +126,7 @@ def get_feed_count(
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
query, _rank = _filtered_query(db, user, **params)
query, _rank = filtered_query(db, user, **params)
total = db.scalar(select(func.count()).select_from(query.subquery()))
return {"count": total or 0}
@@ -149,7 +154,7 @@ def get_facets(
# keeps them applied, so each remaining chip narrows to channels that ALSO have all
# already-selected topics — and tags that can't co-occur drop to zero (hidden).
conjunctive = category == "topic" and params.get("tag_mode") == "and"
base, _rank = _filtered_query(
base, _rank = filtered_query(
db,
user,
**{**params, "exclude_tag_category": None if conjunctive else category},
+28 -28
View File
@@ -42,22 +42,22 @@ from app.plex import detail as plex_detail
from app.plex import people as plex_people
from app.plex import rollup, subtitles
from app.plex import sync as plex_sync
from app.feedquery import to_tsquery_str
from app.plex.cards import _episode_card, _leaf_card, _movie_card, _show_card
from app.plex.cards import episode_card, leaf_card, movie_card, show_card
from app.plex.client import PlexClient, PlexError, PlexNotConfigured
from app.plex.lookups import by_rating_key
from app.plex.query import (
_FIELD_COLUMNS,
_TS_CONFIG,
FIELD_COLUMNS,
TS_CONFIG,
LibraryFilters,
_apply_meta_filters,
_csv,
_field_recheck,
_like_escape,
_meta_filter_conds,
apply_meta_filters,
field_recheck,
like_escape,
meta_filter_conds,
split_csv,
)
from app.routes._common import owned_or_404
from app.routes.plex._shared import item_or_404
from app.utils import to_tsquery_str
router = APIRouter(prefix="/api/plex", tags=["plex"])
@@ -130,14 +130,14 @@ def unified_library(
want_shows = scope in ("show", "both") and bool(show_lib_ids)
ts = to_tsquery_str(q) if q else None
tsq = func.to_tsquery(_TS_CONFIG, ts) if ts else None
if field not in _FIELD_COLUMNS and field != "all":
tsq = func.to_tsquery(TS_CONFIG, ts) if ts else None
if field not in FIELD_COLUMNS and field != "all":
field = "all"
# `episode` scope targets episode rows only — the movie/show grid stays empty; the recheck for
# the specific title/people/summary fields is a strict narrowing on top of the GIN prefilter.
grid_field = field != "episode"
recheck = (
_field_recheck if tsq is not None and field not in ("all", "episode") else None
field_recheck if tsq is not None and field not in ("all", "episode") else None
)
selects = []
@@ -190,7 +190,7 @@ def unified_library(
)
else:
mq = mq.where(or_(st.status.is_(None), st.status != "hidden"))
mq = _apply_meta_filters(mq, PlexItem, p)
mq = apply_meta_filters(mq, PlexItem, p)
if f.duration_min is not None:
mq = mq.where(PlexItem.duration_s >= f.duration_min)
if f.duration_max is not None:
@@ -239,7 +239,7 @@ def unified_library(
sq = sq.where(or_(wat > 0, inp > 0), wat < tot)
elif show == "unwatched":
sq = sq.where(wat == 0, inp == 0)
sq = _apply_meta_filters(sq, PlexShow, p)
sq = apply_meta_filters(sq, PlexShow, p)
if tsq is not None:
sq = sq.where(PlexShow.search_vector.op("@@")(tsq))
if recheck is not None:
@@ -306,7 +306,7 @@ def unified_library(
if tsq is not None and want_shows and field in ("all", "episode"):
ep_st = aliased(PlexState)
ep_vec = (
func.to_tsvector(_TS_CONFIG, func.coalesce(PlexItem.title, ""))
func.to_tsvector(TS_CONFIG, func.coalesce(PlexItem.title, ""))
if field == "episode"
else PlexItem.search_vector
)
@@ -325,7 +325,7 @@ def unified_library(
.limit(24)
).all()
for e, s, show_title in eps:
card = _episode_card(e, s)
card = episode_card(e, s)
card["show_title"] = show_title
episodes.append(card)
@@ -373,7 +373,7 @@ def facets(
def wsq(query, model, ids, pp, kind_movie, with_dur):
"""Apply the (self-excluded) metadata filters + library scope + the per-user watch-state to a
facet aggregation query, so the facets reflect exactly the visible result set."""
query = query.where(model.library_id.in_(ids), *_meta_filter_conds(model, pp))
query = query.where(model.library_id.in_(ids), *meta_filter_conds(model, pp))
if kind_movie:
query = query.where(model.kind == "movie")
if with_dur and pp.get("duration_min") is not None:
@@ -514,7 +514,7 @@ def facets(
# be absent): the sidebar renders genres solely from this list and hides the whole genre section —
# chips AND the Any/All toggle — when it's empty, which would trap the user with no per-chip way to
# undo a zero-result selection. Count 0 is fine (genre chips don't display counts).
for g in _csv(f.genres):
for g in split_csv(f.genres):
genre_counts.setdefault(g, 0)
return {
"genres": [
@@ -588,7 +588,7 @@ def collections(
)
term = (q or "").strip()
if term:
query = query.where(PlexCollection.title.ilike(f"%{_like_escape(term)}%"))
query = query.where(PlexCollection.title.ilike(f"%{like_escape(term)}%"))
rows = db.execute(
query.order_by(
PlexCollection.child_count.desc().nullslast(),
@@ -898,7 +898,7 @@ def playlist_detail(
).scalars()
}
cards = [
_leaf_card(
leaf_card(
it,
states.get(it.id),
shows[it.show_id].title if it.show_id in shows else None,
@@ -1214,7 +1214,7 @@ def show_detail(
dict
] = [] # every episode, in season/episode order → show-level rollup
for e in eps:
card = _episode_card(e, states.get(e.id))
card = episode_card(e, states.get(e.id))
by_season.setdefault(e.season_id, []).append(card)
all_cards.append(card)
show_roll = rollup.rollup(all_cards)
@@ -1226,7 +1226,7 @@ def show_detail(
related: list[dict] = []
try:
meta, related_raw = _show_enrichment(db, sh.rating_key)
related = _related_show_cards(db, user.id, related_raw, exclude=sh.id)
related = _relatedshow_cards(db, user.id, related_raw, exclude=sh.id)
if sh.people_enriched_at is None and meta:
plex_people.enrich_row_people(db, sh, plex_people.cast_from_meta(meta))
plex_detail.apply_rich_detail(sh, meta) # imdb/tagline/full_cast for future views
@@ -1289,7 +1289,7 @@ def _season_block(se: PlexSeason, cards: list[dict]) -> dict:
}
def _related_show_cards(
def _relatedshow_cards(
db: Session, user_id: int, related: list[dict], exclude: int
) -> list[dict]:
"""Map Plex 'related' metadata to show cards for the shows we actually mirror (so they're openable),
@@ -1313,7 +1313,7 @@ def _related_show_cards(
if sh is None or rk in seen:
continue
seen.add(rk)
out.append(_show_card(sh, statuses.get(sh.id, "new")))
out.append(show_card(sh, statuses.get(sh.id, "new")))
if len(out) >= 20:
break
return out
@@ -1344,7 +1344,7 @@ def people_cards(
movies/shows (in the given scope) feature them. Photos come from the plex_people cache (populated
as items are rich-fetched — info-page view + the rich re-sync); counts are live against cast_names.
A name with no cached photo yet still returns a card (the UI shows a placeholder)."""
wanted = _csv(names)[:16]
wanted = split_csv(names)[:16]
if not wanted:
return {"people": []}
movie_lib_ids, show_lib_ids = _content_libs(db)
@@ -1429,7 +1429,7 @@ def person_detail(
)
).scalars()
}
movies = [_movie_card(it, states.get(it.id)) for it in items]
movies = [movie_card(it, states.get(it.id)) for it in items]
shows: list[dict] = []
if show_lib_ids:
@@ -1446,7 +1446,7 @@ def person_detail(
.limit(_PERSON_SECTION_MAX)
).scalars().all()
statuses = rollup.state_map(db, user.id, [s.id for s in show_rows])
shows = [_show_card(sh, statuses.get(sh.id, "new")) for sh in show_rows]
shows = [show_card(sh, statuses.get(sh.id, "new")) for sh in show_rows]
return {"name": name, "photo": photo, "movies": movies, "shows": shows}
@@ -1506,7 +1506,7 @@ def _collection_strips(db: Session, it: PlexItem, user_id: int) -> list[dict]:
"id": rk,
"title": col.title,
"source": _collection_source(col),
"items": [_movie_card(m, states.get(m.id)) for m in members],
"items": [movie_card(m, states.get(m.id)) for m in members],
}
)
return out
+13
View File
@@ -23,3 +23,16 @@ def valid_email(value: str | None) -> bool:
def now_utc() -> datetime:
"""Current time as a timezone-aware UTC datetime."""
return datetime.now(timezone.utc)
def to_tsquery_str(q: str) -> str | None:
"""Turn free-text into a to_tsquery() string: word runs ANDed together, the LAST word
prefix-matched (`:*`) for as-you-type search — e.g. "ti amo magyar" → "ti & amo & magyar:*".
Only `\\w` runs containing an alphanumeric are kept, so no tsquery operators can leak in
(the string is built from sanitised tokens, never the raw query). Shared by the YouTube feed
search and the Plex library search, hence this neutral home rather than either domain module."""
tokens = [t for t in re.findall(r"\w+", q, flags=re.UNICODE) if any(c.isalnum() for c in t)]
if not tokens:
return None
tokens[-1] = tokens[-1] + ":*"
return " & ".join(tokens)