feat: add a single-pass cursor extractor for CursorPaginationStrategy#98
Merged
Conversation
TokenPaginationStrategy was behaviorally identical to
CursorPaginationStrategy: same constructor shape, same parse() logic,
differing only in identifier naming and a default query-param value
("page_token" vs "cursor"). Since CursorPaginationStrategy already
exposes an overridable query-param name, token-style APIs
(next_page_token, pageToken, etc.) are fully served by constructing it
with the desired parameter name.
Remove the redundant type and migrate the two test cases that exercised
unique behavior (streamAll() equivalence and base64 value URL-encoding
through RequestRebuilder) onto CursorPaginationStrategy, preserving
coverage. Regenerate the sdk-core API snapshot for the net surface
reduction and update the docs that enumerated the shipped strategies.
CursorPaginationStrategy took two `(Response) -> …` lambdas — one for the items, one for the next cursor — and called both per page. A response body is single-use, so the second lambda either re-read an exhausted stream or had to be propped up with a per-response cache to read the body only once. That made the single-read constraint a hidden trap in a public API. Add a CursorResult<T> holder (items + next cursor) and a constructor that takes one `(Response) -> CursorResult<T>` extractor, so a caller parses the response once and returns both pieces. The two-lambda constructor is kept for source and binary compatibility but deprecated, with a ReplaceWith pointing at the single-pass form; it now delegates to that path. Tests cover a page walk over the single extractor, a body that throws on a second read (proving exactly one read per page), and the retained two-lambda constructor. The cursor suite no longer needs its IdentityHashMap caching workaround and parses each page in one pass.
CursorResult was a public value type with hand-rolled equals/hashCode/ toString. Switch it to a data class to match the repo convention for public value holders, which also gives componentN/copy; regenerate the sdk-core API snapshot for the added members. Make the thread-safety KDoc honest about the stored list reference instead of claiming an immutability the constructor does not enforce. Replace the public KDoc link to the internal RequestRebuilder with plain prose so the published docs have no dangling reference, and note on the deprecated two-lambda constructor that the suggested replacement still double-drains unless the caller collapses the two reads into one pass. Add a test that pins that double-drain failure mode through the deprecated path.
OmarAlJarrah
changed the base branch from
refactor/remove-duplicate-token-pagination
to
main
…nation # Conflicts: # README.md # docs/architecture.md # sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt
The single-pass (Response) -> CursorResult extractor is now the only way to construct the strategy. The two-lambda form drained the single-use response body twice per page and only existed for back-compat; drop it outright rather than deprecate.
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
1 participant
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.
Summary
CursorPaginationStrategytook two(Response) -> …extractor lambdas — one for the items, one for the next cursor. A response body is single-use, so reading items and cursor in two passes drained it twice; paginating a streamed body forced a per-response caching workaround.This replaces that with a single-pass path:
CursorResult<T>— an immutable(items, nextCursor)holder.(Response) -> CursorResult<T>extractor that reads and parses the response exactly once.Token-style APIs (
next_page_token,pageToken, …) are served by the same strategy constructed with the query-param name set (e.g."page_token"), so the separateTokenPaginationStrategyis removed.This is a breaking API change, which is acceptable at this alpha stage.
Tests
source()throws on a second call, so a double-read is a hard failure rather than a silently-cached read.The
sdk-coreAPI snapshot is updated.Closes #40