fix(smartplaylist): don't let invalid rules hide all playlists - #6316

Empty nested groups were saved as {} and could not be read back, failing
every playlist query. Drop them while parsing, and log and skip unreadable
rules stored in the DB instead of failing the whole listing.

Signed-off-by: DualFroz <me@dualfroz.com>

persistence/playlist_repository.go

@@ -27,8 +27,13 @@ type dbPlaylist struct {
 }
 
 func (p *dbPlaylist) PostScan() error {
-	if p.Rules.String != "" {
-		return json.Unmarshal([]byte(p.Rules.String), &p.Playlist.Rules)
+	if p.Rules.String == "" {
+		return nil
+	}
+	// Rules come from clients and .nsp files, so a bad one must not fail every query listing it
+	if err := json.Unmarshal([]byte(p.Rules.String), &p.Playlist.Rules); err != nil {
+		log.Warn("Ignoring invalid smart playlist rules", "playlist", p.Name, "id", p.ID, "rules", p.Rules.String, err)
+		p.Playlist.Rules = nil
 	}
 	return nil
 }

persistence/smart_playlist_repository_test.go

@@ -75,6 +75,33 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
 				newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
 				Expect(repo.Put(ctx, &newPls)).To(MatchError(ContainSubstring("invalid criteria expression")))
 			})
+
+			Context("already stored in the DB", func() {
+				var broken model.Playlist
+
+				BeforeEach(func() {
+					broken = model.Playlist{Name: "Broken", OwnerID: "userid", Rules: rules}
+					Expect(repo.Put(ctx, &broken)).To(Succeed())
+					DeferCleanup(func() { Expect(repo.Delete(ctx, broken.ID)).To(Succeed()) })
+
+					_, err := GetDBXBuilder().NewQuery("update playlist set rules = {:rules} where id = {:id}").
+						Bind(dbx.Params{"rules": `{"all":[{}],"sort":"+lastplayed","limit":10}`, "id": broken.ID}).Execute()
+					Expect(err).ToNot(HaveOccurred())
+				})
+
+				It("does not hide the other playlists", func() {
+					all, err := repo.GetAll(ctx)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(slice.Map(all, func(p model.Playlist) string { return p.ID })).To(
+						ContainElements(plsBest.ID, plsCool.ID, broken.ID))
+				})
+
+				It("returns the playlist without its rules", func() {
+					pls, err := repo.Get(ctx, broken.ID)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(pls.Rules).To(BeNil())
+				})
+			})
 		})
 
 		Context("re-imported from disk", func() {