Merge Download subroutine for resources

Author: cmaglie

arduino/cores/packagemanager/download.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	"github.com/arduino/arduino-cli/arduino/cores"
+	"github.com/arduino/arduino-cli/arduino/resources"
 	"go.bug.st/downloader/v2"
 	semver "go.bug.st/relaxed-semver"
 )
@@ -117,16 +118,16 @@ func (pm *PackageManager) FindPlatformReleaseDependencies(item *PlatformReferenc
 
 // DownloadToolRelease downloads a ToolRelease. If the tool is already downloaded a nil Downloader
 // is returned.
-func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config) (*downloader.Downloader, error) {
+func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, label string, progressCB resources.DownloadProgressCB) error {
 	resource := tool.GetCompatibleFlavour()
 	if resource == nil {
-		return nil, fmt.Errorf(tr("tool not available for your OS"))
+		return fmt.Errorf(tr("tool not available for your OS"))
 	}
-	return resource.Download(pm.DownloadDir, config)
+	return resource.Download(pm.DownloadDir, config, label, progressCB)
 }
 
 // DownloadPlatformRelease downloads a PlatformRelease. If the platform is already downloaded a
 // nil Downloader is returned.
-func (pm *PackageManager) DownloadPlatformRelease(platform *cores.PlatformRelease, config *downloader.Config) (*downloader.Downloader, error) {
-	return platform.Resource.Download(pm.DownloadDir, config)
+func (pm *PackageManager) DownloadPlatformRelease(platform *cores.PlatformRelease, config *downloader.Config, label string, progressCB resources.DownloadProgressCB) error {
+	return platform.Resource.Download(pm.DownloadDir, config, label, progressCB)
 }

arduino/resources/download.go

@@ -26,11 +26,12 @@ import (
 	"go.bug.st/downloader/v2"
 )
 
-// Download a DownloadResource.
-func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.Config) (*downloader.Downloader, error) {
+// Download performs a download loop using the provided downloader.Downloader.
+// Messages are passed back to the DownloadProgressCB using label as text for the File field.
+func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.Config, label string, downloadCB DownloadProgressCB) error {
 	path, err := r.ArchivePath(downloadDir)
 	if err != nil {
-		return nil, fmt.Errorf(tr("getting archive path: %s"), err)
+		return fmt.Errorf(tr("getting archive path: %s"), err)
 	}
 
 	if _, err := path.Stat(); os.IsNotExist(err) {
@@ -40,17 +41,46 @@ func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.
 		ok, err := r.TestLocalArchiveIntegrity(downloadDir)
 		if err != nil || !ok {
 			if err := path.Remove(); err != nil {
-				return nil, fmt.Errorf(tr("removing corrupted archive file: %s"), err)
+				return fmt.Errorf(tr("removing corrupted archive file: %s"), err)
 			}
 		} else {
 			// File is cached, nothing to do here
-			return nil, nil
+
+			// This signal means that the file is already downloaded
+			downloadCB(&DownloadProgress{
+				File:      label,
+				Completed: true,
+			})
+			return nil
 		}
 	} else {
-		return nil, fmt.Errorf(tr("getting archive file info: %s"), err)
+		return fmt.Errorf(tr("getting archive file info: %s"), err)
+	}
+
+	d, err := downloader.DownloadWithConfig(path.String(), r.URL, *config)
+	if err != nil {
+		return err
+	}
+	downloadCB(&DownloadProgress{
+		File:      label,
+		URL:       d.URL,
+		TotalSize: d.Size(),
+	})
+
+	err = d.RunAndPoll(func(downloaded int64) {
+		downloadCB(&DownloadProgress{Downloaded: downloaded})
+	}, 250*time.Millisecond)
+	if err != nil {
+		return err
+	}
+
+	// The URL is not reachable for some reason
+	if d.Resp.StatusCode >= 400 && d.Resp.StatusCode <= 599 {
+		return &arduino.FailedDownloadError{Message: tr("Server responded with: %s", d.Resp.Status)}
 	}
 
-	return downloader.DownloadWithConfig(path.String(), r.URL, *config)
+	downloadCB(&DownloadProgress{Completed: true})
+	return nil
 }
 
 // GetDownloaderConfig returns the downloader configuration based on
