diff --git a/pkg/github/client.go b/pkg/github/client.go index 3647b98..40cc3f7 100644 --- a/pkg/github/client.go +++ b/pkg/github/client.go @@ -28,12 +28,23 @@ func (f *IssueFetcher) FetchIssues(ctx context.Context, owner, repo string, sinc } var allIssues []*github.Issue + seen := make(map[string]struct{}) for { issues, resp, err := f.client.Issues.ListByRepository(ctx, owner, repo, opt) if err != nil { return nil, fmt.Errorf("failed to list issues: %w", err) } - allIssues = append(allIssues, issues...) + for _, issue := range issues { + key := issue.GetURL() + if key == "" { + key = fmt.Sprintf("id:%d", issue.GetID()) + } + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + allIssues = append(allIssues, issue) + } if resp.NextPage == 0 { break } diff --git a/pkg/github/client_test.go b/pkg/github/client_test.go index 884b946..4c31251 100644 --- a/pkg/github/client_test.go +++ b/pkg/github/client_test.go @@ -21,7 +21,8 @@ func TestFetchIssues_Pagination(t *testing.T) { sinceTime := time.Now().Add(-24 * time.Hour).Truncate(time.Second) sinceStr := sinceTime.Format(time.RFC3339) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { q := r.URL.Query() if q.Get("since") != sinceStr { t.Errorf("expected since parameter %q, got %q", sinceStr, q.Get("since")) @@ -96,7 +97,8 @@ func TestFetchIssues_Pagination(t *testing.T) { } func TestFetchIssues_RateLimit(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("X-RateLimit-Limit", "60") w.Header().Set("X-RateLimit-Remaining", "0") w.Header().Set("X-RateLimit-Reset", strconv.FormatInt(time.Now().Add(1*time.Hour).Unix(), 10)) @@ -122,7 +124,8 @@ func TestFetchIssues_RateLimit(t *testing.T) { } func TestFetchIssues_EmptyPageWithNext(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { q := r.URL.Query() pageStr := q.Get("page") page := 1 @@ -176,6 +179,76 @@ func TestFetchIssues_EmptyPageWithNext(t *testing.T) { } } +func TestFetchIssues_DeduplicatesAcrossPages(t *testing.T) { + sinceTime := time.Now().Add(-24 * time.Hour).Truncate(time.Second) + sinceStr := sinceTime.Format(time.RFC3339) + requests := 0 + + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + if got := r.URL.Query().Get("since"); got != sinceStr { + t.Fatalf("expected since parameter %q, got %q", sinceStr, got) + } + + page := 1 + if rawPage := r.URL.Query().Get("page"); rawPage != "" { + parsedPage, err := strconv.Atoi(rawPage) + if err != nil { + t.Fatalf("invalid page parameter: %v", err) + } + page = parsedPage + } + + duplicateURL := "https://api.github.com/repos/owner/repo/issues/42" + var issues []*github.Issue + switch page { + case 1: + issues = []*github.Issue{ + {ID: github.Int64(42), URL: github.String(duplicateURL), Title: github.String("duplicate")}, + {ID: github.Int64(43), URL: github.String("https://api.github.com/repos/owner/repo/issues/43")}, + } + w.Header().Set("Link", fmt.Sprintf("<%s?page=2&since=%s>; rel=\"next\"", server.URL+r.URL.Path, url.QueryEscape(sinceStr))) + case 2: + issues = []*github.Issue{ + {ID: github.Int64(42), URL: github.String(duplicateURL), Title: github.String("duplicate again")}, + {ID: github.Int64(44), URL: github.String("https://api.github.com/repos/owner/repo/issues/44")}, + } + default: + t.Fatalf("unexpected page request: %d", page) + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(issues) + })) + defer server.Close() + + client := github.NewClient(server.Client()) + baseURL, _ := url.Parse(server.URL + "/") + client.BaseURL = baseURL + + fetcher := NewIssueFetcher(client) + issues, err := fetcher.FetchIssues(context.Background(), "owner", "repo", sinceTime, 2) + if err != nil { + t.Fatalf("FetchIssues failed: %v", err) + } + + if requests != 2 { + t.Fatalf("expected 2 paginated requests, got %d", requests) + } + if len(issues) != 3 { + t.Fatalf("expected 3 unique issues, got %d", len(issues)) + } + + gotIDs := []int64{issues[0].GetID(), issues[1].GetID(), issues[2].GetID()} + wantIDs := []int64{42, 43, 44} + for i := range wantIDs { + if gotIDs[i] != wantIDs[i] { + t.Fatalf("at index %d: expected ID %d, got %d", i, wantIDs[i], gotIDs[i]) + } + } +} + func TestFetchIssues_Integration(t *testing.T) { token := os.Getenv("GITHUB_TOKEN") repoFullName := os.Getenv("GITHUB_REPOSITORY")