Spec #135: owner data-correction actions — Latest Chapter, series_url, Cover, orphan removal #156

Merged
sulthan merged 15 commits from spec-135 into main 2026-08-22 12:10:41 +07:00
2 changed files with 274 additions and 5 deletions
Showing only changes of commit e1d61534bb - Show all commits
+49 -5
View File
@@ -98,6 +98,23 @@ func (p *Poller) fillBlankCover(ctx context.Context, sr store.Series, cover stri
}() }()
} }
// replaceCover is the Forced Poll's Cover path: the owner asked to accept the
// page as it now stands, so where fillBlankCover leaves a non-blank Cover
// alone (ADR-0007) this writes through whatever the page's Cover URL answers
// with, whether one exists or not. The accepted consequence (issue #135):
// refreshing the Cover and re-reading the chapters are one act — there is no
// Cover-only refetch.
func (p *Poller) replaceCover(ctx context.Context, sr store.Series, cover string) {
if cover == "" {
return
}
p.coverWG.Add(1)
go func() {
defer p.coverWG.Done()
p.storeCover(ctx, sr, cover)
}()
}
// prefetchCover heals Series that already carry a third-party source URL but // prefetchCover heals Series that already carry a third-party source URL but
// no stored address — the state left by client-supplied covers before // no stored address — the state left by client-supplied covers before
// acquisition moved server-side. Every Site takes the same path; fetchCoverBytes // acquisition moved server-side. Every Site takes the same path; fetchCoverBytes
@@ -122,17 +139,39 @@ func (p *Poller) prefetchCover(ctx context.Context, sr store.Series) {
p.storeCover(ctx, sr, sr.Cover) p.storeCover(ctx, sr, sr.Cover)
} }
// storeCover fetches bytes for sourceURL and points the Series at them. Every // storeCover fetches bytes for sourceURL and points the Series at them: a
// failure is logged against the Series and swallowed so the chapter poll // fill-only write for an ordinary pass, a write-through for a forced one
// cannot see it. // (issue #135). Every failure is logged against the Series and swallowed so
// the chapter poll cannot see it.
func (p *Poller) storeCover(ctx context.Context, sr store.Series, sourceURL string) { func (p *Poller) storeCover(ctx context.Context, sr store.Series, sourceURL string) {
bytes, contentType, err := fetchCoverBytes(ctx, sourceURL, p.CoverFetch, p.CoverBytesFetch) bytes, contentType, err := fetchCoverBytes(ctx, sourceURL, p.CoverFetch, p.CoverBytesFetch)
if err != nil { if err != nil {
log.Printf("latest poll %q: fetch cover %s: %v", sr.Key(), sourceURL, err) log.Printf("latest poll %q: fetch cover %s: %v", sr.Key(), sourceURL, err)
return return
} }
if err := p.Store.SetSeriesCover(sr.Site, sr.SeriesID, sourceURL, bytes, contentType); err != nil { if !sr.Forced {
if err := p.Store.SetSeriesCover(sr.Site, sr.SeriesID, sourceURL, bytes, contentType); err != nil {
log.Printf("latest poll %q: persist cover: %v", sr.Key(), err)
}
return
}
// The forced write replaces whether or not a Cover exists, and the row
// then tells the three outcomes apart: a blank filled, identical artwork
// re-served — an honest no-op — or a replacement whose previous address
// is stranded: its bytes stay served under the covers table (ADR-0014),
// the row just no longer points at them.
previous, current, err := p.Store.ReplaceSeriesCover(sr.Site, sr.SeriesID, sourceURL, bytes, contentType)
if err != nil {
log.Printf("latest poll %q: persist cover: %v", sr.Key(), err) log.Printf("latest poll %q: persist cover: %v", sr.Key(), err)
return
}
switch {
case previous == "":
log.Printf("latest poll %q: cover filled at %s", sr.Key(), current)
case previous == current:
log.Printf("latest poll %q: cover unchanged, the site re-serves the same bytes", sr.Key())
default:
log.Printf("latest poll %q: cover replaced %s -> %s", sr.Key(), previous, current)
} }
} }
@@ -621,7 +660,12 @@ func (p *Poller) checkOne(ctx context.Context, sr store.Series) (outcome readOut
p.healCover(ctx, sr) p.healCover(ctx, sr)
// Cover fill is independent of the chapter signal: a page that lost its // Cover fill is independent of the chapter signal: a page that lost its
// chapter list may keep its og:image, and a blank Series heals either way. // chapter list may keep its og:image, and a blank Series heals either way.
p.fillBlankCover(ctx, sr, facts.Cover) // A forced pass writes the Cover through the replace path instead.
if sr.Forced {
p.replaceCover(ctx, sr, facts.Cover)
} else {
p.fillBlankCover(ctx, sr, facts.Cover)
}
if !facts.HasLatest { if !facts.HasLatest {
// 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 rest instead of hot-looping. // already stamped, so this waits out a rest instead of hot-looping.
+225
View File
@@ -8,6 +8,7 @@ import (
"fmt" "fmt"
"log" "log"
"os" "os"
"path/filepath"
"strings" "strings"
"sync" "sync"
"testing" "testing"
@@ -2333,3 +2334,227 @@ func TestForcedSeriesWakesSleepingBrowser(t *testing.T) {
t.Fatalf("browser fetches with a forced series = %d, want 2 (the lane wakes)", got) t.Fatalf("browser fetches with a forced series = %d, want 2 (the lane wakes)", got)
} }
} }
// A forced pass accepts the page as it now stands, so it writes the Cover
// through the replace path; an ordinary pass still only fills a blank one
// (issue #135).
func TestRunOnceForcedPassReplacesExistingCover(t *testing.T) {
s, _ := newTestStore(t)
const (
key = "asura:chronicles-of-the-demon-faction-f886a8af"
seriesID = "chronicles-of-the-demon-faction-f886a8af"
seriesURL = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af"
first = "https://cdn.example/covers/first.jpg"
)
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key, Site: "asura", SeriesID: seriesID, SeriesURL: seriesURL, UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
if err := s.SetSeriesCover("asura", seriesID, first, []byte("first"), "image/jpeg"); err != nil {
t.Fatalf("seed cover: %v", err)
}
at := time.UnixMilli(5_000_000)
public := &fakeBytesCoverFetcher{body: []byte("second"), contentType: "image/jpeg"}
t.Run("unforced pass leaves the cover alone", func(t *testing.T) {
p := &Poller{
Store: s, Fetch: &fakeFetcher{body: asuraSeriesFixture + asuraCoverFixture, status: 200},
CoverBytesFetch: public,
Now: func() time.Time { return at },
}
p.runOnce(context.Background())
p.waitCovers()
if got := public.callCount(); got != 0 {
t.Fatalf("cover fetch calls = %d, want 0", got)
}
got := readBookmark(t, s, key)
if want := testCoverBaseURL + "/covers/" + store.CoverAddressForBytes([]byte("first")); got.Cover != want {
t.Fatalf("Cover = %q, want the first one %q", got.Cover, want)
}
})
t.Run("forced pass replaces the cover", func(t *testing.T) {
if err := s.ForceSeriesPoll("asura", seriesID, at.Add(time.Hour).UnixMilli()); err != nil {
t.Fatalf("ForceSeriesPoll: %v", err)
}
p := &Poller{
Store: s, Fetch: &fakeFetcher{body: asuraSeriesFixture + asuraCoverFixture, status: 200},
CoverBytesFetch: public,
Now: func() time.Time { return at },
}
p.runOnce(context.Background())
p.waitCovers()
if got := public.callCount(); got != 1 {
t.Fatalf("cover fetch calls = %d, want 1", got)
}
got := readBookmark(t, s, key)
if want := testCoverBaseURL + "/covers/" + store.CoverAddressForBytes([]byte("second")); got.Cover != want {
t.Fatalf("Cover = %q, want the second one %q", got.Cover, want)
}
body, _, ok, err := s.CoverByAddress(store.CoverAddressForBytes([]byte("second")))
if err != nil || !ok {
t.Fatalf("CoverByAddress: %v found=%v", err, ok)
}
if string(body) != "second" {
t.Fatalf("stored cover = %q, want second", body)
}
})
}
// Identical artwork re-served is an honest no-op the caller can tell apart
// from a replacement: the address comes from the bytes, so the row cannot
// change in substance, and the replace call site reports the three outcomes
// distinctly (issue #135).
func TestRunOnceForcedPassIdenticalBytesLogsNoOp(t *testing.T) {
s, _ := newTestStore(t)
const (
key = "asura:chronicles-of-the-demon-faction-f886a8af"
seriesID = "chronicles-of-the-demon-faction-f886a8af"
seriesURL = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af"
first = "https://cdn.example/covers/first.jpg"
)
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key, Site: "asura", SeriesID: seriesID, SeriesURL: seriesURL, UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
if err := s.SetSeriesCover("asura", seriesID, first, []byte("cover-bytes"), "image/jpeg"); err != nil {
t.Fatalf("seed cover: %v", err)
}
at := time.UnixMilli(5_000_000)
if err := s.ForceSeriesPoll("asura", seriesID, at.Add(time.Hour).UnixMilli()); err != nil {
t.Fatalf("ForceSeriesPoll: %v", err)
}
var logs strings.Builder
prev := log.Writer()
log.SetOutput(&logs)
t.Cleanup(func() { log.SetOutput(prev) })
p := &Poller{
Store: s, Fetch: &fakeFetcher{body: asuraSeriesFixture + asuraCoverFixture, status: 200},
CoverBytesFetch: &fakeBytesCoverFetcher{body: []byte("cover-bytes"), contentType: "image/jpeg"},
Now: func() time.Time { return at },
}
p.runOnce(context.Background())
p.waitCovers()
got := readBookmark(t, s, key)
if want := testCoverBaseURL + "/covers/" + store.CoverAddressForBytes([]byte("cover-bytes")); got.Cover != want {
t.Fatalf("Cover = %q, want unchanged %q", got.Cover, want)
}
if body, _, ok, err := s.CoverByAddress(store.CoverAddressForBytes([]byte("cover-bytes"))); err != nil || !ok || string(body) != "cover-bytes" {
t.Fatalf("stored cover after no-op: found=%v err=%v", ok, err)
}
logged := logs.String()
if !strings.Contains(logged, "cover unchanged") {
t.Fatalf("no-op not reported as unchanged; log:\n%s", logged)
}
if strings.Contains(logged, "cover replaced") {
t.Fatalf("no-op reported as a replacement; log:\n%s", logged)
}
}
// A Series whose sharded Cover file was unlinked out from under it is
// repaired by one forced pass: same bytes mean the same address and the file
// re-linked (issue #135, story 32).
func TestRunOnceForcedPassRelinksUnlinkedCoverFile(t *testing.T) {
coverDir := t.TempDir()
url := pgtest.URL(t)
s, err := store.Open(url, testOwner, coverDir, testCoverBaseURL)
if err != nil {
t.Fatalf("Open: %v", err)
}
t.Cleanup(func() { s.Close() })
const (
key = "asura:chronicles-of-the-demon-faction-f886a8af"
seriesID = "chronicles-of-the-demon-faction-f886a8af"
seriesURL = "https://asurascans.com/comics/chronicles-of-the-demon-faction-f886a8af"
first = "https://cdn.example/covers/first.jpg"
)
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key, Site: "asura", SeriesID: seriesID, SeriesURL: seriesURL, UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
if err := s.SetSeriesCover("asura", seriesID, first, []byte("cover-bytes"), "image/jpeg"); err != nil {
t.Fatalf("seed cover: %v", err)
}
address := store.CoverAddressForBytes([]byte("cover-bytes"))
coverPath := filepath.Join(coverDir, filepath.FromSlash(address[:2]+"/"+address[2:4]+"/"+address))
if err := os.Remove(coverPath); err != nil {
t.Fatalf("unlink cover file: %v", err)
}
if _, _, ok, err := s.CoverByAddress(address); err != nil || ok {
t.Fatalf("CoverByAddress after unlink = found %v err %v; want missing (file gone)", ok, err)
}
at := time.UnixMilli(5_000_000)
if err := s.ForceSeriesPoll("asura", seriesID, at.Add(time.Hour).UnixMilli()); err != nil {
t.Fatalf("ForceSeriesPoll: %v", err)
}
p := &Poller{
Store: s, Fetch: &fakeFetcher{body: asuraSeriesFixture + asuraCoverFixture, status: 200},
CoverBytesFetch: &fakeBytesCoverFetcher{body: []byte("cover-bytes"), contentType: "image/jpeg"},
Now: func() time.Time { return at },
}
p.runOnce(context.Background())
p.waitCovers()
body, contentType, ok, err := s.CoverByAddress(address)
if err != nil || !ok {
t.Fatalf("CoverByAddress after forced pass: found=%v err=%v; want the file re-linked", ok, err)
}
if string(body) != "cover-bytes" || contentType != "image/jpeg" {
t.Fatalf("re-linked cover = (%q, %q), want (cover-bytes, image/jpeg)", body, contentType)
}
}
// A forced pass degrades exactly like an ordinary one when the cover sidecar
// is unreachable: the fetch is skipped and logged, the chapter poll is
// untouched, and the stored Cover is not moved (issue #135, story 6).
func TestRunOnceForcedPassWithoutCoverFetcherStillPolls(t *testing.T) {
s, _ := newTestStore(t)
const (
key = "kagane:019f84bc-9ba0-7ed9-86f5-8b905ec7c28b"
seriesID = "019f84bc-9ba0-7ed9-86f5-8b905ec7c28b"
)
if _, err := s.Upsert(s.OwnerID(), store.Bookmark{
Key: key, Site: "kagane", SeriesID: seriesID,
SeriesURL: "https://kagane.to/series/" + seriesID, UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
if err := s.SetSeriesCover("kagane", seriesID, "https://kagane.to/api/v2/image/019f84bc-9ba0-7ed9-86f5-8b905ec7c28b/compressed", []byte("existing"), "image/webp"); err != nil {
t.Fatalf("seed cover: %v", err)
}
at := time.UnixMilli(5_000_000)
if err := s.ForceSeriesPoll("kagane", seriesID, at.Add(time.Hour).UnixMilli()); err != nil {
t.Fatalf("ForceSeriesPoll: %v", err)
}
var logs strings.Builder
prev := log.Writer()
log.SetOutput(&logs)
t.Cleanup(func() { log.SetOutput(prev) })
p := &Poller{
Store: s, BrowserFetch: &fakeFetcher{body: kaganeAPIFixtureWithCover, status: 200},
Now: func() time.Time { return at },
}
p.runOnce(context.Background())
p.waitCovers()
got := readBookmark(t, s, key)
if got.LatestChapterNum == nil || *got.LatestChapterNum != 41 {
t.Fatalf("LatestChapterNum = %v, want 41", got.LatestChapterNum)
}
if want := testCoverBaseURL + "/covers/" + store.CoverAddressForBytes([]byte("existing")); got.Cover != want {
t.Fatalf("Cover = %q, want the existing one %q untouched", got.Cover, want)
}
if checked := readLatestCheckedAt(t, s, key); checked != at.UnixMilli() {
t.Fatalf("latest_checked_at = %d, want %d", checked, at.UnixMilli())
}
if !strings.Contains(logs.String(), "fetch cover") {
t.Fatalf("missing skipped-cover log; log:\n%s", logs.String())
}
}