fix: use DownloadContents API instead of GetContents to download larg…

…e files (#2714) * fix: use DownloadContents API instead of GetContents to download large files - #2713 * refactor: remove GetContents API * refactor: replace github.RepositoryContent to string
aquaproj · Mar 2, 2024 · b579588 · b579588
1 parent 8caa94e
commit b579588
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 22 deletions.
diff --git a/pkg/download/error.go b/pkg/download/error.go
@@ -3,7 +3,6 @@ package download
 import "errors"
 
 var (
-	errInvalidPackageType      = errors.New("package type is invalid")
-	errGitHubContentMustBeFile = errors.New("path must be not a directory but a file")
-	errInvalidHTTPStatusCode   = errors.New("status code >= 400")
+	errInvalidPackageType    = errors.New("package type is invalid")
+	errInvalidHTTPStatusCode = errors.New("status code >= 400")
 )
diff --git a/pkg/download/github_content.go b/pkg/download/github_content.go
@@ -3,6 +3,8 @@ package download
 import (
 	"context"
 	"fmt"
+	"io"
+	"net/http"
 
 	"github.com/aquaproj/aqua/v2/pkg/domain"
 	"github.com/aquaproj/aqua/v2/pkg/github"
@@ -15,7 +17,7 @@ type GitHubContentFileDownloader struct {
 }
 
 type GitHubContentAPI interface {
-	GetContents(ctx context.Context, repoOwner, repoName, path string, opt *github.RepositoryContentGetOptions) (*github.RepositoryContent, []*github.RepositoryContent, *github.Response, error)
+	DownloadContents(ctx context.Context, owner, repo, filepath string, opts *github.RepositoryContentGetOptions) (io.ReadCloser, *github.Response, error)
 }
 
 func NewGitHubContentFileDownloader(gh GitHubContentAPI, httpDL HTTPDownloader) *GitHubContentFileDownloader {
@@ -42,20 +44,17 @@ func (dl *GitHubContentFileDownloader) DownloadGitHubContentFile(ctx context.Con
 		}
 	}
 
-	file, _, _, err := dl.github.GetContents(ctx, param.RepoOwner, param.RepoName, param.Path, &github.RepositoryContentGetOptions{
+	file, resp, err := dl.github.DownloadContents(ctx, param.RepoOwner, param.RepoName, param.Path, &github.RepositoryContentGetOptions{
 		Ref: param.Ref,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("get a file by Get GitHub Content API: %w", err)
 	}
-	if file == nil {
-		return nil, errGitHubContentMustBeFile
-	}
-	content, err := file.GetContent()
-	if err != nil {
-		return nil, fmt.Errorf("get a GitHub Content file content: %w", err)
+	if resp.StatusCode != http.StatusOK {
+		file.Close()
+		return nil, fmt.Errorf("get a file by Get GitHub Content API: status code %d", resp.StatusCode)
 	}
 	return &domain.GitHubContentFile{
-		String: content,
+		ReadCloser: file,
 	}, nil
 }
diff --git a/pkg/download/github_content_test.go b/pkg/download/github_content_test.go
@@ -9,7 +9,6 @@ import (
 	"github.com/aquaproj/aqua/v2/pkg/domain"
 	"github.com/aquaproj/aqua/v2/pkg/download"
 	"github.com/aquaproj/aqua/v2/pkg/github"
-	"github.com/aquaproj/aqua/v2/pkg/ptr"
 	"github.com/sirupsen/logrus"
 	"github.com/suzuki-shunsuke/flute/flute"
 )
@@ -69,9 +68,7 @@ func TestGitHubContentFileDownloader_DownloadGitHubContentFile(t *testing.T) { /
 			},
 			exp: "foo",
 			github: &github.MockRepositoriesService{
-				Content: &github.RepositoryContent{
-					Content: ptr.String("foo"),
-				},
+				Content: "foo",
 			},
 			httpClient: &http.Client{
 				Transport: &flute.Transport{

diff --git a/pkg/github/github.go b/pkg/github/github.go
@@ -34,7 +34,7 @@ type RepositoriesService interface {
 	GetArchiveLink(ctx context.Context, owner, repo string, archiveformat github.ArchiveFormat, opts *github.RepositoryContentGetOptions, maxRedirects int) (*url.URL, *github.Response, error)
 	GetReleaseByTag(ctx context.Context, owner, repoName, version string) (*github.RepositoryRelease, *github.Response, error)
 	DownloadReleaseAsset(ctx context.Context, owner, repoName string, assetID int64, httpClient *http.Client) (io.ReadCloser, string, error)
-	GetContents(ctx context.Context, repoOwner, repoName, path string, opt *github.RepositoryContentGetOptions) (*github.RepositoryContent, []*github.RepositoryContent, *github.Response, error)
+	DownloadContents(ctx context.Context, owner, repo, filepath string, opts *github.RepositoryContentGetOptions) (io.ReadCloser, *github.Response, error)
 }
 
 func getGitHubToken() string {

diff --git a/pkg/github/mock.go b/pkg/github/mock.go
@@ -23,7 +23,7 @@ var (
 
 type MockRepositoriesService struct {
 	Releases []*github.RepositoryRelease
-	Content  *github.RepositoryContent
+	Content  string
 	Repo     *github.Repository
 	Tags     []*github.RepositoryTag
 	Asset    string
@@ -38,11 +38,15 @@ func (m *MockRepositoriesService) GetLatestRelease(ctx context.Context, repoOwne
 	return m.Releases[0], nil, nil
 }
 
-func (m *MockRepositoriesService) GetContents(ctx context.Context, repoOwner, repoName, path string, opt *github.RepositoryContentGetOptions) (*github.RepositoryContent, []*github.RepositoryContent, *github.Response, error) {
-	if m.Content == nil {
-		return m.Content, nil, nil, errContentNotFound
+func (m *MockRepositoriesService) DownloadContents(ctx context.Context, owner, repo, filepath string, opts *github.RepositoryContentGetOptions) (io.ReadCloser, *github.Response, error) {
+	if m.Content == "" {
+		return nil, nil, errContentNotFound
 	}
-	return m.Content, nil, nil, nil
+	return io.NopCloser(strings.NewReader(m.Content)), &github.Response{
+		Response: &http.Response{
+			StatusCode: http.StatusOK,
+		},
+	}, nil
 }
 
 func (m *MockRepositoriesService) GetReleaseByTag(ctx context.Context, owner, repoName, version string) (*github.RepositoryRelease, *github.Response, error) {