Skip to content

Commit 14a5bd7

Browse files
bradrydzewskibradrydzewski
authored
Merge pull request #42 from chhsia0/cleanup
Cleaned up content service and added missing tests.
2 parents 3731ec1 + 6630d78 commit 14a5bd7

File tree

9 files changed

+79
-78
lines changed

9 files changed

+79
-78
lines changed

‎scm/driver/gitea/git.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ func (s *gitService) FindBranch(ctx context.Context, repo, name string) (*scm.Re
2525
}
2626

2727
func (s *gitService) FindCommit(ctx context.Context, repo, ref string) (*scm.Commit, *scm.Response, error) {
28-
ref = url.PathEscape(ref)
29-
path := fmt.Sprintf("api/v1/repos/%s/git/commits/%s", repo, ref)
28+
path := fmt.Sprintf("api/v1/repos/%s/git/commits/%s", repo, url.PathEscape(ref))
3029
out := new(commitInfo)
3130
res, err := s.client.do(ctx, "GET", path, nil, out)
3231
return convertCommitInfo(out), res, err

‎scm/driver/gitea/git_test.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
// commit sub-tests
2020
//
2121

22-
func TestCommitFind(t *testing.T) {
22+
func TestGitFindCommit(t *testing.T) {
2323
gock.New("https://try.gitea.io").
2424
Get("/api/v1/repos/gitea/gitea/git/commits/c43399cad8766ee521b873a32c1652407c5a4630").
2525
Reply(200).
@@ -46,27 +46,41 @@ func TestCommitFind(t *testing.T) {
4646
}
4747
}
4848

49-
func TestCommitList(t *testing.T) {
49+
func TestGitListCommits(t *testing.T) {
5050
client, _ := New("https://try.gitea.io")
5151
_, _, err := client.Git.ListCommits(context.Background(), "go-gitea/gitea", scm.CommitListOptions{})
5252
if err != scm.ErrNotSupported {
5353
t.Errorf("Expect Not Supported error")
5454
}
5555
}
5656

57-
func TestChangeList(t *testing.T) {
57+
func TestGitListChanges(t *testing.T) {
5858
client, _ := New("https://try.gitea.io")
5959
_, _, err := client.Git.ListChanges(context.Background(), "go-gitea/gitea", "f05f642b892d59a0a9ef6a31f6c905a24b5db13a", scm.ListOptions{})
6060
if err != scm.ErrNotSupported {
6161
t.Errorf("Expect Not Supported error")
6262
}
6363
}
6464

65+
func TestGitCompareChanges(t *testing.T) {
66+
client, _ := New("https://try.gitea.io")
67+
_, _, err := client.Git.CompareChanges(
68+
context.Background(),
69+
"go-gitea/gitea",
70+
"d293a2b9d6722dffde7998c953c3087e47a38a83",
71+
"f05f642b892d59a0a9ef6a31f6c905a24b5db13a",
72+
scm.ListOptions{},
73+
)
74+
if err != scm.ErrNotSupported {
75+
t.Errorf("Expect Not Supported error")
76+
}
77+
}
78+
6579
//
6680
// branch sub-tests
6781
//
6882

69-
func TestBranchFind(t *testing.T) {
83+
func TestGitFindBranch(t *testing.T) {
7084
defer gock.Off()
7185

7286
gock.New("https://try.gitea.io").
@@ -91,7 +105,7 @@ func TestBranchFind(t *testing.T) {
91105
}
92106
}
93107

94-
func TestBranchList(t *testing.T) {
108+
func TestGitListBranches(t *testing.T) {
95109
defer gock.Off()
96110

97111
gock.New("https://try.gitea.io").
@@ -120,15 +134,15 @@ func TestBranchList(t *testing.T) {
120134
// tag sub-tests
121135
//
122136

123-
func TestTagFind(t *testing.T) {
137+
func TestGitFindTag(t *testing.T) {
124138
client, _ := New("https://try.gitea.io")
125139
_, _, err := client.Git.FindTag(context.Background(), "go-gitea/gitea", "v1.0.0")
126140
if err != scm.ErrNotSupported {
127141
t.Errorf("Expect Not Supported error")
128142
}
129143
}
130144

131-
func TestTagList(t *testing.T) {
145+
func TestGitListTags(t *testing.T) {
132146
client, _ := New("https://try.gitea.io")
133147
_, _, err := client.Git.ListTags(context.Background(), "go-gitea/gitea", scm.ListOptions{})
134148
if err != scm.ErrNotSupported {

‎scm/driver/gitlab/content.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"fmt"
1111
"net/url"
1212
"strconv"
13-
"strings"
1413

1514
"github.com/drone/go-scm/scm"
1615
)
@@ -20,9 +19,7 @@ type contentService struct {
2019
}
2120

2221
func (s *contentService) Find(ctx context.Context, repo, path, ref string) (*scm.Content, *scm.Response, error) {
23-
path = url.QueryEscape(path)
24-
path = strings.Replace(path, ".", "%2E", -1)
25-
endpoint := fmt.Sprintf("api/v4/projects/%s/repository/files/%s?ref=%s", encode(repo), path, ref)
22+
endpoint := fmt.Sprintf("api/v4/projects/%s/repository/files/%s?ref=%s", encode(repo), encodePath(path), ref)
2623
out := new(content)
2724
res, err := s.client.do(ctx, "GET", endpoint, nil, out)
2825
raw, berr := base64.StdEncoding.DecodeString(out.Content)
@@ -41,9 +38,7 @@ func (s *contentService) Find(ctx context.Context, repo, path, ref string) (*scm
4138
}
4239

4340
func (s *contentService) Create(ctx context.Context, repo, path string, params *scm.ContentParams) (*scm.Response, error) {
44-
path = url.QueryEscape(path)
45-
path = strings.Replace(path, ".", "%2E", -1)
46-
endpoint := fmt.Sprintf("api/v4/projects/%s/repository/files/%s", encode(repo), path)
41+
endpoint := fmt.Sprintf("api/v4/projects/%s/repository/files/%s", encode(repo), encodePath(path))
4742
in := &createUpdateContent{
4843
Branch: params.Branch,
4944
Content: params.Data,
@@ -58,9 +53,7 @@ func (s *contentService) Create(ctx context.Context, repo, path string, params *
5853
}
5954

6055
func (s *contentService) Update(ctx context.Context, repo, path string, params *scm.ContentParams) (*scm.Response, error) {
61-
path = url.QueryEscape(path)
62-
path = strings.Replace(path, ".", "%2E", -1)
63-
endpoint := fmt.Sprintf("api/v4/projects/%s/repository/files/%s", encode(repo), path)
56+
endpoint := fmt.Sprintf("api/v4/projects/%s/repository/files/%s", encode(repo), encodePath(path))
6457
in := &createUpdateContent{
6558
Branch: params.Branch,
6659
Content: params.Data,

‎scm/driver/gitlab/content_test.go

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -158,41 +158,3 @@ func TestContentList(t *testing.T) {
158158
t.Run("Request", testRequest(res))
159159
t.Run("Rate", testRate(res))
160160
}
161-
162-
var fileContent = []byte(`require 'digest/md5'
163-
164-
class Key < ActiveRecord::Base
165-
include Gitlab::CurrentSettings
166-
include Sortable
167-
168-
belongs_to :user
169-
170-
before_validation :generate_fingerprint
171-
172-
validates :title,
173-
presence: true,
174-
length: { maximum: 255 }
175-
176-
validates :key,
177-
presence: true,
178-
length: { maximum: 5000 },
179-
format: { with: /\A(ssh|ecdsa)-.*\Z/ }
180-
181-
validates :fingerprint,
182-
uniqueness: true,
183-
presence: { message: 'cannot be generated' }
184-
185-
validate :key_meets_restrictions
186-
187-
delegate :name, :email, to: :user, prefix: true
188-
189-
after_commit :add_to_shell, on: :create
190-
after_create :post_create_hook
191-
after_create :refresh_user_cache
192-
after_commit :remove_from_shell, on: :destroy
193-
after_destroy :post_destroy_hook
194-
after_destroy :refresh_user_cache
195-
196-
def key=(value)
197-
value&.delete!("\n\r")
198-
value.strip! unless value.blank`)

‎scm/driver/gitlab/integration/issue_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func testIssue(issue *scm.Issue) func(t *testing.T) {
103103
if got, want := issue.Title, "New issue"; got != want {
104104
t.Errorf("Want issue Title %q, got %q", want, got)
105105
}
106-
if got, want := issue.Link, "https://gitlab.com/gitlab-org/testme/issues/1"; got != want {
106+
if got, want := issue.Link, "https://gitlab.com/gitlab-org/testme/-/issues/1"; got != want {
107107
t.Errorf("Want issue Title %q, got %q", want, got)
108108
}
109109
if got, want := issue.Author.Login, "marin"; got != want {

‎scm/driver/gitlab/integration/pr_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func testPullRequest(pr *scm.PullRequest) func(t *testing.T) {
147147
if got, want := pr.Sha, "12d65c8dd2b2676fa3ac47d955accc085a37a9c1"; got != want {
148148
t.Errorf("Want pr Sha %q, got %q", want, got)
149149
}
150-
if got, want := pr.Link, "https://gitlab.com/gitlab-org/testme/merge_requests/1"; got != want {
150+
if got, want := pr.Link, "https://gitlab.com/gitlab-org/testme/-/merge_requests/1"; got != want {
151151
t.Errorf("Want pr Link %q, got %q", want, got)
152152
}
153153
if got, want := pr.Author.Login, "dblessing"; got != want {

‎scm/driver/gitlab/testdata/content.rb

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
require 'digest/md5'
2+
3+
class Key < ActiveRecord::Base
4+
include Gitlab::CurrentSettings
5+
include Sortable
6+
7+
belongs_to :user
8+
9+
before_validation :generate_fingerprint
10+
11+
validates :title,
12+
presence: true,
13+
length: { maximum: 255 }
14+
15+
validates :key,
16+
presence: true,
17+
length: { maximum: 5000 },
18+
format: { with: /\A(ssh|ecdsa)-.*\Z/ }
19+
20+
validates :fingerprint,
21+
uniqueness: true,
22+
presence: { message: 'cannot be generated' }
23+
24+
validate :key_meets_restrictions
25+
26+
delegate :name, :email, to: :user, prefix: true
27+
28+
after_commit :add_to_shell, on: :create
29+
after_create :post_create_hook
30+
after_create :refresh_user_cache
31+
after_commit :remove_from_shell, on: :destroy
32+
after_destroy :post_destroy_hook
33+
after_destroy :refresh_user_cache
34+
35+
def key=(value)
36+
value&.delete!("\n\r")
37+
value.strip! unless value.blank

‎scm/driver/gitlab/util.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ func encode(s string) string {
1616
return strings.Replace(s, "/", "%2F", -1)
1717
}
1818

19+
func encodePath(s string) string {
20+
// Gitlab documentation provides inconsistent example for whether '.' should be escaped:
21+
// https://docs.gitlab.com/ee/api/README.html#file-path-branches-and-tags-name-encoding
22+
// https://docs.gitlab.com/ee/api/repository_files.html#get-file-from-repository
23+
// Although not escaping '.' seems to work, we still escape it here to be safe.
24+
return strings.Replace(url.PathEscape(s), ".", "%2E", -1)
25+
}
26+
1927
func encodeListOptions(opts scm.ListOptions) string {
2028
params := url.Values{}
2129
if opts.Page != 0 {

‎scm/driver/stash/git.go

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,7 @@ func (s *gitService) ListBranches(ctx context.Context, repo string, opts scm.Lis
6464
path := fmt.Sprintf("rest/api/1.0/projects/%s/repos/%s/branches?%s", namespace, name, encodeListOptions(opts))
6565
out := new(branches)
6666
res, err := s.client.do(ctx, "GET", path, nil, out)
67-
if !out.pagination.LastPage.Bool {
68-
res.Page.First = 1
69-
res.Page.Next = opts.Page + 1
70-
}
67+
copyPagination(out.pagination, res)
7168
return convertBranchList(out), res, err
7269
}
7370

@@ -79,35 +76,26 @@ func (s *gitService) ListTags(ctx context.Context, repo string, opts scm.ListOpt
7976
namespace, name := scm.Split(repo)
8077
path := fmt.Sprintf("rest/api/1.0/projects/%s/repos/%s/tags?%s", namespace, name, encodeListOptions(opts))
8178
out := new(branches)
82-
res, err := s.client.do(ctx, "GET", path, nil, &out)
83-
if !out.pagination.LastPage.Bool {
84-
res.Page.First = 1
85-
res.Page.Next = opts.Page + 1
86-
}
79+
res, err := s.client.do(ctx, "GET", path, nil, out)
80+
copyPagination(out.pagination, res)
8781
return convertTagList(out), res, err
8882
}
8983

9084
func (s *gitService) ListChanges(ctx context.Context, repo, ref string, opts scm.ListOptions) ([]*scm.Change, *scm.Response, error) {
9185
namespace, name := scm.Split(repo)
9286
path := fmt.Sprintf("rest/api/1.0/projects/%s/repos/%s/commits/%s/changes?%s", namespace, name, ref, encodeListOptions(opts))
9387
out := new(diffstats)
94-
res, err := s.client.do(ctx, "GET", path, nil, &out)
95-
if !out.pagination.LastPage.Bool {
96-
res.Page.First = 1
97-
res.Page.Next = opts.Page + 1
98-
}
88+
res, err := s.client.do(ctx, "GET", path, nil, out)
89+
copyPagination(out.pagination, res)
9990
return convertDiffstats(out), res, err
10091
}
10192

10293
func (s *gitService) CompareChanges(ctx context.Context, repo, source, target string, opts scm.ListOptions) ([]*scm.Change, *scm.Response, error) {
10394
namespace, name := scm.Split(repo)
10495
path := fmt.Sprintf("rest/api/1.0/projects/%s/repos/%s/compare/changes?from=%s&to=%s&%s", namespace, name, source, target, encodeListOptions(opts))
10596
out := new(diffstats)
106-
res, err := s.client.do(ctx, "GET", path, nil, &out)
107-
if !out.pagination.LastPage.Bool {
108-
res.Page.First = 1
109-
res.Page.Next = opts.Page + 1
110-
}
97+
res, err := s.client.do(ctx, "GET", path, nil, out)
98+
copyPagination(out.pagination, res)
11199
return convertDiffstats(out), res, err
112100
}
113101

0 commit comments

Comments
 (0)