From d33578a2877c788b935b81ba373d17f7e5ccfd52 Mon Sep 17 00:00:00 2001
From: Philipp Sauter <philipp.sauter@exoscale.ch>
Date: Tue, 29 Aug 2023 15:38:01 +0200
Subject: [PATCH] acc test for renaming downloaded versioned file

---
 cmd/storage_download.go                |  1 +
 internal/acctests/sos/download_test.go | 11 ++++++
 internal/acctests/sos/sos_test.go      | 51 +++++++++++++++++++++++---
 pkg/storage/sos/object.go              | 15 ++++++--
 4 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/cmd/storage_download.go b/cmd/storage_download.go
index 56617d99..f16fd23a 100644
--- a/cmd/storage_download.go
+++ b/cmd/storage_download.go
@@ -135,6 +135,7 @@ Examples:
 			Objects:     objects,
 			Destination: dst,
 			Recursive:   recursive,
+			Versions:    downloadVersions || len(versionFilters) > 0,
 			Overwrite:   force,
 			DryRun:      dryRun,
 		})
diff --git a/internal/acctests/sos/download_test.go b/internal/acctests/sos/download_test.go
index a7ba0b63..d176e53b 100644
--- a/internal/acctests/sos/download_test.go
+++ b/internal/acctests/sos/download_test.go
@@ -65,6 +65,17 @@ func (s *SOSSuite) TestDownloadOverwrittenVersionedFile() {
 					"file1.txt": "new expected content",
 				},
 			},
+			{
+				Description:                    "check if v0 can be downloaded and renamed",
+				PreparedFiles:                  LocalFiles{},
+				ClearDownloadDirBeforeCommands: true,
+				Commands: []string{
+					"exo storage download -f --only-versions v0 {bucket}/file1.txt {downloadDir}/file1-v0.txt",
+				},
+				ExpectedDownloadFiles: LocalFiles{
+					"file1-v0.txt": "original content",
+				},
+			},
 		},
 	})
 }
diff --git a/internal/acctests/sos/sos_test.go b/internal/acctests/sos/sos_test.go
index 121afd39..7afbf01c 100644
--- a/internal/acctests/sos/sos_test.go
+++ b/internal/acctests/sos/sos_test.go
@@ -1,20 +1,57 @@
 package sos_test
 
-import "os"
+import (
+	"os"
+	"path/filepath"
+)
 
 type LocalFiles map[string]string
 
 type Step struct {
-	Description           string
-	PreparedFiles         LocalFiles
-	Commands              []string
-	ExpectedDownloadFiles LocalFiles
+	Description                    string
+	PreparedFiles                  LocalFiles
+	ClearDownloadDirBeforeCommands bool
+	Commands                       []string
+	ExpectedDownloadFiles          LocalFiles
 }
 
 type SOSTest struct {
 	Steps []Step
 }
 
+func emptyDirectory(dirPath string) error {
+	// Open the directory
+	dir, err := os.Open(dirPath)
+	if err != nil {
+		return err
+	}
+	defer dir.Close()
+
+	// Read all directory entries
+	entries, err := dir.Readdir(-1)
+	if err != nil {
+		return err
+	}
+
+	// Loop through the entries and remove them
+	for _, entry := range entries {
+		entryPath := filepath.Join(dirPath, entry.Name())
+
+		// Remove file or directory
+		if entry.IsDir() {
+			err = os.RemoveAll(entryPath)
+		} else {
+			err = os.Remove(entryPath)
+		}
+
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (s *SOSSuite) Execute(test SOSTest) {
 	for stepNr, step := range test.Steps {
 		s.T().Logf("step number: %d %q", stepNr, step.Description)
@@ -22,6 +59,10 @@ func (s *SOSSuite) Execute(test SOSTest) {
 			s.writeFile(filename, content)
 		}
 
+		if step.ClearDownloadDirBeforeCommands && !s.NoError(emptyDirectory(s.DownloadDir)) {
+			return
+		}
+
 		for _, command := range step.Commands {
 			s.exo(command)
 		}
diff --git a/pkg/storage/sos/object.go b/pkg/storage/sos/object.go
index f34ddeea..09402319 100644
--- a/pkg/storage/sos/object.go
+++ b/pkg/storage/sos/object.go
@@ -110,19 +110,24 @@ type DownloadConfig struct {
 	Destination string
 	Objects     []object.ObjectInterface
 	Recursive   bool
+	Versions    bool
 	Overwrite   bool
 	DryRun      bool
 }
 
 func (c *Client) DownloadFiles(ctx context.Context, config *DownloadConfig) error {
-	if len(config.Objects) > 1 && !strings.HasSuffix(config.Destination, "/") {
+	var (
+		downloadToDir = strings.HasSuffix(config.Destination, "/")
+	)
+	if len(config.Objects) > 1 && !downloadToDir {
 		return errors.New(`multiple files to download, destination must end with "/"`)
 	}
 
 	// Handle relative filesystem destination (e.g. ".", "../.." etc.)
 	if dstInfo, err := os.Stat(config.Destination); err == nil {
-		if dstInfo.IsDir() && !strings.HasSuffix(config.Destination, "/") {
+		if dstInfo.IsDir() && !downloadToDir {
 			config.Destination += "/"
+			downloadToDir = true
 		}
 	}
 
@@ -130,11 +135,13 @@ func (c *Client) DownloadFiles(ctx context.Context, config *DownloadConfig) erro
 		fmt.Println("[DRY-RUN]")
 	}
 
+	downloadingSingleObject := len(config.Objects) == 1
+
 	for _, obj := range config.Objects {
 		dst := func() string {
 			versionIDSuffix := ""
 			if objectVersion, ok := obj.(object.ObjectVersionInterface); ok {
-				if !objectVersion.GetIsLatest() {
+				if !objectVersion.GetIsLatest() && !downloadingSingleObject {
 					versionIDSuffix = "-" + *objectVersion.GetVersionId()
 				}
 			}
@@ -143,7 +150,7 @@ func (c *Client) DownloadFiles(ctx context.Context, config *DownloadConfig) erro
 				return path.Join(config.Destination, strings.TrimPrefix(aws.ToString(obj.GetKey()), config.Prefix)+versionIDSuffix)
 			}
 
-			if strings.HasSuffix(config.Destination, "/") {
+			if downloadToDir {
 				return path.Join(config.Destination, path.Base(aws.ToString(obj.GetKey()))+versionIDSuffix)
 			}