diff --git a/github/repos_contents.go b/github/repos_contents.go index 2378cd2330e..daf7ac4f0cf 100644 --- a/github/repos_contents.go +++ b/github/repos_contents.go @@ -17,7 +17,6 @@ import ( "io" "net/http" "net/url" - "path" "strings" ) @@ -125,9 +124,13 @@ func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string, } // DownloadContents returns an io.ReadCloser that reads the contents of the -// specified file. This function will work with files of any size, as opposed -// to GetContents which is limited to 1 Mb files. It is the caller's -// responsibility to close the ReadCloser. +// specified file, along with the file's metadata. This function will work with +// files of any size, as opposed to GetContents which is limited to 1 Mb files. +// It is the caller's responsibility to close the ReadCloser. +// +// The returned RepositoryContent contains metadata such as the file's SHA, name, +// path, URL, etc. If you don't need the metadata, you can ignore the second return +// value. // // It is possible for the download to result in a failed response when the // returned error is nil. Callers should check the returned Response status @@ -136,90 +139,52 @@ func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string, // GitHub API docs: https://docs.github.com/rest/repos/contents#get-repository-content // //meta:operation GET /repos/{owner}/{repo}/contents/{path} -func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, *Response, error) { - dir := path.Dir(filepath) - filename := path.Base(filepath) +func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, *RepositoryContent, *Response, error) { fileContent, _, resp, err := s.GetContents(ctx, owner, repo, filepath, opts) - if err == nil && fileContent != nil { - content, err := fileContent.GetContent() - if err == nil && content != "" { - return io.NopCloser(strings.NewReader(content)), resp, nil - } - } - - _, dirContents, resp, err := s.GetContents(ctx, owner, repo, dir, opts) if err != nil { - return nil, resp, err + return nil, nil, resp, err + } + if fileContent == nil { + return nil, nil, resp, fmt.Errorf("no file found at %v", filepath) } - for _, contents := range dirContents { - if contents.GetName() == filename { - if contents.GetDownloadURL() == "" { - return nil, resp, fmt.Errorf("no download link found for %v", filepath) - } - dlReq, err := http.NewRequestWithContext(ctx, "GET", *contents.DownloadURL, nil) - if err != nil { - return nil, resp, err - } - dlResp, err := s.client.client.Do(dlReq) - if err != nil { - return nil, &Response{Response: dlResp}, err - } - - return dlResp.Body, &Response{Response: dlResp}, nil - } + // Check if this is a file. + if fileContent.Type == nil || *fileContent.Type != "file" { + return nil, nil, resp, fmt.Errorf("cannot download content for type %q (only files are supported)", fileContent.GetType()) } - return nil, resp, fmt.Errorf("no file named %v found in %v", filename, dir) -} + // Handle empty files. + if fileContent.Size != nil && *fileContent.Size == 0 { + return io.NopCloser(strings.NewReader("")), fileContent, resp, nil + } -// DownloadContentsWithMeta is identical to DownloadContents but additionally -// returns the RepositoryContent of the requested file. This additional data -// is useful for future operations involving the requested file. For merely -// reading the content of a file, DownloadContents is perfectly adequate. -// -// It is possible for the download to result in a failed response when the -// returned error is nil. Callers should check the returned Response status -// code to verify the content is from a successful response. -// -// GitHub API docs: https://docs.github.com/rest/repos/contents#get-repository-content -// -//meta:operation GET /repos/{owner}/{repo}/contents/{path} -func (s *RepositoriesService) DownloadContentsWithMeta(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, *RepositoryContent, *Response, error) { - dir := path.Dir(filepath) - filename := path.Base(filepath) - fileContent, _, resp, err := s.GetContents(ctx, owner, repo, filepath, opts) - if err == nil && fileContent != nil { + // Try to use the inline content if available. + if fileContent.Content != nil && *fileContent.Content != "" { content, err := fileContent.GetContent() - if err == nil && content != "" { + if err == nil { return io.NopCloser(strings.NewReader(content)), fileContent, resp, nil } + // If GetContent fails (e.g., encoding "none"), fall through to use DownloadURL. + } + + // Use the download URL. + if fileContent.DownloadURL == nil || *fileContent.DownloadURL == "" { + return nil, fileContent, resp, errors.New("no download URL available for this file") } - _, dirContents, resp, err := s.GetContents(ctx, owner, repo, dir, opts) + req, err := http.NewRequestWithContext(ctx, "GET", *fileContent.DownloadURL, nil) if err != nil { - return nil, nil, resp, err + return nil, fileContent, resp, err } - for _, contents := range dirContents { - if contents.GetName() == filename { - if contents.GetDownloadURL() == "" { - return nil, contents, resp, fmt.Errorf("no download link found for %v", filepath) - } - dlReq, err := http.NewRequestWithContext(ctx, "GET", *contents.DownloadURL, nil) - if err != nil { - return nil, contents, resp, err - } - dlResp, err := s.client.client.Do(dlReq) - if err != nil { - return nil, contents, &Response{Response: dlResp}, err - } - - return dlResp.Body, contents, &Response{Response: dlResp}, nil - } + httpResp, err := s.client.client.Do(req) + if err != nil { + return nil, fileContent, resp, err } - return nil, nil, resp, fmt.Errorf("no file named %v found in %v", filename, dir) + // Return the response body even for non-2xx status codes. + // Callers should check resp.StatusCode to verify success. + return httpResp.Body, fileContent, &Response{Response: httpResp}, nil } // GetContents can return either the metadata and content of a single file diff --git a/github/repos_contents_test.go b/github/repos_contents_test.go index ed782da8701..d84e1103c7c 100644 --- a/github/repos_contents_test.go +++ b/github/repos_contents_test.go @@ -141,7 +141,7 @@ func TestRepositoriesService_DownloadContents_SuccessForFile(t *testing.T) { }) ctx := t.Context() - r, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) + r, c, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) if err != nil { t.Errorf("Repositories.DownloadContents returned error: %v", err) } @@ -160,14 +160,18 @@ func TestRepositoriesService_DownloadContents_SuccessForFile(t *testing.T) { t.Errorf("Repositories.DownloadContents returned %v, want %v", got, want) } + if got, want := c.GetName(), "f"; got != want { + t.Errorf("Repositories.DownloadContents returned content name %v, want %v", got, want) + } + const methodName = "DownloadContents" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.DownloadContents(ctx, "\n", "\n", "\n", nil) + _, _, _, err = client.Repositories.DownloadContents(ctx, "\n", "\n", "\n", nil) return err }) testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) + got, _, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } @@ -182,17 +186,10 @@ func TestRepositoriesService_DownloadContents_SuccessForDirectory(t *testing.T) mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, `{ - "type": "file", - "name": "f" - }`) - }) - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[{ "type": "file", "name": "f", "download_url": "`+serverURL+baseURLPath+`/download/f" - }]`) + }`) }) mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") @@ -200,7 +197,7 @@ func TestRepositoriesService_DownloadContents_SuccessForDirectory(t *testing.T) }) ctx := t.Context() - r, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) + r, c, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) if err != nil { t.Errorf("Repositories.DownloadContents returned error: %v", err) } @@ -219,14 +216,20 @@ func TestRepositoriesService_DownloadContents_SuccessForDirectory(t *testing.T) t.Errorf("Repositories.DownloadContents returned %v, want %v", got, want) } + if got, want := c.GetName(), "f"; got != want { + if c != nil { + t.Errorf("Repositories.DownloadContents returned content name %v, want %v", got, want) + } + } + const methodName = "DownloadContents" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.DownloadContents(ctx, "\n", "\n", "\n", nil) + _, _, _, err = client.Repositories.DownloadContents(ctx, "\n", "\n", "\n", nil) return err }) testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) + got, _, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } @@ -240,18 +243,12 @@ func TestRepositoriesService_DownloadContents_FailedResponse(t *testing.T) { mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprint(w, `{ - "type": "file", - "name": "f" - }`) - }) - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[{ + fmt.Fprint(w, + `{ "type": "file", "name": "f", "download_url": "`+serverURL+baseURLPath+`/download/f" - }]`) + }`) }) mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") @@ -260,7 +257,7 @@ func TestRepositoriesService_DownloadContents_FailedResponse(t *testing.T) { }) ctx := t.Context() - r, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) + r, _, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) if err != nil { t.Errorf("Repositories.DownloadContents returned error: %v", err) } @@ -288,31 +285,26 @@ func TestRepositoriesService_DownloadContents_NoDownloadURL(t *testing.T) { testMethod(t, r, "GET") fmt.Fprint(w, `{ "type": "file", - "name": "f", - "content": "" + "name": "f" }`) }) - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[{ - "type": "file", - "name": "f", - "content": "" - }]`) - }) ctx := t.Context() - reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) + reader, contents, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) if err == nil { t.Error("Repositories.DownloadContents did not return expected error") } + if reader != nil { + t.Error("Repositories.DownloadContents did not return expected reader") + } + if resp == nil { t.Error("Repositories.DownloadContents did not return expected response") } - if reader != nil { - t.Error("Repositories.DownloadContents did not return expected reader") + if contents == nil { + t.Error("Repositories.DownloadContents did not return expected content") } } @@ -322,20 +314,12 @@ func TestRepositoriesService_DownloadContents_NoFile(t *testing.T) { mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprint(w, `{ - "type": "file", - "name": "f", - "content": "" - }`) - }) - - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[]`) + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, `{"message":"Not Found"}`) }) ctx := t.Context() - reader, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) + reader, _, resp, err := client.Repositories.DownloadContents(ctx, "o", "r", "d/f", nil) if err == nil { t.Error("Repositories.DownloadContents did not return expected error") } @@ -349,228 +333,6 @@ func TestRepositoriesService_DownloadContents_NoFile(t *testing.T) { } } -func TestRepositoriesService_DownloadContentsWithMeta_SuccessForFile(t *testing.T) { - t.Parallel() - client, mux, serverURL := setup(t) - - mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `{ - "type": "file", - "name": "f", - "download_url": "`+serverURL+baseURLPath+`/download/f", - "content": "foo" - }`) - }) - - ctx := t.Context() - r, c, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil) - if err != nil { - t.Errorf("Repositories.DownloadContentsWithMeta returned error: %v", err) - } - - if got, want := resp.Response.StatusCode, http.StatusOK; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned status code %v, want %v", got, want) - } - - bytes, err := io.ReadAll(r) - if err != nil { - t.Errorf("Error reading response body: %v", err) - } - r.Close() - - if got, want := string(bytes), "foo"; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned %v, want %v", got, want) - } - - if c != nil && c.Name != nil { - if got, want := *c.Name, "f"; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned content name %v, want %v", got, want) - } - } else { - t.Error("Returned RepositoryContent is null") - } - - const methodName = "DownloadContentsWithMeta" - testBadOptions(t, methodName, func() (err error) { - _, _, _, err = client.Repositories.DownloadContentsWithMeta(ctx, "\n", "\n", "\n", nil) - return err - }) - - testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, cot, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil) - if got != nil { - t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) - } - if cot != nil { - t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, cot) - } - return resp, err - }) -} - -func TestRepositoriesService_DownloadContentsWithMeta_SuccessForDirectory(t *testing.T) { - t.Parallel() - client, mux, serverURL := setup(t) - - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[{ - "type": "file", - "name": "f", - "download_url": "`+serverURL+baseURLPath+`/download/f" - }]`) - }) - mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, "foo") - }) - - ctx := t.Context() - r, c, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil) - if err != nil { - t.Errorf("Repositories.DownloadContentsWithMeta returned error: %v", err) - } - - if got, want := resp.Response.StatusCode, http.StatusOK; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned status code %v, want %v", got, want) - } - - bytes, err := io.ReadAll(r) - if err != nil { - t.Errorf("Error reading response body: %v", err) - } - r.Close() - - if got, want := string(bytes), "foo"; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned %v, want %v", got, want) - } - - if c != nil && c.Name != nil { - if got, want := *c.Name, "f"; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned content name %v, want %v", got, want) - } - } else { - t.Error("Returned RepositoryContent is null") - } -} - -func TestRepositoriesService_DownloadContentsWithMeta_FailedResponse(t *testing.T) { - t.Parallel() - client, mux, serverURL := setup(t) - - downloadURL := fmt.Sprintf("%v%v/download/f", serverURL, baseURLPath) - - mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `{ - "type": "file", - "name": "f", - "download_url": "`+downloadURL+`" - }`) - }) - mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - w.WriteHeader(http.StatusInternalServerError) - fmt.Fprint(w, "foo error") - }) - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[{ - "type": "file", - "name": "f", - "download_url": "`+downloadURL+`" - }]`) - }) - - ctx := t.Context() - r, c, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil) - if err != nil { - t.Errorf("Repositories.DownloadContentsWithMeta returned error: %v", err) - } - - if got, want := resp.Response.StatusCode, http.StatusInternalServerError; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned status code %v, want %v", got, want) - } - - bytes, err := io.ReadAll(r) - if err != nil { - t.Errorf("Error reading response body: %v", err) - } - r.Close() - - if got, want := string(bytes), "foo error"; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned %v, want %v", got, want) - } - - if c != nil && c.Name != nil { - if got, want := *c.Name, "f"; got != want { - t.Errorf("Repositories.DownloadContentsWithMeta returned content name %v, want %v", got, want) - } - } else { - t.Error("Returned RepositoryContent is null") - } -} - -func TestRepositoriesService_DownloadContentsWithMeta_NoDownloadURL(t *testing.T) { - t.Parallel() - client, mux, _ := setup(t) - - mux.HandleFunc("/repos/o/r/contents/d/f", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `{ - "type": "file", - "name": "f", - }`) - }) - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[{ - "type": "file", - "name": "f", - "content": "" - }]`) - }) - - ctx := t.Context() - reader, contents, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil) - if err == nil { - t.Error("Repositories.DownloadContentsWithMeta did not return expected error") - } - - if reader != nil { - t.Error("Repositories.DownloadContentsWithMeta did not return expected reader") - } - - if resp == nil { - t.Error("Repositories.DownloadContentsWithMeta did not return expected response") - } - - if contents == nil { - t.Error("Repositories.DownloadContentsWithMeta did not return expected content") - } -} - -func TestRepositoriesService_DownloadContentsWithMeta_NoFile(t *testing.T) { - t.Parallel() - client, mux, _ := setup(t) - - mux.HandleFunc("/repos/o/r/contents/d", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, `[]`) - }) - - ctx := t.Context() - _, _, resp, err := client.Repositories.DownloadContentsWithMeta(ctx, "o", "r", "d/f", nil) - if err == nil { - t.Error("Repositories.DownloadContentsWithMeta did not return expected error") - } - - if resp == nil { - t.Error("Repositories.DownloadContentsWithMeta did not return expected response") - } -} - func TestRepositoriesService_GetContents_File(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/tools/metadata/openapi.go b/tools/metadata/openapi.go index 549410374e7..b46f9dab22d 100644 --- a/tools/metadata/openapi.go +++ b/tools/metadata/openapi.go @@ -55,7 +55,7 @@ func getOpsFromGithub(ctx context.Context, client *github.Client, gitRef string) } func (o *openapiFile) loadDescription(ctx context.Context, client *github.Client, gitRef string) error { - contents, resp, err := client.Repositories.DownloadContents( + contents, _, resp, err := client.Repositories.DownloadContents( ctx, descriptionsOwnerName, descriptionsRepoName,