remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-10 03:17:27 +02:00
Code Issues Projects Releases Packages Wiki Activity
navidrome/scanner/phase_2_missing_tracks_test.go
Deluan Quintão ee6dd1bc03
 fix(scanner): stop DB lock starvation during scans on slow storage (#6201) 
* fix(artwork): pause the artwork worker while a scan is running

The artwork worker added in 0.64 writes to the database continuously, including while a scan runs. On slow storage the scanner holds the write lock for many seconds per folder, so the two writers keep timing each other out: artwork writes fail with "database is locked", and a single busy timeout on the scanner side aborts the whole scan. The worker now stops dispatching queue items while scanner.IsScanning reports true, including mid-batch, and resumes on the next poll after the scan ends. Artwork requests are unaffected, since they serve local art without the worker.

* fix(db): run ANALYZE one index at a time so writers are not starved

A full ANALYZE is a single write transaction, so every other write waits for it to finish and fails after the 15s busy timeout. On slow NAS storage it was measured taking over 26 minutes. The analysis now runs ANALYZE per index (per table for unindexed and WITHOUT ROWID tables), which produces the same sqlite_stat1 rows as a full ANALYZE, and pauses briefly between steps (up to 150ms, just above SQLite's longest busy-handler sleep) so waiting writers get the lock.

* fix(scanner): ignore Synology @eaDir metadata folders

Synology creates an @eaDir folder next to media files, holding one subfolder per file with generated thumbnails. The scanner and watcher treated them as regular folders, which on one reported library added tens of thousands of extra folders to every scan.

* fix(db): analyze tables with only partial indexes as a whole

A partial index does not record the table's row count, so a table whose only indexes are partial needs a table-level ANALYZE to get the sqlite_stat1 row a full ANALYZE would write. Navidrome's schema has no such table today, but the stepped analysis should match a full ANALYZE for any schema a future migration creates.

* fix(scanner): retry busy folder saves and stop phase 1 on a fatal error

On slow storage, a single SQLITE_BUSY while saving a folder aborted the whole scan, even when another writer held the lock only briefly. The folder save now runs as a retryable unit: on a busy error it waits (5s, 10s, 15s) and reruns the transaction, up to three times, before failing. Side effects that do not survive a rollback (the album ID map consumed by persistAlbum, the artwork queue items, the image-change record) are rebuilt per attempt or recorded only after a successful commit.

When a folder save does fail, phase 1 used to keep walking the library and reading tags for every remaining folder, discarding the results, before reporting the error; a reporter saw 40 silent minutes. The walk now stops as soon as the save fails, and the walker honors cancellation instead of blocking on its channel. Because an early stop leaves folders unvisited, phase 1 no longer marks unvisited folders missing when the phase failed; the resumed scan handles them.

* refactor(persistence): move busy retry into DataStore.WithTxRetry

The scanner retried its folder save itself, which meant it had to know SQLite error codes. WithTxRetry now owns that policy: it reruns the block in a fresh transaction on SQLITE_BUSY, up to three times with growing delays, and runs it only once when already inside a transaction, since the outer transaction would still hold the lock. The block receives the context to use, and attempts that will be retried carry a marker so a busy statement in them is logged as a warning; only the final attempt logs errors. The scanner's inner error logs are folded into wrapped errors, so a recovered retry no longer prints error-level lines, and the folder path travels in the log context.

* fix(persistence): join the enclosing transaction in a nested WithTxRetry

Called on a store that is already inside a transaction, WithTxRetry went through WithTx, which opens a second, independent transaction on another connection. That transaction waits on the lock the outer one holds and fails with SQLITE_BUSY, and if it does succeed the outer transaction cannot roll it back. It now runs the block on the enclosing transaction, which owns the lock, the commit and the rollback. Found by a Codex (gpt-6-sol) review.

* fix(scanner): retry the remaining scan writes on a busy database

Every write step after phase 1 still aborted the whole scan on a single SQLITE_BUSY: phase 1 finalize, phase 2 moves and purge, phase 3 album saves and play count refreshes, the deferred playlist import flag, library ScanBegin, GC, the missing-artwork enqueue, tag counts, and the final library update. They now go through WithTxRetry. The phase 2 move had to be made rerun-safe first: it changed the target track's ID inside the transaction, so a rerun would have deleted the moved track itself, and it marked album annotations as handled even when the transaction rolled back. It now works on a copy per attempt and records the annotation reassignment only after a commit.

Artist.RefreshStats is left alone: it updates artists in batches outside a transaction, and one transaction around all of them would hold the write lock for the whole refresh on slow storage. Phase 4 playlist imports go through the playlist service and are left for a follow-up.

* fix(scanner): claim the album before moving its annotations

The rerun-safe moveMatched checked processedAlbumAnnotations before its transaction and marked the album only after the commit. Phase 2 runs same-library and cross-library moves in separate pipeline stages, so two moves into one album could both pass the check; the second would reassign annotations again and overwrite the album's created_at. The album is now claimed under the lock before the transaction, as the old code effectively did, and the claim is released if the move fails so a later move can still reassign. Found by a Codex (gpt-6-sol) review.

* fix(artwork): keep artwork housekeeping from writing during scans

The artwork worker already pauses while a scan runs, but its housekeeping jobs did not: the hourly missing-artwork recheck (a bulk INSERT ... SELECT over albums and artists), the startup run of the same recheck, and the daily prune all kept competing with the scanner for the write lock. They now run through LockForMaintenance, like the scheduled DB analysis: they skip while a scan is running and keep a scan from starting until they finish. Skipping the recheck loses nothing, since each scan with changes queues missing artwork at its end.

* refactor(scanner): log retried step errors once, from the caller

Blocks passed to WithTxRetry still logged their own errors at error level on every attempt, so a busy error that a retry absorbed printed several error lines (GC printed three). They now return wrapped errors and the callers, which already log them, report the final outcome once. Also: drop a leftover variable in phase 1 finalize, check the walk context once, stop repeating the folder field that is already in the log context, stop shadowing finalize's err in phase 3, and format the WithTxRetry scope the same way as WithTx.

* test(scanner): make the scanner suite's temp DB cleanup best effort

Which DB file the process-wide DB handle opens depends on which spec touches it first. When the Scanner container wins the random order, its temp DB stays open until db.Close after RunSpecs, and on Windows removing the temp dir fails with 'being used by another process'. Ginkgo pins that on the container's last spec, which is now one of the busy-database specs. The sibling suites skip Windows for the same reason; this one now removes its temp dir on a best-effort basis instead, so it keeps running there.
2026-09-23 17:04:50 -04:00

1073 lines
36 KiB
Go
Raw Permalink Blame History

package scanner
import (
"context"
"errors"
"maps"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("phaseMissingTracks", func() {
var (
phase *phaseMissingTracks
ctx context.Context
ds model.DataStore
mr *tests.MockMediaFileRepo
lr *tests.MockLibraryRepo
state *scanState
)
BeforeEach(func() {
ctx = context.Background()
mr = tests.CreateMockMediaFileRepo()
lr = &tests.MockLibraryRepo{}
lr.SetData(model.Libraries{{ID: 1, LastScanStartedAt: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)}})
ds = &tests.MockDataStore{MockedMediaFile: mr, MockedLibrary: lr}
state = &scanState{
libraries: model.Libraries{{ID: 1, LastScanStartedAt: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)}},
totalLibraryCount: 1,
}
phase = createPhaseMissingTracks(ctx, state, ds)
})
Describe("produceMissingTracks", func() {
var (
put func(tracks *missingTracks)
produced []*missingTracks
)
BeforeEach(func() {
produced = nil
put = func(tracks *missingTracks) {
produced = append(produced, tracks)
}
})
When("there are no missing tracks", func() {
It("should not call put", func() {
mr.SetData(model.MediaFiles{
{ID: "1", PID: "A", Missing: false},
{ID: "2", PID: "A", Missing: false},
})
err := phase.produce(put)
Expect(err).ToNot(HaveOccurred())
Expect(produced).To(BeEmpty())
})
})
When("there are missing tracks", func() {
It("should call put for any missing tracks with corresponding matches", func() {
mr.SetData(model.MediaFiles{
{ID: "1", PID: "A", Missing: true, LibraryID: 1},
{ID: "2", PID: "B", Missing: true, LibraryID: 1},
{ID: "3", PID: "A", Missing: false, LibraryID: 1},
})
err := phase.produce(put)
Expect(err).ToNot(HaveOccurred())
Expect(produced).To(HaveLen(2))
// PID A should have both missing and matched tracks
var pidA *missingTracks
for _, p := range produced {
if p.pid == "A" {
pidA = p
break
}
}
Expect(pidA).ToNot(BeNil())
Expect(pidA.missing).To(HaveLen(1))
Expect(pidA.matched).To(HaveLen(1))
// PID B should have only missing tracks
var pidB *missingTracks
for _, p := range produced {
if p.pid == "B" {
pidB = p
break
}
}
Expect(pidB).ToNot(BeNil())
Expect(pidB.missing).To(HaveLen(1))
Expect(pidB.matched).To(HaveLen(0))
})
It("should call put for any missing tracks even without matches", func() {
mr.SetData(model.MediaFiles{
{ID: "1", PID: "A", Missing: true, LibraryID: 1},
{ID: "2", PID: "B", Missing: true, LibraryID: 1},
{ID: "3", PID: "C", Missing: false, LibraryID: 1},
})
err := phase.produce(put)
Expect(err).ToNot(HaveOccurred())
Expect(produced).To(HaveLen(2))
// Both PID A and PID B should be produced even without matches
var pidA, pidB *missingTracks
for _, p := range produced {
if p.pid == "A" {
pidA = p
} else if p.pid == "B" {
pidB = p
}
}
Expect(pidA).ToNot(BeNil())
Expect(pidA.missing).To(HaveLen(1))
Expect(pidA.matched).To(HaveLen(0))
Expect(pidB).ToNot(BeNil())
Expect(pidB.missing).To(HaveLen(1))
Expect(pidB.matched).To(HaveLen(0))
})
})
})
Describe("processMissingTracks", func() {
It("should move the matched track when the missing track is the exact same", func() {
missingTrack := model.MediaFile{ID: "1", PID: "A", Path: "dir1/path1.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
matchedTrack := model.MediaFile{ID: "2", PID: "A", Path: "dir2/path2.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
in := &missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matchedTrack},
}
_, err := phase.processMissingTracks(in)
Expect(err).ToNot(HaveOccurred())
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
movedTrack, _ := ds.MediaFile(ctx).Get("1")
Expect(movedTrack.Path).To(Equal(matchedTrack.Path))
})
Context("claiming the album annotation reassignment", func() {
var probe *probeTxDS
missingTrack := model.MediaFile{ID: "1", PID: "A", AlbumID: "old-album", Path: "dir1/path1.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
matchedTrack := model.MediaFile{ID: "2", PID: "A", AlbumID: "new-album", Path: "dir2/path2.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
BeforeEach(func() {
probe = &probeTxDS{MockDataStore: ds.(*tests.MockDataStore)}
probe.MockedAlbum = tests.CreateMockAlbumRepo()
phase = createPhaseMissingTracks(ctx, state, probe)
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
})
It("claims the target album before the transaction, so a concurrent move skips it", func() {
probe.during = func() {
phase.annotationMutex.RLock()
defer phase.annotationMutex.RUnlock()
Expect(phase.processedAlbumAnnotations).To(HaveKeyWithValue("new-album", true))
}
Expect(phase.moveMatched(matchedTrack, missingTrack)).To(Succeed())
})
It("releases the claim when the move fails, so a later move can reassign", func() {
probe.err = errors.New("boom")
Expect(phase.moveMatched(matchedTrack, missingTrack)).To(MatchError("boom"))
Expect(phase.processedAlbumAnnotations).ToNot(HaveKey("new-album"))
})
})
Context("when the move transaction is rerun after a busy rollback", func() {
var rerunDS *rerunTxDS
BeforeEach(func() {
rerunDS = &rerunTxDS{MockDataStore: ds.(*tests.MockDataStore)}
rerunDS.snapshot = func() func() {
saved := maps.Clone(mr.Data)
return func() { mr.Data = saved }
}
phase = createPhaseMissingTracks(ctx, state, rerunDS)
})
It("keeps the moved track", func() {
missingTrack := model.MediaFile{ID: "1", PID: "A", Path: "dir1/path1.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
matchedTrack := model.MediaFile{ID: "2", PID: "A", Path: "dir2/path2.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
_, err := phase.processMissingTracks(&missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matchedTrack},
})
Expect(err).ToNot(HaveOccurred())
movedTrack, err := ds.MediaFile(ctx).Get("1")
Expect(err).ToNot(HaveOccurred())
Expect(movedTrack.Path).To(Equal(matchedTrack.Path))
})
It("reassigns the album annotations in the attempt that commits", func() {
albumRepo := tests.CreateMockAlbumRepo()
rerunDS.MockedAlbum = albumRepo
restoreTracks := rerunDS.snapshot
rerunDS.snapshot = func() func() {
restore := restoreTracks()
return func() {
restore()
albumRepo.ReassignAnnotationCalls = nil
}
}
missingTrack := model.MediaFile{ID: "1", PID: "A", AlbumID: "old-album", Path: "dir1/path1.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
matchedTrack := model.MediaFile{ID: "2", PID: "A", AlbumID: "new-album", Path: "dir2/path2.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
_, err := phase.processMissingTracks(&missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matchedTrack},
})
Expect(err).ToNot(HaveOccurred())
Expect(albumRepo.ReassignAnnotationCalls).To(HaveKeyWithValue("old-album", "new-album"))
})
})
It("should move the matched track when the missing track has the same tags and filename", func() {
missingTrack := model.MediaFile{ID: "1", PID: "A", Path: "path1.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
matchedTrack := model.MediaFile{ID: "2", PID: "A", Path: "path1.flac", Tags: model.Tags{"title": []string{"title1"}}, Size: 200}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
in := &missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matchedTrack},
}
_, err := phase.processMissingTracks(in)
Expect(err).ToNot(HaveOccurred())
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
movedTrack, _ := ds.MediaFile(ctx).Get("1")
Expect(movedTrack.Path).To(Equal(matchedTrack.Path))
Expect(movedTrack.Size).To(Equal(matchedTrack.Size))
})
It("should move the matched track when there's only one missing track and one matched track (same PID)", func() {
missingTrack := model.MediaFile{ID: "1", PID: "A", Path: "dir1/path1.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
matchedTrack := model.MediaFile{ID: "2", PID: "A", Path: "dir2/path2.flac", Tags: model.Tags{"title": []string{"different title"}}, Size: 200}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
in := &missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matchedTrack},
}
_, err := phase.processMissingTracks(in)
Expect(err).ToNot(HaveOccurred())
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
movedTrack, _ := ds.MediaFile(ctx).Get("1")
Expect(movedTrack.Path).To(Equal(matchedTrack.Path))
Expect(movedTrack.Size).To(Equal(matchedTrack.Size))
})
It("should prioritize exact matches", func() {
missingTrack := model.MediaFile{ID: "1", PID: "A", Path: "dir1/file1.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
matchedEquivalent := model.MediaFile{ID: "2", PID: "A", Path: "dir1/file1.flac", Tags: model.Tags{"title": []string{"title1"}}, Size: 200}
matchedExact := model.MediaFile{ID: "3", PID: "A", Path: "dir2/file2.mp3", Tags: model.Tags{"title": []string{"title1"}}, Size: 100}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedEquivalent)
_ = ds.MediaFile(ctx).Put(&matchedExact)
in := &missingTracks{
missing: []model.MediaFile{missingTrack},
// Note that equivalent comes before the exact match
matched: []model.MediaFile{matchedEquivalent, matchedExact},
}
_, err := phase.processMissingTracks(in)
Expect(err).ToNot(HaveOccurred())
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
movedTrack, _ := ds.MediaFile(ctx).Get("1")
Expect(movedTrack.Path).To(Equal(matchedExact.Path))
Expect(movedTrack.Size).To(Equal(matchedExact.Size))
})
It("should not move anything if there's more than one match and they don't are not exact nor equivalent", func() {
missingTrack := model.MediaFile{ID: "1", PID: "A", Path: "dir1/file1.mp3", Title: "title1", Size: 100}
matched1 := model.MediaFile{ID: "2", PID: "A", Path: "dir1/file2.flac", Title: "another title", Size: 200}
matched2 := model.MediaFile{ID: "3", PID: "A", Path: "dir2/file3.mp3", Title: "different title", Size: 100}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matched1)
_ = ds.MediaFile(ctx).Put(&matched2)
in := &missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matched1, matched2},
}
_, err := phase.processMissingTracks(in)
Expect(err).ToNot(HaveOccurred())
Expect(phase.totalMatched.Load()).To(Equal(uint32(0)))
Expect(state.changesDetected.Load()).To(BeFalse())
// The missing track should still be the same
movedTrack, _ := ds.MediaFile(ctx).Get("1")
Expect(movedTrack.Path).To(Equal(missingTrack.Path))
Expect(movedTrack.Title).To(Equal(missingTrack.Title))
Expect(movedTrack.Size).To(Equal(missingTrack.Size))
})
It("should not match the same target to multiple missing tracks (prevents duplicate paths)", func() {
// Simulate a scenario where two missing tracks from different locations have the same
// base filename and match the same newly imported track via IsEquivalent.
// Without deduplication, both missing tracks would be "moved" to the same target,
// creating two non-missing records with the same path.
missingTrack1 := model.MediaFile{ID: "1", PID: "A", Path: "old_dir1/song.mp3", Title: "title1", Size: 100}
missingTrack2 := model.MediaFile{ID: "2", PID: "A", Path: "old_dir2/song.mp3", Title: "title1", Size: 100}
matchedTrack := model.MediaFile{ID: "3", PID: "A", Path: "new_dir/song.mp3", Title: "title1", Size: 200}
_ = ds.MediaFile(ctx).Put(&missingTrack1)
_ = ds.MediaFile(ctx).Put(&missingTrack2)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
in := &missingTracks{
missing: []model.MediaFile{missingTrack1, missingTrack2},
matched: []model.MediaFile{matchedTrack},
}
_, err := phase.processMissingTracks(in)
Expect(err).ToNot(HaveOccurred())
// Only one of the missing tracks should be matched
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
// The matched track should have been consumed by the first missing track
movedTrack, _ := ds.MediaFile(ctx).Get("1")
Expect(movedTrack.Path).To(Equal(matchedTrack.Path))
// The second missing track should remain unchanged
unmatchedTrack, _ := ds.MediaFile(ctx).Get("2")
Expect(unmatchedTrack.Path).To(Equal(missingTrack2.Path))
})
It("should return an error when there's an error moving the matched track", func() {
missingTrack := model.MediaFile{ID: "1", PID: "A", Path: "path1.mp3", Tags: model.Tags{"title": []string{"title1"}}}
matchedTrack := model.MediaFile{ID: "2", PID: "A", Path: "path1.mp3", Tags: model.Tags{"title": []string{"title1"}}}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
in := &missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matchedTrack},
}
// Simulate an error when moving the matched track by deleting the track from the DB
_ = ds.MediaFile(ctx).Delete("2")
_, err := phase.processMissingTracks(in)
Expect(err).To(HaveOccurred())
Expect(state.changesDetected.Load()).To(BeFalse())
})
})
Describe("finalize", func() {
It("should return nil if no error", func() {
err := phase.finalize(nil)
Expect(err).To(BeNil())
Expect(state.changesDetected.Load()).To(BeFalse())
})
It("should return the error if provided", func() {
err := phase.finalize(context.DeadlineExceeded)
Expect(err).To(Equal(context.DeadlineExceeded))
Expect(state.changesDetected.Load()).To(BeFalse())
})
When("PurgeMissing is 'always'", func() {
BeforeEach(func() {
conf.Server.Scanner.PurgeMissing = consts.PurgeMissingAlways
mr.SetCountAll(3)
mr.DeleteAllMissingValue = 3
})
It("should purge missing files", func() {
Expect(state.changesDetected.Load()).To(BeFalse())
err := phase.finalize(nil)
Expect(err).To(BeNil())
Expect(state.changesDetected.Load()).To(BeTrue())
})
})
When("PurgeMissing is 'full'", func() {
BeforeEach(func() {
conf.Server.Scanner.PurgeMissing = consts.PurgeMissingFull
mr.SetCountAll(2)
mr.DeleteAllMissingValue = 2
})
It("should not purge missing files if not a full scan", func() {
state.fullScan = false
err := phase.finalize(nil)
Expect(err).To(BeNil())
Expect(state.changesDetected.Load()).To(BeFalse())
})
It("should purge missing files if full scan", func() {
Expect(state.changesDetected.Load()).To(BeFalse())
state.fullScan = true
err := phase.finalize(nil)
Expect(err).To(BeNil())
Expect(state.changesDetected.Load()).To(BeTrue())
})
})
When("PurgeMissing is 'never'", func() {
BeforeEach(func() {
conf.Server.Scanner.PurgeMissing = consts.PurgeMissingNever
mr.SetCountAll(1)
mr.DeleteAllMissingValue = 1
})
It("should not purge missing files", func() {
err := phase.finalize(nil)
Expect(err).To(BeNil())
Expect(state.changesDetected.Load()).To(BeFalse())
})
})
})
Describe("processCrossLibraryMoves", func() {
It("should skip processing if input is nil", func() {
result, err := phase.processCrossLibraryMoves(nil)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(BeNil())
})
It("should skip cross-library move detection when only one library is configured", func() {
// Default BeforeEach sets up single library, so we just need to verify skip behavior
Expect(state.totalLibraryCount).To(Equal(1))
missingTrack := model.MediaFile{
ID: "missing1",
LibraryID: 1,
MbzReleaseTrackID: "mbz-track-123",
Title: "Test Track",
Size: 1000,
Suffix: "mp3",
Path: "/lib1/track.mp3",
Missing: true,
CreatedAt: time.Now().Add(-30 * time.Minute),
}
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
// Should return input unchanged (no processing done)
Expect(result).To(Equal(in))
// No matches should be found since cross-library search was skipped
Expect(phase.totalMatched.Load()).To(Equal(uint32(0)))
// No changes should be detected
Expect(state.changesDetected.Load()).To(BeFalse())
})
Context("with multiple libraries", func() {
BeforeEach(func() {
// Set up multiple libraries for cross-library move tests
state.libraries = model.Libraries{
{ID: 1, LastScanStartedAt: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)},
{ID: 2, LastScanStartedAt: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)},
}
state.totalLibraryCount = 2
})
It("should process cross-library moves using MusicBrainz Track ID", func() {
scanStartTime := time.Now().Add(-1 * time.Hour)
missingTrack := model.MediaFile{
ID: "missing1",
LibraryID: 1,
MbzReleaseTrackID: "mbz-track-123",
Title: "Test Track",
Size: 1000,
Suffix: "mp3",
Path: "/lib1/track.mp3",
Missing: true,
CreatedAt: scanStartTime.Add(-30 * time.Minute),
}
movedTrack := model.MediaFile{
ID: "moved1",
LibraryID: 2,
MbzReleaseTrackID: "mbz-track-123",
Title: "Test Track",
Size: 1000,
Suffix: "mp3",
Path: "/lib2/track.mp3",
Missing: false,
CreatedAt: scanStartTime.Add(-10 * time.Minute),
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&movedTrack)
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(Equal(in))
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
// Verify the move was performed
updatedTrack, _ := ds.MediaFile(ctx).Get("missing1")
Expect(updatedTrack.Path).To(Equal("/lib2/track.mp3"))
Expect(updatedTrack.LibraryID).To(Equal(2))
})
It("should fall back to intrinsic properties when MBZ Track ID is empty", func() {
scanStartTime := time.Now().Add(-1 * time.Hour)
missingTrack := model.MediaFile{
ID: "missing2",
LibraryID: 1,
MbzReleaseTrackID: "",
Title: "Test Track 2",
Size: 2000,
Suffix: "flac",
DiscNumber: 1,
TrackNumber: 1,
Album: "Test Album",
Path: "/lib1/track2.flac",
Missing: true,
CreatedAt: scanStartTime.Add(-30 * time.Minute),
}
movedTrack := model.MediaFile{
ID: "moved2",
LibraryID: 2,
MbzReleaseTrackID: "",
Title: "Test Track 2",
Size: 2000,
Suffix: "flac",
DiscNumber: 1,
TrackNumber: 1,
Album: "Test Album",
Path: "/lib2/track2.flac",
Missing: false,
CreatedAt: scanStartTime.Add(-10 * time.Minute),
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&movedTrack)
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(Equal(in))
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
// Verify the move was performed
updatedTrack, _ := ds.MediaFile(ctx).Get("missing2")
Expect(updatedTrack.Path).To(Equal("/lib2/track2.flac"))
Expect(updatedTrack.LibraryID).To(Equal(2))
})
It("should not match files in the same library", func() {
scanStartTime := time.Now().Add(-1 * time.Hour)
missingTrack := model.MediaFile{
ID: "missing3",
LibraryID: 1,
MbzReleaseTrackID: "mbz-track-456",
Title: "Test Track 3",
Size: 3000,
Suffix: "mp3",
Path: "/lib1/track3.mp3",
Missing: true,
CreatedAt: scanStartTime.Add(-30 * time.Minute),
}
sameLibTrack := model.MediaFile{
ID: "same1",
LibraryID: 1, // Same library
MbzReleaseTrackID: "mbz-track-456",
Title: "Test Track 3",
Size: 3000,
Suffix: "mp3",
Path: "/lib1/other/track3.mp3",
Missing: false,
CreatedAt: scanStartTime.Add(-10 * time.Minute),
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&sameLibTrack)
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(Equal(in))
Expect(phase.totalMatched.Load()).To(Equal(uint32(0)))
Expect(state.changesDetected.Load()).To(BeFalse())
})
It("should prioritize MBZ Track ID over intrinsic properties", func() {
scanStartTime := time.Now().Add(-1 * time.Hour)
missingTrack := model.MediaFile{
ID: "missing4",
LibraryID: 1,
MbzReleaseTrackID: "mbz-track-789",
Title: "Test Track 4",
Size: 4000,
Suffix: "mp3",
Path: "/lib1/track4.mp3",
Missing: true,
CreatedAt: scanStartTime.Add(-30 * time.Minute),
}
// Track with same MBZ ID
mbzTrack := model.MediaFile{
ID: "mbz1",
LibraryID: 2,
MbzReleaseTrackID: "mbz-track-789",
Title: "Test Track 4",
Size: 4000,
Suffix: "mp3",
Path: "/lib2/track4.mp3",
Missing: false,
CreatedAt: scanStartTime.Add(-10 * time.Minute),
}
// Track with same intrinsic properties but no MBZ ID
intrinsicTrack := model.MediaFile{
ID: "intrinsic1",
LibraryID: 3,
MbzReleaseTrackID: "",
Title: "Test Track 4",
Size: 4000,
Suffix: "mp3",
DiscNumber: 1,
TrackNumber: 1,
Album: "Test Album",
Path: "/lib3/track4.mp3",
Missing: false,
CreatedAt: scanStartTime.Add(-5 * time.Minute),
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&mbzTrack)
_ = ds.MediaFile(ctx).Put(&intrinsicTrack)
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(Equal(in))
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
// Verify the MBZ track was chosen (not the intrinsic one)
updatedTrack, _ := ds.MediaFile(ctx).Get("missing4")
Expect(updatedTrack.Path).To(Equal("/lib2/track4.mp3"))
Expect(updatedTrack.LibraryID).To(Equal(2))
})
It("should handle equivalent matches correctly", func() {
scanStartTime := time.Now().Add(-1 * time.Hour)
missingTrack := model.MediaFile{
ID: "missing5",
LibraryID: 1,
MbzReleaseTrackID: "",
Title: "Test Track 5",
Size: 5000,
Suffix: "mp3",
Path: "/lib1/path/track5.mp3",
Missing: true,
CreatedAt: scanStartTime.Add(-30 * time.Minute),
}
// Equivalent match (same filename, different directory)
equivalentTrack := model.MediaFile{
ID: "equiv1",
LibraryID: 2,
MbzReleaseTrackID: "",
Title: "Test Track 5",
Size: 5000,
Suffix: "mp3",
Path: "/lib2/different/track5.mp3",
Missing: false,
CreatedAt: scanStartTime.Add(-10 * time.Minute),
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&equivalentTrack)
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(Equal(in))
Expect(phase.totalMatched.Load()).To(Equal(uint32(1)))
Expect(state.changesDetected.Load()).To(BeTrue())
// Verify the equivalent match was accepted
updatedTrack, _ := ds.MediaFile(ctx).Get("missing5")
Expect(updatedTrack.Path).To(Equal("/lib2/different/track5.mp3"))
Expect(updatedTrack.LibraryID).To(Equal(2))
})
It("should skip matching when multiple matches are found but none are exact", func() {
scanStartTime := time.Now().Add(-1 * time.Hour)
missingTrack := model.MediaFile{
ID: "missing6",
LibraryID: 1,
MbzReleaseTrackID: "",
Title: "Test Track 6",
Size: 6000,
Suffix: "mp3",
DiscNumber: 1,
TrackNumber: 1,
Album: "Test Album",
Path: "/lib1/track6.mp3",
Missing: true,
CreatedAt: scanStartTime.Add(-30 * time.Minute),
}
// Multiple matches with different metadata (not exact matches)
match1 := model.MediaFile{
ID: "match1",
LibraryID: 2,
MbzReleaseTrackID: "",
Title: "Test Track 6",
Size: 6000,
Suffix: "mp3",
DiscNumber: 1,
TrackNumber: 1,
Album: "Test Album",
Path: "/lib2/different_track.mp3",
Artist: "Different Artist", // This makes it non-exact
Missing: false,
CreatedAt: scanStartTime.Add(-10 * time.Minute),
}
match2 := model.MediaFile{
ID: "match2",
LibraryID: 3,
MbzReleaseTrackID: "",
Title: "Test Track 6",
Size: 6000,
Suffix: "mp3",
DiscNumber: 1,
TrackNumber: 1,
Album: "Test Album",
Path: "/lib3/another_track.mp3",
Artist: "Another Artist", // This makes it non-exact
Missing: false,
CreatedAt: scanStartTime.Add(-5 * time.Minute),
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&match1)
_ = ds.MediaFile(ctx).Put(&match2)
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(Equal(in))
Expect(phase.totalMatched.Load()).To(Equal(uint32(0)))
Expect(state.changesDetected.Load()).To(BeFalse())
// Verify no move was performed
unchangedTrack, _ := ds.MediaFile(ctx).Get("missing6")
Expect(unchangedTrack.Path).To(Equal("/lib1/track6.mp3"))
Expect(unchangedTrack.LibraryID).To(Equal(1))
})
It("should handle errors gracefully", func() {
// Set up mock to return error
mr.Err = true
missingTrack := model.MediaFile{
ID: "missing7",
LibraryID: 1,
MbzReleaseTrackID: "mbz-track-error",
Title: "Test Track 7",
Size: 7000,
Suffix: "mp3",
Path: "/lib1/track7.mp3",
Missing: true,
CreatedAt: time.Now().Add(-30 * time.Minute),
}
in := &missingTracks{
lib: model.Library{ID: 1, Name: "Library 1"},
missing: []model.MediaFile{missingTrack},
}
// Should not fail completely, just skip the problematic file
result, err := phase.processCrossLibraryMoves(in)
Expect(err).ToNot(HaveOccurred())
Expect(result).To(Equal(in))
Expect(phase.totalMatched.Load()).To(Equal(uint32(0)))
Expect(state.changesDetected.Load()).To(BeFalse())
})
}) // End of Context "with multiple libraries"
})
Describe("CreatedAt preservation (#5050)", func() {
var albumRepo *tests.MockAlbumRepo
BeforeEach(func() {
albumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
albumRepo.ReassignAnnotationCalls = make(map[string]string)
albumRepo.CopyAttributesCalls = make(map[string]string)
})
It("should preserve the missing track's created_at when moving within a library", func() {
originalTime := time.Date(2020, 3, 15, 10, 0, 0, 0, time.UTC)
missingTrack := model.MediaFile{
ID: "1", PID: "A", Path: "old/song.mp3",
AlbumID: "album-1",
LibraryID: 1,
CreatedAt: originalTime,
Tags: model.Tags{"title": []string{"My Song"}},
Size: 100,
}
matchedTrack := model.MediaFile{
ID: "2", PID: "A", Path: "new/song.mp3",
AlbumID: "album-1", // Same album
LibraryID: 1,
CreatedAt: time.Now(), // Much newer
Tags: model.Tags{"title": []string{"My Song"}},
Size: 100,
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
in := &missingTracks{
missing: []model.MediaFile{missingTrack},
matched: []model.MediaFile{matchedTrack},
}
_, err := phase.processMissingTracks(in)
Expect(err).ToNot(HaveOccurred())
movedTrack, _ := ds.MediaFile(ctx).Get("1")
Expect(movedTrack.Path).To(Equal("new/song.mp3"))
Expect(movedTrack.CreatedAt).To(Equal(originalTime))
})
It("should preserve created_at during cross-library moves with album change", func() {
originalTime := time.Date(2019, 6, 1, 12, 0, 0, 0, time.UTC)
missingTrack := model.MediaFile{
ID: "missing-ca", PID: "B", Path: "lib1/song.mp3",
AlbumID: "old-album",
LibraryID: 1,
CreatedAt: originalTime,
}
matchedTrack := model.MediaFile{
ID: "matched-ca", PID: "B", Path: "lib2/song.mp3",
AlbumID: "new-album",
LibraryID: 2,
CreatedAt: time.Now(),
}
// Set up albums so CopyAttributes can find them
albumRepo.SetData(model.Albums{
{ID: "old-album", LibraryID: 1, CreatedAt: originalTime},
{ID: "new-album", LibraryID: 2, CreatedAt: time.Now()},
})
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
err := phase.moveMatched(matchedTrack, missingTrack)
Expect(err).ToNot(HaveOccurred())
// Track's created_at should be preserved from the missing file
movedTrack, _ := ds.MediaFile(ctx).Get("missing-ca")
Expect(movedTrack.CreatedAt).To(Equal(originalTime))
// Album's created_at should be copied from old to new
Expect(albumRepo.CopyAttributesCalls).To(HaveKeyWithValue("old-album", "new-album"))
// Verify the new album's CreatedAt was actually updated
newAlbum, err := albumRepo.Get("new-album")
Expect(err).ToNot(HaveOccurred())
Expect(newAlbum.CreatedAt).To(Equal(originalTime))
})
It("should not copy album created_at when album ID does not change", func() {
originalTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
missingTrack := model.MediaFile{
ID: "missing-same", PID: "C", Path: "dir1/song.mp3",
AlbumID: "same-album",
LibraryID: 1,
CreatedAt: originalTime,
}
matchedTrack := model.MediaFile{
ID: "matched-same", PID: "C", Path: "dir2/song.mp3",
AlbumID: "same-album", // Same album
LibraryID: 1,
CreatedAt: time.Now(),
}
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
err := phase.moveMatched(matchedTrack, missingTrack)
Expect(err).ToNot(HaveOccurred())
// Track's created_at should still be preserved
movedTrack, _ := ds.MediaFile(ctx).Get("missing-same")
Expect(movedTrack.CreatedAt).To(Equal(originalTime))
// CopyAttributes should NOT have been called (same album)
Expect(albumRepo.CopyAttributesCalls).To(BeEmpty())
})
})
Describe("Album Annotation Reassignment", func() {
var (
albumRepo *tests.MockAlbumRepo
missingTrack model.MediaFile
matchedTrack model.MediaFile
oldAlbumID string
newAlbumID string
)
BeforeEach(func() {
albumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
albumRepo.ReassignAnnotationCalls = make(map[string]string)
oldAlbumID = "old-album-id"
newAlbumID = "new-album-id"
missingTrack = model.MediaFile{
ID: "missing-track-id",
PID: "same-pid",
Path: "old/path.mp3",
AlbumID: oldAlbumID,
LibraryID: 1,
Missing: true,
Annotations: model.Annotations{
PlayCount: 5,
Rating: 4,
Starred: true,
},
}
matchedTrack = model.MediaFile{
ID: "matched-track-id",
PID: "same-pid",
Path: "new/path.mp3",
AlbumID: newAlbumID,
LibraryID: 2, // Different library
Missing: false,
Annotations: model.Annotations{
PlayCount: 2,
Rating: 3,
Starred: false,
},
}
// Store both tracks in the database
_ = ds.MediaFile(ctx).Put(&missingTrack)
_ = ds.MediaFile(ctx).Put(&matchedTrack)
})
When("album ID changes during cross-library move", func() {
It("should reassign album annotations when AlbumID changes", func() {
err := phase.moveMatched(matchedTrack, missingTrack)
Expect(err).ToNot(HaveOccurred())
// Verify that ReassignAnnotation was called
Expect(albumRepo.ReassignAnnotationCalls).To(HaveKeyWithValue(oldAlbumID, newAlbumID))
})
It("should not reassign annotations when AlbumID is the same", func() {
missingTrack.AlbumID = newAlbumID // Same album
err := phase.moveMatched(matchedTrack, missingTrack)
Expect(err).ToNot(HaveOccurred())
// Verify that ReassignAnnotation was NOT called
Expect(albumRepo.ReassignAnnotationCalls).To(BeEmpty())
})
})
When("error handling", func() {
It("should handle ReassignAnnotation errors gracefully", func() {
// Make the album repo return an error
albumRepo.SetError(true)
// The move should still succeed even if annotation reassignment fails
err := phase.moveMatched(matchedTrack, missingTrack)
Expect(err).ToNot(HaveOccurred())
// Verify that the track was still moved (ID should be updated)
movedTrack, err := ds.MediaFile(ctx).Get(missingTrack.ID)
Expect(err).ToNot(HaveOccurred())
Expect(movedTrack.Path).To(Equal(matchedTrack.Path))
})
})
})
})
// rerunTxDS runs every WithTxRetry block twice, as a retry after a rolled-back busy attempt would.
// The mock is not transactional, so snapshot returns the function that plays the rollback.
type rerunTxDS struct {
*tests.MockDataStore
snapshot func() (rollback func())
}
func (d *rerunTxDS) WithTxRetry(ctx context.Context, block func(context.Context, model.DataStore) error, _ ...string) error {
rollback := d.snapshot()
_ = block(ctx, d.MockDataStore)
rollback()
return block(ctx, d.MockDataStore)
}
// probeTxDS runs a hook inside each WithTxRetry block, and can fail the transaction after it.
type probeTxDS struct {
*tests.MockDataStore
during func()
err error
}
func (d *probeTxDS) WithTxRetry(ctx context.Context, block func(context.Context, model.DataStore) error, _ ...string) error {
if err := block(ctx, d.MockDataStore); err != nil {
return err
}
if d.during != nil {
d.during()
}
return d.err
}
Reference in a new issue View git blame Copy permalink