Skip to content

Commit

Permalink
Refactor githubclient package
Browse files Browse the repository at this point in the history
This commit addresses a couple of different issues with this package.

Previously, everything was lumped in to `client.go`. This was fine
initially but it became harder to manage as the codebase grew.
Domains are now separated in by file.
Future work is probably needed to subdivide each domain in to it's own
package to prevent leakage.

Secondly, the methods for Tags and Pull were a bit scrappy and not very
efficient.
This was a left over from the "get it done" phase of the
project. These instances have now been resolved and I think the
implementation is closed to O(n) (maybe.. either way it's less garbage
than it was before).
  • Loading branch information
chelnak committed Jul 10, 2022
1 parent fda6fb2 commit e753318
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 158 deletions.
2 changes: 1 addition & 1 deletion internal/changelog/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func hasExcludedLabel(excludedLabels []string, pr githubclient.PullRequest) bool
return false
}

func getSection(labels []githubclient.Label) string {
func getSection(labels []githubclient.PullRequestLabel) string {
sections := configuration.Config.Sections

lookup := make(map[string]string)
Expand Down
4 changes: 2 additions & 2 deletions internal/changelog/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func setupMockGitHubClient() *mocks.GitHubClient {
Number: 1,
Title: "this is a test pr",
User: "test-user",
Labels: []githubclient.Label{
Labels: []githubclient.PullRequestLabel{
{
Name: "enhancement",
},
Expand All @@ -61,7 +61,7 @@ func setupMockGitHubClient() *mocks.GitHubClient {
Number: 2,
Title: "this is a test pr 2",
User: "test-user",
Labels: []githubclient.Label{
Labels: []githubclient.PullRequestLabel{
{
Name: "enhancement",
},
Expand Down
155 changes: 0 additions & 155 deletions internal/githubclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package githubclient
import (
"context"
"fmt"
"sort"
"strings"
"time"

Expand All @@ -32,160 +31,6 @@ type githubClient struct {
httpContext context.Context
}

type Tag struct {
Name string
Sha string
Date time.Time
}

type Label struct {
Name string
}

type PullRequest struct {
Number int
Title string
User string
Labels []Label
}

var TagQuery struct {
Repository struct {
Refs struct {
Nodes []struct {
Name string
Target struct {
TypeName string `graphql:"__typename"`
Tag struct {
Oid string
Tagger struct {
Date time.Time
}
} `graphql:"... on Tag"`
Commit struct {
Oid string
Committer struct {
Date time.Time
}
} `graphql:"... on Commit"`
}
}
PageInfo struct {
EndCursor githubv4.String
HasNextPage bool
}
} `graphql:"refs(refPrefix: \"refs/tags/\", last: 100, after: $cursor)"`
} `graphql:"repository(owner:$repositoryOwner,name:$repositoryName)"`
}

func (client *githubClient) GetTags() ([]Tag, error) {
variables := map[string]interface{}{
"repositoryOwner": githubv4.String(client.repoContext.owner),
"repositoryName": githubv4.String(client.repoContext.name),
"cursor": (*githubv4.String)(nil),
}

var tags []Tag

for {
err := client.base.Query(client.httpContext, &TagQuery, variables)
if err != nil {
return nil, fmt.Errorf("error getting tags: %w", err)
}

for _, node := range TagQuery.Repository.Refs.Nodes {
switch node.Target.TypeName {
case "Tag":
tags = append(tags, Tag{
Name: node.Name,
Sha: node.Target.Tag.Oid,
Date: node.Target.Tag.Tagger.Date,
})
case "Commit":
tags = append(tags, Tag{
Name: node.Name,
Sha: node.Target.Commit.Oid,
Date: node.Target.Commit.Committer.Date,
})
}
}

if !TagQuery.Repository.Refs.PageInfo.HasNextPage {
break
}

variables["cursor"] = TagQuery.Repository.Refs.PageInfo.EndCursor
}

sort.Slice(tags, func(i, j int) bool {
return tags[i].Date.After(tags[j].Date)
})

return tags, nil
}

func (client *githubClient) GetPullRequestsBetweenDates(fromDate, toDate time.Time) ([]PullRequest, error) {
var pullRequestSearchQuery struct {
Search struct {
Edges []struct {
Node struct {
PullRequest struct {
Number int
Title string
Author struct {
Login string
}
Labels struct {
Nodes []struct {
Name string
}
} `graphql:" labels(first: 100)"`
} `graphql:"... on PullRequest"`
}
}
PageInfo struct {
EndCursor githubv4.String
HasNextPage bool
}
} `graphql:"search(query: $query, type: ISSUE, first: 100, after: $cursor)"`
}

variables := map[string]interface{}{
"query": githubv4.String(fmt.Sprintf(`repo:%s/%s is:pr is:merged merged:%s..%s`, client.repoContext.owner, client.repoContext.name, fromDate.Local().Format(time.RFC3339), toDate.Local().Format(time.RFC3339))),
"cursor": (*githubv4.String)(nil),
}

var pullRequests []PullRequest

for {
err := client.base.Query(client.httpContext, &pullRequestSearchQuery, variables)
if err != nil {
return nil, err
}

for _, edge := range pullRequestSearchQuery.Search.Edges {
pullRequests = append(pullRequests, PullRequest{
Number: edge.Node.PullRequest.Number,
Title: edge.Node.PullRequest.Title,
User: edge.Node.PullRequest.Author.Login,
Labels: make([]Label, len(edge.Node.PullRequest.Labels.Nodes)),
})

for i, label := range edge.Node.PullRequest.Labels.Nodes {
pullRequests[len(pullRequests)-1].Labels[i] = Label{
Name: label.Name,
}
}
}

if !pullRequestSearchQuery.Search.PageInfo.HasNextPage {
break
}
variables["cursor"] = pullRequestSearchQuery.Search.PageInfo.EndCursor
}
return pullRequests, nil
}

func (client *githubClient) GetRepoName() string {
return client.repoContext.name
}
Expand Down
88 changes: 88 additions & 0 deletions internal/githubclient/pull_requests.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package githubclient

import (
"fmt"
"time"

"github.com/shurcooL/githubv4"
)

type PullRequestLabel struct {
Name string
}

type PullRequestEdge struct {
Node struct {
PullRequest struct {
Number int
Title string
Author struct {
Login string
}
Labels struct {
Nodes []PullRequestLabel
} `graphql:"labels(first: 100)"`
} `graphql:"... on PullRequest"`
}
}

type PullRequestSearchQuery struct {
Search struct {
Edges []PullRequestEdge
PageInfo struct {
EndCursor githubv4.String
HasNextPage bool
}
} `graphql:"search(query: $query, type: ISSUE, first: 100, after: $cursor)"`
}

type PullRequest struct {
Number int
Title string
User string
Labels []PullRequestLabel
}

func (client *githubClient) GetPullRequestsBetweenDates(fromDate, toDate time.Time) ([]PullRequest, error) {
variables := map[string]interface{}{
"query": githubv4.String(
fmt.Sprintf(
`repo:%s/%s is:pr is:merged merged:%s..%s`,
client.repoContext.owner,
client.repoContext.name,
fromDate.Local().Format(time.RFC3339),
toDate.Local().Format(time.RFC3339),
),
),
"cursor": (*githubv4.String)(nil),
}

var pullRequestSearchQuery PullRequestSearchQuery
var pullRequests []PullRequest
var edges []PullRequestEdge

for {
err := client.base.Query(client.httpContext, &pullRequestSearchQuery, variables)
if err != nil {
return nil, err
}

edges = append(edges, pullRequestSearchQuery.Search.Edges...)

if !pullRequestSearchQuery.Search.PageInfo.HasNextPage {
break
}
variables["cursor"] = pullRequestSearchQuery.Search.PageInfo.EndCursor
}

for _, edge := range edges {
pullRequests = append(pullRequests, PullRequest{
Number: edge.Node.PullRequest.Number,
Title: edge.Node.PullRequest.Title,
User: edge.Node.PullRequest.Author.Login,
Labels: edge.Node.PullRequest.Labels.Nodes,
})
}

return pullRequests, nil
}
96 changes: 96 additions & 0 deletions internal/githubclient/tags.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package githubclient

import (
"fmt"
"sort"
"time"

"github.com/shurcooL/githubv4"
)

type RefNode struct {
Name string
Target struct {
TypeName string `graphql:"__typename"`
Tag struct {
Oid string
Tagger struct {
Date time.Time
}
} `graphql:"... on Tag"`
Commit struct {
Oid string
Committer struct {
Date time.Time
}
} `graphql:"... on Commit"`
}
}

type TagQuery struct {
Repository struct {
Refs struct {
Nodes []RefNode
PageInfo struct {
EndCursor githubv4.String
HasNextPage bool
}
} `graphql:"refs(refPrefix: \"refs/tags/\", last: 100, after: $cursor)"`
} `graphql:"repository(owner:$repositoryOwner,name:$repositoryName)"`
}

type Tag struct {
Name string
Sha string
Date time.Time
}

func (client *githubClient) GetTags() ([]Tag, error) {
variables := map[string]interface{}{
"repositoryOwner": githubv4.String(client.repoContext.owner),
"repositoryName": githubv4.String(client.repoContext.name),
"cursor": (*githubv4.String)(nil),
}

var tags []Tag
var tagQuery TagQuery
var nodes []RefNode

for {
err := client.base.Query(client.httpContext, &tagQuery, variables)
if err != nil {
return nil, fmt.Errorf("error getting tags: %w", err)
}

nodes = append(nodes, tagQuery.Repository.Refs.Nodes...)

if !tagQuery.Repository.Refs.PageInfo.HasNextPage {
break
}

variables["cursor"] = tagQuery.Repository.Refs.PageInfo.EndCursor
}

for _, node := range nodes {
switch node.Target.TypeName {
case "Tag":
tags = append(tags, Tag{
Name: node.Name,
Sha: node.Target.Tag.Oid,
Date: node.Target.Tag.Tagger.Date,
})
case "Commit":
tags = append(tags, Tag{
Name: node.Name,
Sha: node.Target.Commit.Oid,
Date: node.Target.Commit.Committer.Date,
})
}
}

sort.Slice(tags, func(i, j int) bool {
return tags[i].Date.After(tags[j].Date)
})

return tags, nil
}

0 comments on commit e753318

Please sign in to comment.