test: the rewritten write handlers commit, and three more owner filters hold

Each handler that now calls a service and commits is driven through HTTP once.
A second account's watch state, Watch later list and private download profile
are never read or changed. The autotag job's orphan prune is seen committed.

backend/tests/test_autotag_orphans.py

@@ -8,6 +8,8 @@ drops an unknown id, so the view silently showed the whole feed under its old na
 from sqlalchemy import select
 
 from app.catalog.models import Channel
+from app.db import SessionLocal
+from app.sync.autotag import run_autotag_all
 from app.reader.models import ChannelTag, SavedView, Tag
 from app.reader import service as reader_service
 
@@ -66,3 +68,15 @@ def test_a_view_kept_tag_list_larger_than_a_bind_parameter_limit_still_cleans_up
     db.commit()
     assert reader_service.prune_orphan_system_tags(db) == 1
     db.commit()
+
+
+def test_the_job_commits_its_cleanup(db):
+    """The prune only flushes; the job ends its own transaction. Read from a session of its own,
+    the orphan is gone only once the job committed — the job's session would see its own delete
+    either way."""
+    orphan = Tag(user_id=None, name="Cooking", category="topic")
+    db.add(orphan)
+    db.commit()
+    assert run_autotag_all(db, only_missing=True)["orphan_tags_removed"] == 1
+    with SessionLocal() as fresh:
+        assert fresh.execute(select(Tag).where(Tag.name == "Cooking")).first() is None

backend/tests/test_owner_scoping.py

@@ -10,7 +10,10 @@ from datetime import datetime, timedelta, timezone
 from sqlalchemy import func, select
 
 from app import debugsession, notifications, quota
+from app.catalog.models import Channel, Video
+from app.downloads.models import DownloadJob, DownloadProfile
 from app.models import DebugScreenshot, DebugSession, Notification, QuotaEvent
+from app.reader.models import Playlist, PlaylistItem, VideoState
 from app.routes import tokens as tokens_routes
 from app.routes.auth import RESET_TTL, _issue_token
 
@@ -133,3 +136,113 @@ def test_screenshot_storage_counts_only_mine(db, make_user):
         )
     db.commit()
     assert debugsession.stored_screenshot_bytes(db, me.id) == 10
