refactor: genre and tag repositories. add comprehensive tests

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-07-16 10:44:40 -04:00
commit dd7ae6ce2b
5 changed files with 569 additions and 54 deletions

View file

@ -10,20 +10,14 @@ import (
)
type genreRepository struct {
sqlRepository
*baseTagRepository
}
func NewGenreRepository(ctx context.Context, db dbx.Builder) model.GenreRepository {
r := &genreRepository{}
r.ctx = ctx
r.db = db
r.registerModel(&model.Tag{}, map[string]filterFunc{
"name": containsFilter("tag_value"),
})
r.setSortMappings(map[string]string{
"name": "tag_name",
})
return r
genreFilter := model.TagGenre
return &genreRepository{
baseTagRepository: newBaseTagRepository(ctx, db, &genreFilter),
}
}
func (r *genreRepository) selectGenre(opt ...model.QueryOptions) SelectBuilder {
@ -33,8 +27,7 @@ func (r *genreRepository) selectGenre(opt ...model.QueryOptions) SelectBuilder {
"tag_value as name",
"album_count",
"media_file_count as song_count",
).
Where(Eq{"tag.tag_name": model.TagGenre})
)
}
func (r *genreRepository) GetAll(opt ...model.QueryOptions) (model.Genres, error) {
@ -44,9 +37,7 @@ func (r *genreRepository) GetAll(opt ...model.QueryOptions) (model.Genres, error
return res, err
}
func (r *genreRepository) Count(options ...rest.QueryOptions) (int64, error) {
return r.count(r.selectGenre(), r.parseRestOptions(r.ctx, options...))
}
// Override ResourceRepository methods to return Genre objects instead of Tag objects
func (r *genreRepository) Read(id string) (interface{}, error) {
sel := r.selectGenre().Columns("*").Where(Eq{"id": id})
@ -59,10 +50,6 @@ func (r *genreRepository) ReadAll(options ...rest.QueryOptions) (interface{}, er
return r.GetAll(r.parseRestOptions(r.ctx, options...))
}
func (r *genreRepository) EntityName() string {
return r.tableName
}
func (r *genreRepository) NewInstance() interface{} {
return &model.Genre{}
}

View file

@ -0,0 +1,250 @@
package persistence
import (
"context"
"slices"
"strings"
"github.com/Masterminds/squirrel"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/model/request"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("GenreRepository", func() {
var repo model.GenreRepository
var restRepo model.ResourceRepository
var tagRepo model.TagRepository
var ctx context.Context
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
ctx = request.WithUser(log.NewContext(context.TODO()), model.User{ID: "userid", UserName: "johndoe"})
genreRepo := NewGenreRepository(ctx, GetDBXBuilder())
repo = genreRepo
restRepo = genreRepo.(model.ResourceRepository)
tagRepo = NewTagRepository(ctx, GetDBXBuilder())
// Clear any existing tags to ensure test isolation
db := GetDBXBuilder()
_, err := db.NewQuery("DELETE FROM tag").Execute()
Expect(err).ToNot(HaveOccurred())
// Add comprehensive test data that covers all test scenarios
newTag := func(name, value string) model.Tag {
return model.Tag{ID: id.NewTagID(name, value), TagName: model.TagName(name), TagValue: value}
}
err = tagRepo.Add(
newTag("genre", "rock"),
newTag("genre", "pop"),
newTag("genre", "jazz"),
newTag("genre", "electronic"),
newTag("genre", "classical"),
newTag("genre", "ambient"),
newTag("genre", "techno"),
newTag("genre", "house"),
newTag("genre", "trance"),
newTag("genre", "Alternative Rock"),
newTag("genre", "Blues"),
newTag("genre", "Country"),
// These should not be counted as genres
newTag("mood", "happy"),
newTag("mood", "ambient"),
)
Expect(err).ToNot(HaveOccurred())
})
Describe("GetAll", func() {
It("should return all genres", func() {
genres, err := repo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(genres).To(HaveLen(12))
// Verify that all returned items are genres (TagName = "genre")
genreNames := make([]string, len(genres))
for i, genre := range genres {
genreNames[i] = genre.Name
}
Expect(genreNames).To(ContainElement("rock"))
Expect(genreNames).To(ContainElement("pop"))
Expect(genreNames).To(ContainElement("jazz"))
// Should not contain mood tags
Expect(genreNames).ToNot(ContainElement("happy"))
})
It("should support query options", func() {
// Test with limiting results
genres, err := repo.GetAll(model.QueryOptions{Max: 1})
Expect(err).ToNot(HaveOccurred())
Expect(genres).To(HaveLen(1))
})
It("should handle empty results gracefully", func() {
// Clear all genre tags
_, err := GetDBXBuilder().NewQuery("DELETE FROM tag WHERE tag_name = 'genre'").Execute()
Expect(err).ToNot(HaveOccurred())
genres, err := repo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(genres).To(BeEmpty())
})
Describe("filtering and sorting", func() {
It("should filter by name using like match", func() {
// Test filtering by partial name match using the "name" filter which maps to containsFilter("tag_value")
options := model.QueryOptions{
Filters: squirrel.Like{"tag_value": "%rock%"}, // Direct field access
}
genres, err := repo.GetAll(options)
Expect(err).ToNot(HaveOccurred())
Expect(genres).To(HaveLen(2)) // Should match "rock" and "Alternative Rock"
// Verify all returned genres contain "rock" in their name
for _, genre := range genres {
Expect(strings.ToLower(genre.Name)).To(ContainSubstring("rock"))
}
})
It("should sort by name in ascending order", func() {
// Test sorting by name with the fixed mapping
options := model.QueryOptions{
Filters: squirrel.Like{"tag_value": "%e%"}, // Should match genres containing "e"
Sort: "name",
}
genres, err := repo.GetAll(options)
Expect(err).ToNot(HaveOccurred())
Expect(genres).To(HaveLen(7))
Expect(slices.IsSortedFunc(genres, func(a, b model.Genre) int {
return strings.Compare(b.Name, a.Name) // Inverted to check descending order
}))
})
It("should sort by name in descending order", func() {
// Test sorting by name in descending order
options := model.QueryOptions{
Filters: squirrel.Like{"tag_value": "%e%"}, // Should match genres containing "e"
Sort: "name",
Order: "desc",
}
genres, err := repo.GetAll(options)
Expect(err).ToNot(HaveOccurred())
Expect(genres).To(HaveLen(7))
Expect(slices.IsSortedFunc(genres, func(a, b model.Genre) int {
return strings.Compare(a.Name, b.Name)
}))
})
})
})
Describe("Count", func() {
It("should return correct count of genres", func() {
count, err := restRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(int64(12))) // We have 12 genre tags
})
It("should handle zero count", func() {
// Clear all genre tags
_, err := GetDBXBuilder().NewQuery("DELETE FROM tag WHERE tag_name = 'genre'").Execute()
Expect(err).ToNot(HaveOccurred())
count, err := restRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(BeZero())
})
It("should only count genre tags", func() {
// Add a non-genre tag
nonGenreTag := model.Tag{
ID: id.NewTagID("mood", "energetic"),
TagName: "mood",
TagValue: "energetic",
}
err := tagRepo.Add(nonGenreTag)
Expect(err).ToNot(HaveOccurred())
count, err := restRepo.Count()
Expect(err).ToNot(HaveOccurred())
// Count should not include the mood tag
Expect(count).To(Equal(int64(12))) // Should still be 12 genre tags
})
It("should filter by name using like match", func() {
// Test filtering by partial name match using the "name" filter which maps to containsFilter("tag_value")
options := rest.QueryOptions{
Filters: map[string]interface{}{"name": "%rock%"},
}
count, err := restRepo.Count(options)
Expect(err).ToNot(HaveOccurred())
Expect(count).To(BeNumerically("==", 2))
})
})
Describe("Read", func() {
It("should return existing genre", func() {
// Use one of the existing genres from our consolidated dataset
genreID := id.NewTagID("genre", "rock")
result, err := restRepo.Read(genreID)
Expect(err).ToNot(HaveOccurred())
genre := result.(*model.Genre)
Expect(genre.ID).To(Equal(genreID))
Expect(genre.Name).To(Equal("rock"))
})
It("should return error for non-existent genre", func() {
_, err := restRepo.Read("non-existent-id")
Expect(err).To(HaveOccurred())
})
It("should not return non-genre tags", func() {
moodID := id.NewTagID("mood", "happy") // This exists as a mood tag, not genre
_, err := restRepo.Read(moodID)
Expect(err).To(HaveOccurred()) // Should not find it as a genre
})
})
Describe("ReadAll", func() {
It("should return all genres through ReadAll", func() {
result, err := restRepo.ReadAll()
Expect(err).ToNot(HaveOccurred())
genres := result.(model.Genres)
Expect(genres).To(HaveLen(12)) // We have 12 genre tags
genreNames := make([]string, len(genres))
for i, genre := range genres {
genreNames[i] = genre.Name
}
// Check for some of our consolidated dataset genres
Expect(genreNames).To(ContainElement("rock"))
Expect(genreNames).To(ContainElement("pop"))
Expect(genreNames).To(ContainElement("jazz"))
})
It("should support rest query options", func() {
result, err := restRepo.ReadAll()
Expect(err).ToNot(HaveOccurred())
Expect(result).ToNot(BeNil())
})
})
Describe("EntityName", func() {
It("should return correct entity name", func() {
name := restRepo.EntityName()
Expect(name).To(Equal("tag")) // Genre repository uses tag table
})
})
Describe("NewInstance", func() {
It("should return new genre instance", func() {
instance := restRepo.NewInstance()
Expect(instance).To(BeAssignableToTypeOf(&model.Genre{}))
})
})
})

