mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix(scanner): prevent race condition when scanning deleted folders
When the watcher detects changes in a folder that gets deleted before the scanner runs (due to the 10-second delay), the scanner was prematurely removing these folders from the tracking map, preventing them from being marked as missing. The issue occurred because `newFolderEntry` was calling `popLastUpdate` before verifying the folder actually exists on the filesystem. Changes: - Move fs.Stat check before newFolderEntry creation in loadDir to ensure deleted folders remain in lastUpdates for finalize() to handle - Add early existence check in walkDirTree to skip non-existent target folders with a warning log - Add unit test verifying non-existent folders aren't removed from lastUpdates prematurely - Add integration test for deleted folder scenario with ScanFolders Fixes the issue where deleting entire folders (e.g., /music/AC_DC) wouldn't mark tracks as missing when using selective folder scanning.
This commit is contained in:
parent
67c876539e
commit
e0b544a31c
@ -478,6 +478,56 @@ var _ = Describe("Scanner", Ordered, func() {
|
||||
Expect(mf.Missing).To(BeFalse())
|
||||
})
|
||||
|
||||
It("marks tracks as missing when scanning a deleted folder with ScanFolders", func() {
|
||||
By("Adding a third track to Revolver to have more test data")
|
||||
fsys.Add("The Beatles/Revolver/03 - I'm Only Sleeping.mp3", revolver(track(3, "I'm Only Sleeping")))
|
||||
Expect(runScanner(ctx, false)).To(Succeed())
|
||||
|
||||
By("Verifying initial state has 5 tracks")
|
||||
Expect(ds.MediaFile(ctx).CountAll(model.QueryOptions{
|
||||
Filters: squirrel.Eq{"missing": false},
|
||||
})).To(Equal(int64(5)))
|
||||
|
||||
By("Removing the entire Revolver folder from filesystem")
|
||||
fsys.Remove("The Beatles/Revolver/01 - Taxman.mp3")
|
||||
fsys.Remove("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
|
||||
fsys.Remove("The Beatles/Revolver/03 - I'm Only Sleeping.mp3")
|
||||
|
||||
By("Scanning the parent folder (simulating watcher behavior)")
|
||||
targets := []model.ScanTarget{
|
||||
{LibraryID: lib.ID, FolderPath: "The Beatles"},
|
||||
}
|
||||
_, err := s.ScanFolders(ctx, false, targets)
|
||||
Expect(err).To(Succeed())
|
||||
|
||||
By("Checking all Revolver tracks are marked as missing")
|
||||
mf, err := findByPath("The Beatles/Revolver/01 - Taxman.mp3")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(mf.Missing).To(BeTrue())
|
||||
|
||||
mf, err = findByPath("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(mf.Missing).To(BeTrue())
|
||||
|
||||
mf, err = findByPath("The Beatles/Revolver/03 - I'm Only Sleeping.mp3")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(mf.Missing).To(BeTrue())
|
||||
|
||||
By("Checking the Help! tracks are not affected")
|
||||
mf, err = findByPath("The Beatles/Help!/01 - Help!.mp3")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(mf.Missing).To(BeFalse())
|
||||
|
||||
mf, err = findByPath("The Beatles/Help!/02 - The Night Before.mp3")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(mf.Missing).To(BeFalse())
|
||||
|
||||
By("Verifying only 2 non-missing tracks remain (Help! tracks)")
|
||||
Expect(ds.MediaFile(ctx).CountAll(model.QueryOptions{
|
||||
Filters: squirrel.Eq{"missing": false},
|
||||
})).To(Equal(int64(2)))
|
||||
})
|
||||
|
||||
It("does not override artist fields when importing an undertagged file", func() {
|
||||
By("Making sure artist in the DB contains MBID and sort name")
|
||||
aa, err := ds.Artist(ctx).GetAll(model.QueryOptions{
|
||||
|
||||
@ -32,9 +32,18 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<-
|
||||
return
|
||||
}
|
||||
|
||||
// Check if target folder exists before walking it
|
||||
// If it doesn't exist (e.g., deleted between watcher detection and scan execution),
|
||||
// skip it so it remains in job.lastUpdates and gets handled in following steps
|
||||
_, err := fs.Stat(job.fs, folderPath)
|
||||
if err != nil {
|
||||
log.Warn(ctx, "Scanner: Target folder does not exist.", "path", folderPath, err)
|
||||
continue
|
||||
}
|
||||
|
||||
// Create checker and push patterns from root to this folder
|
||||
checker := newIgnoreChecker(job.fs)
|
||||
err := checker.PushAllParents(ctx, folderPath)
|
||||
err = checker.PushAllParents(ctx, folderPath)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Scanner: Error pushing ignore patterns for target folder", "path", folderPath, err)
|
||||
continue
|
||||
@ -82,13 +91,16 @@ func walkFolder(ctx context.Context, job *scanJob, currentFolder string, checker
|
||||
}
|
||||
|
||||
func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreChecker) (folder *folderEntry, children []string, err error) {
|
||||
folder = newFolderEntry(job, dirPath)
|
||||
|
||||
// Check if directory exists before creating the folder entry
|
||||
// This is important to avoid removing the folder from lastUpdates if it doesn't exist
|
||||
dirInfo, err := fs.Stat(job.fs, dirPath)
|
||||
if err != nil {
|
||||
log.Warn(ctx, "Scanner: Error stating dir", "path", dirPath, err)
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
// Now that we know the folder exists, create the entry (which removes it from lastUpdates)
|
||||
folder = newFolderEntry(job, dirPath)
|
||||
folder.modTime = dirInfo.ModTime()
|
||||
|
||||
dir, err := job.fs.Open(dirPath)
|
||||
|
||||
@ -182,6 +182,38 @@ var _ = Describe("walk_dir_tree", func() {
|
||||
Expect(folders).ToNot(HaveKey("Artist/Album2"))
|
||||
Expect(folders).ToNot(HaveKey("Artist/Album2/Sub"))
|
||||
})
|
||||
|
||||
It("should skip non-existent target folders and preserve them in lastUpdates", func() {
|
||||
// Setup job with lastUpdates for both existing and non-existing folders
|
||||
job.lastUpdates = map[string]model.FolderUpdateInfo{
|
||||
model.FolderID(job.lib, "Artist/Album1"): {},
|
||||
model.FolderID(job.lib, "NonExistent/DeletedFolder"): {},
|
||||
model.FolderID(job.lib, "OtherArtist/Album3"): {},
|
||||
}
|
||||
|
||||
// Try to scan existing folder and non-existing folder
|
||||
results, err := walkDirTree(ctx, job, "Artist/Album1", "NonExistent/DeletedFolder")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
// Collect results
|
||||
folders := map[string]struct{}{}
|
||||
for folder := range results {
|
||||
folders[folder.path] = struct{}{}
|
||||
}
|
||||
|
||||
// Should only include the existing folder
|
||||
Expect(folders).To(HaveKey("Artist/Album1"))
|
||||
Expect(folders).ToNot(HaveKey("NonExistent/DeletedFolder"))
|
||||
|
||||
// The non-existent folder should still be in lastUpdates (not removed by popLastUpdate)
|
||||
Expect(job.lastUpdates).To(HaveKey(model.FolderID(job.lib, "NonExistent/DeletedFolder")))
|
||||
|
||||
// The existing folder should have been removed from lastUpdates
|
||||
Expect(job.lastUpdates).ToNot(HaveKey(model.FolderID(job.lib, "Artist/Album1")))
|
||||
|
||||
// Folders not in targets should remain in lastUpdates
|
||||
Expect(job.lastUpdates).To(HaveKey(model.FolderID(job.lib, "OtherArtist/Album3")))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user