chore: simplify pagination iterator state and query-param parsing#193
Merged
Conversation
Three behavior-preserving cleanups in the pagination package:
- Paginator: seed the iterator's nextRequest with initialRequest so the
first fetch reads it like every later page, removing the started flag and
the dead `nextRequest ?: nextPageRequest()` fallback. The page-budget and
cursor mutations stay after the fetch, so the catch-and-retry-hasNext path
still re-attempts the same request on a failed exchange.
- RequestRebuilder.getQueryParam: parse each segment with substringBefore/
substringAfter on the first '=', matching setQueryParam's existing idiom
and dropping the manual indexOf/substring bookkeeping. Edge cases (no '=',
empty value, '=' in value) are identical.
- LinkHeaderPaginationStrategy: drop the empty-string guards around the Link
header join. An empty values list joins to "" and extractNextUrl("")
already returns null, so the guards were dead.
No public-API change; existing pagination tests stay green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three small, behavior-preserving cleanups in the
paginationpackage, grouped because they sit in the same subsystem. No public signatures move and the existing pagination tests stay green.Paginator— drop thestartedflagThe
PaginatorIteratorcarried astartedboolean solely to pickinitialRequeston the first fetch. SeedingnextRequestwithinitialRequestlets the first fetch readnextRequestexactly like every later page, soadvance()'s two-stage request selection collapses into a single guard. ThenextRequest ?: finishedPage.nextPageRequest()fallback was dead: by the timeadvance()re-runs,nextRequestalready holds that page'snextPageRequest()(ornull, which the page's ownhasNextcheck short-circuits).The page-budget increment (
pagesFetched++) and thenextRequestreassignment both remain after the fetch/parse. That ordering is load-bearing: the sync iterator supports retry — ifexecute()throws, a caller can catch it and callhasNext()again, re-enteringadvance()to re-attempt the same request. Mutating either field before the fetch would burn the page budget or advance the cursor on a failed exchange.RequestRebuilder.getQueryParam— usesubstringBefore/substringAfterReplaced the hand-rolled
indexOf('=')plus two conditionalsubstringcalls withsubstringBefore('=', segment)/substringAfter('=', ""), matching the idiom its siblingsetQueryParamalready uses for key extraction. The default-argument forms split on the first=and fall back to the supplied default when absent, so the no-=, empty-value, and=-in-value cases are identical.LinkHeaderPaginationStrategy— drop the deadLink-header guardsThe two empty-string guards around the header join were unreachable: an empty
values(...)list joins to"", andextractNextUrl("")already returnsnull(an empty header splits to no link-values). The intermediate vals and branches fold into oneextractNextUrl(...)call; the explicitseparator = ","is kept so the join uses a bare comma.Build
No public-API change — every edit is internal (
PaginatorIteratoris private,RequestRebuilderis aninternal object, andparse(...)keeps its overriding signature), soapiCheckneeds noapiDump.:sdk-corektlint and the fullorg.dexpace.sdk.core.pagination.*test suite pass locally.Closes #177.