Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SoundCloud] Detect whether there are any more search results #1081

Merged
merged 4 commits into from
Aug 6, 2023

Conversation

TobiGr
Copy link
Contributor

@TobiGr TobiGr commented Jul 14, 2023

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

Previously, the extractor always assumed that there was an infinite number of search results. That has caused some problems. When searching for something with only a few results, that could cause clients such as NewPipe to try to load more results without getting more and thus causing tons of requests, ultimately resulting in reCaptcha requests.

@TobiGr TobiGr added bug Issue is related to a bug soundcloud service, https://soundcloud.com/ labels Jul 14, 2023
@TobiGr TobiGr force-pushed the fix/sc/search-next-page branch from 25718ae to 428a1ec Compare July 20, 2023 22:15
@TobiGr TobiGr force-pushed the fix/sc/search-next-page branch from 428a1ec to 05f2588 Compare August 3, 2023 12:39
@TobiGr TobiGr force-pushed the fix/sc/search-next-page branch from 05f2588 to aa6c17d Compare August 3, 2023 12:41
@Stypox
Copy link
Member

Stypox commented Aug 6, 2023

@TobiGr I pushed 2947257 and changed the <= into a <. There are more results only if the results fetched so far are less than the total amount, not less or equal. Just to make sure, I tested with ITEMS_PER_PAGE=1 and it works. However, the only downside of <= was an additional network request I think, I didn't run into infinite calls, so the code seems resilient. Thanks!

@Stypox Stypox merged commit 8fb6ba3 into dev Aug 6, 2023
Copy link
Member

@AudricV AudricV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A review that I didn't have the time to finish and submit.

@@ -72,8 +63,7 @@ public InfoItemsPage<CommentsInfoItem> getPage(final Page page) throws Extractio
getServiceId());

collectStreamsFrom(collector, json.getArray("collection"));

return new InfoItemsPage<>(collector, new Page(json.getString("next_href")));
return new InfoItemsPage<>(collector, new Page(json.getString("next_href", null)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not needed to specify a null fallback, as that's already the default value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, however this is fine, too

@TobiGr TobiGr deleted the fix/sc/search-next-page branch August 6, 2023 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug soundcloud service, https://soundcloud.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants