From ecf934feabd96b4ab476efab922a661d69d34027 Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Fri, 20 Sep 2024 16:59:46 -0400
Subject: [PATCH] fix(subsonic): random albums not reshuffling.
See: https://github.com/navidrome/navidrome/issues/3277#issuecomment-2364269787
---
persistence/album_repository.go | 4 +-
persistence/mediafile_repository.go | 4 +-
persistence/sql_base_repository.go | 9 +---
persistence/sql_base_repository_test.go | 57 ++++++++++++++++++-------
utils/hasher/hasher.go | 6 +++
5 files changed, 54 insertions(+), 26 deletions(-)
diff --git a/persistence/album_repository.go b/persistence/album_repository.go
index 0047dcacb..2ee94973b 100644
--- a/persistence/album_repository.go
+++ b/persistence/album_repository.go
@@ -75,7 +75,7 @@ func NewAlbumRepository(ctx context.Context, db dbx.Builder) model.AlbumReposito
"artist": "compilation asc, COALESCE(NULLIF(sort_album_artist_name,''),order_album_artist_name) asc, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc",
"album_artist": "compilation asc, COALESCE(NULLIF(sort_album_artist_name,''),order_album_artist_name) asc, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc",
"max_year": "coalesce(nullif(original_date,''), cast(max_year as text)), release_date, name, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc",
- "random": r.seededRandomSort(),
+ "random": "random",
"recently_added": recentlyAddedSort(),
"starred_at": "starred, starred_at",
}
@@ -85,7 +85,7 @@ func NewAlbumRepository(ctx context.Context, db dbx.Builder) model.AlbumReposito
"artist": "compilation asc, order_album_artist_name asc, order_album_name asc",
"album_artist": "compilation asc, order_album_artist_name asc, order_album_name asc",
"max_year": "coalesce(nullif(original_date,''), cast(max_year as text)), release_date, name, order_album_name asc",
- "random": r.seededRandomSort(),
+ "random": "random",
"recently_added": recentlyAddedSort(),
"starred_at": "starred, starred_at",
}
diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go
index a63094cec..39fa4a34f 100644
--- a/persistence/mediafile_repository.go
+++ b/persistence/mediafile_repository.go
@@ -36,7 +36,7 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) *mediaFileRepos
"title": "COALESCE(NULLIF(sort_title,''),title)",
"artist": "COALESCE(NULLIF(sort_artist_name,''),order_artist_name) asc, COALESCE(NULLIF(sort_album_name,''),order_album_name) asc, release_date asc, disc_number asc, track_number asc",
"album": "COALESCE(NULLIF(sort_album_name,''),order_album_name) asc, release_date asc, disc_number asc, track_number asc, COALESCE(NULLIF(sort_artist_name,''),order_artist_name) asc, COALESCE(NULLIF(sort_title,''),title) asc",
- "random": r.seededRandomSort(),
+ "random": "random",
"created_at": "media_file.created_at",
"track_number": "album, release_date, disc_number, track_number",
"starred_at": "starred, starred_at",
@@ -46,7 +46,7 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) *mediaFileRepos
"title": "order_title",
"artist": "order_artist_name asc, order_album_name asc, release_date asc, disc_number asc, track_number asc",
"album": "order_album_name asc, release_date asc, disc_number asc, track_number asc, order_artist_name asc, title asc",
- "random": r.seededRandomSort(),
+ "random": "random",
"created_at": "media_file.created_at",
"track_number": "album, release_date, disc_number, track_number",
"starred_at": "starred, starred_at",
diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go
index d43375a58..f5c33978b 100644
--- a/persistence/sql_base_repository.go
+++ b/persistence/sql_base_repository.go
@@ -167,20 +167,15 @@ func (r sqlRepository) seedKey() string {
return r.tableName + userId(r.ctx)
}
-func (r sqlRepository) seededRandomSort() string {
- return fmt.Sprintf("SEEDEDRAND('%s', %s.id)", r.seedKey(), r.tableName)
-}
-
func (r sqlRepository) resetSeededRandom(options []model.QueryOptions) {
- if len(options) == 0 || !strings.HasPrefix(options[0].Sort, "SEEDEDRAND(") {
+ if len(options) == 0 || options[0].Sort != "random" {
return
}
-
+ options[0].Sort = fmt.Sprintf("SEEDEDRAND('%s', %s.id)", r.seedKey(), r.tableName)
if options[0].Seed != "" {
hasher.SetSeed(r.seedKey(), options[0].Seed)
return
}
-
if options[0].Offset == 0 {
hasher.Reseed(r.seedKey())
}
diff --git a/persistence/sql_base_repository_test.go b/persistence/sql_base_repository_test.go
index 35d802d98..4b380e298 100644
--- a/persistence/sql_base_repository_test.go
+++ b/persistence/sql_base_repository_test.go
@@ -6,16 +6,25 @@ import (
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
+ "github.com/navidrome/navidrome/utils/hasher"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("sqlRepository", func() {
- r := sqlRepository{}
+ var r sqlRepository
+ BeforeEach(func() {
+ r.ctx = request.WithUser(context.Background(), model.User{ID: "user-id"})
+ r.tableName = "table"
+ })
+
Describe("applyOptions", func() {
var sq squirrel.SelectBuilder
BeforeEach(func() {
sq = squirrel.Select("*").From("test")
+ r.sortMappings = map[string]string{
+ "name": "title",
+ }
})
It("does not add any clauses when options is empty", func() {
sq = r.applyOptions(sq, model.QueryOptions{})
@@ -30,17 +39,11 @@ var _ = Describe("sqlRepository", func() {
Offset: 2,
})
sql, _, _ := sq.ToSql()
- Expect(sql).To(Equal("SELECT * FROM test ORDER BY name desc LIMIT 1 OFFSET 2"))
+ Expect(sql).To(Equal("SELECT * FROM test ORDER BY title desc LIMIT 1 OFFSET 2"))
})
})
Describe("toSQL", func() {
- var r sqlRepository
-
- BeforeEach(func() {
- r = sqlRepository{}
- })
-
It("returns error for invalid SQL", func() {
sq := squirrel.Select("*").From("test").Where(1)
_, _, err := r.toSQL(sq)
@@ -175,13 +178,37 @@ var _ = Describe("sqlRepository", func() {
Expect(sql).To(Equal("name asc, coalesce(nullif(release_date, ''), nullif(original_date, '')) desc, status desc"))
})
})
- Context("seededRandomSort", func() {
- It("returns a random sort order function call", func() {
- r.ctx = request.WithUser(context.Background(), model.User{ID: "2"})
- r.tableName = "media_file"
- sql := r.seededRandomSort()
- Expect(sql).To(ContainSubstring("SEEDEDRAND('media_file2', media_file.id)"))
- })
+ })
+
+ Describe("resetSeededRandom", func() {
+ var id string
+ BeforeEach(func() {
+ id = r.seedKey()
+ hasher.SetSeed(id, "")
+ })
+ It("does not reset seed if sort is not random", func() {
+ var options []model.QueryOptions
+ r.resetSeededRandom(options)
+ Expect(hasher.CurrentSeed(id)).To(BeEmpty())
+ })
+ It("resets seed if sort is random", func() {
+ options := []model.QueryOptions{{Sort: "random"}}
+ r.resetSeededRandom(options)
+ Expect(hasher.CurrentSeed(id)).NotTo(BeEmpty())
+ })
+ It("resets seed if sort is random and seed is provided", func() {
+ options := []model.QueryOptions{{Sort: "random", Seed: "seed"}}
+ r.resetSeededRandom(options)
+ Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
+ })
+ It("keeps seed when paginating", func() {
+ options := []model.QueryOptions{{Sort: "random", Seed: "seed", Offset: 0}}
+ r.resetSeededRandom(options)
+ Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
+
+ options = []model.QueryOptions{{Sort: "random", Offset: 1}}
+ r.resetSeededRandom(options)
+ Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
})
})
})
diff --git a/utils/hasher/hasher.go b/utils/hasher/hasher.go
index b07c17c31..279cbe444 100644
--- a/utils/hasher/hasher.go
+++ b/utils/hasher/hasher.go
@@ -18,6 +18,12 @@ func SetSeed(id string, seed string) {
instance.SetSeed(id, seed)
}
+func CurrentSeed(id string) string {
+ instance.mutex.RLock()
+ defer instance.mutex.RUnlock()
+ return instance.seeds[id]
+}
+
func HashFunc() func(id, str string) uint64 {
return instance.HashFunc()
}