Skip to content

Commit

Permalink
bug: Updated the host url for graphql querying. This enabled the remo…
Browse files Browse the repository at this point in the history 
…val of the code added for handling empty returns when executing against a non-gitlab.com repository.

Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
  • Loading branch information
@jimrobison
jimrobison committed May 24, 2023
1 parent c489cb3 commit 831e938
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 183 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,9 @@ The following checks are all run against the target project by default:

Name | Description | Risk Level | Token Required | GitLab Support | Note
----------- | ----------------------------------------- | ---------- | --------------- | -------------- | --- |
[Binary-Artifacts](docs/checks.md#binary-artifacts) | Is the project free of checked-in binaries? | High | PAT, GITHUB_TOKEN | Fully Supported |
[Binary-Artifacts](docs/checks.md#binary-artifacts) | Is the project free of checked-in binaries? | High | PAT, GITHUB_TOKEN | Supported |
[Branch-Protection](docs/checks.md#branch-protection) | Does the project use [Branch Protection](https://docs.github.com/en/free-pro-team@latest/github/administering-a-repository/about-protected-branches) ? | High | PAT (`repo` or `repo> public_repo`), GITHUB_TOKEN | Supported (see notes) | certain settings are only supported with a maintainer PAT
[CI-Tests](docs/checks.md#ci-tests) | Does the project run tests in CI, e.g. [GitHub Actions](https://docs.github.com/en/free-pro-team@latest/actions), [Prow](https://github.com/kubernetes/test-infra/tree/master/prow)? | Low | PAT, GITHUB_TOKEN | Validating
[CI-Tests](docs/checks.md#ci-tests) | Does the project run tests in CI, e.g. [GitHub Actions](https://docs.github.com/en/free-pro-team@latest/actions), [Prow](https://github.com/kubernetes/test-infra/tree/master/prow)? | Low | PAT, GITHUB_TOKEN | Supported
[CII-Best-Practices](docs/checks.md#cii-best-practices) | Has the project earned an [OpenSSF (formerly CII) Best Practices Badge](https://bestpractices.coreinfrastructure.org) at the passing, silver, or gold level? | Low | PAT, GITHUB_TOKEN | Validating |
[Code-Review](docs/checks.md#code-review) | Does the project practice code review before code is merged? | High | PAT, GITHUB_TOKEN | Validating |
[Contributors](docs/checks.md#contributors) | Does the project have contributors from at least two different organizations? | Low | PAT, GITHUB_TOKEN | Validating |
Expand Down
5 changes: 0 additions & 5 deletions clients/gitlabrepo/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,6 @@ func (client *Client) ListCommits() ([]clients.Commit, error) {
// are within them without making a REST call for each commit (~30 by default)
// Making 1 GraphQL query to combine the results of 2 REST calls, we avoid this
mrDetails, err := client.graphql.getMergeRequestsDetail(before)

if len(mrDetails.Project.MergeRequests.Nodes) == 0 {
return client.commits.queryMergeRequestsByCommits(commitsRaw), nil
}

if err != nil {
return []clients.Commit{}, err
}
Expand Down
69 changes: 5 additions & 64 deletions clients/gitlabrepo/commits.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,17 @@ import (
)

type commitsHandler struct {
listMergeRequestByCommit fnListMergeRequestsByCommit
glClient *gitlab.Client
once *sync.Once
errSetup error
repourl *repoURL
commitsRaw []*gitlab.Commit
glClient *gitlab.Client
once *sync.Once
errSetup error
repourl *repoURL
commitsRaw []*gitlab.Commit
}

type fnListMergeRequestsByCommit func(pid interface{}, sha string,
options ...gitlab.RequestOptionFunc) ([]*gitlab.MergeRequest, *gitlab.Response, error)

func (handler *commitsHandler) init(repourl *repoURL) {
handler.repourl = repourl
handler.errSetup = nil
handler.once = new(sync.Once)
handler.listMergeRequestByCommit = handler.glClient.Commits.ListMergeRequestsByCommit
}

func (handler *commitsHandler) setup() error {
Expand Down Expand Up @@ -70,60 +65,6 @@ func (handler *commitsHandler) listRawCommits() ([]*gitlab.Commit, error) {
return handler.commitsRaw, nil
}

func (handler *commitsHandler) queryMergeRequestsByCommits(commitsRaw []*gitlab.Commit) []clients.Commit {
commits := []clients.Commit{}
for _, c := range commitsRaw {
associatedMr := clients.PullRequest{}

mergeRequests, _, err := handler.listMergeRequestByCommit(handler.repourl.projectID, c.ID)
if err != nil {
handler.errSetup = fmt.Errorf("unable to find merge requests associated with commit: %w", err)
continue
}

if len(mergeRequests) > 0 {
mr := mergeRequests[0]
reviews := []clients.Review{}

for _, r := range mr.Reviewers {
reviews = append(reviews, clients.Review{
Author: &clients.User{
Login: r.Username,
ID: int64(r.ID),
},
State: r.State,
})
}

if mr.MergedAt != nil {
associatedMr = clients.PullRequest{
Number: mr.IID,
MergedAt: *mr.MergedAt,
HeadSHA: mr.MergeCommitSHA,
Author: clients.User{
Login: mr.Author.Username,
ID: int64(mr.Author.ID),
},
Reviews: reviews,
MergedBy: clients.User{
Login: mr.MergedBy.Username,
ID: int64(mr.MergedBy.ID),
},
}
}
}

commits = append(commits,
clients.Commit{
CommittedDate: *c.CommittedDate,
Message: c.Message,
SHA: c.ID,
AssociatedMergeRequest: associatedMr,
})
}
return commits
}

// zip combines Commit information from the GitLab REST API with MergeRequests
// information from the GitLab GraphQL API. The REST API doesn't provide any way to
// get from Commits -> MRs that they were part of or vice-versa (MRs -> commits they
Expand Down
111 changes: 0 additions & 111 deletions clients/gitlabrepo/commits_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ package gitlabrepo

import (
"context"
"sync"
"testing"
"time"

"github.com/xanzy/go-gitlab"
)

func Test_Setup(t *testing.T) {
Expand Down Expand Up @@ -105,110 +101,3 @@ func TestParsingEmail(t *testing.T) {
})
}
}

func TestQueryMergeRequestsByCommit(t *testing.T) {
t.Parallel()
tests := []struct {
name string
commits []*gitlab.Commit
mrs []*gitlab.MergeRequest
}{
{
name: "Commit with no MR",
commits: []*gitlab.Commit{
{
ID: "10",
CommittedDate: &time.Time{},
},
},
mrs: []*gitlab.MergeRequest{},
},
{
name: "Commit with a MR with Merged Date",
commits: []*gitlab.Commit{
{
ID: "10",
CommittedDate: &time.Time{},
},
},
mrs: []*gitlab.MergeRequest{
{
ID: 10,
MergedAt: gitlab.Time(time.Now()),
Author: &gitlab.BasicUser{
ID: 100,
Username: "no-one",
},
MergedBy: &gitlab.BasicUser{
ID: 101,
Username: "da-approver",
},
Reviewers: []*gitlab.BasicUser{
{
ID: 102,
Username: "da-manager",
},
},
},
},
},
{
name: "Commit with a MR Not Merged",
commits: []*gitlab.Commit{
{
ID: "10",
CommittedDate: &time.Time{},
},
},
mrs: []*gitlab.MergeRequest{
{
ID: 10,
MergedAt: nil,
Author: &gitlab.BasicUser{
ID: 100,
Username: "no-one",
},
Reviewers: []*gitlab.BasicUser{},
},
},
},
}

for _, tt := range tests {
tt := tt

t.Run(tt.name, func(t *testing.T) {
t.Parallel()

handler := commitsHandler{
once: new(sync.Once),
repourl: &repoURL{
projectID: "5000",
},
listMergeRequestByCommit: func(pid interface{}, sha string,
options ...gitlab.RequestOptionFunc) (
[]*gitlab.MergeRequest, *gitlab.Response, error,
) {
return tt.mrs, nil, nil
},
}
handler.once.Do(func() {})

c := handler.queryMergeRequestsByCommits(tt.commits)

if len(c) != len(tt.commits) {
t.Errorf("Return commit count should equal what was sent in")
}

if len(tt.mrs) > 0 && tt.mrs[0].MergedAt != nil &&
c[0].AssociatedMergeRequest.Author.ID != int64(tt.mrs[0].Author.ID) {
t.Errorf("MR should have been associated to the commit")
}

if len(tt.mrs) > 0 && tt.mrs[0].MergedAt == nil &&
c[0].AssociatedMergeRequest.Author.ID != 0 {
t.Errorf("MR should NOT have been associated to the commit")
}
})
}
}
2 changes: 1 addition & 1 deletion clients/gitlabrepo/graphql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (handler *graphqlHandler) init(ctx context.Context, repourl *repoURL) {
&oauth2.Token{AccessToken: os.Getenv("GITLAB_AUTH_TOKEN")},
)
handler.client = oauth2.NewClient(context.Background(), src)
handler.graphClient = graphql.NewClient("https://gitlab.com/api/graphql", handler.client)
handler.graphClient = graphql.NewClient(fmt.Sprintf("%s/api/graphql", repourl.Host()), handler.client)
}

//nolint:govet
Expand Down

0 comments on commit 831e938

Please sign in to comment.