diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index b403f6d..ba57a0c 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -16,6 +16,7 @@ import ( "strconv" "strings" + "github.com/jackc/pgx/v5/pgconn" _ "github.com/jackc/pgx/v5/stdlib" ) @@ -1082,6 +1083,37 @@ func (s *Store) Delete(readerID int64, key string) error { return nil } +// pgForeignKeyViolation is the SQLSTATE the driver surfaces when a Bookmark +// row refuses a Series delete (bookmarks_series_fk). pgconn exports no named +// constant for it, so the store names it here. +const pgForeignKeyViolation = "23503" + +// ErrSeriesHasBookmarks is RemoveSeries' refusal: a Reader still holds the +// Series, so the owner's removal must not reach past that record. The +// delete is the check — no NOT EXISTS pre-check that can race the insert — +// and the driver's foreign-key violation is translated here so no driver +// type escapes the store (issue #155). +var ErrSeriesHasBookmarks = errors.New("series has bookmarks") + +// RemoveSeries deletes one Series row by (site, series_id). It is refused +// while any Bookmark references the row; deleting an absent key is not an +// error, matching Delete. The caller owns the stranded Cover: read the row's +// cover_address before the delete and call ReclaimCover after it — the +// helper's guard cannot pass while the series row still points at the +// address, so the order is the sequence, not a preference. +func (s *Store) RemoveSeries(site, seriesID string) error { + if _, err := s.db.Exec( + `DELETE FROM series WHERE site = $1 AND series_id = $2`, + site, seriesID); err != nil { + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) && pgErr.Code == pgForeignKeyViolation { + return ErrSeriesHasBookmarks + } + return fmt.Errorf("remove series %s:%s: %w", site, seriesID, err) + } + return nil +} + // RecordLanePass appends one pass and prunes every older row in the same // transaction. retainBefore is supplied by the poller's clock. func (s *Store) RecordLanePass(p LanePass, retainBefore int64) error { diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index db286b9..4cb05c5 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -2340,3 +2340,112 @@ func TestReclaimCoverFailedUnlinkKeepsRow(t *testing.T) { t.Fatal("covers row after failed unlink is gone; want it left for a retry") } } + +// RemoveSeries is the orphan removal (#155): one Series, one delete, refused +// by the database while any Bookmark points at it. The store translates the +// foreign-key violation into its own sentinel so no driver type escapes, and +// the caller reaps the stranded Cover through ReclaimCover. +func TestRemoveSeriesRemovesOrphanAndReclaimsCover(t *testing.T) { + store := newTestStore(t) + seedForCheck(t, store, "asura:solo", "https://asurascans.com/comics/solo", 0) + if err := store.Delete(store.OwnerID(), "asura:solo"); err != nil { + t.Fatalf("orphan the series: %v", err) + } + if err := store.SetSeriesCover("asura", "solo", "https://cdn.example/covers/old.jpg", []byte("old-art"), "image/jpeg"); err != nil { + t.Fatalf("put cover: %v", err) + } + addr := CoverAddressForBytes([]byte("old-art")) + + if err := store.RemoveSeries("asura", "solo"); err != nil { + t.Fatalf("RemoveSeries: %v", err) + } + // The caller's sequence: the row is deleted first, then the address is + // reclaimed — the guard cannot pass while the row still points at it. + if err := store.ReclaimCover(addr); err != nil { + t.Fatalf("ReclaimCover: %v", err) + } + var one int + if err := store.db.QueryRow(`SELECT 1 FROM series WHERE site = $1 AND series_id = $2`, "asura", "solo").Scan(&one); err != sql.ErrNoRows { + t.Fatalf("series row after remove = %v, want sql.ErrNoRows", err) + } + if _, _, ok, err := store.CoverByAddress(addr); err != nil || ok { + t.Fatalf("covers row after remove = found %v err %v, want gone", ok, err) + } + if _, err := os.Stat(coverShardPath(t, store, addr)); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("sharded file after remove = %v, want fs.ErrNotExist", err) + } +} + +// The refusal is the whole point of the sentinel: a Series a Reader still +// holds is not removed, its row is untouched and its Cover keeps serving. +func TestRemoveSeriesRefusedWhileBookmarked(t *testing.T) { + store := newTestStore(t) + seedForCheck(t, store, "asura:solo", "https://asurascans.com/comics/solo", 0) + if err := store.SetSeriesCover("asura", "solo", "https://cdn.example/covers/kept.jpg", []byte("kept-art"), "image/jpeg"); err != nil { + t.Fatalf("put cover: %v", err) + } + addr := CoverAddressForBytes([]byte("kept-art")) + + if err := store.RemoveSeries("asura", "solo"); !errors.Is(err, ErrSeriesHasBookmarks) { + t.Fatalf("RemoveSeries on a bookmarked series = %v, want ErrSeriesHasBookmarks", err) + } + var held int + if err := store.db.QueryRow(`SELECT 1 FROM series WHERE site = $1 AND series_id = $2`, "asura", "solo").Scan(&held); err != nil { + t.Fatal("series row after refusal is gone; want it untouched") + } + if body, _, ok, err := store.CoverByAddress(addr); err != nil || !ok || string(body) != "kept-art" { + t.Fatalf("cover after refusal = found %v err %v, want still served", ok, err) + } +} + +// A Series sharing its Cover address with a second Series is removed while +// the artwork stays readable through CoverByAddress: the series row stops +// referencing the address first, so ReclaimCover's guard passes for this +// caller without touching the shared bytes (ADR-0014). +func TestRemoveSeriesSparesSharedCover(t *testing.T) { + store := newTestStore(t) + seedForCheck(t, store, "asura:solo", "https://asurascans.com/comics/solo", 0) + seedForCheck(t, store, "asura:second", "https://asurascans.com/comics/second", 0) + if err := store.Delete(store.OwnerID(), "asura:solo"); err != nil { + t.Fatalf("orphan solo: %v", err) + } + if err := store.Delete(store.OwnerID(), "asura:second"); err != nil { + t.Fatalf("orphan second: %v", err) + } + const src = "https://cdn.example/covers/shared.jpg" + if err := store.SetSeriesCover("asura", "solo", src, []byte("shared-art"), "image/jpeg"); err != nil { + t.Fatalf("put cover on solo: %v", err) + } + if err := store.SetSeriesCover("asura", "second", src, []byte("shared-art"), "image/jpeg"); err != nil { + t.Fatalf("put cover on second: %v", err) + } + addr := CoverAddressForBytes([]byte("shared-art")) + + if err := store.RemoveSeries("asura", "solo"); err != nil { + t.Fatalf("RemoveSeries: %v", err) + } + // The guard spares the shared bytes even though this caller reclaims. + if err := store.ReclaimCover(addr); err != nil { + t.Fatalf("ReclaimCover over a shared address: %v", err) + } + if body, _, ok, err := store.CoverByAddress(addr); err != nil || !ok || string(body) != "shared-art" { + t.Fatalf("shared bytes after remove = found %v err %v, want still served", ok, err) + } + if _, err := os.Stat(coverShardPath(t, store, addr)); err != nil { + t.Fatalf("sharded file after remove: %v, want present", err) + } + var one int + if err := store.db.QueryRow(`SELECT 1 FROM series WHERE site = $1 AND series_id = $2`, "asura", "second").Scan(&one); err != nil { + t.Fatal("the second series row vanished with the first") + } +} + +// Deleting an absent key removes nothing and is not an error, matching the +// Delete precedent — the handler's own lookups turn the absent case into the +// 404 before the store ever sees it. +func TestRemoveSeriesMissingKeyIsCleanNoOp(t *testing.T) { + store := newTestStore(t) + if err := store.RemoveSeries("asura", "ghost"); err != nil { + t.Fatalf("RemoveSeries on a missing key = %v, want nil", err) + } +} diff --git a/backend/internal/web/admin.go b/backend/internal/web/admin.go index cad99c5..c3f6aff 100644 --- a/backend/internal/web/admin.go +++ b/backend/internal/web/admin.go @@ -49,6 +49,7 @@ func (h *Handler) adminRoutes() []adminRoute { {"POST /admin/series/{key}/poll", h.adminSeriesPoll}, {"POST /admin/series/{key}/latest", h.adminSeriesCorrectLatest}, {"POST /admin/series/{key}/series-url", h.adminSeriesSetURL}, + {"POST /admin/series/{key}/remove", h.adminSeriesRemove}, {"POST /admin/lanes/{site}/pause", h.adminLanePause}, {"POST /admin/lanes/{site}/resume", h.adminLaneResume}, {"GET /ui/admin/lanes", h.uiLanes}, diff --git a/backend/internal/web/admin_series.go b/backend/internal/web/admin_series.go index d94c255..e0cfc35 100644 --- a/backend/internal/web/admin_series.go +++ b/backend/internal/web/admin_series.go @@ -1,6 +1,7 @@ package web import ( + "errors" "fmt" "log" "math" @@ -48,7 +49,6 @@ var seriesFilterOrder = []string{ } // seriesListView is the Series list page's data. The template renders strings -// and flags, and every judgement about what a value means is made here. type seriesListView struct { Filters []seriesFilterOption Sites []string @@ -57,6 +57,9 @@ type seriesListView struct { FilterLabel string Rows []seriesRowView Total int + // OOB marks the out-of-band copy of the heading the removal answer + // carries; on the page itself it is false (issue #155). + OOB bool // KindBoth / KindManga / KindNovel are the Library segment links, and // PrevHref / NextHref the pager's, all carrying the active filter, Site // and Kind so narrowing never drops state. @@ -101,6 +104,12 @@ type seriesRowView struct { CanPoll bool Pending bool Requested string // "requested 3m ago", rendered only while pending + // CanRemove is the Remove control's visibility: only a Series no Reader + // holds can be removed, so the owner is never offered a button that the + // database will always refuse (issue #155). RemovalRefused marks the one + // raced answer: the row stays and says a fresh Bookmark caught the press. + CanRemove bool + RemovalRefused bool } // adminSeries renders the filterable, bookmarkable Series list: filter, Site, @@ -279,6 +288,163 @@ func (h *Handler) adminSeriesSetURL(w http.ResponseWriter, r *http.Request) { h.render(w, http.StatusOK, "series-detail-meta", h.seriesDetailView(a)) } +// seriesListHeadView is the list heading's data. The template renders it +// inline at the top of the Series list and out of band in the removal answer +// (OOB true, like the chrome partials' OOB flag): the count and the filter +// label are one fact (issue #155). +type seriesListHeadView struct { + Total int + FilterLabel string + OOB bool +} + +// adminSeriesRemove is the orphan removal: one Series, one delete, refused by +// the database while any Bookmark exists (translated by the store, never a +// driver error on the page). The owner gate is the route's, not this +// handler's; the body is capped like the API path caps its bodies; the key is +// validated here — a malformed key is a 400 and an unknown one a 404. +// +// The Cover is read from the row before the delete and reclaimed after it: +// ReclaimCover's guard cannot pass while a series row still points at the +// address, so the order is the sequence, not a preference. A reclamation +// failure is not a removal failure — the row is gone and the covers row +// survives for a retry; the handler logs and answers success, because the +// failure has no user-facing surface. +// +// Two callers, one handler, branched on HX-Target like adminSeriesPoll. The +// detail page's remove answers with a navigation — to the No-Readers list on +// success, back to the detail page when a fresh Bookmark raced the press, +// where the new count is visible. The list row's answers with the removed +// row's fragment and the heading re-rendered with the fresh count out of +// band; HX-Reswap deletes the row through the same button that swaps the +// refusal back in, and the count query runs over the press's own filter +// state, so the heading describes the list the owner is looking at. +func (h *Handler) adminSeriesRemove(w http.ResponseWriter, r *http.Request) { + site, seriesID, ok := strings.Cut(r.PathValue("key"), ":") + if !ok || site == "" || seriesID == "" { + http.Error(w, "bad series key", http.StatusBadRequest) + return + } + r.Body = http.MaxBytesReader(w, r.Body, 1<<16) + if err := r.ParseForm(); err != nil { + http.Error(w, "invalid form", http.StatusBadRequest) + return + } + // The row's Cover address is read before the delete because the delete is + // what makes it reclaimable. + a, found, err := h.adminSeriesByKey(site, seriesID) + if err != nil { + log.Printf("series remove %s: %v", site+":"+seriesID, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + if !found { + http.NotFound(w, r) + return + } + cover := a.CoverAddress + if err := h.store.RemoveSeries(site, seriesID); err != nil { + if errors.Is(err, store.ErrSeriesHasBookmarks) { + // A Bookmark landed between the owner's read and the press: the + // row stays, answered at its new count with the fact spelled + // out — never a 500, and never a deleted row. + if r.Header.Get("HX-Target") == "detail-meta" { + seriesRemoveNavigation(w, r, "/admin/series/"+site+":"+seriesID) + return + } + fresh, found, err := h.adminSeriesByKey(site, seriesID) + if err != nil { + log.Printf("series remove %s: %v", site+":"+seriesID, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + if !found { + // A second press removed it while this one was refused; the + // row has nothing left to say. + http.NotFound(w, r) + return + } + band := 0 + if r.PostFormValue("band") == "1" { + band = 1 + } + row := seriesRow(fresh, band, time.Now()) + row.RemovalRefused = true + h.render(w, http.StatusOK, "series-row", row) + return + } + log.Printf("series remove %s: %v", site+":"+seriesID, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + if err := h.store.ReclaimCover(cover); err != nil { + log.Printf("series remove %s: reclaim cover: %v", site+":"+seriesID, err) + } + if r.Header.Get("HX-Target") == "detail-meta" { + seriesRemoveNavigation(w, r, "/admin/series?filter="+store.SeriesFilterNoReaders) + return + } + // The list answer: the removed row's fragment, plus the heading + // re-rendered with the fresh count. HX-Reswap deletes the row through the + // same button that swaps the refusal back in. The count query failing + // does not undo the removal — log it and answer the row alone. + w.Header().Set("HX-Reswap", "delete") + band := 0 + if r.PostFormValue("band") == "1" { + band = 1 + } + h.render(w, http.StatusOK, "series-row", seriesRow(a, band, time.Now())) + if head, err := h.seriesListHeadView(r); err != nil { + log.Printf("series remove %s: %v", site+":"+seriesID, err) + } else { + h.render(w, http.StatusOK, "series-list-head", head) + } +} + +// seriesListHeadView is the list heading with the count as it stands after a +// removal: the same filter, Site and Kind the press's row carried (the list +// row's button hx-includes the filterbar), so the figure describes the list +// the owner is looking at — the All filter and an unknown one stay the +// absent case. The count is the store's window total, one query. +func (h *Handler) seriesListHeadView(r *http.Request) (seriesListHeadView, error) { + filter := r.PostFormValue("filter") + if _, ok := seriesFilterLabels[filter]; !ok { + filter = store.SeriesFilterAll + } + site := r.PostFormValue("site") + kind := r.PostFormValue("kind") + if kind != store.KindManga && kind != store.KindNovel { + kind = "" + } + data, err := h.store.SeriesPage(store.SeriesFilter{ + Site: site, + Kind: kind, + Name: filter, + Cutoff: time.Now().Add(-ownerWindow).UnixMilli(), + Page: 1, + }) + if err != nil { + return seriesListHeadView{}, err + } + return seriesListHeadView{ + Total: data.Total, + FilterLabel: seriesFilterLabels[filter], + OOB: true, + }, nil +} + +// seriesRemoveNavigation answers a removal from the detail page. htmx gets a +// full navigation (HX-Redirect): a bare 303 would be followed by the request +// and the landing page swapped into the press's target, so the header is the +// redirect htmx can see; plain clients get the 303 the ticket names. +func seriesRemoveNavigation(w http.ResponseWriter, r *http.Request, to string) { + if r.Header.Get("HX-Request") != "" { + w.Header().Set("HX-Redirect", to) + return + } + http.Redirect(w, r, to, http.StatusSeeOther) +} + // seriesListView assembles one Series list view from the request's query // string. An unknown filter value is the absent All case, never an error: the // select's options are not the only way this URL can be reached. @@ -403,6 +569,7 @@ func seriesRow(a store.AdminSeries, i int, now time.Time) seriesRowView { Readers: a.ReaderCount, Band: i%2 == 1, CanPoll: canPoll, + CanRemove: a.ReaderCount == 0, Pending: pending, Requested: requested, } diff --git a/backend/internal/web/admin_series_detail.go b/backend/internal/web/admin_series_detail.go index 4166f9a..2f5e889 100644 --- a/backend/internal/web/admin_series_detail.go +++ b/backend/internal/web/admin_series_detail.go @@ -52,6 +52,10 @@ type seriesDetailView struct { CanPoll bool Pending bool Requested string + // CanRemove is the Remove control's visibility (issue #155): only a + // Series no Reader holds can be removed, so the owner is never offered a + // button the database will always refuse. + CanRemove bool } // adminSeriesDetail renders one Series' page, keyed by the composite @@ -122,6 +126,7 @@ func (h *Handler) seriesDetailView(a store.AdminSeries) seriesDetailView { Orphan: a.ReaderCount == 0, SightingRaised: a.RaisedByReader, CanPoll: canPoll, + CanRemove: a.ReaderCount == 0, Pending: pending, Requested: requested, } diff --git a/backend/internal/web/static/admin.css b/backend/internal/web/static/admin.css index dd4f23e..6eabb79 100644 --- a/backend/internal/web/static/admin.css +++ b/backend/internal/web/static/admin.css @@ -653,6 +653,13 @@ margin-left: auto; } +.admin-sheet .tbl.series .row-msg { + grid-column: 1 / -1; + margin-top: 6px; + color: var(--danger-soft); + font: 400 13px/1.4 var(--font-body); +} + .admin-sheet .detail-back { display: inline-block; margin: 18px 0 0; diff --git a/backend/internal/web/templates/series-detail.html b/backend/internal/web/templates/series-detail.html index 71f63c8..1bc2ba5 100644 --- a/backend/internal/web/templates/series-detail.html +++ b/backend/internal/web/templates/series-detail.html @@ -34,6 +34,11 @@
Check now
{{end}} +{{if .CanRemove}} +
+
+
+{{end}} {{end}} {{/* series-detail-meta is the meta line, and the answer a Check now or diff --git a/backend/internal/web/templates/series-list.html b/backend/internal/web/templates/series-list.html index c2cce35..0d51ff2 100644 --- a/backend/internal/web/templates/series-list.html +++ b/backend/internal/web/templates/series-list.html @@ -4,7 +4,7 @@ that can be bookmarked: the two selects submit the GET form, and the Library segment links and the pager preserve the filter and Site. */}} {{define "series-list"}} -
+