Merge 1edcad46cce5e408ed5b002b969f991c48b6133b into e86dc03619ffb8477083de23bb4daed567ef0a2c

Optimized to use the normalized media_file_artists table instead of parsing JSONB

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

model/playlist.go

@@ -9,6 +9,8 @@ import (
 )
 
 type Playlist struct {
+	Annotations `structs:"-" hash:"ignore"`
+
 	ID        string         `structs:"id" json:"id"`
 	Name      string         `structs:"name" json:"name"`
 	Comment   string         `structs:"comment" json:"comment"`
@@ -110,6 +112,7 @@ type Playlists []Playlist
 
 type PlaylistRepository interface {
 	ResourceRepository
+	AnnotatedRepository
 	CountAll(options ...QueryOptions) (int64, error)
 	Exists(id string) (bool, error)
 	Put(pls *Playlist) error

persistence/artist_repository.go

@@ -400,23 +400,16 @@ func (r *artistRepository) RefreshStats(allArtists bool) (int64, error) {
 	// This now calculates per-library statistics and stores them in library_artist.stats
 	batchUpdateStatsSQL := `
     WITH artist_role_counters AS (
-        SELECT jt.atom AS artist_id,
+        SELECT mfa.artist_id,
                mf.library_id,
-               substr(
-                       replace(jt.path, '$.', ''),
-                       1,
-                       CASE WHEN instr(replace(jt.path, '$.', ''), '[') > 0
-                                THEN instr(replace(jt.path, '$.', ''), '[') - 1
-                            ELSE length(replace(jt.path, '$.', ''))
-                           END
-               ) AS role,
+               mfa.role,
                count(DISTINCT mf.album_id) AS album_count,
-               count(mf.id) AS count,
+               count(DISTINCT mf.id) AS count,
                sum(mf.size) AS size
-        FROM media_file mf
-        JOIN json_tree(mf.participants) jt ON jt.key = 'id' AND jt.atom IS NOT NULL
-        WHERE jt.atom IN (ROLE_IDS_PLACEHOLDER) -- Will replace with actual placeholders
-        GROUP BY jt.atom, mf.library_id, role
+        FROM media_file_artists mfa
+        JOIN media_file mf ON mfa.media_file_id = mf.id
+        WHERE mfa.artist_id IN (ROLE_IDS_PLACEHOLDER) -- Will replace with actual placeholders
+        GROUP BY mfa.artist_id, mf.library_id, mfa.role
     ),
     artist_total_counters AS (
         SELECT mfa.artist_id,
@@ -445,24 +438,24 @@ func (r *artistRepository) RefreshStats(allArtists bool) (int64, error) {
     ),
     combined_counters AS (
         SELECT artist_id, library_id, role, album_count, count, size FROM artist_role_counters
-        UNION
+        UNION ALL
         SELECT artist_id, library_id, role, album_count, count, size FROM artist_total_counters
-        UNION
+        UNION ALL
         SELECT artist_id, library_id, role, album_count, count, size FROM artist_participant_counter
     ),
     library_artist_counters AS (
         SELECT artist_id,
                library_id,
                json_group_object(
-                       replace(role, '"', ''),
+                       role,
                        json_object('a', album_count, 'm', count, 's', size)
                ) AS counters
         FROM combined_counters
         GROUP BY artist_id, library_id
     )
     UPDATE library_artist
-    SET stats = coalesce((SELECT counters FROM library_artist_counters lac 
-                         WHERE lac.artist_id = library_artist.artist_id 
+    SET stats = coalesce((SELECT counters FROM library_artist_counters lac
+                         WHERE lac.artist_id = library_artist.artist_id
                          AND lac.library_id = library_artist.library_id), '{}')
     WHERE library_artist.artist_id IN (ROLE_IDS_PLACEHOLDER);` // Will replace with actual placeholders
 

persistence/playlist_repository.go

@@ -51,12 +51,16 @@ func NewPlaylistRepository(ctx context.Context, db dbx.Builder) model.PlaylistRe
 	r := &playlistRepository{}
 	r.ctx = ctx
 	r.db = db
+	r.tableName = "playlist"
 	r.registerModel(&model.Playlist{}, map[string]filterFunc{
-		"q":     playlistFilter,
-		"smart": smartPlaylistFilter,
+		"id":      idFilter(r.tableName),
+		"q":       playlistFilter,
+		"smart":   smartPlaylistFilter,
+		"starred": booleanFilter,
 	})
 	r.setSortMappings(map[string]string{
 		"owner_name": "owner_name",
+		"starred_at": "starred, starred_at",
 	})
 	return r
 }
@@ -87,12 +91,14 @@ func (r *playlistRepository) userFilter() Sqlizer {
 }
 
 func (r *playlistRepository) CountAll(options ...model.QueryOptions) (int64, error) {
-	sq := Select().Where(r.userFilter())
+	sq := r.newSelect()
+	sq = r.withAnnotation(sq, r.tableName+".id")
+	sq = sq.Where(r.userFilter())
 	return r.count(sq, options...)
 }
 
 func (r *playlistRepository) Exists(id string) (bool, error) {
-	return r.exists(And{Eq{"id": id}, r.userFilter()})
+	return r.exists(And{Eq{"playlist.id": id}, r.userFilter()})
 }
 
 func (r *playlistRepository) Delete(id string) error {
@@ -106,7 +112,7 @@ func (r *playlistRepository) Delete(id string) error {
 			return rest.ErrPermissionDenied
 		}
 	}
-	return r.delete(And{Eq{"id": id}, r.userFilter()})
+	return r.delete(And{Eq{"playlist.id": id}, r.userFilter()})
 }
 
 func (r *playlistRepository) Put(p *model.Playlist) error {
@@ -217,8 +223,9 @@ func (r *playlistRepository) GetPlaylists(mediaFileId string) (model.Playlists,
 }
 
 func (r *playlistRepository) selectPlaylist(options ...model.QueryOptions) SelectBuilder {
-	return r.newSelect(options...).Join("user on user.id = owner_id").
+	query := r.newSelect(options...).Join("user on user.id = owner_id").
 		Columns(r.tableName+".*", "user.user_name as owner_name")
+	return r.withAnnotation(query, r.tableName+".id")
 }
 
 func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {

persistence/playlist_repository_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	sq "github.com/Masterminds/squirrel"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
@@ -110,6 +111,45 @@ var _ = Describe("PlaylistRepository", func() {
 			Expect(all[0].ID).To(Equal(plsBest.ID))
 			Expect(all[1].ID).To(Equal(plsCool.ID))
 		})
+
+		It("filters starred playlists", func() {
+			Expect(repo.SetStar(true, plsBest.ID)).To(Succeed())
+
+			all, err := repo.GetAll(model.QueryOptions{Filters: sq.Eq{"starred": true}})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(all).To(HaveLen(1))
+			Expect(all[0].ID).To(Equal(plsBest.ID))
+
+			Expect(repo.SetStar(false, plsBest.ID)).To(Succeed())
+		})
+
+		It("counts starred playlists", func() {
+			Expect(repo.SetStar(true, plsCool.ID)).To(Succeed())
+			count, err := repo.CountAll(model.QueryOptions{Filters: sq.Eq{"starred": true}})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(count).To(Equal(int64(1)))
+
+			Expect(repo.SetStar(false, plsCool.ID)).To(Succeed())
+		})
+	})
+
+	Describe("SetStar", func() {
+		It("should star a playlist", func() {
+			Expect(repo.SetStar(true, plsBest.ID)).To(Succeed())
+
+			updated, err := repo.Get(plsBest.ID)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(updated.Starred).To(BeTrue())
+			Expect(updated.StarredAt).ToNot(BeNil())
+		})
+
+		It("should unstar a playlist", func() {
+			Expect(repo.SetStar(false, plsBest.ID)).To(Succeed())
+
+			updated, err := repo.Get(plsBest.ID)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(updated.Starred).To(BeFalse())
+		})
 	})
 
 	Describe("GetPlaylists", func() {

server/subsonic/media_annotation.go

@@ -138,6 +138,20 @@ func (api *Router) setStar(ctx context.Context, star bool, ids ...string) error
 				event = event.With("artist", id)
 				continue
 			}
+			exist, err = tx.Playlist(ctx).Exists(id)
+			if err != nil {
+				return err
+			}
+			if exist {
+				err = tx.Playlist(ctx).SetStar(star, id)
+				if err != nil {
+					return err
+				}
+				event = event.With("playlist", "*")
+				// Ensure the refresh event is sent to all clients, including the originator
+				ctx = events.BroadcastToAll(ctx)
+				continue
+			}
 			err = tx.MediaFile(ctx).SetStar(star, id)
 			if err != nil {
 				return err

server/subsonic/media_annotation_test.go

@@ -30,6 +30,42 @@ var _ = Describe("MediaAnnotationController", func() {
 		router = New(ds, nil, nil, nil, nil, nil, nil, eventBroker, nil, playTracker, nil, nil, nil)
 	})
 
+	Describe("Star", func() {
+		It("should send refresh resource event when starring a playlist", func() {
+			mockPlaylistRepo := &tests.MockPlaylistRepo{
+				Entity: &model.Playlist{ID: "pls-1", Name: "Test Playlist"},
+			}
+			ds.(*tests.MockDataStore).MockedPlaylist = mockPlaylistRepo
+
+			r := newGetRequest("id=pls-1")
+			_, err := router.Star(r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(eventBroker.Events).To(HaveLen(1))
+
+			event := eventBroker.Events[0].(*events.RefreshResource)
+			data := event.Data(event)
+			Expect(data).To(ContainSubstring(`"playlist":["*"]`))
+		})
+
+		It("should send refresh resource event when unstarring a playlist", func() {
+			mockPlaylistRepo := &tests.MockPlaylistRepo{
+				Entity: &model.Playlist{ID: "pls-1", Name: "Test Playlist"},
+			}
+			ds.(*tests.MockDataStore).MockedPlaylist = mockPlaylistRepo
+
+			r := newGetRequest("id=pls-1")
+			_, err := router.Unstar(r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(eventBroker.Events).To(HaveLen(1))
+
+			event := eventBroker.Events[0].(*events.RefreshResource)
+			data := event.Data(event)
+			Expect(data).To(ContainSubstring(`"playlist":["*"]`))
+		})
+	})
+
 	Describe("Scrobble", func() {
 		It("submit all scrobbles with only the id", func() {
 			submissionTime := time.Now()

tests/mock_playlist_repo.go

@@ -8,8 +8,9 @@ import (
 type MockPlaylistRepo struct {
 	model.PlaylistRepository
 
-	Entity *model.Playlist
-	Error  error
+	Entity           *model.Playlist
+	Error            error
+	SetStarWasCalled bool
 }
 
 func (m *MockPlaylistRepo) Get(_ string) (*model.Playlist, error) {
@@ -31,3 +32,18 @@ func (m *MockPlaylistRepo) Count(_ ...rest.QueryOptions) (int64, error) {
 	}
 	return 1, nil
 }
+
+func (m *MockPlaylistRepo) Exists(_ string) (bool, error) {
+	if m.Error != nil {
+		return false, m.Error
+	}
+	return m.Entity != nil, nil
+}
+
+func (m *MockPlaylistRepo) SetStar(starred bool, itemIDs ...string) error {
+	if m.Error != nil {
+		return m.Error
+	}
+	m.SetStarWasCalled = true
+	return nil
+}

ui/src/audioplayer/Player.jsx

@@ -127,6 +127,7 @@ const Player = () => {
         />
       ),
       locale: locale(translate),
+      sortableOptions: { delay: 200, delayOnTouchOnly: true },
     }),
     [gainInfo, isDesktop, playerTheme, translate, playerState.mode],
   )

ui/src/layout/PlaylistsSubMenu.jsx

@@ -12,7 +12,7 @@ import QueueMusicOutlinedIcon from '@material-ui/icons/QueueMusicOutlined'
 import { BiCog } from 'react-icons/bi'
 import { useDrop } from 'react-dnd'
 import SubMenu from './SubMenu'
-import { canChangeTracks } from '../common'
+import { canChangeTracks, useResourceRefresh } from '../common'
 import { DraggableTypes } from '../consts'
 import config from '../config'
 
@@ -51,6 +51,7 @@ const PlaylistMenuItemLink = ({ pls, sidebarIsOpen }) => {
 
 const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => {
   const history = useHistory()
+  useResourceRefresh()
   const { data, loaded } = useQueryWithStore({
     type: 'getList',
     resource: 'playlist',
@@ -60,6 +61,7 @@ const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => {
         perPage: config.maxSidebarPlaylists,
       },
       sort: { field: 'name' },
+      filter: config.enableFavourites ? { starred: true } : {},
     },
   })
 

ui/src/playlist/PlaylistDetails.jsx

@@ -10,7 +10,13 @@ import { useTranslate } from 'react-admin'
 import { useCallback, useState, useEffect } from 'react'
 import Lightbox from 'react-image-lightbox'
 import 'react-image-lightbox/style.css'
-import { CollapsibleComment, DurationField, SizeField } from '../common'
+import {
+  CollapsibleComment,
+  DurationField,
+  SizeField,
+  LoveButton,
+} from '../common'
+import config from '../config'
 import subsonic from '../subsonic'
 
 const useStyles = makeStyles(
@@ -68,6 +74,10 @@ const useStyles = makeStyles(
     coverLoading: {
       opacity: 0.5,
     },
+    loveButton: {
+      top: theme.spacing(-0.2),
+      left: theme.spacing(0.5),
+    },
     title: {
       overflow: 'hidden',
       textOverflow: 'ellipsis',
@@ -146,6 +156,16 @@ const PlaylistDetails = (props) => {
               className={classes.title}
             >
               {record.name || translate('ra.page.loading')}
+              {config.enableFavourites && (
+                <LoveButton
+                  className={classes.loveButton}
+                  record={record}
+                  resource={'playlist'}
+                  size={isDesktop ? 'default' : 'small'}
+                  aria-label="love"
+                  color="primary"
+                />
+              )}
             </Typography>
             <Typography component="p" className={classes.stats}>
               {record.songCount ? (