navidrome/persistence/sql_base_repository_test.go
Deluan bf72f00760 fix(persistence): keep song pages correct past the large-offset threshold
Past DevOffsetOptimize (50,000 by default) GetAll and the legacy LIKE
search exclude earlier rows with a "rowid NOT IN (subquery)". The
subquery selected only the rowid, so its ORDER BY could not use the
outer column names and SQLite matched them against every joined table.
Sorting songs by id, path, updatedAt or comment failed with "ambiguous
column name", so /api/song returned 500 for those sorts. Album legacy
search failed the same way on name.

The subquery and the outer query also ordered tied rows differently, so
sorts with many equal values (starred, rating, playCount, ...) repeated
some songs and skipped others once past the threshold, without errors.

The subquery now keeps the outer projection, so ORDER BY resolves the
same way in both queries. Paginated queries get a rowid tie-breaker, so
the order is total and every page agrees on where ties go. Every index
ends with the rowid, so index-served sorts stay index-served. createdAt
and updatedAt now sort by (date, id) to match their (date, id) indexes;
with the date alone, the rowid tie-breaker would need a temp B-tree.

Fixes #6285
2026-10-07 13:54:01 -04:00

557 lines
20 KiB
Go

package persistence
import (
"context"
"slices"
"github.com/Masterminds/squirrel"
"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/request"
"github.com/navidrome/navidrome/utils/hasher"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("sqlRepository", func() {
var r sqlRepository
var ctx context.Context
BeforeEach(func() {
ctx = request.WithUser(GinkgoT().Context(), model.User{ID: "user-id"})
r.tableName = "table"
})
Describe("ownedRow", func() {
DescribeTable("matches the row, limited to what the logged-in user may write",
func(user model.User, access writeAccess, expectedSQL string, expectedArgs ...any) {
userCtx := request.WithUser(GinkgoT().Context(), user)
sql, args, err := r.ownedRow(userCtx, "row-1", access).ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(Equal(expectedSQL))
Expect(args).To(Equal(expectedArgs))
},
Entry("admin, ownerOrAdmin: any row", model.User{ID: "admin", IsAdmin: true}, ownerOrAdmin,
"(id = ?)", "row-1"),
Entry("regular, ownerOrAdmin: own rows", model.User{ID: "user"}, ownerOrAdmin,
"(id = ? AND user_id = ?)", "row-1", "user"),
Entry("admin, ownerOnly: own rows", model.User{ID: "admin", IsAdmin: true}, ownerOnly,
"(id = ? AND user_id = ?)", "row-1", "admin"),
Entry("regular, ownerOnly: own rows", model.User{ID: "user"}, ownerOnly,
"(id = ? AND user_id = ?)", "row-1", "user"),
)
})
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{})
sql, _, _ := sq.ToSql()
Expect(sql).To(Equal("SELECT * FROM test"))
})
It("adds all option clauses", func() {
sq = r.applyOptions(sq, model.QueryOptions{
Sort: "name",
Order: "desc",
Max: 1,
Offset: 2,
})
sql, _, _ := sq.ToSql()
Expect(sql).To(Equal("SELECT * FROM test ORDER BY title desc LIMIT 1 OFFSET 2"))
})
})
Describe("toSQL", func() {
It("returns error for invalid SQL", func() {
sq := squirrel.Select("*").From("test").Where(1)
_, _, err := r.toSQL(sq)
Expect(err).To(HaveOccurred())
})
It("returns the same query when there are no placeholders", func() {
sq := squirrel.Select("*").From("test")
query, params, err := r.toSQL(sq)
Expect(err).NotTo(HaveOccurred())
Expect(query).To(Equal("SELECT * FROM test"))
Expect(params).To(BeEmpty())
})
It("replaces one placeholder correctly", func() {
sq := squirrel.Select("*").From("test").Where(squirrel.Eq{"id": 1})
query, params, err := r.toSQL(sq)
Expect(err).NotTo(HaveOccurred())
Expect(query).To(Equal("SELECT * FROM test WHERE id = {:p0}"))
Expect(params).To(HaveKeyWithValue("p0", 1))
})
It("replaces multiple placeholders correctly", func() {
sq := squirrel.Select("*").From("test").Where(squirrel.Eq{"id": 1, "name": "test"})
query, params, err := r.toSQL(sq)
Expect(err).NotTo(HaveOccurred())
Expect(query).To(Equal("SELECT * FROM test WHERE id = {:p0} AND name = {:p1}"))
Expect(params).To(HaveKeyWithValue("p0", 1))
Expect(params).To(HaveKeyWithValue("p1", "test"))
})
})
Describe("sanitizeSort", func() {
BeforeEach(func() {
r.registerModel(&struct {
Field string `structs:"field"`
}{}, nil)
r.sortMappings = map[string]string{
"sort1": "mappedSort1",
}
})
When("sanitizing sort", func() {
It("returns empty if the sort key is not found in the model nor in the mappings", func() {
sort, _ := r.sanitizeSort(ctx, "unknown", "")
Expect(sort).To(BeEmpty())
})
// Validation only: buildSortOrder resolves the mapping, so mapping here too would hand
// sortMapping its own output and re-map values whose parts are themselves keys.
It("accepts a known sort key without resolving it", func() {
sort, _ := r.sanitizeSort(ctx, "sort1", "")
Expect(sort).To(Equal("sort1"))
})
It("is case insensitive", func() {
sort, _ := r.sanitizeSort(ctx, "Sort1", "")
Expect(sort).To(Equal("sort1"))
})
It("still resolves the mapping by the time the SQL is built", func() {
Expect(r.buildSortOrder("sort1", "asc")).To(Equal("mappedSort1 asc"))
})
// A mapping whose parts are themselves keys (media_file rated_at = "rating, rated_at")
// must survive the round trip through sanitizeSort and buildSortOrder unduplicated.
It("does not re-map a value whose parts are also keys", func() {
r.sortMappings = map[string]string{"rating": "rating", "rated_at": "rating, rated_at"}
sort, _ := r.sanitizeSort(ctx, "rated_at", "")
Expect(r.buildSortOrder(sort, "asc")).To(Equal("rating asc, rated_at asc"))
})
It("returns the field if it is a valid field", func() {
sort, _ := r.sanitizeSort(ctx, "field", "")
Expect(sort).To(Equal("field"))
})
It("is case insensitive for fields", func() {
sort, _ := r.sanitizeSort(ctx, "FIELD", "")
Expect(sort).To(Equal("field"))
})
})
When("sanitizing order", func() {
It("returns 'asc' if order is empty", func() {
_, order := r.sanitizeSort(ctx, "", "")
Expect(order).To(Equal(""))
})
It("returns 'asc' if order is 'asc'", func() {
_, order := r.sanitizeSort(ctx, "", "ASC")
Expect(order).To(Equal("asc"))
})
It("returns 'desc' if order is 'desc'", func() {
_, order := r.sanitizeSort(ctx, "", "desc")
Expect(order).To(Equal("desc"))
})
It("returns 'asc' if order is unknown", func() {
_, order := r.sanitizeSort(ctx, "", "something")
Expect(order).To(Equal("asc"))
})
})
})
Describe("sortMapping", func() {
BeforeEach(func() {
r.sortMappings = map[string]string{
"name": "order_album_name, order_album_artist_name",
"recently_added": "album.created_at, album.id",
}
})
It("maps a single key", func() {
Expect(r.sortMapping("recently_added")).To(Equal("album.created_at, album.id"))
})
It("maps every part of a comma list when all of them are known keys", func() {
Expect(r.sortMapping("recently_added, name")).
To(Equal("album.created_at, album.id, order_album_name, order_album_artist_name"))
})
It("resolves the known parts of a mixed list and leaves the rest as columns", func() {
Expect(r.sortMapping("recently_added, play_count")).
To(Equal("album.created_at, album.id, play_count"))
})
// Jellyfin's MusicAlbum SortBy=Runtime,SortName arrives as "duration, name"; duration is a
// plain album column while name is mapped, and the mapping must survive the mix.
It("keeps a mapping when an earlier part is a plain column", func() {
Expect(r.sortMapping("duration, name")).
To(Equal("duration, order_album_name, order_album_artist_name"))
})
It("leaves a raw column list with directions untouched", func() {
Expect(r.sortMapping("starred desc, rating desc")).To(Equal("starred desc, rating desc"))
})
It("does not split an expression on a comma inside its parentheses", func() {
Expect(r.sortMapping("coalesce(name, ''), title")).To(Equal("coalesce(name, ''), title"))
Expect(r.sortMapping("coalesce(nullif(a,''), b) desc, c")).To(Equal("coalesce(nullif(a,''), b) desc, c"))
})
It("keeps a mapping whose value nests commas inside parentheses", func() {
r.sortMappings["max_year"] = "coalesce(nullif(original_date,''), cast(max_year as text)), release_date"
Expect(r.sortMapping("max_year, name")).To(Equal(
"coalesce(nullif(original_date,''), cast(max_year as text)), release_date, " +
"order_album_name, order_album_artist_name"))
})
})
Describe("buildSortOrder", func() {
BeforeEach(func() {
r.sortMappings = map[string]string{}
})
Context("single field", func() {
It("sorts by specified field", func() {
sql := r.buildSortOrder("name", "desc")
Expect(sql).To(Equal("name desc"))
})
It("defaults to 'asc'", func() {
sql := r.buildSortOrder("name", "")
Expect(sql).To(Equal("name asc"))
})
It("inverts pre-defined order", func() {
sql := r.buildSortOrder("name desc", "desc")
Expect(sql).To(Equal("name asc"))
})
It("forces snake case for field names", func() {
sql := r.buildSortOrder("AlbumArtist", "asc")
Expect(sql).To(Equal("album_artist asc"))
})
})
Context("multiple fields", func() {
It("handles multiple fields", func() {
sql := r.buildSortOrder("name desc,age asc, status desc ", "asc")
Expect(sql).To(Equal("name desc, age asc, status desc"))
})
It("inverts multiple fields", func() {
sql := r.buildSortOrder("name desc, age, status asc", "desc")
Expect(sql).To(Equal("name asc, age desc, status desc"))
})
It("handles spaces in mapped field", func() {
r.sortMappings = map[string]string{
"has_lyrics": "(lyrics != '[]'), updated_at",
}
sql := r.buildSortOrder("has_lyrics", "desc")
Expect(sql).To(Equal("(lyrics != '[]') desc, updated_at desc"))
})
})
Context("function fields", func() {
It("handles functions with multiple params", func() {
sql := r.buildSortOrder("substr(id, 7)", "asc")
Expect(sql).To(Equal("substr(id, 7) asc"))
})
It("handles functions with multiple params mixed with multiple fields", func() {
sql := r.buildSortOrder("name desc, substr(id, 7), status asc", "desc")
Expect(sql).To(Equal("name asc, substr(id, 7) desc, status desc"))
})
It("handles nested functions", func() {
sql := r.buildSortOrder("name desc, coalesce(nullif(release_date, ''), nullif(original_date, '')), status asc", "desc")
Expect(sql).To(Equal("name asc, coalesce(nullif(release_date, ''), nullif(original_date, '')) desc, status desc"))
})
})
})
Describe("resetSeededRandom", func() {
var id string
BeforeEach(func() {
id = r.seedKey(ctx)
hasher.SetSeed(id, "")
})
It("does not reset seed if sort is not random", func() {
var options []model.QueryOptions
r.resetSeededRandom(ctx, options)
Expect(hasher.CurrentSeed(id)).To(BeEmpty())
})
It("resets seed if sort is random", func() {
options := []model.QueryOptions{{Sort: "random"}}
r.resetSeededRandom(ctx, 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(ctx, 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(ctx, options)
Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
options = []model.QueryOptions{{Sort: "random", Offset: 1}}
r.resetSeededRandom(ctx, options)
Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
})
})
Describe("applyLibraryFilter", func() {
var sq squirrel.SelectBuilder
var savedDB = r.db
BeforeEach(func() {
sq = squirrel.Select("*").From("test_table")
// Add library 2 so a user granted only library 1 is a genuine strict subset.
savedDB = r.db
r.db = GetDBXBuilder()
_, err := r.db.NewQuery("INSERT OR IGNORE INTO library (id, name, path) VALUES (2, 'Lib 2', '/lib2')").Execute()
Expect(err).ToNot(HaveOccurred())
})
AfterEach(func() {
_, err := r.db.NewQuery("DELETE FROM library WHERE id = 2").Execute()
Expect(err).ToNot(HaveOccurred())
r.db = savedDB
})
Context("Admin User", func() {
BeforeEach(func() {
ctx = request.WithUser(ctx, model.User{ID: "admin", IsAdmin: true})
})
It("should not apply library filter for admin users", func() {
result := r.applyLibraryFilter(ctx, sq)
sql, _, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(Equal("SELECT * FROM test_table"))
})
})
Context("Regular User with a subset of libraries", func() {
BeforeEach(func() {
// Strict subset: granted lib 1, DB has libs 1 and 2, so the filter must apply.
ctx = request.WithUser(ctx, model.User{
ID: "user123", IsAdmin: false, Libraries: model.Libraries{{ID: 1}},
})
})
It("should apply library filter for regular users", func() {
result := r.applyLibraryFilter(ctx, sq)
sql, args, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(ContainSubstring("IN (SELECT ul.library_id FROM user_library ul WHERE ul.user_id = ?)"))
Expect(args).To(ContainElement("user123"))
})
It("should use custom table name when provided", func() {
result := r.applyLibraryFilter(ctx, sq, "custom_table")
sql, args, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(ContainSubstring("custom_table.library_id IN"))
Expect(args).To(ContainElement("user123"))
})
})
Context("Regular User with no libraries", func() {
BeforeEach(func() {
ctx = request.WithUser(ctx, model.User{ID: "empty", IsAdmin: false})
})
It("should apply the library filter (never skip on empty)", func() {
result := r.applyLibraryFilter(ctx, sq)
sql, _, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(ContainSubstring("IN (SELECT ul.library_id FROM user_library ul WHERE ul.user_id = ?)"))
})
})
Context("Regular User who can see all libraries", func() {
BeforeEach(func() {
// Grant every library that currently exists in the (shared) DB, so the filter
// would exclude nothing. Querying the real IDs keeps this correct even if other
// specs left extra libraries behind, which happens under Ginkgo's randomized order.
var ids []int
err := r.db.NewQuery("SELECT id FROM library ORDER BY id").Column(&ids)
Expect(err).ToNot(HaveOccurred())
libs := make(model.Libraries, 0, len(ids))
for _, id := range ids {
libs = append(libs, model.Library{ID: id})
}
ctx = request.WithUser(ctx, model.User{
ID: "alllibs", IsAdmin: false, Libraries: libs,
})
})
It("should not apply the library filter (subquery would filter nothing)", func() {
result := r.applyLibraryFilter(ctx, sq)
sql, _, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(Equal("SELECT * FROM test_table"))
})
It("should not apply the filter even with a custom table name", func() {
result := r.applyLibraryFilter(ctx, sq, "custom_table")
sql, _, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(Equal("SELECT * FROM test_table"))
})
})
Context("Headless Process (No User Context)", func() {
BeforeEach(func() {
ctx = GinkgoT().Context() // No user context
})
It("should not apply library filter for headless processes", func() {
result := r.applyLibraryFilter(ctx, sq)
sql, _, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(Equal("SELECT * FROM test_table"))
})
It("should not apply library filter even with custom table name", func() {
result := r.applyLibraryFilter(ctx, sq, "custom_table")
sql, _, err := result.ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(Equal("SELECT * FROM test_table"))
})
})
})
})
var _ = Describe("paginated queries", func() {
var ctx context.Context
var mr model.MediaFileRepository
BeforeEach(func() {
ctx = request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
DeferCleanup(configtest.SetupConfig())
mr = NewMediaFileRepository(GetDBXBuilder())
})
Describe("SQL", func() {
var r sqlRepository
BeforeEach(func() {
r = sqlRepository{tableName: "media_file", sortMappings: map[string]string{"name": "title"}}
conf.Server.DevOffsetOptimize = 10
})
DescribeTable("adds a rowid tie-breaker in the requested direction",
func(options model.QueryOptions, expectedOrder string) {
sql, _, err := r.paginate(squirrel.Select("*").From("media_file"), options).ToSql()
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(HaveSuffix(expectedOrder))
},
Entry("no sort", model.QueryOptions{Max: 5}, "ORDER BY media_file.rowid asc"),
Entry("desc", model.QueryOptions{Sort: "name", Order: "desc", Max: 5}, "ORDER BY media_file.rowid desc"),
)
It("leaves unpaginated queries alone", func() {
sql, _, _ := r.paginate(squirrel.Select("*").From("media_file"), model.QueryOptions{Sort: "name"}).ToSql()
Expect(sql).To(Equal("SELECT * FROM media_file"))
})
It("uses the rowid subquery only past the threshold, keeping the outer projection", func() {
sq := squirrel.Select("media_file.*", "library.path as library_path").From("media_file").OrderBy("id asc")
atThreshold, _, _ := r.paginate(sq.Limit(5).Offset(10), model.QueryOptions{Max: 5, Offset: 10}).ToSql()
Expect(atThreshold).ToNot(ContainSubstring("not in"))
past, _, _ := r.paginate(sq.Limit(5).Offset(11), model.QueryOptions{Max: 5, Offset: 11}).ToSql()
Expect(past).To(ContainSubstring("media_file.rowid not in (SELECT _paginated_rowid FROM (" +
"SELECT media_file.*, library.path as library_path, media_file.rowid as _paginated_rowid FROM media_file"))
Expect(past).ToNot(ContainSubstring("OFFSET"))
})
})
// walk pages through the repository, crossing the optimizer threshold
walk := func(ctx context.Context, sort, order string, pageSize int) []string {
var ids []string
for offset := 0; ; offset += pageSize {
page, err := mr.GetAll(ctx, model.QueryOptions{Sort: sort, Order: order, Offset: offset, Max: pageSize, Seed: "seed"})
Expect(err).ToNot(HaveOccurred(), "sort=%s order=%s offset=%d", sort, order, offset)
for _, m := range page {
ids = append(ids, m.ID)
}
if len(page) < pageSize {
return ids
}
}
}
DescribeTable("returns every song exactly once, in the same order as a single page",
func(sort string) {
conf.Server.DevOffsetOptimize = 2
for _, order := range []string{"asc", "desc"} {
expected := walk(ctx, sort, order, 1000)
Expect(len(expected)).To(BeNumerically(">=", len(testSongs)))
Expect(slices.Compact(slices.Sorted(slices.Values(expected)))).To(HaveLen(len(expected)))
// Page size 2 hits offset == threshold (plain OFFSET), size 3 hits offset > threshold first.
for _, pageSize := range []int{2, 3} {
Expect(walk(ctx, sort, order, pageSize)).To(Equal(expected), "sort=%s order=%s page=%d", sort, order, pageSize)
}
}
},
Entry("id", "id"),
Entry("path", "path"),
Entry("updatedAt", "updatedAt"),
Entry("comment", "comment"),
Entry("title", "title"),
Entry("createdAt", "createdAt"),
Entry("starred (ties)", "starred"),
Entry("playCount (ties)", "playCount"),
Entry("rating (ties)", "rating"),
Entry("album", "album"),
Entry("seeded random", "random"),
)
It("keeps the library filter on every page", func() {
_, otherLib, restrictedUser := restrictedFixture("paginate")
hidden := model.MediaFile{ID: "paginate-hidden", Title: "Hidden", LibraryID: otherLib.ID, Path: p("other/hidden.mp3")}
Expect(mr.Put(ctx, &hidden)).To(Succeed())
DeferCleanup(func() { _ = mr.Delete(ctx, hidden.ID) })
conf.Server.DevOffsetOptimize = 2
Expect(walk(ctx, "id", "asc", 1000)).To(ContainElement(hidden.ID))
restrictedCtx := request.WithUser(ctx, restrictedUser)
expected := walk(restrictedCtx, "id", "asc", 1000)
Expect(expected).ToNot(BeEmpty())
Expect(expected).ToNot(ContainElement(hidden.ID))
Expect(walk(restrictedCtx, "id", "asc", 3)).To(Equal(expected))
})
Describe("legacy search", func() {
BeforeEach(func() {
conf.Server.Search.Backend = "legacy"
conf.Server.DevOffsetOptimize = 0
})
It("pages albums whose name is shared with the joined library", func() {
alr := NewAlbumRepository(GetDBXBuilder())
all, err := alr.Search(ctx, "abbey", model.QueryOptions{Max: 10})
Expect(err).ToNot(HaveOccurred())
Expect(all).To(HaveLen(2))
second, err := alr.Search(ctx, "abbey", model.QueryOptions{Max: 1, Offset: 1})
Expect(err).ToNot(HaveOccurred())
Expect(second).To(HaveLen(1))
Expect(second[0].ID).To(Equal(all[1].ID))
})
It("pages artists", func() {
arr := NewArtistRepository(GetDBXBuilder())
all, err := arr.Search(ctx, "the", model.QueryOptions{Max: 10})
Expect(err).ToNot(HaveOccurred())
Expect(len(all)).To(BeNumerically(">=", 2))
second, err := arr.Search(ctx, "the", model.QueryOptions{Max: 1, Offset: 1})
Expect(err).ToNot(HaveOccurred())
Expect(second).To(HaveLen(1))
Expect(second[0].ID).To(Equal(all[1].ID))
})
})
})