From beb768cd9cd00f01581fe190a345ccf8617950db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Fri, 14 Mar 2025 21:43:52 -0400
Subject: [PATCH] feat(server): add Role filters to albums (#3829)

* navidrome artist filtering

* address discord feedback

* perPage min 36

* various artist artist_id -> albumartist_id

* artist_id, role_id separate

* remove all ui changes I guess

* Add tests, check for possible SQL injection

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Kendall Garner <17521368+kgarner7@users.noreply.github.com>
---
 persistence/album_repository.go      | 28 ++++++++++++++---
 persistence/album_repository_test.go | 47 ++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/persistence/album_repository.go b/persistence/album_repository.go
index f98375f21..b29a44701 100644
--- a/persistence/album_repository.go
+++ b/persistence/album_repository.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"maps"
 	"slices"
+	"strings"
 	"sync"
 	"time"
 
@@ -119,11 +120,17 @@ var albumFilters = sync.OnceValue(func() map[string]filterFunc {
 		"has_rating":      hasRatingFilter,
 		"missing":         booleanFilter,
 		"genre_id":        tagIDFilter,
+		"role_total_id":   allRolesFilter,
 	}
 	// Add all album tags as filters
 	for tag := range model.AlbumLevelTags() {
 		filters[string(tag)] = tagIDFilter
 	}
+
+	for role := range model.AllRoles {
+		filters["role_"+role+"_id"] = artistRoleFilter
+	}
+
 	return filters
 })
 
@@ -153,14 +160,25 @@ func yearFilter(_ string, value interface{}) Sqlizer {
 	}
 }
 
-// BFR: Support other roles
 func artistFilter(_ string, value interface{}) Sqlizer {
 	return Or{
-		Exists("json_tree(Participants, '$.albumartist')", Eq{"value": value}),
-		Exists("json_tree(Participants, '$.artist')", Eq{"value": value}),
+		Exists("json_tree(participants, '$.albumartist')", Eq{"value": value}),
+		Exists("json_tree(participants, '$.artist')", Eq{"value": value}),
 	}
-	// For any role:
-	//return Like{"Participants": fmt.Sprintf(`%%"%s"%%`, value)}
+}
+
+func artistRoleFilter(name string, value interface{}) Sqlizer {
+	roleName := strings.TrimSuffix(strings.TrimPrefix(name, "role_"), "_id")
+
+	// Check if the role name is valid. If not, return an invalid filter
+	if _, ok := model.AllRoles[roleName]; !ok {
+		return Gt{"": nil}
+	}
+	return Exists(fmt.Sprintf("json_tree(participants, '$.%s')", roleName), Eq{"value": value})
+}
+
+func allRolesFilter(_ string, value interface{}) Sqlizer {
+	return Like{"participants": fmt.Sprintf(`%%"%s"%%`, value)}
 }
 
 func (r *albumRepository) CountAll(options ...model.QueryOptions) (int64, error) {
diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go
index dba347b30..529458c26 100644
--- a/persistence/album_repository_test.go
+++ b/persistence/album_repository_test.go
@@ -2,6 +2,7 @@ package persistence
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/navidrome/navidrome/conf"
@@ -236,6 +237,52 @@ var _ = Describe("AlbumRepository", func() {
 			}
 		})
 	})
+
+	Describe("artistRoleFilter", func() {
+		DescribeTable("creates correct SQL expressions for artist roles",
+			func(filterName, artistID, expectedSQL string) {
+				sqlizer := artistRoleFilter(filterName, artistID)
+				sql, args, err := sqlizer.ToSql()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(sql).To(Equal(expectedSQL))
+				Expect(args).To(Equal([]interface{}{artistID}))
+			},
+			Entry("artist role", "role_artist_id", "123",
+				"exists (select 1 from json_tree(participants, '$.artist') where value = ?)"),
+			Entry("albumartist role", "role_albumartist_id", "456",
+				"exists (select 1 from json_tree(participants, '$.albumartist') where value = ?)"),
+			Entry("composer role", "role_composer_id", "789",
+				"exists (select 1 from json_tree(participants, '$.composer') where value = ?)"),
+		)
+
+		It("works with the actual filter map", func() {
+			filters := albumFilters()
+
+			for roleName := range model.AllRoles {
+				filterName := "role_" + roleName + "_id"
+				filterFunc, exists := filters[filterName]
+				Expect(exists).To(BeTrue(), fmt.Sprintf("Filter %s should exist", filterName))
+
+				sqlizer := filterFunc(filterName, "test-id")
+				sql, args, err := sqlizer.ToSql()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(sql).To(Equal(fmt.Sprintf("exists (select 1 from json_tree(participants, '$.%s') where value = ?)", roleName)))
+				Expect(args).To(Equal([]interface{}{"test-id"}))
+			}
+		})
+
+		It("rejects invalid roles", func() {
+			sqlizer := artistRoleFilter("role_invalid_id", "123")
+			_, _, err := sqlizer.ToSql()
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("rejects invalid filter names", func() {
+			sqlizer := artistRoleFilter("invalid_name", "123")
+			_, _, err := sqlizer.ToSql()
+			Expect(err).To(HaveOccurred())
+		})
+	})
 })
 
 func _p(id, name string, sortName ...string) model.Participant {