diff --git a/backend/handlers.go b/backend/handlers.go index 84f7e32..c427bf1 100644 --- a/backend/handlers.go +++ b/backend/handlers.go @@ -60,14 +60,19 @@ func (h *bookmarkHandler) put(w http.ResponseWriter, r *http.Request) { } } } - b.UpdatedAt = time.Now().UnixMilli() // server-assigned, ignore client value + // Candidate timestamp, not a decision: Upsert keeps the stored one unless + // reading progress actually moved. Any client value is ignored. + b.UpdatedAt = time.Now().UnixMilli() - if err := h.store.Upsert(b); err != nil { + stored, err := h.store.Upsert(b) + if err != nil { log.Printf("upsert: %v", err) http.Error(w, "internal error", http.StatusInternalServerError) return } - writeJSON(w, http.StatusOK, b) + // Echo the stored row: clients adopt this as their cached copy, so it must + // carry the authoritative updated_at rather than the candidate above. + writeJSON(w, http.StatusOK, stored) } // delete removes one bookmark. DELETE /bookmarks/{key} diff --git a/backend/store.go b/backend/store.go index 161c4e3..dc8a09a 100644 --- a/backend/store.go +++ b/backend/store.go @@ -9,33 +9,54 @@ import ( ) // Bookmark is one tracked series, keyed ":" across both sites. +// +// LastChapter* is the user's read progress; LatestChapter* is the newest +// chapter the site has published, captured opportunistically by the userscript. type Bookmark struct { - Key string `json:"key"` - Site string `json:"site"` - SeriesID string `json:"series_id"` - Title string `json:"title"` - SeriesURL string `json:"series_url"` - Cover string `json:"cover"` - LastChapter string `json:"last_chapter"` - LastChapterNum float64 `json:"last_chapter_num"` - LastChapterURL string `json:"last_chapter_url"` - UpdatedAt int64 `json:"updated_at"` // unix ms, server-assigned + Key string `json:"key"` + Site string `json:"site"` + SeriesID string `json:"series_id"` + Title string `json:"title"` + SeriesURL string `json:"series_url"` + Cover string `json:"cover"` + LastChapter string `json:"last_chapter"` + LastChapterNum float64 `json:"last_chapter_num"` + LastChapterURL string `json:"last_chapter_url"` + Favorite bool `json:"favorite"` + LatestChapter string `json:"latest_chapter"` + LatestChapterNum *float64 `json:"latest_chapter_num"` // nil until first captured + UpdatedAt int64 `json:"updated_at"` // unix ms; see Upsert } const schema = ` CREATE TABLE IF NOT EXISTS bookmarks ( - key TEXT PRIMARY KEY, - site TEXT NOT NULL, - series_id TEXT NOT NULL, - title TEXT, - series_url TEXT, - cover TEXT, - last_chapter TEXT, - last_chapter_num REAL, - last_chapter_url TEXT, - updated_at INTEGER NOT NULL + key TEXT PRIMARY KEY, + site TEXT NOT NULL, + series_id TEXT NOT NULL, + title TEXT, + series_url TEXT, + cover TEXT, + last_chapter TEXT, + last_chapter_num REAL, + last_chapter_url TEXT, + favorite INTEGER NOT NULL DEFAULT 0, + latest_chapter TEXT NOT NULL DEFAULT '', + latest_chapter_num REAL, + updated_at INTEGER NOT NULL );` +// The columns above that databases created before them will be missing. +// SQLite has no ADD COLUMN IF NOT EXISTS, so each is added only when absent. +var addedColumns = []struct{ name, ddl string }{ + {"favorite", `ALTER TABLE bookmarks ADD COLUMN favorite INTEGER NOT NULL DEFAULT 0`}, + {"latest_chapter", `ALTER TABLE bookmarks ADD COLUMN latest_chapter TEXT NOT NULL DEFAULT ''`}, + {"latest_chapter_num", `ALTER TABLE bookmarks ADD COLUMN latest_chapter_num REAL`}, +} + +const bookmarkColumns = `key, site, series_id, title, series_url, cover, + last_chapter, last_chapter_num, last_chapter_url, + favorite, latest_chapter, latest_chapter_num, updated_at` + // Store is the SQLite-backed bookmark store. type Store struct { db *sql.DB @@ -58,17 +79,91 @@ func OpenStore(path string) (*Store, error) { db.Close() return nil, fmt.Errorf("apply schema: %w", err) } + if err := migrateColumns(db); err != nil { + db.Close() + return nil, fmt.Errorf("migrate schema: %w", err) + } return &Store{db: db}, nil } +// migrateColumns brings a pre-existing bookmarks table up to the current +// schema. Safe to run on every start: columns already present are skipped. +func migrateColumns(db *sql.DB) error { + have, err := existingColumns(db, "bookmarks") + if err != nil { + return err + } + for _, c := range addedColumns { + if _, ok := have[c.name]; ok { + continue + } + if _, err := db.Exec(c.ddl); err != nil { + return fmt.Errorf("add column %q: %w", c.name, err) + } + } + return nil +} + +func existingColumns(db *sql.DB, table string) (map[string]struct{}, error) { + rows, err := db.Query(`SELECT name FROM pragma_table_info(?)`, table) + if err != nil { + return nil, fmt.Errorf("read %s columns: %w", table, err) + } + defer rows.Close() + + out := map[string]struct{}{} + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + return nil, fmt.Errorf("scan column name: %w", err) + } + out[name] = struct{}{} + } + return out, rows.Err() +} + +// scanBookmark reads one row in bookmarkColumns order, translating SQLite's +// integer bool and nullable latest_chapter_num into Go types. +// +// The optional columns are read through Null* types because rows predating +// this code (or written by hand) may hold NULL where the app only ever writes +// zero values. Only latest_chapter_num distinguishes the two: everywhere else +// NULL and the zero value mean the same thing to clients. +func scanBookmark(scan func(...any) error) (Bookmark, error) { + var ( + b Bookmark + title, seriesURL, cover sql.NullString + lastChapter, lastChapterURL, latestChapter sql.NullString + lastChapterNum, latestChapterNum sql.NullFloat64 + favorite sql.NullInt64 + ) + if err := scan( + &b.Key, &b.Site, &b.SeriesID, &title, &seriesURL, &cover, + &lastChapter, &lastChapterNum, &lastChapterURL, + &favorite, &latestChapter, &latestChapterNum, &b.UpdatedAt, + ); err != nil { + return Bookmark{}, err + } + b.Title = title.String + b.SeriesURL = seriesURL.String + b.Cover = cover.String + b.LastChapter = lastChapter.String + b.LastChapterNum = lastChapterNum.Float64 + b.LastChapterURL = lastChapterURL.String + b.Favorite = favorite.Int64 != 0 + b.LatestChapter = latestChapter.String + if latestChapterNum.Valid { + b.LatestChapterNum = &latestChapterNum.Float64 + } + return b, nil +} + // Close releases the underlying database handle. func (s *Store) Close() error { return s.db.Close() } // List returns every bookmark, newest activity first. func (s *Store) List() ([]Bookmark, error) { - rows, err := s.db.Query(` - SELECT key, site, series_id, title, series_url, cover, - last_chapter, last_chapter_num, last_chapter_url, updated_at + rows, err := s.db.Query(`SELECT ` + bookmarkColumns + ` FROM bookmarks ORDER BY updated_at DESC`) if err != nil { @@ -78,11 +173,8 @@ func (s *Store) List() ([]Bookmark, error) { out := []Bookmark{} for rows.Next() { - var b Bookmark - if err := rows.Scan( - &b.Key, &b.Site, &b.SeriesID, &b.Title, &b.SeriesURL, &b.Cover, - &b.LastChapter, &b.LastChapterNum, &b.LastChapterURL, &b.UpdatedAt, - ); err != nil { + b, err := scanBookmark(rows.Scan) + if err != nil { return nil, fmt.Errorf("scan bookmark: %w", err) } out = append(out, b) @@ -90,24 +182,60 @@ func (s *Store) List() ([]Bookmark, error) { return out, rows.Err() } -// Upsert inserts or replaces a bookmark by key (last-write-wins). -func (s *Store) Upsert(b Bookmark) error { - _, err := s.db.Exec(` - INSERT INTO bookmarks - (key, site, series_id, title, series_url, cover, - last_chapter, last_chapter_num, last_chapter_url, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +// Upsert inserts or replaces a bookmark by key (last-write-wins) and returns +// the row as actually stored. +// +// 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 +// 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) { + tx, err := s.db.Begin() + if err != nil { + return Bookmark{}, fmt.Errorf("begin %q: %w", b.Key, err) + } + defer tx.Rollback() + + var latestNum any + if b.LatestChapterNum != nil { + latestNum = *b.LatestChapterNum + } + + // IS NOT is SQLite's null-safe comparison. 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 (`+bookmarkColumns+`) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(key) DO UPDATE SET site=excluded.site, series_id=excluded.series_id, title=excluded.title, series_url=excluded.series_url, cover=excluded.cover, last_chapter=excluded.last_chapter, last_chapter_num=excluded.last_chapter_num, - last_chapter_url=excluded.last_chapter_url, updated_at=excluded.updated_at`, + last_chapter_url=excluded.last_chapter_url, + favorite=excluded.favorite, + latest_chapter=excluded.latest_chapter, + latest_chapter_num=excluded.latest_chapter_num, + updated_at=CASE + WHEN bookmarks.last_chapter_num IS NOT excluded.last_chapter_num + THEN excluded.updated_at + ELSE bookmarks.updated_at + END`, b.Key, b.Site, b.SeriesID, b.Title, b.SeriesURL, b.Cover, - b.LastChapter, b.LastChapterNum, b.LastChapterURL, b.UpdatedAt) - if err != nil { - return fmt.Errorf("upsert %q: %w", b.Key, err) + b.LastChapter, b.LastChapterNum, b.LastChapterURL, + b.Favorite, b.LatestChapter, latestNum, b.UpdatedAt); err != nil { + return Bookmark{}, fmt.Errorf("upsert %q: %w", b.Key, err) } - return nil + + stored, err := scanBookmark(tx.QueryRow( + `SELECT `+bookmarkColumns+` FROM bookmarks WHERE key = ?`, b.Key).Scan) + if err != nil { + return Bookmark{}, fmt.Errorf("read back %q: %w", b.Key, err) + } + if err := tx.Commit(); err != nil { + return Bookmark{}, fmt.Errorf("commit %q: %w", b.Key, err) + } + return stored, nil } // Delete removes a bookmark by key. Deleting a missing key is not an error. diff --git a/backend/store_test.go b/backend/store_test.go index 3c18906..8c8b081 100644 --- a/backend/store_test.go +++ b/backend/store_test.go @@ -2,11 +2,15 @@ package main import ( "bytes" + "database/sql" "encoding/json" + "fmt" "net/http" "net/http/httptest" "path/filepath" + "strings" "testing" + "time" ) const testToken = "s3cret-token" @@ -187,3 +191,245 @@ func TestBookmarkRoundTrip(t *testing.T) { t.Fatalf("after delete list = %+v, want empty", list) } } + +// putBookmark PUTs b at key and returns the bookmark the server echoes back, +// which is the row as actually stored (not the request payload). +func putBookmark(t *testing.T, srv http.Handler, key string, b Bookmark) Bookmark { + t.Helper() + body, _ := json.Marshal(b) + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, auth(httptest.NewRequest(http.MethodPut, "/bookmarks/"+key, bytes.NewReader(body)))) + if rr.Code != http.StatusOK { + t.Fatalf("PUT %s status = %d, body = %s", key, rr.Code, rr.Body.String()) + } + var out Bookmark + if err := json.Unmarshal(rr.Body.Bytes(), &out); err != nil { + t.Fatalf("decode PUT response: %v", err) + } + return out +} + +func getBookmarks(t *testing.T, srv http.Handler) []Bookmark { + t.Helper() + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, auth(httptest.NewRequest(http.MethodGet, "/bookmarks", nil))) + if rr.Code != http.StatusOK { + t.Fatalf("GET status = %d", rr.Code) + } + var list []Bookmark + if err := json.Unmarshal(rr.Body.Bytes(), &list); err != nil { + t.Fatalf("decode list: %v", err) + } + return list +} + +func floatPtr(f float64) *float64 { return &f } + +// updated_at drives list ordering, so it must move only on a real progress +// advance — never on a favorite toggle or a latest-chapter capture. +func TestUpsertConditionalUpdatedAt(t *testing.T) { + cases := []struct { + name string + mutate func(Bookmark) Bookmark + wantBumped bool + }{ + { + name: "unchanged progress", + mutate: func(b Bookmark) Bookmark { return b }, + wantBumped: false, + }, + { + name: "changed progress", + mutate: func(b Bookmark) Bookmark { + b.LastChapter, b.LastChapterNum = "Chapter 11", 11 + return b + }, + wantBumped: true, + }, + { + name: "favorite only", + mutate: func(b Bookmark) Bookmark { + b.Favorite = true + return b + }, + wantBumped: false, + }, + { + name: "latest chapter only", + mutate: func(b Bookmark) Bookmark { + b.LatestChapter, b.LatestChapterNum = "Chapter 15", floatPtr(15) + return b + }, + wantBumped: false, + }, + { + name: "unrelated metadata only", + mutate: func(b Bookmark) Bookmark { + b.Title, b.Cover = "Renamed", "https://example.test/new.jpg" + return b + }, + wantBumped: false, + }, + } + + for i, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := newTestServer(t) + key := fmt.Sprintf("asura:cond-%d", i) + + first := putBookmark(t, srv, key, Bookmark{ + Title: "Test", + LastChapter: "Chapter 10", + LastChapterNum: 10, + }) + if first.UpdatedAt == 0 { + t.Fatal("new bookmark did not get updated_at set") + } + + // Guarantee a later wall-clock ms so a real bump is observable. + time.Sleep(2 * time.Millisecond) + + second := putBookmark(t, srv, key, tc.mutate(first)) + if tc.wantBumped && second.UpdatedAt <= first.UpdatedAt { + t.Fatalf("updated_at = %d, want > %d", second.UpdatedAt, first.UpdatedAt) + } + if !tc.wantBumped && second.UpdatedAt != first.UpdatedAt { + t.Fatalf("updated_at = %d, want preserved %d", second.UpdatedAt, first.UpdatedAt) + } + + // The PUT response must match what a subsequent GET reports. + list := getBookmarks(t, srv) + if len(list) != 1 { + t.Fatalf("list = %+v, want 1 item", list) + } + if list[0].UpdatedAt != second.UpdatedAt { + t.Fatalf("GET updated_at = %d, PUT echoed %d", list[0].UpdatedAt, second.UpdatedAt) + } + }) + } +} + +func TestFavoriteRoundTrip(t *testing.T) { + srv := newTestServer(t) + key := "demonic:some-series" + + stored := putBookmark(t, srv, key, Bookmark{Title: "Fav", Favorite: true}) + if !stored.Favorite { + t.Fatalf("PUT response favorite = false, want true") + } + + list := getBookmarks(t, srv) + if len(list) != 1 || !list[0].Favorite { + t.Fatalf("favorite did not round-trip: %+v", list) + } + + // Unfavoriting must persist too (guards against a write that only ever ORs in true). + stored = putBookmark(t, srv, key, Bookmark{Title: "Fav", Favorite: false}) + if stored.Favorite { + t.Fatal("PUT response favorite = true after unfavorite") + } + list = getBookmarks(t, srv) + if len(list) != 1 || list[0].Favorite { + t.Fatalf("unfavorite did not round-trip: %+v", list) + } +} + +func TestLatestChapterNullable(t *testing.T) { + srv := newTestServer(t) + key := "asura:latest-test" + + // Never captured: latest_chapter_num must serialize as JSON null. + body, _ := json.Marshal(Bookmark{Title: "No latest yet"}) + 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", rr.Code) + } + if !strings.Contains(rr.Body.String(), `"latest_chapter_num":null`) { + t.Fatalf("want latest_chapter_num null in response, got %s", rr.Body.String()) + } + + list := getBookmarks(t, srv) + if len(list) != 1 || list[0].LatestChapterNum != nil { + t.Fatalf("latest_chapter_num = %v, want nil", list[0].LatestChapterNum) + } + + // Once captured it round-trips as a value. + stored := putBookmark(t, srv, key, Bookmark{ + Title: "No latest yet", + LatestChapter: "Chapter 162", + LatestChapterNum: floatPtr(162), + }) + if stored.LatestChapterNum == nil || *stored.LatestChapterNum != 162 { + t.Fatalf("PUT response latest_chapter_num = %v, want 162", stored.LatestChapterNum) + } + list = getBookmarks(t, srv) + if len(list) != 1 || list[0].LatestChapterNum == nil || *list[0].LatestChapterNum != 162 { + t.Fatalf("latest chapter did not round-trip: %+v", list) + } + if list[0].LatestChapter != "Chapter 162" { + t.Fatalf("latest_chapter = %q, want %q", list[0].LatestChapter, "Chapter 162") + } +} + +// The deployed database predates favorite/latest_chapter*, and CREATE TABLE +// IF NOT EXISTS will not add them — OpenStore must migrate in place. +func TestOpenStoreMigratesLegacySchema(t *testing.T) { + dbPath := filepath.Join(t.TempDir(), "legacy.db") + + legacy, err := sql.Open("sqlite", dbPath) + if err != nil { + t.Fatalf("open legacy db: %v", err) + } + if _, err := legacy.Exec(` + CREATE TABLE bookmarks ( + key TEXT PRIMARY KEY, + site TEXT NOT NULL, + series_id TEXT NOT NULL, + title TEXT, + series_url TEXT, + cover TEXT, + last_chapter TEXT, + last_chapter_num REAL, + last_chapter_url TEXT, + updated_at INTEGER NOT NULL + )`); err != nil { + t.Fatalf("create legacy schema: %v", err) + } + if _, err := legacy.Exec(` + INSERT INTO bookmarks (key, site, series_id, title, last_chapter, last_chapter_num, updated_at) + VALUES ('asura:legacy', 'asura', 'legacy', 'Legacy Series', 'Chapter 7', 7, 123)`); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + if err := legacy.Close(); err != nil { + t.Fatalf("close legacy db: %v", err) + } + + store, err := OpenStore(dbPath) + if err != nil { + t.Fatalf("OpenStore on legacy db: %v", err) + } + t.Cleanup(func() { store.Close() }) + + list, err := store.List() + if err != nil { + t.Fatalf("List: %v", err) + } + if len(list) != 1 || list[0].Key != "asura:legacy" { + t.Fatalf("legacy row lost: %+v", list) + } + got := list[0] + if got.Title != "Legacy Series" || got.LastChapterNum != 7 || got.UpdatedAt != 123 { + t.Fatalf("legacy data mangled: %+v", got) + } + if got.Favorite || got.LatestChapter != "" || got.LatestChapterNum != nil { + t.Fatalf("new columns should default empty, got %+v", got) + } + + // Reopening an already-migrated database must be a no-op, not an error. + store2, err := OpenStore(dbPath) + if err != nil { + t.Fatalf("OpenStore is not idempotent: %v", err) + } + store2.Close() +}