diff --git a/backend/cover_test.go b/backend/cover_test.go index bf9b44f..49cc4a4 100644 --- a/backend/cover_test.go +++ b/backend/cover_test.go @@ -10,7 +10,7 @@ import ( ) // fakeCovers stands in for the headless browser. It counts calls so the test -// can prove the cache spares the browser a second navigation. +// can prove the store spares the browser after the first navigation. type fakeCovers struct { body []byte contentType string @@ -44,33 +44,57 @@ func getCover(t *testing.T, srv http.Handler, path string, cookie *http.Cookie) // kagane serves its covers behind a Cloudflare challenge and with // cross-origin-resource-policy: same-origin, so the UI can only show one by // re-serving the bytes from its own origin. -func TestKaganeCoverProxiesAndCaches(t *testing.T) { +func TestKaganeCoverPersistsAndReusesStoredBytes(t *testing.T) { cf := &fakeCovers{body: []byte("\x00webp-bytes"), contentType: "image/webp"} cfg := testConfig() cfg.Covers = cf srv, st := newWebTestServer(t, cfg) cookie := sessionCookie(t, st) - for i := range 2 { - rr := getCover(t, srv, "/img/kagane/"+testCoverID, cookie) - if rr.Code != http.StatusOK { - t.Fatalf("request %d: status = %d, want 200", i, rr.Code) - } - if got := rr.Body.String(); got != string(cf.body) { - t.Fatalf("request %d: body = %q, want %q", i, got, cf.body) - } - if got := rr.Header().Get("Content-Type"); got != "image/webp" { - t.Fatalf("request %d: Content-Type = %q, want image/webp", i, got) - } + rr := getCover(t, srv, "/img/kagane/"+testCoverID, cookie) + if rr.Code != http.StatusOK { + t.Fatalf("first request: status = %d, want 200", rr.Code) + } + if got := rr.Body.String(); got != string(cf.body) { + t.Fatalf("first request: body = %q, want %q", got, cf.body) + } + + // A new Handler has no process-local state from the first request. The same + // store must still answer without navigating the browser again. + srv = newRouter(st, cfg) + rr = getCover(t, srv, "/img/kagane/"+testCoverID, cookie) + if rr.Code != http.StatusOK { + t.Fatalf("stored request: status = %d, want 200", rr.Code) + } + if got := rr.Body.String(); got != string(cf.body) { + t.Fatalf("stored request: body = %q, want %q", got, cf.body) } if got := cf.calls.Load(); got != 1 { - t.Fatalf("fetcher called %d times, want 1 — the second read must come from the cache", got) + t.Fatalf("fetcher called %d times, want 1 — stored bytes must survive a new handler", got) } if got := cf.lastID.Load(); got != testCoverID { t.Fatalf("fetched image id = %v, want %s", got, testCoverID) } } +func TestKaganeCoverServesStoredBytesWithoutBrowser(t *testing.T) { + cf := &fakeCovers{err: errors.New("browser must not be called")} + cfg := testConfig() + cfg.Covers = cf + srv, st := newWebTestServer(t, cfg) + if err := st.PutKaganeCover(testCoverID, []byte("already-stored"), "image/png"); err != nil { + t.Fatalf("PutKaganeCover: %v", err) + } + + rr := getCover(t, srv, "/img/kagane/"+testCoverID, sessionCookie(t, st)) + if rr.Code != http.StatusOK || rr.Body.String() != "already-stored" { + t.Fatalf("stored request = (%d, %q), want (200, already-stored)", rr.Code, rr.Body.String()) + } + if got := cf.calls.Load(); got != 0 { + t.Fatalf("fetcher called %d times for a stored cover, want 0", got) + } +} + // The proxy reaches a headless browser, so it is not open to the internet. func TestKaganeCoverRequiresSession(t *testing.T) { cf := &fakeCovers{body: []byte("x"), contentType: "image/webp"} @@ -123,6 +147,11 @@ func TestKaganeCoverRejectsBadInput(t *testing.T) { if rr.Code != http.StatusNotFound { t.Fatalf("status = %d, want 404", rr.Code) } + if _, _, ok, err := st.GetKaganeCover(testCoverID); err != nil { + t.Fatalf("GetKaganeCover after rejection: %v", err) + } else if ok { + t.Fatal("rejected cover was persisted") + } }) } } diff --git a/backend/internal/store/migrations/0007_covers.sql b/backend/internal/store/migrations/0007_covers.sql new file mode 100644 index 0000000..92a8063 --- /dev/null +++ b/backend/internal/store/migrations/0007_covers.sql @@ -0,0 +1,8 @@ +-- Kagane cover bytes belong in their own table so image blobs never enter the +-- series queries that drive the latest-chapter poller. +CREATE TABLE covers ( + image_id text PRIMARY KEY, + body bytea NOT NULL, + content_type text NOT NULL, + fetched_at timestamptz NOT NULL DEFAULT now() +); diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index d60e2ec..9c9e918 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -529,6 +529,37 @@ func scanSeries(scan func(...any) error) (Series, error) { // Close releases the underlying database handle. func (s *Store) Close() error { return s.db.Close() } +// GetKaganeCover returns one persisted cover. Missing covers are reported with +// ok=false rather than as an error so the web handler can fetch them once. +func (s *Store) GetKaganeCover(imageID string) ([]byte, string, bool, error) { + var ( + body []byte + contentType string + ) + err := s.db.QueryRow( + `SELECT body, content_type FROM covers WHERE image_id = $1`, imageID, + ).Scan(&body, &contentType) + if errors.Is(err, sql.ErrNoRows) { + return nil, "", false, nil + } + if err != nil { + return nil, "", false, fmt.Errorf("get kagane cover %q: %w", imageID, err) + } + return body, contentType, true, nil +} + +// PutKaganeCover persists one fetched cover. Image ids are immutable, so a +// later fetch cannot replace the bytes already made durable. +func (s *Store) PutKaganeCover(imageID string, body []byte, contentType string) error { + if _, err := s.db.Exec(` + INSERT INTO covers (image_id, body, content_type) + VALUES ($1, $2, $3) + ON CONFLICT (image_id) DO NOTHING`, imageID, body, contentType); err != nil { + return fmt.Errorf("put kagane cover %q: %w", imageID, err) + } + return nil +} + // 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). diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 4aec053..e22a37e 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -1231,3 +1231,30 @@ func TestTwoReadersShareOneSeriesWithIndependentProgress(t *testing.T) { t.Fatalf("due after one Reader left = %+v, want the series still polled", due) } } +func TestKaganeCoverPersistsAcrossReopen(t *testing.T) { + url := pgtest.URL(t) + first, err := Open(url, testOwner) + if err != nil { + t.Fatalf("Open: %v", err) + } + body := []byte("stored-cover") + if err := first.PutKaganeCover("019fe11a-84c3-7fc3-a84b-88787374b617", body, "image/webp"); err != nil { + t.Fatalf("PutKaganeCover: %v", err) + } + if err := first.Close(); err != nil { + t.Fatalf("close first store: %v", err) + } + + second, err := Open(url, testOwner) + if err != nil { + t.Fatalf("reopen: %v", err) + } + defer second.Close() + got, contentType, ok, err := second.GetKaganeCover("019fe11a-84c3-7fc3-a84b-88787374b617") + if err != nil { + t.Fatalf("GetKaganeCover: %v", err) + } + if !ok || !bytes.Equal(got, body) || contentType != "image/webp" { + t.Fatalf("stored cover = (%q, %q, %v), want (%q, image/webp, true)", got, contentType, ok, body) + } +} diff --git a/backend/internal/web/cover.go b/backend/internal/web/cover.go index 18007cc..e33cf41 100644 --- a/backend/internal/web/cover.go +++ b/backend/internal/web/cover.go @@ -5,7 +5,6 @@ import ( "log" "net/http" "regexp" - "sync" "time" ) @@ -39,40 +38,6 @@ var coverTypes = map[string]bool{ // holding a connection open. const coverTimeout = 20 * time.Second -// coverCacheMax caps the in-memory cover cache. Covers are immutable per image -// id and a library holds tens of series, so this is a ceiling that is never -// reached in practice; reaching it clears the map rather than evicting by age. -// -// ponytail: flush-on-full, not LRU. Swap it for an LRU if a library ever grows -// past this and the flush starts costing refetches. -const coverCacheMax = 500 - -type cachedCover struct { - body []byte - contentType string -} - -type coverCache struct { - mu sync.Mutex - m map[string]cachedCover -} - -func (c *coverCache) get(id string) (cachedCover, bool) { - c.mu.Lock() - defer c.mu.Unlock() - v, ok := c.m[id] - return v, ok -} - -func (c *coverCache) put(id string, v cachedCover) { - c.mu.Lock() - defer c.mu.Unlock() - if c.m == nil || len(c.m) >= coverCacheMax { - c.m = make(map[string]cachedCover, coverCacheMax) - } - c.m[id] = v -} - // kaganeCover serves a kagane cover from the backend's own origin. // // kagane answers image requests with a Cloudflare challenge and @@ -81,23 +46,22 @@ func (c *coverCache) put(id string, v cachedCover) { // (verified 2026-08-08). Fetching it through the headless browser that already // clears the challenge, and re-serving it here, is what puts the bytes on an // origin the page may load from. -// -// ponytail: covers are fetched on first view, one browser navigation at a time -// behind the fetcher's mutex, so a first load of a large kagane library -// trickles in over a few seconds. The cache makes it a one-off. Prefetching -// during the poll cycle is the upgrade if that ever grates. func (h *Handler) kaganeCover(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") if !coverIDRe.MatchString(id) { http.NotFound(w, r) return } - if h.covers == nil { - http.NotFound(w, r) + if body, contentType, ok, err := h.store.GetKaganeCover(id); err != nil { + log.Printf("read kagane cover %s: %v", id, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } else if ok { + writeCover(w, body, contentType) return } - if v, ok := h.coverCache.get(id); ok { - writeCover(w, v) + if h.covers == nil { + http.NotFound(w, r) return } @@ -114,16 +78,18 @@ func (h *Handler) kaganeCover(w http.ResponseWriter, r *http.Request) { http.NotFound(w, r) return } - - v := cachedCover{body: body, contentType: contentType} - h.coverCache.put(id, v) - writeCover(w, v) + if err := h.store.PutKaganeCover(id, body, contentType); err != nil { + log.Printf("persist kagane cover %s: %v", id, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + writeCover(w, body, contentType) } // writeCover sends the bytes with a long cache life: an image id names one // immutable rendering, so a client that has it never needs to ask again. -func writeCover(w http.ResponseWriter, v cachedCover) { - w.Header().Set("Content-Type", v.contentType) +func writeCover(w http.ResponseWriter, body []byte, contentType string) { + w.Header().Set("Content-Type", contentType) w.Header().Set("Cache-Control", "private, max-age=604800, immutable") - w.Write(v.body) + w.Write(body) } diff --git a/backend/internal/web/web.go b/backend/internal/web/web.go index 872cf6a..4dd0470 100644 --- a/backend/internal/web/web.go +++ b/backend/internal/web/web.go @@ -52,8 +52,7 @@ type Handler struct { httpClient *http.Client // covers proxies kagane cover images, which no browser can load directly. // Nil disables the endpoint — see CoverFetcher. - covers CoverFetcher - coverCache coverCache + covers CoverFetcher } // listView is what every list-rendering template receives.