Skip to content

Commit 432fb50

Browse files
committed
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.
1 parent ea0cc81 commit 432fb50

7 files changed

Lines changed: 50 additions & 208 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough.
255255
| `http.auth` | `Credential` sealed hierarchy (`KeyCredential`, `NamedKeyCredential`, `BearerToken`), `BearerTokenProvider`, `AuthScheme`, `AuthMetadata`, RFC 7235 challenge parser, `BasicChallengeHandler`, `DigestChallengeHandler`, `CompositeChallengeHandler`. |
256256
| `http.sse` | `ServerSentEventReader` (WHATWG spec), `ServerSentEvent`, `ServerSentEventListener`, `BufferedSource.readServerSentEvents()`. |
257257
| `http.paging` | `PagedIterable<T>`, `PagedResponse<T>`, `PagingOptions` with `byPage()` and `stream()` accessors. |
258-
| `pagination` | `Paginator<T>` (with a `maxPages` safety cap) over cursor / page-number / token / link-header `PaginationStrategy` implementations, plus `Page<T>` / `SimplePage<T>`. |
258+
| `pagination` | `Paginator<T>` (with a `maxPages` safety cap) over cursor / page-number / link-header `PaginationStrategy` implementations, plus `Page<T>` / `SimplePage<T>`. Token-style APIs use `CursorPaginationStrategy` with the query-param name set (e.g. `"page_token"`). |
259259
| `pipeline` | Recovery-aware primitives: `RequestPipeline`, `ResponsePipeline`, `ExecutionPipeline` over a sealed `ResponseOutcome`, with steps (`pipeline.step`, `pipeline.step.retry`) like `RetryStep`, `ResponseRecoveryStep`, `IdempotencyKeyStep`, `ClientIdentityStep`. |
260260
| `serde` | `Serde`, `Serializer`, `Deserializer` abstractions and `Tristate<T>` (absent / null / present). |
261261
| `io` | `Source`, `Sink`, `Buffer`, `BufferedSource`, `BufferedSink`, `IoProvider`, `Io`, `TeeSink`. |

docs/architecture.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ Two complementary surfaces for walking multi-page responses.
346346
|-----------------------------------------------------------------|-----------------------------------------------------------------------|
347347
| `Paginator<T>` | Lazily iterates pages by re-issuing requests through an `HttpClient`; carries a `maxPages` safety cap |
348348
| `PaginationStrategy<T>` | Computes the next-page request (or stops) from the current page |
349-
| `CursorPaginationStrategy` / `PageNumberPaginationStrategy` / `TokenPaginationStrategy` / `LinkHeaderPaginationStrategy` | The four shipped strategies |
349+
| `CursorPaginationStrategy` / `PageNumberPaginationStrategy` / `LinkHeaderPaginationStrategy` | The shipped strategies. Token-style APIs use `CursorPaginationStrategy` with the query-param name set (e.g. `"page_token"`). |
350350
| `PagedIterable<T>` | First/next-page fetcher abstraction over `PagedResponse`, with its own `maxPages` cap |
351351

352352
### Serialization

