From eac2aecce64de8a2f4e34974d20f87c885f88c4c Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Mon, 7 Feb 2022 14:06:05 -0800 Subject: [PATCH] Add support for commit-based lookup to GitHub APIs (#1612) Co-authored-by: Azeem Shaikh --- clients/githubrepo/client.go | 6 +- clients/githubrepo/graphql.go | 140 ++++++++++++++++++---------------- clients/githubrepo/tarball.go | 12 ++- 3 files changed, 87 insertions(+), 71 deletions(-) diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index 49a1b89ea5f..1a1557b6424 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -52,6 +52,7 @@ type Client struct { // InitRepo sets up the GitHub repo in local storage for improving performance and GitHub token usage efficiency. func (client *Client) InitRepo(inputRepo clients.Repo) error { + commitSHA := "HEAD" ghRepo, ok := inputRepo.(*repoURL) if !ok { return fmt.Errorf("%w: %v", errInputRepoType, inputRepo) @@ -67,12 +68,13 @@ func (client *Client) InitRepo(inputRepo clients.Repo) error { client.repoName = repo.GetName() // Init tarballHandler. - if err := client.tarball.init(client.ctx, client.repo); err != nil { + if err := client.tarball.init(client.ctx, client.repo, commitSHA); err != nil { return fmt.Errorf("error during tarballHandler.init: %w", err) } // Setup GraphQL. - client.graphClient.init(client.ctx, client.owner, client.repoName) + client.graphClient.init(client.ctx, client.owner, client.repoName, + client.repo.GetDefaultBranch(), commitSHA) // Setup contributorsHandler. client.contributors.init(client.ctx, client.owner, client.repoName) diff --git a/clients/githubrepo/graphql.go b/clients/githubrepo/graphql.go index ba868bdbf3e..6f66da4408b 100644 --- a/clients/githubrepo/graphql.go +++ b/clients/githubrepo/graphql.go @@ -17,6 +17,7 @@ package githubrepo import ( "context" "fmt" + "strings" "sync" "time" @@ -38,65 +39,63 @@ const ( // nolint: govet type graphqlData struct { Repository struct { - IsArchived githubv4.Boolean - DefaultBranchRef struct { - Target struct { - Commit struct { - History struct { - Nodes []struct { - CommittedDate githubv4.DateTime - Message githubv4.String - Oid githubv4.GitObjectID - Author struct { - User struct { - Login githubv4.String - } + IsArchived githubv4.Boolean + Object struct { + Commit struct { + History struct { + Nodes []struct { + CommittedDate githubv4.DateTime + Message githubv4.String + Oid githubv4.GitObjectID + Author struct { + User struct { + Login githubv4.String } - Committer struct { - Name *string - User struct { - Login *string - } + } + Committer struct { + Name *string + User struct { + Login *string } - AssociatedPullRequests struct { - Nodes []struct { - Repository struct { - Name githubv4.String - Owner struct { - Login githubv4.String - } - } - Author struct { + } + AssociatedPullRequests struct { + Nodes []struct { + Repository struct { + Name githubv4.String + Owner struct { Login githubv4.String } - Number githubv4.Int - HeadRefOid githubv4.String - MergedAt githubv4.DateTime - MergeCommit struct { - // NOTE: only used for sanity check. - // Use original commit oid instead. - Oid githubv4.GitObjectID + } + Author struct { + Login githubv4.String + } + Number githubv4.Int + HeadRefOid githubv4.String + MergedAt githubv4.DateTime + MergeCommit struct { + // NOTE: only used for sanity check. + // Use original commit oid instead. + Oid githubv4.GitObjectID + } + Labels struct { + Nodes []struct { + Name githubv4.String } - Labels struct { - Nodes []struct { - Name githubv4.String - } - } `graphql:"labels(last: $labelsToAnalyze)"` - Reviews struct { - Nodes []struct { - State githubv4.String - Author struct { - Login githubv4.String - } + } `graphql:"labels(last: $labelsToAnalyze)"` + Reviews struct { + Nodes []struct { + State githubv4.String + Author struct { + Login githubv4.String } - } `graphql:"reviews(last: $reviewsToAnalyze)"` - } - } `graphql:"associatedPullRequests(first: $pullRequestsToAnalyze)"` - } - } `graphql:"history(first: $commitsToAnalyze)"` - } `graphql:"... on Commit"` - } - } + } + } `graphql:"reviews(last: $reviewsToAnalyze)"` + } + } `graphql:"associatedPullRequests(first: $pullRequestsToAnalyze)"` + } + } `graphql:"history(first: $commitsToAnalyze)"` + } `graphql:"... on Commit"` + } `graphql:"object(expression: $commitExpression)"` Issues struct { Nodes []struct { // nolint: revive,stylecheck // naming according to githubv4 convention. @@ -115,22 +114,26 @@ type graphqlData struct { } type graphqlHandler struct { - client *githubv4.Client - data *graphqlData - once *sync.Once - ctx context.Context - errSetup error - owner string - repo string - commits []clients.Commit - issues []clients.Issue - archived bool + client *githubv4.Client + data *graphqlData + once *sync.Once + ctx context.Context + errSetup error + owner string + repo string + defaultBranch string + commitSHA string + commits []clients.Commit + issues []clients.Issue + archived bool } -func (handler *graphqlHandler) init(ctx context.Context, owner, repo string) { +func (handler *graphqlHandler) init(ctx context.Context, owner, repo, defaultBranch, commitSHA string) { handler.ctx = ctx handler.owner = owner handler.repo = repo + handler.defaultBranch = defaultBranch + handler.commitSHA = commitSHA handler.data = new(graphqlData) handler.errSetup = nil handler.once = new(sync.Once) @@ -138,6 +141,12 @@ func (handler *graphqlHandler) init(ctx context.Context, owner, repo string) { func (handler *graphqlHandler) setup() error { handler.once.Do(func() { + commitExpression := handler.commitSHA + if strings.EqualFold(handler.commitSHA, "HEAD") { + // TODO(#575): Confirm that this works as expected. + commitExpression = fmt.Sprintf("heads/%s", handler.defaultBranch) + } + vars := map[string]interface{}{ "owner": githubv4.String(handler.owner), "name": githubv4.String(handler.repo), @@ -147,6 +156,7 @@ func (handler *graphqlHandler) setup() error { "reviewsToAnalyze": githubv4.Int(reviewsToAnalyze), "labelsToAnalyze": githubv4.Int(labelsToAnalyze), "commitsToAnalyze": githubv4.Int(commitsToAnalyze), + "commitExpression": githubv4.String(commitExpression), } if err := handler.client.Query(handler.ctx, handler.data, vars); err != nil { handler.errSetup = sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query: %v", err)) @@ -186,7 +196,7 @@ func (handler *graphqlHandler) isArchived() (bool, error) { // nolint: unparam func commitsFrom(data *graphqlData, repoOwner, repoName string) ([]clients.Commit, error) { ret := make([]clients.Commit, 0) - for _, commit := range data.Repository.DefaultBranchRef.Target.Commit.History.Nodes { + for _, commit := range data.Repository.Object.Commit.History.Nodes { var committer string if commit.Committer.User.Login != nil { committer = *commit.Committer.User.Login diff --git a/clients/githubrepo/tarball.go b/clients/githubrepo/tarball.go index 871469e6914..182a2dada56 100644 --- a/clients/githubrepo/tarball.go +++ b/clients/githubrepo/tarball.go @@ -68,14 +68,14 @@ type tarballHandler struct { files []string } -func (handler *tarballHandler) init(ctx context.Context, repo *github.Repository) error { +func (handler *tarballHandler) init(ctx context.Context, repo *github.Repository, commitSHA string) error { // Cleanup any previous state. if err := handler.cleanup(); err != nil { return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) } // Setup temp dir/files and download repo tarball. - if err := handler.getTarball(ctx, repo); errors.Is(err, errTarballNotFound) { + if err := handler.getTarball(ctx, repo, commitSHA); errors.Is(err, errTarballNotFound) { log.Printf("unable to get tarball %v. Skipping...", err) return nil } else if err != nil { @@ -93,10 +93,14 @@ func (handler *tarballHandler) init(ctx context.Context, repo *github.Repository return nil } -func (handler *tarballHandler) getTarball(ctx context.Context, repo *github.Repository) error { +func (handler *tarballHandler) getTarball(ctx context.Context, repo *github.Repository, commitSHA string) error { url := repo.GetArchiveURL() url = strings.Replace(url, "{archive_format}", "tarball/", 1) - url = strings.Replace(url, "{/ref}", "", 1) + if strings.EqualFold(commitSHA, "HEAD") { + url = strings.Replace(url, "{/ref}", "", 1) + } else { + url = strings.Replace(url, "{/ref}", commitSHA, 1) + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return fmt.Errorf("http.NewRequestWithContext: %w", err)