Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions workspace/pkg/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,20 @@ func NewClient(baseURL string, httpClient *http.Client) *Client {

func (c *Client) GetIssues(owner, repo string, limit int) ([]Issue, error) {
var allIssues []Issue
nextURL := fmt.Sprintf("%s/repos/%s/%s/issues?per_page=%d", c.BaseURL, owner, repo, limit)

// Per-page size. A non-positive limit means "no limit"; use a sane page
// size in that case. GitHub caps per_page at 100, so clamp to avoid
// unnecessary round trips (a limit of 5000 would otherwise still fetch
// 100/page but iterate far more pages than needed).
perPage := limit
if perPage <= 0 {
perPage = 100
}
if perPage > 100 {
perPage = 100
}

nextURL := fmt.Sprintf("%s/repos/%s/%s/issues?per_page=%d", c.BaseURL, owner, repo, perPage)

for nextURL != "" {
req, err := http.NewRequest("GET", nextURL, nil)
Expand All @@ -49,19 +62,31 @@ func (c *Client) GetIssues(owner, repo string, limit int) ([]Issue, error) {

var issues []Issue
err = json.NewDecoder(resp.Body).Decode(&issues)
nextLink := resp.Header.Get("Link")
resp.Body.Close()
if err != nil {
return nil, err
}

allIssues = append(allIssues, issues...)

if len(allIssues) >= limit {
// Stop when the user's limit is reached.
if limit > 0 && len(allIssues) >= limit {
allIssues = allIssues[:limit]
break
}

nextURL = getNextPageURL(resp.Header.Get("Link"))
// Safety fallback: an empty page must stop pagination even if a next
// link is present, preventing an infinite loop against a misbehaving
// API that keeps returning empty pages with a next relation.
if len(issues) == 0 {
break
}

// Continue strictly based on the Link header's rel="next" relation,
// never on the number of items returned (a sparse page can carry a
// next link with fewer items than requested).
nextURL = getNextPageURL(nextLink)
}

return allIssues, nil
Expand Down
38 changes: 31 additions & 7 deletions workspace/pkg/github/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,22 +69,19 @@ func TestGetIssues_SparsePagination(t *testing.T) {
}
}

func TestGetIssues_LimitAndZeroItems(t *testing.T) {
func TestGetIssues_Limit(t *testing.T) {
requestCount := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requestCount++
w.Header().Set("Content-Type", "application/json")
if requestCount == 1 {
w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=2&per_page=3>; rel="next"`)
json.NewEncoder(w).Encode([]Issue{})
} else if requestCount == 2 {
issues := []Issue{
{ID: 1, Title: "Issue 1"},
{ID: 2, Title: "Issue 2"},
{ID: 3, Title: "Issue 3"},
{ID: 4, Title: "Issue 4"},
}
w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=3&per_page=3>; rel="next"`)
w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=2&per_page=3>; rel="next"`)
json.NewEncoder(w).Encode(issues)
} else {
t.Errorf("unexpected request count: %d", requestCount)
Expand All @@ -98,11 +95,38 @@ func TestGetIssues_LimitAndZeroItems(t *testing.T) {
t.Fatalf("unexpected error: %v", err)
}

if requestCount != 2 {
t.Errorf("expected exactly 2 requests, got %d", requestCount)
if requestCount != 1 {
t.Errorf("expected exactly 1 request (limit reached on first page), got %d", requestCount)
}

if len(issues) != 3 {
t.Errorf("expected 3 issues, got %d", len(issues))
}
}

func TestGetIssues_EmptyPageStops(t *testing.T) {
requestCount := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requestCount++
w.Header().Set("Content-Type", "application/json")
// Empty page with a next link must NOT be followed — the empty-list
// fallback stops pagination to prevent infinite loops.
w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=2&per_page=3>; rel="next"`)
json.NewEncoder(w).Encode([]Issue{})
}))
defer server.Close()

client := NewClient(server.URL, nil)
issues, err := client.GetIssues("owner", "repo", 0)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if requestCount != 1 {
t.Errorf("expected exactly 1 request (empty page stops pagination), got %d", requestCount)
}

if len(issues) != 0 {
t.Errorf("expected 0 issues, got %d", len(issues))
}
}