diff --git a/db/migrations/20260720015443_uniform_canonical_ids.go b/db/migrations/20260720015443_uniform_canonical_ids.go index 69d0907c1..346f22050 100644 --- a/db/migrations/20260720015443_uniform_canonical_ids.go +++ b/db/migrations/20260720015443_uniform_canonical_ids.go @@ -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. diff --git a/db/migrations/id_canonical_test.go b/db/migrations/id_canonical_test.go index 55cf8160a..0befbe469 100644 --- a/db/migrations/id_canonical_test.go +++ b/db/migrations/id_canonical_test.go @@ -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"} { diff --git a/db/migrations/id_columns_guard_test.go b/db/migrations/id_columns_guard_test.go index 98c44184f..c4cb3b241 100644 --- a/db/migrations/id_columns_guard_test.go +++ b/db/migrations/id_columns_guard_test.go @@ -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_") {