Reduce number of queries for some playlists operations.

Also allow admins to update/delete playlists from other users in the Subsonic API. Closes #1366

core/archiver.go

@@ -54,7 +54,7 @@ func (a *archiver) ZipArtist(ctx context.Context, id string, out io.Writer) erro
 }
 
 func (a *archiver) ZipPlaylist(ctx context.Context, id string, out io.Writer) error {
-	pls, err := a.ds.Playlist(ctx).Get(id)
+	pls, err := a.ds.Playlist(ctx).GetWithTracks(id)
 	if err != nil {
 		log.Error(ctx, "Error loading mediafiles from playlist", "id", id, err)
 		return err

model/playlist.go

@@ -31,9 +31,9 @@ type PlaylistRepository interface {
 	Exists(id string) (bool, error)
 	Put(pls *Playlist) error
 	Get(id string) (*Playlist, error)
+	GetWithTracks(id string) (*Playlist, error)
 	GetAll(options ...QueryOptions) (Playlists, error)
 	FindByPath(path string) (*Playlist, error)
-	FindByID(id string) (*Playlist, error)
 	Delete(id string) error
 	Tracks(playlistId string) PlaylistTrackRepository
 }
@@ -49,6 +49,7 @@ type PlaylistTracks []PlaylistTrack
 
 type PlaylistTrackRepository interface {
 	ResourceRepository
+	GetAll(options ...QueryOptions) (PlaylistTracks, error)
 	Add(mediaFileIds []string) (int, error)
 	AddAlbums(albumIds []string) (int, error)
 	AddArtists(artistIds []string) (int, error)

persistence/playlist_repository.go

@@ -105,6 +105,10 @@ func (r *playlistRepository) Put(p *model.Playlist) error {
 }
 
 func (r *playlistRepository) Get(id string) (*model.Playlist, error) {
+	return r.findBy(And{Eq{"id": id}, r.userFilter()}, false)
+}
+
+func (r *playlistRepository) GetWithTracks(id string) (*model.Playlist, error) {
 	return r.findBy(And{Eq{"id": id}, r.userFilter()}, true)
 }
 
@@ -112,10 +116,6 @@ func (r *playlistRepository) FindByPath(path string) (*model.Playlist, error) {
 	return r.findBy(Eq{"path": path}, false)
 }
 
-func (r *playlistRepository) FindByID(id string) (*model.Playlist, error) {
-	return r.findBy(And{Eq{"id": id}, r.userFilter()}, false)
-}
-
 func (r *playlistRepository) findBy(sql Sqlizer, includeTracks bool) (*model.Playlist, error) {
 	sel := r.newSelect().Columns("*").Where(sql)
 	var pls []dbPlaylist

persistence/playlist_repository_test.go

@@ -55,7 +55,7 @@ var _ = Describe("PlaylistRepository", func() {
 			Expect(err).To(MatchError(model.ErrNotFound))
 		})
 		It("returns all tracks", func() {
-			pls, err := repo.Get(plsBest.ID)
+			pls, err := repo.GetWithTracks(plsBest.ID)
 			Expect(err).To(BeNil())
 			Expect(pls.Name).To(Equal(plsBest.Name))
 			Expect(pls.Tracks).To(Equal(model.MediaFiles{
@@ -76,7 +76,7 @@ var _ = Describe("PlaylistRepository", func() {
 		By("adds repeated songs to a playlist and keeps the order")
 		newPls.Tracks = append(newPls.Tracks, model.MediaFile{ID: "1004"})
 		Expect(repo.Put(&newPls)).To(BeNil())
-		saved, _ := repo.Get(newPls.ID)
+		saved, _ := repo.GetWithTracks(newPls.ID)
 		Expect(saved.Tracks).To(HaveLen(3))
 		Expect(saved.Tracks[0].ID).To(Equal("1004"))
 		Expect(saved.Tracks[1].ID).To(Equal("1003"))

persistence/playlist_track_repository.go

@@ -48,8 +48,8 @@ func (r *playlistTrackRepository) Read(id string) (interface{}, error) {
 	return &trk, err
 }
 
-func (r *playlistTrackRepository) ReadAll(options ...rest.QueryOptions) (interface{}, error) {
+func (r *playlistTrackRepository) GetAll(options ...model.QueryOptions) (model.PlaylistTracks, error) {
-	sel := r.newSelect(r.parseRestOptions(options...)).
+	sel := r.newSelect(options...).
 		LeftJoin("annotation on ("+
 			"annotation.item_id = media_file_id"+
 			" AND annotation.item_type = 'media_file'"+
@@ -62,6 +62,10 @@ func (r *playlistTrackRepository) ReadAll(options ...rest.QueryOptions) (interfa
 	return res, err
 }
 
+func (r *playlistTrackRepository) ReadAll(options ...rest.QueryOptions) (interface{}, error) {
+	return r.GetAll(r.parseRestOptions(options...))
+}
+
 func (r *playlistTrackRepository) EntityName() string {
 	return "playlist_tracks"
 }
@@ -211,7 +215,10 @@ func (r *playlistTrackRepository) Delete(id string) error {
 	if err != nil {
 		return err
 	}
-	return r.updateStats()
+
+	// To renumber the playlist
+	_, err = r.Add(nil)
+	return err
 }
 
 func (r *playlistTrackRepository) Reorder(pos int, newPos int) error {
@@ -231,7 +238,7 @@ func (r *playlistTrackRepository) isWritable() bool {
 	if usr.IsAdmin {
 		return true
 	}
-	pls, err := r.playlistRepo.FindByID(r.playlistId)
+	pls, err := r.playlistRepo.Get(r.playlistId)
 	return err == nil && pls.Owner == usr.UserName
 }
 

server/nativeapi/playlists.go

@@ -46,7 +46,7 @@ func handleExportPlaylist(ds model.DataStore) http.HandlerFunc {
 		ctx := r.Context()
 		plsRepo := ds.Playlist(ctx)
 		plsId := chi.URLParam(r, "playlistId")
-		pls, err := plsRepo.Get(plsId)
+		pls, err := plsRepo.GetWithTracks(plsId)
 		if err == model.ErrNotFound {
 			log.Warn("Playlist not found", "playlistId", plsId)
 			http.Error(w, "not found", http.StatusNotFound)
@@ -114,13 +114,13 @@ func addToPlaylist(ds model.DataStore) http.HandlerFunc {
 
 	return func(w http.ResponseWriter, r *http.Request) {
 		playlistId := utils.ParamString(r, ":playlistId")
-		tracksRepo := ds.Playlist(r.Context()).Tracks(playlistId)
 		var payload addTracksPayload
 		err := json.NewDecoder(r.Body).Decode(&payload)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
+		tracksRepo := ds.Playlist(r.Context()).Tracks(playlistId)
 		count, c := 0, 0
 		if c, err = tracksRepo.Add(payload.Ids); err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
@@ -163,7 +163,6 @@ func reorderItem(ds model.DataStore) http.HandlerFunc {
 			http.Error(w, "invalid id", http.StatusBadRequest)
 			return
 		}
-		tracksRepo := ds.Playlist(r.Context()).Tracks(playlistId)
 		var payload reorderPayload
 		err := json.NewDecoder(r.Body).Decode(&payload)
 		if err != nil {
@@ -175,6 +174,7 @@ func reorderItem(ds model.DataStore) http.HandlerFunc {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
+		tracksRepo := ds.Playlist(r.Context()).Tracks(playlistId)
 		err = tracksRepo.Reorder(id, newPos)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)

server/subsonic/playlists.go

@@ -46,7 +46,7 @@ func (c *PlaylistsController) GetPlaylist(w http.ResponseWriter, r *http.Request
 }
 
 func (c *PlaylistsController) getPlaylist(ctx context.Context, id string) (*responses.Subsonic, error) {
-	pls, err := c.ds.Playlist(ctx).Get(id)
+	pls, err := c.ds.Playlist(ctx).GetWithTracks(id)
 	switch {
 	case err == model.ErrNotFound:
 		log.Error(ctx, err.Error(), "id", id)
@@ -110,27 +110,12 @@ func (c *PlaylistsController) CreatePlaylist(w http.ResponseWriter, r *http.Requ
 	return c.getPlaylist(ctx, id)
 }
 
-func (c *PlaylistsController) delete(ctx context.Context, playlistId string) error {
-	return c.ds.WithTx(func(tx model.DataStore) error {
-		pls, err := tx.Playlist(ctx).Get(playlistId)
-		if err != nil {
-			return err
-		}
-
-		owner := getUser(ctx)
-		if owner != pls.Owner {
-			return model.ErrNotAuthorized
-		}
-		return tx.Playlist(ctx).Delete(playlistId)
-	})
-}
-
 func (c *PlaylistsController) DeletePlaylist(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
 	id, err := requiredParamString(r, "id")
 	if err != nil {
 		return nil, err
 	}
-	err = c.delete(r.Context(), id)
+	err = c.ds.Playlist(r.Context()).Delete(id)
 	if err == model.ErrNotAuthorized {
 		return nil, newError(responses.ErrorAuthorizationFail)
 	}
@@ -143,16 +128,11 @@ func (c *PlaylistsController) DeletePlaylist(w http.ResponseWriter, r *http.Requ
 
 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).Get(playlistId)
+		pls, err := tx.Playlist(ctx).GetWithTracks(playlistId)
 		if err != nil {
 			return err
 		}
 
-		owner := getUser(ctx)
-		if owner != pls.Owner {
-			return model.ErrNotAuthorized
-		}
-
 		if name != nil {
 			pls.Name = *name
 		}