Recursively refresh playlist tracks within smart playlist rules (#3018)

* Recursively refresh playlists within smart playlist rules

Signed-off-by: reillymc <reilly@mackenzie-cree.net>

* Clean up recursive smart playlist functions

Signed-off-by: reillymc <reilly@mackenzie-cree.net>

* Add smart playlist refresh timeout config and tests for nested track refetching

Signed-off-by: reillymc <reilly@mackenzie-cree.net>

* Change SmartPlaylistRefreshTimeout to SmartPlaylistRefreshDelay, increase default value

* Revert `smartPlaylistRefreshDelay` default to 5 seconds

---------

Signed-off-by: reillymc <reilly@mackenzie-cree.net>
Co-authored-by: Deluan <deluan@navidrome.org>

conf/configuration.go

@@ -49,6 +49,7 @@ type configOptions struct {
 	AutoImportPlaylists             bool
 	DefaultPlaylistPublicVisibility bool
 	PlaylistsPath                   string
+	SmartPlaylistRefreshDelay       time.Duration
 	AutoTranscodeDownload           bool
 	DefaultDownsamplingFormat       string
 	SearchFullString                bool
@@ -302,6 +303,7 @@ func init() {
 	viper.SetDefault("autoimportplaylists", true)
 	viper.SetDefault("defaultplaylistpublicvisibility", false)
 	viper.SetDefault("playlistspath", consts.DefaultPlaylistsPath)
+	viper.SetDefault("smartPlaylistRefreshDelay", 5*time.Second)
 	viper.SetDefault("enabledownloads", true)
 	viper.SetDefault("enableexternalservices", true)
 	viper.SetDefault("enablemediafilecoverart", true)

model/criteria/criteria.go

@@ -50,6 +50,21 @@ func (c Criteria) ToSql() (sql string, args []interface{}, err error) {
 	return c.Expression.ToSql()
 }
 
+func (c Criteria) ChildPlaylistIds() (ids []string) {
+	if c.Expression == nil {
+		return ids
+	}
+
+	switch rules := c.Expression.(type) {
+	case Any:
+		ids = rules.ChildPlaylistIds()
+	case All:
+		ids = rules.ChildPlaylistIds()
+	}
+
+	return ids
+}
+
 func (c Criteria) MarshalJSON() ([]byte, error) {
 	aux := struct {
 		All    []Expression `json:"all,omitempty"`

model/criteria/criteria_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 
+	"github.com/google/uuid"
 	. "github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
 )
@@ -89,4 +90,94 @@ var _ = Describe("Criteria", func() {
 		gomega.Expect(newObj.OrderBy()).To(gomega.Equal("random() asc"))
 	})
 
+	It("extracts all child smart playlist IDs from All expression criteria", func() {
+		topLevelInPlaylistID := uuid.NewString()
+		topLevelNotInPlaylistID := uuid.NewString()
+
+		nestedAnyInPlaylistID := uuid.NewString()
+		nestedAnyNotInPlaylistID := uuid.NewString()
+
+		nestedAllInPlaylistID := uuid.NewString()
+		nestedAllNotInPlaylistID := uuid.NewString()
+
+		goObj := Criteria{
+			Expression: All{
+				InPlaylist{"id": topLevelInPlaylistID},
+				NotInPlaylist{"id": topLevelNotInPlaylistID},
+				Any{
+					InPlaylist{"id": nestedAnyInPlaylistID},
+					NotInPlaylist{"id": nestedAnyNotInPlaylistID},
+				},
+				All{
+					InPlaylist{"id": nestedAllInPlaylistID},
+					NotInPlaylist{"id": nestedAllNotInPlaylistID},
+				},
+			},
+		}
+
+		ids := goObj.ChildPlaylistIds()
+
+		gomega.Expect(ids).To(gomega.ConsistOf(topLevelInPlaylistID, topLevelNotInPlaylistID, nestedAnyInPlaylistID, nestedAnyNotInPlaylistID, nestedAllInPlaylistID, nestedAllNotInPlaylistID))
+	})
+
+	It("extracts all child smart playlist IDs from Any expression criteria", func() {
+		topLevelInPlaylistID := uuid.NewString()
+		topLevelNotInPlaylistID := uuid.NewString()
+
+		nestedAnyInPlaylistID := uuid.NewString()
+		nestedAnyNotInPlaylistID := uuid.NewString()
+
+		nestedAllInPlaylistID := uuid.NewString()
+		nestedAllNotInPlaylistID := uuid.NewString()
+
+		goObj := Criteria{
+			Expression: Any{
+				InPlaylist{"id": topLevelInPlaylistID},
+				NotInPlaylist{"id": topLevelNotInPlaylistID},
+				Any{
+					InPlaylist{"id": nestedAnyInPlaylistID},
+					NotInPlaylist{"id": nestedAnyNotInPlaylistID},
+				},
+				All{
+					InPlaylist{"id": nestedAllInPlaylistID},
+					NotInPlaylist{"id": nestedAllNotInPlaylistID},
+				},
+			},
+		}
+
+		ids := goObj.ChildPlaylistIds()
+
+		gomega.Expect(ids).To(gomega.ConsistOf(topLevelInPlaylistID, topLevelNotInPlaylistID, nestedAnyInPlaylistID, nestedAnyNotInPlaylistID, nestedAllInPlaylistID, nestedAllNotInPlaylistID))
+	})
+
+	It("extracts child smart playlist IDs from deeply nested expression", func() {
+		nestedAnyInPlaylistID := uuid.NewString()
+		nestedAnyNotInPlaylistID := uuid.NewString()
+
+		nestedAllInPlaylistID := uuid.NewString()
+		nestedAllNotInPlaylistID := uuid.NewString()
+
+		goObj := Criteria{
+			Expression: Any{
+				Any{
+					All{
+						Any{
+							InPlaylist{"id": nestedAnyInPlaylistID},
+							NotInPlaylist{"id": nestedAnyNotInPlaylistID},
+							Any{
+								All{
+									InPlaylist{"id": nestedAllInPlaylistID},
+									NotInPlaylist{"id": nestedAllNotInPlaylistID},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		ids := goObj.ChildPlaylistIds()
+
+		gomega.Expect(ids).To(gomega.ConsistOf(nestedAnyInPlaylistID, nestedAnyNotInPlaylistID, nestedAllInPlaylistID, nestedAllNotInPlaylistID))
+	})
 })

model/criteria/operators.go

@@ -23,6 +23,10 @@ func (all All) MarshalJSON() ([]byte, error) {
 	return marshalConjunction("all", all)
 }
 
+func (all All) ChildPlaylistIds() (ids []string) {
+	return extractPlaylistIds(all)
+}
+
 type (
 	Any squirrel.Or
 	Or  = Any
@@ -36,6 +40,10 @@ func (any Any) MarshalJSON() ([]byte, error) {
 	return marshalConjunction("any", any)
 }
 
+func (any Any) ChildPlaylistIds() (ids []string) {
+	return extractPlaylistIds(any)
+}
+
 type Is squirrel.Eq
 type Eq = Is
 
@@ -275,3 +283,29 @@ func inList(m map[string]interface{}, negate bool) (sql string, args []interface
 		return "media_file.id IN (" + subQText + ")", subQArgs, nil
 	}
 }
+
+func extractPlaylistIds(inputRule interface{}) (ids []string) {
+	var id string
+	var ok bool
+
+	switch rule := inputRule.(type) {
+	case Any:
+		for _, rules := range rule {
+			ids = append(ids, extractPlaylistIds(rules)...)
+		}
+	case All:
+		for _, rules := range rule {
+			ids = append(ids, extractPlaylistIds(rules)...)
+		}
+	case InPlaylist:
+		if id, ok = rule["id"].(string); ok {
+			ids = append(ids, id)
+		}
+	case NotInPlaylist:
+		if id, ok = rule["id"].(string); ok {
+			ids = append(ids, id)
+		}
+	}
+
+	return
+}

persistence/playlist_repository.go

@@ -10,6 +10,7 @@ import (
 
 	. "github.com/Masterminds/squirrel"
 	"github.com/deluan/rest"
+	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/criteria"
@@ -196,8 +197,8 @@ func (r *playlistRepository) selectPlaylist(options ...model.QueryOptions) Selec
 }
 
 func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
-	// Only refresh if it is a smart playlist and was not refreshed in the last 5 seconds
+	// Only refresh if it is a smart playlist and was not refreshed within the interval provided by the refresh delay config
-	if !pls.IsSmartPlaylist() || (pls.EvaluatedAt != nil && time.Since(*pls.EvaluatedAt) < 5*time.Second) {
+	if !pls.IsSmartPlaylist() || (pls.EvaluatedAt != nil && time.Since(*pls.EvaluatedAt) < conf.Server.SmartPlaylistRefreshDelay) {
 		return false
 	}
 
@@ -221,6 +222,18 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
 
 	// Re-populate playlist based on Smart Playlist criteria
 	rules := *pls.Rules
+
+	// If the playlist depends on other playlists, recursively refresh them first
+	childPlaylistIds := rules.ChildPlaylistIds()
+	for _, id := range childPlaylistIds {
+		childPls, err := r.Get(id)
+		if err != nil {
+			log.Error(r.ctx, "Error loading child playlist", "id", pls.ID, "childId", id, err)
+			return false
+		}
+		r.refreshSmartPlaylist(childPls)
+	}
+
 	sq := Select("row_number() over (order by "+rules.OrderBy()+") as id", "'"+pls.ID+"' as playlist_id", "media_file.id as media_file_id").
 		From("media_file").LeftJoin("annotation on (" +
 		"annotation.item_id = media_file.id" +

persistence/playlist_repository_test.go

@@ -2,7 +2,9 @@ package persistence
 
 import (
 	"context"
+	"time"
 
+	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/db"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
@@ -143,5 +145,63 @@ var _ = Describe("PlaylistRepository", func() {
 				Expect(repo.Put(&newPls)).To(MatchError(ContainSubstring("invalid criteria expression")))
 			})
 		})
+
+		Context("child smart playlists", func() {
+			When("refresh day has expired", func() {
+				It("should refresh tracks for smart playlist referenced in parent smart playlist criteria", func() {
+					conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+
+					nestedPls := model.Playlist{Name: "Nested", OwnerID: "userid", Rules: rules}
+					Expect(repo.Put(&nestedPls)).To(Succeed())
+
+					parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
+						Expression: criteria.All{
+							criteria.InPlaylist{"id": nestedPls.ID},
+						},
+					}}
+					Expect(repo.Put(&parentPls)).To(Succeed())
+
+					nestedPlsRead, err := repo.Get(nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+
+					_, err = repo.GetWithTracks(parentPls.ID, true)
+					Expect(err).ToNot(HaveOccurred())
+
+					// Check that the nested playlist was refreshed by parent get by verifying evaluatedAt is updated since first nestedPls get
+					nestedPlsAfterParentGet, err := repo.Get(nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+
+					Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally(">", *nestedPlsRead.EvaluatedAt))
+				})
+			})
+
+			When("refresh day 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
+
+					nestedPls := model.Playlist{Name: "Nested", OwnerID: "userid", Rules: rules}
+					Expect(repo.Put(&nestedPls)).To(Succeed())
+
+					parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
+						Expression: criteria.All{
+							criteria.InPlaylist{"id": nestedPls.ID},
+						},
+					}}
+					Expect(repo.Put(&parentPls)).To(Succeed())
+
+					nestedPlsRead, err := repo.Get(nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+
+					_, err = repo.GetWithTracks(parentPls.ID, true)
+					Expect(err).ToNot(HaveOccurred())
+
+					// Check that the nested playlist was not refreshed by parent get by verifying evaluatedAt is not updated since first nestedPls get
+					nestedPlsAfterParentGet, err := repo.Get(nestedPls.ID)
+					Expect(err).ToNot(HaveOccurred())
+
+					Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(Equal(*nestedPlsRead.EvaluatedAt))
+				})
+			})
+		})
 	})
 })