fix(nativeapi): make /api/song path filter work and use startsWith (#5566)
The native API exposes a `path` query param on /api/song, but it was not registered in the media file filter map. Unmapped real columns fall through to a default LIKE predicate that emits an unqualified `path LIKE ?`. Since the song query joins the library table (which also has a `path` column), SQLite returned "ambiguous column name: path" and the request failed with HTTP 500. Register a dedicated path filter qualified to media_file.path, resolving the ambiguity. The value is matched with startsWith semantics (LIKE arg || '%') against the library-relative path stored in media_file.path. To register it inline (without a one-off wrapper), startsWithFilter now takes a bound field and returns a filterFunc, mirroring containsFilter. The two existing callers are updated accordingly, and the now-unused withTableName helper is removed. The user 'name' filter, which previously relied on withTableName, is now qualified directly as user.name; tests are added to guard that filter against the same column-ambiguity class (the user query also joins the library table, which has a name column). Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
parent
cf1f190bb5
commit
174621f259
6 changed files with 82 additions and 13 deletions
|
|
@ -104,6 +104,7 @@ var mediaFileFilter = sync.OnceValue(func() map[string]filterFunc {
|
||||||
"missing": booleanFilter,
|
"missing": booleanFilter,
|
||||||
"artists_id": artistFilter,
|
"artists_id": artistFilter,
|
||||||
"library_id": libraryIdFilter,
|
"library_id": libraryIdFilter,
|
||||||
|
"path": startsWithFilter("media_file.path"),
|
||||||
}
|
}
|
||||||
// Add all album tags as filters
|
// Add all album tags as filters
|
||||||
for tag := range model.TagMappings() {
|
for tag := range model.TagMappings() {
|
||||||
|
|
|
||||||
|
|
@ -524,6 +524,34 @@ var _ = Describe("MediaRepository", func() {
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
Describe("path", func() {
|
||||||
|
It("matches files whose path starts with the given prefix", func() {
|
||||||
|
res, err := mr.(model.ResourceRepository).ReadAll(rest.QueryOptions{
|
||||||
|
Filters: map[string]any{"path": "test/"},
|
||||||
|
})
|
||||||
|
Expect(err).ToNot(HaveOccurred())
|
||||||
|
files := res.(model.MediaFiles)
|
||||||
|
|
||||||
|
var found bool
|
||||||
|
for _, f := range files {
|
||||||
|
Expect(f.Path).To(HavePrefix("test/"))
|
||||||
|
if f.ID == mfWithoutAnnotation.ID {
|
||||||
|
found = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Expect(found).To(BeTrue(), "MediaFile with matching path prefix should be included")
|
||||||
|
})
|
||||||
|
|
||||||
|
It("excludes files whose path does not start with the given prefix", func() {
|
||||||
|
res, err := mr.(model.ResourceRepository).ReadAll(rest.QueryOptions{
|
||||||
|
Filters: map[string]any{"path": "no-such-prefix/"},
|
||||||
|
})
|
||||||
|
Expect(err).ToNot(HaveOccurred())
|
||||||
|
files := res.(model.MediaFiles)
|
||||||
|
Expect(files).To(BeEmpty())
|
||||||
|
})
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
Describe("Search", func() {
|
Describe("Search", func() {
|
||||||
|
|
|
||||||
|
|
@ -197,15 +197,6 @@ func (r sqlRepository) applyFilters(sq SelectBuilder, options ...model.QueryOpti
|
||||||
return sq
|
return sq
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *sqlRepository) withTableName(filter filterFunc) filterFunc {
|
|
||||||
return func(field string, value any) Sqlizer {
|
|
||||||
if r.tableName != "" {
|
|
||||||
field = r.tableName + "." + field
|
|
||||||
}
|
|
||||||
return filter(field, value)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// libraryIdFilter is a filter function to be added to resources that have a library_id column.
|
// libraryIdFilter is a filter function to be added to resources that have a library_id column.
|
||||||
func libraryIdFilter(_ string, value any) Sqlizer {
|
func libraryIdFilter(_ string, value any) Sqlizer {
|
||||||
return Eq{"library_id": value}
|
return Eq{"library_id": value}
|
||||||
|
|
|
||||||
|
|
@ -46,7 +46,7 @@ func (r *sqlRepository) parseRestFilters(ctx context.Context, options rest.Query
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
// Default to a "starts with" filter
|
// Default to a "starts with" filter
|
||||||
filters = append(filters, startsWithFilter(f, v))
|
filters = append(filters, Like{f: fmt.Sprintf("%s%%", v)})
|
||||||
}
|
}
|
||||||
return filters
|
return filters
|
||||||
}
|
}
|
||||||
|
|
@ -91,8 +91,10 @@ func eqFilter(field string, value any) Sqlizer {
|
||||||
return Eq{field: value}
|
return Eq{field: value}
|
||||||
}
|
}
|
||||||
|
|
||||||
func startsWithFilter(field string, value any) Sqlizer {
|
func startsWithFilter(field string) func(string, any) Sqlizer {
|
||||||
|
return func(_ string, value any) Sqlizer {
|
||||||
return Like{field: fmt.Sprintf("%s%%", value)}
|
return Like{field: fmt.Sprintf("%s%%", value)}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func containsFilter(field string) func(string, any) Sqlizer {
|
func containsFilter(field string) func(string, any) Sqlizer {
|
||||||
|
|
|
||||||
|
|
@ -59,7 +59,7 @@ func NewUserRepository(ctx context.Context, db dbx.Builder) model.UserRepository
|
||||||
r.registerModel(&model.User{}, map[string]filterFunc{
|
r.registerModel(&model.User{}, map[string]filterFunc{
|
||||||
"id": idFilter(r.tableName),
|
"id": idFilter(r.tableName),
|
||||||
"password": invalidFilter(ctx),
|
"password": invalidFilter(ctx),
|
||||||
"name": r.withTableName(startsWithFilter),
|
"name": startsWithFilter(r.tableName + ".name"),
|
||||||
})
|
})
|
||||||
once.Do(func() {
|
once.Do(func() {
|
||||||
_ = r.initPasswordEncryptionKey()
|
_ = r.initPasswordEncryptionKey()
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@ import (
|
||||||
"github.com/navidrome/navidrome/log"
|
"github.com/navidrome/navidrome/log"
|
||||||
"github.com/navidrome/navidrome/model"
|
"github.com/navidrome/navidrome/model"
|
||||||
"github.com/navidrome/navidrome/model/id"
|
"github.com/navidrome/navidrome/model/id"
|
||||||
|
"github.com/navidrome/navidrome/model/request"
|
||||||
"github.com/navidrome/navidrome/tests"
|
"github.com/navidrome/navidrome/tests"
|
||||||
. "github.com/onsi/ginkgo/v2"
|
. "github.com/onsi/ginkgo/v2"
|
||||||
. "github.com/onsi/gomega"
|
. "github.com/onsi/gomega"
|
||||||
|
|
@ -207,6 +208,52 @@ var _ = Describe("UserRepository", func() {
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
Describe("ReadAll name filter", func() {
|
||||||
|
var adminRepo model.ResourceRepository
|
||||||
|
|
||||||
|
BeforeEach(func() {
|
||||||
|
adminCtx := request.WithUser(GinkgoT().Context(), model.User{ID: "admin-id", UserName: "admin", IsAdmin: true})
|
||||||
|
adminRepo = NewUserRepository(adminCtx, GetDBXBuilder()).(model.ResourceRepository)
|
||||||
|
|
||||||
|
for _, u := range []model.User{
|
||||||
|
{ID: "filter-alice", UserName: "alice_filter", Name: "Alice Filter", NewPassword: "x"},
|
||||||
|
{ID: "filter-bob", UserName: "bob_filter", Name: "Bob Filter", NewPassword: "x"},
|
||||||
|
} {
|
||||||
|
Expect(adminRepo.(model.UserRepository).Put(&u)).To(Succeed())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
AfterEach(func() {
|
||||||
|
ur := adminRepo.(model.UserRepository)
|
||||||
|
_ = ur.Delete("filter-alice")
|
||||||
|
_ = ur.Delete("filter-bob")
|
||||||
|
})
|
||||||
|
|
||||||
|
It("matches users whose name starts with the given prefix", func() {
|
||||||
|
res, err := adminRepo.ReadAll(rest.QueryOptions{Filters: map[string]any{"name": "Alice"}})
|
||||||
|
Expect(err).ToNot(HaveOccurred())
|
||||||
|
users := res.(model.Users)
|
||||||
|
|
||||||
|
var names []string
|
||||||
|
for _, u := range users {
|
||||||
|
names = append(names, u.Name)
|
||||||
|
}
|
||||||
|
Expect(names).To(ContainElement("Alice Filter"))
|
||||||
|
Expect(names).ToNot(ContainElement("Bob Filter"))
|
||||||
|
})
|
||||||
|
|
||||||
|
It("does not match names by mid-string substring (startsWith, not contains)", func() {
|
||||||
|
res, err := adminRepo.ReadAll(rest.QueryOptions{Filters: map[string]any{"name": "Filter"}})
|
||||||
|
Expect(err).ToNot(HaveOccurred())
|
||||||
|
users := res.(model.Users)
|
||||||
|
|
||||||
|
for _, u := range users {
|
||||||
|
Expect(u.ID).ToNot(Or(Equal("filter-alice"), Equal("filter-bob")),
|
||||||
|
"a mid-string substring should not match a startsWith filter")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
Describe("validateUsernameUnique", func() {
|
Describe("validateUsernameUnique", func() {
|
||||||
var repo *tests.MockedUserRepo
|
var repo *tests.MockedUserRepo
|
||||||
var existingUser *model.User
|
var existingUser *model.User
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue