Add caching to local storage provider (#33)

* Add cache to local storage
* Make cache duration configurable

Author: fleskesvor

cli/server.go

@@ -23,10 +23,11 @@ import (
 
 func server() *cobra.Command {
 	var (
-		address     string
-		artifactory string
-		extdir      string
-		repo        string
+		address           string
+		artifactory       string
+		extdir            string
+		repo              string
+		listcacheduration time.Duration
 	)
 
 	cmd := &cobra.Command{
@@ -53,10 +54,11 @@ func server() *cobra.Command {
 			}
 
 			store, err := storage.NewStorage(ctx, &storage.Options{
-				Artifactory: artifactory,
-				ExtDir:      extdir,
-				Logger:      logger,
-				Repo:        repo,
+				Artifactory:       artifactory,
+				ExtDir:            extdir,
+				Logger:            logger,
+				Repo:              repo,
+				ListCacheDuration: listcacheduration,
 			})
 			if err != nil {
 				return err
@@ -137,6 +139,7 @@ func server() *cobra.Command {
 	cmd.Flags().StringVar(&artifactory, "artifactory", "", "Artifactory server URL.")
 	cmd.Flags().StringVar(&repo, "repo", "", "Artifactory repository.")
 	cmd.Flags().StringVar(&address, "address", "127.0.0.1:3001", "The address on which to serve the marketplace API.")
+	cmd.Flags().DurationVar(&listcacheduration, "list-cache-duration", time.Minute, "The duration of the extension cache.")
 
 	return cmd
 }

storage/artifactory.go

@@ -333,13 +333,6 @@ func (s *Artifactory) RemoveExtension(ctx context.Context, publisher, name strin
 	return err
 }
 
-type extension struct {
-	manifest  *VSIXManifest
-	name      string
-	publisher string
-	versions  []Version
-}
-
 func (s *Artifactory) listWithCache(ctx context.Context) *[]ArtifactoryFile {
 	s.listMutex.Lock()
 	defer s.listMutex.Unlock()

storage/local.go

@@ -8,6 +8,8 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"sync"
+	"time"
 
 	"cdr.dev/slog"
 )
@@ -16,21 +18,77 @@ import (
 // copying the VSIX and extracting said VSIX to a tree structure in the form of
 // publisher/extension/version to easily serve individual assets via HTTP.
 type Local struct {
-	extdir string
-	logger slog.Logger
+	listCache      []extension
+	listDuration   time.Duration
+	listExpiration time.Time
+	listMutex      sync.Mutex
+	extdir         string
+	logger         slog.Logger
 }
 
-func NewLocalStorage(extdir string, logger slog.Logger) (*Local, error) {
-	extdir, err := filepath.Abs(extdir)
+type LocalOptions struct {
+	// How long to cache the list of extensions with their manifests.  Zero means 
+	// no cache.
+	ListCacheDuration time.Duration
+	ExtDir            string
+}
+
+func NewLocalStorage(options *LocalOptions, logger slog.Logger) (*Local, error) {
+	extdir, err := filepath.Abs(options.ExtDir)
 	if err != nil {
 		return nil, err
 	}
 	return &Local{
-		extdir: extdir,
-		logger: logger,
+		// TODO: Eject the cache when adding/removing extensions and/or add a
+		// command to eject the cache?
+		extdir:       extdir,
+		listDuration: options.ListCacheDuration,
+		logger:       logger,
 	}, nil
 }
 
+func (s *Local) list(ctx context.Context) []extension {
+	var list []extension
+	publishers, err := s.getDirNames(ctx, s.extdir)
+	if err != nil {
+		s.logger.Error(ctx, "Error reading publisher", slog.Error(err))
+	}
+	for _, publisher := range publishers {
+		ctx := slog.With(ctx, slog.F("publisher", publisher))
+		dir := filepath.Join(s.extdir, publisher)
+
+		extensions, err := s.getDirNames(ctx, dir)
+		if err != nil {
+			s.logger.Error(ctx, "Error reading extensions", slog.Error(err))
+		}
+		for _, name := range extensions {
+			ctx := slog.With(ctx, slog.F("extension", name))
+			versions, err := s.Versions(ctx, publisher, name)
+			if err != nil {
+				s.logger.Error(ctx, "Error reading versions", slog.Error(err))
+			}
+			if len(versions) == 0 {
+				continue
+			}
+
+			// The manifest from the latest version is used for filtering.
+			manifest, err := s.Manifest(ctx, publisher, name, versions[0])
+			if err != nil {
+				s.logger.Error(ctx, "Unable to read extension manifest", slog.Error(err))
+				continue
+			}
+
+			list = append(list, extension{
+				manifest,
+				name,
+				publisher,
+				versions,
+			})
+		}
+	}
+	return list
+}
+
 func (s *Local) AddExtension(ctx context.Context, manifest *VSIXManifest, vsix []byte) (string, error) {
 	// Extract the zip to the correct path.
 	identity := manifest.Metadata.Identity
@@ -118,39 +176,23 @@ func (s *Local) Versions(ctx context.Context, publisher, name string) ([]Version
 	return versions, err
 }
 
-func (s *Local) WalkExtensions(ctx context.Context, fn func(manifest *VSIXManifest, versions []Version) error) error {
-	publishers, err := s.getDirNames(ctx, s.extdir)
-	if err != nil {
-		s.logger.Error(ctx, "Error reading publisher", slog.Error(err))
+func (s *Local) listWithCache(ctx context.Context) []extension {
+	s.listMutex.Lock()
+	defer s.listMutex.Unlock()
+	if s.listCache == nil || time.Now().After(s.listExpiration) {
+		s.listExpiration = time.Now().Add(s.listDuration)
+		s.listCache = s.list(ctx)
 	}
-	for _, publisher := range publishers {
-		ctx := slog.With(ctx, slog.F("publisher", publisher))
-		dir := filepath.Join(s.extdir, publisher)
-
-		extensions, err := s.getDirNames(ctx, dir)
-		if err != nil {
-			s.logger.Error(ctx, "Error reading extensions", slog.Error(err))
-		}
-		for _, extension := range extensions {
-			ctx := slog.With(ctx, slog.F("extension", extension))
-			versions, err := s.Versions(ctx, publisher, extension)
-			if err != nil {
-				s.logger.Error(ctx, "Error reading versions", slog.Error(err))
-			}
-			if len(versions) == 0 {
-				continue
-			}
-
-			// The manifest from the latest version is used for filtering.
-			manifest, err := s.Manifest(ctx, publisher, extension, versions[0])
-			if err != nil {
-				s.logger.Error(ctx, "Unable to read extension manifest", slog.Error(err))
-				continue
-			}
+	return s.listCache
+}
 
-			if err = fn(manifest, versions); err != nil {
-				return err
-			}
+func (s *Local) WalkExtensions(ctx context.Context, fn func(manifest *VSIXManifest, versions []Version) error) error {
+	// Walking through directories on disk and parsing manifest files takes several
+	// minutes with many extensions installed, so if we already did that within
+	// a specified duration, just load extensions from the cache instead.
+	for _, extension := range s.listWithCache(ctx) {
+		if err := fn(extension.manifest, extension.versions); err != nil {
+			return err
 		}
 	}
 	return nil

storage/local_test.go

@@ -15,7 +15,7 @@ import (
 func localFactory(t *testing.T) testStorage {
 	extdir := t.TempDir()
 	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
-	s, err := storage.NewLocalStorage(extdir, logger)
+	s, err := storage.NewLocalStorage(&storage.LocalOptions{ExtDir: extdir}, logger)
 	require.NoError(t, err)
 	return testStorage{
 		storage: s,

storage/storage.go

@@ -123,10 +123,18 @@ type VSIXAsset struct {
 }
 
 type Options struct {
-	Artifactory string
-	ExtDir      string
-	Repo        string
-	Logger      slog.Logger
+	Artifactory       string
+	ExtDir            string
+	Repo              string
+	Logger            slog.Logger
+	ListCacheDuration time.Duration
+}
+
+type extension struct {
+	manifest  *VSIXManifest
+	name      string
+	publisher string
+	versions  []Version
 }
 
 // Version is a subset of database.ExtVersion.
@@ -238,14 +246,17 @@ func NewStorage(ctx context.Context, options *Options) (Storage, error) {
 			return nil, xerrors.Errorf("the %s environment variable must be set", ArtifactoryTokenEnvKey)
 		}
 		return NewArtifactoryStorage(ctx, &ArtifactoryOptions{
-			ListCacheDuration: time.Minute,
+			ListCacheDuration: options.ListCacheDuration,
 			Logger:            options.Logger,
 			Repo:              options.Repo,
 			Token:             token,
 			URI:               options.Artifactory,
 		})
 	} else if options.ExtDir != "" {
-		return NewLocalStorage(options.ExtDir, options.Logger)
+		return NewLocalStorage(&LocalOptions{
+			ListCacheDuration: options.ListCacheDuration,
+			ExtDir:            options.ExtDir,
+		}, options.Logger)
 	}
 	return nil, xerrors.Errorf("must provide an Artifactory repository or local directory")
 }