mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
* perf(db): keep query planner statistics trustworthy with full ANALYZE PRAGMA optimize's internal ANALYZE runs with a limited analysis budget (~2000 rows) that writes wrong sqlite_stat1 entries for low-cardinality indexes: on a 96K-track library it claimed (missing, library_id) narrows to ~2000 rows when it matches the whole table. The planner then prefers that index over the sort index and falls back to a full-table temp B-tree sort per request, turning paginated song listings into multi-second queries (reproduced at 5.5s on real hardware; ~90x slower than with correct stats). Every index-creating migration re-triggered the poisoning via the post-migration optimize, and the daily optimizer could re-trigger it on large library changes. Setting analysis_limit on the connection does not help: optimize ignores it. Run a plain full ANALYZE instead: after migrations with schema changes, and in db.Optimize (daily schedule and scan-end). Stats are stored in the database file, so one connection suffices and the per-connection pool loop is gone. The Optimize call at shutdown is removed: stats are maintained at migration/scan/daily points, and an ANALYZE during shutdown only delays it and races container stop timeouts. * perf(db): drop startup PRAGMA optimize that re-poisons planner stats The startup PRAGMA optimize=0x10002 runs SQLite's budget-limited internal ANALYZE (bit 0x02), which writes truncated sqlite_stat1 rows for low-cardinality indexes -- the exact statistics-poisoning this PR set out to eliminate. Because DevOptimizeDB defaults to true, a restart with no pending migrations would re-poison the planner until the next scan or daily Optimize. Remove it: statistics are already refreshed with a full ANALYZE after schema-changing migrations (Init) and via Optimize at scan-end and on the daily schedule, so nothing on the startup path needs to touch them. Also clarify that Optimize is a no-op unless DevOptimizeDB is enabled. * chore(db): remove the DevOptimizeDB flag and skip Optimize on quick scans The flag only gated the optimize/ANALYZE maintenance calls and there is no reason to leave planner statistics unmaintained; the guards are gone along with the flag. The scan-end Optimize now runs only after full scans — quick scans barely move the statistics, and the daily schedule covers drift. * style(scanner): drop redundant comment in runOptimize * chore(persistence): drop the no-op PRAGMA optimize from ScanEnd Mask 0x10000 only selects candidate tables by size change; without the 0x02 action bit optimize does nothing (verified: sqlite_stat1 stays stale after a 100x table growth). The scan-end statistics refresh is db.Optimize's full ANALYZE, and the expression-collation-index concern the old comment guarded against no longer applies. * fix(scanner): run the post-scan ANALYZE in the server process With the external scanner (the default), the scan pipeline runs in a subprocess, so its ANALYZE was invisible to the server: SQLite loads sqlite_stat1 into the process's shared schema cache, and an ANALYZE from another process does not refresh it — verified with the production DSN that even brand-new pool connections keep planning with the old statistics until the server restarts. An in-process ANALYZE, by contrast, is immediately visible to every pooled connection through the same shared cache. Move the full-scan Optimize from the scanner pipeline to the scan controller, which always runs in the server process. * fix(scanner): honor promoted full scans in the optimize gate A quick scan resuming an interrupted full scan is promoted inside the scanner (possibly in a subprocess); mirror the promotion in the controller so the post-scan ANALYZE isn't skipped. * refactor: apply cleanup review findings - drop forceFullRescan's inline ANALYZE: Init already runs a full ANALYZE after any migration batch with schema changes, so upgrades including a full-rescan migration analyzed the whole DB twice - resumingFullScan uses a filtered CountAll instead of fetching and scanning all libraries - document why CallScan (CLI) deliberately skips the post-scan Optimize * perf(db): make planner analysis maintenance resilient Check analysis freshness every 30 minutes and refresh statistics when the last successful run is over 24 hours old or a scan marked them pending. Persist successful analysis state, retry skipped or failed maintenance, coordinate checks with scans, and cover standalone CLI full scans. * perf(db): avoid analyzing routine quick-scan changes Reserve pending analysis for full scans, unscanned libraries, and retry state. Incremental quick scans now rely on the 24-hour freshness window instead of triggering a full ANALYZE at the next maintenance check. * fix(scan): analyze resumed full scans in CLI * fix(db): back off failed analysis retries * feat(db): allow disabling scheduled analysis * test(db): remove redundant analysis coverage * refactor(db): split ANALYZE maintenance into optimize.go and dedupe call sites - move query-planner statistics code from db.go to its own optimize.go (and matching optimize_test.go) - log ANALYZE elapsed time inside Optimize/OptimizeIfNeeded instead of repeating the timing block at every call site - drop the LastDBAnalyzeAttemptAt write on success: it is only read while failures >= 1, and every failure rewrites it first - extract runPostScanAnalysis (cmd) and anyIncludedLibrary (scanner) helpers
321 lines
9.7 KiB
Go
321 lines
9.7 KiB
Go
package conf_test
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"testing"
|
|
|
|
"github.com/navidrome/navidrome/conf"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
"github.com/spf13/viper"
|
|
)
|
|
|
|
func TestConfiguration(t *testing.T) {
|
|
RegisterFailHandler(Fail)
|
|
RunSpecs(t, "Configuration Suite")
|
|
}
|
|
|
|
var _ = Describe("Configuration", func() {
|
|
BeforeEach(func() {
|
|
// Reset viper configuration
|
|
viper.Reset()
|
|
conf.SetViperDefaults()
|
|
viper.SetDefault("datafolder", GinkgoT().TempDir())
|
|
viper.SetDefault("loglevel", "error")
|
|
conf.ResetConf()
|
|
|
|
// Panic instead of exiting on fatal errors to allow testing error conditions
|
|
DeferCleanup(conf.SetLogFatal(func(args ...any) {
|
|
panic(fmt.Sprint(args...))
|
|
}))
|
|
})
|
|
|
|
Describe("ParseLanguages", func() {
|
|
It("parses single language", func() {
|
|
Expect(conf.ParseLanguages("en")).To(Equal([]string{"en"}))
|
|
})
|
|
|
|
It("parses multiple comma-separated languages", func() {
|
|
Expect(conf.ParseLanguages("pt,en")).To(Equal([]string{"pt", "en"}))
|
|
})
|
|
|
|
It("trims whitespace from languages", func() {
|
|
Expect(conf.ParseLanguages(" pt , en ")).To(Equal([]string{"pt", "en"}))
|
|
})
|
|
|
|
It("returns default 'en' when empty", func() {
|
|
Expect(conf.ParseLanguages("")).To(Equal([]string{"en"}))
|
|
})
|
|
|
|
It("returns default 'en' when only whitespace", func() {
|
|
Expect(conf.ParseLanguages(" ")).To(Equal([]string{"en"}))
|
|
})
|
|
|
|
It("handles multiple languages with various spacing", func() {
|
|
Expect(conf.ParseLanguages("ja, pt, en")).To(Equal([]string{"ja", "pt", "en"}))
|
|
})
|
|
})
|
|
|
|
Describe("scheduled DB analysis", func() {
|
|
It("is enabled by default", func() {
|
|
conf.Load(true)
|
|
Expect(conf.Server.EnableScheduledDBAnalyze).To(BeTrue())
|
|
})
|
|
|
|
It("can be disabled", func() {
|
|
viper.Set("enablescheduleddbanalyze", false)
|
|
conf.Load(true)
|
|
Expect(conf.Server.EnableScheduledDBAnalyze).To(BeFalse())
|
|
})
|
|
})
|
|
|
|
Describe("ValidateURL", func() {
|
|
It("accepts a valid http URL", func() {
|
|
fn := conf.ValidateURL("TestOption", "http://example.com/path")
|
|
Expect(fn()).To(Succeed())
|
|
})
|
|
|
|
It("accepts a valid https URL", func() {
|
|
fn := conf.ValidateURL("TestOption", "https://example.com/path")
|
|
Expect(fn()).To(Succeed())
|
|
})
|
|
|
|
It("rejects a URL with no scheme", func() {
|
|
fn := conf.ValidateURL("TestOption", "example.com/path")
|
|
Expect(fn()).To(MatchError(ContainSubstring("invalid scheme")))
|
|
})
|
|
|
|
It("rejects a URL with an unsupported scheme", func() {
|
|
fn := conf.ValidateURL("TestOption", "javascript://example.com/path")
|
|
Expect(fn()).To(MatchError(ContainSubstring("invalid scheme")))
|
|
})
|
|
|
|
It("accepts an empty URL (optional config)", func() {
|
|
fn := conf.ValidateURL("TestOption", "")
|
|
Expect(fn()).To(Succeed())
|
|
})
|
|
|
|
It("includes the option name in the error message", func() {
|
|
fn := conf.ValidateURL("MyOption", "ftp://example.com")
|
|
Expect(fn()).To(MatchError(ContainSubstring("MyOption")))
|
|
})
|
|
|
|
It("rejects a URL that cannot be parsed", func() {
|
|
fn := conf.ValidateURL("TestOption", "://invalid")
|
|
Expect(fn()).To(HaveOccurred())
|
|
})
|
|
|
|
It("rejects a URL without a host", func() {
|
|
fn := conf.ValidateURL("TestOption", "http:///path")
|
|
Expect(fn()).To(MatchError(ContainSubstring("non-empty host is required")))
|
|
})
|
|
})
|
|
|
|
DescribeTable("NormalizeSearchBackend",
|
|
func(input, expected string) {
|
|
Expect(conf.NormalizeSearchBackend(input)).To(Equal(expected))
|
|
},
|
|
Entry("accepts 'fts'", "fts", "fts"),
|
|
Entry("accepts 'legacy'", "legacy", "legacy"),
|
|
Entry("normalizes 'FTS' to lowercase", "FTS", "fts"),
|
|
Entry("normalizes 'Legacy' to lowercase", "Legacy", "legacy"),
|
|
Entry("trims whitespace", " fts ", "fts"),
|
|
Entry("falls back to 'fts' for 'fts5'", "fts5", "fts"),
|
|
Entry("falls back to 'fts' for unrecognized values", "invalid", "fts"),
|
|
Entry("falls back to 'fts' for empty string", "", "fts"),
|
|
)
|
|
|
|
DescribeTable("ToPascalCase",
|
|
func(input, expected string) {
|
|
Expect(conf.ToPascalCase(input)).To(Equal(expected))
|
|
},
|
|
Entry("simple key", "address", "Address"),
|
|
Entry("dotted key", "scanner.schedule", "Scanner.Schedule"),
|
|
Entry("already capitalized", "Address", "Address"),
|
|
Entry("multi-segment", "lastfm.enabled", "Lastfm.Enabled"),
|
|
Entry("empty string", "", ""),
|
|
)
|
|
|
|
Describe("remapEnvVarKeysFromConfig", func() {
|
|
BeforeEach(func() {
|
|
viper.Reset()
|
|
conf.SetViperDefaults()
|
|
viper.SetDefault("datafolder", GinkgoT().TempDir())
|
|
viper.SetDefault("loglevel", "error")
|
|
conf.ResetConf()
|
|
})
|
|
|
|
It("remaps ND_-prefixed keys to canonical keys", func() {
|
|
filename := filepath.Join("testdata", "cfg_nd_keys.toml")
|
|
conf.InitConfig(filename, false)
|
|
conf.Load(true)
|
|
|
|
Expect(conf.Server.Address).To(Equal("127.0.0.1"))
|
|
Expect(conf.Server.Port).To(Equal(4531))
|
|
Expect(conf.Server.Scanner.Schedule).To(Equal("@every 1h"))
|
|
})
|
|
|
|
It("exits with fatal error when both ND_ and canonical key exist", func() {
|
|
filename := filepath.Join("testdata", "cfg_nd_conflict.toml")
|
|
conf.InitConfig(filename, false)
|
|
|
|
Expect(func() { conf.Load(true) }).To(PanicWith(And(
|
|
ContainSubstring("ND_ADDRESS"),
|
|
ContainSubstring("Address"),
|
|
ContainSubstring("only needed for environment variables"),
|
|
)))
|
|
})
|
|
|
|
It("does nothing when no ND_ keys are present", func() {
|
|
filename := filepath.Join("testdata", "cfg.toml")
|
|
conf.InitConfig(filename, false)
|
|
conf.Load(true)
|
|
|
|
// Verify normal config loading still works
|
|
Expect(conf.Server.MusicFolder).To(Equal("/toml/music"))
|
|
})
|
|
})
|
|
|
|
Describe("logFatal", func() {
|
|
var invalidPath string
|
|
BeforeEach(func() {
|
|
viper.Reset()
|
|
conf.SetViperDefaults()
|
|
viper.SetDefault("loglevel", "error")
|
|
conf.ResetConf()
|
|
|
|
// Create a file so that any path under it is invalid on all OSes
|
|
f, err := os.CreateTemp(GinkgoT().TempDir(), "blocker")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
f.Close()
|
|
invalidPath = filepath.Join(f.Name(), "subdir")
|
|
})
|
|
|
|
It("is called when LoadFromFile gets an invalid config file", func() {
|
|
Expect(func() {
|
|
conf.LoadFromFile(filepath.Join(invalidPath, "file.toml"))
|
|
}).To(PanicWith(ContainSubstring("Error reading config file")))
|
|
})
|
|
|
|
It("is called when LogFile path is not writable", func() {
|
|
viper.SetDefault("datafolder", GinkgoT().TempDir())
|
|
viper.SetDefault("logfile", filepath.Join(invalidPath, "log.txt"))
|
|
Expect(func() {
|
|
conf.Load(true)
|
|
}).To(PanicWith(ContainSubstring("Error creating log file directory")))
|
|
})
|
|
|
|
It("is called when BaseURL is invalid", func() {
|
|
viper.SetDefault("datafolder", GinkgoT().TempDir())
|
|
viper.SetDefault("baseurl", "://invalid")
|
|
Expect(func() {
|
|
conf.Load(true)
|
|
}).To(PanicWith(ContainSubstring("Invalid BaseURL")))
|
|
})
|
|
|
|
})
|
|
|
|
Describe("ValidateMaxImageUploadSize", func() {
|
|
BeforeEach(func() {
|
|
viper.Reset()
|
|
conf.SetViperDefaults()
|
|
viper.SetDefault("datafolder", GinkgoT().TempDir())
|
|
viper.SetDefault("loglevel", "error")
|
|
conf.ResetConf()
|
|
})
|
|
|
|
DescribeTable("accepts valid size values",
|
|
func(input string) {
|
|
conf.Server.MaxImageUploadSize = input
|
|
Expect(conf.ValidateMaxImageUploadSize()).To(Succeed())
|
|
},
|
|
Entry("megabytes", "10MB"),
|
|
Entry("gigabytes", "1GB"),
|
|
Entry("raw bytes", "10485760"),
|
|
Entry("mebibytes", "10MiB"),
|
|
Entry("lower case", "50mb"),
|
|
)
|
|
|
|
DescribeTable("rejects invalid size values",
|
|
func(input string) {
|
|
conf.Server.MaxImageUploadSize = input
|
|
Expect(conf.ValidateMaxImageUploadSize()).To(MatchError(ContainSubstring("invalid MaxImageUploadSize")))
|
|
},
|
|
Entry("garbage string", "not-a-size"),
|
|
Entry("negative-looking", "-10MB"),
|
|
)
|
|
})
|
|
|
|
Describe("EnforceNonRootUser", func() {
|
|
It("defaults to false", func() {
|
|
conf.Load(true)
|
|
|
|
Expect(conf.Server.EnforceNonRootUser).To(BeFalse())
|
|
})
|
|
|
|
It("allows startup for non-root users when enabled", func() {
|
|
DeferCleanup(conf.SetRuntimeInfoForTest("linux", 1000))
|
|
viper.Set("enforcenonrootuser", true)
|
|
|
|
conf.Load(true)
|
|
|
|
Expect(conf.Server.EnforceNonRootUser).To(BeTrue())
|
|
})
|
|
|
|
It("exits when enabled and running as root without having created a data folder", func() {
|
|
// Create a path that doesn't exist yet
|
|
tempBase := GinkgoT().TempDir()
|
|
nonExistentDataFolder := filepath.Join(tempBase, "nonexistent", "data")
|
|
DeferCleanup(conf.SetRuntimeInfoForTest("linux", 0))
|
|
viper.Set("enforcenonrootuser", true)
|
|
viper.Set("datafolder", nonExistentDataFolder)
|
|
|
|
// Attempt to load config as root user - should fail before creating directories
|
|
Expect(func() {
|
|
conf.Load(true)
|
|
}).To(PanicWith(ContainSubstring("EnforceNonRootUser is enabled but Navidrome is running as root")))
|
|
|
|
// Verify that the data folder was NOT created
|
|
Expect(nonExistentDataFolder).ToNot(BeAnExistingFile())
|
|
})
|
|
|
|
It("is a no-op on non-unix platforms", func() {
|
|
DeferCleanup(conf.SetRuntimeInfoForTest("windows", 0))
|
|
viper.Set("enforcenonrootuser", true)
|
|
|
|
conf.Load(true)
|
|
|
|
Expect(conf.Server.EnforceNonRootUser).To(BeTrue())
|
|
})
|
|
})
|
|
|
|
DescribeTable("should load configuration from",
|
|
func(format string) {
|
|
filename := filepath.Join("testdata", "cfg."+format)
|
|
|
|
// Initialize config with the test file
|
|
conf.InitConfig(filename, false)
|
|
// Load the configuration (with noConfigDump=true)
|
|
conf.Load(true)
|
|
|
|
// Execute the format-specific assertions
|
|
Expect(conf.Server.MusicFolder).To(Equal(fmt.Sprintf("/%s/music", format)))
|
|
Expect(conf.Server.UIWelcomeMessage).To(Equal("Welcome " + format))
|
|
Expect(conf.Server.Tags["custom"].Aliases).To(Equal([]string{format, "test"}))
|
|
Expect(conf.Server.Tags["artist"].Split).To(Equal([]string{";"}))
|
|
|
|
// Check deprecated option mapping
|
|
Expect(conf.Server.ExtAuth.UserHeader).To(Equal("X-Auth-User"))
|
|
|
|
// The config file used should be the one we created
|
|
Expect(conf.Server.ConfigFile).To(Equal(filename))
|
|
},
|
|
Entry("TOML format", "toml"),
|
|
Entry("YAML format", "yaml"),
|
|
Entry("INI format", "ini"),
|
|
Entry("JSON format", "json"),
|
|
)
|
|
})
|