diff --git a/internal/audiobooks/abs/collections_handler.go b/internal/audiobooks/abs/collections_handler.go index 94242f2d..ac25397c 100644 --- a/internal/audiobooks/abs/collections_handler.go +++ b/internal/audiobooks/abs/collections_handler.go @@ -223,6 +223,103 @@ func (h *Handler) handleUpdateCollection(w http.ResponseWriter, r *http.Request) writeJSON(w, http.StatusOK, h.collectionFullShape(r, persisted)) } +// handleAddCollectionBook — POST /collections/{id}/book/{bookId}. +// Owner-gated. Validates the item exists via MediaStore (returns 404 +// for unknown items). Idempotent: re-adding is a silent no-op. +// Returns the parent collection's full-shape with updated books[]. +func (h *Handler) handleAddCollectionBook(w http.ResponseWriter, r *http.Request) { + a, ok := absAuthFrom(r) + if !ok || a.UserID == "" { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + if h.deps.CollectionStore == nil { + http.Error(w, "collection not found", http.StatusNotFound) + return + } + id := chiURLID(r) + bookID := chi.URLParam(r, "bookId") + if bookID == "" { + http.Error(w, "bookId required", http.StatusBadRequest) + return + } + + c, err := h.deps.CollectionStore.GetCollection(r.Context(), id) + if errors.Is(err, ErrNotFound) || (err == nil && c.UserID != a.UserID) { + http.Error(w, "collection not found", http.StatusNotFound) + return + } + if err != nil { + slog.Error("abs collection get-for-add failed", "err", err, "id", id) + http.Error(w, "collection get failed", http.StatusInternalServerError) + return + } + + // Item validation — avoid orphan refs. + item, err := h.deps.MediaStore.GetAudiobookByID(r.Context(), bookID) + if err != nil || item == nil { + http.Error(w, "item not found", http.StatusNotFound) + return + } + + if err := h.deps.CollectionStore.AddCollectionItem(r.Context(), id, bookID); err != nil { + slog.Error("abs collection add-item failed", "err", err, "id", id, "book", bookID) + http.Error(w, "collection persist failed", http.StatusInternalServerError) + return + } + + // Re-fetch to surface updated_at bump. + persisted, err := h.deps.CollectionStore.GetCollection(r.Context(), id) + if err != nil { + persisted = c + } + writeJSON(w, http.StatusOK, h.collectionFullShape(r, persisted)) +} + +// handleRemoveCollectionBook — DELETE /collections/{id}/book/{bookId}. +// Owner-gated. Idempotent: removing a non-member is a no-op. +// Returns the parent collection's full-shape with updated books[]. +func (h *Handler) handleRemoveCollectionBook(w http.ResponseWriter, r *http.Request) { + a, ok := absAuthFrom(r) + if !ok || a.UserID == "" { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + if h.deps.CollectionStore == nil { + http.Error(w, "collection not found", http.StatusNotFound) + return + } + id := chiURLID(r) + bookID := chi.URLParam(r, "bookId") + if bookID == "" { + http.Error(w, "bookId required", http.StatusBadRequest) + return + } + + c, err := h.deps.CollectionStore.GetCollection(r.Context(), id) + if errors.Is(err, ErrNotFound) || (err == nil && c.UserID != a.UserID) { + http.Error(w, "collection not found", http.StatusNotFound) + return + } + if err != nil { + slog.Error("abs collection get-for-remove failed", "err", err, "id", id) + http.Error(w, "collection get failed", http.StatusInternalServerError) + return + } + + if err := h.deps.CollectionStore.RemoveCollectionItem(r.Context(), id, bookID); err != nil { + slog.Error("abs collection remove-item failed", "err", err, "id", id, "book", bookID) + http.Error(w, "collection delete failed", http.StatusInternalServerError) + return + } + + persisted, err := h.deps.CollectionStore.GetCollection(r.Context(), id) + if err != nil { + persisted = c + } + writeJSON(w, http.StatusOK, h.collectionFullShape(r, persisted)) +} + // handleDeleteCollection — DELETE /collections/{id}. // Owner-only. Cascade drops abs_collection_items via FK CASCADE. // 204 on success; 404 for unknown or non-owned. diff --git a/internal/audiobooks/abs/collections_handler_test.go b/internal/audiobooks/abs/collections_handler_test.go index 88746470..09b4a2c6 100644 --- a/internal/audiobooks/abs/collections_handler_test.go +++ b/internal/audiobooks/abs/collections_handler_test.go @@ -460,3 +460,104 @@ func TestCollection_Delete_NonOwner_404(t *testing.T) { t.Errorf("collection wrongly deleted: %v", err) } } + +func TestCollection_AddBook_Owner_HydratesInResponse(t *testing.T) { + hb := newCollectionsHarness(t, "book-1") + id := createCollectionForUser(t, hb, "1", "", `{"name":"x"}`) + + rec := dispatchABSWithParams(http.MethodPost, "/api/collections/"+id+"/book/book-1", + map[string]string{"id": id, "bookId": "book-1"}, nil, "1", "", hb.H.handleAddCollectionBook) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + var got map[string]any + _ = json.Unmarshal(rec.Body.Bytes(), &got) + books, _ := got["books"].([]any) + if len(books) != 1 { + t.Fatalf("books len = %d, want 1", len(books)) + } + entry := books[0].(map[string]any) + if entry["id"] != "book-1" { + t.Errorf("book entry id = %v, want book-1", entry["id"]) + } + if _, has := entry["media"]; !has { + t.Errorf("book entry missing media hydration: %v", entry) + } +} + +func TestCollection_AddBook_Idempotent(t *testing.T) { + hb := newCollectionsHarness(t, "book-1") + id := createCollectionForUser(t, hb, "1", "", `{"name":"x"}`) + + _ = dispatchABSWithParams(http.MethodPost, "/api/collections/"+id+"/book/book-1", + map[string]string{"id": id, "bookId": "book-1"}, nil, "1", "", hb.H.handleAddCollectionBook) + rec := dispatchABSWithParams(http.MethodPost, "/api/collections/"+id+"/book/book-1", + map[string]string{"id": id, "bookId": "book-1"}, nil, "1", "", hb.H.handleAddCollectionBook) + if rec.Code != http.StatusOK { + t.Fatalf("second add status = %d, want 200 (idempotent); body=%s", rec.Code, rec.Body.String()) + } + var got map[string]any + _ = json.Unmarshal(rec.Body.Bytes(), &got) + books, _ := got["books"].([]any) + if len(books) != 1 { + t.Errorf("books len after double-add = %d, want 1", len(books)) + } +} + +func TestCollection_AddBook_UnknownItem_404(t *testing.T) { + hb := newCollectionsHarness(t /* no known items */) + id := createCollectionForUser(t, hb, "1", "", `{"name":"x"}`) + + rec := dispatchABSWithParams(http.MethodPost, "/api/collections/"+id+"/book/ghost", + map[string]string{"id": id, "bookId": "ghost"}, nil, "1", "", hb.H.handleAddCollectionBook) + if rec.Code != http.StatusNotFound { + t.Errorf("status = %d, want 404 (item not found); body=%s", rec.Code, rec.Body.String()) + } +} + +func TestCollection_AddBook_NonOwner_404(t *testing.T) { + hb := newCollectionsHarness(t, "book-1") + id := createCollectionForUser(t, hb, "1", "", `{"name":"mine"}`) + + rec := dispatchABSWithParams(http.MethodPost, "/api/collections/"+id+"/book/book-1", + map[string]string{"id": id, "bookId": "book-1"}, nil, "2", "", hb.H.handleAddCollectionBook) + if rec.Code != http.StatusNotFound { + t.Errorf("status = %d, want 404 (no leak); body=%s", rec.Code, rec.Body.String()) + } +} + +func TestCollection_RemoveBook_Idempotent(t *testing.T) { + hb := newCollectionsHarness(t, "book-1") + id := createCollectionForUser(t, hb, "1", "", `{"name":"x"}`) + + // Remove book that was never added — should be 200 with empty books. + rec := dispatchABSWithParams(http.MethodDelete, "/api/collections/"+id+"/book/book-1", + map[string]string{"id": id, "bookId": "book-1"}, nil, "1", "", hb.H.handleRemoveCollectionBook) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + var got map[string]any + _ = json.Unmarshal(rec.Body.Bytes(), &got) + books, _ := got["books"].([]any) + if len(books) != 0 { + t.Errorf("books len = %d, want 0", len(books)) + } +} + +func TestCollection_RemoveBook_NonOwner_404(t *testing.T) { + hb := newCollectionsHarness(t, "book-1") + id := createCollectionForUser(t, hb, "1", "", `{"name":"mine"}`) + _ = dispatchABSWithParams(http.MethodPost, "/api/collections/"+id+"/book/book-1", + map[string]string{"id": id, "bookId": "book-1"}, nil, "1", "", hb.H.handleAddCollectionBook) + + rec := dispatchABSWithParams(http.MethodDelete, "/api/collections/"+id+"/book/book-1", + map[string]string{"id": id, "bookId": "book-1"}, nil, "2", "", hb.H.handleRemoveCollectionBook) + if rec.Code != http.StatusNotFound { + t.Errorf("status = %d, want 404; body=%s", rec.Code, rec.Body.String()) + } + // User 1's items must be intact. + items, _ := hb.Coll.ListCollectionItems(context.Background(), id) + if len(items) != 1 { + t.Errorf("items len = %d, want 1 (non-owner remove leaked)", len(items)) + } +}