+
+
+# --- the units' owner filters, through their routes --------------------------------------------
+# The same rule one layer up: a second account acting on the SAME video, list or profile id must
+# reach only its own row. Each test holds the first account's row unchanged afterwards.
+
+
+def _videos(db, *ids):
+    db.add(Channel(id="UCo", title="O"))
+    db.flush()
+    for vid in ids:
+        db.add(
+            Video(
+                id=vid,
+                channel_id="UCo",
+                published_at=datetime.now(timezone.utc),
+                duration_seconds=3600,
+            )
+        )
+    db.commit()
+
+
+def _video_state(db, user, vid="v1"):
+    db.expire_all()
+    row = db.execute(
+        select(VideoState).where(VideoState.user_id == user.id, VideoState.video_id == vid)
+    ).scalar_one_or_none()
+    return None if row is None else (row.status, row.position_seconds)
+
+
+def test_unmarking_a_video_leaves_another_users_state(db, make_user, as_user):
+    me, other = make_user(), make_user()
+    _videos(db, "v1")
+    db.add(VideoState(user_id=other.id, video_id="v1", status="watched"))
+    db.commit()
+    r = as_user(me).post("/api/videos/v1/state", json={"status": "new"})
+    assert r.status_code == 200, r.text
+    assert _video_state(db, other) == ("watched", 0)
+
+
+def test_clearing_a_video_leaves_another_users_state(db, make_user, as_user):
+    me, other = make_user(), make_user()
+    _videos(db, "v1")
+    db.add(VideoState(user_id=other.id, video_id="v1", status="hidden", position_seconds=90))
+    db.commit()
+    assert as_user(me).delete("/api/videos/v1/state").status_code == 200
+    assert _video_state(db, other) == ("hidden", 90)
+
+
+def test_a_resume_point_is_set_and_cleared_on_my_own_row_only(db, make_user, as_user):
+    me, other = make_user(), make_user()
+    _videos(db, "v1")
+    db.add(VideoState(user_id=other.id, video_id="v1", status="hidden", position_seconds=90))
+    db.commit()
+    client = as_user(me)
+    body = {"position_seconds": 600, "duration_seconds": 3600}
+    assert client.post("/api/videos/v1/progress", json=body).status_code == 200
+    assert (_video_state(db, me), _video_state(db, other)) == (("new", 600), ("hidden", 90))
+    body = {"position_seconds": 0, "duration_seconds": 3600}
+    assert client.post("/api/videos/v1/progress", json=body).status_code == 200
+    assert (_video_state(db, me), _video_state(db, other)) == (None, ("hidden", 90))
+
+
+def _watch_later_ids(db, user):
+    db.expire_all()
+    return sorted(
+        db.execute(
+            select(PlaylistItem.video_id)
+            .join(Playlist, Playlist.id == PlaylistItem.playlist_id)
+            .where(Playlist.user_id == user.id, Playlist.kind == "watch_later")
+        ).scalars()
+    )
+
+
+def test_watch_later_adds_and_removes_use_my_own_list(db, make_user, as_user):
+    me, other = make_user(), make_user()
+    _videos(db, "v1", "v2")
+    theirs = as_user(other).post("/api/playlists/watch-later", json={"video_id": "v1"}).json()
+    client = as_user(me)
+    mine = client.post("/api/playlists/watch-later", json={"video_id": "v2"}).json()
+    assert mine["playlist_id"] != theirs["playlist_id"]
+    # Their saved video is not on my list, so removing it from mine leaves theirs alone.
+    assert client.delete("/api/playlists/watch-later/v1").status_code == 200
+    assert (_watch_later_ids(db, me), _watch_later_ids(db, other)) == (["v2"], ["v1"])
+
+
+def test_another_users_private_profile_is_refused_like_a_missing_one(db, make_user, as_user):
+    me, other = make_user(), make_user()
+    theirs = DownloadProfile(user_id=other.id, name="Theirs", spec={"format": "best"})
+    builtin = DownloadProfile(user_id=None, name="Best", spec={"format": "best"}, is_builtin=True)
+    db.add_all([theirs, builtin])
+    db.commit()
+    client = as_user(me)
+
+    def enqueue(profile_id):
+        return client.post(
+            "/api/downloads", json={"source": "dQw4w9WgXcQ", "profile_id": profile_id}
+        )
+
+    missing = enqueue(theirs.id + builtin.id + 1)
+    refused = enqueue(theirs.id)
+    assert (refused.status_code, refused.json()) == (404, missing.json())
+    assert refused.json()["detail"] == {"code": "not_found", "reason": "profile"}
+    db.expire_all()
+    assert db.execute(select(DownloadJob)).first() is None
+    # A built-in is everyone's to use.
+    r = enqueue(builtin.id)
+    assert r.status_code == 200, r.text
+    db.expire_all()
+    assert db.execute(select(DownloadJob.profile_id)).scalars().all() == [builtin.id]

backend/tests/test_route_commits.py

