nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-31 07:30:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(db): make the id-columns guard classify JSON columns too

Browse Source 
The guard only inspected columns named id/pid/*_id, so it could not see ids
embedded in JSON. Widen it to *_ids and to every JSON column, and drive the
"covered" set from a new embeddedIDColumns list instead of the inline calls
in the migration.

Every JSON column the schema has now carries a verdict. The four denormalized
caches -- media_file/album.participants, media_file/album.tags,
album.folder_ids and artist.similar_artists -- hold only artist, tag and
folder ids. Those all come from id.NewHash, whose 22-char base62 encoding of
a 128-bit MD5 is already in canonical range, so canonicalID is the identity
on them and the migration correctly leaves them alone. A new codec test pins
that invariant, since the exemptions depend on it.

Verified on a copy of a 727MB/96k-track production database: canonicalizing
those four columns changed zero rows, and artist, tag and folder ids were
themselves unchanged by the migration (only media_file ids moved, 95108 of
96666).
This commit is contained in:
Deluan 2026-07-29 23:14:43 -04:00
parent 90b258fdeb
commit d61a670302
3 changed files with 59 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
db/migrations/20260720015443_uniform_canonical_ids.go
View File

@ -79,6 +79,18 @@ var idColumns = []struct{ table, col string }{
{"library_tag", "tag_id"},
}
// embeddedIDColumns holds ids nested inside a larger value; the id-columns guard checks this
// list against the schema, so every JSON column must appear here or be exempted there.
var embeddedIDColumns = []struct {
table, col string
transform func(string) (string, bool)
}{
{"playqueue", "items", canonicalizeIDList},
{"share", "resource_ids", canonicalizeIDList},
{"plugin", "users", canonicalizePluginUsers},
{"playlist", "rules", canonicalizePlaylistRules},
}
func upUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error {
if err := buildIDMap(ctx, tx); err != nil {
return err
@ -88,17 +100,10 @@ func upUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error {
return fmt.Errorf("canonicalizing %s.%s: %w", tc.table, tc.col, err)
}
}
if err := rewriteColumn(ctx, tx, "playqueue", "items", canonicalizeIDList); err != nil {
return err
}
if err := rewriteColumn(ctx, tx, "share", "resource_ids", canonicalizeIDList); err != nil {
return err
}
if err := rewriteColumn(ctx, tx, "plugin", "users", canonicalizePluginUsers); err != nil {
return err
}
if err := rewriteColumn(ctx, tx, "playlist", "rules", canonicalizePlaylistRules); err != nil {
return err
for _, tc := range embeddedIDColumns {
if err := rewriteColumn(ctx, tx, tc.table, tc.col, tc.transform); err != nil {
return fmt.Errorf("canonicalizing %s.%s: %w", tc.table, tc.col, err)
}
}
// Legacy PID specs embed old-shaped album/track ids into composite pids; a full rescan
// rewrites every pid with the new encoding so path-based move matching stays consistent.

15
db/migrations/id_canonical_test.go
View File

@ -1,6 +1,9 @@
package migrations
import (
"strings"
"github.com/navidrome/navidrome/model/id"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -22,6 +25,18 @@ var _ = Describe("canonicalID", func() {
Entry("36 chars without uuid dashes passes through", "000000000000000000000000000000000000", "000000000000000000000000000000000000"),
)
// The exemptions for participants/tags/folder_ids/similar_artists rest on this invariant.
It("is the identity on every NewHash id", func() {
for _, parts := range [][]string{
{""}, {"a"}, {"The Beatles"}, {"genre", "electronic"},
{"/music/Artist/Album", "1"}, {strings.Repeat("x", 500)},
} {
h := id.NewHash(parts...)
Expect(h).To(HaveLen(22))
Expect(canonicalID(h)).To(Equal(h), "NewHash(%v) = %q was rewritten", parts, h)
}
})
It("is idempotent for every shape", func() {
for _, s := range []string{"5cLJPkLA5DK2BADhoeotPk", "zzzzzzzzzzzzzzzzzzzzzz",
"e3b7fc2ae9447bbec37a13bf916e3cf6", "f47ac10b-58cc-4372-a567-0e02b2c3d479"} {

35
db/migrations/id_columns_guard_test.go
View File

@ -35,11 +35,28 @@ var _ = Describe("idColumns inventory", func() {
for _, tc := range idColumns {
covered[tc.table+"."+tc.col] = true
}
// Columns that are id-named but intentionally not Navidrome canonical ids.
for _, tc := range embeddedIDColumns {
covered[tc.table+"."+tc.col] = true
}
// Columns that are id-named or JSON but need no rewrite.
exempt := map[string]string{
"share.id": "public share URLs, generated separately",
"property.id": "property key, not an entity id",
"plugin.id": "plugin name, not an entity id",
"share.id": "public share URLs, generated separately",
"property.id": "property key, not an entity id",
"plugin.id": "plugin name, not an entity id",
"media_file.participants": "hash-family artist ids, unchanged",
"album.participants": "hash-family artist ids, unchanged",
"media_file.tags": "hash-family tag ids, unchanged",
"album.tags": "hash-family tag ids, unchanged",
"album.folder_ids": "hash-family folder ids, unchanged",
"artist.similar_artists": "hash-family artist ids, unchanged",
"media_file.search_participants": "participant names for FTS, not ids",
"album.search_participants": "participant names for FTS, not ids",
"album.discs": "disc number -> title map",
"media_file.lyrics": "synced lyrics, no ids",
"folder.image_files": "image file names, no ids",
"plugin.manifest": "plugin-authored manifest, no Navidrome ids",
"plugin.config": "free-form plugin config, must not be rewritten",
"plugin.libraries": "integer library ids",
}
tables, err := queryColumn(ctx, db, "SELECT name FROM sqlite_master WHERE type='table'")
@ -56,11 +73,15 @@ var _ = Describe("idColumns inventory", func() {
var name, typ string
Expect(rows.Scan(&name, &typ)).To(Succeed())
lname := strings.ToLower(name)
if lname != "id" && lname != "pid" && !strings.HasSuffix(lname, "_id") {
utyp := strings.ToUpper(typ)
// JSON can hide an id under any key, so every JSON column needs a verdict.
isJSON := strings.Contains(utyp, "JSON")
isIDName := lname == "id" || lname == "pid" ||
strings.HasSuffix(lname, "_id") || strings.HasSuffix(lname, "_ids")
if !isJSON && !isIDName {
continue
}
utyp := strings.ToUpper(typ)
if !strings.Contains(utyp, "TEXT") && !strings.Contains(utyp, "CHAR") {
if !isJSON && !strings.Contains(utyp, "TEXT") && !strings.Contains(utyp, "CHAR") {
continue // INTEGER ids (rowid PKs, library.id) are not canonical ids
}
if strings.HasPrefix(lname, "mbz_") {