From a4f33b81882377eafda3213f21b9f1551d7e71b3 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Wed, 17 Jun 2026 05:15:38 +0300 Subject: [PATCH] refactor: unify pagination onto a single Page-based surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK carried two parallel pagination implementations: - pagination/* — Paginator + PaginationStrategy + Page, which drives HTTP requests through an HttpClient, closes each response after the strategy parses it, applies a maxPages safety cap, and ships cursor, page-number, and link-header strategies. - http/paging/* — PagedIterable + PagedResponse + PagingOptions, a thinner first-/next-page fetcher abstraction whose byPage() sequence leaked the underlying response (and pooled connection) on partial consumption unless the caller closed every page by hand. Two surfaces for the same job means duplicated concepts, divergent close semantics, and a real connection leak in the second one. This collapses everything onto the Paginator/Page stack — the more complete one, with correct per-page response lifecycle — and removes the PagedIterable stack entirely. To preserve the page-level signals PagedResponse exposed, the Page interface now carries optional response metadata (statusCode, headers) and the HATEOAS links / continuation token (nextLink, previousLink, firstLink, lastLink, continuationToken). The accessors default to null, so existing third-party Page implementations keep compiling unchanged; the bundled strategies populate the envelope and links they already parse. LinkHeaderPaginationStrategy now extracts all four navigation relations instead of only rel="next". Public API: removes org.dexpace.sdk.core.http.paging.*; adds the default metadata accessors to Page. api/sdk-core.api regenerated accordingly. Closes #30 --- README.md | 3 +- docs/architecture.md | 10 +- docs/refs-comparison.md | 6 +- sdk-core/api/sdk-core.api | 75 +-- .../sdk/core/http/paging/PagedIterable.kt | 193 ------ .../sdk/core/http/paging/PagedResponse.kt | 77 --- .../sdk/core/http/paging/PagingOptions.kt | 47 -- .../pagination/CursorPaginationStrategy.kt | 9 +- .../LinkHeaderPaginationStrategy.kt | 32 +- .../org/dexpace/sdk/core/pagination/Page.kt | 51 ++ .../PageNumberPaginationStrategy.kt | 16 +- .../dexpace/sdk/core/pagination/SimplePage.kt | 12 + .../sdk/core/http/paging/PagedIterableTest.kt | 633 ------------------ .../sdk/core/pagination/PageMetadataTest.kt | 147 ++++ 14 files changed, 280 insertions(+), 1031 deletions(-) delete mode 100644 sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagedIterable.kt delete mode 100644 sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagedResponse.kt delete mode 100644 sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagingOptions.kt delete mode 100644 sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/paging/PagedIterableTest.kt create mode 100644 sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PageMetadataTest.kt diff --git a/README.md b/README.md index f3812d1d..8d241e2d 100644 --- a/README.md +++ b/README.md @@ -271,8 +271,7 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough. | `http.pipeline.steps` | Concrete steps: `RetryStep`, `RedirectStep`, `AuthStep`, `KeyCredentialAuthStep`, `BearerTokenAuthStep`, `InstrumentationStep`, `SetDateStep`, and their `*Options` / `*Condition` types. | | `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 / link-header `PaginationStrategy` implementations, plus `Page` / `SimplePage`. Token-style APIs use `CursorPaginationStrategy` with the query-param name set (e.g. `"page_token"`). | +| `pagination` | `Paginator` (with a `maxPages` safety cap) over cursor / page-number / link-header `PaginationStrategy` implementations, plus `Page` — each page carries its items and optional response metadata (`statusCode`, `headers`) and HATEOAS links / continuation token (`nextLink`, `previousLink`, `firstLink`, `lastLink`, `continuationToken`). `iterateAll()` flattens items, `streamAll()` exposes a Java 8 `Stream`, and the paginator closes each response after the strategy parses it. 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, `Tristate` (absent / null / present), and `SerdeException` (the unchecked failure adapters translate codec errors into). | | `io` | `Source`, `Sink`, `Buffer`, `BufferedSource`, `BufferedSink`, `IoProvider`, `Io`, `TeeSink`. | diff --git a/docs/architecture.md b/docs/architecture.md index d88ac9c2..3ca08104 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -79,7 +79,6 @@ java-sdk/ http/common/ Headers, MediaType, Protocol, CommonMediaTypes, ETag, HttpRange, conditions http/auth/ Credentials, RFC 7235 challenge parsing, Basic/Digest/Composite handlers http/context/ Call/dispatch/request/exchange contexts + ContextStore - http/paging/ PagedIterable, PagedResponse, PagingOptions http/pipeline/ Stage-based sync/async pipeline runtime (+ .steps) http/sse/ WHATWG Server-Sent Events reader/listener/events pipeline/ Recovery-aware Request/Response/Execution pipeline primitives (+ .step, .step.retry) @@ -343,16 +342,16 @@ for usage examples. ### Pagination -**Packages**: `org.dexpace.sdk.core.pagination`, `org.dexpace.sdk.core.http.paging` +**Package**: `org.dexpace.sdk.core.pagination` -Two complementary surfaces for walking multi-page responses. +A single surface for walking multi-page responses. | Type | Role | |-----------------------------------------------------------------|-----------------------------------------------------------------------| -| `Paginator` | Lazily iterates pages by re-issuing requests through an `HttpClient`; carries a `maxPages` safety cap | +| `Paginator` | Lazily iterates pages by re-issuing requests through an `HttpClient`; carries a `maxPages` safety cap; closes each response after the strategy parses it | | `PaginationStrategy` | Computes the next-page request (or stops) from the current page | +| `Page` | One parsed page: items plus optional response metadata (`statusCode`, `headers`) and HATEOAS links / continuation token (`nextLink`, `previousLink`, `firstLink`, `lastLink`, `continuationToken`) | | `CursorPaginationStrategy` / `PageNumberPaginationStrategy` / `LinkHeaderPaginationStrategy` | The shipped strategies | -| `PagedIterable` | 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. @@ -718,7 +717,6 @@ they should construct a fresh one. | `http.common` | Headers, MediaType, CommonMediaTypes, Protocol, ETag, HttpRange, RequestConditions | | `http.auth` | Credential, KeyCredential, BearerToken, ChallengeHandler, Basic/Digest/CompositeChallengeHandler, AuthChallengeParser | | `http.context` | CallContext, DispatchContext, RequestContext, ExchangeContext, ContextStore | -| `http.paging` | PagedIterable, PagedResponse, PagingOptions | | `http.pipeline` | HttpPipeline, HttpPipelineBuilder, HttpStep, Stage, AsyncHttpPipeline (+ `.steps`) | | `http.sse` | ServerSentEvent, ServerSentEventReader, ServerSentEventListener | | `pipeline` | RequestPipeline, ResponsePipeline, ExecutionPipeline, ResponseOutcome | diff --git a/docs/refs-comparison.md b/docs/refs-comparison.md index aea2b3b7..5f9826e7 100644 --- a/docs/refs-comparison.md +++ b/docs/refs-comparison.md @@ -230,8 +230,8 @@ below records where each scheme's design was sourced from: ### Pagination `Paginator` + `Page` ship in `sdk-core`, driven by a `PaginationStrategy` (cursor, -page-number, token, link-header), and `http.paging.PagedIterable` wraps the result. Reference -designs we drew on: +page-number, token, link-header). `Page` carries the items plus the response envelope +(`statusCode`, `headers`) and HATEOAS links / continuation token. Reference designs we drew on: - **Square**: `SyncPagingIterable` (`Iterable` lazy iterator), `SyncPage` (per-page holder), `BiDirectionalPage` (forward + backward cursors), `CustomPager` (user-implementation stub for HATEOAS). - **gax**: `PagedListResponse` driven by `PagedListDescriptor` (`injectToken`, `extractNextToken`, `extractResources`). `iterateAll()` returns lazy `Iterable`. `FixedSizeCollection` repaginates to consumer-chosen page sizes. @@ -333,7 +333,7 @@ adapters where noted): - **Idempotency-key step.** Auto-injects `Idempotency-Key: UUID.randomUUID()` for `POST`/`PUT`/`PATCH`; caller-set header wins; pluggable key strategy. [`pipeline/step/IdempotencyKeyStep.kt`] - **Auth.** `Credential` family + RFC 7235 challenge parsing + Basic/Digest/Composite `ChallengeHandler`s + `AuthStep` pillar. [`http/auth/`, `http/pipeline/steps/`] - **`sdk-serde-jackson` adapter.** Kotlin + JSR-310 + Jdk8 modules; `FAIL_ON_UNKNOWN_PROPERTIES` and `WRITE_DATES_AS_TIMESTAMPS` disabled; `Tristate` via `TristateModule`. -- **Pagination primitives.** `Paginator` + `Page` + `PaginationStrategy` (cursor / page-number / token / link-header) with a `maxPages` cap; `PagedIterable` wrapper. +- **Pagination primitives.** `Paginator` + `Page` + `PaginationStrategy` (cursor / page-number / token / link-header) with a `maxPages` cap; `Page` exposes per-page status / headers / HATEOAS links. - **Client identity header.** `ClientIdentityStep` building the `dexpace-sdk/ jvm/` token line. - **Tracer event vocabulary + metrics seam.** `HttpTracer` with named retry/request/response events; `Meter`/`LongCounter`/`DoubleHistogram` separate from tracing. - **SSE streaming.** WHATWG reader in `sdk-core`; backpressured `Flux` in `sdk-async-reactor`. diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index 7ebcc312..d388f3d9 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -577,64 +577,6 @@ public final class org/dexpace/sdk/core/http/context/RequestContext : org/dexpac public fun toString ()Ljava/lang/String; } -public abstract interface class org/dexpace/sdk/core/http/paging/FirstPageFetcher { - public abstract fun fetch (Lorg/dexpace/sdk/core/http/paging/PagingOptions;)Lorg/dexpace/sdk/core/http/paging/PagedResponse; -} - -public abstract interface class org/dexpace/sdk/core/http/paging/NextPageFetcher { - public abstract fun fetch (Lorg/dexpace/sdk/core/http/paging/PagingOptions;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/paging/PagedResponse; -} - -public final class org/dexpace/sdk/core/http/paging/PagedIterable : java/lang/Iterable, kotlin/jvm/internal/markers/KMappedMarker { - public fun (Lorg/dexpace/sdk/core/http/paging/FirstPageFetcher;)V - public fun (Lorg/dexpace/sdk/core/http/paging/FirstPageFetcher;Lorg/dexpace/sdk/core/http/paging/NextPageFetcher;)V - public fun (Lorg/dexpace/sdk/core/http/paging/FirstPageFetcher;Lorg/dexpace/sdk/core/http/paging/NextPageFetcher;J)V - public synthetic fun (Lorg/dexpace/sdk/core/http/paging/FirstPageFetcher;Lorg/dexpace/sdk/core/http/paging/NextPageFetcher;JILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun byPage ()Lkotlin/sequences/Sequence; - public final fun byPage (Lorg/dexpace/sdk/core/http/paging/PagingOptions;)Lkotlin/sequences/Sequence; - public static synthetic fun byPage$default (Lorg/dexpace/sdk/core/http/paging/PagedIterable;Lorg/dexpace/sdk/core/http/paging/PagingOptions;ILjava/lang/Object;)Lkotlin/sequences/Sequence; - public fun iterator ()Ljava/util/Iterator; - public final fun stream ()Ljava/util/stream/Stream; -} - -public final class org/dexpace/sdk/core/http/paging/PagedResponse : java/io/Closeable { - public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/util/List;)V - public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/util/List;Ljava/lang/String;)V - public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;)V - public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V - public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V - public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun close ()V - public final fun getContinuationToken ()Ljava/lang/String; - public final fun getFirstLink ()Ljava/lang/String; - public final fun getHeaders ()Lorg/dexpace/sdk/core/http/common/Headers; - public final fun getLastLink ()Ljava/lang/String; - public final fun getNextLink ()Ljava/lang/String; - public final fun getPreviousLink ()Ljava/lang/String; - public final fun getRequest ()Lorg/dexpace/sdk/core/http/request/Request; - public final fun getResponse ()Lorg/dexpace/sdk/core/http/response/Response; - public final fun getStatusCode ()I - public final fun getValue ()Ljava/util/List; -} - -public final class org/dexpace/sdk/core/http/paging/PagingOptions { - public fun ()V - public fun (Ljava/lang/Long;)V - public fun (Ljava/lang/Long;Ljava/lang/Long;)V - public fun (Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/Long;)V - public fun (Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/String;)V - public synthetic fun (Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun getContinuationToken ()Ljava/lang/String; - public final fun getOffset ()Ljava/lang/Long; - public final fun getPageIndex ()Ljava/lang/Long; - public final fun getPageSize ()Ljava/lang/Long; - public final fun setContinuationToken (Ljava/lang/String;)V - public final fun setOffset (Ljava/lang/Long;)V - public final fun setPageIndex (Ljava/lang/Long;)V - public final fun setPageSize (Ljava/lang/Long;)V -} - public final class org/dexpace/sdk/core/http/pipeline/AsyncHttpPipeline { public static final field Companion Lorg/dexpace/sdk/core/http/pipeline/AsyncHttpPipeline$Companion; public final fun getHttpClient ()Lorg/dexpace/sdk/core/client/AsyncHttpClient; @@ -2005,11 +1947,28 @@ public final class org/dexpace/sdk/core/pagination/LinkHeaderPaginationStrategy } public abstract interface class org/dexpace/sdk/core/pagination/Page { + public fun getContinuationToken ()Ljava/lang/String; + public fun getFirstLink ()Ljava/lang/String; public abstract fun getHasNext ()Z + public fun getHeaders ()Lorg/dexpace/sdk/core/http/common/Headers; public abstract fun getItems ()Ljava/util/List; + public fun getLastLink ()Ljava/lang/String; + public fun getNextLink ()Ljava/lang/String; + public fun getPreviousLink ()Ljava/lang/String; + public fun getStatusCode ()Ljava/lang/Integer; public abstract fun nextPageRequest ()Lorg/dexpace/sdk/core/http/request/Request; } +public final class org/dexpace/sdk/core/pagination/Page$DefaultImpls { + public static fun getContinuationToken (Lorg/dexpace/sdk/core/pagination/Page;)Ljava/lang/String; + public static fun getFirstLink (Lorg/dexpace/sdk/core/pagination/Page;)Ljava/lang/String; + public static fun getHeaders (Lorg/dexpace/sdk/core/pagination/Page;)Lorg/dexpace/sdk/core/http/common/Headers; + public static fun getLastLink (Lorg/dexpace/sdk/core/pagination/Page;)Ljava/lang/String; + public static fun getNextLink (Lorg/dexpace/sdk/core/pagination/Page;)Ljava/lang/String; + public static fun getPreviousLink (Lorg/dexpace/sdk/core/pagination/Page;)Ljava/lang/String; + public static fun getStatusCode (Lorg/dexpace/sdk/core/pagination/Page;)Ljava/lang/Integer; +} + public final class org/dexpace/sdk/core/pagination/PageNumberPaginationStrategy : 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/http/paging/PagedIterable.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagedIterable.kt deleted file mode 100644 index 5d739630..00000000 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagedIterable.kt +++ /dev/null @@ -1,193 +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.http.paging - -import java.util.Spliterator -import java.util.Spliterators -import java.util.stream.Stream -import java.util.stream.StreamSupport - -/** - * Fetches the first page of results for the given [PagingOptions]. Returns `null` to - * signal that there are no results. - * - * Kotlin callers may pass a lambda; Java callers may use a lambda or implement this interface. - */ -public fun interface FirstPageFetcher { - /** - * Returns the first [PagedResponse] for [options], or `null` for an empty result set. - */ - public fun fetch(options: PagingOptions): PagedResponse? -} - -/** - * Fetches a subsequent page given the continuation token from the previous page. - * - * Kotlin callers may pass a lambda; Java callers may use a lambda or implement this interface. - */ -public fun interface NextPageFetcher { - /** - * Returns the next [PagedResponse] for [options] and [continuationToken], or `null` - * to signal end of stream. - */ - public fun fetch( - options: PagingOptions, - continuationToken: String, - ): PagedResponse? -} - -/** - * Lazy iteration over a paginated REST API. - * - * `PagedIterable` flattens a sequence of pages into an `Iterable` for callers that - * only care about items, while [byPage] exposes the raw `Sequence>` for - * callers that need page-level metadata (status, headers, links). - * - * Construction takes two function references: - * - * - [firstPage] — called once with the caller-supplied [PagingOptions] to fetch the first - * page. May return `null` to signal "no data". - * - [nextPage] — called with the same [PagingOptions] and the link extracted from the - * previous page (either its [PagedResponse.nextLink] or [PagedResponse.continuationToken], - * whichever is present, with `nextLink` winning when both are set). May return `null` to - * signal end of stream. Default is `{ _, _ -> null }`, so single-page APIs can omit it. - * - * **Empty-page handling.** A page with `value.isEmpty()` is yielded as-is; the iterator - * does **not** terminate just because the current page is empty. Termination is driven - * purely by the link/token contract: - * - * - Both `nextLink` and `continuationToken` `null` → done. - * - `nextLink` or `continuationToken` is an empty `String` (`""`) → done. - * - Otherwise → call [nextPage]. If it returns `null`, done. Else yield and repeat. - * - * **Safety cap.** [maxPages] defaults to `Long.MAX_VALUE`. **Production callers should set - * a finite cap** to defend against servers that return the same `nextLink` repeatedly. The - * default is unbounded to match `Iterable` semantics; an explicit cap is the caller's - * responsibility. - * - * **Fetch strategy.** Pages are fetched sequentially and lazily — no pre-fetch. The next - * [nextPage] call only happens when the consumer pulls the next yield. Callers wanting - * parallel pre-fetch must wrap this manually. - * - * **Iteration semantics.** Each call to [iterator] or [byPage] drives a fresh sequence - * (matches the `Iterable` contract). Two concurrent iterators on the same instance maintain - * independent state. - * - * **Stream interop.** [stream] returns a Java 8 `Stream` built via `StreamSupport` over - * the same iterator. No coroutine or third-party dependency is involved. - * - * @param T Element type yielded by [iterator]. - * @property firstPage Fetches the first page. - * @property nextPage Fetches a subsequent page given the link from the previous page. - * @property maxPages Defensive cap on the total number of pages yielded. - */ -public class PagedIterable - @JvmOverloads - constructor( - private val firstPage: FirstPageFetcher, - private val nextPage: NextPageFetcher = NextPageFetcher { _, _ -> null }, - private val maxPages: Long = Long.MAX_VALUE, - ) : Iterable { - /** - * Returns a lazy sequence of pages, starting from the first page produced by - * [firstPage]. Each subsequent page is fetched only when the consumer pulls. - * - * **Page ownership.** Each yielded [PagedResponse] is owned by the consumer; this method - * does not close them. Consumers MUST `close()` each page after use (or wrap iteration - * in `use {}` / try-with-resources) to release the underlying response body. The - * flattening [iterator] path closes pages automatically — each page is closed eagerly, - * as soon as its (already fully-materialized) items are taken — so only direct callers of - * [byPage] need to manage page lifecycle. - * - * [maxPages] defaults to `Long.MAX_VALUE`. **Production callers should set a finite - * cap** to defend against servers that return the same `nextLink` repeatedly. The - * default is unbounded to match `Iterable` semantics; an explicit cap is the caller's - * responsibility. - * - * @param options Caller-supplied paging options forwarded to [firstPage] and - * [nextPage]. Default is a fresh, empty [PagingOptions]. - */ - @JvmOverloads - public fun byPage(options: PagingOptions = PagingOptions()): Sequence> = - sequence { - var page: PagedResponse? = firstPage.fetch(options) - var pageCount = 0L - while (page != null && pageCount < maxPages) { - yield(page) - pageCount++ - if (pageCount >= maxPages) break // Cap reached: do not fetch a page we will not yield. - // Prefer the explicit `nextLink`; fall back to the continuation token. - // Treat an empty string as "no more pages" — servers sometimes serialize a - // missing cursor as `""` instead of omitting the field. - val link = page.nextLink ?: page.continuationToken - page = if (link.isNullOrEmpty()) null else nextPage.fetch(options, link) - } - } - - /** - * Flattens all pages into an iterator of items. Each call starts a fresh iteration - * (re-invokes [firstPage]). - * - * The iterator owns the pages it pulls through [byPage] and closes each one eagerly, - * as soon as its items iterator is taken. Because [PagedResponse.value] is a - * fully-materialized list, the items survive the close, so a partial consume - * (`first()` / `take(n)` / `stream().findFirst()`) never strands an open response body - * or pooled connection. Callers don't need to manage page lifecycle when iterating by - * item. - * - * **Error propagation.** If `pages.next()` throws, the exception propagates immediately - * to the caller. Because Kotlin's `AbstractIterator` transitions to FAILED state after - * an exception escapes `computeNext`, subsequent calls to `hasNext()` throw - * `IllegalArgumentException` from the AbstractIterator machinery. Callers that need - * to detect the cause of the failure should catch the original exception on first throw. - */ - override fun iterator(): Iterator = - object : AbstractIterator() { - private val pages = byPage().iterator() - private var currentItems: Iterator? = null - - override fun computeNext() { - // Advance through pages until we find one with items left, or pages are exhausted. - while (true) { - val items = currentItems - if (items != null && items.hasNext()) { - setNext(items.next()) - return - } - currentItems = null - if (!pages.hasNext()) { - done() - return - } - // Any exception from pages.next() propagates directly to the caller. - // AbstractIterator transitions to FAILED state, so further hasNext() calls - // raise IllegalArgumentException — this is the correct fail-fast behavior. - val next = pages.next() - // PagedResponse.value is a fully-materialized list, so we can take the items - // iterator and close the page immediately. Closing eagerly means a partial - // consume (first()/take(n)) never abandons an open response body or pooled - // connection waiting for a later computeNext() that may never run. - currentItems = next.value.iterator() - next.close() - } - } - } - - /** - * Java 8 `Stream` view over the same iteration as [iterator]. Sequential, - * `ORDERED`, unknown-size. - */ - public fun stream(): Stream { - val spliterator = - Spliterators.spliteratorUnknownSize( - iterator(), - Spliterator.ORDERED, - ) - return StreamSupport.stream(spliterator, false) - } - } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagedResponse.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagedResponse.kt deleted file mode 100644 index c45fa297..00000000 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagedResponse.kt +++ /dev/null @@ -1,77 +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.http.paging - -import org.dexpace.sdk.core.http.common.Headers -import org.dexpace.sdk.core.http.request.Request -import org.dexpace.sdk.core.http.response.Response -import java.io.Closeable -import java.io.IOException - -/** - * A single page of results from a paginated REST API. - * - * Wraps the underlying [Response] and exposes the deserialized [value] list plus paging - * metadata. Composition (rather than inheritance) is used because [Response] is a - * `data class` with a private constructor, which cannot be subclassed cleanly. The - * underlying response is exposed via [response] and the common accessors ([statusCode], - * [headers], [request]) for direct use. - * - * Paging metadata covers two complementary patterns: - * - * - **HATEOAS links** ([nextLink], [previousLink], [firstLink], [lastLink]) — RFC 5988 - * `Link:` header style; each is a full URL the client can `GET` directly. - * - **Continuation token** ([continuationToken]) — opaque cursor that the client - * re-submits as a query parameter to fetch the next page. - * - * Servers typically use one or the other; [PagedIterable] gives precedence to [nextLink] - * when both are present. - * - * The response body is owned by this `PagedResponse`. Call [close] to release it once the - * page is no longer needed, or use Kotlin's `use {}` / Java try-with-resources. - * - * @param T Element type carried in [value]. - * @property response Underlying HTTP response the page was parsed from. - * @property value Deserialized items on this page. May be empty even when more pages - * exist (servers sometimes return empty pages with a non-null [nextLink]). - * @property continuationToken Opaque cursor for the next page, or `null` if absent. - * @property nextLink Full URL of the next page, or `null` if this is the last page. - * @property previousLink Full URL of the previous page, or `null` if this is the first. - * @property firstLink Full URL of the first page, or `null` if unsupported. - * @property lastLink Full URL of the last page, or `null` if unsupported. - */ -public class PagedResponse - @JvmOverloads - constructor( - public val response: Response, - public val value: List, - public val continuationToken: String? = null, - public val nextLink: String? = null, - public val previousLink: String? = null, - public val firstLink: String? = null, - public val lastLink: String? = null, - ) : Closeable { - /** Status code of the underlying [response]. */ - public val statusCode: Int get() = response.status.code - - /** Headers of the underlying [response]. */ - public val headers: Headers get() = response.headers - - /** Request that produced the underlying [response]. */ - public val request: Request get() = response.request - - /** - * Closes the underlying [response] and its body. - * - * @throws IOException If the underlying response close fails. - */ - @Throws(IOException::class) - override fun close() { - response.close() - } - } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagingOptions.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagingOptions.kt deleted file mode 100644 index 8861d9c1..00000000 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/paging/PagingOptions.kt +++ /dev/null @@ -1,47 +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.http.paging - -/** - * Mutable carrier for paging request parameters. - * - * Covers the three common addressing modes used by REST APIs: - * - * - **Offset-based** ([offset] + optional [pageSize]) — e.g. `?offset=20&limit=10`. - * - **Index-based** ([pageIndex] + optional [pageSize]) — e.g. `?page=2&size=10`. - * - **Token-based** ([continuationToken]) — opaque server-issued cursor, e.g. `?cursor=abc123`. - * - * All fields are nullable so a caller can opt in to a subset. Fields are `var` because - * paging options are commonly mutated as iteration progresses (e.g. a custom retriever may - * stash the latest continuation token here before requesting the next page). - * - * Instances are **not** thread-safe. - * - * ## Mutation visibility - * - * [PagedIterable] passes the **same** `PagingOptions` instance to every `nextPage` invocation — - * mutations performed inside one `nextPage` lambda (for example, updating [continuationToken] - * with the cursor returned by the previous page) are observable by all subsequent - * `firstPage` / `nextPage` calls during the iteration. The caller is responsible for choosing - * a coherent mutation strategy (read the cursor, mutate `options`, return the page). - * - * @property offset Zero-based starting index of the first item the server should return. - * @property pageSize Maximum number of items the server should return per page. - * @property pageIndex Zero-based page number to fetch (mutually exclusive with [offset] in - * most server contracts; the server defines precedence). - * @property continuationToken Opaque server-issued cursor to fetch the next page. Mutually - * exclusive with [offset] / [pageIndex] on most servers. - */ -public class PagingOptions - @JvmOverloads - constructor( - public var offset: Long? = null, - public var pageSize: Long? = null, - public var pageIndex: Long? = null, - public var continuationToken: String? = null, - ) 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 9e5f2314..ba839c57 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 @@ -60,6 +60,13 @@ public class CursorPaginationStrategy } else { null } - return SimplePage(items = result.items, hasNext = hasNext, nextRequest = nextRequest) + return SimplePage( + items = result.items, + hasNext = hasNext, + nextRequest = nextRequest, + statusCode = response.status.code, + headers = response.headers, + continuationToken = if (hasNext) nextCursor else null, + ) } } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/LinkHeaderPaginationStrategy.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/LinkHeaderPaginationStrategy.kt index f759da46..d2034582 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/LinkHeaderPaginationStrategy.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/LinkHeaderPaginationStrategy.kt @@ -59,7 +59,9 @@ public class LinkHeaderPaginationStrategy val linkValues: List = response.headers.values(linkHeader) val combined: String = if (linkValues.isEmpty()) "" else linkValues.joinToString(separator = ",") - val nextUrlString: String? = if (combined.isEmpty()) null else extractNextUrl(combined) + val links: Map = + if (combined.isEmpty()) emptyMap() else extractLinks(combined) + val nextUrlString: String? = links["next"] // Resolve the (possibly relative) next-link target against the originating page's // URL. A target that cannot be parsed into a valid URL ends the stream instead of // aborting the whole iteration with a MalformedURLException. @@ -72,7 +74,17 @@ public class LinkHeaderPaginationStrategy } else { null } - return SimplePage(items = items, hasNext = hasNext, nextRequest = nextRequest) + return SimplePage( + items = items, + hasNext = hasNext, + nextRequest = nextRequest, + statusCode = response.status.code, + headers = response.headers, + nextLink = nextUrlString, + previousLink = links["prev"] ?: links["previous"], + firstLink = links["first"], + lastLink = links["last"], + ) } /** @@ -105,18 +117,20 @@ public class LinkHeaderPaginationStrategy /** * Parses an RFC 5988 `Link` header value (possibly the concatenation of multiple - * header values) and returns the URL of the first link-value whose `rel` parameter - * contains the token `next`, or `null` if no such link-value exists. + * header values) into a map from lower-cased relation token to URL. When a relation + * appears on more than one link-value, the first occurrence wins. A link-value that + * declares several space-separated relation types is recorded under each of them. */ - private fun extractNextUrl(header: String): String? { + private fun extractLinks(header: String): Map { + val out: MutableMap = LinkedHashMap() for (entry in splitLinkValues(header)) { val parsed = parseLinkValue(entry) ?: continue - val rels = parsed.second - if (rels.any { it.equals("next", ignoreCase = true) }) { - return parsed.first + val url = parsed.first + for (rel in parsed.second) { + out.putIfAbsent(rel.lowercase(), url) } } - return null + return out } /** diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/Page.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/Page.kt index 23ca9a78..d674bcdf 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/Page.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/Page.kt @@ -7,6 +7,7 @@ package org.dexpace.sdk.core.pagination +import org.dexpace.sdk.core.http.common.Headers import org.dexpace.sdk.core.http.request.Request /** @@ -16,6 +17,21 @@ import org.dexpace.sdk.core.http.request.Request * receives a `Page` per HTTP exchange, yields its [items], and consults [hasNext] + * [nextPageRequest] to decide whether (and how) to fetch the following page. * + * ## Page metadata + * + * Beyond the items, a page may expose the HTTP envelope it was parsed from — [statusCode] + * and [headers] — and the HATEOAS links / continuation token the server advertised + * ([nextLink], [previousLink], [firstLink], [lastLink], [continuationToken]). These are the + * page-level signals callers reach for when they need more than a flat item stream: logging + * the status of each fetch, inspecting rate-limit headers, or rendering "first / previous / + * next / last" navigation. All metadata accessors default to `null`, so a strategy that does + * not know (or care about) a given signal can leave it unset; the bundled strategies populate + * the links and envelope they parse. + * + * The [Paginator] closes the underlying response as soon as the strategy returns this page, + * so a `Page` MUST NOT hold a live response or body — the metadata here is the deserialized, + * response-independent view that survives the close. + * * ## Thread-safety * * Implementations must be immutable and safe to share across threads. Strategies typically @@ -43,6 +59,41 @@ public interface Page { */ public val hasNext: Boolean + /** + * HTTP status code of the response this page was parsed from, or `null` if the strategy + * did not record it. + */ + public val statusCode: Int? get() = null + + /** + * Response headers this page was parsed from, or `null` if the strategy did not record + * them. The headers are a snapshot taken before the response was closed. + */ + public val headers: Headers? get() = null + + /** + * Full URL of the next page (RFC 5988 `rel="next"` style), or `null` if absent or + * unknown to the strategy. Distinct from [nextPageRequest], which is the actual request + * the paginator drives — this is the raw advertised link for callers that want to + * inspect or follow it themselves. + */ + public val nextLink: String? get() = null + + /** Full URL of the previous page (`rel="prev"`), or `null` if absent/unknown. */ + public val previousLink: String? get() = null + + /** Full URL of the first page (`rel="first"`), or `null` if absent/unknown. */ + public val firstLink: String? get() = null + + /** Full URL of the last page (`rel="last"`), or `null` if absent/unknown. */ + public val lastLink: String? get() = null + + /** + * Opaque continuation cursor the server issued for the next page, or `null` if the page + * is not cursor-driven or the cursor is absent. + */ + public val continuationToken: String? get() = null + /** * Returns the [Request] the paginator should execute to fetch the page following this * one, or `null` if there is no next page. Implementations build the next request from diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/PageNumberPaginationStrategy.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/PageNumberPaginationStrategy.kt index 7140aea0..8ab1a55d 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/PageNumberPaginationStrategy.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/PageNumberPaginationStrategy.kt @@ -50,7 +50,13 @@ public class PageNumberPaginationStrategy // Empty page → end of stream. Defensive against servers that "succeed" with an // empty list rather than 404 / a sentinel field once you've paged off the end. if (items.isEmpty()) { - return SimplePage(items = items, hasNext = false, nextRequest = null) + return SimplePage( + items = items, + hasNext = false, + nextRequest = null, + statusCode = response.status.code, + headers = response.headers, + ) } val executedUrl = response.request.url @@ -59,7 +65,13 @@ public class PageNumberPaginationStrategy val nextPage: Int = executedPage + 1 val nextRequest: Request = RequestRebuilder.withQueryParam(initialRequest, pageParam, nextPage.toString()) - return SimplePage(items = items, hasNext = true, nextRequest = nextRequest) + return SimplePage( + items = items, + hasNext = true, + nextRequest = nextRequest, + statusCode = response.status.code, + headers = response.headers, + ) } private fun parsePageOrDefault( diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/SimplePage.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/SimplePage.kt index bf90e5b1..c5d4ed4d 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/SimplePage.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/SimplePage.kt @@ -7,6 +7,7 @@ package org.dexpace.sdk.core.pagination +import org.dexpace.sdk.core.http.common.Headers import org.dexpace.sdk.core.http.request.Request /** @@ -15,11 +16,22 @@ import org.dexpace.sdk.core.http.request.Request * Internal because consumers should not depend on this concrete type — they receive a * [Page] from a [PaginationStrategy] and use it through that interface. Strategies built * outside `sdk-core` are free to provide their own `Page` implementations. + * + * Carries the optional page metadata ([statusCode], [headers], and the HATEOAS links / + * continuation token) so the bundled strategies can surface the response envelope they + * already parsed. Each defaults to `null` for strategies that do not record a given signal. */ internal class SimplePage( override val items: List, override val hasNext: Boolean, private val nextRequest: Request?, + override val statusCode: Int? = null, + override val headers: Headers? = null, + override val nextLink: String? = null, + override val previousLink: String? = null, + override val firstLink: String? = null, + override val lastLink: String? = null, + override val continuationToken: String? = null, ) : Page { override fun nextPageRequest(): Request? = nextRequest } diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/paging/PagedIterableTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/paging/PagedIterableTest.kt deleted file mode 100644 index d8205dad..00000000 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/paging/PagedIterableTest.kt +++ /dev/null @@ -1,633 +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.http.paging - -import org.dexpace.sdk.core.http.common.Protocol -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 org.dexpace.sdk.core.http.response.Status -import java.io.IOException -import java.util.concurrent.atomic.AtomicInteger -import java.util.stream.Collectors -import kotlin.test.BeforeTest -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertFailsWith -import kotlin.test.assertFalse -import kotlin.test.assertNotNull -import kotlin.test.assertSame -import kotlin.test.assertTrue - -class PagedIterableTest { - private lateinit var request: Request - - @BeforeTest - fun setUp() { - request = - Request.builder() - .url("https://api.example.test/items") - .method(Method.GET) - .build() - } - - // --------------------------------------------------------------------- - // Helpers - // --------------------------------------------------------------------- - - private fun emptyResponse(): Response = - Response.builder() - .request(request) - .protocol(Protocol.HTTP_1_1) - .status(Status.OK) - .build() - - /** - * Builds a [Response] whose body increments [closeCount] on each close, so a test can assert - * that the iterator released the page (response body + pooled connection). - */ - private fun closeRecordingResponse(closeCount: AtomicInteger): Response { - val body = - object : org.dexpace.sdk.core.http.response.ResponseBody() { - override fun mediaType() = null - - override fun contentLength() = -1L - - override fun source(): org.dexpace.sdk.core.io.BufferedSource = - throw UnsupportedOperationException("not needed for close test") - - override fun close() { - closeCount.incrementAndGet() - } - } - return Response.builder() - .request(request) - .protocol(Protocol.HTTP_1_1) - .status(Status.OK) - .body(body) - .build() - } - - private fun page( - value: List, - nextLink: String? = null, - continuationToken: String? = null, - previousLink: String? = null, - firstLink: String? = null, - lastLink: String? = null, - response: Response = emptyResponse(), - ): PagedResponse = - PagedResponse( - response = response, - value = value, - continuationToken = continuationToken, - nextLink = nextLink, - previousLink = previousLink, - firstLink = firstLink, - lastLink = lastLink, - ) - - // --------------------------------------------------------------------- - // Basic iteration - // --------------------------------------------------------------------- - - @Test - fun `single page with no nextLink yields all items`() { - val iterable = PagedIterable(firstPage = { page(listOf(1, 2, 3)) }) - assertEquals(listOf(1, 2, 3), iterable.toList()) - } - - @Test - fun `two pages chained via nextLink combine in order`() { - val iterable = - PagedIterable( - firstPage = { page(listOf(1, 2, 3), nextLink = "p2") }, - nextPage = { _, link -> - assertEquals("p2", link) - page(listOf(4, 5, 6)) - }, - ) - assertEquals(listOf(1, 2, 3, 4, 5, 6), iterable.toList()) - } - - @Test - fun `byPage returns each page in order without flattening`() { - val p1 = page(listOf(10, 20), nextLink = "p2") - val p2 = page(listOf(30, 40)) - val iterable = - PagedIterable( - firstPage = { p1 }, - nextPage = { _, _ -> p2 }, - ) - val pages = iterable.byPage().toList() - assertEquals(2, pages.size) - assertSame(p1, pages[0]) - assertSame(p2, pages[1]) - assertEquals(listOf(10, 20), pages[0].value) - assertEquals(listOf(30, 40), pages[1].value) - } - - // --------------------------------------------------------------------- - // Empty / null edge cases - // --------------------------------------------------------------------- - - @Test - fun `empty first page with no nextLink yields nothing`() { - val iterable = PagedIterable(firstPage = { page(emptyList()) }) - assertEquals(emptyList(), iterable.toList()) - assertFalse(iterable.iterator().hasNext()) - } - - @Test - fun `null first page yields nothing`() { - val iterable = PagedIterable(firstPage = { null }) - assertEquals(emptyList(), iterable.toList()) - } - - @Test - fun `empty page with non-null nextLink continues to next page`() { - val iterable = - PagedIterable( - firstPage = { page(emptyList(), nextLink = "p2") }, - nextPage = { _, link -> - assertEquals("p2", link) - page(listOf(1, 2)) - }, - ) - assertEquals(listOf(1, 2), iterable.toList()) - } - - @Test - fun `continuationToken used when nextLink is null`() { - val iterable = - PagedIterable( - firstPage = { page(listOf(1), continuationToken = "tok") }, - nextPage = { _, link -> - assertEquals("tok", link) - page(listOf(2)) - }, - ) - assertEquals(listOf(1, 2), iterable.toList()) - } - - @Test - fun `nextLink wins when both nextLink and continuationToken are set`() { - val iterable = - PagedIterable( - firstPage = { page(listOf(1), nextLink = "link", continuationToken = "tok") }, - nextPage = { _, link -> - assertEquals("link", link, "nextLink should take precedence over continuationToken") - page(listOf(2)) - }, - ) - assertEquals(listOf(1, 2), iterable.toList()) - } - - @Test - fun `empty string nextLink terminates iteration`() { - val nextPageCalled = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(1, 2), nextLink = "") }, - nextPage = { _, _ -> - nextPageCalled.incrementAndGet() - page(listOf(99)) - }, - ) - assertEquals(listOf(1, 2), iterable.toList()) - assertEquals(0, nextPageCalled.get(), "nextPage should not be called for empty nextLink") - } - - @Test - fun `empty string continuationToken terminates iteration`() { - val nextPageCalled = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(1, 2), continuationToken = "") }, - nextPage = { _, _ -> - nextPageCalled.incrementAndGet() - page(listOf(99)) - }, - ) - assertEquals(listOf(1, 2), iterable.toList()) - assertEquals(0, nextPageCalled.get(), "nextPage should not be called for empty continuationToken") - } - - @Test - fun `null nextPage result terminates iteration`() { - val iterable = - PagedIterable( - firstPage = { page(listOf(1), nextLink = "p2") }, - nextPage = { _, _ -> null }, - ) - assertEquals(listOf(1), iterable.toList()) - } - - // --------------------------------------------------------------------- - // Iterator contract - // --------------------------------------------------------------------- - - @Test - fun `iterator called multiple times starts fresh`() { - val firstPageCalls = AtomicInteger(0) - val iterable = - PagedIterable(firstPage = { - firstPageCalls.incrementAndGet() - page(listOf(1, 2, 3)) - }) - - assertEquals(listOf(1, 2, 3), iterable.toList()) - assertEquals(listOf(1, 2, 3), iterable.toList()) - assertEquals(2, firstPageCalls.get(), "firstPage should be re-invoked for each iteration") - } - - @Test - fun `iterator short-circuits when consumer stops pulling`() { - val firstPageCalls = AtomicInteger(0) - val nextPageCalls = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { - firstPageCalls.incrementAndGet() - page(listOf(1, 2, 3), nextLink = "p2") - }, - nextPage = { _, _ -> - nextPageCalls.incrementAndGet() - page(listOf(4, 5, 6)) - }, - ) - - // Take only the first two items — never advance past page 1. - val first = iterable.take(2).toList() - assertEquals(listOf(1, 2), first) - assertEquals(1, firstPageCalls.get()) - assertEquals(0, nextPageCalls.get(), "nextPage must not run when consumer stops mid-page") - } - - // --------------------------------------------------------------------- - // maxPages safety cap - // --------------------------------------------------------------------- - - @Test - fun `maxPages caps iteration when server returns the same nextLink repeatedly`() { - val nextPageCalls = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(1), nextLink = "same") }, - nextPage = { _, _ -> - nextPageCalls.incrementAndGet() - page(listOf(2), nextLink = "same") - }, - maxPages = 3, - ) - - // 3 pages total: first + 2 nextPage calls. Items: [1, 2, 2]. - assertEquals(listOf(1, 2, 2), iterable.toList()) - assertEquals(2, nextPageCalls.get(), "nextPage called twice to fill the cap of 3 pages") - } - - @Test - fun `maxPages of 1 stops after the first page`() { - val nextPageCalls = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(1, 2), nextLink = "p2") }, - nextPage = { _, _ -> - nextPageCalls.incrementAndGet() - page(listOf(3, 4)) - }, - maxPages = 1, - ) - assertEquals(listOf(1, 2), iterable.toList()) - assertEquals(0, nextPageCalls.get()) - } - - @Test - fun `byPage stops fetching once cap is reached on a subsequent iteration`() { - // Need maxPages = 2 to take the *inner* `pageCount >= maxPages break` - // branch — first page yielded, count becomes 1, falls through to fetch - // page 2, yields it, count becomes 2, inner break fires before fetching page 3. - val nextPageCalls = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(1), nextLink = "p2") }, - nextPage = { _, link -> - nextPageCalls.incrementAndGet() - when (link) { - "p2" -> page(listOf(2), nextLink = "p3") - else -> page(listOf(99)) - } - }, - maxPages = 2, - ) - assertEquals(listOf(1, 2), iterable.toList()) - // Exactly one nextPage call — fetch for p2, then cap stops the loop before p3. - assertEquals(1, nextPageCalls.get(), "third page must not be fetched once cap is reached") - } - - @Test - fun `byPage yields exactly maxPages pages`() { - val iterable = - PagedIterable( - firstPage = { page(listOf(1), nextLink = "p2") }, - nextPage = { _, _ -> page(listOf(2), nextLink = "p3") }, - maxPages = 2, - ) - val pages = iterable.byPage().toList() - assertEquals(2, pages.size) - assertEquals(listOf(1), pages[0].value) - assertEquals(listOf(2), pages[1].value) - } - - @Test - fun `maxPages of zero yields nothing even when firstPage returns data`() { - // Exercises the `pageCount < maxPages` false branch on the very first iteration — - // firstPage IS invoked but the loop body is skipped because the cap is reached - // before any yield. - val firstPageCalls = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { - firstPageCalls.incrementAndGet() - page(listOf(1, 2, 3)) - }, - maxPages = 0L, - ) - assertEquals(emptyList(), iterable.toList()) - assertEquals(1, firstPageCalls.get(), "firstPage is still invoked under the lazy contract") - } - - @Test - fun `explicit finite maxPages cap defends against infinite nextLink loop`() { - // Recommended production pattern: always supply a finite maxPages cap. - // Without a cap, a misbehaving server that always returns the same nextLink would - // loop forever. This test verifies that the recommended explicit-cap pattern works. - val callCount = AtomicInteger(0) - val cap = 5L - val iterable = - PagedIterable( - firstPage = { page(listOf(1), nextLink = "loop") }, - nextPage = { _, _ -> - callCount.incrementAndGet() - page(listOf(2), nextLink = "loop") // always returns same link - }, - maxPages = cap, - ) - - val items = iterable.toList() - // cap pages × 1 item each = cap items total (first page + (cap-1) nextPage calls). - assertEquals(cap.toInt(), items.size) - // nextPage was called (cap - 1) times (first page doesn't count). - assertEquals((cap - 1).toInt(), callCount.get()) - } - - // --------------------------------------------------------------------- - // Stream interop - // --------------------------------------------------------------------- - - @Test - fun `stream returns same contents as iterator`() { - val iterable = - PagedIterable( - firstPage = { page(listOf(1, 2), nextLink = "p2") }, - nextPage = { _, _ -> page(listOf(3, 4)) }, - ) - - val streamed: List = iterable.stream().collect(Collectors.toList()) - assertEquals(listOf(1, 2, 3, 4), streamed) - } - - @Test - fun `stream is independent across calls`() { - val firstPageCalls = AtomicInteger(0) - val iterable = - PagedIterable(firstPage = { - firstPageCalls.incrementAndGet() - page(listOf(1, 2, 3)) - }) - - val s1: List = iterable.stream().collect(Collectors.toList()) - val s2: List = iterable.stream().collect(Collectors.toList()) - assertEquals(listOf(1, 2, 3), s1) - assertEquals(listOf(1, 2, 3), s2) - assertEquals(2, firstPageCalls.get()) - } - - // --------------------------------------------------------------------- - // Error propagation - // --------------------------------------------------------------------- - - @Test - fun `network failure in firstPage propagates from iterator advance`() { - val iterable = - PagedIterable(firstPage = { - throw IOException("dns failure") - }) - - val iterator = iterable.iterator() - val ex = assertFailsWith { iterator.hasNext() } - assertEquals("dns failure", ex.message) - } - - @Test - fun `network failure in nextPage propagates after first page is drained`() { - val iterable = - PagedIterable( - firstPage = { page(listOf(1, 2), nextLink = "p2") }, - nextPage = { _, _ -> throw IOException("404 on next page") }, - ) - - val iterator = iterable.iterator() - // First two items come from page 1 — no error yet. - assertTrue(iterator.hasNext()) - assertEquals(1, iterator.next()) - assertEquals(2, iterator.next()) - // Pulling further forces the nextPage call, which throws. - val ex = assertFailsWith { iterator.hasNext() } - assertEquals("404 on next page", ex.message) - } - - @Test - fun `exception from pages next() propagates to the caller on first throw`() { - // When pages.next() throws, the original exception must propagate to the caller. - // AbstractIterator transitions to FAILED state after the throw, so subsequent - // hasNext() calls raise IllegalArgumentException (not the original IOException). - val iterable = - PagedIterable( - firstPage = { page(listOf(1), nextLink = "p2") }, - nextPage = { _, _ -> throw IOException("persistent failure") }, - ) - - val iterator = iterable.iterator() - assertTrue(iterator.hasNext()) - assertEquals(1, iterator.next()) - - // First attempt to advance past page 1 propagates the original IOException. - val ex = assertFailsWith { iterator.hasNext() } - assertEquals("persistent failure", ex.message) - - // AbstractIterator is now in FAILED state; further calls throw its own exception. - val failedStateEx = assertFailsWith { iterator.hasNext() } - assertNotNull(failedStateEx.message) - } - - // --------------------------------------------------------------------- - // PagedResponse.close() - // --------------------------------------------------------------------- - - @Test - fun `PagedResponse close delegates to underlying Response`() { - // Build a response whose body's close() flips a flag. The body overrides close() - // entirely (does NOT call super) — Response.close() calls body.close(), which is - // what we want to observe. - val closed = AtomicInteger(0) - val body = - object : org.dexpace.sdk.core.http.response.ResponseBody() { - override fun mediaType() = null - - override fun contentLength() = -1L - - override fun source(): org.dexpace.sdk.core.io.BufferedSource = - throw UnsupportedOperationException("not needed for close test") - - override fun close() { - closed.incrementAndGet() - } - } - val response = - Response.builder() - .request(request) - .protocol(Protocol.HTTP_1_1) - .status(Status.OK) - .body(body) - .build() - val paged = - PagedResponse( - response = response, - value = listOf(1, 2, 3), - ) - - assertEquals(Status.OK.code, paged.statusCode) - assertSame(response.headers, paged.headers) - assertSame(request, paged.request) - - paged.close() - assertEquals(1, closed.get(), "underlying body.close() should have been called once") - } - - @Test - fun `partial consume via first closes the fetched page`() { - val closeCount = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(1, 2, 3), response = closeRecordingResponse(closeCount)) }, - ) - - val firstItem = iterable.iterator().next() - - assertEquals(1, firstItem) - assertEquals(1, closeCount.get(), "the fetched page must be closed eagerly once its items are taken") - } - - @Test - fun `take(1) closes the fetched page without abandoning an open response`() { - val closeCount = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(10, 20), response = closeRecordingResponse(closeCount)) }, - ) - - val taken = iterable.take(1).toList() - - assertEquals(listOf(10), taken) - assertEquals(1, closeCount.get(), "a single-item consume must still close the page it materialized") - } - - @Test - fun `stream findFirst closes the fetched page`() { - val closeCount = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { page(listOf(7, 8, 9), response = closeRecordingResponse(closeCount)) }, - ) - - val found = iterable.stream().findFirst() - - assertTrue(found.isPresent) - assertEquals(7, found.get()) - assertEquals(1, closeCount.get(), "short-circuiting via stream().findFirst() must not strand the page") - } - - @Test - fun `full iteration closes every page exactly once`() { - val firstClose = AtomicInteger(0) - val secondClose = AtomicInteger(0) - val iterable = - PagedIterable( - firstPage = { - page(listOf(1, 2), nextLink = "p2", response = closeRecordingResponse(firstClose)) - }, - nextPage = { _, _ -> - page(listOf(3, 4), response = closeRecordingResponse(secondClose)) - }, - ) - - assertEquals(listOf(1, 2, 3, 4), iterable.toList()) - assertEquals(1, firstClose.get(), "first page closed exactly once on full iteration") - assertEquals(1, secondClose.get(), "second page closed exactly once on full iteration") - } - - // --------------------------------------------------------------------- - // PagingOptions propagation - // --------------------------------------------------------------------- - - @Test - fun `byPage forwards the supplied PagingOptions to firstPage and nextPage`() { - val seenOptionsFirst = arrayOfNulls(1) - val seenOptionsNext = arrayOfNulls(1) - val iterable = - PagedIterable( - firstPage = { opts -> - seenOptionsFirst[0] = opts - page(listOf(1), nextLink = "p2") - }, - nextPage = { opts, _ -> - seenOptionsNext[0] = opts - page(listOf(2)) - }, - ) - - val opts = PagingOptions(offset = 10, pageSize = 5, pageIndex = 2, continuationToken = "tok") - assertEquals(listOf(1, 2), iterable.byPage(opts).flatMap { it.value }.toList()) - assertSame(opts, seenOptionsFirst[0]) - assertSame(opts, seenOptionsNext[0]) - } - - @Test - fun `PagingOptions defaults are all null`() { - val opts = PagingOptions() - assertEquals(null, opts.offset) - assertEquals(null, opts.pageSize) - assertEquals(null, opts.pageIndex) - assertEquals(null, opts.continuationToken) - } - - @Test - fun `PagingOptions fields are mutable`() { - val opts = PagingOptions() - opts.offset = 100L - opts.pageSize = 25L - opts.pageIndex = 4L - opts.continuationToken = "next" - - assertEquals(100L, opts.offset) - assertEquals(25L, opts.pageSize) - assertEquals(4L, opts.pageIndex) - assertEquals("next", opts.continuationToken) - } -} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PageMetadataTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PageMetadataTest.kt new file mode 100644 index 00000000..ac1e7082 --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/PageMetadataTest.kt @@ -0,0 +1,147 @@ +/* + * 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 kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * Covers the page-level metadata ([Page.statusCode], [Page.headers], and the HATEOAS links / + * continuation token) that the unified pagination surface exposes — the signals the retired + * `PagedResponse` used to carry, now folded into the single [Page] contract. + */ +class PageMetadataTest { + @BeforeTest + fun setup() { + installIoProvider() + } + + private fun initialRequest(): Request = + Request.builder() + .url("https://api.example.com/repo/issues") + .method(Method.GET) + .build() + + private val itemsExtractor: (Response) -> List = { resp -> + val body = resp.body!!.source().use { it.readUtf8() } + if (body.isEmpty()) emptyList() else body.split(",") + } + + @Test + fun `default Page metadata accessors are null`() { + val page: Page = + object : Page { + override val items: List = listOf("a") + override val hasNext: Boolean = false + + override fun nextPageRequest(): Request? = null + } + assertNull(page.statusCode) + assertNull(page.headers) + assertNull(page.nextLink) + assertNull(page.previousLink) + assertNull(page.firstLink) + assertNull(page.lastLink) + assertNull(page.continuationToken) + } + + @Test + fun `link header strategy surfaces status, headers, and all four navigation links`() { + val request = initialRequest() + val linkHeader = + "; rel=\"next\", " + + "; rel=\"prev\", " + + "; rel=\"first\", " + + "; rel=\"last\"" + val response = + textResponse( + request, + "i1,i2", + extraHeaders = mapOf("Link" to linkHeader, "X-Total-Count" to "84"), + ) + + val page = LinkHeaderPaginationStrategy(itemsExtractor).parse(response, request) + + assertEquals(200, page.statusCode) + assertEquals("84", page.headers?.get("X-Total-Count")) + assertEquals("https://api.example.com/repo/issues?page=2", page.nextLink) + assertEquals("https://api.example.com/repo/issues?page=1", page.previousLink) + assertEquals("https://api.example.com/repo/issues?page=1", page.firstLink) + assertEquals("https://api.example.com/repo/issues?page=42", page.lastLink) + assertNull(page.continuationToken) + } + + @Test + fun `link header strategy leaves links null when no Link header is present`() { + val request = initialRequest() + val response = textResponse(request, "only") + + val page = LinkHeaderPaginationStrategy(itemsExtractor).parse(response, request) + + assertEquals(200, page.statusCode) + assertNull(page.nextLink) + assertNull(page.previousLink) + assertNull(page.firstLink) + assertNull(page.lastLink) + } + + @Test + fun `cursor strategy carries the next cursor as continuationToken`() { + val request = initialRequest() + val response = textResponse(request, "i1,i2") + val extractor: (Response) -> CursorResult = { resp -> + val items = resp.body!!.source().use { it.readUtf8() }.split(",") + CursorResult(items, nextCursor = "cursor-xyz") + } + + val page = CursorPaginationStrategy(extractor).parse(response, request) + + assertEquals(200, page.statusCode) + assertEquals("cursor-xyz", page.continuationToken) + assertNull(page.nextLink) + } + + @Test + fun `cursor strategy leaves continuationToken null on the terminal page`() { + val request = initialRequest() + val response = textResponse(request, "i1") + val extractor: (Response) -> CursorResult = { resp -> + val items = resp.body!!.source().use { it.readUtf8() }.split(",") + CursorResult(items, nextCursor = null) + } + + val page = CursorPaginationStrategy(extractor).parse(response, request) + + assertEquals(200, page.statusCode) + assertNull(page.continuationToken) + } + + @Test + fun `page number strategy records status and headers on both populated and empty pages`() { + val request = initialRequest() + val populated = + textResponse(request, "a,b", extraHeaders = mapOf("X-Page" to "1")) + val empty = + textResponse(request, "", extraHeaders = mapOf("X-Page" to "9")) + val strategy = PageNumberPaginationStrategy(itemsExtractor) + + val populatedPage = strategy.parse(populated, request) + assertEquals(200, populatedPage.statusCode) + assertEquals("1", populatedPage.headers?.get("X-Page")) + + val emptyPage = strategy.parse(empty, request) + assertEquals(200, emptyPage.statusCode) + assertEquals("9", emptyPage.headers?.get("X-Page")) + assertEquals(false, emptyPage.hasNext) + } +}