public/go-fa-api
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 7 Wiki Activity

fix(favorites): use cursor-based pagination instead of page numbers

Browse Source 
FA's /favorites/{user}/ pagination is cursor-addressed by the fave-ID
of the last item on the previous page (e.g.
/favorites/{user}/1951234825/next), not by sequential integers. The
previous URL builder generated /favorites/{user}/{N}/ for N>=2; FA
interpreted that as a malformed cursor and silently returned page 1,
which caused the Favorites iterator to loop forever and the new
FavoritesPage to report HasNext=true on every call.

Changes:
- urls.Favorites(name) returns the first-page URL; new
  urls.FavoritesCursor(name, cursor) builds /favorites/.../next URLs.
- FavoritesPage now takes a cursor string; empty = first page.
  Returns ListingPage.NextPage as the opaque fave-ID for the next call.
- ListingPage gains NextPage string (decimal page number for
  Gallery/Scraps, fave-ID cursor for Favorites) and drops the Page int
  field that conflated those two notions.
- Client.Favorites iterator now walks cursors internally; StartPage
  is ignored for favorites (documented).
- detectNextPage / nextPageURL now parse the form action so the same
  helper works for both page-number and cursor pagination.
- Added regression test that fails on the infinite-loop bug.
- Example: examples/favorites_page demonstrates cursor walking.
This commit is contained in:
SoXX
2026-06-02 22:44:14 +02:00
parent 8f4767966a
commit 83487e531a
7 changed files with 343 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

133
gallery_page_test.go
View File

