From 5e5473533328143ef2f89a46d54985622cae7258 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 26 Sep 2026 13:25:14 +0200 Subject: [PATCH 01/11] fix: keep a caller's Content-Type on spclient requests Requests with a body always had their Content-Type overwritten with application/x-protobuf, so endpoints that expect another type could not be reached. Only default to protobuf when the caller set nothing. Co-Authored-By: Claude Opus 5.5 --- spclient/request_test.go | 9 +++++++++ spclient/spclient.go | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/spclient/request_test.go b/spclient/request_test.go index ab1e5dd4..ad711760 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) diff --git a/spclient/spclient.go b/spclient/spclient.go index db035955..1150fbae 100644 --- a/spclient/spclient.go +++ b/spclient/spclient.go @@ -101,7 +101,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 From e8c5772bd2123b7443ee7b952d445c006ff1bc16 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 26 Sep 2026 13:27:59 +0200 Subject: [PATCH 02/11] feat: save to Liked Songs and append to playlists via spclient Add the collection service's WriteRequest proto and Spclient.CollectionWrite, which adds items to or removes them from a collection set such as Liked Songs ("collection"), using the service's own vendor content type. Add Spclient.PlaylistRevision and Spclient.PlaylistAppend, which appends items as an ADD change based on the playlist's current revision and reports a 409 as ErrPlaylistConflict. Co-Authored-By: Claude Opus 5.5 --- proto/spotify/collection/v2/collection.pb.go | 208 +++++++++++++++++++ proto/spotify/collection/v2/collection.proto | 18 ++ spclient/library_write.go | 135 ++++++++++++ spclient/library_write_test.go | 106 ++++++++++ 4 files changed, 467 insertions(+) create mode 100644 proto/spotify/collection/v2/collection.pb.go create mode 100644 proto/spotify/collection/v2/collection.proto create mode 100644 spclient/library_write.go create mode 100644 spclient/library_write_test.go diff --git a/proto/spotify/collection/v2/collection.pb.go b/proto/spotify/collection/v2/collection.pb.go new file mode 100644 index 00000000..c9332b3e --- /dev/null +++ b/proto/spotify/collection/v2/collection.pb.go @@ -0,0 +1,208 @@ +// 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 +} + +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\x05itemsB\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, 2) +var file_spotify_collection_v2_collection_proto_goTypes = []any{ + (*CollectionItem)(nil), // 0: spotify.collection.v2.CollectionItem + (*WriteRequest)(nil), // 1: spotify.collection.v2.WriteRequest +} +var file_spotify_collection_v2_collection_proto_depIdxs = []int32{ + 0, // 0: spotify.collection.v2.WriteRequest.items:type_name -> spotify.collection.v2.CollectionItem + 1, // [1:1] is the sub-list for method output_type + 1, // [1:1] is the sub-list for method input_type + 1, // [1:1] is the sub-list for extension type_name + 1, // [1:1] is the sub-list for extension extendee + 0, // [0:1] 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: 2, + 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..23f784e6 --- /dev/null +++ b/proto/spotify/collection/v2/collection.proto @@ -0,0 +1,18 @@ +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; +} diff --git a/spclient/library_write.go b/spclient/library_write.go new file mode 100644 index 00000000..880ffc76 --- /dev/null +++ b/spclient/library_write.go @@ -0,0 +1,135 @@ +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" + +// 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 fmt.Errorf("invalid status code from collection write: %d", resp.StatusCode) + } + + return 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, fmt.Errorf("invalid status code from playlist: %d", 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) + } + + resp, err := c.Request(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 fmt.Errorf("invalid status code from playlist changes: %d", resp.StatusCode) + } +} diff --git a/spclient/library_write_test.go b/spclient/library_write_test.go new file mode 100644 index 00000000..05fa451d --- /dev/null +++ b/spclient/library_write_test.go @@ -0,0 +1,106 @@ +//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) +} From e589b1413bd7c63db67b7c89171d6d68f97bac56 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 26 Sep 2026 13:27:59 +0200 Subject: [PATCH 03/11] feat: add POST /library/liked and /library/playlists/add_tracks /library/liked saves tracks to or removes them from Liked Songs; /library/playlists/add_tracks appends tracks or episodes to a playlist, retrying once when the playlist changed in between. Both validate their URIs up front and run off the player loop. /library/playlists now reports can_edit, so clients can offer only the playlists the user may add to. Co-Authored-By: Claude Opus 5.5 --- api-spec.yml | 78 ++++++++++++++++++++++++ daemon/api_gen.go | 63 ++++++++++++++++++++ daemon/api_server.go | 51 +++++++++++++++- daemon/api_server_test.go | 121 +++++++++++++++++++++++++++++++------- daemon/library.go | 28 +++++++++ daemon/library_test.go | 9 +++ daemon/player.go | 29 +++++++++ 7 files changed, 356 insertions(+), 23 deletions(-) diff --git a/api-spec.yml b/api-spec.yml index ffbda67b..fafe14a4 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,34 @@ components: type: array items: $ref: '#/components/schemas/libraryPlaylist' + 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 @@ -660,6 +694,50 @@ paths: description: No active session 400: description: The offset or limit is out of range + /library/liked: + 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 + 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..0ad7f279 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"` @@ -134,6 +137,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 +179,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 @@ -324,6 +345,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 +396,15 @@ type ServerInterface interface { // (GET /events) GetEvents(w http.ResponseWriter, r *http.Request) + // (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 +545,20 @@ func (siw *ServerInterfaceWrapper) GetEvents(w http.ResponseWriter, r *http.Requ 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 +594,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 +984,9 @@ 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("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..612c1d35 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,8 @@ const ( ApiRequestTypeReopenOutput ApiRequestType = "reopen_output" ApiRequestTypeContextTracks ApiRequestType = "context_tracks" ApiRequestTypeLibraryPlaylists ApiRequestType = "library_playlists" + ApiRequestTypeSetLiked ApiRequestType = "set_liked" + ApiRequestTypePlaylistAddTracks ApiRequestType = "playlist_add_tracks" ) // ApiRequestDataContextTracks carries the uri query parameter of the context @@ -478,7 +482,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 +572,51 @@ func (s *ConcreteApiServer) GetLibraryPlaylists(w http.ResponseWriter, r *http.R s.handleRequest(ApiRequest{Type: ApiRequestTypeLibraryPlaylists, Data: ApiRequestDataLibraryPlaylists{Offset: params.Offset, Limit: params.Limit}}, w) } +// maxLibraryWriteUris caps how many items one library write may carry. +const maxLibraryWriteUris = 50 + +// validItemUris reports whether uris holds 1 to maxLibraryWriteUris URIs, all +// of one of the given types. +func validItemUris(uris []string, types ...librespot.SpotifyIdType) bool { + if len(uris) == 0 || len(uris) > maxLibraryWriteUris { + 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) SetLiked(w http.ResponseWriter, r *http.Request) { + var data ApiSetLiked + if err := jsonDecode(r, &data); err != nil || !validItemUris(data.Uris, librespot.SpotifyIdTypeTrack) { + w.WriteHeader(http.StatusBadRequest) + return + } + + s.handleRequest(ApiRequest{Type: ApiRequestTypeSetLiked, Data: data}, 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..3462efa5 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.MethodPost}, + "/library/playlists/add_tracks": {http.MethodPost}, } func TestApiRejectsWrongMethod(t *testing.T) { @@ -612,7 +615,81 @@ 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 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{ + "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..59af94ad 100644 --- a/daemon/library.go +++ b/daemon/library.go @@ -3,11 +3,13 @@ package daemon import ( "context" "encoding/hex" + "errors" "fmt" "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 +17,9 @@ import ( // libraryPlaylistsTimeout bounds fetching the rootlist for an API caller. const libraryPlaylistsTimeout = 30 * time.Second +// libraryWriteTimeout bounds a write to Liked Songs or a playlist. +const libraryWriteTimeout = 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 +107,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 +151,25 @@ 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 + } + } +} diff --git a/daemon/library_test.go b/daemon/library_test.go index 488ca7ff..4b5d28f2 100644 --- a/daemon/library_test.go +++ b/daemon/library_test.go @@ -6,6 +6,7 @@ import ( "testing" playlist4pb "github.com/devgianlu/go-librespot/proto/spotify/playlist4" + playlist_permissionpb "github.com/devgianlu/go-librespot/proto/spotify/playlist_permission" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" ) @@ -57,6 +58,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)) diff --git a/daemon/player.go b/daemon/player.go index e6f34b63..d9d72a1e 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,34 @@ 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(libraryWriteTimeout, func(ctx context.Context) { + if err := spc.CollectionWrite(ctx, username, spclient.CollectionSetLikedSongs, data.Uris, !data.Liked); err != nil { + reply.done(nil, fmt.Errorf("failed writing liked songs: %w", err)) + return + } + + reply.done(nil, nil) + }) + + return nil, errReplyDeferred + case ApiRequestTypePlaylistAddTracks: + data := req.Data.(ApiPlaylistAddTracks) + reply := apiReply(req) + spc, username := p.sess.Spclient(), p.sess.Username() + p.goDetached(libraryWriteTimeout, func(ctx context.Context) { + if err := appendToPlaylist(ctx, spc, username, data.PlaylistUri, data.Uris); err != nil { + reply.done(nil, fmt.Errorf("failed appending to playlist: %w", err)) + return + } + + reply.done(nil, nil) + }) + return nil, errReplyDeferred case ApiRequestSetDeviceName: p.setDeviceName(req.Data.(string)) From 412cce7034f2bdeab257edf28d56264224029c31 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 26 Sep 2026 15:40:55 +0200 Subject: [PATCH 04/11] feat: read collection sets page by page via spclient Add the collection service's PageRequest/PageResponse and Spclient.CollectionPage, which reads one page of a set such as Liked Songs and hands back the token of the next page. Co-Authored-By: Claude Opus 5.5 --- proto/spotify/collection/v2/collection.pb.go | 157 ++++++++++++++++++- proto/spotify/collection/v2/collection.proto | 13 ++ spclient/library_write.go | 40 +++++ spclient/library_write_test.go | 26 +++ 4 files changed, 228 insertions(+), 8 deletions(-) diff --git a/proto/spotify/collection/v2/collection.pb.go b/proto/spotify/collection/v2/collection.pb.go index c9332b3e..3cdc8861 100644 --- a/proto/spotify/collection/v2/collection.pb.go +++ b/proto/spotify/collection/v2/collection.pb.go @@ -141,6 +141,134 @@ func (x *WriteRequest) GetItems() []*CollectionItem { return nil } +type PageRequest 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"` + PaginationToken string `protobuf:"bytes,3,opt,name=pagination_token,json=paginationToken,proto3" json:"pagination_token,omitempty"` + Limit int32 `protobuf:"varint,4,opt,name=limit,proto3" json:"limit,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *PageRequest) Reset() { + *x = PageRequest{} + mi := &file_spotify_collection_v2_collection_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *PageRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*PageRequest) ProtoMessage() {} + +func (x *PageRequest) 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 PageRequest.ProtoReflect.Descriptor instead. +func (*PageRequest) Descriptor() ([]byte, []int) { + return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{2} +} + +func (x *PageRequest) GetUsername() string { + if x != nil { + return x.Username + } + return "" +} + +func (x *PageRequest) GetSet() string { + if x != nil { + return x.Set + } + return "" +} + +func (x *PageRequest) GetPaginationToken() string { + if x != nil { + return x.PaginationToken + } + return "" +} + +func (x *PageRequest) GetLimit() int32 { + if x != nil { + return x.Limit + } + return 0 +} + +type PageResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + Items []*CollectionItem `protobuf:"bytes,1,rep,name=items,proto3" json:"items,omitempty"` + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` + SyncToken string `protobuf:"bytes,3,opt,name=sync_token,json=syncToken,proto3" json:"sync_token,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *PageResponse) Reset() { + *x = PageResponse{} + mi := &file_spotify_collection_v2_collection_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *PageResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*PageResponse) ProtoMessage() {} + +func (x *PageResponse) 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 PageResponse.ProtoReflect.Descriptor instead. +func (*PageResponse) Descriptor() ([]byte, []int) { + return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{3} +} + +func (x *PageResponse) GetItems() []*CollectionItem { + if x != nil { + return x.Items + } + return nil +} + +func (x *PageResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + +func (x *PageResponse) GetSyncToken() string { + if x != nil { + return x.SyncToken + } + return "" +} + var File_spotify_collection_v2_collection_proto protoreflect.FileDescriptor const file_spotify_collection_v2_collection_proto_rawDesc = "" + @@ -154,7 +282,17 @@ const file_spotify_collection_v2_collection_proto_rawDesc = "" + "\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\x05itemsB\xee\x01\n" + + "\x05items\x18\x03 \x03(\v2%.spotify.collection.v2.CollectionItemR\x05items\"|\n" + + "\vPageRequest\x12\x1a\n" + + "\busername\x18\x01 \x01(\tR\busername\x12\x10\n" + + "\x03set\x18\x02 \x01(\tR\x03set\x12)\n" + + "\x10pagination_token\x18\x03 \x01(\tR\x0fpaginationToken\x12\x14\n" + + "\x05limit\x18\x04 \x01(\x05R\x05limit\"\x92\x01\n" + + "\fPageResponse\x12;\n" + + "\x05items\x18\x01 \x03(\v2%.spotify.collection.v2.CollectionItemR\x05items\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\x12\x1d\n" + + "\n" + + "sync_token\x18\x03 \x01(\tR\tsyncTokenB\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 ( @@ -169,18 +307,21 @@ func file_spotify_collection_v2_collection_proto_rawDescGZIP() []byte { return file_spotify_collection_v2_collection_proto_rawDescData } -var file_spotify_collection_v2_collection_proto_msgTypes = make([]protoimpl.MessageInfo, 2) +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 + (*PageRequest)(nil), // 2: spotify.collection.v2.PageRequest + (*PageResponse)(nil), // 3: spotify.collection.v2.PageResponse } var file_spotify_collection_v2_collection_proto_depIdxs = []int32{ 0, // 0: spotify.collection.v2.WriteRequest.items:type_name -> spotify.collection.v2.CollectionItem - 1, // [1:1] is the sub-list for method output_type - 1, // [1:1] is the sub-list for method input_type - 1, // [1:1] is the sub-list for extension type_name - 1, // [1:1] is the sub-list for extension extendee - 0, // [0:1] is the sub-list for field type_name + 0, // 1: spotify.collection.v2.PageResponse.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() } @@ -194,7 +335,7 @@ func file_spotify_collection_v2_collection_proto_init() { 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: 2, + NumMessages: 4, NumExtensions: 0, NumServices: 0, }, diff --git a/proto/spotify/collection/v2/collection.proto b/proto/spotify/collection/v2/collection.proto index 23f784e6..7246ac73 100644 --- a/proto/spotify/collection/v2/collection.proto +++ b/proto/spotify/collection/v2/collection.proto @@ -16,3 +16,16 @@ message WriteRequest { string set = 2; repeated CollectionItem items = 3; } + +message PageRequest { + string username = 1; + string set = 2; + string pagination_token = 3; + int32 limit = 4; +} + +message PageResponse { + repeated CollectionItem items = 1; + string next_page_token = 2; + string sync_token = 3; +} diff --git a/spclient/library_write.go b/spclient/library_write.go index 880ffc76..cfaf26e2 100644 --- a/spclient/library_write.go +++ b/spclient/library_write.go @@ -60,6 +60,46 @@ func (c *Spclient) CollectionWrite(ctx context.Context, username, set string, ur return nil } +// CollectionPage reads one page of a collection set; an empty token asks for +// the first page. The response names the next page, empty on the last one. +func (c *Spclient) CollectionPage(ctx context.Context, username, set, token string, limit int) (*collectionpb.PageResponse, error) { + body, err := proto.Marshal(&collectionpb.PageRequest{ + Username: username, + Set: set, + PaginationToken: token, + Limit: int32(limit), + }) + if err != nil { + return nil, fmt.Errorf("failed marshalling PageRequest: %w", err) + } + + resp, err := c.Request(ctx, "POST", "/collection/v2/paging", 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, fmt.Errorf("invalid status code from collection paging: %d", resp.StatusCode) + } + + respBytes, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed reading response body: %w", err) + } + + var page collectionpb.PageResponse + if err := proto.Unmarshal(respBytes, &page); err != nil { + return nil, fmt.Errorf("failed unmarshalling PageResponse: %w", err) + } + + return &page, 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) { diff --git a/spclient/library_write_test.go b/spclient/library_write_test.go index 05fa451d..7049de50 100644 --- a/spclient/library_write_test.go +++ b/spclient/library_write_test.go @@ -104,3 +104,29 @@ func (suite *RequestSuite) TestPlaylistAppendReportsConflict() { []string{"spotify:track:a"}) suite.ErrorIs(err, spclient.ErrPlaylistConflict) } + +func (suite *RequestSuite) TestCollectionPageReadsPage() { + suite.handler = func(_ int, w http.ResponseWriter) { + body, _ := proto.Marshal(&collectionpb.PageResponse{ + Items: []*collectionpb.CollectionItem{{Uri: "spotify:track:a", AddedAt: 1}}, + NextPageToken: "next", + }) + _, _ = w.Write(body) + } + + page, err := suite.spclient.CollectionPage(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, "tok", 300) + suite.Require().NoError(err) + suite.Equal("next", page.GetNextPageToken()) + suite.Require().Len(page.GetItems(), 1) + + got := suite.requests()[0] + suite.Equal("/collection/v2/paging", got.path) + suite.Equal("application/vnd.collection-v2.spotify.proto", got.header.Get("Content-Type")) + + var req collectionpb.PageRequest + suite.Require().NoError(proto.Unmarshal(got.body, &req)) + suite.Equal("user", req.GetUsername()) + suite.Equal("collection", req.GetSet()) + suite.Equal("tok", req.GetPaginationToken()) + suite.Equal(int32(300), req.GetLimit()) +} From 068c505616530d370d03f0079fdbd5ba476793d0 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 26 Sep 2026 15:40:55 +0200 Subject: [PATCH 05/11] feat: add GET /library/liked Tell whether tracks are in Liked Songs. The daemon reads the whole set through the collection service, keeps only tracks (the set also holds saved albums) and reuses it for a minute; writes through POST /library/liked are applied to it at once. Co-Authored-By: Claude Opus 5.5 --- api-spec.yml | 52 ++++++++++++++++++ daemon/api_gen.go | 59 ++++++++++++++++++++ daemon/api_server.go | 10 ++++ daemon/api_server_test.go | 28 +++++++++- daemon/app.go | 4 ++ daemon/liked.go | 110 ++++++++++++++++++++++++++++++++++++++ daemon/liked_test.go | 98 +++++++++++++++++++++++++++++++++ daemon/player.go | 16 ++++++ 8 files changed, 376 insertions(+), 1 deletion(-) create mode 100644 daemon/liked.go create mode 100644 daemon/liked_test.go diff --git a/api-spec.yml b/api-spec.yml index fafe14a4..7e111700 100644 --- a/api-spec.yml +++ b/api-spec.yml @@ -329,6 +329,29 @@ 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 @@ -695,6 +718,35 @@ paths: 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. The daemon reads + the whole set through the internal collection service and reuses it + for a minute, so changes made on other devices show up within that + time; changes made through POST /library/liked show up 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: >- diff --git a/daemon/api_gen.go b/daemon/api_gen.go index 0ad7f279..968359da 100644 --- a/daemon/api_gen.go +++ b/daemon/api_gen.go @@ -110,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. @@ -336,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 @@ -396,6 +417,9 @@ 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) @@ -545,6 +569,40 @@ 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) { @@ -984,6 +1042,7 @@ 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) diff --git a/daemon/api_server.go b/daemon/api_server.go index 612c1d35..8a848cb0 100644 --- a/daemon/api_server.go +++ b/daemon/api_server.go @@ -136,6 +136,7 @@ const ( ApiRequestTypeContextTracks ApiRequestType = "context_tracks" ApiRequestTypeLibraryPlaylists ApiRequestType = "library_playlists" ApiRequestTypeSetLiked ApiRequestType = "set_liked" + ApiRequestTypeGetLiked ApiRequestType = "get_liked" ApiRequestTypePlaylistAddTracks ApiRequestType = "playlist_add_tracks" ) @@ -592,6 +593,15 @@ func validItemUris(uris []string, types ...librespot.SpotifyIdType) bool { 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) { var data ApiSetLiked if err := jsonDecode(r, &data); err != nil || !validItemUris(data.Uris, librespot.SpotifyIdTypeTrack) { diff --git a/daemon/api_server_test.go b/daemon/api_server_test.go index 3462efa5..e2cd55d0 100644 --- a/daemon/api_server_test.go +++ b/daemon/api_server_test.go @@ -153,7 +153,7 @@ var endpointMethods = map[string][]string{ "/player/output": {http.MethodPost}, "/context/tracks": {http.MethodGet}, "/library/playlists": {http.MethodGet}, - "/library/liked": {http.MethodPost}, + "/library/liked": {http.MethodGet, http.MethodPost}, "/library/playlists/add_tracks": {http.MethodPost}, } @@ -619,6 +619,32 @@ func TestApiLibraryPlaylists(t *testing.T) { }) } +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) diff --git a/daemon/app.go b/daemon/app.go index 32bb42f7..9cb25f51 100644 --- a/daemon/app.go +++ b/daemon/app.go @@ -52,6 +52,9 @@ type App struct { metaCache *trackMetaCache contextLists *contextListCache + // liked caches the Liked Songs set for GET /library/liked. + liked *likedTracks + // stateMu guards state and the store behind it. Two AppPlayers overlap // briefly whenever a zeroconf session is replaced, and volume changes are // written back from off the player loop. @@ -86,6 +89,7 @@ func New(opts *Options) (*App, error) { } app := &App{ + liked: newLikedTracks(), log: opts.Logger, cfg: opts.Config, stateStore: opts.StateStore, diff --git a/daemon/liked.go b/daemon/liked.go new file mode 100644 index 00000000..df27e4b0 --- /dev/null +++ b/daemon/liked.go @@ -0,0 +1,110 @@ +package daemon + +import ( + "context" + "fmt" + "strings" + "sync" + "time" + + "github.com/devgianlu/go-librespot/spclient" +) + +// likedTracksTTL bounds how long the Liked Songs set is trusted before it is +// read again, which is how changes made on other devices get picked up. +const likedTracksTTL = time.Minute + +// collectionPageSize is how many items are asked for per collection page. +const collectionPageSize = 500 + +// collectionMaxPages caps the pages read for one set, in case the service +// keeps handing out page tokens. +const collectionMaxPages = 100 + +// likedTracks caches the track URIs in the user's Liked Songs. The set is +// read whole, so answering whether a few tracks are liked costs nothing while +// it is fresh. +type likedTracks struct { + mu sync.Mutex + username string + uris map[string]bool + fetched time.Time + + // now is the clock the TTL is checked against; replaced by tests. + now func() time.Time +} + +func newLikedTracks() *likedTracks { + return &likedTracks{now: time.Now} +} + +// likedFetcher reads the complete Liked Songs set of a user. +type likedFetcher func(ctx context.Context, username string) (map[string]bool, error) + +// contains reports for each uri whether it is liked, reading the set first +// when it is stale or belongs to another user. Concurrent callers wait for +// one read rather than each starting their own. +func (l *likedTracks) contains(ctx context.Context, username string, uris []string, fetch likedFetcher) ([]ApiLikedState, error) { + l.mu.Lock() + defer l.mu.Unlock() + + if l.uris == nil || l.username != username || l.now().Sub(l.fetched) > likedTracksTTL { + set, err := fetch(ctx, username) + if err != nil { + return nil, err + } + l.username, l.uris, l.fetched = username, set, l.now() + } + + states := make([]ApiLikedState, len(uris)) + for i, uri := range uris { + states[i] = ApiLikedState{Uri: uri, Liked: l.uris[uri]} + } + return states, nil +} + +// apply records a write made through this daemon, so it is visible before +// the next read of the set. +func (l *likedTracks) apply(username string, uris []string, liked bool) { + l.mu.Lock() + defer l.mu.Unlock() + + if l.uris == nil || l.username != username { + return + } + for _, uri := range uris { + if liked { + l.uris[uri] = true + } else { + delete(l.uris, uri) + } + } +} + +// fetchLikedTracks pages through the user's Liked Songs collection set, which +// also holds saved albums; only tracks are kept. +func fetchLikedTracks(spc *spclient.Spclient) likedFetcher { + return func(ctx context.Context, username string) (map[string]bool, error) { + set := map[string]bool{} + token := "" + for page := 0; page < collectionMaxPages; page++ { + resp, err := spc.CollectionPage(ctx, username, spclient.CollectionSetLikedSongs, token, collectionPageSize) + if err != nil { + return nil, err + } + + for _, item := range resp.GetItems() { + if !item.GetIsRemoved() && strings.HasPrefix(item.GetUri(), "spotify:track:") { + set[item.GetUri()] = true + } + } + + token = resp.GetNextPageToken() + if token == "" { + return set, nil + } + } + + return nil, fmt.Errorf("liked songs still paging after %d pages", collectionMaxPages) + } +} diff --git a/daemon/liked_test.go b/daemon/liked_test.go new file mode 100644 index 00000000..0550823e --- /dev/null +++ b/daemon/liked_test.go @@ -0,0 +1,98 @@ +//go:build test_unit + +package daemon + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +type fakeLikedFetcher struct { + calls int + sets map[string]map[string]bool + err error +} + +func (f *fakeLikedFetcher) fetch(_ context.Context, username string) (map[string]bool, error) { + f.calls++ + if f.err != nil { + return nil, f.err + } + set := map[string]bool{} + for uri := range f.sets[username] { + set[uri] = true + } + return set, nil +} + +func likedFlags(states []ApiLikedState) []bool { + out := make([]bool, len(states)) + for i, s := range states { + out[i] = s.Liked + } + return out +} + +func TestLikedTracksContains(t *testing.T) { + now := time.Unix(0, 0) + l := newLikedTracks() + l.now = func() time.Time { return now } + f := &fakeLikedFetcher{sets: map[string]map[string]bool{ + "alice": {"spotify:track:a": true}, + "bob": {"spotify:track:b": true}, + }} + + states, err := l.contains(context.Background(), "alice", []string{"spotify:track:a", "spotify:track:b"}, f.fetch) + require.NoError(t, err) + require.Equal(t, []bool{true, false}, likedFlags(states)) + require.Equal(t, "spotify:track:b", states[1].Uri) + + _, err = l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) + require.NoError(t, err) + require.Equal(t, 1, f.calls, "a fresh set is reused") + + now = now.Add(likedTracksTTL + time.Second) + _, err = l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) + require.NoError(t, err) + require.Equal(t, 2, f.calls, "a stale set is read again") + + states, err = l.contains(context.Background(), "bob", []string{"spotify:track:a", "spotify:track:b"}, f.fetch) + require.NoError(t, err) + require.Equal(t, 3, f.calls, "another user's set is never reused") + require.Equal(t, []bool{false, true}, likedFlags(states)) +} + +func TestLikedTracksApply(t *testing.T) { + l := newLikedTracks() + f := &fakeLikedFetcher{sets: map[string]map[string]bool{"alice": {"spotify:track:a": true}}} + + l.apply("alice", []string{"spotify:track:x"}, true) // nothing loaded yet: ignored + _, err := l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) + require.NoError(t, err) + + l.apply("alice", []string{"spotify:track:x"}, true) + l.apply("alice", []string{"spotify:track:a"}, false) + l.apply("bob", []string{"spotify:track:a"}, true) // other user: ignored + + states, err := l.contains(context.Background(), "alice", []string{"spotify:track:a", "spotify:track:x"}, f.fetch) + require.NoError(t, err) + require.Equal(t, []bool{false, true}, likedFlags(states)) + require.Equal(t, 1, f.calls) +} + +func TestLikedTracksFetchError(t *testing.T) { + l := newLikedTracks() + f := &fakeLikedFetcher{err: errors.New("boom")} + + _, err := l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) + require.Error(t, err) + + f.err, f.sets = nil, map[string]map[string]bool{"alice": {"spotify:track:a": true}} + states, err := l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) + require.NoError(t, err) + require.True(t, states[0].Liked, "a failed read is not cached") +} diff --git a/daemon/player.go b/daemon/player.go index d9d72a1e..e088aadd 100644 --- a/daemon/player.go +++ b/daemon/player.go @@ -912,9 +912,25 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { return } + p.app.liked.apply(username, data.Uris, data.Liked) 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(libraryWriteTimeout, func(ctx context.Context) { + states, err := p.app.liked.contains(ctx, username, uris, fetchLikedTracks(spc)) + if err != nil { + reply.done(nil, fmt.Errorf("failed reading liked songs: %w", err)) + return + } + + reply.done(&ApiLikedStates{Items: states}, nil) + }) + return nil, errReplyDeferred case ApiRequestTypePlaylistAddTracks: data := req.Data.(ApiPlaylistAddTracks) From 8868c05d527f08eab863ac6805b5b6176502d715 Mon Sep 17 00:00:00 2001 From: jmt Date: Thu, 1 Oct 2026 12:35:53 +0200 Subject: [PATCH 06/11] fix: list a context again after writing to it /context/tracks reuses a listing for five minutes, so a track appended through /library/playlists/add_tracks, or saved to or removed from Liked Songs through /library/liked, stayed missing from the listing a client asks for right after. Drop the listing of the changed context after a successful write; an enumeration already running at that moment may have read the context before the change, so its result is not cached either. Co-Authored-By: Claude Opus 5.5 --- api-spec.yml | 7 ++++--- daemon/liked.go | 5 +++++ daemon/player.go | 4 ++++ daemon/track_meta_cache.go | 30 +++++++++++++++++++++++++-- daemon/track_meta_cache_test.go | 36 +++++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/api-spec.yml b/api-spec.yml index 7e111700..028ee824 100644 --- a/api-spec.yml +++ b/api-spec.yml @@ -654,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 diff --git a/daemon/liked.go b/daemon/liked.go index df27e4b0..fe2c8676 100644 --- a/daemon/liked.go +++ b/daemon/liked.go @@ -81,6 +81,11 @@ func (l *likedTracks) apply(username string, uris []string, liked bool) { } } +// likedSongsContextUris are the context URIs Liked Songs is listed under. +func likedSongsContextUris(username string) []string { + return []string{"spotify:user:" + username + ":collection", "spotify:collection:tracks"} +} + // fetchLikedTracks pages through the user's Liked Songs collection set, which // also holds saved albums; only tracks are kept. func fetchLikedTracks(spc *spclient.Spclient) likedFetcher { diff --git a/daemon/player.go b/daemon/player.go index e088aadd..f3a16f26 100644 --- a/daemon/player.go +++ b/daemon/player.go @@ -913,6 +913,9 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { } p.app.liked.apply(username, data.Uris, data.Liked) + for _, uri := range likedSongsContextUris(username) { + p.app.contextLists.invalidate(uri) + } reply.done(nil, nil) }) @@ -942,6 +945,7 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { return } + p.app.contextLists.invalidate(data.PlaylistUri) reply.done(nil, nil) }) diff --git a/daemon/track_meta_cache.go b/daemon/track_meta_cache.go index c37e33dd..462554d7 100644 --- a/daemon/track_meta_cache.go +++ b/daemon/track_meta_cache.go @@ -131,11 +131,14 @@ type contextListCache struct { // polling every second does not spawn an enumeration per poll. mu sync.Mutex inFlight map[string]bool + // stale marks in-flight enumerations that began before an invalidation; + // their result may predate the change, so it is not cached. + stale map[string]bool } 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]bool{}, stale: map[string]bool{}} } func (c *contextListCache) get(uri string) ([]string, bool) { @@ -151,7 +154,30 @@ func (c *contextListCache) get(uri string) ([]string, bool) { } func (c *contextListCache) put(uri string, uris []string) { - c.lru.Add(uri, contextListEntry{uris: uris, fetched: c.now()}) + c.mu.Lock() + stale := c.stale[uri] + delete(c.stale, uri) + c.mu.Unlock() + + if !stale { + c.lru.Add(uri, contextListEntry{uris: uris, fetched: c.now()}) + } +} + +// 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 + } + c.lru.Remove(uri) + + c.mu.Lock() + defer c.mu.Unlock() + if c.inFlight[uri] { + c.stale[uri] = true + } } // beginFetch claims the right to enumerate uri, reporting false when the diff --git a/daemon/track_meta_cache_test.go b/daemon/track_meta_cache_test.go index 89cf7cc8..c19bb163 100644 --- a/daemon/track_meta_cache_test.go +++ b/daemon/track_meta_cache_test.go @@ -211,6 +211,42 @@ func TestContextListCacheServesRepeatLookups(t *testing.T) { 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"}) + + c.invalidate("spotify:playlist:a") + _, ok := c.get("spotify:playlist:a") + require.False(t, ok, "an invalidated listing is a miss") + require.True(t, c.beginFetch("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() + require.True(t, c.beginFetch("spotify:playlist:a")) + + c.invalidate("spotify:playlist:a") + c.put("spotify:playlist:a", []string{"spotify:track:old"}) + c.endFetch("spotify:playlist:a") + + _, ok := c.get("spotify:playlist:a") + require.False(t, ok, "the possibly stale result was dropped") + + require.True(t, c.beginFetch("spotify:playlist:a")) + c.put("spotify:playlist:a", []string{"spotify:track:new"}) + 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) +} + // 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) { From 0012490ac41644ba4517b296caff6f36ab1eb85d Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 3 Oct 2026 14:51:18 +0200 Subject: [PATCH 07/11] fix: require liked in POST /library/liked A body without liked, or with liked: null, decoded to liked=false and silently removed the tracks from Liked Songs with a 200, although the spec marks the field required. Reject it with 400. Co-Authored-By: Claude Opus 5.5 --- daemon/api_server.go | 11 ++++++++--- daemon/api_server_test.go | 10 ++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/daemon/api_server.go b/daemon/api_server.go index 8a848cb0..33fed06d 100644 --- a/daemon/api_server.go +++ b/daemon/api_server.go @@ -603,13 +603,18 @@ func (s *ConcreteApiServer) GetLiked(w http.ResponseWriter, _ *http.Request, par } func (s *ConcreteApiServer) SetLiked(w http.ResponseWriter, r *http.Request) { - var data ApiSetLiked - if err := jsonDecode(r, &data); err != nil || !validItemUris(data.Uris, librespot.SpotifyIdTypeTrack) { + // 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: data}, w) + s.handleRequest(ApiRequest{Type: ApiRequestTypeSetLiked, Data: ApiSetLiked{Uris: data.Uris, Liked: *data.Liked}}, w) } func (s *ConcreteApiServer) PlaylistAddTracks(w http.ResponseWriter, r *http.Request) { diff --git a/daemon/api_server_test.go b/daemon/api_server_test.go index e2cd55d0..89a7b58e 100644 --- a/daemon/api_server_test.go +++ b/daemon/api_server_test.go @@ -662,10 +662,12 @@ func TestApiSetLiked(t *testing.T) { tooMany[i] = "spotify:track:4uLU6hMCjMI75M1A2tKUQC" } for name, payload := range map[string]any{ - "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}, + "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) From 852d4ba4c289e93ed6b74828a2c4335fb2b80939 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 3 Oct 2026 14:52:13 +0200 Subject: [PATCH 08/11] fix: do not resend playlist changes on transient failures Requests went through innerRequestWith, which repeats them on network errors and 5xx. A playlist change is not idempotent: if the server applied an append and then answered 503/504, or the connection dropped before the answer, the same change was sent again and the tracks landed twice. Add Spclient.RequestOnce, which still renews the token on 401 (the server refused the request) but leaves other failures to the caller, and send playlist changes through it. appendToPlaylist only resends after a 409, which means the change was not applied. Co-Authored-By: Claude Opus 5.5 --- spclient/library_write.go | 4 +++- spclient/library_write_test.go | 10 ++++++++++ spclient/request_test.go | 31 +++++++++++++++++++++++++++++++ spclient/spclient.go | 25 +++++++++++++++++++++---- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/spclient/library_write.go b/spclient/library_write.go index cfaf26e2..c56653ac 100644 --- a/spclient/library_write.go +++ b/spclient/library_write.go @@ -157,7 +157,9 @@ func (c *Spclient) PlaylistAppend(ctx context.Context, playlist librespot.Spotif return fmt.Errorf("failed marshalling ListChanges: %w", err) } - resp, err := c.Request(ctx, "POST", fmt.Sprintf("/playlist/v2/playlist/%s/changes", playlist.Base62()), nil, nil, body) + // 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 } diff --git a/spclient/library_write_test.go b/spclient/library_write_test.go index 7049de50..799e8433 100644 --- a/spclient/library_write_test.go +++ b/spclient/library_write_test.go @@ -130,3 +130,13 @@ func (suite *RequestSuite) TestCollectionPageReadsPage() { suite.Equal("tok", req.GetPaginationToken()) suite.Equal(int32(300), req.GetLimit()) } + +// 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 ad711760..f86f407c 100644 --- a/spclient/request_test.go +++ b/spclient/request_test.go @@ -421,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 1150fbae..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() } @@ -137,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 } @@ -147,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( @@ -169,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) @@ -178,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 From 3c44d9e0db12ff726f3f02bf0e417904f7f42b53 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 3 Oct 2026 14:53:37 +0200 Subject: [PATCH 09/11] fix: close races when invalidating a context listing The stale flag that kept an enumeration's possibly pre-change result out of the cache had two holes. It was only cleared in put, so an invalidated enumeration that failed, or an invalidation landing between put and endFetch, left it set and the next, correct enumeration was dropped. And put read the flag under the mutex but added to the LRU after unlocking, while invalidate removed from the LRU before locking, so a pre-change listing could still be cached after the invalidation. Track the generation each in-flight enumeration started at instead: invalidate bumps it, put caches only a result whose generation is current, and both change the LRU under the same mutex. The generation goes with the enumeration in endFetch, so nothing is left behind. Co-Authored-By: Claude Opus 5.5 --- daemon/track_meta_cache.go | 58 ++++++++++--------- daemon/track_meta_cache_test.go | 99 +++++++++++++++++++++++++++------ 2 files changed, 113 insertions(+), 44 deletions(-) diff --git a/daemon/track_meta_cache.go b/daemon/track_meta_cache.go index 462554d7..d16e3839 100644 --- a/daemon/track_meta_cache.go +++ b/daemon/track_meta_cache.go @@ -127,18 +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 - // stale marks in-flight enumerations that began before an invalidation; - // their result may predate the change, so it is not cached. - stale 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{}, stale: map[string]bool{}} + return &contextListCache{lru: l, now: time.Now, inFlight: map[string]uint64{}} } func (c *contextListCache) get(uri string) ([]string, bool) { @@ -153,15 +153,16 @@ 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() - stale := c.stale[uri] - delete(c.stale, uri) - c.mu.Unlock() + defer c.mu.Unlock() - if !stale { - c.lru.Add(uri, contextListEntry{uris: uris, fetched: c.now()}) + if started, ok := c.inFlight[uri]; ok && started != gen { + return } + c.lru.Add(uri, contextListEntry{uris: uris, fetched: c.now()}) } // invalidate drops the listing of uri after it was changed through the @@ -171,33 +172,35 @@ func (c *contextListCache) invalidate(uri string) { if c == nil { return } - c.lru.Remove(uri) c.mu.Lock() defer c.mu.Unlock() - if c.inFlight[uri] { - c.stale[uri] = true + + c.lru.Remove(uri) + if gen, ok := c.inFlight[uri]; ok { + c.inFlight[uri] = gen + 1 } } -// 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 { +// 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 false + 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) { @@ -564,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 } @@ -587,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 c19bb163..3d5821b8 100644 --- a/daemon/track_meta_cache_test.go +++ b/daemon/track_meta_cache_test.go @@ -198,13 +198,18 @@ 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) @@ -215,12 +220,12 @@ func TestContextListCacheServesRepeatLookups(t *testing.T) { // served from the cache afterwards. func TestContextListCacheInvalidate(t *testing.T) { c := newContextListCache() - c.put("spotify:playlist:a", []string{"spotify:track:1"}) + 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, c.beginFetch("spotify:playlist:a"), "and is enumerated again") + require.True(t, claimed(c, "spotify:playlist:a"), "and is enumerated again") var nilCache *contextListCache nilCache.invalidate("spotify:playlist:a") // metadata disabled: no-op @@ -230,23 +235,83 @@ func TestContextListCacheInvalidate(t *testing.T) { // read it before the change; its result must not land in the cache. func TestContextListCacheInvalidateDuringEnumeration(t *testing.T) { c := newContextListCache() - require.True(t, c.beginFetch("spotify:playlist:a")) + 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"}) + c.put("spotify:playlist:a", []string{"spotify:track:old"}, gen) c.endFetch("spotify:playlist:a") - _, ok := c.get("spotify:playlist:a") + _, ok = c.get("spotify:playlist:a") require.False(t, ok, "the possibly stale result was dropped") - require.True(t, c.beginFetch("spotify:playlist:a")) - c.put("spotify:playlist:a", []string{"spotify:track:new"}) + 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) { @@ -255,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") @@ -264,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()) @@ -289,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 @@ -313,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{}} From cbcbb2c2e73071f109337458e842fe4d9b2f105f Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 3 Oct 2026 14:54:32 +0200 Subject: [PATCH 10/11] fix: answer 403 and 404 for playlists the user may not edit or that are gone The collection and playlist services' 403 and 404 came back as generic "invalid status code" errors, so the API answered 500 and logged them at Error level; clients could not tell a permission problem from a fault. Return a spclient.StatusError carrying the status, map 403 and 404 to ErrForbidden and ErrNotFound for the library writes, and document both for /library/playlists/add_tracks. Co-Authored-By: Claude Opus 5.5 --- api-spec.yml | 4 ++++ daemon/library.go | 17 +++++++++++++++++ daemon/library_test.go | 14 ++++++++++++++ daemon/player.go | 4 ++-- spclient/library_write.go | 20 ++++++++++++++++---- 5 files changed, 53 insertions(+), 6 deletions(-) diff --git a/api-spec.yml b/api-spec.yml index 028ee824..75ff12e9 100644 --- a/api-spec.yml +++ b/api-spec.yml @@ -789,6 +789,10 @@ paths: 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: diff --git a/daemon/library.go b/daemon/library.go index 59af94ad..d6c56f76 100644 --- a/daemon/library.go +++ b/daemon/library.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "errors" "fmt" + "net/http" "net/url" "strings" "time" @@ -173,3 +174,19 @@ func appendToPlaylist(ctx context.Context, spc *spclient.Spclient, username, pla } } } + +// 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 4b5d28f2..b6ca4718 100644 --- a/daemon/library_test.go +++ b/daemon/library_test.go @@ -7,6 +7,7 @@ import ( 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" ) @@ -133,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/player.go b/daemon/player.go index f3a16f26..931fa808 100644 --- a/daemon/player.go +++ b/daemon/player.go @@ -908,7 +908,7 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { spc, username := p.sess.Spclient(), p.sess.Username() p.goDetached(libraryWriteTimeout, func(ctx context.Context) { if err := spc.CollectionWrite(ctx, username, spclient.CollectionSetLikedSongs, data.Uris, !data.Liked); err != nil { - reply.done(nil, fmt.Errorf("failed writing liked songs: %w", err)) + reply.done(nil, libraryError("failed writing liked songs", err)) return } @@ -941,7 +941,7 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { spc, username := p.sess.Spclient(), p.sess.Username() p.goDetached(libraryWriteTimeout, func(ctx context.Context) { if err := appendToPlaylist(ctx, spc, username, data.PlaylistUri, data.Uris); err != nil { - reply.done(nil, fmt.Errorf("failed appending to playlist: %w", err)) + reply.done(nil, libraryError("failed appending to playlist", err)) return } diff --git a/spclient/library_write.go b/spclient/library_write.go index c56653ac..98bd614e 100644 --- a/spclient/library_write.go +++ b/spclient/library_write.go @@ -21,6 +21,18 @@ 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") @@ -54,7 +66,7 @@ func (c *Spclient) CollectionWrite(ctx context.Context, username, set string, ur defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return fmt.Errorf("invalid status code from collection write: %d", resp.StatusCode) + return &StatusError{Op: "collection write", StatusCode: resp.StatusCode} } return nil @@ -84,7 +96,7 @@ func (c *Spclient) CollectionPage(ctx context.Context, username, set, token stri defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("invalid status code from collection paging: %d", resp.StatusCode) + return nil, &StatusError{Op: "collection paging", StatusCode: resp.StatusCode} } respBytes, err := io.ReadAll(resp.Body) @@ -116,7 +128,7 @@ func (c *Spclient) PlaylistRevision(ctx context.Context, playlist librespot.Spot defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("invalid status code from playlist: %d", resp.StatusCode) + return nil, &StatusError{Op: "playlist", StatusCode: resp.StatusCode} } respBytes, err := io.ReadAll(resp.Body) @@ -172,6 +184,6 @@ func (c *Spclient) PlaylistAppend(ctx context.Context, playlist librespot.Spotif case http.StatusConflict: return ErrPlaylistConflict default: - return fmt.Errorf("invalid status code from playlist changes: %d", resp.StatusCode) + return &StatusError{Op: "playlist changes", StatusCode: resp.StatusCode} } } From 0a90214a566b4b5a0eae9d42b2639b43904bd243 Mon Sep 17 00:00:00 2001 From: jmt Date: Sat, 3 Oct 2026 14:56:08 +0200 Subject: [PATCH 11/11] fix: tell liked tracks with the collection service's contains GET /library/liked paged through the whole collection, albums included, to answer for at most 50 tracks: up to 100 sequential pages, again after every minute, holding a lock across the fetch that also delayed the 200 of a successful like, and failing with 500 on libraries past the page cap. A track requested by its 21-character id was also reported as not liked, since the collection holds canonical URIs. Ask /collection/v2/contains for just the requested tracks instead, in one round trip, with their canonical URIs; the answer still names them as requested. The cache, its lock and the collection paging go, and changes made on other devices show at once. The request limit and timeout constants are renamed, as reads use them too. Co-Authored-By: Claude Opus 5.5 --- api-spec.yml | 7 +- daemon/api_server.go | 8 +- daemon/app.go | 4 - daemon/library.go | 4 +- daemon/liked.go | 116 ++++--------------- daemon/liked_test.go | 94 +++------------ daemon/player.go | 14 ++- proto/spotify/collection/v2/collection.pb.go | 112 +++++++----------- proto/spotify/collection/v2/collection.proto | 11 +- spclient/library_write.go | 35 +++--- spclient/library_write_test.go | 32 +++-- 11 files changed, 139 insertions(+), 298 deletions(-) diff --git a/api-spec.yml b/api-spec.yml index 75ff12e9..3791a793 100644 --- a/api-spec.yml +++ b/api-spec.yml @@ -722,10 +722,9 @@ paths: get: operationId: getLiked description: >- - Tell whether tracks are in the user's Liked Songs. The daemon reads - the whole set through the internal collection service and reuses it - for a minute, so changes made on other devices show up within that - time; changes made through POST /library/liked show up at once. + 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 diff --git a/daemon/api_server.go b/daemon/api_server.go index 33fed06d..c49805dc 100644 --- a/daemon/api_server.go +++ b/daemon/api_server.go @@ -573,13 +573,13 @@ func (s *ConcreteApiServer) GetLibraryPlaylists(w http.ResponseWriter, r *http.R s.handleRequest(ApiRequest{Type: ApiRequestTypeLibraryPlaylists, Data: ApiRequestDataLibraryPlaylists{Offset: params.Offset, Limit: params.Limit}}, w) } -// maxLibraryWriteUris caps how many items one library write may carry. -const maxLibraryWriteUris = 50 +// maxLibraryUris caps how many items one library request may name. +const maxLibraryUris = 50 -// validItemUris reports whether uris holds 1 to maxLibraryWriteUris URIs, all +// 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) > maxLibraryWriteUris { + if len(uris) == 0 || len(uris) > maxLibraryUris { return false } diff --git a/daemon/app.go b/daemon/app.go index 9cb25f51..32bb42f7 100644 --- a/daemon/app.go +++ b/daemon/app.go @@ -52,9 +52,6 @@ type App struct { metaCache *trackMetaCache contextLists *contextListCache - // liked caches the Liked Songs set for GET /library/liked. - liked *likedTracks - // stateMu guards state and the store behind it. Two AppPlayers overlap // briefly whenever a zeroconf session is replaced, and volume changes are // written back from off the player loop. @@ -89,7 +86,6 @@ func New(opts *Options) (*App, error) { } app := &App{ - liked: newLikedTracks(), log: opts.Logger, cfg: opts.Config, stateStore: opts.StateStore, diff --git a/daemon/library.go b/daemon/library.go index d6c56f76..e4451306 100644 --- a/daemon/library.go +++ b/daemon/library.go @@ -18,8 +18,8 @@ import ( // libraryPlaylistsTimeout bounds fetching the rootlist for an API caller. const libraryPlaylistsTimeout = 30 * time.Second -// libraryWriteTimeout bounds a write to Liked Songs or a playlist. -const libraryWriteTimeout = 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 diff --git a/daemon/liked.go b/daemon/liked.go index fe2c8676..85a855ac 100644 --- a/daemon/liked.go +++ b/daemon/liked.go @@ -3,113 +3,39 @@ package daemon import ( "context" "fmt" - "strings" - "sync" - "time" - "github.com/devgianlu/go-librespot/spclient" + librespot "github.com/devgianlu/go-librespot" ) -// likedTracksTTL bounds how long the Liked Songs set is trusted before it is -// read again, which is how changes made on other devices get picked up. -const likedTracksTTL = time.Minute - -// collectionPageSize is how many items are asked for per collection page. -const collectionPageSize = 500 - -// collectionMaxPages caps the pages read for one set, in case the service -// keeps handing out page tokens. -const collectionMaxPages = 100 - -// likedTracks caches the track URIs in the user's Liked Songs. The set is -// read whole, so answering whether a few tracks are liked costs nothing while -// it is fresh. -type likedTracks struct { - mu sync.Mutex - username string - uris map[string]bool - fetched time.Time - - // now is the clock the TTL is checked against; replaced by tests. - now func() time.Time -} - -func newLikedTracks() *likedTracks { - return &likedTracks{now: time.Now} +// likedSongsContextUris are the context URIs Liked Songs is listed under. +func likedSongsContextUris(username string) []string { + return []string{"spotify:user:" + username + ":collection", "spotify:collection:tracks"} } -// likedFetcher reads the complete Liked Songs set of a user. -type likedFetcher func(ctx context.Context, username string) (map[string]bool, error) - -// contains reports for each uri whether it is liked, reading the set first -// when it is stale or belongs to another user. Concurrent callers wait for -// one read rather than each starting their own. -func (l *likedTracks) contains(ctx context.Context, username string, uris []string, fetch likedFetcher) ([]ApiLikedState, error) { - l.mu.Lock() - defer l.mu.Unlock() +// likedContains asks the collection service which of uris are in Liked Songs. +type likedContains func(ctx context.Context, uris []string) ([]bool, error) - if l.uris == nil || l.username != username || l.now().Sub(l.fetched) > likedTracksTTL { - set, err := fetch(ctx, username) +// 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, err + return nil, fmt.Errorf("invalid track uri %s: %w", uri, err) } - l.username, l.uris, l.fetched = username, set, l.now() + 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: l.uris[uri]} + states[i] = ApiLikedState{Uri: uri, Liked: found[i]} } return states, nil } - -// apply records a write made through this daemon, so it is visible before -// the next read of the set. -func (l *likedTracks) apply(username string, uris []string, liked bool) { - l.mu.Lock() - defer l.mu.Unlock() - - if l.uris == nil || l.username != username { - return - } - for _, uri := range uris { - if liked { - l.uris[uri] = true - } else { - delete(l.uris, uri) - } - } -} - -// likedSongsContextUris are the context URIs Liked Songs is listed under. -func likedSongsContextUris(username string) []string { - return []string{"spotify:user:" + username + ":collection", "spotify:collection:tracks"} -} - -// fetchLikedTracks pages through the user's Liked Songs collection set, which -// also holds saved albums; only tracks are kept. -func fetchLikedTracks(spc *spclient.Spclient) likedFetcher { - return func(ctx context.Context, username string) (map[string]bool, error) { - set := map[string]bool{} - token := "" - for page := 0; page < collectionMaxPages; page++ { - resp, err := spc.CollectionPage(ctx, username, spclient.CollectionSetLikedSongs, token, collectionPageSize) - if err != nil { - return nil, err - } - - for _, item := range resp.GetItems() { - if !item.GetIsRemoved() && strings.HasPrefix(item.GetUri(), "spotify:track:") { - set[item.GetUri()] = true - } - } - - token = resp.GetNextPageToken() - if token == "" { - return set, nil - } - } - - return nil, fmt.Errorf("liked songs still paging after %d pages", collectionMaxPages) - } -} diff --git a/daemon/liked_test.go b/daemon/liked_test.go index 0550823e..b02c9956 100644 --- a/daemon/liked_test.go +++ b/daemon/liked_test.go @@ -6,93 +6,31 @@ import ( "context" "errors" "testing" - "time" "github.com/stretchr/testify/require" ) -type fakeLikedFetcher struct { - calls int - sets map[string]map[string]bool - err error -} - -func (f *fakeLikedFetcher) fetch(_ context.Context, username string) (map[string]bool, error) { - f.calls++ - if f.err != nil { - return nil, f.err - } - set := map[string]bool{} - for uri := range f.sets[username] { - set[uri] = true - } - return set, nil -} - -func likedFlags(states []ApiLikedState) []bool { - out := make([]bool, len(states)) - for i, s := range states { - out[i] = s.Liked +func TestLikedStatesAsksWithCanonicalUris(t *testing.T) { + var asked []string + contains := func(_ context.Context, uris []string) ([]bool, error) { + asked = uris + return []bool{true, false}, nil } - return out -} - -func TestLikedTracksContains(t *testing.T) { - now := time.Unix(0, 0) - l := newLikedTracks() - l.now = func() time.Time { return now } - f := &fakeLikedFetcher{sets: map[string]map[string]bool{ - "alice": {"spotify:track:a": true}, - "bob": {"spotify:track:b": true}, - }} - - states, err := l.contains(context.Background(), "alice", []string{"spotify:track:a", "spotify:track:b"}, f.fetch) - require.NoError(t, err) - require.Equal(t, []bool{true, false}, likedFlags(states)) - require.Equal(t, "spotify:track:b", states[1].Uri) - - _, err = l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) - require.NoError(t, err) - require.Equal(t, 1, f.calls, "a fresh set is reused") - now = now.Add(likedTracksTTL + time.Second) - _, err = l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) + // 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, 2, f.calls, "a stale set is read again") - states, err = l.contains(context.Background(), "bob", []string{"spotify:track:a", "spotify:track:b"}, f.fetch) - require.NoError(t, err) - require.Equal(t, 3, f.calls, "another user's set is never reused") - require.Equal(t, []bool{false, true}, likedFlags(states)) + 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 TestLikedTracksApply(t *testing.T) { - l := newLikedTracks() - f := &fakeLikedFetcher{sets: map[string]map[string]bool{"alice": {"spotify:track:a": true}}} - - l.apply("alice", []string{"spotify:track:x"}, true) // nothing loaded yet: ignored - _, err := l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) - require.NoError(t, err) - - l.apply("alice", []string{"spotify:track:x"}, true) - l.apply("alice", []string{"spotify:track:a"}, false) - l.apply("bob", []string{"spotify:track:a"}, true) // other user: ignored - - states, err := l.contains(context.Background(), "alice", []string{"spotify:track:a", "spotify:track:x"}, f.fetch) - require.NoError(t, err) - require.Equal(t, []bool{false, true}, likedFlags(states)) - require.Equal(t, 1, f.calls) -} - -func TestLikedTracksFetchError(t *testing.T) { - l := newLikedTracks() - f := &fakeLikedFetcher{err: errors.New("boom")} - - _, err := l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) +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) - - f.err, f.sets = nil, map[string]map[string]bool{"alice": {"spotify:track:a": true}} - states, err := l.contains(context.Background(), "alice", []string{"spotify:track:a"}, f.fetch) - require.NoError(t, err) - require.True(t, states[0].Liked, "a failed read is not cached") } diff --git a/daemon/player.go b/daemon/player.go index 931fa808..8833642a 100644 --- a/daemon/player.go +++ b/daemon/player.go @@ -906,13 +906,12 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { data := req.Data.(ApiSetLiked) reply := apiReply(req) spc, username := p.sess.Spclient(), p.sess.Username() - p.goDetached(libraryWriteTimeout, func(ctx context.Context) { + 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 } - p.app.liked.apply(username, data.Uris, data.Liked) for _, uri := range likedSongsContextUris(username) { p.app.contextLists.invalidate(uri) } @@ -924,10 +923,13 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { uris := req.Data.([]string) reply := apiReply(req) spc, username := p.sess.Spclient(), p.sess.Username() - p.goDetached(libraryWriteTimeout, func(ctx context.Context) { - states, err := p.app.liked.contains(ctx, username, uris, fetchLikedTracks(spc)) + 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, fmt.Errorf("failed reading liked songs: %w", err)) + reply.done(nil, libraryError("failed reading liked songs", err)) return } @@ -939,7 +941,7 @@ func (p *AppPlayer) handleApiRequest(req ApiRequest) (any, error) { data := req.Data.(ApiPlaylistAddTracks) reply := apiReply(req) spc, username := p.sess.Spclient(), p.sess.Username() - p.goDetached(libraryWriteTimeout, func(ctx context.Context) { + 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 diff --git a/proto/spotify/collection/v2/collection.pb.go b/proto/spotify/collection/v2/collection.pb.go index 3cdc8861..c8afb8ee 100644 --- a/proto/spotify/collection/v2/collection.pb.go +++ b/proto/spotify/collection/v2/collection.pb.go @@ -141,30 +141,29 @@ func (x *WriteRequest) GetItems() []*CollectionItem { return nil } -type PageRequest 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"` - PaginationToken string `protobuf:"bytes,3,opt,name=pagination_token,json=paginationToken,proto3" json:"pagination_token,omitempty"` - Limit int32 `protobuf:"varint,4,opt,name=limit,proto3" json:"limit,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache -} - -func (x *PageRequest) Reset() { - *x = PageRequest{} +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 *PageRequest) String() string { +func (x *ContainsRequest) String() string { return protoimpl.X.MessageStringOf(x) } -func (*PageRequest) ProtoMessage() {} +func (*ContainsRequest) ProtoMessage() {} -func (x *PageRequest) ProtoReflect() protoreflect.Message { +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)) @@ -176,62 +175,53 @@ func (x *PageRequest) ProtoReflect() protoreflect.Message { return mi.MessageOf(x) } -// Deprecated: Use PageRequest.ProtoReflect.Descriptor instead. -func (*PageRequest) Descriptor() ([]byte, []int) { +// Deprecated: Use ContainsRequest.ProtoReflect.Descriptor instead. +func (*ContainsRequest) Descriptor() ([]byte, []int) { return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{2} } -func (x *PageRequest) GetUsername() string { +func (x *ContainsRequest) GetUsername() string { if x != nil { return x.Username } return "" } -func (x *PageRequest) GetSet() string { +func (x *ContainsRequest) GetSet() string { if x != nil { return x.Set } return "" } -func (x *PageRequest) GetPaginationToken() string { +func (x *ContainsRequest) GetItems() []*CollectionItem { if x != nil { - return x.PaginationToken - } - return "" -} - -func (x *PageRequest) GetLimit() int32 { - if x != nil { - return x.Limit + return x.Items } - return 0 + return nil } -type PageResponse struct { +type ContainsResponse struct { state protoimpl.MessageState `protogen:"open.v1"` - Items []*CollectionItem `protobuf:"bytes,1,rep,name=items,proto3" json:"items,omitempty"` - NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` - SyncToken string `protobuf:"bytes,3,opt,name=sync_token,json=syncToken,proto3" json:"sync_token,omitempty"` + Found []bool `protobuf:"varint,1,rep,packed,name=found,proto3" json:"found,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } -func (x *PageResponse) Reset() { - *x = PageResponse{} +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 *PageResponse) String() string { +func (x *ContainsResponse) String() string { return protoimpl.X.MessageStringOf(x) } -func (*PageResponse) ProtoMessage() {} +func (*ContainsResponse) ProtoMessage() {} -func (x *PageResponse) ProtoReflect() protoreflect.Message { +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)) @@ -243,32 +233,18 @@ func (x *PageResponse) ProtoReflect() protoreflect.Message { return mi.MessageOf(x) } -// Deprecated: Use PageResponse.ProtoReflect.Descriptor instead. -func (*PageResponse) Descriptor() ([]byte, []int) { +// Deprecated: Use ContainsResponse.ProtoReflect.Descriptor instead. +func (*ContainsResponse) Descriptor() ([]byte, []int) { return file_spotify_collection_v2_collection_proto_rawDescGZIP(), []int{3} } -func (x *PageResponse) GetItems() []*CollectionItem { +func (x *ContainsResponse) GetFound() []bool { if x != nil { - return x.Items + return x.Found } return nil } -func (x *PageResponse) GetNextPageToken() string { - if x != nil { - return x.NextPageToken - } - return "" -} - -func (x *PageResponse) GetSyncToken() string { - if x != nil { - return x.SyncToken - } - return "" -} - var File_spotify_collection_v2_collection_proto protoreflect.FileDescriptor const file_spotify_collection_v2_collection_proto_rawDesc = "" + @@ -283,16 +259,12 @@ const file_spotify_collection_v2_collection_proto_rawDesc = "" + "\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" + - "\vPageRequest\x12\x1a\n" + + "\x0fContainsRequest\x12\x1a\n" + "\busername\x18\x01 \x01(\tR\busername\x12\x10\n" + - "\x03set\x18\x02 \x01(\tR\x03set\x12)\n" + - "\x10pagination_token\x18\x03 \x01(\tR\x0fpaginationToken\x12\x14\n" + - "\x05limit\x18\x04 \x01(\x05R\x05limit\"\x92\x01\n" + - "\fPageResponse\x12;\n" + - "\x05items\x18\x01 \x03(\v2%.spotify.collection.v2.CollectionItemR\x05items\x12&\n" + - "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\x12\x1d\n" + - "\n" + - "sync_token\x18\x03 \x01(\tR\tsyncTokenB\xee\x01\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 ( @@ -309,14 +281,14 @@ func file_spotify_collection_v2_collection_proto_rawDescGZIP() []byte { 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 - (*PageRequest)(nil), // 2: spotify.collection.v2.PageRequest - (*PageResponse)(nil), // 3: spotify.collection.v2.PageResponse + (*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.PageResponse.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 diff --git a/proto/spotify/collection/v2/collection.proto b/proto/spotify/collection/v2/collection.proto index 7246ac73..3a5ea32f 100644 --- a/proto/spotify/collection/v2/collection.proto +++ b/proto/spotify/collection/v2/collection.proto @@ -17,15 +17,12 @@ message WriteRequest { repeated CollectionItem items = 3; } -message PageRequest { +message ContainsRequest { string username = 1; string set = 2; - string pagination_token = 3; - int32 limit = 4; + repeated CollectionItem items = 3; } -message PageResponse { - repeated CollectionItem items = 1; - string next_page_token = 2; - string sync_token = 3; +message ContainsResponse { + repeated bool found = 1; } diff --git a/spclient/library_write.go b/spclient/library_write.go index 98bd614e..1119ad82 100644 --- a/spclient/library_write.go +++ b/spclient/library_write.go @@ -72,20 +72,20 @@ func (c *Spclient) CollectionWrite(ctx context.Context, username, set string, ur return nil } -// CollectionPage reads one page of a collection set; an empty token asks for -// the first page. The response names the next page, empty on the last one. -func (c *Spclient) CollectionPage(ctx context.Context, username, set, token string, limit int) (*collectionpb.PageResponse, error) { - body, err := proto.Marshal(&collectionpb.PageRequest{ - Username: username, - Set: set, - PaginationToken: token, - Limit: int32(limit), - }) +// 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 PageRequest: %w", err) + return nil, fmt.Errorf("failed marshalling ContainsRequest: %w", err) } - resp, err := c.Request(ctx, "POST", "/collection/v2/paging", nil, http.Header{ + resp, err := c.Request(ctx, "POST", "/collection/v2/contains", nil, http.Header{ "Content-Type": {collectionContentType}, "Accept": {collectionContentType}, }, body) @@ -96,7 +96,7 @@ func (c *Spclient) CollectionPage(ctx context.Context, username, set, token stri defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return nil, &StatusError{Op: "collection paging", StatusCode: resp.StatusCode} + return nil, &StatusError{Op: "collection contains", StatusCode: resp.StatusCode} } respBytes, err := io.ReadAll(resp.Body) @@ -104,12 +104,15 @@ func (c *Spclient) CollectionPage(ctx context.Context, username, set, token stri return nil, fmt.Errorf("failed reading response body: %w", err) } - var page collectionpb.PageResponse - if err := proto.Unmarshal(respBytes, &page); err != nil { - return nil, fmt.Errorf("failed unmarshalling PageResponse: %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 &page, nil + return contains.GetFound(), nil } // PlaylistRevision returns the current revision of a playlist, which a change diff --git a/spclient/library_write_test.go b/spclient/library_write_test.go index 799e8433..c35b933e 100644 --- a/spclient/library_write_test.go +++ b/spclient/library_write_test.go @@ -105,30 +105,38 @@ func (suite *RequestSuite) TestPlaylistAppendReportsConflict() { suite.ErrorIs(err, spclient.ErrPlaylistConflict) } -func (suite *RequestSuite) TestCollectionPageReadsPage() { +func (suite *RequestSuite) TestCollectionContainsAsksForTheItems() { suite.handler = func(_ int, w http.ResponseWriter) { - body, _ := proto.Marshal(&collectionpb.PageResponse{ - Items: []*collectionpb.CollectionItem{{Uri: "spotify:track:a", AddedAt: 1}}, - NextPageToken: "next", - }) + body, _ := proto.Marshal(&collectionpb.ContainsResponse{Found: []bool{true, false}}) _, _ = w.Write(body) } - page, err := suite.spclient.CollectionPage(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, "tok", 300) + found, err := suite.spclient.CollectionContains(suite.T().Context(), "user", spclient.CollectionSetLikedSongs, + []string{"spotify:track:a", "spotify:track:b"}) suite.Require().NoError(err) - suite.Equal("next", page.GetNextPageToken()) - suite.Require().Len(page.GetItems(), 1) + suite.Equal([]bool{true, false}, found) got := suite.requests()[0] - suite.Equal("/collection/v2/paging", got.path) + suite.Equal("/collection/v2/contains", got.path) suite.Equal("application/vnd.collection-v2.spotify.proto", got.header.Get("Content-Type")) - var req collectionpb.PageRequest + var req collectionpb.ContainsRequest suite.Require().NoError(proto.Unmarshal(got.body, &req)) suite.Equal("user", req.GetUsername()) suite.Equal("collection", req.GetSet()) - suite.Equal("tok", req.GetPaginationToken()) - suite.Equal(int32(300), req.GetLimit()) + 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,