mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix(scanner): detect in-place playlist edits via the folder content hash (#5914)
* fix(scanner): detect in-place playlist edits via the folder content hash Signed-off-by: junkerderprovinz <jdp@braethoria.com> * docs(scanner): clarify the playlist entries in the folder hash Shorten the comment on the playlist loop, and record why the playlist count stays in the hash header: it is redundant with the loop for change detection, but removing it changes the hashed byte stream for every folder, including folders without playlists, which would mark every folder outdated on the first scan after upgrade. Signed-off-by: junkerderprovinz <jdp@braethoria.com> * test(scanner): pin filename and size into the playlist hash assertions The playlist size test called time.Now() twice, so the modtime differed too and carried the assertion — dropping info.Size() from the hash left the suite green. It now shares one baseTime. A new rename test swaps the map key with count, size and modtime held constant, so dropping the filename from the hash fails. Both mutations were verified to fail before this change and pass after. --------- Signed-off-by: junkerderprovinz <jdp@braethoria.com> Co-authored-by: Deluan <deluan@navidrome.org>
This commit is contained in:
parent
c362519f76
commit
dff9e47c2e
@ -23,6 +23,7 @@ func newFolderEntry(job *scanJob, id, path string, info model.FolderUpdateInfo)
|
||||
path: path,
|
||||
audioFiles: make(map[string]fs.DirEntry),
|
||||
imageFiles: make(map[string]fs.DirEntry),
|
||||
playlistFiles: make(map[string]fs.DirEntry),
|
||||
albumIDMap: make(map[string]string),
|
||||
updTime: info.UpdatedAt,
|
||||
prevHash: info.Hash,
|
||||
@ -41,7 +42,7 @@ type folderEntry struct {
|
||||
updTime time.Time // from DB
|
||||
audioFiles map[string]fs.DirEntry
|
||||
imageFiles map[string]fs.DirEntry
|
||||
numPlaylists int
|
||||
playlistFiles map[string]fs.DirEntry
|
||||
numSubFolders int
|
||||
imagesUpdatedAt time.Time
|
||||
prevHash string // Previous hash from DB
|
||||
@ -57,7 +58,7 @@ type folderEntry struct {
|
||||
}
|
||||
|
||||
func (f *folderEntry) hasNoFiles() bool {
|
||||
return len(f.audioFiles) == 0 && len(f.imageFiles) == 0 && f.numPlaylists == 0
|
||||
return len(f.audioFiles) == 0 && len(f.imageFiles) == 0 && len(f.playlistFiles) == 0
|
||||
}
|
||||
|
||||
func (f *folderEntry) isEmpty() bool {
|
||||
@ -94,7 +95,7 @@ func (f *folderEntry) toFolder() *model.Folder {
|
||||
folder := model.NewFolder(f.job.lib, f.path)
|
||||
folder.NumAudioFiles = len(f.audioFiles)
|
||||
if playlists.InPath(*folder) {
|
||||
folder.NumPlaylists = f.numPlaylists
|
||||
folder.NumPlaylists = len(f.playlistFiles)
|
||||
}
|
||||
folder.ImageFiles = slices.Collect(maps.Keys(f.imageFiles))
|
||||
folder.ImagesUpdatedAt = f.imagesUpdatedAt
|
||||
@ -108,16 +109,18 @@ func (f *folderEntry) hash() string {
|
||||
h,
|
||||
"%s:%d:%d:%s",
|
||||
f.modTime.UTC(),
|
||||
f.numPlaylists,
|
||||
len(f.playlistFiles), // redundant with the loop below, but dropping it re-hashes every folder
|
||||
f.numSubFolders,
|
||||
f.imagesUpdatedAt.UTC(),
|
||||
)
|
||||
|
||||
// Sort the keys of audio and image files to ensure consistent hashing
|
||||
// Sort the keys of audio, image and playlist files to ensure consistent hashing
|
||||
audioKeys := slices.Collect(maps.Keys(f.audioFiles))
|
||||
slices.Sort(audioKeys)
|
||||
imageKeys := slices.Collect(maps.Keys(f.imageFiles))
|
||||
slices.Sort(imageKeys)
|
||||
playlistKeys := slices.Collect(maps.Keys(f.playlistFiles))
|
||||
slices.Sort(playlistKeys)
|
||||
|
||||
// Include audio files with their size and modtime
|
||||
for _, key := range audioKeys {
|
||||
@ -135,5 +138,14 @@ func (f *folderEntry) hash() string {
|
||||
}
|
||||
}
|
||||
|
||||
// Include playlist files, so a content edit is detected even when the folder's
|
||||
// mtime is preserved (rsync -a) or the playlist is not the newest file.
|
||||
for _, key := range playlistKeys {
|
||||
_, _ = io.WriteString(h, key)
|
||||
if info, err := f.playlistFiles[key].Info(); err == nil {
|
||||
_, _ = fmt.Fprintf(h, ":%d:%s", info.Size(), info.ModTime().UTC().String())
|
||||
}
|
||||
}
|
||||
|
||||
return hex.EncodeToString(h.Sum(nil))
|
||||
}
|
||||
|
||||
@ -48,6 +48,7 @@ var _ = Describe("folder_entry", func() {
|
||||
Expect(entry.path).To(Equal(path))
|
||||
Expect(entry.audioFiles).To(BeEmpty())
|
||||
Expect(entry.imageFiles).To(BeEmpty())
|
||||
Expect(entry.playlistFiles).To(BeEmpty())
|
||||
Expect(entry.albumIDMap).To(BeEmpty())
|
||||
Expect(entry.updTime).To(Equal(updateInfo.UpdatedAt))
|
||||
Expect(entry.prevHash).To(Equal(updateInfo.Hash))
|
||||
@ -95,7 +96,7 @@ var _ = Describe("folder_entry", func() {
|
||||
})
|
||||
|
||||
It("returns false when folder has playlists", func() {
|
||||
entry.numPlaylists = 1
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"}
|
||||
Expect(entry.hasNoFiles()).To(BeFalse())
|
||||
})
|
||||
|
||||
@ -107,7 +108,7 @@ var _ = Describe("folder_entry", func() {
|
||||
It("returns false when folder has multiple types of content", func() {
|
||||
entry.audioFiles["test.mp3"] = &fakeDirEntry{name: "test.mp3"}
|
||||
entry.imageFiles["cover.jpg"] = &fakeDirEntry{name: "cover.jpg"}
|
||||
entry.numPlaylists = 2
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"}
|
||||
entry.numSubFolders = 3
|
||||
Expect(entry.hasNoFiles()).To(BeFalse())
|
||||
})
|
||||
@ -149,7 +150,11 @@ var _ = Describe("folder_entry", func() {
|
||||
"cover.jpg": &fakeDirEntry{name: "cover.jpg"},
|
||||
"folder.png": &fakeDirEntry{name: "folder.png"},
|
||||
}
|
||||
entry.numPlaylists = 3
|
||||
entry.playlistFiles = map[string]fs.DirEntry{
|
||||
"list1.m3u": &fakeDirEntry{name: "list1.m3u"},
|
||||
"list2.m3u": &fakeDirEntry{name: "list2.m3u"},
|
||||
"list3.m3u": &fakeDirEntry{name: "list3.m3u"},
|
||||
}
|
||||
entry.imagesUpdatedAt = time.Now()
|
||||
})
|
||||
|
||||
@ -200,7 +205,10 @@ var _ = Describe("folder_entry", func() {
|
||||
"z.jpg": &fakeDirEntry{name: "z.jpg"},
|
||||
"x.png": &fakeDirEntry{name: "x.png"},
|
||||
}
|
||||
entry.numPlaylists = 2
|
||||
entry.playlistFiles = map[string]fs.DirEntry{
|
||||
"q.m3u": &fakeDirEntry{name: "q.m3u"},
|
||||
"p.m3u": &fakeDirEntry{name: "p.m3u"},
|
||||
}
|
||||
entry.numSubFolders = 3
|
||||
|
||||
hash1 := entry.hash()
|
||||
@ -214,6 +222,10 @@ var _ = Describe("folder_entry", func() {
|
||||
"x.png": &fakeDirEntry{name: "x.png"},
|
||||
"z.jpg": &fakeDirEntry{name: "z.jpg"},
|
||||
}
|
||||
entry.playlistFiles = map[string]fs.DirEntry{
|
||||
"p.m3u": &fakeDirEntry{name: "p.m3u"},
|
||||
"q.m3u": &fakeDirEntry{name: "q.m3u"},
|
||||
}
|
||||
|
||||
hash2 := entry.hash()
|
||||
Expect(hash1).To(Equal(hash2))
|
||||
@ -252,10 +264,10 @@ var _ = Describe("folder_entry", func() {
|
||||
Expect(hash1).ToNot(Equal(hash2))
|
||||
})
|
||||
|
||||
It("produces different hash when playlist count changes", func() {
|
||||
It("produces different hash when playlist files change", func() {
|
||||
hash1 := entry.hash()
|
||||
|
||||
entry.numPlaylists = 5
|
||||
entry.playlistFiles["new.m3u"] = &fakeDirEntry{name: "new.m3u"}
|
||||
hash2 := entry.hash()
|
||||
|
||||
Expect(hash1).ToNot(Equal(hash2))
|
||||
@ -377,6 +389,58 @@ var _ = Describe("folder_entry", func() {
|
||||
Expect(hash1).ToNot(Equal(hash2))
|
||||
})
|
||||
|
||||
It("produces different hash when playlist file size changes", func() {
|
||||
baseTime := time.Now()
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
|
||||
name: "list.m3u",
|
||||
fileInfo: &fakeFileInfo{name: "list.m3u", size: 1000, modTime: baseTime},
|
||||
}
|
||||
hash1 := entry.hash()
|
||||
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
|
||||
name: "list.m3u",
|
||||
fileInfo: &fakeFileInfo{name: "list.m3u", size: 2000, modTime: baseTime},
|
||||
}
|
||||
hash2 := entry.hash()
|
||||
|
||||
Expect(hash1).ToNot(Equal(hash2))
|
||||
})
|
||||
|
||||
It("produces different hash when playlist file modification time changes", func() {
|
||||
baseTime := time.Now()
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
|
||||
name: "list.m3u",
|
||||
fileInfo: &fakeFileInfo{name: "list.m3u", size: 1000, modTime: baseTime},
|
||||
}
|
||||
hash1 := entry.hash()
|
||||
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
|
||||
name: "list.m3u",
|
||||
fileInfo: &fakeFileInfo{name: "list.m3u", size: 1000, modTime: baseTime.Add(1 * time.Hour)},
|
||||
}
|
||||
hash2 := entry.hash()
|
||||
|
||||
Expect(hash1).ToNot(Equal(hash2))
|
||||
})
|
||||
|
||||
It("produces different hash when a playlist is renamed", func() {
|
||||
baseTime := time.Now()
|
||||
entry.playlistFiles["old.m3u"] = &fakeDirEntry{
|
||||
name: "old.m3u",
|
||||
fileInfo: &fakeFileInfo{name: "old.m3u", size: 1000, modTime: baseTime},
|
||||
}
|
||||
hash1 := entry.hash()
|
||||
|
||||
delete(entry.playlistFiles, "old.m3u")
|
||||
entry.playlistFiles["new.m3u"] = &fakeDirEntry{
|
||||
name: "new.m3u",
|
||||
fileInfo: &fakeFileInfo{name: "new.m3u", size: 1000, modTime: baseTime},
|
||||
}
|
||||
hash2 := entry.hash()
|
||||
|
||||
Expect(hash1).ToNot(Equal(hash2))
|
||||
})
|
||||
|
||||
It("produces valid hex-encoded hash", func() {
|
||||
hash := entry.hash()
|
||||
Expect(hash).To(HaveLen(32)) // MD5 hash should be 32 hex characters
|
||||
@ -421,7 +485,7 @@ var _ = Describe("folder_entry", func() {
|
||||
})
|
||||
|
||||
It("returns true when hash has changed", func() {
|
||||
entry.numPlaylists = 10 // Change something to change the hash
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"} // Change something to change the hash
|
||||
Expect(entry.isOutdated()).To(BeTrue())
|
||||
})
|
||||
|
||||
@ -445,7 +509,7 @@ var _ = Describe("folder_entry", func() {
|
||||
|
||||
It("returns true when full scan condition is not met but hash changed", func() {
|
||||
entry.updTime = entry.job.lib.LastScanStartedAt.Add(1 * time.Hour)
|
||||
entry.numPlaylists = 10 // Change hash
|
||||
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"} // Change hash
|
||||
Expect(entry.isOutdated()).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
@ -165,7 +165,7 @@ func (p *phaseFolders) producer() ppl.Producer[*folderEntry] {
|
||||
log.Trace(p.ctx, "Scanner: Checking folder state", " folder", folder.path, "_updTime", folder.updTime,
|
||||
"_modTime", folder.modTime, "_lastScanStartedAt", folder.job.lib.LastScanStartedAt,
|
||||
"numAudioFiles", len(folder.audioFiles), "numImageFiles", len(folder.imageFiles),
|
||||
"numPlaylists", folder.numPlaylists, "numSubfolders", folder.numSubFolders)
|
||||
"numPlaylists", len(folder.playlistFiles), "numSubfolders", folder.numSubFolders)
|
||||
|
||||
// Check if folder is outdated
|
||||
if folder.isOutdated() {
|
||||
@ -492,7 +492,7 @@ func (p *phaseFolders) logFolder(entry *folderEntry) (*folderEntry, error) {
|
||||
logCall = log.Trace
|
||||
}
|
||||
logCall(p.ctx, "Scanner: Completed processing folder",
|
||||
"audioCount", len(entry.audioFiles), "imageCount", len(entry.imageFiles), "plsCount", entry.numPlaylists,
|
||||
"audioCount", len(entry.audioFiles), "imageCount", len(entry.imageFiles), "plsCount", len(entry.playlistFiles),
|
||||
"elapsed", entry.elapsed.Elapsed(), "tracksMissing", len(entry.missingTracks),
|
||||
"tracksImported", len(entry.tracks), "library", entry.job.lib.Name, consts.Zwsp+"folder", entry.path)
|
||||
return entry, nil
|
||||
|
||||
@ -82,7 +82,7 @@ func walkFolder(ctx context.Context, job *scanJob, currentFolder string, checker
|
||||
|
||||
dir := path.Clean(currentFolder)
|
||||
log.Trace(ctx, "Scanner: Found directory", " path", dir, "audioFiles", maps.Keys(folder.audioFiles),
|
||||
"images", maps.Keys(folder.imageFiles), "playlists", folder.numPlaylists, "imagesUpdatedAt", folder.imagesUpdatedAt,
|
||||
"images", maps.Keys(folder.imageFiles), "playlists", len(folder.playlistFiles), "imagesUpdatedAt", folder.imagesUpdatedAt,
|
||||
"updTime", folder.updTime, "modTime", folder.modTime, "numChildren", len(children))
|
||||
folder.path = dir
|
||||
folder.elapsed.Start()
|
||||
@ -157,7 +157,7 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC
|
||||
case model.IsAudioFile(name):
|
||||
folder.audioFiles[entry.Name()] = entry
|
||||
case model.IsValidPlaylist(name):
|
||||
folder.numPlaylists++
|
||||
folder.playlistFiles[entry.Name()] = entry
|
||||
case model.IsImageFile(name):
|
||||
folder.imageFiles[entry.Name()] = entry
|
||||
folder.imagesUpdatedAt = utils.TimeNewest(folder.imagesUpdatedAt, fileInfo.ModTime(), folder.modTime)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user