Peter-Schorn / SpotifyAPI

A Swift library for the Spotify web API. Supports all endpoints.
https://peter-schorn.github.io/SpotifyAPI/documentation/spotifywebapi
MIT License
269 stars 33 forks source link

`extendPages` aren't available on `.search` #35

Closed shaundon closed 2 years ago

shaundon commented 2 years ago

I've noticed that the extendPages and extendPagesConcurrently methods aren't available when using the .search function.

The below code produces the following error:

Referencing instance method 'extendPages(_:maxExtraPages:)' on 'Publisher' requires that 'SearchResult' conform to 'Paginated'
let api = SpotifyAPI(
  authorizationManager: AuthorizationCodeFlowManager(
  clientId: clientId, 
  clientSecret: clientSecret
))

var cancellables = Set<AnyCancellable>()

api.search(query: "My query", categories: [.tracks], limit: 50)
  .extendPages(api)
  .receive(on: RunLoop.main)
  .sink(
    receiveCompletion: { completion in
      // Handle completion
    },
    receiveValue: { resultsPage in
      // Handle results page
    }
  )
  .store(in: &cancellables)

I get the same error when trying extendPagesConcurrently, too.

As far as I can tell from Spotify's API documentation, paging should work fine on this endpoint, as it does for others.

Is this a deliberate omission, or is it a bug?

shaundon commented 2 years ago

Update: I took a look at the source code for SearchResult and it appears that the properties within implement PagingObject (e.g. PagingObject<Track>). So I might just be trying to extend the pages in the wrong way.

Peter-Schorn commented 2 years ago

SearchResult does not conform to Paginated because it does not have a next property. This is the only requirement for conforming to this protocol.

Furthermore, I have just now discovered that extendPages and extendPagesConcurrently don't work with any of the paging objects within SearchResult either because when a request is made to the next property of these paging objects, Spotify returns a JSON response that must be decoded into SearchResult, instead of PagingObject.

The Paginated protocol relies on the assumption that the next property of a paging object will return a JSON response that can be decoded into another paging object of the same type.

Therefore, you must use the SpotifyAPI.getFromHref(_:responseType:) method instead with these paging objects, and pass in SearchResult for the responseType. Or, just make multiple requests to the search method directly with different offsets.

shaundon commented 2 years ago

Thanks for the detailed response!