Use RepoClient API for Contributors check (#854)

Co-authored-by: Azeem Shaikh <azeems@google.com>

checks/contributors.go

@@ -18,8 +18,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/google/go-github/v38/github"
-
 	"github.com/ossf/scorecard/v2/checker"
 	sce "github.com/ossf/scorecard/v2/errors"
 )
@@ -38,31 +36,25 @@ func init() {
 
 // Contributors run Contributors check.
 func Contributors(c *checker.CheckRequest) checker.CheckResult {
-	contribs, _, err := c.Client.Repositories.ListContributors(c.Ctx, c.Owner, c.Repo, &github.ListContributorsOptions{})
+	contribs, err := c.RepoClient.ListContributors()
 	if err != nil {
 		e := sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.Repositories.ListContributors: %v", err))
 		return checker.CreateRuntimeErrorResult(CheckContributors, e)
 	}
 
 	companies := map[string]struct{}{}
 	for _, contrib := range contribs {
-		if contrib.GetContributions() < minContributionsPerUser {
+		if contrib.NumContributions < minContributionsPerUser {
 			continue
 		}
-		u, _, err := c.Client.Users.Get(c.Ctx, contrib.GetLogin())
-		if err != nil {
-			e := sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.Users.Get: %v", err))
-			return checker.CreateRuntimeErrorResult(CheckContributors, e)
-		}
-		orgs, _, err := c.Client.Organizations.List(c.Ctx, contrib.GetLogin(), nil)
-		if err != nil {
-			c.Dlogger.Debug("unable to get org members for %s: %v", contrib.GetLogin(), err)
-		} else if len(orgs) > 0 {
-			companies[*orgs[0].Login] = struct{}{}
-			continue
+
+		for _, org := range contrib.Organizations {
+			if org.Login != "" {
+				companies[org.Login] = struct{}{}
+			}
 		}
 
-		company := u.GetCompany()
+		company := contrib.Company
 		if company != "" {
 			company = strings.ToLower(company)
 			company = strings.ReplaceAll(company, "inc.", "")

clients/contributor.go

@@ -0,0 +1,23 @@
+// Copyright 2021 Security Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package clients
+
+// Contributor represents a contributor to a repo.
+type Contributor struct {
+	Company          string
+	User             User
+	Organizations    []User
+	NumContributions int
+}

clients/githubrepo/client.go

@@ -27,16 +27,17 @@ import (
 
 // Client is GitHub-specific implementation of RepoClient.
 type Client struct {
-	repo        *github.Repository
-	repoClient  *github.Client
-	graphClient *graphqlHandler
-	ctx         context.Context
-	tarball     tarballHandler
+	repo         *github.Repository
+	repoClient   *github.Client
+	graphClient  *graphqlHandler
+	contributors *contributorsHandler
+	ctx          context.Context
+	tarball      tarballHandler
 }
 
 // InitRepo sets up the GitHub repo in local storage for improving performance and GitHub token usage efficiency.
 func (client *Client) InitRepo(owner, repoName string) error {
-	// Sanity check
+	// Sanity check.
 	repo, _, err := client.repoClient.Repositories.Get(client.ctx, owner, repoName)
 	if err != nil {
 		// nolint: wrapcheck
@@ -49,11 +50,16 @@ func (client *Client) InitRepo(owner, repoName string) error {
 		return fmt.Errorf("error during tarballHandler.init: %w", err)
 	}
 
-	// Setup GraphQL
+	// Setup GraphQL.
 	if err := client.graphClient.init(client.ctx, owner, repoName); err != nil {
 		return fmt.Errorf("error during graphqlHandler.init: %w", err)
 	}
 
+	// Setup contributors.
+	if err := client.contributors.init(client.ctx, owner, repoName); err != nil {
+		return fmt.Errorf("error during contributorsHandler.init: %w", err)
+	}
+
 	return nil
 }
 
@@ -82,6 +88,11 @@ func (client *Client) ListReleases() ([]clients.Release, error) {
 	return client.graphClient.getReleases()
 }
 
+// ListContributors implements RepoClient.ListContributors.
+func (client *Client) ListContributors() ([]clients.Contributor, error) {
+	return client.contributors.getContributors()
+}
+
 // IsArchived implements RepoClient.IsArchived.
 func (client *Client) IsArchived() (bool, error) {
 	return client.graphClient.isArchived()
@@ -106,5 +117,8 @@ func CreateGithubRepoClient(ctx context.Context,
 		graphClient: &graphqlHandler{
 			client: graphClient,
 		},
+		contributors: &contributorsHandler{
+			ghClient: client,
+		},
 	}
 }

clients/githubrepo/contributors.go

@@ -0,0 +1,68 @@
+// Copyright 2021 Security Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package githubrepo
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/google/go-github/v38/github"
+
+	"github.com/ossf/scorecard/v2/clients"
+)
+
+type contributorsHandler struct {
+	ghClient     *github.Client
+	contributors []clients.Contributor
+}
+
+func (handler *contributorsHandler) init(ctx context.Context, owner, repo string) error {
+	contribs, _, err := handler.ghClient.Repositories.ListContributors(ctx, owner, repo, &github.ListContributorsOptions{})
+	if err != nil {
+		return fmt.Errorf("error during ListContributors: %w", err)
+	}
+
+	for _, contrib := range contribs {
+		if contrib.GetLogin() == "" {
+			continue
+		}
+		contributor := clients.Contributor{
+			NumContributions: contrib.GetContributions(),
+			User: clients.User{
+				Login: contrib.GetLogin(),
+			},
+		}
+		orgs, _, err := handler.ghClient.Organizations.List(ctx, contrib.GetLogin(), nil)
+		// This call can fail due to token scopes. So ignore error.
+		if err == nil {
+			for _, org := range orgs {
+				contributor.Organizations = append(contributor.Organizations, clients.User{
+					Login: org.GetLogin(),
+				})
+			}
+		}
+		user, _, err := handler.ghClient.Users.Get(ctx, contrib.GetLogin())
+		if err != nil {
+			return fmt.Errorf("error during Users.Get: %w", err)
+		}
+		contributor.Company = user.GetCompany()
+		handler.contributors = append(handler.contributors, contributor)
+	}
+	return nil
+}
+
+func (handler *contributorsHandler) getContributors() ([]clients.Contributor, error) {
+	return handler.contributors, nil
+}

clients/repo_client.go

@@ -52,5 +52,6 @@ type RepoClient interface {
 	GetDefaultBranch() (BranchRef, error)
 	ListCommits() ([]Commit, error)
 	ListReleases() ([]Release, error)
+	ListContributors() ([]Contributor, error)
 	Close() error
 }

e2e/contributors_test.go

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// nolint: dupl
 package e2e
 
 import (
@@ -22,18 +23,22 @@ import (
 
 	"github.com/ossf/scorecard/v2/checker"
 	"github.com/ossf/scorecard/v2/checks"
+	"github.com/ossf/scorecard/v2/clients/githubrepo"
 	scut "github.com/ossf/scorecard/v2/utests"
 )
 
-var _ = Describe("E2E TEST:Contributors", func() {
+var _ = Describe("E2E TEST:"+checks.CheckContributors, func() {
 	Context("E2E TEST:Validating project contributors", func() {
 		It("Should return valid project contributors", func() {
 			dl := scut.TestDetailLogger{}
+			repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient, graphClient)
+			err := repoClient.InitRepo("ossf", "scorecard")
+			Expect(err).Should(BeNil())
 			req := checker.CheckRequest{
 				Ctx:         context.Background(),
 				Client:      ghClient,
 				HTTPClient:  httpClient,
-				RepoClient:  nil,
+				RepoClient:  repoClient,
 				Owner:       "ossf",
 				Repo:        "scorecard",
 				GraphClient: graphClient,