From 0623694c0a5a1e5cd22597704da2b509f0b3537f Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 20 Jul 2026 07:49:39 -0400 Subject: [PATCH] refactor(ids): compile-time Encode128 contract and unified column rewrite helper --- .../20260720015443_uniform_canonical_ids.go | 63 +++++-------------- db/migrations/id_canonical.go | 6 +- model/id/id.go | 13 ++-- model/id/id_test.go | 8 +-- model/metadata/legacy_ids.go | 4 +- 5 files changed, 30 insertions(+), 64 deletions(-) diff --git a/db/migrations/20260720015443_uniform_canonical_ids.go b/db/migrations/20260720015443_uniform_canonical_ids.go index a55b0932f..a0fdd9e4d 100644 --- a/db/migrations/20260720015443_uniform_canonical_ids.go +++ b/db/migrations/20260720015443_uniform_canonical_ids.go @@ -53,16 +53,16 @@ func upUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error { return fmt.Errorf("canonicalizing %s.%s: %w", tc.table, tc.col, err) } } - if err := rewriteListColumn(ctx, tx, "playqueue", "items"); err != nil { + if err := rewriteColumn(ctx, tx, "playqueue", "items", canonicalizeIDList); err != nil { return err } - if err := rewriteListColumn(ctx, tx, "share", "resource_ids"); err != nil { + if err := rewriteColumn(ctx, tx, "share", "resource_ids", canonicalizeIDList); err != nil { return err } - if err := rewriteJSONColumn(ctx, tx, "plugin", "users", canonicalizePluginUsers); err != nil { + if err := rewriteColumn(ctx, tx, "plugin", "users", canonicalizePluginUsers); err != nil { return err } - if err := rewriteJSONColumn(ctx, tx, "playlist", "rules", canonicalizePlaylistRules); err != nil { + if err := rewriteColumn(ctx, tx, "playlist", "rules", canonicalizePlaylistRules); err != nil { return err } // Legacy PID specs embed old-shaped album/track ids into composite pids; a full rescan @@ -124,53 +124,24 @@ func applyIDMap(ctx context.Context, tx *sql.Tx, table, col string) error { return err } -// rewriteListColumn canonicalizes comma-separated id lists element-wise. -func rewriteListColumn(ctx context.Context, tx *sql.Tx, table, col string) error { - rows, err := tx.QueryContext(ctx, fmt.Sprintf( - "SELECT rowid, %[2]s FROM %[1]s WHERE ifnull(%[2]s, '') <> ''", table, col)) - if err != nil { - return err - } - type change struct { - rowid int64 - val string - } - var changes []change - for rows.Next() { - var rowid int64 - var val string - if err := rows.Scan(&rowid, &val); err != nil { - _ = rows.Close() - return err - } - parts := strings.Split(val, ",") - changed := false - for i, p := range parts { - if n := canonicalID(p); n != p { - parts[i] = n - changed = true - } - } - if changed { - changes = append(changes, change{rowid, strings.Join(parts, ",")}) +// canonicalizeIDList canonicalizes a comma-separated id list element-wise. +func canonicalizeIDList(s string) (string, bool) { + parts := strings.Split(s, ",") + changed := false + for i, p := range parts { + if n := canonicalID(p); n != p { + parts[i] = n + changed = true } } - if err := rows.Err(); err != nil { - _ = rows.Close() - return err + if !changed { + return s, false } - _ = rows.Close() - for _, c := range changes { - if _, err := tx.ExecContext(ctx, fmt.Sprintf( - "UPDATE %s SET %s = ? WHERE rowid = ?", table, col), c.val, c.rowid); err != nil { - return err - } - } - return nil + return strings.Join(parts, ","), true } -// rewriteJSONColumn applies transform to each non-empty JSON cell, updating only rows it changed. -func rewriteJSONColumn(ctx context.Context, tx *sql.Tx, table, col string, transform func(string) (string, bool)) error { +// rewriteColumn applies transform to each non-empty cell, updating only rows it changed. +func rewriteColumn(ctx context.Context, tx *sql.Tx, table, col string, transform func(string) (string, bool)) error { rows, err := tx.QueryContext(ctx, fmt.Sprintf( "SELECT rowid, %[2]s FROM %[1]s WHERE ifnull(%[2]s, '') <> ''", table, col)) if err != nil { diff --git a/db/migrations/id_canonical.go b/db/migrations/id_canonical.go index afd6765f5..f080a1723 100644 --- a/db/migrations/id_canonical.go +++ b/db/migrations/id_canonical.go @@ -18,13 +18,13 @@ func canonicalID(s string) string { return s } sum := md5.Sum([]byte(s)) - return id.Encode128(sum[:]) + return id.Encode128(sum) case 32: b, err := hex.DecodeString(s) if err != nil { return s } - return id.Encode128(b) + return id.Encode128([16]byte(b)) case 36: if s[8] != '-' || s[13] != '-' || s[18] != '-' || s[23] != '-' { return s @@ -33,7 +33,7 @@ func canonicalID(s string) string { if err != nil { return s } - return id.Encode128(b) + return id.Encode128([16]byte(b)) } return s } diff --git a/model/id/id.go b/model/id/id.go index 1588f38c3..43332cff8 100644 --- a/model/id/id.go +++ b/model/id/id.go @@ -6,22 +6,17 @@ import ( "fmt" "math/big" "strings" - - "github.com/navidrome/navidrome/log" ) func NewRandom() string { var b [16]byte _, _ = rand.Read(b[:]) // never fails since Go 1.24 - return Encode128(b[:]) + return Encode128(b) } // Encode128 renders a 16-byte value as the canonical 22-char zero-padded base62 id. -func Encode128(b []byte) string { - if len(b) != 16 { - log.Fatal("Encode128: expected 16 bytes", "got", len(b)) - } - return fmt.Sprintf("%022s", new(big.Int).SetBytes(b).Text(62)) +func Encode128(b [16]byte) string { + return fmt.Sprintf("%022s", new(big.Int).SetBytes(b[:]).Text(62)) } // Decode128 is the exact inverse of Encode128. @@ -45,7 +40,7 @@ func NewHash(data ...string) string { hash.Write([]byte(d)) hash.Write([]byte(string('\u200b'))) } - return Encode128(hash.Sum(nil)) + return Encode128([16]byte(hash.Sum(nil))) } func NewTagID(name, value string) string { diff --git a/model/id/id_test.go b/model/id/id_test.go index fc311016c..6ee9a345d 100644 --- a/model/id/id_test.go +++ b/model/id/id_test.go @@ -8,18 +8,18 @@ import ( var _ = Describe("Encode128/Decode128", func() { It("encodes 16 bytes as 22-char zero-padded base62", func() { - Expect(id.Encode128(make([]byte, 16))).To(Equal("0000000000000000000000")) - allFF := []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + Expect(id.Encode128([16]byte{})).To(Equal("0000000000000000000000")) + allFF := [16]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff} Expect(id.Encode128(allFF)).To(Equal("7N42dgm5tFLK9N8MT7fHC7")) }) It("round-trips arbitrary 16-byte values", func() { - b := []byte{0xe3, 0xb7, 0xfc, 0x2a, 0xe9, 0x44, 0x7b, 0xbe, + b := [16]byte{0xe3, 0xb7, 0xfc, 0x2a, 0xe9, 0x44, 0x7b, 0xbe, 0xc3, 0x7a, 0x13, 0xbf, 0x91, 0x6e, 0x3c, 0xf6} s := id.Encode128(b) Expect(s).To(Equal("6VHl3uR4kss6sUPKA8Cwnk")) - Expect(id.Decode128(s)).To(Equal(b)) + Expect(id.Decode128(s)).To(Equal(b[:])) }) It("rejects invalid input", func() { diff --git a/model/metadata/legacy_ids.go b/model/metadata/legacy_ids.go index a54777c26..57c367b44 100644 --- a/model/metadata/legacy_ids.go +++ b/model/metadata/legacy_ids.go @@ -21,7 +21,7 @@ func legacyTrackID(mf model.MediaFile, prependLibId bool) string { key = fmt.Sprintf("%d\\%s", mf.LibraryID, key) } sum := md5.Sum([]byte(key)) - return id.Encode128(sum[:]) + return id.Encode128(sum) } func legacyAlbumID(mf model.MediaFile, md Metadata, prependLibId bool) string { @@ -36,7 +36,7 @@ func legacyAlbumID(mf model.MediaFile, md Metadata, prependLibId bool) string { albumPath = fmt.Sprintf("%d\\%s", mf.LibraryID, albumPath) } sum := md5.Sum([]byte(albumPath)) - return id.Encode128(sum[:]) + return id.Encode128(sum) } func legacyMapAlbumArtistName(md Metadata) string {