diff --git a/.github/workflows/download-link-on-pr.yml b/.github/workflows/download-link-on-pr.yml index 076f963d4..5b421331b 100644 --- a/.github/workflows/download-link-on-pr.yml +++ b/.github/workflows/download-link-on-pr.yml @@ -8,7 +8,7 @@ jobs: if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' runs-on: ubuntu-latest steps: - - uses: actions/github-script@v7 + - uses: actions/github-script@v9 with: # This snippet is public-domain, taken from # https://github.com/oprypin/nightly.link/blob/master/.github/workflows/pr-comment.yml diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 86a1055f8..8e6e8126a 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -166,7 +166,7 @@ jobs: - name: Cache ffmpeg id: ffmpeg-cache - uses: actions/cache@v5 + uses: actions/cache@v6 with: path: C:\ffmpeg key: ffmpeg-${{ env.FFMPEG_VERSION }}-win64 @@ -323,7 +323,7 @@ jobs: - name: Set up QEMU for smoke test if: env.IS_LINUX == 'true' - uses: docker/setup-qemu-action@v3 + uses: docker/setup-qemu-action@v4 # The binary is static, so binfmt+qemu runs it directly on the runner. # Catches startup crashes in cross-compiled binaries before they ship, diff --git a/cmd/root.go b/cmd/root.go index 08773176a..b231aae0d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -86,7 +86,7 @@ func runNavidrome(ctx context.Context) { g.Go(startPlaybackServer(ctx)) g.Go(schedulePeriodicBackup(ctx)) g.Go(startInsightsCollector(ctx)) - g.Go(scheduleDBOptimizer(ctx)) + g.Go(scheduleDBAnalyzer(ctx)) g.Go(startPluginManager(ctx)) g.Go(runInitialScan(ctx)) if conf.Server.Scanner.Enabled { @@ -275,16 +275,24 @@ func schedulePeriodicBackup(ctx context.Context) func() error { } } -func scheduleDBOptimizer(ctx context.Context) func() error { +func scheduleDBAnalyzer(ctx context.Context) func() error { return func() error { - log.Info(ctx, "Scheduling DB optimizer", "schedule", consts.OptimizeDBSchedule) + if !conf.Server.EnableScheduledDBAnalyze { + log.Info(ctx, "Scheduled DB analysis is DISABLED") + return nil + } + log.Info(ctx, "Scheduling DB analysis check", "schedule", consts.DBAnalyzeCheckSchedule) schedulerInstance := scheduler.GetInstance() - _, err := schedulerInstance.Add(consts.OptimizeDBSchedule, func() { - if scanner.IsScanning() { - log.Debug(ctx, "Skipping DB optimization because a scan is in progress") + _, err := schedulerInstance.Add(consts.DBAnalyzeCheckSchedule, func() { + release, ok := scanner.LockForMaintenance() + if !ok { + log.Debug(ctx, "Skipping DB analysis check because a scan is in progress") return } - db.Optimize(ctx) + defer release() + if _, err := db.OptimizeIfNeeded(ctx); err != nil { + log.Error(ctx, "Error analyzing DB", err) + } }) return err } diff --git a/cmd/scan.go b/cmd/scan.go index d8a563396..320b401d4 100644 --- a/cmd/scan.go +++ b/cmd/scan.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "encoding/gob" + "errors" "fmt" "os" "strings" @@ -43,15 +44,20 @@ var scanCmd = &cobra.Command{ }, } -func trackScanInteractively(ctx context.Context, progress <-chan *scanner.ProgressInfo) { +func trackScanInteractively(ctx context.Context, progress <-chan *scanner.ProgressInfo) (bool, error) { + var changesDetected bool + var scanErrors []error for status := range pl.ReadOrDone(ctx, progress) { if status.Warning != "" { log.Warn(ctx, "Scan warning", "error", status.Warning) } if status.Error != "" { log.Error(ctx, "Scan error", "error", status.Error) + scanErrors = append(scanErrors, errors.New(status.Error)) + } + if status.ChangesDetected { + changesDetected = true } - // Discard the progress status, we only care about errors } if fullScan { @@ -59,6 +65,7 @@ func trackScanInteractively(ctx context.Context, progress <-chan *scanner.Progre } else { log.Info("Finished rescan") } + return changesDetected, errors.Join(scanErrors...) } func trackScanAsSubprocess(ctx context.Context, progress <-chan *scanner.ProgressInfo) { @@ -95,6 +102,16 @@ func runScanner(ctx context.Context) { log.Info(ctx, "Scanning specific folders", "numTargets", len(scanTargets)) } + effectiveFullScan := fullScan + if !subprocess { + effectiveFullScan = scanner.EffectiveFullScan(ctx, ds, fullScan, scanTargets) + if effectiveFullScan { + if err := db.MarkOptimizePending(ctx); err != nil { + log.Error(ctx, "Error marking DB analysis pending", err) + } + } + } + progress, err := scanner.CallScan(ctx, ds, pls, fullScan, scanTargets) if err != nil { log.Fatal(ctx, "Failed to scan", err) @@ -104,7 +121,21 @@ func runScanner(ctx context.Context) { if subprocess { trackScanAsSubprocess(ctx, progress) } else { - trackScanInteractively(ctx, progress) + changesDetected, scanErr := trackScanInteractively(ctx, progress) + runPostScanAnalysis(ctx, changesDetected, effectiveFullScan, scanErr) + } +} + +func runPostScanAnalysis(ctx context.Context, changesDetected, effectiveFullScan bool, scanErr error) { + if changesDetected { + if err := db.MarkOptimizePending(ctx); err != nil { + log.Error(ctx, "Error marking DB analysis pending", err) + } + } + if effectiveFullScan && scanErr == nil { + if err := db.Optimize(ctx); err != nil { + log.Error(ctx, "Error analyzing DB", err) + } } } diff --git a/cmd/scan_test.go b/cmd/scan_test.go index beeecca19..309d09f98 100644 --- a/cmd/scan_test.go +++ b/cmd/scan_test.go @@ -1,14 +1,29 @@ package cmd import ( + "context" "os" "path/filepath" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/scanner" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) +var _ = Describe("trackScanInteractively", func() { + It("reports changes and scan errors", func() { + progress := make(chan *scanner.ProgressInfo, 2) + progress <- &scanner.ProgressInfo{ChangesDetected: true} + progress <- &scanner.ProgressInfo{Error: "scan failed"} + close(progress) + + changesDetected, err := trackScanInteractively(context.Background(), progress) + Expect(changesDetected).To(BeTrue()) + Expect(err).To(MatchError("scan failed")) + }) +}) + var _ = Describe("readTargetsFromFile", func() { var tempDir string diff --git a/conf/configuration.go b/conf/configuration.go index 8646bf075..e939ebb7e 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -51,6 +51,7 @@ type configOptions struct { EnableExternalServices bool EnableM3UExternalAlbumArt bool EnableInsightsCollector bool + EnableScheduledDBAnalyze bool EnableMediaFileCoverArt bool TranscodingCacheSize string ImageCacheSize string @@ -147,7 +148,6 @@ type configOptions struct { DevEnablePluginsInsights bool DevPluginCompilationTimeout time.Duration DevExternalArtistFetchMultiplier float64 - DevOptimizeDB bool DevPreserveUnicodeInExternalCalls bool DevEnableMediaFileProbe bool } @@ -800,6 +800,7 @@ func setViperDefaults() { viper.SetDefault("defaultdownloadableshare", false) viper.SetDefault("gatrackingid", "") viper.SetDefault("enableinsightscollector", true) + viper.SetDefault("enablescheduleddbanalyze", true) viper.SetDefault("enablelogredacting", true) viper.SetDefault("authrequestlimit", 5) viper.SetDefault("authwindowlength", 20*time.Second) @@ -891,7 +892,6 @@ func setViperDefaults() { viper.SetDefault("devenablepluginsinsights", true) viper.SetDefault("devplugincompilationtimeout", time.Minute) viper.SetDefault("devexternalartistfetchmultiplier", 1.5) - viper.SetDefault("devoptimizedb", true) viper.SetDefault("devpreserveunicodeinexternalcalls", false) viper.SetDefault("devenablemediafileprobe", true) } diff --git a/conf/configuration_test.go b/conf/configuration_test.go index 9c25a0d19..e43c91a4b 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -58,6 +58,19 @@ var _ = Describe("Configuration", func() { }) }) + 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") diff --git a/consts/consts.go b/consts/consts.go index 3795b590a..73f89b450 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -20,6 +20,10 @@ const ( LastScanErrorKey = "LastScanError" LastScanTypeKey = "LastScanType" LastScanStartTimeKey = "LastScanStartTime" + LastDBAnalyzeAtKey = "LastDBAnalyzeAt" + LastDBAnalyzeAttemptAtKey = "LastDBAnalyzeAttemptAt" + DBAnalyzePendingKey = "DBAnalyzePending" + DBAnalyzeFailureCountKey = "DBAnalyzeFailureCount" UIAuthorizationHeader = "X-ND-Authorization" UIClientUniqueIDHeader = "X-ND-Client-Unique-Id" @@ -28,7 +32,8 @@ const ( DefaultSessionTimeout = 48 * time.Hour CookieExpiry = 365 * 24 * 3600 // One year - OptimizeDBSchedule = "@every 24h" + DBAnalyzeCheckSchedule = "@every 30m" + DBAnalyzeMaxAge = 24 * time.Hour // DefaultEncryptionKey This is the encryption key used if none is specified in the `PasswordEncryptionKey` option // Never ever change this! Or it will break all Navidrome installations that don't set the config option diff --git a/core/metrics/insights.go b/core/metrics/insights.go index bcd0343c2..78391779a 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -223,6 +223,7 @@ var staticData = sync.OnceValue(func() insights.Data { data.Config.ScanSchedule = conf.Server.Scanner.Schedule data.Config.ScanWatcherWait = uint64(math.Trunc(conf.Server.Scanner.WatcherWait.Seconds())) data.Config.ScanOnStartup = conf.Server.Scanner.ScanOnStartup + data.Config.EnableScheduledDBAnalyze = conf.Server.EnableScheduledDBAnalyze data.Config.ReverseProxyConfigured = conf.Server.ExtAuth.TrustedSources != "" data.Config.HasCustomPID = conf.Server.PID.Track != consts.DefaultTrackPID || conf.Server.PID.Album != consts.DefaultAlbumPID data.Config.HasCustomTags = len(conf.Server.Tags) > 0 diff --git a/core/metrics/insights/data.go b/core/metrics/insights/data.go index 34648a49b..126d759bc 100644 --- a/core/metrics/insights/data.go +++ b/core/metrics/insights/data.go @@ -43,45 +43,46 @@ type Data struct { FileSuffixes map[string]int64 `json:"fileSuffixes,omitempty"` } `json:"library"` Config struct { - LogLevel string `json:"logLevel,omitempty"` - LogFileConfigured bool `json:"logFileConfigured,omitempty"` - TLSConfigured bool `json:"tlsConfigured,omitempty"` - ScannerEnabled bool `json:"scannerEnabled,omitempty"` - ScannerExtractor string `json:"scannerExtractor,omitempty"` - ScanSchedule string `json:"scanSchedule,omitempty"` - ScanWatcherWait uint64 `json:"scanWatcherWait,omitempty"` - ScanOnStartup bool `json:"scanOnStartup,omitempty"` - TranscodingCacheSize string `json:"transcodingCacheSize,omitempty"` - ImageCacheSize string `json:"imageCacheSize,omitempty"` - EnableArtworkPrecache bool `json:"enableArtworkPrecache,omitempty"` - EnableDownloads bool `json:"enableDownloads,omitempty"` - EnableSharing bool `json:"enableSharing,omitempty"` - EnableStarRating bool `json:"enableStarRating,omitempty"` - EnableLastFM bool `json:"enableLastFM,omitempty"` - EnableListenBrainz bool `json:"enableListenBrainz,omitempty"` - EnableDeezer bool `json:"enableDeezer,omitempty"` - EnableMediaFileCoverArt bool `json:"enableMediaFileCoverArt,omitempty"` - EnableJukebox bool `json:"enableJukebox,omitempty"` - EnablePrometheus bool `json:"enablePrometheus,omitempty"` - EnableArtworkUpload bool `json:"enableArtworkUpload,omitempty"` - CoverArtQuality int `json:"coverArtQuality,omitempty"` - EnableWebPEncoding bool `json:"enableWebPEncoding,omitempty"` - UICoverArtSize int `json:"uiCoverArtSize,omitempty"` - EnableCoverAnimation bool `json:"enableCoverAnimation,omitempty"` - EnableNowPlaying bool `json:"enableNowPlaying,omitempty"` - SessionTimeout uint64 `json:"sessionTimeout,omitempty"` - SearchFullString bool `json:"searchFullString,omitempty"` - SearchBackend string `json:"searchBackend,omitempty"` - RecentlyAddedByModTime bool `json:"recentlyAddedByModTime,omitempty"` - PreferSortTags bool `json:"preferSortTags,omitempty"` - BackupSchedule string `json:"backupSchedule,omitempty"` - BackupCount int `json:"backupCount,omitempty"` - DevActivityPanel bool `json:"devActivityPanel,omitempty"` - DefaultBackgroundURLSet bool `json:"defaultBackgroundURL,omitempty"` - HasSmartPlaylists bool `json:"hasSmartPlaylists,omitempty"` - ReverseProxyConfigured bool `json:"reverseProxyConfigured,omitempty"` - HasCustomPID bool `json:"hasCustomPID,omitempty"` - HasCustomTags bool `json:"hasCustomTags,omitempty"` + LogLevel string `json:"logLevel,omitempty"` + LogFileConfigured bool `json:"logFileConfigured,omitempty"` + TLSConfigured bool `json:"tlsConfigured,omitempty"` + ScannerEnabled bool `json:"scannerEnabled,omitempty"` + ScannerExtractor string `json:"scannerExtractor,omitempty"` + ScanSchedule string `json:"scanSchedule,omitempty"` + ScanWatcherWait uint64 `json:"scanWatcherWait,omitempty"` + ScanOnStartup bool `json:"scanOnStartup,omitempty"` + EnableScheduledDBAnalyze bool `json:"enableScheduledDBAnalyze,omitempty"` + TranscodingCacheSize string `json:"transcodingCacheSize,omitempty"` + ImageCacheSize string `json:"imageCacheSize,omitempty"` + EnableArtworkPrecache bool `json:"enableArtworkPrecache,omitempty"` + EnableDownloads bool `json:"enableDownloads,omitempty"` + EnableSharing bool `json:"enableSharing,omitempty"` + EnableStarRating bool `json:"enableStarRating,omitempty"` + EnableLastFM bool `json:"enableLastFM,omitempty"` + EnableListenBrainz bool `json:"enableListenBrainz,omitempty"` + EnableDeezer bool `json:"enableDeezer,omitempty"` + EnableMediaFileCoverArt bool `json:"enableMediaFileCoverArt,omitempty"` + EnableJukebox bool `json:"enableJukebox,omitempty"` + EnablePrometheus bool `json:"enablePrometheus,omitempty"` + EnableArtworkUpload bool `json:"enableArtworkUpload,omitempty"` + CoverArtQuality int `json:"coverArtQuality,omitempty"` + EnableWebPEncoding bool `json:"enableWebPEncoding,omitempty"` + UICoverArtSize int `json:"uiCoverArtSize,omitempty"` + EnableCoverAnimation bool `json:"enableCoverAnimation,omitempty"` + EnableNowPlaying bool `json:"enableNowPlaying,omitempty"` + SessionTimeout uint64 `json:"sessionTimeout,omitempty"` + SearchFullString bool `json:"searchFullString,omitempty"` + SearchBackend string `json:"searchBackend,omitempty"` + RecentlyAddedByModTime bool `json:"recentlyAddedByModTime,omitempty"` + PreferSortTags bool `json:"preferSortTags,omitempty"` + BackupSchedule string `json:"backupSchedule,omitempty"` + BackupCount int `json:"backupCount,omitempty"` + DevActivityPanel bool `json:"devActivityPanel,omitempty"` + DefaultBackgroundURLSet bool `json:"defaultBackgroundURL,omitempty"` + HasSmartPlaylists bool `json:"hasSmartPlaylists,omitempty"` + ReverseProxyConfigured bool `json:"reverseProxyConfigured,omitempty"` + HasCustomPID bool `json:"hasCustomPID,omitempty"` + HasCustomTags bool `json:"hasCustomTags,omitempty"` } `json:"config"` Plugins map[string]PluginInfo `json:"plugins,omitempty"` } diff --git a/core/scrobbler/play_tracker_test.go b/core/scrobbler/play_tracker_test.go index 74b4be893..831b0ce0d 100644 --- a/core/scrobbler/play_tracker_test.go +++ b/core/scrobbler/play_tracker_test.go @@ -290,7 +290,7 @@ var _ = Describe("PlayTracker", func() { Expect(mockScrobble.RecordedScrobbles).To(HaveLen(1)) Expect(mockScrobble.RecordedScrobbles[0].MediaFileID).To(Equal("123")) Expect(mockScrobble.RecordedScrobbles[0].UserID).To(Equal("u-1")) - Expect(mockScrobble.RecordedScrobbles[0].SubmissionTime).To(Equal(ts)) + Expect(mockScrobble.RecordedScrobbles[0].SubmissionTime).To(Equal(ts.Unix())) }) It("does not record scrobble when history is disabled", func() { diff --git a/db/db.go b/db/db.go index 6e5b2f569..3f3f61d71 100644 --- a/db/db.go +++ b/db/db.go @@ -6,6 +6,7 @@ import ( "embed" "fmt" "runtime" + "time" "github.com/mattn/go-sqlite3" "github.com/navidrome/navidrome/conf" @@ -47,12 +48,6 @@ func Db() *sql.DB { if err != nil { log.Fatal("Error opening database", err) } - if conf.Server.DevOptimizeDB { - _, err = db.Exec("PRAGMA optimize=0x10002") - if err != nil { - log.Error("Error applying PRAGMA optimize", err) - } - } return db }) } @@ -61,9 +56,6 @@ func Close(ctx context.Context) { // Ignore cancellations when closing the DB ctx = context.WithoutCancel(ctx) - // Run optimize before closing - Optimize(ctx) - log.Info(ctx, "Closing Database") err := Db().Close() if err != nil { @@ -102,11 +94,11 @@ func Init(ctx context.Context) func() { log.Fatal(ctx, "Failed to apply new migrations", err) } - if hasSchemaChanges && conf.Server.DevOptimizeDB { - log.Debug(ctx, "Applying PRAGMA optimize after schema changes") - _, err = db.ExecContext(ctx, "PRAGMA optimize") + if hasSchemaChanges { + log.Debug(ctx, "Running ANALYZE after schema changes") + err = optimizeAt(ctx, db, time.Now()) if err != nil { - log.Error(ctx, "Error applying PRAGMA optimize", err) + log.Error(ctx, "Error running ANALYZE", err) } } @@ -115,37 +107,6 @@ func Init(ctx context.Context) func() { } } -// Optimize runs PRAGMA optimize on each connection in the pool -func Optimize(ctx context.Context) { - if !conf.Server.DevOptimizeDB { - return - } - numConns := Db().Stats().OpenConnections - if numConns == 0 { - log.Debug(ctx, "No open connections to optimize") - return - } - log.Debug(ctx, "Optimizing open connections", "numConns", numConns) - var conns []*sql.Conn - for range numConns { - conn, err := Db().Conn(ctx) - conns = append(conns, conn) - if err != nil { - log.Error(ctx, "Error getting connection from pool", err) - continue - } - _, err = conn.ExecContext(ctx, "PRAGMA optimize;") - if err != nil { - log.Error(ctx, "Error running PRAGMA optimize", err) - } - } - - // Return all connections to the Connection Pool - for _, conn := range conns { - conn.Close() - } -} - type statusLogger struct{ numPending int } func (*statusLogger) Fatalf(format string, v ...any) { log.Fatal(fmt.Sprintf(format, v...)) } diff --git a/db/export_test.go b/db/export_test.go index 734a4462f..02b88cd66 100644 --- a/db/export_test.go +++ b/db/export_test.go @@ -2,6 +2,9 @@ package db // Definitions for testing private methods var ( - IsSchemaEmpty = isSchemaEmpty - BackupPath = backupPath + IsSchemaEmpty = isSchemaEmpty + BackupPath = backupPath + OptimizeDBAt = optimizeAt + OptimizeDBIfNeeded = optimizeIfNeeded + RecordAnalyzeFailure = recordAnalyzeFailure ) diff --git a/db/migrations/20260712211040_add_primary_key_and_update_index_for_scrobbles.sql b/db/migrations/20260712211040_add_primary_key_and_update_index_for_scrobbles.sql new file mode 100644 index 000000000..220d7cf75 --- /dev/null +++ b/db/migrations/20260712211040_add_primary_key_and_update_index_for_scrobbles.sql @@ -0,0 +1,39 @@ +-- +goose Up +CREATE TABLE scrobbles_tmp( + id INTEGER PRIMARY KEY, + media_file_id VARCHAR(255) NOT NULL + REFERENCES media_file(id) + ON DELETE CASCADE + ON UPDATE CASCADE, + user_id VARCHAR(255) NOT NULL + REFERENCES user(id) + ON DELETE CASCADE + ON UPDATE CASCADE, + submission_time INTEGER NOT NULL +); +INSERT INTO scrobbles_tmp SELECT ROWID, media_file_id, user_id, submission_time FROM scrobbles; + +DROP INDEX scrobbles_date; +DROP TABLE scrobbles; +ALTER TABLE scrobbles_tmp RENAME TO scrobbles; +CREATE INDEX scrobbles_user_time ON scrobbles(user_id, submission_time); + + +-- +goose Down +CREATE TABLE scrobbles_tmp( + media_file_id VARCHAR(255) NOT NULL + REFERENCES media_file(id) + ON DELETE CASCADE + ON UPDATE CASCADE, + user_id VARCHAR(255) NOT NULL + REFERENCES user(id) + ON DELETE CASCADE + ON UPDATE CASCADE, + submission_time INTEGER NOT NULL +); +INSERT INTO scrobbles_tmp SELECT media_file_id, user_id, submission_time FROM scrobbles; + +DROP INDEX scrobbles_user_time; +DROP TABLE scrobbles; +ALTER TABLE scrobbles_tmp RENAME TO scrobbles; +CREATE INDEX scrobbles_date ON scrobbles(submission_time); \ No newline at end of file diff --git a/db/migrations/migration.go b/db/migrations/migration.go index 9b1098af1..df1c392a5 100644 --- a/db/migrations/migration.go +++ b/db/migrations/migration.go @@ -7,7 +7,6 @@ import ( "strings" "sync" - "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" ) @@ -21,13 +20,6 @@ func notice(ctx context.Context, tx *sql.Tx, msg string) { // Call this in migrations that requires a full rescan func forceFullRescan(ctx context.Context, tx *sql.Tx) error { - // If a full scan is required, most probably the query optimizer is outdated, so we run `analyze`. - if conf.Server.DevOptimizeDB { - _, err := tx.ExecContext(ctx, `ANALYZE;`) - if err != nil { - return err - } - } _, err := tx.ExecContext(ctx, fmt.Sprintf(` INSERT OR REPLACE into property (id, value) values ('%s', '1'); `, consts.FullScanAfterMigrationFlagKey)) diff --git a/db/optimize.go b/db/optimize.go new file mode 100644 index 000000000..f46906c4e --- /dev/null +++ b/db/optimize.go @@ -0,0 +1,224 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strconv" + "sync" + "time" + + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/log" +) + +var analyzeMux sync.Mutex + +// Optimize refreshes the query-planner statistics with a full ANALYZE. PRAGMA optimize is avoided +// because its limited analysis misestimates Navidrome's low-cardinality indexes. +func Optimize(ctx context.Context) error { + analyzeMux.Lock() + defer analyzeMux.Unlock() + start := time.Now() + if err := optimizeAt(ctx, Db(), start); err != nil { + return err + } + log.Info(ctx, "DB analysis complete", "elapsed", time.Since(start)) + return nil +} + +// OptimizeIfNeeded refreshes statistics when they are stale or a database-changing operation +// marked them for refresh. +func OptimizeIfNeeded(ctx context.Context) (bool, error) { + analyzeMux.Lock() + defer analyzeMux.Unlock() + start := time.Now() + ran, err := optimizeIfNeeded(ctx, Db(), start) + if err != nil || !ran { + return ran, err + } + log.Info(ctx, "DB analysis complete", "elapsed", time.Since(start)) + return true, nil +} + +func optimizeIfNeeded(ctx context.Context, db *sql.DB, now time.Time) (bool, error) { + due, err := optimizeDue(ctx, db, now) + if err != nil || !due { + return false, err + } + return true, optimizeAt(ctx, db, now) +} + +func optimizeDue(ctx context.Context, db *sql.DB, now time.Time) (bool, error) { + backingOff, err := analyzeRetryBackoffActive(ctx, db, now) + if err != nil || backingOff { + return false, err + } + + pending, found, err := getProperty(ctx, db, consts.DBAnalyzePendingKey) + if err != nil { + return false, err + } + if found && pending == "1" { + return true, nil + } + + value, found, err := getProperty(ctx, db, consts.LastDBAnalyzeAtKey) + if err != nil { + return false, err + } + if !found { + return true, nil + } + + lastAnalyze, valid := parseAnalyzeTime(value) + if !valid || lastAnalyze.After(now) { + return true, nil + } + return now.Sub(lastAnalyze) >= consts.DBAnalyzeMaxAge, nil +} + +func parseAnalyzeTime(value string) (time.Time, bool) { + parsed, err := time.Parse(time.RFC3339Nano, value) + return parsed, err == nil +} + +func analyzeRetryBackoffActive(ctx context.Context, db *sql.DB, now time.Time) (bool, error) { + value, found, err := getProperty(ctx, db, consts.DBAnalyzeFailureCountKey) + if err != nil || !found { + return false, err + } + failures, _ := strconv.Atoi(value) + if failures < 1 { + return false, nil + } + + value, found, err = getProperty(ctx, db, consts.LastDBAnalyzeAttemptAtKey) + if err != nil || !found { + return false, err + } + lastAttempt, valid := parseAnalyzeTime(value) + if !valid || lastAttempt.After(now) { + return false, nil + } + return now.Sub(lastAttempt) < analyzeRetryDelay(failures), nil +} + +func analyzeRetryDelay(failures int) time.Duration { + switch failures { + case 1: + return 30 * time.Minute + case 2: + return time.Hour + case 3: + return 2 * time.Hour + default: + return 24 * time.Hour + } +} + +// MarkOptimizePending requests a statistics refresh on the next scheduled maintenance check. +func MarkOptimizePending(ctx context.Context) error { + analyzeMux.Lock() + defer analyzeMux.Unlock() + return markOptimizePending(ctx, Db()) +} + +func markOptimizePending(ctx context.Context, db *sql.DB) error { + return putProperty(ctx, db, consts.DBAnalyzePendingKey, "1") +} + +func optimizeAt(ctx context.Context, db *sql.DB, now time.Time) error { + if err := markOptimizePending(ctx, db); err != nil { + return recordAnalyzeError(ctx, db, now, fmt.Errorf("marking ANALYZE pending: %w", err)) + } + log.Debug(ctx, "Refreshing query planner statistics") + _, err := db.ExecContext(ctx, "ANALYZE") + if err != nil { + return recordAnalyzeError(ctx, db, now, fmt.Errorf("running ANALYZE: %w", err)) + } + if err = recordAnalyzeSuccess(ctx, db, now); err != nil { + return recordAnalyzeError(ctx, db, now, err) + } + return nil +} + +func recordAnalyzeSuccess(ctx context.Context, db *sql.DB, now time.Time) error { + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("recording ANALYZE time: %w", err) + } + defer func() { _ = tx.Rollback() }() + if err = putProperty(ctx, tx, consts.LastDBAnalyzeAtKey, now.UTC().Format(time.RFC3339Nano)); err != nil { + return fmt.Errorf("recording ANALYZE time: %w", err) + } + if err = putProperty(ctx, tx, consts.DBAnalyzePendingKey, "0"); err != nil { + return fmt.Errorf("clearing pending ANALYZE: %w", err) + } + if err = putProperty(ctx, tx, consts.DBAnalyzeFailureCountKey, "0"); err != nil { + return fmt.Errorf("clearing ANALYZE failure count: %w", err) + } + if err = tx.Commit(); err != nil { + return fmt.Errorf("recording ANALYZE state: %w", err) + } + return nil +} + +func recordAnalyzeError(ctx context.Context, db *sql.DB, now time.Time, analyzeErr error) error { + if err := recordAnalyzeFailure(ctx, db, now); err != nil { + return errors.Join(analyzeErr, fmt.Errorf("recording ANALYZE failure: %w", err)) + } + return analyzeErr +} + +func recordAnalyzeFailure(ctx context.Context, db *sql.DB, now time.Time) error { + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + + value, found, err := getProperty(ctx, tx, consts.DBAnalyzeFailureCountKey) + if err != nil { + return err + } + failures := 0 + if found { + failures, _ = strconv.Atoi(value) + failures = max(failures, 0) + } + if err = putProperty(ctx, tx, consts.DBAnalyzePendingKey, "1"); err != nil { + return err + } + if err = putProperty(ctx, tx, consts.DBAnalyzeFailureCountKey, strconv.Itoa(failures+1)); err != nil { + return err + } + if err = putProperty(ctx, tx, consts.LastDBAnalyzeAttemptAtKey, now.UTC().Format(time.RFC3339Nano)); err != nil { + return err + } + return tx.Commit() +} + +type sqlExecer interface { + ExecContext(context.Context, string, ...any) (sql.Result, error) +} + +type sqlQueryer interface { + QueryRowContext(context.Context, string, ...any) *sql.Row +} + +func putProperty(ctx context.Context, db sqlExecer, key, value string) error { + _, err := db.ExecContext(ctx, `insert into property(id, value) values(?, ?) + on conflict(id) do update set value=excluded.value`, key, value) + return err +} + +func getProperty(ctx context.Context, db sqlQueryer, key string) (string, bool, error) { + var value string + err := db.QueryRowContext(ctx, "select value from property where id=?", key).Scan(&value) + if errors.Is(err, sql.ErrNoRows) { + return "", false, nil + } + return value, err == nil, err +} diff --git a/db/optimize_test.go b/db/optimize_test.go new file mode 100644 index 000000000..da9b3b9c9 --- /dev/null +++ b/db/optimize_test.go @@ -0,0 +1,162 @@ +package db_test + +import ( + "context" + "database/sql" + "time" + + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/db" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Optimize", func() { + var ( + ctx context.Context + database *sql.DB + now time.Time + ) + + BeforeEach(func() { + ctx = context.Background() + now = time.Date(2026, time.July, 9, 12, 0, 0, 0, time.UTC) + var err error + database, err = sql.Open(db.Dialect, "file::memory:") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(database.Close) + + _, err = database.Exec(`create table property( + id varchar(255) primary key, + value varchar(255) not null default '' + )`) + Expect(err).ToNot(HaveOccurred()) + _, err = database.Exec("create table analyze_probe(id integer primary key, flag int)") + Expect(err).ToNot(HaveOccurred()) + _, err = database.Exec(`insert into analyze_probe(flag) + with recursive s(x) as (select 1 union all select x+1 from s where x < 3000) + select 0 from s`) + Expect(err).ToNot(HaveOccurred()) + _, err = database.Exec("create index probe_flag on analyze_probe(flag)") + Expect(err).ToNot(HaveOccurred()) + _, err = database.Exec("analyze") + Expect(err).ToNot(HaveOccurred()) + }) + + putProperty := func(key, value string) { + _, err := database.Exec(`insert into property(id, value) values(?, ?) + on conflict(id) do update set value=excluded.value`, key, value) + Expect(err).ToNot(HaveOccurred()) + } + + getProperty := func(key string) string { + var value string + Expect(database.QueryRow("select value from property where id=?", key).Scan(&value)).To(Succeed()) + return value + } + + poisonStats := func() { + _, err := database.Exec("update sqlite_stat1 set stat='3000 50' where idx='probe_flag'") + Expect(err).ToNot(HaveOccurred()) + } + + It("replaces poisoned planner statistics with full-quality ones", func() { + poisonStats() + putProperty(consts.DBAnalyzePendingKey, "1") + + Expect(db.OptimizeDBAt(ctx, database, now)).To(Succeed()) + + var stat string + err := database.QueryRow("select stat from sqlite_stat1 where idx='probe_flag'").Scan(&stat) + Expect(err).ToNot(HaveOccurred()) + // A full ANALYZE sees all 3000 rows share one value: avg rows per key = row count. + Expect(stat).To(Equal("3000 3000")) + Expect(getProperty(consts.LastDBAnalyzeAtKey)).To(Equal(now.Format(time.RFC3339Nano))) + Expect(getProperty(consts.DBAnalyzePendingKey)).To(Equal("0")) + }) + + It("runs when no previous analysis was recorded", func() { + ran, err := db.OptimizeDBIfNeeded(ctx, database, now) + Expect(err).ToNot(HaveOccurred()) + Expect(ran).To(BeTrue()) + Expect(getProperty(consts.LastDBAnalyzeAtKey)).To(Equal(now.Format(time.RFC3339Nano))) + }) + + It("skips a recent analysis when no refresh is pending", func() { + lastAnalyze := now.Add(-23 * time.Hour) + putProperty(consts.LastDBAnalyzeAtKey, lastAnalyze.Format(time.RFC3339Nano)) + putProperty(consts.DBAnalyzePendingKey, "0") + poisonStats() + + ran, err := db.OptimizeDBIfNeeded(ctx, database, now) + Expect(err).ToNot(HaveOccurred()) + Expect(ran).To(BeFalse()) + Expect(getProperty(consts.LastDBAnalyzeAtKey)).To(Equal(lastAnalyze.Format(time.RFC3339Nano))) + + var stat string + Expect(database.QueryRow("select stat from sqlite_stat1 where idx='probe_flag'").Scan(&stat)).To(Succeed()) + Expect(stat).To(Equal("3000 50")) + }) + + It("runs when the previous analysis is stale", func() { + putProperty(consts.LastDBAnalyzeAtKey, now.Add(-consts.DBAnalyzeMaxAge).Format(time.RFC3339Nano)) + putProperty(consts.DBAnalyzePendingKey, "0") + + ran, err := db.OptimizeDBIfNeeded(ctx, database, now) + Expect(err).ToNot(HaveOccurred()) + Expect(ran).To(BeTrue()) + Expect(getProperty(consts.LastDBAnalyzeAtKey)).To(Equal(now.Format(time.RFC3339Nano))) + }) + + It("runs when a refresh is pending even if the previous analysis is recent", func() { + putProperty(consts.LastDBAnalyzeAtKey, now.Format(time.RFC3339Nano)) + putProperty(consts.DBAnalyzePendingKey, "1") + + ran, err := db.OptimizeDBIfNeeded(ctx, database, now.Add(time.Hour)) + Expect(err).ToNot(HaveOccurred()) + Expect(ran).To(BeTrue()) + Expect(getProperty(consts.DBAnalyzePendingKey)).To(Equal("0")) + }) + + DescribeTable("backs off after consecutive analysis failures", + func(failures string, retryDelay time.Duration) { + putProperty(consts.DBAnalyzePendingKey, "1") + putProperty(consts.DBAnalyzeFailureCountKey, failures) + putProperty(consts.LastDBAnalyzeAttemptAtKey, now.Format(time.RFC3339Nano)) + + ran, err := db.OptimizeDBIfNeeded(ctx, database, now.Add(retryDelay-time.Nanosecond)) + Expect(err).ToNot(HaveOccurred()) + Expect(ran).To(BeFalse()) + + ran, err = db.OptimizeDBIfNeeded(ctx, database, now.Add(retryDelay)) + Expect(err).ToNot(HaveOccurred()) + Expect(ran).To(BeTrue()) + Expect(getProperty(consts.DBAnalyzeFailureCountKey)).To(Equal("0")) + Expect(getProperty(consts.DBAnalyzePendingKey)).To(Equal("0")) + }, + Entry("for 30 minutes after the first failure", "1", 30*time.Minute), + Entry("for one hour after the second failure", "2", time.Hour), + Entry("for two hours after the third failure", "3", 2*time.Hour), + Entry("for 24 hours after the fourth failure", "4", 24*time.Hour), + ) + + It("records consecutive analysis failures", func() { + putProperty(consts.DBAnalyzeFailureCountKey, "2") + + Expect(db.RecordAnalyzeFailure(ctx, database, now)).To(Succeed()) + + Expect(getProperty(consts.DBAnalyzeFailureCountKey)).To(Equal("3")) + Expect(getProperty(consts.LastDBAnalyzeAttemptAtKey)).To(Equal(now.Format(time.RFC3339Nano))) + Expect(getProperty(consts.DBAnalyzePendingKey)).To(Equal("1")) + }) + + It("does not record success when analysis fails", func() { + lastAnalyze := now.Add(-48 * time.Hour).Format(time.RFC3339Nano) + putProperty(consts.LastDBAnalyzeAtKey, lastAnalyze) + canceledCtx, cancel := context.WithCancel(ctx) + cancel() + + Expect(db.OptimizeDBAt(canceledCtx, database, now)).To(MatchError(ContainSubstring("context canceled"))) + Expect(getProperty(consts.LastDBAnalyzeAtKey)).To(Equal(lastAnalyze)) + }) +}) diff --git a/go.mod b/go.mod index a60afb76a..0a9c7398f 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( github.com/kardianos/service v1.3.0 github.com/kr/pretty v0.3.1 github.com/lestrrat-go/jwx/v3 v3.1.1 - github.com/mattn/go-sqlite3 v1.14.47 + github.com/mattn/go-sqlite3 v1.14.48 github.com/microcosm-cc/bluemonday v1.0.27 github.com/mileusna/useragent v1.3.5 github.com/onsi/ginkgo/v2 v2.32.0 @@ -59,12 +59,12 @@ require ( github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342 go.senan.xyz/taglib v0.11.1 go.uber.org/goleak v1.3.0 - golang.org/x/image v0.43.0 - golang.org/x/net v0.56.0 - golang.org/x/sync v0.21.0 - golang.org/x/sys v0.46.0 - golang.org/x/term v0.44.0 - golang.org/x/text v0.39.0 + golang.org/x/image v0.44.0 + golang.org/x/net v0.57.0 + golang.org/x/sync v0.22.0 + golang.org/x/sys v0.47.0 + golang.org/x/term v0.45.0 + golang.org/x/text v0.40.0 golang.org/x/time v0.15.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -76,7 +76,7 @@ require ( github.com/atombender/go-jsonschema v0.20.0 // indirect github.com/aymerick/douceur v0.2.0 // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/cespare/reflex v0.3.1 // indirect + github.com/cespare/reflex v0.3.2 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/creack/pty v1.1.24 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect @@ -92,7 +92,7 @@ require ( github.com/goccy/go-json v0.10.6 // indirect github.com/goccy/go-yaml v1.19.2 // indirect github.com/google/go-cmp v0.7.0 // indirect - github.com/google/pprof v0.0.0-20260604005048-7023385849c0 // indirect + github.com/google/pprof v0.0.0-20260709232956-b9395ee17fa0 // indirect github.com/google/subcommands v1.2.0 // indirect github.com/gorilla/css v1.0.1 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect @@ -136,10 +136,10 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.53.0 // indirect - golang.org/x/mod v0.37.0 // indirect - golang.org/x/telemetry v0.0.0-20260625142307-59b4966ccb57 // indirect - golang.org/x/tools v0.47.0 // indirect + golang.org/x/crypto v0.54.0 // indirect + golang.org/x/mod v0.38.0 // indirect + golang.org/x/telemetry v0.0.0-20260708182218-49f421fb7959 // indirect + golang.org/x/tools v0.48.0 // indirect google.golang.org/protobuf v1.36.11 // indirect gopkg.in/ini.v1 v1.67.3 // indirect gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce // indirect diff --git a/go.sum b/go.sum index 3de4b0660..708f0fb38 100644 --- a/go.sum +++ b/go.sum @@ -18,13 +18,12 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bmatcuk/doublestar/v4 v4.10.0 h1:zU9WiOla1YA122oLM6i4EXvGW62DvKZVxIe6TYWexEs= github.com/bmatcuk/doublestar/v4 v4.10.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= -github.com/cespare/reflex v0.3.1 h1:N4Y/UmRrjwOkNT0oQQnYsdr6YBxvHqtSfPB4mqOyAKk= -github.com/cespare/reflex v0.3.1/go.mod h1:I+0Pnu2W693i7Hv6ZZG76qHTY0mgUa7uCIfCtikXojE= +github.com/cespare/reflex v0.3.2 h1:SBN/trM94Ifs/ozz77cR3KxKm4dNE22zfG+0+54y5bQ= +github.com/cespare/reflex v0.3.2/go.mod h1:3hfHPnuDWHtNWk0aLKwwP6pomRkS3r2nM127108jY/4= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/davecgh/go-spew v0.0.0-20161028175848-04cdfd42973b/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -64,7 +63,6 @@ github.com/fatih/structs v1.1.0 h1:Q7juDM0QtcnhCpeyLGQKyg4TOIghuNXrkL32pHAUMxo= github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= -github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= github.com/gen2brain/webp v0.6.4 h1:SUDdmxADOAiPQ+5ylNmuHhuYf2dOi0KgKZHL5vpVCNU= @@ -111,8 +109,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-pipeline v0.0.0-20230411140531-6cbedfc1d3fc h1:hd+uUVsB1vdxohPneMrhGH2YfQuH5hRIK9u4/XCeUtw= github.com/google/go-pipeline v0.0.0-20230411140531-6cbedfc1d3fc/go.mod h1:SL66SJVysrh7YbDCP9tH30b8a9o/N2HeiQNUm85EKhc= -github.com/google/pprof v0.0.0-20260604005048-7023385849c0 h1:h1QTMDl6q9wDvDCJVpKQSjgleGFYnd2fOxmg2K+6BGE= -github.com/google/pprof v0.0.0-20260604005048-7023385849c0/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= +github.com/google/pprof v0.0.0-20260709232956-b9395ee17fa0 h1:du0WGc8xSKq/++e0cglxhS/mXVqsR7+c7jLEi5Vqduw= +github.com/google/pprof v0.0.0-20260709232956-b9395ee17fa0/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= github.com/google/subcommands v1.2.0 h1:vWQspBTo2nEqTUFita5/KeEWlUL8kQObDFbub/EN9oE= github.com/google/subcommands v1.2.0/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3yTrtFlrHVk= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -149,11 +147,8 @@ github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl github.com/klauspost/cpuid/v2 v2.4.0 h1:S6Hrbc7+ywsr0r+RLapfGBHfyefhCTwEh3A0tV913Dw= github.com/klauspost/cpuid/v2 v2.4.0/go.mod h1:19jmZ9mjzoF//ddRSUsv0zfBTJWh3QJh9FNxZTMrGxU= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= -github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= @@ -180,8 +175,8 @@ github.com/maruel/natural v1.3.0 h1:VsmCsBmEyrR46RomtgHs5hbKADGRVtliHTyCOLFBpsg= github.com/maruel/natural v1.3.0/go.mod h1:v+Rfd79xlw1AgVBjbO0BEQmptqb5HvL/k9GRHB7ZKEg= github.com/mattn/go-isatty v0.0.21 h1:xYae+lCNBP7QuW4PUnNG61ffM4hVIfm+zUzDuSzYLGs= github.com/mattn/go-isatty v0.0.21/go.mod h1:ZXfXG4SQHsB/w3ZeOYbR0PrPwLy+n6xiMrJlRFqopa4= -github.com/mattn/go-sqlite3 v1.14.47 h1:jOBI62gS7nKeZv+as1oGEy0+1qISgXwH/QBlR6KbfIo= -github.com/mattn/go-sqlite3 v1.14.47/go.mod h1:6JTjA44L93a0QCyJef5YvlPoKXntQPjzWv5gtm9sB6w= +github.com/mattn/go-sqlite3 v1.14.48 h1:7XHIgl0a8HwOaiK4E47ozLkST78rR9+OtNGx27D/TFs= +github.com/mattn/go-sqlite3 v1.14.48/go.mod h1:6JTjA44L93a0QCyJef5YvlPoKXntQPjzWv5gtm9sB6w= github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6BbAxPY= github.com/mfridman/interpolate v0.0.2/go.mod h1:p+7uk6oE07mpE/Ik1b8EckO0O4ZXiGAfshKBWLUM9Xg= github.com/mfridman/tparse v0.18.0 h1:wh6dzOKaIwkUGyKgOntDW4liXSo37qg5AXbIhkMV3vE= @@ -315,39 +310,38 @@ go.yaml.in/yaml/v2 v2.4.3/go.mod h1:zSxWcmIDjOzPXpjlTTbAsKokqkDNAVtZO0WOMiT90s8= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= -golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= -golang.org/x/image v0.43.0 h1:FLxcP4ec2350nTfOC8ysKtqYSIFbk/QGjw1ZHNP4tsY= -golang.org/x/image v0.43.0/go.mod h1:rrpelvGFt+kLPAjPM4HeWPgrl0FtafueU//e5N0qk/Q= -golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= -golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= +golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= +golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/image v0.44.0 h1:+tDekMZED9+LrtB3G5xzRggpVh9CARjZqROla3R3R+I= +golang.org/x/image v0.44.0/go.mod h1:V8K3KE9KKKE+pLpQDOeN18w9oacNSvy1tDOirTu4xtY= +golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= +golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= -golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= -golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= -golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= -golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= +golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= +golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= +golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20180926160741-c2ed4eda69e7/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20220615213510-4f61da869c0c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= -golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/telemetry v0.0.0-20260625142307-59b4966ccb57 h1:nwGZBCt+FnXUrGsj5vjzAsEmkcaFvd82BbOjECiFYZc= -golang.org/x/telemetry v0.0.0-20260625142307-59b4966ccb57/go.mod h1:3AWMyWHS+caVoiEXpiq6+tzKA40J4vQT3MYr80ZtQpc= -golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc= -golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/telemetry v0.0.0-20260708182218-49f421fb7959 h1:RJhm5l6Fo4rmEIcndxDllNhhf/fAx8qIm4t6A7vpm2A= +golang.org/x/telemetry v0.0.0-20260708182218-49f421fb7959/go.mod h1:LV7u5Oco+Z/g6XI7PqN+EUUUGGkEcmB1uj2ceI0fOVg= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= -golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= -golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= -golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= -golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= +golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= +golang.org/x/tools v0.48.0/go.mod h1:08xX0orndb/F7jJxGDicx061tyd5pcMto75YMAXr6lk= google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= diff --git a/model/scrobble.go b/model/scrobble.go index e1567abc3..a8022fc16 100644 --- a/model/scrobble.go +++ b/model/scrobble.go @@ -3,11 +3,17 @@ package model import "time" type Scrobble struct { - MediaFileID string - UserID string - SubmissionTime time.Time + ID int64 `structs:"id" json:"id"` + MediaFileID string `structs:"media_file_id" json:"mediaFileId"` + UserID string `json:"-"` + SubmissionTime int64 `structs:"submission_time" json:"submissionTime"` } type ScrobbleRepository interface { + CountAll(options ...QueryOptions) (int64, error) + Get(id string) (*Scrobble, error) + GetAll(options ...QueryOptions) (Scrobbles, error) RecordScrobble(mediaFileID string, submissionTime time.Time) error } + +type Scrobbles []Scrobble diff --git a/persistence/library_repository.go b/persistence/library_repository.go index 3789a71c9..5a0142423 100644 --- a/persistence/library_repository.go +++ b/persistence/library_repository.go @@ -173,15 +173,6 @@ func (r *libraryRepository) ScanEnd(id int) error { Set("last_scan_started_at", time.Time{}). Where(Eq{"id": id}) _, err := r.executeSQL(sq) - if err != nil { - return err - } - // https://www.sqlite.org/pragma.html#pragma_optimize - // Use mask 0x10000 to check table sizes without running ANALYZE - // Running ANALYZE can cause query planner issues with expression-based collation indexes - if conf.Server.DevOptimizeDB { - _, err = r.executeSQL(Expr("PRAGMA optimize=0x10000;")) - } return err } diff --git a/persistence/persistence.go b/persistence/persistence.go index 83211bdd5..1164eb70f 100644 --- a/persistence/persistence.go +++ b/persistence/persistence.go @@ -123,6 +123,8 @@ func (s *SQLStore) Resource(ctx context.Context, m any) model.ResourceRepository return s.Tag(ctx).(model.ResourceRepository) case model.Plugin: return s.Plugin(ctx).(model.ResourceRepository) + case model.Scrobble: + return s.Scrobble(ctx).(model.ResourceRepository) } log.Error("Resource not implemented", "model", reflect.TypeOf(m).Name()) return nil diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index abc5c4b6a..4f2fd7fe2 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -4,6 +4,7 @@ import ( "context" "path/filepath" "testing" + "time" "github.com/Masterminds/squirrel" _ "github.com/mattn/go-sqlite3" @@ -157,6 +158,13 @@ var ( testUsers = model.Users{adminUser, regularUser, thirdUser} ) +var ( + firstScrobble = model.Scrobble{ID: 1, MediaFileID: "1001", UserID: "userid", SubmissionTime: time.Date(1970, 1, 1, 0, 0, 0, 0, time.UTC).Unix()} + secondScrobble = model.Scrobble{ID: 2, MediaFileID: "1003", UserID: "2222", SubmissionTime: time.Date(1970, 2, 1, 0, 0, 0, 0, time.UTC).Unix()} + thirdScrobble = model.Scrobble{ID: 3, MediaFileID: "1002", UserID: "userid", SubmissionTime: time.Date(1970, 3, 1, 0, 0, 0, 0, time.UTC).Unix()} + scrobbles = model.Scrobbles{firstScrobble, secondScrobble, thirdScrobble} +) + func p(path string) string { return filepath.FromSlash(path) } @@ -304,6 +312,18 @@ var _ = BeforeSuite(func() { songComeTogether.Starred = true songComeTogether.StarredAt = mf.StarredAt testSongs[1] = songComeTogether + + scrobbleRepo := NewScrobbleRepository(ctx, conn).(*scrobbleRepository) + for _, s := range scrobbles { + _, err := scrobbleRepo.executeSQL(squirrel.Insert("scrobbles").SetMap(map[string]any{ + "media_file_id": s.MediaFileID, + "user_id": s.UserID, + "submission_time": s.SubmissionTime, + })) + if err != nil { + panic(err) + } + } }) func GetDBXBuilder() *dbx.DB { diff --git a/persistence/scrobble_repository.go b/persistence/scrobble_repository.go index 219a48198..7cc60ae23 100644 --- a/persistence/scrobble_repository.go +++ b/persistence/scrobble_repository.go @@ -5,6 +5,7 @@ import ( "time" . "github.com/Masterminds/squirrel" + "github.com/deluan/rest" "github.com/navidrome/navidrome/model" "github.com/pocketbase/dbx" ) @@ -13,11 +14,34 @@ type scrobbleRepository struct { sqlRepository } +func fromTs(_ string, value any) Sqlizer { + return GtOrEq{"scrobbles.submission_time": value} +} + +func toTs(_ string, value any) Sqlizer { + return LtOrEq{"scrobbles.submission_time": value} +} + +func (r *scrobbleRepository) baseQuery(options ...model.QueryOptions) SelectBuilder { + user := loggedUser(r.ctx) + + return r.newSelect(options...). + Columns("id", "media_file_id", "submission_time"). + Where(Eq{"scrobbles.user_id": user.ID}) +} + func NewScrobbleRepository(ctx context.Context, db dbx.Builder) model.ScrobbleRepository { r := &scrobbleRepository{} r.ctx = ctx r.db = db r.tableName = "scrobbles" + r.registerModel(&model.Scrobble{}, map[string]filterFunc{ + "from": fromTs, + "to": toTs, + }) + r.setSortMappings(map[string]string{ + "submission_time": "submission_time", + }) return r } @@ -32,3 +56,44 @@ func (r *scrobbleRepository) RecordScrobble(mediaFileID string, submissionTime t _, err := r.executeSQL(insert) return err } + +func (r *scrobbleRepository) CountAll(options ...model.QueryOptions) (int64, error) { + return r.count(r.baseQuery(), options...) +} + +func (r *scrobbleRepository) Count(options ...rest.QueryOptions) (int64, error) { + return r.CountAll(r.parseRestOptions(r.ctx, options...)) +} + +func (r *scrobbleRepository) Get(id string) (*model.Scrobble, error) { + sel := r.baseQuery().Where(Eq{"id": id}) + var res model.Scrobble + err := r.queryOne(sel, &res) + return &res, err +} + +func (r *scrobbleRepository) GetAll(options ...model.QueryOptions) (model.Scrobbles, error) { + sel := r.baseQuery(options...) + var scrobbles model.Scrobbles + err := r.queryAll(sel, &scrobbles) + return scrobbles, err +} + +func (r *scrobbleRepository) Read(id string) (any, error) { + return r.Get(id) +} + +func (r *scrobbleRepository) ReadAll(options ...rest.QueryOptions) (any, error) { + return r.GetAll(r.parseRestOptions(r.ctx, options...)) +} + +func (r *scrobbleRepository) EntityName() string { + return "scrobble" +} + +func (r *scrobbleRepository) NewInstance() any { + return &model.Scrobble{} +} + +var _ model.ScrobbleRepository = (*scrobbleRepository)(nil) +var _ model.ResourceRepository = (*scrobbleRepository)(nil) diff --git a/persistence/scrobble_repository_test.go b/persistence/scrobble_repository_test.go index d43848d03..e9103b127 100644 --- a/persistence/scrobble_repository_test.go +++ b/persistence/scrobble_repository_test.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" @@ -15,32 +16,33 @@ import ( var _ = Describe("ScrobbleRepository", func() { var repo model.ScrobbleRepository - var rawRepo sqlRepository var ctx context.Context - var fileID string - var userID string - - BeforeEach(func() { - fileID = id.NewRandom() - userID = id.NewRandom() - ctx = request.WithUser(log.NewContext(GinkgoT().Context()), model.User{ID: userID, UserName: "johndoe", IsAdmin: true}) - db := GetDBXBuilder() - repo = NewScrobbleRepository(ctx, db) - - rawRepo = sqlRepository{ - ctx: ctx, - tableName: "scrobbles", - db: db, - } - }) - - AfterEach(func() { - _, _ = rawRepo.db.Delete("scrobbles", dbx.HashExp{"media_file_id": fileID}).Execute() - _, _ = rawRepo.db.Delete("media_file", dbx.HashExp{"id": fileID}).Execute() - _, _ = rawRepo.db.Delete("user", dbx.HashExp{"id": userID}).Execute() - }) Describe("RecordScrobble", func() { + var fileID string + var userID string + var rawRepo sqlRepository + + BeforeEach(func() { + fileID = id.NewRandom() + userID = id.NewRandom() + ctx = request.WithUser(log.NewContext(GinkgoT().Context()), model.User{ID: userID, UserName: "johndoe", IsAdmin: true}) + db := GetDBXBuilder() + repo = NewScrobbleRepository(ctx, db) + + rawRepo = sqlRepository{ + ctx: ctx, + tableName: "scrobbles", + db: db, + } + }) + + AfterEach(func() { + _, _ = rawRepo.db.Delete("scrobbles", dbx.HashExp{"media_file_id": fileID}).Execute() + _, _ = rawRepo.db.Delete("media_file", dbx.HashExp{"id": fileID}).Execute() + _, _ = rawRepo.db.Delete("user", dbx.HashExp{"id": userID}).Execute() + }) + It("records a scrobble event", func() { submissionTime := time.Now().UTC() @@ -81,4 +83,137 @@ var _ = Describe("ScrobbleRepository", func() { Expect(scrobble.SubmissionTime).To(Equal(submissionTime.Unix())) }) }) + + Context("admin user (id userid)", func() { + BeforeEach(func() { + ctx = request.WithUser(log.NewContext(context.TODO()), adminUser) + repo = NewScrobbleRepository(ctx, GetDBXBuilder()) + }) + + Describe("Count", func() { + It("Returns the number of scrobbles in the DB for admin user", func() { + Expect(repo.CountAll()).To(Equal(int64(2))) + }) + + It("returns scrobbles in a range", func() { + Expect(repo.CountAll(model.QueryOptions{Filters: squirrel.LtOrEq{"submission_time": 1}})).To(Equal(int64(1))) + }) + }) + + Describe("Get", func() { + It("returns an existing scrobble for the user", func() { + scrobble, err := repo.Get("1") + Expect(err).To(BeNil()) + Expect(scrobble.ID).To(Equal(int64(1))) + Expect(scrobble.MediaFileID).To(Equal("1001")) + Expect(scrobble.SubmissionTime).To(Equal(firstScrobble.SubmissionTime)) + + }) + + It("does not return a scrobble that exists for another user", func() { + _, err := repo.Get("2") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + + It("does not return a scrobble that does not exist", func() { + _, err := repo.Get("444") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + }) + + Describe("GetAll", func() { + It("returns all scrobbles in reverse order", func() { + scrobbles, err := repo.GetAll(model.QueryOptions{ + Sort: "submission_time", + Order: "DESC", + }) + Expect(err).To(BeNil()) + Expect(scrobbles).To(HaveLen(2)) + + Expect(scrobbles[0].ID).To(Equal(int64(3))) + Expect(scrobbles[0].MediaFileID).To(Equal("1002")) + Expect(scrobbles[0].SubmissionTime).To(Equal(thirdScrobble.SubmissionTime)) + + Expect(scrobbles[1].ID).To(Equal(int64(1))) + Expect(scrobbles[1].MediaFileID).To(Equal("1001")) + Expect(scrobbles[1].SubmissionTime).To(Equal(firstScrobble.SubmissionTime)) + }) + + It("returns scrobbles in a range", func() { + scrobbles, err := repo.GetAll(model.QueryOptions{ + Filters: squirrel.GtOrEq{"submission_time": 1}}) + + Expect(err).To(BeNil()) + Expect(scrobbles).To(HaveLen(1)) + + Expect(scrobbles[0].ID).To(Equal(int64(3))) + Expect(scrobbles[0].MediaFileID).To(Equal("1002")) + Expect(scrobbles[0].SubmissionTime).To(Equal(thirdScrobble.SubmissionTime)) + }) + }) + }) + + Context("non-admin user", func() { + BeforeEach(func() { + ctx = request.WithUser(log.NewContext(context.TODO()), regularUser) + repo = NewScrobbleRepository(ctx, GetDBXBuilder()) + }) + + Describe("Count", func() { + It("Returns the number of scrobbles in the DB for admin user", func() { + Expect(repo.CountAll()).To(Equal(int64(1))) + }) + + It("returns scrobbles in a range", func() { + Expect(repo.CountAll(model.QueryOptions{Filters: squirrel.LtOrEq{"submission_time": 1}})).To(Equal(int64(0))) + }) + }) + + Describe("Get", func() { + It("returns an existing scrobble for the user", func() { + scrobble, err := repo.Get("2") + Expect(err).To(BeNil()) + Expect(scrobble.ID).To(Equal(int64(2))) + Expect(scrobble.MediaFileID).To(Equal("1003")) + Expect(scrobble.SubmissionTime).To(Equal(secondScrobble.SubmissionTime)) + }) + + It("does not return a scrobble that exists for another user", func() { + _, err := repo.Get("1") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + + It("does not return a scrobble that does not exist", func() { + _, err := repo.Get("444") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + }) + + Describe("GetAll", func() { + It("returns all scrobbles in reverse order", func() { + scrobbles, err := repo.GetAll(model.QueryOptions{ + Sort: "submission_time", + Order: "DESC", + }) + Expect(err).To(BeNil()) + Expect(scrobbles).To(HaveLen(1)) + + Expect(scrobbles[0].ID).To(Equal(int64(2))) + Expect(scrobbles[0].MediaFileID).To(Equal("1003")) + Expect(scrobbles[0].SubmissionTime).To(Equal(secondScrobble.SubmissionTime)) + }) + + It("returns scrobbles in a range", func() { + scrobbles, err := repo.GetAll(model.QueryOptions{ + Filters: squirrel.GtOrEq{"submission_time": 1}}) + + Expect(err).To(BeNil()) + Expect(scrobbles).To(HaveLen(1)) + + Expect(scrobbles[0].ID).To(Equal(int64(2))) + Expect(scrobbles[0].MediaFileID).To(Equal("1003")) + Expect(scrobbles[0].SubmissionTime).To(Equal(secondScrobble.SubmissionTime)) + }) + }) + }) }) diff --git a/scanner/controller.go b/scanner/controller.go index 175b92e26..463718ba3 100644 --- a/scanner/controller.go +++ b/scanner/controller.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "slices" + "sync" "sync/atomic" "time" @@ -13,6 +15,7 @@ import ( "github.com/navidrome/navidrome/core/auth" "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/core/playlists" + "github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" @@ -211,6 +214,16 @@ func (s *controller) ScanFolders(requestCtx context.Context, fullScan bool, targ ctx := request.AddValues(s.rootCtx, requestCtx) ctx = auth.WithAdminUser(ctx, s.ds) + // A quick scan is promoted to a full one when it resumes an interrupted full scan; that happens + // inside the scanner (possibly in a subprocess), so mirror it here for the analysis gate. Must + // be read before the scan: ScanEnd clears the flag. + effectiveFullScan := EffectiveFullScan(ctx, s.ds, fullScan, targets) + if effectiveFullScan || s.includesUnscannedLibrary(ctx, targets) { + if err := db.MarkOptimizePending(ctx); err != nil { + log.Error(ctx, "Scanner: Error marking DB analysis pending", err) + } + } + // Send the initial scan status event s.sendMessage(ctx, &events.ScanStatus{Scanning: true, Count: 0, FolderCount: 0}) progress := make(chan *ProgressInfo, 100) @@ -229,6 +242,15 @@ func (s *controller) ScanFolders(requestCtx context.Context, fullScan bool, targ if scanError != nil { _ = s.ds.Property(ctx).Put(consts.LastScanErrorKey, scanError.Error()) } + // Refresh the query-planner statistics after a successful full scan. This must run in the + // server process: with the external scanner, an ANALYZE in the subprocess is invisible to the + // server's pooled connections; their shared schema cache keeps the old statistics until the + // process restarts. + if effectiveFullScan && scanError == nil { + if err := db.Optimize(ctx); err != nil { + log.Error(ctx, "Scanner: Error analyzing DB", err) + } + } // If changes were detected, send a refresh event to all clients if s.changesDetected { log.Debug(ctx, "Library changes imported. Sending refresh event") @@ -255,18 +277,73 @@ func (s *controller) ScanFolders(requestCtx context.Context, fullScan bool, targ // This is a global variable that is used to prevent multiple scans from running at the same time. // "There can be only one" - https://youtu.be/sqcLjcSloXs?si=VlsjEOjTJZ68zIyg -var running atomic.Bool +var ( + running atomic.Bool + scanMaintenanceMux sync.Mutex +) func lockScan(ctx context.Context) (func(), error) { if !running.CompareAndSwap(false, true) { log.Debug(ctx, "Scanner already running, ignoring request") return func() {}, ErrAlreadyScanning } + scanMaintenanceMux.Lock() return func() { + scanMaintenanceMux.Unlock() running.Store(false) }, nil } +// LockForMaintenance prevents a scan from starting while database maintenance is running. +func LockForMaintenance() (func(), bool) { + if !scanMaintenanceMux.TryLock() { + return func() {}, false + } + if running.Load() { + scanMaintenanceMux.Unlock() + return func() {}, false + } + return scanMaintenanceMux.Unlock, true +} + +// EffectiveFullScan reports whether a scan was requested as full or will resume an interrupted +// full scan in one of the included libraries. +func EffectiveFullScan(ctx context.Context, ds model.DataStore, fullScan bool, targets []model.ScanTarget) bool { + if fullScan { + return true + } + return anyIncludedLibrary(ctx, ds, targets, func(library model.Library) bool { + return library.FullScanInProgress + }) +} + +func (s *controller) includesUnscannedLibrary(ctx context.Context, targets []model.ScanTarget) bool { + return anyIncludedLibrary(ctx, s.ds, targets, func(library model.Library) bool { + return library.LastScanAt.IsZero() + }) +} + +// anyIncludedLibrary reports whether any library included in the scan (all of them when targets is +// empty) matches pred. +func anyIncludedLibrary(ctx context.Context, ds model.DataStore, targets []model.ScanTarget, pred func(model.Library) bool) bool { + libraries, err := ds.Library(ctx).GetAll() + if err != nil { + return false + } + if len(targets) == 0 { + return slices.ContainsFunc(libraries, pred) + } + + targeted := make(map[int]struct{}, len(targets)) + for _, target := range targets { + targeted[target.LibraryID] = struct{}{} + } + return slices.ContainsFunc(libraries, func(library model.Library) bool { + _, ok := targeted[library.ID] + return ok && pred(library) + }) +} + func (s *controller) trackProgress(ctx context.Context, progress <-chan *ProgressInfo) ([]string, error) { s.count.Store(0) s.folderCount.Store(0) diff --git a/scanner/controller_test.go b/scanner/controller_test.go index d60d432b4..e4814da64 100644 --- a/scanner/controller_test.go +++ b/scanner/controller_test.go @@ -55,3 +55,41 @@ var _ = Describe("Controller", func() { }) }) }) + +var _ = Describe("LockForMaintenance", func() { + It("allows only one database maintenance operation at a time", func() { + release, ok := scanner.LockForMaintenance() + Expect(ok).To(BeTrue()) + DeferCleanup(release) + + _, ok = scanner.LockForMaintenance() + Expect(ok).To(BeFalse()) + }) +}) + +var _ = Describe("EffectiveFullScan", func() { + var ds *tests.MockDataStore + + BeforeEach(func() { + libraries := &tests.MockLibraryRepo{} + libraries.SetData(model.Libraries{ + {ID: 1, FullScanInProgress: true}, + {ID: 2}, + }) + ds = &tests.MockDataStore{MockedLibrary: libraries} + }) + + It("detects an interrupted full scan in a targeted library", func() { + targets := []model.ScanTarget{{LibraryID: 1, FolderPath: "."}} + Expect(scanner.EffectiveFullScan(context.Background(), ds, false, targets)).To(BeTrue()) + }) + + It("detects an interrupted full scan when scanning all libraries", func() { + Expect(scanner.EffectiveFullScan(context.Background(), ds, false, nil)).To(BeTrue()) + }) + + It("ignores interrupted full scans in untargeted libraries", func() { + targets := []model.ScanTarget{{LibraryID: 2, FolderPath: "."}} + Expect(scanner.EffectiveFullScan(context.Background(), ds, false, targets)).To(BeFalse()) + }) +}) diff --git a/scanner/scanner.go b/scanner/scanner.go index 871b0c696..27e2b19d2 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -13,7 +13,6 @@ import ( "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/playlists" - "github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/run" @@ -161,9 +160,6 @@ func (s *scannerImpl) scanFolders(ctx context.Context, fullScan bool, targets [] // Update last_scan_completed_at for all libraries s.runUpdateLibraries(ctx, &state), - - // Optimize DB - s.runOptimize(ctx), ) if err != nil { log.Error(ctx, "Scanner: Finished with error", "duration", time.Since(startTime), err) @@ -280,15 +276,6 @@ func (s *scannerImpl) runRefreshStats(ctx context.Context, state *scanState) fun } } -func (s *scannerImpl) runOptimize(ctx context.Context) func() error { - return func() error { - start := time.Now() - db.Optimize(ctx) - log.Debug(ctx, "Scanner: Optimized DB", "elapsed", time.Since(start)) - return nil - } -} - func (s *scannerImpl) runUpdateLibraries(ctx context.Context, state *scanState) func() error { return func() error { start := time.Now() diff --git a/scanner/scanner_selective_test.go b/scanner/scanner_selective_test.go index 6c70eb268..17772bf9d 100644 --- a/scanner/scanner_selective_test.go +++ b/scanner/scanner_selective_test.go @@ -4,10 +4,12 @@ import ( "context" "path/filepath" "testing/fstest" + "time" "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/metrics" @@ -80,7 +82,7 @@ var _ = Describe("ScanFolders", Ordered, func() { rock := template(_t{"albumartist": "Rock Artist", "album": "Rock Album"}) jazz := template(_t{"albumartist": "Jazz Artist", "album": "Jazz Album"}) pop := template(_t{"albumartist": "Pop Artist", "album": "Pop Album"}) - createFS(fstest.MapFS{ + fsys = createFS(fstest.MapFS{ "rock/track1.mp3": rock(track(1, "Rock Track 1")), "rock/track2.mp3": rock(track(2, "Rock Track 2")), "rock/subdir/track3.mp3": rock(track(3, "Rock Track 3")), @@ -122,6 +124,38 @@ var _ = Describe("ScanFolders", Ordered, func() { // Verify files in the pop folder were NOT scanned Expect(paths).ToNot(ContainElement("pop/track6.mp3")) + Expect(ds.Property(ctx).Get(consts.DBAnalyzePendingKey)).To(Equal("1")) + }) + }) + + Describe("Planner statistics maintenance", func() { + It("does not mark routine quick-scan changes for immediate analysis", func() { + rock := template(_t{"albumartist": "Rock Artist", "album": "Rock Album"}) + fsys = createFS(fstest.MapFS{ + "rock/track1.mp3": rock(track(1, "Rock Track 1")), + }) + _, err := s.ScanAll(ctx, true) + Expect(err).ToNot(HaveOccurred()) + Expect(ds.Property(ctx).Get(consts.DBAnalyzePendingKey)).To(Equal("0")) + + fsys.Add("rock/track2.mp3", rock(track(2, "Rock Track 2")), time.Now().Add(time.Second)) + _, err = s.ScanAll(ctx, false) + Expect(err).ToNot(HaveOccurred()) + Expect(ds.Property(ctx).Get(consts.DBAnalyzePendingKey)).To(Equal("0")) + }) + + It("does not treat an interrupted scan in an untargeted library as a full scan", func() { + otherLib := model.Library{ID: 2, Name: "Other Library", Path: "fake:///other"} + Expect(ds.Library(ctx).Put(&otherLib)).To(Succeed()) + Expect(ds.Library(ctx).ScanBegin(lib.ID, true)).To(Succeed()) + + lastAnalyze := "2026-07-09T12:00:00Z" + Expect(ds.Property(ctx).Put(consts.LastDBAnalyzeAtKey, lastAnalyze)).To(Succeed()) + Expect(ds.Property(ctx).Put(consts.DBAnalyzePendingKey, "0")).To(Succeed()) + + _, err := s.ScanFolders(ctx, false, []model.ScanTarget{{LibraryID: otherLib.ID, FolderPath: "."}}) + Expect(err).ToNot(HaveOccurred()) + Expect(ds.Property(ctx).Get(consts.LastDBAnalyzeAtKey)).To(Equal(lastAnalyze)) }) }) diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 4859d5160..df05c062f 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -72,7 +72,8 @@ func (api *Router) routes() http.Handler { api.R(r, "/player", model.Player{}, true) api.R(r, "/transcoding", model.Transcoding{}, conf.Server.EnableTranscodingConfig) api.addRadioRoute(r) - api.R(r, "/tag", model.Tag{}, true) + api.R(r, "/tag", model.Tag{}, false) + api.R(r, "/scrobble", model.Scrobble{}, false) if conf.Server.EnableSharing { api.RX(r, "/share", api.share.NewRepository, true) } diff --git a/server/public/handle_shares.go b/server/public/handle_shares.go index 18bfcc01c..76f674483 100644 --- a/server/public/handle_shares.go +++ b/server/public/handle_shares.go @@ -97,6 +97,22 @@ func (pub *Router) mapShareToM3U(r *http.Request, s model.Share) *model.Share { return &s } +// encodeMediafileShare builds the signed token embedded in a public share link +// for a single track. +// +// NOTE ON JWT USAGE: This is deliberately NOT part of Navidrome's authentication. +// The token is a signed, opaque capability that identifies one shared track +// (plus its transcode format/bitrate and the parent share id). We use a JWT here +// (reusing the library we already have) because it is a simple way to get three +// properties for a public link: the embedded ids can't be enumerated by guessing, +// the signature +// makes the claims tamper-evident, and the self-contained exp lets us reject +// stale links without a DB lookup. It carries no user identity (no subject, no +// admin flag) and grants access to nothing beyond the share it belongs to; the +// stream handler still verifies the share exists, is unexpired, and that the +// track is actually a member of it. An attacker who can forge these tokens +// necessarily already holds the signing secret, which also signs real user +// sessions, so that scenario is out of scope for the share boundary specifically. func encodeMediafileShare(s model.Share, id string) string { claims := auth.Claims{ ID: id, diff --git a/server/public/handle_streams.go b/server/public/handle_streams.go index 8fc407e9e..15abab693 100644 --- a/server/public/handle_streams.go +++ b/server/public/handle_streams.go @@ -3,6 +3,7 @@ package public import ( "errors" "net/http" + "slices" "strconv" "time" @@ -25,23 +26,20 @@ func (pub *Router) handleStream(w http.ResponseWriter, r *http.Request) { return } - var shareOwner *model.User - if info.shareID != "" { - share, err := pub.ds.Share(ctx).Get(info.shareID) - if err != nil { - checkShareError(ctx, w, err, info.shareID) - return - } - if expiresAt := V(share.ExpiresAt); !expiresAt.IsZero() && expiresAt.Before(time.Now()) { - checkShareError(ctx, w, model.ErrExpired, info.shareID) - return - } - shareOwner, err = pub.ds.User(ctx).Get(share.UserID) - if err != nil { - log.Error(ctx, "Error retrieving share owner for shared stream", "share", info.shareID, "owner", share.UserID, err) - http.Error(w, "internal error", http.StatusInternalServerError) - return - } + share, err := pub.ds.Share(ctx).Get(info.shareID) + if err != nil { + checkShareError(ctx, w, err, info.shareID) + return + } + if expiresAt := V(share.ExpiresAt); !expiresAt.IsZero() && expiresAt.Before(time.Now()) { + checkShareError(ctx, w, model.ErrExpired, info.shareID) + return + } + shareOwner, err := pub.ds.User(ctx).Get(share.UserID) + if err != nil { + log.Error(ctx, "Error retrieving share owner for shared stream", "share", info.shareID, "owner", share.UserID, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return } mf, err := pub.ds.MediaFile(ctx).Get(info.id) @@ -56,7 +54,8 @@ func (pub *Router) handleStream(w http.ResponseWriter, r *http.Request) { } // 404 rather than 403 so the response doesn't reveal whether the id exists. - if shareOwner != nil && !shareOwner.HasLibraryAccess(mf.LibraryID) { + // The track must belong to the share AND be within the owner's libraries. + if !shareContainsTrack(share, mf.ID) || !shareOwner.HasLibraryAccess(mf.LibraryID) { http.Error(w, "not found", http.StatusNotFound) return } @@ -98,6 +97,15 @@ type shareTrackInfo struct { shareID string } +func shareContainsTrack(share *model.Share, mediaFileID string) bool { + return slices.ContainsFunc(share.Tracks, func(mf model.MediaFile) bool { + return mf.ID == mediaFileID + }) +} + +// decodeStreamInfo decodes the signed share-link token. This is a scoped +// public-share capability, not an auth credential; see encodeMediafileShare for +// why a JWT is used here. func decodeStreamInfo(tokenString string) (shareTrackInfo, error) { c, err := auth.Validate(tokenString) if err != nil { @@ -106,6 +114,9 @@ func decodeStreamInfo(tokenString string) (shareTrackInfo, error) { if c.ID == "" { return shareTrackInfo{}, errors.New("required claim \"id\" not found") } + if c.ShareID == "" { + return shareTrackInfo{}, errors.New("required claim \"sid\" not found") + } return shareTrackInfo{ id: c.ID, format: c.Format, diff --git a/server/public/handle_streams_test.go b/server/public/handle_streams_test.go index 6fa083045..2f32ea6f2 100644 --- a/server/public/handle_streams_test.go +++ b/server/public/handle_streams_test.go @@ -71,14 +71,11 @@ var _ = Describe("decodeStreamInfo", func() { Expect(err).To(HaveOccurred()) }) - It("handles tokens without shareID (backward compat)", func() { + It("rejects a token without a shareID claim", func() { claims := auth.Claims{ID: "mf-123", Format: "opus"} token, _ := auth.CreatePublicToken(claims) - info, err := decodeStreamInfo(token) - Expect(err).NotTo(HaveOccurred()) - Expect(info.id).To(Equal("mf-123")) - Expect(info.format).To(Equal("opus")) - Expect(info.shareID).To(BeEmpty()) + _, err := decodeStreamInfo(token) + Expect(err).To(HaveOccurred()) }) }) @@ -133,7 +130,7 @@ var _ = Describe("handleStream", func() { shareOwnedBy := func(owner model.User, mf model.MediaFile) { shareRepo.ID = "share123" - shareRepo.Entity = &model.Share{ID: "share123", UserID: owner.ID} + shareRepo.Entity = &model.Share{ID: "share123", UserID: owner.ID, Tracks: model.MediaFiles{mf}} userRepo := tests.CreateMockUserRepo() Expect(userRepo.Put(&owner)).To(Succeed()) ds.MockedUser = userRepo @@ -171,6 +168,25 @@ var _ = Describe("handleStream", func() { Expect(streamer.called).To(BeFalse()) }) + It("returns 404 when the track is not a member of the share", func() { + owner := model.User{ID: "owner1", UserName: "owner1", IsAdmin: true} + userRepo := tests.CreateMockUserRepo() + Expect(userRepo.Put(&owner)).To(Succeed()) + ds.MockedUser = userRepo + mfRepo := tests.CreateMockMediaFileRepo() + mfRepo.SetData(model.MediaFiles{{ID: "mf-shared"}, {ID: "mf-other"}}) + ds.MockedMediaFile = mfRepo + shareRepo.ID = "share123" + shareRepo.Entity = &model.Share{ID: "share123", UserID: owner.ID, Tracks: model.MediaFiles{{ID: "mf-shared"}}} + + claims := auth.Claims{ID: "mf-other", ShareID: "share123"} + token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims) + w := makeRequest(token) + + Expect(w.Code).To(Equal(http.StatusNotFound)) + Expect(streamer.called).To(BeFalse()) + }) + It("streams a track inside the share owner's libraries", func() { shareOwnedBy( model.User{ID: "owner1", UserName: "owner1", Libraries: model.Libraries{{ID: 1}}}, @@ -217,12 +233,12 @@ var _ = Describe("handleStream", func() { Expect(w.Code).To(Equal(http.StatusInternalServerError)) }) - It("skips share check for tokens without shareID (backward compat)", func() { + It("returns 400 for tokens without a shareID", func() { claims := auth.Claims{ID: "mf-123"} token, _ := auth.CreatePublicToken(claims) w := makeRequest(token) - // Should get past share check, then fail on media file lookup (no mock data) - Expect(w.Code).To(Equal(http.StatusNotFound)) + Expect(w.Code).To(Equal(http.StatusBadRequest)) + Expect(streamer.called).To(BeFalse()) }) It("returns 400 for an invalid token", func() { diff --git a/tests/mock_scrobble_repo.go b/tests/mock_scrobble_repo.go index 34561c257..d6d88d221 100644 --- a/tests/mock_scrobble_repo.go +++ b/tests/mock_scrobble_repo.go @@ -2,6 +2,7 @@ package tests import ( "context" + "strconv" "time" "github.com/navidrome/navidrome/model" @@ -13,12 +14,32 @@ type MockScrobbleRepo struct { ctx context.Context } +func (m *MockScrobbleRepo) Get(id string) (*model.Scrobble, error) { + for idx := range m.RecordedScrobbles { + if strconv.FormatInt(m.RecordedScrobbles[idx].ID, 10) == id { + return &m.RecordedScrobbles[idx], nil + } + } + + return nil, model.ErrNotFound +} + +func (m *MockScrobbleRepo) GetAll(options ...model.QueryOptions) (model.Scrobbles, error) { + return m.RecordedScrobbles, nil +} + +func (m *MockScrobbleRepo) CountAll(options ...model.QueryOptions) (int64, error) { + return int64(len(m.RecordedScrobbles)), nil +} + func (m *MockScrobbleRepo) RecordScrobble(fileID string, submissionTime time.Time) error { user, _ := request.UserFrom(m.ctx) m.RecordedScrobbles = append(m.RecordedScrobbles, model.Scrobble{ MediaFileID: fileID, UserID: user.ID, - SubmissionTime: submissionTime, + SubmissionTime: submissionTime.Unix(), }) return nil } + +var _ model.ScrobbleRepository = (*MockScrobbleRepo)(nil)