mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters an album's tracks because every track has its own timestamp. There was no way to express "newest albums first, tracks in album order". Adds five fields backed by the album table: albumdateadded, albumdatemodified, albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded, tracknumber"` now works. They filter as well as sort, which also enables rules like "everything from albums added this month" or "skip singles and EPs". These need the album table rather than the album_annotation table the existing album* fields join, so they get their own bit in the join mask. The bitmask already unions joins from both the expression and the sort fields, so a sort-only reference pulls in the join for the main query while correctly staying out of the percentage-limit count query. No COALESCE default is used. That mechanism exists because an album_annotation row is genuinely often absent; the album row always exists and song_count, duration and size are NOT NULL, so leaving the columns bare keeps filters index-friendly. albumdatemodified follows the existing track-level `datemodified` naming rather than the `albumdateupdated` spelling used in the request. albumreleasedate was considered and rejected: album.release_date is allOrNothing() over the tracks, so it equals the track-level releasedate when they agree and collapses to empty when they disagree. Closes https://github.com/navidrome/navidrome/discussions/5347
This commit is contained in:
parent
d764a1e9d7
commit
dce48ff650
@ -71,6 +71,11 @@ var fieldMap = map[string]FieldInfo{
|
||||
"albumlastplayed": {},
|
||||
"albumdateloved": {},
|
||||
"albumdaterated": {},
|
||||
"albumdateadded": {},
|
||||
"albumdatemodified": {},
|
||||
"albumduration": {},
|
||||
"albumsongcount": {},
|
||||
"albumsize": {},
|
||||
"artistrating": {},
|
||||
"artistloved": {Boolean: true},
|
||||
"artistplaycount": {},
|
||||
|
||||
@ -21,6 +21,7 @@ const (
|
||||
smartPlaylistJoinNone smartPlaylistJoinType = 0
|
||||
smartPlaylistJoinAlbumAnnotation smartPlaylistJoinType = 1 << iota
|
||||
smartPlaylistJoinArtistAnnotation
|
||||
smartPlaylistJoinAlbum
|
||||
)
|
||||
|
||||
func (j smartPlaylistJoinType) has(other smartPlaylistJoinType) bool {
|
||||
@ -103,6 +104,11 @@ var smartPlaylistFields = map[string]smartPlaylistField{
|
||||
"albumlastplayed": {expr: "album_annotation.play_date", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumdateloved": {expr: "album_annotation.starred_at", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumdaterated": {expr: "album_annotation.rated_at", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumdateadded": {expr: "album.created_at", joinType: smartPlaylistJoinAlbum},
|
||||
"albumdatemodified": {expr: "album.updated_at", joinType: smartPlaylistJoinAlbum},
|
||||
"albumduration": {expr: "album.duration", joinType: smartPlaylistJoinAlbum},
|
||||
"albumsongcount": {expr: "album.song_count", joinType: smartPlaylistJoinAlbum},
|
||||
"albumsize": {expr: "album.size", joinType: smartPlaylistJoinAlbum},
|
||||
"artistrating": {expr: "artist_annotation.rating", coalesceDefault: 0, joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistloved": {expr: "artist_annotation.starred", coalesceDefault: false, joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistplaycount": {expr: "artist_annotation.play_count", coalesceDefault: 0, joinType: smartPlaylistJoinArtistAnnotation},
|
||||
|
||||
@ -51,6 +51,11 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
||||
Entry("not in playlist", criteria.NotInPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id NOT IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1),
|
||||
Entry("album annotation", criteria.Gt{"albumRating": 3}, "album_annotation.rating > ?", 3),
|
||||
Entry("artist annotation", criteria.Is{"artistLoved": true}, "artist_annotation.starred = ?", true),
|
||||
Entry("album column", criteria.Gt{"albumSongCount": 5}, "album.song_count > ?", 5),
|
||||
Entry("album duration column", criteria.Lt{"albumDuration": 600}, "album.duration < ?", 600),
|
||||
Entry("album size column", criteria.Gt{"albumSize": 1000}, "album.size > ?", 1000),
|
||||
Entry("album date column", criteria.After{"albumDateAdded": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "album.created_at > ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
|
||||
Entry("album modified column", criteria.Before{"albumDateModified": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "album.updated_at < ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
|
||||
// Annotation fields use a COALESCE default (0 for numeric, false for bool) so that tracks
|
||||
// with no annotation row behave as that default. To keep the annotation index usable, the
|
||||
// COALESCE is dropped when the compared value cannot match the default (the missing-row
|
||||
@ -298,6 +303,11 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
||||
Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "random"}).OrderBy()).To(Equal("random() asc"))
|
||||
})
|
||||
|
||||
It("sorts by album columns bare, with no COALESCE default", func() {
|
||||
Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "-albumDateAdded,trackNumber"}).OrderBy()).
|
||||
To(Equal("album.created_at desc, media_file.track_number asc"))
|
||||
})
|
||||
|
||||
It("sorts by multiple fields", func() {
|
||||
Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "title,-rating"}).OrderBy()).To(Equal("media_file.title asc, COALESCE(annotation.rating, 0) desc"))
|
||||
})
|
||||
@ -323,6 +333,28 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
||||
}
|
||||
})
|
||||
|
||||
It("declares a joinType matching the table each field selects from", func() {
|
||||
// Omitting the joinType still compiles, so without this the field would only fail at
|
||||
// refresh time with "no such column".
|
||||
joinByTable := map[string]smartPlaylistJoinType{
|
||||
"media_file": smartPlaylistJoinNone,
|
||||
"annotation": smartPlaylistJoinNone,
|
||||
"album": smartPlaylistJoinAlbum,
|
||||
"album_annotation": smartPlaylistJoinAlbumAnnotation,
|
||||
"artist_annotation": smartPlaylistJoinArtistAnnotation,
|
||||
}
|
||||
for name, field := range smartPlaylistFields {
|
||||
if field.expr == "" {
|
||||
continue
|
||||
}
|
||||
table, _, ok := strings.Cut(field.expr, ".")
|
||||
Expect(ok).To(BeTrue(), "field %q has expr %q with no table prefix", name, field.expr)
|
||||
want, known := joinByTable[table]
|
||||
Expect(known).To(BeTrue(), "field %q selects from unknown table %q", name, table)
|
||||
Expect(field.joinType).To(Equal(want), "field %q selects from %q but declares the wrong joinType", name, table)
|
||||
}
|
||||
})
|
||||
|
||||
Describe("JSON condition merging", func() {
|
||||
It("merges multiple role conditions in an OR group into a single EXISTS", func() {
|
||||
expr := criteria.Any{
|
||||
@ -585,5 +617,21 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
||||
|
||||
Expect(newSmartPlaylistCriteria(c).RequiredJoins().has(smartPlaylistJoinArtistAnnotation)).To(BeTrue())
|
||||
})
|
||||
|
||||
It("keeps a sort-only album join out of the expression joins", func() {
|
||||
c := criteria.Criteria{Expression: criteria.All{criteria.Contains{"title": "love"}}, Sort: "-albumDateAdded"}
|
||||
cSQL := newSmartPlaylistCriteria(c)
|
||||
|
||||
Expect(cSQL.ExpressionJoins()).To(Equal(smartPlaylistJoinNone))
|
||||
Expect(cSQL.RequiredJoins().has(smartPlaylistJoinAlbum)).To(BeTrue())
|
||||
})
|
||||
|
||||
It("distinguishes the album join from the album annotation join", func() {
|
||||
c := criteria.Criteria{Expression: criteria.All{criteria.Gt{"albumRating": 3}}}
|
||||
joins := newSmartPlaylistCriteria(c).RequiredJoins()
|
||||
|
||||
Expect(joins.has(smartPlaylistJoinAlbumAnnotation)).To(BeTrue())
|
||||
Expect(joins.has(smartPlaylistJoinAlbum)).To(BeFalse())
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -204,6 +204,43 @@ var _ = Describe("Smart Playlists", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Album aggregate fields", func() {
|
||||
// Abbey Road and IV have two tracks each; the other four albums have one.
|
||||
It("matches albums with more than one track", func() {
|
||||
results := evaluateRule(`{"all":[{"gt":{"albumsongcount":1}}]}`)
|
||||
Expect(results).To(ConsistOf("Come Together", "Something", "Stairway To Heaven", "Black Dog"))
|
||||
})
|
||||
|
||||
It("matches single-track albums", func() {
|
||||
results := evaluateRule(`{"all":[{"is":{"albumsongcount":1}}]}`)
|
||||
Expect(results).To(ConsistOf("So What", "Bohemian Rhapsody", "All Along the Watchtower",
|
||||
"We Are the Champions"))
|
||||
})
|
||||
|
||||
It("matches albumDateAdded inTheLast 1 day", func() {
|
||||
results := evaluateRule(`{"all":[{"inTheLast":{"albumdateadded":1}}]}`)
|
||||
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("matches albumDateModified before a far-future date", func() {
|
||||
results := evaluateRule(`{"all":[{"before":{"albumdatemodified":"2099-01-01"}}]}`)
|
||||
Expect(results).To(HaveLen(8))
|
||||
})
|
||||
|
||||
// Fixture durations are randomized, so only the aggregate being populated can be asserted.
|
||||
It("resolves albumDuration and albumSize to non-zero aggregates", func() {
|
||||
Expect(evaluateRule(`{"all":[{"gt":{"albumduration":0}}]}`)).To(HaveLen(8))
|
||||
Expect(evaluateRule(`{"all":[{"gt":{"albumsize":0}}]}`)).To(HaveLen(8))
|
||||
})
|
||||
|
||||
It("groups by album date and orders within the album (issue #5347)", func() {
|
||||
results := evaluateRuleOrdered(
|
||||
`{"all":[{"is":{"album":"Abbey Road"}}],"sort":"-albumdateadded,tracknumber"}`)
|
||||
Expect(results).To(Equal([]string{"Come Together", "Something"}))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Logic operators", func() {
|
||||
It("matches with ALL (AND)", func() {
|
||||
results := evaluateRule(`{"all":[{"is":{"genre":"Blues"}},{"gt":{"bpm":130}}]}`)
|
||||
|
||||
@ -122,7 +122,7 @@ func (r *playlistRepository) resolvePercentageLimit(pls *model.Playlist, rulesSQ
|
||||
exprJoins := rulesSQL.ExpressionJoins()
|
||||
countSq := Select("count(*) as count").From("media_file")
|
||||
countSq = r.addMediaFileAnnotationJoin(countSq, userID)
|
||||
countSq = r.addSmartPlaylistAnnotationJoins(countSq, exprJoins, userID)
|
||||
countSq = r.addSmartPlaylistJoins(countSq, exprJoins, userID)
|
||||
countSq = r.applyLibraryFilter(countSq, "media_file")
|
||||
|
||||
cond, err := rulesSQL.Where()
|
||||
@ -144,7 +144,7 @@ func (r *playlistRepository) resolvePercentageLimit(pls *model.Playlist, rulesSQ
|
||||
}
|
||||
|
||||
// buildSmartPlaylistQuery constructs the SQL query to select media files matching the smart playlist criteria,
|
||||
// including necessary joins for annotations and library filtering.
|
||||
// including the joins its fields require and library filtering.
|
||||
func (r *playlistRepository) buildSmartPlaylistQuery(pls *model.Playlist, rulesSQL smartPlaylistCriteria, userID string) SelectBuilder {
|
||||
orderBy := rulesSQL.OrderBy()
|
||||
sq := Select("row_number() over (order by "+orderBy+") as id", "'"+pls.ID+"' as playlist_id", "media_file.id as media_file_id").
|
||||
@ -152,7 +152,7 @@ func (r *playlistRepository) buildSmartPlaylistQuery(pls *model.Playlist, rulesS
|
||||
sq = r.addMediaFileAnnotationJoin(sq, userID)
|
||||
|
||||
requiredJoins := rulesSQL.RequiredJoins()
|
||||
sq = r.addSmartPlaylistAnnotationJoins(sq, requiredJoins, userID)
|
||||
sq = r.addSmartPlaylistJoins(sq, requiredJoins, userID)
|
||||
sq = r.applyLibraryFilter(sq, "media_file")
|
||||
return sq
|
||||
}
|
||||
@ -166,9 +166,8 @@ func (r *playlistRepository) addMediaFileAnnotationJoin(sq SelectBuilder, userID
|
||||
" AND annotation.user_id = ?)", userID)
|
||||
}
|
||||
|
||||
// addSmartPlaylistAnnotationJoins adds left joins to the annotation table for albums and artists as needed based on
|
||||
// the smart playlist criteria, filtering by user ID to include user-specific annotations in the evaluation.
|
||||
func (r *playlistRepository) addSmartPlaylistAnnotationJoins(sq SelectBuilder, joins smartPlaylistJoinType, userID string) SelectBuilder {
|
||||
// addSmartPlaylistJoins adds the left joins required by the criteria's fields.
|
||||
func (r *playlistRepository) addSmartPlaylistJoins(sq SelectBuilder, joins smartPlaylistJoinType, userID string) SelectBuilder {
|
||||
if joins.has(smartPlaylistJoinAlbumAnnotation) {
|
||||
sq = sq.LeftJoin("annotation AS album_annotation ON ("+
|
||||
"album_annotation.item_id = media_file.album_id"+
|
||||
@ -181,6 +180,9 @@ func (r *playlistRepository) addSmartPlaylistAnnotationJoins(sq SelectBuilder, j
|
||||
" AND artist_annotation.item_type = 'artist'"+
|
||||
" AND artist_annotation.user_id = ?)", userID)
|
||||
}
|
||||
if joins.has(smartPlaylistJoinAlbum) {
|
||||
sq = sq.LeftJoin("album ON album.id = media_file.album_id")
|
||||
}
|
||||
return sq
|
||||
}
|
||||
|
||||
|
||||
@ -9,6 +9,7 @@ import (
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/criteria"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
"github.com/pocketbase/dbx"
|
||||
@ -392,6 +393,40 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Smart Playlists with Album Aggregate Criteria", func() {
|
||||
BeforeEach(func() {
|
||||
DeferCleanup(configtest.SetupConfig())
|
||||
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
|
||||
})
|
||||
|
||||
trackIDsOf := func(rules *criteria.Criteria) []string {
|
||||
newPls := model.Playlist{Name: "Album Aggregates", OwnerID: "userid", Rules: rules}
|
||||
Expect(repo.Put(&newPls)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(newPls.ID) })
|
||||
|
||||
pls, err := repo.GetWithTracks(newPls.ID, true, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
return slice.Map(pls.Tracks, func(t model.PlaylistTrack) string { return t.MediaFileID })
|
||||
}
|
||||
|
||||
It("filters on albumSongCount", func() {
|
||||
// albumMultiDisc (ID "104") is the only fixture album with SongCount > 3
|
||||
rules := &criteria.Criteria{Expression: criteria.All{criteria.Gt{"albumSongCount": 3}}}
|
||||
|
||||
Expect(trackIDsOf(rules)).To(ConsistOf("2001", "2002", "2003", "2004"))
|
||||
})
|
||||
|
||||
It("sorts by an album field not referenced in the expression (issue #5347)", func() {
|
||||
// All four tracks share album 104, so the album date ties and disc/track number decide.
|
||||
rules := &criteria.Criteria{
|
||||
Expression: criteria.All{criteria.Is{"album": "Multi Disc Album"}},
|
||||
Sort: "-albumDateAdded,discNumber,trackNumber",
|
||||
}
|
||||
|
||||
Expect(trackIDsOf(rules)).To(HaveExactElements("2002", "2004", "2003", "2001"))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Smart Playlists with Tag Criteria", func() {
|
||||
var mfRepo model.MediaFileRepository
|
||||
var testPlaylistID string
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user