mirror of
https://github.com/navidrome/navidrome.git
synced 2026-05-03 06:51:16 +00:00
1bd736dae9
* test: add tests for recordingdate alias resolution in smart playlists
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: update FieldInfo structure and simplify fieldMap initialization
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move sort parsing logic from persistence to criteria package
Extracted sort field parsing, validation, and direction handling from
persistence/criteria_sql.go into model/criteria/sort.go. The new
OrderByFields method on Criteria parses the Sort/Order strings into
validated SortField structs (field name + direction), resolving aliases
and handling +/- prefixes and order inversion. The persistence layer now
consumes these parsed fields and only handles SQL expression mapping.
This centralizes sort parsing to enforce consistent implementations.
* refactor: standardize field access in smartPlaylistCriteria structure
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: add ResolveLimit method to Criteria
Moved the percentage-limit resolution logic from playlist_repository
into Criteria.ResolveLimit, replacing the 3-line mutate-after-query
pattern with a single method call. The method preserves LimitPercent
rather than zeroing it, since IsPercentageLimit already returns false
once Limit is set, making the clear redundant and lossy.
* refactor: improve child playlist loading and error handling in refresh logic
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: extract smart playlist logic to dedicated files
Moved refreshSmartPlaylist, addSmartPlaylistAnnotationJoins, and
addCriteria methods from playlist_repository.go to a new
smart_playlist_repository.go file. Extracted all smart playlist tests
to smart_playlist_repository_test.go. Added DeferCleanup to the
"valid rules" test to fix ordering flakiness when Ginkgo randomizes
test execution across files.
* refactor: break refreshSmartPlaylist into smaller focused methods
Split the monolithic refreshSmartPlaylist method into discrete helpers
for readability: shouldRefreshSmartPlaylist for guard checks,
refreshChildPlaylists for recursive dependency refresh,
resolvePercentageLimit for count-based limit resolution,
buildSmartPlaylistQuery for assembling the SELECT with joins, and
addMediaFileAnnotationJoin to DRY up the repeated annotation join clause.
* refactor: deduplicate child playlist IDs in Criteria
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: simplify withSmartPlaylistOwner to accept model.User
Replaced separate ownerID string and ownerIsAdmin bool parameters with a
single model.User struct, reducing the field count in smartPlaylistCriteria
and making the option function signature clearer. Updated all call sites
and tests accordingly.
* fix: handle empty sort fields and propagate child playlist load errors
OrderByFields now falls back to [{title, asc}] when all user-supplied
sort fields are invalid, preventing empty ORDER BY clauses that would
produce invalid SQL in row_number() window functions. Also restored the
original behavior where a DB error loading child playlists aborts the
parent smart playlist refresh, by making refreshChildPlaylists return a
bool.
* refactor: log warning when no valid sort fields are found
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
321 lines
9.1 KiB
Go
321 lines
9.1 KiB
Go
package criteria
|
|
|
|
import (
|
|
"bytes"
|
|
"encoding/json"
|
|
|
|
"github.com/google/uuid"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
"github.com/onsi/gomega"
|
|
)
|
|
|
|
var _ = Describe("Criteria", func() {
|
|
var goObj Criteria
|
|
var jsonObj string
|
|
|
|
Context("with a complex criteria", func() {
|
|
BeforeEach(func() {
|
|
goObj = Criteria{
|
|
Expression: All{
|
|
Contains{"title": "love"},
|
|
NotContains{"title": "hate"},
|
|
Any{
|
|
IsNot{"artist": "u2"},
|
|
Is{"album": "best of"},
|
|
},
|
|
All{
|
|
StartsWith{"comment": "this"},
|
|
InTheRange{"year": []int{1980, 1990}},
|
|
IsNot{"genre": "Rock"},
|
|
Gt{"albumrating": 3},
|
|
},
|
|
},
|
|
Sort: "title",
|
|
Order: "asc",
|
|
Limit: 20,
|
|
Offset: 10,
|
|
}
|
|
var b bytes.Buffer
|
|
err := json.Compact(&b, []byte(`
|
|
{
|
|
"all": [
|
|
{ "contains": {"title": "love"} },
|
|
{ "notContains": {"title": "hate"} },
|
|
{ "any": [
|
|
{ "isNot": {"artist": "u2"} },
|
|
{ "is": {"album": "best of"} }
|
|
]
|
|
},
|
|
{ "all": [
|
|
{ "startsWith": {"comment": "this"} },
|
|
{ "inTheRange": {"year":[1980,1990]} },
|
|
{ "isNot": { "genre": "Rock" }},
|
|
{ "gt": { "albumrating": 3 } }
|
|
]
|
|
}
|
|
],
|
|
"sort": "title",
|
|
"order": "asc",
|
|
"limit": 20,
|
|
"offset": 10
|
|
}
|
|
`))
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
jsonObj = b.String()
|
|
})
|
|
It("marshals to JSON", func() {
|
|
j, err := json.Marshal(goObj)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(string(j)).To(gomega.Equal(jsonObj))
|
|
})
|
|
It("is reversible to/from JSON", func() {
|
|
var newObj Criteria
|
|
err := json.Unmarshal([]byte(jsonObj), &newObj)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
j, err := json.Marshal(newObj)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(string(j)).To(gomega.Equal(jsonObj))
|
|
})
|
|
})
|
|
|
|
Describe("LimitPercent", func() {
|
|
Describe("JSON round-trip", func() {
|
|
It("marshals and unmarshals limitPercent", func() {
|
|
goObj := Criteria{
|
|
Expression: All{Contains{"title": "love"}},
|
|
Sort: "title",
|
|
Order: "asc",
|
|
LimitPercent: 10,
|
|
}
|
|
j, err := json.Marshal(goObj)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(string(j)).To(gomega.ContainSubstring(`"limitPercent":10`))
|
|
gomega.Expect(string(j)).ToNot(gomega.ContainSubstring(`"limit"`))
|
|
|
|
var newObj Criteria
|
|
err = json.Unmarshal(j, &newObj)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(newObj.LimitPercent).To(gomega.Equal(10))
|
|
gomega.Expect(newObj.Limit).To(gomega.Equal(0))
|
|
})
|
|
|
|
It("does not include limitPercent when zero", func() {
|
|
goObj := Criteria{
|
|
Expression: All{Contains{"title": "love"}},
|
|
Limit: 50,
|
|
}
|
|
j, err := json.Marshal(goObj)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(string(j)).To(gomega.ContainSubstring(`"limit":50`))
|
|
gomega.Expect(string(j)).ToNot(gomega.ContainSubstring(`limitPercent`))
|
|
})
|
|
|
|
It("backward compatible: JSON with only limit still works", func() {
|
|
jsonStr := `{"all":[{"contains":{"title":"love"}}],"limit":20}`
|
|
var c Criteria
|
|
err := json.Unmarshal([]byte(jsonStr), &c)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(c.Limit).To(gomega.Equal(20))
|
|
gomega.Expect(c.LimitPercent).To(gomega.Equal(0))
|
|
})
|
|
})
|
|
|
|
Describe("UnmarshalJSON clamping", func() {
|
|
It("clamps values above 100 to 100", func() {
|
|
jsonStr := `{"all":[{"contains":{"title":"love"}}],"limitPercent":150}`
|
|
var c Criteria
|
|
err := json.Unmarshal([]byte(jsonStr), &c)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(c.LimitPercent).To(gomega.Equal(100))
|
|
})
|
|
|
|
It("clamps negative values to 0", func() {
|
|
jsonStr := `{"all":[{"contains":{"title":"love"}}],"limitPercent":-5}`
|
|
var c Criteria
|
|
err := json.Unmarshal([]byte(jsonStr), &c)
|
|
gomega.Expect(err).ToNot(gomega.HaveOccurred())
|
|
gomega.Expect(c.LimitPercent).To(gomega.Equal(0))
|
|
})
|
|
})
|
|
|
|
Describe("EffectiveLimit", func() {
|
|
It("returns fixed limit when Limit is set", func() {
|
|
c := Criteria{Limit: 50, LimitPercent: 10}
|
|
gomega.Expect(c.EffectiveLimit(1000)).To(gomega.Equal(50))
|
|
})
|
|
|
|
It("returns percentage-based limit", func() {
|
|
c := Criteria{LimitPercent: 10}
|
|
gomega.Expect(c.EffectiveLimit(450)).To(gomega.Equal(45))
|
|
})
|
|
|
|
It("returns minimum 1 when totalCount > 0 and percentage rounds to 0", func() {
|
|
c := Criteria{LimitPercent: 1}
|
|
gomega.Expect(c.EffectiveLimit(5)).To(gomega.Equal(1))
|
|
})
|
|
|
|
It("returns 0 when totalCount is 0", func() {
|
|
c := Criteria{LimitPercent: 10}
|
|
gomega.Expect(c.EffectiveLimit(0)).To(gomega.Equal(0))
|
|
})
|
|
|
|
It("returns 0 when no limit is set", func() {
|
|
c := Criteria{}
|
|
gomega.Expect(c.EffectiveLimit(1000)).To(gomega.Equal(0))
|
|
})
|
|
|
|
It("returns full count for 100%", func() {
|
|
c := Criteria{LimitPercent: 100}
|
|
gomega.Expect(c.EffectiveLimit(450)).To(gomega.Equal(450))
|
|
})
|
|
|
|
It("returns 1 for 1% of 50 items", func() {
|
|
c := Criteria{LimitPercent: 1}
|
|
gomega.Expect(c.EffectiveLimit(50)).To(gomega.Equal(1))
|
|
})
|
|
})
|
|
|
|
Describe("ResolveLimit", func() {
|
|
It("resolves percentage to absolute limit preserving LimitPercent", func() {
|
|
c := Criteria{LimitPercent: 10}
|
|
c.ResolveLimit(450)
|
|
gomega.Expect(c.Limit).To(gomega.Equal(45))
|
|
})
|
|
|
|
It("does nothing when Limit is already set", func() {
|
|
c := Criteria{Limit: 50, LimitPercent: 10}
|
|
c.ResolveLimit(1000)
|
|
gomega.Expect(c.Limit).To(gomega.Equal(50))
|
|
})
|
|
|
|
It("does nothing when no limit is configured", func() {
|
|
c := Criteria{}
|
|
c.ResolveLimit(1000)
|
|
gomega.Expect(c.Limit).To(gomega.Equal(0))
|
|
})
|
|
|
|
It("sets minimum 1 when percentage rounds to 0 and totalCount > 0", func() {
|
|
c := Criteria{LimitPercent: 1}
|
|
c.ResolveLimit(5)
|
|
gomega.Expect(c.Limit).To(gomega.Equal(1))
|
|
})
|
|
|
|
It("is idempotent when called twice", func() {
|
|
c := Criteria{LimitPercent: 10}
|
|
c.ResolveLimit(450)
|
|
c.ResolveLimit(450)
|
|
gomega.Expect(c.Limit).To(gomega.Equal(45))
|
|
})
|
|
})
|
|
|
|
Describe("IsPercentageLimit", func() {
|
|
It("returns true when LimitPercent is set and Limit is 0", func() {
|
|
c := Criteria{LimitPercent: 10}
|
|
gomega.Expect(c.IsPercentageLimit()).To(gomega.BeTrue())
|
|
})
|
|
|
|
It("returns false when Limit is set", func() {
|
|
c := Criteria{Limit: 50, LimitPercent: 10}
|
|
gomega.Expect(c.IsPercentageLimit()).To(gomega.BeFalse())
|
|
})
|
|
|
|
It("returns false when neither is set", func() {
|
|
c := Criteria{}
|
|
gomega.Expect(c.IsPercentageLimit()).To(gomega.BeFalse())
|
|
})
|
|
|
|
It("returns false when LimitPercent is out of range", func() {
|
|
c := Criteria{LimitPercent: 150}
|
|
gomega.Expect(c.IsPercentageLimit()).To(gomega.BeFalse())
|
|
})
|
|
})
|
|
})
|
|
|
|
Context("with child playlists", func() {
|
|
var (
|
|
topLevelInPlaylistID string
|
|
topLevelNotInPlaylistID string
|
|
nestedAnyInPlaylistID string
|
|
nestedAnyNotInPlaylistID string
|
|
nestedAllInPlaylistID string
|
|
nestedAllNotInPlaylistID string
|
|
)
|
|
BeforeEach(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},
|
|
},
|
|
},
|
|
}
|
|
})
|
|
It("extracts all child smart playlist IDs from expression criteria", func() {
|
|
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() {
|
|
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))
|
|
})
|
|
It("returns empty list when no child playlist IDs are present", func() {
|
|
ids := Criteria{}.ChildPlaylistIds()
|
|
gomega.Expect(ids).To(gomega.BeEmpty())
|
|
})
|
|
It("returns empty list for leaf expressions", func() {
|
|
ids := Criteria{Expression: Is{"title": "Low Rider"}}.ChildPlaylistIds()
|
|
gomega.Expect(ids).To(gomega.BeEmpty())
|
|
})
|
|
It("deduplicates repeated playlist IDs", func() {
|
|
sharedID := uuid.NewString()
|
|
goObj = Criteria{
|
|
Expression: All{
|
|
InPlaylist{"id": sharedID},
|
|
Any{
|
|
InPlaylist{"id": sharedID},
|
|
NotInPlaylist{"id": sharedID},
|
|
},
|
|
},
|
|
}
|
|
ids := goObj.ChildPlaylistIds()
|
|
gomega.Expect(ids).To(gomega.Equal([]string{sharedID}))
|
|
})
|
|
})
|
|
})
|