diff --git a/openspec/changes/jdk-httpclient-transport/.openspec.yaml b/openspec/changes/jdk-httpclient-transport/.openspec.yaml new file mode 100644 index 000000000..a8821c74d --- /dev/null +++ b/openspec/changes/jdk-httpclient-transport/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-11 diff --git a/openspec/changes/jdk-httpclient-transport/design.md b/openspec/changes/jdk-httpclient-transport/design.md new file mode 100644 index 000000000..f7b3fb7e0 --- /dev/null +++ b/openspec/changes/jdk-httpclient-transport/design.md @@ -0,0 +1,83 @@ +## Context + +`symphony-bdk-http-api` defines the transport-agnostic contract (`ApiClient`, `ApiClientBuilder`, `ApiClientBuilderProvider`, `ApiException`, `Pair`, `ApiClientBodyPart`, `DistributedTracingContext`) that OpenAPI-generated code and `symphony-bdk-core` call into. Two implementations exist today: + +- **`symphony-bdk-http-jersey2`** — Jersey 2 client + Apache HttpClient connector + `jjwt`/BouncyCastle for TLS material handling. `@API(STABLE)`, the module `symphony-bdk-core` itself depends on at `testImplementation` scope, and the one the Spring Boot starter (`BdkCoreConfig`) hardcodes via `new ApiClientBuilderProviderJersey2()`. +- **`symphony-bdk-http-webclient`** — Spring `WebClient` + Reactor Netty, internally reactive but blocking (`.block()`) at the `invokeAPI` boundary to satisfy the same synchronous contract. `@API(EXPERIMENTAL)`. + +Both are selected at runtime purely via `ServiceLoader`: `ServiceLookup.lookupSingleService(ApiClientBuilderProvider.class)` in `symphony-bdk-core`'s `ApiClientFactory`/`SymphonyBdkBuilder` throws `IllegalStateException` if zero or more than one `ApiClientBuilderProvider` is found on the classpath. There is no classpath sniffing beyond this — a consumer picks exactly one http module as a runtime dependency. + +`RetryWithRecoveryBuilder.isNetworkIssueOrMinorError` in `symphony-bdk-core` inspects the root cause of exceptions surfaced from `invokeAPI` for exactly `java.net.SocketException`, `java.net.ConnectException`, `java.net.SocketTimeoutException`, and `java.net.UnknownHostException`. Both existing implementations translate their own transport-specific exceptions into these types at the `ApiClient` boundary; this is a real, load-bearing interop contract, not incidental to jersey2. + +This design adds a third implementation, `symphony-bdk-http-jdk`, built on `java.net.http.HttpClient` (JDK 11+, available since BDK's Java 17 toolchain baseline). + +## Goals / Non-Goals + +**Goals:** +- Implement `ApiClient`/`ApiClientBuilder`/`ApiClientBuilderProvider` on `java.net.http.HttpClient` with zero new external HTTP dependencies. +- Match jersey2's wire-level behavior (JSON shape, multipart encoding, trace-id header, file downloads) closely enough that a consumer switching implementations sees no functional difference. +- Preserve `symphony-bdk-core`'s retry behavior unmodified by translating exceptions into the same root-cause types jersey2/webclient already produce. +- Keep the module lean: reuse `symphony-bdk-http-api`'s shared helpers (`ApiUtils`, `DistributedTracingContext`, `Pair`) rather than re-deriving them. +- Replace jersey2 as the **documented default** HTTP implementation for `symphony-bdk-core`, and mark jersey2 `@API(DEPRECATED)` in that role — a soft, non-removing signal, not a functional change (D12). + +**Non-Goals:** +- Making the Spring Boot starter's HTTP implementation choice pluggable. `BdkCoreConfig` keeps its hardcoded `ApiClientBuilderProviderJersey2`; this module is `symphony-bdk-core`-only for v1. webclient's status as the Spring Boot starter's default is unaffected — this change only touches which module `symphony-bdk-core` docs recommend. +- Closing the timeout and filter-chain semantic gaps described below by extending the shared `ApiClientBuilder` contract. Those gaps are documented and absorbed at the implementation level, not solved by changing `symphony-bdk-http-api`. +- HTTP/2 push, WebSocket, or async (`sendAsync`) exposure through the public contract. `invokeAPI` is synchronous; internal use of `send` vs `sendAsync().join()` is an implementation detail (D-Sync). +- Removing `symphony-bdk-http-jersey2`, or forcing existing consumers off it. Deprecation here is documentation-and-annotation only (D12); the module keeps shipping and working. + +## Decisions + +**D1 — Timeout mapping.** JDK `HttpClient` exposes `Builder#connectTimeout(Duration)` at the client level and `HttpRequest.Builder#timeout(Duration)` as a per-request *total* timeout; there is no separate socket/read timeout concept the way Apache HttpClient (jersey2) or Reactor Netty (webclient) expose one. `ApiClientBuilder#withConnectionTimeout` maps directly to `HttpClient.Builder#connectTimeout`. `ApiClientBuilder#withReadTimeout` maps to `HttpRequest.Builder#timeout` on every built request — the closest available approximation, covering "the whole request took too long" rather than "no bytes arrived for N ms". This is called out explicitly in `ApiClientBuilderJdk`'s javadoc rather than presented as an exact equivalent. +- *Alternative considered*: leave `withReadTimeout` a no-op. Rejected — a silently ignored timeout configuration is worse than an approximated one; a consumer who sets a read timeout expects *some* bound on hung requests. + +**D2 — Exception translation for retry compatibility.** `HttpConnectTimeoutException` already extends `java.net.ConnectException`, so it needs no translation. `HttpTimeoutException` extends `java.io.IOException` directly (not `SocketTimeoutException`), so a timeout during `send`/`sendAsync().join()` (`ExecutionException` unwrapped) is caught and rethrown as `java.net.SocketTimeoutException` with the original as cause, mirroring how jersey2 rewraps Apache HC's `ConnectTimeoutException`/`NoHttpResponseException`. `java.net.ConnectException` and `java.net.UnknownHostException` propagate as-is since JDK `HttpClient` already throws them directly for connection refusal and DNS failure. +- *Alternative considered*: extend `RetryWithRecoveryBuilder.isNetworkIssueOrMinorError` to also check `HttpTimeoutException`. Rejected — that predicate is shared infrastructure used by every implementation; changing it to accommodate one new module's exception vocabulary is exactly the kind of blast-radius creep the existing two modules already avoided by translating locally instead. + +**D3 — Filter support (`addFilter`).** JDK `HttpClient` has no request/response filter chain API (unlike Jersey's `ClientRequestFilter`/`ClientResponseFilter` or WebClient's `ExchangeFilterFunction`). `ApiClientBuilderJdk#addFilter` accepts a new minimal functional type, `Function` (request-mutation only, applied before send), and throws `IllegalArgumentException` for anything else — mirroring `ApiClientBuilderWebClient`'s defensive `instanceof` check rather than silently no-op'ing. Outgoing request/response logging (status, URL, elapsed time to `com.symphony.bdk.requests.outgoing`) is implemented as a manual wrap around the `send` call, not through this filter mechanism, since it needs response data a request-only filter can't see. +- *Alternative considered*: full response-side filtering via a custom `HttpResponse.BodyHandler` wrapper. Rejected as unnecessary complexity — no current use case in the codebase (custom filters) needs response mutation, only request mutation (e.g., adding a header) or observation (logging, handled separately). + +**D4 — Connection pooling knobs remain no-ops.** JDK `HttpClient` has no per-instance connection-pool-size configuration (only JVM-wide system properties like `jdk.httpclient.connectionPoolSize`, outside any single `HttpClient.Builder`'s control). `withConnectionPoolMax`/`withConnectionPoolPerRoute` are left as the inherited no-op default from `ApiClientBuilder`, the same precedent `ApiClientBuilderWebClient` already sets. + +**D5 — JSON serialization: duplicate, don't extract.** A `JSON` class in the new module configures its own `ObjectMapper` (RFC3339 dates via the same `RFC3339DateFormat` pattern, `JavaTimeModule`, `JsonNullableModule`, `NON_NULL` inclusion, `FAIL_ON_UNKNOWN_PROPERTIES=false`, `FAIL_ON_INVALID_SUBTYPE=false`, enums via `toString`), duplicating jersey2's `JSON`/`RFC3339DateFormat` logic rather than extracting a shared helper into `symphony-bdk-http-api`. This is deliberate: it's ~50 lines, two implementations already exist without a shared abstraction, and a third module reaching for one is exactly the "wait for the third occurrence" point — but extracting now would touch the stable `symphony-bdk-http-api` module for the convenience of a module that doesn't need to be there yet. If a fourth implementation ever appears, that's the point to revisit. + +**D6 — Multipart body encoding is hand-rolled.** JDK `HttpClient` has no multipart body builder. `multipart/form-data` requests are encoded by hand: a boundary is generated per request, and each form param is written as a part per jersey2's supported value types — `File`, `Collection`, `ApiClientBodyPart`, `ApiClientBodyPart[]`, or a plain string field — using `HttpRequest.BodyPublishers.ofByteArrays` (or an equivalent streaming publisher for large files, avoiding buffering entire files into memory). This is the largest implementation-risk area in the module and gets dedicated unit tests per value type (see tasks). + +**D7 — File download responses.** When `returnType` resolves to `File`, the response body is written directly to `temporaryFolderPath` using `HttpResponse.BodyHandlers.ofFile(Path)`, with the target filename derived from the `Content-Disposition` response header when present — matching jersey2's `downloadFileFromResponse`/`prepareDownloadFile` behavior. + +**D8 — TLS.** `ApiClientBuilderJdk` builds a `javax.net.ssl.SSLContext` from the supplied keystore/truststore bytes + passwords using standard `KeyManagerFactory`/`TrustManagerFactory` APIs (no Jersey `SslConfigurator` or Netty `SslContextBuilder` needed), reusing `ApiUtils.addDefaultRootCaCertificates` so a custom truststore doesn't shadow the JVM's default CAs — the same reuse pattern both existing modules already follow. The context is passed to `HttpClient.Builder#sslContext`. + +**D9 — Proxy.** Proxy host/port is configured via `ProxySelector.of(InetSocketAddress)` on `HttpClient.Builder#proxy`. Proxy credentials are handled via `HttpClient.Builder#authenticator(Authenticator)`, implementing `Authenticator#getPasswordAuthentication` to respond only to `RequestorType.PROXY` challenges — JDK `HttpClient` drives the proxy Basic-Auth handshake itself once an `Authenticator` is registered, so no manual `Proxy-Authorization` header construction is needed. + +**D10 — Sync vs async internally.** `invokeAPI` stays synchronous at the contract boundary (matching every existing implementation). Internally, the implementation uses the blocking `HttpClient#send(...)` overload rather than `sendAsync(...).join()` — no internal concurrency benefit is available to extract here since the contract is synchronous end-to-end, and `send` avoids the extra `CompletableFuture`/`ExecutionException` unwrapping `sendAsync().join()` would otherwise require at every call site. + +**D11 — Module structure.** New Gradle module `symphony-bdk-http/symphony-bdk-http-jdk`, applying `bdk.java-library-conventions` + `bdk.java-publish-conventions` (the same two plugins jersey2/webclient apply), `api project(':symphony-bdk-http:symphony-bdk-http-api')`, plus `jackson-databind`, `jackson-datatype-jsr310`, `jackson-databind-nullable`, `slf4j-api`, `apiguardian-api` — all already BOM-pinned, so no new BOM entries beyond the module's own artifact coordinate. No Jersey, Apache HttpClient, `jjwt`, BouncyCastle, or Reactor Netty dependency. + +**D12 — Deprecating jersey2's default status.** `symphony-bdk-http-jdk` becomes the module `docs/tech/architecture.md`, `docs/getting-started.md`, and `docs/migration.md` present first/as-default for `symphony-bdk-core`. `symphony-bdk-http-jersey2`'s public classes (`ApiClientJersey2`, `ApiClientBuilderJersey2`, `ApiClientBuilderProviderJersey2`) move from `@API(STABLE)`/`@API(INTERNAL)` to `@API(status = API.Status.DEPRECATED)` — apiguardian's existing status vocabulary already used throughout this codebase, so no new annotation type is introduced. Deprecation is a signal only: the module keeps its `ServiceLoader` registration, keeps publishing, and gets no behavioral changes. A consumer who already declares `symphony-bdk-http-jersey2` as a runtime dependency sees nothing beyond an IDE/build warning; nothing forces them to switch. `docs/migration.md` gains a short "migrating off jersey2" note (not a required migration, an optional one) pointing at the two documented gaps (D1, D3) a switcher should know about before adopting the jdk module. +- *Alternative considered*: leave jersey2's `@API` status untouched and only change documentation prose. Rejected — the codebase already uses `@API` status as the machine-readable signal of a class's support tier (`STABLE`/`EXPERIMENTAL`/`INTERNAL`); presenting jersey2 as "no longer the default" in prose while its annotations still say `STABLE` would be inconsistent with how every other status change in this codebase is communicated. +- *Alternative considered*: skip the jdk module's own `@API(EXPERIMENTAL)` status (see Open Questions) and ship it `STABLE` immediately, on the reasoning that "the default" shouldn't be experimental. Rejected for now — the hand-rolled multipart encoder (D6) is real, un-battle-tested risk; shipping `EXPERIMENTAL`-but-default is an intentional, disclosed trade-off (see Risks) rather than overstating confidence to match the new default status. + +## Risks / Trade-offs + +- **[Risk]** Hand-rolled multipart encoding has subtle correctness edge cases (boundary collision, charset handling, large-file memory use) that a mature library like Jersey's `FormDataMultiPart` already handles. → **Mitigation**: dedicated unit tests per value type (`File`, `Collection`, `ApiClientBodyPart`, `ApiClientBodyPart[]`, plain field), plus an integration test that round-trips a real multipart upload against MockServer, mirroring `ApiClientBuilderJersey2Test`'s approach. +- **[Risk]** `withReadTimeout`'s approximation (per-request total timeout instead of a true socket/read timeout) could surprise a consumer migrating from jersey2 who relies on the distinction (e.g., large file downloads that legitimately take longer than a fixed read timeout would allow under the old semantics). → **Mitigation**: explicit javadoc on `ApiClientBuilderJdk#withReadTimeout` and a callout in `docs/migration.md`'s module-selection section. +- **[Risk]** `HttpTimeoutException`'s translation to `SocketTimeoutException` is a best-effort mapping done once, in one place (`ApiClientJdk`'s exception handling), and any future change to `RetryWithRecoveryBuilder`'s predicate could silently stop covering this module if the mapping isn't kept in sync. → **Mitigation**: a unit test that asserts the translated exception's root cause is exactly `SocketTimeoutException`, so a regression fails a test rather than surfacing as "retries stopped working" in production. +- **[Trade-off]** No filter chain means consumers relying on Jersey-style response-inspecting filters (e.g., a custom auth-refresh-on-401 filter) cannot port that pattern to this module. Accepted: no such filter exists in the current codebase, and the request-mutation-only filter type covers the documented `addFilter` use cases (adding headers). +- **[Trade-off]** Duplicated JSON configuration (D5) means a future date-format or Jackson-module change must be applied in two places (jersey2 and this module) plus a third if webclient's implicit Spring config ever needs the same explicit treatment. Accepted per the "wait for the third occurrence" reasoning in D5; revisit if a fourth implementation appears. +- **[Risk]** Making an `@API(EXPERIMENTAL)` module (see Open Questions) the documented default means new users following `docs/getting-started.md` land on the least battle-tested implementation by default, right as the hand-rolled multipart encoder (D6) is at its highest real-world-untested risk. → **Mitigation**: `docs/getting-started.md` explicitly calls out the `EXPERIMENTAL` status next to the default snippet, and names `symphony-bdk-http-jersey2` as the fallback for consumers who need `STABLE`-only dependencies; promotion to `@API(STABLE)` is tracked as a fast-follow once the module has a release cycle of real usage. +- **[Trade-off]** Existing consumers who read `docs/getting-started.md`/`architecture.md` again after upgrading BDK versions will see jersey2 no longer presented as the default, and its classes newly marked `@API(DEPRECATED)`, even though nothing in their build breaks. Accepted: this is the intended signal (D12), and `docs/migration.md`'s note makes clear it's optional, not a forced migration. + +## Migration Plan + +This is additive plus a documentation/annotation-only deprecation — there is no code migration for existing consumers. Adoption of the new default is opt-in for anyone already pinned to a specific HTTP module, automatic only for new consumers following updated docs: +1. New consumers following `docs/getting-started.md` now get `symphony-bdk-http-jdk` in the default dependency snippet. +2. Existing consumers with an explicit `symphony-bdk-http-jersey2` (or `-webclient`) runtime dependency are unaffected — nothing in their build or runtime behavior changes; they will simply notice jersey2's classes are now `@API(DEPRECATED)` if they inspect javadoc or get IDE warnings. +3. A consumer who *chooses* to switch replaces their `symphony-bdk-http-jersey2`/`-webclient` runtime dependency with `symphony-bdk-http-jdk`; `ServiceLoader` picks up the new module's `ApiClientBuilderProvider` automatically, no code change to `SymphonyBdkBuilder` usage required. +4. `docs/migration.md` gets a short, explicitly optional "migrating off jersey2" section listing the two documented gaps (D1 timeout semantics, D3 filter support) so a consumer can decide before switching, not discover it after. +No rollback concern beyond reverting the dependency swap or the doc/annotation change, since neither makes any change to shared code paths or removes jersey2. + +## Open Questions + +- Should `symphony-bdk-http-jdk` be promoted to `@API(STABLE)` immediately or ship `@API(EXPERIMENTAL)` for an initial release cycle before stabilizing, given it's now also the documented default? Leaning `EXPERIMENTAL` despite the default status — it's a new module with a hand-rolled multipart encoder as its highest-risk component, and the default-status risk is addressed by disclosing it prominently in docs (see Risks) rather than by overstating stability. +- Is there consumer demand to eventually make the Spring Boot starter's HTTP implementation pluggable (closing the `BdkCoreConfig` TODO) and switch its default too, and if so, should that be scoped as a follow-up change rather than folded in here? Left as a Non-Goal for this change either way. +- Should jersey2's `@API(DEPRECATED)` annotations carry a `since` version pointing at a future removal-consideration release, or stay open-ended given the Non-Goal that removal isn't planned? Leaning open-ended — apiguardian's `DEPRECATED` status doesn't require a removal date, and committing to one now would overstate this change's scope. diff --git a/openspec/changes/jdk-httpclient-transport/proposal.md b/openspec/changes/jdk-httpclient-transport/proposal.md new file mode 100644 index 000000000..5c3eb96fe --- /dev/null +++ b/openspec/changes/jdk-httpclient-transport/proposal.md @@ -0,0 +1,57 @@ +## Why + +`symphony-bdk-core` talks to the Symphony REST APIs through a pluggable `ApiClient` contract (`symphony-bdk-http-api`), and today exactly two runtime implementations exist: `symphony-bdk-http-jersey2` (Jersey 2 + Apache HttpClient) and `symphony-bdk-http-webclient` (Spring WebClient + Reactor Netty). Both pull in a third-party HTTP stack as a mandatory runtime dependency, and both require a consumer to accept that stack's own transitive dependency and CVE surface. + +Since JDK 11, `java.net.http.HttpClient` ships inside the JDK itself, with mutual-TLS, HTTP/2, proxying, and both synchronous and asynchronous sends built in. BDK's toolchain has been on Java 17 since `modernize-build-toolchain`, with `adopt-java-25-baseline` proposing to move further, so this capability has been available and unused for the entire lifetime of BDK 3.x/4.x. A third implementation that adds **zero** new external HTTP dependencies is now straightforward to offer, and gives consumers who care about minimal footprint, dependency hygiene, or avoiding Jersey/Apache-HC or Reactor Netty specifically, a first-class alternative rather than a reason to fork or hand-roll their own `ApiClient`. + +Once that alternative exists, keeping `symphony-bdk-http-jersey2` as the module every "getting started" doc and dependency snippet points to by default stops making sense: it's the option with the largest dependency and CVE surface (Jersey 2, Apache HttpClient, `jjwt`, BouncyCastle), for a benefit — Jersey-specific request/response filters — that nothing in the current codebase actually uses. This proposal makes `symphony-bdk-http-jdk` the new documented default for `symphony-bdk-core` and formally deprecates jersey2 in that role. Deprecation here means a soft, non-removing signal (`@API(status = API.Status.DEPRECATED)` plus javadoc pointing at the replacement, matching the annotation-based status system this codebase already uses for `STABLE`/`EXPERIMENTAL`/`INTERNAL`) — existing consumers who already depend on `symphony-bdk-http-jersey2` see no functional change and are not forced to migrate. + +## What Changes + +- **New module `symphony-bdk-http/symphony-bdk-http-jdk`**, published as `org.finos.symphony.bdk:symphony-bdk-http-jdk`, implementing `ApiClient`, `ApiClientBuilder`, and `ApiClientBuilderProvider` on top of `java.net.http.HttpClient`. Registered in `settings.gradle` and added as a BOM constraint in `symphony-bdk-bom/build.gradle`, following the exact pattern of the two existing modules. +- **Full behavioral parity with `symphony-bdk-http-jersey2`** wherever the shared contract requires it, so a consumer can swap implementations without observing a difference on the wire or in application code: + - Query/form/multipart param encoding, including `File`, `Collection`, and `ApiClientBodyPart`/`ApiClientBodyPart[]` form values (JDK `HttpClient` has no built-in multipart body support, so this is hand-rolled — see design D-Multipart). + - JSON serialization matching jersey2's `ObjectMapper` configuration: RFC3339 date strings, enums via `toString`, `NON_NULL` inclusion, `FAIL_ON_UNKNOWN_PROPERTIES=false`, `JsonNullableModule`/`JavaTimeModule` registered. + - `X-Trace-Id` header injection/cleanup via `DistributedTracingContext`, matching `ApiClientJersey2`'s generate-if-absent-then-clear behavior. + - File-download responses streamed to `temporaryFolderPath`, honoring `Content-Disposition` filename. + - TLS client certificate configuration (keystore/truststore bytes + password) via `HttpClient.Builder#sslContext`. + - HTTP proxy host/port and credentials. + - Outgoing request logging under the existing `com.symphony.bdk.requests.outgoing` logger. +- **Exception translation into the same root-cause types `symphony-bdk-core`'s retry predicate already inspects** (`RetryWithRecoveryBuilder.isNetworkIssueOrMinorError` checks for `java.net.SocketException`, `java.net.ConnectException`, `java.net.SocketTimeoutException`, `java.net.UnknownHostException`). This keeps retry behavior working unmodified for consumers who switch implementations — no changes to `symphony-bdk-core`'s retry code are in scope. +- **SPI registration** via `META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider`, the same `ServiceLoader` mechanism the two existing modules use. `ServiceLookup.lookupSingleService` already enforces exactly one implementation on the runtime classpath at a time — unchanged, and applies to the new module the same as the existing two. +- **Two documented, deliberate gaps versus jersey2**, not silently absorbed: + - JDK `HttpClient` has no distinct read/socket timeout, only a connect timeout and a per-request total timeout. `withReadTimeout` is mapped to the per-request timeout as the closest available approximation (design D1). + - JDK `HttpClient` has no request/response filter chain. `addFilter` accepts a narrower request-mutation-only functional type instead of an arbitrary Jersey filter (design D3). + - `withConnectionPoolMax`/`withConnectionPoolPerRoute` remain no-ops, the same precedent already set by `symphony-bdk-http-webclient`, since JDK `HttpClient` has no per-instance pool-sizing knob. +- **`symphony-bdk-http-jdk` becomes the documented default for `symphony-bdk-core`**, replacing `symphony-bdk-http-jersey2` in that role: `docs/tech/architecture.md`, `docs/getting-started.md`, and `docs/migration.md` are updated so the default dependency snippet and prose lead with `symphony-bdk-http-jdk`, with jersey2 and webclient presented as alternatives (webclient keeps its existing status as the Spring Boot starter's default, which this change does not touch). +- **`symphony-bdk-http-jersey2` is formally deprecated, not removed**: `ApiClientJersey2`, `ApiClientBuilderJersey2`, and `ApiClientBuilderProviderJersey2` move from `@API(STABLE)` to `@API(status = API.Status.DEPRECATED)`, with javadoc pointing consumers at `symphony-bdk-http-jdk`. The module keeps shipping, keeps its `ServiceLoader` registration, and gets no functional changes — a consumer with an existing `symphony-bdk-http-jersey2` runtime dependency observes nothing beyond a `@Deprecated`-style IDE warning. +- **Documentation**: `docs/tech/architecture.md`, `docs/getting-started.md`, and `docs/migration.md` updated to present the new module as the default `symphony-bdk-core` HTTP implementation, with a short "migrating off jersey2" note in `docs/migration.md` for consumers who want to switch. +- **Explicitly out of scope**: the Spring Boot starter's hardcoded `ApiClientBuilderProviderJersey2` wiring in `BdkCoreConfig` is untouched — webclient remains the Spring Boot starter's default, and making the Spring starter's HTTP implementation pluggable is separate work. Removing the jersey2 module or forcing migration is also out of scope; this change only changes the *recommendation*. +- **Not breaking.** This is an additive module plus a soft, non-removing deprecation of jersey2's default status. No existing `ApiClient`/`ApiClientBuilder` contract, generated API code, or published artifact changes; `symphony-bdk-http-jersey2` keeps working exactly as before for consumers who keep it as a runtime dependency. + +## Capabilities + +### New Capabilities +- `jdk-http-transport`: a `java.net.http.HttpClient`-based implementation of the BDK's `ApiClient`/`ApiClientBuilder` contract — its behavioral parity with the existing implementations, its documented semantic gaps, and its SPI discoverability. + +### Modified Capabilities +*(none tracked in `openspec/specs` — `symphony-bdk-http-jersey2` has no existing capability spec to modify. Its deprecation is described in this proposal's "What Changes" and in design.md D12, not as a spec delta)* + +## Impact + +**Code** +- New module tree `symphony-bdk-http/symphony-bdk-http-jdk/` (`build.gradle`, `src/main/java/com/symphony/bdk/http/jdk/*`, `src/main/resources/META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider`, `src/test/java/com/symphony/bdk/http/jdk/*`) +- `settings.gradle`: add `include(':symphony-bdk-http:symphony-bdk-http-jdk')` +- `symphony-bdk-bom/build.gradle`: add `api "org.finos.symphony.bdk:symphony-bdk-http-jdk:$project.version"` +- `symphony-bdk-http/symphony-bdk-http-jersey2/src/main/java/com/symphony/bdk/http/jersey2/{ApiClientJersey2,ApiClientBuilderJersey2,ApiClientBuilderProviderJersey2}.java`: annotation-only change, `@API(STABLE)`/`@API(INTERNAL)` → `@API(status = API.Status.DEPRECATED)`, plus javadoc pointing at `symphony-bdk-http-jdk` +- `docs/tech/architecture.md`, `docs/getting-started.md`, `docs/migration.md`: flip the default HTTP module recommendation from jersey2 to jdk + +**APIs**: +- New artifact `org.finos.symphony.bdk:symphony-bdk-http-jdk` (additive) +- No changes to `symphony-bdk-http-api`'s `ApiClient`, `ApiClientBuilder`, `ApiClientBuilderProvider`, `ApiException`, or any generated class +- Consumers choosing the jdk module get a subset of `ApiClientBuilder`'s optional knobs (see the two documented gaps above); this is implementation-specific behavior, not a contract change +- `symphony-bdk-http-jersey2`'s public classes gain a `DEPRECATED` `@API` status; this is a source-compatible annotation change, not a binary or behavioral change — the module still compiles, ships, and functions identically for existing consumers + +**Dependencies**: none new beyond what's already BOM-pinned (`jackson-databind`, `jackson-datatype-jsr310`, `jackson-databind-nullable`, `slf4j-api`, `apiguardian-api`). No new external HTTP library — that is the point of the module. + +**Docs**: three files updated so the new module is the one new users are pointed to first, jersey2 is presented as deprecated-but-supported, and webclient's status (Spring Boot starter default) is unchanged; no versioned-docs split needed since this remains additive and non-breaking, not a forced migration. diff --git a/openspec/changes/jdk-httpclient-transport/specs/jdk-http-transport/spec.md b/openspec/changes/jdk-httpclient-transport/specs/jdk-http-transport/spec.md new file mode 100644 index 000000000..28af78553 --- /dev/null +++ b/openspec/changes/jdk-httpclient-transport/specs/jdk-http-transport/spec.md @@ -0,0 +1,99 @@ +## ADDED Requirements + +### Requirement: SPI Discoverability +The module SHALL register its `ApiClientBuilderProvider` implementation via `META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider` so it is discoverable by `java.util.ServiceLoader` using the same mechanism as `symphony-bdk-http-jersey2` and `symphony-bdk-http-webclient`. + +#### Scenario: Sole HTTP implementation on the runtime classpath +- **WHEN** `symphony-bdk-http-jdk` is the only `ApiClientBuilderProvider` implementation on the runtime classpath +- **THEN** `SymphonyBdkBuilder.build()` resolves it via `ServiceLookup.lookupSingleService` without any explicit configuration + +#### Scenario: Coexisting with another HTTP implementation +- **WHEN** `symphony-bdk-http-jdk` and another `ApiClientBuilderProvider` implementation (e.g. `symphony-bdk-http-jersey2`) are both present on the runtime classpath +- **THEN** `ServiceLookup.lookupSingleService` throws `IllegalStateException`, the same behavior already enforced for any two coexisting implementations + +### Requirement: ApiClient Contract Conformance +`ApiClientJdk` SHALL implement `ApiClient#invokeAPI` fully, encoding query params, form params, and multipart form data equivalently to `ApiClientJersey2` for every value type the shared contract supports. + +#### Scenario: Multipart file upload +- **WHEN** a request is built with `contentType = multipart/form-data` and a form param of type `File` +- **THEN** the request body contains a multipart part with the file's content and filename, matching the encoding `ApiClientJersey2` produces for the same input + +#### Scenario: Multipart collection of files +- **WHEN** a request is built with `contentType = multipart/form-data` and a form param of type `Collection` +- **THEN** the request body contains one multipart part per file in the collection + +#### Scenario: URL-encoded form body +- **WHEN** a request is built with `contentType = application/x-www-form-urlencoded` and one or more form params +- **THEN** the request body is encoded as `key=value` pairs joined with `&`, with each key and value percent-encoded + +### Requirement: Distributed Tracing Header Propagation +`ApiClientJdk` SHALL set the `X-Trace-Id` header on every outgoing request via `DistributedTracingContext`, generating a trace ID when none is set and clearing only the IDs it generated itself. + +#### Scenario: No trace ID set before the call +- **WHEN** `invokeAPI` is called and `DistributedTracingContext.hasTraceId()` is `false` beforehand +- **THEN** a trace ID is generated and set as the `X-Trace-Id` header, and `DistributedTracingContext` is cleared after the call completes + +#### Scenario: Trace ID already set before the call +- **WHEN** `invokeAPI` is called and `DistributedTracingContext.hasTraceId()` is `true` beforehand +- **THEN** the existing trace ID is reused as the `X-Trace-Id` header and is NOT cleared after the call completes + +### Requirement: Exception Translation for Retry Compatibility +`ApiClientJdk` SHALL translate transport-level exceptions from `java.net.http.HttpClient` into root causes that `symphony-bdk-core`'s `RetryWithRecoveryBuilder.isNetworkIssueOrMinorError` predicate already recognizes: `java.net.SocketException`, `java.net.ConnectException`, `java.net.SocketTimeoutException`, or `java.net.UnknownHostException`. + +#### Scenario: Connect timeout +- **WHEN** the underlying `HttpClient` fails to establish a connection within the configured connect timeout +- **THEN** `invokeAPI` throws an exception whose root cause is `java.net.ConnectException` (via `HttpConnectTimeoutException`, which already extends it) + +#### Scenario: Request timeout during send +- **WHEN** a request exceeds its configured per-request timeout while in flight, causing `HttpTimeoutException` +- **THEN** `invokeAPI` throws an exception whose root cause is `java.net.SocketTimeoutException` + +### Requirement: TLS Client Certificate Support +`ApiClientBuilderJdk` SHALL build an `SSLContext` from supplied keystore and truststore bytes and passwords, merging in the JVM's default root CA certificates so a custom truststore does not shadow public CAs, and apply it to the built `HttpClient` via `HttpClient.Builder#sslContext`. + +#### Scenario: Mutual TLS against a server requiring client certificates +- **WHEN** `ApiClientBuilderJdk` is configured with a keystore and truststore via `withKeyStore`/`withTrustStore` and a request is sent to a server enforcing mutual TLS authentication +- **THEN** the TLS handshake succeeds and the request completes without a certificate error + +### Requirement: HTTP Proxy Support +`ApiClientBuilderJdk` SHALL route requests through a configured HTTP proxy host and port, and SHALL answer proxy Basic-Auth challenges when proxy credentials are configured. + +#### Scenario: Proxy host and port configured +- **WHEN** `withProxy` is configured with a host and port +- **THEN** outgoing requests are routed through that proxy + +#### Scenario: Proxy credentials configured +- **WHEN** `withProxyCredentials` is configured and the proxy responds with a `407 Proxy Authentication Required` challenge +- **THEN** the request is retried with proxy credentials supplied via an `Authenticator` responding to `RequestorType.PROXY`, and completes successfully + +### Requirement: JSON Serialization Parity +`ApiClientJdk` SHALL serialize and deserialize request and response bodies using an `ObjectMapper` configuration equivalent to `ApiClientJersey2`'s: dates as RFC3339 strings, enums via `toString`, `NON_NULL` inclusion, unknown properties ignored, and `JsonNullable`-wrapped fields supported. + +#### Scenario: Date field round-trip +- **WHEN** a model containing a date field is serialized and then deserialized +- **THEN** the date is represented on the wire as an RFC3339 string identical in format to what `ApiClientJersey2` produces for the same value + +#### Scenario: Unknown JSON property in a response +- **WHEN** a response body contains a JSON property not present on the target model class +- **THEN** deserialization succeeds and ignores the unknown property, rather than throwing + +### Requirement: File Download Responses +When the expected return type is `File`, `ApiClientJdk` SHALL stream the response body directly to `temporaryFolderPath`, using the filename from the `Content-Disposition` header when present. + +#### Scenario: Downloading a file with a Content-Disposition header +- **WHEN** a request whose declared return type is `File` receives a response with a `Content-Disposition: attachment; filename="report.pdf"` header +- **THEN** the response body is written to a file named `report.pdf` inside `temporaryFolderPath` + +### Requirement: Timeout Configuration Mapping +`ApiClientBuilderJdk` SHALL map `withConnectionTimeout` to the underlying `HttpClient`'s connect timeout, and SHALL map `withReadTimeout` to the per-request total timeout, since JDK `HttpClient` exposes no distinct socket/read timeout. + +#### Scenario: Read timeout configured +- **WHEN** `withReadTimeout` is configured with a duration +- **THEN** every request built by the resulting `ApiClient` has that duration applied via `HttpRequest.Builder#timeout` + +### Requirement: Outgoing Request Logging +`ApiClientJdk` SHALL log each outgoing request's status code, URL, and elapsed time at DEBUG level under the `com.symphony.bdk.requests.outgoing` logger, matching the existing logging contract from `ApiClientJersey2RequestLogFilter`. + +#### Scenario: A request completes +- **WHEN** a request sent through `ApiClientJdk` receives a response +- **THEN** a DEBUG-level log entry is written to `com.symphony.bdk.requests.outgoing` containing the response status code, the request URL, and the elapsed time diff --git a/openspec/changes/jdk-httpclient-transport/tasks.md b/openspec/changes/jdk-httpclient-transport/tasks.md new file mode 100644 index 000000000..7023276bb --- /dev/null +++ b/openspec/changes/jdk-httpclient-transport/tasks.md @@ -0,0 +1,90 @@ +## 1. Module Scaffolding + +- [ ] 1.1 Create `symphony-bdk-http/symphony-bdk-http-jdk/build.gradle` applying `bdk.java-library-conventions` + `bdk.java-publish-conventions`, with `api project(':symphony-bdk-http:symphony-bdk-http-api')` and `jackson-databind`, `jackson-datatype-jsr310`, `jackson-databind-nullable`, `slf4j-api`, `apiguardian-api`, plus test deps mirroring jersey2 (`junit-jupiter`, `logback-classic`, `mockserver-netty`, `mockito-core`, `mockito-junit-jupiter`, `junit-platform-launcher`) +- [ ] 1.2 Add `include(':symphony-bdk-http:symphony-bdk-http-jdk')` to root `settings.gradle` +- [ ] 1.3 Add `api "org.finos.symphony.bdk:symphony-bdk-http-jdk:$project.version"` to `symphony-bdk-bom/build.gradle` +- [ ] 1.4 Confirm `./gradlew :symphony-bdk-http:symphony-bdk-http-jdk:build` succeeds with an empty module before adding implementation code + +## 2. JSON Serialization (D5) + +- [ ] 2.1 Create `com.symphony.bdk.http.jdk.JSON`, configuring an `ObjectMapper` matching jersey2's: `NON_NULL` inclusion, `FAIL_ON_UNKNOWN_PROPERTIES=false`, `FAIL_ON_INVALID_SUBTYPE=false`, enums via `toString`, `JavaTimeModule` and `JsonNullableModule` registered +- [ ] 2.2 Create an `RFC3339DateFormat`-equivalent date formatter producing the same wire format as jersey2's +- [ ] 2.3 Unit test: date field serializes/deserializes to the same RFC3339 string jersey2 produces for the same value (spec: JSON Serialization Parity) +- [ ] 2.4 Unit test: response containing an unrecognized JSON property deserializes without throwing (spec: JSON Serialization Parity) + +## 3. Core `ApiClientJdk` — Request Building + +- [ ] 3.1 Create `com.symphony.bdk.http.jdk.ApiClientJdk implements ApiClient`, building `HttpRequest`s from `basePath` + `path`, query params via `parameterToPairs`/`escapeString`, headers, and cookies +- [ ] 3.2 Implement `X-Trace-Id` header injection via `DistributedTracingContext`: generate-if-absent, clear only if generated (spec: Distributed Tracing Header Propagation) +- [ ] 3.3 Unit test: trace ID generated and cleared when absent before the call +- [ ] 3.4 Unit test: existing trace ID preserved and not cleared when already set before the call +- [ ] 3.5 Implement `application/x-www-form-urlencoded` body encoding (spec: ApiClient Contract Conformance) +- [ ] 3.6 Unit test: form params encode as `key=value` pairs joined with `&`, percent-encoded + +## 4. Multipart Body Encoding (D6) + +- [ ] 4.1 Implement multipart/form-data body construction: boundary generation, part encoding for `File` +- [ ] 4.2 Extend multipart encoding to `Collection` (one part per file), `ApiClientBodyPart`, `ApiClientBodyPart[]`, and plain string fields +- [ ] 4.3 Use a streaming `BodyPublisher` for file parts to avoid buffering entire files into memory +- [ ] 4.4 Unit test: `File` form param produces a multipart part with matching content and filename +- [ ] 4.5 Unit test: `Collection` form param produces one multipart part per file +- [ ] 4.6 Unit test: `ApiClientBodyPart`/`ApiClientBodyPart[]` form params produce parts from their `InputStream` content and filename +- [ ] 4.7 Integration test against MockServer: a real multipart file upload round-trips correctly + +## 5. Response Handling + +- [ ] 5.1 Implement response deserialization via the `JSON` `ObjectMapper` for non-`File`/`byte[]` return types +- [ ] 5.2 Implement file-download responses via `HttpResponse.BodyHandlers.ofFile(Path)`, writing into `temporaryFolderPath`, honoring `Content-Disposition` filename when present (spec: File Download Responses) +- [ ] 5.3 Implement `byte[]` return type handling +- [ ] 5.4 Implement `204 No Content` → `ApiResponse` with null data +- [ ] 5.5 Implement non-2xx handling: read body as string, throw `ApiException(status, message, headers, body)` +- [ ] 5.6 Unit test: response with `Content-Disposition: attachment; filename="report.pdf"` and return type `File` writes a file named `report.pdf` into `temporaryFolderPath` + +## 6. Exception Translation (D2) + +- [ ] 6.1 Catch `HttpTimeoutException` around `send`/unwrapped `sendAsync().join()` calls and rethrow as `java.net.SocketTimeoutException` with the original as cause +- [ ] 6.2 Confirm `HttpConnectTimeoutException`, `java.net.ConnectException`, and `java.net.UnknownHostException` propagate unmodified (no translation needed — already the right root-cause types) +- [ ] 6.3 Unit test: a connect-timeout scenario surfaces `java.net.ConnectException` as the root cause (spec: Exception Translation for Retry Compatibility) +- [ ] 6.4 Unit test: a request-timeout scenario surfaces `java.net.SocketTimeoutException` as the root cause (spec: Exception Translation for Retry Compatibility) +- [ ] 6.5 Integration test: `RetryWithRecoveryBuilder`'s retry predicate treats both translated exceptions as retryable, confirming interop with `symphony-bdk-core`'s existing retry logic unmodified + +## 7. `ApiClientBuilderJdk` — TLS, Proxy, Timeouts (D1, D8, D9) + +- [ ] 7.1 Create `com.symphony.bdk.http.jdk.ApiClientBuilderJdk implements ApiClientBuilder` +- [ ] 7.2 Implement `withKeyStore`/`withTrustStore`: build an `SSLContext` via `KeyManagerFactory`/`TrustManagerFactory`, merging default JVM root CAs via `ApiUtils.addDefaultRootCaCertificates`, applied via `HttpClient.Builder#sslContext` +- [ ] 7.3 Implement `withProxy`: `ProxySelector.of(InetSocketAddress)` on `HttpClient.Builder#proxy` +- [ ] 7.4 Implement `withProxyCredentials`: `HttpClient.Builder#authenticator(Authenticator)` responding only to `RequestorType.PROXY` +- [ ] 7.5 Implement `withConnectionTimeout` → `HttpClient.Builder#connectTimeout`; `withReadTimeout` → per-request `HttpRequest.Builder#timeout`, with javadoc documenting the approximation (D1) +- [ ] 7.6 Leave `withConnectionPoolMax`/`withConnectionPoolPerRoute` as the inherited no-op default (D4) — no override needed +- [ ] 7.7 Integration test against MockServer with mutual TLS enforced: keystore/truststore wiring succeeds end-to-end (mirrors `ApiClientBuilderJersey2Test#sslContextIsUsed`) +- [ ] 7.8 Integration test: request routed through a configured proxy +- [ ] 7.9 Integration test: proxy credentials answer a `407` challenge successfully + +## 8. Filters and Request Logging (D3) + +- [ ] 8.1 Define `addFilter` to accept `Function` and throw `IllegalArgumentException` for any other type +- [ ] 8.2 Implement outgoing request logging as a manual wrap around `send`, logging status code, URL, and elapsed time at DEBUG to `com.symphony.bdk.requests.outgoing` +- [ ] 8.3 Unit test: a custom filter function is applied to the outgoing request +- [ ] 8.4 Unit test: passing a non-`Function` filter throws `IllegalArgumentException` +- [ ] 8.5 Unit test: a completed request produces the expected DEBUG log entry (spec: Outgoing Request Logging) + +## 9. SPI Registration + +- [ ] 9.1 Create `com.symphony.bdk.http.jdk.ApiClientBuilderProviderJdk implements ApiClientBuilderProvider` +- [ ] 9.2 Create `src/main/resources/META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider` containing `com.symphony.bdk.http.jdk.ApiClientBuilderProviderJdk` +- [ ] 9.3 Integration test: with only `symphony-bdk-http-jdk` on the test classpath, `ServiceLookup.lookupSingleService(ApiClientBuilderProvider.class)` resolves it +- [ ] 9.4 Integration test: with `symphony-bdk-http-jdk` and `symphony-bdk-http-jersey2` both on the test classpath, `ServiceLookup.lookupSingleService` throws `IllegalStateException` + +## 10. Coverage and Build Verification + +- [ ] 10.1 Confirm `jacocoTestCoverageVerification` passes at the same per-class line-coverage bar `symphony-bdk-http-webclient` enforces +- [ ] 10.2 Run `./gradlew :symphony-bdk-http:symphony-bdk-http-jdk:build` clean and confirm no Jersey/Apache-HC/Reactor-Netty transitive dependency appears in `dependencies` output +- [ ] 10.3 Run the full `./gradlew build` to confirm no regression in `symphony-bdk-core` or other modules + +## 11. Documentation and Default Flip (D12) + +- [ ] 11.1 Update `docs/tech/architecture.md` line 38 so `symphony-bdk-http-jdk` is listed as the default implementation for `symphony-bdk-core`, with `symphony-bdk-http-jersey2` reworded as deprecated-but-supported; leave line 39 (webclient as Spring Boot's default) unchanged +- [ ] 11.2 Update `docs/getting-started.md`'s dependency snippets (Maven line ~62, Gradle line ~104) so `symphony-bdk-http-jdk` is the leading example, with jersey2/webclient in the "or" comment; call out the module's `EXPERIMENTAL` status next to the default snippet (per Open Questions/Risks) +- [ ] 11.3 Update `docs/migration.md`'s equivalent dependency snippets (lines ~119, ~222) to match the new default, and add a short, explicitly optional "migrating off jersey2" section documenting the two semantic gaps (D1 timeout mapping, D3 filter support) so a consumer can decide before switching implementations +- [ ] 11.4 Add `@API(status = API.Status.DEPRECATED)` and javadoc pointing at `symphony-bdk-http-jdk` to `ApiClientJersey2`, `ApiClientBuilderJersey2`, and `ApiClientBuilderProviderJersey2` in `symphony-bdk-http-jersey2` (replacing their current `@API(STABLE)`/`@API(INTERNAL)` status) +- [ ] 11.5 Confirm `./gradlew :symphony-bdk-http:symphony-bdk-http-jersey2:build` still passes after the annotation-only change — no behavioral or test changes expected in that module