Skip to content

Commit c16bee3

Browse files
TP HoneyDan Wilson
TP Honey
and
Dan Wilson
authored
(fix) handle nil repos in github responses (#168)
* (fix) handle nil repos in github responses * convertRepoList change and tests * add new line at end of test file * implement Brad's suggested changes Co-authored-by: Dan Wilson <[email protected]>
1 parent f73f51e commit c16bee3

File tree

3 files changed

+209
-1
lines changed

3 files changed

+209
-1
lines changed

‎scm/driver/github/repo.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package github
66

77
import (
88
"context"
9+
"errors"
910
"fmt"
1011
"strconv"
1112
"time"
@@ -63,6 +64,13 @@ func (s *RepositoryService) Find(ctx context.Context, repo string) (*scm.Reposit
6364
path := fmt.Sprintf("repos/%s", repo)
6465
out := new(repository)
6566
res, err := s.client.do(ctx, "GET", path, nil, out)
67+
if err != nil {
68+
return nil, res, err
69+
}
70+
convertedRepo := convertRepository(out)
71+
if convertedRepo == nil {
72+
return nil, res, errors.New("GitHub returned an unexpected null repository")
73+
}
6674
return convertRepository(out), res, err
6775
}
6876

@@ -79,6 +87,13 @@ func (s *RepositoryService) FindPerms(ctx context.Context, repo string) (*scm.Pe
7987
path := fmt.Sprintf("repos/%s", repo)
8088
out := new(repository)
8189
res, err := s.client.do(ctx, "GET", path, nil, out)
90+
if err != nil {
91+
return nil, res, err
92+
}
93+
convertedRepo := convertRepository(out)
94+
if convertedRepo == nil {
95+
return nil, res, errors.New("GitHub returned an unexpected null repository")
96+
}
8297
return convertRepository(out).Perm, res, err
8398
}
8499

@@ -187,14 +202,19 @@ func (s *RepositoryService) DeleteHook(ctx context.Context, repo, id string) (*s
187202
func convertRepositoryList(from []*repository) []*scm.Repository {
188203
to := []*scm.Repository{}
189204
for _, v := range from {
190-
to = append(to, convertRepository(v))
205+
if repo := convertRepository(v); repo != nil {
206+
to = append(to, repo)
207+
}
191208
}
192209
return to
193210
}
194211

195212
// helper function to convert from the gogs repository structure
196213
// to the common repository structure.
197214
func convertRepository(from *repository) *scm.Repository {
215+
if from == nil {
216+
return nil
217+
}
198218
return &scm.Repository{
199219
ID: strconv.Itoa(from.ID),
200220
Name: from.Name,

‎scm/driver/github/repo_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"context"
99
"encoding/json"
1010
"io/ioutil"
11+
"reflect"
1112
"testing"
1213

1314
"github.com/drone/go-scm/scm"
@@ -528,3 +529,77 @@ func TestHookEvents(t *testing.T) {
528529
}
529530
}
530531
}
532+
533+
func Test_convertRepository(t *testing.T) {
534+
permissionsStruct := &repository{
535+
Name: "bla",
536+
}
537+
permissionsStruct.Permissions.Admin = true
538+
tests := []struct {
539+
name string
540+
from *repository
541+
want *scm.Repository
542+
}{
543+
{
544+
name: "Simple",
545+
from: &repository{
546+
Name: "hello-world",
547+
ID: 1,
548+
Permissions: permissionsStruct.Permissions,
549+
},
550+
want: &scm.Repository{
551+
Name: "hello-world",
552+
ID: "1",
553+
Perm: &scm.Perm{
554+
Admin: true,
555+
},
556+
},
557+
},
558+
{
559+
name: "null",
560+
from: nil,
561+
want: nil,
562+
},
563+
}
564+
for _, tt := range tests {
565+
t.Run(tt.name, func(t *testing.T) {
566+
if got := convertRepository(tt.from); !reflect.DeepEqual(got, tt.want) {
567+
t.Errorf("convertRepository() = %v, want %v", got, tt.want)
568+
}
569+
})
570+
}
571+
}
572+
573+
func TestRepositoryListWithNull(t *testing.T) {
574+
defer gock.Off()
575+
576+
gock.New("https://api.github.com").
577+
Get("/user/repos").
578+
MatchParam("page", "1").
579+
MatchParam("per_page", "30").
580+
Reply(200).
581+
Type("application/json").
582+
SetHeaders(mockHeaders).
583+
SetHeaders(mockPageHeaders).
584+
File("testdata/reposWithNull.json")
585+
586+
client := NewDefault()
587+
got, res, err := client.Repositories.List(context.Background(), scm.ListOptions{Page: 1, Size: 30})
588+
if err != nil {
589+
t.Error(err)
590+
return
591+
}
592+
593+
want := []*scm.Repository{}
594+
raw, _ := ioutil.ReadFile("testdata/repos.json.golden")
595+
_ = json.Unmarshal(raw, &want)
596+
597+
if diff := cmp.Diff(got, want); diff != "" {
598+
t.Errorf("Unexpected Results")
599+
t.Log(diff)
600+
}
601+
602+
t.Run("Request", testRequest(res))
603+
t.Run("Rate", testRate(res))
604+
t.Run("Page", testPage(res))
605+
}

‎scm/driver/github/testdata/reposWithNull.json‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
[
2+
{
3+
"id": 1296269,
4+
"owner": {
5+
"login": "octocat",
6+
"id": 1,
7+
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
8+
"gravatar_id": "",
9+
"url": "https://api.github.com/users/octocat",
10+
"html_url": "https://github.com/octocat",
11+
"followers_url": "https://api.github.com/users/octocat/followers",
12+
"following_url": "https://api.github.com/users/octocat/following{/other_user}",
13+
"gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
14+
"starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
15+
"subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
16+
"organizations_url": "https://api.github.com/users/octocat/orgs",
17+
"repos_url": "https://api.github.com/users/octocat/repos",
18+
"events_url": "https://api.github.com/users/octocat/events{/privacy}",
19+
"received_events_url": "https://api.github.com/users/octocat/received_events",
20+
"type": "User",
21+
"site_admin": false
22+
},
23+
"name": "Hello-World",
24+
"full_name": "octocat/Hello-World",
25+
"description": "This your first repo!",
26+
"private": true,
27+
"fork": true,
28+
"visibility": "public",
29+
"url": "https://api.github.com/repos/octocat/Hello-World",
30+
"html_url": "https://github.com/octocat/Hello-World",
31+
"archive_url": "http://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}",
32+
"assignees_url": "http://api.github.com/repos/octocat/Hello-World/assignees{/user}",
33+
"blobs_url": "http://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}",
34+
"branches_url": "http://api.github.com/repos/octocat/Hello-World/branches{/branch}",
35+
"clone_url": "https://github.com/octocat/Hello-World.git",
36+
"collaborators_url": "http://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}",
37+
"comments_url": "http://api.github.com/repos/octocat/Hello-World/comments{/number}",
38+
"commits_url": "http://api.github.com/repos/octocat/Hello-World/commits{/sha}",
39+
"compare_url": "http://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}",
40+
"contents_url": "http://api.github.com/repos/octocat/Hello-World/contents/{+path}",
41+
"contributors_url": "http://api.github.com/repos/octocat/Hello-World/contributors",
42+
"deployments_url": "http://api.github.com/repos/octocat/Hello-World/deployments",
43+
"downloads_url": "http://api.github.com/repos/octocat/Hello-World/downloads",
44+
"events_url": "http://api.github.com/repos/octocat/Hello-World/events",
45+
"forks_url": "http://api.github.com/repos/octocat/Hello-World/forks",
46+
"git_commits_url": "http://api.github.com/repos/octocat/Hello-World/git/commits{/sha}",
47+
"git_refs_url": "http://api.github.com/repos/octocat/Hello-World/git/refs{/sha}",
48+
"git_tags_url": "http://api.github.com/repos/octocat/Hello-World/git/tags{/sha}",
49+
"git_url": "git:github.com/octocat/Hello-World.git",
50+
"hooks_url": "http://api.github.com/repos/octocat/Hello-World/hooks",
51+
"issue_comment_url": "http://api.github.com/repos/octocat/Hello-World/issues/comments{/number}",
52+
"issue_events_url": "http://api.github.com/repos/octocat/Hello-World/issues/events{/number}",
53+
"issues_url": "http://api.github.com/repos/octocat/Hello-World/issues{/number}",
54+
"keys_url": "http://api.github.com/repos/octocat/Hello-World/keys{/key_id}",
55+
"labels_url": "http://api.github.com/repos/octocat/Hello-World/labels{/name}",
56+
"languages_url": "http://api.github.com/repos/octocat/Hello-World/languages",
57+
"merges_url": "http://api.github.com/repos/octocat/Hello-World/merges",
58+
"milestones_url": "http://api.github.com/repos/octocat/Hello-World/milestones{/number}",
59+
"mirror_url": "git:git.example.com/octocat/Hello-World",
60+
"notifications_url": "http://api.github.com/repos/octocat/Hello-World/notifications{?since, all, participating}",
61+
"pulls_url": "http://api.github.com/repos/octocat/Hello-World/pulls{/number}",
62+
"releases_url": "http://api.github.com/repos/octocat/Hello-World/releases{/id}",
63+
"ssh_url": "[email protected]:octocat/Hello-World.git",
64+
"stargazers_url": "http://api.github.com/repos/octocat/Hello-World/stargazers",
65+
"statuses_url": "http://api.github.com/repos/octocat/Hello-World/statuses/{sha}",
66+
"subscribers_url": "http://api.github.com/repos/octocat/Hello-World/subscribers",
67+
"subscription_url": "http://api.github.com/repos/octocat/Hello-World/subscription",
68+
"svn_url": "https://svn.github.com/octocat/Hello-World",
69+
"tags_url": "http://api.github.com/repos/octocat/Hello-World/tags",
70+
"teams_url": "http://api.github.com/repos/octocat/Hello-World/teams",
71+
"trees_url": "http://api.github.com/repos/octocat/Hello-World/git/trees{/sha}",
72+
"homepage": "https://github.com",
73+
"language": null,
74+
"forks_count": 9,
75+
"stargazers_count": 80,
76+
"watchers_count": 80,
77+
"size": 108,
78+
"default_branch": "master",
79+
"open_issues_count": 0,
80+
"topics": [
81+
"octocat",
82+
"atom",
83+
"electron",
84+
"API"
85+
],
86+
"has_issues": true,
87+
"has_wiki": true,
88+
"has_pages": false,
89+
"has_downloads": true,
90+
"archived": false,
91+
"pushed_at": "2011-01-26T19:06:43Z",
92+
"created_at": "2011-01-26T19:01:12Z",
93+
"updated_at": "2011-01-26T19:14:43Z",
94+
"permissions": {
95+
"admin": true,
96+
"push": true,
97+
"pull": true
98+
},
99+
"allow_rebase_merge": true,
100+
"allow_squash_merge": true,
101+
"allow_merge_commit": true,
102+
"subscribers_count": 42,
103+
"network_count": 0,
104+
"license": {
105+
"key": "mit",
106+
"name": "MIT License",
107+
"spdx_id": "MIT",
108+
"url": "https://api.github.com/licenses/mit",
109+
"html_url": "http://choosealicense.com/licenses/mit/"
110+
}
111+
},
112+
null
113+
]

0 commit comments

Comments
 (0)