Skip to content

Commit 0a33c1e

Browse files
Add pagination support to GitHub API client (#33)
Fix issue where only 30 followed users were synced instead of all 102. GitHub API returns 30 results per page by default, but we were only fetching the first page. Changes: - Add getPaginated() helper that fetches all pages (100 items/page) - Update all list methods to use pagination: * GetFollowedUsers * GetFollowedUsersByUsername * GetStarredRepos * GetStarredReposByUsername * GetOwnedRepos * GetOwnedReposByUsername * GetRecentEvents * GetReceivedEvents - Add comprehensive pagination tests This ensures users like 'expede' who were beyond the first 30 results will now appear in the feed. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent c910e1c commit 0a33c1e

File tree

2 files changed

+211
-8
lines changed

2 files changed

+211
-8
lines changed

github/client.go

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import (
88
"io"
99
"log/slog"
1010
"net/http"
11+
"reflect"
1112
"strconv"
13+
"strings"
1214
"sync"
1315
"time"
1416
)
@@ -281,78 +283,140 @@ func (c *Client) parseRateLimitHeaders(resp *http.Response) {
281283
}
282284
}
283285

286+
// getPaginated fetches all pages of results for a given path.
287+
// It handles GitHub's pagination by requesting 100 items per page until
288+
// no more results are returned.
289+
func (c *Client) getPaginated(ctx context.Context, basePath string, result any) error {
290+
// Use reflection to work with any slice type
291+
resultVal := reflect.ValueOf(result)
292+
if resultVal.Kind() != reflect.Ptr || resultVal.Elem().Kind() != reflect.Slice {
293+
return fmt.Errorf("result must be a pointer to a slice")
294+
}
295+
296+
sliceVal := resultVal.Elem()
297+
298+
page := 1
299+
perPage := 100 // GitHub's maximum per_page value
300+
301+
for {
302+
// Build path with pagination parameters
303+
separator := "?"
304+
if strings.Contains(basePath, "?") {
305+
separator = "&"
306+
}
307+
path := fmt.Sprintf("%s%spage=%d&per_page=%d", basePath, separator, page, perPage)
308+
309+
// Create a new slice to hold this page's results
310+
pageResult := reflect.New(sliceVal.Type()).Interface()
311+
312+
if err := c.get(ctx, path, pageResult); err != nil {
313+
return err
314+
}
315+
316+
// Get the slice value from the pointer
317+
pageSlice := reflect.ValueOf(pageResult).Elem()
318+
319+
// If we got no results, we're done
320+
if pageSlice.Len() == 0 {
321+
break
322+
}
323+
324+
// Append this page's results to the total
325+
sliceVal = reflect.AppendSlice(sliceVal, pageSlice)
326+
327+
// If we got fewer results than per_page, this is the last page
328+
if pageSlice.Len() < perPage {
329+
break
330+
}
331+
332+
page++
333+
}
334+
335+
// Set the final result
336+
resultVal.Elem().Set(sliceVal)
337+
return nil
338+
}
339+
284340
// GetFollowedUsers returns the users that the authenticated user follows.
341+
// This method automatically handles pagination to fetch all followed users.
285342
func (c *Client) GetFollowedUsers(ctx context.Context) ([]User, error) {
286343
var users []User
287-
if err := c.get(ctx, "/user/following", &users); err != nil {
344+
if err := c.getPaginated(ctx, "/user/following", &users); err != nil {
288345
return nil, fmt.Errorf("fetching followed users: %w", err)
289346
}
290347
return users, nil
291348
}
292349

293350
// GetFollowedUsersByUsername returns the users that a specific user follows.
351+
// This method automatically handles pagination to fetch all followed users.
294352
func (c *Client) GetFollowedUsersByUsername(ctx context.Context, username string) ([]User, error) {
295353
var users []User
296354
path := fmt.Sprintf("/users/%s/following", username)
297-
if err := c.get(ctx, path, &users); err != nil {
355+
if err := c.getPaginated(ctx, path, &users); err != nil {
298356
return nil, fmt.Errorf("fetching users followed by %s: %w", username, err)
299357
}
300358
return users, nil
301359
}
302360

303361
// GetStarredRepos returns repositories starred by the authenticated user.
362+
// This method automatically handles pagination to fetch all starred repos.
304363
func (c *Client) GetStarredRepos(ctx context.Context) ([]Repository, error) {
305364
var repos []Repository
306-
if err := c.get(ctx, "/user/starred", &repos); err != nil {
365+
if err := c.getPaginated(ctx, "/user/starred", &repos); err != nil {
307366
return nil, fmt.Errorf("fetching starred repos: %w", err)
308367
}
309368
return repos, nil
310369
}
311370

312371
// GetStarredReposByUsername returns repositories starred by a specific user.
372+
// This method automatically handles pagination to fetch all starred repos.
313373
func (c *Client) GetStarredReposByUsername(ctx context.Context, username string) ([]Repository, error) {
314374
var repos []Repository
315375
path := fmt.Sprintf("/users/%s/starred", username)
316-
if err := c.get(ctx, path, &repos); err != nil {
376+
if err := c.getPaginated(ctx, path, &repos); err != nil {
317377
return nil, fmt.Errorf("fetching repos starred by %s: %w", username, err)
318378
}
319379
return repos, nil
320380
}
321381

322382
// GetOwnedRepos returns repositories owned by the authenticated user.
383+
// This method automatically handles pagination to fetch all owned repos.
323384
func (c *Client) GetOwnedRepos(ctx context.Context) ([]Repository, error) {
324385
var repos []Repository
325-
if err := c.get(ctx, "/user/repos?type=owner", &repos); err != nil {
386+
if err := c.getPaginated(ctx, "/user/repos?type=owner", &repos); err != nil {
326387
return nil, fmt.Errorf("fetching owned repos: %w", err)
327388
}
328389
return repos, nil
329390
}
330391

331392
// GetOwnedReposByUsername returns repositories owned by a specific user.
393+
// This method automatically handles pagination to fetch all owned repos.
332394
func (c *Client) GetOwnedReposByUsername(ctx context.Context, username string) ([]Repository, error) {
333395
var repos []Repository
334396
path := fmt.Sprintf("/users/%s/repos?type=owner", username)
335-
if err := c.get(ctx, path, &repos); err != nil {
397+
if err := c.getPaginated(ctx, path, &repos); err != nil {
336398
return nil, fmt.Errorf("fetching repos owned by %s: %w", username, err)
337399
}
338400
return repos, nil
339401
}
340402

341403
// GetRecentEvents returns recent events for the authenticated user.
404+
// This method automatically handles pagination to fetch all recent events.
342405
func (c *Client) GetRecentEvents(ctx context.Context, username string) ([]Event, error) {
343406
var events []Event
344407
path := fmt.Sprintf("/users/%s/events", username)
345-
if err := c.get(ctx, path, &events); err != nil {
408+
if err := c.getPaginated(ctx, path, &events); err != nil {
346409
return nil, fmt.Errorf("fetching events for %s: %w", username, err)
347410
}
348411
return events, nil
349412
}
350413

351414
// GetReceivedEvents returns events received by a user (their feed).
415+
// This method automatically handles pagination to fetch all received events.
352416
func (c *Client) GetReceivedEvents(ctx context.Context, username string) ([]Event, error) {
353417
var events []Event
354418
path := fmt.Sprintf("/users/%s/received_events", username)
355-
if err := c.get(ctx, path, &events); err != nil {
419+
if err := c.getPaginated(ctx, path, &events); err != nil {
356420
return nil, fmt.Errorf("fetching received events for %s: %w", username, err)
357421
}
358422
return events, nil

github/client_test.go

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"encoding/json"
7+
"fmt"
78
"log/slog"
89
"net/http"
910
"net/http/httptest"
@@ -549,3 +550,141 @@ func TestWithLogger(t *testing.T) {
549550
t.Error("expected custom logger to be set")
550551
}
551552
}
553+
554+
func TestGetFollowedUsersPagination(t *testing.T) {
555+
// Create test users across multiple pages
556+
// Page 1: 100 users, Page 2: 100 users, Page 3: 2 users (total: 202)
557+
page1Users := make([]User, 100)
558+
for i := 0; i < 100; i++ {
559+
page1Users[i] = User{Login: fmt.Sprintf("user%d", i), ID: int64(i)}
560+
}
561+
562+
page2Users := make([]User, 100)
563+
for i := 0; i < 100; i++ {
564+
page2Users[i] = User{Login: fmt.Sprintf("user%d", i+100), ID: int64(i + 100)}
565+
}
566+
567+
page3Users := []User{
568+
{Login: "user200", ID: 200},
569+
{Login: "user201", ID: 201},
570+
}
571+
572+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
573+
// Check path
574+
if r.URL.Path != "/user/following" {
575+
t.Errorf("unexpected path: %s", r.URL.Path)
576+
}
577+
578+
// Parse query params
579+
query := r.URL.Query()
580+
page := query.Get("page")
581+
perPage := query.Get("per_page")
582+
583+
// Verify per_page is set to 100
584+
if perPage != "100" {
585+
t.Errorf("expected per_page=100, got %s", perPage)
586+
}
587+
588+
w.Header().Set("Content-Type", "application/json")
589+
590+
// Return appropriate page
591+
switch page {
592+
case "1":
593+
if err := json.NewEncoder(w).Encode(page1Users); err != nil {
594+
t.Fatalf("encoding page 1: %v", err)
595+
}
596+
case "2":
597+
if err := json.NewEncoder(w).Encode(page2Users); err != nil {
598+
t.Fatalf("encoding page 2: %v", err)
599+
}
600+
case "3":
601+
if err := json.NewEncoder(w).Encode(page3Users); err != nil {
602+
t.Fatalf("encoding page 3: %v", err)
603+
}
604+
default:
605+
t.Errorf("unexpected page number: %s", page)
606+
}
607+
}))
608+
defer server.Close()
609+
610+
c := NewClient("test-token", WithBaseURL(server.URL))
611+
result, err := c.GetFollowedUsers(context.Background())
612+
if err != nil {
613+
t.Fatalf("GetFollowedUsers() error: %v", err)
614+
}
615+
616+
// Should have fetched all 202 users
617+
if len(result) != 202 {
618+
t.Errorf("expected 202 users, got %d", len(result))
619+
}
620+
621+
// Verify first user from page 1
622+
if result[0].Login != "user0" {
623+
t.Errorf("expected first user 'user0', got %q", result[0].Login)
624+
}
625+
626+
// Verify last user from page 3
627+
if result[201].Login != "user201" {
628+
t.Errorf("expected last user 'user201', got %q", result[201].Login)
629+
}
630+
631+
// Verify a user from page 2
632+
if result[150].Login != "user150" {
633+
t.Errorf("expected middle user 'user150', got %q", result[150].Login)
634+
}
635+
}
636+
637+
func TestGetStarredReposPagination(t *testing.T) {
638+
// Test with exactly 100 repos (single page, should not request page 2)
639+
repos := make([]Repository, 100)
640+
for i := 0; i < 100; i++ {
641+
repos[i] = Repository{
642+
ID: int64(i),
643+
Name: fmt.Sprintf("repo%d", i),
644+
FullName: fmt.Sprintf("owner/repo%d", i),
645+
}
646+
}
647+
648+
requestCount := 0
649+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
650+
requestCount++
651+
652+
query := r.URL.Query()
653+
page := query.Get("page")
654+
655+
if page == "1" {
656+
w.Header().Set("Content-Type", "application/json")
657+
if err := json.NewEncoder(w).Encode(repos); err != nil {
658+
t.Fatalf("encoding repos: %v", err)
659+
}
660+
} else if page == "2" {
661+
// Should not request page 2 if page 1 had exactly 100 items
662+
// Return empty to stop pagination
663+
w.Header().Set("Content-Type", "application/json")
664+
if err := json.NewEncoder(w).Encode([]Repository{}); err != nil {
665+
t.Fatalf("encoding empty repos: %v", err)
666+
}
667+
} else {
668+
t.Errorf("unexpected page: %s", page)
669+
}
670+
}))
671+
defer server.Close()
672+
673+
c := NewClient("test-token", WithBaseURL(server.URL))
674+
result, err := c.GetStarredRepos(context.Background())
675+
if err != nil {
676+
t.Fatalf("GetStarredRepos() error: %v", err)
677+
}
678+
679+
if len(result) != 100 {
680+
t.Errorf("expected 100 repos, got %d", len(result))
681+
}
682+
683+
// Should have made exactly 2 requests (page 1 and page 2 to check if more data exists)
684+
// Actually, with the < perPage check, it should only make 1 request
685+
// Let me fix the logic - if we get exactly perPage items, we need to check the next page
686+
// So it should make 2 requests
687+
if requestCount != 2 {
688+
t.Errorf("expected 2 requests, got %d", requestCount)
689+
}
690+
}

0 commit comments

Comments
 (0)