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

listStationsWithAdvancedSearch No paging provided #126

Closed
ChildProdigy opened this issue Aug 2, 2024 · 7 comments
Closed

listStationsWithAdvancedSearch No paging provided #126

ChildProdigy opened this issue Aug 2, 2024 · 7 comments
Assignees

Comments

@ChildProdigy
Copy link

ChildProdigy commented Aug 2, 2024

https://de1.api.radio-browser.info/#General

Advanced station search

http://de1.api.radio-browser.info/json/stations/search
The official API provides offset and limit

At present, the lib has been hardcoded to prevent users from operating it,
but sometimes users want to control how much data is preloaded.

@sfuhrm sfuhrm self-assigned this Aug 18, 2024
@sfuhrm
Copy link
Owner

sfuhrm commented Aug 18, 2024

There are multiple levels to see here.
On one hand, the library does not provide a way to support paging for the listStationsWithAdvancedSearch() call. Since other calls have the paging possibility, this call should also have it.

On the other hand, internally usually there is paging used instead of fetching all data with no way for the user to change the fetch size.

The HTTP API usually has a default of limit 100000.

Can you explain to me the use case why it makes sense to increase the default fetch-limit for the user?

sfuhrm added a commit that referenced this issue Aug 18, 2024
…ing from the parameters and the paging spliterator is used. #126
@sfuhrm
Copy link
Owner

sfuhrm commented Aug 18, 2024

I've added support for paging in the advanced search call.
Besides that I also fixed conflicts with the explicit paging and the implicit paging that were in the code for some calls.

@ChildProdigy
Copy link
Author

ChildProdigy commented Aug 21, 2024

At present, Lib will automatically use the stream method to load pagging,
but some third-party components will control the loading timing themselves.
What needs to be returned is the data array,
some third-party components also have their own loading mechanism,
so there is this requirement.

@sfuhrm
Copy link
Owner

sfuhrm commented Aug 21, 2024

At the moment the code re-uses the stream part and does the paging internally.
Added in the commt b698f81 there's now the advanced search pagable. This is now restricting the view of the returned stream, not exactly controlling how the library code fetches data.

The libraries main job is to abstract from how things look on HTTP and provide a comfortable Java class view of that. Timeouts are handled with one central mechanism in the library.
My gut feeling is that fine-grain control would break the abstraction.

So if you could wish something, what would it be?

@ChildProdigy
Copy link
Author

ChildProdigy commented Aug 22, 2024

I want

int page = 0;
int offset = 0;
int limit = 10;

AdvancedSearch advancedTWSearch = AdvancedSearch.builder();
advancedTWSearch.offset(offset).limit(limit); // (offset = page * limit;)

List stations = radioBrowser.listStationsWithAdvancedSearch(advancedTWSearch, page); // return page data

if(isEndPage + 1) return emptyList; // new ArrayList();

or

int page = 0;
int limit = 10;

AdvancedSearch advancedTWSearch = AdvancedSearch.builder();
advancedTWSearch.page(page).limit(limit); // (offset = page * limit;)

List stations = radioBrowser.listStationsWithAdvancedSearch(advancedTWSearch); // return page data

if(isEndPage + 1) return emptyList; // new ArrayList();

or

int page = 0;
int limit = 10;

AdvancedSearch advancedTWSearch = AdvancedSearch.builder();
advancedTWSearch.page(Page.of(page ,limit)); // (offset = page * limit;)

List stations = radioBrowser.listStationsWithAdvancedSearch(advancedTWSearch); // return page data

if(isEndPage + 1) return emptyList; // new ArrayList();

@sfuhrm
Copy link
Owner

sfuhrm commented Aug 25, 2024

To summarize, your examples are returning List instances with explicit paging boundaries. The current advanced search only returns Stream instances with automatic paging that has no possibility of intervention in the page size.

Since the other calls are also providing List variants, the most obvious solution is to provide also an advanced search call with paging and a List return.

@sfuhrm
Copy link
Owner

sfuhrm commented Aug 25, 2024

Thanks for the input here, that's appreciated!
All changes discussed so far are in todays release 'Release 3.1.0'.

@sfuhrm sfuhrm closed this as completed Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants