diff --git a/api-spec.yml b/api-spec.yml index ffbda67b..3791a793 100644 --- a/api-spec.yml +++ b/api-spec.yml @@ -270,6 +270,7 @@ components: - length - image_url - collaborative + - can_edit - folder properties: uri: @@ -297,6 +298,11 @@ components: collaborative: description: Whether the playlist is collaborative type: boolean + can_edit: + description: >- + Whether the user may add items to the playlist, true for their own + and for collaborative playlists + type: boolean folder: description: >- Names of the folders containing the playlist, outermost first; @@ -323,6 +329,57 @@ components: type: array items: $ref: '#/components/schemas/libraryPlaylist' + likedState: + x-go-name: ApiLikedState + description: Whether a track is in the user's Liked Songs + type: object + required: [ uri, liked ] + properties: + uri: + description: Track URI + type: string + liked: + description: Whether the track is in Liked Songs + type: boolean + likedStates: + x-go-name: ApiLikedStates + description: Liked Songs membership of the requested tracks + type: object + required: [ items ] + properties: + items: + description: One entry per requested URI, in request order + type: array + items: + $ref: '#/components/schemas/likedState' + setLiked: + x-go-name: ApiSetLiked + description: A save to or remove from Liked Songs payload + type: object + required: [ uris, liked ] + properties: + uris: + description: Track URIs, 1 to 50 + type: array + items: + type: string + liked: + description: True to add the tracks to Liked Songs, false to remove them + type: boolean + playlistAddTracks: + x-go-name: ApiPlaylistAddTracks + description: An append to playlist payload + type: object + required: [ playlist_uri, uris ] + properties: + playlist_uri: + description: URI of the playlist to append to + type: string + uris: + description: Track or episode URIs to append, 1 to 50 + type: array + items: + type: string root: x-go-name: ApiRoot description: API reachability and playback readiness @@ -597,9 +654,10 @@ paths: Tracks and episodes are both listed, so shows enumerate to their episodes. Long contexts are truncated to metadata.max_tracks. Listings - are reused for five minutes, so an edit made to a playlist within that - window is not reflected. Requires metadata.enabled; responds 404 - otherwise. + are reused for five minutes, so an edit made elsewhere within that + window is not reflected; edits made through /library/liked and + /library/playlists/add_tracks are. Requires metadata.enabled; responds + 404 otherwise. parameters: - name: uri in: query @@ -660,6 +718,82 @@ paths: description: No active session 400: description: The offset or limit is out of range + /library/liked: + get: + operationId: getLiked + description: >- + Tell whether tracks are in the user's Liked Songs, asking the + collection service for just these tracks, so changes made on other + devices show at once. + parameters: + - name: uris + in: query + required: true + description: Comma-separated track 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/likedStates' + 204: + description: No active session + 400: + description: The uris are missing, too many, or not track URIs + post: + operationId: setLiked + description: >- + Save tracks to, or remove them from, the user's Liked Songs through the + internal collection service. + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/setLiked' + responses: + 200: + description: Successful response + 204: + description: No active session + 400: + description: The uris are missing, too many, or not track URIs + /library/playlists/add_tracks: + post: + operationId: playlistAddTracks + description: >- + Append tracks or episodes to the end of a playlist the user may edit + (see can_edit in /library/playlists). The change is based on the + playlist's current revision and retried once if the playlist changed + in between. + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/playlistAddTracks' + responses: + 200: + description: Successful response + 204: + description: No active session + 400: + description: >- + The playlist_uri is not a playlist URI, or the uris are missing, + too many, or not track or episode URIs + 403: + description: The user may not edit the playlist + 404: + description: The playlist does not exist + 409: + description: The playlist kept changing while appending /player/play: post: operationId: playerPlay diff --git a/daemon/api_gen.go b/daemon/api_gen.go index 2c995d77..968359da 100644 --- a/daemon/api_gen.go +++ b/daemon/api_gen.go @@ -68,6 +68,9 @@ type ApiDeviceAuth struct { // ApiLibraryPlaylist A playlist in the user's library type ApiLibraryPlaylist struct { + // CanEdit Whether the user may add items to the playlist, true for their own and for collaborative playlists + CanEdit bool `json:"can_edit"` + // Collaborative Whether the playlist is collaborative Collaborative bool `json:"collaborative"` @@ -107,6 +110,21 @@ type ApiLibraryPlaylists struct { Total int `json:"total"` } +// ApiLikedState Whether a track is in the user's Liked Songs +type ApiLikedState struct { + // Liked Whether the track is in Liked Songs + Liked bool `json:"liked"` + + // Uri Track URI + Uri string `json:"uri"` +} + +// ApiLikedStates Liked Songs membership of the requested tracks +type ApiLikedStates struct { + // Items One entry per requested URI, in request order + Items []ApiLikedState `json:"items"` +} + // ApiNext A skip to next payload type ApiNext struct { // Uri The track URI to skip to. When omitted the next track in the context is played. @@ -134,6 +152,15 @@ type ApiPlay struct { Uri string `json:"uri"` } +// ApiPlaylistAddTracks An append to playlist payload +type ApiPlaylistAddTracks struct { + // PlaylistUri URI of the playlist to append to + PlaylistUri string `json:"playlist_uri"` + + // Uris Track or episode URIs to append, 1 to 50 + Uris []string `json:"uris"` +} + // ApiRepeatContext A toggle repeating context payload type ApiRepeatContext struct { // RepeatContext Whether repeating context should be enabled @@ -167,6 +194,15 @@ type ApiSetDeviceName struct { Name string `json:"name"` } +// ApiSetLiked A save to or remove from Liked Songs payload +type ApiSetLiked struct { + // Liked True to add the tracks to Liked Songs, false to remove them + Liked bool `json:"liked"` + + // Uris Track URIs, 1 to 50 + Uris []string `json:"uris"` +} + // ApiSetVolume A set volume payload type ApiSetVolume struct { // Relative Whether to change the volume relative to the current volume @@ -315,6 +351,12 @@ type GetContextTracksParams struct { Uri string `form:"uri" json:"uri"` } +// GetLikedParams defines parameters for GetLiked. +type GetLikedParams struct { + // Uris Comma-separated track URIs, 1 to 50 + Uris []string `form:"uris" json:"uris"` +} + // GetLibraryPlaylistsParams defines parameters for GetLibraryPlaylists. type GetLibraryPlaylistsParams struct { // Offset Index of the first playlist to return @@ -324,6 +366,12 @@ type GetLibraryPlaylistsParams struct { Limit int `form:"limit,omitempty" json:"limit,omitempty"` } +// SetLikedJSONRequestBody defines body for SetLiked for application/json ContentType. +type SetLikedJSONRequestBody = ApiSetLiked + +// PlaylistAddTracksJSONRequestBody defines body for PlaylistAddTracks for application/json ContentType. +type PlaylistAddTracksJSONRequestBody = ApiPlaylistAddTracks + // PlayerAddToQueueJSONRequestBody defines body for PlayerAddToQueue for application/json ContentType. type PlayerAddToQueueJSONRequestBody = ApiAddToQueue @@ -369,9 +417,18 @@ type ServerInterface interface { // (GET /events) GetEvents(w http.ResponseWriter, r *http.Request) + // (GET /library/liked) + GetLiked(w http.ResponseWriter, r *http.Request, params GetLikedParams) + + // (POST /library/liked) + SetLiked(w http.ResponseWriter, r *http.Request) + // (GET /library/playlists) GetLibraryPlaylists(w http.ResponseWriter, r *http.Request, params GetLibraryPlaylistsParams) + // (POST /library/playlists/add_tracks) + PlaylistAddTracks(w http.ResponseWriter, r *http.Request) + // (POST /player/add_to_queue) PlayerAddToQueue(w http.ResponseWriter, r *http.Request) @@ -512,6 +569,54 @@ func (siw *ServerInterfaceWrapper) GetEvents(w http.ResponseWriter, r *http.Requ handler.ServeHTTP(w, r) } +// GetLiked operation middleware +func (siw *ServerInterfaceWrapper) GetLiked(w http.ResponseWriter, r *http.Request) { + + var err error + + // Parameter object where we will unmarshal all parameters from the context + var params GetLikedParams + + // ------------- 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.GetLiked(w, r, params) + })) + + for _, middleware := range siw.HandlerMiddlewares { + handler = middleware(handler) + } + + handler.ServeHTTP(w, r) +} + +// SetLiked operation middleware +func (siw *ServerInterfaceWrapper) SetLiked(w http.ResponseWriter, r *http.Request) { + + handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + siw.Handler.SetLiked(w, r) + })) + + for _, middleware := range siw.HandlerMiddlewares { + handler = middleware(handler) + } + + handler.ServeHTTP(w, r) +} + // GetLibraryPlaylists operation middleware func (siw *ServerInterfaceWrapper) GetLibraryPlaylists(w http.ResponseWriter, r *http.Request) { @@ -547,6 +652,20 @@ func (siw *ServerInterfaceWrapper) GetLibraryPlaylists(w http.ResponseWriter, r handler.ServeHTTP(w, r) } +// PlaylistAddTracks operation middleware +func (siw *ServerInterfaceWrapper) PlaylistAddTracks(w http.ResponseWriter, r *http.Request) { + + handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + siw.Handler.PlaylistAddTracks(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) { @@ -923,7 +1042,10 @@ func HandlerWithOptions(si ServerInterface, options StdHTTPServerOptions) http.H m.HandleFunc("GET "+options.BaseURL+"/auth/code", wrapper.GetAuthCode) m.HandleFunc("GET "+options.BaseURL+"/context/tracks", wrapper.GetContextTracks) m.HandleFunc("GET "+options.BaseURL+"/events", wrapper.GetEvents) + m.HandleFunc("GET "+options.BaseURL+"/library/liked", wrapper.GetLiked) + 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("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 363acf86..c49805dc 100644 --- a/daemon/api_server.go +++ b/daemon/api_server.go @@ -8,6 +8,7 @@ import ( "io" "net" "net/http" + "slices" "strings" "sync" "sync/atomic" @@ -18,6 +19,7 @@ import ( librespot "github.com/devgianlu/go-librespot" "github.com/devgianlu/go-librespot/player" metadatapb "github.com/devgianlu/go-librespot/proto/spotify/metadata" + "github.com/devgianlu/go-librespot/spclient" "github.com/rs/cors" ) @@ -133,6 +135,9 @@ const ( ApiRequestTypeReopenOutput ApiRequestType = "reopen_output" ApiRequestTypeContextTracks ApiRequestType = "context_tracks" ApiRequestTypeLibraryPlaylists ApiRequestType = "library_playlists" + ApiRequestTypeSetLiked ApiRequestType = "set_liked" + ApiRequestTypeGetLiked ApiRequestType = "get_liked" + ApiRequestTypePlaylistAddTracks ApiRequestType = "playlist_add_tracks" ) // ApiRequestDataContextTracks carries the uri query parameter of the context @@ -478,7 +483,7 @@ func (s *ConcreteApiServer) handleRequest(req ApiRequest, w http.ResponseWriter) case errors.Is(resp.err, ErrTooManyRequests): w.WriteHeader(http.StatusTooManyRequests) return - case errors.Is(resp.err, ErrSuperseded), errors.Is(resp.err, ErrLoaderBusy): + case errors.Is(resp.err, ErrSuperseded), errors.Is(resp.err, ErrLoaderBusy), errors.Is(resp.err, spclient.ErrPlaylistConflict): w.WriteHeader(http.StatusConflict) return case errors.Is(resp.err, ErrBadRequest): @@ -568,6 +573,65 @@ func (s *ConcreteApiServer) GetLibraryPlaylists(w http.ResponseWriter, r *http.R s.handleRequest(ApiRequest{Type: ApiRequestTypeLibraryPlaylists, Data: ApiRequestDataLibraryPlaylists{Offset: params.Offset, Limit: params.Limit}}, w) } +// maxLibraryUris caps how many items one library request may name. +const maxLibraryUris = 50 + +// validItemUris reports whether uris holds 1 to maxLibraryUris URIs, all +// of one of the given types. +func validItemUris(uris []string, types ...librespot.SpotifyIdType) bool { + if len(uris) == 0 || len(uris) > maxLibraryUris { + return false + } + + for _, uri := range uris { + id, err := librespot.SpotifyIdFromUri(uri) + if err != nil || !slices.Contains(types, id.Type()) { + return false + } + } + + return true +} + +func (s *ConcreteApiServer) GetLiked(w http.ResponseWriter, _ *http.Request, params GetLikedParams) { + if !validItemUris(params.Uris, librespot.SpotifyIdTypeTrack) { + w.WriteHeader(http.StatusBadRequest) + return + } + + s.handleRequest(ApiRequest{Type: ApiRequestTypeGetLiked, Data: params.Uris}, w) +} + +func (s *ConcreteApiServer) SetLiked(w http.ResponseWriter, r *http.Request) { + // liked is required: decoded into a plain bool, a missing or null value + // would read as false and remove the tracks from Liked Songs. + var data struct { + Uris []string `json:"uris"` + Liked *bool `json:"liked"` + } + if err := jsonDecode(r, &data); err != nil || data.Liked == nil || !validItemUris(data.Uris, librespot.SpotifyIdTypeTrack) { + w.WriteHeader(http.StatusBadRequest) + return + } + + s.handleRequest(ApiRequest{Type: ApiRequestTypeSetLiked, Data: ApiSetLiked{Uris: data.Uris, Liked: *data.Liked}}, w) +} + +func (s *ConcreteApiServer) PlaylistAddTracks(w http.ResponseWriter, r *http.Request) { + var data ApiPlaylistAddTracks + if err := jsonDecode(r, &data); err != nil || !validItemUris(data.Uris, librespot.SpotifyIdTypeTrack, librespot.SpotifyIdTypeEpisode) { + w.WriteHeader(http.StatusBadRequest) + return + } + + if id, err := librespot.SpotifyIdFromUri(data.PlaylistUri); err != nil || id.Type() != librespot.SpotifyIdTypePlaylist { + w.WriteHeader(http.StatusBadRequest) + return + } + + s.handleRequest(ApiRequest{Type: ApiRequestTypePlaylistAddTracks, Data: data}, 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 a28ca52d..89a7b58e 100644 --- a/daemon/api_server_test.go +++ b/daemon/api_server_test.go @@ -17,6 +17,7 @@ import ( "github.com/coder/websocket" librespot "github.com/devgianlu/go-librespot" + "github.com/devgianlu/go-librespot/spclient" "github.com/stretchr/testify/require" ) @@ -131,27 +132,29 @@ 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}, + "/": {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}, } func TestApiRejectsWrongMethod(t *testing.T) { @@ -612,7 +615,109 @@ func TestApiLibraryPlaylists(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) require.JSONEq(t, `{"total":1,"offset":0,"limit":50,"items":[{ "uri":"spotify:playlist:xxx","name":"Mix","description":"","owner_username":"", - "length":0,"image_url":null,"collaborative":false,"folder":[]}]}`, body(t, resp)) + "length":0,"image_url":null,"collaborative":false,"can_edit":false,"folder":[]}]}`, body(t, resp)) + }) +} + +func TestApiGetLiked(t *testing.T) { + t.Run("forwards comma-separated uris", func(t *testing.T) { + ts := newTestServer(t, func(ApiRequest) (any, error) { + return &ApiLikedStates{Items: []ApiLikedState{{Uri: "spotify:track:4uLU6hMCjMI75M1A2tKUQC", Liked: true}}}, nil + }) + + resp := ts.do(http.MethodGet, "/library/liked?uris=spotify:track:4uLU6hMCjMI75M1A2tKUQC,spotify:track:37i9dQZF1DXcBWIGoYBM5M", nil) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.JSONEq(t, `{"items":[{"uri":"spotify:track:4uLU6hMCjMI75M1A2tKUQC","liked":true}]}`, body(t, resp)) + + req := ts.request() + require.Equal(t, ApiRequestTypeGetLiked, req.Type) + require.Equal(t, []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC", "spotify:track:37i9dQZF1DXcBWIGoYBM5M"}, req.Data) + }) + + for _, query := range []string{"", "?uris=", "?uris=spotify:album:4uLU6hMCjMI75M1A2tKUQC", "?uris=nope"} { + t.Run("rejects "+query, func(t *testing.T) { + ts := newTestServer(t, okReply) + + resp := ts.do(http.MethodGet, "/library/liked"+query, nil) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + ts.requireNoRequest() + }) + } +} + +func TestApiSetLiked(t *testing.T) { + t.Run("forwards the payload", func(t *testing.T) { + ts := newTestServer(t, okReply) + + resp := ts.do(http.MethodPost, "/library/liked", map[string]any{"uris": []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC"}, "liked": true}) + require.Equal(t, http.StatusOK, resp.StatusCode) + + req := ts.request() + require.Equal(t, ApiRequestTypeSetLiked, req.Type) + require.Equal(t, ApiSetLiked{Uris: []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC"}, Liked: true}, req.Data) + }) + + tooMany := make([]string, 51) + for i := range tooMany { + tooMany[i] = "spotify:track:4uLU6hMCjMI75M1A2tKUQC" + } + for name, payload := range map[string]any{ + "missing liked": map[string]any{"uris": []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC"}}, + "null liked": map[string]any{"uris": []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC"}, "liked": nil}, + "no uris": map[string]any{"uris": []string{}, "liked": true}, + "too many": map[string]any{"uris": tooMany, "liked": true}, + "not track": map[string]any{"uris": []string{"spotify:album:4uLU6hMCjMI75M1A2tKUQC"}, "liked": true}, + "garbage": map[string]any{"uris": []string{"nope"}, "liked": true}, + } { + t.Run("rejects "+name, func(t *testing.T) { + ts := newTestServer(t, okReply) + + resp := ts.do(http.MethodPost, "/library/liked", payload) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + ts.requireNoRequest() + }) + } +} + +func TestApiPlaylistAddTracks(t *testing.T) { + t.Run("forwards the payload", func(t *testing.T) { + ts := newTestServer(t, okReply) + + resp := ts.do(http.MethodPost, "/library/playlists/add_tracks", map[string]any{ + "playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", + "uris": []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC", "spotify:episode:4uLU6hMCjMI75M1A2tKUQC"}, + }) + require.Equal(t, http.StatusOK, resp.StatusCode) + + req := ts.request() + require.Equal(t, ApiRequestTypePlaylistAddTracks, req.Type) + require.Equal(t, "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", req.Data.(ApiPlaylistAddTracks).PlaylistUri) + }) + + for name, payload := range map[string]any{ + "album as playlist": map[string]any{"playlist_uri": "spotify:album:37i9dQZF1DXcBWIGoYBM5M", "uris": []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC"}}, + "no uris": map[string]any{"playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", "uris": []string{}}, + "artist item": map[string]any{"playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", "uris": []string{"spotify:artist:4uLU6hMCjMI75M1A2tKUQC"}}, + } { + t.Run("rejects "+name, func(t *testing.T) { + ts := newTestServer(t, okReply) + + resp := ts.do(http.MethodPost, "/library/playlists/add_tracks", payload) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + ts.requireNoRequest() + }) + } + + t.Run("maps a lasting conflict to 409", func(t *testing.T) { + ts := newTestServer(t, func(ApiRequest) (any, error) { + return nil, fmt.Errorf("failed appending to playlist: %w", spclient.ErrPlaylistConflict) + }) + + resp := ts.do(http.MethodPost, "/library/playlists/add_tracks", map[string]any{ + "playlist_uri": "spotify:playlist:37i9dQZF1DXcBWIGoYBM5M", + "uris": []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC"}, + }) + require.Equal(t, http.StatusConflict, resp.StatusCode) }) } diff --git a/daemon/library.go b/daemon/library.go index d26186ef..e4451306 100644 --- a/daemon/library.go +++ b/daemon/library.go @@ -3,11 +3,14 @@ package daemon import ( "context" "encoding/hex" + "errors" "fmt" + "net/http" "net/url" "strings" "time" + librespot "github.com/devgianlu/go-librespot" playlist4pb "github.com/devgianlu/go-librespot/proto/spotify/playlist4" "github.com/devgianlu/go-librespot/spclient" ) @@ -15,6 +18,9 @@ import ( // libraryPlaylistsTimeout bounds fetching the rootlist for an API caller. const libraryPlaylistsTimeout = 30 * time.Second +// libraryRequestTimeout bounds a request to Liked Songs or a playlist. +const libraryRequestTimeout = 30 * time.Second + // rootlistPageSize is how many rootlist entries are asked for per request. // Folder markers count as entries, so a library needs a few more requests // than its playlist count alone would suggest. @@ -102,6 +108,7 @@ func libraryPlaylist(uri string, meta *playlist4pb.MetaItem, folders []string) A Length: meta.GetLength(), ImageUrl: playlistImageUrl(attrs), Collaborative: attrs.GetCollaborative(), + CanEdit: meta.GetCapabilities().GetCanEditItems(), Folder: append([]string{}, folders...), } } @@ -145,3 +152,41 @@ func pageLibraryPlaylists(playlists []ApiLibraryPlaylist, offset, limit int) *Ap Items: playlists[start:end], } } + +// appendToPlaylist appends uris to a playlist. The change has to name the +// revision it is based on; if the playlist moves on in between, it is read +// again and the append retried once. +func appendToPlaylist(ctx context.Context, spc *spclient.Spclient, username, playlistUri string, uris []string) error { + id, err := librespot.SpotifyIdFromUri(playlistUri) + if err != nil { + return err + } + + for attempt := 0; ; attempt++ { + revision, err := spc.PlaylistRevision(ctx, *id) + if err != nil { + return err + } + + err = spc.PlaylistAppend(ctx, *id, username, revision, uris) + if !errors.Is(err, spclient.ErrPlaylistConflict) || attempt > 0 { + return err + } + } +} + +// libraryError wraps a failed library request, carrying a 403 or 404 from +// Spotify over as ErrForbidden or ErrNotFound: a playlist the user may not +// edit, or one that is gone, is the client's to handle, not a daemon fault. +func libraryError(what string, err error) error { + var status *spclient.StatusError + if errors.As(err, &status) { + switch status.StatusCode { + case http.StatusForbidden: + return fmt.Errorf("%s: %w: %w", what, ErrForbidden, err) + case http.StatusNotFound: + return fmt.Errorf("%s: %w: %w", what, ErrNotFound, err) + } + } + return fmt.Errorf("%s: %w", what, err) +} diff --git a/daemon/library_test.go b/daemon/library_test.go index 488ca7ff..b6ca4718 100644 --- a/daemon/library_test.go +++ b/daemon/library_test.go @@ -6,6 +6,8 @@ import ( "testing" playlist4pb "github.com/devgianlu/go-librespot/proto/spotify/playlist4" + playlist_permissionpb "github.com/devgianlu/go-librespot/proto/spotify/playlist_permission" + "github.com/devgianlu/go-librespot/spclient" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" ) @@ -57,6 +59,14 @@ func TestFlattenRootlist(t *testing.T) { require.Empty(t, playlists[3].Folder) } +func TestLibraryPlaylistCanEdit(t *testing.T) { + editable := namedMeta("Mine", 1) + editable.Capabilities = &playlist_permissionpb.Capabilities{CanEditItems: proto.Bool(true)} + + require.True(t, libraryPlaylist("spotify:playlist:mine", editable, nil).CanEdit) + require.False(t, libraryPlaylist("spotify:playlist:theirs", namedMeta("Theirs", 1), nil).CanEdit) +} + func TestFlattenRootlistToleratesUnbalancedGroups(t *testing.T) { item, meta := rootlistEntry("spotify:end-group:zzz", nil) pl, plMeta := rootlistEntry("spotify:playlist:x", namedMeta("X", 1)) @@ -124,3 +134,16 @@ func uris(playlists []ApiLibraryPlaylist) []string { } return out } + +func TestLibraryErrorMapsStatuses(t *testing.T) { + forbidden := libraryError("failed appending", &spclient.StatusError{Op: "playlist changes", StatusCode: 403}) + require.ErrorIs(t, forbidden, ErrForbidden) + require.Contains(t, forbidden.Error(), "403", "the cause stays in the message") + + require.ErrorIs(t, libraryError("x", &spclient.StatusError{Op: "playlist", StatusCode: 404}), ErrNotFound) + + other := libraryError("x", &spclient.StatusError{Op: "playlist", StatusCode: 500}) + require.NotErrorIs(t, other, ErrForbidden) + require.NotErrorIs(t, other, ErrNotFound) + require.ErrorIs(t, libraryError("x", spclient.ErrPlaylistConflict), spclient.ErrPlaylistConflict) +} diff --git a/daemon/liked.go b/daemon/liked.go new file mode 100644 index 00000000..85a855ac --- /dev/null +++ b/daemon/liked.go @@ -0,0 +1,41 @@ +package daemon + +import ( + "context" + "fmt" + + librespot "github.com/devgianlu/go-librespot" +) + +// likedSongsContextUris are the context URIs Liked Songs is listed under. +func likedSongsContextUris(username string) []string { + return []string{"spotify:user:" + username + ":collection", "spotify:collection:tracks"} +} + +// likedContains asks the collection service which of uris are in Liked Songs. +type likedContains func(ctx context.Context, uris []string) ([]bool, error) + +// likedStates tells for each of uris whether it is in Liked Songs. The +// collection holds canonical URIs, so a 21-character id is padded to its +// canonical form before asking; the answer names the URIs as requested. +func likedStates(ctx context.Context, contains likedContains, uris []string) ([]ApiLikedState, error) { + canonical := make([]string, len(uris)) + for i, uri := range uris { + id, err := librespot.SpotifyIdFromUri(uri) + if err != nil { + return nil, fmt.Errorf("invalid track uri %s: %w", uri, err) + } + canonical[i] = id.Uri() + } + + found, err := contains(ctx, canonical) + if err != nil { + return nil, err + } + + states := make([]ApiLikedState, len(uris)) + for i, uri := range uris { + states[i] = ApiLikedState{Uri: uri, Liked: found[i]} + } + return states, nil +} diff --git a/daemon/liked_test.go b/daemon/liked_test.go new file mode 100644 index 00000000..b02c9956 --- /dev/null +++ b/daemon/liked_test.go @@ -0,0 +1,36 @@ +//go:build test_unit + +package daemon + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestLikedStatesAsksWithCanonicalUris(t *testing.T) { + var asked []string + contains := func(_ context.Context, uris []string) ([]bool, error) { + asked = uris + return []bool{true, false}, nil + } + + // A 21-character id is the same track as its zero-padded 22-character form. + short := "spotify:track:4uLU6hMCjMI75M1A2tKUQC"[:14] + "uLU6hMCjMI75M1A2tKUQC" + states, err := likedStates(context.Background(), contains, []string{short, "spotify:track:4uLU6hMCjMI75M1A2tKUQC"}) + require.NoError(t, err) + + require.Equal(t, "spotify:track:0uLU6hMCjMI75M1A2tKUQC", asked[0], "asked in canonical form") + require.Equal(t, []ApiLikedState{ + {Uri: short, Liked: true}, + {Uri: "spotify:track:4uLU6hMCjMI75M1A2tKUQC", Liked: false}, + }, states, "answered with the URIs as requested") +} + +func TestLikedStatesPassesErrorsOn(t *testing.T) { + contains := func(context.Context, []string) ([]bool, error) { return nil, errors.New("boom") } + _, err := likedStates(context.Background(), contains, []string{"spotify:track:4uLU6hMCjMI75M1A2tKUQC"}) + require.Error(t, err) +} diff --git a/daemon/player.go b/daemon/player.go index e6f34b63..8833642a 100644 --- a/daemon/player.go +++ b/daemon/player.go @@ -24,6 +24,7 @@ import ( "github.com/devgianlu/go-librespot/player" connectpb "github.com/devgianlu/go-librespot/proto/spotify/connectstate" "github.com/devgianlu/go-librespot/session" + "github.com/devgianlu/go-librespot/spclient" "github.com/devgianlu/go-librespot/tracks" ) @@ -900,6 +901,56 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { reply.done(pageLibraryPlaylists(playlists, data.Offset, data.Limit), nil) }) + return nil, errReplyDeferred + case ApiRequestTypeSetLiked: + data := req.Data.(ApiSetLiked) + reply := apiReply(req) + spc, username := p.sess.Spclient(), p.sess.Username() + p.goDetached(libraryRequestTimeout, func(ctx context.Context) { + if err := spc.CollectionWrite(ctx, username, spclient.CollectionSetLikedSongs, data.Uris, !data.Liked); err != nil { + reply.done(nil, libraryError("failed writing liked songs", err)) + return + } + + for _, uri := range likedSongsContextUris(username) { + p.app.contextLists.invalidate(uri) + } + reply.done(nil, nil) + }) + + return nil, errReplyDeferred + case ApiRequestTypeGetLiked: + uris := req.Data.([]string) + reply := apiReply(req) + spc, username := p.sess.Spclient(), p.sess.Username() + p.goDetached(libraryRequestTimeout, func(ctx context.Context) { + contains := func(ctx context.Context, uris []string) ([]bool, error) { + return spc.CollectionContains(ctx, username, spclient.CollectionSetLikedSongs, uris) + } + states, err := likedStates(ctx, contains, uris) + if err != nil { + reply.done(nil, libraryError("failed reading liked songs", err)) + return + } + + reply.done(&ApiLikedStates{Items: states}, nil) + }) + + return nil, errReplyDeferred + case ApiRequestTypePlaylistAddTracks: + data := req.Data.(ApiPlaylistAddTracks) + reply := apiReply(req) + spc, username := p.sess.Spclient(), p.sess.Username() + p.goDetached(libraryRequestTimeout, func(ctx context.Context) { + if err := appendToPlaylist(ctx, spc, username, data.PlaylistUri, data.Uris); err != nil { + reply.done(nil, libraryError("failed appending to playlist", err)) + return + } + + p.app.contextLists.invalidate(data.PlaylistUri) + reply.done(nil, nil) + }) + return nil, errReplyDeferred case ApiRequestSetDeviceName: p.setDeviceName(req.Data.(string)) diff --git a/daemon/track_meta_cache.go b/daemon/track_meta_cache.go index c37e33dd..d16e3839 100644 --- a/daemon/track_meta_cache.go +++ b/daemon/track_meta_cache.go @@ -127,15 +127,18 @@ type contextListCache struct { // now is the clock the TTL is checked against; replaced by tests. now func() time.Time - // inFlight is the set of uris being enumerated right now, so that a client - // polling every second does not spawn an enumeration per poll. + // inFlight maps the uris being enumerated right now to the generation + // they started at, so that a client polling every second does not spawn + // an enumeration per poll, and an invalidation during an enumeration can + // tell put to drop its possibly pre-change result. Changes to the LRU go + // through mu too, so a put and an invalidation cannot interleave. mu sync.Mutex - inFlight map[string]bool + inFlight map[string]uint64 } func newContextListCache() *contextListCache { l, _ := lru.New[string, contextListEntry](contextListCacheLimit) - return &contextListCache{lru: l, now: time.Now, inFlight: map[string]bool{}} + return &contextListCache{lru: l, now: time.Now, inFlight: map[string]uint64{}} } func (c *contextListCache) get(uri string) ([]string, bool) { @@ -150,28 +153,54 @@ func (c *contextListCache) get(uri string) ([]string, bool) { return e.uris, true } -func (c *contextListCache) put(uri string, uris []string) { +// put caches the listing an enumeration begun at gen produced, unless the +// context was invalidated since: the listing may then predate the change. +func (c *contextListCache) put(uri string, uris []string, gen uint64) { + c.mu.Lock() + defer c.mu.Unlock() + + if started, ok := c.inFlight[uri]; ok && started != gen { + return + } c.lru.Add(uri, contextListEntry{uris: uris, fetched: c.now()}) } -// beginFetch claims the right to enumerate uri, reporting false when the -// listing is already cached or another goroutine is already enumerating it. -func (c *contextListCache) beginFetch(uri string) bool { +// invalidate drops the listing of uri after it was changed through the +// daemon, so the next request enumerates it again instead of serving the +// old listing for the rest of contextListTTL. +func (c *contextListCache) invalidate(uri string) { if c == nil { - return false + return + } + + c.mu.Lock() + defer c.mu.Unlock() + + c.lru.Remove(uri) + if gen, ok := c.inFlight[uri]; ok { + c.inFlight[uri] = gen + 1 + } +} + +// beginFetch claims the right to enumerate uri and returns the generation to +// hand to put, reporting false when the listing is already cached or another +// goroutine is already enumerating it. +func (c *contextListCache) beginFetch(uri string) (gen uint64, ok bool) { + if c == nil { + return 0, false } if _, ok := c.get(uri); ok { - return false + return 0, false } c.mu.Lock() defer c.mu.Unlock() - if c.inFlight[uri] { - return false + if _, ok := c.inFlight[uri]; ok { + return 0, false } - c.inFlight[uri] = true - return true + c.inFlight[uri] = 0 + return 0, true } func (c *contextListCache) endFetch(uri string) { @@ -538,7 +567,8 @@ func (p *AppPlayer) scheduleContextEnumerate(contextUri string) { p.scheduleMetaSweep(uris, contextUri) return } - if !p.app.contextLists.beginFetch(contextUri) { + gen, ok := p.app.contextLists.beginFetch(contextUri) + if !ok { return } @@ -561,7 +591,7 @@ func (p *AppPlayer) scheduleContextEnumerate(contextUri string) { p.app.log.Debugf("context listing truncated to %d tracks: %s", len(uris), contextUri) } - p.app.contextLists.put(contextUri, uris) + p.app.contextLists.put(contextUri, uris, gen) p.scheduleMetaSweep(uris, contextUri) }) } diff --git a/daemon/track_meta_cache_test.go b/daemon/track_meta_cache_test.go index 89cf7cc8..3d5821b8 100644 --- a/daemon/track_meta_cache_test.go +++ b/daemon/track_meta_cache_test.go @@ -198,19 +198,120 @@ func TestTrackMetaCacheCapacityFitsMaxTracks(t *testing.T) { // A client polling a filling sweep asks for the same context every second or // two; enumerating pages over the network, so those polls must be served from // memory rather than re-paging the whole playlist each time. +func claimed(c *contextListCache, uri string) bool { + _, ok := c.beginFetch(uri) + return ok +} + func TestContextListCacheServesRepeatLookups(t *testing.T) { c := newContextListCache() _, ok := c.get("spotify:playlist:a") require.False(t, ok, "miss on an empty cache") - c.put("spotify:playlist:a", []string{"spotify:track:1", "spotify:track:2"}) + c.put("spotify:playlist:a", []string{"spotify:track:1", "spotify:track:2"}, 0) uris, ok := c.get("spotify:playlist:a") require.True(t, ok) require.Len(t, uris, 2) } +// A write through the daemon changes the context, so its listing must not be +// served from the cache afterwards. +func TestContextListCacheInvalidate(t *testing.T) { + c := newContextListCache() + c.put("spotify:playlist:a", []string{"spotify:track:1"}, 0) + + c.invalidate("spotify:playlist:a") + _, ok := c.get("spotify:playlist:a") + require.False(t, ok, "an invalidated listing is a miss") + require.True(t, claimed(c, "spotify:playlist:a"), "and is enumerated again") + + var nilCache *contextListCache + nilCache.invalidate("spotify:playlist:a") // metadata disabled: no-op +} + +// An enumeration that was already running when the context changed may have +// read it before the change; its result must not land in the cache. +func TestContextListCacheInvalidateDuringEnumeration(t *testing.T) { + c := newContextListCache() + gen, ok := c.beginFetch("spotify:playlist:a") + require.True(t, ok) + + c.invalidate("spotify:playlist:a") + c.put("spotify:playlist:a", []string{"spotify:track:old"}, gen) + c.endFetch("spotify:playlist:a") + + _, ok = c.get("spotify:playlist:a") + require.False(t, ok, "the possibly stale result was dropped") + + gen, ok = c.beginFetch("spotify:playlist:a") + require.True(t, ok) + c.put("spotify:playlist:a", []string{"spotify:track:new"}, gen) + c.endFetch("spotify:playlist:a") + uris, ok := c.get("spotify:playlist:a") + require.True(t, ok, "the next enumeration is cached again") + require.Equal(t, []string{"spotify:track:new"}, uris) +} + +// An invalidated enumeration that fails never calls put; that must not mark +// the next, correct enumeration as stale. +func TestContextListCacheInvalidatedEnumerationThatFails(t *testing.T) { + c := newContextListCache() + _, ok := c.beginFetch("spotify:playlist:a") + require.True(t, ok) + c.invalidate("spotify:playlist:a") + c.endFetch("spotify:playlist:a") // failed: no put + + gen, ok := c.beginFetch("spotify:playlist:a") + require.True(t, ok) + c.put("spotify:playlist:a", []string{"spotify:track:new"}, gen) + c.endFetch("spotify:playlist:a") + _, ok = c.get("spotify:playlist:a") + require.True(t, ok, "the next enumeration is cached") +} + +// An invalidation between put and endFetch drops the listing put just cached +// and must not leave anything that drops the next enumeration. +func TestContextListCacheInvalidateAfterPut(t *testing.T) { + c := newContextListCache() + gen, ok := c.beginFetch("spotify:playlist:a") + require.True(t, ok) + c.put("spotify:playlist:a", []string{"spotify:track:old"}, gen) + c.invalidate("spotify:playlist:a") + c.endFetch("spotify:playlist:a") + + _, ok = c.get("spotify:playlist:a") + require.False(t, ok, "the invalidated listing is gone") + + gen, ok = c.beginFetch("spotify:playlist:a") + require.True(t, ok) + c.put("spotify:playlist:a", []string{"spotify:track:new"}, gen) + c.endFetch("spotify:playlist:a") + uris, ok := c.get("spotify:playlist:a") + require.True(t, ok) + require.Equal(t, []string{"spotify:track:new"}, uris) +} + +// put and invalidate run from different goroutines; neither may let a +// listing from before an invalidation survive it. +func TestContextListCacheInvalidateRacesPut(t *testing.T) { + for range 200 { + c := newContextListCache() + gen, _ := c.beginFetch("spotify:playlist:a") + + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.put("spotify:playlist:a", []string{"spotify:track:old"}, gen) }() + go func() { defer wg.Done(); c.invalidate("spotify:playlist:a") }() + wg.Wait() + c.endFetch("spotify:playlist:a") + + _, ok := c.get("spotify:playlist:a") + require.False(t, ok, "whichever ran first, the pre-change listing must not be served") + } +} + // The listing carries no revision, so a stale entry is the only way a client // could miss an edit; the TTL bounds how long that can last. func TestContextListCacheExpires(t *testing.T) { @@ -219,7 +320,7 @@ func TestContextListCacheExpires(t *testing.T) { now := time.Now() c.now = func() time.Time { return now } - c.put("spotify:playlist:a", []string{"spotify:track:1"}) + c.put("spotify:playlist:a", []string{"spotify:track:1"}, 0) now = now.Add(contextListTTL) _, ok := c.get("spotify:playlist:a") @@ -228,14 +329,14 @@ func TestContextListCacheExpires(t *testing.T) { now = now.Add(time.Second) _, ok = c.get("spotify:playlist:a") require.False(t, ok, "an entry older than the TTL is a miss") - require.True(t, c.beginFetch("spotify:playlist:a"), "an expired listing is re-enumerated") + require.True(t, claimed(c, "spotify:playlist:a"), "an expired listing is re-enumerated") } func TestContextListCacheEviction(t *testing.T) { c := newContextListCache() for i := 0; i < contextListCacheLimit+3; i++ { - c.put(fmt.Sprintf("spotify:playlist:%d", i), []string{"spotify:track:1"}) + c.put(fmt.Sprintf("spotify:playlist:%d", i), []string{"spotify:track:1"}, 0) } require.Equal(t, contextListCacheLimit, c.lru.Len()) @@ -253,15 +354,15 @@ func TestContextListCacheEviction(t *testing.T) { func TestContextListCacheSingleFlightsEnumeration(t *testing.T) { c := newContextListCache() - require.True(t, c.beginFetch("spotify:playlist:a"), "the first caller claims the enumeration") - require.False(t, c.beginFetch("spotify:playlist:a"), "a concurrent caller is turned away") + require.True(t, claimed(c, "spotify:playlist:a"), "the first caller claims the enumeration") + require.False(t, claimed(c, "spotify:playlist:a"), "a concurrent caller is turned away") c.endFetch("spotify:playlist:a") - require.True(t, c.beginFetch("spotify:playlist:a"), "claimable again once the previous one finished") + require.True(t, claimed(c, "spotify:playlist:a"), "claimable again once the previous one finished") c.endFetch("spotify:playlist:a") - c.put("spotify:playlist:a", []string{"spotify:track:1"}) - require.False(t, c.beginFetch("spotify:playlist:a"), "a cached listing needs no enumeration") + c.put("spotify:playlist:a", []string{"spotify:track:1"}, 0) + require.False(t, claimed(c, "spotify:playlist:a"), "a cached listing needs no enumeration") } // The entire feature is opt-in: with metadata.enabled false the caches are @@ -277,7 +378,7 @@ func TestMetadataDisabledIsNoop(t *testing.T) { var cl *contextListCache _, ok := cl.get("spotify:playlist:a") require.False(t, ok) - require.False(t, cl.beginFetch("spotify:playlist:a"), "a nil cache never claims an enumeration") + require.False(t, claimed(cl, "spotify:playlist:a"), "a nil cache never claims an enumeration") // A disabled player: no timer is armed and no session is touched. p := &AppPlayer{app: &App{cfg: &Config{}}, state: &State{}} diff --git a/proto/spotify/collection/v2/collection.pb.go b/proto/spotify/collection/v2/collection.pb.go new file mode 100644 index 00000000..c8afb8ee --- /dev/null +++ b/proto/spotify/collection/v2/collection.pb.go @@ -0,0 +1,321 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.10 +// protoc (unknown) +// source: spotify/collection/v2/collection.proto + +package collectionv2 + +import ( + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + reflect "reflect" + sync "sync" + unsafe "unsafe" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +type CollectionItem struct { + state protoimpl.MessageState `protogen:"open.v1"` + Uri string `protobuf:"bytes,1,opt,name=uri,proto3" json:"uri,omitempty"` + AddedAt int64 `protobuf:"varint,2,opt,name=added_at,json=addedAt,proto3" json:"added_at,omitempty"` + IsRemoved bool `protobuf:"varint,3,opt,name=is_removed,json=isRemoved,proto3" json:"is_removed,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *CollectionItem) Reset() { + *x = CollectionItem{} + mi := &file_spotify_collection_v2_collection_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *CollectionItem) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*CollectionItem) ProtoMessage() {} + +func (x *CollectionItem) ProtoReflect() protoreflect.Message { + mi := &file_spotify_collection_v2_collection_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use CollectionItem.ProtoReflect.Descriptor instead. +func (*CollectionItem) Descriptor() ([]byte, []int) { + return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{0} +} + +func (x *CollectionItem) GetUri() string { + if x != nil { + return x.Uri + } + return "" +} + +func (x *CollectionItem) GetAddedAt() int64 { + if x != nil { + return x.AddedAt + } + return 0 +} + +func (x *CollectionItem) GetIsRemoved() bool { + if x != nil { + return x.IsRemoved + } + return false +} + +type WriteRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` + Set string `protobuf:"bytes,2,opt,name=set,proto3" json:"set,omitempty"` + Items []*CollectionItem `protobuf:"bytes,3,rep,name=items,proto3" json:"items,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *WriteRequest) Reset() { + *x = WriteRequest{} + mi := &file_spotify_collection_v2_collection_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *WriteRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*WriteRequest) ProtoMessage() {} + +func (x *WriteRequest) ProtoReflect() protoreflect.Message { + mi := &file_spotify_collection_v2_collection_proto_msgTypes[1] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use WriteRequest.ProtoReflect.Descriptor instead. +func (*WriteRequest) Descriptor() ([]byte, []int) { + return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{1} +} + +func (x *WriteRequest) GetUsername() string { + if x != nil { + return x.Username + } + return "" +} + +func (x *WriteRequest) GetSet() string { + if x != nil { + return x.Set + } + return "" +} + +func (x *WriteRequest) GetItems() []*CollectionItem { + if x != nil { + return x.Items + } + return nil +} + +type ContainsRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` + Set string `protobuf:"bytes,2,opt,name=set,proto3" json:"set,omitempty"` + Items []*CollectionItem `protobuf:"bytes,3,rep,name=items,proto3" json:"items,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *ContainsRequest) Reset() { + *x = ContainsRequest{} + mi := &file_spotify_collection_v2_collection_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *ContainsRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ContainsRequest) ProtoMessage() {} + +func (x *ContainsRequest) ProtoReflect() protoreflect.Message { + mi := &file_spotify_collection_v2_collection_proto_msgTypes[2] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ContainsRequest.ProtoReflect.Descriptor instead. +func (*ContainsRequest) Descriptor() ([]byte, []int) { + return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{2} +} + +func (x *ContainsRequest) GetUsername() string { + if x != nil { + return x.Username + } + return "" +} + +func (x *ContainsRequest) GetSet() string { + if x != nil { + return x.Set + } + return "" +} + +func (x *ContainsRequest) GetItems() []*CollectionItem { + if x != nil { + return x.Items + } + return nil +} + +type ContainsResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + Found []bool `protobuf:"varint,1,rep,packed,name=found,proto3" json:"found,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *ContainsResponse) Reset() { + *x = ContainsResponse{} + mi := &file_spotify_collection_v2_collection_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *ContainsResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ContainsResponse) ProtoMessage() {} + +func (x *ContainsResponse) ProtoReflect() protoreflect.Message { + mi := &file_spotify_collection_v2_collection_proto_msgTypes[3] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ContainsResponse.ProtoReflect.Descriptor instead. +func (*ContainsResponse) Descriptor() ([]byte, []int) { + return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{3} +} + +func (x *ContainsResponse) GetFound() []bool { + if x != nil { + return x.Found + } + return nil +} + +var File_spotify_collection_v2_collection_proto protoreflect.FileDescriptor + +const file_spotify_collection_v2_collection_proto_rawDesc = "" + + "\n" + + "&spotify/collection/v2/collection.proto\x12\x15spotify.collection.v2\"\\\n" + + "\x0eCollectionItem\x12\x10\n" + + "\x03uri\x18\x01 \x01(\tR\x03uri\x12\x19\n" + + "\badded_at\x18\x02 \x01(\x03R\aaddedAt\x12\x1d\n" + + "\n" + + "is_removed\x18\x03 \x01(\bR\tisRemoved\"y\n" + + "\fWriteRequest\x12\x1a\n" + + "\busername\x18\x01 \x01(\tR\busername\x12\x10\n" + + "\x03set\x18\x02 \x01(\tR\x03set\x12;\n" + + "\x05items\x18\x03 \x03(\v2%.spotify.collection.v2.CollectionItemR\x05items\"|\n" + + "\x0fContainsRequest\x12\x1a\n" + + "\busername\x18\x01 \x01(\tR\busername\x12\x10\n" + + "\x03set\x18\x02 \x01(\tR\x03set\x12;\n" + + "\x05items\x18\x03 \x03(\v2%.spotify.collection.v2.CollectionItemR\x05items\"(\n" + + "\x10ContainsResponse\x12\x14\n" + + "\x05found\x18\x01 \x03(\bR\x05foundB\xee\x01\n" + + "\x19com.spotify.collection.v2B\x0fCollectionProtoP\x01ZJgithub.com/devgianlu/go-librespot/proto/spotify/collection/v2;collectionv2\xa2\x02\x03SCX\xaa\x02\x15Spotify.Collection.V2\xca\x02\x15Spotify\\Collection\\V2\xe2\x02!Spotify\\Collection\\V2\\GPBMetadata\xea\x02\x17Spotify::Collection::V2b\x06proto3" + +var ( + file_spotify_collection_v2_collection_proto_rawDescOnce sync.Once + file_spotify_collection_v2_collection_proto_rawDescData []byte +) + +func file_spotify_collection_v2_collection_proto_rawDescGZIP() []byte { + file_spotify_collection_v2_collection_proto_rawDescOnce.Do(func() { + file_spotify_collection_v2_collection_proto_rawDescData = protoimpl.X.CompressGZIP(unsafe.Slice(unsafe.StringData(file_spotify_collection_v2_collection_proto_rawDesc), len(file_spotify_collection_v2_collection_proto_rawDesc))) + }) + return file_spotify_collection_v2_collection_proto_rawDescData +} + +var file_spotify_collection_v2_collection_proto_msgTypes = make([]protoimpl.MessageInfo, 4) +var file_spotify_collection_v2_collection_proto_goTypes = []any{ + (*CollectionItem)(nil), // 0: spotify.collection.v2.CollectionItem + (*WriteRequest)(nil), // 1: spotify.collection.v2.WriteRequest + (*ContainsRequest)(nil), // 2: spotify.collection.v2.ContainsRequest + (*ContainsResponse)(nil), // 3: spotify.collection.v2.ContainsResponse +} +var file_spotify_collection_v2_collection_proto_depIdxs = []int32{ + 0, // 0: spotify.collection.v2.WriteRequest.items:type_name -> spotify.collection.v2.CollectionItem + 0, // 1: spotify.collection.v2.ContainsRequest.items:type_name -> spotify.collection.v2.CollectionItem + 2, // [2:2] is the sub-list for method output_type + 2, // [2:2] is the sub-list for method input_type + 2, // [2:2] is the sub-list for extension type_name + 2, // [2:2] is the sub-list for extension extendee + 0, // [0:2] is the sub-list for field type_name +} + +func init() { file_spotify_collection_v2_collection_proto_init() } +func file_spotify_collection_v2_collection_proto_init() { + if File_spotify_collection_v2_collection_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: unsafe.Slice(unsafe.StringData(file_spotify_collection_v2_collection_proto_rawDesc), len(file_spotify_collection_v2_collection_proto_rawDesc)), + NumEnums: 0, + NumMessages: 4, + NumExtensions: 0, + NumServices: 0, + }, + GoTypes: file_spotify_collection_v2_collection_proto_goTypes, + DependencyIndexes: file_spotify_collection_v2_collection_proto_depIdxs, + MessageInfos: file_spotify_collection_v2_collection_proto_msgTypes, + }.Build() + File_spotify_collection_v2_collection_proto = out.File + file_spotify_collection_v2_collection_proto_goTypes = nil + file_spotify_collection_v2_collection_proto_depIdxs = nil +} diff --git a/proto/spotify/collection/v2/collection.proto b/proto/spotify/collection/v2/collection.proto new file mode 100644 index 00000000..3a5ea32f --- /dev/null +++ b/proto/spotify/collection/v2/collection.proto @@ -0,0 +1,28 @@ +syntax = "proto3"; + +package spotify.collection.v2; + +// Messages of the collection service (/collection/v2), which stores the +// user's saved items such as Liked Songs (the "collection" set). + +message CollectionItem { + string uri = 1; + int64 added_at = 2; + bool is_removed = 3; +} + +message WriteRequest { + string username = 1; + string set = 2; + repeated CollectionItem items = 3; +} + +message ContainsRequest { + string username = 1; + string set = 2; + repeated CollectionItem items = 3; +} + +message ContainsResponse { + repeated bool found = 1; +} diff --git a/spclient/library_write.go b/spclient/library_write.go new file mode 100644 index 00000000..1119ad82 --- /dev/null +++ b/spclient/library_write.go @@ -0,0 +1,192 @@ +package spclient + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "time" + + librespot "github.com/devgianlu/go-librespot" + collectionpb "github.com/devgianlu/go-librespot/proto/spotify/collection/v2" + playlist4pb "github.com/devgianlu/go-librespot/proto/spotify/playlist4" + "google.golang.org/protobuf/proto" +) + +// collectionContentType is the media type the collection service speaks. +const collectionContentType = "application/vnd.collection-v2.spotify.proto" + +// CollectionSetLikedSongs is the collection set holding the user's Liked Songs. +const CollectionSetLikedSongs = "collection" + +// StatusError reports a status the collection or playlist service answered +// a library request with, so callers can tell a permission problem (403) or a +// missing playlist (404) from a fault. +type StatusError struct { + Op string + StatusCode int +} + +func (e *StatusError) Error() string { + return fmt.Sprintf("invalid status code from %s: %d", e.Op, e.StatusCode) +} + +// ErrPlaylistConflict reports that a playlist changed between reading its +// revision and writing to it. +var ErrPlaylistConflict = errors.New("playlist changed concurrently") + +// CollectionWrite adds uris to, or with remove set removes them from, one of +// the user's collection sets. +func (c *Spclient) CollectionWrite(ctx context.Context, username, set string, uris []string, remove bool) error { + now := time.Now().Unix() + req := &collectionpb.WriteRequest{Username: username, Set: set} + for _, uri := range uris { + item := &collectionpb.CollectionItem{Uri: uri, IsRemoved: remove} + if !remove { + item.AddedAt = now + } + req.Items = append(req.Items, item) + } + + body, err := proto.Marshal(req) + if err != nil { + return fmt.Errorf("failed marshalling WriteRequest: %w", err) + } + + resp, err := c.Request(ctx, "POST", "/collection/v2/write", nil, http.Header{ + "Content-Type": {collectionContentType}, + "Accept": {collectionContentType}, + }, body) + if err != nil { + return err + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return &StatusError{Op: "collection write", StatusCode: resp.StatusCode} + } + + return nil +} + +// CollectionContains tells for each of uris whether it is in one of the +// user's collection sets, in one round trip. +func (c *Spclient) CollectionContains(ctx context.Context, username, set string, uris []string) ([]bool, error) { + req := &collectionpb.ContainsRequest{Username: username, Set: set} + for _, uri := range uris { + req.Items = append(req.Items, &collectionpb.CollectionItem{Uri: uri}) + } + + body, err := proto.Marshal(req) + if err != nil { + return nil, fmt.Errorf("failed marshalling ContainsRequest: %w", err) + } + + resp, err := c.Request(ctx, "POST", "/collection/v2/contains", nil, http.Header{ + "Content-Type": {collectionContentType}, + "Accept": {collectionContentType}, + }, body) + if err != nil { + return nil, err + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return nil, &StatusError{Op: "collection contains", StatusCode: resp.StatusCode} + } + + respBytes, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed reading response body: %w", err) + } + + var contains collectionpb.ContainsResponse + if err := proto.Unmarshal(respBytes, &contains); err != nil { + return nil, fmt.Errorf("failed unmarshalling ContainsResponse: %w", err) + } + if len(contains.GetFound()) != len(uris) { + return nil, fmt.Errorf("collection contains answered %d items for %d", len(contains.GetFound()), len(uris)) + } + + return contains.GetFound(), nil +} + +// PlaylistRevision returns the current revision of a playlist, which a change +// has to be based on. +func (c *Spclient) PlaylistRevision(ctx context.Context, playlist librespot.SpotifyId) ([]byte, error) { + query := url.Values{} + query.Set("decorate", "revision") + query.Set("from", "0") + query.Set("length", "1") + + resp, err := c.Request(ctx, "GET", fmt.Sprintf("/playlist/v2/playlist/%s", playlist.Base62()), query, nil, nil) + if err != nil { + return nil, err + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return nil, &StatusError{Op: "playlist", StatusCode: resp.StatusCode} + } + + respBytes, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed reading response body: %w", err) + } + + var content playlist4pb.SelectedListContent + if err := proto.Unmarshal(respBytes, &content); err != nil { + return nil, fmt.Errorf("failed unmarshalling SelectedListContent: %w", err) + } + + return content.GetRevision(), nil +} + +// PlaylistAppend appends uris to the end of a playlist, as a change based on +// revision. It returns ErrPlaylistConflict when the playlist moved on since. +func (c *Spclient) PlaylistAppend(ctx context.Context, playlist librespot.SpotifyId, username string, revision []byte, uris []string) error { + items := make([]*playlist4pb.Item, 0, len(uris)) + for _, uri := range uris { + items = append(items, &playlist4pb.Item{Uri: proto.String(uri)}) + } + + 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)}, + }}, + Info: &playlist4pb.ChangeInfo{ + User: proto.String(username), + Timestamp: proto.Int64(time.Now().UnixMilli()), + }, + }}, + }) + if err != nil { + return fmt.Errorf("failed marshalling ListChanges: %w", err) + } + + // A change is not idempotent: resent after the server applied it but its + // answer got lost, an append would land twice. + resp, err := c.RequestOnce(ctx, "POST", fmt.Sprintf("/playlist/v2/playlist/%s/changes", playlist.Base62()), nil, nil, body) + if err != nil { + return err + } + + defer func() { _ = resp.Body.Close() }() + + switch resp.StatusCode { + case http.StatusOK: + return nil + case http.StatusConflict: + return ErrPlaylistConflict + default: + return &StatusError{Op: "playlist changes", StatusCode: resp.StatusCode} + } +} diff --git a/spclient/library_write_test.go b/spclient/library_write_test.go new file mode 100644 index 00000000..c35b933e --- /dev/null +++ b/spclient/library_write_test.go @@ -0,0 +1,150 @@ +//go:build test_unit + +package spclient_test + +import ( + "net/http" + + librespot "github.com/devgianlu/go-librespot" + collectionpb "github.com/devgianlu/go-librespot/proto/spotify/collection/v2" + playlist4pb "github.com/devgianlu/go-librespot/proto/spotify/playlist4" + "github.com/devgianlu/go-librespot/spclient" + "google.golang.org/protobuf/proto" +) + +func (suite *RequestSuite) TestCollectionWriteSavesToLikedSongs() { + err := suite.spclient.CollectionWrite(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, + []string{"spotify:track:a", "spotify:track:b"}, false) + suite.Require().NoError(err) + + got := suite.requests() + suite.Require().Len(got, 1) + suite.Equal("POST", got[0].method) + suite.Equal("/collection/v2/write", got[0].path) + suite.Equal("application/vnd.collection-v2.spotify.proto", got[0].header.Get("Content-Type")) + + var req collectionpb.WriteRequest + suite.Require().NoError(proto.Unmarshal(got[0].body, &req)) + suite.Equal("user", req.GetUsername()) + suite.Equal("collection", req.GetSet()) + suite.Require().Len(req.GetItems(), 2) + suite.Equal("spotify:track:a", req.GetItems()[0].GetUri()) + suite.False(req.GetItems()[0].GetIsRemoved()) + suite.Positive(req.GetItems()[0].GetAddedAt()) +} + +func (suite *RequestSuite) TestCollectionWriteRemoves() { + err := suite.spclient.CollectionWrite(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, + []string{"spotify:track:a"}, true) + suite.Require().NoError(err) + + var req collectionpb.WriteRequest + suite.Require().NoError(proto.Unmarshal(suite.requests()[0].body, &req)) + suite.True(req.GetItems()[0].GetIsRemoved()) + suite.Zero(req.GetItems()[0].GetAddedAt()) +} + +func (suite *RequestSuite) TestCollectionWriteFailsOnError() { + suite.handler = func(_ int, w http.ResponseWriter) { w.WriteHeader(http.StatusForbidden) } + + err := suite.spclient.CollectionWrite(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, + []string{"spotify:track:a"}, false) + suite.Error(err) +} + +func testPlaylistId(suite *RequestSuite) librespot.SpotifyId { + id, err := librespot.SpotifyIdFromUri("spotify:playlist:37i9dQZF1DXcBWIGoYBM5M") + suite.Require().NoError(err) + return *id +} + +func (suite *RequestSuite) TestPlaylistRevisionReadsRevision() { + suite.handler = func(_ int, w http.ResponseWriter) { + body, _ := proto.Marshal(&playlist4pb.SelectedListContent{Revision: []byte{1, 2, 3}}) + _, _ = w.Write(body) + } + + revision, err := suite.spclient.PlaylistRevision(suite.T().Context(), testPlaylistId(suite)) + suite.Require().NoError(err) + suite.Equal([]byte{1, 2, 3}, revision) + + got := suite.requests()[0] + suite.Equal("/playlist/v2/playlist/37i9dQZF1DXcBWIGoYBM5M", got.path) + suite.Equal("revision", got.query.Get("decorate")) +} + +func (suite *RequestSuite) TestPlaylistAppendSendsAddLastChange() { + err := suite.spclient.PlaylistAppend(suite.T().Context(), testPlaylistId(suite), "user", []byte{9}, + []string{"spotify:track:a"}) + suite.Require().NoError(err) + + got := suite.requests()[0] + suite.Equal("POST", got.method) + suite.Equal("/playlist/v2/playlist/37i9dQZF1DXcBWIGoYBM5M/changes", got.path) + suite.Equal("application/x-protobuf", got.header.Get("Content-Type")) + + var changes playlist4pb.ListChanges + suite.Require().NoError(proto.Unmarshal(got.body, &changes)) + suite.Equal([]byte{9}, changes.GetBaseRevision()) + suite.Require().Len(changes.GetDeltas(), 1) + delta := changes.GetDeltas()[0] + suite.Equal("user", delta.GetInfo().GetUser()) + suite.Require().Len(delta.GetOps(), 1) + op := delta.GetOps()[0] + suite.Equal(playlist4pb.Op_ADD, op.GetKind()) + suite.True(op.GetAdd().GetAddLast()) + suite.Require().Len(op.GetAdd().GetItems(), 1) + suite.Equal("spotify:track:a", op.GetAdd().GetItems()[0].GetUri()) +} + +func (suite *RequestSuite) TestPlaylistAppendReportsConflict() { + suite.handler = func(_ int, w http.ResponseWriter) { w.WriteHeader(http.StatusConflict) } + + err := suite.spclient.PlaylistAppend(suite.T().Context(), testPlaylistId(suite), "user", []byte{9}, + []string{"spotify:track:a"}) + suite.ErrorIs(err, spclient.ErrPlaylistConflict) +} + +func (suite *RequestSuite) TestCollectionContainsAsksForTheItems() { + suite.handler = func(_ int, w http.ResponseWriter) { + body, _ := proto.Marshal(&collectionpb.ContainsResponse{Found: []bool{true, false}}) + _, _ = w.Write(body) + } + + found, err := suite.spclient.CollectionContains(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, + []string{"spotify:track:a", "spotify:track:b"}) + suite.Require().NoError(err) + suite.Equal([]bool{true, false}, found) + + got := suite.requests()[0] + suite.Equal("/collection/v2/contains", got.path) + suite.Equal("application/vnd.collection-v2.spotify.proto", got.header.Get("Content-Type")) + + var req collectionpb.ContainsRequest + suite.Require().NoError(proto.Unmarshal(got.body, &req)) + suite.Equal("user", req.GetUsername()) + suite.Equal("collection", req.GetSet()) + suite.Require().Len(req.GetItems(), 2) + suite.Equal("spotify:track:b", req.GetItems()[1].GetUri(), "items are collection items, not bare strings") +} + +func (suite *RequestSuite) TestCollectionContainsRejectsAShortAnswer() { + suite.handler = func(_ int, w http.ResponseWriter) { + body, _ := proto.Marshal(&collectionpb.ContainsResponse{Found: []bool{true}}) + _, _ = w.Write(body) + } + + _, err := suite.spclient.CollectionContains(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, + []string{"spotify:track:a", "spotify:track:b"}) + suite.Error(err) +} + +// An append the server may have applied before failing must not be resent, +// or the tracks would land twice. +func (suite *RequestSuite) TestPlaylistAppendIsSentOnce() { + suite.handler = func(_ int, w http.ResponseWriter) { w.WriteHeader(http.StatusGatewayTimeout) } + + err := suite.spclient.PlaylistAppend(suite.T().Context(), testPlaylistId(suite), "user", []byte{9}, []string{"spotify:track:a"}) + suite.Error(err) + suite.Len(suite.requests(), 1) +} diff --git a/spclient/request_test.go b/spclient/request_test.go index ab1e5dd4..f86f407c 100644 --- a/spclient/request_test.go +++ b/spclient/request_test.go @@ -142,6 +142,15 @@ func (suite *RequestSuite) TestSetsProtobufContentTypeForBodies() { suite.Equal([]byte("payload"), got[0].body) } +func (suite *RequestSuite) TestKeepsCallerContentType() { + header := http.Header{"Content-Type": []string{"application/vnd.collection-v2.spotify.proto"}} + resp, err := suite.spclient.Request(suite.T().Context(), "POST", "/x", nil, header, []byte("payload")) + suite.Require().NoError(err) + defer func() { _ = resp.Body.Close() }() + + suite.Equal("application/vnd.collection-v2.spotify.proto", suite.requests()[0].header.Get("Content-Type")) +} + func (suite *RequestSuite) TestNoContentTypeWithoutBody() { resp, err := suite.spclient.Request(suite.T().Context(), "GET", "/x", nil, nil, nil) suite.Require().NoError(err) @@ -412,3 +421,34 @@ func (suite *RequestSuite) TestContextResolveUrlReportsTheStatus() { func TestRequestSuite(t *testing.T) { suite.Run(t, new(RequestSuite)) } + +// A request that is not idempotent may already have been applied when the +// server or the network fails, so it is not sent again. +func (suite *RequestSuite) TestRequestOnceDoesNotRepeatTransientFailures() { + suite.handler = func(_ int, w http.ResponseWriter) { w.WriteHeader(http.StatusServiceUnavailable) } + + resp, err := suite.spclient.RequestOnce(suite.T().Context(), "POST", "/x", nil, nil, []byte("change")) + suite.Require().NoError(err) + defer func() { _ = resp.Body.Close() }() + + suite.Equal(http.StatusServiceUnavailable, resp.StatusCode, "the caller sees the status") + suite.Len(suite.requests(), 1, "sent once") +} + +// A 401 means the server refused the request, so renewing the token and +// sending it again is safe even when it is not idempotent. +func (suite *RequestSuite) TestRequestOnceStillRenewsTheToken() { + suite.handler = func(attempt int, w http.ResponseWriter) { + if attempt == 0 { + w.WriteHeader(http.StatusUnauthorized) + } + } + + resp, err := suite.spclient.RequestOnce(suite.T().Context(), "POST", "/x", nil, nil, []byte("change")) + suite.Require().NoError(err) + defer func() { _ = resp.Body.Close() }() + + suite.Equal(http.StatusOK, resp.StatusCode) + suite.Len(suite.requests(), 2) + suite.Equal("Bearer fresh-token", suite.requests()[1].header.Get("Authorization")) +} diff --git a/spclient/spclient.go b/spclient/spclient.go index db035955..63915986 100644 --- a/spclient/spclient.go +++ b/spclient/spclient.go @@ -78,10 +78,14 @@ func NewSpclient(ctx context.Context, log librespot.Logger, client *http.Client, } func (c *Spclient) innerRequest(ctx context.Context, method string, reqUrl *url.URL, query url.Values, header http.Header, body []byte) (*http.Response, error) { - return c.innerRequestWith(ctx, c.client, method, reqUrl, query, header, body) + return c.innerRequestWith(ctx, c.client, method, reqUrl, query, header, body, true) } -func (c *Spclient) innerRequestWith(ctx context.Context, client *http.Client, method string, reqUrl *url.URL, query url.Values, header http.Header, body []byte) (*http.Response, error) { +// innerRequestWith sends a request, renewing the access token on 401. With +// retryTransient it also repeats the request on network errors and transient +// statuses, which is only safe when the request is idempotent: the server may +// have acted on it before the failure. +func (c *Spclient) innerRequestWith(ctx context.Context, client *http.Client, method string, reqUrl *url.URL, query url.Values, header http.Header, body []byte, retryTransient bool) (*http.Response, error) { if query != nil { reqUrl.RawQuery = query.Encode() } @@ -101,7 +105,11 @@ func (c *Spclient) innerRequestWith(ctx context.Context, client *http.Client, me } if body != nil { - req.Header.Set("Content-Type", "application/x-protobuf") + // Most endpoints take plain protobuf, some (the collection service) + // a vendor type of their own, which the caller then sets. + if req.Header.Get("Content-Type") == "" { + req.Header.Set("Content-Type", "application/x-protobuf") + } req.GetBody = func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(body)), nil @@ -133,6 +141,9 @@ func (c *Spclient) innerRequestWith(ctx context.Context, client *http.Client, me resp, err := client.Do(req.WithContext(ctx)) if err != nil { + if !retryTransient { + return nil, backoff.Permanent(err) + } return nil, err } @@ -143,7 +154,7 @@ func (c *Spclient) innerRequestWith(ctx context.Context, client *http.Client, me return nil, fmt.Errorf("unauthorized") } - if isRetryableHTTPStatus(resp.StatusCode) { + if retryTransient && isRetryableHTTPStatus(resp.StatusCode) { status := resp.StatusCode _ = resp.Body.Close() c.log.Debugf( @@ -165,6 +176,16 @@ func (c *Spclient) innerRequestWith(ctx context.Context, client *http.Client, me return resp, nil } +// RequestOnce is Request for requests that are not idempotent: it does not +// repeat them on network errors or transient statuses, since the server may +// already have acted on them, and leaves such failures to the caller. A 401 +// is still answered by renewing the token and sending again, as the server +// refused the request. +func (c *Spclient) RequestOnce(ctx context.Context, method string, path string, query url.Values, header http.Header, body []byte) (*http.Response, error) { + reqUrl := c.baseUrl.JoinPath(path) + return c.innerRequestWith(ctx, c.client, method, reqUrl, query, header, body, false) +} + func (c *Spclient) Request(ctx context.Context, method string, path string, query url.Values, header http.Header, body []byte) (*http.Response, error) { reqUrl := c.baseUrl.JoinPath(path) return c.innerRequest(ctx, method, reqUrl, query, header, body) @@ -174,7 +195,7 @@ func (c *Spclient) Request(ctx context.Context, method string, path string, quer // following it, for endpoints that answer with a Location instead of a body. func (c *Spclient) RequestNoRedirect(ctx context.Context, method string, path string, query url.Values, header http.Header, body []byte) (*http.Response, error) { reqUrl := c.baseUrl.JoinPath(path) - return c.innerRequestWith(ctx, c.noRedirectClient, method, reqUrl, query, header, body) + return c.innerRequestWith(ctx, c.noRedirectClient, method, reqUrl, query, header, body, true) } // RequestHm issues a request against an hm:// URL, the form Spotify uses to name