@@ -82,33 +112,3 @@ type DownloadProgress struct {
 
 // DownloadProgressCB is a callback function to report download progress
 type DownloadProgressCB func(progress *DownloadProgress)
-
-// Download performs a download loop using the provided downloader.Downloader.
-// Messages are passed back to the DownloadProgressCB using label as text for the File field.
-func Download(d *downloader.Downloader, label string, downloadCB DownloadProgressCB) error {
-	if d == nil {
-		// This signal means that the file is already downloaded
-		downloadCB(&DownloadProgress{
-			File:      label,
-			Completed: true,
-		})
-		return nil
-	}
-	downloadCB(&DownloadProgress{
-		File:      label,
-		URL:       d.URL,
-		TotalSize: d.Size(),
-	})
-	d.RunAndPoll(func(downloaded int64) {
-		downloadCB(&DownloadProgress{Downloaded: downloaded})
-	}, 250*time.Millisecond)
-	if d.Error() != nil {
-		return d.Error()
-	}
-	// The URL is not reachable for some reason
-	if d.Resp.StatusCode >= 400 && d.Resp.StatusCode <= 599 {
-		return &arduino.FailedDownloadError{Message: tr("Server responded with: %s", d.Resp.Status)}
-	}
-	downloadCB(&DownloadProgress{Completed: true})
-	return nil
-}

arduino/resources/helpers_test.go

@@ -55,9 +55,7 @@ func TestDownloadApplyUserAgentHeaderUsingConfig(t *testing.T) {
 
 	httpClient := httpclient.NewWithConfig(&httpclient.Config{UserAgent: goldUserAgentValue})
 
-	d, err := r.Download(tmp, &downloader.Config{HttpClient: *httpClient})
-	require.NoError(t, err)
-	err = d.Run()
+	err = r.Download(tmp, &downloader.Config{HttpClient: *httpClient}, "", func(progress *DownloadProgress) {})
 	require.NoError(t, err)
 
 	// leverage the download helper to download the echo for the request made by the downloader itself

arduino/resources/resources_test.go

@@ -44,9 +44,7 @@ func TestDownloadAndChecksums(t *testing.T) {
 	require.NoError(t, err)
 
 	downloadAndTestChecksum := func() {
-		d, err := r.Download(tmp, &downloader.Config{})
-		require.NoError(t, err)
-		err = d.Run()
+		err := r.Download(tmp, &downloader.Config{}, "", func(*DownloadProgress) {})
 		require.NoError(t, err)
 
 		data, err := testFile.ReadFile()
@@ -60,9 +58,8 @@ func TestDownloadAndChecksums(t *testing.T) {
 	downloadAndTestChecksum()
 
 	// Download with cached file
-	d, err := r.Download(tmp, &downloader.Config{})
+	err = r.Download(tmp, &downloader.Config{}, "", func(*DownloadProgress) {})
 	require.NoError(t, err)
-	require.Nil(t, d)
 
 	// Download if cached file has data in excess (redownload)
 	data, err := testFile.ReadFile()

commands/bundled_tools.go

@@ -29,11 +29,7 @@ func DownloadToolRelease(pm *packagemanager.PackageManager, toolRelease *cores.T
 	if err != nil {
 		return err
 	}
-	resp, err := pm.DownloadToolRelease(toolRelease, config)
-	if err != nil {
-		return err
-	}
-	return resources.Download(resp, toolRelease.String(), downloadCB.FromRPC())
+	return pm.DownloadToolRelease(toolRelease, config, toolRelease.String(), downloadCB.FromRPC())
 }
 
 // InstallToolRelease installs a ToolRelease

commands/core/download.go

@@ -71,11 +71,7 @@ func downloadPlatform(pm *packagemanager.PackageManager, platformRelease *cores.
 	if err != nil {
 		return &arduino.FailedDownloadError{Message: tr("Error downloading platform %s", platformRelease), Cause: err}
 	}
-	resp, err := pm.DownloadPlatformRelease(platformRelease, config)
-	if err != nil {
-		return &arduino.FailedDownloadError{Message: tr("Error downloading platform %s", platformRelease), Cause: err}
-	}
-	return resources.Download(resp, platformRelease.String(), downloadCB.FromRPC())
+	return pm.DownloadPlatformRelease(platformRelease, config, platformRelease.String(), downloadCB.FromRPC())
 }
 
 func downloadTool(pm *packagemanager.PackageManager, tool *cores.ToolRelease, downloadCB commands.DownloadProgressCB) error {

commands/instances.go

@@ -711,9 +711,7 @@ func Upgrade(ctx context.Context, req *rpc.UpgradeRequest, downloadCB DownloadPr
 
 			// Downloads latest library release
 			taskCB(&rpc.TaskProgress{Name: tr("Downloading %s", available)})
-			if d, err := available.Resource.Download(lm.DownloadsDir, downloaderConfig); err != nil {
-				return &arduino.FailedDownloadError{Message: tr("Error downloading library"), Cause: err}
-			} else if err := resources.Download(d, available.String(), downloadCB.FromRPC()); err != nil {
+			if err := available.Resource.Download(lm.DownloadsDir, downloaderConfig, available.String(), downloadCB.FromRPC()); err != nil {
 				return &arduino.FailedDownloadError{Message: tr("Error downloading library"), Cause: err}
 			}
 
@@ -794,9 +792,7 @@ func Upgrade(ctx context.Context, req *rpc.UpgradeRequest, downloadCB DownloadPr
 				}
 
 				// Downloads platform
-				if d, err := pm.DownloadPlatformRelease(latest, downloaderConfig); err != nil {
-					return &arduino.FailedDownloadError{Message: tr("Error downloading platform %s", latest), Cause: err}
-				} else if err := resources.Download(d, latest.String(), downloadCB.FromRPC()); err != nil {
+				if err := pm.DownloadPlatformRelease(latest, downloaderConfig, latest.String(), downloadCB.FromRPC()); err != nil {
 					return &arduino.FailedDownloadError{Message: tr("Error downloading platform %s", latest), Cause: err}
 				}
 

commands/lib/download.go

@@ -61,9 +61,7 @@ func downloadLibrary(lm *librariesmanager.LibrariesManager, libRelease *librarie
 	if err != nil {
 		return &arduino.FailedDownloadError{Message: tr("Can't download library"), Cause: err}
 	}
-	if d, err := libRelease.Resource.Download(lm.DownloadsDir, config); err != nil {
-		return &arduino.FailedDownloadError{Message: tr("Can't download library"), Cause: err}
-	} else if err := resources.Download(d, libRelease.String(), downloadCB.FromRPC()); err != nil {
+	if err := libRelease.Resource.Download(lm.DownloadsDir, config, libRelease.String(), downloadCB.FromRPC()); err != nil {
 		return &arduino.FailedDownloadError{Message: tr("Can't download library"), Cause: err}
 	}
 	taskCB(&rpc.TaskProgress{Completed: true})