fix(plex): address S1 review round-2 findings

- by_rating_key: generic-typed (type[_M] -> _M | None) so call sites keep their
  Model | None static type.
- Migrate the last 2 open-coded PlexCollection-by-rating_key lookups (sync.py
  resync_collection + delete_collection_local) to by_rating_key; zero open-coded
  rating_key lookups now remain in the package.
- Add test_playlist_bulk_add_and_remove covering the bulk add/remove endpoints
  and pinning the remove-bulk synchronize_session=False path.

Full suite 258 passed, 2 skipped; ruff clean.
This commit is contained in:
2026-08-16 07:14:09 +02:00
parent e008c48284
commit 73a58674cf
3 changed files with 38 additions and 12 deletions
+7 -2
View File
@@ -1,11 +1,16 @@
"""Small shared Plex row lookups, so the get-by-rating_key pattern lives in one place.""" """Small shared Plex row lookups, so the get-by-rating_key pattern lives in one place."""
from typing import TypeVar
from sqlalchemy import select from sqlalchemy import select
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
_M = TypeVar("_M")
def by_rating_key(db: Session, model, rating_key: str):
def by_rating_key(db: Session, model: type[_M], rating_key: str) -> _M | None:
"""The mirrored row for a Plex rating_key, or None. `model` is any of the rating_key-keyed Plex """The mirrored row for a Plex rating_key, or None. `model` is any of the rating_key-keyed Plex
tables (PlexItem / PlexShow / PlexSeason / PlexCollection).""" tables (PlexItem / PlexShow / PlexSeason / PlexCollection); the return type is that model, so call
sites keep their `Model | None` static type."""
return db.execute( return db.execute(
select(model).where(model.rating_key == str(rating_key)) select(model).where(model.rating_key == str(rating_key))
).scalars().first() ).scalars().first()
+3 -10
View File
@@ -19,6 +19,7 @@ from app import sysconfig
from app.models import PlexCollection, PlexItem, PlexLibrary, PlexSeason, PlexShow from app.models import PlexCollection, PlexItem, PlexLibrary, PlexSeason, PlexShow
from app.plex import people from app.plex import people
from app.plex.client import PlexClient, PlexError, PlexNotConfigured from app.plex.client import PlexClient, PlexError, PlexNotConfigured
from app.plex.lookups import by_rating_key
log = logging.getLogger("siftlode.plex") log = logging.getLogger("siftlode.plex")
@@ -382,13 +383,7 @@ def resync_collection(db: Session, plex: PlexClient, lib: PlexLibrary, rk: str)
collection's membership on the affected member rows.""" collection's membership on the affected member rows."""
meta = plex.metadata(rk) or {} meta = plex.metadata(rk) or {}
col = _upsert_collection( col = _upsert_collection(
db, db, by_rating_key(db, PlexCollection, rk), rk, lib.id, meta
db.execute(
select(PlexCollection).where(PlexCollection.rating_key == rk)
).scalars().first(),
rk,
lib.id,
meta,
) )
Model = PlexItem if lib.kind == "movie" else PlexShow Model = PlexItem if lib.kind == "movie" else PlexShow
new_members = {str(c.get("ratingKey")) for c in plex.collection_children(rk) if c.get("ratingKey")} new_members = {str(c.get("ratingKey")) for c in plex.collection_children(rk) if c.get("ratingKey")}
@@ -415,9 +410,7 @@ def delete_collection_local(db: Session, lib: PlexLibrary, rk: str) -> None:
select(Model).where(Model.collection_keys.contains([rk])) select(Model).where(Model.collection_keys.contains([rk]))
).scalars(): ).scalars():
row.collection_keys = [k for k in (row.collection_keys or []) if k != rk] or None row.collection_keys = [k for k in (row.collection_keys or []) if k != rk] or None
col = db.execute( col = by_rating_key(db, PlexCollection, rk)
select(PlexCollection).where(PlexCollection.rating_key == rk)
).scalars().first()
if col is not None: if col is not None:
db.delete(col) db.delete(col)
db.flush() db.flush()
@@ -285,6 +285,34 @@ class TestPlaylistAndCollectionEndpoints:
_request(db, user, monkeypatch, "DELETE", f"/api/plex/playlists/{pid}") _request(db, user, monkeypatch, "DELETE", f"/api/plex/playlists/{pid}")
assert _get(db, user, monkeypatch, "/api/plex/playlists").json()["playlists"] == [] assert _get(db, user, monkeypatch, "/api/plex/playlists").json()["playlists"] == []
def test_playlist_bulk_add_and_remove(self, db, monkeypatch):
# Covers the bulk endpoints (add-bulk / remove-bulk) and pins the remove-bulk delete's
# synchronize_session=False path, which the single add/remove roundtrip never exercises.
lib = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True)
db.add(lib)
db.flush()
db.add_all([self._movie(db, lib.id, rk, rk.upper()) for rk in ("m1", "m2", "m3")])
user = User(email="bulk@example.com")
db.add(user)
db.commit()
created = _request(db, user, monkeypatch, "POST", "/api/plex/playlists", {"title": "Bulk"})
pid = created.json()["id"]
added = _request(db, user, monkeypatch, "POST", f"/api/plex/playlists/{pid}/items/bulk",
{"item_rating_keys": ["m1", "m2", "m3"]})
assert added.status_code == 200, added.text
assert added.json() == {"added": 3, "item_count": 3}
removed = _request(db, user, monkeypatch, "POST",
f"/api/plex/playlists/{pid}/items/remove-bulk",
{"item_rating_keys": ["m2"]})
assert removed.status_code == 200, removed.text
assert removed.json() == {"removed": 1, "item_count": 2}
detail = _get(db, user, monkeypatch, f"/api/plex/playlists/{pid}").json()
assert [it["id"] for it in detail["items"]] == ["m1", "m3"] # input order preserved, m2 gone
def test_collections_list(self, db, monkeypatch): def test_collections_list(self, db, monkeypatch):
from app.models import PlexCollection from app.models import PlexCollection
lib = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True) lib = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True)