Skip to content

Commit a5183ba

Browse files
julieqiujulieqiu
authored
refactor(internal/fetch): unexport functions and fix names (#4870)
Unexport functions that are only used inside internal/fetch. Remove "tarball" from non-tarball-specific function names and comments.
1 parent 648ed1d commit a5183ba

3 files changed

Lines changed: 52 additions & 52 deletions

File tree

‎internal/fetch/cache.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func RepoDir(ctx context.Context, repo, commit, expectedSHA256 string) (string,
8585
if err := os.MkdirAll(outDir, 0755); err != nil {
8686
return "", fmt.Errorf("failed creating %q: %w", outDir, err)
8787
}
88-
if err := ExtractTarball(tgz, outDir); err == nil {
88+
if err := extractTarball(tgz, outDir); err == nil {
8989
return outDir, nil
9090
}
9191
}
@@ -103,10 +103,10 @@ func RepoDir(ctx context.Context, repo, commit, expectedSHA256 string) (string,
103103
if err := os.MkdirAll(outDir, 0755); err != nil {
104104
return "", fmt.Errorf("failed creating %q: %w", outDir, err)
105105
}
106-
if err := DownloadTarball(ctx, tgz, sourceURL, expectedSHA256); err != nil {
106+
if err := download(ctx, tgz, sourceURL, expectedSHA256); err != nil {
107107
return "", err
108108
}
109-
if err := ExtractTarball(tgz, outDir); err != nil {
109+
if err := extractTarball(tgz, outDir); err != nil {
110110
return "", fmt.Errorf("failed to extract tarball: %w", err)
111111
}
112112
return outDir, nil

‎internal/fetch/fetch.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,17 @@ type Repo struct {
6666
Repo string
6767
}
6868

69-
// RepoFromTarballLink extracts the gitHub account and repository (such as
70-
// `googleapis/googleapis`, or `googleapis/google-cloud-rust`) from the tarball
69+
// repoFromArchiveLink extracts the GitHub account and repository (such as
70+
// `googleapis/googleapis`, or `googleapis/google-cloud-rust`) from an archive
7171
// link.
7272
// Note: This does **not** set [Repo.Branch] as it is not derivable from a
7373
// commit-based archive URL.
74-
func RepoFromTarballLink(githubDownload, tarballLink string) (*Repo, error) {
75-
urlPath := strings.TrimPrefix(tarballLink, githubDownload)
74+
func repoFromArchiveLink(githubDownload, archiveLink string) (*Repo, error) {
75+
urlPath := strings.TrimPrefix(archiveLink, githubDownload)
7676
urlPath = strings.TrimPrefix(urlPath, "/")
7777
components := strings.Split(urlPath, "/")
7878
if len(components) < 2 {
79-
return nil, fmt.Errorf("invalid tarball URL %q", tarballLink)
79+
return nil, fmt.Errorf("invalid archive URL %q", archiveLink)
8080
}
8181
repo := &Repo{
8282
Org: components[0],
@@ -138,26 +138,26 @@ func LatestCommitAndChecksum(endpoints *Endpoints, repo *Repo) (commit, sha256 s
138138
return "", "", err
139139
}
140140

141-
tarballURL := TarballLink(endpoints.Download, repo, commit)
141+
tarballURL := tarballLink(endpoints.Download, repo, commit)
142142
sha256, err = urlSha256(tarballURL)
143143
if err != nil {
144144
return "", "", err
145145
}
146146
return commit, sha256, nil
147147
}
148148

149-
// TarballLink constructs a GitHub tarball download URL for the given
149+
// tarballLink constructs a GitHub tarball download URL for the given
150150
// repository and commit SHA.
151151
// Note: This does **not** incorporate the [Repo.Branch] as this produces a
152152
// commit-based archive URL.
153-
func TarballLink(githubDownload string, repo *Repo, sha string) string {
153+
func tarballLink(githubDownload string, repo *Repo, sha string) string {
154154
return fmt.Sprintf("%s/%s/%s/archive/%s.tar.gz", githubDownload, repo.Org, repo.Repo, sha)
155155
}
156156

157-
// DownloadTarball downloads a tarball from the given url to the target
158-
// path, verifying its SHA256 checksum matches expectedSha256. It retries up to
157+
// download downloads a file from the given url to the target path, verifying
158+
// its SHA256 checksum matches expectedSha256. It retries up to
159159
// maxDownloadRetries times with exponential backoff on failure.
160-
func DownloadTarball(ctx context.Context, target, url, expectedSha256 string) error {
160+
func download(ctx context.Context, target, url, expectedSha256 string) error {
161161
if fileExists(target) {
162162
return nil
163163
}
@@ -180,7 +180,7 @@ func DownloadTarball(ctx context.Context, target, url, expectedSha256 string) er
180180
}
181181
}()
182182

183-
if err := downloadTarball(ctx, tempPath, url); err != nil {
183+
if err := downloadFile(ctx, tempPath, url); err != nil {
184184
return err
185185
}
186186
sha, err := computeSHA256(tempPath)
@@ -193,9 +193,9 @@ func DownloadTarball(ctx context.Context, target, url, expectedSha256 string) er
193193
return os.Rename(tempPath, target)
194194
}
195195

196-
// downloadTarball downloads a tarball from the given source URL to the target
197-
// path. It retries up to maxDownloadRetries times with exponential backoff on failure.
198-
func downloadTarball(ctx context.Context, target, source string) error {
196+
// downloadFile downloads a file from the given source URL to the target path.
197+
// It retries up to maxDownloadRetries times with exponential backoff on failure.
198+
func downloadFile(ctx context.Context, target, source string) error {
199199
var err error
200200
for i := range maxDownloadRetries {
201201
if i > 0 {
@@ -260,9 +260,9 @@ func fileExists(name string) bool {
260260
return stat.Mode().IsRegular()
261261
}
262262

263-
// ExtractTarball extracts a gzipped tarball to the specified directory,
263+
// extractTarball extracts a gzipped tarball to the specified directory,
264264
// stripping the top-level directory prefix that GitHub adds to tarballs.
265-
func ExtractTarball(tarballPath, destDir string) error {
265+
func extractTarball(tarballPath, destDir string) error {
266266
f, err := os.Open(tarballPath)
267267
if err != nil {
268268
return err

‎internal/fetch/fetch_test.go‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ import (
3535

3636
const (
3737
testGitHubDn = "https://localhost:12345"
38-
tarballPathTrailer = "/archive/5d5b1bf126485b0e2c972bac41b376438601e266.tar.gz"
38+
archivePathTrailer = "/archive/5d5b1bf126485b0e2c972bac41b376438601e266.tar.gz"
3939
closedServerURL = "https://127.0.0.1:54321"
4040
)
4141

42-
func TestRepoFromTarballLink(t *testing.T) {
43-
got, err := RepoFromTarballLink(testGitHubDn, testGitHubDn+"/org-name/repo-name"+tarballPathTrailer)
42+
func TestRepoFromArchiveLink(t *testing.T) {
43+
got, err := repoFromArchiveLink(testGitHubDn, testGitHubDn+"/org-name/repo-name"+archivePathTrailer)
4444
if err != nil {
4545
t.Fatal(err)
4646
}
@@ -53,22 +53,22 @@ func TestRepoFromTarballLink(t *testing.T) {
5353
}
5454
}
5555

56-
func TestRepoFromTarballLinkErrors(t *testing.T) {
56+
func TestRepoFromArchiveLink_Error(t *testing.T) {
5757
for _, test := range []struct {
5858
name string
59-
tarballLink string
59+
archiveLink string
6060
}{
6161
{
6262
name: "URL path does not match prefix",
63-
tarballLink: "too-short",
63+
archiveLink: "too-short",
6464
},
6565
{
6666
name: "URL path has only one component after prefix removal",
67-
tarballLink: testGitHubDn + "/org",
67+
archiveLink: testGitHubDn + "/org",
6868
},
6969
} {
7070
t.Run(test.name, func(t *testing.T) {
71-
if got, err := RepoFromTarballLink(testGitHubDn, test.tarballLink); err == nil {
71+
if got, err := repoFromArchiveLink(testGitHubDn, test.archiveLink); err == nil {
7272
t.Errorf("expected an error, got=%v", got)
7373
}
7474
})
@@ -207,26 +207,26 @@ func TestTarballLink(t *testing.T) {
207207
want: "https://test.example.com/my-org/my-repo/archive/def456.tar.gz",
208208
},
209209
} {
210-
got := TarballLink(test.githubDownload, test.repo, test.sha)
210+
got := tarballLink(test.githubDownload, test.repo, test.sha)
211211
if got != test.want {
212-
t.Errorf("TarballLink() = %q, want %q", got, test.want)
212+
t.Errorf("tarballLink() = %q, want %q", got, test.want)
213213
}
214214
}
215215
}
216216

217-
func TestDownloadTarballTgzExists(t *testing.T) {
217+
func TestDownload_TgzExists(t *testing.T) {
218218
testDir := t.TempDir()
219219
tarball := makeTestContents(t)
220220
target := path.Join(testDir, "existing-file")
221221
if err := os.WriteFile(target, tarball.Contents, 0644); err != nil {
222222
t.Fatal(err)
223223
}
224-
if err := DownloadTarball(t.Context(), target, "https://unused/placeholder.tar.gz", tarball.Sha256); err != nil {
224+
if err := download(t.Context(), target, "https://unused/placeholder.tar.gz", tarball.Sha256); err != nil {
225225
t.Fatal(err)
226226
}
227227
}
228228

229-
func TestDownloadTarballNeedsDownload(t *testing.T) {
229+
func TestDownload_NeedsDownload(t *testing.T) {
230230
testDir := t.TempDir()
231231
tarball := makeTestContents(t)
232232
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -239,7 +239,7 @@ func TestDownloadTarballNeedsDownload(t *testing.T) {
239239
defer server.Close()
240240

241241
expected := path.Join(testDir, "new-file")
242-
if err := DownloadTarball(t.Context(), expected, server.URL+"/placeholder.tar.gz", tarball.Sha256); err != nil {
242+
if err := download(t.Context(), expected, server.URL+"/placeholder.tar.gz", tarball.Sha256); err != nil {
243243
t.Fatal(err)
244244
}
245245
got, err := os.ReadFile(expected)
@@ -251,7 +251,7 @@ func TestDownloadTarballNeedsDownload(t *testing.T) {
251251
}
252252
}
253253

254-
func TestDownloadTarballChecksumMismatch(t *testing.T) {
254+
func TestDownload_ChecksumMismatch(t *testing.T) {
255255
testDir := t.TempDir()
256256
tarball := makeTestContents(t)
257257
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -263,7 +263,7 @@ func TestDownloadTarballChecksumMismatch(t *testing.T) {
263263
target := path.Join(testDir, "target-file")
264264
wrongSha := "0000000000000000000000000000000000000000000000000000000000000000"
265265

266-
err := DownloadTarball(t.Context(), target, server.URL+"/test.tar.gz", wrongSha)
266+
err := download(t.Context(), target, server.URL+"/test.tar.gz", wrongSha)
267267
if !errors.Is(err, errChecksumMismatch) {
268268
t.Fatalf("expected errChecksumMismatch, got: %v", err)
269269
}
@@ -272,7 +272,7 @@ func TestDownloadTarballChecksumMismatch(t *testing.T) {
272272
}
273273
}
274274

275-
func TestDownloadTarball_ContextCanceled(t *testing.T) {
275+
func TestDownload_ContextCanceled(t *testing.T) {
276276
testDir := t.TempDir()
277277
// Set up a mock web server that sleeps to simulate a long download.
278278
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -285,13 +285,13 @@ func TestDownloadTarball_ContextCanceled(t *testing.T) {
285285
// Create a context that will be canceled explicitly after a short delay.
286286
ctx, cancel := context.WithCancel(t.Context())
287287
// Start a goroutine to cancel the context after a brief period,
288-
// so that `DownloadTarball` is still in progress when the cancellation occurs.
288+
// so that `download` is still in progress when the cancellation occurs.
289289
go func() {
290290
time.Sleep(50 * time.Millisecond)
291291
cancel()
292292
}()
293293

294-
err := DownloadTarball(ctx, target, server.URL+"/test.tar.gz", "any-sha")
294+
err := download(ctx, target, server.URL+"/test.tar.gz", "any-sha")
295295
if !errors.Is(err, context.Canceled) {
296296
t.Fatalf("expected context.Canceled, got: %v", err)
297297
}
@@ -332,7 +332,7 @@ func TestExtractTarball(t *testing.T) {
332332
}
333333

334334
destDir := t.TempDir()
335-
if err := ExtractTarball(tarballPath, destDir); err != nil {
335+
if err := extractTarball(tarballPath, destDir); err != nil {
336336
t.Fatal(err)
337337
}
338338

@@ -393,7 +393,7 @@ func createTestTarball(t *testing.T, topLevelDir string, files map[string]string
393393
return buf.Bytes()
394394
}
395395

396-
func TestExtractTarballErrors(t *testing.T) {
396+
func TestExtractTarball_Errors(t *testing.T) {
397397
for _, test := range []struct {
398398
name string
399399
tarballPath func(t *testing.T) string // Function to create the test file
@@ -467,15 +467,15 @@ func TestExtractTarballErrors(t *testing.T) {
467467
},
468468
} {
469469
t.Run(test.name, func(t *testing.T) {
470-
err := ExtractTarball(test.tarballPath(t), test.dest(t))
470+
err := extractTarball(test.tarballPath(t), test.dest(t))
471471
if (err != nil) != test.wantErr {
472-
t.Errorf("ExtractTarball() error = %v, wantErr %v", err, test.wantErr)
472+
t.Errorf("extractTarball() error = %v, wantErr %v", err, test.wantErr)
473473
}
474474
})
475475
}
476476
}
477477

478-
func TestDownloadTarballErrors(t *testing.T) {
478+
func TestDownload_Error(t *testing.T) {
479479
for _, test := range []struct {
480480
name string
481481
target func(t *testing.T) string
@@ -523,17 +523,17 @@ func TestDownloadTarballErrors(t *testing.T) {
523523
t.Cleanup(func() {
524524
defaultBackoff = 10 * time.Second
525525
})
526-
err := DownloadTarball(context.Background(), test.target(t), test.url(t), test.sha)
526+
err := download(context.Background(), test.target(t), test.url(t), test.sha)
527527
if (err != nil) != test.wantErr {
528-
t.Errorf("DownloadTarball() error = %v, wantErr %v", err, test.wantErr)
528+
t.Errorf("download() error = %v, wantErr %v", err, test.wantErr)
529529
}
530530
})
531531
}
532532
}
533533

534-
func TestDownloadTarballEmptySha(t *testing.T) {
534+
func TestDownload_EmptySha(t *testing.T) {
535535
target := path.Join(t.TempDir(), "target")
536-
err := DownloadTarball(t.Context(), target, "https://any-url", "")
536+
err := download(t.Context(), target, "https://any-url", "")
537537
if !errors.Is(err, errMissingSHA256) {
538538
t.Errorf("expected errMissingSHA256, got: %v", err)
539539
}
@@ -632,7 +632,7 @@ func TestLatestCommitAndChecksum(t *testing.T) {
632632
}
633633
}
634634

635-
func TestDownloadTarballRetryErrorIncludesLastFailure(t *testing.T) {
635+
func TestDownload_RetryErrorIncludesLastFailure(t *testing.T) {
636636
defaultBackoff = time.Millisecond
637637
t.Cleanup(func() {
638638
defaultBackoff = 10 * time.Second
@@ -643,7 +643,7 @@ func TestDownloadTarballRetryErrorIncludesLastFailure(t *testing.T) {
643643
defer server.Close()
644644

645645
target := path.Join(t.TempDir(), "target-file")
646-
err := DownloadTarball(t.Context(), target, server.URL+"/test.tar.gz", "any-sha")
646+
err := download(t.Context(), target, server.URL+"/test.tar.gz", "any-sha")
647647
if err == nil {
648648
t.Fatal("expected an error")
649649
}
@@ -655,7 +655,7 @@ func TestDownloadTarballRetryErrorIncludesLastFailure(t *testing.T) {
655655
}
656656
}
657657

658-
func TestDownloadTarballRetrySucceeds(t *testing.T) {
658+
func TestDownload_RetrySucceeds(t *testing.T) {
659659
defaultBackoff = time.Millisecond
660660
t.Cleanup(func() {
661661
defaultBackoff = 10 * time.Second
@@ -674,7 +674,7 @@ func TestDownloadTarballRetrySucceeds(t *testing.T) {
674674
defer server.Close()
675675

676676
target := path.Join(t.TempDir(), "target-file")
677-
if err := DownloadTarball(t.Context(), target, server.URL+"/test.tar.gz", tarball.Sha256); err != nil {
677+
if err := download(t.Context(), target, server.URL+"/test.tar.gz", tarball.Sha256); err != nil {
678678
t.Fatal(err)
679679
}
680680

0 commit comments

Comments
 (0)