@@ -0,0 +1,356 @@
+"""Writes driven through HTTP, each read back: the handler called its unit and committed.
+
+A route's helper only flushes; the handler commits. The `db` fixture fails a test whose request left
+a write uncommitted, so every test here drives one successful write through its route and then
+reads the result back — a handler that lost its commit fails on both counts. The services' own
+tests hold what each write does; these hold that the route finishes it. Nothing here reaches
+YouTube or Plex: the YouTube client is a stub and no user has a Plex link. Needs the DB.
+"""
+
+from datetime import datetime, timezone
+
+import pytest
+from sqlalchemy import select
+
+from app import quota
+from app.auth import require_write_scope
+from app.catalog.models import Channel, ExploredChannel, SearchFind, Subscription, Video
+from app.downloads.models import DownloadJob, DownloadProfile, DownloadShare
+from app.plex.models import PlexItem, PlexLibrary, PlexSeason, PlexShow, PlexState
+from app.models import Notification
+from app.reader.models import SavedView, Tag, VideoState
+from app.routes import channels
+
+NOW = datetime.now(timezone.utc)
+
+
+def _video(db, vid="v1", **kw):
+    if db.get(Channel, "UCr") is None:
+        db.add(Channel(id="UCr", title="R"))
+        db.flush()
+    db.add(Video(id=vid, channel_id="UCr", published_at=NOW, duration_seconds=60, **kw))
+    db.commit()
+
+
+# --- tags ------------------------------------------------------------------------------------
+
+
+def test_a_tag_made_through_the_route_is_listed_back(db, make_user, as_user):
+    user = make_user()
+    client = as_user(user)
+    r = client.post("/api/tags", json={"name": "Science"})
+    assert r.status_code == 200, r.text
+    assert [t["name"] for t in client.get("/api/tags").json()] == ["Science"]
+
+
+def test_a_tag_renamed_through_the_route_is_listed_back(db, make_user, as_user):
+    user = make_user()
+    tag = Tag(user_id=user.id, name="Old")
+    db.add(tag)
+    db.commit()
+    client = as_user(user)
+    assert client.patch(f"/api/tags/{tag.id}", json={"name": "New"}).status_code == 200
+    assert [t["name"] for t in client.get("/api/tags").json()] == ["New"]
+
+
+def test_a_tag_deleted_through_the_route_is_gone(db, make_user, as_user):
+    user = make_user()
+    tag = Tag(user_id=user.id, name="Gone")
+    db.add(tag)
+    db.commit()
+    client = as_user(user)
+    assert client.delete(f"/api/tags/{tag.id}").json() == {"deleted": tag.id}
+    assert client.get("/api/tags").json() == []
+
+
+# --- watch state ------------------------------------------------------------------------------
+
+
+def _state(db, user, vid="v1"):
+    db.expire_all()
+    return db.execute(
+        select(VideoState).where(VideoState.user_id == user.id, VideoState.video_id == vid)
+    ).scalar_one_or_none()
+
+
+def test_a_video_marked_watched_through_the_route_stays_watched(db, make_user, as_user):
+    user = make_user()
+    _video(db)
+    r = as_user(user).post("/api/videos/v1/state", json={"status": "watched"})
+    assert r.status_code == 200, r.text
+    assert _state(db, user).status == "watched"
+
+
+def test_a_video_unmarked_through_the_route_stays_unmarked(db, make_user, as_user):
+    # "new" is its own branch of the handler, with its own commit.
+    user = make_user()
+    _video(db)
+    db.add(VideoState(user_id=user.id, video_id="v1", status="watched"))
+    db.commit()
+    r = as_user(user).post("/api/videos/v1/state", json={"status": "new"})
+    assert r.status_code == 200, r.text
+    assert _state(db, user) is None
+
+
+def test_a_video_state_cleared_through_the_route_is_gone(db, make_user, as_user):
+    user = make_user()
+    _video(db)
+    db.add(VideoState(user_id=user.id, video_id="v1", status="hidden", position_seconds=90))
+    db.commit()
+    assert as_user(user).delete("/api/videos/v1/state").status_code == 200
+    assert _state(db, user) is None
+
+
+# --- saved views ------------------------------------------------------------------------------
+
+
+def test_views_reordered_through_the_route_list_back_in_that_order(db, make_user, as_user):
+    user = make_user()
+    a = SavedView(user_id=user.id, source="feed", name="A", filters={}, position=0)
+    b = SavedView(user_id=user.id, source="feed", name="B", filters={}, position=1)
+    db.add_all([a, b])
+    db.commit()
+    client = as_user(user)
+    assert client.post("/api/saved-views/reorder", json={"ids": [b.id, a.id]}).status_code == 200
+    listed = client.get("/api/saved-views").json()
+    assert [(v["name"], v["position"]) for v in listed] == [("B", 0), ("A", 1)]
+
+
+# --- download profiles and shares -------------------------------------------------------------
+
+_SPEC = {"format": "best"}
+
+
+def _profile(db, user, name="Mine"):
+    p = DownloadProfile(user_id=user.id, name=name, spec=_SPEC, is_builtin=False, sort_order=101)
+    db.add(p)
+    db.commit()
+    return p
+
+
+def _profile_names(client):
+    return [p["name"] for p in client.get("/api/downloads/profiles").json()]
+
+
+def test_a_profile_made_through_the_route_is_listed_back(db, make_user, as_user):
+    user = make_user()
+    client = as_user(user)
+    r = client.post("/api/downloads/profiles", json={"name": "Phone", "spec": _SPEC})
+    assert r.status_code == 200, r.text
+    assert _profile_names(client) == ["Phone"]
+
+
+def test_a_profile_renamed_through_the_route_is_listed_back(db, make_user, as_user):
+    user = make_user()
+    p = _profile(db, user)
+    client = as_user(user)
+    r = client.patch(f"/api/downloads/profiles/{p.id}", json={"name": "Renamed"})
+    assert r.status_code == 200, r.text
+    assert _profile_names(client) == ["Renamed"]
+
+
+def test_a_profile_deleted_through_the_route_is_gone(db, make_user, as_user):
+    user = make_user()
+    p = _profile(db, user)
+    client = as_user(user)
+    assert client.delete(f"/api/downloads/profiles/{p.id}").json() == {"deleted": p.id}
+    assert _profile_names(client) == []
+
+
+def test_a_share_taken_back_through_the_route_is_gone(db, make_user, as_user):
+    owner, friend = make_user(), make_user()
+    job = DownloadJob(
+        user_id=owner.id, source_kind="youtube", source_ref="dQw4w9WgXcQ", profile_snapshot={}
+    )
+    db.add(job)
+    db.flush()
+    db.add(DownloadShare(job_id=job.id, from_user_id=owner.id, to_user_id=friend.id))
+    db.commit()
+    r = as_user(owner).delete(f"/api/downloads/{job.id}/share/{friend.email}")
+    assert r.json() == {"ok": True}
+    db.expire_all()
+    assert db.execute(select(DownloadShare)).first() is None
+
+
+# --- channels ---------------------------------------------------------------------------------
+
+
+class _YouTube:
+    """The YouTube client, answering every write the channel routes make."""
+
+    def __init__(self, db, user):
+        pass
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *exc):
+        return False
+
+    def insert_subscription(self, channel_id):
+        return "yt-sub-1"
+
+    def delete_subscription(self, sub_id):
+        return None
+
+    def get_channels(self, ids):
+        return []
+
+
+@pytest.fixture
+def youtube(monkeypatch):
+    monkeypatch.setattr(channels, "YouTubeClient", _YouTube)
+    monkeypatch.setattr(quota, "backfill_reserve_reached", lambda: False)
+
+
+def _channel(db, cid="UCc", **kw):
+    ch = Channel(id=cid, title="C", details_synced_at=NOW, **kw)
+    db.add(ch)
+    db.commit()
+    return ch
+
+
+def _subscription(db, user, cid="UCc"):
+    db.expire_all()
+    return db.execute(
+        select(Subscription).where(Subscription.user_id == user.id, Subscription.channel_id == cid)
+    ).scalar_one_or_none()
+
+
+def test_a_subscribe_through_the_route_is_recorded(db, make_user, as_user, override_dep, youtube):
+    user = make_user()
+    _channel(db)
+    client = as_user(user)
+    override_dep(require_write_scope, lambda: user)
+    assert client.post("/api/channels/UCc/subscribe").json() == {"ok": True, "subscribed": "UCc"}
+    assert _subscription(db, user).yt_subscription_id == "yt-sub-1"
+
+
+def test_an_explore_through_the_route_is_recorded(db, make_user, as_user, youtube, monkeypatch):
+    user = make_user()
+    _channel(db)
+    # The ingest pages YouTube; what the route itself records is the exploration.
+    monkeypatch.setattr(channels, "details_stale", lambda ch, now: False)
+    monkeypatch.setattr(
+        channels,
+        "explore_ingest_page",
+        lambda db, yt, ch, token: {"ingested": 0, "next_page_token": None},
+    )
+    r = as_user(user).post("/api/channels/UCc/explore")
+    assert r.status_code == 200, r.text
+    db.expire_all()
+    assert db.execute(select(ExploredChannel.user_id)).scalars().all() == [user.id]
+    assert db.get(Channel, "UCc").from_explore is True
+
+
+def test_an_unsubscribe_through_the_route_drops_the_subscription(
+    db, make_user, as_user, override_dep, youtube
+):
+    user = make_user()
+    _channel(db)
+    db.add(Subscription(user_id=user.id, channel_id="UCc", yt_subscription_id="yt-sub-1"))
+    db.commit()
+    client = as_user(user)
+    override_dep(require_write_scope, lambda: user)
+    r = client.delete("/api/channels/UCc/subscription")
+    assert r.json() == {"ok": True, "unsubscribed": "UCc"}
+    assert _subscription(db, user) is None
+
+
+def test_a_backfill_reset_through_the_route_is_kept(db, make_user, as_user, monkeypatch):
+    admin, reader = make_user(role="admin"), make_user()
+    _channel(db, backfill_done=True, backfill_cursor="page-9", recent_synced_at=NOW)
+    db.add(Subscription(user_id=reader.id, channel_id="UCc", deep_requested=False))
+    db.commit()
+    # The recent pull afterwards reaches YouTube; the reset is the route's own write.
+    monkeypatch.setattr(channels, "run_recent_backfill", lambda *a, **kw: None)
+    r = as_user(admin).post("/api/channels/UCc/reset-backfill")
+    assert r.status_code == 200, r.text
+    db.expire_all()
+    ch = db.get(Channel, "UCc")
+    assert (ch.backfill_done, ch.backfill_cursor, ch.recent_synced_at) == (False, None, None)
+    assert _subscription(db, reader).deep_requested is True
+
+
+# --- live search ------------------------------------------------------------------------------
+
+
+def test_search_results_cleared_through_the_route_are_gone(db, make_user, as_user):
+    user = make_user()
+    _video(db, "s1", 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"]})
+    assert r.json() == {"ok": True, "removed": 1}
+    db.expire_all()
+    assert db.get(Video, "s1") is None
+    assert db.execute(select(SearchFind)).first() is None
+
+
+# --- Plex show and season marks ---------------------------------------------------------------
+
+
+def _show(db):
+    lib = PlexLibrary(plex_key="2", title="Shows", kind="show", enabled=True)
+    db.add(lib)
+    db.flush()
+    sh = PlexShow(rating_key="sh1", library_id=lib.id, title="Show")
+    db.add(sh)
+    db.flush()
+    se = PlexSeason(rating_key="se1", show_id=sh.id, season_number=1)
+    db.add(se)
+    db.flush()
+    for n in (1, 2):
+        db.add(
+            PlexItem(
+                rating_key=f"e{n}",
+                library_id=lib.id,
+                kind="episode",
+                title=f"E{n}",
+                show_id=sh.id,
+                season_id=se.id,
+                season_number=1,
+                episode_number=n,
+            )
+        )
+    db.commit()
+
+
+def _plex_marks(db, user):
+    db.expire_all()
+    return sorted(
+        db.execute(
+            select(PlexItem.rating_key, PlexState.status)
+            .join(PlexState, PlexState.item_id == PlexItem.id)
+            .where(PlexState.user_id == user.id)
+        ).all()
+    )
+
+
+def test_a_show_marked_watched_through_the_route_stays_watched(db, make_user, as_user):
+    user = make_user()
+    _show(db)
+    r = as_user(user).post("/api/plex/show/sh1/state", json={"watched": True})
+    assert r.json() == {"ok": True, "changed": 2, "watched": True}
+    assert _plex_marks(db, user) == [("e1", "watched"), ("e2", "watched")]
+
+
+def test_a_season_marked_watched_through_the_route_stays_watched(db, make_user, as_user):
+    user = make_user()
+    _show(db)
+    r = as_user(user).post("/api/plex/season/se1/state", json={"watched": True})
+    assert r.json() == {"ok": True, "changed": 2, "watched": True}
+    assert _plex_marks(db, user) == [("e1", "watched"), ("e2", "watched")]
+
+
+def test_reading_one_notice_is_committed_with_its_trim(db, make_user, as_user):
+    """Marking a notice read and trimming the read ones past the cap are one write the handler
+    commits — the trim no longer commits on its own."""
+    user = make_user()
+    n = Notification(user_id=user.id, type="t", title="x")
+    db.add(n)
+    db.commit()
+    r = as_user(user).post(f"/api/me/notifications/{n.id}/read")
+    assert r.json() == {"ok": True}
+    db.expire_all()
+    assert db.get(Notification, n.id).read is True