View file

@ -1,12 +1,15 @@
package persistence
import (
"context"
"encoding/json"
"fmt"
"strings"
. "github.com/Masterminds/squirrel"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/model"
"github.com/pocketbase/dbx"
)
// Format of a tag in the DB
@ -55,3 +58,69 @@ func tagIDFilter(name string, idValue any) Sqlizer {
},
)
}
// baseTagRepository provides common functionality for all tag-based repositories.
// It handles CRUD operations with optional filtering by tag name.
type baseTagRepository struct {
sqlRepository
tagFilter *model.TagName // nil = no filter (all tags), non-nil = filter by specific tag name
}
// newBaseTagRepository creates a new base tag repository with optional tag filtering.
// If tagFilter is nil, the repository will work with all tags.
// If tagFilter is provided, the repository will only work with tags of that specific name.
func newBaseTagRepository(ctx context.Context, db dbx.Builder, tagFilter *model.TagName) *baseTagRepository {
r := &baseTagRepository{
tagFilter: tagFilter,
}
r.ctx = ctx
r.db = db
r.tableName = "tag"
r.registerModel(&model.Tag{}, map[string]filterFunc{
"name": containsFilter("tag_value"),
})
r.setSortMappings(map[string]string{
"name": "tag_value",
})
return r
}
// newSelect overrides the base implementation to conditionally apply tag name filtering.
func (r *baseTagRepository) newSelect(options ...model.QueryOptions) SelectBuilder {
sq := r.sqlRepository.newSelect(options...)
if r.tagFilter != nil {
sq = sq.Where(Eq{"tag_name": *r.tagFilter})
}
return sq
}
// ResourceRepository interface implementation
func (r *baseTagRepository) Count(options ...rest.QueryOptions) (int64, error) {
return r.count(r.newSelect(), r.parseRestOptions(r.ctx, options...))
}
func (r *baseTagRepository) Read(id string) (interface{}, error) {
query := r.newSelect().Columns("*").Where(Eq{"id": id})
var res model.Tag
err := r.queryOne(query, &res)
return &res, err
}
func (r *baseTagRepository) ReadAll(options ...rest.QueryOptions) (interface{}, error) {
query := r.newSelect(r.parseRestOptions(r.ctx, options...)).Columns("*")
var res model.TagList
err := r.queryAll(query, &res)
return res, err
}
func (r *baseTagRepository) EntityName() string {
return "tag"
}
func (r *baseTagRepository) NewInstance() interface{} {
return model.Tag{}
}
// Interface compliance check
var _ model.ResourceRepository = (*baseTagRepository)(nil)

