[PR #483] [MERGED] Fix unnecessary request in sync pagination #486

Closed
opened 2026-02-27 20:24:54 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/483
Author: @thirteenowls
Created: 5/22/2024
Status: Merged
Merged: 5/25/2024
Merged by: @ramsayleung

Base: masterHead: fix-extra-request


📝 Commits (1)

  • 864a781 Fix unnecessary request in sync pagination

📊 Changes

1 file changed (+10 additions, -6 deletions)

View changed files

📝 src/clients/pagination/iter.rs (+10 -6)

📄 Description

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.

  • 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


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/ramsayleung/rspotify/pull/483 **Author:** [@thirteenowls](https://github.com/thirteenowls) **Created:** 5/22/2024 **Status:** ✅ Merged **Merged:** 5/25/2024 **Merged by:** [@ramsayleung](https://github.com/ramsayleung) **Base:** `master` ← **Head:** `fix-extra-request` --- ### 📝 Commits (1) - [`864a781`](https://github.com/ramsayleung/rspotify/commit/864a781831a8c77cd98a89920f13d2c64f788db1) Fix unnecessary request in sync pagination ### 📊 Changes **1 file changed** (+10 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `src/clients/pagination/iter.rs` (+10 -6) </details> ### 📄 Description ## 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 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-02-27 20:24:54 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/rspotify#486
No description provided.