2025-05-21 09:30:23 -04:00
|
|
|
package scanner_test
|
|
|
|
|
|
|
|
|
|
import (
|
|
|
|
|
"context"
|
|
|
|
|
|
|
|
|
|
"github.com/navidrome/navidrome/conf/configtest"
|
|
|
|
|
"github.com/navidrome/navidrome/consts"
|
2026-03-15 22:19:55 -04:00
|
|
|
"github.com/navidrome/navidrome/core"
|
2025-05-21 09:30:23 -04:00
|
|
|
"github.com/navidrome/navidrome/core/artwork"
|
|
|
|
|
"github.com/navidrome/navidrome/core/metrics"
|
refactor: move playlist business logic from repositories to service layer (#5027)
* refactor: move playlist business logic from repositories to core.Playlists service
Move authorization, permission checks, and orchestration logic from
playlist repositories to the core.Playlists service, following the
existing pattern used by core.Share and core.Library.
Changes:
- Expand core.Playlists interface with read, mutation, track management,
and REST adapter methods
- Add playlistRepositoryWrapper for REST Save/Update/Delete with
permission checks (follows Share/Library pattern)
- Simplify persistence/playlist_repository.go: remove isWritable(),
auth checks from Delete()/Put()/updatePlaylist()
- Simplify persistence/playlist_track_repository.go: remove
isTracksEditable() and permission checks from Add/Delete/Reorder
- Update Subsonic API handlers to route through service
- Update Native API handlers to accept core.Playlists instead of
model.DataStore
* test: add coverage for playlist service methods and REST wrapper
Add 30 new tests covering the service methods added during the playlist
refactoring:
- Delete: owner, admin, denied, not found
- Create: new playlist, replace tracks, admin bypass, denied, not found
- AddTracks: owner, admin, denied, smart playlist, not found
- RemoveTracks: owner, smart playlist denied, non-owner denied
- ReorderTrack: owner, smart playlist denied
- NewRepository wrapper: Save (owner assignment, ID clearing),
Update (owner, admin, denied, ownership change, not found),
Delete (delegation with permission checks)
Expand mockedPlaylistRepo with Get, Delete, Tracks, GetWithTracks, and
rest.Persistable methods. Add mockedPlaylistTrackRepo for track
operation verification.
* fix: add authorization check to playlist Update method
Added ownership verification to the Subsonic Update endpoint in the
playlist service layer. The authorization check was present in the old
repository code but was not carried over during the refactoring to the
service layer, allowing any authenticated user to modify playlists they
don't own via the Subsonic API. Also added corresponding tests for the
Update method's permission logic.
* refactor: improve playlist permission checks and error handling, add e2e tests
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: rename core.Playlists to playlists package and update references
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: rename playlists_internal_test.go to parse_m3u_test.go and update tests; add new parse_nsp.go and rest_adapter.go files
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: block track mutations on smart playlists in Create and Update
Create now rejects replacing tracks on smart playlists (pre-existing
gap). Update now uses checkTracksEditable instead of checkWritable
when track changes are requested, restoring the protection that was
removed from the repository layer during the refactoring. Metadata-only
updates on smart playlists remain allowed.
* test: add smart playlist protection tests to ensure readonly behavior and mutation restrictions
* refactor: optimize track removal and renumbering in playlists
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: implement track reordering in playlists with SQL updates
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: wrap track deletion and reordering in transactions for consistency
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: remove unused getTracks method from playlistTrackRepository
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: optimize playlist track renumbering with CTE-based UPDATE
Replace the DELETE + re-INSERT renumbering strategy with a two-step
UPDATE approach using a materialized CTE and ROW_NUMBER() window
function. The previous approach (SELECT all IDs, DELETE all tracks,
re-INSERT in chunks of 200) required 13 SQL operations for a 2000-track
playlist. The new approach uses just 2 UPDATEs: first negating all IDs
to clear the positive space, then assigning sequential positions via
UPDATE...FROM with a CTE. This avoids the UNIQUE constraint violations
that affected the original correlated subquery while reducing per-delete
request time from ~110ms to ~12ms on a 2000-track playlist.
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: rename New function to NewPlaylists for clarity
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: update mock playlist repository and tests for consistency
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-02-21 19:57:13 -05:00
|
|
|
"github.com/navidrome/navidrome/core/playlists"
|
2025-05-21 09:30:23 -04:00
|
|
|
"github.com/navidrome/navidrome/db"
|
2025-11-14 22:15:43 -05:00
|
|
|
"github.com/navidrome/navidrome/model"
|
2025-05-21 09:30:23 -04:00
|
|
|
"github.com/navidrome/navidrome/persistence"
|
|
|
|
|
"github.com/navidrome/navidrome/scanner"
|
|
|
|
|
"github.com/navidrome/navidrome/server/events"
|
|
|
|
|
"github.com/navidrome/navidrome/tests"
|
|
|
|
|
. "github.com/onsi/ginkgo/v2"
|
|
|
|
|
. "github.com/onsi/gomega"
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
var _ = Describe("Controller", func() {
|
|
|
|
|
var ctx context.Context
|
|
|
|
|
var ds *tests.MockDataStore
|
2025-11-14 22:15:43 -05:00
|
|
|
var ctrl model.Scanner
|
2025-05-21 09:30:23 -04:00
|
|
|
|
|
|
|
|
Describe("Status", func() {
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
ctx = context.Background()
|
|
|
|
|
db.Init(ctx)
|
|
|
|
|
DeferCleanup(func() { Expect(tests.ClearDB()).To(Succeed()) })
|
|
|
|
|
DeferCleanup(configtest.SetupConfig())
|
|
|
|
|
ds = &tests.MockDataStore{RealDS: persistence.New(db.Db())}
|
|
|
|
|
ds.MockedProperty = &tests.MockedPropertyRepo{}
|
2026-03-15 22:19:55 -04:00
|
|
|
ctrl = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance())
|
2025-05-21 09:30:23 -04:00
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("includes last scan error", func() {
|
|
|
|
|
Expect(ds.Property(ctx).Put(consts.LastScanErrorKey, "boom")).To(Succeed())
|
|
|
|
|
status, err := ctrl.Status(ctx)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(status.LastError).To(Equal("boom"))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("includes scan type and error in status", func() {
|
|
|
|
|
// Set up test data in property repo
|
|
|
|
|
Expect(ds.Property(ctx).Put(consts.LastScanErrorKey, "test error")).To(Succeed())
|
|
|
|
|
Expect(ds.Property(ctx).Put(consts.LastScanTypeKey, "full")).To(Succeed())
|
|
|
|
|
|
|
|
|
|
// Get status and verify basic info
|
|
|
|
|
status, err := ctrl.Status(ctx)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(status.LastError).To(Equal("test error"))
|
|
|
|
|
Expect(status.ScanType).To(Equal("full"))
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
})
|
perf(db): keep query planner statistics trustworthy with full ANALYZE (#5740)
* 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
2026-07-13 12:04:29 -04:00
|
|
|
|
|
|
|
|
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())
|
|
|
|
|
})
|
|
|
|
|
})
|