From 7e387e989b2d87a7031365ad15f2749f35fb1a46 Mon Sep 17 00:00:00 2001 From: jmt Date: Fri, 2 Oct 2026 09:57:10 +0200 Subject: [PATCH] feat: remove from playlists and tell whether one holds a track Add POST /library/playlists/remove_track, which removes one entry from a playlist the user may edit: a REM change by index range, one entry at the position the client listed it, or else at the first occurrence. Playlists may hold an item more than once, and keying the removal by the item (Rem.items_as_key) would remove every duplicate, so the daemon checks that the position still holds the track and removes that entry only. The removal is based on the playlist's current revision and retried once when it changed; an entry no longer at its position is reported as 409 so the client lists the playlist again. Like appending, it drops the cached listing. Add GET /library/playlists/contains, which tells whether a playlist already holds items, so clients can ask before adding a duplicate. Both read the playlist through the new Spclient.PlaylistContents, which pages through it and reports a change between pages as a conflict. PlaylistAppend and PlaylistRemove now share how a change is sent. Co-Authored-By: Claude Opus 5.5 --- api-spec.yml | 110 +++++++++++++++++++++++++++++++++ daemon/api_gen.go | 110 +++++++++++++++++++++++++++++++++ daemon/api_server.go | 39 +++++++++++- daemon/api_server_test.go | 104 ++++++++++++++++++++++++------- daemon/library.go | 67 ++++++++++++++++++++ daemon/library_test.go | 21 +++++++ daemon/player.go | 30 +++++++++ spclient/library_write.go | 81 ++++++++++++++++++++++-- spclient/library_write_test.go | 56 +++++++++++++++++ 9 files changed, 589 insertions(+), 29 deletions(-) diff --git a/api-spec.yml b/api-spec.yml index 3791a793..74749cc2 100644 --- a/api-spec.yml +++ b/api-spec.yml @@ -366,6 +366,48 @@ components: liked: description: True to add the tracks to Liked Songs, false to remove them type: boolean + playlistRemoveTrack: + x-go-name: ApiPlaylistRemoveTrack + description: A remove from playlist payload + type: object + required: [ playlist_uri, uri ] + properties: + playlist_uri: + description: URI of the playlist to remove from + type: string + uri: + description: URI of the track or episode to remove + type: string + position: + description: >- + Position of the item to remove, as listed by /context/tracks; the + item there must be uri. Without it the first occurrence of uri is + removed. + type: integer + minimum: 0 + playlistContainsState: + x-go-name: ApiPlaylistContainsState + description: Whether a playlist holds an item + type: object + required: [ uri, contained ] + properties: + uri: + description: Track or episode URI + type: string + contained: + description: Whether the playlist holds the item at least once + type: boolean + playlistContains: + x-go-name: ApiPlaylistContains + description: Membership of the requested items in a playlist + type: object + required: [ items ] + properties: + items: + description: One entry per requested URI, in request order + type: array + items: + $ref: '#/components/schemas/playlistContainsState' playlistAddTracks: x-go-name: ApiPlaylistAddTracks description: An append to playlist payload @@ -794,6 +836,74 @@ paths: description: The playlist does not exist 409: description: The playlist kept changing while appending + /library/playlists/remove_track: + post: + operationId: playlistRemoveTrack + description: >- + Remove one item from a playlist the user may edit. Playlists may hold + an item more than once, so clients that list the playlist pass the + position of the entry to remove; the change is based on the + playlist's current revision and retried once if it changed in + between. + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/playlistRemoveTrack' + responses: + 200: + description: Successful response + 204: + description: No active session + 400: + description: The playlist_uri or uri is not valid, or the position is negative + 403: + description: The user may not edit the playlist + 404: + description: The playlist does not exist or does not hold uri + 409: + description: >- + The item at position is no longer uri, or the playlist kept + changing; list the playlist again + /library/playlists/contains: + get: + operationId: playlistContains + description: >- + Tell whether a playlist already holds items, for example to ask + before adding a duplicate. The playlist is read fresh on every call. + parameters: + - name: playlist_uri + in: query + required: true + description: URI of the playlist + schema: + type: string + - name: uris + in: query + required: true + description: Comma-separated track or episode URIs, 1 to 50 + style: form + explode: false + schema: + type: array + items: + type: string + responses: + 200: + description: Successful response + content: + application/json: + schema: + $ref: '#/components/schemas/playlistContains' + 204: + description: No active session + 400: + description: The playlist_uri or the uris are not valid + 403: + description: The user may not read the playlist + 404: + description: The playlist does not exist /player/play: post: operationId: playerPlay diff --git a/daemon/api_gen.go b/daemon/api_gen.go index 968359da..3cfbcda9 100644 --- a/daemon/api_gen.go +++ b/daemon/api_gen.go @@ -161,6 +161,33 @@ type ApiPlaylistAddTracks struct { Uris []string `json:"uris"` } +// ApiPlaylistContains Membership of the requested items in a playlist +type ApiPlaylistContains struct { + // Items One entry per requested URI, in request order + Items []ApiPlaylistContainsState `json:"items"` +} + +// ApiPlaylistContainsState Whether a playlist holds an item +type ApiPlaylistContainsState struct { + // Contained Whether the playlist holds the item at least once + Contained bool `json:"contained"` + + // Uri Track or episode URI + Uri string `json:"uri"` +} + +// ApiPlaylistRemoveTrack A remove from playlist payload +type ApiPlaylistRemoveTrack struct { + // PlaylistUri URI of the playlist to remove from + PlaylistUri string `json:"playlist_uri"` + + // Position Position of the item to remove, as listed by /context/tracks; the item there must be uri. Without it the first occurrence of uri is removed. + Position *int `json:"position,omitempty"` + + // Uri URI of the track or episode to remove + Uri string `json:"uri"` +} + // ApiRepeatContext A toggle repeating context payload type ApiRepeatContext struct { // RepeatContext Whether repeating context should be enabled @@ -366,12 +393,24 @@ type GetLibraryPlaylistsParams struct { Limit int `form:"limit,omitempty" json:"limit,omitempty"` } +// PlaylistContainsParams defines parameters for PlaylistContains. +type PlaylistContainsParams struct { + // PlaylistUri URI of the playlist + PlaylistUri string `form:"playlist_uri" json:"playlist_uri"` + + // Uris Comma-separated track or episode URIs, 1 to 50 + Uris []string `form:"uris" json:"uris"` +} + // SetLikedJSONRequestBody defines body for SetLiked for application/json ContentType. type SetLikedJSONRequestBody = ApiSetLiked // PlaylistAddTracksJSONRequestBody defines body for PlaylistAddTracks for application/json ContentType. type PlaylistAddTracksJSONRequestBody = ApiPlaylistAddTracks +// PlaylistRemoveTrackJSONRequestBody defines body for PlaylistRemoveTrack for application/json ContentType. +type PlaylistRemoveTrackJSONRequestBody = ApiPlaylistRemoveTrack + // PlayerAddToQueueJSONRequestBody defines body for PlayerAddToQueue for application/json ContentType. type PlayerAddToQueueJSONRequestBody = ApiAddToQueue @@ -429,6 +468,12 @@ type ServerInterface interface { // (POST /library/playlists/add_tracks) PlaylistAddTracks(w http.ResponseWriter, r *http.Request) + // (GET /library/playlists/contains) + PlaylistContains(w http.ResponseWriter, r *http.Request, params PlaylistContainsParams) + + // (POST /library/playlists/remove_track) + PlaylistRemoveTrack(w http.ResponseWriter, r *http.Request) + // (POST /player/add_to_queue) PlayerAddToQueue(w http.ResponseWriter, r *http.Request) @@ -666,6 +711,69 @@ func (siw *ServerInterfaceWrapper) PlaylistAddTracks(w http.ResponseWriter, r *h handler.ServeHTTP(w, r) } +// PlaylistContains operation middleware +func (siw *ServerInterfaceWrapper) PlaylistContains(w http.ResponseWriter, r *http.Request) { + + var err error + + // Parameter object where we will unmarshal all parameters from the context + var params PlaylistContainsParams + + // ------------- Required query parameter "playlist_uri" ------------- + + if paramValue := r.URL.Query().Get("playlist_uri"); paramValue != "" { + + } else { + siw.ErrorHandlerFunc(w, r, &RequiredParamError{ParamName: "playlist_uri"}) + return + } + + err = runtime.BindQueryParameter("form", true, true, "playlist_uri", r.URL.Query(), ¶ms.PlaylistUri) + if err != nil { + siw.ErrorHandlerFunc(w, r, &InvalidParamFormatError{ParamName: "playlist_uri", Err: err}) + return + } + + // ------------- Required query parameter "uris" ------------- + + if paramValue := r.URL.Query().Get("uris"); paramValue != "" { + + } else { + siw.ErrorHandlerFunc(w, r, &RequiredParamError{ParamName: "uris"}) + return + } + + err = runtime.BindQueryParameter("form", false, true, "uris", r.URL.Query(), ¶ms.Uris) + if err != nil { + siw.ErrorHandlerFunc(w, r, &InvalidParamFormatError{ParamName: "uris", Err: err}) + return + } + + handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + siw.Handler.PlaylistContains(w, r, params) + })) + + for _, middleware := range siw.HandlerMiddlewares { + handler = middleware(handler) + } + + handler.ServeHTTP(w, r) +} + +// PlaylistRemoveTrack operation middleware +func (siw *ServerInterfaceWrapper) PlaylistRemoveTrack(w http.ResponseWriter, r *http.Request) { + + handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + siw.Handler.PlaylistRemoveTrack(w, r) + })) + + for _, middleware := range siw.HandlerMiddlewares { + handler = middleware(handler) + } + + handler.ServeHTTP(w, r) +} + // PlayerAddToQueue operation middleware func (siw *ServerInterfaceWrapper) PlayerAddToQueue(w http.ResponseWriter, r *http.Request) { @@ -1046,6 +1154,8 @@ func HandlerWithOptions(si ServerInterface, options StdHTTPServerOptions) http.H m.HandleFunc("POST "+options.BaseURL+"/library/liked", wrapper.SetLiked) m.HandleFunc("GET "+options.BaseURL+"/library/playlists", wrapper.GetLibraryPlaylists) m.HandleFunc("POST "+options.BaseURL+"/library/playlists/add_tracks", wrapper.PlaylistAddTracks) + m.HandleFunc("GET "+options.BaseURL+"/library/playlists/contains", wrapper.PlaylistContains) + m.HandleFunc("POST "+options.BaseURL+"/library/playlists/remove_track", wrapper.PlaylistRemoveTrack) m.HandleFunc("POST "+options.BaseURL+"/player/add_to_queue", wrapper.PlayerAddToQueue) m.HandleFunc("POST "+options.BaseURL+"/player/next", wrapper.PlayerNext) m.HandleFunc("POST "+options.BaseURL+"/player/output", wrapper.PlayerOutput) diff --git a/daemon/api_server.go b/daemon/api_server.go index c49805dc..3c868363 100644 --- a/daemon/api_server.go +++ b/daemon/api_server.go @@ -138,6 +138,8 @@ const ( ApiRequestTypeSetLiked ApiRequestType = "set_liked" ApiRequestTypeGetLiked ApiRequestType = "get_liked" ApiRequestTypePlaylistAddTracks ApiRequestType = "playlist_add_tracks" + ApiRequestTypePlaylistRemoveTrack ApiRequestType = "playlist_remove_track" + ApiRequestTypePlaylistContains ApiRequestType = "playlist_contains" ) // ApiRequestDataContextTracks carries the uri query parameter of the context @@ -624,7 +626,7 @@ func (s *ConcreteApiServer) PlaylistAddTracks(w http.ResponseWriter, r *http.Req return } - if id, err := librespot.SpotifyIdFromUri(data.PlaylistUri); err != nil || id.Type() != librespot.SpotifyIdTypePlaylist { + if !validPlaylistUri(data.PlaylistUri) { w.WriteHeader(http.StatusBadRequest) return } @@ -632,6 +634,41 @@ func (s *ConcreteApiServer) PlaylistAddTracks(w http.ResponseWriter, r *http.Req s.handleRequest(ApiRequest{Type: ApiRequestTypePlaylistAddTracks, Data: data}, w) } +// validPlaylistUri reports whether uri names a playlist. +func validPlaylistUri(uri string) bool { + id, err := librespot.SpotifyIdFromUri(uri) + return err == nil && id.Type() == librespot.SpotifyIdTypePlaylist +} + +func (s *ConcreteApiServer) PlaylistRemoveTrack(w http.ResponseWriter, r *http.Request) { + var data ApiPlaylistRemoveTrack + if err := jsonDecode(r, &data); err != nil || + !validPlaylistUri(data.PlaylistUri) || + !validItemUris([]string{data.Uri}, librespot.SpotifyIdTypeTrack, librespot.SpotifyIdTypeEpisode) || + (data.Position != nil && *data.Position < 0) { + w.WriteHeader(http.StatusBadRequest) + return + } + + s.handleRequest(ApiRequest{Type: ApiRequestTypePlaylistRemoveTrack, Data: data}, w) +} + +// ApiRequestDataPlaylistContains carries the query of a contains request. +type ApiRequestDataPlaylistContains struct { + PlaylistUri string + Uris []string +} + +func (s *ConcreteApiServer) PlaylistContains(w http.ResponseWriter, _ *http.Request, params PlaylistContainsParams) { + if !validPlaylistUri(params.PlaylistUri) || + !validItemUris(params.Uris, librespot.SpotifyIdTypeTrack, librespot.SpotifyIdTypeEpisode) { + w.WriteHeader(http.StatusBadRequest) + return + } + + s.handleRequest(ApiRequest{Type: ApiRequestTypePlaylistContains, Data: ApiRequestDataPlaylistContains{PlaylistUri: params.PlaylistUri, Uris: params.Uris}}, w) +} + func (s *ConcreteApiServer) PlayerResume(w http.ResponseWriter, _ *http.Request) { s.handleRequest(ApiRequest{Type: ApiRequestTypeResume}, w) } diff --git a/daemon/api_server_test.go b/daemon/api_server_test.go index 89a7b58e..ba1c7b1a 100644 --- a/daemon/api_server_test.go +++ b/daemon/api_server_test.go @@ -132,29 +132,31 @@ func body(t *testing.T, resp *http.Response) string { // Every endpoint and the methods it accepts. Anything else must be refused // before the daemon is involved. var endpointMethods = map[string][]string{ - "/": {http.MethodGet}, - "/status": {http.MethodGet}, - "/auth/code": {http.MethodGet}, - "/token": {http.MethodPost}, - "/set_device_name": {http.MethodPost}, - "/player/play": {http.MethodPost}, - "/player/resume": {http.MethodPost}, - "/player/pause": {http.MethodPost}, - "/player/playpause": {http.MethodPost}, - "/player/stop": {http.MethodPost}, - "/player/next": {http.MethodPost}, - "/player/prev": {http.MethodPost}, - "/player/seek": {http.MethodPost}, - "/player/volume": {http.MethodGet, http.MethodPost}, - "/player/repeat_context": {http.MethodPost}, - "/player/repeat_track": {http.MethodPost}, - "/player/shuffle_context": {http.MethodPost}, - "/player/add_to_queue": {http.MethodPost}, - "/player/output": {http.MethodPost}, - "/context/tracks": {http.MethodGet}, - "/library/playlists": {http.MethodGet}, - "/library/liked": {http.MethodGet, http.MethodPost}, - "/library/playlists/add_tracks": {http.MethodPost}, + "/": {http.MethodGet}, + "/status": {http.MethodGet}, + "/auth/code": {http.MethodGet}, + "/token": {http.MethodPost}, + "/set_device_name": {http.MethodPost}, + "/player/play": {http.MethodPost}, + "/player/resume": {http.MethodPost}, + "/player/pause": {http.MethodPost}, + "/player/playpause": {http.MethodPost}, + "/player/stop": {http.MethodPost}, + "/player/next": {http.MethodPost}, + "/player/prev": {http.MethodPost}, + "/player/seek": {http.MethodPost}, + "/player/volume": {http.MethodGet, http.MethodPost}, + "/player/repeat_context": {http.MethodPost}, + "/player/repeat_track": {http.MethodPost}, + "/player/shuffle_context": {http.MethodPost}, + "/player/add_to_queue": {http.MethodPost}, + "/player/output": {http.MethodPost}, + "/context/tracks": {http.MethodGet}, + "/library/playlists": {http.MethodGet}, + "/library/liked": {http.MethodGet, http.MethodPost}, + "/library/playlists/add_tracks": {http.MethodPost}, + "/library/playlists/remove_track": {http.MethodPost}, + "/library/playlists/contains": {http.MethodGet}, } func TestApiRejectsWrongMethod(t *testing.T) { @@ -721,6 +723,62 @@ func TestApiPlaylistAddTracks(t *testing.T) { }) } +func TestApiPlaylistRemoveTrack(t *testing.T) { + t.Run("forwards the payload", func(t *testing.T) { + ts := newTestServer(t, okReply) + + resp := ts.do(http.MethodPost, "/library/playlists/remove_track", map[string]any{ + "playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", "uri": "spotify:track:4uLU6hMCjMI75M1A2tKUQC", "position": 3, + }) + require.Equal(t, http.StatusOK, resp.StatusCode) + + data := ts.request().Data.(ApiPlaylistRemoveTrack) + require.NotNil(t, data.Position) + require.Equal(t, 3, *data.Position) + }) + + for name, payload := range map[string]any{ + "album as playlist": map[string]any{"playlist_uri": "spotify:album:37i9dQZF1DXcBWIGoYBM5M", "uri": "spotify:track:4uLU6hMCjMI75M1A2tKUQC"}, + "artist item": map[string]any{"playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", "uri": "spotify:artist:4uLU6hMCjMI75M1A2tKUQC"}, + "negative position": map[string]any{"playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", "uri": "spotify:track:4uLU6hMCjMI75M1A2tKUQC", "position": -1}, + } { + t.Run("rejects "+name, func(t *testing.T) { + ts := newTestServer(t, okReply) + + resp := ts.do(http.MethodPost, "/library/playlists/remove_track", payload) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + ts.requireNoRequest() + }) + } + + t.Run("maps a moved item to 409", func(t *testing.T) { + ts := newTestServer(t, func(ApiRequest) (any, error) { + return nil, fmt.Errorf("failed removing from playlist: %w", errPlaylistItemMoved) + }) + + resp := ts.do(http.MethodPost, "/library/playlists/remove_track", map[string]any{ + "playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", "uri": "spotify:track:4uLU6hMCjMI75M1A2tKUQC", "position": 3, + }) + require.Equal(t, http.StatusConflict, resp.StatusCode) + }) +} + +func TestApiPlaylistContains(t *testing.T) { + ts := newTestServer(t, func(ApiRequest) (any, error) { + return &ApiPlaylistContains{Items: []ApiPlaylistContainsState{{Uri: "spotify:track:4uLU6hMCjMI75M1A2tKUQC", Contained: true}}}, nil + }) + + resp := ts.do(http.MethodGet, "/library/playlists/contains?playlist_uri=spotify:playlist:37i9dQZF1DXcBWIGoYBM5M&uris=spotify:track:4uLU6hMCjMI75M1A2tKUQC", nil) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.JSONEq(t, `{"items":[{"uri":"spotify:track:4uLU6hMCjMI75M1A2tKUQC","contained":true}]}`, body(t, resp)) + + data := ts.request().Data.(ApiRequestDataPlaylistContains) + require.Equal(t, "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", data.PlaylistUri) + + resp = ts.do(http.MethodGet, "/library/playlists/contains?playlist_uri=spotify:album:x&uris=spotify:track:4uLU6hMCjMI75M1A2tKUQC", nil) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) +} + func TestApiSetDeviceName(t *testing.T) { t.Run("forwards the name", func(t *testing.T) { ts := newTestServer(t, okReply) diff --git a/daemon/library.go b/daemon/library.go index e4451306..bb987095 100644 --- a/daemon/library.go +++ b/daemon/library.go @@ -190,3 +190,70 @@ func libraryError(what string, err error) error { } return fmt.Errorf("%s: %w", what, err) } + +// errPlaylistItemMoved reports that the entry a removal names is no longer +// there; the client's listing is out of date. +var errPlaylistItemMoved = fmt.Errorf("%w: the item is no longer at that position", spclient.ErrPlaylistConflict) + +// itemPosition finds the entry to remove: the one at position, which must +// still be uri, or without a position the first occurrence of uri. +func itemPosition(uris []string, uri string, position *int) (int, error) { + if position != nil { + if *position < len(uris) && uris[*position] == uri { + return *position, nil + } + return 0, errPlaylistItemMoved + } + for i, u := range uris { + if u == uri { + return i, nil + } + } + return 0, ErrNotFound +} + +// removeFromPlaylist removes one entry of uri from a playlist. The change +// names the revision it is based on; if the playlist moves on in between, it +// is read again and the removal retried once. +func removeFromPlaylist(ctx context.Context, spc *spclient.Spclient, username, playlistUri, uri string, position *int) error { + id, err := librespot.SpotifyIdFromUri(playlistUri) + if err != nil { + return err + } + + for attempt := 0; ; attempt++ { + revision, uris, err := spc.PlaylistContents(ctx, *id) + if err == nil { + var index int + if index, err = itemPosition(uris, uri, position); err != nil { + return err + } + err = spc.PlaylistRemove(ctx, *id, username, revision, uri, index) + } + if !errors.Is(err, spclient.ErrPlaylistConflict) || attempt > 0 { + return err + } + } +} + +// playlistContains tells for each of uris whether the playlist holds it. +func playlistContains(ctx context.Context, spc *spclient.Spclient, playlistUri string, uris []string) ([]ApiPlaylistContainsState, error) { + id, err := librespot.SpotifyIdFromUri(playlistUri) + if err != nil { + return nil, err + } + _, items, err := spc.PlaylistContents(ctx, *id) + if err != nil { + return nil, err + } + + held := make(map[string]bool, len(items)) + for _, item := range items { + held[item] = true + } + states := make([]ApiPlaylistContainsState, len(uris)) + for i, uri := range uris { + states[i] = ApiPlaylistContainsState{Uri: uri, Contained: held[uri]} + } + return states, nil +} diff --git a/daemon/library_test.go b/daemon/library_test.go index b6ca4718..802ed7f3 100644 --- a/daemon/library_test.go +++ b/daemon/library_test.go @@ -147,3 +147,24 @@ func TestLibraryErrorMapsStatuses(t *testing.T) { require.NotErrorIs(t, other, ErrNotFound) require.ErrorIs(t, libraryError("x", spclient.ErrPlaylistConflict), spclient.ErrPlaylistConflict) } + +func TestItemPosition(t *testing.T) { + uris := []string{"spotify:track:a", "spotify:track:b", "spotify:track:a"} + at := func(i int) *int { return &i } + + got, err := itemPosition(uris, "spotify:track:a", at(2)) + require.NoError(t, err) + require.Equal(t, 2, got, "the named duplicate, not the first one") + + got, err = itemPosition(uris, "spotify:track:a", nil) + require.NoError(t, err) + require.Equal(t, 0, got, "without a position, the first occurrence") + + _, err = itemPosition(uris, "spotify:track:a", at(1)) + require.ErrorIs(t, err, spclient.ErrPlaylistConflict, "another item at the position: the listing is stale") + _, err = itemPosition(uris, "spotify:track:a", at(7)) + require.ErrorIs(t, err, spclient.ErrPlaylistConflict, "a position past the end: the listing is stale") + + _, err = itemPosition(uris, "spotify:track:z", nil) + require.ErrorIs(t, err, ErrNotFound) +} diff --git a/daemon/player.go b/daemon/player.go index 8833642a..e991197e 100644 --- a/daemon/player.go +++ b/daemon/player.go @@ -936,6 +936,36 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { reply.done(&ApiLikedStates{Items: states}, nil) }) + return nil, errReplyDeferred + case ApiRequestTypePlaylistRemoveTrack: + data := req.Data.(ApiPlaylistRemoveTrack) + reply := apiReply(req) + spc, username := p.sess.Spclient(), p.sess.Username() + p.goDetached(libraryRequestTimeout, func(ctx context.Context) { + if err := removeFromPlaylist(ctx, spc, username, data.PlaylistUri, data.Uri, data.Position); err != nil { + reply.done(nil, libraryError("failed removing from playlist", err)) + return + } + + p.app.contextLists.invalidate(data.PlaylistUri) + reply.done(nil, nil) + }) + + return nil, errReplyDeferred + case ApiRequestTypePlaylistContains: + data := req.Data.(ApiRequestDataPlaylistContains) + reply := apiReply(req) + spc := p.sess.Spclient() + p.goDetached(libraryRequestTimeout, func(ctx context.Context) { + states, err := playlistContains(ctx, spc, data.PlaylistUri, data.Uris) + if err != nil { + reply.done(nil, libraryError("failed reading playlist", err)) + return + } + + reply.done(&ApiPlaylistContains{Items: states}, nil) + }) + return nil, errReplyDeferred case ApiRequestTypePlaylistAddTracks: data := req.Data.(ApiPlaylistAddTracks) diff --git a/spclient/library_write.go b/spclient/library_write.go index 1119ad82..7472cfdc 100644 --- a/spclient/library_write.go +++ b/spclient/library_write.go @@ -1,12 +1,14 @@ package spclient import ( + "bytes" "context" "errors" "fmt" "io" "net/http" "net/url" + "strconv" "time" librespot "github.com/devgianlu/go-librespot" @@ -123,6 +125,53 @@ func (c *Spclient) PlaylistRevision(ctx context.Context, playlist librespot.Spot query.Set("from", "0") query.Set("length", "1") + content, err := c.getPlaylist(ctx, playlist, query) + if err != nil { + return nil, err + } + return content.GetRevision(), nil +} + +// playlistContentsPageSize is how many items PlaylistContents reads per page. +const playlistContentsPageSize = 1000 + +// playlistMaxPages caps the pages read for one playlist. +const playlistMaxPages = 50 + +// PlaylistContents returns a playlist's revision and its item URIs in order, +// so items can be found by position. It reports ErrPlaylistConflict when the +// playlist changes between two pages. +func (c *Spclient) PlaylistContents(ctx context.Context, playlist librespot.SpotifyId) ([]byte, []string, error) { + var revision []byte + var uris []string + for page := 0; page < playlistMaxPages; page++ { + query := url.Values{} + query.Set("decorate", "revision") + query.Set("from", strconv.Itoa(len(uris))) + query.Set("length", strconv.Itoa(playlistContentsPageSize)) + + content, err := c.getPlaylist(ctx, playlist, query) + if err != nil { + return nil, nil, err + } + if page == 0 { + revision = content.GetRevision() + } else if !bytes.Equal(revision, content.GetRevision()) { + return nil, nil, ErrPlaylistConflict + } + + items := content.GetContents().GetItems() + for _, item := range items { + uris = append(uris, item.GetUri()) + } + if !content.GetContents().GetTruncated() || len(items) == 0 { + return revision, uris, nil + } + } + return nil, nil, fmt.Errorf("playlist still truncated after %d pages", playlistMaxPages) +} + +func (c *Spclient) getPlaylist(ctx context.Context, playlist librespot.SpotifyId, query url.Values) (*playlist4pb.SelectedListContent, error) { resp, err := c.Request(ctx, "GET", fmt.Sprintf("/playlist/v2/playlist/%s", playlist.Base62()), query, nil, nil) if err != nil { return nil, err @@ -143,8 +192,24 @@ func (c *Spclient) PlaylistRevision(ctx context.Context, playlist librespot.Spot if err := proto.Unmarshal(respBytes, &content); err != nil { return nil, fmt.Errorf("failed unmarshalling SelectedListContent: %w", err) } + return &content, nil +} - return content.GetRevision(), nil +// PlaylistRemove removes the one item at position, which must be uri, as a +// change based on revision. It returns ErrPlaylistConflict when the playlist +// moved on since. +// +// The removal is by index range. Rem.items_as_key would make the items the +// key instead and remove every entry of uri, duplicates included. +func (c *Spclient) PlaylistRemove(ctx context.Context, playlist librespot.SpotifyId, username string, revision []byte, uri string, position int) error { + return c.playlistChange(ctx, playlist, username, revision, &playlist4pb.Op{ + Kind: playlist4pb.Op_REM.Enum(), + Rem: &playlist4pb.Rem{ + FromIndex: proto.Int32(int32(position)), + Length: proto.Int32(1), + Items: []*playlist4pb.Item{{Uri: proto.String(uri)}}, + }, + }) } // PlaylistAppend appends uris to the end of a playlist, as a change based on @@ -155,13 +220,19 @@ func (c *Spclient) PlaylistAppend(ctx context.Context, playlist librespot.Spotif items = append(items, &playlist4pb.Item{Uri: proto.String(uri)}) } + return c.playlistChange(ctx, playlist, username, revision, &playlist4pb.Op{ + Kind: playlist4pb.Op_ADD.Enum(), + Add: &playlist4pb.Add{Items: items, AddLast: proto.Bool(true)}, + }) +} + +// playlistChange sends one operation as a change to a playlist based on +// revision. +func (c *Spclient) playlistChange(ctx context.Context, playlist librespot.SpotifyId, username string, revision []byte, op *playlist4pb.Op) error { body, err := proto.Marshal(&playlist4pb.ListChanges{ BaseRevision: revision, Deltas: []*playlist4pb.Delta{{ - Ops: []*playlist4pb.Op{{ - Kind: playlist4pb.Op_ADD.Enum(), - Add: &playlist4pb.Add{Items: items, AddLast: proto.Bool(true)}, - }}, + Ops: []*playlist4pb.Op{op}, Info: &playlist4pb.ChangeInfo{ User: proto.String(username), Timestamp: proto.Int64(time.Now().UnixMilli()), diff --git a/spclient/library_write_test.go b/spclient/library_write_test.go index c35b933e..ab2d9822 100644 --- a/spclient/library_write_test.go +++ b/spclient/library_write_test.go @@ -148,3 +148,59 @@ func (suite *RequestSuite) TestPlaylistAppendIsSentOnce() { suite.Error(err) suite.Len(suite.requests(), 1) } + +func playlistPage(revision []byte, pos int32, truncated bool, uris ...string) []byte { + items := make([]*playlist4pb.Item, len(uris)) + for i, uri := range uris { + items[i] = &playlist4pb.Item{Uri: proto.String(uri)} + } + body, _ := proto.Marshal(&playlist4pb.SelectedListContent{ + Revision: revision, + Contents: &playlist4pb.ListItems{Pos: proto.Int32(pos), Truncated: proto.Bool(truncated), Items: items}, + }) + return body +} + +func (suite *RequestSuite) TestPlaylistContentsReadsAllPages() { + suite.handler = func(attempt int, w http.ResponseWriter) { + if attempt == 0 { + _, _ = w.Write(playlistPage([]byte{1}, 0, true, "spotify:track:a", "spotify:track:b")) + return + } + _, _ = w.Write(playlistPage([]byte{1}, 2, false, "spotify:track:c")) + } + + revision, uris, err := suite.spclient.PlaylistContents(suite.T().Context(), testPlaylistId(suite)) + suite.Require().NoError(err) + suite.Equal([]byte{1}, revision) + suite.Equal([]string{"spotify:track:a", "spotify:track:b", "spotify:track:c"}, uris) + + got := suite.requests() + suite.Require().Len(got, 2) + suite.Equal("0", got[0].query.Get("from")) + suite.Equal("2", got[1].query.Get("from"), "the next page starts after the items read") +} + +func (suite *RequestSuite) TestPlaylistContentsReportsAChangeBetweenPages() { + suite.handler = func(attempt int, w http.ResponseWriter) { + _, _ = w.Write(playlistPage([]byte{byte(attempt)}, int32(attempt), attempt == 0, "spotify:track:a")) + } + + _, _, err := suite.spclient.PlaylistContents(suite.T().Context(), testPlaylistId(suite)) + suite.ErrorIs(err, spclient.ErrPlaylistConflict) +} + +func (suite *RequestSuite) TestPlaylistRemoveSendsAnIndexRange() { + err := suite.spclient.PlaylistRemove(suite.T().Context(), testPlaylistId(suite), "user", []byte{9}, "spotify:track:b", 4) + suite.Require().NoError(err) + + var changes playlist4pb.ListChanges + suite.Require().NoError(proto.Unmarshal(suite.requests()[0].body, &changes)) + suite.Equal([]byte{9}, changes.GetBaseRevision()) + op := changes.GetDeltas()[0].GetOps()[0] + suite.Equal(playlist4pb.Op_REM, op.GetKind()) + suite.Equal(int32(4), op.GetRem().GetFromIndex()) + suite.Equal(int32(1), op.GetRem().GetLength()) + suite.False(op.GetRem().GetItemsAsKey(), "keyed by the item, Spotify would remove every duplicate of it") + suite.Equal("spotify:track:b", op.GetRem().GetItems()[0].GetUri()) +}