Fix unnecessary request in sync pagination

thirteenowls commented 3 months ago

Description

Makes use of the next field to end pagination early, instead of waiting for Spotify's API to return no data.

Motivation and Context

Currently, PageIterator always sends one extra request to the API, because it does not check next to know if more data is available (unlike its async equivalent).

Dependencies

None.

Type of change

Please delete options that are not relevant.

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

How has this been tested?

This has been tested by adding a dbg! call right before sending a request in PageIterator's impl, then running the pagination_sync example with and without this change. Tests were run using an account with 51 saved tracks, which should require two requests with limit set to 50.

This is pagination_sync's output before the change:

Items:
[src\clients\pagination\iter.rs:56:9] self.offset = 0
* Track 1
-snip-
* Track 50
[src\clients\pagination\iter.rs:56:9] self.offset = 50
* Track 51
[src\clients\pagination\iter.rs:56:9] self.offset = 51

And this is its output after the change:

Items:
[src\clients\pagination\iter.rs:56:9] self.offset = 0
* Track 1
-snip-
* Track 50
[src\clients\pagination\iter.rs:56:9] self.offset = 50
* Track 51

The output is the same, but the extraneous request has been eliminated.

Is this change properly documented?

N/A

ramsayleung commented 3 months ago

Thanks for your contribution, the CI tasks are failed, would you consider to fix them?

thirteenowls commented 3 months ago

Sure. The failure is unrelated to this contribution, so I've opened a new PR. I'll rebase this one on it once it's merged.

ramsayleung commented 3 months ago

Merged, thanks for your contribution.

ramsayleung / rspotify