From 569531f160af02aa0dbb0371ca12a3a69e5c8bf4 Mon Sep 17 00:00:00 2001 From: M3ML1NE Date: Tue, 11 Aug 2026 10:50:30 +0000 Subject: [PATCH] fix(client): refactor pagination loop to rely on Link header and pageInfo instead of page item count - Ensure pagination continues when Link header rel="next" (REST) or hasNextPage (GraphQL) exists, even if items returned < perPage or 0 items. - Added comprehensive unit tests covering sparse pages, empty intermediate pages, and max items capping. --- go.mod | 3 + pkg/client/pagination.go | 115 ++++++++++++++++++++++++++++++++++ pkg/client/pagination_test.go | 104 ++++++++++++++++++++++++++++++ 3 files changed, 222 insertions(+) create mode 100644 go.mod create mode 100644 pkg/client/pagination.go create mode 100644 pkg/client/pagination_test.go diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..bc94db8 --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module github.com/EncarnacionP/cli + +go 1.22.0 diff --git a/pkg/client/pagination.go b/pkg/client/pagination.go new file mode 100644 index 0000000..5da91a2 --- /dev/null +++ b/pkg/client/pagination.go @@ -0,0 +1,115 @@ +package client + +import ( + "context" + "net/http" + "regexp" + "strings" +) + +var linkNextRegex = regexp.MustCompile(`<([^>]+)>;\s*rel="next"`) + +// ItemFetcher defines a function that fetches a single page of items given a page number and page size. +// It returns the list of items for that page, the HTTP response, and any error. +type ItemFetcher[T any] func(ctx context.Context, page int, perPage int) ([]T, *http.Response, error) + +// PaginationOptions controls the behavior of the pagination loop. +type PaginationOptions struct { + MaxItems int // Optional limit on total items to accumulate (0 or negative means unlimited) +} + +// FetchAllPages iterates through paginated endpoints according to HTTP Link header rel="next". +// It continues fetching as long as rel="next" is present in the Link header (or until MaxItems is reached), +// regardless of whether individual pages return fewer items than perPage or even 0 items. +func FetchAllPages[T any](ctx context.Context, fetcher ItemFetcher[T], perPage int, opts ...PaginationOptions) ([]T, error) { + var maxItems int + if len(opts) > 0 { + maxItems = opts[0].MaxItems + } + + var allItems []T + page := 1 + + for { + items, resp, err := fetcher(ctx, page, perPage) + if err != nil { + return nil, err + } + + allItems = append(allItems, items...) + + if maxItems > 0 && len(allItems) >= maxItems { + allItems = allItems[:maxItems] + break + } + + if resp == nil { + break + } + + linkHeader := resp.Header.Get("Link") + if !HasNextPageLink(linkHeader) { + break + } + + page++ + } + + return allItems, nil +} + +// HasNextPageLink checks if the provided HTTP Link header string contains rel="next". +func HasNextPageLink(linkHeader string) bool { + if linkHeader == "" { + return false + } + parts := strings.Split(linkHeader, ",") + for _, part := range parts { + if linkNextRegex.MatchString(strings.TrimSpace(part)) { + return true + } + } + return false +} + +// GraphQLPageInfo represents GraphQL pagination metadata. +type GraphQLPageInfo struct { + HasNextPage bool + EndCursor string +} + +// GraphQLFetcher defines a function that fetches a page using GraphQL. +type GraphQLFetcher[T any] func(ctx context.Context, cursor string, perPage int) ([]T, GraphQLPageInfo, error) + +// FetchAllPagesGraphQL iterates through GraphQL paginated queries according to pageInfo.HasNextPage. +func FetchAllPagesGraphQL[T any](ctx context.Context, fetcher GraphQLFetcher[T], perPage int, opts ...PaginationOptions) ([]T, error) { + var maxItems int + if len(opts) > 0 { + maxItems = opts[0].MaxItems + } + + var allItems []T + cursor := "" + + for { + items, pageInfo, err := fetcher(ctx, cursor, perPage) + if err != nil { + return nil, err + } + + allItems = append(allItems, items...) + + if maxItems > 0 && len(allItems) >= maxItems { + allItems = allItems[:maxItems] + break + } + + if !pageInfo.HasNextPage { + break + } + + cursor = pageInfo.EndCursor + } + + return allItems, nil +} diff --git a/pkg/client/pagination_test.go b/pkg/client/pagination_test.go new file mode 100644 index 0000000..0e39fad --- /dev/null +++ b/pkg/client/pagination_test.go @@ -0,0 +1,104 @@ +package client_test + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/EncarnacionP/cli/pkg/client" +) + +type Item struct { + ID int + Name string +} + +func TestFetchAllPages_SparseAndEmptyPages(t *testing.T) { + // Scenario: + // Page 1 (per_page=5): returns 2 items (< per_page!), but has rel="next" Link header pointing to page 2. + // Page 2 (per_page=5): returns 0 items, but has rel="next" Link header pointing to page 3. + // Page 3 (per_page=5): returns 3 items, no rel="next" Link header (final page). + // Total items returned across all pages should be 2 + 0 + 3 = 5 items over 3 requests. + + requestsMade := 0 + + fetcher := func(ctx context.Context, page int, perPage int) ([]Item, *http.Response, error) { + requestsMade++ + resp := &http.Response{Header: make(http.Header)} + + switch page { + case 1: + resp.Header.Set("Link", `; rel="next", ; rel="last"`) + return []Item{{ID: 1, Name: "Item 1"}, {ID: 2, Name: "Item 2"}}, resp, nil + case 2: + resp.Header.Set("Link", `; rel="next"`) + return []Item{}, resp, nil + case 3: + // Final page, no "next" link + resp.Header.Set("Link", `; rel="first"`) + return []Item{{ID: 3, Name: "Item 3"}, {ID: 4, Name: "Item 4"}, {ID: 5, Name: "Item 5"}}, resp, nil + default: + return nil, resp, fmt.Errorf("unexpected page %d", page) + } + } + + items, err := client.FetchAllPages(context.Background(), fetcher, 5) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if requestsMade != 3 { + t.Errorf("expected 3 requests, got %d", requestsMade) + } + + if len(items) != 5 { + t.Errorf("expected 5 items accumulated, got %d", len(items)) + } +} + +func TestFetchAllPages_MaxItemsLimit(t *testing.T) { + fetcher := func(ctx context.Context, page int, perPage int) ([]Item, *http.Response, error) { + resp := &http.Response{Header: make(http.Header)} + resp.Header.Set("Link", fmt.Sprintf(`; rel="next"`, page+1, perPage)) + return []Item{{ID: page*10 + 1}, {ID: page*10 + 2}}, resp, nil + } + + items, err := client.FetchAllPages(context.Background(), fetcher, 2, client.PaginationOptions{MaxItems: 3}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(items) != 3 { + t.Errorf("expected 3 items capped by MaxItems, got %d", len(items)) + } +} + +func TestFetchAllPagesGraphQL_SparsePage(t *testing.T) { + requestsMade := 0 + + fetcher := func(ctx context.Context, cursor string, perPage int) ([]Item, client.GraphQLPageInfo, error) { + requestsMade++ + switch cursor { + case "": + return []Item{{ID: 101}}, client.GraphQLPageInfo{HasNextPage: true, EndCursor: "cursor_1"}, nil + case "cursor_1": + return []Item{{ID: 102}, {ID: 103}}, client.GraphQLPageInfo{HasNextPage: false, EndCursor: "cursor_2"}, nil + default: + return nil, client.GraphQLPageInfo{}, fmt.Errorf("unexpected cursor %s", cursor) + } + } + + items, err := client.FetchAllPagesGraphQL(context.Background(), fetcher, 5) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if requestsMade != 2 { + t.Errorf("expected 2 requests, got %d", requestsMade) + } + + if len(items) != 3 { + t.Errorf("expected 3 items, got %d", len(items)) + } +}