fix(scanner): prevent duplicate tracks when multiple missing files match same target (#5183)
In processMissingTracks, matched tracks were not removed from the candidate pool after being consumed by moveMatched. This allowed the same target track to be paired with multiple missing tracks, creating duplicate non-missing records with the same path. Track consumed matches in a usedMatched map so each target is used at most once. Fixes #5169
This commit is contained in:
@@ -114,6 +114,10 @@ func (p *phaseMissingTracks) stages() []ppl.Stage[*missingTracks] {
|
|||||||
|
|
||||||
func (p *phaseMissingTracks) processMissingTracks(in *missingTracks) (*missingTracks, error) {
|
func (p *phaseMissingTracks) processMissingTracks(in *missingTracks) (*missingTracks, error) {
|
||||||
hasMatches := false
|
hasMatches := false
|
||||||
|
// Track which matched entries have already been consumed, so each matched track
|
||||||
|
// is only used once. Without this, the same matched track could be paired with
|
||||||
|
// multiple missing tracks, creating duplicate records with the same path.
|
||||||
|
usedMatched := make(map[string]bool, len(in.matched))
|
||||||
|
|
||||||
for _, ms := range in.missing {
|
for _, ms := range in.missing {
|
||||||
var exactMatch model.MediaFile
|
var exactMatch model.MediaFile
|
||||||
@@ -121,6 +125,9 @@ func (p *phaseMissingTracks) processMissingTracks(in *missingTracks) (*missingTr
|
|||||||
|
|
||||||
// Identify exact and equivalent matches
|
// Identify exact and equivalent matches
|
||||||
for _, mt := range in.matched {
|
for _, mt := range in.matched {
|
||||||
|
if usedMatched[mt.ID] {
|
||||||
|
continue
|
||||||
|
}
|
||||||
if ms.Equals(mt) {
|
if ms.Equals(mt) {
|
||||||
exactMatch = mt
|
exactMatch = mt
|
||||||
break // Prioritize exact match
|
break // Prioritize exact match
|
||||||
@@ -138,13 +145,14 @@ func (p *phaseMissingTracks) processMissingTracks(in *missingTracks) (*missingTr
|
|||||||
log.Error(p.ctx, "Scanner: Error moving matched track", "missing", ms.Path, "movedTo", exactMatch.Path, "lib", in.lib.Name, err)
|
log.Error(p.ctx, "Scanner: Error moving matched track", "missing", ms.Path, "movedTo", exactMatch.Path, "lib", in.lib.Name, err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
usedMatched[exactMatch.ID] = true
|
||||||
p.totalMatched.Add(1)
|
p.totalMatched.Add(1)
|
||||||
hasMatches = true
|
hasMatches = true
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
// If there is only one missing and one matched track, consider them equivalent (same PID)
|
// If there is only one missing and one matched track, consider them equivalent (same PID)
|
||||||
if len(in.missing) == 1 && len(in.matched) == 1 {
|
if len(in.missing) == 1 && len(in.matched) == 1 && !usedMatched[in.matched[0].ID] {
|
||||||
singleMatch := in.matched[0]
|
singleMatch := in.matched[0]
|
||||||
log.Debug(p.ctx, "Scanner: Found track with same persistent ID in a new place", "missing", ms.Path, "movedTo", singleMatch.Path, "lib", in.lib.Name)
|
log.Debug(p.ctx, "Scanner: Found track with same persistent ID in a new place", "missing", ms.Path, "movedTo", singleMatch.Path, "lib", in.lib.Name)
|
||||||
err := p.moveMatched(singleMatch, ms)
|
err := p.moveMatched(singleMatch, ms)
|
||||||
@@ -152,6 +160,7 @@ func (p *phaseMissingTracks) processMissingTracks(in *missingTracks) (*missingTr
|
|||||||
log.Error(p.ctx, "Scanner: Error updating matched track", "missing", ms.Path, "movedTo", singleMatch.Path, "lib", in.lib.Name, err)
|
log.Error(p.ctx, "Scanner: Error updating matched track", "missing", ms.Path, "movedTo", singleMatch.Path, "lib", in.lib.Name, err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
usedMatched[singleMatch.ID] = true
|
||||||
p.totalMatched.Add(1)
|
p.totalMatched.Add(1)
|
||||||
hasMatches = true
|
hasMatches = true
|
||||||
continue
|
continue
|
||||||
@@ -165,6 +174,7 @@ func (p *phaseMissingTracks) processMissingTracks(in *missingTracks) (*missingTr
|
|||||||
log.Error(p.ctx, "Scanner: Error updating matched track", "missing", ms.Path, "movedTo", equivalentMatch.Path, "lib", in.lib.Name, err)
|
log.Error(p.ctx, "Scanner: Error updating matched track", "missing", ms.Path, "movedTo", equivalentMatch.Path, "lib", in.lib.Name, err)
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
usedMatched[equivalentMatch.ID] = true
|
||||||
p.totalMatched.Add(1)
|
p.totalMatched.Add(1)
|
||||||
hasMatches = true
|
hasMatches = true
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -241,6 +241,39 @@ var _ = Describe("phaseMissingTracks", func() {
|
|||||||
Expect(movedTrack.Size).To(Equal(missingTrack.Size))
|
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() {
|
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"}}}
|
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"}}}
|
matchedTrack := model.MediaFile{ID: "2", PID: "A", Path: "path1.mp3", Tags: model.Tags{"title": []string{"title1"}}}
|
||||||
|
|||||||
Reference in New Issue
Block a user