From cad9cdc53e7e3f37551df0ceefa936ca531b83c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Tue, 17 Feb 2026 08:37:05 -0500 Subject: [PATCH] fix(scanner): preserve created_at when moving songs between libraries (#5055) * fix: preserve created_at when moving songs between libraries (#5050) When songs are moved between libraries, their creation date was being reset to the current time, causing them to incorrectly appear in "Recently Added". Three changes fix this: 1. Add hash:"ignore" to AlbumID in MediaFile struct so that Equals() works for cross-library moves (AlbumID includes library prefix, making hashes always differ between libraries) 2. Preserve album created_at in moveMatched() via CopyAttributes, matching the pattern already used in persistAlbum() for within-library album ID changes 3. Only set CreatedAt in Put() when it's zero (new files), and explicitly copy missing.CreatedAt to the target in moveMatched() as defense-in-depth for the INSERT code path * test: add regression tests for created_at preservation (#5050) Add tests covering the three aspects of the fix: - Scanner: moveMatched preserves missing track's created_at - Scanner: CopyAttributes called for album created_at on album change - Scanner: CopyAttributes not called when album ID stays the same - Persistence: Put sets CreatedAt to now for new files with zero value - Persistence: Put preserves non-zero CreatedAt on insert - Persistence: Put does not reset CreatedAt on update Also adds CopyAttributes to MockAlbumRepo for test support. * test: verify album created_at is updated in cross-library move test (#5050) Added end-to-end assertion in the cross-library move test to verify that the new album's CreatedAt field is actually set to the original value after CopyAttributes runs, not just that the method was called. This strengthens the test by confirming the mock correctly propagates the timestamp. --- model/mediafile.go | 2 +- persistence/mediafile_repository.go | 4 +- persistence/mediafile_repository_test.go | 62 ++++++++++++ scanner/phase_2_missing_tracks.go | 13 +++ scanner/phase_2_missing_tracks_test.go | 114 +++++++++++++++++++++++ tests/mock_album_repo.go | 27 ++++++ 6 files changed, 220 insertions(+), 2 deletions(-) diff --git a/model/mediafile.go b/model/mediafile.go index a2fec6a1e..831f006bf 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -38,7 +38,7 @@ type MediaFile struct { AlbumArtistID string `structs:"album_artist_id" json:"albumArtistId"` // Deprecated: Use Participants instead // AlbumArtist is the display name used for the album artist. AlbumArtist string `structs:"album_artist" json:"albumArtist"` - AlbumID string `structs:"album_id" json:"albumId"` + AlbumID string `structs:"album_id" json:"albumId" hash:"ignore"` HasCoverArt bool `structs:"has_cover_art" json:"hasCoverArt"` TrackNumber int `structs:"track_number" json:"trackNumber"` DiscNumber int `structs:"disc_number" json:"discNumber"` diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index 9b59a4bd1..617cce4c8 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -148,7 +148,9 @@ func (r *mediaFileRepository) Exists(id string) (bool, error) { } func (r *mediaFileRepository) Put(m *model.MediaFile) error { - m.CreatedAt = time.Now() + if m.CreatedAt.IsZero() { + m.CreatedAt = time.Now() + } id, err := r.putByMatch(Eq{"path": m.Path, "library_id": m.LibraryID}, m.ID, &dbMediaFile{MediaFile: m}) if err != nil { return err diff --git a/persistence/mediafile_repository_test.go b/persistence/mediafile_repository_test.go index 6b4d0db3b..84cfd464b 100644 --- a/persistence/mediafile_repository_test.go +++ b/persistence/mediafile_repository_test.go @@ -104,6 +104,68 @@ var _ = Describe("MediaRepository", func() { } }) + Describe("Put CreatedAt behavior (#5050)", func() { + It("sets CreatedAt to now when inserting a new file with zero CreatedAt", func() { + before := time.Now().Add(-time.Second) + newFile := model.MediaFile{ID: id.NewRandom(), LibraryID: 1, Path: "/test/created-at-zero.mp3"} + Expect(mr.Put(&newFile)).To(Succeed()) + + retrieved, err := mr.Get(newFile.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(retrieved.CreatedAt).To(BeTemporally(">", before)) + + _ = mr.Delete(newFile.ID) + }) + + It("preserves CreatedAt when inserting a new file with non-zero CreatedAt", func() { + originalTime := time.Date(2020, 3, 15, 10, 30, 0, 0, time.UTC) + newFile := model.MediaFile{ + ID: id.NewRandom(), + LibraryID: 1, + Path: "/test/created-at-preserved.mp3", + CreatedAt: originalTime, + } + Expect(mr.Put(&newFile)).To(Succeed()) + + retrieved, err := mr.Get(newFile.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(retrieved.CreatedAt).To(BeTemporally("~", originalTime, time.Second)) + + _ = mr.Delete(newFile.ID) + }) + + It("does not reset CreatedAt when updating an existing file", func() { + originalTime := time.Date(2019, 6, 1, 12, 0, 0, 0, time.UTC) + fileID := id.NewRandom() + newFile := model.MediaFile{ + ID: fileID, + LibraryID: 1, + Path: "/test/created-at-update.mp3", + Title: "Original Title", + CreatedAt: originalTime, + } + Expect(mr.Put(&newFile)).To(Succeed()) + + // Update the file with a new title but zero CreatedAt + updatedFile := model.MediaFile{ + ID: fileID, + LibraryID: 1, + Path: "/test/created-at-update.mp3", + Title: "Updated Title", + // CreatedAt is zero - should NOT overwrite the stored value + } + Expect(mr.Put(&updatedFile)).To(Succeed()) + + retrieved, err := mr.Get(fileID) + Expect(err).ToNot(HaveOccurred()) + Expect(retrieved.Title).To(Equal("Updated Title")) + // CreatedAt should still be the original time (not reset) + Expect(retrieved.CreatedAt).To(BeTemporally("~", originalTime, time.Second)) + + _ = mr.Delete(fileID) + }) + }) + It("checks existence of mediafiles in the DB", func() { Expect(mr.Exists(songAntenna.ID)).To(BeTrue()) Expect(mr.Exists("666")).To(BeFalse()) diff --git a/scanner/phase_2_missing_tracks.go b/scanner/phase_2_missing_tracks.go index 023944d00..c47565036 100644 --- a/scanner/phase_2_missing_tracks.go +++ b/scanner/phase_2_missing_tracks.go @@ -2,6 +2,7 @@ package scanner import ( "context" + "errors" "fmt" "sync" "sync/atomic" @@ -267,6 +268,10 @@ func (p *phaseMissingTracks) moveMatched(target, missing model.MediaFile) error oldAlbumID := missing.AlbumID newAlbumID := target.AlbumID + // Preserve the original created_at from the missing file, so moved tracks + // don't appear in "Recently Added" + target.CreatedAt = missing.CreatedAt + // Update the target media file with the missing file's ID. This effectively "moves" the track // to the new location while keeping its annotations and references intact. target.ID = missing.ID @@ -298,6 +303,14 @@ func (p *phaseMissingTracks) moveMatched(target, missing model.MediaFile) error log.Warn(p.ctx, "Scanner: Could not reassign album annotations", "from", oldAlbumID, "to", newAlbumID, err) } + // Keep created_at field from previous instance of the album, so moved albums + // don't appear in "Recently Added" + if err := tx.Album(p.ctx).CopyAttributes(oldAlbumID, newAlbumID, "created_at"); err != nil { + if !errors.Is(err, model.ErrNotFound) { + log.Warn(p.ctx, "Scanner: Could not copy album created_at", "from", oldAlbumID, "to", newAlbumID, err) + } + } + // Note: RefreshPlayCounts will be called in later phases, so we don't need to call it here p.processedAlbumAnnotations[newAlbumID] = true } diff --git a/scanner/phase_2_missing_tracks_test.go b/scanner/phase_2_missing_tracks_test.go index 6c25ec7e8..fa6ef5724 100644 --- a/scanner/phase_2_missing_tracks_test.go +++ b/scanner/phase_2_missing_tracks_test.go @@ -724,6 +724,120 @@ var _ = Describe("phaseMissingTracks", func() { }) // 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 diff --git a/tests/mock_album_repo.go b/tests/mock_album_repo.go index 642ce6b41..8b5f5d9c1 100644 --- a/tests/mock_album_repo.go +++ b/tests/mock_album_repo.go @@ -21,6 +21,7 @@ type MockAlbumRepo struct { Err bool Options model.QueryOptions ReassignAnnotationCalls map[string]string // prevID -> newID + CopyAttributesCalls map[string]string // fromID -> toID } func (m *MockAlbumRepo) SetError(err bool) { @@ -142,6 +143,32 @@ func (m *MockAlbumRepo) ReassignAnnotation(prevID string, newID string) error { return nil } +// CopyAttributes copies attributes from one album to another +func (m *MockAlbumRepo) CopyAttributes(fromID, toID string, columns ...string) error { + if m.Err { + return errors.New("unexpected error") + } + from, ok := m.Data[fromID] + if !ok { + return model.ErrNotFound + } + to, ok := m.Data[toID] + if !ok { + return model.ErrNotFound + } + for _, col := range columns { + switch col { + case "created_at": + to.CreatedAt = from.CreatedAt + } + } + if m.CopyAttributesCalls == nil { + m.CopyAttributesCalls = make(map[string]string) + } + m.CopyAttributesCalls[fromID] = toID + return nil +} + // SetRating sets the rating for an album func (m *MockAlbumRepo) SetRating(rating int, itemID string) error { if m.Err {