feat: pkg/uil/downloader Enhancement

pkg/util/downloader/downloader.go

@@ -6,22 +6,20 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-
-	"github.com/devstream-io/devstream/pkg/util/log"
 )
 
 // if filename is "", use the remote filename at local.
 func Download(url, filename, targetDir string) (int64, error) {
-	log.Debugf("Target dir: %s.", targetDir)
-	log.Debugf("URL: %s.", url)
+	if url == "" {
+		return 0, fmt.Errorf("url must not be empty: %s", url)
+	}
+	if filename == "." {
+		return 0, fmt.Errorf("filename must not be [%s]", filename)
+	}
 	if filename == "" {
 		// when url is empty filepath.Base(url) will return "."
 		filename = filepath.Base(url)
 	}
-	log.Debugf("Filename: %s.", filename)
-	if filename == "." {
-		return 0, fmt.Errorf("failed to get the filename from url: %s", url)
-	}
 
 	if err := os.MkdirAll(targetDir, 0755); err != nil {
 		return 0, err
@@ -31,11 +29,13 @@ func Download(url, filename, targetDir string) (int64, error) {
 	if err != nil {
 		return 0, err
 	}
-
+	defer f.Close()
 	resp, err := http.Get(url)
 	if err != nil {
 		return 0, err
 	}
-
+	if resp.Body != nil {
+		defer resp.Body.Close()
+	}
 	return io.Copy(f, resp.Body)
 }

pkg/util/downloader/downloader_test.go

@@ -2,36 +2,60 @@ package downloader_test
 
 import (
 	"os"
+	"path/filepath"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
 	"github.com/devstream-io/devstream/pkg/util/downloader"
 )
 
+func CreateFile(dir, filename string) (*os.File, error) {
+	return os.Create(filepath.Join(dir, filename))
+}
+
 var _ = Describe("Downloader", func() {
 	Context("Downloader test", func() {
 		var url = "https://github.com/devstream-io/devstream/releases/download/v0.0.1/argocdapp_0.0.1.so"
+		var error_url = "://github.com/devstream-io/devstream/releases/download/v0.0.1/argocdapp_0.0.1.so"
 		var targetDir = "tmp"
 
-		It("returns error if filename is empty and can't be parsed from the url", func() {
-			size, err := downloader.Download(url, ".", targetDir)
+		It("returns an error when url is empty", func() {
+			size, err := downloader.Download("", ".", targetDir)
 			Expect(err).To(HaveOccurred())
 			Expect(size).To(Equal(int64(0)))
 		})
 
-		It("returns an error when url and filename are empty", func() {
-			size, err := downloader.Download("", "", targetDir)
+		It("returns an error when filename is [.]", func() {
+			size, err := downloader.Download(url, ".", targetDir)
 			Expect(err).To(HaveOccurred())
 			Expect(size).To(Equal(int64(0)))
 		})
 
-		It("should download the file properly", func() {
+		It("returns an error when filename is empty and download properly", func() {
 			size, err := downloader.Download(url, "", targetDir)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(size).NotTo(Equal(int64(0)))
 		})
 
+		It("returns an error when filename is dir", func() {
+			size, err := downloader.Download(url, "/", targetDir)
+			Expect(err).To(HaveOccurred())
+			Expect(size).To(Equal(int64(0)))
+		})
+
+		It("returns an error when the targetDir is empty", func() {
+			size, err := downloader.Download(url, "download.txt", "")
+			Expect(err).To(HaveOccurred())
+			Expect(size).To(Equal(int64(0)))
+		})
+
+		It("returns an error when the url is not right", func() {
+			size, err := downloader.Download(error_url, "download.txt", targetDir)
+			Expect(err).To(HaveOccurred())
+			Expect(size).To(Equal(int64(0)))
+		})
+
 		AfterEach(func() {
 			err := os.RemoveAll(targetDir)
 			Expect(err).NotTo(HaveOccurred())