mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
fix(core): keep the target track's history when remapping a missing file onto it
The remap discards the target's row, and GC then dropped its play counts, stars, ratings, bookmarks and every playlist entry pointing at it. That is harmless in the scanner, whose target was imported seconds earlier, but the CLI lets the user pick any existing track. Move those references onto the surviving id first; where a user already has a row for both, theirs on the missing file wins.
This commit is contained in:
parent
b4addc59c7
commit
497ab102f2
@ -88,7 +88,10 @@ func (s *maintenanceService) RemapMissingFile(ctx context.Context, missingID, ta
|
||||
if err := tx.MediaFile(ctx).Put(target); err != nil {
|
||||
return fmt.Errorf("update matched track: %w", err)
|
||||
}
|
||||
// Discard the target's original row
|
||||
// Unlike the scanner's freshly-imported target, this one may carry history of its own
|
||||
if err := tx.MediaFile(ctx).ReassignReferences(discardedID, missing.ID); err != nil {
|
||||
return fmt.Errorf("reassign target references: %w", err)
|
||||
}
|
||||
if err := tx.MediaFile(ctx).Delete(discardedID); err != nil {
|
||||
return fmt.Errorf("delete discarded track: %w", err)
|
||||
}
|
||||
|
||||
@ -272,6 +272,17 @@ var _ = Describe("Maintenance", func() {
|
||||
Expect(ds.GCCalled).To(BeTrue())
|
||||
})
|
||||
|
||||
It("moves the target's annotations, bookmarks and playlist entries onto the surviving id", func() {
|
||||
mfRepo.SetData(model.MediaFiles{
|
||||
{ID: "m1", AlbumID: "album1", Missing: true},
|
||||
{ID: "t1", AlbumID: "album1", Missing: false},
|
||||
})
|
||||
|
||||
Expect(service.RemapMissingFile(ctx, "m1", "t1")).To(Succeed())
|
||||
|
||||
Expect(mfRepo.ReassignReferencesCalls).To(HaveKeyWithValue("t1", "m1"))
|
||||
})
|
||||
|
||||
It("reassigns album annotations when the old album is emptied", func() {
|
||||
albumRepo := ds.MockedAlbum.(*extendedAlbumRepo)
|
||||
mfRepo.SetData(model.MediaFiles{
|
||||
|
||||
@ -563,6 +563,9 @@ type MediaFileRepository interface {
|
||||
DeleteMissing(ids []string) error
|
||||
DeleteAllMissing() (int64, error)
|
||||
FindByPaths(paths []string) (MediaFiles, error)
|
||||
// ReassignReferences moves annotations, bookmarks and playlist entries from prevID to newID,
|
||||
// keeping newID's own row wherever a user has both.
|
||||
ReassignReferences(prevID, newID string) error
|
||||
|
||||
// The following methods are used exclusively by the scanner:
|
||||
MarkMissing(bool, ...*MediaFile) error
|
||||
|
||||
@ -405,6 +405,20 @@ func (r *mediaFileRepository) Delete(id string) error {
|
||||
return r.delete(Eq{"id": id})
|
||||
}
|
||||
|
||||
func (r *mediaFileRepository) ReassignReferences(prevID, newID string) error {
|
||||
if err := r.ReassignAnnotation(prevID, newID); err != nil {
|
||||
return fmt.Errorf("reassigning annotations: %w", err)
|
||||
}
|
||||
if err := r.reassignBookmark(prevID, newID); err != nil {
|
||||
return fmt.Errorf("reassigning bookmarks: %w", err)
|
||||
}
|
||||
upd := Update("playlist_tracks").Set("media_file_id", newID).Where(Eq{"media_file_id": prevID})
|
||||
if _, err := r.executeSQL(upd); err != nil {
|
||||
return fmt.Errorf("reassigning playlist tracks: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *mediaFileRepository) DeleteAllMissing() (int64, error) {
|
||||
user := loggedUser(r.ctx)
|
||||
if !user.IsAdmin {
|
||||
|
||||
@ -918,6 +918,68 @@ var _ = Describe("MediaRepository", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("ReassignReferences", func() {
|
||||
var prev, next model.MediaFile
|
||||
var pr model.PlaylistRepository
|
||||
var pls model.Playlist
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx := request.WithUser(log.NewContext(context.TODO()), model.User{ID: "userid"})
|
||||
pr = NewPlaylistRepository(ctx, GetDBXBuilder())
|
||||
prev = model.MediaFile{ID: "reassign-prev", LibraryID: 1, Path: "reassign/prev.mp3", Title: "Prev"}
|
||||
next = model.MediaFile{ID: "reassign-next", LibraryID: 1, Path: "reassign/next.mp3", Title: "Next"}
|
||||
Expect(mr.Put(&prev)).To(Succeed())
|
||||
Expect(mr.Put(&next)).To(Succeed())
|
||||
pls = model.Playlist{Name: "Reassign", OwnerID: "userid"}
|
||||
pls.AddMediaFilesByID([]string{prev.ID})
|
||||
Expect(pr.Put(&pls)).To(Succeed())
|
||||
})
|
||||
|
||||
AfterEach(func() {
|
||||
_ = pr.Delete(pls.ID)
|
||||
_ = mr.Delete(prev.ID)
|
||||
_ = mr.Delete(next.ID)
|
||||
_, _ = mr.(*mediaFileRepository).executeSQL(squirrel.Delete("annotation").Where(squirrel.Eq{"item_id": []string{prev.ID, next.ID}}))
|
||||
_, _ = mr.(*mediaFileRepository).executeSQL(squirrel.Delete("bookmark").Where(squirrel.Eq{"item_id": []string{prev.ID, next.ID}}))
|
||||
})
|
||||
|
||||
It("moves annotations, bookmarks and playlist entries onto the new id", func() {
|
||||
Expect(mr.SetRating(5, prev.ID)).To(Succeed())
|
||||
Expect(mr.AddBookmark(prev.ID, "here", 42)).To(Succeed())
|
||||
|
||||
Expect(mr.ReassignReferences(prev.ID, next.ID)).To(Succeed())
|
||||
|
||||
got, err := mr.Get(next.ID)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(got.Rating).To(Equal(5))
|
||||
|
||||
bookmarks, err := mr.GetBookmarks()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(bookmarks).To(ContainElement(HaveField("Item.ID", next.ID)))
|
||||
|
||||
withTracks, err := pr.GetWithTracks(pls.ID, false, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(withTracks.Tracks).To(HaveLen(1))
|
||||
Expect(withTracks.Tracks[0].MediaFileID).To(Equal(next.ID))
|
||||
})
|
||||
|
||||
It("keeps the new id's own annotation and bookmark when both exist", func() {
|
||||
Expect(mr.SetRating(5, prev.ID)).To(Succeed())
|
||||
Expect(mr.SetRating(1, next.ID)).To(Succeed())
|
||||
Expect(mr.AddBookmark(prev.ID, "prev", 42)).To(Succeed())
|
||||
Expect(mr.AddBookmark(next.ID, "next", 7)).To(Succeed())
|
||||
|
||||
Expect(mr.ReassignReferences(prev.ID, next.ID)).To(Succeed())
|
||||
|
||||
got, err := mr.Get(next.ID)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(got.Rating).To(Equal(1))
|
||||
bookmarks, err := mr.GetBookmarks()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(bookmarks).To(ContainElement(SatisfyAll(HaveField("Item.ID", next.ID), HaveField("Comment", "next"))))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("FindByPaths", func() {
|
||||
// Test fixtures for Unicode and case-sensitivity tests
|
||||
var testFiles []model.MediaFile
|
||||
|
||||
@ -144,6 +144,13 @@ func (r sqlRepository) GetBookmarks() (model.Bookmarks, error) {
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
func (r sqlRepository) reassignBookmark(prevID, newID string) error {
|
||||
upd := Expr("update or ignore "+bookmarkTable+" set item_id = ? where item_type = ? and item_id = ?",
|
||||
newID, r.tableName, prevID)
|
||||
_, err := r.executeSQL(upd)
|
||||
return err
|
||||
}
|
||||
|
||||
func (r sqlRepository) cleanBookmarks() error {
|
||||
del := Delete(bookmarkTable).Where(Eq{"item_type": r.tableName}).Where("item_id not in (select id from " + r.tableName + ")")
|
||||
c, err := r.executeSQL(del)
|
||||
|
||||
@ -29,7 +29,9 @@ type MockMediaFileRepo struct {
|
||||
CountAllValue *int64
|
||||
CountAllOptions model.QueryOptions
|
||||
DeleteAllMissingValue int64
|
||||
Options model.QueryOptions
|
||||
// ReassignReferencesCalls records prevID -> newID
|
||||
ReassignReferencesCalls map[string]string
|
||||
Options model.QueryOptions
|
||||
// Add fields for cross-library move detection tests
|
||||
FindRecentFilesByMBZTrackIDFunc func(missing model.MediaFile, since time.Time) (model.MediaFiles, error)
|
||||
FindRecentFilesByPropertiesFunc func(missing model.MediaFile, since time.Time) (model.MediaFiles, error)
|
||||
@ -168,6 +170,17 @@ func (m *MockMediaFileRepo) Delete(id string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) ReassignReferences(prevID, newID string) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
}
|
||||
if m.ReassignReferencesCalls == nil {
|
||||
m.ReassignReferencesCalls = make(map[string]string)
|
||||
}
|
||||
m.ReassignReferencesCalls[prevID] = newID
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) IncPlayCount(id string, timestamp time.Time) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user