diff --git a/db/migrations/uniform_canonical_ids_test.go b/db/migrations/uniform_canonical_ids_test.go index ee547c64d..345579931 100644 --- a/db/migrations/uniform_canonical_ids_test.go +++ b/db/migrations/uniform_canonical_ids_test.go @@ -71,8 +71,8 @@ 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) - // uuid id, random owner, smart-playlist rules with an embedded inPlaylist id - seed(`INSERT INTO playlist VALUES (?, ?, ?)`, uuidOld, randOld, `{"all":[{"inPlaylist":{"id":"`+uuidOld+`"}}]}`) + // uuid id, random owner, smart-playlist rules with an embedded inPlaylist id and a sibling operator + seed(`INSERT INTO playlist VALUES (?, ?, ?)`, uuidOld, randOld, `{"all":[{"inPlaylist":{"id":"`+uuidOld+`"}},{"inTheLast":{"lastPlayed":30}}]}`) 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) @@ -83,6 +83,9 @@ var _ = Describe("upUniformCanonicalIds", func() { 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 + // malformed JSON in both a plugin list and a playlist rule: must pass through byte-for-byte + seed(`INSERT INTO plugin VALUES ('broken', 'not-json')`) + seed(`INSERT INTO playlist VALUES (?, ?, '{broken')`, hashID, hashID) tx, err = db.Begin() Expect(err).ToNot(HaveOccurred()) @@ -103,8 +106,8 @@ var _ = Describe("upUniformCanonicalIds", func() { Expect(get(`SELECT album_id FROM media_file`)).To(Equal(legacyNew)) Expect(get(`SELECT id FROM album`)).To(Equal(legacyNew)) Expect(get(`SELECT id FROM user`)).To(Equal(randNew)) - Expect(get(`SELECT id FROM playlist`)).To(Equal(uuidNew)) - Expect(get(`SELECT owner_id FROM playlist`)).To(Equal(randNew)) + Expect(get(`SELECT id FROM playlist WHERE owner_id='` + randNew + `'`)).To(Equal(uuidNew)) + Expect(get(`SELECT owner_id FROM playlist WHERE id='` + uuidNew + `'`)).To(Equal(randNew)) Expect(get(`SELECT item_id FROM annotation`)).To(Equal(legacyNew)) Expect(get(`SELECT user_id FROM annotation`)).To(Equal(randNew)) }) @@ -136,15 +139,40 @@ var _ = Describe("upUniformCanonicalIds", func() { 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()) + Expect(json.Unmarshal([]byte(get(`SELECT rules FROM playlist WHERE id='`+uuidNew+`'`)), &rules)).To(Succeed()) all, ok := rules["all"].([]any) Expect(ok).To(BeTrue()) - Expect(all).To(HaveLen(1)) + Expect(all).To(HaveLen(2)) inPl := all[0].(map[string]any)["inPlaylist"].(map[string]any) Expect(inPl["id"]).To(Equal(uuidNew)) }) + It("preserves sibling operators alongside a rewritten inPlaylist id", func() { + var rules map[string]any + Expect(json.Unmarshal([]byte(get(`SELECT rules FROM playlist WHERE id='`+uuidNew+`'`)), &rules)).To(Succeed()) + all := rules["all"].([]any) + var sawInPlaylist, sawInTheLast bool + for _, e := range all { + op := e.(map[string]any) + if pl, ok := op["inPlaylist"].(map[string]any); ok { + Expect(pl["id"]).To(Equal(uuidNew)) + sawInPlaylist = true + } + if last, ok := op["inTheLast"].(map[string]any); ok { + Expect(last["lastPlayed"]).To(Equal(float64(30))) + sawInTheLast = true + } + } + Expect(sawInPlaylist).To(BeTrue()) + Expect(sawInTheLast).To(BeTrue()) + }) + It("leaves exempt JSON rows untouched", func() { Expect(get(`SELECT users FROM plugin WHERE id='empty'`)).To(Equal("[]")) }) + + It("passes malformed JSON columns through byte-for-byte", func() { + Expect(get(`SELECT users FROM plugin WHERE id='broken'`)).To(Equal("not-json")) + Expect(get(`SELECT rules FROM playlist WHERE id='` + hashID + `'`)).To(Equal("{broken")) + }) }) diff --git a/model/id/id.go b/model/id/id.go index d94e7868b..61dc8d443 100644 --- a/model/id/id.go +++ b/model/id/id.go @@ -16,6 +16,9 @@ func NewRandom() string { // Encode128 renders a 16-byte value as the canonical 22-char zero-padded base62 id. func Encode128(b []byte) string { + if len(b) != 16 { + panic(fmt.Sprintf("id.Encode128: expected 16 bytes, got %d", len(b))) + } return fmt.Sprintf("%022s", new(big.Int).SetBytes(b).Text(62)) } diff --git a/model/id/id_test.go b/model/id/id_test.go index fc311016c..d3858af6b 100644 --- a/model/id/id_test.go +++ b/model/id/id_test.go @@ -22,6 +22,10 @@ var _ = Describe("Encode128/Decode128", func() { Expect(id.Decode128(s)).To(Equal(b)) }) + It("panics on non-16-byte input", func() { + Expect(func() { id.Encode128(make([]byte, 15)) }).To(Panic()) + }) + It("rejects invalid input", func() { _, err := id.Decode128("short") Expect(err).To(HaveOccurred()) diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index 57bb47e08..57a5b1308 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -223,6 +223,16 @@ var _ = Describe("getPID", func() { // md5("/music/a.mp3") = e3b7fc2ae9447bbec37a13bf916e3cf6 re-encoded as base62 Expect(legacyTrackID(mf, false)).To(Equal("6VHl3uR4kss6sUPKA8Cwnk")) }) + It("prepends the library id for a non-default library", func() { + mf := model.MediaFile{Path: "/music/a.mp3", LibraryID: 2} + // id.Encode128(md5.Sum([]byte("2\\/music/a.mp3"))) + Expect(legacyTrackID(mf, true)).To(Equal("4EK5DHQBMeFuDHw6S3iooO")) + }) + It("emits a canonical album id (golden)", func() { + mf := model.MediaFile{LibraryID: 1} + // id.Encode128(md5.Sum([]byte("[unknown artist]\\[unknown album]"))) + Expect(legacyAlbumID(mf, Metadata{}, false)).To(Equal("6xBmxSAUFJSQuW7UvwCq8X")) + }) Context("track_legacy", func() { When("library ID is default (1)", func() { It("should not prepend library ID even when prependLibId is true", func() { diff --git a/server/jellyfin/dto/ids_test.go b/server/jellyfin/dto/ids_test.go index 26a957604..d85786d99 100644 --- a/server/jellyfin/dto/ids_test.go +++ b/server/jellyfin/dto/ids_test.go @@ -6,7 +6,7 @@ import ( ) var _ = Describe("id codec", func() { - It("round-trips a base62 nanoid through Encode/Decode", func() { + It("round-trips a base62 id", func() { id := "5QFKvMsJrd57QE2Le2dKKo" Expect(DecodeID(EncodeID(id))).To(Equal(id)) }) diff --git a/server/jellyfin/truncated_ids.go b/server/jellyfin/truncated_ids.go index 822b696d5..bb3e4ce9b 100644 --- a/server/jellyfin/truncated_ids.go +++ b/server/jellyfin/truncated_ids.go @@ -11,7 +11,8 @@ import ( ) // truncatedIDLen is what Finamp's saved-queue persistence cuts item ids to (16 bytes, assuming -// Jellyfin GUIDs). All Navidrome ids are 22 chars (share ids 10), so length alone flags a truncated id. See README. +// Jellyfin GUIDs). All Navidrome ids are 22 chars (share ids 10), so length alone flags a +// truncated id. See README. // // Handlers taking an item id resolve it via resolveItemID/resolveItemIDs; playlist-write handlers // and ParentId scoping don't (a restored queue never edits playlists or browses by container id).