sfuhrm / radiobrowser4j

RadioBrowser Java API library
Other
50 stars 8 forks source link

listStationsWithAdvancedSearch No paging provided #126

Closed ChildProdigy closed 2 months ago

ChildProdigy commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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