refactor(catalog): the HTTP layer reads through queries and writes through the service

Channels, search and sync do no database work in their routes any more: reads go through
catalog/queries, writes through catalog/service, and a live search's ingest is catalog/search.
What a catalog route needs from Reader (a playlist's channels, a user's tag links, the feed
projection of results) it asks reader/queries and reader/service. The scheduler and the runs page
read channel titles and the work queue through catalog/queries. Catalog is fenced below its HTTP
layer: its owner filters go through userscope, it reads no other unit's tables, and its ingest
jobs commit only where they are named.
This commit is contained in:
2026-10-19 02:32:20 +02:00
parent 238e6f8a61
commit 5229fd96be
21 changed files with 1223 additions and 726 deletions
+295
View File
@@ -0,0 +1,295 @@
"""Catalog's service and queries, one use case each, below the HTTP layer.
The routes are thin callers now, and their own tests stay the contract a client sees. These hold
what each write does and leaves on its own: that it flushes and leaves the commit to its caller,
that a row of someone else's reads as a missing one, and the rule each write carries (an explored
channel flagged only while nobody follows it, a subscription that keeps it, a cleared search that
spares what anyone holds). DB-backed (real Postgres).
"""
from datetime import datetime, timedelta, timezone
from sqlalchemy import select
from app.catalog import queries, search, service
from app.catalog.models import (
BlockedChannel,
Channel,
ExploredChannel,
SearchFind,
Subscription,
Video,
)
from app.reader import queries as reader_queries
from app.reader import service as reader_service
def _channel(db, cid="UCc", **kw):
ch = Channel(id=cid, title=kw.pop("title", cid), **kw)
db.add(ch)
db.commit()
return ch
def _video(db, vid, cid="UCc", **kw):
v = Video(id=vid, channel_id=cid, published_at=datetime.now(timezone.utc), **kw)
db.add(v)
db.commit()
return v
def _sub(db, user, cid="UCc", **kw):
s = Subscription(user_id=user.id, channel_id=cid, **kw)
db.add(s)
db.commit()
return s
def _uncommitted(db):
return bool(db.new or db.dirty or db.deleted) or db.in_transaction()
# --- whose a row is ------------------------------------------------------------------------
def test_a_users_subscriptions_blocks_and_explorations_are_theirs_alone(db, make_user):
owner, stranger = make_user(), make_user()
ch = _channel(db)
_sub(db, owner)
service.explore(db, owner, ch)
service.block(db, owner, "UCc")
db.commit()
assert queries.subscription(db, owner, "UCc") is not None
assert queries.subscription(db, stranger, "UCc") is None
assert queries.exploration(db, stranger, "UCc") is None
assert queries.is_blocked(db, stranger, "UCc") is False
assert queries.blocked_ids(db, stranger) == set()
assert [s.channel_id for s, _ in queries.subscriptions(db, stranger, "en")] == []
# --- subscription settings -------------------------------------------------------------------
def test_full_history_reports_only_the_switch_on(db, make_user):
user = make_user()
_channel(db)
sub = _sub(db, user)
assert service.update_subscription(db, sub, priority=3) is False
assert service.update_subscription(db, sub, deep_requested=True) is True
assert service.update_subscription(db, sub, deep_requested=True) is False # already on
assert _uncommitted(db)
db.commit()
assert (sub.priority, sub.hidden, sub.deep_requested) == (3, False, True)
def test_full_history_for_all_moves_only_the_callers_rows(db, make_user):
user, other = make_user(), make_user()
_channel(db)
_sub(db, user)
theirs = _sub(db, other)
assert service.set_deep_all(db, user, True) == 1
db.commit()
db.expire_all()
assert theirs.deep_requested is False
def test_a_reset_asks_every_subscriber_for_full_history(db, make_user):
a, b = make_user(), make_user()
ch = _channel(db, backfill_done=True, recent_synced_at=datetime.now(timezone.utc))
_sub(db, a)
_sub(db, b)
service.reset_backfill(db, ch)
db.commit()
db.expire_all()
assert (ch.backfill_done, ch.recent_synced_at) == (False, None)
assert set(db.scalars(select(Subscription.deep_requested))) == {True}
# --- explore, subscribe, block ---------------------------------------------------------------
def test_an_exploration_is_one_row_and_flags_the_channel_only_while_nobody_follows_it(
db, make_user
):
explorer, follower = make_user(), make_user()
lone = _channel(db, "UClone")
kept = _channel(db, "UCkept")
_sub(db, follower, "UCkept")
service.explore(db, explorer, lone)
service.explore(db, explorer, kept)
db.commit()
first = queries.exploration(db, explorer, "UClone").created_at
service.explore(db, explorer, lone) # refreshes the grace clock
db.commit()
assert db.scalar(select(ExploredChannel.id).where(ExploredChannel.channel_id == "UClone"))
assert len(db.scalars(select(ExploredChannel.id)).all()) == 2
assert queries.exploration(db, explorer, "UClone").created_at >= first
assert (lone.from_explore, kept.from_explore) == (True, False)
def test_subscribing_keeps_an_explored_channel_and_its_videos(db, make_user):
user = make_user()
ch = _channel(db, from_explore=True)
v = _video(db, "v1", via_explore=True)
service.record_subscription(db, user, ch, "")
db.commit()
db.expire_all()
sub = queries.subscription(db, user, "UCc")
assert (sub.yt_subscription_id, ch.from_explore, v.via_explore) == (None, False, False)
service.drop_subscription(db, sub)
db.commit()
assert queries.subscription(db, user, "UCc") is None
def test_a_block_is_once_and_ends_the_exploration(db, make_user):
user = make_user()
ch = _channel(db)
service.explore(db, user, ch)
db.commit()
service.block(db, user, "UCc")
service.block(db, user, "UCc")
db.commit()
assert len(db.scalars(select(BlockedChannel.id)).all()) == 1
assert queries.exploration(db, user, "UCc") is None
assert [c.id for c in queries.blocked_channels(db, user, "en")] == ["UCc"]
service.unblock(db, user, "UCc")
db.commit()
assert queries.is_blocked(db, user, "UCc") is False
# --- what the status cards count -------------------------------------------------------------
def test_my_progress_counts_only_my_channels_and_my_unhidden_videos(db, make_user):
user, other = make_user(), make_user()
now = datetime.now(timezone.utc)
_channel(db, "UCa", details_synced_at=now, recent_synced_at=now, backfill_done=True)
_channel(db, "UCb")
_channel(db, "UCtheirs")
_video(db, "a1", "UCa")
_video(db, "b1", "UCb")
_video(db, "t1", "UCtheirs")
_sub(db, user, "UCa", deep_requested=True)
_sub(db, user, "UCb", hidden=True)
_sub(db, other, "UCtheirs")
assert queries.my_progress(db, user) == (2, 1, 1, 1)
assert [c.id for c in queries.my_deep_requested(db, user)] == ["UCa"]
assert queries.my_video_count(db, user) == 1
assert queries.video_total(db) == 3
assert queries.catalog_totals(db, user)["subscriptions"] == 2
def test_a_channel_with_no_title_is_left_out_of_the_titles(db):
_channel(db, "UCt", title="Titled")
db.add(Channel(id="UCn", title=None))
db.commit()
assert queries.channel_titles(db, {"UCt", "UCn", "UCgone"}) == {"UCt": "Titled"}
assert queries.channel_title(db, "UCt") == "Titled"
def test_the_deep_backlog_is_the_unfinished_channels_someone_asked_for(db, make_user):
user = make_user()
_channel(db, "UCwant")
_channel(db, "UCdone", backfill_done=True)
_channel(db, "UCnobody")
_sub(db, user, "UCwant", deep_requested=True)
_sub(db, user, "UCdone", deep_requested=True)
_sub(db, user, "UCnobody")
assert [c.id for c in queries.deep_backlog(db)] == ["UCwant"]
assert queries.deep_queued(db, ["UCwant", "UCdone", "UCnobody"]) == {"UCwant", "UCdone"}
# --- live search -----------------------------------------------------------------------------
def test_a_cleared_search_spares_what_anyone_holds(db, make_user):
user, other, follower = make_user(), make_user(), make_user()
_channel(db, "UCs")
_channel(db, "UCfollowed")
_sub(db, follower, "UCfollowed")
for vid in ("loose", "watched", "found_by_other"):
_video(db, vid, "UCs", via_search=True)
_video(db, "followed", "UCfollowed", via_search=True)
_video(db, "organic", "UCs")
ids = ["loose", "watched", "found_by_other", "followed", "organic"]
search.record_finds(db, user, ids, "cat")
search.record_finds(db, user, ids, "cat") # idempotent
search.record_finds(db, other, ["found_by_other"], "dog")
reader_service.mark(db, other, "watched", "watched")
db.commit()
assert search.clear(db, user, ids) == 1
db.commit()
left = set(db.scalars(select(Video.id)))
assert left == {"watched", "found_by_other", "followed", "organic"}
assert db.scalars(select(SearchFind.user_id)).all() == [other.id]
def test_a_search_puts_what_the_user_already_has_last(db, make_user):
user = make_user()
_channel(db, "UCs")
_channel(db, "UCmine")
_sub(db, user, "UCmine")
for vid, cid in (("new1", "UCs"), ("mine", "UCmine"), ("seen", "UCs"), ("new2", "UCs")):
_video(db, vid, cid)
reader_service.mark(db, user, "seen", "hidden")
db.commit()
assert search.new_first(db, user, ["mine", "new1", "seen", "new2"]) == [
"new1",
"new2",
"mine",
"seen",
]
# --- what Catalog asks Reader ----------------------------------------------------------------
def test_a_users_tag_links_are_made_once_and_only_theirs_are_read(db, make_user):
user, other = make_user(), make_user()
_channel(db)
mine = reader_service.create_tag(db, user, "Mine", "other", None)
theirs = reader_service.create_tag(db, other, "Theirs", "other", None)
reader_service.attach_user_tag(db, user, "UCc", mine.id)
reader_service.attach_user_tag(db, user, "UCc", mine.id)
reader_service.attach_user_tag(db, other, "UCc", theirs.id)
db.commit()
assert reader_queries.user_tag_links(db, user) == {"UCc": [mine.id]}
assert reader_queries.user_tag_links(db, user, "UCelse") == {}
reader_service.detach_user_tag(db, user, "UCc", mine.id)
db.commit()
assert reader_queries.user_tag_links(db, user) == {}
assert reader_queries.user_tag_links(db, other) == {"UCc": [theirs.id]}
def test_discovery_offers_the_playlist_channels_not_followed_and_not_ones_own(db, make_user):
user = make_user()
for cid in ("UCnew", "UCfollowed", "UCown"):
_channel(db, cid)
for vid, cid in (("n1", "UCnew"), ("n2", "UCnew"), ("f1", "UCfollowed"), ("o1", "UCown")):
_video(db, vid, cid)
_sub(db, user, "UCfollowed")
pl = reader_service.create_playlist(db, user, "L")
for vid in ("n1", "n2", "f1", "o1"):
reader_service.add_item(db, pl, vid, mark_dirty=False)
db.commit()
rows = reader_queries.playlist_channels(db, user, "UCown", "en")
assert [(ch.id, n, lists) for ch, n, lists in rows] == [("UCnew", 2, 1)]
def test_an_exploration_older_than_its_grace_is_still_one_row(db, make_user):
"""The grace clock is refreshed in place: re-exploring after the cutoff does not add a row."""
user = make_user()
ch = _channel(db)
db.add(
ExploredChannel(
user_id=user.id,
channel_id="UCc",
created_at=datetime.now(timezone.utc) - timedelta(days=30),
)
)
db.commit()
service.explore(db, user, ch)
db.commit()
rows = db.scalars(select(ExploredChannel)).all()
assert len(rows) == 1
assert rows[0].created_at > datetime.now(timezone.utc) - timedelta(minutes=1)
@@ -15,7 +15,7 @@ from sqlalchemy.dialects import postgresql
from app.feedquery import filtered_query
from app.catalog.models import Video
from app.routes.channels import channel_aggregates
from app.catalog.queries import channel_aggregates
INDEX = "ix_videos_channel_pub_cover"
+1 -1
View File
@@ -10,7 +10,7 @@ from datetime import datetime, timezone
from sqlalchemy import select
from app.catalog.models import Channel, Subscription, Video
from app.routes.search import _fold_term
from app.catalog.search import _fold_term
def _video(vid: str, ch: str, title: str, search_terms: str | None = None) -> Video:
-3
View File
@@ -20,9 +20,6 @@ COMMITTING_HELPERS = {
"not hand the token back",
"auth.py:_complete_link": "the tail of the Google callback route, which answers with its result",
"auth.py:_register_account": "a background task with its own session, i.e. a job wrapper",
"search.py:_ingest_candidates": "stages around YouTube calls: the stubs must survive an enrich "
"that fails",
"search.py:_classify_shorts": "the last stage of that ingest, after its probes",
"auth.py:upsert_pending_invite": "commits only when it writes — the contract its two "
"callers rely on (`_register_account` persists its own flushed user because a no-op here "
"commits nothing)",
+5 -5
View File
@@ -16,7 +16,7 @@ from datetime import datetime, timezone
from app import sysconfig
from app.models import User
from app.catalog.models import Video
from app.routes.search import _ingest_candidates
from app.catalog.search import ingest_candidates
class _FakeYT:
@@ -71,7 +71,7 @@ def test_scrape_ingest_dateless_stub_ingests_and_takes_real_enrich_date(db):
"vid_bbbbbbb": _enrich_item("vid_bbbbbbb", "2023-10-30T17:00:08Z"),
}
kept = _ingest_candidates(db, _FakeYT(enrich), user, candidates)
kept = ingest_candidates(db, _FakeYT(enrich), user, candidates)
# No IntegrityError, and both non-Short/non-live videos are kept in input order.
assert kept == ["vid_aaaaaaa", "vid_bbbbbbb"]
@@ -89,7 +89,7 @@ def test_scrape_ingest_enrich_miss_keeps_nonnull_placeholder(db):
db.add(user)
db.commit()
kept = _ingest_candidates(db, _FakeYT({}), user, [_candidate("vid_ccccccc", "UC_chan_c")])
kept = ingest_candidates(db, _FakeYT({}), user, [_candidate("vid_ccccccc", "UC_chan_c")])
assert kept == []
c = db.get(Video, "vid_ccccccc")
@@ -101,7 +101,7 @@ def test_search_youtube_route_returns_422_when_ingest_fails(db, monkeypatch, ove
# 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,
# burning a second quota-costed search), and the session is rolled back. Here the page fetch succeeds
# (one candidate) and _ingest_candidates raises → the endpoint returns 422.
# (one candidate) and ingest_candidates raises → the endpoint returns 422.
from fastapi.testclient import TestClient
import app.state as state
@@ -144,7 +144,7 @@ def test_search_youtube_route_returns_422_when_ingest_fails(db, monkeypatch, ove
def _boom(db_, yt, user_, candidates):
raise RuntimeError("ingest exploded")
monkeypatch.setattr(search_mod, "_ingest_candidates", _boom)
monkeypatch.setattr(search_mod.catalog_search, "ingest_candidates", _boom)
override_dep(get_db, db.info["request"])
override_dep(current_user, lambda: user)
+42 -8
View File
@@ -110,7 +110,6 @@ UNIT_ROUTES = {
# What each HTTP file still does, by file: the debt a unit's sprint pays off. Exact counts. A core
# route's count is only the unit tables it imports; it is paid by the sprint of the unit it reaches.
ALLOWED = {
"channels.py": 83,
"plex/_shared.py": 1,
"plex/admin.py": 12,
"plex/browse.py": 93,
@@ -120,10 +119,6 @@ ALLOWED = {
"plex/people.py": 20,
"plex/playback.py": 42,
"plex/playlists.py": 80,
"runs.py": 1,
"scheduler.py": 3,
"search.py": 46,
"sync.py": 38,
}
# The session methods that read or write data. `commit` and `rollback` stay with the handler.
@@ -341,11 +336,22 @@ UNIT_PACKAGES = {
"sync/playlist_merge.py",
"sync/tagsuggest.py",
),
# The catalog's ingest jobs: the subscription import, the video sync and backfills, the explore
# and search ingests, the maintenance re-checks and the auto-tagger.
"catalog": (
"catalog/",
"sync/autotag.py",
"sync/explore.py",
"sync/maintenance.py",
"sync/runner.py",
"sync/subscriptions.py",
"sync/videos.py",
),
}
# The units fenced so far; the rest join with their sprint.
FENCED = {"capture", "reader"}
FENCED = {"capture", "reader", "catalog"}
# Whose tables a unit's own code may READ. Core code reads a fenced unit's tables through its queries.
READS = {"capture": {"catalog", "plex"}, "reader": {"catalog", "plex"}}
READS = {"capture": {"catalog", "plex"}, "reader": {"catalog", "plex"}, "catalog": set()}
# The columns that say whose a row is.
OWNER_COLUMNS = {"user_id", "to_user_id", "from_user_id", "created_by"}
# Where a fenced unit's code may commit: the jobs that own their session (the worker and what it
@@ -372,6 +378,32 @@ UNIT_COMMITS = {
"sync/playlists.py:merge_playlist",
"sync/playlists.py:repull_playlist",
},
# The catalog's ingest jobs commit per page or batch, a YouTube call between each: a page
# fetched and stored is not lost when the next call fails, and an enrich that fails rolls back
# to the stubs the step before committed (a search's results, a playlist's unseen videos).
"catalog": {
"catalog/search.py:classify_shorts",
"catalog/search.py:ingest_candidates",
"sync/autotag.py:apply_channel_autotags",
"sync/autotag.py:run_autotag_all",
"sync/explore.py:_enrich_new",
"sync/explore.py:purge_unkept_explored",
"sync/explore.py:purge_unkept_search",
"sync/maintenance.py:_notify_removed",
"sync/maintenance.py:_recheck_flagged",
"sync/maintenance.py:_revalidate_rolling",
"sync/runner.py:run_feed_fallback",
"sync/runner.py:_stamp_fallback_attempt",
"sync/subscriptions.py:import_subscriptions",
"sync/videos.py:insert_stubs",
"sync/videos.py:apply_rss_feed",
"sync/videos.py:backfill_channel_recent",
"sync/videos.py:backfill_channel_deep",
"sync/videos.py:reconcile_full_history",
"sync/videos.py:_apply_video_batch",
"sync/videos.py:run_shorts_classification",
"sync/videos.py:ensure_videos",
},
}
SQL_WRITES = {"insert", "update", "delete", "pg_insert"}
@@ -506,7 +538,9 @@ PACKAGE_CASES = {
("downloads/x.py", "from app.models import Video\nupdate(Video)"): 1,
("downloads/x.py", "from app.downloads.models import DownloadJob\nDownloadJob()"): 0,
("search.py", "from app.downloads.models import DownloadJob"): 1, # core reads capture
("search.py", "from app.models import Video\nVideo()"): 0, # neither side fenced yet
("search.py", "from app.models import PlexItem\nPlexItem()"): 0, # neither side fenced yet
("search.py", "from app.models import Video\nVideo()"): 2, # core reads and writes catalog
("sync/videos.py", "from app.models import Playlist"): 1, # no catalog→reader edge
("downloads/x.py", "q = DownloadJob.user_id == 1"): 1,
("downloads/x.py", "q = owned_by(DownloadJob.user_id, 1)"): 0,
("downloads/x.py", "def f(db):\n db.commit()"): 1,