refactor: remove duplicate TokenPaginationStrategy in favor of CursorPaginationStrategy (#94)

PR: #94

Author: OmarAlJarrah

README.md

@@ -256,7 +256,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<T>`, `PagedResponse<T>`, `PagingOptions` with `byPage()` and `stream()` accessors. |
-| `pagination` | `Paginator<T>` (with a `maxPages` safety cap) over cursor / page-number / token / link-header `PaginationStrategy` implementations, plus `Page<T>` / `SimplePage<T>`. |
+| `pagination` | `Paginator<T>` (with a `maxPages` safety cap) over cursor / page-number / link-header `PaginationStrategy` implementations, plus `Page<T>` / `SimplePage<T>`. |
 | `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<T>` (absent / null / present). |
 | `io` | `Source`, `Sink`, `Buffer`, `BufferedSource`, `BufferedSink`, `IoProvider`, `Io`, `TeeSink`. |
@@ -266,6 +266,9 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough.
 | `util` | `Clock`, `Uuids` (non-blocking v4), `DateTimeRfc1123`, `RetryUtils`, `ProxyOptions`, `Futures`. |
 | `generics` | `Builder<T>` — the generic builder interface every SDK builder implements. |
 
+Token-style APIs (`next_page_token`, `pageToken`, …) are served by `CursorPaginationStrategy`:
+construct it with the desired query-param name, e.g. `CursorPaginationStrategy(items, extractor, "page_token")`.
+
 ## Building
 
 ```bash

docs/architecture.md

@@ -350,9 +350,12 @@ Two complementary surfaces for walking multi-page responses.
 |-----------------------------------------------------------------|-----------------------------------------------------------------------|
 | `Paginator<T>`                                                  | Lazily iterates pages by re-issuing requests through an `HttpClient`; carries a `maxPages` safety cap |
 | `PaginationStrategy<T>`                                         | 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 |
 | `PagedIterable<T>`                                              | First/next-page fetcher abstraction over `PagedResponse`, with its own `maxPages` cap |
 
+Token-style APIs (`next_page_token`, `pageToken`, …) are handled by `CursorPaginationStrategy`
+constructed with the query-param name set (e.g. `"page_token"`), so no separate token strategy is needed.
+
 ### Serialization
 
 **Package**: `org.dexpace.sdk.core.serde`

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<T>(cursorPath, itemsPath, parser)` — read `next_cursor` from body
   - `PageNumberPaginationStrategy<T>(pageParam, itemsPath, parser)` — increment page number
   - `LinkHeaderPaginationStrategy<T>(itemsPath, parser)` — RFC 5988 `Link: <url>; rel="next"`
-  - `TokenPaginationStrategy<T>(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:**

sdk-core/api/sdk-core.api

@@ -1958,13 +1958,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 <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V
-	public fun <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V
-	public synthetic fun <init> (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 <init> (Lorg/dexpace/sdk/core/client/HttpClient;)V
 	public fun <init> (Lorg/dexpace/sdk/core/client/HttpClient;Lorg/dexpace/sdk/core/pipeline/RequestPipeline;)V

sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationStrategy.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,73 @@ 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<String> = 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.
+        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?cursor=$encoded") { req ->
+            textResponse(req, "items=two\ncursor=")
+        }
+
+        val (items, cursor) = buildCachedExtractors()
+        val strategy = CursorPaginationStrategy(items, cursor)
+        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?cursor=$encoded",
+            ),
+            client.receivedUrls,
+        )
+    }
+
+    @Test
+    fun `custom query-param name is used for the next-page cursor`() {
+        // Token-style APIs (next_page_token, pageToken, …) are served by setting
+        // cursorQueryParam; the next request must carry the cursor under that name.
+        val client = StubHttpClient()
+        client.on("https://api.example.com/items") { req ->
+            textResponse(req, "items=one\ncursor=tok1")
+        }
+        client.on("https://api.example.com/items?page_token=tok1") { 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=tok1",
+            ),
+            client.receivedUrls,
+        )
+    }
 }