diff --git a/chronicle/release/releasers/github/gh_issue.go b/chronicle/release/releasers/github/gh_issue.go index abd9c34..6f4715d 100644 --- a/chronicle/release/releasers/github/gh_issue.go +++ b/chronicle/release/releasers/github/gh_issue.go @@ -155,14 +155,18 @@ func issuesWithoutLabels() issueFilter { } //nolint:funlen -func fetchClosedIssues(user, repo string) ([]ghIssue, error) { +func fetchClosedIssues(user, repo string, since *time.Time) ([]ghIssue, error) { src := oauth2.StaticTokenSource( // TODO: DI this &oauth2.Token{AccessToken: os.Getenv("GITHUB_TOKEN")}, ) httpClient := oauth2.NewClient(context.Background(), src) client := githubv4.NewClient(httpClient) - var allIssues []ghIssue + var ( + pages = 0 + saw = 0 + allIssues []ghIssue + ) { // TODO: act on hitting a rate limit @@ -192,6 +196,7 @@ func fetchClosedIssues(user, repo string) ([]ghIssue, error) { } Closed githubv4.Boolean ClosedAt githubv4.DateTime + UpdatedAt githubv4.DateTime StateReason githubv4.String Labels struct { Edges []struct { @@ -202,7 +207,7 @@ func fetchClosedIssues(user, repo string) ([]ghIssue, error) { } `graphql:"labels(first:100)"` } } - } `graphql:"issues(first:100, states:CLOSED, after:$issuesCursor)"` + } `graphql:"issues(first:100, states:CLOSED, after:$issuesCursor, orderBy:{field: UPDATED_AT, direction: DESC})"` } `graphql:"repository(owner:$repositoryOwner, name:$repositoryName)"` RateLimit rateLimit @@ -214,14 +219,28 @@ func fetchClosedIssues(user, repo string) ([]ghIssue, error) { } // var limit rateLimit - for { + var ( + process bool + terminate = false + ) + + for !terminate { + log.WithFields("user", user, "repo", repo, "page", pages).Trace("fetching closed issues from github.com") + err := client.Query(context.Background(), &query, variables) if err != nil { return nil, err } + // limit = query.RateLimit for _, iEdge := range query.Repository.Issues.Edges { + saw++ + process, terminate = checkSearchTermination(since, &iEdge.Node.UpdatedAt, &iEdge.Node.ClosedAt) + if !process || terminate { + continue + } + var labels []string for _, lEdge := range iEdge.Node.Labels.Edges { labels = append(labels, string(lEdge.Node.Name)) @@ -242,6 +261,7 @@ func fetchClosedIssues(user, repo string) ([]ghIssue, error) { break } variables["issuesCursor"] = githubv4.NewString(query.Repository.Issues.PageInfo.EndCursor) + pages++ } // for idx, is := range allIssues { @@ -250,5 +270,7 @@ func fetchClosedIssues(user, repo string) ([]ghIssue, error) { // printJSON(limit) } + log.WithFields("kept", len(allIssues), "saw", saw, "pages", pages, "since", since).Trace("closed PRs fetched from github.com") + return allIssues, nil } diff --git a/chronicle/release/releasers/github/gh_pull_request.go b/chronicle/release/releasers/github/gh_pull_request.go index cb80011..411e652 100644 --- a/chronicle/release/releasers/github/gh_pull_request.go +++ b/chronicle/release/releasers/github/gh_pull_request.go @@ -238,14 +238,18 @@ func keepPRsWithCommits(prs []ghPullRequest, commits []string, filters ...prFilt } //nolint:funlen -func fetchMergedPRs(user, repo string) ([]ghPullRequest, error) { +func fetchMergedPRs(user, repo string, since *time.Time) ([]ghPullRequest, error) { src := oauth2.StaticTokenSource( // TODO: DI this &oauth2.Token{AccessToken: os.Getenv("GITHUB_TOKEN")}, ) httpClient := oauth2.NewClient(context.Background(), src) client := githubv4.NewClient(httpClient) - var allPRs []ghPullRequest + var ( + pages = 0 + saw = 0 + allPRs []ghPullRequest + ) { // TODO: act on hitting a rate limit @@ -276,8 +280,9 @@ func fetchMergedPRs(user, repo string) ([]ghPullRequest, error) { MergeCommit struct { OID githubv4.String } - MergedAt githubv4.DateTime - Labels struct { + UpdatedAt githubv4.DateTime + MergedAt githubv4.DateTime + Labels struct { Edges []struct { Node struct { Name githubv4.String @@ -305,7 +310,7 @@ func fetchMergedPRs(user, repo string) ([]ghPullRequest, error) { } `graphql:"closingIssuesReferences(last:10)"` } } - } `graphql:"pullRequests(first:100, states:MERGED, after:$prCursor)"` + } `graphql:"pullRequests(first:100, states:MERGED, after:$prCursor, orderBy:{field: UPDATED_AT, direction: DESC})"` } `graphql:"repository(owner:$repositoryOwner, name:$repositoryName)"` RateLimit rateLimit @@ -317,14 +322,28 @@ func fetchMergedPRs(user, repo string) ([]ghPullRequest, error) { } // var limit rateLimit - for { + var ( + process bool + terminate = false + ) + + for !terminate { + log.WithFields("user", user, "repo", repo, "page", pages).Trace("fetching merged PRs from github.com") + err := client.Query(context.Background(), &query, variables) if err != nil { return nil, err } + // limit = query.RateLimit for _, prEdge := range query.Repository.PullRequests.Edges { + saw++ + process, terminate = checkSearchTermination(since, &prEdge.Node.UpdatedAt, &prEdge.Node.MergedAt) + if !process || terminate { + continue + } + var labels []string for _, lEdge := range prEdge.Node.Labels.Edges { labels = append(labels, string(lEdge.Node.Name)) @@ -359,8 +378,28 @@ func fetchMergedPRs(user, repo string) ([]ghPullRequest, error) { break } variables["prCursor"] = githubv4.NewString(query.Repository.PullRequests.PageInfo.EndCursor) + pages++ } } + log.WithFields("kept", len(allPRs), "saw", saw, "pages", pages, "since", since).Trace("merged PRs fetched from github.com") + return allPRs, nil } + +func checkSearchTermination(since *time.Time, updatedAt, closedAt *githubv4.DateTime) (process bool, terminate bool) { + process = true + if since == nil { + return + } + + if closedAt.Before(*since) { + process = false + } + + if updatedAt.Before(*since) { + terminate = true + } + + return +} diff --git a/chronicle/release/releasers/github/gh_pull_request_test.go b/chronicle/release/releasers/github/gh_pull_request_test.go index 89a02ae..bb3979d 100644 --- a/chronicle/release/releasers/github/gh_pull_request_test.go +++ b/chronicle/release/releasers/github/gh_pull_request_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/anchore/chronicle/chronicle/release/change" @@ -471,3 +472,80 @@ func Test_prsWithoutLinkedIssues(t *testing.T) { }) } } + +func Test_checkSearchTermination(t *testing.T) { + since := githubv4.DateTime{Time: time.Date(1987, time.September, 16, 19, 34, 0, 0, time.UTC)} + hourAfter := &githubv4.DateTime{Time: since.Add(time.Hour)} + minuteAfter := &githubv4.DateTime{Time: since.Add(time.Minute)} + minuteBefore := &githubv4.DateTime{Time: since.Add(-time.Minute)} + + type args struct { + since *time.Time + updatedAt *githubv4.DateTime + mergedAt *githubv4.DateTime + } + tests := []struct { + name string + args args + wantProcess bool + wantTerminate bool + }{ + { + name: "go case candidate", + args: args{ + since: &since.Time, + updatedAt: hourAfter, + mergedAt: hourAfter, + }, + wantProcess: true, + wantTerminate: false, + }, + { + name: "candidate updated after the merge, and merged after the compare date", + args: args{ + since: &since.Time, + updatedAt: hourAfter, + mergedAt: minuteAfter, + }, + wantProcess: true, + wantTerminate: false, + }, + { + name: "candidate updated after the merge, but merged before the compare date", + args: args{ + since: &since.Time, + updatedAt: hourAfter, + mergedAt: minuteBefore, + }, + wantProcess: false, + wantTerminate: false, + }, + { + name: "candidate updated before the merge, and merged before the compare date", + args: args{ + since: &since.Time, + updatedAt: minuteBefore, + mergedAt: minuteBefore, + }, + wantProcess: false, + wantTerminate: true, + }, + { + name: "impossible: candidate updated before the merge, but merged after the compare date", + args: args{ + since: &since.Time, + updatedAt: minuteBefore, + mergedAt: minuteAfter, + }, + wantProcess: true, + wantTerminate: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotProcess, gotTerminate := checkSearchTermination(tt.args.since, tt.args.updatedAt, tt.args.mergedAt) + assert.Equalf(t, tt.wantProcess, gotProcess, "wantProcess: checkSearchTermination(%v, %v, %v)", tt.args.since, tt.args.updatedAt, tt.args.mergedAt) + assert.Equalf(t, tt.wantTerminate, gotTerminate, "wantTerminate: checkSearchTermination(%v, %v, %v)", tt.args.since, tt.args.updatedAt, tt.args.mergedAt) + }) + } +} diff --git a/chronicle/release/releasers/github/summarizer.go b/chronicle/release/releasers/github/summarizer.go index 992d097..c3f2406 100644 --- a/chronicle/release/releasers/github/summarizer.go +++ b/chronicle/release/releasers/github/summarizer.go @@ -4,6 +4,7 @@ import ( "fmt" "net/url" "strings" + "time" "github.com/anchore/chronicle/chronicle/release" "github.com/anchore/chronicle/chronicle/release/change" @@ -118,6 +119,15 @@ func (s *Summarizer) Changes(sinceRef, untilRef string) ([]change.Change, error) includeStart = true } + var sinceTime *time.Time + if sinceTag != nil { + sinceRelease, err := fetchRelease(s.userName, s.repoName, sinceTag.Name) + if err != nil { + return nil, fmt.Errorf("unable to fetch release %q: %w", sinceTag.Name, err) + } + sinceTime = &sinceRelease.Date + } + var untilTag *git.Tag untilHash := untilRef if untilRef != "" { @@ -150,18 +160,18 @@ func (s *Summarizer) Changes(sinceRef, untilRef string) ([]change.Change, error) logCommits(includeCommits) } - allMergedPRs, err := fetchMergedPRs(s.userName, s.repoName) + allMergedPRs, err := fetchMergedPRs(s.userName, s.repoName, sinceTime) if err != nil { return nil, err } - log.Debugf("total merged PRs discovered: %d", len(allMergedPRs)) + log.WithFields("since", sinceTime).Debugf("total merged PRs discovered: %d", len(allMergedPRs)) if s.config.IncludePRs { changes = append(changes, changesFromStandardPRFilters(s.config, allMergedPRs, sinceTag, untilTag, includeCommits)...) } - allClosedIssues, err := fetchClosedIssues(s.userName, s.repoName) + allClosedIssues, err := fetchClosedIssues(s.userName, s.repoName, sinceTime) if err != nil { return nil, err } @@ -170,7 +180,7 @@ func (s *Summarizer) Changes(sinceRef, untilRef string) ([]change.Change, error) allClosedIssues = filterIssues(allClosedIssues, excludeIssuesNotPlanned(allMergedPRs)) } - log.Debugf("total closed issues discovered: %d", len(allClosedIssues)) + log.WithFields("since", sinceTime).Debugf("total closed issues discovered: %d", len(allClosedIssues)) if s.config.IncludeIssues { if s.config.IssuesRequireLinkedPR { @@ -342,7 +352,7 @@ func changesFromUnlabeledPRs(config Config, allMergedPRs []ghPullRequest, sinceT filteredIssues, _ := filterPRs(allMergedPRs, filters...) - log.Debugf("unlabeled prs contributing to changelog: %d", len(filteredIssues)) + log.Debugf("unlabeled PRs contributing to changelog: %d", len(filteredIssues)) return createChangesFromPRs(config, filteredIssues) }