Skip to content

Commit 37fbfe5

Browse files
committed
[download] Delete partial file at the beginning
The partial file should be deleted at the beginning before checking the ingoreExist flag because if the user make use of this flag and a partial file is present would not be deleted as it was before. Modified the unit test as well because it was checking if the file is deleted by calling the downloadFile function and this action is talking place before that now.
1 parent ea21088 commit 37fbfe5

File tree

2 files changed

+21
-13
lines changed

2 files changed

+21
-13
lines changed

download/download.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,14 @@ func fileCase(args []string, token string, fileList bool) error {
252252

253253
for _, filePath := range files {
254254
outputPath := filepath.Join(outDir, filePath)
255+
// Cleanup .part if it exists since we are skipping
256+
partPath := outputPath + ".part"
257+
if _, err := os.Stat(partPath); err == nil {
258+
if err := os.Remove(partPath); err != nil {
259+
fmt.Printf("Warning: could not remove old partial file %s: %v\n", partPath, err)
260+
}
261+
}
262+
255263
if ignoreExisting {
256264
if _, err := os.Stat(outputPath); err == nil {
257265
fmt.Printf("Skipping download to %s, file already exists\n", outputPath)
@@ -351,15 +359,6 @@ func handleExistingFile(filePath string) (bool, error) {
351359
return false, nil
352360
}
353361

354-
partPath := filePath + ".part"
355-
356-
// If a full file exists, any partial file is definitely garbage and should be removed
357-
if _, err := os.Stat(partPath); err == nil {
358-
if err := os.Remove(partPath); err != nil {
359-
fmt.Printf("Warning: could not remove old partial file %s: %v\n", partPath, err)
360-
}
361-
}
362-
363362
if ignoreExisting {
364363
fmt.Printf("Skipping download to %s, file already exists\n", filePath)
365364

download/download_test.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -514,11 +514,16 @@ func (s *DownloadTestSuite) TestDownloadCleanupOnFailure() {
514514
}
515515

516516
func (s *DownloadTestSuite) TestDownloadCleanupPartialFileWhenFullExists() {
517-
targetFile := "cleanup-part-test.c4gh"
517+
// We need to use a file that exists in our mock server to test through fileCase/Download
518+
targetFile := "files/dummy-file.txt.c4gh"
518519
fullPath := filepath.Join(s.tempDir, targetFile)
519520
partPath := fullPath + ".part"
520521

521-
err := os.WriteFile(fullPath, []byte("old content"), 0600)
522+
// Create the subdirectory first
523+
err := os.MkdirAll(filepath.Dir(fullPath), 0750)
524+
s.Require().NoError(err)
525+
526+
err = os.WriteFile(fullPath, []byte("old content"), 0600)
522527
s.Require().NoError(err)
523528
err = os.WriteFile(partPath, []byte("partial content"), 0600)
524529
s.Require().NoError(err)
@@ -533,9 +538,13 @@ func (s *DownloadTestSuite) TestDownloadCleanupPartialFileWhenFullExists() {
533538
_ = w.Close()
534539
}()
535540

536-
outDir = s.tempDir
541+
os.Args = []string{"", "download", targetFile}
542+
downloadCmd.Flag("url").Value.Set(s.httpTestServer.URL)
543+
downloadCmd.Flag("outdir").Value.Set(s.tempDir)
544+
downloadCmd.Flag("dataset-id").Value.Set("TES01")
545+
537546
sessionOverwrite = helpers.OverwriteNone
538-
err = downloadFile(s.httpTestServer.URL, s.accessToken, "", targetFile)
547+
err = downloadCmd.Execute()
539548
s.NoError(err)
540549

541550
// Verify full content is NOT overwritten

0 commit comments

Comments
 (0)