sulthan/mangaBookmark
1
0
Fork 0
Code Issues 4 Pull Requests Actions Packages Projects Releases Wiki Activity

Give every Bookmark an owner (Reader table) (#30)

Browse Source 
Closes #22

## What

A `readers` table appears; every Bookmark belongs to one. The owner is seeded as the first and only Reader, and all existing rows are attached to them.

- **Migration 0003**: `readers` (discord_id UNIQUE, token_sha256 UNIQUE, created_at).
- **Migration 0004** (run-once, version-table-gated): attaches existing bookmarks to the seeded owner, drops the surrogate `key` column, composite PK `(reader_id, site, series_id)`, FK to readers `ON DELETE CASCADE` — a duplicate Bookmark for one Reader and Series is impossible at the database level.
- **Seed**: `Store.Open` runs schema to 0003, seeds exactly one owner row from `OWNER_DISCORD_ID` (hash = SHA-256 of `API_TOKEN`, refreshed on every start so rotation stays current), then migrates the rest.
- **Scoping**: `List/Get/Upsert/Delete` take `readerID`; the wire `key` is derived as `site:series_id` on read. Handlers act as `Store.OwnerID()` while the global token remains the only credential.
- **Unchanged**: authentication and the flat wire format — nothing observable changes from outside.
- **New env** `OWNER_DISCORD_ID` (required): compose, .env.example, DEPLOY.md, README.md, backend/AGENTS.md updated.

Series-level methods (due queue, mark-checked, set-latest-chapter) stay unscoped deliberately: series are shared rows polled once per due cycle, and the reader_count ordering requires cross-reader visibility (ADR-0003).

## Verification

- `go test ./...` green, including new tests: seed idempotency + hash refresh, 0004 attach migration, DB-level duplicate impossibility, per-reader scoping, reader-delete cascade.
- Live smoke test on fresh Postgres: seed → PUT/GET (flat wire intact) → restart idempotent; stored hash matches SHA-256 of the token.

## Deploy note

`OWNER_DISCORD_ID` is required after this lands — the backend refuses to start without it. Set it to the owner's Discord snowflake (Settings → Advanced → Developer Mode → right-click name → Copy User ID).

Reviewed-on: #30
Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com>
Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
This commit was merged in pull request #30.
This commit is contained in:
Sulthan Zaki
2026-08-08 08:05:17 +07:00
committed by sulthan
parent 984965ed9f
commit 8cebb94b92
15 changed files with 553 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/internal/api/handlers.go
+7 -4
View File
@@ -13,6 +13,9 @@ import (
// Handler serves the userscript-facing JSON bookmark API.
type Handler struct {
Store *store.Store
// ReaderID is the Reader this request acts as. Authentication is still the
// single global token, so that is always the seeded owner (issue #22).
ReaderID int64
}
func writeJSON(w http.ResponseWriter, status int, v any) {
@@ -25,9 +28,9 @@ func writeJSON(w http.ResponseWriter, status int, v any) {
}
}
// List returns all bookmarks. GET /bookmarks
// List returns all bookmarks of the acting Reader. GET /bookmarks
func (h *Handler) List(w http.ResponseWriter, r *http.Request) {
items, err := h.Store.List()
items, err := h.Store.List(h.ReaderID)
if err != nil {
log.Printf("list: %v", err)
http.Error(w, "internal error", http.StatusInternalServerError)
@@ -91,7 +94,7 @@ func (h *Handler) Put(w http.ResponseWriter, r *http.Request) {
// reading progress actually moved. Any client value is ignored.
b.UpdatedAt = time.Now().UnixMilli()
stored, err := h.Store.Upsert(b)
stored, err := h.Store.Upsert(h.ReaderID, b)
if err != nil {
log.Printf("upsert: %v", err)
http.Error(w, "internal error", http.StatusInternalServerError)
@@ -109,7 +112,7 @@ func (h *Handler) Delete(w http.ResponseWriter, r *http.Request) {
http.Error(w, "missing key", http.StatusBadRequest)
return
}
if err := h.Store.Delete(key); err != nil {
if err := h.Store.Delete(h.ReaderID, key); err != nil {
log.Printf("delete: %v", err)
http.Error(w, "internal error", http.StatusInternalServerError)
return
backend/internal/latest/poller_test.go
+55 -40
View File
@@ -2,6 +2,7 @@ package latest
import (
"context"
"crypto/sha256"
"errors"
"os"
"strings"
@@ -15,15 +16,22 @@ import (
func TestMain(m *testing.M) { os.Exit(pgtest.Main(m)) }
// newTestStore opens a store on a Postgres database of this test's own.
func newTestStore(t *testing.T) *store.Store {
// testOwner is the owner every test store seeds. A second reader, where a
// test needs one, is created by opening the same database as a second owner.
var testOwner = store.Owner{DiscordID: "test-owner", TokenHash: sha256.Sum256([]byte("owner-token-hash"))}
// newTestStore opens a store on a Postgres database of this test's own and
// returns the URL, for helpers that need a second connection to the same
// database (see TestRunOnceFetchesSharedSeriesOnce).
func newTestStore(t *testing.T) (*store.Store, string) {
t.Helper()
s, err := store.Open(pgtest.URL(t))
url := pgtest.URL(t)
s, err := store.Open(url, testOwner)
if err != nil {
t.Fatalf("Open: %v", err)
}
t.Cleanup(func() { s.Close() })
return s
return s, url
}
// seedForCheck inserts a bookmark (and with it its series) and forces the
@@ -34,7 +42,7 @@ func seedForCheck(t *testing.T, s *store.Store, key, seriesURL string, checkedAt
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
if _, err := s.Upsert(store.Bookmark{
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key,
Site: site,
SeriesID: seriesID,
@@ -111,7 +119,7 @@ func newTestPoller(t *testing.T, s *store.Store, f Fetcher, at time.Time) *Polle
}
func TestRunOnceRecordsLatestChapter(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
const url = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af"
seedForCheck(t, s, "asura:chronicles-of-the-demon-faction-f886a8af", url, 0)
@@ -119,7 +127,7 @@ func TestRunOnceRecordsLatestChapter(t *testing.T) {
f := &fakeFetcher{body: asuraSeriesFixture, status: 200}
newTestPoller(t, s, f, now).runOnce(context.Background())
b, ok, err := s.Get("asura:chronicles-of-the-demon-faction-f886a8af")
b, ok, err := s.Get(s.OwnerID(), "asura:chronicles-of-the-demon-faction-f886a8af")
if err != nil || !ok {
t.Fatalf("Get: %v ok=%v", err, ok)
}
@@ -137,19 +145,19 @@ func TestRunOnceRecordsLatestChapter(t *testing.T) {
// The whole point of the updated_at CASE in Upsert: a newly published chapter is
// not reading progress and must not move the series up the list.
func TestRunOnceDoesNotReorderList(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
const url = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af"
const key = "asura:chronicles-of-the-demon-faction-f886a8af"
// "other" is the most recently read, so it must stay at the top of List().
if _, err := s.Upsert(store.Bookmark{
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: "asura:other", Site: "asura", SeriesID: "other",
SeriesURL: "https://asurascans.com/comics/other", UpdatedAt: 9_000_000,
}); err != nil {
t.Fatalf("seed other: %v", err)
}
seedForCheck(t, s, key, url, 0)
before, _, err := s.Get(key)
before, _, err := s.Get(s.OwnerID(), key)
if err != nil {
t.Fatalf("Get before: %v", err)
}
@@ -157,7 +165,7 @@ func TestRunOnceDoesNotReorderList(t *testing.T) {
f := &fakeFetcher{body: asuraSeriesFixture, status: 200}
newTestPoller(t, s, f, time.UnixMilli(9_999_999)).runOnce(context.Background())
after, _, err := s.Get(key)
after, _, err := s.Get(s.OwnerID(), key)
if err != nil {
t.Fatalf("Get after: %v", err)
}
@@ -165,7 +173,7 @@ func TestRunOnceDoesNotReorderList(t *testing.T) {
t.Fatalf("updated_at moved from %d to %d on a latest-chapter bump",
before.UpdatedAt, after.UpdatedAt)
}
list, err := s.List()
list, err := s.List(s.OwnerID())
if err != nil {
t.Fatalf("List: %v", err)
}
@@ -188,7 +196,7 @@ func TestRunOnceMarksCheckedOnFailure(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
const url = "https://asurascans.com/comics/x"
seedForCheck(t, s, "asura:x", url, 0)
@@ -199,7 +207,7 @@ func TestRunOnceMarksCheckedOnFailure(t *testing.T) {
if got := readLatestCheckedAt(t, s, "asura:x"); got != now.UnixMilli() {
t.Fatalf("latest_checked_at = %d, want %d", got, now.UnixMilli())
}
b, _, err := s.Get("asura:x")
b, _, err := s.Get(s.OwnerID(), "asura:x")
if err != nil {
t.Fatalf("Get: %v", err)
}
@@ -211,7 +219,7 @@ func TestRunOnceMarksCheckedOnFailure(t *testing.T) {
}
func TestRunOnceRespectsBatchLimit(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
for i := 0; i < 20; i++ {
key := "asura:s" + string(rune('a'+i))
seedForCheck(t, s, key, "https://asurascans.com/comics/"+key, 0)
@@ -228,18 +236,25 @@ 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).
// is fetched once per due cycle, not once per bookmark. Two bookmarks share a
// series when two readers track it (issue #22).
func TestRunOnceFetchesSharedSeriesOnce(t *testing.T) {
s := newTestStore(t)
s, url := 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,
const seriesURL = "https://asurascans.com/comics/" + slug
seedForCheck(t, s, "asura:"+slug, seriesURL, 0)
// A second reader tracks the same series. The seed is the only
// reader-creation path, so a second Open as a different owner is how a
// test gets a second reader on the same database.
other, err := store.Open(url, store.Owner{DiscordID: "second-reader", TokenHash: sha256.Sum256([]byte("second-token-hash"))})
if err != nil {
t.Fatalf("Open second reader: %v", err)
}
t.Cleanup(func() { other.Close() })
if _, err := s.Upsert(other.OwnerID(), store.Bookmark{
Key: "asura:" + slug, Site: "asura", SeriesID: slug, UpdatedAt: 2000,
}); err != nil {
t.Fatalf("seed second reader: %v", err)
}
@@ -251,20 +266,20 @@ func TestRunOnceFetchesSharedSeriesOnce(t *testing.T) {
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)
for _, st := range []*store.Store{s, other} {
b, ok, err := st.Get(st.OwnerID(), "asura:"+slug)
if err != nil || !ok {
t.Fatalf("Get %s: %v ok=%v", key, err, ok)
t.Fatalf("Get: %v ok=%v", err, ok)
}
if b.LatestChapterNum == nil || *b.LatestChapterNum != 181 {
t.Fatalf("%s LatestChapterNum = %v, want 181", key, b.LatestChapterNum)
t.Fatalf("LatestChapterNum = %v, want 181", b.LatestChapterNum)
}
}
}
// One unreachable series must not abandon the rest of the batch.
func TestRunOnceOneBadSeriesDoesNotStallBatch(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
keys := []string{"asura:a", "asura:b", "asura:c", "asura:d", "asura:e"}
for _, k := range keys {
seedForCheck(t, s, k, "https://asurascans.com/comics/"+k, 0)
@@ -292,7 +307,7 @@ func TestRunOnceOneBadSeriesDoesNotStallBatch(t *testing.T) {
// The cooldown is enforced by the due query, so a second immediate pass must do
// nothing at all — this is what makes the tick interval independent of it.
func TestRunOnceHonoursCooldownAcrossPasses(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
const url = "https://asurascans.com/comics/x"
seedForCheck(t, s, "asura:x", url, 0)
@@ -322,12 +337,12 @@ func TestRunOnceHonoursCooldownAcrossPasses(t *testing.T) {
// A site that retracts a chapter should correct the stored number downward,
// mirroring the userscript's equality check (L427) rather than a >.
func TestRunOnceCorrectsDownward(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
const url = "https://demonicscans.org/manga/Catastrophic-Necromancer"
const key = "demonic:Catastrophic-Necromancer"
high := 400.0
if _, err := s.Upsert(store.Bookmark{
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key, Site: "demonic", SeriesID: "Catastrophic-Necromancer",
SeriesURL: url, LatestChapter: "Chapter 400", LatestChapterNum: &high,
UpdatedAt: 1000,
@@ -338,7 +353,7 @@ func TestRunOnceCorrectsDownward(t *testing.T) {
f := &fakeFetcher{body: demonicSeriesFixture, status: 200}
newTestPoller(t, s, f, time.UnixMilli(5_000_000)).runOnce(context.Background())
b, _, err := s.Get(key)
b, _, err := s.Get(s.OwnerID(), key)
if err != nil {
t.Fatalf("Get: %v", err)
}
@@ -364,9 +379,9 @@ func TestCheckOneValidatesSeriesURLBeforeFetching(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
key := tt.site + ":x"
if _, err := s.Upsert(store.Bookmark{
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key, Site: tt.site, SeriesID: "x", SeriesURL: tt.seriesURL,
UpdatedAt: 1000,
}); err != nil {
@@ -391,7 +406,7 @@ func TestCheckOneValidatesSeriesURLBeforeFetching(t *testing.T) {
// A cancelled context must abandon the batch rather than run it to completion.
func TestRunOnceStopsOnCancelledContext(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
for _, k := range []string{"asura:a", "asura:b", "asura:c"} {
seedForCheck(t, s, k, "https://asurascans.com/comics/"+k, 0)
}
@@ -440,8 +455,8 @@ func TestFetchableSeriesURL(t *testing.T) {
// receive a challenge page, and the browser fetcher is the whole reason kagane
// is pollable at all.
func TestKaganeSkippedWhenNoBrowserFetcher(t *testing.T) {
s := newTestStore(t)
if _, err := s.Upsert(store.Bookmark{
s, _ := newTestStore(t)
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: "kagane:019f84bc-9ba0-7ed9-86f5-8b905ec7c28b",
Site: "kagane",
SeriesID: "019f84bc-9ba0-7ed9-86f5-8b905ec7c28b",
@@ -466,9 +481,9 @@ func TestKaganeSkippedWhenNoBrowserFetcher(t *testing.T) {
// With a browser fetcher wired up, kagane goes to it and not to the TLS one.
func TestKaganeUsesBrowserFetcher(t *testing.T) {
s := newTestStore(t)
s, _ := newTestStore(t)
key := "kagane:019f84bc-9ba0-7ed9-86f5-8b905ec7c28b"
if _, err := s.Upsert(store.Bookmark{
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key,
Site: "kagane",
SeriesID: "019f84bc-9ba0-7ed9-86f5-8b905ec7c28b",
@@ -493,7 +508,7 @@ func TestKaganeUsesBrowserFetcher(t *testing.T) {
if len(browserF.calls) != 1 {
t.Fatalf("browser fetcher calls = %v, want 1", browserF.calls)
}
got, found, err := s.Get(key)
got, found, err := s.Get(s.OwnerID(), key)
if err != nil || !found {
t.Fatalf("Get: %v found=%v", err, found)
}
backend/internal/store/migrations/0003_reader.sql
+17
View File
@@ -0,0 +1,17 @@
-- One row per person. Keyed by their Discord user ID; carries the SHA-256 of
-- their userscript token and when they were created. Hashed because a token
-- in the database is a token anyone with the database can replay; SHA-256 is
-- enough because the tokens are high-entropy random values with nothing to
-- brute-force. No one can register yet, so this table holds exactly the one
-- owner row the seed creates at startup (see Store.Open).
CREATE TABLE readers (
id bigserial PRIMARY KEY,
discord_id text NOT NULL UNIQUE,
token_sha256 bytea NOT NULL UNIQUE,
created_at timestamptz NOT NULL DEFAULT now()
);
-- Every bookmark now belongs to a reader. Added nullable: rows created before
-- this migration have no owner yet — 0004 attaches them to the seeded owner
-- before NOT NULL and the composite key land.
ALTER TABLE bookmarks ADD COLUMN reader_id bigint;
backend/internal/store/migrations/0004_owner_bookmarks.sql
+19
View File
@@ -0,0 +1,19 @@
-- Attach every pre-existing bookmark to the owner reader, seeded between the
-- two migrate passes (Store.Open). The oldest reader is the owner by
-- construction: only the seed creates readers, and it runs once per database.
-- Run-once via the version table, like every migration.
UPDATE bookmarks SET reader_id = (SELECT id FROM readers ORDER BY id LIMIT 1);
-- Ownership lands structurally: reader_id becomes part of the key, so a
-- bookmark is one Reader's progress on one Series and a duplicate for the
-- same pair is impossible at the database level. Deleting a Reader takes
-- their bookmarks with them. The old text key is gone — the wire "key" is
-- derived as site:series_id on read, and nothing references the column.
-- Dropping it drops the primary key it carried; the composite key replaces
-- it, and the FK index the series constraint needs is created automatically.
ALTER TABLE bookmarks
ALTER COLUMN reader_id SET NOT NULL,
DROP COLUMN key,
ADD PRIMARY KEY (reader_id, site, series_id),
ADD CONSTRAINT bookmarks_reader_fk
FOREIGN KEY (reader_id) REFERENCES readers (id) ON DELETE CASCADE;
backend/internal/store/store.go
+118 -37
View File
@@ -159,7 +159,7 @@ var migrations embed.FS
// compile-time constant; every request value is bound as a parameter. The
// series-owned fields are joined in from the series table, in scanBookmark
// order, so the flat Bookmark reads back whole despite the split (ADR-0004).
const bookmarkColumns = `b.key, b.site, b.series_id, s.title, s.series_url, s.cover,
const bookmarkColumns = `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`
@@ -169,32 +169,91 @@ const bookmarkColumns = `b.key, b.site, b.series_id, s.title, s.series_url, s.co
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`
// Owner is the person running the service: the first Reader, and the only one
// until registration exists. The seed makes sure exactly one readers row
// matches their Discord ID, carrying the SHA-256 of their userscript token —
// which today is the global API token.
type Owner struct {
DiscordID string
// TokenHash is the SHA-256 of the userscript token; the array shape makes
// it a compile error to store anything that is not a hash.
TokenHash [32]byte
}
// Store is the Postgres-backed bookmark store.
type Store struct {
db *sql.DB
// ownerID is the seeded owner Reader (issue #22). Authentication is still
// the single global token, so every request acts as this Reader; the store
// methods take the id explicitly so the scoping survives per-Reader auth.
ownerID int64
}
// OwnerID returns the seeded owner Reader's id — the Reader every request
// acts as while the global token is still the only credential.
func (s *Store) OwnerID() int64 { return s.ownerID }
// readersMigration is the version that creates the readers table. The owner
// seed runs between two migrate passes, so that the run-once migration which
// attaches existing bookmarks (0004) finds the owner row.
const readersMigration = 3
// allMigrations is the migrate() cap that applies every pending version.
const allMigrations = 0
// Open connects to Postgres at url — a libpq connection URL such as
// "postgres://user:pass@host:5432/bookmarks?sslmode=disable" — and brings its
// schema up to date.
func Open(url string) (*Store, error) {
// "postgres://user:pass@host:5432/bookmarks?sslmode=disable" — brings its
// schema up to date, and seeds the owner Reader.
func Open(url string, owner Owner) (*Store, error) {
db, err := sql.Open("pgx", url)
if err != nil {
return nil, fmt.Errorf("open postgres: %w", err)
}
if err := migrate(db); err != nil {
// Schema runs in two passes with the seed between: 0003 creates the
// readers table, the owner row must exist before 0004 attaches the
// existing bookmarks to it. Anything past 0004 is applied by the second
// pass.
if err := migrate(db, readersMigration); err != nil {
db.Close()
return nil, fmt.Errorf("migrate schema: %w", err)
}
if err := seedOwner(db, owner); err != nil {
db.Close()
return nil, fmt.Errorf("seed owner: %w", err)
}
if err := migrate(db, allMigrations); err != nil {
db.Close()
return nil, fmt.Errorf("migrate: %w", err)
}
return &Store{db: db}, nil
var ownerID int64
if err := db.QueryRow(
`SELECT id FROM readers WHERE discord_id = $1`, owner.DiscordID).Scan(&ownerID); err != nil {
db.Close()
return nil, fmt.Errorf("resolve owner: %w", err)
}
return &Store{db: db, ownerID: ownerID}, nil
}
// seedOwner makes sure the configured owner exists as exactly one readers row,
// and keeps its token hash current on every start: rotating the userscript
// token must refresh the hash, or the stored credential goes stale.
func seedOwner(db *sql.DB, o Owner) error {
if _, err := db.Exec(`
INSERT INTO readers (discord_id, token_sha256) VALUES ($1, $2)
ON CONFLICT (discord_id) DO UPDATE SET token_sha256 = EXCLUDED.token_sha256`,
o.DiscordID, o.TokenHash[:]); err != nil {
return fmt.Errorf("seed owner: %w", err)
}
return nil
}
// migrate applies every embedded migration this database has not recorded, in
// filename order, each in its own transaction. Files are named
// "<version>_<name>.sql" and are append-only: editing an applied file changes
// nothing, because schema_migrations is how a database remembers what it ran.
// Runs on every start and is a no-op once current.
func migrate(db *sql.DB) error {
// filename order, each in its own transaction. upto caps the highest version
// applied; 0 means all. Files are named "<version>_<name>.sql" and are
// append-only: editing an applied file changes nothing, because
// schema_migrations is how a database remembers what it ran. Runs on every
// start and is a no-op once current.
func migrate(db *sql.DB, upto int64) error {
if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations (
version bigint PRIMARY KEY,
applied_at timestamptz NOT NULL DEFAULT now())`); err != nil {
@@ -212,6 +271,9 @@ func migrate(db *sql.DB) error {
if err != nil {
return fmt.Errorf("migration %q: filename must start with a version number", name)
}
if upto > 0 && version > upto {
continue
}
body, err := migrations.ReadFile(name)
if err != nil {
return err
@@ -261,7 +323,7 @@ func scanBookmark(scan func(...any) error) (Bookmark, error) {
latestChapterNum sql.NullFloat64
)
if err := scan(
&b.Key, &b.Site, &b.SeriesID, &b.Title, &b.SeriesURL, &b.Cover,
&b.Site, &b.SeriesID, &b.Title, &b.SeriesURL, &b.Cover,
&b.LastChapter, &b.LastChapterNum, &b.LastChapterURL,
&b.Favorite, &b.LatestChapter, &latestChapterNum, &b.UpdatedAt, &b.Status, &b.Kind,
); err != nil {
@@ -270,6 +332,9 @@ func scanBookmark(scan func(...any) error) (Bookmark, error) {
if latestChapterNum.Valid {
b.LatestChapterNum = &latestChapterNum.Float64
}
// The wire identity is derived: there is no stored key column, the
// bookmark is keyed (reader_id, site, series_id) (issue #22).
b.Key = b.Site + ":" + b.SeriesID
// An unrecognised bucket (a hand-edited row) would leave the row in no list
// at all, so anything outside the three known buckets reads as the default
// rather than being passed through.
@@ -303,13 +368,15 @@ func scanSeries(scan func(...any) error) (Series, error) {
// Close releases the underlying database handle.
func (s *Store) Close() error { return s.db.Close() }
// 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) {
rows, err := s.db.Query(`SELECT ` + bookmarkColumns + `
// List returns every bookmark of one reader, newest activity first.
// Series-owned fields are joined in, so each Bookmark reads back whole and
// flat (ADR-0004).
func (s *Store) List(readerID int64) ([]Bookmark, error) {
rows, err := s.db.Query(`SELECT `+bookmarkColumns+`
FROM bookmarks b
JOIN series s ON s.site = b.site AND s.series_id = b.series_id
ORDER BY b.updated_at DESC`)
WHERE b.reader_id = $1
ORDER BY b.updated_at DESC`, readerID)
if err != nil {
return nil, fmt.Errorf("query bookmarks: %w", err)
}
@@ -326,14 +393,19 @@ func (s *Store) List() ([]Bookmark, error) {
return out, rows.Err()
}
// Get returns one bookmark by key. A missing key is not an error: ok is false
// and err is nil. UI mutations read-modify-write through this so they preserve
// the fields they do not touch.
func (s *Store) Get(key string) (Bookmark, bool, error) {
// Get returns one bookmark of one reader by key. A missing key is not an
// error: ok is false and err is nil. UI mutations read-modify-write through
// this so they preserve the fields they do not touch.
func (s *Store) Get(readerID int64, key string) (Bookmark, bool, error) {
site, seriesID, ok := strings.Cut(key, ":")
if !ok {
return Bookmark{}, false, nil
}
b, err := scanBookmark(s.db.QueryRow(
`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)
WHERE b.reader_id = $1 AND b.site = $2 AND b.series_id = $3`,
readerID, site, seriesID).Scan)
if errors.Is(err, sql.ErrNoRows) {
return Bookmark{}, false, nil
}
@@ -343,9 +415,11 @@ func (s *Store) Get(key string) (Bookmark, bool, error) {
return b, true, nil
}
// Upsert inserts or replaces a bookmark by key (last-write-wins) and returns
// the row as actually stored — one flat object with the series-owned fields
// joined in, exactly as GET reports it (ADR-0004).
// Upsert inserts or replaces one reader's bookmark by key (last-write-wins)
// and returns the row as actually stored — one flat object with the
// series-owned fields joined in, exactly as GET reports it (ADR-0004). A
// bookmark is keyed (reader_id, site, series_id), so the same key upserts two
// independent rows for two readers.
//
// 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
@@ -359,7 +433,7 @@ func (s *Store) Get(key string) (Bookmark, bool, error) {
// order their list by updated_at, so favoriting a series or recording a newly
// published chapter must not disturb that order — only real reading progress
// does. Callers must therefore use the returned bookmark, not the argument.
func (s *Store) Upsert(b Bookmark) (Bookmark, error) {
func (s *Store) Upsert(readerID int64, b Bookmark) (Bookmark, error) {
tx, err := s.db.Begin()
if err != nil {
return Bookmark{}, fmt.Errorf("begin %q: %w", b.Key, err)
@@ -404,13 +478,12 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) {
// 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,
INSERT INTO bookmarks (reader_id, 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'),
COALESCE(NULLIF($8::text, ''), (SELECT status FROM bookmarks WHERE reader_id = $1 AND site = $2 AND series_id = $3), 'reading'),
$9)
ON CONFLICT (key) DO UPDATE SET
site=excluded.site, series_id=excluded.series_id,
ON CONFLICT (reader_id, site, series_id) DO UPDATE SET
last_chapter=excluded.last_chapter, last_chapter_num=excluded.last_chapter_num,
last_chapter_url=excluded.last_chapter_url,
favorite=excluded.favorite,
@@ -420,7 +493,7 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) {
THEN excluded.updated_at
ELSE bookmarks.updated_at
END`,
b.Key, b.Site, b.SeriesID,
readerID, b.Site, b.SeriesID,
b.LastChapter, b.LastChapterNum, b.LastChapterURL,
b.Favorite, b.Status, b.UpdatedAt); err != nil {
return Bookmark{}, fmt.Errorf("upsert %q: %w", b.Key, err)
@@ -429,7 +502,8 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) {
stored, err := scanBookmark(tx.QueryRow(
`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)
WHERE b.reader_id = $1 AND b.site = $2 AND b.series_id = $3`,
readerID, b.Site, b.SeriesID).Scan)
if err != nil {
return Bookmark{}, fmt.Errorf("read back %q: %w", b.Key, err)
}
@@ -439,9 +513,16 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) {
return stored, nil
}
// Delete removes a bookmark by key. Deleting a missing key is not an error.
func (s *Store) Delete(key string) error {
if _, err := s.db.Exec(`DELETE FROM bookmarks WHERE key = $1`, key); err != nil {
// Delete removes one reader's bookmark by key. Deleting a missing key is not
// an error.
func (s *Store) Delete(readerID int64, key string) error {
site, seriesID, ok := strings.Cut(key, ":")
if !ok {
return nil
}
if _, err := s.db.Exec(
`DELETE FROM bookmarks WHERE reader_id = $1 AND site = $2 AND series_id = $3`,
readerID, site, seriesID); err != nil {
return fmt.Errorf("delete %q: %w", key, err)
}
return nil
@@ -466,14 +547,14 @@ func (s *Store) Delete(key string) error {
// 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) ([]Series, error) {
rows, err := s.db.Query(`SELECT `+seriesColumns+`, COUNT(b.key) AS reader_count
rows, err := s.db.Query(`SELECT `+seriesColumns+`, COUNT(*) AS reader_count
FROM series s
JOIN bookmarks b ON b.site = s.site AND b.series_id = s.series_id
WHERE s.series_url <> ''
AND s.latest_checked_at <= $1
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
HAVING COUNT(*) FILTER (WHERE b.status <> 'finished') > 0
ORDER BY reader_count DESC, s.latest_checked_at ASC
LIMIT $2`, cutoffMs, limit)
if err != nil {
backend/internal/store/store_test.go
+265 -50
View File
@@ -1,8 +1,11 @@
package store
import (
"bytes"
"crypto/sha256"
"database/sql"
"os"
"strconv"
"strings"
"testing"
"time"
@@ -12,9 +15,13 @@ import (
func TestMain(m *testing.M) { os.Exit(pgtest.Main(m)) }
// testOwner is the owner every test store seeds. Tests that need a second
// reader insert one directly (see secondReader).
var testOwner = Owner{DiscordID: "test-owner", TokenHash: sha256.Sum256([]byte("owner-token-hash"))}
func newTestStore(t *testing.T) *Store {
t.Helper()
store, err := Open(pgtest.URL(t))
store, err := Open(pgtest.URL(t), testOwner)
if err != nil {
t.Fatalf("Open: %v", err)
}
@@ -22,29 +29,44 @@ func newTestStore(t *testing.T) *Store {
return store
}
// secondReader inserts an extra reader row and returns its id. The store API
// has no reader-creation path yet — the seed is the only one — so tests that
// need reader isolation insert directly.
func secondReader(t *testing.T, s *Store) int64 {
t.Helper()
hash := sha256.Sum256([]byte("second-token-hash"))
var id int64
if err := s.db.QueryRow(
`INSERT INTO readers (discord_id, token_sha256) VALUES ($1, $2) RETURNING id`,
"second-"+strconv.FormatInt(time.Now().UnixNano(), 10), hash[:]).Scan(&id); err != nil {
t.Fatalf("seed second reader: %v", err)
}
return id
}
// The migration runner runs on every start, so a second Open against a
// database it already built must be a no-op rather than a duplicate-table
// error, and must leave the rows alone.
func TestOpenIsIdempotent(t *testing.T) {
url := pgtest.URL(t)
first, err := Open(url)
first, err := Open(url, testOwner)
if err != nil {
t.Fatalf("Open: %v", err)
}
if _, err := first.Upsert(Bookmark{
if _, err := first.Upsert(first.OwnerID(), Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo", UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
first.Close()
second, err := Open(url)
second, err := Open(url, testOwner)
if err != nil {
t.Fatalf("reopen: %v", err)
}
t.Cleanup(func() { second.Close() })
list, err := second.List()
list, err := second.List(second.OwnerID())
if err != nil {
t.Fatalf("List: %v", err)
}
@@ -55,14 +77,14 @@ func TestOpenIsIdempotent(t *testing.T) {
func TestStoreGet(t *testing.T) {
store := newTestStore(t)
if _, err := store.Upsert(Bookmark{
if _, err := store.Upsert(store.OwnerID(), Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Title: "Solo Leveling", LastChapterNum: 45, UpdatedAt: 1000,
}); err != nil {
t.Fatalf("Upsert: %v", err)
}
got, ok, err := store.Get("asura:solo")
got, ok, err := store.Get(store.OwnerID(), "asura:solo")
if err != nil {
t.Fatalf("Get: %v", err)
}
@@ -76,7 +98,7 @@ func TestStoreGet(t *testing.T) {
func TestStoreGetMissing(t *testing.T) {
store := newTestStore(t)
_, ok, err := store.Get("asura:nope")
_, ok, err := store.Get(store.OwnerID(), "asura:nope")
if err != nil {
t.Fatalf("Get missing returned error %v, want nil", err)
}
@@ -151,7 +173,7 @@ func seedForCheck(t *testing.T, s *Store, key, seriesURL string, checkedAt int64
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
if _, err := s.Upsert(Bookmark{
if _, err := s.Upsert(s.OwnerID(), Bookmark{
Key: key,
Site: site,
SeriesID: seriesID,
@@ -239,12 +261,12 @@ func TestUpsertPreservesLatestCheckedAt(t *testing.T) {
s := newTestStore(t)
seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", 999)
b, ok, err := s.Get("asura:x")
b, ok, err := s.Get(s.OwnerID(), "asura:x")
if err != nil || !ok {
t.Fatalf("Get: %v ok=%v", err, ok)
}
b.Title = "changed"
if _, err := s.Upsert(b); err != nil {
if _, err := s.Upsert(s.OwnerID(), b); err != nil {
t.Fatalf("Upsert: %v", err)
}
if got := readLatestCheckedAt(t, s, "asura:x"); got != 999 {
@@ -254,7 +276,7 @@ func TestUpsertPreservesLatestCheckedAt(t *testing.T) {
func TestUpsertDefaultsStatusToReading(t *testing.T) {
store := newTestStore(t)
stored, err := store.Upsert(Bookmark{
stored, err := store.Upsert(store.OwnerID(), Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
UpdatedAt: time.Now().UnixMilli(),
})
@@ -274,13 +296,13 @@ func TestUpsertEmptyStatusPreservesStored(t *testing.T) {
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Status: StatusArchived, UpdatedAt: time.Now().UnixMilli(),
}
if _, err := store.Upsert(base); err != nil {
if _, err := store.Upsert(store.OwnerID(), base); err != nil {
t.Fatalf("seed: %v", err)
}
base.Status = ""
base.LastChapterNum = 12
stored, err := store.Upsert(base)
stored, err := store.Upsert(store.OwnerID(), base)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
@@ -299,11 +321,11 @@ func TestLatestPollRoundTripPreservesArchived(t *testing.T) {
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Status: StatusArchived, UpdatedAt: time.Now().UnixMilli(),
}
if _, err := store.Upsert(base); err != nil {
if _, err := store.Upsert(store.OwnerID(), base); err != nil {
t.Fatalf("seed: %v", err)
}
cur, found, err := store.Get(base.Key)
cur, found, err := store.Get(store.OwnerID(), base.Key)
if err != nil || !found {
t.Fatalf("Get: found=%v err=%v", found, err)
}
@@ -313,7 +335,7 @@ func TestLatestPollRoundTripPreservesArchived(t *testing.T) {
cur.LatestChapterNum = &num
cur.UpdatedAt = time.Now().UnixMilli()
stored, err := store.Upsert(cur)
stored, err := store.Upsert(store.OwnerID(), cur)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
@@ -328,12 +350,12 @@ func TestUpsertReplacesStatusWhenGiven(t *testing.T) {
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Status: StatusArchived, UpdatedAt: time.Now().UnixMilli(),
}
if _, err := store.Upsert(base); err != nil {
if _, err := store.Upsert(store.OwnerID(), base); err != nil {
t.Fatalf("seed: %v", err)
}
base.Status = StatusReading
stored, err := store.Upsert(base)
stored, err := store.Upsert(store.OwnerID(), base)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
@@ -350,14 +372,14 @@ func TestUpsertStatusChangeKeepsUpdatedAt(t *testing.T) {
LastChapter: "45", LastChapterNum: 45,
UpdatedAt: time.Now().UnixMilli(),
}
first, err := store.Upsert(base)
first, err := store.Upsert(store.OwnerID(), base)
if err != nil {
t.Fatalf("seed: %v", err)
}
base.Status = StatusArchived
base.UpdatedAt = first.UpdatedAt + 60_000
stored, err := store.Upsert(base)
stored, err := store.Upsert(store.OwnerID(), base)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
@@ -375,7 +397,7 @@ func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) {
{"asura:archived", StatusArchived},
{"asura:finished", StatusFinished},
} {
if _, err := store.Upsert(Bookmark{
if _, err := store.Upsert(store.OwnerID(), Bookmark{
Key: tc.key, Site: "asura", SeriesID: strings.TrimPrefix(tc.key, "asura:"),
SeriesURL: "https://asurascans.com/comics/" + tc.key,
Status: tc.status, UpdatedAt: time.Now().UnixMilli(),
@@ -440,7 +462,7 @@ func TestDisplayChapter(t *testing.T) {
func TestUpsertKindDefaultsToManga(t *testing.T) {
store := newTestStore(t)
got, err := store.Upsert(Bookmark{
got, err := store.Upsert(store.OwnerID(), Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo", UpdatedAt: 1000,
})
if err != nil {
@@ -453,7 +475,7 @@ func TestUpsertKindDefaultsToManga(t *testing.T) {
func TestUpsertKindRoundTrips(t *testing.T) {
store := newTestStore(t)
got, err := store.Upsert(Bookmark{
got, err := store.Upsert(store.OwnerID(), Bookmark{
Key: "lightnovelworld:a-will-eternal", Site: "lightnovelworld",
SeriesID: "a-will-eternal", Kind: KindNovel, UpdatedAt: 1000,
})
@@ -469,14 +491,14 @@ func TestUpsertKindRoundTrips(t *testing.T) {
// must keep the stored library, not silently demote a novel to manga.
func TestUpsertEmptyKindKeepsStoredValue(t *testing.T) {
store := newTestStore(t)
if _, err := store.Upsert(Bookmark{
if _, err := store.Upsert(store.OwnerID(), Bookmark{
Key: "lightnovelworld:a-will-eternal", Site: "lightnovelworld",
SeriesID: "a-will-eternal", Kind: KindNovel, LastChapterNum: 10, UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
got, err := store.Upsert(Bookmark{
got, err := store.Upsert(store.OwnerID(), Bookmark{
Key: "lightnovelworld:a-will-eternal", Site: "lightnovelworld",
SeriesID: "a-will-eternal", Kind: "", LastChapterNum: 11, UpdatedAt: 2000,
})
@@ -528,10 +550,14 @@ func TestMigration0002BackfillsExistingBookmarks(t *testing.T) {
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)
// Bring it current through the production path: Open runs the schema to
// 0003, seeds the owner, then applies 0004 which attaches this row. 0002
// must have backfilled the series row, not lost data.
st, err := Open(url, testOwner)
if err != nil {
t.Fatalf("Open after migrate: %v", err)
}
defer st.Close()
var (
title string
checked int64
@@ -545,20 +571,15 @@ func TestMigration0002BackfillsExistingBookmarks(t *testing.T) {
if title != "Solo Leveling" || checked != 123456 {
t.Fatalf("series = (%q, %d), want backfilled title and latest_checked_at", title, checked)
}
// The key column is gone; the bookmark is read by its composite key.
if err := db.QueryRow(`SELECT favorite, last_chapter_num FROM bookmarks
WHERE key = 'asura:solo'`).Scan(&fav, &lastNum); err != nil {
WHERE reader_id = $1 AND site = 'asura' AND series_id = 'solo'`,
st.OwnerID()).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
@@ -578,7 +599,7 @@ func readSeries(t *testing.T, s *Store, site, seriesID string) Series {
// and URL — there is no other source for them (ADR-0003).
func TestUpsertCreatesSeriesFromClient(t *testing.T) {
store := newTestStore(t)
if _, err := store.Upsert(Bookmark{
if _, err := store.Upsert(store.OwnerID(), 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,
@@ -604,7 +625,7 @@ func TestUpsertExistingSeriesIgnoresClientTitleCoverURL(t *testing.T) {
Cover: "https://asurascans.com/covers/solo.jpg", LastChapterNum: 10,
UpdatedAt: 1000,
}
if _, err := store.Upsert(base); err != nil {
if _, err := store.Upsert(store.OwnerID(), base); err != nil {
t.Fatalf("seed: %v", err)
}
@@ -613,7 +634,7 @@ func TestUpsertExistingSeriesIgnoresClientTitleCoverURL(t *testing.T) {
base.SeriesURL = "https://evil.example/solo"
base.Cover = "https://evil.example/solo.jpg"
base.LastChapterNum = 11
got, err := store.Upsert(base)
got, err := store.Upsert(store.OwnerID(), base)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
@@ -635,7 +656,7 @@ func TestUpsertExistingSeriesAcceptsKindAndLatest(t *testing.T) {
Key: "asura:solo", Site: "asura", SeriesID: "solo", Kind: KindManga,
UpdatedAt: 1000,
}
if _, err := store.Upsert(base); err != nil {
if _, err := store.Upsert(store.OwnerID(), base); err != nil {
t.Fatalf("seed: %v", err)
}
@@ -643,7 +664,7 @@ func TestUpsertExistingSeriesAcceptsKindAndLatest(t *testing.T) {
base.Kind = KindNovel
base.LatestChapter = "Chapter 12"
base.LatestChapterNum = &num
got, err := store.Upsert(base)
got, err := store.Upsert(store.OwnerID(), base)
if err != nil {
t.Fatalf("Upsert: %v", err)
}
@@ -657,14 +678,14 @@ func TestUpsertExistingSeriesAcceptsKindAndLatest(t *testing.T) {
// 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{
if _, err := store.Upsert(store.OwnerID(), 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 {
if err := store.Delete(store.OwnerID(), "asura:solo"); err != nil {
t.Fatalf("Delete: %v", err)
}
@@ -674,7 +695,7 @@ func TestDeleteKeepsSeriesRow(t *testing.T) {
}
// Re-bookmark with nothing but progress: the stored title/cover come back.
stored, err := store.Upsert(Bookmark{
stored, err := store.Upsert(store.OwnerID(), Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
LastChapterNum: 5, UpdatedAt: 2000,
})
@@ -686,13 +707,12 @@ func TestDeleteKeepsSeriesRow(t *testing.T) {
}
}
// 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.
// seedSecondReader inserts an extra bookmark on an existing series, owned by a
// second reader. Two bookmarks can share a series only across readers now
// (issue #22); the due queue's reader-count ordering counts them all.
func seedSecondReader(t *testing.T, s *Store, key, site, seriesID string, updatedAt int64) {
t.Helper()
if _, err := s.Upsert(Bookmark{
if _, err := s.Upsert(secondReader(t, s), Bookmark{
Key: key, Site: site, SeriesID: seriesID, UpdatedAt: updatedAt,
}); err != nil {
t.Fatalf("seed second reader %q: %v", key, err)
@@ -751,3 +771,198 @@ func TestDueForLatestCheckExcludesOrphanSeries(t *testing.T) {
t.Fatalf("orphan series count = %d, want 1 (never deleted)", n)
}
}
// The seed must never multiply the owner row: reopening the same database with
// a different token hash refreshes the stored hash, not the row. That is what
// keeps the readers table at exactly one row across restarts and token
// rotations.
func TestSeedOwnerIdempotentAndRefreshesTokenHash(t *testing.T) {
url := pgtest.URL(t)
first, err := Open(url, Owner{DiscordID: "owner", TokenHash: sha256.Sum256([]byte("hash-v1"))})
if err != nil {
t.Fatalf("Open: %v", err)
}
ownerID := first.OwnerID()
first.Close()
second, err := Open(url, Owner{DiscordID: "owner", TokenHash: sha256.Sum256([]byte("hash-v2"))})
if err != nil {
t.Fatalf("reopen: %v", err)
}
defer second.Close()
if second.OwnerID() != ownerID {
t.Fatalf("owner id = %d after reopen, want %d (same row)", second.OwnerID(), ownerID)
}
var (
n int
hash []byte
)
if err := second.db.QueryRow(`SELECT count(*), (SELECT token_sha256 FROM readers LIMIT 1) FROM readers`).Scan(&n, &hash); err != nil {
t.Fatalf("read readers: %v", err)
}
if n != 1 {
t.Fatalf("readers count = %d, want 1", n)
}
want := sha256.Sum256([]byte("hash-v2"))
if !bytes.Equal(hash, want[:]) {
t.Fatalf("token hash = %x, want the refreshed sha256", hash)
}
}
// The upgrade path for a deployed database: bookmarks created before readers
// existed must all land on the seeded owner, the key column must be gone, and
// the same database must be able to hold two readers' bookmarks for one series.
func TestMigration0004AttachesBookmarksToOwner(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() })
// A database at the state before #21 shipped: 0001 applied, bookmarks
// keyed by <site>:<series_id>, no series table. Rows land before 0002, the
// way a real deployment's data did.
if err := migrate(db, 1); err != nil {
t.Fatalf("migrate to 0001: %v", err)
}
for _, key := range []string{"asura:solo", "demonic:catastrophic-necromancer"} {
site, seriesID, ok := strings.Cut(key, ":")
if !ok {
t.Fatalf("key %q: no ':' separator", key)
}
if _, err := db.Exec(`
INSERT INTO bookmarks (key, site, series_id, updated_at)
VALUES ($1, $2, $3, 1000)`, key, site, seriesID); err != nil {
t.Fatalf("seed legacy row %q: %v", key, err)
}
}
// 0002 backfills the series rows, as it did in the real upgrade.
if err := migrate(db, 2); err != nil {
t.Fatalf("migrate to 0002: %v", err)
}
st, err := Open(url, testOwner)
if err != nil {
t.Fatalf("Open: %v", err)
}
defer st.Close()
var (
attached int
readers int
)
if err := st.db.QueryRow(
`SELECT count(*) FROM bookmarks WHERE reader_id = $1`, st.OwnerID()).Scan(&attached); err != nil {
t.Fatalf("count attached bookmarks: %v", err)
}
if attached != 2 {
t.Fatalf("bookmarks attached to owner = %d, want all 2", attached)
}
if err := st.db.QueryRow(`SELECT count(*) FROM readers`).Scan(&readers); err != nil {
t.Fatalf("count readers: %v", err)
}
if readers != 1 {
t.Fatalf("readers = %d, want 1", readers)
}
// The surrogate key column is gone; only the composite key remains.
if _, err := st.db.Query(`SELECT key FROM bookmarks`); err == nil {
t.Fatal("bookmarks.key still exists after the migration")
}
}
// One Reader and Series pair must admit at most one bookmark, enforced by the
// primary key itself — a raw INSERT that skips the upsert must fail.
func TestBookmarkDuplicateImpossibleAtDatabaseLevel(t *testing.T) {
st := newTestStore(t)
// A series row on its own, no bookmark: the raw inserts below must only
// ever collide on the bookmark primary key.
if _, err := st.db.Exec(
`INSERT INTO series (site, series_id) VALUES ('asura', 'solo')`); err != nil {
t.Fatalf("seed series: %v", err)
}
insert := func() error {
_, err := st.db.Exec(`
INSERT INTO bookmarks (reader_id, site, series_id, updated_at)
VALUES ($1, 'asura', 'solo', 1000)`, st.OwnerID())
return err
}
if err := insert(); err != nil {
t.Fatalf("first insert: %v", err)
}
if err := insert(); err == nil {
t.Fatal("duplicate bookmark for the same reader and series was accepted")
}
}
// Every read and write is scoped to the reader it names: a second reader sees
// an empty list, cannot read or delete the owner's row, and a delete by the
// wrong reader leaves the row alone.
func TestStoreScopesBookmarksToReader(t *testing.T) {
st := newTestStore(t)
other := secondReader(t, st)
if _, err := st.Upsert(st.OwnerID(), Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo", UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed owner bookmark: %v", err)
}
otherList, err := st.List(other)
if err != nil {
t.Fatalf("List(other): %v", err)
}
if len(otherList) != 0 {
t.Fatalf("other reader's list = %+v, want empty", otherList)
}
if _, ok, err := st.Get(other, "asura:solo"); err != nil || ok {
t.Fatalf("Get(other, asura:solo) = ok:%v err:%v, want not found", ok, err)
}
if err := st.Delete(other, "asura:solo"); err != nil {
t.Fatalf("Delete(other): %v", err)
}
ownerList, err := st.List(st.OwnerID())
if err != nil {
t.Fatalf("List(owner): %v", err)
}
if len(ownerList) != 1 || ownerList[0].Key != "asura:solo" {
t.Fatalf("owner's list after other's delete = %+v, want the row intact", ownerList)
}
// The same key under a second reader is an independent bookmark.
if _, err := st.Upsert(other, Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo", UpdatedAt: 2000,
}); err != nil {
t.Fatalf("upsert other's bookmark: %v", err)
}
if got, err := st.List(other); err != nil || len(got) != 1 {
t.Fatalf("other's list after own upsert = %+v err:%v, want 1 row", got, err)
}
}
// Deleting a reader must take their bookmarks with them (ON DELETE CASCADE)
// while leaving the shared series row behind.
func TestDeleteReaderCascadesToBookmarks(t *testing.T) {
st := newTestStore(t)
other := secondReader(t, st)
if _, err := st.Upsert(other, Bookmark{
Key: "asura:solo", Site: "asura", SeriesID: "solo",
Title: "Solo Leveling", UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed other's bookmark: %v", err)
}
if _, err := st.db.Exec(`DELETE FROM readers WHERE id = $1`, other); err != nil {
t.Fatalf("delete reader: %v", err)
}
var n int
if err := st.db.QueryRow(`SELECT count(*) FROM bookmarks`).Scan(&n); err != nil {
t.Fatalf("count bookmarks: %v", err)
}
if n != 0 {
t.Fatalf("bookmarks after reader delete = %d, want 0 (cascade)", n)
}
sr := readSeries(t, st, "asura", "solo")
if sr.Title != "Solo Leveling" {
t.Fatalf("series = %+v, want it kept after its only reader was deleted", sr)
}
}
backend/internal/web/web.go
+9 -5
View File
@@ -32,6 +32,9 @@ const RecentCount = 5
// representations (HTML versus JSON) to different clients under different auth.
type Handler struct {
store *store.Store
// readerID is the Reader this UI acts as — the seeded owner, while the web
// password is still the only credential (issue #22).
readerID int64
tmpl *template.Template
key []byte
password string
@@ -81,13 +84,14 @@ type loginView struct {
// New parses every template up front so a broken one kills the process at
// startup rather than the first request that touches it.
func New(s *store.Store, apiToken, webPassword string) (*Handler, error) {
func New(s *store.Store, readerID int64, apiToken, webPassword string) (*Handler, error) {
tmpl, err := template.ParseFS(templateFS, "templates/*.html")
if err != nil {
return nil, err
}
return &Handler{
store: s,
readerID: readerID,
tmpl: tmpl,
key: session.Key(apiToken, webPassword),
password: webPassword,
@@ -216,7 +220,7 @@ func libOf(q string) string {
// archived favourite therefore shows only under Archived: Favourites means
// "favourites I am currently reading".
func (h *Handler) buildListView(lib, tab string) (listView, error) {
all, err := h.store.List() // already ordered updated_at DESC
all, err := h.store.List(h.readerID) // already ordered updated_at DESC
if err != nil {
return listView{}, err
}
@@ -374,7 +378,7 @@ func (h *Handler) loadForMutation(w http.ResponseWriter, r *http.Request) (store
http.Error(w, "missing key", http.StatusBadRequest)
return store.Bookmark{}, false
}
b, ok, err := h.store.Get(key)
b, ok, err := h.store.Get(h.readerID, key)
if err != nil {
log.Printf("ui get %q: %v", key, err)
http.Error(w, "internal error", http.StatusInternalServerError)
@@ -397,7 +401,7 @@ func (h *Handler) loadForMutation(w http.ResponseWriter, r *http.Request) (store
// describe the whole library, so they are rebuilt out of band on every
// mutation, at the cost of one extra list read per toggle.
func (h *Handler) saveAndRenderCard(w http.ResponseWriter, r *http.Request, b store.Bookmark) {
stored, err := h.store.Upsert(b)
stored, err := h.store.Upsert(h.readerID, b)
if err != nil {
log.Printf("ui upsert %q: %v", b.Key, err)
http.Error(w, "internal error", http.StatusInternalServerError)
@@ -489,7 +493,7 @@ func (h *Handler) uiDelete(w http.ResponseWriter, r *http.Request) {
http.Error(w, "missing key", http.StatusBadRequest)
return
}
if err := h.store.Delete(key); err != nil {
if err := h.store.Delete(h.readerID, key); err != nil {
log.Printf("ui delete %q: %v", key, err)
http.Error(w, "internal error", http.StatusInternalServerError)
return