Compare commits

...

1 Commits

Author SHA1 Message Date
sulthan a48aba67b8 feat(backend)!: split Series from Bookmark, keeping the wire format flat
Series becomes a shared row keyed (site, series_id) owning title, cover,
canonical URL, kind, latest chapter and last-checked time (ADR-0003).
A bookmark keeps only progress, favourite, lifecycle bucket, updated_at.

Store.Upsert decomposes one flat body across both tables in one
transaction: client title/series_url/cover apply only when the series row
is new, then only the poll may change them (security boundary — the row
is shared and the values are scraped page content). Reads join series
back in, so GET/PUT emit and accept exactly the flat field set they did
before (ADR-0004), asserted by TestFlatWireFieldSet.

The poller walks Series instead of Bookmarks: one fetch per shared
series per due cycle, due queue ordered reader_count DESC then
latest_checked_at ASC, orphaned series never due and never deleted, row
still stamped before the fetch. Batch/stagger/interval unchanged.

Migration 0002 backfills series from existing bookmarks; verified by
TestMigration0002BackfillsExistingBookmarks.
2026-08-08 07:19:36 +07:00
8 changed files with 756 additions and 158 deletions
+2 -1
View File
@@ -59,7 +59,7 @@ network can reach it — so every command below goes in through the container:
```bash ```bash
$COMPOSE exec -T postgres psql -U bookmarks -d bookmarks -c '\dt' $COMPOSE exec -T postgres psql -U bookmarks -d bookmarks -c '\dt'
# -> bookmarks, schema_migrations # -> bookmarks, schema_migrations, series
``` ```
Inside the container that connects over the local socket as the `bookmarks` Inside the container that connects over the local socket as the `bookmarks`
@@ -100,6 +100,7 @@ docker run --rm -v "$BACKUP_DIR":/backup postgres:17-alpine \
pg_restore --list "/backup/bookmarks-$STAMP.dump" | grep 'TABLE DATA' pg_restore --list "/backup/bookmarks-$STAMP.dump" | grep 'TABLE DATA'
# -> 1234; 0 0 TABLE DATA public bookmarks bookmarks # -> 1234; 0 0 TABLE DATA public bookmarks bookmarks
# -> 1235; 0 0 TABLE DATA public schema_migrations bookmarks # -> 1235; 0 0 TABLE DATA public schema_migrations bookmarks
# -> 1236; 0 0 TABLE DATA public series series
# 2. Sanity-check the live row count you just captured. # 2. Sanity-check the live row count you just captured.
$COMPOSE exec -T postgres psql -U bookmarks -d bookmarks \ $COMPOSE exec -T postgres psql -U bookmarks -d bookmarks \
+21 -10
View File
@@ -21,7 +21,15 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN
container per test binary (`TestMain` -> `pgtest.Main`) and hands each test container per test binary (`TestMain` -> `pgtest.Main`) and hands each test
its own database (`pgtest.URL(t)`). A package whose tests touch the store its own database (`pgtest.URL(t)`). A package whose tests touch the store
must have that `TestMain`. must have that `TestMain`.
- **Single-user store.** One `bookmarks` table keyed `<site>:<series_id>` (`asura`|`demonic`|`comix`|`kagane`|`novelfull`|`lightnovelworld`), with a `kind` column (`manga`|`novel`) splitting the two libraries. Sync **last-write-wins**. Schema and endpoint list in plan. - **Single-user store, two tables.** `series` keyed `(site, series_id)`
(`asura`|`demonic`|`comix`|`kagane`|`novelfull`|`lightnovelworld`) owns the
shared facts — title, cover, canonical URL, `kind` (`manga`|`novel`),
Latest Chapter, `latest_checked_at` — and `bookmarks` holds only what
differs between readers: progress, favourite, lifecycle bucket,
`updated_at`. Sync **last-write-wins**; the wire format stays flat
(ADR-0004). `Store.Upsert` decomposes one flat body across both tables and
enforces the ownership rule: client `title`/`series_url`/`cover` are
written only when the series row is new (ADR-0003).
- **Endpoints:** `GET /bookmarks`, `PUT /bookmarks/{key}` (upsert; see `updated_at` rule below), `DELETE /bookmarks/{key}`, `GET /healthz` (no auth). - **Endpoints:** `GET /bookmarks`, `PUT /bookmarks/{key}` (upsert; see `updated_at` rule below), `DELETE /bookmarks/{key}`, `GET /healthz` (no auth).
- **Web UI:** same binary serve password-gated browser UI on second - **Web UI:** same binary serve password-gated browser UI on second
hostname — `GET /` (list, or login page when no session), hostname — `GET /` (list, or login page when no session),
@@ -53,22 +61,25 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN
network access, so `latest_chapter` stay fresh when user not network access, so `latest_chapter` stay fresh when user not
browsing. Second, parallel signal — userscript keep own browsing. Second, parallel signal — userscript keep own
`maybeCaptureLatestOnSeriesPage`/`backgroundRefreshLatest` logic unchanged. `maybeCaptureLatestOnSeriesPage`/`backgroundRefreshLatest` logic unchanged.
Two independent clocks: per-bookmark cooldown (`latest_checked_at` column, Two independent clocks: per-series cooldown (`series.latest_checked_at`,
enforced by `Store.DueForLatestCheck`'s WHERE clause) and wake interval. enforced by `Store.DueForLatestCheck`'s WHERE clause) and wake interval.
Row stamped *before* fetch so broken series wait out full The poller walks **Series, not Bookmarks** — a series referenced by several
cooldown instead of retrying every tick, and writes go through bookmarks is fetched once per cycle, and the due queue orders
`Store.Get` + `Store.Upsert` so new chapter never reorders list. `reader_count DESC, latest_checked_at ASC` (ADR-0003). Series row stamped
*before* fetch so broken series wait out full cooldown instead of retrying
every tick; found chapter written straight to the series row via
`Store.SetLatestChapter`, so a bookmark's `updated_at` — and the list
order — is never touched.
Fetches use `bogdanfinn/tls-client` with Chrome profile as defence in depth Fetches use `bogdanfinn/tls-client` with Chrome profile as defence in depth
against fingerprint-based blocking; any failure log and skip. kagane and against fingerprint-based blocking; any failure log and skip. kagane and
novelfull sit behind Cloudflare JavaScript challenges the TLS client can't novelfull sit behind Cloudflare JavaScript challenges the TLS client can't
clear, so they are browser-only: fetched over CDP via `BROWSER_WS_URL`, and clear, so they are browser-only: fetched over CDP via `BROWSER_WS_URL`, and
simply not polled when that's unset. See simply not polled when that's unset. See
`docs/superpowers/specs/2026-07-26-server-latest-chapter-polling-design.md`. `docs/superpowers/specs/2026-07-26-server-latest-chapter-polling-design.md`.
Poller's `Store.Get` + `Store.Upsert` not wrapped in transaction, so The poller's series write is a single-column UPDATE
userscript `PUT` that commits between the two can get overwritten by (`Store.SetLatestChapter`), not a read-modify-write of the whole bookmark:
poller's stale re-read — reverting that read progress and, since stored it cannot revert read progress or move `updated_at`, so the old
value now differs, moving `updated_at` and reordering list. Known, stale-re-read race is gone with the Get+Upsert flow.
accepted limitation for single-user deployment, not bug to fix.
- **`updated_at` drives list order, so moves only on real reading progress:** server apply its timestamp when row new or `last_chapter_num` changes, else keep stored value — favouriting series or recording newly published chapter must not reorder list. `PUT` therefore returns row **as stored**, clients must adopt that response rather than own payload. See `plans/2026-07-25-bookmark-list-favorites-design.md` §4. - **`updated_at` drives list order, so moves only on real reading progress:** server apply its timestamp when row new or `last_chapter_num` changes, else keep stored value — favouriting series or recording newly published chapter must not reorder list. `PUT` therefore returns row **as stored**, clients must adopt that response rather than own payload. See `plans/2026-07-25-bookmark-list-favorites-design.md` §4.
- **Lifecycle buckets:** `status` on each bookmark is `reading` | `archived` | - **Lifecycle buckets:** `status` on each bookmark is `reading` | `archived` |
`finished`, orthogonal to `favorite`. Archived and finished appear only in `finished`, orthogonal to `favorite`. Archived and finished appear only in
+133 -5
View File
@@ -50,26 +50,35 @@ func auth(req *http.Request) *http.Request {
func floatPtr(f float64) *float64 { return &f } func floatPtr(f float64) *float64 { return &f }
// seedForCheck inserts a bookmark and forces its latest_checked_at. // seedForCheck inserts a bookmark (and with it its series) and forces the
// series' latest_checked_at.
func seedForCheck(t *testing.T, s *store.Store, key, seriesURL string, checkedAt int64) { func seedForCheck(t *testing.T, s *store.Store, key, seriesURL string, checkedAt int64) {
t.Helper() t.Helper()
site, seriesID, ok := strings.Cut(key, ":")
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
if _, err := s.Upsert(store.Bookmark{ if _, err := s.Upsert(store.Bookmark{
Key: key, Key: key,
Site: "asura", Site: site,
SeriesID: key, SeriesID: seriesID,
SeriesURL: seriesURL, SeriesURL: seriesURL,
UpdatedAt: 1000, UpdatedAt: 1000,
}); err != nil { }); err != nil {
t.Fatalf("seed %q: %v", key, err) t.Fatalf("seed %q: %v", key, err)
} }
if err := s.MarkLatestChecked(key, checkedAt); err != nil { if err := s.MarkLatestChecked(site, seriesID, checkedAt); err != nil {
t.Fatalf("seed mark %q: %v", key, err) t.Fatalf("seed mark %q: %v", key, err)
} }
} }
func readLatestCheckedAt(t *testing.T, s *store.Store, key string) int64 { func readLatestCheckedAt(t *testing.T, s *store.Store, key string) int64 {
t.Helper() t.Helper()
ts, err := s.LatestCheckedAt(key) site, seriesID, ok := strings.Cut(key, ":")
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
ts, err := s.LatestCheckedAt(site, seriesID)
if err != nil { if err != nil {
t.Fatalf("LatestCheckedAt %q: %v", key, err) t.Fatalf("LatestCheckedAt %q: %v", key, err)
} }
@@ -229,6 +238,125 @@ func TestBookmarkRoundTrip(t *testing.T) {
} }
} }
// The wire contract (ADR-0004): GET and PUT speak exactly the flat field set
// they always did, with the series-owned fields as siblings of the bookmark
// fields, not nested. Asserted as a key set, not by inspection.
func TestFlatWireFieldSet(t *testing.T) {
srv := newTestServer(t)
key := "comix:some-title"
in := store.Bookmark{
Key: key,
Site: "comix",
SeriesID: "some-title",
Title: "Some Title",
SeriesURL: "https://comix.to/title/some-title",
Cover: "https://comix.to/covers/some-title.jpg",
LastChapter: "Chapter 7",
LastChapterNum: 7,
LastChapterURL: "https://comix.to/title/some-title/ch/7",
Favorite: true,
LatestChapter: "Chapter 8",
LatestChapterNum: floatPtr(8),
Status: store.StatusArchived,
Kind: store.KindManga,
}
body, _ := json.Marshal(in)
wantKeys := map[string]bool{
"key": true, "site": true, "series_id": true, "title": true,
"series_url": true, "cover": true, "last_chapter": true,
"last_chapter_num": true, "last_chapter_url": true, "favorite": true,
"latest_chapter": true, "latest_chapter_num": true, "updated_at": true,
"status": true, "kind": true,
}
checkFlat := func(t *testing.T, payload []byte) map[string]json.RawMessage {
t.Helper()
var obj map[string]json.RawMessage
if err := json.Unmarshal(payload, &obj); err != nil {
t.Fatalf("decode: %v", err)
}
if len(obj) != len(wantKeys) {
t.Fatalf("field count = %d, want %d (%s)", len(obj), len(wantKeys), payload)
}
for k := range obj {
if !wantKeys[k] {
t.Fatalf("unexpected field %q", k)
}
}
return obj
}
// PUT
rr := httptest.NewRecorder()
srv.ServeHTTP(rr, auth(httptest.NewRequest(http.MethodPut, "/bookmarks/"+key, bytes.NewReader(body))))
if rr.Code != http.StatusOK {
t.Fatalf("PUT status = %d, want 200", rr.Code)
}
checkFlat(t, rr.Body.Bytes())
// Every field round-trips with its value, and updated_at is server-stamped.
var stored store.Bookmark
if err := json.Unmarshal(rr.Body.Bytes(), &stored); err != nil {
t.Fatalf("decode PUT response: %v", err)
}
latestNum := floatPtr(8)
want := store.Bookmark{
Key: key, Site: "comix", SeriesID: "some-title",
Title: in.Title, SeriesURL: in.SeriesURL, Cover: in.Cover,
LastChapter: in.LastChapter, LastChapterNum: in.LastChapterNum,
LastChapterURL: in.LastChapterURL, Favorite: true,
LatestChapter: in.LatestChapter, LatestChapterNum: latestNum,
Status: store.StatusArchived, Kind: store.KindManga,
}
if stored.Title != want.Title || stored.SeriesURL != want.SeriesURL || stored.Cover != want.Cover ||
stored.LastChapter != want.LastChapter || stored.LastChapterNum != want.LastChapterNum ||
stored.LastChapterURL != want.LastChapterURL || stored.Favorite != want.Favorite ||
stored.LatestChapter != want.LatestChapter ||
stored.LatestChapterNum == nil || *stored.LatestChapterNum != *want.LatestChapterNum ||
stored.Status != want.Status || stored.Kind != want.Kind {
t.Fatalf("PUT response = %+v, want %+v", stored, want)
}
if stored.UpdatedAt == 0 {
t.Fatal("updated_at not server-stamped")
}
// GET reports the same flat shape.
list := getBookmarks(t, srv)
if len(list) != 1 {
t.Fatalf("list = %d items, want 1", len(list))
}
body2, _ := json.Marshal(list[0])
checkFlat(t, body2)
}
// A PUT naming an existing series must ignore client-supplied title, cover and
// URL — the security boundary from ADR-0003, where a hostile site's scraped
// values could otherwise land on a shared row — while progress still lands.
func TestPutExistingSeriesIgnoresClientTitleCoverURL(t *testing.T) {
srv := newTestServer(t)
key := "asura:solo"
first := putBookmark(t, srv, key, store.Bookmark{
Title: "Solo Leveling",
SeriesURL: "https://asurascans.com/comics/solo",
Cover: "https://asurascans.com/covers/solo.jpg",
LastChapterNum: 10,
})
second := putBookmark(t, srv, key, store.Bookmark{
Title: "Scraped Rename",
SeriesURL: "https://evil.example/solo",
Cover: "https://evil.example/solo.jpg",
LastChapterNum: 11,
})
if second.Title != first.Title || second.SeriesURL != first.SeriesURL || second.Cover != first.Cover {
t.Fatalf("stored = %+v, want original title/url/cover kept", second)
}
if second.LastChapterNum != 11 {
t.Fatalf("LastChapterNum = %v, want 11 — progress must still land", second.LastChapterNum)
}
}
// putBookmark PUTs b at key and returns the bookmark the server echoes back, // putBookmark PUTs b at key and returns the bookmark the server echoes back,
// which is the row as actually stored (not the request payload). // which is the row as actually stored (not the request payload).
func putBookmark(t *testing.T, srv http.Handler, key string, b store.Bookmark) store.Bookmark { func putBookmark(t *testing.T, srv http.Handler, key string, b store.Bookmark) store.Bookmark {
+29 -46
View File
@@ -23,9 +23,9 @@ type Fetcher interface {
// Two clocks, deliberately independent: // Two clocks, deliberately independent:
// //
// - Interval is how often this goroutine wakes up and looks. // - Interval is how often this goroutine wakes up and looks.
// - Cooldown is how long one bookmark rests since its own last check. // - Cooldown is how long one series rests since its own last check.
// //
// Only the cooldown is per bookmark, and it is enforced by the WHERE clause in // Only the cooldown is per series, and it is enforced by the WHERE clause in
// DueForLatestCheck rather than by any timer. Shortening Interval therefore // DueForLatestCheck rather than by any timer. Shortening Interval therefore
// cannot shorten anyone's cooldown; it only makes the poller wake up and find // cannot shorten anyone's cooldown; it only makes the poller wake up and find
// nothing due more often. // nothing due more often.
@@ -78,7 +78,7 @@ func (p *Poller) Run(ctx context.Context) {
} }
} }
// runOnce processes one batch of due bookmarks. // runOnce processes one batch of due series.
func (p *Poller) runOnce(ctx context.Context) { func (p *Poller) runOnce(ctx context.Context) {
cutoff := p.Now().Add(-p.Cooldown).UnixMilli() cutoff := p.Now().Add(-p.Cooldown).UnixMilli()
due, err := p.Store.DueForLatestCheck(cutoff, p.Batch) due, err := p.Store.DueForLatestCheck(cutoff, p.Batch)
@@ -88,7 +88,7 @@ func (p *Poller) runOnce(ctx context.Context) {
} }
checked := 0 checked := 0
for i, b := range due { for i, sr := range due {
if ctx.Err() != nil { if ctx.Err() != nil {
break break
} }
@@ -107,7 +107,7 @@ func (p *Poller) runOnce(ctx context.Context) {
if stopped { if stopped {
break break
} }
p.checkOne(ctx, b) p.checkOne(ctx, sr)
checked++ checked++
} }
// due vs checked is how you tell which constraint is binding: ticks that // due vs checked is how you tell which constraint is binding: ticks that
@@ -119,10 +119,10 @@ func (p *Poller) runOnce(ctx context.Context) {
// checkOne re-checks one series. Every failure path here is "log and move on": // checkOne re-checks one series. Every failure path here is "log and move on":
// the poller is a best-effort enhancement, and no single bad series may stall a // the poller is a best-effort enhancement, and no single bad series may stall a
// batch or take down the process. // batch or take down the process.
func (p *Poller) checkOne(ctx context.Context, b store.Bookmark) { func (p *Poller) checkOne(ctx context.Context, sr store.Series) {
defer func() { defer func() {
if r := recover(); r != nil { if r := recover(); r != nil {
log.Printf("latest poll %q: recovered from panic: %v", b.Key, r) log.Printf("latest poll %q: recovered from panic: %v", sr.Key(), r)
} }
}() }()
@@ -130,8 +130,8 @@ func (p *Poller) checkOne(ctx context.Context, b store.Bookmark) {
// mid-request still consumes the cooldown. Otherwise a renamed or deleted // mid-request still consumes the cooldown. Otherwise a renamed or deleted
// series would be retried on every single tick forever. The userscript // series would be retried on every single tick forever. The userscript
// stamps in the same order and for the same reason (L471-473). // stamps in the same order and for the same reason (L471-473).
if err := p.Store.MarkLatestChecked(b.Key, p.Now().UnixMilli()); err != nil { if err := p.Store.MarkLatestChecked(sr.Site, sr.SeriesID, p.Now().UnixMilli()); err != nil {
log.Printf("latest poll %q: mark checked: %v", b.Key, err) log.Printf("latest poll %q: mark checked: %v", sr.Key(), err)
return return
} }
@@ -142,69 +142,52 @@ func (p *Poller) checkOne(ctx context.Context, b store.Bookmark) {
// link-local/internal addresses or non-https schemes. The cooldown above // link-local/internal addresses or non-https schemes. The cooldown above
// is already consumed, so a row that never passes this check is retried at // is already consumed, so a row that never passes this check is retried at
// cooldown pace rather than hot-looping. // cooldown pace rather than hot-looping.
if !fetchableSeriesURL(b.Site, b.SeriesURL) { if !fetchableSeriesURL(sr.Site, sr.SeriesURL) {
log.Printf("latest poll %q: not fetchable: site=%q url=%q", b.Key, b.Site, b.SeriesURL) log.Printf("latest poll %q: not fetchable: site=%q url=%q", sr.Key(), sr.Site, sr.SeriesURL)
return return
} }
f := p.fetcherFor(b.Site) f := p.fetcherFor(sr.Site)
if f == nil { if f == nil {
log.Printf("latest poll %q: no fetcher for site %q", b.Key, b.Site) log.Printf("latest poll %q: no fetcher for site %q", sr.Key(), sr.Site)
return return
} }
body, status, err := f.Get(ctx, b.SeriesURL) body, status, err := f.Get(ctx, sr.SeriesURL)
if err != nil { if err != nil {
log.Printf("latest poll %q: fetch %s: %v", b.Key, b.SeriesURL, err) log.Printf("latest poll %q: fetch %s: %v", sr.Key(), sr.SeriesURL, err)
return return
} }
if status != 200 { if status != 200 {
log.Printf("latest poll %q: fetch %s: status %d", b.Key, b.SeriesURL, status) log.Printf("latest poll %q: fetch %s: status %d", sr.Key(), sr.SeriesURL, status)
return return
} }
latest, ok := latestChapterFrom(b.Site, b.SeriesURL, body) latest, ok := latestChapterFrom(sr.Site, sr.SeriesURL, body)
if !ok { if !ok {
// Most likely a challenge page or a layout change. Either way the row is // Most likely a challenge page or a layout change. Either way the row is
// already stamped, so this waits out a cooldown instead of hot-looping. // already stamped, so this waits out a cooldown instead of hot-looping.
log.Printf("latest poll %q: no chapter links in %d bytes", b.Key, len(body)) log.Printf("latest poll %q: no chapter links in %d bytes", sr.Key(), len(body))
return return
} }
// Re-read: the row may have been updated or deleted while the fetch was in
// flight, and writing b back wholesale would undo that.
//
// ponytail: non-transactional read-modify-write, wrap Get+Upsert in a tx if
// this ever runs for more than one user. A client PUT that commits between
// these two statements is lost to the stale re-read — reverting read
// progress or a status change, and moving updated_at because the stored
// value now differs. Accepted for a single-user deployment: the window is
// milliseconds and the loser is one poll cycle.
cur, found, err := p.Store.Get(b.Key)
if err != nil {
log.Printf("latest poll %q: reread: %v", b.Key, err)
return
}
if !found {
return
}
// Equality, not >, mirroring the userscript (L427): a site that retracts a // Equality, not >, mirroring the userscript (L427): a site that retracts a
// chapter should correct the stored number downward. // chapter should correct the stored number downward. The comparison is
if cur.LatestChapterNum != nil && *cur.LatestChapterNum == latest.Num { // against the due-query snapshot; a concurrent write in between only costs
// one redundant UPDATE of the same absolute value, never a wrong one.
if sr.LatestChapterNum != nil && *sr.LatestChapterNum == latest.Num {
return return
} }
num := latest.Num // Series-level write: the row is shared, so one update refreshes every
cur.LatestChapter = latest.Label // bookmark joining to it, and the bookmark's updated_at is never touched —
cur.LatestChapterNum = &num // a newly published chapter is not reading progress and must not reorder
// A candidate only. last_chapter_num is untouched, so the CASE in Upsert // the list.
// keeps the stored updated_at and the bookmark list does not reorder. if err := p.Store.SetLatestChapter(sr.Site, sr.SeriesID, latest.Label, latest.Num); err != nil {
cur.UpdatedAt = p.Now().UnixMilli() log.Printf("latest poll %q: set latest chapter: %v", sr.Key(), err)
if _, err := p.Store.Upsert(cur); err != nil {
log.Printf("latest poll %q: upsert: %v", b.Key, err)
return return
} }
log.Printf("latest poll %q: latest is now %s", b.Key, latest.Label) log.Printf("latest poll %q: latest is now %s", sr.Key(), latest.Label)
} }
// fetchableSeriesURL reports whether site is a site latestChapterFrom knows how // fetchableSeriesURL reports whether site is a site latestChapterFrom knows how
+52 -7
View File
@@ -4,6 +4,7 @@ import (
"context" "context"
"errors" "errors"
"os" "os"
"strings"
"sync" "sync"
"testing" "testing"
"time" "time"
@@ -25,26 +26,35 @@ func newTestStore(t *testing.T) *store.Store {
return s return s
} }
// seedForCheck inserts a bookmark and forces its latest_checked_at. // seedForCheck inserts a bookmark (and with it its series) and forces the
// series' latest_checked_at.
func seedForCheck(t *testing.T, s *store.Store, key, seriesURL string, checkedAt int64) { func seedForCheck(t *testing.T, s *store.Store, key, seriesURL string, checkedAt int64) {
t.Helper() t.Helper()
site, seriesID, ok := strings.Cut(key, ":")
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
if _, err := s.Upsert(store.Bookmark{ if _, err := s.Upsert(store.Bookmark{
Key: key, Key: key,
Site: "asura", Site: site,
SeriesID: key, SeriesID: seriesID,
SeriesURL: seriesURL, SeriesURL: seriesURL,
UpdatedAt: 1000, UpdatedAt: 1000,
}); err != nil { }); err != nil {
t.Fatalf("seed %q: %v", key, err) t.Fatalf("seed %q: %v", key, err)
} }
if err := s.MarkLatestChecked(key, checkedAt); err != nil { if err := s.MarkLatestChecked(site, seriesID, checkedAt); err != nil {
t.Fatalf("seed mark %q: %v", key, err) t.Fatalf("seed mark %q: %v", key, err)
} }
} }
func readLatestCheckedAt(t *testing.T, s *store.Store, key string) int64 { func readLatestCheckedAt(t *testing.T, s *store.Store, key string) int64 {
t.Helper() t.Helper()
ts, err := s.LatestCheckedAt(key) site, seriesID, ok := strings.Cut(key, ":")
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
ts, err := s.LatestCheckedAt(site, seriesID)
if err != nil { if err != nil {
t.Fatalf("LatestCheckedAt %q: %v", key, err) t.Fatalf("LatestCheckedAt %q: %v", key, err)
} }
@@ -217,6 +227,41 @@ func TestRunOnceRespectsBatchLimit(t *testing.T) {
} }
} }
// The point of the split (ADR-0003): a series referenced by several bookmarks
// is fetched once per due cycle, not once per bookmark. Today the bookmark key
// is <site>:<series_id>, so the second bookmark only exists once keys stop
// being derived from the series identity (issue #22).
func TestRunOnceFetchesSharedSeriesOnce(t *testing.T) {
s := newTestStore(t)
// The slug must match the fixture's own anchors: asura's parser scopes
// chapter links to the stored slug.
const slug = "chronicles-of-the-demon-faction-f886a8af"
const url = "https://asurascans.com/comics/" + slug
seedForCheck(t, s, "asura:"+slug, url, 0)
if _, err := s.Upsert(store.Bookmark{
Key: "asura:" + slug + ":2", Site: "asura", SeriesID: slug, UpdatedAt: 2000,
}); err != nil {
t.Fatalf("seed second reader: %v", err)
}
f := &fakeFetcher{body: asuraSeriesFixture, status: 200}
newTestPoller(t, s, f, time.UnixMilli(5_000_000)).runOnce(context.Background())
if got := f.callCount(); got != 1 {
t.Fatalf("fetched shared series %d times, want 1", got)
}
// Both bookmarks join to the same updated series row.
for _, key := range []string{"asura:" + slug, "asura:" + slug + ":2"} {
b, ok, err := s.Get(key)
if err != nil || !ok {
t.Fatalf("Get %s: %v ok=%v", key, err, ok)
}
if b.LatestChapterNum == nil || *b.LatestChapterNum != 181 {
t.Fatalf("%s LatestChapterNum = %v, want 181", key, b.LatestChapterNum)
}
}
}
// One unreachable series must not abandon the rest of the batch. // One unreachable series must not abandon the rest of the batch.
func TestRunOnceOneBadSeriesDoesNotStallBatch(t *testing.T) { func TestRunOnceOneBadSeriesDoesNotStallBatch(t *testing.T) {
s := newTestStore(t) s := newTestStore(t)
@@ -330,8 +375,8 @@ func TestCheckOneValidatesSeriesURLBeforeFetching(t *testing.T) {
now := time.UnixMilli(4_000_000) now := time.UnixMilli(4_000_000)
f := &fakeFetcher{body: asuraSeriesFixture, status: 200} f := &fakeFetcher{body: asuraSeriesFixture, status: 200}
newTestPoller(t, s, f, now).checkOne(context.Background(), store.Bookmark{ newTestPoller(t, s, f, now).checkOne(context.Background(), store.Series{
Key: key, Site: tt.site, SeriesURL: tt.seriesURL, Site: tt.site, SeriesID: "x", SeriesURL: tt.seriesURL,
}) })
if got := f.callCount(); got != tt.wantCalls { if got := f.callCount(); got != tt.wantCalls {
@@ -0,0 +1,45 @@
-- One row per distinct work, shared by every bookmark that tracks it
-- (ADR-0003). Keyed (site, series_id), the pair a bookmark key decomposes
-- into. title/series_url/cover are written once, at creation, and never
-- again: client-supplied values are ignored once the row exists and the
-- poller is the only party that may change them. kind and the latest-chapter
-- fields are last-write-wins like the bookmark's own fields.
CREATE TABLE series (
site text NOT NULL,
series_id text NOT NULL,
title text NOT NULL DEFAULT '',
series_url text NOT NULL DEFAULT '',
cover text NOT NULL DEFAULT '',
kind text NOT NULL DEFAULT 'manga',
latest_chapter text NOT NULL DEFAULT '',
latest_chapter_num double precision,
-- When the server last polled this series, unix ms; 0 means never, and sorts
-- first so a new bookmark is picked up on the next tick with no special case.
latest_checked_at bigint NOT NULL DEFAULT 0,
PRIMARY KEY (site, series_id)
);
-- Backfill from today's rows. The bookmark key's uniqueness makes
-- (site, series_id) unique in practice; DISTINCT is belt and braces.
INSERT INTO series (site, series_id, title, series_url, cover, kind,
latest_chapter, latest_chapter_num, latest_checked_at)
SELECT DISTINCT site, series_id, title, series_url, cover, kind,
latest_chapter, latest_chapter_num, latest_checked_at
FROM bookmarks;
-- The bookmark keeps only what differs between readers (ADR-0003): progress,
-- favourite, lifecycle bucket. The dropped columns now live on series.
ALTER TABLE bookmarks
DROP COLUMN title,
DROP COLUMN series_url,
DROP COLUMN cover,
DROP COLUMN kind,
DROP COLUMN latest_chapter,
DROP COLUMN latest_chapter_num,
DROP COLUMN latest_checked_at;
-- A bookmark may not point at a series that does not exist. No cascade: a
-- series outlives its last bookmark, and deleting one is not a store operation.
ALTER TABLE bookmarks
ADD CONSTRAINT bookmarks_series_fk
FOREIGN KEY (site, series_id) REFERENCES series (site, series_id);
+183 -73
View File
@@ -19,6 +19,11 @@ import (
// //
// LastChapter* is the user's read progress; LatestChapter* is the newest // LastChapter* is the user's read progress; LatestChapter* is the newest
// chapter the site has published, captured opportunistically by the userscript. // chapter the site has published, captured opportunistically by the userscript.
//
// Title, SeriesURL, Cover, Kind and LatestChapter* live on the shared Series
// row (ADR-0003) and are joined in on read; Bookmark carries only what differs
// between readers: progress, favourite, lifecycle bucket, updated_at. The wire
// format stays flat regardless — see ADR-0004.
type Bookmark struct { type Bookmark struct {
Key string `json:"key"` Key string `json:"key"`
Site string `json:"site"` Site string `json:"site"`
@@ -42,6 +47,35 @@ type Bookmark struct {
Kind string `json:"kind"` Kind string `json:"kind"`
} }
// Series is one distinct work, shared by every bookmark that tracks it. It is
// keyed (site, series_id) — the pair a bookmark key decomposes into — and
// exists once no matter how many bookmarks point at it (ADR-0003).
//
// Title, SeriesURL and Cover are written once, at creation: a PUT naming an
// existing Series has them ignored, and only the backend's own Poll may change
// them. Kind and the latest-chapter fields are last-write-wins like the
// bookmark's own fields. Never serialized: the wire format is the flat
// Bookmark (ADR-0004).
type Series struct {
Site string
SeriesID string
Title string
SeriesURL string
Cover string
Kind string
LatestChapter string
LatestChapterNum *float64 // nil until first captured
LatestCheckedAt int64 // unix ms; see MarkLatestChecked
// readerCount is the number of bookmarks referencing this series, filled
// only by the due-queue query that orders on it.
readerCount int
}
// Key returns the canonical identity in bookmark-key form ("<site>:<series_id>"),
// used by the poller's logs and by tests asserting on the due queue.
func (s Series) Key() string { return s.Site + ":" + s.SeriesID }
// HasNewChapter reports whether the site has published past the read point. // HasNewChapter reports whether the site has published past the read point.
// A nil LatestChapterNum means nothing has been captured yet, which is not the // A nil LatestChapterNum means nothing has been captured yet, which is not the
// same as "nothing new". // same as "nothing new".
@@ -122,10 +156,18 @@ const (
var migrations embed.FS var migrations embed.FS
// bookmarkColumns is the only value ever concatenated into query text. It is a // bookmarkColumns is the only value ever concatenated into query text. It is a
// compile-time constant; every request value is bound as a parameter. // compile-time constant; every request value is bound as a parameter. The
const bookmarkColumns = `key, site, series_id, title, series_url, cover, // series-owned fields are joined in from the series table, in scanBookmark
last_chapter, last_chapter_num, last_chapter_url, // order, so the flat Bookmark reads back whole despite the split (ADR-0004).
favorite, latest_chapter, latest_chapter_num, updated_at, status, kind` const bookmarkColumns = `b.key, b.site, b.series_id, s.title, s.series_url, s.cover,
b.last_chapter, b.last_chapter_num, b.last_chapter_url,
b.favorite, s.latest_chapter, s.latest_chapter_num, b.updated_at, b.status, s.kind`
// seriesColumns is the series row in scanSeries order, used by the poller's
// due query. latest_checked_at lives only on series — see MarkLatestChecked
// for why it stays off every client-visible write.
const seriesColumns = `s.site, s.series_id, s.title, s.series_url, s.cover,
s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at`
// Store is the Postgres-backed bookmark store. // Store is the Postgres-backed bookmark store.
type Store struct { type Store struct {
@@ -237,14 +279,37 @@ func scanBookmark(scan func(...any) error) (Bookmark, error) {
return b, nil return b, nil
} }
// scanSeries reads one row in seriesColumns order, plus the due query's
// reader_count column. latest_chapter_num is NULL until the first capture,
// same as on the bookmark read path.
func scanSeries(scan func(...any) error) (Series, error) {
var (
sr Series
latestChapterNum sql.NullFloat64
)
if err := scan(
&sr.Site, &sr.SeriesID, &sr.Title, &sr.SeriesURL, &sr.Cover,
&sr.Kind, &sr.LatestChapter, &latestChapterNum, &sr.LatestCheckedAt,
&sr.readerCount,
); err != nil {
return Series{}, err
}
if latestChapterNum.Valid {
sr.LatestChapterNum = &latestChapterNum.Float64
}
return sr, nil
}
// Close releases the underlying database handle. // Close releases the underlying database handle.
func (s *Store) Close() error { return s.db.Close() } func (s *Store) Close() error { return s.db.Close() }
// List returns every bookmark, newest activity first. // List returns every bookmark, newest activity first. Series-owned fields are
// joined in, so each Bookmark reads back whole and flat (ADR-0004).
func (s *Store) List() ([]Bookmark, error) { func (s *Store) List() ([]Bookmark, error) {
rows, err := s.db.Query(`SELECT ` + bookmarkColumns + ` rows, err := s.db.Query(`SELECT ` + bookmarkColumns + `
FROM bookmarks FROM bookmarks b
ORDER BY updated_at DESC`) JOIN series s ON s.site = b.site AND s.series_id = b.series_id
ORDER BY b.updated_at DESC`)
if err != nil { if err != nil {
return nil, fmt.Errorf("query bookmarks: %w", err) return nil, fmt.Errorf("query bookmarks: %w", err)
} }
@@ -266,7 +331,9 @@ func (s *Store) List() ([]Bookmark, error) {
// the fields they do not touch. // the fields they do not touch.
func (s *Store) Get(key string) (Bookmark, bool, error) { func (s *Store) Get(key string) (Bookmark, bool, error) {
b, err := scanBookmark(s.db.QueryRow( b, err := scanBookmark(s.db.QueryRow(
`SELECT `+bookmarkColumns+` FROM bookmarks WHERE key = $1`, key).Scan) `SELECT `+bookmarkColumns+` FROM bookmarks b
JOIN series s ON s.site = b.site AND s.series_id = b.series_id
WHERE b.key = $1`, key).Scan)
if errors.Is(err, sql.ErrNoRows) { if errors.Is(err, sql.ErrNoRows) {
return Bookmark{}, false, nil return Bookmark{}, false, nil
} }
@@ -277,7 +344,15 @@ func (s *Store) Get(key string) (Bookmark, bool, error) {
} }
// Upsert inserts or replaces a bookmark by key (last-write-wins) and returns // Upsert inserts or replaces a bookmark by key (last-write-wins) and returns
// the row as actually stored. // the row as actually stored — one flat object with the series-owned fields
// joined in, exactly as GET reports it (ADR-0004).
//
// The flat body is decomposed across two tables in one transaction. The series
// row is written first (the bookmarks FK requires it to exist), then the
// bookmark row. On the series side, title/series_url/cover are applied only
// when the row is brand new: once a series exists, client-supplied values are
// ignored, because the row is shared and the values are scraped page content —
// see ADR-0003. Kind and the latest-chapter fields are last-write-wins.
// //
// b.UpdatedAt is only a candidate: it is applied when the row is new or when // b.UpdatedAt is only a candidate: it is applied when the row is new or when
// last_chapter_num changes, and otherwise the stored value is kept. Clients // last_chapter_num changes, and otherwise the stored value is kept. Clients
@@ -296,53 +371,65 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) {
latestNum = *b.LatestChapterNum latestNum = *b.LatestChapterNum
} }
// IS DISTINCT FROM is Postgres's null-safe comparison, and it is what // The kind column resolves on the VALUES side, not in the conflict clause:
// implements the ordering rule. Within DO UPDATE, a bare column is the // excluded.* is the row *after* these expressions are evaluated, so a
// stored row and excluded.* is the incoming one; a brand-new key never // default applied there would look identical to a real 'manga' and would
// reaches this clause, so it keeps the fresh timestamp from VALUES. // overwrite a novel series on every PUT from a client that knows nothing
// // about the column. Resolved once here, an empty incoming kind means "keep
// The status and kind columns resolve on the VALUES side, not in the // what is stored", and only a brand-new row falls through to the literal
// conflict clause: excluded.* is the row *after* these expressions are // default. The subquery runs inside this transaction, so it sees the row
// evaluated, so a default applied there would look identical to a real // this statement is about to conflict with. Same pattern as the status
// 'reading' / 'manga' and would overwrite an archived or novel row on // COALESCE on the bookmark insert below.
// every PUT from a client that knows nothing about the column. Resolved
// once here, an empty incoming status or kind means "keep what is
// stored", and only a brand-new row falls through to the literal
// default. The subquery runs inside this transaction, so it sees the
// row this statement is about to conflict with.
// //
// The ::text casts are load-bearing: inside COALESCE/NULLIF there is no // The ::text casts are load-bearing: inside COALESCE/NULLIF there is no
// target column to infer the parameter type from, and Postgres rejects the // target column to infer the parameter type from, and Postgres rejects the
// statement rather than guessing. // statement rather than guessing.
if _, err := tx.Exec(` if _, err := tx.Exec(`
INSERT INTO bookmarks (`+bookmarkColumns+`) INSERT INTO series (site, series_id, title, series_url, cover, kind,
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, latest_chapter, latest_chapter_num)
COALESCE(NULLIF($14::text, ''), (SELECT status FROM bookmarks WHERE key = $1), 'reading'), VALUES ($1, $2, $3, $4, $5,
COALESCE(NULLIF($15::text, ''), (SELECT kind FROM bookmarks WHERE key = $1), 'manga')) COALESCE(NULLIF($6::text, ''), (SELECT kind FROM series WHERE site = $1 AND series_id = $2), 'manga'),
$7, $8)
ON CONFLICT (site, series_id) DO UPDATE SET
kind=excluded.kind,
latest_chapter=excluded.latest_chapter,
latest_chapter_num=excluded.latest_chapter_num`,
b.Site, b.SeriesID, b.Title, b.SeriesURL, b.Cover, b.Kind,
b.LatestChapter, latestNum); err != nil {
return Bookmark{}, fmt.Errorf("upsert series for %q: %w", b.Key, err)
}
// IS DISTINCT FROM is Postgres's null-safe comparison, and it is what
// implements the ordering rule. Within DO UPDATE, a bare column is the
// stored row and excluded.* is the incoming one; a brand-new key never
// reaches this clause, so it keeps the fresh timestamp from VALUES.
if _, err := tx.Exec(`
INSERT INTO bookmarks (key, site, series_id, last_chapter, last_chapter_num,
last_chapter_url, favorite, status, updated_at)
VALUES ($1, $2, $3, $4, $5, $6, $7,
COALESCE(NULLIF($8::text, ''), (SELECT status FROM bookmarks WHERE key = $1), 'reading'),
$9)
ON CONFLICT (key) DO UPDATE SET ON CONFLICT (key) DO UPDATE SET
site=excluded.site, series_id=excluded.series_id, title=excluded.title, site=excluded.site, series_id=excluded.series_id,
series_url=excluded.series_url, cover=excluded.cover,
last_chapter=excluded.last_chapter, last_chapter_num=excluded.last_chapter_num, last_chapter=excluded.last_chapter, last_chapter_num=excluded.last_chapter_num,
last_chapter_url=excluded.last_chapter_url, last_chapter_url=excluded.last_chapter_url,
favorite=excluded.favorite, favorite=excluded.favorite,
latest_chapter=excluded.latest_chapter,
latest_chapter_num=excluded.latest_chapter_num,
status=excluded.status, status=excluded.status,
kind=excluded.kind,
updated_at=CASE updated_at=CASE
WHEN bookmarks.last_chapter_num IS DISTINCT FROM excluded.last_chapter_num WHEN bookmarks.last_chapter_num IS DISTINCT FROM excluded.last_chapter_num
THEN excluded.updated_at THEN excluded.updated_at
ELSE bookmarks.updated_at ELSE bookmarks.updated_at
END`, END`,
b.Key, b.Site, b.SeriesID, b.Title, b.SeriesURL, b.Cover, b.Key, b.Site, b.SeriesID,
b.LastChapter, b.LastChapterNum, b.LastChapterURL, b.LastChapter, b.LastChapterNum, b.LastChapterURL,
b.Favorite, b.LatestChapter, latestNum, b.UpdatedAt, b.Favorite, b.Status, b.UpdatedAt); err != nil {
b.Status, b.Kind); err != nil {
return Bookmark{}, fmt.Errorf("upsert %q: %w", b.Key, err) return Bookmark{}, fmt.Errorf("upsert %q: %w", b.Key, err)
} }
stored, err := scanBookmark(tx.QueryRow( stored, err := scanBookmark(tx.QueryRow(
`SELECT `+bookmarkColumns+` FROM bookmarks WHERE key = $1`, b.Key).Scan) `SELECT `+bookmarkColumns+` FROM bookmarks b
JOIN series s ON s.site = b.site AND s.series_id = b.series_id
WHERE b.key = $1`, b.Key).Scan)
if err != nil { if err != nil {
return Bookmark{}, fmt.Errorf("read back %q: %w", b.Key, err) return Bookmark{}, fmt.Errorf("read back %q: %w", b.Key, err)
} }
@@ -360,71 +447,94 @@ func (s *Store) Delete(key string) error {
return nil return nil
} }
// DueForLatestCheck returns bookmarks whose server-side latest-chapter check has // DueForLatestCheck returns series whose server-side latest-chapter check has
// aged past cutoffMs, least-recently-checked first, at most limit of them. // aged past cutoffMs, ordered by how many bookmarks reference them (descending)
// then least-recently-checked first, at most limit of them.
// //
// Oldest-first is what keeps the poller fair when the backlog outgrows its // The reader_count ordering is the point of the split (ADR-0003): a series
// shared by several readers is fetched once per due cycle, and the popular
// ones stay freshest while the long tail absorbs any shortfall. Within one
// reader count, oldest-first keeps the poll fair when the backlog outgrows its
// throughput: the most neglected series is always next, so a large collection // throughput: the most neglected series is always next, so a large collection
// refreshes uniformly slower rather than leaving a tail that never refreshes at // refreshes uniformly slower rather than leaving a tail that never refreshes at
// all. The userscript sorts its own queue the same way (L453). // all. The userscript sorts its own queue the same way (L453).
// //
// Bookmarks with no series_url are skipped — there is nothing to fetch, which // Series with no series_url are skipped — there is nothing to fetch, which is
// is the same filter the userscript applies at L452. // the same filter the userscript applies at L452. Series whose only bookmarks
// // are finished are skipped too: nothing more is coming, so fetching them only
// Finished series are excluded: nothing more is coming, so fetching them only // burns requests. Archived bookmarks still count — knowing what a shelved
// burns requests. Archived ones are deliberately still polled — knowing what a // series is up to is the whole reason for archiving instead of deleting.
// shelved series is up to is the whole reason for archiving instead of deleting. // A series with no bookmarks at all never appears: the join excludes it.
func (s *Store) DueForLatestCheck(cutoffMs int64, limit int) ([]Bookmark, error) { func (s *Store) DueForLatestCheck(cutoffMs int64, limit int) ([]Series, error) {
rows, err := s.db.Query(`SELECT `+bookmarkColumns+` rows, err := s.db.Query(`SELECT `+seriesColumns+`, COUNT(b.key) AS reader_count
FROM bookmarks FROM series s
WHERE series_url <> '' JOIN bookmarks b ON b.site = s.site AND b.series_id = s.series_id
AND status IS DISTINCT FROM 'finished' WHERE s.series_url <> ''
AND latest_checked_at <= $1 AND s.latest_checked_at <= $1
ORDER BY latest_checked_at ASC GROUP BY s.site, s.series_id, s.title, s.series_url, s.cover,
s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at
HAVING COUNT(b.key) FILTER (WHERE b.status <> 'finished') > 0
ORDER BY reader_count DESC, s.latest_checked_at ASC
LIMIT $2`, cutoffMs, limit) LIMIT $2`, cutoffMs, limit)
if err != nil { if err != nil {
return nil, fmt.Errorf("query due bookmarks: %w", err) return nil, fmt.Errorf("query due series: %w", err)
} }
defer rows.Close() defer rows.Close()
out := []Bookmark{} out := []Series{}
for rows.Next() { for rows.Next() {
b, err := scanBookmark(rows.Scan) sr, err := scanSeries(rows.Scan)
if err != nil { if err != nil {
return nil, fmt.Errorf("scan due bookmark: %w", err) return nil, fmt.Errorf("scan due series: %w", err)
} }
out = append(out, b) out = append(out, sr)
} }
return out, rows.Err() return out, rows.Err()
} }
// MarkLatestChecked records that the server looked at key at ts, whatever the // MarkLatestChecked records that the server looked at a series at ts, whatever
// look turned up. Marking a missing key is not an error: the row may have been // the look turned up. Marking a missing series is not an error: the row may
// deleted while a fetch was in flight. // have been orphaned while a fetch was in flight.
// //
// This is the one write that does not go through Upsert, and the column is kept // This is the one write that does not go through Upsert, and the column is kept
// out of bookmarkColumns on purpose. PUT /bookmarks/{key} decodes a whole // out of the client-visible read path on purpose. PUT /bookmarks/{key} decodes
// Bookmark from the client and Upsert writes every column it knows about, so a // a whole Bookmark from the client and Upsert writes every series column it
// userscript PUT — which has no idea this field exists — would write a zero and // knows about, so a userscript PUT — which has no idea this field exists —
// reset the cooldown, making the poller re-fetch that series every tick for as // would write a zero and reset the cooldown, making the poller re-fetch that
// long as the user kept reading it. // series every tick for as long as the user kept reading it.
func (s *Store) MarkLatestChecked(key string, ts int64) error { func (s *Store) MarkLatestChecked(site, seriesID string, ts int64) error {
if _, err := s.db.Exec( if _, err := s.db.Exec(
`UPDATE bookmarks SET latest_checked_at = $1 WHERE key = $2`, ts, key); err != nil { `UPDATE series SET latest_checked_at = $1 WHERE site = $2 AND series_id = $3`,
return fmt.Errorf("mark checked %q: %w", key, err) ts, site, seriesID); err != nil {
return fmt.Errorf("mark checked %s:%s: %w", site, seriesID, err)
} }
return nil return nil
} }
// LatestCheckedAt reads the column MarkLatestChecked writes. It exists for // LatestCheckedAt reads the column MarkLatestChecked writes. It exists for
// tests outside this package (the poller's own tests assert on cooldown // tests outside this package (the poller's own tests assert on cooldown
// bookkeeping) — see MarkLatestChecked for why the field itself stays off // bookkeeping) — see MarkLatestChecked for why the field stays off the
// Bookmark. // client-visible row.
func (s *Store) LatestCheckedAt(key string) (int64, error) { func (s *Store) LatestCheckedAt(site, seriesID string) (int64, error) {
var ts int64 var ts int64
if err := s.db.QueryRow( if err := s.db.QueryRow(
`SELECT latest_checked_at FROM bookmarks WHERE key = $1`, key).Scan(&ts); err != nil { `SELECT latest_checked_at FROM series WHERE site = $1 AND series_id = $2`,
return 0, fmt.Errorf("latest checked at %q: %w", key, err) site, seriesID).Scan(&ts); err != nil {
return 0, fmt.Errorf("latest checked at %s:%s: %w", site, seriesID, err)
} }
return ts, nil return ts, nil
} }
// SetLatestChapter records the newest chapter the poll found on a series page.
// The poller walks Series rather than Bookmarks, so this is a series-level
// write: the row is shared, and updating it once refreshes every bookmark that
// joins to it. Touching a missing series is not an error.
func (s *Store) SetLatestChapter(site, seriesID, label string, num float64) error {
if _, err := s.db.Exec(
`UPDATE series SET latest_chapter = $3, latest_chapter_num = $4
WHERE site = $1 AND series_id = $2`,
site, seriesID, label, num); err != nil {
return fmt.Errorf("set latest chapter %s:%s: %w", site, seriesID, err)
}
return nil
}
+291 -16
View File
@@ -1,7 +1,9 @@
package store package store
import ( import (
"database/sql"
"os" "os"
"strings"
"testing" "testing"
"time" "time"
@@ -124,30 +126,41 @@ func TestBookmarkContinueURL(t *testing.T) {
} }
// readLatestCheckedAt reads the column directly. It is deliberately absent from // readLatestCheckedAt reads the column directly. It is deliberately absent from
// Bookmark (see Store.Upsert), so tests cannot assert on it any other way. // Series (see Store.MarkLatestChecked), so tests cannot assert on it any other
// way. The key splits the same way the API handler derives site/series_id.
func readLatestCheckedAt(t *testing.T, s *Store, key string) int64 { func readLatestCheckedAt(t *testing.T, s *Store, key string) int64 {
t.Helper() t.Helper()
site, seriesID, ok := strings.Cut(key, ":")
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
var ts int64 var ts int64
if err := s.db.QueryRow( if err := s.db.QueryRow(
`SELECT latest_checked_at FROM bookmarks WHERE key = $1`, key).Scan(&ts); err != nil { `SELECT latest_checked_at FROM series WHERE site = $1 AND series_id = $2`,
site, seriesID).Scan(&ts); err != nil {
t.Fatalf("read latest_checked_at %q: %v", key, err) t.Fatalf("read latest_checked_at %q: %v", key, err)
} }
return ts return ts
} }
// seedForCheck inserts a bookmark and forces its latest_checked_at. // seedForCheck inserts a bookmark (and with it its series) and forces the
// series' latest_checked_at.
func seedForCheck(t *testing.T, s *Store, key, seriesURL string, checkedAt int64) { func seedForCheck(t *testing.T, s *Store, key, seriesURL string, checkedAt int64) {
t.Helper() t.Helper()
site, seriesID, ok := strings.Cut(key, ":")
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
if _, err := s.Upsert(Bookmark{ if _, err := s.Upsert(Bookmark{
Key: key, Key: key,
Site: "asura", Site: site,
SeriesID: key, SeriesID: seriesID,
SeriesURL: seriesURL, SeriesURL: seriesURL,
UpdatedAt: 1000, UpdatedAt: 1000,
}); err != nil { }); err != nil {
t.Fatalf("seed %q: %v", key, err) t.Fatalf("seed %q: %v", key, err)
} }
if err := s.MarkLatestChecked(key, checkedAt); err != nil { if err := s.MarkLatestChecked(site, seriesID, checkedAt); err != nil {
t.Fatalf("seed mark %q: %v", key, err) t.Fatalf("seed mark %q: %v", key, err)
} }
} }
@@ -198,8 +211,8 @@ func TestDueForLatestCheckOldestFirstAndLimited(t *testing.T) {
if len(due) != 2 { if len(due) != 2 {
t.Fatalf("got %d rows, want 2 (limit)", len(due)) t.Fatalf("got %d rows, want 2 (limit)", len(due))
} }
if due[0].Key != "asura:a" || due[1].Key != "asura:b" { if due[0].Key() != "asura:a" || due[1].Key() != "asura:b" {
t.Fatalf("got %q,%q; want asura:a,asura:b (oldest first)", due[0].Key, due[1].Key) t.Fatalf("got %q,%q; want asura:a,asura:b (oldest first)", due[0].Key(), due[1].Key())
} }
} }
@@ -207,20 +220,21 @@ func TestMarkLatestChecked(t *testing.T) {
s := newTestStore(t) s := newTestStore(t)
seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", 0) seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", 0)
if err := s.MarkLatestChecked("asura:x", 4242); err != nil { if err := s.MarkLatestChecked("asura", "x", 4242); err != nil {
t.Fatalf("MarkLatestChecked: %v", err) t.Fatalf("MarkLatestChecked: %v", err)
} }
if got := readLatestCheckedAt(t, s, "asura:x"); got != 4242 { if got := readLatestCheckedAt(t, s, "asura:x"); got != 4242 {
t.Fatalf("latest_checked_at = %d, want 4242", got) t.Fatalf("latest_checked_at = %d, want 4242", got)
} }
// A missing key is not an error: the row may have been deleted mid-fetch. // A missing series is not an error: its bookmarks may have been deleted
if err := s.MarkLatestChecked("asura:gone", 1); err != nil { // mid-fetch.
t.Fatalf("MarkLatestChecked on missing key: %v", err) if err := s.MarkLatestChecked("asura", "gone", 1); err != nil {
t.Fatalf("MarkLatestChecked on missing series: %v", err)
} }
} }
// Upsert must not touch latest_checked_at. If the column ever migrates into // Upsert must not touch latest_checked_at. If the column ever migrates into
// bookmarkColumns, this fails and the cooldown is silently dead. // the client-visible write path, this fails and the cooldown is silently dead.
func TestUpsertPreservesLatestCheckedAt(t *testing.T) { func TestUpsertPreservesLatestCheckedAt(t *testing.T) {
s := newTestStore(t) s := newTestStore(t)
seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", 999) seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", 999)
@@ -362,7 +376,7 @@ func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) {
{"asura:finished", StatusFinished}, {"asura:finished", StatusFinished},
} { } {
if _, err := store.Upsert(Bookmark{ if _, err := store.Upsert(Bookmark{
Key: tc.key, Site: "asura", SeriesID: tc.key, Key: tc.key, Site: "asura", SeriesID: strings.TrimPrefix(tc.key, "asura:"),
SeriesURL: "https://asurascans.com/comics/" + tc.key, SeriesURL: "https://asurascans.com/comics/" + tc.key,
Status: tc.status, UpdatedAt: time.Now().UnixMilli(), Status: tc.status, UpdatedAt: time.Now().UnixMilli(),
}); err != nil { }); err != nil {
@@ -375,8 +389,8 @@ func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) {
t.Fatalf("DueForLatestCheck: %v", err) t.Fatalf("DueForLatestCheck: %v", err)
} }
got := map[string]bool{} got := map[string]bool{}
for _, b := range due { for _, sr := range due {
got[b.Key] = true got[sr.Key()] = true
} }
if !got["asura:reading"] || !got["asura:archived"] { if !got["asura:reading"] || !got["asura:archived"] {
t.Fatalf("due = %v, want reading and archived present", got) t.Fatalf("due = %v, want reading and archived present", got)
@@ -476,3 +490,264 @@ func TestUpsertEmptyKindKeepsStoredValue(t *testing.T) {
t.Fatalf("LastChapterNum = %v, want 11 — progress in the same request must still land", got.LastChapterNum) t.Fatalf("LastChapterNum = %v, want 11 — progress in the same request must still land", got.LastChapterNum)
} }
} }
// The 0002 backfill must survive a database that already ran 0001 with real
// rows: one series row per distinct (site, series_id) carrying the moved
// columns, and the bookmark keeping the rest. That is the upgrade path for
// every deployed database, so it is exercised rather than trusted.
func TestMigration0002BackfillsExistingBookmarks(t *testing.T) {
url := pgtest.URL(t)
db, err := sql.Open("pgx", url)
if err != nil {
t.Fatalf("open: %v", err)
}
t.Cleanup(func() { db.Close() })
// Run only 0001, as a database created before this change would have.
// migrate() normally creates the version table first; do the same here.
if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations (
version bigint PRIMARY KEY,
applied_at timestamptz NOT NULL DEFAULT now())`); err != nil {
t.Fatalf("create version table: %v", err)
}
body, err := migrations.ReadFile("migrations/0001_bookmarks.sql")
if err != nil {
t.Fatalf("read 0001: %v", err)
}
if err := applyMigration(db, 1, string(body)); err != nil {
t.Fatalf("apply 0001: %v", err)
}
if _, err := db.Exec(`
INSERT INTO bookmarks (key, site, series_id, title, series_url, cover,
last_chapter, last_chapter_num, last_chapter_url, favorite, latest_chapter,
latest_chapter_num, latest_checked_at, status, kind, updated_at)
VALUES ('asura:solo', 'asura', 'solo', 'Solo Leveling',
'https://asurascans.com/comics/solo', 'https://asurascans.com/covers/solo.jpg',
'Chapter 10', 10, 'https://asurascans.com/comics/solo/ch/10', true,
'Chapter 11', 11, 123456, 'reading', 'manga', 1000)`); err != nil {
t.Fatalf("seed legacy row: %v", err)
}
// Bring it current: 0002 must backfill the series row, not lose data.
if err := migrate(db); err != nil {
t.Fatalf("migrate: %v", err)
}
var (
title string
checked int64
fav bool
lastNum float64
)
if err := db.QueryRow(`SELECT title, latest_checked_at FROM series
WHERE site = 'asura' AND series_id = 'solo'`).Scan(&title, &checked); err != nil {
t.Fatalf("series row missing after migration: %v", err)
}
if title != "Solo Leveling" || checked != 123456 {
t.Fatalf("series = (%q, %d), want backfilled title and latest_checked_at", title, checked)
}
if err := db.QueryRow(`SELECT favorite, last_chapter_num FROM bookmarks
WHERE key = 'asura:solo'`).Scan(&fav, &lastNum); err != nil {
t.Fatalf("bookmark row missing after migration: %v", err)
}
if !fav || lastNum != 10 {
t.Fatalf("bookmark = (%v, %v), want favorite and progress kept", fav, lastNum)
}
// The migrated database opens as a normal store.
st, err := Open(url)
if err != nil {
t.Fatalf("Open after migrate: %v", err)
}
defer st.Close()
}
// readSeries reads the series row directly, for asserting on what Upsert
// actually stored rather than what the joined Bookmark reports.
func readSeries(t *testing.T, s *Store, site, seriesID string) Series {
t.Helper()
sr, err := scanSeries(s.db.QueryRow(
`SELECT `+seriesColumns+`, 0 AS reader_count FROM series s
WHERE s.site = $1 AND s.series_id = $2`, site, seriesID).Scan)
if err != nil {
t.Fatalf("read series %s:%s: %v", site, seriesID, err)
}
return sr
}
// The first PUT for a series creates its row from the client's title, cover
// and URL — there is no other source for them (ADR-0003).
func TestUpsertCreatesSeriesFromClient(t *testing.T) {
store := newTestStore(t)
if _, err := store.Upsert(Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Title: "Solo Leveling", SeriesURL: "https://asurascans.com/comics/solo",
Cover: "https://asurascans.com/covers/solo.jpg", Kind: KindManga,
UpdatedAt: 1000,
}); err != nil {
t.Fatalf("Upsert: %v", err)
}
sr := readSeries(t, store, "asura", "solo")
if sr.Title != "Solo Leveling" || sr.SeriesURL != "https://asurascans.com/comics/solo" ||
sr.Cover != "https://asurascans.com/covers/solo.jpg" {
t.Fatalf("series = %+v, want client title/url/cover stored", sr)
}
}
// A PUT naming an existing series must not overwrite its title, cover or URL:
// the row is shared, and those values are scraped page content (ADR-0003).
func TestUpsertExistingSeriesIgnoresClientTitleCoverURL(t *testing.T) {
store := newTestStore(t)
base := Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Title: "Solo Leveling", SeriesURL: "https://asurascans.com/comics/solo",
Cover: "https://asurascans.com/covers/solo.jpg", LastChapterNum: 10,
UpdatedAt: 1000,
}
if _, err := store.Upsert(base); err != nil {
t.Fatalf("seed: %v", err)
}
// Same series, hostile/compromised values, real progress advance.
base.Title = "Scraped Rename"
base.SeriesURL = "https://evil.example/solo"
base.Cover = "https://evil.example/solo.jpg"
base.LastChapterNum = 11
got, err := store.Upsert(base)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
if got.Title != "Solo Leveling" || got.SeriesURL != "https://asurascans.com/comics/solo" ||
got.Cover != "https://asurascans.com/covers/solo.jpg" {
t.Fatalf("stored = %+v, want original title/url/cover kept", got)
}
if got.LastChapterNum != 11 {
t.Fatalf("LastChapterNum = %v, want 11 — progress in the same request must still land", got.LastChapterNum)
}
}
// Kind and latest-chapter are last-write-wins even on an existing series: the
// poller and the userscript both report the latest chapter, and kind is only
// known to whichever client created the row.
func TestUpsertExistingSeriesAcceptsKindAndLatest(t *testing.T) {
store := newTestStore(t)
base := Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo", Kind: KindManga,
UpdatedAt: 1000,
}
if _, err := store.Upsert(base); err != nil {
t.Fatalf("seed: %v", err)
}
num := 12.0
base.Kind = KindNovel
base.LatestChapter = "Chapter 12"
base.LatestChapterNum = &num
got, err := store.Upsert(base)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
if got.Kind != KindNovel || got.LatestChapter != "Chapter 12" ||
got.LatestChapterNum == nil || *got.LatestChapterNum != 12 {
t.Fatalf("stored = %+v, want kind and latest chapter updated", got)
}
}
// Deleting the last bookmark must leave the series row behind, so a later
// re-bookmark shows title and cover immediately instead of waiting for a poll.
func TestDeleteKeepsSeriesRow(t *testing.T) {
store := newTestStore(t)
if _, err := store.Upsert(Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Title: "Solo Leveling", Cover: "https://asurascans.com/covers/solo.jpg",
UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
if err := store.Delete("asura:solo"); err != nil {
t.Fatalf("Delete: %v", err)
}
sr := readSeries(t, store, "asura", "solo")
if sr.Title != "Solo Leveling" {
t.Fatalf("series = %+v, want it kept after the last bookmark is deleted", sr)
}
// Re-bookmark with nothing but progress: the stored title/cover come back.
stored, err := store.Upsert(Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
LastChapterNum: 5, UpdatedAt: 2000,
})
if err != nil {
t.Fatalf("re-upsert: %v", err)
}
if stored.Title != "Solo Leveling" || stored.Cover != "https://asurascans.com/covers/solo.jpg" {
t.Fatalf("re-bookmark = %+v, want title/cover from the surviving series row", stored)
}
}
// seedSecondReader inserts an extra bookmark on an existing series. Today the
// bookmark key is <site>:<series_id>, so two bookmarks can share a series only
// once keys stop being derived from the series identity (issue #22); the due
// queue's reader-count ordering must already be right for that world.
func seedSecondReader(t *testing.T, s *Store, key, site, seriesID string, updatedAt int64) {
t.Helper()
if _, err := s.Upsert(Bookmark{
Key: key, Site: site, SeriesID: seriesID, UpdatedAt: updatedAt,
}); err != nil {
t.Fatalf("seed second reader %q: %v", key, err)
}
}
// The whole point of the split: a shared series is due once, ordered ahead of
// single-reader series by how many bookmarks reference it.
func TestDueForLatestCheckOrdersByReaderCountThenAge(t *testing.T) {
s := newTestStore(t)
// "pop" has two bookmarks but was checked most recently; "solo" has one and
// was checked long ago. Reader count must win over age.
seedForCheck(t, s, "asura:pop", "https://asurascans.com/comics/pop", 900)
seedSecondReader(t, s, "asura:pop:2", "asura", "pop", 1001)
seedForCheck(t, s, "asura:solo", "https://asurascans.com/comics/solo", 100)
due, err := s.DueForLatestCheck(1000, 10)
if err != nil {
t.Fatalf("DueForLatestCheck: %v", err)
}
if len(due) != 2 {
t.Fatalf("due = %d rows, want 2", len(due))
}
if due[0].Key() != "asura:pop" || due[1].Key() != "asura:solo" {
t.Fatalf("due order = %q, %q; want asura:pop (2 readers), asura:solo (1)",
due[0].Key(), due[1].Key())
}
}
// A series with no bookmarks must never appear in the due queue, and nothing
// in the store ever deletes it (see TestDeleteKeepsSeriesRow).
func TestDueForLatestCheckExcludesOrphanSeries(t *testing.T) {
s := newTestStore(t)
seedForCheck(t, s, "asura:kept", "https://asurascans.com/comics/kept", 0)
if _, err := s.db.Exec(`
INSERT INTO series (site, series_id, title, series_url, cover, kind,
latest_chapter, latest_chapter_num, latest_checked_at)
VALUES ('asura', 'orphan', 'Orphan', 'https://asurascans.com/comics/orphan',
'', 'manga', '', NULL, 0)`); err != nil {
t.Fatalf("seed orphan series: %v", err)
}
due, err := s.DueForLatestCheck(1000, 10)
if err != nil {
t.Fatalf("DueForLatestCheck: %v", err)
}
if len(due) != 1 || due[0].Key() != "asura:kept" {
t.Fatalf("due = %+v, want only the bookmarked series", due)
}
var n int
if err := s.db.QueryRow(
`SELECT count(*) FROM series WHERE site = 'asura' AND series_id = 'orphan'`).Scan(&n); err != nil {
t.Fatalf("count orphan series: %v", err)
}
if n != 1 {
t.Fatalf("orphan series count = %d, want 1 (never deleted)", n)
}
}