Remember when a Site called a work completed (#169)
Add series.site_completed_at (epoch-ms, default 0): the durable answer #168's predicates compute. checkOne writes it after the successful read on the zero/non-zero transition only, so a Series still completed keeps its original stamp (the age #170 prints is 'since the Site first said so'), one that stopped is zeroed, one that became completed is stamped. The due query projects the column, so the transition is a field comparison on the snapshot checkOne already holds. Refused, unreachable and errored reads reach nothing (AC4 is placement, not a guard); the write sits before the no-chapter and unchanged-number returns, so a completed page whose chapter did not change still writes. Store failure logs and carries on — the outcome word never changes. Series-level like Latest Chapter: a bookmark's updated_at is never touched. #170 is the surface; nothing reads the column yet.
This commit is contained in:
@@ -678,6 +678,25 @@ func (p *Poller) checkOne(ctx context.Context, sr store.Series) (outcome readOut
|
|||||||
} else {
|
} else {
|
||||||
p.fillBlankCover(ctx, sr, facts.Cover)
|
p.fillBlankCover(ctx, sr, facts.Cover)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Learned from the successful read: the write sits after the error switch
|
||||||
|
// (a refused, unreachable or errored read reaches nothing) and before the
|
||||||
|
// returns below — a completed page whose chapter number did not change
|
||||||
|
// still has to write. The transition is zero-versus-nonzero, not the
|
||||||
|
// stamp's value: a Series still completed keeps its original stamp, so the
|
||||||
|
// age #170 prints is "since the Site first said so"; one that stopped
|
||||||
|
// being completed is zeroed.
|
||||||
|
want := int64(0)
|
||||||
|
if facts.SiteCompleted {
|
||||||
|
want = p.Now().UnixMilli()
|
||||||
|
}
|
||||||
|
if (sr.SiteCompletedAt == 0) != (want == 0) {
|
||||||
|
if err := p.Store.SetSiteCompletedAt(sr.Site, sr.SeriesID, want); err != nil {
|
||||||
|
// Best-effort, like every poller write: never change the outcome
|
||||||
|
// word the pass counts.
|
||||||
|
log.Printf("latest poll %q: set site completed: %v", sr.Key(), err)
|
||||||
|
}
|
||||||
|
}
|
||||||
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.
|
||||||
|
|||||||
@@ -2725,3 +2725,154 @@ func TestRunOnceForcedPassReclaimFailureDoesNotFailPoll(t *testing.T) {
|
|||||||
t.Fatalf("Cover = %q, want the replacement %q", got.Cover, want)
|
t.Fatalf("Cover = %q, want the replacement %q", got.Cover, want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// readSiteCompletedAt reads the poller's site_completed_at column directly:
|
||||||
|
// it is a poller fact with no client-visible getter, and #170 is the surface
|
||||||
|
// that will read it.
|
||||||
|
func readSiteCompletedAt(t *testing.T, dbURL, site, seriesID string) int64 {
|
||||||
|
t.Helper()
|
||||||
|
db, err := sql.Open("pgx", dbURL)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("open %s: %v", dbURL, err)
|
||||||
|
}
|
||||||
|
defer db.Close()
|
||||||
|
var at int64
|
||||||
|
if err := db.QueryRow(
|
||||||
|
`SELECT site_completed_at FROM series WHERE site = $1 AND series_id = $2`,
|
||||||
|
site, seriesID).Scan(&at); err != nil {
|
||||||
|
t.Fatalf("read site_completed_at %s:%s: %v", site, seriesID, err)
|
||||||
|
}
|
||||||
|
return at
|
||||||
|
}
|
||||||
|
|
||||||
|
// seedSiteCompletedAt writes the column directly, for the refused/unreachable
|
||||||
|
// tests that need a pre-existing stamp.
|
||||||
|
func seedSiteCompletedAt(t *testing.T, dbURL, site, seriesID string, at int64) {
|
||||||
|
t.Helper()
|
||||||
|
db, err := sql.Open("pgx", dbURL)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("open %s: %v", dbURL, err)
|
||||||
|
}
|
||||||
|
defer db.Close()
|
||||||
|
if _, err := db.Exec(
|
||||||
|
`UPDATE series SET site_completed_at = $3 WHERE site = $1 AND series_id = $2`,
|
||||||
|
site, seriesID, at); err != nil {
|
||||||
|
t.Fatalf("seed site_completed_at %s:%s: %v", site, seriesID, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A completed page stamps site_completed_at with the pass's own clock; the
|
||||||
|
// same page an hour later is due again but must not move the stamp — the age
|
||||||
|
// #170 prints is "since the Site first said so", not the age of the last
|
||||||
|
// Poll. The transition is zero-versus-nonzero, so the exact value asserted
|
||||||
|
// here is load-bearing.
|
||||||
|
func TestRunOnceSiteCompletedStampsOnce(t *testing.T) {
|
||||||
|
s, dbURL := 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"
|
||||||
|
)
|
||||||
|
completed := asuraSeriesFixture + asuraCompletedFixture
|
||||||
|
at := time.UnixMilli(5_000_000)
|
||||||
|
seedForCheck(t, s, key, seriesURL, at.Add(-time.Hour).UnixMilli())
|
||||||
|
|
||||||
|
newTestPoller(t, s, &fakeFetcher{body: completed, status: 200}, at).runOnce(context.Background())
|
||||||
|
if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != at.UnixMilli() {
|
||||||
|
t.Fatalf("site_completed_at after completed poll = %d, want %d", got, at.UnixMilli())
|
||||||
|
}
|
||||||
|
|
||||||
|
hourLater := at.Add(time.Hour)
|
||||||
|
newTestPoller(t, s, &fakeFetcher{body: completed, status: 200}, hourLater).runOnce(context.Background())
|
||||||
|
if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != at.UnixMilli() {
|
||||||
|
t.Fatalf("site_completed_at after a second completed poll = %d, want the original stamp %d", got, at.UnixMilli())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// An ongoing page is the inverse transition: a never-hinted Series stays zero
|
||||||
|
// (an ordinary Poll of an ongoing Series writes nothing), and a hinted one is
|
||||||
|
// zeroed — the Site no longer says completed, so the claim must not outlive
|
||||||
|
// the evidence.
|
||||||
|
func TestRunOnceOngoingPageClearsOrSkipsSiteCompleted(t *testing.T) {
|
||||||
|
s, dbURL := 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"
|
||||||
|
)
|
||||||
|
ongoing := asuraSeriesFixture + asuraOngoingFixture
|
||||||
|
at := time.UnixMilli(5_000_000)
|
||||||
|
seedForCheck(t, s, key, seriesURL, at.Add(-time.Hour).UnixMilli())
|
||||||
|
|
||||||
|
newTestPoller(t, s, &fakeFetcher{body: ongoing, status: 200}, at).runOnce(context.Background())
|
||||||
|
if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != 0 {
|
||||||
|
t.Fatalf("site_completed_at after ongoing poll of a never-hinted series = %d, want 0", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
seedSiteCompletedAt(t, dbURL, "asura", seriesID, at.UnixMilli())
|
||||||
|
hourLater := at.Add(time.Hour)
|
||||||
|
newTestPoller(t, s, &fakeFetcher{body: ongoing, status: 200}, hourLater).runOnce(context.Background())
|
||||||
|
if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != 0 {
|
||||||
|
t.Fatalf("site_completed_at after ongoing poll of a hinted series = %d, want 0", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A refused or unreachable read reaches nothing after checkOne's error switch:
|
||||||
|
// a challenge is not evidence about the work, so a pre-seeded stamp must
|
||||||
|
// survive both.
|
||||||
|
func TestRunOnceRefusedOrUnreachableLeavesSiteCompletedUntouched(t *testing.T) {
|
||||||
|
now := time.UnixMilli(5_000_000)
|
||||||
|
|
||||||
|
t.Run("refused", func(t *testing.T) {
|
||||||
|
s, dbURL := 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"
|
||||||
|
)
|
||||||
|
seedForCheck(t, s, key, seriesURL, 0)
|
||||||
|
seedSiteCompletedAt(t, dbURL, "asura", seriesID, now.UnixMilli())
|
||||||
|
newTestPoller(t, s, &fakeFetcher{status: 403}, now).runOnce(context.Background())
|
||||||
|
if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != now.UnixMilli() {
|
||||||
|
t.Fatalf("site_completed_at after refused poll = %d, want the pre-seeded %d", got, now.UnixMilli())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("mid-loop unreachable", func(t *testing.T) {
|
||||||
|
s, dbURL := newTestStore(t)
|
||||||
|
seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0)
|
||||||
|
seedSiteCompletedAt(t, dbURL, "comix", "c", now.UnixMilli())
|
||||||
|
interrupted := fmt.Errorf("%w: %w", errBrowserInterrupted, errors.New("restart"))
|
||||||
|
browser := &fakeFetcher{status: 200, err: interrupted}
|
||||||
|
p := newTestPoller(t, s, &fakeFetcher{status: 200}, now)
|
||||||
|
p.BrowserFetch = browser
|
||||||
|
p.runLanePass(context.Background(), "comix", false)
|
||||||
|
if got := readSiteCompletedAt(t, dbURL, "comix", "c"); got != now.UnixMilli() {
|
||||||
|
t.Fatalf("site_completed_at after unreachable poll = %d, want the pre-seeded %d", got, now.UnixMilli())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// The regression AC2's second sentence exists to prevent: the completed write
|
||||||
|
// must not ride the chapter setter, so a completed page whose chapter number
|
||||||
|
// is unchanged — checkOne's equality early return — still stamps the column.
|
||||||
|
func TestRunOnceSiteCompletedWritesDespiteUnchangedChapter(t *testing.T) {
|
||||||
|
s, dbURL := 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"
|
||||||
|
)
|
||||||
|
completed := asuraSeriesFixture + asuraCompletedFixture
|
||||||
|
at := time.UnixMilli(5_000_000)
|
||||||
|
seedForCheck(t, s, key, seriesURL, 0)
|
||||||
|
// The page parses Chapter 181 as the latest; store the same number so the
|
||||||
|
// equality early return fires and only the completed write can run.
|
||||||
|
if err := s.SetLatestChapter("asura", seriesID, "Chapter 181", 181); err != nil {
|
||||||
|
t.Fatalf("seed latest chapter: %v", err)
|
||||||
|
}
|
||||||
|
newTestPoller(t, s, &fakeFetcher{body: completed, status: 200}, at).runOnce(context.Background())
|
||||||
|
if got := readSiteCompletedAt(t, dbURL, "asura", seriesID); got != at.UnixMilli() {
|
||||||
|
t.Fatalf("site_completed_at after unchanged-chapter completed poll = %d, want %d", got, at.UnixMilli())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -6,15 +6,19 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
)
|
)
|
||||||
|
|
||||||
// seriesRead carries the two facts the poll and the acquirer both extract
|
// seriesRead carries the facts the poll and the acquirer both extract from a
|
||||||
// from a series page. Persistence, stamps and scheduling stay with the
|
// series page. Persistence, stamps and scheduling stay with the callers, so
|
||||||
// callers, so the policies that keep the two flows distinct (stamp order,
|
// the policies that keep the two flows distinct (stamp order, rests) are not
|
||||||
// rests) are not swallowed by the module.
|
// swallowed by the module.
|
||||||
type seriesRead struct {
|
type seriesRead struct {
|
||||||
Latest latestChapter
|
Latest latestChapter
|
||||||
HasLatest bool
|
HasLatest bool
|
||||||
Cover string
|
Cover string
|
||||||
HasCover bool
|
HasCover bool
|
||||||
|
// SiteCompleted is whether the Site's own completed value was on the page.
|
||||||
|
// A challenge body and a redesign both read false — an absent hint, never
|
||||||
|
// a claim (issue #168).
|
||||||
|
SiteCompleted bool
|
||||||
// BodyLen is the fetched body's length, surfaced because the no-chapter
|
// BodyLen is the fetched body's length, surfaced because the no-chapter
|
||||||
// log uses it to tell a markup change from a body the size cap cut short.
|
// log uses it to tell a markup change from a body the size cap cut short.
|
||||||
BodyLen int
|
BodyLen int
|
||||||
@@ -35,8 +39,8 @@ var (
|
|||||||
|
|
||||||
// readSeriesPage performs the series-page read the poll and the acquirer have
|
// readSeriesPage performs the series-page read the poll and the acquirer have
|
||||||
// in common: gate the address, choose the route, fetch the page, extract the
|
// in common: gate the address, choose the route, fetch the page, extract the
|
||||||
// Latest Chapter and the Cover address. It persists nothing and stamps
|
// Latest Chapter, the Cover address and the Site's completed value. It
|
||||||
// nothing.
|
// persists nothing and stamps nothing.
|
||||||
//
|
//
|
||||||
// series_url arrives in a client-supplied PUT body (PUT /bookmarks/{key}
|
// series_url arrives in a client-supplied PUT body (PUT /bookmarks/{key}
|
||||||
// accepts any string), so the gate is not an optimisation against burning a
|
// accepts any string), so the gate is not an optimisation against burning a
|
||||||
@@ -75,5 +79,12 @@ func readSeriesPage(ctx context.Context, site, seriesURL string, browser, tls Fe
|
|||||||
}
|
}
|
||||||
latest, hasLatest := latestChapterFrom(site, seriesURL, body)
|
latest, hasLatest := latestChapterFrom(site, seriesURL, body)
|
||||||
cover, hasCover := coverFrom(site, seriesURL, body)
|
cover, hasCover := coverFrom(site, seriesURL, body)
|
||||||
return seriesRead{Latest: latest, HasLatest: hasLatest, Cover: cover, HasCover: hasCover, BodyLen: len(body)}, nil
|
return seriesRead{
|
||||||
|
Latest: latest,
|
||||||
|
HasLatest: hasLatest,
|
||||||
|
Cover: cover,
|
||||||
|
HasCover: hasCover,
|
||||||
|
SiteCompleted: siteCompletedFrom(site, seriesURL, body),
|
||||||
|
BodyLen: len(body),
|
||||||
|
}, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,4 @@
|
|||||||
|
-- When the last successful Poll read saw the Site's own completed value
|
||||||
|
-- (#168): epoch-ms, zero meaning it did not. DEFAULT 0 keeps pre-existing
|
||||||
|
-- rows readable; nothing reads it yet — #170 is the surface.
|
||||||
|
ALTER TABLE series ADD COLUMN site_completed_at bigint NOT NULL DEFAULT 0;
|
||||||
@@ -84,6 +84,10 @@ type Series struct {
|
|||||||
LatestChapter string
|
LatestChapter string
|
||||||
LatestChapterNum *float64 // nil until first captured
|
LatestChapterNum *float64 // nil until first captured
|
||||||
LatestCheckedAt int64 // unix ms; see MarkLatestChecked
|
LatestCheckedAt int64 // unix ms; see MarkLatestChecked
|
||||||
|
// SiteCompletedAt is when the last successful Poll read saw the Site's
|
||||||
|
// own completed value, zero meaning it did not (issue #168). A poller
|
||||||
|
// fact, not reading progress: Upsert never touches it.
|
||||||
|
SiteCompletedAt int64
|
||||||
// LatestRaisedBy is the Reader whose Sighting last raised LatestChapter,
|
// LatestRaisedBy is the Reader whose Sighting last raised LatestChapter,
|
||||||
// and nil when the stored value is a Poll's own finding. It is what lets a
|
// and nil when the stored value is a Poll's own finding. It is what lets a
|
||||||
// Poll that contradicts the value downwards name a Reader instead of
|
// Poll that contradicts the value downwards name a Reader instead of
|
||||||
@@ -240,7 +244,8 @@ const bookmarkColumns = `b.site, b.series_id, s.title, s.series_url, s.cover_add
|
|||||||
// due query. latest_checked_at lives only on series — see MarkLatestChecked
|
// due query. latest_checked_at lives only on series — see MarkLatestChecked
|
||||||
// for why it stays off every client-visible write.
|
// for why it stays off every client-visible write.
|
||||||
const seriesColumns = `s.site, s.series_id, s.title, s.series_url, s.cover, s.cover_address,
|
const seriesColumns = `s.site, s.series_id, s.title, s.series_url, s.cover, s.cover_address,
|
||||||
s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at, s.latest_raised_by`
|
s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at, s.latest_raised_by,
|
||||||
|
s.site_completed_at`
|
||||||
|
|
||||||
const lanePassColumns = `p.site, p.ran_at, p.skip, p.due, p.checked, p.gap_ms, p.clamped,
|
const lanePassColumns = `p.site, p.ran_at, p.skip, p.due, p.checked, p.gap_ms, p.clamped,
|
||||||
p.refused, p.unreachable, p.no_chapter, p.unfetchable, p.errors, p.not_found,
|
p.refused, p.unreachable, p.no_chapter, p.unfetchable, p.errors, p.not_found,
|
||||||
@@ -646,7 +651,7 @@ func scanSeries(scan func(...any) error) (Series, error) {
|
|||||||
if err := scan(
|
if err := scan(
|
||||||
&sr.Site, &sr.SeriesID, &sr.Title, &sr.SeriesURL, &sr.Cover, &sr.CoverAddress,
|
&sr.Site, &sr.SeriesID, &sr.Title, &sr.SeriesURL, &sr.Cover, &sr.CoverAddress,
|
||||||
&sr.Kind, &sr.LatestChapter, &latestChapterNum, &sr.LatestCheckedAt, &latestRaisedBy,
|
&sr.Kind, &sr.LatestChapter, &latestChapterNum, &sr.LatestCheckedAt, &latestRaisedBy,
|
||||||
&sr.Forced, &sr.readerCount,
|
&sr.SiteCompletedAt, &sr.Forced, &sr.readerCount,
|
||||||
); err != nil {
|
); err != nil {
|
||||||
return Series{}, err
|
return Series{}, err
|
||||||
}
|
}
|
||||||
@@ -1356,7 +1361,7 @@ func (s *Store) DueForLatestCheck(site string, cutoffMs, ceilingMs int64) ([]Ser
|
|||||||
AND (s.finished_at = 0 OR s.force_poll_at > s.latest_checked_at)
|
AND (s.finished_at = 0 OR s.force_poll_at > s.latest_checked_at)
|
||||||
GROUP BY s.site, s.series_id, s.title, s.series_url, s.cover,
|
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,
|
s.kind, s.latest_chapter, s.latest_chapter_num, s.latest_checked_at,
|
||||||
s.force_poll_at, s.finished_at
|
s.site_completed_at, s.force_poll_at, s.finished_at
|
||||||
HAVING (COUNT(*) > 1
|
HAVING (COUNT(*) > 1
|
||||||
OR s.latest_sighted_at <= $2::bigint
|
OR s.latest_sighted_at <= $2::bigint
|
||||||
OR s.latest_checked_at <= $3::bigint
|
OR s.latest_checked_at <= $3::bigint
|
||||||
@@ -1491,6 +1496,22 @@ func (s *Store) SetLatestChapter(site, seriesID, label string, num float64) erro
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SetSiteCompletedAt stores when the last successful read saw the Site's
|
||||||
|
// completed value, zero meaning it did not. Series-level, like the Latest
|
||||||
|
// Chapter: it is a fact about the work, not about one Reader's bookmark, and
|
||||||
|
// the bookmark's updated_at is never touched — this is not reading progress
|
||||||
|
// and must not reorder any Reader's list, the same rule SetLatestChapter's
|
||||||
|
// comment states. Touching a missing series is not an error: the row may
|
||||||
|
// have been orphaned, and the caller's read decides what exists.
|
||||||
|
func (s *Store) SetSiteCompletedAt(site, seriesID string, at int64) error {
|
||||||
|
if _, err := s.db.Exec(
|
||||||
|
`UPDATE series SET site_completed_at = $1 WHERE site = $2 AND series_id = $3`,
|
||||||
|
at, site, seriesID); err != nil {
|
||||||
|
return fmt.Errorf("set site completed %s:%s: %w", site, seriesID, err)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// SetSeriesURL stores the owner's repair for a Series' source address
|
// SetSeriesURL stores the owner's repair for a Series' source address
|
||||||
// (issue #151): the one write that lifts the write-once rule documented on
|
// (issue #151): the one write that lifts the write-once rule documented on
|
||||||
// Series.SeriesURL. It is a store, not a verification — the caller has
|
// Series.SeriesURL. It is a store, not a verification — the caller has
|
||||||
|
|||||||
Reference in New Issue
Block a user