diff --git a/db/migrations/20260410201914_fix_zero_album_created_at.sql b/db/migrations/20260410201914_fix_zero_album_created_at.sql new file mode 100644 index 000000000..ff47eb95f --- /dev/null +++ b/db/migrations/20260410201914_fix_zero_album_created_at.sql @@ -0,0 +1,22 @@ +-- +goose Up + +-- Backfill album.created_at for rows poisoned by early scanner versions or +-- propagated via CopyAttributes during metadata-driven ID changes. Prefer the +-- oldest valid birth_time from the album's media files, fall back to updated_at. +UPDATE album +SET created_at = COALESCE( + (SELECT MIN(birth_time) + FROM media_file + WHERE media_file.album_id = album.id + AND birth_time IS NOT NULL + AND birth_time != '' + AND birth_time NOT LIKE '0001-%'), + updated_at +) +WHERE created_at IS NULL + OR created_at = '' + OR created_at LIKE '0001-%'; + +-- +goose Down + +SELECT 1; diff --git a/model/mediafile.go b/model/mediafile.go index ec83b76fd..6be8402ae 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -361,6 +361,9 @@ func older(t1, t2 time.Time) time.Time { if t1.IsZero() { return t2 } + if t2.IsZero() { + return t1 + } if t1.After(t2) { return t2 } diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 038ac93d5..8b0c13da2 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -119,6 +119,20 @@ var _ = Describe("MediaFiles", func() { Expect(a.MinYear).To(Equal(1999)) }) }) + Context("CreatedAt aggregation", func() { + It("ignores zero BirthTime values when computing the oldest", func() { + mfs = MediaFiles{ + {BirthTime: t("2022-12-19 08:30")}, + {BirthTime: time.Time{}}, + {BirthTime: t("2022-12-18 10:00")}, + } + Expect(mfs.ToAlbum().CreatedAt).To(Equal(t("2022-12-18 10:00"))) + }) + It("returns zero when all BirthTime values are zero", func() { + mfs = MediaFiles{{BirthTime: time.Time{}}, {BirthTime: time.Time{}}} + Expect(mfs.ToAlbum().CreatedAt).To(BeZero()) + }) + }) }) When("we have multiple songs with same dates", func() { BeforeEach(func() { diff --git a/persistence/album_repository.go b/persistence/album_repository.go index c51a5beb1..99ed10877 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -252,7 +252,17 @@ func (r *albumRepository) CopyAttributes(fromID, toID string, columns ...string) } to := make(map[string]any) for _, col := range columns { - to[col] = from[col] + v := from[col] + // created_at is aggregated from song birth_times and must never be + // overwritten with a zero/poisoned value, or it propagates forward on + // every metadata-driven album ID change. + if col == "created_at" && (!v.Valid || v.String == "" || strings.HasPrefix(v.String, "0001-")) { + continue + } + to[col] = v + } + if len(to) == 0 { + return nil } _, err = r.executeSQL(Update(r.tableName).SetMap(to).Where(Eq{"id": toID})) return err diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 2792cec97..a6270933f 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -41,6 +41,32 @@ var _ = Describe("AlbumRepository", func() { }) }) + Describe("CopyAttributes", func() { + var srcTime, dstTime time.Time + BeforeEach(func() { + srcTime = time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC) + dstTime = time.Date(2024, 6, 7, 8, 9, 10, 0, time.UTC) + Expect(albumRepo.Put(&model.Album{ID: "copy-src", Name: "src", LibraryID: 1, CreatedAt: srcTime})).To(Succeed()) + Expect(albumRepo.Put(&model.Album{ID: "copy-dst", Name: "dst", LibraryID: 1, CreatedAt: dstTime})).To(Succeed()) + Expect(albumRepo.Put(&model.Album{ID: "copy-zero", Name: "zero", LibraryID: 1})).To(Succeed()) + DeferCleanup(func() { + _, _ = albumRepo.executeSQL(squirrel.Delete("album").Where(squirrel.Eq{"id": []string{"copy-src", "copy-dst", "copy-zero"}})) + }) + }) + It("copies a valid created_at from source to destination", func() { + Expect(albumRepo.CopyAttributes("copy-src", "copy-dst", "created_at")).To(Succeed()) + got, err := albumRepo.Get("copy-dst") + Expect(err).ToNot(HaveOccurred()) + Expect(got.CreatedAt).To(BeTemporally("~", srcTime, time.Second)) + }) + It("leaves destination untouched when source created_at is zero", func() { + Expect(albumRepo.CopyAttributes("copy-zero", "copy-dst", "created_at")).To(Succeed()) + got, err := albumRepo.Get("copy-dst") + Expect(err).ToNot(HaveOccurred()) + Expect(got.CreatedAt).To(BeTemporally("~", dstTime, time.Second)) + }) + }) + Describe("GetAll", func() { var GetAll = func(opts ...model.QueryOptions) (model.Albums, error) { albums, err := albumRepo.GetAll(opts...) diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index e930aa630..ffa10898e 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -10,6 +10,7 @@ import ( "slices" "sort" "strings" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" @@ -17,6 +18,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" + . "github.com/navidrome/navidrome/utils/gg" "github.com/navidrome/navidrome/utils/number" "github.com/navidrome/navidrome/utils/req" "github.com/navidrome/navidrome/utils/slice" @@ -317,6 +319,20 @@ func sanitizeSlashes(target string) string { return strings.ReplaceAll(target, "/", "_") } +// albumCreatedAt returns a best-effort timestamp for the album's `created` +// field, which is required by the OpenSubsonic spec but may be zero on legacy +// DB rows. Falls back to UpdatedAt → ImportedAt; can still return zero if all +// three are unset. +func albumCreatedAt(al model.Album) time.Time { + if !al.CreatedAt.IsZero() { + return al.CreatedAt + } + if !al.UpdatedAt.IsZero() { + return al.UpdatedAt + } + return al.ImportedAt +} + func childFromAlbum(ctx context.Context, al model.Album) responses.Child { child := responses.Child{} child.Id = al.ID @@ -329,7 +345,7 @@ func childFromAlbum(ctx context.Context, al model.Album) responses.Child { child.Year = int32(cmp.Or(al.MaxOriginalYear, al.MaxYear)) child.Genre = al.Genre child.CoverArt = al.CoverArtID().String() - child.Created = &al.CreatedAt + child.Created = P(albumCreatedAt(al)) child.Parent = al.AlbumArtistID child.ArtistId = al.AlbumArtistID child.Duration = int32(al.Duration) @@ -421,9 +437,7 @@ func buildAlbumID3(ctx context.Context, album model.Album) responses.AlbumID3 { dir.PlayCount = album.PlayCount dir.Year = int32(cmp.Or(album.MaxOriginalYear, album.MaxYear)) dir.Genre = album.Genre - if !album.CreatedAt.IsZero() { - dir.Created = &album.CreatedAt - } + dir.Created = P(albumCreatedAt(album)) if album.Starred { dir.Starred = album.StarredAt } diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index 4eb756b98..abf6116f3 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -571,6 +571,38 @@ var _ = Describe("helpers", func() { }) }) + Describe("buildAlbumID3 Created field", func() { + It("uses CreatedAt when set", func() { + t := time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC) + al := model.Album{ID: "a1", Name: "A", CreatedAt: t} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + Expect(*dir.Created).To(Equal(t)) + }) + + It("falls back to UpdatedAt when CreatedAt is zero", func() { + updated := time.Date(2019, 5, 6, 7, 8, 9, 0, time.UTC) + al := model.Album{ID: "a2", Name: "A", UpdatedAt: updated} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + Expect(*dir.Created).To(Equal(updated)) + }) + + It("falls back to ImportedAt when CreatedAt and UpdatedAt are zero", func() { + imported := time.Date(2021, 8, 9, 10, 11, 12, 0, time.UTC) + al := model.Album{ID: "a3", Name: "A", ImportedAt: imported} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + Expect(*dir.Created).To(Equal(imported)) + }) + + It("never leaves Created nil even when all timestamps are zero", func() { + al := model.Album{ID: "a4", Name: "A"} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + }) + }) + Describe("EnableAverageRating config", func() { It("excludes averageRating when disabled", func() { conf.Server.Subsonic.EnableAverageRating = false