Use new FileCache in cover service

core/core_suite_test.go

@@ -1,13 +1,10 @@
 package core
 
 import (
-	"io/ioutil"
-	"os"
 	"testing"
 
 	"github.com/deluan/navidrome/log"
 	"github.com/deluan/navidrome/tests"
-	"github.com/djherbis/fscache"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -18,18 +15,3 @@ func TestEngine(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Core Suite")
 }
-
-var testCache fscache.Cache
-var testCacheDir string
-
-var _ = Describe("Core Suite Setup", func() {
-	BeforeSuite(func() {
-		testCacheDir, _ = ioutil.TempDir("", "core_test_cache")
-		fs, _ := fscache.NewFs(testCacheDir, 0755)
-		testCache, _ = fscache.NewCache(fs, nil)
-	})
-
-	AfterSuite(func() {
-		os.RemoveAll(testCacheDir)
-	})
-})

core/cover.go

@@ -22,22 +22,30 @@ import (
 	"github.com/deluan/navidrome/utils"
 	"github.com/dhowden/tag"
 	"github.com/disintegration/imaging"
-	"github.com/djherbis/fscache"
 )
 
 type Cover interface {
 	Get(ctx context.Context, id string, size int, out io.Writer) error
 }
 
-type ImageCache fscache.Cache
+func NewCover(ds model.DataStore, cache *FileCache) Cover {
-
-func NewCover(ds model.DataStore, cache ImageCache) Cover {
 	return &cover{ds: ds, cache: cache}
 }
 
 type cover struct {
 	ds    model.DataStore
-	cache fscache.Cache
+	cache *FileCache
+}
+
+type coverInfo struct {
+	c          *cover
+	path       string
+	size       int
+	lastUpdate time.Time
+}
+
+func (ci *coverInfo) String() string {
+	return fmt.Sprintf("%s.%d.%s.%d", ci.path, ci.size, ci.lastUpdate.Format(time.RFC3339Nano), conf.Server.CoverJpegQuality)
 }
 
 func (c *cover) Get(ctx context.Context, id string, size int, out io.Writer) error {
@@ -46,41 +54,18 @@ func (c *cover) Get(ctx context.Context, id string, size int, out io.Writer) err
 		return err
 	}
 
-	// If cache is disabled, just read the coverart directly from file
+	info := &coverInfo{
-	if c.cache == nil {
+		c:          c,
-		log.Trace(ctx, "Retrieving cover art from file", "path", path, "size", size, err)
+		path:       path,
-		reader, err := c.getCover(ctx, path, size)
+		size:       size,
-		if err != nil {
+		lastUpdate: lastUpdate,
-			log.Error(ctx, "Error loading cover art", "path", path, "size", size, err)
-		} else {
-			_, err = io.Copy(out, reader)
-		}
-		return err
 	}
 
-	cacheKey := imageCacheKey(path, size, lastUpdate)
+	r, err := c.cache.Get(ctx, info)
-	r, w, err := c.cache.Get(cacheKey)
 	if err != nil {
-		log.Error(ctx, "Error reading from image cache", "path", path, "size", size, err)
+		log.Error(ctx, "Error accessing image cache", "path", path, "size", size, err)
 		return err
 	}
-	defer r.Close()
-	if w != nil {
-		log.Trace(ctx, "Image cache miss", "path", path, "size", size, "lastUpdate", lastUpdate)
-		go func() {
-			defer w.Close()
-			reader, err := c.getCover(ctx, path, size)
-			if err != nil {
-				log.Error(ctx, "Error loading cover art", "path", path, "size", size, err)
-				return
-			}
-			if _, err := io.Copy(w, reader); err != nil {
-				log.Error(ctx, "Error saving covert art to cache", "path", path, "size", size, err)
-			}
-		}()
-	} else {
-		log.Trace(ctx, "Loading image from cache", "path", path, "size", size, "lastUpdate", lastUpdate)
-	}
 
 	_, err = io.Copy(out, r)
 	return err
@@ -118,10 +103,6 @@ func (c *cover) getCoverPath(ctx context.Context, id string) (path string, lastU
 	return c.getCoverPath(ctx, "al-"+mf.AlbumID)
 }
 
-func imageCacheKey(path string, size int, lastUpdate time.Time) string {
-	return fmt.Sprintf("%s.%d.%s.%d", path, size, lastUpdate.Format(time.RFC3339Nano), conf.Server.CoverJpegQuality)
-}
-
 func (c *cover) getCover(ctx context.Context, path string, size int) (reader io.Reader, err error) {
 	defer func() {
 		if err != nil {
@@ -201,6 +182,15 @@ func readFromFile(path string) ([]byte, error) {
 	return buf.Bytes(), nil
 }
 
-func NewImageCache() (ImageCache, error) {
+func NewImageCache() (*FileCache, error) {
-	return newFSCache("Image", conf.Server.ImageCacheSize, consts.ImageCacheDir, consts.DefaultImageCacheMaxItems)
+	return NewFileCache("Image", conf.Server.ImageCacheSize, consts.ImageCacheDir, consts.DefaultImageCacheMaxItems,
+		func(ctx context.Context, arg fmt.Stringer) (io.Reader, error) {
+			info := arg.(*coverInfo)
+			reader, err := info.c.getCover(ctx, info.path, info.size)
+			if err != nil {
+				log.Error(ctx, "Error loading cover art", "path", info.path, "size", info.size, err)
+				return nil, err
+			}
+			return reader, nil
+		})
 }

core/cover_test.go

@@ -4,7 +4,10 @@ import (
 	"bytes"
 	"context"
 	"image"
+	"io/ioutil"
+	"os"
 
+	"github.com/deluan/navidrome/conf"
 	"github.com/deluan/navidrome/log"
 	"github.com/deluan/navidrome/model"
 	"github.com/deluan/navidrome/persistence"
@@ -25,7 +28,14 @@ var _ = Describe("Cover", func() {
 
 	Context("Cache is configured", func() {
 		BeforeEach(func() {
-			cover = NewCover(ds, testCache)
+			conf.Server.DataFolder, _ = ioutil.TempDir("", "file_caches")
+			conf.Server.ImageCacheSize = "100MB"
+			cache, _ := NewImageCache()
+			cover = NewCover(ds, cache)
+		})
+
+		AfterEach(func() {
+			os.RemoveAll(conf.Server.DataFolder)
 		})
 
 		It("retrieves the external cover art for an album", func() {
@@ -118,19 +128,4 @@ var _ = Describe("Cover", func() {
 			})
 		})
 	})
-	Context("Cache is NOT configured", func() {
-		BeforeEach(func() {
-			cover = NewCover(ds, nil)
-		})
-
-		It("retrieves the original cover art from an album", func() {
-			buf := new(bytes.Buffer)
-
-			Expect(cover.Get(ctx, "al-222", 0, buf)).To(BeNil())
-
-			_, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
-			Expect(err).To(BeNil())
-			Expect(format).To(Equal("jpeg"))
-		})
-	})
 })

core/file_caches.go

@@ -123,14 +123,15 @@ func copyAndClose(ctx context.Context, w io.WriteCloser, r io.Reader) {
 }
 
 func newFSCache(name, cacheSize, cacheFolder string, maxItems int) (fscache.Cache, error) {
-	if cacheSize == "0" {
-		log.Warn(fmt.Sprintf("%s cache disabled", name))
-		return nil, nil
-	}
 	size, err := humanize.ParseBytes(cacheSize)
 	if err != nil {
+		log.Error("Invalid cache size. Using default size", "cache", name, "size", cacheSize, "defaultSize", consts.DefaultCacheSize)
 		size = consts.DefaultCacheSize
 	}
+	if size == 0 {
+		log.Warn(fmt.Sprintf("%s cache disabled", name))
+		return nil, nil
+	}
 	lru := fscache.NewLRUHaunter(maxItems, int64(size), consts.DefaultCacheCleanUpInterval)
 	h := fscache.NewLRUHaunterStrategy(lru)
 	cacheFolder = filepath.Join(conf.Server.DataFolder, cacheFolder)

core/file_caches_test.go

@@ -1,9 +1,13 @@
 package core
 
 import (
+	"context"
+	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/deluan/navidrome/conf"
 	. "github.com/onsi/ginkgo"
@@ -20,21 +24,21 @@ var _ = Describe("File Caches", func() {
 
 	Describe("NewFileCache", func() {
 		It("creates the cache folder", func() {
-			Expect(NewFileCache("test", "1k", "test", 10, nil)).ToNot(BeNil())
+			Expect(NewFileCache("test", "1k", "test", 0, nil)).ToNot(BeNil())
 
 			_, err := os.Stat(filepath.Join(conf.Server.DataFolder, "test"))
 			Expect(os.IsNotExist(err)).To(BeFalse())
 		})
 
 		It("creates the cache folder with invalid size", func() {
-			fc, err := NewFileCache("test", "abc", "test", 10, nil)
+			fc, err := NewFileCache("test", "abc", "test", 0, nil)
 			Expect(err).To(BeNil())
 			Expect(fc.cache).ToNot(BeNil())
 			Expect(fc.disabled).To(BeFalse())
 		})
 
 		It("returns empty if cache size is '0'", func() {
-			fc, err := NewFileCache("test", "0", "test", 10, nil)
+			fc, err := NewFileCache("test", "0", "test", 0, nil)
 			Expect(err).To(BeNil())
 			Expect(fc.cache).To(BeNil())
 			Expect(fc.disabled).To(BeTrue())
@@ -42,6 +46,46 @@ var _ = Describe("File Caches", func() {
 	})
 
 	Describe("FileCache", func() {
+		It("caches data if cache is enabled", func() {
+			called := false
+			fc, _ := NewFileCache("test", "1KB", "test", 0, func(ctx context.Context, arg fmt.Stringer) (io.Reader, error) {
+				called = true
+				return strings.NewReader(arg.String()), nil
+			})
+			// First call is a MISS
+			s, err := fc.Get(context.TODO(), &testArg{"test"})
+			Expect(err).To(BeNil())
+			Expect(ioutil.ReadAll(s)).To(Equal([]byte("test")))
 
+			// Second call is a HIT
+			called = false
+			s, err = fc.Get(context.TODO(), &testArg{"test"})
+			Expect(err).To(BeNil())
+			Expect(ioutil.ReadAll(s)).To(Equal([]byte("test")))
+			Expect(called).To(BeFalse())
+		})
+
+		It("does not cache data if cache is disabled", func() {
+			called := false
+			fc, _ := NewFileCache("test", "0", "test", 0, func(ctx context.Context, arg fmt.Stringer) (io.Reader, error) {
+				called = true
+				return strings.NewReader(arg.String()), nil
+			})
+			// First call is a MISS
+			s, err := fc.Get(context.TODO(), &testArg{"test"})
+			Expect(err).To(BeNil())
+			Expect(ioutil.ReadAll(s)).To(Equal([]byte("test")))
+
+			// Second call is also a MISS
+			called = false
+			s, err = fc.Get(context.TODO(), &testArg{"test"})
+			Expect(err).To(BeNil())
+			Expect(ioutil.ReadAll(s)).To(Equal([]byte("test")))
+			Expect(called).To(BeTrue())
 		})
 	})
+})
+
+type testArg struct{ s string }
+
+func (t *testArg) String() string { return t.s }

core/media_streamer.go

@@ -83,7 +83,7 @@ func (ms *mediaStreamer) NewStream(ctx context.Context, id string, reqFormat str
 	}
 	r, err := ms.cache.Get(ctx, job)
 	if err != nil {
-		log.Error(ctx, "Error accessing cache", "id", mf.ID, err)
+		log.Error(ctx, "Error accessing transcoding cache", "id", mf.ID, err)
 		return nil, err
 	}
 

core/media_streamer_test.go

@@ -21,7 +21,6 @@ var _ = Describe("MediaStreamer", func() {
 	var ds model.DataStore
 	ffmpeg := &fakeFFmpeg{Data: "fake data"}
 	ctx := log.NewContext(context.TODO())
-	log.SetLevel(log.LevelTrace)
 
 	BeforeEach(func() {
 		conf.Server.DataFolder, _ = ioutil.TempDir("", "file_caches")
@@ -61,6 +60,8 @@ var _ = Describe("MediaStreamer", func() {
 		It("returns a seekable stream if the file is complete in the cache", func() {
 			s, err := streamer.NewStream(ctx, "123", "mp3", 32)
 			Expect(err).To(BeNil())
+			_, _ = ioutil.ReadAll(s)
+			_ = s.Close()
 			Eventually(func() bool { return ffmpeg.closed }, "3s").Should(BeTrue())
 
 			s, err = streamer.NewStream(ctx, "123", "mp3", 32)