From 7e1cbdde9e18ca0ec9bbbd9f0fcd85f20a1c2b2d Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sat, 22 Aug 2026 09:19:15 +0700 Subject: [PATCH] Forced Poll replaces the Cover; ordinary pass still fills only a blank one (#153) --- backend/internal/latest/poller.go | 54 +++++- backend/internal/latest/poller_test.go | 225 +++++++++++++++++++++++++ 2 files changed, 274 insertions(+), 5 deletions(-) diff --git a/backend/internal/latest/poller.go b/backend/internal/latest/poller.go index 035ff84..3bb3f35 100644 --- a/backend/internal/latest/poller.go +++ b/backend/internal/latest/poller.go @@ -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 // no stored address — the state left by client-supplied covers before // 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) } -// storeCover fetches bytes for sourceURL and points the Series at them. Every -// failure is logged against the Series and swallowed so the chapter poll -// cannot see it. +// storeCover fetches bytes for sourceURL and points the Series at them: a +// fill-only write for an ordinary pass, a write-through for a forced one +// (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) { bytes, contentType, err := fetchCoverBytes(ctx, sourceURL, p.CoverFetch, p.CoverBytesFetch) if err != nil { log.Printf("latest poll %q: fetch cover %s: %v", sr.Key(), sourceURL, err) 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 three outcomes apart: a blank filled, identical artwork + // re-served — an honest no-op — or a replacement that strands previous, + // which the next wave reclaims at this exact call site. + 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) + 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) // 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. - 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 { // 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. diff --git a/backend/internal/latest/poller_test.go b/backend/internal/latest/poller_test.go index 3bfc797..7a41c04 100644 --- a/backend/internal/latest/poller_test.go +++ b/backend/internal/latest/poller_test.go @@ -8,6 +8,7 @@ import ( "fmt" "log" "os" + "path/filepath" "strings" "sync" "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) } } + +// 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()) + } +}