From 022505ff1866ed3707d253130250182885ea58e9 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 26 Jun 2026 03:15:53 +0200 Subject: [PATCH] refactor(playlists,feed): share feed-row projection & playlist helpers - feed_columns() is the single feed-row projection, consumed by both /feed and the playlist-detail query (was copy-pasted, drift risk on every new column). - _replace_items_from_youtube() dedups the YouTube playlist-mirror block shared by sync_user_playlists and repull_playlist. - _next_playlist_position/_next_item_position/_add_item replace the append-position idiom (x4) and the exists-then-insert blocks (x2). --- backend/app/routes/feed.py | 43 ++++++----- backend/app/routes/playlists.py | 125 ++++++++++++++------------------ backend/app/sync/playlists.py | 68 ++++++++--------- 3 files changed, 112 insertions(+), 124 deletions(-) diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index e2f331a..7e43f08 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -62,6 +62,29 @@ def _saved_expr(user: User): ) +def feed_columns(user: User, state) -> list: + """The shared feed-row projection (joined Channel fields + per-user watch state + + `saved`), consumed by both the feed query and the playlist-detail query so a new column + is added in exactly one place. `state` is the caller's aliased VideoState (outer-joined).""" + return [ + Video.id, + Video.title, + Video.channel_id, + Channel.title.label("channel_title"), + Channel.thumbnail_url.label("channel_thumbnail"), + Channel.handle.label("channel_handle"), + Video.published_at, + Video.thumbnail_url, + Video.duration_seconds, + Video.view_count, + Video.is_short, + Video.live_status, + func.coalesce(state.status, "new").label("status"), + func.coalesce(state.position_seconds, 0).label("position_seconds"), + _saved_expr(user), + ] + + def _serialize(row) -> dict: return { "id": row.id, @@ -115,23 +138,9 @@ def _filtered_query( status_expr = func.coalesce(state.status, "new") position_expr = func.coalesce(state.position_seconds, 0) - query = select( - Video.id, - Video.title, - Video.channel_id, - Channel.title.label("channel_title"), - Channel.thumbnail_url.label("channel_thumbnail"), - Channel.handle.label("channel_handle"), - Video.published_at, - Video.thumbnail_url, - Video.duration_seconds, - Video.view_count, - Video.is_short, - Video.live_status, - status_expr.label("status"), - position_expr.label("position_seconds"), - _saved_expr(user), - ).join(Channel, Channel.id == Video.channel_id) + query = select(*feed_columns(user, state)).join( + Channel, Channel.id == Video.channel_id + ) if scope == "all": # Whole shared catalog; subscription is optional (only for priority sort). diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py index 89228cb..a3ebf8a 100644 --- a/backend/app/routes/playlists.py +++ b/backend/app/routes/playlists.py @@ -9,7 +9,7 @@ from app import quota from app.auth import current_user, has_read_scope, has_write_scope from app.db import get_db from app.models import Channel, Playlist, PlaylistItem, User, Video, VideoState -from app.routes.feed import _saved_expr, _serialize +from app.routes.feed import _serialize, feed_columns from app.sync.playlists import ( plan_push, push_playlist, @@ -29,6 +29,51 @@ def _own_playlist(db: Session, user: User, playlist_id: int) -> Playlist: return pl +def _next_playlist_position(db: Session, user_id: int) -> int: + """Append position for a new playlist in this user's rail.""" + return ( + db.scalar( + select(func.coalesce(func.max(Playlist.position), -1)).where( + Playlist.user_id == user_id + ) + ) + + 1 + ) + + +def _next_item_position(db: Session, playlist_id: int) -> int: + """Append position for a new item at the end of a playlist.""" + return ( + db.scalar( + select(func.coalesce(func.max(PlaylistItem.position), -1)).where( + PlaylistItem.playlist_id == playlist_id + ) + ) + + 1 + ) + + +def _add_item(db: Session, pl: Playlist, video_id: str, *, mark_dirty: bool) -> bool: + """Append a video to a playlist if not already present (idempotent). Returns whether it + was added. `mark_dirty` recomputes the YouTube-sync dirty flag (skip for Watch later).""" + exists = db.scalar( + select(PlaylistItem).where( + PlaylistItem.playlist_id == pl.id, PlaylistItem.video_id == video_id + ) + ) + if exists is not None: + return False + db.add( + PlaylistItem( + playlist_id=pl.id, video_id=video_id, position=_next_item_position(db, pl.id) + ) + ) + if mark_dirty: + recompute_dirty(db, pl) + db.commit() + return True + + def _summary( db: Session, pl: Playlist, @@ -142,15 +187,7 @@ def create_playlist( name = (payload.get("name") or "").strip() if not name: raise HTTPException(status_code=400, detail="name is required") - next_pos = ( - db.scalar( - select(func.coalesce(func.max(Playlist.position), -1)).where( - Playlist.user_id == user.id - ) - ) - + 1 - ) - pl = Playlist(user_id=user.id, name=name, position=next_pos) + pl = Playlist(user_id=user.id, name=name, position=_next_playlist_position(db, user.id)) db.add(pl) db.commit() return _summary(db, pl, 0) @@ -164,16 +201,11 @@ def _watch_later(db: Session, user: User) -> Playlist: ) ) if pl is None: - next_pos = ( - db.scalar( - select(func.coalesce(func.max(Playlist.position), -1)).where( - Playlist.user_id == user.id - ) - ) - + 1 - ) pl = Playlist( - user_id=user.id, name="Watch later", kind="watch_later", position=next_pos + user_id=user.id, + name="Watch later", + kind="watch_later", + position=_next_playlist_position(db, user.id), ) db.add(pl) db.commit() @@ -191,22 +223,7 @@ def add_watch_later( if not video_id or db.get(Video, video_id) is None: raise HTTPException(status_code=404, detail="Unknown video") pl = _watch_later(db, user) - exists = db.scalar( - select(PlaylistItem).where( - PlaylistItem.playlist_id == pl.id, PlaylistItem.video_id == video_id - ) - ) - if exists is None: - next_pos = ( - db.scalar( - select(func.coalesce(func.max(PlaylistItem.position), -1)).where( - PlaylistItem.playlist_id == pl.id - ) - ) - + 1 - ) - db.add(PlaylistItem(playlist_id=pl.id, video_id=video_id, position=next_pos)) - db.commit() + _add_item(db, pl, video_id, mark_dirty=False) return {"saved": True, "playlist_id": pl.id} @@ -344,23 +361,7 @@ def get_playlist( pl = _own_playlist(db, user, playlist_id) state = aliased(VideoState) rows = db.execute( - select( - Video.id, - Video.title, - Video.channel_id, - Channel.title.label("channel_title"), - Channel.thumbnail_url.label("channel_thumbnail"), - Channel.handle.label("channel_handle"), - Video.published_at, - Video.thumbnail_url, - Video.duration_seconds, - Video.view_count, - Video.is_short, - Video.live_status, - func.coalesce(state.status, "new").label("status"), - func.coalesce(state.position_seconds, 0).label("position_seconds"), - _saved_expr(user), - ) + select(*feed_columns(user, state)) .join(PlaylistItem, PlaylistItem.video_id == Video.id) .join(Channel, Channel.id == Video.channel_id) .outerjoin(state, and_(state.video_id == Video.id, state.user_id == user.id)) @@ -439,25 +440,7 @@ def add_item( video_id = payload.get("video_id") if not video_id or db.get(Video, video_id) is None: raise HTTPException(status_code=404, detail="Unknown video") - exists = db.scalar( - select(PlaylistItem).where( - PlaylistItem.playlist_id == pl.id, PlaylistItem.video_id == video_id - ) - ) - if exists is None: - next_pos = ( - db.scalar( - select(func.coalesce(func.max(PlaylistItem.position), -1)).where( - PlaylistItem.playlist_id == pl.id - ) - ) - + 1 - ) - db.add( - PlaylistItem(playlist_id=pl.id, video_id=video_id, position=next_pos) - ) - recompute_dirty(db, pl) - db.commit() + _add_item(db, pl, video_id, mark_dirty=True) return {"playlist_id": pl.id, "video_id": video_id} diff --git a/backend/app/sync/playlists.py b/backend/app/sync/playlists.py index 4dcbaac..547dc55 100644 --- a/backend/app/sync/playlists.py +++ b/backend/app/sync/playlists.py @@ -94,6 +94,34 @@ def _ensure_videos(db: Session, yt: YouTubeClient, video_ids: list[str]) -> None db.commit() +def _replace_items_from_youtube( + db: Session, yt: YouTubeClient, pl: Playlist, ids: list[str] +) -> list[str]: + """Mirror a YouTube playlist's ordered video ids into `pl`'s local items (authoritative + replace): ensure the videos exist in the catalog, keep only those present in their YT + order (de-duplicated), swap the items out, and reset the synced fingerprint + dirty flag. + `pl.name` must already be set to the desired name. The caller commits. Returns the final + ordered ids.""" + _ensure_videos(db, yt, ids) + present = ( + set(db.execute(select(Video.id).where(Video.id.in_(ids))).scalars().all()) + if ids + else set() + ) + ordered: list[str] = [] + seen: set[str] = set() + for v in ids: + if v in present and v not in seen: + seen.add(v) + ordered.append(v) + db.execute(delete(PlaylistItem).where(PlaylistItem.playlist_id == pl.id)) + for pos, vid in enumerate(ordered): + db.add(PlaylistItem(playlist_id=pl.id, video_id=vid, position=pos)) + pl.synced_fingerprint = fingerprint(pl.name, ordered) + pl.dirty = False + return ordered + + def sync_user_playlists(db: Session, user: User) -> dict: """Mirror the user's YouTube playlists into local source='youtube' playlists. Leaves the user's local playlists and the built-in Watch later untouched.""" @@ -152,25 +180,9 @@ def sync_user_playlists(db: Session, user: User) -> dict: db.flush() else: pl.name = p.get("title") or pl.name - ids = list(yt.iter_my_playlist_video_ids(ytid)) - _ensure_videos(db, yt, ids) - present = ( - set(db.execute(select(Video.id).where(Video.id.in_(ids))).scalars().all()) - if ids - else set() - ) - ordered: list[str] = [] - seen: set[str] = set() - for v in ids: - if v in present and v not in seen: - seen.add(v) - ordered.append(v) # Mirror is authoritative: replace the items to match YouTube's order. - db.execute(delete(PlaylistItem).where(PlaylistItem.playlist_id == pl.id)) - for pos, vid in enumerate(ordered): - db.add(PlaylistItem(playlist_id=pl.id, video_id=vid, position=pos)) - pl.synced_fingerprint = fingerprint(pl.name, ordered) - pl.dirty = False + ids = list(yt.iter_my_playlist_video_ids(ytid)) + _replace_items_from_youtube(db, yt, pl, ids) db.commit() synced += 1 return {"synced": synced} @@ -349,25 +361,9 @@ def repull_playlist(db: Session, user: User, pl: Playlist) -> dict: snippet = yt.get_playlist_snippet(pl.yt_playlist_id) if snippet is None: raise YouTubeError("Playlist no longer exists on YouTube") - ids = list(yt.iter_my_playlist_video_ids(pl.yt_playlist_id)) - _ensure_videos(db, yt, ids) - present = ( - set(db.execute(select(Video.id).where(Video.id.in_(ids))).scalars().all()) - if ids - else set() - ) - ordered: list[str] = [] - seen: set[str] = set() - for v in ids: - if v in present and v not in seen: - seen.add(v) - ordered.append(v) pl.name = snippet.get("title") or pl.name - db.execute(delete(PlaylistItem).where(PlaylistItem.playlist_id == pl.id)) - for pos, vid in enumerate(ordered): - db.add(PlaylistItem(playlist_id=pl.id, video_id=vid, position=pos)) - pl.synced_fingerprint = fingerprint(pl.name, ordered) - pl.dirty = False + ids = list(yt.iter_my_playlist_video_ids(pl.yt_playlist_id)) + ordered = _replace_items_from_youtube(db, yt, pl, ids) db.commit() return {"items": len(ordered)}