diff --git a/README.md b/README.md index 9b8617fa..2378844f 100644 --- a/README.md +++ b/README.md @@ -265,7 +265,7 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough. | `http.request` | `Request`, `RequestBody`, `FileRequestBody`, `LoggableRequestBody`, `Method`. | | `http.response` | `Response`, `ResponseBody`, `LoggableResponseBody`, `Status` (a value-carrying class with a total `fromCode`), plus the raw-vs-parsed seam: `ResponseHandler` (with dep-free `string()`/`empty()` handlers) and a lazy, parse-once `ParsedResponse`. | | `http.response.exception` | Typed `HttpException` hierarchy (`BadRequestException`, `RequestTimeoutException`, `TooManyRequestsException`, `ServiceUnavailableException`, …) with `isRetryable` derived from `RetryUtils.isRetryable` and exposed via the `Retryable` interface, plus `NetworkException` and `HttpExceptionFactory`. | -| `http.common` | `Headers`, `HttpHeaderName` (interned), `MediaType`, `Protocol`, `HttpRange`, `ETag`, `RequestConditions`. | +| `http.common` | `Headers`, `HttpHeaderName` (interned), `QueryParams` (RFC 3986 query multimap), `MediaType`, `Protocol`, `HttpRange`, `ETag`, `RequestConditions`. | | `http.context` | `CallContext` → `DispatchContext` → `RequestContext` → `ExchangeContext` chain, `ContextStore`. | | `http.pipeline` | Sync (`HttpStep` / `HttpPipeline` / `HttpPipelineBuilder` / `PipelineNext` / `Stage`) and async (`AsyncHttpStep` / `AsyncHttpPipeline` / `AsyncHttpPipelineBuilder` / `AsyncPipelineNext`) pipeline machinery, plus `AsyncPipelineBridges`. | | `http.pipeline.steps` | Concrete steps: `RetryStep`, `RedirectStep`, `AuthStep`, `KeyCredentialAuthStep`, `BearerTokenAuthStep`, `InstrumentationStep`, `SetDateStep`, and their `*Options` / `*Condition` types. | @@ -273,6 +273,7 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough. | `http.paging` | `PagedIterable`, `PagedResponse`, `PagingOptions` with `byPage()` and `stream()` accessors. | | `auth` | `Credential` sealed hierarchy (`KeyCredential`, `NamedKeyCredential`, `BearerToken`), `BearerTokenProvider`, `AuthScheme`, per-operation `AuthRequirement` / `AuthDescriptor` with `AuthDescriptorResolver` precedence ladder, RFC 7235 challenge parser, `BasicChallengeHandler`, `DigestChallengeHandler`, `CompositeChallengeHandler`. | | `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"`). | +| `operation` | `OperationParams` — SPI projecting an operation's typed inputs (path / query / header / body) into a `Request` and the context chain, via `toRequest(baseUrl)` / `toRequestContext(baseUrl, dispatch)`. | | `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 f0bc5d5a..df227a5f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -28,6 +28,7 @@ concerns. - [JDK 8 Compatibility](#jdk-8-compatibility) - [Cross-Compile Toolchain Discipline](#cross-compile-toolchain-discipline) - [Immutability and Builders](#immutability-and-builders) + - [Request URL Model](#request-url-model) - [Virtual Thread Safety](#virtual-thread-safety) - [Internal Visibility](#internal-visibility) - [Cancellation](#cancellation) @@ -596,6 +597,18 @@ data class Request private constructor( - **`newBuilder()`**: Creates a pre-filled builder for modification - **`Builder`**: Generic interface ensuring all builders have `fun build(): T` +### Request URL Model + +`Request` stores its target as a single resolved `java.net.URL` and layers the `QueryParams` +multimap (`http.common`) on top for structured query manipulation, rather than modelling a fully +deconstructed URL value object. This preserves the DNS-free equality contract — `Request` compares +`url.toExternalForm()`, a pure string comparison, because `java.net.URL.equals` / `hashCode` +resolve the host via DNS (blocking, and wrong for virtual hosts) — and puts a structured model +where the manipulation pressure actually is (the query string). A deconstructed `Url` model and/or +a `java.net.URI` migration remain deferred until richer path handling earns them; minimal +path-template substitution lives in the `OperationParams` SPI. Full rationale and the alternatives +weighed: see the "Request URL Model" section in [`http.md`](http.md). + ### Virtual Thread Safety The SDK uses `ReentrantLock` over `synchronized` wherever locking is needed: diff --git a/docs/http.md b/docs/http.md index 3a4d8c7c..25ac8ac1 100644 --- a/docs/http.md +++ b/docs/http.md @@ -438,6 +438,67 @@ headers.get("content-type") // "application/json" (case-insensitive) headers.values("Cache-Control") // ["no-cache", "no-store"] ``` +### QueryParams + +`QueryParams` is an immutable, insertion-ordered, multi-valued model of a URL query string — +the `?name=value&...` portion of a URL. It mirrors `Headers` in shape (private constructor, +mutable `Builder`, multi-value semantics) but differs in three ways: names are +**case-sensitive** (`?page=1` and `?Page=1` are distinct), values may be **empty or value-less** +(`?flag` and `?flag=` both occur in the wild), and equality is **order-sensitive** — two +instances are equal only if they `encode()` identically. (That last point is the one divergence +from `Headers`, whose case-folded names make name order non-semantic; here order is a rendered +property, so it counts.) + +```kotlin +class QueryParams private constructor( + private val paramsMap: Map> +) +``` + +**Role — building queries, not editing URLs.** `QueryParams` is an *origination* model: it +builds a query string from decoded names/values (for example, projecting an operation's inputs +into a request). It is **not** a fidelity-preserving editor of an existing URL. `encode()` +re-renders every parameter in canonical form, so round-tripping an arbitrary URL through `parse` +then `encode` can change the wire form of parameters you never touched (`?flag` → `flag=`, +reserved characters percent-encoded). Code that must edit one parameter of an existing URL while +leaving the rest byte-for-byte — pagination's `RequestRebuilder` — splices the raw query string +directly instead of going through `encode()`. + +**API:** + +| Method | Description | +|------------------|----------------------------------------------------------------------------| +| `get(name)` | First value for the name, or `null` if absent (`""` for a value-less param)| +| `values(name)` | All values for the name (unmodifiable), or empty list | +| `contains(name)` | Whether any value is present for the name | +| `names()` | Immutable, insertion-ordered snapshot of all parameter names | +| `entries()` | Immutable snapshot as `Map.Entry>` | +| `size()` | Total number of values across all names (derived, not tracked) | +| `isEmpty()` | Whether there are no parameters | +| `encode()` | RFC 3986 query string (space → `%20`, literal `+` → `%2B`), no leading `?` | +| `newBuilder()` | Returns a pre-filled `Builder` for modification | + +**Encoding.** `encode()` / `parse()` use **RFC 3986 query semantics** (via the internal +`PercentEncoding` helper): a space is `%20` (not `+`), and a literal `+` is `%2B` — it is **not** +read back as a space. This is deliberately *not* `application/x-www-form-urlencoded`: a query +*assembled as a request body* uses the form scheme (`+` for spaces) and will be a separate +form-body type, not `QueryParams.encode()`. `parse(encode(...))` round-trips names, values, and +order; malformed percent-encoding falls back to raw text rather than throwing. + +**Builder:** + +```kotlin +val params = QueryParams.builder() + .add("tag", "a") + .add("tag", "b") // multi-value + .set("page", "2") // replaces any existing "page" + .build() + +params.values("tag") // ["a", "b"] +params.get("page") // "2" +params.encode() // "tag=a&tag=b&page=2" +``` + ### MediaType `MediaType` represents a parsed MIME type with optional parameters: @@ -685,6 +746,70 @@ Both implement `HttpClient` and `AsyncHttpClient` on a single class. See the REA --- +## Operation Input Projection + +`OperationParams` (`org.dexpace.sdk.core.operation`) is the SPI a thin generated service implements +once per operation to declare where each typed input belongs on the wire — **path**, **query**, +**header**, or **body** — so generated code (and typed pagination) never splices a URL string. The +runtime assembles the `Request` and feeds it into the context chain. + +```kotlin +interface OperationParams { + val method: Method + val pathTemplate: String // "/pets/{petId}"; leading "/" optional + val operationName: String? // for the tracing seam; default null + + fun pathParams(): Map // default emptyMap() + fun queryParams(): QueryParams // default empty + fun headers(): Headers // default empty + fun body(): RequestBody? // default null + + fun toRequest(baseUrl: String): Request + fun toRequestContext(baseUrl: String, dispatch: DispatchContext): RequestContext +} +``` + +Only `method` and `pathTemplate` are required; the four projections default to empty, so a +parameterless operation overrides almost nothing. + +**Assembly** (`toRequest`): + +- **Path** — each `{name}` in `pathTemplate` is replaced with its `pathParams()` value, + percent-encoded as a path segment (`/` → `%2F`), so a value cannot inject extra segments. A + `{name}` with no value throws `IllegalArgumentException`. +- **Query** — `queryParams().encode()` (RFC 3986) is appended after `?`. +- **Base URL** — the scheme/host/port/base-path carry through unchanged; a trailing `/` is trimmed + and exactly one `/` joins it to the resolved path, so `https://api.example.com/v1` + `/pets` → + `…/v1/pets`. A query already on the base URL is preserved: the resolved path is inserted **before** + it and the operation's query is appended after it, so a signed base + `https://host/c?sig=…` + `/pets?limit=20` → `https://host/c/pets?sig=…&limit=20`. A **fragment** + on the base URL is rejected (`IllegalArgumentException`) — it cannot be composed with a path/query + and is never sent on the wire — and a base URL that resolves to a malformed URL (e.g. no scheme) + also throws `IllegalArgumentException` rather than leaking a checked `MalformedURLException`. +- **Headers / body / method** — set verbatim from the projections; `Request.build()` validates + body/method compatibility. + +`toRequestContext` builds the `Request` and promotes a `DispatchContext` into a `RequestContext` +carrying it, in one step. `operationName` (when set) is carried onto that `RequestContext` and +forwarded down the chain to the `ExchangeContext`, so the tracing seam can label the operation; it +never alters the assembled request. Execution stays the pipeline's job — the SPI stops at producing +the request/context (error-mapping and deserialization compose at the service layer, not as pipeline +stages). + +```kotlin +class ListPets(private val limit: Int?) : OperationParams { + override val method = Method.GET + override val pathTemplate = "/pets" + override fun queryParams() = + QueryParams.builder().apply { limit?.let { set("limit", it.toString()) } }.build() +} + +val request = ListPets(limit = 20).toRequest("https://api.example.com") // GET …/pets?limit=20 +val response = httpClient.execute(request) +``` + +--- + ## Design Decisions ### Bodies Over the SDK's I/O Abstraction @@ -749,6 +874,36 @@ Specific API choices driven by JDK 8 targeting: | `java.net.http.HttpClient` (Java 11+) | `HttpClient` interface (transport-agnostic) | | `HttpHeaders` (Java 11+) | Custom `Headers` class | +### Request URL Model + +`Request` stores its target as a single resolved `java.net.URL` (a string-backed container), +**not** a fully deconstructed URL value object (scheme / host / port / path-segments / query). +Structured query manipulation is layered on top via the `QueryParams` multimap. + +**Decision: keep `java.net.URL` as the URL container; layer `QueryParams` for query +manipulation.** + +- **DNS-free equality is preserved.** `Request` equality compares `url.toExternalForm()` — a + pure string comparison with no network I/O — because `java.net.URL.equals` / `hashCode` + resolve the host via DNS (blocking, and wrong for virtual hosts sharing an address). Keeping + the resolved-URL container carries that contract over unchanged. +- **The query is where the manipulation pressure is.** Pagination and (later) operation-input + projection manipulate the query, not the host or path. `QueryParams` puts a structured, + multi-valued, well-tested model exactly there, without forcing a rewrite of how transports + consume a URL. +- **Transports already speak `java.net.URL` / strings.** Both reference transports accept a + resolved URL or string directly; a deconstructed model would add an assembly step at every + transport boundary for no functional gain today. + +Path-template *substitution* (`/pets/{id}` + values → an encoded path) lands minimally with the +`OperationParams` SPI — see "Operation Input Projection" above. What remains **deferred** is a +*structured* URL model: a deconstructed `Url` value object and/or a move from `java.net.URL` to +`java.net.URI`. `URI` gives DNS-free equality natively (no `toExternalForm()` workaround) and +exposes the raw query and path, but parses more strictly and touches every transport boundary. The +container choice (`URL` vs `URI` vs deconstructed) is best decided when richer path handling +(per-segment typing, matrix params) actually earns it; the minimal template substitution above does +not require it. + --- ## Usage Examples @@ -860,6 +1015,10 @@ exchangeCtx.close() | `NetworkException.kt` | `http.response.exception`| public | Transport-level failure (IOException sibling)| | `HttpExceptionFactory.kt` | `http.response.exception`| public | `Response` → typed exception dispatcher | | `Headers.kt` | `http.common` | public | Immutable multi-map + builder | +| `QueryParams.kt` | `http.common` | public | Immutable query-string multi-map + builder | +| `PercentEncoding.kt` | `http.common` | internal | RFC 3986 URL-component percent-encoding (query + path) | +| `OperationParams.kt` | `operation` | public | SPI: project operation inputs → `Request` + context | +| `OperationRequestAssembler.kt` | `operation` | internal | Assembles a `Request` from an `OperationParams` | | `MediaType.kt` | `http.common` | public | Parsed MIME type with charset extraction | | `CommonMediaTypes.kt` | `http.common` | public | Media type constants | | `Protocol.kt` | `http.common` | public | HTTP protocol version enum | diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index 5a93032d..bb720f5e 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -580,6 +580,44 @@ public final class org/dexpace/sdk/core/http/common/Protocol$Companion { public final fun get (Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/Protocol; } +public final class org/dexpace/sdk/core/http/common/QueryParams { + public static final field Companion Lorg/dexpace/sdk/core/http/common/QueryParams$Companion; + public synthetic fun (Ljava/util/Map;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public static final fun builder ()Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public final fun contains (Ljava/lang/String;)Z + public static final fun empty ()Lorg/dexpace/sdk/core/http/common/QueryParams; + public final fun encode ()Ljava/lang/String; + public final fun entries ()Ljava/util/Set; + public fun equals (Ljava/lang/Object;)Z + public final fun get (Ljava/lang/String;)Ljava/lang/String; + public fun hashCode ()I + public final fun isEmpty ()Z + public final fun names ()Ljava/util/Set; + public final fun newBuilder ()Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public static final fun parse (Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/QueryParams; + public final fun size ()I + public fun toString ()Ljava/lang/String; + public final fun values (Ljava/lang/String;)Ljava/util/List; +} + +public final class org/dexpace/sdk/core/http/common/QueryParams$Builder { + public fun ()V + public fun (Lorg/dexpace/sdk/core/http/common/QueryParams;)V + public final fun add (Ljava/lang/String;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public final fun add (Ljava/lang/String;Ljava/util/List;)Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public final fun addAll (Lorg/dexpace/sdk/core/http/common/QueryParams;)Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public final fun build ()Lorg/dexpace/sdk/core/http/common/QueryParams; + public final fun remove (Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public final fun set (Ljava/lang/String;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public final fun set (Ljava/lang/String;Ljava/util/List;)Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; +} + +public final class org/dexpace/sdk/core/http/common/QueryParams$Companion { + public final fun builder ()Lorg/dexpace/sdk/core/http/common/QueryParams$Builder; + public final fun empty ()Lorg/dexpace/sdk/core/http/common/QueryParams; + public final fun parse (Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/QueryParams; +} + public final class org/dexpace/sdk/core/http/common/RequestConditions { public static final field Companion Lorg/dexpace/sdk/core/http/common/RequestConditions$Companion; public synthetic fun (Ljava/util/List;Ljava/util/List;Ljava/time/Instant;Ljava/time/Instant;Lkotlin/jvm/internal/DefaultConstructorMarker;)V @@ -639,6 +677,8 @@ public final class org/dexpace/sdk/core/http/context/DispatchContext : org/dexpa public fun getInstrumentationContext ()Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext; public fun hashCode ()I public final fun toRequestContext (Lorg/dexpace/sdk/core/http/request/Request;)Lorg/dexpace/sdk/core/http/context/RequestContext; + public final fun toRequestContext (Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/context/RequestContext; + public static synthetic fun toRequestContext$default (Lorg/dexpace/sdk/core/http/context/DispatchContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/context/RequestContext; public fun toString ()Ljava/lang/String; } @@ -647,18 +687,20 @@ public final class org/dexpace/sdk/core/http/context/DispatchContext$Companion { } public final class org/dexpace/sdk/core/http/context/ExchangeContext : org/dexpace/sdk/core/http/context/CallContext { - public fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V - public synthetic fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/String;)V + public synthetic fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun close ()V public final fun component1 ()Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext; public final fun component2 ()Lorg/dexpace/sdk/core/http/request/Request; public final fun component3 ()Lorg/dexpace/sdk/core/http/response/Response; public final fun component4 ()Ljava/lang/String; - public final fun copy (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/context/ExchangeContext; - public static synthetic fun copy$default (Lorg/dexpace/sdk/core/http/context/ExchangeContext;Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/context/ExchangeContext; + public final fun component5 ()Ljava/lang/String; + public final fun copy (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/context/ExchangeContext; + public static synthetic fun copy$default (Lorg/dexpace/sdk/core/http/context/ExchangeContext;Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/context/ExchangeContext; public fun equals (Ljava/lang/Object;)Z public fun getCallKey ()Ljava/lang/String; public fun getInstrumentationContext ()Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext; + public final fun getOperationName ()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 fun hashCode ()I @@ -666,17 +708,19 @@ public final class org/dexpace/sdk/core/http/context/ExchangeContext : org/dexpa } public final class org/dexpace/sdk/core/http/context/RequestContext : org/dexpace/sdk/core/http/context/CallContext { - public fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;)V - public synthetic fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;Ljava/lang/String;)V + public synthetic fun (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun close ()V public final fun component1 ()Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext; public final fun component2 ()Lorg/dexpace/sdk/core/http/request/Request; public final fun component3 ()Ljava/lang/String; - public final fun copy (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/context/RequestContext; - public static synthetic fun copy$default (Lorg/dexpace/sdk/core/http/context/RequestContext;Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/context/RequestContext; + public final fun component4 ()Ljava/lang/String; + public final fun copy (Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/context/RequestContext; + public static synthetic fun copy$default (Lorg/dexpace/sdk/core/http/context/RequestContext;Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext;Lorg/dexpace/sdk/core/http/request/Request;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/context/RequestContext; public fun equals (Ljava/lang/Object;)Z public fun getCallKey ()Ljava/lang/String; public fun getInstrumentationContext ()Lorg/dexpace/sdk/core/instrumentation/InstrumentationContext; + public final fun getOperationName ()Ljava/lang/String; public final fun getRequest ()Lorg/dexpace/sdk/core/http/request/Request; public fun hashCode ()I public final fun toExchangeContext (Lorg/dexpace/sdk/core/http/response/Response;)Lorg/dexpace/sdk/core/http/context/ExchangeContext; @@ -2234,6 +2278,28 @@ public abstract interface class org/dexpace/sdk/core/io/Source : java/io/Closeab public abstract fun read (Lorg/dexpace/sdk/core/io/Buffer;J)J } +public abstract interface class org/dexpace/sdk/core/operation/OperationParams { + public fun body ()Lorg/dexpace/sdk/core/http/request/RequestBody; + public abstract fun getMethod ()Lorg/dexpace/sdk/core/http/request/Method; + public fun getOperationName ()Ljava/lang/String; + public abstract fun getPathTemplate ()Ljava/lang/String; + public fun headers ()Lorg/dexpace/sdk/core/http/common/Headers; + public fun pathParams ()Ljava/util/Map; + public fun queryParams ()Lorg/dexpace/sdk/core/http/common/QueryParams; + public fun toRequest (Ljava/lang/String;)Lorg/dexpace/sdk/core/http/request/Request; + public fun toRequestContext (Ljava/lang/String;Lorg/dexpace/sdk/core/http/context/DispatchContext;)Lorg/dexpace/sdk/core/http/context/RequestContext; +} + +public final class org/dexpace/sdk/core/operation/OperationParams$DefaultImpls { + public static fun body (Lorg/dexpace/sdk/core/operation/OperationParams;)Lorg/dexpace/sdk/core/http/request/RequestBody; + public static fun getOperationName (Lorg/dexpace/sdk/core/operation/OperationParams;)Ljava/lang/String; + public static fun headers (Lorg/dexpace/sdk/core/operation/OperationParams;)Lorg/dexpace/sdk/core/http/common/Headers; + public static fun pathParams (Lorg/dexpace/sdk/core/operation/OperationParams;)Ljava/util/Map; + public static fun queryParams (Lorg/dexpace/sdk/core/operation/OperationParams;)Lorg/dexpace/sdk/core/http/common/QueryParams; + public static fun toRequest (Lorg/dexpace/sdk/core/operation/OperationParams;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/request/Request; + public static fun toRequestContext (Lorg/dexpace/sdk/core/operation/OperationParams;Ljava/lang/String;Lorg/dexpace/sdk/core/http/context/DispatchContext;)Lorg/dexpace/sdk/core/http/context/RequestContext; +} + public final class org/dexpace/sdk/core/pagination/AsyncPaginator { public fun (Lorg/dexpace/sdk/core/client/AsyncHttpClient;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/pagination/PaginationStrategy;)V public fun (Lorg/dexpace/sdk/core/client/AsyncHttpClient;Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/pagination/PaginationStrategy;J)V diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/QueryParam.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/QueryParam.kt deleted file mode 100644 index f738463d..00000000 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/QueryParam.kt +++ /dev/null @@ -1,20 +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 - -/** - * Placeholder for the query-parameter abstraction. - * - * This class is a stub — its implementation is not yet finalized. It is kept `internal` - * to prevent accidental use by consumers until the API is ready for public exposure. - * - * TODO(omar 2026-08-01): finalize QueryParam API and re-export as public - */ -internal class QueryParam { - fun implementation(): Nothing = TODO() -} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/PercentEncoding.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/PercentEncoding.kt new file mode 100644 index 00000000..0a6afff2 --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/PercentEncoding.kt @@ -0,0 +1,62 @@ +/* + * 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.common + +import java.net.URLDecoder +import java.net.URLEncoder + +/** + * Shared percent-encoding for a single URL **component** — a query parameter name/value, or a + * path segment — using RFC 3986 semantics for spaces and `+`. + * + * This is deliberately *not* `application/x-www-form-urlencoded`: a space is `%20` (not `+`), + * and a literal `+` is preserved as `%2B` rather than being read back as a space. Both + * directions go through the JDK's [URLEncoder] / [URLDecoder] (so multibyte UTF-8 and malformed + * input are handled by battle-tested code), with the `+`/space convention corrected on each side: + * + * - **encode**: `URLEncoder` emits `+` for a space, so the `+`s it produces are exactly the + * spaces — rewrite them to `%20`. Any literal `+` in the input is already `%2B` by then. + * `URLEncoder`'s notion of "safe" also diverges from RFC 3986 in two characters, both + * corrected here so the output matches the RFC 3986 unreserved set exactly: it leaves `*` + * (a sub-delimiter, *reserved*) unencoded — rewritten to `%2A` — and over-encodes `~` + * (unreserved) to `%7E` — rewritten back to `~`. + * - **decode**: `URLDecoder` would turn `+` into a space, so escape literal `+` to `%2B` + * before decoding, leaving real spaces (`%20`) to decode normally. (`%2A` decodes back to + * `*` and a bare `~` is left untouched, so the encode corrections round-trip cleanly.) + * + * The encoder percent-encodes everything except the RFC 3986 unreserved set + * (`A–Z`, `a–z`, `0–9`, `-`, `.`, `_`, `~`), which makes it safe for both query components and + * path segments: a `/` becomes `%2F`, so a path-parameter value cannot inject extra path + * segments. A query string assembled as a form *body* would need the form scheme instead; that + * is a separate concern (a future form-body type). + */ +internal object PercentEncoding { + private const val UTF_8: String = "UTF-8" + + /** + * Percent-encodes a single URL component to the RFC 3986 unreserved set: space → `%20`, + * literal `+` → `%2B`, `/` → `%2F`, `*` → `%2A`; the unreserved `~` is left as `~`. + */ + internal fun encodeComponent(component: String): String = + URLEncoder.encode(component, UTF_8) + .replace("+", "%20") + .replace("*", "%2A") + .replace("%7E", "~") + + /** + * Decodes a single URL component. A literal `+` stays a `+` (RFC 3986, not a space). + * Malformed percent-encoding falls back to the raw text rather than throwing, so a legacy + * URL a transport already accepted cannot break parsing. + */ + internal fun decodeComponent(component: String): String = + try { + URLDecoder.decode(component.replace("+", "%2B"), UTF_8) + } catch (ignored: IllegalArgumentException) { + component + } +} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/QueryParams.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/QueryParams.kt new file mode 100644 index 00000000..ad8be243 --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/QueryParams.kt @@ -0,0 +1,290 @@ +/* + * 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.common + +import java.util.Collections + +// Public API surface — not every accessor/mutator on this class is referenced within this module; SDK consumers may use any. + +/** + * Immutable, insertion-ordered, multi-valued model of a URL query string. + * + * Where [Headers] models HTTP header fields, [QueryParams] models the `?name=value&...` + * portion of a URL. The two share their shape deliberately — private constructor, mutable + * [Builder], multi-value semantics — but differ in three respects: + * + * 1. **Names are case-sensitive.** Unlike header field names, query-parameter names are + * opaque to HTTP and case-sensitive by convention (`?page=1` and `?Page=1` are distinct + * parameters), so no normalisation is applied. + * 2. **Values may be empty or value-less.** `?flag` (a name with no `=`) and `?flag=` (a + * name with an empty value) both occur in the wild; the former is modelled as an empty + * string value. + * 3. **Equality is order-sensitive.** Two [QueryParams] are equal only if they carry the + * same names *and* values in the same order — i.e. equality agrees with [encode]. (This + * is the one place the shape diverges from [Headers], whose case-folded names make + * name order non-semantic; here order is a rendered property, so it counts.) + * + * ## Role: building queries, not editing URLs + * + * [QueryParams] is an *origination* model — it builds a query string from decoded + * names/values (e.g. projecting an operation's inputs into a request). It is **not** a + * fidelity-preserving editor of an existing URL: [encode] re-renders every parameter in + * canonical form, so round-tripping an arbitrary URL through [parse] then [encode] may + * change the wire form of untouched parameters (`?flag` → `flag=`, reserved characters + * percent-encoded). Code that must edit one parameter of an existing URL while leaving the + * rest byte-for-byte (e.g. pagination's rebuilder) should splice the raw query instead. + * + * ## External vs encoded form + * + * The model stores **decoded** names and values. [encode] renders the query string with + * RFC 3986 percent-encoding (space → `%20`, literal `+` → `%2B`) via [PercentEncoding]; + * [parse] is its inverse. Round-tripping `parse(encode(...))` preserves names, values, and + * order. A query rendered as an `application/x-www-form-urlencoded` *body* is a separate + * concern (it uses `+` for spaces) and is not what [encode] produces. + * + * ## Thread-safety + * + * [QueryParams] itself is immutable and therefore freely shareable. [Builder] is **not** + * thread-safe — confine each builder instance to a single thread or guard externally. + */ +@Suppress("unused", "TooManyFunctions") +public class QueryParams private constructor( + private val paramsMap: Map>, +) { + /** + * Returns the first value for the given parameter name, or `null` if the name is absent. + * + * A value-less parameter (`?flag`) reports `""` here, which is distinct from `null` + * (absent). Use [contains] to disambiguate "present but empty" from "absent". + */ + public fun get(name: String): String? = paramsMap[name]?.firstOrNull() + + /** + * Returns all values for the given parameter name in insertion order, or an empty list + * if the name is absent. The returned list is unmodifiable. + */ + public fun values(name: String): List = paramsMap[name]?.let(Collections::unmodifiableList) ?: emptyList() + + /** + * Returns `true` if any value is present for the given parameter name. + */ + public fun contains(name: String): Boolean = paramsMap.containsKey(name) + + /** + * Returns an unmodifiable snapshot of all parameter names at the time of the call, in + * insertion order. + */ + public fun names(): Set = Collections.unmodifiableSet(LinkedHashSet(paramsMap.keys)) + + /** + * Returns an unmodifiable snapshot of all parameter entries at the time of the call. + * + * Each entry's value list is itself unmodifiable, so callers cannot mutate this + * [QueryParams] instance through the returned set, its entries + * ([Map.Entry.setValue]), or the per-name value lists. + */ + public fun entries(): Set>> { + val snapshot = LinkedHashMap>(paramsMap.size) + paramsMap.forEach { (key, value) -> + snapshot[key] = Collections.unmodifiableList(value) + } + return Collections.unmodifiableMap(snapshot).entries + } + + /** + * Returns `true` if no parameters are present. + */ + public fun isEmpty(): Boolean = paramsMap.isEmpty() + + /** + * Returns the total number of values across all names (not the number of names). + * + * `?a=1&a=2&b=3` reports `3`. Size is derived from the backing map rather than tracked, + * so it always agrees with [entries]. + */ + public fun size(): Int = paramsMap.values.sumOf { it.size } + + /** + * Renders this query string with RFC 3986 percent-encoding (space → `%20`, literal `+` + * → `%2B`), via [PercentEncoding]. Insertion order is preserved; repeated names are emitted + * once per value (`a=1&a=2`). Returns an empty string when there are no parameters. + * + * The leading `?` is **not** included — callers splice it in when assembling a URL. + */ + public fun encode(): String { + if (paramsMap.isEmpty()) return "" + val sb = StringBuilder() + paramsMap.forEach { (name, valueList) -> + val encodedName = PercentEncoding.encodeComponent(name) + valueList.forEach { value -> + if (sb.isNotEmpty()) sb.append('&') + sb.append(encodedName).append('=').append(PercentEncoding.encodeComponent(value)) + } + } + return sb.toString() + } + + /** + * Returns a new [Builder] initialised with these parameters. + */ + public fun newBuilder(): Builder = Builder(this) + + override fun toString(): String = paramsMap.toString() + + /** + * Value equality over names and values **in order** (see the class note). Two instances + * are equal iff they would [encode] identically. + */ + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is QueryParams) return false + return paramsMap.entries.toList() == other.paramsMap.entries.toList() + } + + override fun hashCode(): Int = paramsMap.entries.toList().hashCode() + + /** + * Builder for constructing [QueryParams] instances. Mutators return `this` so calls can + * be chained. + */ + @Suppress("TooManyFunctions") + public class Builder { + private val paramsMap: MutableMap> = LinkedHashMap() + + /** Creates an empty builder. */ + public constructor() + + /** Creates a builder pre-filled with the parameters from [params]. */ + public constructor(params: QueryParams) : this() { + params.paramsMap.forEach { (key, values) -> + paramsMap[key] = values.toMutableList() + } + } + + /** + * Appends a single value under [name]. Repeated calls accumulate, producing a + * multi-valued parameter (`?a=1&a=2`). A `null` [value] is stored as the empty + * string, modelling a value-less parameter (`?flag`). + */ + public fun add( + name: String, + value: String?, + ): Builder = apply { paramsMap.getOrPut(name) { mutableListOf() }.add(value ?: "") } + + /** + * Appends all [values] under [name] in order. Existing values for [name] are kept. + */ + public fun add( + name: String, + values: List, + ): Builder = apply { paramsMap.getOrPut(name) { mutableListOf() }.addAll(values) } + + /** + * Replaces all values for [name] with the single [value]. A `null` [value] removes + * the parameter entirely (matching [Headers.Builder.set]). + */ + public fun set( + name: String, + value: String?, + ): Builder = + apply { + if (value == null) { + remove(name) + } else { + paramsMap[name] = mutableListOf(value) + } + } + + /** + * Replaces all values for [name] with [values]. An empty [values] list drops the + * parameter on [build] (a name with no values renders nothing), so it never leaves a + * phantom entry; pass `null` to [set] to remove a parameter explicitly. + */ + public fun set( + name: String, + values: List, + ): Builder = apply { paramsMap[name] = values.toMutableList() } + + /** + * Removes all values for [name]. A no-op if the name is absent. + */ + public fun remove(name: String): Builder = apply { paramsMap.remove(name) } + + /** + * Merges every entry from [params] into this builder, appending values for names + * that already exist. + */ + public fun addAll(params: QueryParams): Builder = + apply { + params.paramsMap.forEach { (key, values) -> + paramsMap.getOrPut(key) { mutableListOf() }.addAll(values) + } + } + + /** Builds an immutable [QueryParams] from the builder's current state. */ + public fun build(): QueryParams { + // Deep, defensive copy so later builder mutations cannot reach into the built + // instance and accessor-returned lists cannot be mutated via a cast. + // + // A name whose value list is empty (e.g. `add(name, emptyList())`) contributes + // nothing to encode(), so it is dropped here: keeping it would leave a phantom + // entry that is contains()==true yet invisible to encode(), breaking the + // "equal iff encode() identical" invariant. A value-less param keeps a single + // empty-string value (`[""]`), which is not an empty list and is retained. + val snapshot = LinkedHashMap>(paramsMap.size) + paramsMap.forEach { (key, values) -> + if (values.isNotEmpty()) { + snapshot[key] = Collections.unmodifiableList(ArrayList(values)) + } + } + return QueryParams(snapshot) + } + } + + public companion object { + /** Returns an empty builder. */ + @JvmStatic + public fun builder(): Builder = Builder() + + /** Returns an empty [QueryParams]. */ + @JvmStatic + public fun empty(): QueryParams = Builder().build() + + /** + * Parses a raw query string (the part after `?`, without the leading `?`) into a + * [QueryParams], decoding names and values with RFC 3986 semantics (space `%20`, + * literal `+`) via [PercentEncoding]. + * + * - A `null` or blank input yields an empty [QueryParams]. + * - A leading `?` is tolerated and stripped. + * - `a=1&a=2` becomes a multi-valued `a`; `flag` (no `=`) becomes `flag` → `""`; + * `flag=` becomes `flag` → `""`. + * - Empty segments (a stray `&`) are skipped. + * - Malformed percent-encoding falls back to the raw text rather than throwing. + */ + @JvmStatic + public fun parse(query: String?): QueryParams { + if (query.isNullOrEmpty()) return empty() + val raw = if (query.startsWith("?")) query.substring(1) else query + if (raw.isEmpty()) return empty() + val builder = Builder() + for (segment in raw.split('&')) { + if (segment.isEmpty()) continue + val eq = segment.indexOf('=') + if (eq < 0) { + builder.add(PercentEncoding.decodeComponent(segment), "") + } else { + val name = PercentEncoding.decodeComponent(segment.substring(0, eq)) + val value = PercentEncoding.decodeComponent(segment.substring(eq + 1)) + builder.add(name, value) + } + } + return builder.build() + } + } +} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt index a0e0b03c..2e80a281 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt @@ -37,15 +37,23 @@ public data class DispatchContext( ) : CallContext { /** * Promotes this dispatch context into a [RequestContext] bound to [request] and stores - * the new context in [ContextStore] under this chain's [callKey]. After promotion this - * dispatch context becomes an intermediate link and must not be closed independently — - * close the returned [RequestContext] (or its own successor) instead. + * the new context in [ContextStore] under this chain's [callKey]. The optional + * [operationName] (a schema-defined operation id such as `"GetUser"`) is carried onto the + * [RequestContext] and forwarded down the chain so the tracing seam can label the operation; + * it does not affect the request or dispatch decision. After promotion this dispatch context + * becomes an intermediate link and must not be closed independently — close the returned + * [RequestContext] (or its own successor) instead. */ - public fun toRequestContext(request: Request): RequestContext = + @JvmOverloads + public fun toRequestContext( + request: Request, + operationName: String? = null, + ): RequestContext = RequestContext( instrumentationContext = instrumentationContext, request = request, callKey = callKey, + operationName = operationName, ).also { ContextStore.set(it.callKey, it) } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt index 58720e2f..3b647f5d 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt @@ -25,10 +25,16 @@ import org.dexpace.sdk.core.instrumentation.InstrumentationContext * a collision-safe store key. One consequence of the generated default: two default-constructed * instances are not structurally equal, since each generates a distinct key — pin an explicit * [callKey] if you need equality. + * + * @property operationName Optional schema-defined operation id (e.g. `"GetUser"`) for this call, + * or `null` for a raw request with no associated operation. Carried forward from the + * [RequestContext] for the tracing seam ([InstrumentationContext.httpTracerFactory]) so + * post-exchange span finalizers can label the operation. */ public data class ExchangeContext( override val instrumentationContext: InstrumentationContext, val request: Request, val response: Response, override val callKey: String = DispatchContext.generateCallKey(instrumentationContext), + val operationName: String? = null, ) : CallContext diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt index 8f3f1f41..9effc0e3 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt @@ -24,17 +24,23 @@ import org.dexpace.sdk.core.instrumentation.InstrumentationContext * a collision-safe store key. One consequence of the generated default: two default-constructed * instances are not structurally equal, since each generates a distinct key — pin an explicit * [callKey] if you need equality. + * + * @property operationName Optional schema-defined operation id (e.g. `"GetUser"`) for this call, + * or `null` for a raw request with no associated operation. Carried for the tracing seam + * ([InstrumentationContext.httpTracerFactory]) and forwarded to the [ExchangeContext]; it does + * not affect the request or dispatch decision. */ public data class RequestContext( override val instrumentationContext: InstrumentationContext, val request: Request, override val callKey: String = DispatchContext.generateCallKey(instrumentationContext), + val operationName: String? = null, ) : CallContext { /** * Promotes this request context into an [ExchangeContext] bound to [response] and stores - * the new context in [ContextStore] under this chain's [callKey]. After promotion this - * request context becomes an intermediate link and must not be closed independently — - * close the returned [ExchangeContext] instead. + * the new context in [ContextStore] under this chain's [callKey]. The [operationName] is + * carried forward. After promotion this request context becomes an intermediate link and + * must not be closed independently — close the returned [ExchangeContext] instead. */ public fun toExchangeContext(response: Response): ExchangeContext = ExchangeContext( @@ -42,6 +48,7 @@ public data class RequestContext( request = request, response = response, callKey = callKey, + operationName = operationName, ).also { ContextStore.set(it.callKey, it) } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/operation/OperationParams.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/operation/OperationParams.kt new file mode 100644 index 00000000..af4548f8 --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/operation/OperationParams.kt @@ -0,0 +1,102 @@ +/* + * 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.operation + +import org.dexpace.sdk.core.http.common.Headers +import org.dexpace.sdk.core.http.common.QueryParams +import org.dexpace.sdk.core.http.context.DispatchContext +import org.dexpace.sdk.core.http.context.RequestContext +import org.dexpace.sdk.core.http.request.Method +import org.dexpace.sdk.core.http.request.Request +import org.dexpace.sdk.core.http.request.RequestBody + +/** + * SPI that projects an operation's typed inputs onto an HTTP request. + * + * A thin generated service implements this once per operation, exposing where each input + * belongs on the wire — **path**, **query**, **header**, or **body** — and the runtime + * ([toRequest] / [toRequestContext]) assembles the [Request] and feeds it into the context + * chain. This is the seam #57 defines so generated code never does URL string surgery: + * cursors, page numbers, and operation arguments flow through the typed projections below + * (and through [QueryParams] / [Headers]) rather than being spliced into a URL. + * + * Only [method] and [pathTemplate] are required; the four projections default to empty so a + * parameterless operation overrides almost nothing: + * + * ```kotlin + * class ListPets(private val limit: Int?) : OperationParams { + * override val method = Method.GET + * override val pathTemplate = "/pets" + * override fun queryParams() = + * QueryParams.builder().apply { limit?.let { set("limit", it.toString()) } }.build() + * } + * + * val request = ListPets(limit = 20).toRequest("https://api.example.com") + * ``` + * + * Path parameters are substituted into [pathTemplate] (`/pets/{petId}`) and percent-encoded as + * path segments, so a value cannot inject extra `/` segments. Query parameters are appended via + * [QueryParams.encode] (RFC 3986). The body is whatever [RequestBody] the operation already + * built (JSON via the serde seam, a form body, etc.) — this SPI carries it, it does not encode it. + */ +public interface OperationParams { + /** The HTTP method for this operation. */ + public val method: Method + + /** + * The path template, relative to the base URL, with `{name}` placeholders for path + * parameters — e.g. `/pets/{petId}`. A leading `/` is optional. + */ + public val pathTemplate: String + + /** + * A stable operation identifier (e.g. `"ListPets"`) for instrumentation, or `null`. + * [toRequestContext] carries it onto the [RequestContext] (and through to the + * [org.dexpace.sdk.core.http.context.ExchangeContext]) so the tracing seam can label the + * operation; it does **not** affect the assembled request's URL, headers, or body. + */ + public val operationName: String? get() = null + + /** + * Decoded path-parameter values keyed by placeholder name. Every `{name}` in [pathTemplate] + * must have an entry; values are percent-encoded as path segments during assembly. + */ + public fun pathParams(): Map = emptyMap() + + /** Query parameters to append to the URL. */ + public fun queryParams(): QueryParams = QueryParams.empty() + + /** Headers to set on the request. */ + public fun headers(): Headers = Headers.builder().build() + + /** The request body, or `null` for a bodyless operation. */ + public fun body(): RequestBody? = null + + /** + * Assembles the [Request] for this operation against [baseUrl] (e.g. + * `https://api.example.com` or `https://api.example.com/v1`). A trailing `/` on [baseUrl] is + * trimmed before the resolved path is joined. If [baseUrl] already carries a query (e.g. a + * signed/SAS base such as `https://host/c?sig=…`), the resolved path is inserted before it and + * this operation's query is appended after it. + * + * @throws IllegalArgumentException if a `{name}` in [pathTemplate] has no [pathParams] value, + * if [baseUrl] carries a fragment (`#…`), or if [baseUrl] resolves to a malformed URL + * (e.g. it has no scheme). + */ + public fun toRequest(baseUrl: String): Request = OperationRequestAssembler.assemble(baseUrl, this) + + /** + * Assembles the [Request] ([toRequest]) and promotes [dispatch] into a [RequestContext] + * carrying it — feeding the request into the context chain in one step. [operationName] is + * carried onto the resulting context for the tracing seam. + */ + public fun toRequestContext( + baseUrl: String, + dispatch: DispatchContext, + ): RequestContext = dispatch.toRequestContext(toRequest(baseUrl), operationName) +} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/operation/OperationRequestAssembler.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/operation/OperationRequestAssembler.kt new file mode 100644 index 00000000..036e47eb --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/operation/OperationRequestAssembler.kt @@ -0,0 +1,114 @@ +/* + * 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.operation + +import org.dexpace.sdk.core.http.common.PercentEncoding +import org.dexpace.sdk.core.http.request.Request +import java.net.MalformedURLException + +/** + * Assembles a [Request] from an [OperationParams] projection and a base URL. Internal — callers + * go through [OperationParams.toRequest]. + * + * The path template is resolved by substituting each `{name}` with its percent-encoded path + * parameter; the query is rendered by [org.dexpace.sdk.core.http.common.QueryParams.encode]; and + * the base URL's scheme/host/port/base-path carry through unchanged. The resolved path is inserted + * after the base path but **before** any query the base URL already carries (e.g. a signed/SAS + * base such as `https://host/c?sig=…`), and the operation's query is appended after the base + * query. A fragment on the base URL is rejected — it cannot be composed with a path/query and is + * never sent on the wire, so it is almost certainly a mistake. + */ +internal object OperationRequestAssembler { + // A `{name}` placeholder: any run of characters up to the first '}'. The class is `[^}]` (not + // `[^/}]`) so a stray '/' inside the braces is captured as the variable name and fails fast as + // a missing path parameter, rather than leaving an unsubstituted `{a/b}` literal in the URL. + private val TEMPLATE_VARIABLE = Regex("""\{([^}]+)}""") + + internal fun assemble( + baseUrl: String, + params: OperationParams, + ): Request { + val path = resolvePath(params.pathTemplate, params.pathParams()) + val query = params.queryParams().encode() + val url = buildUrl(baseUrl, path, query) + return try { + Request.builder() + .method(params.method) + .url(url) + .headers(params.headers()) + .body(params.body()) + .build() + } catch (e: MalformedURLException) { + // Surface a malformed base URL (e.g. missing scheme) as the SPI's documented + // IllegalArgumentException rather than leaking a checked IOException to callers. + throw IllegalArgumentException( + "Cannot assemble a request: base URL '$baseUrl' resolved to a malformed URL '$url'", + e, + ) + } + } + + private fun resolvePath( + template: String, + pathParams: Map, + ): String = + TEMPLATE_VARIABLE.replace(template) { match -> + val name = match.groupValues[1] + val value = + pathParams[name] + ?: throw IllegalArgumentException("Missing path parameter '$name' for template '$template'") + PercentEncoding.encodeComponent(value) + } + + private fun buildUrl( + baseUrl: String, + path: String, + query: String, + ): String { + require('#' !in baseUrl) { + "Base URL must not contain a fragment ('#'): '$baseUrl'" + } + // Split off any query the base URL already carries so the resolved path is inserted + // before it (`host/base?sig=…` + `/pets` → `host/base/pets?sig=…`), instead of being + // swallowed into a query value. + val queryStart = baseUrl.indexOf('?') + val prefix = if (queryStart < 0) baseUrl else baseUrl.substring(0, queryStart) + val baseQuery = if (queryStart < 0) "" else baseUrl.substring(queryStart + 1) + + val sb = StringBuilder(prefix) + // Join the resolved path with exactly one '/'. A trailing '/' on the base is only + // significant when there is no path to join, so an empty path leaves the base untouched + // (e.g. `https://host/v1/` stays `…/v1/`) rather than being silently stripped. + if (path.isNotEmpty()) { + val baseEndsWithSlash = prefix.endsWith('/') + val pathStartsWithSlash = path.startsWith('/') + if (baseEndsWithSlash && pathStartsWithSlash) { + sb.setLength(sb.length - 1) + } else if (!baseEndsWithSlash && !pathStartsWithSlash) { + sb.append('/') + } + sb.append(path) + } + // Base query first (it is ambient — auth tokens, API keys), operation query appended. + // Both sides are already percent-encoded wire form, so they concatenate verbatim. A + // trailing '&' on the base query is its own separator: drop it before joining, so it + // neither doubles into an empty `&&` segment when an operation query follows nor survives + // as a dangling separator when the base contributes the only query. + val trimmedBase = baseQuery.removeSuffix("&") + val mergedQuery = + when { + trimmedBase.isEmpty() -> query + query.isEmpty() -> trimmedBase + else -> "$trimmedBase&$query" + } + if (mergedQuery.isNotEmpty()) { + sb.append('?').append(mergedQuery) + } + return sb.toString() + } +} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/RequestRebuilder.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/RequestRebuilder.kt index 50cee4ac..5b73add1 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/RequestRebuilder.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pagination/RequestRebuilder.kt @@ -7,27 +7,32 @@ package org.dexpace.sdk.core.pagination +import org.dexpace.sdk.core.http.common.PercentEncoding +import org.dexpace.sdk.core.http.common.QueryParams import org.dexpace.sdk.core.http.request.Request import java.net.URL -import java.net.URLDecoder -import java.net.URLEncoder /** * Internal helpers for building the next-page [Request] from an initial [Request] plus a * paging-state mutation. * - * The functions here are pure URL-rewriting utilities — no I/O, no mutation of the - * supplied [Request], and no allocation beyond the new request and URL. They exist so - * pagination strategies can stay focused on extracting paging state from responses, while - * the mechanical work of "produce a new request with this query param set" lives in one - * place. + * The functions here are pure URL-rewriting utilities — no I/O and no mutation of the + * supplied [Request]. They exist so pagination strategies can stay focused on extracting + * paging state from responses, while the mechanical work of "produce a new request with this + * query param set" lives in one place. * - * ## URL encoding + * ## Query manipulation * - * All query manipulation uses `application/x-www-form-urlencoded` semantics (the JDK's - * `URLEncoder` / `URLDecoder` with UTF-8) — the same convention browsers and servers use - * for query strings. Pre-existing query segments are preserved verbatim; only the targeted - * parameter is replaced. + * Both reading ([getQueryParam]) and writing ([setQueryParam]) operate on the raw query string + * directly — scanning or splicing segments and decoding/encoding only the targeted parameter via + * the shared [PercentEncoding] codec (RFC 3986 semantics: space → `%20`, literal `+` preserved). + * Every other segment is copied **verbatim**, so parameters the caller did not touch keep their + * exact wire form (a value-less `?flag` stays value-less; reserved characters are not rewritten). + * + * [QueryParams.encode] is deliberately *not* used to reassemble the whole query: it re-renders + * every parameter in canonical form (altering untouched segments) and would allocate a full + * multimap per page. This editor favours fidelity — and avoids that per-page allocation — over + * funnelling everything through the model; see the note on [QueryParams]. * * ## URL.fragment / userInfo * @@ -35,8 +40,6 @@ import java.net.URLEncoder * (fragment) exactly. Only the query string is mutated. */ internal object RequestRebuilder { - private const val UTF_8: String = "UTF-8" - /** * Returns a new [Request] cloned from [request] with the query parameter [name] set to * [value]. If [value] is `null`, removes the parameter entirely. If [name] already @@ -76,7 +79,6 @@ internal object RequestRebuilder { name: String, value: String?, ): URL { - val encodedName = URLEncoder.encode(name, UTF_8) val existing = url.query val outParams: MutableList = ArrayList() var replaced = false @@ -84,55 +86,54 @@ internal object RequestRebuilder { for (segment in existing.split('&')) { if (segment.isEmpty()) continue val key = segment.substringBefore('=', segment) - if (decodeOrRaw(key) == name) { + if (PercentEncoding.decodeComponent(key) == name) { if (!replaced && value != null) { - outParams.add(encodedName + "=" + URLEncoder.encode(value, UTF_8)) + outParams.add(encodeParam(name, value)) replaced = true } // else: dropping this param (value == null) or already replaced once. } else { + // Untouched parameter — copy its raw segment byte-for-byte. outParams.add(segment) } } } if (!replaced && value != null) { - outParams.add(encodedName + "=" + URLEncoder.encode(value, UTF_8)) + outParams.add(encodeParam(name, value)) } val newQuery: String? = if (outParams.isEmpty()) null else outParams.joinToString("&") return rebuildUrl(url, newQuery) } + private fun encodeParam( + name: String, + value: String, + ): String = PercentEncoding.encodeComponent(name) + "=" + PercentEncoding.encodeComponent(value) + /** - * Returns the value of the query parameter [name] from [url], or `null` if absent. - * The value is URL-decoded with UTF-8. + * Returns the value of the query parameter [name] from [url], or `null` if absent. The raw + * query is scanned directly (mirroring [setQueryParam]) and the matched name/value are + * decoded with RFC 3986 semantics via [PercentEncoding] — first match wins, and a value-less + * `?flag` reports `""`. No full [QueryParams] model is built per page. */ fun getQueryParam( url: URL, name: String, ): String? { - val existing = url.query ?: return null - if (existing.isEmpty()) return null + val existing = url.query + if (existing.isNullOrEmpty()) return null for (segment in existing.split('&')) { if (segment.isEmpty()) continue - val key = segment.substringBefore('=', segment) - if (decodeOrRaw(key) == name) { - return decodeOrRaw(segment.substringAfter('=', "")) + val eq = segment.indexOf('=') + val key = if (eq < 0) segment else segment.substring(0, eq) + if (PercentEncoding.decodeComponent(key) == name) { + val rawValue = if (eq < 0) "" else segment.substring(eq + 1) + return PercentEncoding.decodeComponent(rawValue) } } return null } - private fun decodeOrRaw(raw: String): String = - try { - URLDecoder.decode(raw, UTF_8) - } catch (ignored: IllegalArgumentException) { - // Malformed percent-encoding — return raw so equality with caller's name still works - // for unencoded ASCII identifiers (the common case for pagination params). - // We intentionally swallow the exception here; pagination should not fail on malformed - // legacy URLs that the transport accepted. - raw - } - private fun rebuildUrl( source: URL, query: String?, diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/QueryParamTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/QueryParamTest.kt deleted file mode 100644 index 3bb6ea59..00000000 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/QueryParamTest.kt +++ /dev/null @@ -1,36 +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 - -import kotlin.test.Test -import kotlin.test.assertFailsWith -import kotlin.test.assertNotNull - -/** - * Covers the placeholder [QueryParam] class. The class is currently a stub — its - * single method `implementation()` throws `NotImplementedError` via `TODO()`. The - * test exists to (a) bring the type onto the coverage instrumentation's radar and - * (b) document the placeholder behaviour so a real implementation arrives with a - * test that should be replaced rather than added alongside. - */ -class QueryParamTest { - @Test - fun `default constructor produces an instance`() { - // The no-arg constructor itself must succeed even though all methods are TODO. - assertNotNull(QueryParam()) - } - - @Test - fun `implementation method throws NotImplementedError`() { - // `TODO()` raises kotlin.NotImplementedError. Verify the contract so callers - // know touching this method is an explicit "not ready" signal. - assertFailsWith { - QueryParam().implementation() - } - } -} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/common/PercentEncodingTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/common/PercentEncodingTest.kt new file mode 100644 index 00000000..a5e65fee --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/common/PercentEncodingTest.kt @@ -0,0 +1,114 @@ +/* + * 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.common + +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * Unit tests for the [PercentEncoding] helper — the shared RFC 3986 URL-component + * percent-encoder used by [QueryParams] (query names/values) and operation path segments. + * + * The defining property versus `application/x-www-form-urlencoded` is space and `+` + * handling: a space encodes to `%20` (not `+`), and a literal `+` is preserved as `%2B` + * (it is **not** treated as a space on the way back). + */ +class PercentEncodingTest { + @Test + fun `encode renders a space as percent-20 not plus`() { + assertEquals("a%20b", PercentEncoding.encodeComponent("a b")) + } + + @Test + fun `encode renders a literal plus as percent-2B`() { + assertEquals("a%2Bb", PercentEncoding.encodeComponent("a+b")) + } + + @Test + fun `encode percent-encodes the structural delimiters and slash`() { + assertEquals("a%26b", PercentEncoding.encodeComponent("a&b")) + assertEquals("a%3Db", PercentEncoding.encodeComponent("a=b")) + assertEquals("a%2Fb", PercentEncoding.encodeComponent("a/b")) + assertEquals("a%23b", PercentEncoding.encodeComponent("a#b")) + } + + @Test + fun `encode leaves unreserved ascii untouched`() { + assertEquals("Abc-123_x.y", PercentEncoding.encodeComponent("Abc-123_x.y")) + } + + @Test + fun `encode leaves the unreserved tilde untouched`() { + // '~' is in the RFC 3986 unreserved set; URLEncoder over-encodes it to %7E, which the + // codec corrects back to '~'. + assertEquals("~", PercentEncoding.encodeComponent("~")) + assertEquals("~user", PercentEncoding.encodeComponent("~user")) + } + + @Test + fun `encode percent-encodes the reserved asterisk`() { + // '*' is a sub-delimiter (reserved), not unreserved; URLEncoder leaves it raw, which the + // codec corrects to %2A so a component value cannot smuggle a reserved character. + assertEquals("%2A", PercentEncoding.encodeComponent("*")) + assertEquals("a%2Ab", PercentEncoding.encodeComponent("a*b")) + } + + @Test + fun `encode then decode round-trips the corrected tilde and asterisk`() { + val original = "~a*b" + assertEquals("~a%2Ab", PercentEncoding.encodeComponent(original)) + assertEquals(original, PercentEncoding.decodeComponent(PercentEncoding.encodeComponent(original))) + } + + @Test + fun `encode percent-encodes multibyte utf8 as utf8 bytes`() { + assertEquals("%C3%A9", PercentEncoding.encodeComponent("é")) // é + } + + @Test + fun `encode of empty string is empty`() { + assertEquals("", PercentEncoding.encodeComponent("")) + } + + @Test + fun `decode turns percent-20 into a space`() { + assertEquals("a b", PercentEncoding.decodeComponent("a%20b")) + } + + @Test + fun `decode keeps a literal plus as a plus and does not turn it into a space`() { + assertEquals("a+b", PercentEncoding.decodeComponent("a+b")) + } + + @Test + fun `decode turns percent-2B into a literal plus`() { + assertEquals("a+b", PercentEncoding.decodeComponent("a%2Bb")) + } + + @Test + fun `decode reconstructs multibyte utf8`() { + assertEquals("é", PercentEncoding.decodeComponent("%C3%A9")) + } + + @Test + fun `decode keeps raw text on malformed percent-encoding rather than throwing`() { + assertEquals("%zz", PercentEncoding.decodeComponent("%zz")) + assertEquals("%2", PercentEncoding.decodeComponent("%2")) + } + + @Test + fun `decode keeps a literal plus even when the rest is malformed`() { + assertEquals("a+%zz", PercentEncoding.decodeComponent("a+%zz")) + } + + @Test + fun `encode then decode round-trips spaces plus slashes and unicode`() { + val original = "hello world+1/2=3 é" + assertEquals(original, PercentEncoding.decodeComponent(PercentEncoding.encodeComponent(original))) + } +} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/common/QueryParamsTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/common/QueryParamsTest.kt new file mode 100644 index 00000000..3f8b6851 --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/common/QueryParamsTest.kt @@ -0,0 +1,311 @@ +/* + * 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.common + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class QueryParamsTest { + @Test + fun `empty params have no entries`() { + val params = QueryParams.empty() + assertTrue(params.isEmpty()) + assertEquals(0, params.size()) + assertTrue(params.names().isEmpty()) + assertTrue(params.entries().isEmpty()) + assertEquals("", params.encode()) + } + + @Test + fun `add accumulates multiple values for one name in insertion order`() { + val params = + QueryParams.builder() + .add("tag", "a") + .add("tag", "b") + .add("tag", "c") + .build() + assertEquals(listOf("a", "b", "c"), params.values("tag")) + assertEquals("a", params.get("tag")) + assertEquals(3, params.size()) + } + + @Test + fun `set replaces all existing values`() { + val params = + QueryParams.builder() + .add("page", "1") + .add("page", "2") + .set("page", "9") + .build() + assertEquals(listOf("9"), params.values("page")) + } + + @Test + fun `set list replaces with multiple values`() { + val params = + QueryParams.builder() + .add("k", "old") + .set("k", listOf("x", "y")) + .build() + assertEquals(listOf("x", "y"), params.values("k")) + } + + @Test + fun `set null removes the parameter`() { + val params = + QueryParams.builder() + .add("a", "1") + .add("b", "2") + .set("a", null) + .build() + assertFalse(params.contains("a")) + assertTrue(params.contains("b")) + } + + @Test + fun `remove drops the name and is a no-op when absent`() { + val params = + QueryParams.builder() + .add("a", "1") + .remove("a") + .remove("missing") + .build() + assertTrue(params.isEmpty()) + } + + @Test + fun `add with null value stores empty string for a value-less param`() { + val params = QueryParams.builder().add("flag", null as String?).build() + assertTrue(params.contains("flag")) + assertEquals("", params.get("flag")) + } + + @Test + fun `names are case-sensitive`() { + val params = + QueryParams.builder() + .add("Page", "1") + .add("page", "2") + .build() + assertEquals(listOf("1"), params.values("Page")) + assertEquals(listOf("2"), params.values("page")) + assertEquals(setOf("Page", "page"), params.names()) + } + + @Test + fun `get returns null and values empty for absent name`() { + val params = QueryParams.builder().add("a", "1").build() + assertNull(params.get("b")) + assertTrue(params.values("b").isEmpty()) + assertFalse(params.contains("b")) + } + + @Test + fun `encode percent-encodes names and values per rfc3986`() { + val params = + QueryParams.builder() + .add("q", "a b&c") + .add("name+key", "x") + .build() + // Space is %20 (not +); literal + is %2B; & is %26. + assertEquals("q=a%20b%26c&name%2Bkey=x", params.encode()) + } + + @Test + fun `encode renders a space as percent-20`() { + val params = QueryParams.builder().add("q", "hello world").build() + assertEquals("q=hello%20world", params.encode()) + } + + @Test + fun `encode emits one pair per value for repeated names`() { + val params = + QueryParams.builder() + .add("a", "1") + .add("a", "2") + .add("b", "3") + .build() + assertEquals("a=1&a=2&b=3", params.encode()) + } + + @Test + fun `parse decodes names and values`() { + val params = QueryParams.parse("q=a%20b%26c&page=2") + assertEquals("a b&c", params.get("q")) + assertEquals("2", params.get("page")) + } + + @Test + fun `parse reads a literal plus as a plus not a space`() { + // RFC 3986: in a URL query, `+` is a literal `+`, unlike form-encoding. + val params = QueryParams.parse("q=a+b") + assertEquals("a+b", params.get("q")) + } + + @Test + fun `parse handles multi-value names`() { + val params = QueryParams.parse("tag=a&tag=b&tag=c") + assertEquals(listOf("a", "b", "c"), params.values("tag")) + } + + @Test + fun `parse treats a bare name as a value-less param`() { + val params = QueryParams.parse("flag&other=1") + assertTrue(params.contains("flag")) + assertEquals("", params.get("flag")) + assertEquals("1", params.get("other")) + } + + @Test + fun `parse treats name equals empty as empty value`() { + val params = QueryParams.parse("flag=") + assertEquals("", params.get("flag")) + } + + @Test + fun `parse tolerates a leading question mark`() { + val params = QueryParams.parse("?a=1&b=2") + assertEquals("1", params.get("a")) + assertEquals("2", params.get("b")) + } + + @Test + fun `parse skips empty segments`() { + val params = QueryParams.parse("a=1&&b=2&") + assertEquals(listOf("a", "b"), params.names().toList()) + } + + @Test + fun `parse of null or empty yields empty`() { + assertTrue(QueryParams.parse(null).isEmpty()) + assertTrue(QueryParams.parse("").isEmpty()) + assertTrue(QueryParams.parse("?").isEmpty()) + } + + @Test + fun `parse keeps raw text on malformed percent-encoding`() { + // A stray '%' that is not a valid escape must not throw. + val params = QueryParams.parse("bad=%zz") + assertEquals("%zz", params.get("bad")) + } + + @Test + fun `encode then parse round-trips names values and order`() { + val original = + QueryParams.builder() + .add("q", "hello world") + .add("tag", "a") + .add("tag", "b") + .add("special", "100%") + .add("plus", "a+b") + .build() + val roundTripped = QueryParams.parse(original.encode()) + assertEquals(original, roundTripped) + } + + @Test + fun `newBuilder is prefilled and does not mutate the source`() { + val original = QueryParams.builder().add("a", "1").build() + val derived = original.newBuilder().add("b", "2").build() + assertFalse(original.contains("b")) + assertEquals("1", derived.get("a")) + assertEquals("2", derived.get("b")) + } + + @Test + fun `addAll merges and appends values for existing names`() { + val base = QueryParams.builder().add("a", "1").add("b", "2").build() + val extra = QueryParams.builder().add("a", "3").add("c", "4").build() + val merged = base.newBuilder().addAll(extra).build() + assertEquals(listOf("1", "3"), merged.values("a")) + assertEquals(listOf("2"), merged.values("b")) + assertEquals(listOf("4"), merged.values("c")) + } + + @Test + fun `values list is unmodifiable`() { + val params = QueryParams.builder().add("a", "1").build() + + @Suppress("UNCHECKED_CAST") + val asMutable = params.values("a") as MutableList + assertFailsWith { asMutable.add("x") } + } + + @Test + fun `entries snapshot is immutable and reflects all names`() { + val params = QueryParams.builder().add("a", "1").add("a", "2").add("b", "3").build() + val entries = params.entries() + assertEquals(2, entries.size) + assertFailsWith { + (entries as MutableSet<*>).clear() + } + } + + @Test + fun `build is decoupled from the builder after the fact`() { + val builder = QueryParams.builder().add("a", "1") + val first = builder.build() + builder.add("a", "2") + assertEquals(listOf("1"), first.values("a")) + } + + @Test + fun `equality is by value and includes hashCode`() { + val a = QueryParams.builder().add("x", "1").add("y", "2").build() + val b = QueryParams.builder().add("x", "1").add("y", "2").build() + assertEquals(a, b) + assertEquals(a.hashCode(), b.hashCode()) + } + + @Test + fun `equality is order-sensitive across names`() { + val a = QueryParams.builder().add("x", "1").add("y", "2").build() + val b = QueryParams.builder().add("y", "2").add("x", "1").build() + assertNotEquals(a, b) + } + + @Test + fun `equality is order-sensitive across values of one name`() { + val a = QueryParams.builder().add("t", "1").add("t", "2").build() + val b = QueryParams.builder().add("t", "2").add("t", "1").build() + assertNotEquals(a, b) + } + + @Test + fun `a name added with an empty value list yields params equal to empty`() { + // A name with no values contributes nothing to encode(), so it must not leave a + // phantom entry that breaks the "equal iff encode() identical" invariant. + val params = QueryParams.builder().add("a", emptyList()).build() + assertFalse(params.contains("a")) + assertTrue(params.isEmpty()) + assertEquals("", params.encode()) + assertEquals(QueryParams.empty(), params) + assertEquals(QueryParams.empty().hashCode(), params.hashCode()) + } + + @Test + fun `set with an empty value list drops the name`() { + val params = QueryParams.builder().add("a", "1").set("a", emptyList()).build() + assertFalse(params.contains("a")) + assertTrue(params.isEmpty()) + } + + @Test + fun `a value-less param is retained and not confused with an empty value list`() { + // add(name, null) stores a single empty-string value ("?flag"); that is NOT an empty + // list and must survive build(). + val params = QueryParams.builder().add("flag", null as String?).build() + assertTrue(params.contains("flag")) + assertEquals("flag=", params.encode()) + } +} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/DispatchContextTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/DispatchContextTest.kt index a821aa2c..a40152b4 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/DispatchContextTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/DispatchContextTest.kt @@ -14,6 +14,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertSame class DispatchContextTest { @@ -49,6 +50,19 @@ class DispatchContextTest { assertEquals(dispatch.callKey, promoted.callKey) // Promotion registers the new context under the chain's call key. assertSame(promoted, ContextStore.get(promoted.callKey)) + // No operation name supplied — the context carries null. + assertNull(promoted.operationName) + } + + @Test + fun `toRequestContext carries an explicit operation name onto the promoted context`() { + val id = owned("opname") + val dispatch = DispatchContext(FakeInstrumentationContext(TraceId(id))) + ownedIds.add(dispatch.callKey) + + val promoted = dispatch.toRequestContext(request(), "GetUser") + + assertEquals("GetUser", promoted.operationName) } @Test diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt index 594214c7..35208751 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt @@ -52,6 +52,17 @@ class RequestContextTest { assertSame(promoted, ContextStore.get(promoted.callKey)) } + @Test + fun `toExchangeContext carries the operation name forward`() { + val instr = FakeInstrumentationContext(TraceId(owned("opname"))) + val parent = RequestContext(instr, request(), operationName = "GetUser") + ownedIds.add(parent.callKey) + + val promoted = parent.toExchangeContext(response()) + + assertEquals("GetUser", promoted.operationName) + } + @Test fun `data class equality is by content`() { val instr = FakeInstrumentationContext(TraceId(owned("eq"))) diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/operation/OperationParamsTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/operation/OperationParamsTest.kt new file mode 100644 index 00000000..26ed7a64 --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/operation/OperationParamsTest.kt @@ -0,0 +1,220 @@ +/* + * 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.operation + +import org.dexpace.sdk.core.http.common.CommonMediaTypes +import org.dexpace.sdk.core.http.common.Headers +import org.dexpace.sdk.core.http.common.QueryParams +import org.dexpace.sdk.core.http.context.ContextStore +import org.dexpace.sdk.core.http.context.DispatchContext +import org.dexpace.sdk.core.http.request.Method +import org.dexpace.sdk.core.http.request.RequestBody +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNull +import kotlin.test.assertSame +import kotlin.test.assertTrue + +class OperationParamsTest { + /** Fully-specified test operation. */ + private class TestOp( + override val method: Method, + override val pathTemplate: String, + private val path: Map = emptyMap(), + private val query: QueryParams = QueryParams.empty(), + private val hdrs: Headers = Headers.builder().build(), + private val bdy: RequestBody? = null, + override val operationName: String? = null, + ) : OperationParams { + override fun pathParams(): Map = path + + override fun queryParams(): QueryParams = query + + override fun headers(): Headers = hdrs + + override fun body(): RequestBody? = bdy + } + + /** Minimal operation — exercises every default projection. */ + private class MinimalOp : OperationParams { + override val method: Method = Method.GET + override val pathTemplate: String = "/ping" + } + + private val base = "https://api.example.com" + + @Test + fun `toRequest builds method and url from base and static path`() { + val request = TestOp(Method.GET, "/pets").toRequest(base) + assertEquals(Method.GET, request.method) + assertEquals("https://api.example.com/pets", request.url.toExternalForm()) + } + + @Test + fun `toRequest substitutes and percent-encodes path params`() { + val request = TestOp(Method.GET, "/pets/{petId}", path = mapOf("petId" to "a b/c")).toRequest(base) + assertEquals("https://api.example.com/pets/a%20b%2Fc", request.url.toExternalForm()) + } + + @Test + fun `toRequest appends query params in order`() { + val query = QueryParams.builder().add("tag", "a").add("tag", "b").add("page", "2").build() + val request = TestOp(Method.GET, "/pets", query = query).toRequest(base) + assertEquals("https://api.example.com/pets?tag=a&tag=b&page=2", request.url.toExternalForm()) + } + + @Test + fun `toRequest composes path query and headers`() { + val request = + TestOp( + Method.GET, + "/pets/{id}", + path = mapOf("id" to "7"), + query = QueryParams.builder().add("limit", "10").build(), + hdrs = Headers.builder().add("Accept", "application/json").build(), + ).toRequest(base) + assertEquals("https://api.example.com/pets/7?limit=10", request.url.toExternalForm()) + assertEquals("application/json", request.headers.get("Accept")) + } + + @Test + fun `toRequest carries the body and method`() { + val body = RequestBody.create("{}", CommonMediaTypes.APPLICATION_JSON) + val request = TestOp(Method.POST, "/pets", bdy = body).toRequest(base) + assertEquals(Method.POST, request.method) + assertSame(body, request.body) + } + + @Test + fun `toRequest trims a trailing slash on the base before joining`() { + val request = TestOp(Method.GET, "/pets").toRequest("https://api.example.com/") + assertEquals("https://api.example.com/pets", request.url.toExternalForm()) + } + + @Test + fun `toRequest joins a base that already has a path prefix`() { + val request = TestOp(Method.GET, "/pets").toRequest("https://api.example.com/v1") + assertEquals("https://api.example.com/v1/pets", request.url.toExternalForm()) + } + + @Test + fun `toRequest throws when a template variable has no value`() { + assertFailsWith { + TestOp(Method.GET, "/pets/{petId}").toRequest(base) + } + } + + @Test + fun `toRequest fails fast on a template variable containing a slash rather than emitting it literally`() { + // A malformed `{a/b}` placeholder is captured whole as the variable name; with no matching + // path parameter it fails fast instead of leaking an unsubstituted `{a/b}` into the URL. + assertFailsWith { + TestOp(Method.GET, "/pets/{a/b}").toRequest(base) + } + } + + @Test + fun `minimal operation uses empty defaults`() { + val op = MinimalOp() + assertTrue(op.pathParams().isEmpty()) + assertTrue(op.queryParams().isEmpty()) + assertTrue(op.headers().names().isEmpty()) + assertNull(op.body()) + assertNull(op.operationName) + assertEquals("https://api.example.com/ping", op.toRequest(base).url.toExternalForm()) + } + + @Test + fun `toRequestContext feeds the request into the context chain`() { + val dispatch = DispatchContext.default() + val op = TestOp(Method.GET, "/pets") + val ctx = op.toRequestContext(base, dispatch) + try { + assertEquals(op.toRequest(base), ctx.request) + assertEquals(dispatch.callKey, ctx.callKey) + assertSame(dispatch.instrumentationContext, ctx.instrumentationContext) + assertSame(ctx, ContextStore.get(ctx.callKey)) + } finally { + ctx.close() + } + } + + @Test + fun `toRequestContext carries the operation name onto the context for the tracing seam`() { + val dispatch = DispatchContext.default() + val op = TestOp(Method.GET, "/pets", operationName = "ListPets") + val ctx = op.toRequestContext(base, dispatch) + try { + assertEquals("ListPets", ctx.operationName) + } finally { + ctx.close() + } + } + + @Test + fun `toRequestContext leaves the operation name null when the operation declares none`() { + val dispatch = DispatchContext.default() + val ctx = TestOp(Method.GET, "/pets").toRequestContext(base, dispatch) + try { + assertNull(ctx.operationName) + } finally { + ctx.close() + } + } + + @Test + fun `toRequest merges a base URL query ahead of the operation query`() { + val query = QueryParams.builder().add("limit", "20").build() + val request = TestOp(Method.GET, "/pets", query = query).toRequest("https://api.example.com/v1?sv=X&sig=Y") + assertEquals("https://api.example.com/v1/pets?sv=X&sig=Y&limit=20", request.url.toExternalForm()) + } + + @Test + fun `toRequest preserves a base URL query when the operation contributes none`() { + val request = TestOp(Method.GET, "/pets").toRequest("https://api.example.com?sv=X") + assertEquals("https://api.example.com/pets?sv=X", request.url.toExternalForm()) + } + + @Test + fun `toRequest preserves a significant trailing slash when the path is empty`() { + // An empty path template targets the base resource itself; the base's trailing slash is + // semantic (some routers distinguish /v1/ from /v1) and must not be silently stripped. + val request = TestOp(Method.GET, "").toRequest("https://api.example.com/v1/") + assertEquals("https://api.example.com/v1/", request.url.toExternalForm()) + } + + @Test + fun `toRequest does not double the separator when the base query ends with an ampersand`() { + val query = QueryParams.builder().add("b", "2").build() + val request = TestOp(Method.GET, "/pets", query = query).toRequest("https://api.example.com/v1?a=1&") + assertEquals("https://api.example.com/v1/pets?a=1&b=2", request.url.toExternalForm()) + } + + @Test + fun `toRequest drops a dangling base query separator when the operation contributes none`() { + // A trailing '&' on the base query is its own separator, not an empty parameter; with no + // operation query to join it must not survive as a dangling `?a=1&`. + val request = TestOp(Method.GET, "/pets").toRequest("https://api.example.com/v1?a=1&") + assertEquals("https://api.example.com/v1/pets?a=1", request.url.toExternalForm()) + } + + @Test + fun `toRequest rejects a base URL carrying a fragment`() { + assertFailsWith { + TestOp(Method.GET, "/pets").toRequest("https://api.example.com/v1#frag") + } + } + + @Test + fun `toRequest throws IllegalArgumentException for a malformed base URL`() { + assertFailsWith { + TestOp(Method.GET, "/pets").toRequest("api.example.com") + } + } +} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/RequestRebuilderTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/RequestRebuilderTest.kt new file mode 100644 index 00000000..0b20c8c6 --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pagination/RequestRebuilderTest.kt @@ -0,0 +1,104 @@ +/* + * 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 java.net.URL +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * Direct tests for the internal [RequestRebuilder] URL helpers. + * + * Two properties matter here and are easy to regress: + * 1. **New values use RFC 3986 encoding** — a space becomes `%20`, not `+`. + * 2. **Untouched parameters are preserved byte-for-byte** — the rebuilder splices the raw + * query rather than decoding and re-encoding the whole thing, so a value-less flag stays + * value-less and reserved characters in other params are not rewritten. + */ +class RequestRebuilderTest { + private fun url(spec: String): URL = URL(spec) + + @Test + fun `setQueryParam appends a new param when absent`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items"), "page", "2") + assertEquals("page=2", result.query) + } + + @Test + fun `setQueryParam replaces an existing param in place`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items?page=1&sort=asc"), "page", "2") + assertEquals("page=2&sort=asc", result.query) + } + + @Test + fun `setQueryParam with null removes the param but keeps the others`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items?page=1&sort=asc"), "page", null) + assertEquals("sort=asc", result.query) + } + + @Test + fun `setQueryParam dropping the only param yields no query`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items?page=1"), "page", null) + assertNull(result.query) + } + + @Test + fun `setQueryParam encodes a space in the new value as percent-20`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items"), "q", "a b") + assertEquals("q=a%20b", result.query) + } + + @Test + fun `setQueryParam encodes reserved characters in the new value`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items"), "token", "a+b/c=") + assertEquals("token=a%2Bb%2Fc%3D", result.query) + } + + @Test + fun `setQueryParam preserves a value-less flag verbatim`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items?flag&page=1"), "page", "2") + assertEquals("flag&page=2", result.query) + } + + @Test + fun `setQueryParam preserves reserved chars in untouched params verbatim`() { + val result = RequestRebuilder.setQueryParam(url("https://api.example.com/items?filter=a:b&page=1"), "page", "2") + assertEquals("filter=a:b&page=2", result.query) + } + + @Test + fun `getQueryParam returns the decoded value`() { + assertEquals("2", RequestRebuilder.getQueryParam(url("https://api.example.com/items?page=2"), "page")) + } + + @Test + fun `getQueryParam returns null when absent`() { + assertNull(RequestRebuilder.getQueryParam(url("https://api.example.com/items?page=2"), "cursor")) + } + + @Test + fun `getQueryParam returns null when there is no query`() { + assertNull(RequestRebuilder.getQueryParam(url("https://api.example.com/items"), "page")) + } + + @Test + fun `getQueryParam decodes percent-escapes`() { + assertEquals("a b", RequestRebuilder.getQueryParam(url("https://api.example.com/items?q=a%20b"), "q")) + } + + @Test + fun `getQueryParam reads a literal plus as a plus`() { + assertEquals("a+b", RequestRebuilder.getQueryParam(url("https://api.example.com/items?q=a+b"), "q")) + } + + @Test + fun `getQueryParam reports an empty string for a value-less flag`() { + assertEquals("", RequestRebuilder.getQueryParam(url("https://api.example.com/items?flag&page=2"), "flag")) + } +}