diff --git a/go.mod b/go.mod index 29cd5fc..98590cc 100644 --- a/go.mod +++ b/go.mod @@ -6,3 +6,11 @@ require ( github.com/google/go-github/v57 v57.0.0 golang.org/x/oauth2 v0.15.0 ) + +require ( + github.com/golang/protobuf v1.5.3 // indirect + github.com/google/go-querystring v1.1.0 // indirect + golang.org/x/net v0.19.0 // indirect + google.golang.org/appengine v1.6.7 // indirect + google.golang.org/protobuf v1.31.0 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..33b743b --- /dev/null +++ b/go.sum @@ -0,0 +1,29 @@ +github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= +github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= +github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-github/v57 v57.0.0 h1:L+Y3UPTY8ALM8x+TV0lg+IEBI+upibemtBD8Q9u7zHs= +github.com/google/go-github/v57 v57.0.0/go.mod h1:s0omdnye0hvK/ecLvpsGfJMiRt85PimQh4oygmLIxHw= +github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= +github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= +golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= +golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= +golang.org/x/oauth2 v0.15.0 h1:s8pnnxNVzjWyrvYdFUQq5llS1PX2zhPXmccZv99h7uQ= +golang.org/x/oauth2 v0.15.0/go.mod h1:q48ptWNTY5XWf+JNten23lcvHpLJ0ZSxF5ttTHKVCAM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= +google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= +google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= +google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= +google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= +google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= diff --git a/pkg/github/client.go b/pkg/github/client.go index 3647b98..d44c4d5 100644 --- a/pkg/github/client.go +++ b/pkg/github/client.go @@ -28,12 +28,24 @@ func (f *IssueFetcher) FetchIssues(ctx context.Context, owner, repo string, sinc } var allIssues []*github.Issue + seenIssues := make(map[string]struct{}) for { - issues, resp, err := f.client.Issues.ListByRepository(ctx, owner, repo, opt) + issues, resp, err := f.client.Issues.ListByRepo(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 := issueDedupKey(issue) + if key == "" { + allIssues = append(allIssues, issue) + continue + } + if _, ok := seenIssues[key]; ok { + continue + } + seenIssues[key] = struct{}{} + allIssues = append(allIssues, issue) + } if resp.NextPage == 0 { break } @@ -41,3 +53,22 @@ func (f *IssueFetcher) FetchIssues(ctx context.Context, owner, repo string, sinc } return allIssues, nil } + +func issueDedupKey(issue *github.Issue) string { + if issue == nil { + return "" + } + if id := issue.GetID(); id != 0 { + return fmt.Sprintf("id:%d", id) + } + if nodeID := issue.GetNodeID(); nodeID != "" { + return "node:" + nodeID + } + if url := issue.GetURL(); url != "" { + return "url:" + url + } + if number := issue.GetNumber(); number != 0 { + return fmt.Sprintf("number:%d", number) + } + return "" +} diff --git a/pkg/github/client_test.go b/pkg/github/client_test.go index 884b946..64f4b72 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")) @@ -122,7 +123,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 +178,123 @@ func TestFetchIssues_EmptyPageWithNext(t *testing.T) { } } +func TestFetchIssues_UsesResponseNextPageWhenPageSizeIsShort(t *testing.T) { + requestedPages := []int{} + 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 + if pageStr != "" { + var err error + page, err = strconv.Atoi(pageStr) + if err != nil { + t.Fatalf("invalid page parameter: %v", err) + } + } + requestedPages = append(requestedPages, page) + + var issues []*github.Issue + switch page { + case 1: + issues = []*github.Issue{ + {ID: github.Int64(1), Title: github.String("Issue 1")}, + } + linkHeader := fmt.Sprintf("<%s?page=4>; rel=\"next\"", server.URL+r.URL.Path) + w.Header().Set("Link", linkHeader) + case 4: + issues = []*github.Issue{ + {ID: github.Int64(4), Title: github.String("Issue 4")}, + } + default: + t.Fatalf("unexpected page request: %d", page) + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(issues) + })) + defer server.Close() + + httpClient := server.Client() + client := github.NewClient(httpClient) + baseURL, _ := url.Parse(server.URL + "/") + client.BaseURL = baseURL + + fetcher := NewIssueFetcher(client) + issues, err := fetcher.FetchIssues(context.Background(), "owner", "repo", time.Now(), 3) + if err != nil { + t.Fatalf("FetchIssues failed: %v", err) + } + + if len(issues) != 2 { + t.Fatalf("expected 2 issues, got %d", len(issues)) + } + + expectedPages := []int{1, 4} + if fmt.Sprint(requestedPages) != fmt.Sprint(expectedPages) { + t.Fatalf("expected requested pages %v, got %v", expectedPages, requestedPages) + } +} + +func TestFetchIssues_DeduplicatesIssuesAcrossPages(t *testing.T) { + 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 + if pageStr != "" { + var err error + page, err = strconv.Atoi(pageStr) + if err != nil { + t.Fatalf("invalid page parameter: %v", err) + } + } + + var issues []*github.Issue + switch page { + case 1: + issues = []*github.Issue{ + {ID: github.Int64(1), Title: github.String("Issue 1")}, + {ID: github.Int64(2), Title: github.String("Issue 2")}, + } + linkHeader := fmt.Sprintf("<%s?page=2>; rel=\"next\"", server.URL+r.URL.Path) + w.Header().Set("Link", linkHeader) + case 2: + issues = []*github.Issue{ + {ID: github.Int64(2), Title: github.String("Issue 2 duplicate")}, + {ID: github.Int64(3), Title: github.String("Issue 3")}, + } + default: + t.Fatalf("unexpected page request: %d", page) + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(issues) + })) + defer server.Close() + + httpClient := server.Client() + client := github.NewClient(httpClient) + baseURL, _ := url.Parse(server.URL + "/") + client.BaseURL = baseURL + + fetcher := NewIssueFetcher(client) + issues, err := fetcher.FetchIssues(context.Background(), "owner", "repo", time.Now(), 2) + if err != nil { + t.Fatalf("FetchIssues failed: %v", err) + } + + expectedIDs := []int64{1, 2, 3} + if len(issues) != len(expectedIDs) { + t.Fatalf("expected %d issues, got %d", len(expectedIDs), len(issues)) + } + for i, issue := range issues { + if issue.GetID() != expectedIDs[i] { + t.Fatalf("at index %d: expected ID %d, got %d", i, expectedIDs[i], issue.GetID()) + } + } +} + func TestFetchIssues_Integration(t *testing.T) { token := os.Getenv("GITHUB_TOKEN") repoFullName := os.Getenv("GITHUB_REPOSITORY")