feat(audiobooks): PATCH + DELETE /collections/{id}

Owner-gated mutation with partial-body PATCH semantics (only fields present in the body are updated). Non-owner attempts return 404 matching the bookmarks anti-enumeration pattern. DELETE cascades to abs_collection_items via FK. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-26 18:44:25 +02:00
parent aedf545481
commit da21cab37d
2 changed files with 172 additions and 0 deletions
@@ -169,3 +169,88 @@ func (h *Handler) handleGetCollection(w http.ResponseWriter, r *http.Request) {
 	}
 	writeJSON(w, http.StatusOK, h.collectionFullShape(r, c))
 }
+
+// handleUpdateCollection — PATCH /collections/{id}.
+// Owner-only. Partial body: only fields explicitly present are
+// modified. Non-owner gets 404 (no leak).
+func (h *Handler) handleUpdateCollection(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)
+	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-update failed", "err", err, "id", id)
+		http.Error(w, "collection get failed", http.StatusInternalServerError)
+		return
+	}
+
+	var body collectionBody
+	if err := json.NewDecoder(io.LimitReader(r.Body, 1<<20)).Decode(&body); err != nil {
+		http.Error(w, "invalid body", http.StatusBadRequest)
+		return
+	}
+	if body.Name != nil {
+		c.Name = *body.Name
+	}
+	if body.Description != nil {
+		c.Description = *body.Description
+	}
+	if body.IsPublic != nil {
+		c.IsPublic = *body.IsPublic
+	}
+	if err := h.deps.CollectionStore.UpdateCollection(r.Context(), c); err != nil {
+		slog.Error("abs collection update failed", "err", err, "id", id)
+		http.Error(w, "collection persist failed", http.StatusInternalServerError)
+		return
+	}
+
+	persisted, err := h.deps.CollectionStore.GetCollection(r.Context(), id)
+	if err != nil {
+		slog.Warn("abs collection get-after-update failed", "err", err, "id", id)
+		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.
+func (h *Handler) handleDeleteCollection(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)
+	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-delete failed", "err", err, "id", id)
+		http.Error(w, "collection get failed", http.StatusInternalServerError)
+		return
+	}
+	if err := h.deps.CollectionStore.DeleteCollection(r.Context(), id); err != nil {
+		slog.Error("abs collection delete failed", "err", err, "id", id)
+		http.Error(w, "collection delete failed", http.StatusInternalServerError)
+		return
+	}
+	w.WriteHeader(http.StatusNoContent)
+}
@@ -373,3 +373,90 @@ func TestCollection_Get_Unknown_404(t *testing.T) {
 		t.Errorf("status = %d, want 404; body=%s", rec.Code, rec.Body.String())
 	}
 }
+
+func TestCollection_Patch_OwnerUpdatesNameAndDescription(t *testing.T) {
+	hb := newCollectionsHarness(t)
+	id := createCollectionForUser(t, hb, "1", "", `{"name":"old","description":"d1"}`)
+
+	body := []byte(`{"name":"new","description":"d2","isPublic":true}`)
+	rec := dispatchABSWithParams(http.MethodPatch, "/api/collections/"+id, map[string]string{"id": id}, body, "1", "", hb.H.handleUpdateCollection)
+	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)
+	if got["name"] != "new" {
+		t.Errorf("name = %v, want 'new'", got["name"])
+	}
+	if got["description"] != "d2" {
+		t.Errorf("description = %v, want 'd2'", got["description"])
+	}
+	if got["isPublic"] != true {
+		t.Errorf("isPublic = %v, want true", got["isPublic"])
+	}
+}
+
+func TestCollection_Patch_PartialOnlyChangesPresentFields(t *testing.T) {
+	hb := newCollectionsHarness(t)
+	id := createCollectionForUser(t, hb, "1", "", `{"name":"keep","description":"d1"}`)
+
+	// PATCH only name; description and isPublic must stay.
+	body := []byte(`{"name":"renamed"}`)
+	rec := dispatchABSWithParams(http.MethodPatch, "/api/collections/"+id, map[string]string{"id": id}, body, "1", "", hb.H.handleUpdateCollection)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status = %d", rec.Code)
+	}
+	var got map[string]any
+	_ = json.Unmarshal(rec.Body.Bytes(), &got)
+	if got["name"] != "renamed" {
+		t.Errorf("name = %v, want 'renamed'", got["name"])
+	}
+	if got["description"] != "d1" {
+		t.Errorf("description = %v, want 'd1' (unchanged)", got["description"])
+	}
+}
+
+func TestCollection_Patch_NonOwner_404(t *testing.T) {
+	hb := newCollectionsHarness(t)
+	id := createCollectionForUser(t, hb, "1", "", `{"name":"mine"}`)
+
+	body := []byte(`{"name":"hijack"}`)
+	rec := dispatchABSWithParams(http.MethodPatch, "/api/collections/"+id, map[string]string{"id": id}, body, "2", "", hb.H.handleUpdateCollection)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("status = %d, want 404 (no leak); body=%s", rec.Code, rec.Body.String())
+	}
+	// User 1's collection must be untouched.
+	c, _ := hb.Coll.GetCollection(context.Background(), id)
+	if c.Name != "mine" {
+		t.Errorf("collection name = %q, want 'mine'; non-owner mutation leaked", c.Name)
+	}
+}
+
+func TestCollection_Delete_Owner_204(t *testing.T) {
+	hb := newCollectionsHarness(t)
+	id := createCollectionForUser(t, hb, "1", "", `{"name":"x"}`)
+
+	rec := dispatchABSWithParams(http.MethodDelete, "/api/collections/"+id, map[string]string{"id": id}, nil, "1", "", hb.H.handleDeleteCollection)
+	if rec.Code != http.StatusNoContent {
+		t.Fatalf("status = %d, want 204; body=%s", rec.Code, rec.Body.String())
+	}
+	// Subsequent GET must 404.
+	rec2 := dispatchABSWithParams(http.MethodGet, "/api/collections/"+id, map[string]string{"id": id}, nil, "1", "", hb.H.handleGetCollection)
+	if rec2.Code != http.StatusNotFound {
+		t.Errorf("post-delete GET status = %d, want 404", rec2.Code)
+	}
+}
+
+func TestCollection_Delete_NonOwner_404(t *testing.T) {
+	hb := newCollectionsHarness(t)
+	id := createCollectionForUser(t, hb, "1", "", `{"name":"mine"}`)
+
+	rec := dispatchABSWithParams(http.MethodDelete, "/api/collections/"+id, map[string]string{"id": id}, nil, "2", "", hb.H.handleDeleteCollection)
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("status = %d, want 404; body=%s", rec.Code, rec.Body.String())
+	}
+	// User 1's collection still exists.
+	if _, err := hb.Coll.GetCollection(context.Background(), id); err != nil {
+		t.Errorf("collection wrongly deleted: %v", err)
+	}
+}