Fix/Optimized Playlist tracks deletion

model/playlist.go

@@ -111,6 +111,6 @@ type PlaylistTrackRepository interface {
 	AddAlbums(albumIds []string) (int, error)
 	AddArtists(artistIds []string) (int, error)
 	AddDiscs(discs []DiscID) (int, error)
-	Delete(id string) error
+	Delete(id ...string) error
 	Reorder(pos int, newPos int) error
 }

persistence/playlist_repository.go

@@ -89,10 +89,6 @@ func (r *playlistRepository) Put(p *model.Playlist) error {
 	}
 	pls.UpdatedAt = time.Now()
 
-	// Save tracks for later and set it to nil, to avoid trying to save it to the DB
-	tracks := pls.Tracks
-	pls.Tracks = nil
-
 	id, err := r.put(pls.ID, pls)
 	if err != nil {
 		return err
@@ -104,7 +100,7 @@ func (r *playlistRepository) Put(p *model.Playlist) error {
 		return nil
 	}
 	// Only update tracks if they were specified
-	if tracks == nil {
+	if len(pls.Tracks) == 0 {
 		return nil
 	}
 	return r.updateTracks(id, p.MediaFiles())
@@ -405,15 +401,24 @@ func (r *playlistRepository) removeOrphans() error {
 		}
 		log.Debug(r.ctx, "Deleted tracks, now reordering", "id", pl.Id, "name", pl.Name, "deleted", n)
 
-		// To reorganize the playlist, just add an empty list of new tracks
-		tracks := r.Tracks(pl.Id)
-		if _, err := tracks.Add(nil); err != nil {
+		// Renumber the playlist if any track was removed
+		if err := r.renumber(pl.Id); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
+func (r *playlistRepository) renumber(id string) error {
+	var ids []string
+	sql := Select("media_file_id").From("playlist_tracks").Where(Eq{"playlist_id": id}).OrderBy("id")
+	err := r.queryAll(sql, &ids)
+	if err != nil {
+		return err
+	}
+	return r.updatePlaylist(id, ids)
+}
+
 func (r *playlistRepository) isWritable(playlistId string) bool {
 	usr := loggedUser(r.ctx)
 	if usr.IsAdmin {

persistence/playlist_track_repository.go

@@ -164,18 +164,16 @@ func (r *playlistTrackRepository) getTracks() ([]string, error) {
 	return ids, nil
 }
 
-func (r *playlistTrackRepository) Delete(id string) error {
+func (r *playlistTrackRepository) Delete(ids ...string) error {
 	if !r.isTracksEditable() {
 		return rest.ErrPermissionDenied
 	}
-	err := r.delete(And{Eq{"playlist_id": r.playlistId}, Eq{"id": id}})
+	err := r.delete(And{Eq{"playlist_id": r.playlistId}, Eq{"id": ids}})
 	if err != nil {
 		return err
 	}
 
-	// To renumber the playlist
-	_, err = r.Add(nil)
-	return err
+	return r.playlistRepo.renumber(r.playlistId)
 }
 
 func (r *playlistTrackRepository) Reorder(pos int, newPos int) error {

server/nativeapi/native_api.go

@@ -87,6 +87,14 @@ func (n *Router) addPlaylistTrackRoute(r chi.Router) {
 		r.Get("/", func(w http.ResponseWriter, r *http.Request) {
 			getPlaylist(n.ds)(w, r)
 		})
+		r.With(urlParams).Route("/", func(r chi.Router) {
+			r.Delete("/", func(w http.ResponseWriter, r *http.Request) {
+				deleteFromPlaylist(n.ds)(w, r)
+			})
+			r.Post("/", func(w http.ResponseWriter, r *http.Request) {
+				addToPlaylist(n.ds)(w, r)
+			})
+		})
 		r.Route("/{id}", func(r chi.Router) {
 			r.Use(urlParams)
 			r.Put("/", func(w http.ResponseWriter, r *http.Request) {
@@ -96,9 +104,6 @@ func (n *Router) addPlaylistTrackRoute(r chi.Router) {
 				deleteFromPlaylist(n.ds)(w, r)
 			})
 		})
-		r.With(urlParams).Post("/", func(w http.ResponseWriter, r *http.Request) {
-			addToPlaylist(n.ds)(w, r)
-		})
 	})
 }
 

server/nativeapi/playlists.go

@@ -84,20 +84,34 @@ func handleExportPlaylist(ds model.DataStore) http.HandlerFunc {
 func deleteFromPlaylist(ds model.DataStore) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		playlistId := utils.ParamString(r, ":playlistId")
-		id := r.URL.Query().Get(":id")
-		tracksRepo := ds.Playlist(r.Context()).Tracks(playlistId)
-		err := tracksRepo.Delete(id)
-		if err == model.ErrNotFound {
-			log.Warn("Track not found in playlist", "playlistId", playlistId, "id", id)
+		ids := r.URL.Query()["id"]
+		err := ds.WithTx(func(tx model.DataStore) error {
+			tracksRepo := tx.Playlist(r.Context()).Tracks(playlistId)
+			return tracksRepo.Delete(ids...)
+		})
+		if len(ids) == 1 && err == model.ErrNotFound {
+			log.Warn(r.Context(), "Track not found in playlist", "playlistId", playlistId, "id", ids[0])
 			http.Error(w, "not found", http.StatusNotFound)
 			return
 		}
 		if err != nil {
-			log.Error("Error deleting track from playlist", "playlistId", playlistId, "id", id, err)
+			log.Error(r.Context(), "Error deleting tracks from playlist", "playlistId", playlistId, "ids", ids, err)
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
-		_, err = w.Write([]byte("{}"))
+		var resp []byte
+		if len(ids) == 1 {
+			resp = []byte(`{"id":"` + ids[0] + `"}`)
+		} else {
+			resp, err = json.Marshal(&struct {
+				Ids []string `json:"ids"`
+			}{Ids: ids})
+			if err != nil {
+				log.Error(r.Context(), "Error marshaling delete response", "playlistId", playlistId, "ids", ids, err)
+				http.Error(w, err.Error(), http.StatusInternalServerError)
+			}
+		}
+		_, err = w.Write(resp)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 		}

server/subsonic/playlists.go

@@ -69,7 +69,6 @@ func (c *PlaylistsController) create(ctx context.Context, playlistId, name strin
 		var pls *model.Playlist
 		var err error
 
-		// If playlistID is present, override tracks
 		if playlistId != "" {
 			pls, err = tx.Playlist(ctx).Get(playlistId)
 			if err != nil {

ui/src/dataProvider/wrapperDataProvider.js

@@ -19,6 +19,14 @@ const mapResource = (resource, params) => {
   }
 }
 
+const callDeleteMany = (resource, params) => {
+  const ids = params.ids.map((id) => `id=${id}`)
+  const idsParam = ids.join('&')
+  return httpClient(`${REST_URL}/${resource}?${idsParam}`, {
+    method: 'DELETE',
+  }).then((response) => ({ data: response.json.ids || [] }))
+}
+
 const wrapperDataProvider = {
   ...dataProvider,
   getList: (resource, params) => {
@@ -55,6 +63,9 @@ const wrapperDataProvider = {
   },
   deleteMany: (resource, params) => {
     const [r, p] = mapResource(resource, params)
+    if (r.endsWith('/tracks')) {
+      return callDeleteMany(r, p)
+    }
     return dataProvider.deleteMany(r, p)
   },
   addToPlaylist: (playlistId, data) => {