Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ Simplify RunScorecard with functional optionals #4106

Merged
merged 15 commits into from
Jun 10, 2024
4 changes: 2 additions & 2 deletions clients/githubrepo/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,13 @@ type branchesHandler struct {
once *sync.Once
ctx context.Context
errSetup error
repourl *repoURL
repourl *Repo
defaultBranchRef *clients.BranchRef
defaultBranchName string
ruleSets []*repoRuleSet
}

func (handler *branchesHandler) init(ctx context.Context, repourl *repoURL) {
func (handler *branchesHandler) init(ctx context.Context, repourl *Repo) {
handler.ctx = ctx
handler.repourl = repourl
handler.errSetup = nil
Expand Down
14 changes: 7 additions & 7 deletions clients/githubrepo/branches_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() {
It("Should not have increased for HEAD query", func() {
skipIfTokenIsNot(patTokenType, "PAT only")

repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -51,7 +51,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() {
It("Should fail for non-HEAD query", func() {
skipIfTokenIsNot(patTokenType, "PAT only")

repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: "de5224bbc56eceb7a25aece55d2d53bbc561ed2d",
Expand All @@ -66,7 +66,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() {
It("Should return the correct default branch", func() {
skipIfTokenIsNot(patTokenType, "PAT only")

repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -83,7 +83,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() {
It("Should return a branch", func() {
skipIfTokenIsNot(patTokenType, "PAT only")

repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -98,7 +98,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() {
It("Should return an error for nonexistent branch", func() {
skipIfTokenIsNot(patTokenType, "PAT only")

repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -114,7 +114,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() {
It("Should return a branch", func() {
skipIfTokenIsNot(patTokenType, "PAT only")

repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -126,7 +126,7 @@ var _ = Describe("E2E TEST: githubrepo.branchesHandler", func() {
It("Should fail for non-HEAD query", func() {
skipIfTokenIsNot(patTokenType, "PAT only")

repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: "de5224bbc56eceb7a25aece55d2d53bbc561ed2d",
Expand Down
4 changes: 2 additions & 2 deletions clients/githubrepo/checkruns.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type checkRunsByRef = map[string][]clients.CheckRun
type checkrunsHandler struct {
client *github.Client
graphClient *githubv4.Client
repourl *repoURL
repourl *Repo
logger *log.Logger
checkData *checkRunsGraphqlData
setupOnce *sync.Once
Expand All @@ -81,7 +81,7 @@ type checkrunsHandler struct {
errSetup error
}

func (handler *checkrunsHandler) init(ctx context.Context, repourl *repoURL, commitDepth int) {
func (handler *checkrunsHandler) init(ctx context.Context, repourl *Repo, commitDepth int) {
handler.ctx = ctx
handler.repourl = repourl
handler.commitDepth = commitDepth
Expand Down
6 changes: 3 additions & 3 deletions clients/githubrepo/checkruns_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var _ = Describe("E2E TEST: githubrepo.checkrunsHandler", func() {

Context("E2E TEST: Validate query cost", func() {
It("Should not have increased query cost", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -54,7 +54,7 @@ var _ = Describe("E2E TEST: githubrepo.checkrunsHandler", func() {
})
Context("E2E TEST: listCheckRunsForRef", func() {
It("Should return check runs for a valid ref", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -75,7 +75,7 @@ var _ = Describe("E2E TEST: githubrepo.checkrunsHandler", func() {
})
})
It("Should return an error for an invalid ref", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand Down
6 changes: 3 additions & 3 deletions clients/githubrepo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var (

// Client is GitHub-specific implementation of RepoClient.
type Client struct {
repourl *repoURL
repourl *Repo
repo *github.Repository
repoClient *github.Client
graphClient *graphqlHandler
Expand All @@ -66,7 +66,7 @@ const defaultGhHost = "github.com"

// InitRepo sets up the GitHub repo in local storage for improving performance and GitHub token usage efficiency.
func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitDepth int) error {
ghRepo, ok := inputRepo.(*repoURL)
ghRepo, ok := inputRepo.(*Repo)
if !ok {
return fmt.Errorf("%w: %v", errInputRepoType, inputRepo)
}
Expand All @@ -81,7 +81,7 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD
}
client.commitDepth = commitDepth
client.repo = repo
client.repourl = &repoURL{
client.repourl = &Repo{
owner: repo.Owner.GetLogin(),
repo: repo.GetName(),
defaultBranch: repo.GetDefaultBranch(),
Expand Down
4 changes: 2 additions & 2 deletions clients/githubrepo/contributors.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ type contributorsHandler struct {
once *sync.Once
ctx context.Context
errSetup error
repourl *repoURL
repourl *Repo
contributors []clients.User
}

func (handler *contributorsHandler) init(ctx context.Context, repourl *repoURL) {
func (handler *contributorsHandler) init(ctx context.Context, repourl *Repo) {
handler.ctx = ctx
handler.repourl = repourl
handler.errSetup = nil
Expand Down
2 changes: 1 addition & 1 deletion clients/githubrepo/contributors_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var _ = Describe("E2E TEST: githubrepo.contributorsHandler", func() {
Context("getContributors()", func() {
skipIfTokenIsNot(patTokenType, "PAT only")
It("returns contributors for valid HEAD query", func() {
repoURL := repoURL{
repoURL := Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand Down
4 changes: 2 additions & 2 deletions clients/githubrepo/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,14 @@ type graphqlHandler struct {
setupOnce *sync.Once
ctx context.Context
errSetup error
repourl *repoURL
repourl *Repo
commits []clients.Commit
issues []clients.Issue
archived bool
commitDepth int
}

func (handler *graphqlHandler) init(ctx context.Context, repourl *repoURL, commitDepth int) {
func (handler *graphqlHandler) init(ctx context.Context, repourl *Repo, commitDepth int) {
handler.ctx = ctx
handler.repourl = repourl
handler.data = new(graphqlData)
Expand Down
10 changes: 5 additions & 5 deletions clients/githubrepo/graphql_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ var _ = Describe("E2E TEST: githubrepo.graphqlHandler", func() {

Context("E2E TEST: Confirm Paging Commits Works", func() {
It("Should only have 1 commit", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand Down Expand Up @@ -71,7 +71,7 @@ var _ = Describe("E2E TEST: githubrepo.graphqlHandler", func() {
Expect(len(commits)).Should(BeEquivalentTo(1))
})
It("Should have 30 commits", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand Down Expand Up @@ -104,7 +104,7 @@ var _ = Describe("E2E TEST: githubrepo.graphqlHandler", func() {
Expect(len(commits)).Should(BeEquivalentTo(30))
})
It("Should have 101 commits", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand Down Expand Up @@ -140,7 +140,7 @@ var _ = Describe("E2E TEST: githubrepo.graphqlHandler", func() {

Context("E2E TEST: Validate query cost", func() {
It("Should not have increased for HEAD query", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand All @@ -152,7 +152,7 @@ var _ = Describe("E2E TEST: githubrepo.graphqlHandler", func() {
Expect(*graphqlhandler.data.RateLimit.Cost).Should(BeNumerically("<=", 1))
})
It("Should not have increased for commit query", func() {
repourl := &repoURL{
repourl := &Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: "de5224bbc56eceb7a25aece55d2d53bbc561ed2d",
Expand Down
2 changes: 1 addition & 1 deletion clients/githubrepo/graphql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func Test_getCommits_retry(t *testing.T) {
Transport: rt,
}),
}
handler.init(context.Background(), &repoURL{}, 1)
handler.init(context.Background(), &Repo{}, 1)
_, err := handler.getCommits()
if err == nil {
t.Error("expected error")
Expand Down
4 changes: 2 additions & 2 deletions clients/githubrepo/languages.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ type languagesHandler struct {
once *sync.Once
ctx context.Context
errSetup error
repourl *repoURL
repourl *Repo
languages []clients.Language
}

func (handler *languagesHandler) init(ctx context.Context, repourl *repoURL) {
func (handler *languagesHandler) init(ctx context.Context, repourl *Repo) {
handler.ctx = ctx
handler.repourl = repourl
handler.errSetup = nil
Expand Down
2 changes: 1 addition & 1 deletion clients/githubrepo/languages_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var _ = Describe("E2E TEST: githubrepo.languagesHandler", func() {
})
Context("listProgrammingLanguages()", func() {
It("returns a list of programming languages for a valid repository", func() {
repoURL := repoURL{
repoURL := Repo{
owner: "ossf",
repo: "scorecard",
}
Expand Down
4 changes: 2 additions & 2 deletions clients/githubrepo/licenses.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ type licensesHandler struct {
once *sync.Once
ctx context.Context
errSetup error
repourl *repoURL
repourl *Repo
licenses []clients.License
}

func (handler *licensesHandler) init(ctx context.Context, repourl *repoURL) {
func (handler *licensesHandler) init(ctx context.Context, repourl *Repo) {
handler.ctx = ctx
handler.repourl = repourl
handler.errSetup = nil
Expand Down
2 changes: 1 addition & 1 deletion clients/githubrepo/licenses_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var _ = Describe("E2E TEST: githubrepo.licensesHandler", func() {
})
Context("listLicenses()", func() {
It("returns licenses", func() {
repoURL := repoURL{
repoURL := Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand Down
4 changes: 2 additions & 2 deletions clients/githubrepo/releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ type releasesHandler struct {
once *sync.Once
ctx context.Context
errSetup error
repourl *repoURL
repourl *Repo
releases []clients.Release
}

func (handler *releasesHandler) init(ctx context.Context, repourl *repoURL) {
func (handler *releasesHandler) init(ctx context.Context, repourl *Repo) {
handler.ctx = ctx
handler.repourl = repourl
handler.errSetup = nil
Expand Down
2 changes: 1 addition & 1 deletion clients/githubrepo/releases_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var _ = Describe("E2E TEST: githubrepo.releasesHandler", func() {
})
Context("getReleases()", func() {
It("returns releases", func() {
repoURL := repoURL{
repoURL := Repo{
owner: "ossf",
repo: "scorecard",
commitSHA: clients.HeadSHA,
Expand Down
22 changes: 11 additions & 11 deletions clients/githubrepo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ import (
sce "github.com/ossf/scorecard/v5/errors"
)

type repoURL struct {
type Repo struct {
host, owner, repo, defaultBranch, commitSHA string
metadata []string
}

// Parses input string into repoURL struct.
// Accepts "owner/repo" or "github.com/owner/repo".
func (r *repoURL) parse(input string) error {
func (r *Repo) parse(input string) error {
var t string

const two = 2
Expand Down Expand Up @@ -73,21 +73,21 @@ func (r *repoURL) parse(input string) error {
}

// URI implements Repo.URI().
func (r *repoURL) URI() string {
func (r *Repo) URI() string {
return fmt.Sprintf("%s/%s/%s", r.host, r.owner, r.repo)
}

func (r *repoURL) Host() string {
func (r *Repo) Host() string {
return r.host
}

// String implements Repo.String.
func (r *repoURL) String() string {
func (r *Repo) String() string {
return fmt.Sprintf("%s-%s-%s", r.host, r.owner, r.repo)
}

// IsValid implements Repo.IsValid.
func (r *repoURL) IsValid() error {
func (r *Repo) IsValid() error {
githubHost := os.Getenv("GH_HOST")
switch r.host {
case "github.com":
Expand All @@ -103,16 +103,16 @@ func (r *repoURL) IsValid() error {
return nil
}

func (r *repoURL) AppendMetadata(metadata ...string) {
func (r *Repo) AppendMetadata(metadata ...string) {
r.metadata = append(r.metadata, metadata...)
}

// Metadata implements Repo.Metadata.
func (r *repoURL) Metadata() []string {
func (r *Repo) Metadata() []string {
return r.metadata
}

func (r *repoURL) commitExpression() string {
func (r *Repo) commitExpression() string {
if strings.EqualFold(r.commitSHA, clients.HeadSHA) {
// TODO(#575): Confirm that this works as expected.
return fmt.Sprintf("heads/%s", r.defaultBranch)
Expand All @@ -123,7 +123,7 @@ func (r *repoURL) commitExpression() string {
// MakeGithubRepo takes input of form "owner/repo" or "github.com/owner/repo"
// and returns an implementation of clients.Repo interface.
func MakeGithubRepo(input string) (clients.Repo, error) {
var repo repoURL
var repo Repo
if err := repo.parse(input); err != nil {
return nil, fmt.Errorf("error during parse: %w", err)
}
Expand All @@ -134,6 +134,6 @@ func MakeGithubRepo(input string) (clients.Repo, error) {
}

// Path() implements RepoClient.Path.
func (r *repoURL) Path() string {
func (r *Repo) Path() string {
return fmt.Sprintf("%s/%s", r.owner, r.repo)
}
Loading
Loading