diff --git a/model/criteria/criteria.go b/model/criteria/criteria.go index 54ac5969..bc3fe801 100644 --- a/model/criteria/criteria.go +++ b/model/criteria/criteria.go @@ -95,6 +95,25 @@ func (c Criteria) ToSql() (sql string, args []any, err error) { return c.Expression.ToSql() } +// RequiredJoins inspects the expression tree and Sort field to determine which +// additional JOINs are needed when evaluating this criteria. +func (c Criteria) RequiredJoins() JoinType { + result := JoinNone + if c.Expression != nil { + result |= extractJoinTypes(c.Expression) + } + // Also check Sort fields + if c.Sort != "" { + for _, p := range strings.Split(c.Sort, ",") { + p = strings.TrimSpace(p) + p = strings.TrimLeft(p, "+-") + p = strings.TrimSpace(p) + result |= fieldJoinType(p) + } + } + return result +} + func (c Criteria) ChildPlaylistIds() []string { if c.Expression == nil { return nil diff --git a/model/criteria/criteria_test.go b/model/criteria/criteria_test.go index 032ead5c..9a4da360 100644 --- a/model/criteria/criteria_test.go +++ b/model/criteria/criteria_test.go @@ -27,6 +27,7 @@ var _ = Describe("Criteria", func() { StartsWith{"comment": "this"}, InTheRange{"year": []int{1980, 1990}}, IsNot{"genre": "Rock"}, + Gt{"albumrating": 3}, }, }, Sort: "title", @@ -48,7 +49,8 @@ var _ = Describe("Criteria", func() { { "all": [ { "startsWith": {"comment": "this"} }, { "inTheRange": {"year":[1980,1990]} }, - { "isNot": { "genre": "Rock" }} + { "isNot": { "genre": "Rock" }}, + { "gt": { "albumrating": 3 } } ] } ], @@ -68,10 +70,10 @@ var _ = Describe("Criteria", func() { gomega.Expect(err).ToNot(gomega.HaveOccurred()) gomega.Expect(sql).To(gomega.Equal( `(media_file.title LIKE ? AND media_file.title NOT LIKE ? ` + - `AND (not exists (select 1 from json_tree(participants, '$.artist') where key='name' and value = ?) ` + + `AND (not exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value = ?) ` + `OR media_file.album = ?) AND (media_file.comment LIKE ? AND (media_file.year >= ? AND media_file.year <= ?) ` + - `AND not exists (select 1 from json_tree(tags, '$.genre') where key='value' and value = ?)))`)) - gomega.Expect(args).To(gomega.HaveExactElements("%love%", "%hate%", "u2", "best of", "this%", 1980, 1990, "Rock")) + `AND not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value = ?) AND COALESCE(album_annotation.rating, 0) > ?))`)) + gomega.Expect(args).To(gomega.HaveExactElements("%love%", "%hate%", "u2", "best of", "this%", 1980, 1990, "Rock", 3)) }) It("marshals to JSON", func() { j, err := json.Marshal(goObj) @@ -172,13 +174,95 @@ var _ = Describe("Criteria", func() { sql, args, err := goObj.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) gomega.Expect(sql).To(gomega.Equal( - `(exists (select 1 from json_tree(participants, '$.artist') where key='name' and value = ?) AND ` + - `exists (select 1 from json_tree(participants, '$.composer') where key='name' and value LIKE ?))`, + `(exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value = ?) AND ` + + `exists (select 1 from json_tree(media_file.participants, '$.composer') where key='name' and value LIKE ?))`, )) gomega.Expect(args).To(gomega.HaveExactElements("The Beatles", "%Lennon%")) }) }) + Describe("RequiredJoins", func() { + It("returns JoinNone when no annotation fields are used", func() { + c := Criteria{ + Expression: All{ + Contains{"title": "love"}, + }, + } + gomega.Expect(c.RequiredJoins()).To(gomega.Equal(JoinNone)) + }) + It("returns JoinNone for media_file annotation fields", func() { + c := Criteria{ + Expression: All{ + Is{"loved": true}, + Gt{"playCount": 5}, + }, + } + gomega.Expect(c.RequiredJoins()).To(gomega.Equal(JoinNone)) + }) + It("returns JoinAlbumAnnotation for album annotation fields", func() { + c := Criteria{ + Expression: All{ + Gt{"albumRating": 3}, + }, + } + gomega.Expect(c.RequiredJoins()).To(gomega.Equal(JoinAlbumAnnotation)) + }) + It("returns JoinArtistAnnotation for artist annotation fields", func() { + c := Criteria{ + Expression: All{ + Is{"artistLoved": true}, + }, + } + gomega.Expect(c.RequiredJoins()).To(gomega.Equal(JoinArtistAnnotation)) + }) + It("returns both join types when both are used", func() { + c := Criteria{ + Expression: All{ + Gt{"albumRating": 3}, + Is{"artistLoved": true}, + }, + } + j := c.RequiredJoins() + gomega.Expect(j.Has(JoinAlbumAnnotation)).To(gomega.BeTrue()) + gomega.Expect(j.Has(JoinArtistAnnotation)).To(gomega.BeTrue()) + }) + It("detects join types in nested expressions", func() { + c := Criteria{ + Expression: All{ + Any{ + All{ + Is{"albumLoved": true}, + }, + }, + Any{ + Gt{"artistPlayCount": 10}, + }, + }, + } + j := c.RequiredJoins() + gomega.Expect(j.Has(JoinAlbumAnnotation)).To(gomega.BeTrue()) + gomega.Expect(j.Has(JoinArtistAnnotation)).To(gomega.BeTrue()) + }) + It("detects join types from Sort field", func() { + c := Criteria{ + Expression: All{ + Contains{"title": "love"}, + }, + Sort: "albumRating", + } + gomega.Expect(c.RequiredJoins().Has(JoinAlbumAnnotation)).To(gomega.BeTrue()) + }) + It("detects join types from Sort field with direction prefix", func() { + c := Criteria{ + Expression: All{ + Contains{"title": "love"}, + }, + Sort: "-artistRating", + } + gomega.Expect(c.RequiredJoins().Has(JoinArtistAnnotation)).To(gomega.BeTrue()) + }) + }) + Context("with child playlists", func() { var ( topLevelInPlaylistID string diff --git a/model/criteria/fields.go b/model/criteria/fields.go index b5ac9271..4dfc50f1 100644 --- a/model/criteria/fields.go +++ b/model/criteria/fields.go @@ -9,45 +9,71 @@ import ( "github.com/navidrome/navidrome/log" ) +// JoinType is a bitmask indicating which additional JOINs are needed by a smart playlist expression. +type JoinType int + +const ( + JoinNone JoinType = 0 + JoinAlbumAnnotation JoinType = 1 << iota + JoinArtistAnnotation +) + +// Has returns true if j contains all bits in other. +func (j JoinType) Has(other JoinType) bool { return j&other != 0 } + var fieldMap = map[string]*mappedField{ - "title": {field: "media_file.title"}, - "album": {field: "media_file.album"}, - "hascoverart": {field: "media_file.has_cover_art"}, - "tracknumber": {field: "media_file.track_number"}, - "discnumber": {field: "media_file.disc_number"}, - "year": {field: "media_file.year"}, - "date": {field: "media_file.date", alias: "recordingdate"}, - "originalyear": {field: "media_file.original_year"}, - "originaldate": {field: "media_file.original_date"}, - "releaseyear": {field: "media_file.release_year"}, - "releasedate": {field: "media_file.release_date"}, - "size": {field: "media_file.size"}, - "compilation": {field: "media_file.compilation"}, - "explicitstatus": {field: "media_file.explicit_status"}, - "dateadded": {field: "media_file.created_at"}, - "datemodified": {field: "media_file.updated_at"}, - "discsubtitle": {field: "media_file.disc_subtitle"}, - "comment": {field: "media_file.comment"}, - "lyrics": {field: "media_file.lyrics"}, - "sorttitle": {field: "media_file.sort_title"}, - "sortalbum": {field: "media_file.sort_album_name"}, - "sortartist": {field: "media_file.sort_artist_name"}, - "sortalbumartist": {field: "media_file.sort_album_artist_name"}, - "albumcomment": {field: "media_file.mbz_album_comment"}, - "catalognumber": {field: "media_file.catalog_num"}, - "filepath": {field: "media_file.path"}, - "filetype": {field: "media_file.suffix"}, - "duration": {field: "media_file.duration"}, - "bitrate": {field: "media_file.bit_rate"}, - "bitdepth": {field: "media_file.bit_depth"}, - "bpm": {field: "media_file.bpm"}, - "channels": {field: "media_file.channels"}, - "loved": {field: "COALESCE(annotation.starred, false)"}, - "dateloved": {field: "annotation.starred_at"}, - "lastplayed": {field: "annotation.play_date"}, - "daterated": {field: "annotation.rated_at"}, - "playcount": {field: "COALESCE(annotation.play_count, 0)"}, - "rating": {field: "COALESCE(annotation.rating, 0)"}, + "title": {field: "media_file.title"}, + "album": {field: "media_file.album"}, + "hascoverart": {field: "media_file.has_cover_art"}, + "tracknumber": {field: "media_file.track_number"}, + "discnumber": {field: "media_file.disc_number"}, + "year": {field: "media_file.year"}, + "date": {field: "media_file.date", alias: "recordingdate"}, + "originalyear": {field: "media_file.original_year"}, + "originaldate": {field: "media_file.original_date"}, + "releaseyear": {field: "media_file.release_year"}, + "releasedate": {field: "media_file.release_date"}, + "size": {field: "media_file.size"}, + "compilation": {field: "media_file.compilation"}, + "explicitstatus": {field: "media_file.explicit_status"}, + "dateadded": {field: "media_file.created_at"}, + "datemodified": {field: "media_file.updated_at"}, + "discsubtitle": {field: "media_file.disc_subtitle"}, + "comment": {field: "media_file.comment"}, + "lyrics": {field: "media_file.lyrics"}, + "sorttitle": {field: "media_file.sort_title"}, + "sortalbum": {field: "media_file.sort_album_name"}, + "sortartist": {field: "media_file.sort_artist_name"}, + "sortalbumartist": {field: "media_file.sort_album_artist_name"}, + "albumcomment": {field: "media_file.mbz_album_comment"}, + "catalognumber": {field: "media_file.catalog_num"}, + "filepath": {field: "media_file.path"}, + "filetype": {field: "media_file.suffix"}, + "duration": {field: "media_file.duration"}, + "bitrate": {field: "media_file.bit_rate"}, + "bitdepth": {field: "media_file.bit_depth"}, + "bpm": {field: "media_file.bpm"}, + "channels": {field: "media_file.channels"}, + "loved": {field: "COALESCE(annotation.starred, false)"}, + "dateloved": {field: "annotation.starred_at"}, + "lastplayed": {field: "annotation.play_date"}, + "daterated": {field: "annotation.rated_at"}, + "playcount": {field: "COALESCE(annotation.play_count, 0)"}, + "rating": {field: "COALESCE(annotation.rating, 0)"}, + "albumrating": {field: "COALESCE(album_annotation.rating, 0)", joinType: JoinAlbumAnnotation}, + "albumloved": {field: "COALESCE(album_annotation.starred, false)", joinType: JoinAlbumAnnotation}, + "albumplaycount": {field: "COALESCE(album_annotation.play_count, 0)", joinType: JoinAlbumAnnotation}, + "albumlastplayed": {field: "album_annotation.play_date", joinType: JoinAlbumAnnotation}, + "albumdateloved": {field: "album_annotation.starred_at", joinType: JoinAlbumAnnotation}, + "albumdaterated": {field: "album_annotation.rated_at", joinType: JoinAlbumAnnotation}, + + "artistrating": {field: "COALESCE(artist_annotation.rating, 0)", joinType: JoinArtistAnnotation}, + "artistloved": {field: "COALESCE(artist_annotation.starred, false)", joinType: JoinArtistAnnotation}, + "artistplaycount": {field: "COALESCE(artist_annotation.play_count, 0)", joinType: JoinArtistAnnotation}, + "artistlastplayed": {field: "artist_annotation.play_date", joinType: JoinArtistAnnotation}, + "artistdateloved": {field: "artist_annotation.starred_at", joinType: JoinArtistAnnotation}, + "artistdaterated": {field: "artist_annotation.rated_at", joinType: JoinArtistAnnotation}, + "mbz_album_id": {field: "media_file.mbz_album_id"}, "mbz_album_artist_id": {field: "media_file.mbz_album_artist_id"}, "mbz_artist_id": {field: "media_file.mbz_artist_id"}, @@ -65,12 +91,13 @@ var fieldMap = map[string]*mappedField{ } type mappedField struct { - field string - order string - isRole bool // true if the field is a role (e.g. "artist", "composer", "conductor", etc.) - isTag bool // true if the field is a tag imported from the file metadata - alias string // name from `mappings.yml` that may differ from the name used in the smart playlist - numeric bool // true if the field/tag should be treated as numeric + field string + order string + isRole bool // true if the field is a role (e.g. "artist", "composer", "conductor", etc.) + isTag bool // true if the field is a tag imported from the file metadata + alias string // name from `mappings.yml` that may differ from the name used in the smart playlist + numeric bool // true if the field/tag should be treated as numeric + joinType JoinType // which additional JOINs this field requires } func mapFields(expr map[string]any) map[string]any { @@ -169,7 +196,7 @@ func (e tagCond) ToSql() (string, []any, error) { } } - cond = fmt.Sprintf("exists (select 1 from json_tree(tags, '$.%s') where key='value' and %s)", + cond = fmt.Sprintf("exists (select 1 from json_tree(media_file.tags, '$.%s') where key='value' and %s)", tagName, cond) if e.not { cond = "not " + cond @@ -189,7 +216,7 @@ type roleCond struct { func (e roleCond) ToSql() (string, []any, error) { cond, args, err := e.cond.ToSql() - cond = fmt.Sprintf(`exists (select 1 from json_tree(participants, '$.%s') where key='name' and %s)`, + cond = fmt.Sprintf(`exists (select 1 from json_tree(media_file.participants, '$.%s') where key='name' and %s)`, e.role, cond) if e.not { cond = "not " + cond @@ -197,6 +224,38 @@ func (e roleCond) ToSql() (string, []any, error) { return cond, args, err } +// fieldJoinType returns the JoinType for a given field name (case-insensitive). +func fieldJoinType(name string) JoinType { + if f, ok := fieldMap[strings.ToLower(name)]; ok { + return f.joinType + } + return JoinNone +} + +// extractJoinTypes walks an expression tree and collects all required JoinType flags. +func extractJoinTypes(expr any) JoinType { + result := JoinNone + switch e := expr.(type) { + case All: + for _, sub := range e { + result |= extractJoinTypes(sub) + } + case Any: + for _, sub := range e { + result |= extractJoinTypes(sub) + } + default: + // Leaf expression: use reflection to check if it's a map with field names + rv := reflect.ValueOf(expr) + if rv.Kind() == reflect.Map && rv.Type().Key().Kind() == reflect.String { + for _, key := range rv.MapKeys() { + result |= fieldJoinType(key.String()) + } + } + } + return result +} + // AddRoles adds roles to the field map. This is used to add all artist roles to the field map, so they can be used in // smart playlists. If a role already exists in the field map, it is ignored, so calls to this function are idempotent. func AddRoles(roles []string) { diff --git a/model/criteria/operators_test.go b/model/criteria/operators_test.go index 4c1db130..f0681af6 100644 --- a/model/criteria/operators_test.go +++ b/model/criteria/operators_test.go @@ -54,23 +54,43 @@ var _ = Describe("Operators", func() { Entry("inTheLast", InTheLast{"lastPlayed": 30}, "annotation.play_date > ?", StartOfPeriod(30, time.Now())), Entry("notInTheLast", NotInTheLast{"lastPlayed": 30}, "(annotation.play_date < ? OR annotation.play_date IS NULL)", StartOfPeriod(30, time.Now())), + // Album annotation fields + Entry("albumRating", Gt{"albumRating": 3}, "COALESCE(album_annotation.rating, 0) > ?", 3), + Entry("albumLoved", Is{"albumLoved": true}, "COALESCE(album_annotation.starred, false) = ?", true), + Entry("albumPlayCount", Gt{"albumPlayCount": 5}, "COALESCE(album_annotation.play_count, 0) > ?", 5), + Entry("albumLastPlayed", After{"albumLastPlayed": rangeStart}, "album_annotation.play_date > ?", rangeStart), + Entry("albumDateLoved", Before{"albumDateLoved": rangeStart}, "album_annotation.starred_at < ?", rangeStart), + Entry("albumDateRated", After{"albumDateRated": rangeStart}, "album_annotation.rated_at > ?", rangeStart), + Entry("albumLastPlayed inTheLast", InTheLast{"albumLastPlayed": 30}, "album_annotation.play_date > ?", StartOfPeriod(30, time.Now())), + Entry("albumLastPlayed notInTheLast", NotInTheLast{"albumLastPlayed": 30}, "(album_annotation.play_date < ? OR album_annotation.play_date IS NULL)", StartOfPeriod(30, time.Now())), + + // Artist annotation fields + Entry("artistRating", Gt{"artistRating": 3}, "COALESCE(artist_annotation.rating, 0) > ?", 3), + Entry("artistLoved", Is{"artistLoved": true}, "COALESCE(artist_annotation.starred, false) = ?", true), + Entry("artistPlayCount", Gt{"artistPlayCount": 5}, "COALESCE(artist_annotation.play_count, 0) > ?", 5), + Entry("artistLastPlayed", After{"artistLastPlayed": rangeStart}, "artist_annotation.play_date > ?", rangeStart), + Entry("artistDateLoved", Before{"artistDateLoved": rangeStart}, "artist_annotation.starred_at < ?", rangeStart), + Entry("artistDateRated", After{"artistDateRated": rangeStart}, "artist_annotation.rated_at > ?", rangeStart), + Entry("artistLastPlayed inTheLast", InTheLast{"artistLastPlayed": 30}, "artist_annotation.play_date > ?", StartOfPeriod(30, time.Now())), + Entry("artistLastPlayed notInTheLast", NotInTheLast{"artistLastPlayed": 30}, "(artist_annotation.play_date < ? OR artist_annotation.play_date IS NULL)", StartOfPeriod(30, time.Now())), + // Tag tests - Entry("tag is [string]", Is{"genre": "Rock"}, "exists (select 1 from json_tree(tags, '$.genre') where key='value' and value = ?)", "Rock"), - Entry("tag isNot [string]", IsNot{"genre": "Rock"}, "not exists (select 1 from json_tree(tags, '$.genre') where key='value' and value = ?)", "Rock"), - Entry("tag gt", Gt{"genre": "A"}, "exists (select 1 from json_tree(tags, '$.genre') where key='value' and value > ?)", "A"), - Entry("tag lt", Lt{"genre": "Z"}, "exists (select 1 from json_tree(tags, '$.genre') where key='value' and value < ?)", "Z"), - Entry("tag contains", Contains{"genre": "Rock"}, "exists (select 1 from json_tree(tags, '$.genre') where key='value' and value LIKE ?)", "%Rock%"), - Entry("tag not contains", NotContains{"genre": "Rock"}, "not exists (select 1 from json_tree(tags, '$.genre') where key='value' and value LIKE ?)", "%Rock%"), - Entry("tag startsWith", StartsWith{"genre": "Soft"}, "exists (select 1 from json_tree(tags, '$.genre') where key='value' and value LIKE ?)", "Soft%"), - Entry("tag endsWith", EndsWith{"genre": "Rock"}, "exists (select 1 from json_tree(tags, '$.genre') where key='value' and value LIKE ?)", "%Rock"), + Entry("tag is [string]", Is{"genre": "Rock"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value = ?)", "Rock"), + Entry("tag isNot [string]", IsNot{"genre": "Rock"}, "not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value = ?)", "Rock"), + Entry("tag gt", Gt{"genre": "A"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value > ?)", "A"), + Entry("tag lt", Lt{"genre": "Z"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value < ?)", "Z"), + Entry("tag contains", Contains{"genre": "Rock"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value LIKE ?)", "%Rock%"), + Entry("tag not contains", NotContains{"genre": "Rock"}, "not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value LIKE ?)", "%Rock%"), + Entry("tag startsWith", StartsWith{"genre": "Soft"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value LIKE ?)", "Soft%"), + Entry("tag endsWith", EndsWith{"genre": "Rock"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value LIKE ?)", "%Rock"), // Artist roles tests - Entry("role is [string]", Is{"artist": "u2"}, "exists (select 1 from json_tree(participants, '$.artist') where key='name' and value = ?)", "u2"), - Entry("role isNot [string]", IsNot{"artist": "u2"}, "not exists (select 1 from json_tree(participants, '$.artist') where key='name' and value = ?)", "u2"), - Entry("role contains [string]", Contains{"artist": "u2"}, "exists (select 1 from json_tree(participants, '$.artist') where key='name' and value LIKE ?)", "%u2%"), - Entry("role not contains [string]", NotContains{"artist": "u2"}, "not exists (select 1 from json_tree(participants, '$.artist') where key='name' and value LIKE ?)", "%u2%"), - Entry("role startsWith [string]", StartsWith{"composer": "John"}, "exists (select 1 from json_tree(participants, '$.composer') where key='name' and value LIKE ?)", "John%"), - Entry("role endsWith [string]", EndsWith{"composer": "Lennon"}, "exists (select 1 from json_tree(participants, '$.composer') where key='name' and value LIKE ?)", "%Lennon"), + Entry("role is [string]", Is{"artist": "u2"}, "exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value = ?)", "u2"), + Entry("role isNot [string]", IsNot{"artist": "u2"}, "not exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value = ?)", "u2"), + Entry("role contains [string]", Contains{"artist": "u2"}, "exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value LIKE ?)", "%u2%"), + Entry("role not contains [string]", NotContains{"artist": "u2"}, "not exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value LIKE ?)", "%u2%"), + Entry("role startsWith [string]", StartsWith{"composer": "John"}, "exists (select 1 from json_tree(media_file.participants, '$.composer') where key='name' and value LIKE ?)", "John%"), + Entry("role endsWith [string]", EndsWith{"composer": "Lennon"}, "exists (select 1 from json_tree(media_file.participants, '$.composer') where key='name' and value LIKE ?)", "%Lennon"), ) // TODO Validate operators that are not valid for each field type. @@ -88,7 +108,7 @@ var _ = Describe("Operators", func() { op := EndsWith{"mood": "Soft"} sql, args, err := op.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.mood') where key='value' and value LIKE ?)")) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(media_file.tags, '$.mood') where key='value' and value LIKE ?)")) gomega.Expect(args).To(gomega.HaveExactElements("%Soft")) }) It("casts numeric comparisons", func() { @@ -96,7 +116,7 @@ var _ = Describe("Operators", func() { op := Lt{"rate": 6} sql, args, err := op.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.rate') where key='value' and CAST(value AS REAL) < ?)")) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(media_file.tags, '$.rate') where key='value' and CAST(value AS REAL) < ?)")) gomega.Expect(args).To(gomega.HaveExactElements(6)) }) It("skips unknown tag names", func() { @@ -110,7 +130,7 @@ var _ = Describe("Operators", func() { op := Contains{"releasetype": "soundtrack"} sql, args, err := op.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value LIKE ?)")) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(media_file.tags, '$.releasetype') where key='value' and value LIKE ?)")) gomega.Expect(args).To(gomega.HaveExactElements("%soundtrack%")) }) It("supports albumtype as alias for releasetype", func() { @@ -118,7 +138,7 @@ var _ = Describe("Operators", func() { op := Contains{"albumtype": "live"} sql, args, err := op.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value LIKE ?)")) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(media_file.tags, '$.releasetype') where key='value' and value LIKE ?)")) gomega.Expect(args).To(gomega.HaveExactElements("%live%")) }) It("supports albumtype alias with Is operator", func() { @@ -127,7 +147,7 @@ var _ = Describe("Operators", func() { sql, args, err := op.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) // Should query $.releasetype, not $.albumtype - gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value = ?)")) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(media_file.tags, '$.releasetype') where key='value' and value = ?)")) gomega.Expect(args).To(gomega.HaveExactElements("album")) }) It("supports albumtype alias with IsNot operator", func() { @@ -136,7 +156,7 @@ var _ = Describe("Operators", func() { sql, args, err := op.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) // Should query $.releasetype, not $.albumtype - gomega.Expect(sql).To(gomega.Equal("not exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value = ?)")) + gomega.Expect(sql).To(gomega.Equal("not exists (select 1 from json_tree(media_file.tags, '$.releasetype') where key='value' and value = ?)")) gomega.Expect(args).To(gomega.HaveExactElements("compilation")) }) }) @@ -147,7 +167,7 @@ var _ = Describe("Operators", func() { op := EndsWith{"producer": "Eno"} sql, args, err := op.ToSql() gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(participants, '$.producer') where key='name' and value LIKE ?)")) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(media_file.participants, '$.producer') where key='name' and value LIKE ?)")) gomega.Expect(args).To(gomega.HaveExactElements("%Eno")) }) It("skips unknown roles", func() { diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index 955a13bc..11b9cd8b 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -241,10 +241,25 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { } 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" + - " AND annotation.item_type = 'media_file'" + - " AND annotation.user_id = '" + usr.ID + "')") + From("media_file").LeftJoin("annotation on ("+ + "annotation.item_id = media_file.id"+ + " AND annotation.item_type = 'media_file'"+ + " AND annotation.user_id = ?)", usr.ID) + + // Conditionally join album/artist annotation tables only when referenced by criteria or sort + requiredJoins := rules.RequiredJoins() + if requiredJoins.Has(criteria.JoinAlbumAnnotation) { + sq = sq.LeftJoin("annotation AS album_annotation ON ("+ + "album_annotation.item_id = media_file.album_id"+ + " AND album_annotation.item_type = 'album'"+ + " AND album_annotation.user_id = ?)", usr.ID) + } + if requiredJoins.Has(criteria.JoinArtistAnnotation) { + sq = sq.LeftJoin("annotation AS artist_annotation ON ("+ + "artist_annotation.item_id = media_file.artist_id"+ + " AND artist_annotation.item_type = 'artist'"+ + " AND artist_annotation.user_id = ?)", usr.ID) + } // Only include media files from libraries the user has access to sq = r.applyLibraryFilter(sq, "media_file") diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 5230390f..c091cb32 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -287,6 +287,106 @@ var _ = Describe("PlaylistRepository", func() { }) }) + Describe("Smart Playlists with Album/Artist Annotation Criteria", func() { + var testPlaylistID string + + AfterEach(func() { + if testPlaylistID != "" { + _ = repo.Delete(testPlaylistID) + testPlaylistID = "" + } + }) + + It("matches tracks from starred albums using albumLoved", func() { + // albumRadioactivity (ID "103") is starred in test fixtures + // Songs in album 103: 1003, 1004, 1005, 1006 + rules := &criteria.Criteria{ + Expression: criteria.All{ + criteria.Is{"albumLoved": true}, + }, + } + newPls := model.Playlist{Name: "Starred Album Songs", OwnerID: "userid", Rules: rules} + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second + pls, err := repo.GetWithTracks(newPls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + + trackIDs := make([]string, len(pls.Tracks)) + for i, t := range pls.Tracks { + trackIDs[i] = t.MediaFileID + } + Expect(trackIDs).To(ConsistOf("1003", "1004", "1005", "1006")) + }) + + It("matches tracks from starred artists using artistLoved", func() { + // artistBeatles (ID "3") is starred in test fixtures + // Songs with ArtistID "3": 1001, 1002, 3002 + rules := &criteria.Criteria{ + Expression: criteria.All{ + criteria.Is{"artistLoved": true}, + }, + } + newPls := model.Playlist{Name: "Starred Artist Songs", OwnerID: "userid", Rules: rules} + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second + pls, err := repo.GetWithTracks(newPls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + + trackIDs := make([]string, len(pls.Tracks)) + for i, t := range pls.Tracks { + trackIDs[i] = t.MediaFileID + } + Expect(trackIDs).To(ConsistOf("1001", "1002", "3002")) + }) + + It("matches tracks with combined album and artist criteria", func() { + // albumLoved=true → songs from album 103 (1003, 1004, 1005, 1006) + // artistLoved=true → songs with artist 3 (1001, 1002) + // Using Any: union of both sets + rules := &criteria.Criteria{ + Expression: criteria.Any{ + criteria.Is{"albumLoved": true}, + criteria.Is{"artistLoved": true}, + }, + } + newPls := model.Playlist{Name: "Combined Album+Artist", OwnerID: "userid", Rules: rules} + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second + pls, err := repo.GetWithTracks(newPls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + + trackIDs := make([]string, len(pls.Tracks)) + for i, t := range pls.Tracks { + trackIDs[i] = t.MediaFileID + } + Expect(trackIDs).To(ConsistOf("1001", "1002", "1003", "1004", "1005", "1006", "3002")) + }) + + It("returns no tracks when no albums/artists match", func() { + // No album has rating 5 in fixtures + rules := &criteria.Criteria{ + Expression: criteria.All{ + criteria.Is{"albumRating": 5}, + }, + } + newPls := model.Playlist{Name: "No Match", OwnerID: "userid", Rules: rules} + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second + pls, err := repo.GetWithTracks(newPls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + + Expect(pls.Tracks).To(BeEmpty()) + }) + }) + Describe("Smart Playlists with Tag Criteria", func() { var mfRepo model.MediaFileRepository var testPlaylistID string