From 863222b1e28876285c1d0584dccb2c195cbd44df Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Wed, 15 Aug 2018 15:26:31 +0200 Subject: [PATCH] Also use the terminology Inplace in DownloadConfig for clarity/consistency * swapped boolean checks * swapped in tests too --- common/download.go | 17 ++++++++++------- common/download_test.go | 11 ++--------- common/step_download.go | 4 ++-- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/common/download.go b/common/download.go index 33fa7fed9..84776bee9 100644 --- a/common/download.go +++ b/common/download.go @@ -38,10 +38,10 @@ type DownloadConfig struct { // DownloaderMap maps a schema to a Download. DownloaderMap map[string]Downloader - // If true, this will copy even a local file to the target - // location. If false, then it will "download" the file by just - // returning the local path to the file. - CopyFile bool + // Inplace indicates wether to copy file + // versus using it inplace. + // Inplace can only be true when referencing local files. + Inplace bool // The hashing implementation to use to checksum the downloaded file. Hash hash.Hash @@ -154,10 +154,13 @@ func (d *DownloadClient) Get() (string, error) { return "", fmt.Errorf("Unable to treat uri scheme %s as a Downloader. : %T", u.Scheme, d.downloader) } - local, _ := d.downloader.(LocalDownloader) + local, ok := d.downloader.(LocalDownloader) + if !ok && d.config.Inplace { + d.config.Inplace = false + } // If we're copying the file, then just use the actual downloader - if d.config.CopyFile { + if d.config.Inplace == false { var f *os.File finalPath = d.config.TargetPath @@ -189,7 +192,7 @@ func (d *DownloadClient) Get() (string, error) { verify, err = d.VerifyChecksum(finalPath) if err == nil && !verify { // Only delete the file if we made a copy or downloaded it - if d.config.CopyFile { + if d.config.Inplace == false { os.Remove(finalPath) } diff --git a/common/download_test.go b/common/download_test.go index c9175b013..835b2d54b 100644 --- a/common/download_test.go +++ b/common/download_test.go @@ -58,7 +58,6 @@ func TestDownloadClient_basic(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL + "/basic.txt", TargetPath: tf.Name(), - CopyFile: true, }) path, err := client.Get() @@ -94,7 +93,6 @@ func TestDownloadClient_checksumBad(t *testing.T) { TargetPath: tf.Name(), Hash: HashForType("md5"), Checksum: checksum, - CopyFile: true, }) if _, err := client.Get(); err == nil { @@ -120,7 +118,6 @@ func TestDownloadClient_checksumGood(t *testing.T) { TargetPath: tf.Name(), Hash: HashForType("md5"), Checksum: checksum, - CopyFile: true, }) path, err := client.Get() @@ -152,7 +149,6 @@ func TestDownloadClient_checksumNoDownload(t *testing.T) { TargetPath: "./test-fixtures/root/another.txt", Hash: HashForType("md5"), Checksum: checksum, - CopyFile: true, }) path, err := client.Get() if err != nil { @@ -210,7 +206,6 @@ func TestDownloadClient_resume(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL, TargetPath: tf.Name(), - CopyFile: true, }) path, err := client.Get() @@ -270,7 +265,6 @@ func TestDownloadClient_usesDefaultUserAgent(t *testing.T) { config := &DownloadConfig{ Url: server.URL, TargetPath: tf.Name(), - CopyFile: true, } client := NewDownloadClient(config) @@ -303,7 +297,6 @@ func TestDownloadClient_setsUserAgent(t *testing.T) { Url: server.URL, TargetPath: tf.Name(), UserAgent: "fancy user agent", - CopyFile: true, } client := NewDownloadClient(config) @@ -402,7 +395,7 @@ func TestDownloadFileUrl(t *testing.T) { // This should be wrong. We want to make sure we don't delete Checksum: []byte("nope"), Hash: HashForType("sha256"), - CopyFile: false, + Inplace: true, } client := NewDownloadClient(config) @@ -432,7 +425,7 @@ func SimulateFileUriDownload(t *testing.T, uri string) (string, error) { // This should be wrong. We want to make sure we don't delete Checksum: []byte("nope"), Hash: HashForType("sha256"), - CopyFile: false, + Inplace: true, } // go go go diff --git a/common/step_download.go b/common/step_download.go index c35e9abef..6d2b850a0 100644 --- a/common/step_download.go +++ b/common/step_download.go @@ -48,7 +48,7 @@ type StepDownload struct { // Inplace indicates wether to copy file // versus using it inplace. - // Inplace is only used when referencing local ISO files. + // Inplace can only be true when referencing local files. Inplace bool } @@ -94,7 +94,7 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste config := &DownloadConfig{ Url: url, TargetPath: targetPath, - CopyFile: !s.Inplace, + Inplace: s.Inplace, Hash: HashForType(s.ChecksumType), Checksum: checksum, UserAgent: useragent.String(),