feat(audiobooks): DELETE /me/item/{id}/bookmark/{time} — ABS delete
Idempotent: returns 200 with the caller's current bookmark list regardless of whether the row existed. Skips item validation so bookmarks remain removable even after the underlying item is deleted. Realtime user_updated event with reason=bookmark_deleted fires only when a row actually existed (carries the pre-delete title). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
462ce40ef7
commit
c23e10a184
@@ -6,6 +6,7 @@ import (
|
||||
"log/slog"
|
||||
"math"
|
||||
"net/http"
|
||||
"strconv"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
)
|
||||
@@ -76,6 +77,82 @@ func (h *Handler) handleUpsertBookmark(reason string) http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// handleDeleteBookmark — DELETE /me/item/{itemId}/bookmark/{time}.
|
||||
//
|
||||
// Idempotent: returns 200 with the caller's current bookmark list,
|
||||
// whether or not the (item, time) row existed. Crucially, this means
|
||||
// a DELETE against another user's bookmark returns the caller's own
|
||||
// (empty-or-other) list — no enumeration vector.
|
||||
//
|
||||
// Item validation is intentionally skipped: a bookmark whose item was
|
||||
// just deleted should still be removable. (Upsert keeps validation
|
||||
// because it would create a new orphan row.)
|
||||
func (h *Handler) handleDeleteBookmark(w http.ResponseWriter, r *http.Request) {
|
||||
a, ok := absAuthFrom(r)
|
||||
if !ok || a.UserID == "" {
|
||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
if h.deps.BookmarkStore == nil {
|
||||
http.Error(w, "bookmark store unavailable", http.StatusServiceUnavailable)
|
||||
return
|
||||
}
|
||||
|
||||
itemID := chi.URLParam(r, "itemId")
|
||||
if itemID == "" {
|
||||
http.Error(w, "itemId required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
t, ok := parseBookmarkTime(chi.URLParam(r, "time"))
|
||||
if !ok {
|
||||
http.Error(w, "time required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
// Snapshot the pre-delete row so the realtime payload carries the
|
||||
// title that just got removed (clients prefer this over a bare ID).
|
||||
var pre Bookmark
|
||||
if rows, err := h.deps.BookmarkStore.List(r.Context(), a.UserID, a.ProfileID, itemID); err == nil {
|
||||
for _, b := range rows {
|
||||
if b.Time == t {
|
||||
pre = b
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if err := h.deps.BookmarkStore.Delete(r.Context(), a.UserID, a.ProfileID, itemID, t); err != nil {
|
||||
slog.Error("abs bookmark delete failed", "err", err, "user", a.UserID, "item", itemID)
|
||||
http.Error(w, "bookmark delete failed", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
// Only publish when the row actually existed (pre.ID is empty
|
||||
// otherwise). Avoids notifying other devices about a phantom delete.
|
||||
if pre.ID != "" {
|
||||
h.publish(a.UserID, "user_updated", map[string]any{
|
||||
"reason": "bookmark_deleted",
|
||||
"bookmark": bookmarkToABS(pre),
|
||||
})
|
||||
}
|
||||
|
||||
writeBookmarkList(w, r, h, a.UserID, a.ProfileID, itemID)
|
||||
}
|
||||
|
||||
// parseBookmarkTime parses the {time} URL parameter on DELETE
|
||||
// /me/item/{itemId}/bookmark/{time}. Returns (0, false) on parse
|
||||
// failure.
|
||||
func parseBookmarkTime(s string) (float64, bool) {
|
||||
if s == "" {
|
||||
return 0, false
|
||||
}
|
||||
v, err := strconv.ParseFloat(s, 64)
|
||||
if err != nil || math.IsNaN(v) {
|
||||
return 0, false
|
||||
}
|
||||
return v, true
|
||||
}
|
||||
|
||||
// writeBookmarkList re-fetches the item's bookmarks and writes them as
|
||||
// the JSON response. On list-fetch failure after a successful mutation,
|
||||
// degrade to 200 + empty list + slog.Warn (the mutation already
|
||||
|
||||
@@ -273,3 +273,37 @@ func TestUpsert_SameTime_UpdatesTitleNoDuplicate(t *testing.T) {
|
||||
t.Errorf("id changed across upsert: was %v, now %v (id must be preserved)", firstID, patchList[0]["id"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestDelete_ExistingBookmark_RemovedFromList(t *testing.T) {
|
||||
hb := newBookmarksHarness(t, "book-1")
|
||||
|
||||
// Seed a bookmark via POST.
|
||||
postBody := []byte(`{"title":"to delete","time":99}`)
|
||||
_ = dispatchBookmark(hb.H, http.MethodPost, "/api/me/item/book-1/bookmark", "book-1", "", postBody, "1", "", hb.H.handleUpsertBookmark("bookmark_created"))
|
||||
|
||||
// DELETE it.
|
||||
rec := dispatchBookmark(hb.H, http.MethodDelete, "/api/me/item/book-1/bookmark/99", "book-1", "99", nil, "1", "", hb.H.handleDeleteBookmark)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var list []map[string]any
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &list); err != nil {
|
||||
t.Fatalf("decode: %v; body=%s", err, rec.Body.String())
|
||||
}
|
||||
if len(list) != 0 {
|
||||
t.Errorf("list len = %d, want 0; body=%s", len(list), rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestDelete_NonExistentTime_IdempotentReturnsEmptyList(t *testing.T) {
|
||||
hb := newBookmarksHarness(t, "book-1")
|
||||
rec := dispatchBookmark(hb.H, http.MethodDelete, "/api/me/item/book-1/bookmark/123", "book-1", "123", nil, "1", "", hb.H.handleDeleteBookmark)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200 (idempotent); body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var list []map[string]any
|
||||
_ = json.Unmarshal(rec.Body.Bytes(), &list)
|
||||
if len(list) != 0 {
|
||||
t.Errorf("list len = %d, want 0", len(list))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user