@@ -6,14 +6,15 @@ import (
"net/http"
"net/http/httptest"
"strings"
"sync"
"sync/atomic"
"testing"
)
// fakeGalleryPage builds a minimal gallery-page response with two figures.
// hasNext controls whether the "Next" anchor is included so detectNextPage
// flips.
func fakeGalleryPage(startID int, hasNext bool) string {
// nextHref is the next-page URL emitted in the Next form; empty means no
// Next button (last page).
func fakeGalleryPage(startID int, nextHref string) string {
var b strings.Builder
b.WriteString(`<html><body>`)
for i := 0; i < 2; i++ {
@@ -29,8 +30,8 @@ func fakeGalleryPage(startID int, hasNext bool) string {
</figcaption>
</figure>`, id, id, id, id, id)
}
if hasNext {
b.WriteString(`<a class="button standard" href="/gallery/u/2/">Next</a>`)
if nextHref != "" {
fmt.Fprintf(&b, `<form action=%q method="get"><button class="button standard" type="submit">Next</button></form>`, nextHref)
}
b.WriteString(`</body></html>`)
return b.String()
@@ -41,11 +42,11 @@ func TestGalleryPage_HasNextPropagates(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/gallery/u/", func(w http.ResponseWriter, _ *http.Request) {
requests.Add(1)
_, _ = w.Write([]byte(fakeGalleryPage(1000, true)))
_, _ = w.Write([]byte(fakeGalleryPage(1000, "/gallery/u/2/")))
})
mux.HandleFunc("/gallery/u/2/", func(w http.ResponseWriter, _ *http.Request) {
requests.Add(1)
_, _ = w.Write([]byte(fakeGalleryPage(2000, false)))
_, _ = w.Write([]byte(fakeGalleryPage(2000, "")))
})
srv := httptest.NewServer(mux)
defer srv.Close()
@@ -55,19 +56,18 @@ func TestGalleryPage_HasNextPropagates(t *testing.T) {
if err != nil {
t.Fatalf("GalleryPage(1): %v", err)
}
if first.Page != 1 {
t.Errorf("first.Page = %d; want 1", first.Page)
}
if !first.HasNext {
t.Error("first.HasNext = false; want true")
}
if first.NextPage != "2" {
t.Errorf("first.NextPage = %q; want \"2\"", first.NextPage)
}
if len(first.Items) != 2 {
t.Fatalf("first.Items len = %d; want 2", len(first.Items))
}
if first.Items[0].ID != 1000 {
t.Errorf("first.Items[0].ID = %d; want 1000", first.Items[0].ID)
}
// data-tags routed through to the page method too.
if len(first.Items[0].Tags) == 0 || len(first.Items[0].CategorizedTags.Species) == 0 {
t.Errorf("first.Items[0]: tags not populated from data-tags: %+v", first.Items[0])
}
@@ -79,8 +79,8 @@ func TestGalleryPage_HasNextPropagates(t *testing.T) {
if last.HasNext {
t.Error("last.HasNext = true; want false (last page)")
}
if last.Page != 2 {
t.Errorf("last.Page = %d; want 2", last.Page)
if last.NextPage != "" {
t.Errorf("last.NextPage = %q; want empty", last.NextPage)
}
if requests.Load() != 2 {
@@ -88,30 +88,12 @@ func TestGalleryPage_HasNextPropagates(t *testing.T) {
}
}
func TestGalleryPage_ZeroPageDefaultsToOne(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/gallery/u/", func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(fakeGalleryPage(1, false)))
})
srv := httptest.NewServer(mux)
defer srv.Close()
client := newE2EClient(t, srv)
page, err := client.GalleryPage(context.Background(), "u", 0)
if err != nil {
t.Fatalf("GalleryPage(0): %v", err)
}
if page.Page != 1 {
t.Errorf("page.Page = %d; want 1 (zero should normalise)", page.Page)
}
}
func TestScrapsPage_HitsScrapsRoute(t *testing.T) {
var gotPath string
mux := http.NewServeMux()
mux.HandleFunc("/scraps/u/", func(w http.ResponseWriter, r *http.Request) {
gotPath = r.URL.Path
_, _ = w.Write([]byte(fakeGalleryPage(1, false)))
_, _ = w.Write([]byte(fakeGalleryPage(1, "")))
})
srv := httptest.NewServer(mux)
defer srv.Close()
@@ -125,25 +107,92 @@ func TestScrapsPage_HitsScrapsRoute(t *testing.T) {
}
}
func TestFavoritesPage_HitsFavoritesRoute(t *testing.T) {
var gotPath string
func TestFavoritesPage_CursorChain(t *testing.T) {
var requests []string
var mu sync.Mutex
record := func(p string) {
mu.Lock()
requests = append(requests, p)
mu.Unlock()
}
mux := http.NewServeMux()
mux.HandleFunc("/favorites/u/", func(w http.ResponseWriter, r *http.Request) {
gotPath = r.URL.Path
_, _ = w.Write([]byte(fakeGalleryPage(1, true)))
record(r.URL.Path)
_, _ = w.Write([]byte(fakeGalleryPage(1000, "/favorites/u/9999/next")))
})
mux.HandleFunc("/favorites/u/9999/next", func(w http.ResponseWriter, r *http.Request) {
record(r.URL.Path)
_, _ = w.Write([]byte(fakeGalleryPage(2000, "")))
})
srv := httptest.NewServer(mux)
defer srv.Close()
client := newE2EClient(t, srv)
p, err := client.FavoritesPage(context.Background(), "u", 1)
first, err := client.FavoritesPage(context.Background(), "u", "")
if err != nil {
t.Fatalf("FavoritesPage: %v", err)
t.Fatalf("FavoritesPage(first): %v", err)
}
if gotPath != "/favorites/u/" {
t.Errorf("gotPath = %q; want /favorites/u/", gotPath)
if !first.HasNext {
t.Fatal("first.HasNext = false; want true")
}
if !p.HasNext {
t.Error("p.HasNext = false; want true")
if first.NextPage != "9999" {
t.Errorf("first.NextPage = %q; want \"9999\" (cursor)", first.NextPage)
}
last, err := client.FavoritesPage(context.Background(), "u", first.NextPage)
if err != nil {
t.Fatalf("FavoritesPage(cursor): %v", err)
}
if last.HasNext {
t.Error("last.HasNext = true; want false")
}
if last.NextPage != "" {
t.Errorf("last.NextPage = %q; want empty", last.NextPage)
}
want := []string{"/favorites/u/", "/favorites/u/9999/next"}
mu.Lock()
defer mu.Unlock()
if len(requests) != len(want) {
t.Fatalf("requests = %v; want %v", requests, want)
}
for i, w := range want {
if requests[i] != w {
t.Errorf("requests[%d] = %q; want %q", i, requests[i], w)
}
}
}
// TestFavorites_IteratorTerminates guards against the cursor-loop
// regression that brought us here: with sequential page numbers, the
// Favorites iterator never terminated because FA fell back to page 1
// for every fake-numbered cursor.
func TestFavorites_IteratorTerminates(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/favorites/u/", func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(fakeGalleryPage(1, "/favorites/u/42/next")))
})
mux.HandleFunc("/favorites/u/42/next", func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(fakeGalleryPage(3, "")))
})
srv := httptest.NewServer(mux)
defer srv.Close()
client := newE2EClient(t, srv)
count := 0
for sub, err := range client.Favorites(context.Background(), "u", ListOptions{}) {
if err != nil {
t.Fatalf("Favorites: %v", err)
}
if sub == nil {
t.Fatal("nil sub")
}
count++
if count > 10 {
t.Fatalf("iterator did not terminate; count > 10")
}
}
if count != 4 {
t.Errorf("count = %d; want 4 (2 per page * 2 pages)", count)
}
}