mirror of
https://github.com/navidrome/navidrome.git
synced 2026-05-03 06:51:16 +00:00
feat(smartplaylist): add isMissing and isPresent operators (#5436)
* feat(smartplaylist): add IsMissing and IsPresent operator types Add two new Expression types for detecting absent/present tags and roles in smart playlist criteria. Includes JSON marshal/unmarshal support and Walk visitor registration. * test(smartplaylist): add JSON marshal/unmarshal tests for isMissing/isPresent * feat(smartplaylist): add SQL generation for isMissing/isPresent operators Tags check json_tree(media_file.tags) for key existence. Roles check json_tree(media_file.participants) for key existence. Regular DB column fields are rejected with an error. * test(smartplaylist): add e2e tests for isMissing/isPresent operators Tests cover tag presence/absence with selective matching (grouping), universal absence (lyricist role), universal presence (composer role), and combined operator usage. * refactor(smartplaylist): use strconv.ParseBool in IsTruthy Replace hand-rolled string truthiness check with strconv.ParseBool, which correctly handles standard boolean strings and rejects unrecognized values as false. * refactor(smartplaylist): clarify missingExpr parameter naming Rename defaultNegate to checkAbsence and extract truthy local for readability. The XNOR logic (checkAbsence == truthy) is now easier to follow: isMissing passes true, isPresent passes false. * refactor(smartplaylist): reuse jsonExpr in missingExpr, improve errors - tagCond/roleCond now handle nil cond (existence-only check) - missingExpr delegates to jsonExpr(info, nil, negate) instead of building SQL manually - Better error messages: unknown fields now report the field name
This commit is contained in:
parent
d5ba61adf8
commit
46b4dcd5f6
@ -69,6 +69,10 @@ func unmarshalExpression(opName string, rawValue json.RawMessage) Expression {
|
||||
return InPlaylist(m)
|
||||
case "notinplaylist":
|
||||
return NotInPlaylist(m)
|
||||
case "ismissing":
|
||||
return IsMissing(m)
|
||||
case "ispresent":
|
||||
return IsPresent(m)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -1,6 +1,9 @@
|
||||
package criteria
|
||||
|
||||
import "time"
|
||||
import (
|
||||
"strconv"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Conjunctions need to implement this interface, to allow Criteria to extract child playlist IDs recursively
|
||||
type conjunction interface {
|
||||
@ -162,6 +165,36 @@ func (nipl NotInPlaylist) MarshalJSON() ([]byte, error) {
|
||||
|
||||
func (nipl NotInPlaylist) fields() map[string]any { return nipl }
|
||||
|
||||
type IsMissing map[string]any
|
||||
|
||||
func (im IsMissing) MarshalJSON() ([]byte, error) {
|
||||
return marshalExpression("isMissing", im)
|
||||
}
|
||||
|
||||
func (im IsMissing) fields() map[string]any { return im }
|
||||
|
||||
type IsPresent map[string]any
|
||||
|
||||
func (ip IsPresent) MarshalJSON() ([]byte, error) {
|
||||
return marshalExpression("isPresent", ip)
|
||||
}
|
||||
|
||||
func (ip IsPresent) fields() map[string]any { return ip }
|
||||
|
||||
func IsTruthy(v any) bool {
|
||||
switch val := v.(type) {
|
||||
case bool:
|
||||
return val
|
||||
case float64:
|
||||
return val != 0
|
||||
case string:
|
||||
b, err := strconv.ParseBool(val)
|
||||
return err == nil && b
|
||||
default:
|
||||
return v != nil
|
||||
}
|
||||
}
|
||||
|
||||
func extractPlaylistIds(inputRule any) (ids []string) {
|
||||
var id string
|
||||
var ok bool
|
||||
|
||||
@ -46,5 +46,9 @@ var _ = Describe("Operators", func() {
|
||||
Entry("notInTheLast", NotInTheLast{"lastPlayed": 30.0}, `{"notInTheLast":{"lastPlayed":30}}`),
|
||||
Entry("inPlaylist", InPlaylist{"id": "deadbeef-dead-beef"}, `{"inPlaylist":{"id":"deadbeef-dead-beef"}}`),
|
||||
Entry("notInPlaylist", NotInPlaylist{"id": "deadbeef-dead-beef"}, `{"notInPlaylist":{"id":"deadbeef-dead-beef"}}`),
|
||||
Entry("isMissing [true]", IsMissing{"genre": true}, `{"isMissing":{"genre":true}}`),
|
||||
Entry("isMissing [false]", IsMissing{"genre": false}, `{"isMissing":{"genre":false}}`),
|
||||
Entry("isPresent [true]", IsPresent{"genre": true}, `{"isPresent":{"genre":true}}`),
|
||||
Entry("isPresent [false]", IsPresent{"genre": false}, `{"isPresent":{"genre":false}}`),
|
||||
)
|
||||
})
|
||||
|
||||
@ -24,7 +24,7 @@ func Walk(expr Expression, visit Visitor) error {
|
||||
return err
|
||||
}
|
||||
}
|
||||
case Is, IsNot, Gt, Lt, Before, After, Contains, NotContains, StartsWith, EndsWith, InTheRange, InTheLast, NotInTheLast, InPlaylist, NotInPlaylist:
|
||||
case Is, IsNot, Gt, Lt, Before, After, Contains, NotContains, StartsWith, EndsWith, InTheRange, InTheLast, NotInTheLast, InPlaylist, NotInPlaylist, IsMissing, IsPresent:
|
||||
return nil
|
||||
default:
|
||||
return fmt.Errorf("unknown criteria expression type %T", expr)
|
||||
|
||||
@ -185,6 +185,10 @@ func (c smartPlaylistCriteria) exprSQL(expr criteria.Expression) (squirrel.Sqliz
|
||||
return c.inList(e, false)
|
||||
case criteria.NotInPlaylist:
|
||||
return c.inList(e, true)
|
||||
case criteria.IsMissing:
|
||||
return missingExpr(e, true)
|
||||
case criteria.IsPresent:
|
||||
return missingExpr(e, false)
|
||||
default:
|
||||
return nil, fmt.Errorf("unknown criteria expression type %T", expr)
|
||||
}
|
||||
@ -201,6 +205,22 @@ func isNotExpr(values map[string]any) (squirrel.Sqlizer, error) {
|
||||
return squirrel.NotEq(fields), nil
|
||||
}
|
||||
|
||||
func missingExpr(values map[string]any, checkAbsence bool) (squirrel.Sqlizer, error) {
|
||||
field, value, info, ok := singleField(values)
|
||||
if !ok {
|
||||
if len(values) != 1 {
|
||||
return nil, fmt.Errorf("invalid field in criteria: isMissing/isPresent requires exactly one field")
|
||||
}
|
||||
return nil, fmt.Errorf("invalid field in criteria: %s", field)
|
||||
}
|
||||
if !info.IsTag && !info.IsRole {
|
||||
return nil, fmt.Errorf("isMissing/isPresent operator is only supported for tag and role fields, got: %s", field)
|
||||
}
|
||||
|
||||
negate := checkAbsence == criteria.IsTruthy(value)
|
||||
return jsonExpr(info, nil, negate), nil
|
||||
}
|
||||
|
||||
func mapExpr(values map[string]any, makeCond func(map[string]any) squirrel.Sqlizer, negateJSON bool) (squirrel.Sqlizer, error) {
|
||||
if _, value, info, ok := singleField(values); ok && (info.IsTag || info.IsRole) {
|
||||
return jsonExpr(info, makeCond(map[string]any{"value": value}), negateJSON), nil
|
||||
@ -327,11 +347,18 @@ type tagCond struct {
|
||||
}
|
||||
|
||||
func (e tagCond) ToSql() (string, []any, error) {
|
||||
cond, args, err := e.cond.ToSql()
|
||||
if e.numeric {
|
||||
cond = strings.ReplaceAll(cond, "value", "CAST(value AS REAL)")
|
||||
var cond string
|
||||
var args []any
|
||||
var err error
|
||||
if e.cond != nil {
|
||||
cond, args, err = e.cond.ToSql()
|
||||
if e.numeric {
|
||||
cond = strings.ReplaceAll(cond, "value", "CAST(value AS REAL)")
|
||||
}
|
||||
cond = fmt.Sprintf("exists (select 1 from json_tree(media_file.tags, '$.%s') where key='value' and %s)", e.tag, cond)
|
||||
} else {
|
||||
cond = fmt.Sprintf("exists (select 1 from json_tree(media_file.tags, '$.%s') where key='value')", e.tag)
|
||||
}
|
||||
cond = fmt.Sprintf("exists (select 1 from json_tree(media_file.tags, '$.%s') where key='value' and %s)", e.tag, cond)
|
||||
if e.not {
|
||||
cond = "not " + cond
|
||||
}
|
||||
@ -345,8 +372,15 @@ 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(media_file.participants, '$.%s') where key='name' and %s)", e.role, cond)
|
||||
var cond string
|
||||
var args []any
|
||||
var err error
|
||||
if e.cond != nil {
|
||||
cond, args, err = e.cond.ToSql()
|
||||
cond = fmt.Sprintf("exists (select 1 from json_tree(media_file.participants, '$.%s') where key='name' and %s)", e.role, cond)
|
||||
} else {
|
||||
cond = fmt.Sprintf("exists (select 1 from json_tree(media_file.participants, '$.%s') where key='name')", e.role)
|
||||
}
|
||||
if e.not {
|
||||
cond = "not " + cond
|
||||
}
|
||||
|
||||
@ -59,6 +59,26 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
||||
Entry("role is", criteria.Is{"artist": "u2"}, "exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value = ?)", "u2"),
|
||||
Entry("role contains", criteria.Contains{"composer": "Lennon"}, "exists (select 1 from json_tree(media_file.participants, '$.composer') where key='name' and value LIKE ?)", "%Lennon%"),
|
||||
Entry("role not contains", criteria.NotContains{"artist": "u2"}, "not exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value LIKE ?)", "%u2%"),
|
||||
// isMissing — tags
|
||||
Entry("isMissing tag [true]", criteria.IsMissing{"genre": true},
|
||||
"not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')"),
|
||||
Entry("isMissing tag [false]", criteria.IsMissing{"genre": false},
|
||||
"exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')"),
|
||||
// isMissing — roles
|
||||
Entry("isMissing role [true]", criteria.IsMissing{"artist": true},
|
||||
"not exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name')"),
|
||||
Entry("isMissing role [false]", criteria.IsMissing{"artist": false},
|
||||
"exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name')"),
|
||||
// isPresent — tags
|
||||
Entry("isPresent tag [true]", criteria.IsPresent{"genre": true},
|
||||
"exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')"),
|
||||
Entry("isPresent tag [false]", criteria.IsPresent{"genre": false},
|
||||
"not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')"),
|
||||
// isPresent — roles
|
||||
Entry("isPresent role [true]", criteria.IsPresent{"composer": true},
|
||||
"exists (select 1 from json_tree(media_file.participants, '$.composer') where key='name')"),
|
||||
Entry("isPresent role [false]", criteria.IsPresent{"composer": false},
|
||||
"not exists (select 1 from json_tree(media_file.participants, '$.composer') where key='name')"),
|
||||
)
|
||||
|
||||
Describe("playlist permissions", func() {
|
||||
@ -115,6 +135,16 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
||||
Expect(err).To(MatchError("invalid field in criteria: unknown"))
|
||||
})
|
||||
|
||||
It("returns an error when isMissing is used with a regular field", func() {
|
||||
_, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: criteria.IsMissing{"year": true}}).Where()
|
||||
Expect(err).To(MatchError(ContainSubstring("isMissing/isPresent operator is only supported for tag and role fields")))
|
||||
})
|
||||
|
||||
It("returns an error when isPresent is used with a regular field", func() {
|
||||
_, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: criteria.IsPresent{"title": true}}).Where()
|
||||
Expect(err).To(MatchError(ContainSubstring("isMissing/isPresent operator is only supported for tag and role fields")))
|
||||
})
|
||||
|
||||
Describe("sort", func() {
|
||||
It("sorts by regular fields", func() {
|
||||
Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "title"}).OrderBy()).To(Equal("media_file.title asc"))
|
||||
|
||||
@ -116,9 +116,9 @@ func buildTestFS() {
|
||||
fs := storagetest.FakeFS{}
|
||||
fs.SetFiles(fstest.MapFS{
|
||||
"Rock/The Beatles/Abbey Road/01 - Come Together.mp3": abbeyRoad(track(1, "Come Together",
|
||||
_t{"genre": "Rock;Blues", "composer": "Lennon/McCartney", "bpm": 120})),
|
||||
_t{"genre": "Rock;Blues", "composer": "Lennon/McCartney", "bpm": 120, "grouping": "Beatles Tracks"})),
|
||||
"Rock/The Beatles/Abbey Road/02 - Something.mp3": abbeyRoad(track(2, "Something",
|
||||
_t{"genre": "Rock", "composer": "Harrison", "bpm": 100})),
|
||||
_t{"genre": "Rock", "composer": "Harrison", "bpm": 100, "grouping": "Beatles Tracks"})),
|
||||
"Rock/Led Zeppelin/IV/01 - Stairway To Heaven.flac": ledZepIV(track(1, "Stairway To Heaven",
|
||||
_t{"genre": "Rock;Folk", "composer": "Page/Plant", "bpm": 82, "suffix": "flac",
|
||||
"bitrate": 900, "samplerate": 44100, "bitdepth": 16})),
|
||||
|
||||
@ -330,4 +330,45 @@ var _ = Describe("Smart Playlists", func() {
|
||||
})
|
||||
|
||||
})
|
||||
|
||||
Describe("isMissing/isPresent operators", func() {
|
||||
It("isMissing finds tracks without grouping tag", func() {
|
||||
results := evaluateRule(`{"all":[{"isMissing":{"grouping":true}}]}`)
|
||||
Expect(results).To(ConsistOf("Stairway To Heaven", "Black Dog", "So What",
|
||||
"Bohemian Rhapsody", "All Along the Watchtower", "We Are the Champions"))
|
||||
})
|
||||
|
||||
It("isMissing false finds tracks with grouping tag", func() {
|
||||
results := evaluateRule(`{"all":[{"isMissing":{"grouping":false}}]}`)
|
||||
Expect(results).To(ConsistOf("Come Together", "Something"))
|
||||
})
|
||||
|
||||
It("isPresent finds tracks with grouping tag", func() {
|
||||
results := evaluateRule(`{"all":[{"isPresent":{"grouping":true}}]}`)
|
||||
Expect(results).To(ConsistOf("Come Together", "Something"))
|
||||
})
|
||||
|
||||
It("isPresent false finds tracks without grouping tag", func() {
|
||||
results := evaluateRule(`{"all":[{"isPresent":{"grouping":false}}]}`)
|
||||
Expect(results).To(ConsistOf("Stairway To Heaven", "Black Dog", "So What",
|
||||
"Bohemian Rhapsody", "All Along the Watchtower", "We Are the Champions"))
|
||||
})
|
||||
|
||||
It("isMissing returns all tracks for a tag nobody has", func() {
|
||||
results := evaluateRule(`{"all":[{"isMissing":{"lyricist":true}}]}`)
|
||||
Expect(results).To(ConsistOf("Come Together", "Something", "Stairway To Heaven", "Black Dog",
|
||||
"So What", "Bohemian Rhapsody", "All Along the Watchtower", "We Are the Champions"))
|
||||
})
|
||||
|
||||
It("isPresent returns all tracks for a role everyone has", func() {
|
||||
results := evaluateRule(`{"all":[{"isPresent":{"composer":true}}]}`)
|
||||
Expect(results).To(ConsistOf("Come Together", "Something", "Stairway To Heaven", "Black Dog",
|
||||
"So What", "Bohemian Rhapsody", "All Along the Watchtower", "We Are the Champions"))
|
||||
})
|
||||
|
||||
It("combines isMissing with other operators", func() {
|
||||
results := evaluateRule(`{"all":[{"isMissing":{"grouping":true}},{"is":{"genre":"Blues"}}]}`)
|
||||
Expect(results).To(ConsistOf("Black Dog", "All Along the Watchtower"))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user