Skip to content

Commit 9c0ad62

Browse files
stevehipwellstevehipwell
authored
feat: Add repo download contents sentinel errors (#4192)
1 parent 6c643b8 commit 9c0ad62

2 files changed

Lines changed: 146 additions & 44 deletions

File tree

‎github/repos_contents.go‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@ import (
2020
"strings"
2121
)
2222

23+
// ErrContentsDirectory indicates that the contents are not available for a directory.
24+
var ErrContentsDirectory = errors.New("contents not available for directory")
25+
26+
// ErrContentsSubmodule indicates that the contents are not available for a submodule.
27+
var ErrContentsSubmodule = errors.New("contents not available for submodule")
28+
29+
// ErrContentsNoDownloadURL indicates that the contents download URL is empty, which may occur when file size > 100 MB.
30+
var ErrContentsNoDownloadURL = errors.New("contents download url is empty")
31+
2332
// RepositoryContent represents a file or directory in a github repository.
2433
type RepositoryContent struct {
2534
Type *string `json:"type,omitempty"`
@@ -130,6 +139,10 @@ func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string,
130139
// returned error is nil. Callers should check the returned Response status
131140
// code to verify the content is from a successful response.
132141
//
142+
// DownloadContents returns [ErrContentsDirectory] if the path references a
143+
// directory, [ErrContentsSubmodule] if the path references a submodule, and
144+
// [ErrContentsNoDownloadURL] if the file's download URL is empty.
145+
//
133146
// GitHub API docs: https://docs.github.com/rest/repos/contents?apiVersion=2022-11-28#get-repository-content
134147
//
135148
//meta:operation GET /repos/{owner}/{repo}/contents/{path}
@@ -147,6 +160,11 @@ func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo,
147160
// returned error is nil. Callers should check the returned Response status
148161
// code to verify the content is from a successful response.
149162
//
163+
// DownloadContentsWithMeta returns [ErrContentsDirectory] if the path
164+
// references a directory, [ErrContentsSubmodule] if the path references a
165+
// submodule, and [ErrContentsNoDownloadURL] if the file's download URL is
166+
// empty.
167+
//
150168
// GitHub API docs: https://docs.github.com/rest/repos/contents?apiVersion=2022-11-28#get-repository-content
151169
//
152170
//meta:operation GET /repos/{owner}/{repo}/contents/{path}
@@ -157,7 +175,11 @@ func (s *RepositoriesService) DownloadContentsWithMeta(ctx context.Context, owne
157175
}
158176

159177
if fileContent == nil {
160-
return nil, nil, resp, errors.New("no file content found")
178+
return nil, nil, resp, ErrContentsDirectory
179+
}
180+
181+
if fileContent.GetType() == "submodule" {
182+
return nil, fileContent, resp, ErrContentsSubmodule
161183
}
162184

163185
content, err := fileContent.GetContent()
@@ -167,7 +189,7 @@ func (s *RepositoriesService) DownloadContentsWithMeta(ctx context.Context, owne
167189

168190
downloadURL := fileContent.GetDownloadURL()
169191
if downloadURL == "" {
170-
return nil, fileContent, resp, errors.New("download url is empty")
192+
return nil, fileContent, resp, ErrContentsNoDownloadURL
171193
}
172194

173195
dlReq, err := http.NewRequestWithContext(ctx, "GET", downloadURL, nil)

‎github/repos_contents_test.go‎

Lines changed: 122 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func TestRepositoriesService_DownloadContents_SuccessByDownload(t *testing.T) {
229229
})
230230
}
231231

232-
func TestRepositoriesService_DownloadContents_FailedResponse(t *testing.T) {
232+
func TestRepositoriesService_DownloadContents_FailedDownloadResponse(t *testing.T) {
233233
t.Parallel()
234234
client, mux, serverURL := setup(t)
235235

@@ -269,26 +269,50 @@ func TestRepositoriesService_DownloadContents_FailedResponse(t *testing.T) {
269269
}
270270
}
271271

272-
func TestRepositoriesService_DownloadContents_NoDownloadURL(t *testing.T) {
272+
func TestRepositoriesService_DownloadContents_NotFound(t *testing.T) {
273273
t.Parallel()
274274
client, mux, _ := setup(t)
275275

276276
mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
277277
testMethod(t, r, "GET")
278-
fmt.Fprint(w, `{
278+
w.WriteHeader(http.StatusNotFound)
279+
})
280+
281+
ctx := t.Context()
282+
reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
283+
if err == nil {
284+
t.Error("Repositories.DownloadContents did not return expected error")
285+
}
286+
287+
if reader != nil {
288+
t.Error("Repositories.DownloadContents did not return expected reader")
289+
}
290+
291+
if resp == nil || resp.Response.StatusCode != http.StatusNotFound {
292+
t.Error("Repositories.DownloadContents did not return expected response")
293+
}
294+
}
295+
296+
func TestRepositoriesService_DownloadContents_NotFile(t *testing.T) {
297+
t.Parallel()
298+
client, mux, _ := setup(t)
299+
300+
mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
301+
testMethod(t, r, "GET")
302+
fmt.Fprint(w, `[{
279303
"type": "file",
280304
"name": "f",
281305
"content": ""
282-
}`)
306+
}]`)
283307
})
284308

285309
ctx := t.Context()
286-
reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
287-
if err == nil {
310+
reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d", nil)
311+
if err == nil || !errors.Is(err, ErrContentsDirectory) {
288312
t.Error("Repositories.DownloadContents did not return expected error")
289313
}
290314

291-
if resp == nil {
315+
if resp == nil || resp.Response.StatusCode != http.StatusOK {
292316
t.Error("Repositories.DownloadContents did not return expected response")
293317
}
294318

@@ -297,22 +321,28 @@ func TestRepositoriesService_DownloadContents_NoDownloadURL(t *testing.T) {
297321
}
298322
}
299323

300-
func TestRepositoriesService_DownloadContents_NoFile(t *testing.T) {
324+
func TestRepositoriesService_DownloadContents_Submodule(t *testing.T) {
301325
t.Parallel()
302326
client, mux, _ := setup(t)
303327

304-
mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
328+
mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
305329
testMethod(t, r, "GET")
306-
w.WriteHeader(http.StatusNotFound)
330+
fmt.Fprint(w, `{
331+
"type": "submodule",
332+
"name": "f",
333+
"content": "",
334+
"download_url": "",
335+
"submodule_git_url": "http://example.com/submodule.git"
336+
}`)
307337
})
308338

309339
ctx := t.Context()
310-
reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
311-
if err == nil {
340+
reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d", nil)
341+
if err == nil || !errors.Is(err, ErrContentsSubmodule) {
312342
t.Error("Repositories.DownloadContents did not return expected error")
313343
}
314344

315-
if resp == nil {
345+
if resp == nil || resp.Response.StatusCode != http.StatusOK {
316346
t.Error("Repositories.DownloadContents did not return expected response")
317347
}
318348

@@ -321,26 +351,26 @@ func TestRepositoriesService_DownloadContents_NoFile(t *testing.T) {
321351
}
322352
}
323353

324-
func TestRepositoriesService_DownloadContents_NotFile(t *testing.T) {
354+
func TestRepositoriesService_DownloadContents_NoDownloadURL(t *testing.T) {
325355
t.Parallel()
326356
client, mux, _ := setup(t)
327357

328-
mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
358+
mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
329359
testMethod(t, r, "GET")
330-
fmt.Fprint(w, `[{
360+
fmt.Fprint(w, `{
331361
"type": "file",
332362
"name": "f",
333363
"content": ""
334-
}]`)
364+
}`)
335365
})
336366

337367
ctx := t.Context()
338-
reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d", nil)
339-
if err == nil {
368+
reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil)
369+
if err == nil || !errors.Is(err, ErrContentsNoDownloadURL) {
340370
t.Error("Repositories.DownloadContents did not return expected error")
341371
}
342372

343-
if resp == nil {
373+
if resp == nil || resp.Response.StatusCode != http.StatusOK {
344374
t.Error("Repositories.DownloadContents did not return expected response")
345375
}
346376

@@ -456,7 +486,7 @@ func TestRepositoriesService_DownloadContentsWithMeta_SuccessByDownload(t *testi
456486
}
457487
}
458488

459-
func TestRepositoriesService_DownloadContentsWithMeta_FailedResponse(t *testing.T) {
489+
func TestRepositoriesService_DownloadContentsWithMeta_FailedDownloadResponse(t *testing.T) {
460490
t.Parallel()
461491
client, mux, serverURL := setup(t)
462492

@@ -503,17 +533,13 @@ func TestRepositoriesService_DownloadContentsWithMeta_FailedResponse(t *testing.
503533
}
504534
}
505535

506-
func TestRepositoriesService_DownloadContentsWithMeta_NoDownloadURL(t *testing.T) {
536+
func TestRepositoriesService_DownloadContentsWithMeta_NotFound(t *testing.T) {
507537
t.Parallel()
508538
client, mux, _ := setup(t)
509539

510540
mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
511541
testMethod(t, r, "GET")
512-
fmt.Fprint(w, `{
513-
"type": "file",
514-
"name": "f",
515-
"content": ""
516-
}`)
542+
w.WriteHeader(http.StatusNotFound)
517543
})
518544

519545
ctx := t.Context()
@@ -526,55 +552,109 @@ func TestRepositoriesService_DownloadContentsWithMeta_NoDownloadURL(t *testing.T
526552
t.Error("Repositories.DownloadContentsWithMeta did not return expected reader")
527553
}
528554

529-
if resp == nil {
555+
if contents != nil {
556+
t.Error("Repositories.DownloadContentsWithMeta did not return expected content")
557+
}
558+
559+
if resp == nil || resp.Response.StatusCode != http.StatusNotFound {
530560
t.Error("Repositories.DownloadContentsWithMeta did not return expected response")
531561
}
562+
}
532563

533-
if contents == nil {
564+
func TestRepositoriesService_DownloadContentsWithMeta_NotFile(t *testing.T) {
565+
t.Parallel()
566+
client, mux, _ := setup(t)
567+
568+
mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
569+
testMethod(t, r, "GET")
570+
fmt.Fprint(w, `[{
571+
"type": "file",
572+
"name": "f",
573+
"content": ""
574+
}]`)
575+
})
576+
577+
ctx := t.Context()
578+
reader, contents, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d", nil)
579+
if err == nil || !errors.Is(err, ErrContentsDirectory) {
580+
t.Error("Repositories.DownloadContentsWithMeta did not return expected error")
581+
}
582+
583+
if reader != nil {
584+
t.Error("Repositories.DownloadContentsWithMeta did not return expected reader")
585+
}
586+
587+
if contents != nil {
534588
t.Error("Repositories.DownloadContentsWithMeta did not return expected content")
535589
}
590+
591+
if resp == nil || resp.Response.StatusCode != http.StatusOK {
592+
t.Error("Repositories.DownloadContentsWithMeta did not return expected response")
593+
}
536594
}
537595

538-
func TestRepositoriesService_DownloadContentsWithMeta_NoFile(t *testing.T) {
596+
func TestRepositoriesService_DownloadContentsWithMeta_Submodule(t *testing.T) {
539597
t.Parallel()
540598
client, mux, _ := setup(t)
541599

542-
mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
600+
mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
543601
testMethod(t, r, "GET")
544-
w.WriteHeader(http.StatusNotFound)
602+
fmt.Fprint(w, `{
603+
"type": "submodule",
604+
"name": "f",
605+
"content": "",
606+
"download_url": "",
607+
"submodule_git_url": "http://example.com/submodule.git"
608+
}`)
545609
})
546610

547611
ctx := t.Context()
548-
_, _, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil)
549-
if err == nil {
612+
reader, contents, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d", nil)
613+
if err == nil || !errors.Is(err, ErrContentsSubmodule) {
550614
t.Error("Repositories.DownloadContentsWithMeta did not return expected error")
551615
}
552616

553-
if resp == nil {
617+
if reader != nil {
618+
t.Error("Repositories.DownloadContentsWithMeta did not return expected reader")
619+
}
620+
621+
if contents == nil || contents.GetType() != "submodule" {
622+
t.Error("Repositories.DownloadContentsWithMeta did not return expected content")
623+
}
624+
625+
if resp == nil || resp.Response.StatusCode != http.StatusOK {
554626
t.Error("Repositories.DownloadContentsWithMeta did not return expected response")
555627
}
556628
}
557629

558-
func TestRepositoriesService_DownloadContentsWithMeta_NotFile(t *testing.T) {
630+
func TestRepositoriesService_DownloadContentsWithMeta_NoDownloadURL(t *testing.T) {
559631
t.Parallel()
560632
client, mux, _ := setup(t)
561633

562-
mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) {
634+
mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) {
563635
testMethod(t, r, "GET")
564-
fmt.Fprint(w, `[{
636+
fmt.Fprint(w, `{
565637
"type": "file",
566638
"name": "f",
567639
"content": ""
568-
}]`)
640+
}`)
569641
})
570642

571643
ctx := t.Context()
572-
_, _, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d", nil)
573-
if err == nil {
644+
reader, contents, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil)
645+
if err == nil || !errors.Is(err, ErrContentsNoDownloadURL) {
574646
t.Error("Repositories.DownloadContentsWithMeta did not return expected error")
575647
}
576648

577-
if resp == nil {
649+
if reader != nil {
650+
t.Error("Repositories.DownloadContentsWithMeta did not return expected reader")
651+
}
652+
653+
if contents == nil {
654+
t.Error("Repositories.DownloadContentsWithMeta did not return expected content")
655+
}
656+
657+
if resp == nil || resp.Response.StatusCode != http.StatusOK {
578658
t.Error("Repositories.DownloadContentsWithMeta did not return expected response")
579659
}
580660
}

0 commit comments

Comments
 (0)