diff --git a/db/migrations/20260729140000_renormalize_album_created_at.sql b/db/migrations/20260729140000_renormalize_album_created_at.sql new file mode 100644 index 000000000..32252bd3e --- /dev/null +++ b/db/migrations/20260729140000_renormalize_album_created_at.sql @@ -0,0 +1,19 @@ +-- +goose Up + +-- Repairs album.created_at values stored in RFC3339 T-format ("2026-07-24T17:05:39.706028243Z"). +-- 20260316000000_normalize_timestamps already did a one-off pass over every timestamp column, but +-- albumRepository.CopyAttributes kept re-introducing the T-format on this one column: it read the +-- value into a Go string (go-sqlite3 decodes `datetime` columns as time.Time, and database/sql then +-- formats that as RFC3339Nano) and wrote it straight back. It runs whenever an album ID changes +-- because its tags were edited, which is routine for beets/Picard users. +-- +-- Since "Recently Added" compares these timestamps as raw strings (see +-- 20260629123100_recently_added_plain_indexes) and 'T' (ASCII 84) sorts above ' ' (ASCII 32), +-- every affected album stays pinned to the top of the list. + +UPDATE album SET created_at = replace(replace(created_at, 'T', ' '), 'Z', '+00:00') +WHERE created_at LIKE '%T%'; + +-- +goose Down + +SELECT 1; diff --git a/persistence/album_repository.go b/persistence/album_repository.go index 2bb541003..7cf99419a 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -278,26 +278,32 @@ func (r *albumRepository) GetYears(libraryIDs ...int) ([]int, error) { } func (r *albumRepository) CopyAttributes(fromID, toID string, columns ...string) error { - var from dbx.NullStringMap - err := r.queryOne(Select(columns...).From(r.tableName).Where(Eq{"id": fromID}), &from) + if len(columns) == 0 { + return nil + } + var from struct{ ID string } + err := r.queryOne(Select("id").From(r.tableName).Where(Eq{"id": fromID}), &from) if err != nil { return fmt.Errorf("getting album to copy fields from: %w", err) } - to := make(map[string]any) + // The values are copied entirely in SQL, never round-tripped through Go: go-sqlite3 decodes + // `datetime` columns into time.Time, and scanning that into a string reformats it as RFC3339 + // ("2026-07-24T17:05:39.706028243Z"). Writing it back would store a T-separated timestamp, + // which string-sorts above the space-separated format the driver writes, pinning the album to + // the top of "Recently Added" forever. + upd := Update(r.tableName).Where(Eq{"id": toID}) for _, col := range columns { - 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 + src := fmt.Sprintf("(select %[1]s from %[2]s where id = ?)", col, r.tableName) + if col == "created_at" { + // 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. + src = fmt.Sprintf("coalesce(nullif((select %[1]s from %[2]s where id = ? and %[1]s not like '0001-%%'), ''), %[1]s)", + col, r.tableName) } - to[col] = v + upd = upd.Set(col, Expr(src, fromID)) } - if len(to) == 0 { - return nil - } - _, err = r.executeSQL(Update(r.tableName).SetMap(to).Where(Eq{"id": toID})) + _, err = r.executeSQL(upd) return err } diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 465617b10..fadb03333 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -17,6 +17,16 @@ import ( . "github.com/onsi/gomega" ) +// rawColumn returns a column exactly as stored, bypassing go-sqlite3's decoding of +// `datetime` columns into time.Time. +func rawColumn(r sqlRepository, id, column string) string { + var res struct{ Value string } + sel := squirrel.Select("cast(" + column + " as text) as value"). + From(r.tableName).Where(squirrel.Eq{"id": id}) + ExpectWithOffset(1, r.queryOne(sel, &res)).To(Succeed()) + return res.Value +} + var _ = Describe("AlbumRepository", func() { var albumRepo *albumRepository @@ -66,6 +76,22 @@ var _ = Describe("AlbumRepository", func() { Expect(err).ToNot(HaveOccurred()) Expect(got.CreatedAt).To(BeTemporally("~", dstTime, time.Second)) }) + It("returns not found and leaves destination untouched when source does not exist", func() { + err := albumRepo.CopyAttributes("copy-missing", "copy-dst", "created_at") + Expect(errors.Is(err, model.ErrNotFound)).To(BeTrue()) + got, getErr := albumRepo.Get("copy-dst") + Expect(getErr).ToNot(HaveOccurred()) + Expect(got.CreatedAt).To(BeTemporally("~", dstTime, time.Second)) + }) + It("keeps the copied created_at in the driver's space-separated format", func() { + // Copying through a Go string would rewrite it as RFC3339 ("2020-01-02T03:04:05Z"), + // which string-sorts above every space-format timestamp and pins the album to the + // top of "Recently Added". + Expect(albumRepo.CopyAttributes("copy-src", "copy-dst", "created_at")).To(Succeed()) + Expect(rawColumn(albumRepo.sqlRepository, "copy-dst", "created_at")). + To(Equal(rawColumn(albumRepo.sqlRepository, "copy-src", "created_at"))) + Expect(rawColumn(albumRepo.sqlRepository, "copy-dst", "created_at")).ToNot(ContainSubstring("T")) + }) }) Describe("GetCursor", func() {