Also use the terminology Inplace in DownloadConfig for clarity/consistency

* swapped boolean checks
* swapped in tests too
This commit is contained in:
Adrien Delorme 2018-08-15 15:26:31 +02:00
parent 82e480a285
commit 863222b1e2
3 changed files with 14 additions and 18 deletions

View File

@ -38,10 +38,10 @@ type DownloadConfig struct {
// DownloaderMap maps a schema to a Download. // DownloaderMap maps a schema to a Download.
DownloaderMap map[string]Downloader DownloaderMap map[string]Downloader
// If true, this will copy even a local file to the target // Inplace indicates wether to copy file
// location. If false, then it will "download" the file by just // versus using it inplace.
// returning the local path to the file. // Inplace can only be true when referencing local files.
CopyFile bool Inplace bool
// The hashing implementation to use to checksum the downloaded file. // The hashing implementation to use to checksum the downloaded file.
Hash hash.Hash 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) 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 we're copying the file, then just use the actual downloader
if d.config.CopyFile { if d.config.Inplace == false {
var f *os.File var f *os.File
finalPath = d.config.TargetPath finalPath = d.config.TargetPath
@ -189,7 +192,7 @@ func (d *DownloadClient) Get() (string, error) {
verify, err = d.VerifyChecksum(finalPath) verify, err = d.VerifyChecksum(finalPath)
if err == nil && !verify { if err == nil && !verify {
// Only delete the file if we made a copy or downloaded it // Only delete the file if we made a copy or downloaded it
if d.config.CopyFile { if d.config.Inplace == false {
os.Remove(finalPath) os.Remove(finalPath)
} }

View File

@ -58,7 +58,6 @@ func TestDownloadClient_basic(t *testing.T) {
client := NewDownloadClient(&DownloadConfig{ client := NewDownloadClient(&DownloadConfig{
Url: ts.URL + "/basic.txt", Url: ts.URL + "/basic.txt",
TargetPath: tf.Name(), TargetPath: tf.Name(),
CopyFile: true,
}) })
path, err := client.Get() path, err := client.Get()
@ -94,7 +93,6 @@ func TestDownloadClient_checksumBad(t *testing.T) {
TargetPath: tf.Name(), TargetPath: tf.Name(),
Hash: HashForType("md5"), Hash: HashForType("md5"),
Checksum: checksum, Checksum: checksum,
CopyFile: true,
}) })
if _, err := client.Get(); err == nil { if _, err := client.Get(); err == nil {
@ -120,7 +118,6 @@ func TestDownloadClient_checksumGood(t *testing.T) {
TargetPath: tf.Name(), TargetPath: tf.Name(),
Hash: HashForType("md5"), Hash: HashForType("md5"),
Checksum: checksum, Checksum: checksum,
CopyFile: true,
}) })
path, err := client.Get() path, err := client.Get()
@ -152,7 +149,6 @@ func TestDownloadClient_checksumNoDownload(t *testing.T) {
TargetPath: "./test-fixtures/root/another.txt", TargetPath: "./test-fixtures/root/another.txt",
Hash: HashForType("md5"), Hash: HashForType("md5"),
Checksum: checksum, Checksum: checksum,
CopyFile: true,
}) })
path, err := client.Get() path, err := client.Get()
if err != nil { if err != nil {
@ -210,7 +206,6 @@ func TestDownloadClient_resume(t *testing.T) {
client := NewDownloadClient(&DownloadConfig{ client := NewDownloadClient(&DownloadConfig{
Url: ts.URL, Url: ts.URL,
TargetPath: tf.Name(), TargetPath: tf.Name(),
CopyFile: true,
}) })
path, err := client.Get() path, err := client.Get()
@ -270,7 +265,6 @@ func TestDownloadClient_usesDefaultUserAgent(t *testing.T) {
config := &DownloadConfig{ config := &DownloadConfig{
Url: server.URL, Url: server.URL,
TargetPath: tf.Name(), TargetPath: tf.Name(),
CopyFile: true,
} }
client := NewDownloadClient(config) client := NewDownloadClient(config)
@ -303,7 +297,6 @@ func TestDownloadClient_setsUserAgent(t *testing.T) {
Url: server.URL, Url: server.URL,
TargetPath: tf.Name(), TargetPath: tf.Name(),
UserAgent: "fancy user agent", UserAgent: "fancy user agent",
CopyFile: true,
} }
client := NewDownloadClient(config) 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 // This should be wrong. We want to make sure we don't delete
Checksum: []byte("nope"), Checksum: []byte("nope"),
Hash: HashForType("sha256"), Hash: HashForType("sha256"),
CopyFile: false, Inplace: true,
} }
client := NewDownloadClient(config) 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 // This should be wrong. We want to make sure we don't delete
Checksum: []byte("nope"), Checksum: []byte("nope"),
Hash: HashForType("sha256"), Hash: HashForType("sha256"),
CopyFile: false, Inplace: true,
} }
// go go go // go go go

View File

@ -48,7 +48,7 @@ type StepDownload struct {
// Inplace indicates wether to copy file // Inplace indicates wether to copy file
// versus using it inplace. // versus using it inplace.
// Inplace is only used when referencing local ISO files. // Inplace can only be true when referencing local files.
Inplace bool Inplace bool
} }
@ -94,7 +94,7 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste
config := &DownloadConfig{ config := &DownloadConfig{
Url: url, Url: url,
TargetPath: targetPath, TargetPath: targetPath,
CopyFile: !s.Inplace, Inplace: s.Inplace,
Hash: HashForType(s.ChecksumType), Hash: HashForType(s.ChecksumType),
Checksum: checksum, Checksum: checksum,
UserAgent: useragent.String(), UserAgent: useragent.String(),