fix(smartplaylists): ignore empty inPlaylist id and path references

An empty path matched every playlist without a file path, including the
referencing playlist itself, so the refresh recursed until the stack overflowed.
An empty id also shadowed a valid path in the same rule.

model/criteria/criteria_test.go

@@ -369,6 +369,14 @@ var _ = Describe("Criteria", func() {
 			ids := goObj.ChildPlaylistPaths()
 			gomega.Expect(ids).To(gomega.ConsistOf(topLevelInPlaylistPath, nestedAnyNotInPlaylistPath))
 		})
+		It("ignores empty child playlist paths", func() {
+			c := Criteria{Expression: All{InPlaylist{"path": ""}, NotInPlaylist{"path": ""}}}
+			gomega.Expect(c.ChildPlaylistPaths()).To(gomega.BeEmpty())
+		})
+		It("ignores empty child playlist ids", func() {
+			c := Criteria{Expression: All{InPlaylist{"id": ""}, NotInPlaylist{"id": ""}}}
+			gomega.Expect(c.ChildPlaylistIds()).To(gomega.BeEmpty())
+		})
 		It("extracts child smart playlist IDs from deeply nested expression", func() {
 			goObj = Criteria{
 				Expression: Any{

model/criteria/operators.go

@@ -191,7 +191,7 @@ func extractPlaylistField(inputRule any, field string) (values []string) {
 			values = append(values, extractPlaylistField(rules, field)...)
 		}
 	case InPlaylist, NotInPlaylist:
-		if value, ok := rule.(Expression).fields()[field].(string); ok {
+		if value, ok := rule.(Expression).fields()[field].(string); ok && value != "" {
 			values = append(values, value)
 		}
 	}

persistence/criteria_sql.go

@@ -345,7 +345,7 @@ func startOfPeriod(numDays int64, from time.Time) string {
 
 func (c smartPlaylistCriteria) inList(values map[string]any, negate bool) (squirrel.Sqlizer, error) {
 	var condition squirrel.Sqlizer
-	if playlistId, ok := values["id"].(string); ok {
+	if playlistId, ok := values["id"].(string); ok && playlistId != "" {
 		condition = squirrel.Eq{"pl.playlist_id": playlistId}
 	} else if playlistPath, ok := values["path"].(string); ok && playlistPath != "" {
 		condition = squirrel.Eq{"playlist.path": playlistPath}

persistence/criteria_sql_test.go

@@ -49,6 +49,7 @@ var _ = Describe("Smart playlist criteria SQL", func() {
 		Entry("after", criteria.After{"lastPlayed": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "annotation.play_date > ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
 		Entry("in playlist [path]", criteria.InPlaylist{"path": "lacuslacus.nsp"}, "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (playlist.path = ? AND playlist.public = ?))", "lacuslacus.nsp", 1),
 		Entry("in playlist [id]", criteria.InPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1),
+		Entry("in playlist [empty id falls back to path]", criteria.InPlaylist{"id": "", "path": "/music/x.nsp"}, "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (playlist.path = ? AND playlist.public = ?))", "/music/x.nsp", 1),
 		Entry("not in playlist", criteria.NotInPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id NOT IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1),
 		Entry("album annotation", criteria.Gt{"albumRating": 3}, "album_annotation.rating > ?", 3),
 		Entry("artist annotation", criteria.Is{"artistLoved": true}, "artist_annotation.starred = ?", true),

persistence/smart_playlist_repository.go

@@ -97,9 +97,17 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL
 		return true
 	}
 
-	childPlaylists, err := r.GetAll(model.QueryOptions{Filters: Or{Eq{"playlist.id": childPlaylistIds}, Eq{"playlist.path": childPlaylistPaths}}})
+	var conditions Or
+	if len(childPlaylistIds) > 0 {
+		conditions = append(conditions, Eq{"playlist.id": childPlaylistIds})
+	}
+	if len(childPlaylistPaths) > 0 {
+		conditions = append(conditions, Eq{"playlist.path": childPlaylistPaths})
+	}
+
+	childPlaylists, err := r.GetAll(model.QueryOptions{Filters: conditions})
 	if err != nil {
-		log.Error(r.ctx, "Error loading child playlists for smart playlist refresh", "playlist", pls.Name, "id", pls.ID, "childIds", childPlaylistIds, err)
+		log.Error(r.ctx, "Error loading child playlists for smart playlist refresh", "playlist", pls.Name, "id", pls.ID, "childIds", childPlaylistIds, "childPaths", childPlaylistPaths, err)
 		return false
 	}
 

persistence/smart_playlist_repository_test.go

@@ -174,6 +174,29 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
 				})
 			})
 
+			It("does not treat an empty path as a reference to every playlist without a path", func() {
+				conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+
+				bystander := model.Playlist{Name: "Bystander", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.Contains{"title": "Day"}},
+				}}
+				Expect(repo.Put(&bystander)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(bystander.ID) })
+
+				parent := model.Playlist{Name: "Empty Path", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
+					Expression: criteria.All{criteria.InPlaylist{"path": ""}},
+				}}
+				Expect(repo.Put(&parent)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(parent.ID) })
+
+				_, err := repo.GetWithTracks(parent.ID, true, false)
+				Expect(err).ToNot(HaveOccurred())
+
+				reloaded, err := repo.Get(bystander.ID)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reloaded.EvaluatedAt).To(BeNil())
+			})
+
 			When("refresh delay has not expired", func() {
 				It("should NOT refresh tracks for smart playlist referenced in parent smart playlist criteria", func() {
 					conf.Server.SmartPlaylistRefreshDelay = 1 * time.Hour