docs/implementation-plan.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,8 +372,8 @@ defaults (per Square: `FAIL_ON_UNKNOWN_PROPERTIES=false`, `WRITE_DATES_AS_TIMEST
372372

373373
### WU-9: Pagination primitives
374374

375-
**Status: shipped.** `Page`, `Paginator`, `PaginationStrategy`, and the four strategies
376-
(`Cursor` / `PageNumber` / `LinkHeader` / `Token`) are in `sdk-core/.../pagination`, alongside
375+
**Status: shipped.** `Page`, `Paginator`, `PaginationStrategy`, and the three strategies
376+
(`Cursor` / `PageNumber` / `LinkHeader`) are in `sdk-core/.../pagination`, alongside
377377
helper types `SimplePage` and `RequestRebuilder`. `Paginator` gained a `maxPages` safety cap
378378
(default `Long.MAX_VALUE`) beyond the original sketch, to bound runaway iteration against servers
379379
that never advance their cursor.
@@ -390,7 +390,6 @@ link-header strategies without over-engineering. Sync first; async adapter follo
390390
- `CursorPaginationStrategy<T>(cursorPath, itemsPath, parser)` — read `next_cursor` from body
391391
- `PageNumberPaginationStrategy<T>(pageParam, itemsPath, parser)` — increment page number
392392
- `LinkHeaderPaginationStrategy<T>(itemsPath, parser)` — RFC 5988 `Link: <url>; rel="next"`
393-
- `TokenPaginationStrategy<T>(tokenPath, tokenParam, itemsPath, parser)` — token in body, sent as query param
394393
- `sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/PaginatorTests.kt` (test) — table-driven tests against MockWebServer fixtures.
395394

396395
**Acceptance criteria:**

sdk-core/api/sdk-core.api

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,13 +1937,6 @@ public final class org/dexpace/sdk/core/pagination/Paginator {
19371937
public final fun streamAll ()Ljava/util/stream/Stream;
19381938
}
19391939

1940-
public final class org/dexpace/sdk/core/pagination/TokenPaginationStrategy : org/dexpace/sdk/core/pagination/PaginationStrategy {
1941-
public fun <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V
1942-
public fun <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;)V
1943-
public synthetic fun <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
1944-
public fun parse (Lorg/dexpace/sdk/core/http/response/Response;Lorg/dexpace/sdk/core/http/request/Request;)Lorg/dexpace/sdk/core/pagination/Page;
1945-
}
1946-
19471940
public final class org/dexpace/sdk/core/pipeline/ExecutionPipeline {
19481941
public fun <init> (Lorg/dexpace/sdk/core/client/HttpClient;)V
19491942
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

Lines changed: 0 additions & 58 deletions
This file was deleted.

sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/CursorPaginationTest.kt

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import org.dexpace.sdk.core.http.request.Method
1111
import org.dexpace.sdk.core.http.request.Request
1212
import org.dexpace.sdk.core.http.response.Response
1313
import java.util.IdentityHashMap
14+
import java.util.stream.Collectors
1415
import kotlin.test.BeforeTest
1516
import kotlin.test.Test
1617
import kotlin.test.assertEquals
@@ -127,4 +128,49 @@ class CursorPaginationTest {
127128
val paginator = Paginator(client, authRequest, strategy)
128129
assertEquals(listOf("a", "b"), paginator.iterateAll().toList())
129130
}
131+
132+
@Test
133+
fun `streamAll yields the same items as iterateAll`() {
134+
val client = StubHttpClient()
135+
client.on("https://api.example.com/items") { req ->
136+
textResponse(req, "items=1,2\ncursor=c1")
137+
}
138+
client.on("https://api.example.com/items?cursor=c1") { req ->
139+
textResponse(req, "items=3,4\ncursor=")
140+
}
141+
142+
val (items, cursor) = buildCachedExtractors()
143+
val strategy = CursorPaginationStrategy(items, cursor)
144+
val paginator = Paginator(client, initialRequest(), strategy)
145+
val streamed: List<String> = paginator.streamAll().collect(Collectors.toList())
146+
assertEquals(listOf("1", "2", "3", "4"), streamed)
147+
}
148+
149+
@Test
150+
fun `cursor with special characters is URL encoded in next request`() {
151+
// Opaque cursors may contain `=` `+` `/` characters (base64) — the rebuilder must
152+
// URL-encode them so the server sees the original value unmangled. A custom query
153+
// param name (e.g. `page_token`) covers token-style APIs that reuse this strategy.
154+
val rawCursor = "a+b/c="
155+
val encoded = "a%2Bb%2Fc%3D"
156+
val client = StubHttpClient()
157+
client.on("https://api.example.com/items") { req ->
158+
textResponse(req, "items=one\ncursor=$rawCursor")
159+
}
160+
client.on("https://api.example.com/items?page_token=$encoded") { req ->
161+
textResponse(req, "items=two\ncursor=")
162+
}
163+
164+
val (items, cursor) = buildCachedExtractors()
165+
val strategy = CursorPaginationStrategy(items, cursor, cursorQueryParam = "page_token")
166+
val paginator = Paginator(client, initialRequest(), strategy)
167+
assertEquals(listOf("one", "two"), paginator.iterateAll().toList())
168+
assertEquals(
169+
listOf(
170+
"https://api.example.com/items",
171+
"https://api.example.com/items?page_token=$encoded",
172+
),
173+
client.receivedUrls,
174+
)
175+
}
130176
}

sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/TokenPaginationTest.kt

Lines changed: 0 additions & 138 deletions
This file was deleted.

0 commit comments

Comments
 (0)