Optimize playlist updates

cmd/wire_gen.go

@@ -55,8 +55,9 @@ func CreateSubsonicAPIRouter() *subsonic.Router {
 	externalMetadata := core.NewExternalMetadata(dataStore, agentsAgents)
 	scanner := GetScanner()
 	broker := events.GetBroker()
+	playlists := core.NewPlaylists(dataStore)
 	playTracker := scrobbler.GetPlayTracker(dataStore, broker)
-	router := subsonic.New(dataStore, artwork, mediaStreamer, archiver, players, externalMetadata, scanner, broker, playTracker)
+	router := subsonic.New(dataStore, artwork, mediaStreamer, archiver, players, externalMetadata, scanner, broker, playlists, playTracker)
 	return router
 }
 

core/playlists.go

@@ -21,6 +21,7 @@ import (
 
 type Playlists interface {
 	ImportFile(ctx context.Context, dir string, fname string) (*model.Playlist, error)
+	Update(ctx context.Context, playlistId string, name *string, comment *string, public *bool, idsToAdd []string, idxToRemove []int) error
 }
 
 type playlists struct {
@@ -184,3 +185,49 @@ func scanLines(data []byte, atEOF bool) (advance int, token []byte, err error) {
 	// Request more data.
 	return 0, nil, nil
 }
+
+func (s *playlists) Update(ctx context.Context, playlistId string,
+	name *string, comment *string, public *bool,
+	idsToAdd []string, idxToRemove []int) error {
+
+	needsInfoUpdate := name != nil || comment != nil || public != nil
+	needsTrackRefresh := len(idxToRemove) > 0
+
+	return s.ds.WithTx(func(tx model.DataStore) error {
+		var pls *model.Playlist
+		var err error
+		repo := tx.Playlist(ctx)
+		if needsTrackRefresh {
+			pls, err = repo.GetWithTracks(playlistId)
+			pls.RemoveTracks(idxToRemove)
+			pls.AddTracks(idsToAdd)
+		} else {
+			if len(idsToAdd) > 0 {
+				_, err = repo.Tracks(playlistId).Add(idsToAdd)
+				if err != nil {
+					return err
+				}
+			}
+			if needsInfoUpdate {
+				pls, err = repo.Get(playlistId)
+			}
+		}
+		if err != nil {
+			return err
+		}
+		if !needsTrackRefresh && !needsInfoUpdate {
+			return nil
+		}
+
+		if name != nil {
+			pls.Name = *name
+		}
+		if comment != nil {
+			pls.Comment = *comment
+		}
+		if public != nil {
+			pls.Public = *public
+		}
+		return repo.Put(pls)
+	})
+}

model/playlist.go

@@ -90,6 +90,7 @@ type PlaylistRepository interface {
 	GetWithTracks(id string) (*Playlist, error)
 	GetAll(options ...QueryOptions) (Playlists, error)
 	FindByPath(path string) (*Playlist, error)
+	RefreshStatus(playlistId string) error
 	Delete(id string) error
 	Tracks(playlistId string) PlaylistTrackRepository
 }

persistence/playlist_repository.go

@@ -218,7 +218,7 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
 	}
 
 	// Update playlist stats
-	err = r.updateStats(pls.ID)
+	err = r.RefreshStatus(pls.ID)
 	if err != nil {
 		log.Error(r.ctx, "Error updating smart playlist stats", "playlist", pls.Name, "id", pls.ID, err)
 		return false
@@ -268,28 +268,32 @@ func (r *playlistRepository) updatePlaylist(playlistId string, mediaFileIds []st
 		return err
 	}
 
+	return r.addTracks(playlistId, 1, mediaFileIds)
+}
+
+func (r *playlistRepository) addTracks(playlistId string, startingPos int, mediaFileIds []string) error {
 	// Break the track list in chunks to avoid hitting SQLITE_MAX_FUNCTION_ARG limit
-	chunks := utils.BreakUpStringSlice(mediaFileIds, 100)
+	chunks := utils.BreakUpStringSlice(mediaFileIds, 200)
 
 	// Add new tracks, chunk by chunk
-	pos := 1
+	pos := startingPos
 	for i := range chunks {
 		ins := Insert("playlist_tracks").Columns("playlist_id", "media_file_id", "id")
 		for _, t := range chunks[i] {
 			ins = ins.Values(playlistId, t, pos)
 			pos++
 		}
-		_, err = r.executeSQL(ins)
+		_, err := r.executeSQL(ins)
 		if err != nil {
 			return err
 		}
 	}
 
-	return r.updateStats(playlistId)
+	return r.RefreshStatus(playlistId)
 }
 
-// updateStats updates total playlist duration, size and count
+// RefreshStatus updates total playlist duration, size and count
-func (r *playlistRepository) updateStats(playlistId string) error {
+func (r *playlistRepository) RefreshStatus(playlistId string) error {
 	statsSql := Select("sum(duration) as duration", "sum(size) as size", "count(*) as count").
 		From("media_file").
 		Join("playlist_tracks f on f.media_file_id = media_file.id").

persistence/playlist_track_repository.go

@@ -92,18 +92,19 @@ func (r *playlistTrackRepository) Add(mediaFileIds []string) (int, error) {
 
 	if len(mediaFileIds) > 0 {
 		log.Debug(r.ctx, "Adding songs to playlist", "playlistId", r.playlistId, "mediaFileIds", mediaFileIds)
+	} else {
+		return 0, nil
 	}
 
-	ids, err := r.getTracks()
+	// Get next pos (ID) in playlist
+	sql := r.newSelect().Columns("max(id) as max").Where(Eq{"playlist_id": r.playlistId})
+	var res struct{ Max int }
+	err := r.queryOne(sql, &res)
 	if err != nil {
 		return 0, err
 	}
 
-	// Append new tracks
+	return len(mediaFileIds), r.playlistRepo.addTracks(r.playlistId, res.Max+1, mediaFileIds)
-	ids = append(ids, mediaFileIds...)
-
-	// Update tracks and playlist
-	return len(mediaFileIds), r.playlistRepo.updatePlaylist(r.playlistId, ids)
 }
 
 func (r *playlistTrackRepository) AddAlbums(albumIds []string) (int, error) {

server/subsonic/api.go

@@ -32,13 +32,15 @@ type Router struct {
 	Archiver         core.Archiver
 	Players          core.Players
 	ExternalMetadata core.ExternalMetadata
+	Playlists        core.Playlists
 	Scanner          scanner.Scanner
 	Broker           events.Broker
 	Scrobbler        scrobbler.PlayTracker
 }
 
-func New(ds model.DataStore, artwork core.Artwork, streamer core.MediaStreamer, archiver core.Archiver, players core.Players,
+func New(ds model.DataStore, artwork core.Artwork, streamer core.MediaStreamer, archiver core.Archiver,
-	externalMetadata core.ExternalMetadata, scanner scanner.Scanner, broker events.Broker, scrobbler scrobbler.PlayTracker) *Router {
+	players core.Players, externalMetadata core.ExternalMetadata, scanner scanner.Scanner, broker events.Broker,
+	playlists core.Playlists, scrobbler scrobbler.PlayTracker) *Router {
 	r := &Router{
 		DataStore:        ds,
 		Artwork:          artwork,
@@ -46,6 +48,7 @@ func New(ds model.DataStore, artwork core.Artwork, streamer core.MediaStreamer,
 		Archiver:         archiver,
 		Players:          players,
 		ExternalMetadata: externalMetadata,
+		Playlists:        playlists,
 		Scanner:          scanner,
 		Broker:           broker,
 		Scrobbler:        scrobbler,

server/subsonic/playlists.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/navidrome/navidrome/core"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/server/subsonic/responses"
@@ -13,11 +14,12 @@ import (
 )
 
 type PlaylistsController struct {
-	ds model.DataStore
+	ds  model.DataStore
+	pls core.Playlists
 }
 
-func NewPlaylistsController(ds model.DataStore) *PlaylistsController {
+func NewPlaylistsController(ds model.DataStore, pls core.Playlists) *PlaylistsController {
-	return &PlaylistsController{ds: ds}
+	return &PlaylistsController{ds: ds, pls: pls}
 }
 
 func (c *PlaylistsController) GetPlaylists(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
@@ -124,30 +126,6 @@ func (c *PlaylistsController) DeletePlaylist(w http.ResponseWriter, r *http.Requ
 	return newResponse(), nil
 }
 
-func (c *PlaylistsController) update(ctx context.Context, playlistId string, name *string, comment *string, public *bool, idsToAdd []string, idxToRemove []int) error {
-	return c.ds.WithTx(func(tx model.DataStore) error {
-		pls, err := tx.Playlist(ctx).GetWithTracks(playlistId)
-		if err != nil {
-			return err
-		}
-
-		if name != nil {
-			pls.Name = *name
-		}
-		if comment != nil {
-			pls.Comment = *comment
-		}
-		if public != nil {
-			pls.Public = *public
-		}
-
-		pls.RemoveTracks(idxToRemove)
-		pls.AddTracks(idsToAdd)
-
-		return tx.Playlist(ctx).Put(pls)
-	})
-}
-
 func (c *PlaylistsController) UpdatePlaylist(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
 	playlistId, err := requiredParamString(r, "playlistId")
 	if err != nil {
@@ -176,7 +154,7 @@ func (c *PlaylistsController) UpdatePlaylist(w http.ResponseWriter, r *http.Requ
 	log.Trace(r, fmt.Sprintf("-- Adding: '%v'", songsToAdd))
 	log.Trace(r, fmt.Sprintf("-- Removing: '%v'", songIndexesToRemove))
 
-	err = c.update(r.Context(), playlistId, plsName, comment, public, songsToAdd, songIndexesToRemove)
+	err = c.pls.Update(r.Context(), playlistId, plsName, comment, public, songsToAdd, songIndexesToRemove)
 	if err == model.ErrNotAuthorized {
 		return nil, newError(responses.ErrorAuthorizationFail)
 	}

server/subsonic/wire_gen.go

@@ -41,7 +41,8 @@ func initMediaAnnotationController(router *Router) *MediaAnnotationController {
 
 func initPlaylistsController(router *Router) *PlaylistsController {
 	dataStore := router.DataStore
-	playlistsController := NewPlaylistsController(dataStore)
+	playlists := router.Playlists
+	playlistsController := NewPlaylistsController(dataStore, playlists)
 	return playlistsController
 }
 
@@ -106,5 +107,6 @@ var allProviders = wire.NewSet(
 		"Scanner",
 		"Broker",
 		"Scrobbler",
+		"Playlists",
 	),
 )

server/subsonic/wire_injectors.go

@@ -29,6 +29,7 @@ var allProviders = wire.NewSet(
 		"Scanner",
 		"Broker",
 		"Scrobbler",
+		"Playlists",
 	),
 )