From d62bc7b4b2cb7775cb213d03800774af0130e453 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 19 Jul 2026 22:20:51 -0400 Subject: [PATCH] fix(db): canonicalize ids in junction tables and JSON columns --- .../20260720015443_uniform_canonical_ids.go | 118 ++++++++++++++++++ db/migrations/uniform_canonical_ids_test.go | 47 ++++++- 2 files changed, 163 insertions(+), 2 deletions(-) diff --git a/db/migrations/20260720015443_uniform_canonical_ids.go b/db/migrations/20260720015443_uniform_canonical_ids.go index 38e3d0eaf..e15fe4396 100644 --- a/db/migrations/20260720015443_uniform_canonical_ids.go +++ b/db/migrations/20260720015443_uniform_canonical_ids.go @@ -3,6 +3,7 @@ package migrations import ( "context" "database/sql" + "encoding/json" "fmt" "strings" @@ -35,6 +36,11 @@ var idColumns = []struct{ table, col string }{ {"share", "user_id"}, {"scrobble_buffer", "id"}, {"scrobble_buffer", "user_id"}, {"scrobble_buffer", "media_file_id"}, {"playqueue", "id"}, {"playqueue", "user_id"}, + {"user_library", "user_id"}, + {"scrobbles", "user_id"}, {"scrobbles", "media_file_id"}, + {"media_file_artists", "media_file_id"}, {"media_file_artists", "artist_id"}, + {"album_artists", "album_id"}, {"album_artists", "artist_id"}, + {"library_tag", "tag_id"}, } func upUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error { @@ -52,6 +58,12 @@ func upUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error { if err := rewriteListColumn(ctx, tx, "share", "resource_ids"); err != nil { return err } + if err := rewriteJSONColumn(ctx, tx, "plugin", "users", canonicalizePluginUsers); err != nil { + return err + } + if err := rewriteJSONColumn(ctx, tx, "playlist", "rules", canonicalizePlaylistRules); err != nil { + return err + } _, err := tx.ExecContext(ctx, "DROP TABLE _id_map") return err } @@ -149,6 +161,112 @@ func rewriteListColumn(ctx context.Context, tx *sql.Tx, table, col string) error return nil } +// 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 { + 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 + } + if newVal, ok := transform(val); ok { + changes = append(changes, change{rowid, newVal}) + } + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return err + } + _ = 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 +} + +// canonicalizePluginUsers maps a JSON array of user ids; malformed JSON passes through untouched. +func canonicalizePluginUsers(s string) (string, bool) { + var users []string + if err := json.Unmarshal([]byte(s), &users); err != nil { + return s, false + } + changed := false + for i, u := range users { + if n := canonicalID(u); n != u { + users[i] = n + changed = true + } + } + if !changed { + return s, false + } + out, err := json.Marshal(users) + if err != nil { + return s, false + } + return string(out), true +} + +// canonicalizePlaylistRules rewrites inPlaylist/notInPlaylist ids in smart-playlist criteria; malformed JSON passes through. +func canonicalizePlaylistRules(s string) (string, bool) { + var root map[string]any + if err := json.Unmarshal([]byte(s), &root); err != nil { + return s, false + } + if !canonicalizeRulesNode(root) { + return s, false + } + out, err := json.Marshal(root) + if err != nil { + return s, false + } + return string(out), true +} + +// canonicalizeRulesNode walks the criteria tree, canonicalizing the id of any inPlaylist/notInPlaylist object. +func canonicalizeRulesNode(node any) bool { + changed := false + switch v := node.(type) { + case map[string]any: + for k, val := range v { + if lk := strings.ToLower(k); lk == "inplaylist" || lk == "notinplaylist" { + if obj, ok := val.(map[string]any); ok { + if id, ok := obj["id"].(string); ok { + if n := canonicalID(id); n != id { + obj["id"] = n + changed = true + } + } + } + } + if canonicalizeRulesNode(val) { + changed = true + } + } + case []any: + for _, item := range v { + if canonicalizeRulesNode(item) { + changed = true + } + } + } + return changed +} + func downUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error { return nil // irreversible data migration } diff --git a/db/migrations/uniform_canonical_ids_test.go b/db/migrations/uniform_canonical_ids_test.go index 63e160aa3..ee547c64d 100644 --- a/db/migrations/uniform_canonical_ids_test.go +++ b/db/migrations/uniform_canonical_ids_test.go @@ -3,6 +3,7 @@ package migrations import ( "context" "database/sql" + "encoding/json" _ "github.com/mattn/go-sqlite3" . "github.com/onsi/ginkgo/v2" @@ -42,7 +43,7 @@ var _ = Describe("upUniformCanonicalIds", func() { CREATE TABLE library_artist (artist_id text); CREATE TABLE user (id text); CREATE TABLE user_props (user_id text); - CREATE TABLE playlist (id text, owner_id text); + CREATE TABLE playlist (id text, owner_id text, rules text); CREATE TABLE playlist_tracks (playlist_id text, media_file_id text); CREATE TABLE playlist_fields (playlist_id text); CREATE TABLE annotation (user_id text, item_id text, item_type text); @@ -53,6 +54,12 @@ var _ = Describe("upUniformCanonicalIds", func() { CREATE TABLE share (id text, user_id text, resource_ids text, contents text); CREATE TABLE scrobble_buffer (id text, user_id text, media_file_id text); CREATE TABLE playqueue (id text, user_id text, items text); + CREATE TABLE user_library (user_id text, library_id integer); + CREATE TABLE scrobbles (id integer, media_file_id text, user_id text); + CREATE TABLE media_file_artists (media_file_id text, artist_id text); + CREATE TABLE album_artists (album_id text, artist_id text); + CREATE TABLE library_tag (tag_id text, library_id integer); + CREATE TABLE plugin (id text, users text); `) Expect(err).ToNot(HaveOccurred()) @@ -64,10 +71,18 @@ var _ = Describe("upUniformCanonicalIds", func() { seed(`INSERT INTO media_file VALUES (?, ?, ?, ?, '', '', ?)`, legacyOld, legacyOld, hashID, legacyOld, uuidOld) seed(`INSERT INTO album VALUES (?, ?)`, legacyOld, hashID) seed(`INSERT INTO user VALUES (?)`, randOld) - seed(`INSERT INTO playlist VALUES (?, ?)`, uuidOld, randOld) // uuid id, random owner + // uuid id, random owner, smart-playlist rules with an embedded inPlaylist id + seed(`INSERT INTO playlist VALUES (?, ?, ?)`, uuidOld, randOld, `{"all":[{"inPlaylist":{"id":"`+uuidOld+`"}}]}`) seed(`INSERT INTO annotation VALUES (?, ?, 'media_file')`, randOld, legacyOld) seed(`INSERT INTO playqueue VALUES (?, ?, ?)`, randOld, randOld, legacyOld+","+hashID) seed(`INSERT INTO share VALUES (?, ?, ?, 'Album Foo...')`, shareID, randOld, legacyOld+","+uuidOld) + seed(`INSERT INTO user_library VALUES (?, 1)`, randOld) + seed(`INSERT INTO scrobbles VALUES (1, ?, ?)`, legacyOld, randOld) + seed(`INSERT INTO media_file_artists VALUES (?, ?)`, legacyOld, hashID) + seed(`INSERT INTO album_artists VALUES (?, ?)`, legacyOld, hashID) + seed(`INSERT INTO library_tag VALUES (?, 1)`, hashID) + seed(`INSERT INTO plugin VALUES ('lastfm', ?)`, `["`+randOld+`","`+hashID+`"]`) + seed(`INSERT INTO plugin VALUES ('empty', '[]')`) // exempt: empty user list untouched tx, err = db.Begin() Expect(err).ToNot(HaveOccurred()) @@ -104,4 +119,32 @@ var _ = Describe("upUniformCanonicalIds", func() { Expect(get(`SELECT contents FROM share`)).To(Equal("Album Foo...")) Expect(get(`SELECT mbz_recording_id FROM media_file`)).To(Equal(uuidOld)) }) + + It("canonicalizes junction and membership tables", func() { + Expect(get(`SELECT user_id FROM user_library`)).To(Equal(randNew)) + Expect(get(`SELECT media_file_id FROM scrobbles`)).To(Equal(legacyNew)) + Expect(get(`SELECT user_id FROM scrobbles`)).To(Equal(randNew)) + Expect(get(`SELECT media_file_id FROM media_file_artists`)).To(Equal(legacyNew)) + Expect(get(`SELECT artist_id FROM media_file_artists`)).To(Equal(hashID)) // hash-family, unchanged + Expect(get(`SELECT album_id FROM album_artists`)).To(Equal(legacyNew)) + Expect(get(`SELECT artist_id FROM album_artists`)).To(Equal(hashID)) // hash-family, unchanged + Expect(get(`SELECT tag_id FROM library_tag`)).To(Equal(hashID)) // hash-family, unchanged + }) + + It("rewrites JSON columns element-wise", func() { + Expect(get(`SELECT users FROM plugin WHERE id='lastfm'`)).To(Equal(`["` + randNew + `","` + hashID + `"]`)) + Expect(get(`SELECT id FROM plugin WHERE id='lastfm'`)).To(Equal("lastfm")) // plugin name, untouched + + var rules map[string]any + Expect(json.Unmarshal([]byte(get(`SELECT rules FROM playlist`)), &rules)).To(Succeed()) + all, ok := rules["all"].([]any) + Expect(ok).To(BeTrue()) + Expect(all).To(HaveLen(1)) + inPl := all[0].(map[string]any)["inPlaylist"].(map[string]any) + Expect(inPl["id"]).To(Equal(uuidNew)) + }) + + It("leaves exempt JSON rows untouched", func() { + Expect(get(`SELECT users FROM plugin WHERE id='empty'`)).To(Equal("[]")) + }) })