From 2c5d8bc290451fedac284fbc132220a10c626b10 Mon Sep 17 00:00:00 2001 From: RXWatcher <14085001+RXWatcher@users.noreply.github.com> Date: Wed, 27 May 2026 13:49:41 +0200 Subject: [PATCH] refactor(audiobooks): ABS collection store queries canonical tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrites ABSCollectionStore's 8 methods to read/write user_personal_collections (collection_type='manual') instead of the dropped abs_user_collections / abs_collection_items tables. The in-memory abs.Collection / abs.CollectionItem struct shape is unchanged; ToABS() emitter helpers in internal/audiobooks/abs/ preserve the wire-shape contract. Notable mappings: - abs.Collection.IsPublic ↔ user_personal_collections.is_shared. - profile_id is now text NOT NULL (defaults to ''), so we bind the empty string for primary-profile rows rather than SQL NULL. - user_personal_collection_items.user_id is NOT NULL with no FK cascade from the parent — AddCollectionItem uses an INSERT ... SELECT to copy user_id from the parent collection, and DeleteCollection explicitly drops items before the parent within a single tx. - ListCollectionItems filters sub_item_id='' to avoid leaking the playlist podcast-episode rows that share the items table. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/audiobooks/abs_collection_store.go | 141 ++++++++++++++------ 1 file changed, 97 insertions(+), 44 deletions(-) diff --git a/internal/audiobooks/abs_collection_store.go b/internal/audiobooks/abs_collection_store.go index 1b2923a9..a6100b27 100644 --- a/internal/audiobooks/abs_collection_store.go +++ b/internal/audiobooks/abs_collection_store.go @@ -12,10 +12,16 @@ import ( "github.com/Silo-Server/silo-server/internal/audiobooks/abs" ) -// ABSCollectionStore implements abs.CollectionStore against the -// abs_user_collections + abs_collection_items tables (migrations -// 149 + 150). One row per collection in the parent table; one row -// per (collection_id, library_item_id) in the items table. +// ABSCollectionStore implements abs.CollectionStore against the canonical +// user_personal_collections + user_personal_collection_items tables +// (migration 156). Manual ABS collections live in user_personal_collections +// with collection_type = 'manual'; their member books are rows in +// user_personal_collection_items with sub_item_id = '' (the empty string +// distinguishes whole-item membership from playlist podcast-episode rows). +// +// abs.Collection.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". type ABSCollectionStore struct { Pool *pgxpool.Pool } @@ -23,19 +29,23 @@ type ABSCollectionStore struct { // Compile-time assertion. var _ abs.CollectionStore = (*ABSCollectionStore)(nil) +// absCollectionTypeManual is the discriminator value for ABS manual +// collections in the canonical user_personal_collections table. +const absCollectionTypeManual = "manual" + func (s *ABSCollectionStore) ListUserCollections(ctx context.Context, userID, profileID string) ([]abs.Collection, error) { uid, err := strconv.Atoi(userID) if err != nil { return nil, fmt.Errorf("abs_collection_store: invalid user id %q: %w", userID, err) } rows, err := s.Pool.Query(ctx, ` - SELECT id, user_id, profile_id, name, description, is_public, created_at, updated_at - FROM abs_user_collections - 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, absCollectionTypeManual, ) if err != nil { return nil, fmt.Errorf("abs_collection_store: list: %w", err) @@ -45,14 +55,10 @@ func (s *ABSCollectionStore) ListUserCollections(ctx context.Context, userID, pr for rows.Next() { var c abs.Collection var uidScan int - var profileScan *string - if err := rows.Scan(&c.ID, &uidScan, &profileScan, &c.Name, &c.Description, &c.IsPublic, &c.CreatedAt, &c.UpdatedAt); err != nil { + if err := rows.Scan(&c.ID, &uidScan, &c.ProfileID, &c.Name, &c.Description, &c.IsPublic, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, fmt.Errorf("abs_collection_store: list scan: %w", err) } c.UserID = strconv.Itoa(uidScan) - if profileScan != nil { - c.ProfileID = *profileScan - } out = append(out, c) } if err := rows.Err(); err != nil { @@ -64,20 +70,19 @@ func (s *ABSCollectionStore) ListUserCollections(ctx context.Context, userID, pr func (s *ABSCollectionStore) GetCollection(ctx context.Context, id string) (abs.Collection, error) { var c abs.Collection var uidScan int - var profileScan *string row := s.Pool.QueryRow(ctx, ` - SELECT id, user_id, profile_id, name, description, is_public, created_at, updated_at - FROM abs_user_collections WHERE id = $1`, id) - if err := row.Scan(&c.ID, &uidScan, &profileScan, &c.Name, &c.Description, &c.IsPublic, &c.CreatedAt, &c.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, absCollectionTypeManual, + ) + if err := row.Scan(&c.ID, &uidScan, &c.ProfileID, &c.Name, &c.Description, &c.IsPublic, &c.CreatedAt, &c.UpdatedAt); err != nil { if errors.Is(err, pgx.ErrNoRows) { return abs.Collection{}, abs.ErrNotFound } return abs.Collection{}, fmt.Errorf("abs_collection_store: get: %w", err) } c.UserID = strconv.Itoa(uidScan) - if profileScan != nil { - c.ProfileID = *profileScan - } return c, nil } @@ -87,9 +92,11 @@ func (s *ABSCollectionStore) CreateCollection(ctx context.Context, c abs.Collect return fmt.Errorf("abs_collection_store: invalid user id %q: %w", c.UserID, err) } if _, err := s.Pool.Exec(ctx, ` - INSERT INTO abs_user_collections (id, user_id, profile_id, name, description, is_public) - VALUES ($1, $2, $3::uuid, $4, $5, $6)`, - c.ID, uid, profileArg(c.ProfileID), c.Name, c.Description, c.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())`, + c.ID, uid, c.ProfileID, c.Name, c.Description, absCollectionTypeManual, c.IsPublic, ); err != nil { return fmt.Errorf("abs_collection_store: create: %w", err) } @@ -98,10 +105,10 @@ func (s *ABSCollectionStore) CreateCollection(ctx context.Context, c abs.Collect func (s *ABSCollectionStore) UpdateCollection(ctx context.Context, c abs.Collection) error { if _, err := s.Pool.Exec(ctx, ` - UPDATE abs_user_collections - SET name = $2, description = $3, is_public = $4, updated_at = now() - WHERE id = $1`, - c.ID, c.Name, c.Description, c.IsPublic, + UPDATE user_personal_collections + SET name = $2, description = $3, is_shared = $4, updated_at = now() + WHERE id = $1 AND collection_type = $5`, + c.ID, c.Name, c.Description, c.IsPublic, absCollectionTypeManual, ); err != nil { return fmt.Errorf("abs_collection_store: update: %w", err) } @@ -109,18 +116,39 @@ func (s *ABSCollectionStore) UpdateCollection(ctx context.Context, c abs.Collect } func (s *ABSCollectionStore) DeleteCollection(ctx context.Context, id string) error { - if _, err := s.Pool.Exec(ctx, `DELETE FROM abs_user_collections 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_collection_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_collection_store: delete-items: %w", err) + } + if _, err := tx.Exec(ctx, + `DELETE FROM user_personal_collections WHERE id = $1 AND collection_type = $2`, + id, absCollectionTypeManual, + ); err != nil { return fmt.Errorf("abs_collection_store: delete: %w", err) } + if err := tx.Commit(ctx); err != nil { + return fmt.Errorf("abs_collection_store: commit: %w", err) + } return nil } func (s *ABSCollectionStore) ListCollectionItems(ctx context.Context, collectionID string) ([]abs.CollectionItem, error) { rows, err := s.Pool.Query(ctx, ` - SELECT collection_id, library_item_id, added_at - FROM abs_collection_items - WHERE collection_id = $1 - ORDER BY added_at ASC`, collectionID) + SELECT media_item_id, added_at + FROM user_personal_collection_items + WHERE collection_id = $1 AND sub_item_id = '' + ORDER BY position ASC, added_at ASC`, + collectionID, + ) if err != nil { return nil, fmt.Errorf("abs_collection_store: list-items: %w", err) } @@ -128,9 +156,10 @@ func (s *ABSCollectionStore) ListCollectionItems(ctx context.Context, collection out := make([]abs.CollectionItem, 0) for rows.Next() { var it abs.CollectionItem - if err := rows.Scan(&it.CollectionID, &it.LibraryItemID, &it.AddedAt); err != nil { + if err := rows.Scan(&it.LibraryItemID, &it.AddedAt); err != nil { return nil, fmt.Errorf("abs_collection_store: list-items scan: %w", err) } + it.CollectionID = collectionID out = append(out, it) } if err := rows.Err(); err != nil { @@ -144,16 +173,35 @@ func (s *ABSCollectionStore) AddCollectionItem(ctx context.Context, collectionID if err != nil { return fmt.Errorf("abs_collection_store: begin tx: %w", err) } - defer tx.Rollback(ctx) + defer tx.Rollback(ctx) //nolint:errcheck + + // user_personal_collection_items.user_id is NOT NULL and has no + // default; the canonical schema requires the inserter to repeat the + // owning user_id from the parent. INSERT ... SELECT preserves the + // silent-no-op semantics of the ABS interface when the parent is + // missing (zero rows selected → zero rows inserted) and lets the + // PK ON CONFLICT keep re-adds idempotent. if _, err := tx.Exec(ctx, ` - INSERT INTO abs_collection_items (collection_id, library_item_id) - VALUES ($1, $2) - ON CONFLICT (collection_id, library_item_id) DO NOTHING`, - collectionID, libraryItemID, + 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, '', + 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 = $3 + ON CONFLICT (user_id, collection_id, media_item_id) DO NOTHING`, + collectionID, libraryItemID, absCollectionTypeManual, ); err != nil { return fmt.Errorf("abs_collection_store: add-item: %w", err) } - if _, err := tx.Exec(ctx, `UPDATE abs_user_collections SET updated_at = now() WHERE id = $1`, collectionID); err != nil { + if _, err := tx.Exec(ctx, + `UPDATE user_personal_collections SET updated_at = now() WHERE id = $1 AND collection_type = $2`, + collectionID, absCollectionTypeManual, + ); err != nil { return fmt.Errorf("abs_collection_store: bump-parent: %w", err) } if err := tx.Commit(ctx); err != nil { @@ -167,13 +215,18 @@ func (s *ABSCollectionStore) RemoveCollectionItem(ctx context.Context, collectio if err != nil { return fmt.Errorf("abs_collection_store: begin tx: %w", err) } - defer tx.Rollback(ctx) - if _, err := tx.Exec(ctx, `DELETE FROM abs_collection_items WHERE collection_id = $1 AND library_item_id = $2`, + 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 = ''`, collectionID, libraryItemID, ); err != nil { return fmt.Errorf("abs_collection_store: remove-item: %w", err) } - if _, err := tx.Exec(ctx, `UPDATE abs_user_collections SET updated_at = now() WHERE id = $1`, collectionID); err != nil { + if _, err := tx.Exec(ctx, + `UPDATE user_personal_collections SET updated_at = now() WHERE id = $1 AND collection_type = $2`, + collectionID, absCollectionTypeManual, + ); err != nil { return fmt.Errorf("abs_collection_store: bump-parent: %w", err) } if err := tx.Commit(ctx); err != nil {