From 432fb509def7efb4304541ff96073e04715951c0 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Mon, 15 Jun 2026 22:29:29 +0300 Subject: [PATCH 1/4] refactor: remove duplicate TokenPaginationStrategy 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. --- README.md | 2 +- docs/architecture.md | 2 +- docs/implementation-plan.md | 5 +- sdk-core/api/sdk-core.api | 7 - .../pagination/TokenPaginationStrategy.kt | 58 -------- .../core/pagination/CursorPaginationTest.kt | 46 ++++++ .../core/pagination/TokenPaginationTest.kt | 138 ------------------ 7 files changed, 50 insertions(+), 208 deletions(-) delete mode 100644 sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationStrategy.kt delete mode 100644 sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationTest.kt diff --git a/README.md b/README.md index 56a7de8e..cff93e96 100644 --- a/README.md +++ b/README.md @@ -255,7 +255,7 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough. | `http.auth` | `Credential` sealed hierarchy (`KeyCredential`, `NamedKeyCredential`, `BearerToken`), `BearerTokenProvider`, `AuthScheme`, `AuthMetadata`, RFC 7235 challenge parser, `BasicChallengeHandler`, `DigestChallengeHandler`, `CompositeChallengeHandler`. | | `http.sse` | `ServerSentEventReader` (WHATWG spec), `ServerSentEvent`, `ServerSentEventListener`, `BufferedSource.readServerSentEvents()`. | | `http.paging` | `PagedIterable`, `PagedResponse`, `PagingOptions` with `byPage()` and `stream()` accessors. | -| `pagination` | `Paginator` (with a `maxPages` safety cap) over cursor / page-number / token / link-header `PaginationStrategy` implementations, plus `Page` / `SimplePage`. | +| `pagination` | `Paginator` (with a `maxPages` safety cap) over cursor / page-number / link-header `PaginationStrategy` implementations, plus `Page` / `SimplePage`. Token-style APIs use `CursorPaginationStrategy` with the query-param name set (e.g. `"page_token"`). | | `pipeline` | Recovery-aware primitives: `RequestPipeline`, `ResponsePipeline`, `ExecutionPipeline` over a sealed `ResponseOutcome`, with steps (`pipeline.step`, `pipeline.step.retry`) like `RetryStep`, `ResponseRecoveryStep`, `IdempotencyKeyStep`, `ClientIdentityStep`. | | `serde` | `Serde`, `Serializer`, `Deserializer` abstractions and `Tristate` (absent / null / present). | | `io` | `Source`, `Sink`, `Buffer`, `BufferedSource`, `BufferedSink`, `IoProvider`, `Io`, `TeeSink`. | diff --git a/docs/architecture.md b/docs/architecture.md index 32c8a5e3..c07e7fe8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -346,7 +346,7 @@ Two complementary surfaces for walking multi-page responses. |-----------------------------------------------------------------|-----------------------------------------------------------------------| | `Paginator` | Lazily iterates pages by re-issuing requests through an `HttpClient`; carries a `maxPages` safety cap | | `PaginationStrategy` | Computes the next-page request (or stops) from the current page | -| `CursorPaginationStrategy` / `PageNumberPaginationStrategy` / `TokenPaginationStrategy` / `LinkHeaderPaginationStrategy` | The four shipped strategies | +| `CursorPaginationStrategy` / `PageNumberPaginationStrategy` / `LinkHeaderPaginationStrategy` | The shipped strategies. Token-style APIs use `CursorPaginationStrategy` with the query-param name set (e.g. `"page_token"`). | | `PagedIterable` | First/next-page fetcher abstraction over `PagedResponse`, with its own `maxPages` cap | ### Serialization diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index d7ae2188..cd3874b8 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -372,8 +372,8 @@ defaults (per Square: `FAIL_ON_UNKNOWN_PROPERTIES=false`, `WRITE_DATES_AS_TIMEST ### WU-9: Pagination primitives -**Status: shipped.** `Page`, `Paginator`, `PaginationStrategy`, and the four strategies -(`Cursor` / `PageNumber` / `LinkHeader` / `Token`) are in `sdk-core/.../pagination`, alongside +**Status: shipped.** `Page`, `Paginator`, `PaginationStrategy`, and the three strategies +(`Cursor` / `PageNumber` / `LinkHeader`) are in `sdk-core/.../pagination`, alongside helper types `SimplePage` and `RequestRebuilder`. `Paginator` gained a `maxPages` safety cap (default `Long.MAX_VALUE`) beyond the original sketch, to bound runaway iteration against servers that never advance their cursor. @@ -390,7 +390,6 @@ link-header strategies without over-engineering. Sync first; async adapter follo - `CursorPaginationStrategy(cursorPath, itemsPath, parser)` — read `next_cursor` from body - `PageNumberPaginationStrategy(pageParam, itemsPath, parser)` — increment page number - `LinkHeaderPaginationStrategy(itemsPath, parser)` — RFC 5988 `Link: ; rel="next"` - - `TokenPaginationStrategy(tokenPath, tokenParam, itemsPath, parser)` — token in body, sent as query param - `sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/PaginatorTests.kt` (test) — table-driven tests against MockWebServer fixtures. **Acceptance criteria:** diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index de3a5d14..3f2301e0 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -1937,13 +1937,6 @@ public final class org/dexpace/sdk/core/pagination/Paginator { public final fun streamAll ()Ljava/util/stream/Stream; } -public final class org/dexpace/sdk/core/pagination/TokenPaginationStrategy : org/dexpace/sdk/core/pagination/PaginationStrategy { - public fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V - public fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V - public synthetic fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun parse (Lorg/dexpace/sdk/core/http/response/Response;Lorg/dexpace/sdk/core/http/request/Request;)Lorg/dexpace/sdk/core/pagination/Page; -} - public final class org/dexpace/sdk/core/pipeline/ExecutionPipeline { public fun (Lorg/dexpace/sdk/core/client/HttpClient;)V public fun (Lorg/dexpace/sdk/core/client/HttpClient;Lorg/dexpace/sdk/core/pipeline/RequestPipeline;)V diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationStrategy.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationStrategy.kt deleted file mode 100644 index c256be23..00000000 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationStrategy.kt +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright (c) 2026 dexpace and Omar Aljarrah - * - * Licensed under the MIT License. See LICENSE in the project root. - * SPDX-License-Identifier: MIT - */ - -package org.dexpace.sdk.core.pagination - -import org.dexpace.sdk.core.http.request.Request -import org.dexpace.sdk.core.http.response.Response - -/** - * Token [PaginationStrategy]. Logically identical to [CursorPaginationStrategy] but with - * a different naming convention — many APIs (Google Cloud, Slack, GitHub GraphQL legacy) - * use the word *token* (`next_page_token`, `pageToken`) rather than *cursor*. - * - * Wire shape: - * - * - Page-N request: `GET /things?page_token=`. - * - Page-N response: JSON body containing both the items and the next token. - * - End of stream: response carries an empty / absent next token. - * - * Use this strategy when the token shape and naming differs from a typical "cursor" - * scheme — primarily for readability at call sites and for the default [tokenQueryParam] - * of `"page_token"`. - * - * @param T Element type extracted from the response. - * @property itemsExtractor Reads the list of items from the response. Called once per - * page; must drain the response body synchronously. - * @property tokenExtractor Reads the next-page token from the response, or returns - * `null` if there are no more pages. - * @property tokenQueryParam Query parameter name used to send the token (default - * `"page_token"`). - */ -public class TokenPaginationStrategy - @JvmOverloads - constructor( - private val itemsExtractor: (Response) -> List, - private val tokenExtractor: (Response) -> String?, - private val tokenQueryParam: String = "page_token", - ) : PaginationStrategy { - override fun parse( - response: Response, - initialRequest: Request, - ): Page { - val items: List = itemsExtractor(response) - val nextToken: String? = tokenExtractor(response) - val hasNext: Boolean = !nextToken.isNullOrEmpty() - val nextRequest: Request? = - if (hasNext) { - RequestRebuilder.withQueryParam(initialRequest, tokenQueryParam, nextToken) - } else { - null - } - return SimplePage(items = items, hasNext = hasNext, nextRequest = nextRequest) - } - } diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt index dee1902b..075caf47 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt @@ -11,6 +11,7 @@ import org.dexpace.sdk.core.http.request.Method import org.dexpace.sdk.core.http.request.Request import org.dexpace.sdk.core.http.response.Response import java.util.IdentityHashMap +import java.util.stream.Collectors import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -127,4 +128,49 @@ class CursorPaginationTest { val paginator = Paginator(client, authRequest, strategy) assertEquals(listOf("a", "b"), paginator.iterateAll().toList()) } + + @Test + fun `streamAll yields the same items as iterateAll`() { + val client = StubHttpClient() + client.on("https://api.example.com/items") { req -> + textResponse(req, "items=1,2\ncursor=c1") + } + client.on("https://api.example.com/items?cursor=c1") { req -> + textResponse(req, "items=3,4\ncursor=") + } + + val (items, cursor) = buildCachedExtractors() + val strategy = CursorPaginationStrategy(items, cursor) + val paginator = Paginator(client, initialRequest(), strategy) + val streamed: List = paginator.streamAll().collect(Collectors.toList()) + assertEquals(listOf("1", "2", "3", "4"), streamed) + } + + @Test + fun `cursor with special characters is URL encoded in next request`() { + // Opaque cursors may contain `=` `+` `/` characters (base64) — the rebuilder must + // URL-encode them so the server sees the original value unmangled. A custom query + // param name (e.g. `page_token`) covers token-style APIs that reuse this strategy. + val rawCursor = "a+b/c=" + val encoded = "a%2Bb%2Fc%3D" + val client = StubHttpClient() + client.on("https://api.example.com/items") { req -> + textResponse(req, "items=one\ncursor=$rawCursor") + } + client.on("https://api.example.com/items?page_token=$encoded") { req -> + textResponse(req, "items=two\ncursor=") + } + + val (items, cursor) = buildCachedExtractors() + val strategy = CursorPaginationStrategy(items, cursor, cursorQueryParam = "page_token") + val paginator = Paginator(client, initialRequest(), strategy) + assertEquals(listOf("one", "two"), paginator.iterateAll().toList()) + assertEquals( + listOf( + "https://api.example.com/items", + "https://api.example.com/items?page_token=$encoded", + ), + client.receivedUrls, + ) + } } diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationTest.kt deleted file mode 100644 index c7e3db82..00000000 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationTest.kt +++ /dev/null @@ -1,138 +0,0 @@ -/* - * Copyright (c) 2026 dexpace and Omar Aljarrah - * - * Licensed under the MIT License. See LICENSE in the project root. - * SPDX-License-Identifier: MIT - */ - -package org.dexpace.sdk.core.pagination - -import org.dexpace.sdk.core.http.request.Method -import org.dexpace.sdk.core.http.request.Request -import org.dexpace.sdk.core.http.response.Response -import java.util.IdentityHashMap -import java.util.stream.Collectors -import kotlin.test.BeforeTest -import kotlin.test.Test -import kotlin.test.assertEquals - -class TokenPaginationTest { - @BeforeTest - fun setup() { - installIoProvider() - } - - private fun initialRequest(): Request = - Request.builder() - .url("https://api.example.com/list") - .method(Method.GET) - .build() - - /** - * Parses `items=\ntoken=` once; returns (items, nextToken). - */ - private fun parsePayload(resp: Response): Pair, String?> { - val body = resp.body!!.source().use { it.readUtf8() } - val itemsLine = body.lineSequence().firstOrNull { it.startsWith("items=") } ?: "items=" - val tokenLine = body.lineSequence().firstOrNull { it.startsWith("token=") } ?: "token=" - val itemsRaw = itemsLine.removePrefix("items=") - val tokenRaw = tokenLine.removePrefix("token=") - val items = if (itemsRaw.isEmpty()) emptyList() else itemsRaw.split(",") - val token: String? = tokenRaw.ifEmpty { null } - return Pair(items, token) - } - - private fun cachedExtractors(): Pair<(Response) -> List, (Response) -> String?> { - val cache: MutableMap, String?>> = IdentityHashMap() - val items: (Response) -> List = { r -> - cache.getOrPut(r) { parsePayload(r) }.first - } - val token: (Response) -> String? = { r -> - cache.getOrPut(r) { parsePayload(r) }.second - } - return Pair(items, token) - } - - @Test - fun `token pagination walks pages and stops on empty token`() { - val client = StubHttpClient() - client.on("https://api.example.com/list") { req -> - textResponse(req, "items=alpha,beta\ntoken=t1") - } - client.on("https://api.example.com/list?page_token=t1") { req -> - textResponse(req, "items=gamma\ntoken=t2") - } - client.on("https://api.example.com/list?page_token=t2") { req -> - textResponse(req, "items=delta,epsilon\ntoken=") - } - - val (items, token) = cachedExtractors() - val strategy = TokenPaginationStrategy(items, token) - val paginator = Paginator(client, initialRequest(), strategy) - val collected = paginator.iterateAll().toList() - assertEquals(listOf("alpha", "beta", "gamma", "delta", "epsilon"), collected) - assertEquals(3, client.callCount) - } - - @Test - fun `custom token query param name works`() { - val client = StubHttpClient() - client.on("https://api.example.com/list") { req -> - textResponse(req, "items=x\ntoken=ab") - } - client.on("https://api.example.com/list?nextToken=ab") { req -> - textResponse(req, "items=y\ntoken=") - } - - val (items, token) = cachedExtractors() - val strategy = - TokenPaginationStrategy(items, token, tokenQueryParam = "nextToken") - val paginator = Paginator(client, initialRequest(), strategy) - assertEquals(listOf("x", "y"), paginator.iterateAll().toList()) - assertEquals(2, client.callCount) - } - - @Test - fun `streamAll yields the same items as iterateAll`() { - val client = StubHttpClient() - client.on("https://api.example.com/list") { req -> - textResponse(req, "items=1,2\ntoken=t1") - } - client.on("https://api.example.com/list?page_token=t1") { req -> - textResponse(req, "items=3,4\ntoken=") - } - - val (items, token) = cachedExtractors() - val strategy = TokenPaginationStrategy(items, token) - val paginator = Paginator(client, initialRequest(), strategy) - val streamed: List = paginator.streamAll().collect(Collectors.toList()) - assertEquals(listOf("1", "2", "3", "4"), streamed) - } - - @Test - fun `token with special characters is URL encoded in next request`() { - // Tokens may contain `=` `+` `/` characters (base64) — the rebuilder must URL-encode - // them so the server sees the original token unmangled. - val rawToken = "a+b/c=" - val encoded = "a%2Bb%2Fc%3D" - val client = StubHttpClient() - client.on("https://api.example.com/list") { req -> - textResponse(req, "items=one\ntoken=$rawToken") - } - client.on("https://api.example.com/list?page_token=$encoded") { req -> - textResponse(req, "items=two\ntoken=") - } - - val (items, token) = cachedExtractors() - val strategy = TokenPaginationStrategy(items, token) - val paginator = Paginator(client, initialRequest(), strategy) - assertEquals(listOf("one", "two"), paginator.iterateAll().toList()) - assertEquals( - listOf( - "https://api.example.com/list", - "https://api.example.com/list?page_token=$encoded", - ), - client.receivedUrls, - ) - } -} From 3bc92dc1922daf34d3ff576d8e02c89dd8e7588a Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 16:16:56 +0300 Subject: [PATCH 2/4] feat: add single-pass cursor extractor for CursorPaginationStrategy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 holder (items + next cursor) and a constructor that takes one `(Response) -> CursorResult` 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. --- sdk-core/api/sdk-core.api | 12 ++ .../pagination/CursorPaginationStrategy.kt | 59 +++++-- .../sdk/core/pagination/CursorResult.kt | 47 +++++ .../core/pagination/CursorPaginationTest.kt | 42 ++--- .../core/pagination/CursorSingleReadTest.kt | 164 ++++++++++++++++++ .../core/pagination/PaginationTestSupport.kt | 38 ++++ 6 files changed, 318 insertions(+), 44 deletions(-) create mode 100644 sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt create mode 100644 sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index 3f2301e0..28ef4716 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -1898,12 +1898,24 @@ public abstract interface class org/dexpace/sdk/core/io/Source : java/io/Closeab } public final class org/dexpace/sdk/core/pagination/CursorPaginationStrategy : org/dexpace/sdk/core/pagination/PaginationStrategy { + public fun (Lkotlin/jvm/functions/Function1;)V + public fun (Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V + public synthetic fun (Lkotlin/jvm/functions/Function1;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V public fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V public synthetic fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun parse (Lorg/dexpace/sdk/core/http/response/Response;Lorg/dexpace/sdk/core/http/request/Request;)Lorg/dexpace/sdk/core/pagination/Page; } +public final class org/dexpace/sdk/core/pagination/CursorResult { + public fun (Ljava/util/List;Ljava/lang/String;)V + public fun equals (Ljava/lang/Object;)Z + public final fun getItems ()Ljava/util/List; + public final fun getNextCursor ()Ljava/lang/String; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + public final class org/dexpace/sdk/core/pagination/LinkHeaderPaginationStrategy : org/dexpace/sdk/core/pagination/PaginationStrategy { public fun (Lkotlin/jvm/functions/Function1;)V public fun (Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt index 2d5bc9eb..361e8ff4 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt @@ -21,15 +21,22 @@ import org.dexpace.sdk.core.http.response.Response * - Page-N response: arbitrary body containing both the items and the next cursor. * - End of stream: response carries a `null` or absent next cursor. * - * The strategy is **stateless**: it relies on [itemsExtractor] and [cursorExtractor] - * lambdas to pull data out of the response, and on [RequestRebuilder] to mutate the - * initial request's URL with the new cursor query parameter. + * The strategy is **stateless**: it relies on a single [extractor] to pull both the items + * and the next cursor out of the response in one pass — see [CursorResult] — and on + * [RequestRebuilder] to mutate the initial request's URL with the new cursor query + * parameter. + * + * ## Single read + * + * A response body is single-use. Because cursor APIs carry the items and the next cursor in + * the same payload, the extractor reads the body **once** and returns both via a + * [CursorResult]. This avoids the double-drain trap of pulling items and cursor with two + * independent `(Response) -> …` lambdas, which forces either a failed second read or a + * per-response cache to work around it. * * @param T Element type extracted from the response. - * @property itemsExtractor Reads the list of items from the response. Called once per - * page; must drain the response body synchronously. - * @property cursorExtractor Reads the next cursor from the response, or returns `null` - * if there are no more pages. + * @property extractor Reads the items and next cursor from the response in a single pass. + * Called once per page; must drain the response body synchronously and return both pieces. * @property cursorQueryParam Query parameter name used to send the cursor (default * `"cursor"`). Servers vary; common alternatives include `"after"`, `"next"`, * `"pageCursor"`. @@ -37,16 +44,44 @@ import org.dexpace.sdk.core.http.response.Response public class CursorPaginationStrategy @JvmOverloads constructor( - private val itemsExtractor: (Response) -> List, - private val cursorExtractor: (Response) -> String?, + private val extractor: (Response) -> CursorResult, private val cursorQueryParam: String = "cursor", ) : PaginationStrategy { + /** + * Creates a strategy from two body-reading lambdas. + * + * @deprecated Each lambda drains the single-use response body separately, so the body + * is read twice per page. Use the single-pass [extractor] constructor that returns a + * [CursorResult] instead. + */ + @Deprecated( + message = + "Two body-reading lambdas drain the single-use response body twice per page. " + + "Pass a single extractor returning a CursorResult instead.", + replaceWith = + ReplaceWith( + "CursorPaginationStrategy({ response -> " + + "CursorResult(itemsExtractor(response), cursorExtractor(response)) }, " + + "cursorQueryParam)", + "org.dexpace.sdk.core.pagination.CursorResult", + ), + ) + @JvmOverloads + public constructor( + itemsExtractor: (Response) -> List, + cursorExtractor: (Response) -> String?, + cursorQueryParam: String = "cursor", + ) : this( + extractor = { response -> CursorResult(itemsExtractor(response), cursorExtractor(response)) }, + cursorQueryParam = cursorQueryParam, + ) + override fun parse( response: Response, initialRequest: Request, ): Page { - val items: List = itemsExtractor(response) - val nextCursor: String? = cursorExtractor(response) + val result: CursorResult = extractor(response) + val nextCursor: String? = result.nextCursor val hasNext: Boolean = !nextCursor.isNullOrEmpty() val nextRequest: Request? = if (hasNext) { @@ -54,6 +89,6 @@ public class CursorPaginationStrategy } else { null } - return SimplePage(items = items, hasNext = hasNext, nextRequest = nextRequest) + return SimplePage(items = result.items, hasNext = hasNext, nextRequest = nextRequest) } } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt new file mode 100644 index 00000000..573a7485 --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2026 dexpace and Omar Aljarrah + * + * Licensed under the MIT License. See LICENSE in the project root. + * SPDX-License-Identifier: MIT + */ + +package org.dexpace.sdk.core.pagination + +import org.dexpace.sdk.core.http.response.Response + +/** + * Result of a single-pass cursor extraction: the items on a page together with the cursor + * for the next page, pulled from one read of the [Response]. + * + * Cursor APIs return both the items and the next cursor in the same payload, so reading + * them with two separate `(Response) -> …` lambdas means draining the body twice. A response + * body is single-use, so the second drain either fails or — at best — is worked around with + * a per-response cache. `CursorResult` lets a [CursorPaginationStrategy] extractor parse the + * response once and hand back both pieces, keeping the public API honest about the + * single-read constraint. + * + * ## Thread-safety + * + * Instances are immutable and safe to share, provided [items] is not mutated by the caller + * after construction. The bundled extractor path always builds a `CursorResult` over a list + * the extractor owns. + * + * @param T Element type carried in [items]. + * @property items Items on the page, in server-defined order. Never `null`; may be empty. + * @property nextCursor Opaque cursor to send on the next request, or `null` (or empty) when + * the response signals end-of-stream. + */ +public class CursorResult( + public val items: List, + public val nextCursor: String?, +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is CursorResult<*>) return false + return items == other.items && nextCursor == other.nextCursor + } + + override fun hashCode(): Int = 31 * items.hashCode() + (nextCursor?.hashCode() ?: 0) + + override fun toString(): String = "CursorResult(items=$items, nextCursor=$nextCursor)" +} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt index 075caf47..3a4a8c8d 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt @@ -10,7 +10,6 @@ package org.dexpace.sdk.core.pagination import org.dexpace.sdk.core.http.request.Method import org.dexpace.sdk.core.http.request.Request import org.dexpace.sdk.core.http.response.Response -import java.util.IdentityHashMap import java.util.stream.Collectors import kotlin.test.BeforeTest import kotlin.test.Test @@ -29,34 +28,18 @@ class CursorPaginationTest { .build() /** - * Parses the body format `items=\ncursor=` into a (items, cursor) - * pair. Reads the body exactly once. + * Single-pass extractor for the body format `items=\ncursor=`. Reads + * the body exactly once and returns both the items and the next cursor as a + * [CursorResult], so there is no double-drain of the single-use response body. */ - private fun parsePayload(resp: Response): Pair, String?> { + private val extractor: (Response) -> CursorResult = { resp -> val body = resp.body!!.source().use { it.readUtf8() } val itemsLine = body.lineSequence().firstOrNull { it.startsWith("items=") } ?: "items=" val cursorLine = body.lineSequence().firstOrNull { it.startsWith("cursor=") } ?: "cursor=" val itemsRaw = itemsLine.removePrefix("items=") val cursorRaw = cursorLine.removePrefix("cursor=") val items = if (itemsRaw.isEmpty()) emptyList() else itemsRaw.split(",") - val cursor: String? = cursorRaw.ifEmpty { null } - return Pair(items, cursor) - } - - /** - * Pair of extractors that share a per-Response identity-keyed cache so the - * single-use body is read exactly once per page even though the strategy's contract - * splits items + cursor into two calls. - */ - private fun buildCachedExtractors(): Pair<(Response) -> List, (Response) -> String?> { - val cache: MutableMap, String?>> = IdentityHashMap() - val items: (Response) -> List = { r -> - cache.getOrPut(r) { parsePayload(r) }.first - } - val cursor: (Response) -> String? = { r -> - cache.getOrPut(r) { parsePayload(r) }.second - } - return Pair(items, cursor) + CursorResult(items, cursorRaw.ifEmpty { null }) } @Test @@ -72,8 +55,7 @@ class CursorPaginationTest { textResponse(req, "items=g,h,i\ncursor=") } - val (items, cursor) = buildCachedExtractors() - val strategy = CursorPaginationStrategy(items, cursor, cursorQueryParam = "cursor") + val strategy = CursorPaginationStrategy(extractor, cursorQueryParam = "cursor") val paginator = Paginator(client, initialRequest(), strategy) val collected: List = paginator.iterateAll().toList() @@ -96,8 +78,7 @@ class CursorPaginationTest { client.on("https://api.example.com/items") { req -> textResponse(req, "items=only-1,only-2\ncursor=") } - val (items, cursor) = buildCachedExtractors() - val strategy = CursorPaginationStrategy(items, cursor, "cursor") + val strategy = CursorPaginationStrategy(extractor, "cursor") val paginator = Paginator(client, initialRequest(), strategy) assertEquals(listOf("only-1", "only-2"), paginator.iterateAll().toList()) assertEquals(1, client.callCount) @@ -123,8 +104,7 @@ class CursorPaginationTest { .addHeader("Authorization", "Bearer xyz") .build() - val (items, cursor) = buildCachedExtractors() - val strategy = CursorPaginationStrategy(items, cursor) + val strategy = CursorPaginationStrategy(extractor) val paginator = Paginator(client, authRequest, strategy) assertEquals(listOf("a", "b"), paginator.iterateAll().toList()) } @@ -139,8 +119,7 @@ class CursorPaginationTest { textResponse(req, "items=3,4\ncursor=") } - val (items, cursor) = buildCachedExtractors() - val strategy = CursorPaginationStrategy(items, cursor) + val strategy = CursorPaginationStrategy(extractor) val paginator = Paginator(client, initialRequest(), strategy) val streamed: List = paginator.streamAll().collect(Collectors.toList()) assertEquals(listOf("1", "2", "3", "4"), streamed) @@ -161,8 +140,7 @@ class CursorPaginationTest { textResponse(req, "items=two\ncursor=") } - val (items, cursor) = buildCachedExtractors() - val strategy = CursorPaginationStrategy(items, cursor, cursorQueryParam = "page_token") + val strategy = CursorPaginationStrategy(extractor, cursorQueryParam = "page_token") val paginator = Paginator(client, initialRequest(), strategy) assertEquals(listOf("one", "two"), paginator.iterateAll().toList()) assertEquals( diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt new file mode 100644 index 00000000..3a787857 --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt @@ -0,0 +1,164 @@ +/* + * Copyright (c) 2026 dexpace and Omar Aljarrah + * + * Licensed under the MIT License. See LICENSE in the project root. + * SPDX-License-Identifier: MIT + */ + +package org.dexpace.sdk.core.pagination + +import org.dexpace.sdk.core.http.request.Method +import org.dexpace.sdk.core.http.request.Request +import org.dexpace.sdk.core.http.response.Response +import java.util.concurrent.atomic.AtomicInteger +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * Exercises the single-read extractor on [CursorPaginationStrategy]: items and the next + * cursor are pulled from one pass over the response, so a single-use body is read exactly + * once per page. + */ +class CursorSingleReadTest { + @BeforeTest + fun setup() { + installIoProvider() + } + + private fun initialRequest(): Request = + Request.builder() + .url("https://api.example.com/items") + .method(Method.GET) + .build() + + /** + * Parses the body format `items=\ncursor=` into a [CursorResult]. + * Reads the body exactly once and reports each read through [reads]. + */ + private fun singleReadExtractor(reads: AtomicInteger): (Response) -> CursorResult = + { resp -> + reads.incrementAndGet() + val body = resp.body!!.source().use { it.readUtf8() } + val itemsLine = body.lineSequence().firstOrNull { it.startsWith("items=") } ?: "items=" + val cursorLine = body.lineSequence().firstOrNull { it.startsWith("cursor=") } ?: "cursor=" + val itemsRaw = itemsLine.removePrefix("items=") + val cursorRaw = cursorLine.removePrefix("cursor=") + val items = if (itemsRaw.isEmpty()) emptyList() else itemsRaw.split(",") + CursorResult(items, cursorRaw.ifEmpty { null }) + } + + @Test + fun `single read extractor reads the body once per page`() { + val client = StubHttpClient() + client.on("https://api.example.com/items") { req -> + textResponse(req, "items=a,b,c\ncursor=abc") + } + client.on("https://api.example.com/items?cursor=abc") { req -> + textResponse(req, "items=d,e,f\ncursor=") + } + + val reads = AtomicInteger(0) + val strategy = CursorPaginationStrategy(singleReadExtractor(reads)) + val paginator = Paginator(client, initialRequest(), strategy) + + val collected: List = paginator.iterateAll().toList() + + assertEquals(listOf("a", "b", "c", "d", "e", "f"), collected) + // Two pages fetched, two HTTP calls, and exactly one body read per page — no double drain. + assertEquals(2, client.callCount) + assertEquals(2, reads.get()) + } + + @Test + fun `single read extractor parses each response exactly once`() { + // A response whose body cannot be read twice: the second source() call throws. This + // makes a double-drain a hard failure rather than a silently-cached read. + val parses = AtomicInteger(0) + val client = StubHttpClient() + client.on("https://api.example.com/items") { req -> + singleUseResponse(req, "items=x,y\ncursor=") + } + + val parsing: (Response) -> CursorResult = { resp -> + parses.incrementAndGet() + val body = resp.body!!.source().use { it.readUtf8() } + val itemsRaw = body.substringAfter("items=").substringBefore('\n') + CursorResult(itemsRaw.split(","), null) + } + val strategy = CursorPaginationStrategy(parsing) + val paginator = Paginator(client, initialRequest(), strategy) + + assertEquals(listOf("x", "y"), paginator.iterateAll().toList()) + assertEquals(1, parses.get()) + assertEquals(1, client.callCount) + } + + @Test + fun `CursorResult carries items and next cursor`() { + val present = CursorResult(listOf("one", "two"), "next-cursor") + assertEquals(listOf("one", "two"), present.items) + assertEquals("next-cursor", present.nextCursor) + + val end = CursorResult(listOf("last"), null) + assertEquals(listOf("last"), end.items) + assertNull(end.nextCursor) + } + + @Test + @Suppress("DEPRECATION") + fun `deprecated two-lambda constructor still paginates for back-compat`() { + // The legacy two-lambda constructor is retained but deprecated; it delegates to the + // single-pass path. This proves existing callers keep working. The two lambdas read + // disjoint parts of the response (items from the body, cursor from a header) so the + // single-use body is not the thing being drained twice — that double-drain trap is + // exactly what the deprecation steers callers away from. + val client = StubHttpClient() + client.on("https://api.example.com/items") { req -> + textResponse(req, "a,b", extraHeaders = mapOf("X-Next-Cursor" to "more")) + } + client.on("https://api.example.com/items?cursor=more") { req -> + textResponse(req, "c") + } + + val itemsExtractor: (Response) -> List = { resp -> + val body = resp.body!!.source().use { it.readUtf8() } + if (body.isEmpty()) emptyList() else body.split(",") + } + val cursorExtractor: (Response) -> String? = { resp -> + resp.headers.get("X-Next-Cursor") + } + + val strategy = CursorPaginationStrategy(itemsExtractor, cursorExtractor) + val paginator = Paginator(client, initialRequest(), strategy) + + assertEquals(listOf("a", "b", "c"), paginator.iterateAll().toList()) + assertEquals(2, client.callCount) + } + + @Test + fun `single read extractor honours a custom cursor query param`() { + val client = StubHttpClient() + client.on("https://api.example.com/items") { req -> + textResponse(req, "items=one\ncursor=tok") + } + client.on("https://api.example.com/items?page_token=tok") { req -> + textResponse(req, "items=two\ncursor=") + } + + val reads = AtomicInteger(0) + val strategy = + CursorPaginationStrategy(singleReadExtractor(reads), cursorQueryParam = "page_token") + val paginator = Paginator(client, initialRequest(), strategy) + + assertEquals(listOf("one", "two"), paginator.iterateAll().toList()) + assertEquals( + listOf( + "https://api.example.com/items", + "https://api.example.com/items?page_token=tok", + ), + client.receivedUrls, + ) + } +} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PaginationTestSupport.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PaginationTestSupport.kt index 01d727d2..9657ffa8 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PaginationTestSupport.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PaginationTestSupport.kt @@ -52,6 +52,44 @@ private fun stringResponseBody(content: String): ResponseBody { } } +/** + * Convenience: build a 200 OK response whose body may be drained exactly once. A second + * `source()` call throws, so any double-read of the body is a hard failure rather than a + * silently-served cache — used to prove a single-pass extractor really reads once. + */ +internal fun singleUseResponse( + request: Request, + body: String, +): Response = + Response.builder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .status(Status.OK) + .headers(Headers.Builder().build()) + .body(singleUseResponseBody(body)) + .build() + +private fun singleUseResponseBody(content: String): ResponseBody { + val bytes = content.toByteArray(Charsets.UTF_8) + return object : ResponseBody() { + private var consumed = false + + override fun mediaType(): org.dexpace.sdk.core.http.common.MediaType? = null + + override fun contentLength(): Long = bytes.size.toLong() + + override fun source(): org.dexpace.sdk.core.io.BufferedSource { + check(!consumed) { "Response body already consumed: source() called twice." } + consumed = true + return Io.provider.source(bytes) + } + + override fun close() { + consumed = true + } + } +} + /** Install the Okio IoProvider so test response bodies are readable. */ internal fun installIoProvider() { Io.installProvider(OkioIoProvider) From c1e45dc3c3a25dd13d8e2fe1723f9639f518aefb Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 22:26:32 +0300 Subject: [PATCH 3/4] refactor: make CursorResult a data class and tidy strategy docs 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. --- sdk-core/api/sdk-core.api | 4 +++ .../pagination/CursorPaginationStrategy.kt | 10 ++++--- .../sdk/core/pagination/CursorResult.kt | 22 +++++---------- .../core/pagination/CursorSingleReadTest.kt | 27 +++++++++++++++++++ 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index 28ef4716..e9e73684 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -1909,6 +1909,10 @@ public final class org/dexpace/sdk/core/pagination/CursorPaginationStrategy : or public final class org/dexpace/sdk/core/pagination/CursorResult { public fun (Ljava/util/List;Ljava/lang/String;)V + public final fun component1 ()Ljava/util/List; + public final fun component2 ()Ljava/lang/String; + public final fun copy (Ljava/util/List;Ljava/lang/String;)Lorg/dexpace/sdk/core/pagination/CursorResult; + public static synthetic fun copy$default (Lorg/dexpace/sdk/core/pagination/CursorResult;Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lorg/dexpace/sdk/core/pagination/CursorResult; public fun equals (Ljava/lang/Object;)Z public final fun getItems ()Ljava/util/List; public final fun getNextCursor ()Ljava/lang/String; diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt index 361e8ff4..10747c4b 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt @@ -22,9 +22,9 @@ import org.dexpace.sdk.core.http.response.Response * - End of stream: response carries a `null` or absent next cursor. * * The strategy is **stateless**: it relies on a single [extractor] to pull both the items - * and the next cursor out of the response in one pass — see [CursorResult] — and on - * [RequestRebuilder] to mutate the initial request's URL with the new cursor query - * parameter. + * and the next cursor out of the response in one pass — see [CursorResult] — and rewrites the + * initial request's URL, setting the cursor query parameter to the new value, for the next + * page. * * ## Single read * @@ -57,7 +57,9 @@ public class CursorPaginationStrategy @Deprecated( message = "Two body-reading lambdas drain the single-use response body twice per page. " + - "Pass a single extractor returning a CursorResult instead.", + "Pass a single extractor returning a CursorResult instead. The suggested " + + "replacement only changes the API shape: collapse the two lambdas into one " + + "that reads the body a single time, otherwise it still double-drains.", replaceWith = ReplaceWith( "CursorPaginationStrategy({ response -> " + diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt index 573a7485..2a459311 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorResult.kt @@ -22,26 +22,18 @@ import org.dexpace.sdk.core.http.response.Response * * ## Thread-safety * - * Instances are immutable and safe to share, provided [items] is not mutated by the caller - * after construction. The bundled extractor path always builds a `CursorResult` over a list - * the extractor owns. + * The [items] list passed at construction is stored by reference, so a caller that retains a + * mutable list can mutate it afterwards and the change will be visible through this result. + * The bundled extractor path always builds a fresh list per page, so values produced by the + * SDK are effectively immutable; hand-built results that share a mutable list should copy it + * (`items.toList()`) before constructing. * * @param T Element type carried in [items]. * @property items Items on the page, in server-defined order. Never `null`; may be empty. * @property nextCursor Opaque cursor to send on the next request, or `null` (or empty) when * the response signals end-of-stream. */ -public class CursorResult( +public data class CursorResult( public val items: List, public val nextCursor: String?, -) { - override fun equals(other: Any?): Boolean { - if (this === other) return true - if (other !is CursorResult<*>) return false - return items == other.items && nextCursor == other.nextCursor - } - - override fun hashCode(): Int = 31 * items.hashCode() + (nextCursor?.hashCode() ?: 0) - - override fun toString(): String = "CursorResult(items=$items, nextCursor=$nextCursor)" -} +) diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt index 3a787857..d5767a2d 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt @@ -14,6 +14,7 @@ import java.util.concurrent.atomic.AtomicInteger import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNull /** @@ -137,6 +138,32 @@ class CursorSingleReadTest { assertEquals(2, client.callCount) } + @Test + @Suppress("DEPRECATION") + fun `deprecated two-lambda constructor double-drains when both lambdas read the body`() { + // Documents the hazard the single-pass API exists to remove: when both legacy lambdas + // read the body, the single-use response is drained twice and the second read fails. + val client = StubHttpClient() + client.on("https://api.example.com/items") { req -> + singleUseResponse(req, "items=a,b\ncursor=") + } + + val itemsExtractor: (Response) -> List = { resp -> + val body = resp.body!!.source().use { it.readUtf8() } + body.substringAfter("items=").substringBefore('\n').split(",") + } + val cursorExtractor: (Response) -> String? = { resp -> + // Second read of the same single-use body — this is the double-drain. + resp.body!!.source().use { it.readUtf8() } + null + } + + val strategy = CursorPaginationStrategy(itemsExtractor, cursorExtractor) + val paginator = Paginator(client, initialRequest(), strategy) + + assertFailsWith { paginator.iterateAll().toList() } + } + @Test fun `single read extractor honours a custom cursor query param`() { val client = StubHttpClient() From 493f2325dddcb1c6739f1ac36f6bb3cd2e22c581 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 23:34:35 +0300 Subject: [PATCH 4/4] refactor: remove the two-lambda CursorPaginationStrategy constructor 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. --- sdk-core/api/sdk-core.api | 3 - .../pagination/CursorPaginationStrategy.kt | 31 ---------- .../core/pagination/CursorSingleReadTest.kt | 58 ------------------- 3 files changed, 92 deletions(-) diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index 616d14f8..b4ec3744 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -1963,9 +1963,6 @@ public final class org/dexpace/sdk/core/pagination/CursorPaginationStrategy : or public fun (Lkotlin/jvm/functions/Function1;)V public fun (Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V public synthetic fun (Lkotlin/jvm/functions/Function1;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V - public fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V - public synthetic fun (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun parse (Lorg/dexpace/sdk/core/http/response/Response;Lorg/dexpace/sdk/core/http/request/Request;)Lorg/dexpace/sdk/core/pagination/Page; } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt index 10747c4b..9e5f2314 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationStrategy.kt @@ -47,37 +47,6 @@ public class CursorPaginationStrategy private val extractor: (Response) -> CursorResult, private val cursorQueryParam: String = "cursor", ) : PaginationStrategy { - /** - * Creates a strategy from two body-reading lambdas. - * - * @deprecated Each lambda drains the single-use response body separately, so the body - * is read twice per page. Use the single-pass [extractor] constructor that returns a - * [CursorResult] instead. - */ - @Deprecated( - message = - "Two body-reading lambdas drain the single-use response body twice per page. " + - "Pass a single extractor returning a CursorResult instead. The suggested " + - "replacement only changes the API shape: collapse the two lambdas into one " + - "that reads the body a single time, otherwise it still double-drains.", - replaceWith = - ReplaceWith( - "CursorPaginationStrategy({ response -> " + - "CursorResult(itemsExtractor(response), cursorExtractor(response)) }, " + - "cursorQueryParam)", - "org.dexpace.sdk.core.pagination.CursorResult", - ), - ) - @JvmOverloads - public constructor( - itemsExtractor: (Response) -> List, - cursorExtractor: (Response) -> String?, - cursorQueryParam: String = "cursor", - ) : this( - extractor = { response -> CursorResult(itemsExtractor(response), cursorExtractor(response)) }, - cursorQueryParam = cursorQueryParam, - ) - override fun parse( response: Response, initialRequest: Request, diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt index d5767a2d..20201ef1 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorSingleReadTest.kt @@ -14,7 +14,6 @@ import java.util.concurrent.atomic.AtomicInteger import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFailsWith import kotlin.test.assertNull /** @@ -107,63 +106,6 @@ class CursorSingleReadTest { assertNull(end.nextCursor) } - @Test - @Suppress("DEPRECATION") - fun `deprecated two-lambda constructor still paginates for back-compat`() { - // The legacy two-lambda constructor is retained but deprecated; it delegates to the - // single-pass path. This proves existing callers keep working. The two lambdas read - // disjoint parts of the response (items from the body, cursor from a header) so the - // single-use body is not the thing being drained twice — that double-drain trap is - // exactly what the deprecation steers callers away from. - val client = StubHttpClient() - client.on("https://api.example.com/items") { req -> - textResponse(req, "a,b", extraHeaders = mapOf("X-Next-Cursor" to "more")) - } - client.on("https://api.example.com/items?cursor=more") { req -> - textResponse(req, "c") - } - - val itemsExtractor: (Response) -> List = { resp -> - val body = resp.body!!.source().use { it.readUtf8() } - if (body.isEmpty()) emptyList() else body.split(",") - } - val cursorExtractor: (Response) -> String? = { resp -> - resp.headers.get("X-Next-Cursor") - } - - val strategy = CursorPaginationStrategy(itemsExtractor, cursorExtractor) - val paginator = Paginator(client, initialRequest(), strategy) - - assertEquals(listOf("a", "b", "c"), paginator.iterateAll().toList()) - assertEquals(2, client.callCount) - } - - @Test - @Suppress("DEPRECATION") - fun `deprecated two-lambda constructor double-drains when both lambdas read the body`() { - // Documents the hazard the single-pass API exists to remove: when both legacy lambdas - // read the body, the single-use response is drained twice and the second read fails. - val client = StubHttpClient() - client.on("https://api.example.com/items") { req -> - singleUseResponse(req, "items=a,b\ncursor=") - } - - val itemsExtractor: (Response) -> List = { resp -> - val body = resp.body!!.source().use { it.readUtf8() } - body.substringAfter("items=").substringBefore('\n').split(",") - } - val cursorExtractor: (Response) -> String? = { resp -> - // Second read of the same single-use body — this is the double-drain. - resp.body!!.source().use { it.readUtf8() } - null - } - - val strategy = CursorPaginationStrategy(itemsExtractor, cursorExtractor) - val paginator = Paginator(client, initialRequest(), strategy) - - assertFailsWith { paginator.iterateAll().toList() } - } - @Test fun `single read extractor honours a custom cursor query param`() { val client = StubHttpClient()