mirror of
https://github.com/navidrome/navidrome.git
synced 2026-05-03 06:51:16 +00:00
9b0bfc606b
* 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>
23 lines
609 B
SQL
23 lines
609 B
SQL
-- +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;
|