Skip to content

Commit a8e1adc

Browse files
worlpakerworlpaker
committed
refactor gitty methods
1 parent 846d9b9 commit a8e1adc

File tree

4 files changed

+51
-59
lines changed

4 files changed

+51
-59
lines changed

‎gitty/gitty.go

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ type Git struct {
1414
// Gitty defines methods for interacting with cmd.
1515
type Gitty interface {
1616
Status(ctx context.Context) error
17-
Download(ctx context.Context, url string) error
1817
Auth(ctx context.Context) error
18+
Download(ctx context.Context, url string) error
1919
}
2020

2121
// Ensure Git implements the Gitty interface.
@@ -32,47 +32,30 @@ func New() Gitty {
3232

3333
// Status reports the status of the client.
3434
func (g *Git) Status(ctx context.Context) error {
35-
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
36-
defer cancel()
37-
38-
if err := g.repo.clientStatus(ctx); err != nil {
39-
return fmt.Errorf("failed to check status: %v", err)
40-
}
35+
return g.repo.status(ctx)
36+
}
4137

42-
return nil
38+
// Auth reports the authenticated username.
39+
func (g *Git) Auth(ctx context.Context) error {
40+
return g.repo.auth(ctx)
4341
}
4442

45-
// Download downloads the contents of the given URL. It extracts url,
46-
// collects contents, and downloads files concurrently.
43+
// Download downloads the contents from the given URL. It extracts the URL,
44+
// collects the contents, and downloads files concurrently.
4745
func (g *Git) Download(ctx context.Context, url string) error {
48-
ctx, cancel := context.WithTimeout(ctx, downloadLimit*time.Second)
46+
fmt.Println("Downloading:", url)
4947
start := time.Now()
50-
defer func() {
51-
cancel()
52-
fmt.Println(time.Since(start))
53-
}()
5448

5549
if err := g.repo.extract(url); err != nil {
5650
return err
5751
}
5852

59-
fmt.Println("Downloading:", url)
60-
if err := g.repo.downloadContents(ctx); err != nil {
61-
return fmt.Errorf("failed to download: %v", err)
53+
if err := g.repo.download(ctx); err != nil {
54+
return err
6255
}
63-
fmt.Println("Download Completed")
64-
65-
return nil
66-
}
6756

68-
// Auth reports the auth status of the user.
69-
func (g *Git) Auth(ctx context.Context) error {
70-
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
71-
defer cancel()
72-
73-
if err := g.repo.clientAuth(ctx); err != nil {
74-
return fmt.Errorf("failed to check auth: %v", err)
75-
}
57+
fmt.Println("Download Completed")
58+
fmt.Println(time.Since(start))
7659

7760
return nil
7861
}

‎gitty/gitty_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,9 @@ func TestDownload(t *testing.T) {
6868
expected error
6969
}{
7070
{
71-
name: "success download",
72-
repo: fakeRepository(&mockSuccess{}),
73-
url: "https://github.com/owner/repo/tree/branch/directory",
74-
// Path names got from -> repository_test.go `contentsData`.
71+
name: "success download",
72+
repo: fakeRepository(&mockSuccess{}),
73+
url: "https://github.com/owner/repo/tree/branch/directory",
7574
paths: []string{contentsData[0].GetPath(), contentsData[1].GetPath()},
7675
expected: nil,
7776
},

‎gitty/repository.go

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ var (
2828
// Repository defines methods for interacting with GitHub.
2929
type Repository interface {
3030
extract(url string) error
31-
downloadContents(ctx context.Context) error
31+
download(ctx context.Context) error
3232
contents(ctx context.Context, wg *sync.WaitGroup, path string, errCh chan error)
33-
downloadFile(url, path string) error
34-
clientStatus(ctx context.Context) error
35-
clientAuth(ctx context.Context) error
33+
getFile(url, path string) error
34+
status(ctx context.Context) error
35+
auth(ctx context.Context) error
3636
}
3737

3838
// Ensure GitHub implements the Repository interface.
@@ -69,10 +69,12 @@ func (g *GitHub) extract(url string) error {
6969
return nil
7070
}
7171

72-
// downloadContents downloads the contents concurrently.
73-
func (g *GitHub) downloadContents(ctx context.Context) error {
72+
// download downloads the contents concurrently.
73+
func (g *GitHub) download(ctx context.Context) error {
7474
wg := &sync.WaitGroup{}
7575
errCh := make(chan error, 1)
76+
ctx, cancel := context.WithTimeout(ctx, downloadLimit*time.Second)
77+
defer cancel()
7678

7779
wg.Add(1)
7880
go g.contents(ctx, wg, g.Path, errCh)
@@ -87,7 +89,7 @@ func (g *GitHub) downloadContents(ctx context.Context) error {
8789
select {
8890
case err := <-errCh:
8991
if err != nil {
90-
return err
92+
return fmt.Errorf("failed to download: %v", err)
9193
}
9294
case <-ctx.Done():
9395
if errors.Is(ctx.Err(), context.Canceled) {
@@ -112,7 +114,7 @@ func (g *GitHub) contents(ctx context.Context, wg *sync.WaitGroup, path string,
112114

113115
// If the URL points to a file, only the file is downloaded.
114116
if len(directoryContent) == 0 && fileContent != nil {
115-
if err := g.downloadFile(fileContent.GetDownloadURL(), fileContent.GetPath()); err != nil {
117+
if err := g.getFile(fileContent.GetDownloadURL(), fileContent.GetPath()); err != nil {
116118
errCh <- err
117119
return
118120
}
@@ -127,7 +129,7 @@ func (g *GitHub) contents(ctx context.Context, wg *sync.WaitGroup, path string,
127129
switch content.GetType() {
128130
case "file":
129131
// Download the file directly.
130-
if err := g.downloadFile(content.GetDownloadURL(), content.GetPath()); err != nil {
132+
if err := g.getFile(content.GetDownloadURL(), content.GetPath()); err != nil {
131133
errCh <- err
132134
return
133135
}
@@ -140,8 +142,8 @@ func (g *GitHub) contents(ctx context.Context, wg *sync.WaitGroup, path string,
140142
}
141143
}
142144

143-
// downloadFile downloads a file from the given URL and saves it.
144-
func (g *GitHub) downloadFile(url, path string) error {
145+
// getFile retrieves a file from the given URL and saves it.
146+
func (g *GitHub) getFile(url, path string) error {
145147
if url == "" || path == "" {
146148
return ErrInvalidPathURL
147149
}
@@ -156,14 +158,17 @@ func (g *GitHub) downloadFile(url, path string) error {
156158
return saveFile(g.Path, path, resp.Body)
157159
}
158160

159-
// clientStatus reports the status of the client, the remaining hourly
161+
// status reports the status of the client, the remaining hourly
160162
// rate limit, and the time at which the current rate limit will reset.
161163
// This function does not reduce the rate limit. It can be used freely.
162-
func (g *GitHub) clientStatus(ctx context.Context) error {
164+
func (g *GitHub) status(ctx context.Context) error {
163165
auth := "NOT Authorized"
166+
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
167+
defer cancel()
168+
164169
rate, _, err := g.Client.RateLimit(ctx)
165170
if err != nil {
166-
return err
171+
return fmt.Errorf("failed to check status: %v", err)
167172
}
168173

169174
if token.Get() != "" && rate.Core.Limit > baseRateLimit {
@@ -176,13 +181,17 @@ func (g *GitHub) clientStatus(ctx context.Context) error {
176181
return nil
177182
}
178183

179-
// clientAuth reports the authenticated username, if applicable.
184+
// auth reports the authenticated username, if applicable.
180185
// This function reduces the rate limit for each request.
181-
func (g *GitHub) clientAuth(ctx context.Context) error {
186+
func (g *GitHub) auth(ctx context.Context) error {
187+
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
188+
defer cancel()
189+
182190
u, _, err := g.Client.GetUser(ctx, "")
183191
if err != nil {
184-
return err
192+
return fmt.Errorf("failed to check auth: %v", err)
185193
}
194+
186195
fmt.Printf("Authenticated as @%s \n", u.GetLogin())
187196

188197
return nil

‎gitty/repository_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func TestExtract(t *testing.T) {
249249
}
250250
}
251251

252-
func TestDownloadFile(t *testing.T) {
252+
func TestGetFile(t *testing.T) {
253253
// Discard output during tests.
254254
defer func(stdout *os.File) {
255255
os.Stdout = stdout
@@ -291,7 +291,7 @@ func TestDownloadFile(t *testing.T) {
291291

292292
for _, test := range tests {
293293
t.Run(test.name, func(t *testing.T) {
294-
err := test.repo.downloadFile(test.url, test.path)
294+
err := test.repo.getFile(test.url, test.path)
295295
assert.Equal(t, test.expected, err)
296296
if test.expected == nil {
297297
err := os.Remove(test.path)
@@ -347,7 +347,7 @@ func TestDownloadContents(t *testing.T) {
347347
name: "error get contents",
348348
repo: fakeRepository(&mockError{}),
349349
ctx: context.Background(),
350-
expected: errMockContents,
350+
expected: fmt.Errorf("failed to download: %v", errMockContents),
351351
},
352352
{
353353
name: "error ctx with timeout",
@@ -365,14 +365,15 @@ func TestDownloadContents(t *testing.T) {
365365

366366
for _, test := range tests {
367367
t.Run(test.name, func(t *testing.T) {
368+
t.Parallel()
368369
if test.expected == nil {
369370
t.Cleanup(func() {
370371
paths := []string{contentsData[0].GetPath(), contentsData[1].GetPath()}
371372
deleteTestFiles(t, paths...)
372373
})
373374
}
374375

375-
err := test.repo.downloadContents(test.ctx)
376+
err := test.repo.download(test.ctx)
376377
assert.Equal(t, test.expected, err)
377378
})
378379
}
@@ -472,7 +473,7 @@ func TestClientStatus(t *testing.T) {
472473
name: "error with not authorized",
473474
repo: fakeRepository(&mockError{}),
474475
expected: "",
475-
expectedErr: errMockRateLimit,
476+
expectedErr: fmt.Errorf("failed to check status: %v", errMockRateLimit),
476477
},
477478
}
478479

@@ -490,7 +491,7 @@ func TestClientStatus(t *testing.T) {
490491
r, w, _ := os.Pipe()
491492
os.Stdout = w
492493

493-
err := test.repo.clientStatus(context.Background())
494+
err := test.repo.status(context.Background())
494495
assert.Equal(t, test.expectedErr, err)
495496

496497
w.Close()
@@ -525,13 +526,13 @@ func TestClientAuth(t *testing.T) {
525526
{
526527
name: "error auth",
527528
repo: fakeRepository(&mockError{}),
528-
expected: errMockGetUser,
529+
expected: fmt.Errorf("failed to check auth: %v", errMockGetUser),
529530
},
530531
}
531532

532533
for _, test := range tests {
533534
t.Run(test.name, func(t *testing.T) {
534-
err := test.repo.clientAuth(context.Background())
535+
err := test.repo.auth(context.Background())
535536
assert.Equal(t, test.expected, err)
536537
})
537538
}

0 commit comments

Comments
 (0)