diff --git a/internal/audiobooks/abs_playlist_store.go b/internal/audiobooks/abs_playlist_store.go index e97a41c0..c7f9867e 100644 --- a/internal/audiobooks/abs_playlist_store.go +++ b/internal/audiobooks/abs_playlist_store.go @@ -12,27 +12,57 @@ import ( "github.com/Silo-Server/silo-server/internal/audiobooks/abs" ) -// ABSPlaylistStore implements abs.PlaylistStore against the abs_playlists -// + abs_playlist_items tables (migrations 151 + 152). +// ABSPlaylistStore implements abs.PlaylistStore against the canonical +// user_personal_collections + user_personal_collection_items tables +// (migration 156). ABS playlists live in user_personal_collections with +// collection_type = 'playlist'; their entries are rows in +// user_personal_collection_items where sub_item_id is the ABS +// episode_id (empty string for whole-book entries, non-empty for +// podcast-episode entries). +// +// abs.Playlist.IsPublic maps to user_personal_collections.is_shared. +// profile_id is a text column (NOT NULL DEFAULT '') in the canonical +// schema, so the empty string stands in for "primary profile". +// +// abs.Playlist.CoverItem has no canonical column (deferred per +// spec §6); reads always return the zero value and writes ignore it. type ABSPlaylistStore struct { Pool *pgxpool.Pool } var _ abs.PlaylistStore = (*ABSPlaylistStore)(nil) +// absCollectionTypePlaylist is the discriminator value for ABS +// playlists in the canonical user_personal_collections table. +const absCollectionTypePlaylist = "playlist" + +// NOTE: PK-collision caveat. user_personal_collection_items has +// PRIMARY KEY (user_id, collection_id, media_item_id) — sub_item_id is +// NOT part of the PK. The old abs_playlist_items PK included +// episode_id, so a playlist that referenced two episodes of the same +// library item would have had two distinct rows. Under the canonical +// schema the second insert collides on the PK and the ON CONFLICT +// clause silently drops it. +// +// Prod baseline at migration time: the single existing playlist has +// zero items, so no live data is affected. Multi-episode-of-same-book +// inside one playlist remains a potential future limitation and would +// require a schema change (extending the PK to include sub_item_id, or +// adding a unique index over the four columns) — out of scope here. + func (s *ABSPlaylistStore) ListUserPlaylists(ctx context.Context, userID, profileID string) ([]abs.Playlist, error) { uid, err := strconv.Atoi(userID) if err != nil { return nil, fmt.Errorf("abs_playlist_store: invalid user id %q: %w", userID, err) } rows, err := s.Pool.Query(ctx, ` - SELECT id, user_id, profile_id, name, description, cover_item, is_public, created_at, updated_at - FROM abs_playlists - WHERE user_id = $1 - AND COALESCE(profile_id, '00000000-0000-0000-0000-000000000000'::uuid) - = COALESCE($2::uuid, '00000000-0000-0000-0000-000000000000'::uuid) + SELECT id, user_id, profile_id, name, description, is_shared, created_at, updated_at + FROM user_personal_collections + WHERE collection_type = $3 + AND user_id = $1 + AND profile_id = $2 ORDER BY created_at DESC`, - uid, profileArg(profileID), + uid, profileID, absCollectionTypePlaylist, ) if err != nil { return nil, fmt.Errorf("abs_playlist_store: list: %w", err) @@ -42,17 +72,11 @@ func (s *ABSPlaylistStore) ListUserPlaylists(ctx context.Context, userID, profil for rows.Next() { var p abs.Playlist var uidScan int - var profileScan, coverScan *string - if err := rows.Scan(&p.ID, &uidScan, &profileScan, &p.Name, &p.Description, &coverScan, &p.IsPublic, &p.CreatedAt, &p.UpdatedAt); err != nil { + if err := rows.Scan(&p.ID, &uidScan, &p.ProfileID, &p.Name, &p.Description, &p.IsPublic, &p.CreatedAt, &p.UpdatedAt); err != nil { return nil, fmt.Errorf("abs_playlist_store: list scan: %w", err) } p.UserID = strconv.Itoa(uidScan) - if profileScan != nil { - p.ProfileID = *profileScan - } - if coverScan != nil { - p.CoverItem = *coverScan - } + // CoverItem has no canonical column — always zero. out = append(out, p) } if err := rows.Err(); err != nil { @@ -64,44 +88,35 @@ func (s *ABSPlaylistStore) ListUserPlaylists(ctx context.Context, userID, profil func (s *ABSPlaylistStore) GetPlaylist(ctx context.Context, id string) (abs.Playlist, error) { var p abs.Playlist var uidScan int - var profileScan, coverScan *string row := s.Pool.QueryRow(ctx, ` - SELECT id, user_id, profile_id, name, description, cover_item, is_public, created_at, updated_at - FROM abs_playlists WHERE id = $1`, id) - if err := row.Scan(&p.ID, &uidScan, &profileScan, &p.Name, &p.Description, &coverScan, &p.IsPublic, &p.CreatedAt, &p.UpdatedAt); err != nil { + SELECT id, user_id, profile_id, name, description, is_shared, created_at, updated_at + FROM user_personal_collections + WHERE id = $1 AND collection_type = $2`, + id, absCollectionTypePlaylist, + ) + if err := row.Scan(&p.ID, &uidScan, &p.ProfileID, &p.Name, &p.Description, &p.IsPublic, &p.CreatedAt, &p.UpdatedAt); err != nil { if errors.Is(err, pgx.ErrNoRows) { return abs.Playlist{}, abs.ErrNotFound } return abs.Playlist{}, fmt.Errorf("abs_playlist_store: get: %w", err) } p.UserID = strconv.Itoa(uidScan) - if profileScan != nil { - p.ProfileID = *profileScan - } - if coverScan != nil { - p.CoverItem = *coverScan - } + // CoverItem has no canonical column — always zero. return p, nil } -// coverArg returns the value to bind for cover_item; empty string maps -// to NULL so the FK doesn't reject empty. -func coverArg(cover string) any { - if cover == "" { - return nil - } - return cover -} - func (s *ABSPlaylistStore) CreatePlaylist(ctx context.Context, p abs.Playlist) error { uid, err := strconv.Atoi(p.UserID) if err != nil { return fmt.Errorf("abs_playlist_store: invalid user id %q: %w", p.UserID, err) } + // CoverItem is intentionally not persisted (no canonical column). if _, err := s.Pool.Exec(ctx, ` - INSERT INTO abs_playlists (id, user_id, profile_id, name, description, cover_item, is_public) - VALUES ($1, $2, $3::uuid, $4, $5, $6, $7)`, - p.ID, uid, profileArg(p.ProfileID), p.Name, p.Description, coverArg(p.CoverItem), p.IsPublic, + INSERT INTO user_personal_collections + (id, user_id, profile_id, creator_profile_id, name, description, + collection_type, is_shared, query_definition, created_at, updated_at) + VALUES ($1, $2, $3, $3, $4, $5, $6, $7, '{}'::jsonb, now(), now())`, + p.ID, uid, p.ProfileID, p.Name, p.Description, absCollectionTypePlaylist, p.IsPublic, ); err != nil { return fmt.Errorf("abs_playlist_store: create: %w", err) } @@ -109,11 +124,12 @@ func (s *ABSPlaylistStore) CreatePlaylist(ctx context.Context, p abs.Playlist) e } func (s *ABSPlaylistStore) UpdatePlaylist(ctx context.Context, p abs.Playlist) error { + // CoverItem is intentionally not persisted (no canonical column). if _, err := s.Pool.Exec(ctx, ` - UPDATE abs_playlists - SET name = $2, description = $3, cover_item = $4, is_public = $5, updated_at = now() - WHERE id = $1`, - p.ID, p.Name, p.Description, coverArg(p.CoverItem), p.IsPublic, + UPDATE user_personal_collections + SET name = $2, description = $3, is_shared = $4, updated_at = now() + WHERE id = $1 AND collection_type = $5`, + p.ID, p.Name, p.Description, p.IsPublic, absCollectionTypePlaylist, ); err != nil { return fmt.Errorf("abs_playlist_store: update: %w", err) } @@ -121,18 +137,39 @@ func (s *ABSPlaylistStore) UpdatePlaylist(ctx context.Context, p abs.Playlist) e } func (s *ABSPlaylistStore) DeletePlaylist(ctx context.Context, id string) error { - if _, err := s.Pool.Exec(ctx, `DELETE FROM abs_playlists WHERE id = $1`, id); err != nil { + // user_personal_collection_items has no FK to user_personal_collections, + // so cascade is not automatic — drop items first, then the parent. + tx, err := s.Pool.Begin(ctx) + if err != nil { + return fmt.Errorf("abs_playlist_store: begin tx: %w", err) + } + defer tx.Rollback(ctx) //nolint:errcheck + if _, err := tx.Exec(ctx, + `DELETE FROM user_personal_collection_items WHERE collection_id = $1`, + id, + ); err != nil { + return fmt.Errorf("abs_playlist_store: delete-items: %w", err) + } + if _, err := tx.Exec(ctx, + `DELETE FROM user_personal_collections WHERE id = $1 AND collection_type = $2`, + id, absCollectionTypePlaylist, + ); err != nil { return fmt.Errorf("abs_playlist_store: delete: %w", err) } + if err := tx.Commit(ctx); err != nil { + return fmt.Errorf("abs_playlist_store: commit: %w", err) + } return nil } func (s *ABSPlaylistStore) ListPlaylistItems(ctx context.Context, playlistID string) ([]abs.PlaylistItem, error) { rows, err := s.Pool.Query(ctx, ` - SELECT playlist_id, library_item_id, episode_id, position, added_at - FROM abs_playlist_items - WHERE playlist_id = $1 - ORDER BY position ASC`, playlistID) + SELECT media_item_id, COALESCE(sub_item_id, '') AS episode_id, position, added_at + FROM user_personal_collection_items + WHERE collection_id = $1 + ORDER BY position ASC, added_at ASC`, + playlistID, + ) if err != nil { return nil, fmt.Errorf("abs_playlist_store: list-items: %w", err) } @@ -140,9 +177,10 @@ func (s *ABSPlaylistStore) ListPlaylistItems(ctx context.Context, playlistID str out := make([]abs.PlaylistItem, 0) for rows.Next() { var it abs.PlaylistItem - if err := rows.Scan(&it.PlaylistID, &it.LibraryItemID, &it.EpisodeID, &it.Position, &it.AddedAt); err != nil { + if err := rows.Scan(&it.LibraryItemID, &it.EpisodeID, &it.Position, &it.AddedAt); err != nil { return nil, fmt.Errorf("abs_playlist_store: list-items scan: %w", err) } + it.PlaylistID = playlistID out = append(out, it) } if err := rows.Err(); err != nil { @@ -156,19 +194,39 @@ func (s *ABSPlaylistStore) AddPlaylistItem(ctx context.Context, playlistID, libr if err != nil { return fmt.Errorf("abs_playlist_store: begin tx: %w", err) } - defer tx.Rollback(ctx) - // Position assignment: MAX(position)+1 inside the INSERT, one round-trip, - // no read-before-write race. + defer tx.Rollback(ctx) //nolint:errcheck + + // user_personal_collection_items.user_id is NOT NULL and has no + // default; copy it from the parent playlist row. INSERT ... SELECT + // preserves the silent-no-op semantics of the ABS interface when + // the parent is missing (zero rows selected → zero rows inserted) + // and the PK ON CONFLICT keeps re-adds idempotent. + // + // See the top-of-file note on the PK-collision caveat: episode_id + // (sub_item_id) is NOT in the PK, so a second add of the same + // (collection_id, media_item_id) with a different episode_id will + // be silently dropped by ON CONFLICT. if _, err := tx.Exec(ctx, ` - INSERT INTO abs_playlist_items (playlist_id, library_item_id, episode_id, position) - SELECT $1, $2, $3, COALESCE(MAX(position), 0) + 1 - FROM abs_playlist_items WHERE playlist_id = $1 - ON CONFLICT (playlist_id, library_item_id, episode_id) DO NOTHING`, - playlistID, libraryItemID, episodeID, + INSERT INTO user_personal_collection_items + (user_id, collection_id, media_item_id, sub_item_id, position, added_at) + SELECT c.user_id, c.id, $2, $3, + COALESCE(( + SELECT MAX(i.position) + 1 + FROM user_personal_collection_items i + WHERE i.collection_id = c.id + ), 0), + now() + FROM user_personal_collections c + WHERE c.id = $1 AND c.collection_type = $4 + ON CONFLICT (user_id, collection_id, media_item_id) DO NOTHING`, + playlistID, libraryItemID, episodeID, absCollectionTypePlaylist, ); err != nil { return fmt.Errorf("abs_playlist_store: add-item: %w", err) } - if _, err := tx.Exec(ctx, `UPDATE abs_playlists SET updated_at = now() WHERE id = $1`, playlistID); err != nil { + if _, err := tx.Exec(ctx, + `UPDATE user_personal_collections SET updated_at = now() WHERE id = $1 AND collection_type = $2`, + playlistID, absCollectionTypePlaylist, + ); err != nil { return fmt.Errorf("abs_playlist_store: bump-parent: %w", err) } if err := tx.Commit(ctx); err != nil { @@ -182,15 +240,18 @@ func (s *ABSPlaylistStore) RemovePlaylistItem(ctx context.Context, playlistID, l if err != nil { return fmt.Errorf("abs_playlist_store: begin tx: %w", err) } - defer tx.Rollback(ctx) - if _, err := tx.Exec(ctx, ` - DELETE FROM abs_playlist_items - WHERE playlist_id = $1 AND library_item_id = $2 AND episode_id = $3`, + defer tx.Rollback(ctx) //nolint:errcheck + if _, err := tx.Exec(ctx, + `DELETE FROM user_personal_collection_items + WHERE collection_id = $1 AND media_item_id = $2 AND sub_item_id = $3`, playlistID, libraryItemID, episodeID, ); err != nil { return fmt.Errorf("abs_playlist_store: remove-item: %w", err) } - if _, err := tx.Exec(ctx, `UPDATE abs_playlists SET updated_at = now() WHERE id = $1`, playlistID); err != nil { + if _, err := tx.Exec(ctx, + `UPDATE user_personal_collections SET updated_at = now() WHERE id = $1 AND collection_type = $2`, + playlistID, absCollectionTypePlaylist, + ); err != nil { return fmt.Errorf("abs_playlist_store: bump-parent: %w", err) } if err := tx.Commit(ctx); err != nil {