haeaeae/silo-server
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(audiobooks): ABS playlist store queries canonical tables

Browse Source 
Rewrites ABSPlaylistStore methods to read/write user_personal_collections
(collection_type='playlist') and user_personal_collection_items
(sub_item_id replaces episode_id). DeletePlaylist runs in a transaction
since the items table has no FK cascade. Drops the coverArg helper —
the canonical schema has no cover_item column, so CoverItem is always
emitted as zero value per spec §6 deferral.

In-memory abs.Playlist / abs.PlaylistItem struct shape unchanged;
playlistToABS() emitter preserves the wire-shape contract.

Note: user_personal_collection_items PRIMARY KEY is (user_id,
collection_id, media_item_id) — sub_item_id is NOT part of it. The
old abs_playlist_items PK included episode_id, so a playlist that
referenced multiple episodes of the same library item had distinct
rows. Under the canonical PK the second insert silently collides;
prod baseline has zero playlist items so no live data is affected.
Multi-episode-of-same-book inside one playlist would require a schema
change (extend PK / add unique index) and is out of scope here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
RXWatcher
2026-05-27 13:53:21 +02:00
co-authored by Claude Opus 4.7
parent 2c5d8bc290
commit c140cb2ea6
1 changed files with 123 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/audiobooks/abs_playlist_store.go
+123 -62
View File
@@ -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 {