View file

@ -7,23 +7,19 @@ import (
"time"
. "github.com/Masterminds/squirrel"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/pocketbase/dbx"
)
type tagRepository struct {
sqlRepository
*baseTagRepository
}
func NewTagRepository(ctx context.Context, db dbx.Builder) model.TagRepository {
r := &tagRepository{}
r.ctx = ctx
r.db = db
r.tableName = "tag"
r.registerModel(&model.Tag{}, nil)
return r
return &tagRepository{
baseTagRepository: newBaseTagRepository(ctx, db, nil), // nil = no filter, works with all tags
}
}
func (r *tagRepository) Add(tags ...model.Tag) error {
@ -87,30 +83,4 @@ func (r *tagRepository) purgeUnused() error {
return err
}
func (r *tagRepository) Count(options ...rest.QueryOptions) (int64, error) {
return r.count(r.newSelect(), r.parseRestOptions(r.ctx, options...))
}
func (r *tagRepository) Read(id string) (interface{}, error) {
query := r.newSelect().Columns("*").Where(Eq{"id": id})
var res model.Tag
err := r.queryOne(query, &res)
return &res, err
}
func (r *tagRepository) ReadAll(options ...rest.QueryOptions) (interface{}, error) {
query := r.newSelect(r.parseRestOptions(r.ctx, options...)).Columns("*")
var res model.TagList
err := r.queryAll(query, &res)
return res, err
}
func (r *tagRepository) EntityName() string {
return "tag"
}
func (r *tagRepository) NewInstance() interface{} {
return model.Tag{}
}
var _ model.ResourceRepository = &tagRepository{}

View file

@ -0,0 +1,239 @@
package persistence
import (
"context"
"slices"
"strings"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/model/request"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("TagRepository", func() {
var repo model.TagRepository
var restRepo model.ResourceRepository
var ctx context.Context
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
ctx = request.WithUser(log.NewContext(context.TODO()), model.User{ID: "userid", UserName: "johndoe"})
tagRepo := NewTagRepository(ctx, GetDBXBuilder())
repo = tagRepo
restRepo = tagRepo.(model.ResourceRepository)
// Clean the database before each test to ensure isolation
db := GetDBXBuilder()
_, err := db.NewQuery("DELETE FROM tag").Execute()
Expect(err).ToNot(HaveOccurred())
// Add comprehensive test data that covers all test scenarios
newTag := func(name, value string) model.Tag {
return model.Tag{ID: id.NewTagID(name, value), TagName: model.TagName(name), TagValue: value}
}
err = repo.Add(
// Genre tags
newTag("genre", "rock"),
newTag("genre", "pop"),
newTag("genre", "jazz"),
newTag("genre", "electronic"),
newTag("genre", "classical"),
newTag("genre", "ambient"),
newTag("genre", "techno"),
newTag("genre", "house"),
newTag("genre", "trance"),
newTag("genre", "Alternative Rock"),
newTag("genre", "Blues"),
newTag("genre", "Country"),
// Mood tags
newTag("mood", "happy"),
newTag("mood", "sad"),
newTag("mood", "energetic"),
newTag("mood", "calm"),
// Other tag types
newTag("instrument", "guitar"),
newTag("instrument", "piano"),
newTag("decade", "1980s"),
newTag("decade", "1990s"),
)
Expect(err).ToNot(HaveOccurred())
})
Describe("Add", func() {
It("should handle adding new tags", func() {
newTag := model.Tag{
ID: id.NewTagID("genre", "experimental"),
TagName: "genre",
TagValue: "experimental",
}
err := repo.Add(newTag)
Expect(err).ToNot(HaveOccurred())
// Verify tag was added
result, err := restRepo.Read(newTag.ID)
Expect(err).ToNot(HaveOccurred())
resultTag := result.(*model.Tag)
Expect(resultTag.TagValue).To(Equal("experimental"))
// Check count increased
count, err := restRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(int64(21))) // 20 from dataset + 1 new
})
It("should handle duplicate tags gracefully", func() {
// Try to add a duplicate tag
duplicateTag := model.Tag{
ID: id.NewTagID("genre", "rock"), // This already exists
TagName: "genre",
TagValue: "rock",
}
count, err := restRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(int64(20))) // Still 20 tags
err = repo.Add(duplicateTag)
Expect(err).ToNot(HaveOccurred()) // Should not error
// Count should remain the same
count, err = restRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(int64(20))) // Still 20 tags
})
})
Describe("UpdateCounts", func() {
It("should update tag counts successfully", func() {
err := repo.UpdateCounts()
Expect(err).ToNot(HaveOccurred())
})
It("should handle empty database gracefully", func() {
// Clear the database first
db := GetDBXBuilder()
_, err := db.NewQuery("DELETE FROM tag").Execute()
Expect(err).ToNot(HaveOccurred())
err = repo.UpdateCounts()
Expect(err).ToNot(HaveOccurred())
})
})
Describe("Count", func() {
It("should return correct count of tags", func() {
count, err := restRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(int64(20))) // From the test dataset
})
})
Describe("Read", func() {
It("should return existing tag", func() {
rockID := id.NewTagID("genre", "rock")
result, err := restRepo.Read(rockID)
Expect(err).ToNot(HaveOccurred())
resultTag := result.(*model.Tag)
Expect(resultTag.ID).To(Equal(rockID))
Expect(resultTag.TagName).To(Equal(model.TagName("genre")))
Expect(resultTag.TagValue).To(Equal("rock"))
})
It("should return error for non-existent tag", func() {
_, err := restRepo.Read("non-existent-id")
Expect(err).To(HaveOccurred())
})
})
Describe("ReadAll", func() {
It("should return all tags from dataset", func() {
result, err := restRepo.ReadAll()
Expect(err).ToNot(HaveOccurred())
tags := result.(model.TagList)
Expect(tags).To(HaveLen(20))
})
It("should filter tags by partial value correctly", func() {
options := rest.QueryOptions{
Filters: map[string]interface{}{"name": "%rock%"}, // Tags containing 'rock'
}
result, err := restRepo.ReadAll(options)
Expect(err).ToNot(HaveOccurred())
tags := result.(model.TagList)
Expect(tags).To(HaveLen(2)) // "rock" and "Alternative Rock"
// Verify all returned tags contain 'rock' in their value
for _, tag := range tags {
Expect(strings.ToLower(tag.TagValue)).To(ContainSubstring("rock"))
}
})
It("should filter tags by partial value using LIKE", func() {
options := rest.QueryOptions{
Filters: map[string]interface{}{"name": "%e%"}, // Tags containing 'e'
}
result, err := restRepo.ReadAll(options)
Expect(err).ToNot(HaveOccurred())
tags := result.(model.TagList)
Expect(tags).To(HaveLen(8)) // electronic, house, trance, energetic, Blues, decade x2, Alternative Rock
// Verify all returned tags contain 'e' in their value
for _, tag := range tags {
Expect(strings.ToLower(tag.TagValue)).To(ContainSubstring("e"))
}
})
It("should sort tags by value ascending", func() {
options := rest.QueryOptions{
Filters: map[string]interface{}{"name": "%r%"}, // Tags containing 'r'
Sort: "name",
Order: "asc",
}
result, err := restRepo.ReadAll(options)
Expect(err).ToNot(HaveOccurred())
tags := result.(model.TagList)
Expect(tags).To(HaveLen(7))
Expect(slices.IsSortedFunc(tags, func(a, b model.Tag) int {
return strings.Compare(strings.ToLower(a.TagValue), strings.ToLower(b.TagValue))
}))
})
It("should sort tags by value descending", func() {
options := rest.QueryOptions{
Filters: map[string]interface{}{"name": "%r%"}, // Tags containing 'r'
Sort: "name",
Order: "desc",
}
result, err := restRepo.ReadAll(options)
Expect(err).ToNot(HaveOccurred())
tags := result.(model.TagList)
Expect(tags).To(HaveLen(7))
Expect(slices.IsSortedFunc(tags, func(a, b model.Tag) int {
return strings.Compare(strings.ToLower(b.TagValue), strings.ToLower(a.TagValue)) // Descending order
}))
})
})
Describe("EntityName", func() {
It("should return correct entity name", func() {
name := restRepo.EntityName()
Expect(name).To(Equal("tag"))
})
})
Describe("NewInstance", func() {
It("should return new tag instance", func() {
instance := restRepo.NewInstance()
Expect(instance).To(BeAssignableToTypeOf(model.Tag{}))
})
})
})