mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
* fix(playlist): preserve smart playlist counters on re-import (#5907) * perf(playlist): skip re-importing unchanged NSP files (#5907) * feat(playlist): also store content hash for M3U imports (unused for now) * fix(playlist): return stored record when skipping unchanged NSP import Skipping before copying the stored identity broke the ImportFile(sync=false) contract: callers received an ID-less playlist and the requested Sync change was silently dropped. * refactor(playlist): hash imports once at the caller; protect smart counters in Put Move content hashing out of both parsers into the code that owns the file (parsePlaylist and ImportFile), removing the NSP double-buffer and the duplicated hashing idiom. Put now drops song_count/duration/size for smart playlists (PostMapArgs), disarming the counter-zeroing trap for all callers. * fix(playlist): invalidate imported hash when rules are edited via API Without this, a rules edit through the REST API kept the stored file hash, so every scan skipped the unchanged file and never restored the file-backed rules while sync was on. * test(playlist): verify smart counters survive a re-import, end to end The existing Put test seeds the stored counters with a raw SQL update, so it pins the guard in PostMapArgs but not the pipeline around it. This test drives the counters through a real evaluation instead: it saves a smart playlist, reads it with GetWithTracks to populate song_count/duration/size, then saves the playlist the way the scanner rebuilds it after parsing the .nsp file, with the counters back at zero. Both routes fail without the guard, and the new one covers the exact sequence reported in #5907. Test taken from #5970, which diagnosed the same root cause independently. Co-authored-by: Junker der Provinz <133605895+junkerderprovinz@users.noreply.github.com> * test(playlist): build the service with artwork.NewUploader The artwork pipeline in #5847 replaced core.NewImageUploadService() with artwork.NewUploader(ds) and updated every call site it could see. The five call sites this branch adds were written against the old constructor, so the merge applied cleanly but left the package uncompilable. * fix(db): re-stamp the imported_hash migration after the master merge Master gained three migrations while this branch was open, the newest being 20260816180040. The original 20260808200333 stamp now sorts before them, so any database already upgraded past that point would skip this migration entirely and never get the imported_hash column. Same SQL, current timestamp. * refactor(playlist): hash imported playlists with xxh3 and the id encoding ImportedHash is a change detector, not a security boundary, so it does not need a cryptographic digest. xxh3 is already a direct dependency and is used the same way to fingerprint files in the artwork image store. Encoding the 128-bit digest with id.Encode stores it in the same 22-char base62 form as every other id in the schema, down from 64 hex chars. No migration is needed: the imported_hash column has not shipped in a release, so no database holds a value in the old format. * refactor(playlist): extract the imported-playlist fingerprint helper Both import paths encoded the hash inline, so how a playlist file is fingerprinted lived in two places. A third import path that encoded it differently would silently never match the stored value, turning the unchanged-file skip into a no-op. --------- Co-authored-by: Junker der Provinz <133605895+junkerderprovinz@users.noreply.github.com>
711 lines
26 KiB
Go
711 lines
26 KiB
Go
package persistence
|
|
|
|
import (
|
|
"time"
|
|
|
|
"github.com/navidrome/navidrome/conf"
|
|
"github.com/navidrome/navidrome/conf/configtest"
|
|
"github.com/navidrome/navidrome/log"
|
|
"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"
|
|
)
|
|
|
|
var _ = Describe("PlaylistRepository - Smart Playlists", func() {
|
|
var repo model.PlaylistRepository
|
|
|
|
BeforeEach(func() {
|
|
ctx := log.NewContext(GinkgoT().Context())
|
|
ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true})
|
|
repo = NewPlaylistRepository(ctx, GetDBXBuilder())
|
|
})
|
|
|
|
Context("Smart Playlists", func() {
|
|
var rules *criteria.Criteria
|
|
BeforeEach(func() {
|
|
rules = &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Contains{"title": "love"},
|
|
},
|
|
}
|
|
})
|
|
Context("valid rules", func() {
|
|
Specify("Put/Get", func() {
|
|
newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
|
|
Expect(repo.Put(&newPls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(newPls.ID) })
|
|
|
|
savedPls, err := repo.Get(newPls.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(savedPls.Rules).To(Equal(rules))
|
|
})
|
|
})
|
|
|
|
Context("invalid rules", func() {
|
|
It("fails to Put it in the DB", func() {
|
|
rules = &criteria.Criteria{
|
|
// This is invalid because "contains" cannot have multiple fields
|
|
Expression: criteria.All{
|
|
criteria.Contains{"genre": "Hardcore", "filetype": "mp3"},
|
|
},
|
|
}
|
|
newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
|
|
Expect(repo.Put(&newPls)).To(MatchError(ContainSubstring("invalid criteria expression")))
|
|
})
|
|
})
|
|
|
|
Context("re-imported from disk", func() {
|
|
// The scanner re-imports every playlist in a touched folder, and a freshly parsed
|
|
// .nsp carries no counters — saving it must not wipe the ones already evaluated.
|
|
It("keeps the stored counters when a freshly parsed playlist is saved over it", func() {
|
|
rules = &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Contains{"title": "Antenna"},
|
|
},
|
|
}
|
|
pls := model.Playlist{Name: "Smart", OwnerID: "userid", Rules: rules, Path: "/music/smart.nsp", Sync: true}
|
|
Expect(repo.Put(&pls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(pls.ID) })
|
|
|
|
evaluated, err := repo.GetWithTracks(pls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(evaluated.SongCount).To(BeNumerically(">", 0))
|
|
|
|
stored, err := repo.Get(pls.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(stored.SongCount).To(Equal(evaluated.SongCount))
|
|
|
|
reimported := model.Playlist{
|
|
ID: pls.ID, Name: pls.Name, OwnerID: "userid", Rules: rules,
|
|
Path: pls.Path, Sync: true,
|
|
}
|
|
Expect(repo.Put(&reimported)).To(Succeed())
|
|
|
|
afterImport, err := repo.Get(pls.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(afterImport.SongCount).To(Equal(stored.SongCount))
|
|
Expect(afterImport.Duration).To(Equal(stored.Duration))
|
|
Expect(afterImport.Size).To(Equal(stored.Size))
|
|
})
|
|
})
|
|
|
|
Context("child smart playlists", func() {
|
|
BeforeEach(func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
})
|
|
|
|
When("refresh delay has expired", func() {
|
|
It("should refresh tracks for smart playlist referenced in parent smart playlist criteria", func() {
|
|
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
|
|
|
|
childRules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Contains{"title": "Day"},
|
|
},
|
|
}
|
|
nestedPls := model.Playlist{Name: "Nested", OwnerID: "userid", Public: true, Rules: childRules}
|
|
Expect(repo.Put(&nestedPls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(nestedPls.ID) })
|
|
|
|
parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.InPlaylist{"id": nestedPls.ID},
|
|
},
|
|
}}
|
|
Expect(repo.Put(&parentPls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(parentPls.ID) })
|
|
|
|
// Nested playlist has not been evaluated yet
|
|
nestedPlsRead, err := repo.Get(nestedPls.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(nestedPlsRead.EvaluatedAt).To(BeNil())
|
|
|
|
// Getting parent with refresh should recursively refresh the nested playlist
|
|
pls, err := repo.GetWithTracks(parentPls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(pls.EvaluatedAt).ToNot(BeNil())
|
|
Expect(*pls.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
|
|
|
|
// Parent should have tracks from the nested playlist
|
|
Expect(pls.Tracks).To(HaveLen(1))
|
|
Expect(pls.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID))
|
|
|
|
// Nested playlist should now have been refreshed (EvaluatedAt set)
|
|
nestedPlsAfterParentGet, err := repo.Get(nestedPls.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(nestedPlsAfterParentGet.EvaluatedAt).ToNot(BeNil())
|
|
Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
|
|
})
|
|
})
|
|
|
|
When("refresh delay 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
|
|
childEvaluatedAt := time.Now().Add(-30 * time.Minute)
|
|
|
|
childRules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Contains{"title": "Day"},
|
|
},
|
|
}
|
|
nestedPls := model.Playlist{Name: "Nested", OwnerID: "userid", Public: true, Rules: childRules, EvaluatedAt: &childEvaluatedAt}
|
|
Expect(repo.Put(&nestedPls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(nestedPls.ID) })
|
|
|
|
// Parent has no EvaluatedAt, so it WILL refresh, but the child should not
|
|
parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.InPlaylist{"id": nestedPls.ID},
|
|
},
|
|
}}
|
|
Expect(repo.Put(&parentPls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(parentPls.ID) })
|
|
|
|
nestedPlsRead, err := repo.Get(nestedPls.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
// Getting parent with refresh should NOT recursively refresh the nested playlist
|
|
parent, err := repo.GetWithTracks(parentPls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
// Parent should have been refreshed (its EvaluatedAt was nil)
|
|
Expect(parent.EvaluatedAt).ToNot(BeNil())
|
|
Expect(*parent.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
|
|
|
|
// Nested playlist should NOT have been refreshed (still within delay window)
|
|
nestedPlsAfterParentGet, err := repo.Get(nestedPls.ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally("~", childEvaluatedAt, time.Second))
|
|
Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(Equal(*nestedPlsRead.EvaluatedAt))
|
|
})
|
|
})
|
|
|
|
Context("per-playlist refreshDelay", func() {
|
|
BeforeEach(func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
})
|
|
|
|
It("does NOT refresh when the per-playlist delay has not elapsed, even if global has", func() {
|
|
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
|
|
evaluatedAt := time.Now().Add(-1 * time.Hour)
|
|
|
|
rules := &criteria.Criteria{
|
|
Expression: criteria.All{criteria.Contains{"title": "Day"}},
|
|
RefreshDelay: 24 * time.Hour,
|
|
}
|
|
pls := model.Playlist{Name: "Frozen Daily", OwnerID: "userid", Rules: rules, EvaluatedAt: &evaluatedAt}
|
|
Expect(repo.Put(&pls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(pls.ID) })
|
|
|
|
got, err := repo.GetWithTracks(pls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
// Not re-evaluated: EvaluatedAt unchanged, no tracks materialized
|
|
Expect(*got.EvaluatedAt).To(BeTemporally("~", evaluatedAt, time.Second))
|
|
Expect(got.Tracks).To(BeEmpty())
|
|
})
|
|
|
|
It("refreshes when the per-playlist delay has elapsed, even if global has not", func() {
|
|
conf.Server.SmartPlaylistRefreshDelay = 1 * time.Hour
|
|
evaluatedAt := time.Now().Add(-10 * time.Minute)
|
|
|
|
rules := &criteria.Criteria{
|
|
Expression: criteria.All{criteria.Contains{"title": "Day"}},
|
|
RefreshDelay: 5 * time.Minute,
|
|
}
|
|
pls := model.Playlist{Name: "Fast Refresh", OwnerID: "userid", Rules: rules, EvaluatedAt: &evaluatedAt}
|
|
Expect(repo.Put(&pls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(pls.ID) })
|
|
|
|
got, err := repo.GetWithTracks(pls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(*got.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
|
|
Expect(got.Tracks).To(HaveLen(1))
|
|
Expect(got.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID))
|
|
})
|
|
})
|
|
})
|
|
})
|
|
|
|
Describe("Playlist Track Sorting", func() {
|
|
var testPlaylistID string
|
|
|
|
AfterEach(func() {
|
|
if testPlaylistID != "" {
|
|
Expect(repo.Delete(testPlaylistID)).To(BeNil())
|
|
testPlaylistID = ""
|
|
}
|
|
})
|
|
|
|
It("sorts tracks correctly by album (disc and track number)", func() {
|
|
By("creating a playlist with multi-disc album tracks in arbitrary order")
|
|
newPls := model.Playlist{Name: "Multi-Disc Test", OwnerID: "userid"}
|
|
// Add tracks in intentionally scrambled order
|
|
newPls.AddMediaFilesByID([]string{"2001", "2002", "2003", "2004"})
|
|
Expect(repo.Put(&newPls)).To(Succeed())
|
|
testPlaylistID = newPls.ID
|
|
|
|
By("retrieving tracks sorted by album")
|
|
tracksRepo := repo.Tracks(newPls.ID, false)
|
|
tracks, err := tracksRepo.GetAll(model.QueryOptions{Sort: "album", Order: "asc"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
By("verifying tracks are sorted by disc number then track number")
|
|
Expect(tracks).To(HaveLen(4))
|
|
// Expected order: Disc 1 Track 1, Disc 1 Track 2, Disc 2 Track 1, Disc 2 Track 11
|
|
Expect(tracks[0].MediaFileID).To(Equal("2002")) // Disc 1, Track 1
|
|
Expect(tracks[1].MediaFileID).To(Equal("2004")) // Disc 1, Track 2
|
|
Expect(tracks[2].MediaFileID).To(Equal("2003")) // Disc 2, Track 1
|
|
Expect(tracks[3].MediaFileID).To(Equal("2001")) // Disc 2, Track 11
|
|
})
|
|
})
|
|
|
|
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())
|
|
})
|
|
|
|
It("matches loved tracks when loved value is a string in nested group (issue #4826)", func() {
|
|
// songComeTogether (ID "1002") is starred in test fixtures
|
|
rules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Any{
|
|
criteria.Is{"loved": "true"},
|
|
},
|
|
},
|
|
}
|
|
newPls := model.Playlist{Name: "String Loved Nested", 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(ContainElement("1002"))
|
|
Expect(len(pls.Tracks)).To(BeNumerically(">=", 1))
|
|
})
|
|
|
|
It("returns same results for string and bool loved values (issue #4826)", func() {
|
|
boolRules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Any{
|
|
criteria.Is{"loved": true},
|
|
},
|
|
},
|
|
}
|
|
boolPls := model.Playlist{Name: "Bool Loved", OwnerID: "userid", Rules: boolRules}
|
|
Expect(repo.Put(&boolPls)).To(Succeed())
|
|
DeferCleanup(func() { _ = repo.Delete(boolPls.ID) })
|
|
|
|
stringRules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Any{
|
|
criteria.Is{"loved": "true"},
|
|
},
|
|
},
|
|
}
|
|
stringPls := model.Playlist{Name: "String Loved", OwnerID: "userid", Rules: stringRules}
|
|
Expect(repo.Put(&stringPls)).To(Succeed())
|
|
testPlaylistID = stringPls.ID
|
|
|
|
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
|
|
boolResult, err := repo.GetWithTracks(boolPls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
stringResult, err := repo.GetWithTracks(stringPls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
boolIDs := make([]string, len(boolResult.Tracks))
|
|
for i, t := range boolResult.Tracks {
|
|
boolIDs[i] = t.MediaFileID
|
|
}
|
|
stringIDs := make([]string, len(stringResult.Tracks))
|
|
for i, t := range stringResult.Tracks {
|
|
stringIDs[i] = t.MediaFileID
|
|
}
|
|
Expect(stringIDs).To(ConsistOf(boolIDs))
|
|
})
|
|
})
|
|
|
|
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
|
|
var songWithGrouping, songWithoutGrouping model.MediaFile
|
|
|
|
BeforeEach(func() {
|
|
ctx := log.NewContext(GinkgoT().Context())
|
|
ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true})
|
|
mfRepo = NewMediaFileRepository(ctx, GetDBXBuilder())
|
|
|
|
// Register 'grouping' as a valid tag for smart playlists
|
|
criteria.AddTagNames([]string{"grouping"})
|
|
|
|
// Create a song with the grouping tag
|
|
songWithGrouping = model.MediaFile{
|
|
ID: "test-grouping-1",
|
|
Title: "Song With Grouping",
|
|
Artist: "Test Artist",
|
|
ArtistID: "1",
|
|
Album: "Test Album",
|
|
AlbumID: "101",
|
|
Path: "test/grouping/song1.mp3",
|
|
Tags: model.Tags{
|
|
"grouping": []string{"My Crate"},
|
|
},
|
|
Participants: model.Participants{},
|
|
LibraryID: 1,
|
|
Lyrics: "[]",
|
|
}
|
|
Expect(mfRepo.Put(&songWithGrouping)).To(Succeed())
|
|
|
|
// Create a song without the grouping tag
|
|
songWithoutGrouping = model.MediaFile{
|
|
ID: "test-grouping-2",
|
|
Title: "Song Without Grouping",
|
|
Artist: "Test Artist",
|
|
ArtistID: "1",
|
|
Album: "Test Album",
|
|
AlbumID: "101",
|
|
Path: "test/grouping/song2.mp3",
|
|
Tags: model.Tags{},
|
|
Participants: model.Participants{},
|
|
LibraryID: 1,
|
|
Lyrics: "[]",
|
|
}
|
|
Expect(mfRepo.Put(&songWithoutGrouping)).To(Succeed())
|
|
})
|
|
|
|
AfterEach(func() {
|
|
if testPlaylistID != "" {
|
|
_ = repo.Delete(testPlaylistID)
|
|
testPlaylistID = ""
|
|
}
|
|
// Clean up test media files
|
|
_, _ = GetDBXBuilder().Delete("media_file", dbx.HashExp{"id": "test-grouping-1"}).Execute()
|
|
_, _ = GetDBXBuilder().Delete("media_file", dbx.HashExp{"id": "test-grouping-2"}).Execute()
|
|
})
|
|
|
|
It("matches tracks with a tag value using 'contains' with empty string (issue #4728 workaround)", func() {
|
|
By("creating a smart playlist that checks if grouping tag has any value")
|
|
// This is the workaround for issue #4728: using 'contains' with empty string
|
|
// generates SQL: value LIKE '%%' which matches any non-empty string
|
|
rules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Contains{"grouping": ""},
|
|
},
|
|
}
|
|
newPls := model.Playlist{Name: "Tracks with Grouping", OwnerID: "userid", Rules: rules}
|
|
Expect(repo.Put(&newPls)).To(Succeed())
|
|
testPlaylistID = newPls.ID
|
|
|
|
By("refreshing the smart playlist")
|
|
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second // Force refresh
|
|
pls, err := repo.GetWithTracks(newPls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
By("verifying only the track with grouping tag is matched")
|
|
Expect(pls.Tracks).To(HaveLen(1))
|
|
Expect(pls.Tracks[0].MediaFileID).To(Equal(songWithGrouping.ID))
|
|
})
|
|
|
|
It("excludes tracks with a tag value using 'notContains' with empty string", func() {
|
|
By("creating a smart playlist that checks if grouping tag is NOT set")
|
|
rules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.NotContains{"grouping": ""},
|
|
},
|
|
}
|
|
newPls := model.Playlist{Name: "Tracks without Grouping", OwnerID: "userid", Rules: rules}
|
|
Expect(repo.Put(&newPls)).To(Succeed())
|
|
testPlaylistID = newPls.ID
|
|
|
|
By("refreshing the smart playlist")
|
|
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second // Force refresh
|
|
pls, err := repo.GetWithTracks(newPls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
By("verifying the track with grouping is NOT in the playlist")
|
|
for _, track := range pls.Tracks {
|
|
Expect(track.MediaFileID).ToNot(Equal(songWithGrouping.ID))
|
|
}
|
|
|
|
By("verifying the track without grouping IS in the playlist")
|
|
var foundWithoutGrouping bool
|
|
for _, track := range pls.Tracks {
|
|
if track.MediaFileID == songWithoutGrouping.ID {
|
|
foundWithoutGrouping = true
|
|
break
|
|
}
|
|
}
|
|
Expect(foundWithoutGrouping).To(BeTrue())
|
|
})
|
|
})
|
|
|
|
Describe("Smart Playlists Library Filtering", func() {
|
|
var mfRepo model.MediaFileRepository
|
|
var testPlaylistID string
|
|
var lib2ID int
|
|
var restrictedUserID string
|
|
var uniqueLibPath string
|
|
|
|
BeforeEach(func() {
|
|
db := GetDBXBuilder()
|
|
|
|
// Generate unique IDs for this test run
|
|
uniqueSuffix := time.Now().Format("20060102150405.000")
|
|
restrictedUserID = "restricted-user-" + uniqueSuffix
|
|
uniqueLibPath = "/music/lib2-" + uniqueSuffix
|
|
|
|
// Create a second library with unique name and path to avoid conflicts with other tests
|
|
_, err := db.DB().Exec("INSERT INTO library (name, path, created_at, updated_at) VALUES (?, ?, datetime('now'), datetime('now'))", "Library 2-"+uniqueSuffix, uniqueLibPath)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
err = db.DB().QueryRow("SELECT last_insert_rowid()").Scan(&lib2ID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
// Create a restricted user with access only to library 1
|
|
_, err = db.DB().Exec("INSERT INTO user (id, user_name, name, is_admin, password, created_at, updated_at) VALUES (?, ?, 'Restricted User', false, 'pass', datetime('now'), datetime('now'))", restrictedUserID, restrictedUserID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
_, err = db.DB().Exec("INSERT INTO user_library (user_id, library_id) VALUES (?, 1)", restrictedUserID)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
// Create test media files in each library
|
|
ctx := log.NewContext(GinkgoT().Context())
|
|
ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true})
|
|
mfRepo = NewMediaFileRepository(ctx, db)
|
|
|
|
// Song in library 1 (accessible by restricted user)
|
|
songLib1 := model.MediaFile{
|
|
ID: "lib1-song",
|
|
Title: "Song in Lib1",
|
|
Artist: "Test Artist",
|
|
ArtistID: "1",
|
|
Album: "Test Album",
|
|
AlbumID: "101",
|
|
Path: "lib1/song.mp3",
|
|
LibraryID: 1,
|
|
Participants: model.Participants{},
|
|
Tags: model.Tags{},
|
|
Lyrics: "[]",
|
|
}
|
|
Expect(mfRepo.Put(&songLib1)).To(Succeed())
|
|
|
|
// Song in library 2 (NOT accessible by restricted user)
|
|
songLib2 := model.MediaFile{
|
|
ID: "lib2-song",
|
|
Title: "Song in Lib2",
|
|
Artist: "Test Artist",
|
|
ArtistID: "1",
|
|
Album: "Test Album",
|
|
AlbumID: "101",
|
|
Path: "lib2/song.mp3",
|
|
LibraryID: lib2ID,
|
|
Participants: model.Participants{},
|
|
Tags: model.Tags{},
|
|
Lyrics: "[]",
|
|
}
|
|
Expect(mfRepo.Put(&songLib2)).To(Succeed())
|
|
})
|
|
|
|
AfterEach(func() {
|
|
db := GetDBXBuilder()
|
|
if testPlaylistID != "" {
|
|
_ = repo.Delete(testPlaylistID)
|
|
testPlaylistID = ""
|
|
}
|
|
// Clean up test data
|
|
_, _ = db.Delete("media_file", dbx.HashExp{"id": "lib1-song"}).Execute()
|
|
_, _ = db.Delete("media_file", dbx.HashExp{"id": "lib2-song"}).Execute()
|
|
_, _ = db.Delete("user_library", dbx.HashExp{"user_id": restrictedUserID}).Execute()
|
|
_, _ = db.Delete("user", dbx.HashExp{"id": restrictedUserID}).Execute()
|
|
_, _ = db.DB().Exec("DELETE FROM library WHERE id = ?", lib2ID)
|
|
})
|
|
|
|
It("should only include tracks from libraries the user has access to (issue #4738)", func() {
|
|
db := GetDBXBuilder()
|
|
ctx := log.NewContext(GinkgoT().Context())
|
|
|
|
// Create the smart playlist as the restricted user
|
|
restrictedUser := model.User{ID: restrictedUserID, UserName: restrictedUserID, IsAdmin: false}
|
|
ctx = request.WithUser(ctx, restrictedUser)
|
|
restrictedRepo := NewPlaylistRepository(ctx, db)
|
|
|
|
// Create a smart playlist that matches all songs
|
|
rules := &criteria.Criteria{
|
|
Expression: criteria.All{
|
|
criteria.Gt{"playCount": -1}, // Matches everything
|
|
},
|
|
}
|
|
newPls := model.Playlist{Name: "All Songs", OwnerID: restrictedUserID, Rules: rules}
|
|
Expect(restrictedRepo.Put(&newPls)).To(Succeed())
|
|
testPlaylistID = newPls.ID
|
|
|
|
By("refreshing the smart playlist")
|
|
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second // Force refresh
|
|
pls, err := restrictedRepo.GetWithTracks(newPls.ID, true, false)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
By("verifying only the track from library 1 is in the playlist")
|
|
var foundLib1Song, foundLib2Song bool
|
|
for _, track := range pls.Tracks {
|
|
if track.MediaFileID == "lib1-song" {
|
|
foundLib1Song = true
|
|
}
|
|
if track.MediaFileID == "lib2-song" {
|
|
foundLib2Song = true
|
|
}
|
|
}
|
|
Expect(foundLib1Song).To(BeTrue(), "Song from library 1 should be in the playlist")
|
|
Expect(foundLib2Song).To(BeFalse(), "Song from library 2 should NOT be in the playlist")
|
|
|
|
By("verifying playlist_tracks table only contains the accessible track")
|
|
var playlistTracksCount int
|
|
err = db.DB().QueryRow("SELECT count(*) FROM playlist_tracks WHERE playlist_id = ?", newPls.ID).Scan(&playlistTracksCount)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
// Count should only include tracks visible to the user (lib1-song)
|
|
// The count may include other test songs from library 1, but NOT lib2-song
|
|
var lib2TrackCount int
|
|
err = db.DB().QueryRow("SELECT count(*) FROM playlist_tracks WHERE playlist_id = ? AND media_file_id = 'lib2-song'", newPls.ID).Scan(&lib2TrackCount)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(lib2TrackCount).To(Equal(0), "lib2-song should not be in playlist_tracks")
|
|
|
|
By("verifying SongCount matches visible tracks")
|
|
Expect(pls.SongCount).To(Equal(len(pls.Tracks)), "SongCount should match the number of visible tracks")
|
|
})
|
|
})
|
|
})
|