Cache package list in memory instead of filesystem (#4309)

pkg/pkg/cache.go

@@ -1,27 +1,23 @@
 package pkg
 
 import (
-	"fmt"
-	"io"
-	"os"
-	"path/filepath"
 	"time"
-
-	"github.com/stashapp/stash/pkg/hash/md5"
-	"github.com/stashapp/stash/pkg/logger"
 )
 
-const cacheSubDir = "package_lists"
-
-type repositoryCache struct {
-	cachePath string
+type cacheEntry struct {
+	lastModified time.Time
+	data         []RemotePackage
 }
 
-func (c *repositoryCache) path(url string) string {
-	// convert the url to md5
-	hash := md5.FromString(url)
+type repositoryCache struct {
+	// cache maps the URL to the last modified time and the data
+	cache map[string]cacheEntry
+}
 
-	return filepath.Join(c.cachePath, cacheSubDir, hash)
+func (c *repositoryCache) ensureCache() {
+	if c.cache == nil {
+		c.cache = make(map[string]cacheEntry)
+	}
 }
 
 func (c *repositoryCache) lastModified(url string) *time.Time {
@@ -29,54 +25,35 @@ func (c *repositoryCache) lastModified(url string) *time.Time {
 		return nil
 	}
 
-	path := c.path(url)
-	s, err := os.Stat(path)
-	if err != nil {
-		// ignore
-		logger.Debugf("error getting cached file %s: %v", path, err)
+	c.ensureCache()
+	e, found := c.cache[url]
+
+	if !found {
 		return nil
 	}
 
-	ret := s.ModTime()
-	return &ret
+	return &e.lastModified
 }
 
-func (c *repositoryCache) getPackageList(url string) (io.ReadCloser, error) {
-	path := c.path(url)
-	ret, err := os.Open(path)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get file %q: %w", path, err)
+func (c *repositoryCache) getPackageList(url string) []RemotePackage {
+	c.ensureCache()
+	e, found := c.cache[url]
+
+	if !found {
+		return nil
 	}
 
-	return ret, nil
+	return e.data
 }
 
-func (c *repositoryCache) cacheFile(url string, data io.ReadCloser) (io.ReadCloser, error) {
+func (c *repositoryCache) cacheList(url string, lastModified time.Time, data []RemotePackage) {
 	if c == nil {
-		return data, nil
+		return
 	}
 
-	path := c.path(url)
-
-	if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
-		// ignore, just return the original file
-		logger.Debugf("error creating cache path %s: %v", filepath.Dir(path), err)
-		return data, nil
+	c.ensureCache()
+	c.cache[url] = cacheEntry{
+		lastModified: lastModified,
+		data:         data,
 	}
-
-	f, err := os.Create(path)
-	if err != nil {
-		// ignore, just return the original file
-		logger.Debugf("error creating cached file %s: %v", path, err)
-		return data, nil
-	}
-
-	defer data.Close()
-	if _, err := io.Copy(f, data); err != nil {
-		_ = f.Close()
-		return nil, fmt.Errorf("writing to cache file %s - %w", path, err)
-	}
-
-	_ = f.Close()
-	return c.getPackageList(url)
 }

pkg/pkg/manager.go

@@ -14,10 +14,6 @@ import (
 	"github.com/stashapp/stash/pkg/models"
 )
 
-type CachePathGetter interface {
-	GetCachePath() string
-}
-
 // SourcePathGetter gets the source path for a given package URL.
 type SourcePathGetter interface {
 	// GetAllSourcePaths gets all source paths.
@@ -31,9 +27,18 @@ type SourcePathGetter interface {
 type Manager struct {
 	Local             *Store
 	PackagePathGetter SourcePathGetter
-	CachePathGetter   CachePathGetter
 
 	Client *http.Client
+
+	cache *repositoryCache
+}
+
+func (m *Manager) getCache() *repositoryCache {
+	if m.cache == nil {
+		m.cache = &repositoryCache{}
+	}
+
+	return m.cache
 }
 
 func (m *Manager) remoteFromURL(path string) (*httpRepository, error) {
@@ -42,13 +47,7 @@ func (m *Manager) remoteFromURL(path string) (*httpRepository, error) {
 		return nil, fmt.Errorf("parsing path: %w", err)
 	}
 
-	cachePath := m.CachePathGetter.GetCachePath()
-	var cache *repositoryCache
-	if cachePath != "" {
-		cache = &repositoryCache{cachePath: cachePath}
-	}
-
-	return newHttpRepository(*u, m.Client, cache), nil
+	return newHttpRepository(*u, m.Client, m.getCache()), nil
 }
 
 func (m *Manager) ListInstalled(ctx context.Context) (LocalPackageIndex, error) {

pkg/pkg/repository_http.go

@@ -16,9 +16,6 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-// DefaultCacheTTL is the default time to live for the index cache.
-const DefaultCacheTTL = 5 * time.Minute
-
 // httpRepository is a HTTP based repository.
 // It is configured with a package list URL. Packages are located from the Path field of the package.
 //
@@ -51,13 +48,28 @@ func (r *httpRepository) List(ctx context.Context) ([]RemotePackage, error) {
 
 	// the package list URL may be file://, in which case we need to use the local file system
 	var (
-		f   io.ReadCloser
-		err error
+		f       io.ReadCloser
+		modTime *time.Time
+		err     error
 	)
-	if u.Scheme == "file" {
+
+	isLocal := u.Scheme == "file"
+
+	if isLocal {
 		f, err = r.getLocalFile(ctx, u.Path)
 	} else {
-		f, err = r.getFileCached(ctx, u)
+		// try to get the cached list first
+		var cachedList []RemotePackage
+		cachedList, err = r.getCachedList(ctx, u)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get cached package list: %w", err)
+		}
+
+		if cachedList != nil {
+			return cachedList, nil
+		}
+
+		f, modTime, err = r.getFile(ctx, u)
 	}
 
 	if err != nil {
@@ -76,6 +88,11 @@ func (r *httpRepository) List(ctx context.Context) ([]RemotePackage, error) {
 		return nil, fmt.Errorf("reading package list: %w", err)
 	}
 
+	// cache if not local file
+	if !isLocal {
+		r.cache.cacheList(u.String(), *modTime, index)
+	}
+
 	return index, nil
 }
 
@@ -124,7 +141,7 @@ func (r *httpRepository) GetPackageZip(ctx context.Context, pkg RemotePackage) (
 
 		f, err = r.getLocalFile(ctx, u.Path)
 	} else {
-		f, err = r.getFile(ctx, u)
+		f, _, err = r.getFile(ctx, u)
 	}
 
 	if err != nil {
@@ -135,8 +152,8 @@ func (r *httpRepository) GetPackageZip(ctx context.Context, pkg RemotePackage) (
 }
 
 // getFileCached tries to get the list from the local cache.
-// If it is not found or is stale, then it gets it normally.
-func (r *httpRepository) getFileCached(ctx context.Context, u url.URL) (io.ReadCloser, error) {
+// If it is not found or is stale, then nil is returned.
+func (r *httpRepository) getCachedList(ctx context.Context, u url.URL) ([]RemotePackage, error) {
 	// check if the file is in the cache first
 	localModTime := r.cache.lastModified(u.String())
 
@@ -163,34 +180,41 @@ func (r *httpRepository) getFileCached(ctx context.Context, u url.URL) (io.ReadC
 
 			if !remoteModTime.After(*localModTime) {
 				logger.Debugf("cached version of %s is equal or newer than remote", u.String())
-				return r.cache.getPackageList(u.String())
+				return r.cache.getPackageList(u.String()), nil
 			}
 		}
 
 		logger.Debugf("cached version of %s is older than remote", u.String())
 	}
 
-	return r.getFile(ctx, u)
+	return nil, nil
 }
 
-func (r *httpRepository) getFile(ctx context.Context, u url.URL) (io.ReadCloser, error) {
+// getFile gets the file from the remote server. Returns the file and the last modified time.
+func (r *httpRepository) getFile(ctx context.Context, u url.URL) (io.ReadCloser, *time.Time, error) {
 	logger.Debugf("fetching %s", u.String())
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
 	if err != nil {
 		// shouldn't happen
-		return nil, err
+		return nil, nil, err
 	}
 
 	resp, err := r.client.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get remote file: %w", err)
+		return nil, nil, fmt.Errorf("failed to get remote file: %w", err)
 	}
 
 	if resp.StatusCode >= 400 {
-		return nil, fmt.Errorf("failed to get remote file: %s", resp.Status)
+		return nil, nil, fmt.Errorf("failed to get remote file: %s", resp.Status)
 	}
 
-	return r.cache.cacheFile(u.String(), resp.Body)
+	lastModified := resp.Header.Get("Last-Modified")
+	var remoteModTime time.Time
+	if lastModified != "" {
+		remoteModTime, _ = time.Parse(http.TimeFormat, lastModified)
+	}
+
+	return resp.Body, &remoteModTime, nil
 }
 
 func (r *httpRepository) getLocalFile(ctx context.Context, path string) (fs.File, error) {