From 8b90a0a0c52d86fdaed836e9961702e771ddaeda Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Thu, 14 Nov 2024 04:30:04 +0200
Subject: [PATCH] Split Download() to smaller functions

Extract getCache() to handle getting and file from the cache, and
fetch() for fetching a file from the remote to the cache. This will
allow locking around the code checking and updating the cache, and much
easier to maintain and understand.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/downloader/downloader.go | 86 +++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 30 deletions(-)

diff --git a/pkg/downloader/downloader.go b/pkg/downloader/downloader.go
index ade98bd3b90..5f1f729b90d 100644
--- a/pkg/downloader/downloader.go
+++ b/pkg/downloader/downloader.go
@@ -229,6 +229,20 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
 		return res, nil
 	}
 
+	res, err := getCached(ctx, localPath, remote, o)
+	if err != nil {
+		return nil, err
+	}
+	if res != nil {
+		return res, nil
+	}
+	return fetch(ctx, localPath, remote, o)
+}
+
+// getCached tries to copy the file from the cache to local path. Return result,
+// nil if the file was copied, nil, nil if the file is not in the cache or the
+// cache needs update, or nil, error on fatal error.
+func getCached(ctx context.Context, localPath, remote string, o options) (*Result, error) {
 	shad := cacheDirectoryPath(o.cacheDir, remote)
 	shadData := filepath.Join(shad, "data")
 	shadTime := filepath.Join(shad, "time")
@@ -237,41 +251,53 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
 	if err != nil {
 		return nil, err
 	}
-	if _, err := os.Stat(shadData); err == nil {
-		logrus.Debugf("file %q is cached as %q", localPath, shadData)
-		useCache := true
-		if _, err := os.Stat(shadDigest); err == nil {
-			logrus.Debugf("Comparing digest %q with the cached digest file %q, not computing the actual digest of %q",
-				o.expectedDigest, shadDigest, shadData)
-			if err := validateCachedDigest(shadDigest, o.expectedDigest); err != nil {
-				return nil, err
-			}
-			if err := copyLocal(ctx, localPath, shadData, ext, o.decompress, "", ""); err != nil {
+	if _, err := os.Stat(shadData); err != nil {
+		return nil, nil
+	}
+	ext := path.Ext(remote)
+	logrus.Debugf("file %q is cached as %q", localPath, shadData)
+	if _, err := os.Stat(shadDigest); err == nil {
+		logrus.Debugf("Comparing digest %q with the cached digest file %q, not computing the actual digest of %q",
+			o.expectedDigest, shadDigest, shadData)
+		if err := validateCachedDigest(shadDigest, o.expectedDigest); err != nil {
+			return nil, err
+		}
+		if err := copyLocal(ctx, localPath, shadData, ext, o.decompress, "", ""); err != nil {
+			return nil, err
+		}
+	} else {
+		if match, lmCached, lmRemote, err := matchLastModified(ctx, shadTime, remote); err != nil {
+			logrus.WithError(err).Info("Failed to retrieve last-modified for cached digest-less image; using cached image.")
+		} else if match {
+			if err := copyLocal(ctx, localPath, shadData, ext, o.decompress, o.description, o.expectedDigest); err != nil {
 				return nil, err
 			}
 		} else {
-			if match, lmCached, lmRemote, err := matchLastModified(ctx, shadTime, remote); err != nil {
-				logrus.WithError(err).Info("Failed to retrieve last-modified for cached digest-less image; using cached image.")
-			} else if match {
-				if err := copyLocal(ctx, localPath, shadData, ext, o.decompress, o.description, o.expectedDigest); err != nil {
-					return nil, err
-				}
-			} else {
-				logrus.Infof("Re-downloading digest-less image: last-modified mismatch (cached: %q, remote: %q)", lmCached, lmRemote)
-				useCache = false
-			}
-		}
-		if useCache {
-			res := &Result{
-				Status:          StatusUsedCache,
-				CachePath:       shadData,
-				LastModified:    readTime(shadTime),
-				ContentType:     readFile(shadType),
-				ValidatedDigest: o.expectedDigest != "",
-			}
-			return res, nil
+			logrus.Infof("Re-downloading digest-less image: last-modified mismatch (cached: %q, remote: %q)", lmCached, lmRemote)
+			return nil, nil
 		}
 	}
+	res := &Result{
+		Status:          StatusUsedCache,
+		CachePath:       shadData,
+		LastModified:    readTime(shadTime),
+		ContentType:     readFile(shadType),
+		ValidatedDigest: o.expectedDigest != "",
+	}
+	return res, nil
+}
+
+// fetch downloads remote to the cache and copy the cached file to local path.
+func fetch(ctx context.Context, localPath, remote string, o options) (*Result, error) {
+	shad := cacheDirectoryPath(o.cacheDir, remote)
+	shadData := filepath.Join(shad, "data")
+	shadTime := filepath.Join(shad, "time")
+	shadType := filepath.Join(shad, "type")
+	shadDigest, err := cacheDigestPath(shad, o.expectedDigest)
+	if err != nil {
+		return nil, err
+	}
+	ext := path.Ext(remote)
 	if err := os.MkdirAll(shad, 0o700); err != nil {
 		return nil, err
 	}