From 257ccc5f4323bb2f39e09fa903546edf7cdf370a Mon Sep 17 00:00:00 2001 From: tomleb Date: Fri, 2 Jun 2023 17:14:11 -0400 Subject: [PATCH] Allow configuring cache folder (#2357) * Set all clients to dev_download for make get-music * Use multiple TranscodingCache instances in tests This fixes flaky tests. The issue is that the TranscodingCache object was being reused in tests from media_stream_Internal_test.go and media_stream_test.go. If tests from the former was run first, the cache would be filled up, so that when running tests from the latter, the `NON seekable` test would fail. * Allow configuring cache folder This commit introduces a new configuration option to configure the cache folder. This allows the cache to be in a separate folder such as /var/cache/navidrome on Linux distributions. * Fix tests * Removed unused test setup code --------- Co-authored-by: Deluan Co-authored-by: Deluan --- Makefile | 6 ++--- cmd/root.go | 4 +++- conf/configuration.go | 17 +++++++++++++ consts/consts.go | 4 ++-- core/media_streamer.go | 36 +++++++++++++++------------- core/media_streamer_Internal_test.go | 15 +----------- core/media_streamer_test.go | 6 ++--- tests/init_tests.go | 2 +- utils/cache/file_caches.go | 2 +- utils/cache/file_caches_test.go | 4 ++-- 10 files changed, 53 insertions(+), 43 deletions(-) diff --git a/Makefile b/Makefile index dbaae6c3..43dca10a 100644 --- a/Makefile +++ b/Makefile @@ -108,9 +108,9 @@ get-music: ##@Development Download some free music from Navidrome's demo instanc mkdir -p music ( cd music; \ curl "https://demo.navidrome.org/rest/download?u=demo&p=demo&f=json&v=1.8.0&c=dev_download&id=ec2093ec4801402f1e17cc462195cdbb" > brock.zip; \ - curl "https://demo.navidrome.org/rest/download?u=demo&p=demo&f=json&v=1.8.0&c=NavidromeUI&id=b376eeb4652d2498aa2b25ba0696725e" > back_on_earth.zip; \ - curl "https://demo.navidrome.org/rest/download?u=demo&p=demo&f=json&v=1.8.0&c=NavidromeUI&id=e49c609b542fc51899ee8b53aa858cb4" > ugress.zip; \ - curl "https://demo.navidrome.org/rest/download?u=demo&p=demo&f=json&v=1.8.0&c=NavidromeUI&id=350bcab3a4c1d93869e39ce496464f03" > voodoocuts.zip; \ + curl "https://demo.navidrome.org/rest/download?u=demo&p=demo&f=json&v=1.8.0&c=dev_download&id=b376eeb4652d2498aa2b25ba0696725e" > back_on_earth.zip; \ + curl "https://demo.navidrome.org/rest/download?u=demo&p=demo&f=json&v=1.8.0&c=dev_download&id=e49c609b542fc51899ee8b53aa858cb4" > ugress.zip; \ + curl "https://demo.navidrome.org/rest/download?u=demo&p=demo&f=json&v=1.8.0&c=dev_download&id=350bcab3a4c1d93869e39ce496464f03" > voodoocuts.zip; \ for file in *.zip; do unzip -n $${file}; done ) @echo "Done. Remember to set your MusicFolder to ./music" .PHONY: get-music diff --git a/cmd/root.go b/cmd/root.go index 39bb062c..d9fd9d61 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -155,11 +155,13 @@ func init() { rootCmd.PersistentFlags().StringVarP(&cfgFile, "configfile", "c", "", `config file (default "./navidrome.toml")`) rootCmd.PersistentFlags().BoolVarP(&noBanner, "nobanner", "n", false, `don't show banner`) rootCmd.PersistentFlags().String("musicfolder", viper.GetString("musicfolder"), "folder where your music is stored") - rootCmd.PersistentFlags().String("datafolder", viper.GetString("datafolder"), "folder to store application data (DB, cache...), needs write access") + rootCmd.PersistentFlags().String("datafolder", viper.GetString("datafolder"), "folder to store application data (DB), needs write access") + rootCmd.PersistentFlags().String("cachefolder", viper.GetString("cachefolder"), "folder to store cache data (transcoding, images...), needs write access") rootCmd.PersistentFlags().StringP("loglevel", "l", viper.GetString("loglevel"), "log level, possible values: error, info, debug, trace") _ = viper.BindPFlag("musicfolder", rootCmd.PersistentFlags().Lookup("musicfolder")) _ = viper.BindPFlag("datafolder", rootCmd.PersistentFlags().Lookup("datafolder")) + _ = viper.BindPFlag("cachefolder", rootCmd.PersistentFlags().Lookup("cachefolder")) _ = viper.BindPFlag("loglevel", rootCmd.PersistentFlags().Lookup("loglevel")) rootCmd.Flags().StringP("address", "a", viper.GetString("address"), "IP address to bind to") diff --git a/conf/configuration.go b/conf/configuration.go index 192559a2..377f1d95 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -23,6 +23,7 @@ type configOptions struct { Port int MusicFolder string DataFolder string + CacheFolder string DbPath string LogLevel string ScanInterval time.Duration @@ -135,6 +136,11 @@ var ( func LoadFromFile(confFile string) { viper.SetConfigFile(confFile) + err := viper.ReadInConfig() + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, "FATAL: Error reading config file:", err) + os.Exit(1) + } Load() } @@ -149,6 +155,16 @@ func Load() { _, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating data path:", "path", Server.DataFolder, err) os.Exit(1) } + + if Server.CacheFolder == "" { + Server.CacheFolder = filepath.Join(Server.DataFolder, "cache") + } + err = os.MkdirAll(Server.CacheFolder, os.ModePerm) + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating cache path:", "path", Server.CacheFolder, err) + os.Exit(1) + } + Server.ConfigFile = viper.GetViper().ConfigFileUsed() if Server.DbPath == "" { Server.DbPath = filepath.Join(Server.DataFolder, consts.DefaultDbPath) @@ -242,6 +258,7 @@ func AddHook(hook func()) { func init() { viper.SetDefault("musicfolder", filepath.Join(".", "music")) + viper.SetDefault("cachefolder", "") viper.SetDefault("datafolder", ".") viper.SetDefault("loglevel", "info") viper.SetDefault("address", "0.0.0.0") diff --git a/consts/consts.go b/consts/consts.go index af8d74d3..d169661d 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -71,10 +71,10 @@ const ( // Cache options const ( - TranscodingCacheDir = "cache/transcoding" + TranscodingCacheDir = "transcoding" DefaultTranscodingCacheMaxItems = 0 // Unlimited - ImageCacheDir = "cache/images" + ImageCacheDir = "images" DefaultImageCacheMaxItems = 0 // Unlimited DefaultCacheSize = 100 * 1024 * 1024 // 100MB diff --git a/core/media_streamer.go b/core/media_streamer.go index 8d32b258..69585219 100644 --- a/core/media_streamer.go +++ b/core/media_streamer.go @@ -184,22 +184,26 @@ var ( func GetTranscodingCache() TranscodingCache { onceTranscodingCache.Do(func() { - instanceTranscodingCache = cache.NewFileCache("Transcoding", conf.Server.TranscodingCacheSize, - consts.TranscodingCacheDir, consts.DefaultTranscodingCacheMaxItems, - func(ctx context.Context, arg cache.Item) (io.Reader, error) { - job := arg.(*streamJob) - t, err := job.ms.ds.Transcoding(ctx).FindByFormat(job.format) - if err != nil { - log.Error(ctx, "Error loading transcoding command", "format", job.format, err) - return nil, os.ErrInvalid - } - out, err := job.ms.transcoder.Transcode(ctx, t.Command, job.mf.Path, job.bitRate) - if err != nil { - log.Error(ctx, "Error starting transcoder", "id", job.mf.ID, err) - return nil, os.ErrInvalid - } - return out, nil - }) + instanceTranscodingCache = NewTranscodingCache() }) return instanceTranscodingCache } + +func NewTranscodingCache() TranscodingCache { + return cache.NewFileCache("Transcoding", conf.Server.TranscodingCacheSize, + consts.TranscodingCacheDir, consts.DefaultTranscodingCacheMaxItems, + func(ctx context.Context, arg cache.Item) (io.Reader, error) { + job := arg.(*streamJob) + t, err := job.ms.ds.Transcoding(ctx).FindByFormat(job.format) + if err != nil { + log.Error(ctx, "Error loading transcoding command", "format", job.format, err) + return nil, os.ErrInvalid + } + out, err := job.ms.transcoder.Transcode(ctx, t.Command, job.mf.Path, job.bitRate) + if err != nil { + log.Error(ctx, "Error starting transcoder", "id", job.mf.ID, err) + return nil, os.ErrInvalid + } + return out, nil + }) +} diff --git a/core/media_streamer_Internal_test.go b/core/media_streamer_Internal_test.go index 06947869..00103716 100644 --- a/core/media_streamer_Internal_test.go +++ b/core/media_streamer_Internal_test.go @@ -2,10 +2,8 @@ package core import ( "context" - "os" "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" @@ -16,21 +14,10 @@ import ( var _ = Describe("MediaStreamer", func() { var ds model.DataStore - ctx := log.NewContext(context.TODO()) + ctx := log.NewContext(context.Background()) BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - conf.Server.DataFolder, _ = os.MkdirTemp("", "file_caches") - conf.Server.TranscodingCacheSize = "100MB" ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}} - ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ - {ID: "123", Path: "tests/fixtures/test.mp3", Suffix: "mp3", BitRate: 128, Duration: 257.0}, - }) - testCache := GetTranscodingCache() - Eventually(func() bool { return testCache.Available(context.TODO()) }).Should(BeTrue()) - }) - AfterEach(func() { - _ = os.RemoveAll(conf.Server.DataFolder) }) Context("selectTranscodingOptions", func() { diff --git a/core/media_streamer_test.go b/core/media_streamer_test.go index 588db41b..1499450d 100644 --- a/core/media_streamer_test.go +++ b/core/media_streamer_test.go @@ -23,18 +23,18 @@ var _ = Describe("MediaStreamer", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) - conf.Server.DataFolder, _ = os.MkdirTemp("", "file_caches") + conf.Server.CacheFolder, _ = os.MkdirTemp("", "file_caches") conf.Server.TranscodingCacheSize = "100MB" ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}} ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ {ID: "123", Path: "tests/fixtures/test.mp3", Suffix: "mp3", BitRate: 128, Duration: 257.0}, }) - testCache := core.GetTranscodingCache() + testCache := core.NewTranscodingCache() Eventually(func() bool { return testCache.Available(context.TODO()) }).Should(BeTrue()) streamer = core.NewMediaStreamer(ds, ffmpeg, testCache) }) AfterEach(func() { - _ = os.RemoveAll(conf.Server.DataFolder) + _ = os.RemoveAll(conf.Server.CacheFolder) }) Context("NewStream", func() { diff --git a/tests/init_tests.go b/tests/init_tests.go index 4b18db76..582ad95f 100644 --- a/tests/init_tests.go +++ b/tests/init_tests.go @@ -23,7 +23,7 @@ func Init(t *testing.T, skipOnShort bool) { confPath, _ := filepath.Abs(filepath.Join(appPath, "tests", "navidrome-test.toml")) println("Loading test configuration file from " + confPath) _ = os.Chdir(appPath) - conf.LoadFromFile("tests/navidrome-test.toml") + conf.LoadFromFile(confPath) noLog := os.Getenv("NOLOG") if noLog != "" { diff --git a/utils/cache/file_caches.go b/utils/cache/file_caches.go index 316fa088..1157d473 100644 --- a/utils/cache/file_caches.go +++ b/utils/cache/file_caches.go @@ -209,7 +209,7 @@ func newFSCache(name, cacheSize, cacheFolder string, maxItems int) (fscache.Cach lru := NewFileHaunter(name, maxItems, int64(size), consts.DefaultCacheCleanUpInterval) h := fscache.NewLRUHaunterStrategy(lru) - cacheFolder = filepath.Join(conf.Server.DataFolder, cacheFolder) + cacheFolder = filepath.Join(conf.Server.CacheFolder, cacheFolder) var fs *spreadFS log.Info(fmt.Sprintf("Creating %s cache", name), "path", cacheFolder, "maxSize", humanize.Bytes(size)) diff --git a/utils/cache/file_caches_test.go b/utils/cache/file_caches_test.go index 5884d8f4..3679c79f 100644 --- a/utils/cache/file_caches_test.go +++ b/utils/cache/file_caches_test.go @@ -28,14 +28,14 @@ var _ = Describe("File Caches", func() { configtest.SetupConfig() _ = os.RemoveAll(tmpDir) }) - conf.Server.DataFolder = tmpDir + conf.Server.CacheFolder = tmpDir }) Describe("NewFileCache", func() { It("creates the cache folder", func() { Expect(callNewFileCache("test", "1k", "test", 0, nil)).ToNot(BeNil()) - _, err := os.Stat(filepath.Join(conf.Server.DataFolder, "test")) + _, err := os.Stat(filepath.Join(conf.Server.CacheFolder, "test")) Expect(os.IsNotExist(err)).To(BeFalse()) })