fix(subsonic): always emit required created field on AlbumID3 (#5340)

* fix(subsonic): always emit required `created` field on AlbumID3

Strict OpenSubsonic clients (e.g. Navic via dev.zt64.subsonic) reject
search3/getAlbum/getAlbumList2 responses that omit the `created` field,
which the spec marks as required. Navidrome was dropping it whenever
the album's CreatedAt was zero.

Root cause was threefold:

1. buildAlbumID3/childFromAlbum conditionally emitted `created`, so a
   zero CreatedAt became a missing JSON key.
2. ToAlbum's `older()` helper treated a zero BirthTime as the minimum,
   so a single track with missing filesystem birth time could poison
   the album aggregation.
3. phase_1_folders' CopyAttributes copied `created_at` from the previous
   album row unconditionally, propagating an already-zero value forward
   on every metadata-driven album ID change. Since sql_base_repository
   drops `created_at` on UPDATE, a poisoned row could never self-heal.

Fixes:
- Always emit `created`, falling back to UpdatedAt/ImportedAt when
  CreatedAt is zero. Adds albumCreatedAt() helper used by both
  buildAlbumID3 and childFromAlbum.
- Guard `older()` against a zero second argument.
- Skip the CopyAttributes call in phase_1_folders when the previous
  album's created_at is zero, so the freshly-computed value survives.
- New migration backfills existing broken rows from media_file.birth_time
  (falling back to updated_at).

Tested against a real DB: repaired 605/6922 affected rows, no side
effects on healthy rows.

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(subsonic): return albumCreatedAt by value to avoid heap escape

Returning *time.Time from albumCreatedAt caused Go escape analysis to
move the entire model.Album parameter to the heap, since the returned
pointer aliased a field of the value receiver. For hot endpoints like
getAlbumList2 and search3, this meant one full-struct heap allocation
per album result.

Return time.Time by value and let callers wrap it with gg.P() to take
the address locally. Only the small time.Time value escapes; the
model.Album struct stays on the stack. Also corrects the doc comment
to reflect the actual guarantee ("best-effort" rather than "non-zero"),
matching the test case that exercises the all-zero fallback.

---------

Signed-off-by: Deluan <deluan@navidrome.org>

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;

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
 	}

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() {

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

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...)

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
 	}

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