diff --git a/conf/configuration.go b/conf/configuration.go index eb2394229..00893a19e 100644 --- a/conf/configuration.go +++ b/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) diff --git a/model/criteria/criteria.go b/model/criteria/criteria.go index bcd72cb79..76aab0ba8 100644 --- a/model/criteria/criteria.go +++ b/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"` diff --git a/model/criteria/criteria_test.go b/model/criteria/criteria_test.go index 6b4926226..35ce1d22a 100644 --- a/model/criteria/criteria_test.go +++ b/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)) + }) }) diff --git a/model/criteria/operators.go b/model/criteria/operators.go index 86acfab1d..c0a0adcb3 100644 --- a/model/criteria/operators.go +++ b/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 +} diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index efc072da5..422275f50 100644 --- a/persistence/playlist_repository.go +++ b/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 - if !pls.IsSmartPlaylist() || (pls.EvaluatedAt != nil && time.Since(*pls.EvaluatedAt) < 5*time.Second) { + // 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) < 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" + diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 8391f5c39..a6fd1beb0 100644 --- a/persistence/playlist_repository_test.go +++ b/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)) + }) + }) + }) }) })