diff --git a/docs/getting-started.md b/docs/getting-started.md index e5291058d..f655059dc 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -24,6 +24,11 @@ yo @finos/symphony ## Creating your project _from scratch_ This section will help you to understand how to create your bot application from scratch. +> :warning: The default HTTP client dependency below, `symphony-bdk-http-jdk`, is currently `@API(EXPERIMENTAL)`. It has +> zero third-party HTTP dependencies (built directly on `java.net.http.HttpClient`), but if you need an `@API(STABLE)`-only +> dependency tree, use `symphony-bdk-http-jersey` instead. See [Migration Guide](migration.md) for details on switching +> between HTTP client implementations. + ### Maven-based project If you want to use [Maven](https://maven.apache.org/) as build system, you have to configure your root `pom.xml` as such: ```xml @@ -59,7 +64,7 @@ If you want to use [Maven](https://maven.apache.org/) as build system, you have org.finos.symphony.bdk - symphony-bdk-http-jersey + symphony-bdk-http-jdk runtime @@ -101,7 +106,7 @@ dependencies { // define dependencies without versions implementation 'org.finos.symphony.bdk:symphony-bdk-core' - runtimeOnly 'org.finos.symphony.bdk:symphony-bdk-http-jersey' // or symphony-bdk-http-webclient + runtimeOnly 'org.finos.symphony.bdk:symphony-bdk-http-jdk' // or symphony-bdk-http-jersey / symphony-bdk-http-webclient runtimeOnly 'org.finos.symphony.bdk:symphony-bdk-template-freemarker' // or symphony-bdk-http-handlebars // logger configuration diff --git a/docs/migration-4.x.md b/docs/migration-4.x.md index b1d2c19ce..e4b770050 100644 --- a/docs/migration-4.x.md +++ b/docs/migration-4.x.md @@ -167,6 +167,36 @@ No other generated class changes shape, method signatures, `equals`/`hashCode`/` consumer-visible way. Fluent builder method names (`addXxxItem`, `putXxxItem`, etc.) and all constructors are unchanged. +## 9. New default HTTP client module: `symphony-bdk-http-jdk` + +BDK 4.x introduces `symphony-bdk-http-jdk`, a third `ApiClient` implementation built directly on +`java.net.http.HttpClient` (available since Java 11, and part of the JDK itself), with **no third-party HTTP +dependency**. It is now the module `docs/getting-started.md` and `docs/tech/architecture.md` present as the default +for `symphony-bdk-core`, ahead of `symphony-bdk-http-jersey`. + +This is **not a required migration**. `symphony-bdk-http-jersey` keeps shipping and working exactly as before — +existing consumers with an explicit `symphony-bdk-http-jersey` runtime dependency see no functional change, beyond +its classes now being annotated `@API(status = API.Status.DEPRECATED)` (a documentation/IDE-warning signal only, +not a removal notice). + +If you want to switch, replace your `symphony-bdk-http-jersey` (or `symphony-bdk-http-webclient`) runtime dependency +with `symphony-bdk-http-jdk` — `ServiceLoader` picks up the new module's `ApiClientBuilderProvider` automatically, no +code change to `SymphonyBdkBuilder` usage required. Before switching, be aware of two behavioral differences from +`symphony-bdk-http-jersey`: + +- **Read timeout semantics.** `java.net.http.HttpClient` has no distinct socket/read timeout, only a connect timeout + and a per-request *total* timeout. `ApiClientBuilder#withReadTimeout` is mapped to the per-request total timeout — + the closest available approximation, but it bounds "the whole request took too long" rather than "no bytes arrived + for N ms". This can matter for large, legitimately slow responses (e.g. large file downloads) that previously + fit comfortably under a read-timeout-only budget. +- **Filter support.** `java.net.http.HttpClient` has no request/response filter chain. `ApiClientBuilder#addFilter` + on `ApiClientBuilderJdk` only accepts a `Function` (request mutation + only, e.g. adding a header) instead of an arbitrary Jersey `ClientRequestFilter`/`ClientResponseFilter`. Response- + inspecting filters cannot be ported to this module. + +`symphony-bdk-http-jdk` is currently `@API(status = API.Status.EXPERIMENTAL)`. If you need an `@API(STABLE)`-only +dependency tree, stay on `symphony-bdk-http-jersey` for now. + ## Support window BDK 3.x will receive critical security fixes for **6 months** following the BDK 4.0.0 release, where Symphony is diff --git a/docs/tech/architecture.md b/docs/tech/architecture.md index 637b9c490..bef1277b9 100644 --- a/docs/tech/architecture.md +++ b/docs/tech/architecture.md @@ -34,8 +34,9 @@ also provides a utility `com.symphony.bdk.http.api.HttpClient` class helping dev > :warning: It is important to notice that interface `com.symphony.bdk.http.api.ApiClient` is used by generated code. > Changing contract would break the build. See [Code Generation](#code-generation). -At the moment, two different implementations have been created for the `com.symphony.bdk.http.api.ApiClient` interface: -- `com.symphony.bdk.http.jersey2.ApiClientJersey2` contained in module `symphony-bdk-http-jersey` (default implementation for [Core](#symphony-bdk-core)) +At the moment, three different implementations have been created for the `com.symphony.bdk.http.api.ApiClient` interface: +- `com.symphony.bdk.http.jdk.ApiClientJdk` contained in module `symphony-bdk-http-jdk` (default implementation for [Core](#symphony-bdk-core), built on `java.net.http.HttpClient` with no third-party HTTP dependency) +- `com.symphony.bdk.http.jersey2.ApiClientJersey2` contained in module `symphony-bdk-http-jersey` (deprecated as [Core](#symphony-bdk-core)'s default in favor of `symphony-bdk-http-jdk`, but still supported) - `com.symphony.bdk.http.webclient.ApiClientWebClient` contained in module `symphony-bdk-http-webclient` (default implementation for [Spring Boot](#symphony-bdk-spring)) ### symphony-bdk-template diff --git a/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/.openspec.yaml b/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/.openspec.yaml new file mode 100644 index 000000000..a8821c74d --- /dev/null +++ b/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-11 diff --git a/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/design.md b/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/design.md new file mode 100644 index 000000000..f7b3fb7e0 --- /dev/null +++ b/openspec/changes/archive/2026-08-12-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/archive/2026-08-12-jdk-httpclient-transport/proposal.md b/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/proposal.md new file mode 100644 index 000000000..5c3eb96fe --- /dev/null +++ b/openspec/changes/archive/2026-08-12-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/archive/2026-08-12-jdk-httpclient-transport/specs/jdk-http-transport/spec.md b/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/specs/jdk-http-transport/spec.md new file mode 100644 index 000000000..28af78553 --- /dev/null +++ b/openspec/changes/archive/2026-08-12-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/archive/2026-08-12-jdk-httpclient-transport/tasks.md b/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/tasks.md new file mode 100644 index 000000000..fc7329309 --- /dev/null +++ b/openspec/changes/archive/2026-08-12-jdk-httpclient-transport/tasks.md @@ -0,0 +1,90 @@ +## 1. Module Scaffolding + +- [x] 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`) +- [x] 1.2 Add `include(':symphony-bdk-http:symphony-bdk-http-jdk')` to root `settings.gradle` +- [x] 1.3 Add `api "org.finos.symphony.bdk:symphony-bdk-http-jdk:$project.version"` to `symphony-bdk-bom/build.gradle` +- [x] 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) + +- [x] 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 +- [x] 2.2 Create an `RFC3339DateFormat`-equivalent date formatter producing the same wire format as jersey2's +- [x] 2.3 Unit test: date field serializes/deserializes to the same RFC3339 string jersey2 produces for the same value (spec: JSON Serialization Parity) +- [x] 2.4 Unit test: response containing an unrecognized JSON property deserializes without throwing (spec: JSON Serialization Parity) + +## 3. Core `ApiClientJdk` — Request Building + +- [x] 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 +- [x] 3.2 Implement `X-Trace-Id` header injection via `DistributedTracingContext`: generate-if-absent, clear only if generated (spec: Distributed Tracing Header Propagation) +- [x] 3.3 Unit test: trace ID generated and cleared when absent before the call +- [x] 3.4 Unit test: existing trace ID preserved and not cleared when already set before the call +- [x] 3.5 Implement `application/x-www-form-urlencoded` body encoding (spec: ApiClient Contract Conformance) +- [x] 3.6 Unit test: form params encode as `key=value` pairs joined with `&`, percent-encoded + +## 4. Multipart Body Encoding (D6) + +- [x] 4.1 Implement multipart/form-data body construction: boundary generation, part encoding for `File` +- [x] 4.2 Extend multipart encoding to `Collection` (one part per file), `ApiClientBodyPart`, `ApiClientBodyPart[]`, and plain string fields +- [x] 4.3 Use a streaming `BodyPublisher` for file parts to avoid buffering entire files into memory +- [x] 4.4 Unit test: `File` form param produces a multipart part with matching content and filename +- [x] 4.5 Unit test: `Collection` form param produces one multipart part per file +- [x] 4.6 Unit test: `ApiClientBodyPart`/`ApiClientBodyPart[]` form params produce parts from their `InputStream` content and filename +- [x] 4.7 Integration test against MockServer: a real multipart file upload round-trips correctly + +## 5. Response Handling + +- [x] 5.1 Implement response deserialization via the `JSON` `ObjectMapper` for non-`File`/`byte[]` return types +- [x] 5.2 Implement file-download responses via `HttpResponse.BodyHandlers.ofFile(Path)`, writing into `temporaryFolderPath`, honoring `Content-Disposition` filename when present (spec: File Download Responses) +- [x] 5.3 Implement `byte[]` return type handling +- [x] 5.4 Implement `204 No Content` → `ApiResponse` with null data +- [x] 5.5 Implement non-2xx handling: read body as string, throw `ApiException(status, message, headers, body)` +- [x] 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) + +- [x] 6.1 Catch `HttpTimeoutException` around `send`/unwrapped `sendAsync().join()` calls and rethrow as `java.net.SocketTimeoutException` with the original as cause +- [x] 6.2 Confirm `HttpConnectTimeoutException`, `java.net.ConnectException`, and `java.net.UnknownHostException` propagate unmodified (no translation needed — already the right root-cause types) +- [x] 6.3 Unit test: a connect-timeout scenario surfaces `java.net.ConnectException` as the root cause (spec: Exception Translation for Retry Compatibility) +- [x] 6.4 Unit test: a request-timeout scenario surfaces `java.net.SocketTimeoutException` as the root cause (spec: Exception Translation for Retry Compatibility) +- [x] 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) + +- [x] 7.1 Create `com.symphony.bdk.http.jdk.ApiClientBuilderJdk implements ApiClientBuilder` +- [x] 7.2 Implement `withKeyStore`/`withTrustStore`: build an `SSLContext` via `KeyManagerFactory`/`TrustManagerFactory`, merging default JVM root CAs via `ApiUtils.addDefaultRootCaCertificates`, applied via `HttpClient.Builder#sslContext` +- [x] 7.3 Implement `withProxy`: `ProxySelector.of(InetSocketAddress)` on `HttpClient.Builder#proxy` +- [x] 7.4 Implement `withProxyCredentials`: `HttpClient.Builder#authenticator(Authenticator)` responding only to `RequestorType.PROXY` +- [x] 7.5 Implement `withConnectionTimeout` → `HttpClient.Builder#connectTimeout`; `withReadTimeout` → per-request `HttpRequest.Builder#timeout`, with javadoc documenting the approximation (D1) +- [x] 7.6 Leave `withConnectionPoolMax`/`withConnectionPoolPerRoute` as the inherited no-op default (D4) — no override needed +- [x] 7.7 Integration test against MockServer with mutual TLS enforced: keystore/truststore wiring succeeds end-to-end (mirrors `ApiClientBuilderJersey2Test#sslContextIsUsed`) +- [x] 7.8 Integration test: request routed through a configured proxy +- [x] 7.9 Integration test: proxy credentials answer a `407` challenge successfully + +## 8. Filters and Request Logging (D3) + +- [x] 8.1 Define `addFilter` to accept `Function` and throw `IllegalArgumentException` for any other type +- [x] 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` +- [x] 8.3 Unit test: a custom filter function is applied to the outgoing request +- [x] 8.4 Unit test: passing a non-`Function` filter throws `IllegalArgumentException` +- [x] 8.5 Unit test: a completed request produces the expected DEBUG log entry (spec: Outgoing Request Logging) + +## 9. SPI Registration + +- [x] 9.1 Create `com.symphony.bdk.http.jdk.ApiClientBuilderProviderJdk implements ApiClientBuilderProvider` +- [x] 9.2 Create `src/main/resources/META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider` containing `com.symphony.bdk.http.jdk.ApiClientBuilderProviderJdk` +- [x] 9.3 Integration test: with only `symphony-bdk-http-jdk` on the test classpath, `ServiceLookup.lookupSingleService(ApiClientBuilderProvider.class)` resolves it +- [x] 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 + +- [x] 10.1 Confirm `jacocoTestCoverageVerification` passes at the same per-class line-coverage bar `symphony-bdk-http-webclient` enforces +- [x] 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 +- [x] 10.3 Run the full `./gradlew build` to confirm no regression in `symphony-bdk-core` or other modules + +## 11. Documentation and Default Flip (D12) + +- [x] 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 +- [x] 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) +- [x] 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 +- [x] 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) +- [x] 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 diff --git a/openspec/specs/jdk-http-transport/spec.md b/openspec/specs/jdk-http-transport/spec.md new file mode 100644 index 000000000..58dda049f --- /dev/null +++ b/openspec/specs/jdk-http-transport/spec.md @@ -0,0 +1,103 @@ +# jdk-http-transport Specification + +## Purpose +TBD - created by archiving change jdk-httpclient-transport. Update Purpose after archive. +## 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/settings.gradle b/settings.gradle index 08a2d91cd..8a82ec2b8 100644 --- a/settings.gradle +++ b/settings.gradle @@ -12,6 +12,7 @@ include(':symphony-bdk-cli') // http client include(':symphony-bdk-http:symphony-bdk-http-api') include(':symphony-bdk-http:symphony-bdk-http-jersey') +include(':symphony-bdk-http:symphony-bdk-http-jdk') include(':symphony-bdk-http:symphony-bdk-http-webclient') // template API diff --git a/symphony-bdk-bom/build.gradle b/symphony-bdk-bom/build.gradle index 505fbef49..ab7fd3d71 100644 --- a/symphony-bdk-bom/build.gradle +++ b/symphony-bdk-bom/build.gradle @@ -38,6 +38,7 @@ dependencies { api "org.finos.symphony.bdk:symphony-bdk-extension-api:$project.version" api "org.finos.symphony.bdk:symphony-bdk-http-api:$project.version" api "org.finos.symphony.bdk:symphony-bdk-http-jersey:$project.version" + api "org.finos.symphony.bdk:symphony-bdk-http-jdk:$project.version" api "org.finos.symphony.bdk:symphony-bdk-http-webclient:$project.version" api "org.finos.symphony.bdk:symphony-bdk-core-spring-boot-starter:$project.version" api "org.finos.symphony.bdk:symphony-bdk-app-spring-boot-starter:$project.version" diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/build.gradle b/symphony-bdk-http/symphony-bdk-http-jdk/build.gradle new file mode 100644 index 000000000..8770afbb9 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/build.gradle @@ -0,0 +1,66 @@ +plugins { + id 'bdk.java-library-conventions' + id 'bdk.java-publish-conventions' +} + +description = 'Symphony Java BDK Core Http Jdk' + +// Dedicated source set + Test task for the "two ApiClientBuilderProvider implementations on the classpath" +// SPI scenario (spec: SPI Discoverability / Coexisting with another HTTP implementation). This has to run with +// its own classpath (adding symphony-bdk-http-jersey) that is disjoint from the main `test` task's classpath, +// which must keep symphony-bdk-http-jdk as the *sole* provider for the "resolves automatically" scenario. +sourceSets { + spiConflictTest { + java.srcDir 'src/spiConflictTest/java' + compileClasspath += sourceSets.main.output + configurations.testCompileClasspath + runtimeClasspath += output + compileClasspath + configurations.testRuntimeClasspath + } +} + +configurations { + spiConflictTestImplementation.extendsFrom testImplementation + spiConflictTestRuntimeOnly.extendsFrom testRuntimeOnly +} + +tasks.register('spiConflictTest', Test) { + description = 'Verifies ServiceLookup#lookupSingleService fails fast when both symphony-bdk-http-jdk and ' + + 'symphony-bdk-http-jersey are on the classpath.' + group = 'verification' + testClassesDirs = sourceSets.spiConflictTest.output.classesDirs + classpath = sourceSets.spiConflictTest.runtimeClasspath + useJUnitPlatform() +} + +check.dependsOn spiConflictTest + +jacocoTestCoverageVerification { + violationRules { + rule { + limit { + counter = 'LINE' + value = 'COVEREDRATIO' + minimum = 0.9 + } + element = 'CLASS' + } + } +} + +dependencies { + api project(':symphony-bdk-http:symphony-bdk-http-api') + + implementation 'org.slf4j:slf4j-api' + implementation 'org.apiguardian:apiguardian-api' + implementation 'tools.jackson.core:jackson-databind' + implementation 'org.openapitools:jackson-databind-nullable' + + testImplementation 'org.junit.jupiter:junit-jupiter' + testImplementation 'ch.qos.logback:logback-classic' + testImplementation 'org.mock-server:mockserver-netty' + testImplementation 'org.mockito:mockito-core' + testImplementation 'org.mockito:mockito-junit-jupiter' + testImplementation project(':symphony-bdk-core') + testRuntimeOnly 'org.junit.platform:junit-platform-launcher' + + spiConflictTestImplementation project(':symphony-bdk-http:symphony-bdk-http-jersey') +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientBuilderJdk.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientBuilderJdk.java new file mode 100644 index 000000000..f569ebf64 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientBuilderJdk.java @@ -0,0 +1,315 @@ +package com.symphony.bdk.http.jdk; + +import static com.symphony.bdk.http.api.util.ApiUtils.addDefaultRootCaCertificates; + +import com.symphony.bdk.http.api.ApiClient; +import com.symphony.bdk.http.api.ApiClientBuilder; +import com.symphony.bdk.http.api.auth.Authentication; +import com.symphony.bdk.http.api.util.ApiUtils; + +import org.apiguardian.api.API; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.Authenticator; +import java.net.InetSocketAddress; +import java.net.PasswordAuthentication; +import java.net.ProxySelector; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.time.Duration; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; + +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManagerFactory; + +/** + * Specific implementation of {@link ApiClientBuilder} which creates a new instance of an {@link ApiClientJdk}, + * backed by {@code java.net.http.HttpClient} rather than a third-party HTTP stack. + * + *

Please note that overriding this class is an {@link org.apiguardian.api.API.Status#EXPERIMENTAL} feature + * that we offer to developers for {@link ApiClient} customization. The internal contract of this class (e.g. + * protected methods) is subject to changes in the future.

+ * + *

Two semantic gaps versus {@code ApiClientBuilderJersey2} are documented here rather than silently absorbed: + *

    + *
  • {@code java.net.http.HttpClient} has no distinct read/socket timeout, only a connect timeout and a + * per-request total timeout. {@link #withReadTimeout} is mapped to the per-request total timeout as the + * closest available approximation.
    • + *
  • {@code java.net.http.HttpClient} has no request/response filter chain. {@link #addFilter} accepts a + * narrower request-mutation-only functional type, {@code Function}, + * instead of an arbitrary Jersey filter.
    • + *
+ *

+ */ +@API(status = API.Status.EXPERIMENTAL) +public class ApiClientBuilderJdk implements ApiClientBuilder { + + private static final String TRUSTSTORE_FORMAT = "JKS"; + + protected String basePath; + protected byte[] keyStoreBytes; + protected String keyStorePassword; + protected byte[] trustStoreBytes; + protected String trustStorePassword; + protected Map defaultHeaders; + protected int connectionTimeout; + protected int readTimeout; + protected String proxyHost; + protected int proxyPort; + protected String proxyUser; + protected String proxyPassword; + protected Map authentications; + protected List> filters; + protected String temporaryFolderPath; + + public ApiClientBuilderJdk() { + this.basePath = "https://acme.symphony.com"; + this.keyStoreBytes = null; + this.keyStorePassword = null; + this.trustStoreBytes = null; + this.trustStorePassword = null; + this.defaultHeaders = new HashMap<>(); + this.connectionTimeout = DEFAULT_CONNECT_TIMEOUT; + this.readTimeout = DEFAULT_READ_TIMEOUT; + this.proxyHost = null; + this.proxyPort = -1; + this.proxyUser = null; + this.proxyPassword = null; + this.authentications = new HashMap<>(); + this.filters = new ArrayList<>(); + this.withUserAgent(ApiUtils.getUserAgent()); + } + + /** + * Specific implementation of {@link ApiClientBuilder#build()} which returns an {@link ApiClientJdk} instance. + */ + @Override + public ApiClient build() { + // Force HTTP/1.1: the JDK HttpClient's default (HTTP_2 with an automatic cleartext "h2c" upgrade attempt + // for plain http:// requests) is not supported by the Symphony REST APIs' servers, which would otherwise + // reject the upgrade with a 426. + HttpClient.Builder httpClientBuilder = HttpClient.newBuilder() + .version(HttpClient.Version.HTTP_1_1) + .sslContext(this.createSSLContext()) + .connectTimeout(Duration.ofMillis(this.connectionTimeout)); + + if (this.proxyHost != null) { + this.configureProxy(httpClientBuilder); + } + + HttpClient httpClient = httpClientBuilder.build(); + + final ApiClient apiClient = + new ApiClientJdk(httpClient, this.basePath, this.defaultHeaders, this.temporaryFolderPath, + Duration.ofMillis(this.readTimeout), this.filters); + this.authentications.forEach(apiClient.getAuthentications()::put); + return apiClient; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withBasePath(String basePath) { + this.basePath = basePath; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withUserAgent(String userAgent) { + this.withDefaultHeader("User-Agent", userAgent); + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withKeyStore(byte[] keyStoreBytes, String keyStorePassword) { + this.keyStoreBytes = keyStoreBytes; + this.keyStorePassword = keyStorePassword; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withTrustStore(byte[] trustStoreBytes, String trustStorePassword) { + this.trustStoreBytes = trustStoreBytes; + this.trustStorePassword = trustStorePassword; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withDefaultHeader(String key, String value) { + this.defaultHeaders.put(key, value); + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withTemporaryFolderPath(String temporaryFolderPath) { + this.temporaryFolderPath = temporaryFolderPath; + return this; + } + + /** + * {@inheritDoc} + * + *

Maps directly to {@link HttpClient.Builder#connectTimeout(Duration)}.

+ */ + @Override + public ApiClientBuilder withConnectionTimeout(Integer connectionTimeout) { + this.connectionTimeout = connectionTimeout == null ? DEFAULT_CONNECT_TIMEOUT : connectionTimeout; + return this; + } + + /** + * {@inheritDoc} + * + *

Note: {@code java.net.http.HttpClient} exposes no distinct socket/read timeout, only a per-request + * total timeout ({@link HttpRequest.Builder#timeout(Duration)}). This is the closest available approximation: + * it bounds "the whole request took too long" rather than "no bytes arrived for N ms", which may behave + * differently than {@code ApiClientBuilderJersey2}'s read timeout for large, legitimately slow responses (e.g. + * large file downloads).

+ */ + @Override + public ApiClientBuilder withReadTimeout(Integer readTimeout) { + this.readTimeout = readTimeout == null ? DEFAULT_READ_TIMEOUT : readTimeout; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withProxy(String proxyHost, int proxyPort) { + this.proxyHost = proxyHost; + this.proxyPort = proxyPort; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withProxyCredentials(String proxyUser, String proxyPassword) { + this.proxyUser = proxyUser; + this.proxyPassword = proxyPassword; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public ApiClientBuilder withAuthentication(String name, Authentication authentication) { + this.authentications.put(name, authentication); + return this; + } + + /** + * {@inheritDoc} + * + *

Note: {@code java.net.http.HttpClient} has no request/response filter chain. This implementation + * only accepts {@code Function} instances (request-mutation only, + * applied before send) and throws {@link IllegalArgumentException} for anything else.

+ * + *

Warning: due to type erasure, this method can only verify that {@code filter} is a + * {@link Function}, not that its type parameters are actually + * {@code }. Supplying a {@link Function} with a different + * signature will not fail here, but will throw a {@link ClassCastException} later, when the filter is + * applied to an actual request.

+ */ + @Override + @SuppressWarnings("unchecked") + public ApiClientBuilder addFilter(Object filter) { + if (!(filter instanceof Function)) { + throw new IllegalArgumentException( + "The filter " + filter.getClass().getName() + + " must be an instance of " + Function.class.getName() + + " (specifically Function) to be used with the JDK " + + "HttpClient HTTP client"); + } + this.filters.add((Function) filter); + return this; + } + + @API(status = API.Status.EXPERIMENTAL) + protected void configureProxy(HttpClient.Builder httpClientBuilder) { + httpClientBuilder.proxy(ProxySelector.of(new InetSocketAddress(this.proxyHost, this.proxyPort))); + if (this.proxyUser != null) { + httpClientBuilder.authenticator(new Authenticator() { + @Override + protected PasswordAuthentication getPasswordAuthentication() { + if (getRequestorType() == RequestorType.PROXY) { + return new PasswordAuthentication( + ApiClientBuilderJdk.this.proxyUser, + ApiClientBuilderJdk.this.proxyPassword == null + ? new char[0] + : ApiClientBuilderJdk.this.proxyPassword.toCharArray()); + } + return null; + } + }); + } + } + + @API(status = API.Status.EXPERIMENTAL) + protected SSLContext createSSLContext() { + try { + TrustManagerFactory trustManagerFactory = null; + KeyManagerFactory keyManagerFactory = null; + + if (isNotEmpty(this.trustStoreBytes) && isNotEmpty(this.trustStorePassword)) { + final KeyStore trustStore = KeyStore.getInstance(TRUSTSTORE_FORMAT); + trustStore.load(new ByteArrayInputStream(this.trustStoreBytes), this.trustStorePassword.toCharArray()); + addDefaultRootCaCertificates(trustStore); + ApiUtils.logTrustStore(trustStore); + trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + trustManagerFactory.init(trustStore); + } + + if (isNotEmpty(this.keyStoreBytes) && isNotEmpty(this.keyStorePassword)) { + final KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + keyStore.load(new ByteArrayInputStream(this.keyStoreBytes), this.keyStorePassword.toCharArray()); + keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); + keyManagerFactory.init(keyStore, this.keyStorePassword.toCharArray()); + } + + SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init( + keyManagerFactory == null ? null : keyManagerFactory.getKeyManagers(), + trustManagerFactory == null ? null : trustManagerFactory.getTrustManagers(), + null); + return sslContext; + } catch (IOException | GeneralSecurityException e) { + throw new IllegalStateException(e.getMessage(), e); + } + } + + private static boolean isNotEmpty(byte[] bytes) { + return bytes != null && bytes.length > 0; + } + + private static boolean isNotEmpty(String str) { + return str != null && !str.isEmpty(); + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientBuilderProviderJdk.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientBuilderProviderJdk.java new file mode 100644 index 000000000..466e0e509 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientBuilderProviderJdk.java @@ -0,0 +1,24 @@ +package com.symphony.bdk.http.jdk; + +import com.symphony.bdk.http.api.ApiClientBuilder; +import com.symphony.bdk.http.api.ApiClientBuilderProvider; + +import org.apiguardian.api.API; + +/** + * Provides new {@link ApiClientBuilderJdk} implementation of the {@link ApiClientBuilder} interface. + */ +@API(status = API.Status.EXPERIMENTAL) +public class ApiClientBuilderProviderJdk implements ApiClientBuilderProvider { + + /** + * Creates a new {@link ApiClientBuilder} instance. + * The provided builder instance will build an {@link ApiClientJdk} instance. + * + * @return a new {@link ApiClientBuilder} instance. + */ + @Override + public ApiClientBuilder newInstance() { + return new ApiClientBuilderJdk(); + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientJdk.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientJdk.java new file mode 100644 index 000000000..3a5f98527 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/ApiClientJdk.java @@ -0,0 +1,656 @@ +package com.symphony.bdk.http.jdk; + +import static com.symphony.bdk.http.api.util.ApiUtils.isCollectionOfFiles; + +import com.symphony.bdk.http.api.ApiClient; +import com.symphony.bdk.http.api.ApiClientBodyPart; +import com.symphony.bdk.http.api.ApiException; +import com.symphony.bdk.http.api.ApiResponse; +import com.symphony.bdk.http.api.Pair; +import com.symphony.bdk.http.api.auth.Authentication; +import com.symphony.bdk.http.api.tracing.DistributedTracingContext; +import com.symphony.bdk.http.api.util.TypeReference; + +import org.apiguardian.api.API; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.ConnectException; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.net.URLEncoder; +import java.net.http.HttpClient; +import java.net.http.HttpConnectTimeoutException; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import tools.jackson.core.JacksonException; + +/** + * {@code java.net.http.HttpClient}-based implementation for the {@link ApiClient} interface called by generated + * code. Mirrors {@code com.symphony.bdk.http.jersey2.ApiClientJersey2}'s behavior wherever the shared {@link + * ApiClient} contract requires it. + */ +@API(status = API.Status.EXPERIMENTAL) +public class ApiClientJdk implements ApiClient { + + private static final Logger log = LoggerFactory.getLogger("com.symphony.bdk.requests.outgoing"); + + protected static final String MULTIPART_FORM_DATA = "multipart/form-data"; + protected static final String APPLICATION_FORM_URLENCODED = "application/x-www-form-urlencoded"; + + protected final HttpClient httpClient; + protected final String basePath; + protected final Map defaultHeaderMap; + protected final String tempFolderPath; + protected final Duration readTimeout; + protected final List> filters; + protected final JSON json; + protected Map authentications; + protected List enforcedAuthenticationSchemes; + + public ApiClientJdk( + final HttpClient httpClient, + String basePath, + Map defaultHeaders, + String temporaryFolderPath, + Duration readTimeout, + List> filters + ) { + this.httpClient = httpClient; + this.basePath = basePath; + this.defaultHeaderMap = new HashMap<>(defaultHeaders); + this.tempFolderPath = temporaryFolderPath; + this.readTimeout = readTimeout; + this.filters = filters; + this.json = new JSON(); + this.authentications = new HashMap<>(); + this.enforcedAuthenticationSchemes = new ArrayList<>(); + } + + /** + * {@inheritDoc} + */ + @Override + public ApiResponse invokeAPI( + final String path, + final String method, + final List queryParams, + final Object body, + final Map headerParams, + final Map cookieParams, + final Map formParams, + final String accept, + final String contentType, + final String[] authNames, + final TypeReference returnType + ) throws ApiException { + + this.updateParamsForAuth(authNames, headerParams); + + boolean clearTraceId = false; + if (!DistributedTracingContext.hasTraceId()) { + DistributedTracingContext.setTraceId(); + clearTraceId = true; + } + + try { + HttpRequest request = + this.buildRequest(path, method, queryParams, body, headerParams, cookieParams, formParams, accept, + contentType); + + if (returnType != null && returnType.getType() == File.class) { + return this.invokeForFileDownload(request); + } else { + return this.invokeForBytes(request, returnType); + } + } finally { + if (clearTraceId) { + DistributedTracingContext.clear(); + } + } + } + + private HttpRequest buildRequest( + String path, + String method, + List queryParams, + Object body, + Map headerParams, + Map cookieParams, + Map formParams, + String accept, + String contentType + ) throws ApiException { + + HttpRequest.Builder requestBuilder = HttpRequest.newBuilder().uri(this.buildUri(path, queryParams)); + + if (accept != null && !accept.isEmpty()) { + requestBuilder.header("Accept", accept); + } + + requestBuilder.header(DistributedTracingContext.TRACE_ID, DistributedTracingContext.getTraceId()); + + if (headerParams != null) { + for (Map.Entry entry : headerParams.entrySet()) { + if (entry.getValue() != null) { + requestBuilder.header(entry.getKey(), entry.getValue()); + } + } + } + + if (cookieParams != null && !cookieParams.isEmpty()) { + String cookieHeader = cookieParams.entrySet().stream() + .filter(entry -> entry.getValue() != null) + .map(entry -> entry.getKey() + "=" + entry.getValue()) + .collect(Collectors.joining("; ")); + if (!cookieHeader.isEmpty()) { + requestBuilder.header("Cookie", cookieHeader); + } + } + + for (Map.Entry entry : this.defaultHeaderMap.entrySet()) { + if (headerParams == null || !headerParams.containsKey(entry.getKey())) { + if (entry.getValue() != null) { + requestBuilder.header(entry.getKey(), entry.getValue()); + } + } + } + + Body serializedBody = this.serialize(body, formParams, contentType); + if (serializedBody.contentType != null && !serializedBody.contentType.isEmpty()) { + requestBuilder.header("Content-Type", serializedBody.contentType); + } + requestBuilder.method(method, serializedBody.publisher); + + if (this.readTimeout != null) { + requestBuilder.timeout(this.readTimeout); + } + + for (Function filter : this.filters) { + requestBuilder = filter.apply(requestBuilder); + } + + return requestBuilder.build(); + } + + private ApiResponse invokeForBytes(HttpRequest request, TypeReference returnType) throws ApiException { + HttpResponse response = this.send(request, HttpResponse.BodyHandlers.ofByteArray()); + + int statusCode = response.statusCode(); + Map> responseHeaders = response.headers().map(); + + if (statusCode == 204) { + return new ApiResponse<>(statusCode, responseHeaders); + } else if (statusCode / 100 == 2) { + if (returnType == null) { + return new ApiResponse<>(statusCode, responseHeaders); + } + return new ApiResponse<>(statusCode, responseHeaders, this.deserialize(response.body(), returnType)); + } else { + String message = new String(response.body(), StandardCharsets.UTF_8); + throw new ApiException(statusCode, message, responseHeaders, message); + } + } + + @SuppressWarnings("unchecked") + private ApiResponse invokeForFileDownload(HttpRequest request) throws ApiException { + HttpResponse response = this.send(request, responseInfo -> { + try { + return HttpResponse.BodySubscribers.ofFile(this.prepareDownloadFile(responseInfo.headers()).toPath()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + + int statusCode = response.statusCode(); + Map> responseHeaders = response.headers().map(); + + if (statusCode == 204) { + return new ApiResponse<>(statusCode, responseHeaders); + } else if (statusCode / 100 == 2) { + return new ApiResponse<>(statusCode, responseHeaders, (T) response.body().toFile()); + } else { + String message; + try { + message = new String(Files.readAllBytes(response.body()), StandardCharsets.UTF_8); + } catch (IOException e) { + message = "error"; + } + throw new ApiException(statusCode, message, responseHeaders, message); + } + } + + private HttpResponse send(HttpRequest request, HttpResponse.BodyHandler bodyHandler) + throws ApiException { + long startTime = System.currentTimeMillis(); + try { + HttpResponse response = this.httpClient.send(request, bodyHandler); + if (log.isDebugEnabled()) { + long elapsed = System.currentTimeMillis() - startTime; + log.debug("status={}, url={}, time={}", response.statusCode(), request.uri(), elapsed); + } + return response; + } catch (IOException e) { + throw this.translateAndWrap(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(e); + } + } + + /** + * Translates transport-level {@link IOException}s thrown by {@link HttpClient#send} into the root-cause types + * {@code RetryWithRecoveryBuilder#isNetworkIssueOrMinorError} already recognizes ({@link ConnectException}, + * {@link SocketTimeoutException}), then wraps the result as an {@link UncheckedIOException} since {@link + * ApiClient#invokeAPI} does not declare {@link IOException} in its {@code throws} clause. + * + *

Note: {@link HttpConnectTimeoutException} and {@link HttpTimeoutException} both extend {@link + * java.io.IOException} directly (not {@link ConnectException}/{@link SocketTimeoutException}), so an explicit + * translation is required here rather than relying on inheritance.

+ */ + UncheckedIOException translateAndWrap(IOException e) { + if (e instanceof HttpConnectTimeoutException) { + ConnectException translated = new ConnectException(e.getMessage()); + translated.initCause(e); + return new UncheckedIOException(translated); + } else if (e instanceof HttpTimeoutException) { + SocketTimeoutException translated = new SocketTimeoutException(e.getMessage()); + translated.initCause(e); + return new UncheckedIOException(translated); + } else { + return new UncheckedIOException(e); + } + } + + @Override + public String getBasePath() { + return this.basePath; + } + + /** + * {@inheritDoc} + */ + @Override + public String parameterToString(Object param) { + if (param == null) { + return ""; + } else if (param instanceof Collection) { + StringBuilder b = new StringBuilder(); + for (Object o : (Collection) param) { + if (b.length() > 0) { + b.append(','); + } + b.append(o); + } + return b.toString(); + } else { + return String.valueOf(param); + } + } + + /** + * {@inheritDoc} + */ + @Override + public List parameterToPairs(String collectionFormat, String name, Object value) { + List params = new ArrayList<>(); + + if (name == null || name.isEmpty() || value == null) { + return params; + } + + Collection valueCollection; + if (value instanceof Collection) { + valueCollection = (Collection) value; + } else { + params.add(new Pair(name, parameterToString(value))); + return params; + } + + if (valueCollection.isEmpty()) { + return params; + } + + String format = (collectionFormat == null || collectionFormat.isEmpty() ? "csv" : collectionFormat); + + if ("multi".equals(format)) { + for (Object item : valueCollection) { + params.add(new Pair(name, parameterToString(item))); + } + return params; + } + + String delimiter; + switch (format) { + case "ssv": + delimiter = " "; + break; + case "tsv": + delimiter = "\t"; + break; + case "pipes": + delimiter = "|"; + break; + default: + delimiter = ","; + break; + } + + StringBuilder sb = new StringBuilder(); + for (Object item : valueCollection) { + sb.append(delimiter); + sb.append(parameterToString(item)); + } + + params.add(new Pair(name, sb.substring(1))); + + return params; + } + + /** + * {@inheritDoc} + */ + @Override + public String selectHeaderAccept(String[] accepts) { + if (accepts.length == 0) { + return null; + } + for (String accept : accepts) { + if (isJsonMime(accept)) { + return accept; + } + } + return String.join(",", accepts); + } + + /** + * {@inheritDoc} + */ + @Override + public String selectHeaderContentType(String[] contentTypes) { + if (contentTypes.length == 0) { + return "application/json"; + } + for (String contentType : contentTypes) { + if (isJsonMime(contentType)) { + return contentType; + } + } + return contentTypes[0]; + } + + /** + * {@inheritDoc} + */ + @Override + public String escapeString(String str) { + return URLEncoder.encode(str, StandardCharsets.UTF_8).replace("+", "%20"); + } + + /** + * {@inheritDoc} + */ + @Override + public Map getAuthentications() { + return this.authentications; + } + + /** + * {@inheritDoc} + */ + @Override + public void addEnforcedAuthenticationScheme(String name) { + this.enforcedAuthenticationSchemes.add(name); + } + + /** + * Check if the given MIME is a JSON MIME. + * + * @param mime MIME + * @return True if the MIME type is JSON + */ + protected boolean isJsonMime(String mime) { + String jsonMime = "(?i)^(application/json|[^;/ \t]+/[^;/ \t]+[+]json)[ \t]*(;.*)?$"; + return mime != null && (mime.matches(jsonMime) || mime.equals("*/*")); + } + + private URI buildUri(String path, List queryParams) { + String normalizedBasePath = + this.basePath.endsWith("/") ? this.basePath.substring(0, this.basePath.length() - 1) : this.basePath; + String normalizedPath = path.startsWith("/") ? path : "/" + path; + StringBuilder urlBuilder = new StringBuilder(normalizedBasePath).append(normalizedPath); + if (queryParams != null && !queryParams.isEmpty()) { + String query = queryParams.stream() + .filter(param -> param.getValue() != null) + .map(param -> param.getName() + "=" + this.escapeString(param.getValue())) + .collect(Collectors.joining("&")); + if (!query.isEmpty()) { + urlBuilder.append(path.contains("?") ? "&" : "?").append(query); + } + } + return URI.create(urlBuilder.toString()); + } + + private Body serialize(Object body, Map formParams, String contentType) throws ApiException { + if (contentType != null && contentType.startsWith(MULTIPART_FORM_DATA)) { + String boundary = "----BdkJdkBoundary" + UUID.randomUUID(); + return new Body("multipart/form-data; boundary=" + boundary, this.buildMultipartBody(formParams, boundary)); + } else if (contentType != null && contentType.startsWith(APPLICATION_FORM_URLENCODED)) { + return new Body(contentType, this.buildFormUrlEncodedBody(formParams)); + } else if (body != null) { + return new Body(contentType, HttpRequest.BodyPublishers.ofByteArray(this.serializeToJson(body))); + } else { + return new Body(contentType, HttpRequest.BodyPublishers.noBody()); + } + } + + private byte[] serializeToJson(Object body) throws ApiException { + try { + if (body instanceof String) { + return ((String) body).getBytes(StandardCharsets.UTF_8); + } + return this.json.getMapper().writeValueAsBytes(body); + } catch (JacksonException e) { + throw new ApiException("Unable to serialize request body", e); + } + } + + @SuppressWarnings("unchecked") + private T deserialize(byte[] responseBody, TypeReference returnType) throws ApiException { + if (returnType == null) { + return null; + } + if (returnType.getType() == byte[].class) { + return (T) responseBody; + } + try { + return this.json.getMapper() + .readValue(responseBody, this.json.getMapper().getTypeFactory().constructType(returnType.getType())); + } catch (JacksonException e) { + throw new ApiException("Unable to deserialize response body", e); + } + } + + private HttpRequest.BodyPublisher buildFormUrlEncodedBody(Map formParams) { + if (formParams == null || formParams.isEmpty()) { + return HttpRequest.BodyPublishers.noBody(); + } + String encoded = formParams.entrySet().stream() + .map(entry -> this.escapeString(entry.getKey()) + "=" + this.escapeString( + this.parameterToString(entry.getValue()))) + .collect(Collectors.joining("&")); + return HttpRequest.BodyPublishers.ofString(encoded, StandardCharsets.UTF_8); + } + + private static final String CRLF = "\r\n"; + + private HttpRequest.BodyPublisher buildMultipartBody(Map formParams, String boundary) + throws ApiException { + List publishers = new ArrayList<>(); + + if (formParams != null) { + for (Map.Entry param : formParams.entrySet()) { + Object value = param.getValue(); + if (value instanceof File) { + this.addFilePart(publishers, boundary, param.getKey(), (File) value); + } else if (isCollectionOfFiles(value)) { + for (Object file : (Collection) value) { + this.addFilePart(publishers, boundary, param.getKey(), (File) file); + } + } else if (value instanceof ApiClientBodyPart[]) { + for (ApiClientBodyPart part : (ApiClientBodyPart[]) value) { + this.addStreamPart(publishers, boundary, param.getKey(), part); + } + } else if (value instanceof ApiClientBodyPart) { + this.addStreamPart(publishers, boundary, param.getKey(), (ApiClientBodyPart) value); + } else { + this.addFieldPart(publishers, boundary, param.getKey(), this.parameterToString(value)); + } + } + } + + publishers.add(HttpRequest.BodyPublishers.ofByteArray( + ("--" + boundary + "--" + CRLF).getBytes(StandardCharsets.UTF_8))); + + return HttpRequest.BodyPublishers.concat(publishers.toArray(new HttpRequest.BodyPublisher[0])); + } + + private void addFilePart(List publishers, String boundary, String key, File file) + throws ApiException { + String header = "--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"" + key + "\"; filename=\"" + file.getName() + "\"" + CRLF + + "Content-Type: application/octet-stream" + CRLF + CRLF; + publishers.add(HttpRequest.BodyPublishers.ofByteArray(header.getBytes(StandardCharsets.UTF_8))); + try { + publishers.add(HttpRequest.BodyPublishers.ofFile(file.toPath())); + } catch (java.io.FileNotFoundException e) { + throw new ApiException("Unable to read file for multipart upload: " + file, e); + } catch (InvalidPathException e) { + throw new ApiException("Invalid file path for multipart upload: " + file, e); + } + publishers.add(HttpRequest.BodyPublishers.ofByteArray(CRLF.getBytes(StandardCharsets.UTF_8))); + } + + private void addStreamPart(List publishers, String boundary, String key, + ApiClientBodyPart part) { + String header = "--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"" + key + "\"; filename=\"" + part.getFilename() + "\"" + CRLF + + "Content-Type: application/octet-stream" + CRLF + CRLF; + publishers.add(HttpRequest.BodyPublishers.ofByteArray(header.getBytes(StandardCharsets.UTF_8))); + publishers.add(HttpRequest.BodyPublishers.ofInputStream(part::getContent)); + publishers.add(HttpRequest.BodyPublishers.ofByteArray(CRLF.getBytes(StandardCharsets.UTF_8))); + } + + private void addFieldPart(List publishers, String boundary, String key, String value) { + String part = "--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"" + key + "\"" + CRLF + CRLF + + value + CRLF; + publishers.add(HttpRequest.BodyPublishers.ofByteArray(part.getBytes(StandardCharsets.UTF_8))); + } + + /** + * Prepares the target {@link File} onto which a file-download response will be streamed, deriving the file + * name from the {@code Content-Disposition} header when present. + * + *

Note: like {@code ApiClientJersey2#prepareDownloadFile}, the returned file name is a unique temp-file + * name derived from (not identical to) the {@code Content-Disposition} filename, via {@link + * File#createTempFile}.

+ */ + protected File prepareDownloadFile(java.net.http.HttpHeaders headers) throws IOException { + String filename = null; + String contentDisposition = headers.firstValue("Content-Disposition").orElse(null); + if (contentDisposition != null && !contentDisposition.isEmpty()) { + Pattern pattern = Pattern.compile("filename=['\"]?([^'\"\\s]+)['\"]?"); + Matcher matcher = pattern.matcher(contentDisposition); + if (matcher.find()) { + filename = matcher.group(1); + } + } + + String prefix; + String suffix; + if (filename == null) { + prefix = "download-"; + suffix = ""; + } else { + int pos = filename.lastIndexOf('.'); + if (pos == -1) { + prefix = filename + "-"; + suffix = null; + } else { + prefix = filename.substring(0, pos) + "-"; + suffix = filename.substring(pos); + } + if (prefix.length() < 3) { + prefix = "download-"; + } + } + + if (this.tempFolderPath == null) { + return File.createTempFile(prefix, suffix); + } else { + return File.createTempFile(prefix, suffix, new File(this.tempFolderPath)); + } + } + + /** + * Update query and header parameters based on authentication settings. + * + * @param authNames The authentications to apply + */ + protected void updateParamsForAuth(String[] authNames, Map headerParams) throws ApiException { + if (authNames == null && this.enforcedAuthenticationSchemes.isEmpty()) { + return; + } + + authNames = withEnforcedSecurityScheme(authNames); + + for (String authName : authNames) { + Authentication auth = this.authentications.get(authName); + if (auth == null) { + throw new RuntimeException("Authentication undefined: " + authName); + } + auth.apply(headerParams); + } + } + + private String[] withEnforcedSecurityScheme(String[] authNames) { + if (authNames == null) { + authNames = new String[0]; + } + return Stream.concat(this.enforcedAuthenticationSchemes.stream(), Arrays.stream(authNames)) + .toArray(String[]::new); + } + + private static final class Body { + private final String contentType; + private final HttpRequest.BodyPublisher publisher; + + private Body(String contentType, HttpRequest.BodyPublisher publisher) { + this.contentType = contentType; + this.publisher = publisher; + } + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/JSON.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/JSON.java new file mode 100644 index 000000000..a2c4f9508 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/JSON.java @@ -0,0 +1,57 @@ +package com.symphony.bdk.http.jdk; + +import com.fasterxml.jackson.annotation.JsonInclude; +import org.apiguardian.api.API; +import org.openapitools.jackson.nullable.JsonNullableJackson3Module; +import tools.jackson.databind.DeserializationFeature; +import tools.jackson.databind.ObjectMapper; +import tools.jackson.databind.cfg.DateTimeFeature; +import tools.jackson.databind.cfg.EnumFeature; +import tools.jackson.databind.json.JsonMapper; + +import java.text.DateFormat; + +/** + * Configures the {@link ObjectMapper} used by {@link ApiClientJdk} for request/response (de)serialization. + * + *

Duplicated from {@code com.symphony.bdk.http.jersey2.JSON} rather than shared, per design decision D5 in + * the {@code jdk-httpclient-transport} OpenSpec change.

+ */ +@API(status = API.Status.INTERNAL) +public class JSON { + + private ObjectMapper mapper; + + public JSON() { + this.mapper = buildMapper(new RFC3339DateFormat()); + } + + /** + * Set the date format for JSON (de)serialization with Date properties. + * + * @param dateFormat Date format + */ + public void setDateFormat(DateFormat dateFormat) { + this.mapper = buildMapper(dateFormat); + } + + /** + * @return the configured {@link ObjectMapper} instance. + */ + public ObjectMapper getMapper() { + return this.mapper; + } + + private static ObjectMapper buildMapper(DateFormat dateFormat) { + return JsonMapper.builder() + .changeDefaultPropertyInclusion(inclusion -> inclusion.withValueInclusion(JsonInclude.Include.NON_NULL)) + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) + .configure(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE, false) + .disable(DateTimeFeature.WRITE_DATES_AS_TIMESTAMPS) + .enable(EnumFeature.WRITE_ENUMS_USING_TO_STRING) + .enable(EnumFeature.READ_ENUMS_USING_TO_STRING) + .defaultDateFormat(dateFormat) + .addModule(new JsonNullableJackson3Module()) + .build(); + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/RFC3339DateFormat.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/RFC3339DateFormat.java new file mode 100644 index 000000000..0298d623e --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/java/com/symphony/bdk/http/jdk/RFC3339DateFormat.java @@ -0,0 +1,56 @@ +package com.symphony.bdk.http.jdk; + +import org.apiguardian.api.API; + +import java.text.DateFormat; +import java.text.FieldPosition; +import java.text.NumberFormat; +import java.text.ParsePosition; +import java.time.OffsetDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.Calendar; +import java.util.Date; +import java.util.GregorianCalendar; +import java.util.Locale; +import java.util.TimeZone; + +/** + * Formats dates as RFC 3339 (ISO 8601 with a fixed UTC offset), always including milliseconds. + * + *

Duplicated from {@code com.symphony.bdk.http.jersey2.RFC3339DateFormat} rather than shared, per design + * decision D5 in the {@code jdk-httpclient-transport} OpenSpec change: two implementations already existed + * without a shared abstraction, and this is deliberately kept that way until a fourth implementation appears.

+ */ +@API(status = API.Status.INTERNAL) +public class RFC3339DateFormat extends DateFormat { + + private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"); + + public RFC3339DateFormat() { + final Calendar calendar = new GregorianCalendar(TimeZone.getTimeZone("UTC"), Locale.ROOT); + calendar.setLenient(false); + this.calendar = calendar; + final NumberFormat numberFormat = NumberFormat.getIntegerInstance(Locale.ROOT); + numberFormat.setGroupingUsed(false); + this.numberFormat = numberFormat; + } + + @Override + public StringBuffer format(Date date, StringBuffer toAppendTo, FieldPosition fieldPosition) { + toAppendTo.append(FORMATTER.format(date.toInstant().atOffset(ZoneOffset.UTC))); + return toAppendTo; + } + + @Override + public Date parse(String source, ParsePosition pos) { + try { + final OffsetDateTime parsed = OffsetDateTime.parse(source, DateTimeFormatter.ISO_OFFSET_DATE_TIME); + pos.setIndex(source.length()); + return Date.from(parsed.toInstant()); + } catch (java.time.format.DateTimeParseException e) { + pos.setErrorIndex(pos.getIndex()); + return null; + } + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/main/resources/META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/resources/META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider new file mode 100644 index 000000000..0fc42c694 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/main/resources/META-INF/services/com.symphony.bdk.http.api.ApiClientBuilderProvider @@ -0,0 +1 @@ +com.symphony.bdk.http.jdk.ApiClientBuilderProviderJdk diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/spiConflictTest/java/com/symphony/bdk/http/jdk/ServiceLookupConflictTest.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/spiConflictTest/java/com/symphony/bdk/http/jdk/ServiceLookupConflictTest.java new file mode 100644 index 000000000..4882511da --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/spiConflictTest/java/com/symphony/bdk/http/jdk/ServiceLookupConflictTest.java @@ -0,0 +1,23 @@ +package com.symphony.bdk.http.jdk; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.symphony.bdk.core.util.ServiceLookup; +import com.symphony.bdk.http.api.ApiClientBuilderProvider; + +import org.junit.jupiter.api.Test; + +/** + * Runs in a dedicated {@code spiConflictTest} source set/Test task (see {@code build.gradle}) whose classpath + * has both {@code symphony-bdk-http-jdk} and {@code symphony-bdk-http-jersey} present, so that {@link + * ServiceLookup#lookupSingleService} sees two {@link ApiClientBuilderProvider} implementations via {@link + * java.util.ServiceLoader}. This must stay out of the main {@code test} task, whose classpath is relied upon by + * {@link ApiClientBuilderProviderJdkTest} to have {@code symphony-bdk-http-jdk} as the sole implementation. + */ +class ServiceLookupConflictTest { + + @Test + void lookupSingleService_throwsIllegalStateException_whenTwoProvidersOnClasspath() { + assertThrows(IllegalStateException.class, () -> ServiceLookup.lookupSingleService(ApiClientBuilderProvider.class)); + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientBuilderJdkTest.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientBuilderJdkTest.java new file mode 100644 index 000000000..9c93cdc90 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientBuilderJdkTest.java @@ -0,0 +1,201 @@ +package com.symphony.bdk.http.jdk; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.symphony.bdk.http.api.ApiClient; +import com.symphony.bdk.http.api.ApiClientBuilder; +import com.symphony.bdk.http.api.ApiException; +import com.symphony.bdk.http.api.ApiResponse; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockserver.configuration.ConfigurationProperties; +import org.mockserver.integration.ClientAndServer; +import org.mockserver.logging.MockServerLogger; +import org.mockserver.model.HttpRequest; +import org.mockserver.model.HttpResponse; +import org.mockserver.socket.tls.KeyStoreFactory; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.net.Authenticator; +import java.net.InetAddress; +import java.net.PasswordAuthentication; +import java.net.http.HttpClient; +import java.net.http.HttpRequest.Builder; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateException; +import java.util.Base64; +import java.util.Collections; +import java.util.concurrent.atomic.AtomicBoolean; + +class ApiClientBuilderJdkTest { + + private static final String KEY_STORE_PWD = "changeit"; + + private ClientAndServer mockServer; + + @BeforeEach + void setUp() { + this.mockServer = ClientAndServer.startClientAndServer(); + } + + @AfterEach + void tearDown() { + this.mockServer.stop(); + } + + @Test + void sslContextIsUsed() + throws ApiException, CertificateException, KeyStoreException, IOException, NoSuchAlgorithmException { + // ConfigurationProperties is process-wide global state (not scoped to a ClientAndServer instance), so it + // is scoped to this test only via try/finally, or other tests/classes in the same JVM would unexpectedly + // require mutual TLS too. + ConfigurationProperties.tlsMutualAuthenticationRequired(true); + try { + ByteArrayOutputStream keyStoreData = this.getMockServerKeyStore(); + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("https://localhost:" + this.mockServer.getPort()) + .withKeyStore(keyStoreData.toByteArray(), "changeit") + .withTrustStore(keyStoreData.toByteArray(), "changeit") + .build(); + + this.mockServer.withSecure(true) + .when(HttpRequest.request().withMethod("GET").withPath("/test")) + .respond(HttpResponse.response().withStatusCode(200)); + + ApiResponse response = + client.invokeAPI("/test", "GET", Collections.emptyList(), null, Collections.emptyMap(), + Collections.emptyMap(), null, "application/json", "", null, null); + + assertEquals(200, response.getStatusCode()); + } finally { + ConfigurationProperties.tlsMutualAuthenticationRequired(false); + } + } + + @Test + void addFilter_appliesFilterToOutgoingRequest() { + AtomicBoolean filterCalled = new AtomicBoolean(false); + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .addFilter((java.util.function.Function) builder -> { + filterCalled.set(true); + return builder; + }) + .build(); + + this.mockServer + .when(HttpRequest.request().withMethod("GET").withPath("/test")) + .respond(HttpResponse.response().withStatusCode(200)); + + try { + client.invokeAPI("/test", "GET", Collections.emptyList(), null, Collections.emptyMap(), + Collections.emptyMap(), null, "application/json", "", null, null); + } catch (ApiException e) { + // the filter's invocation is what's under test here; a downstream failure is irrelevant + } + + assertTrue(filterCalled.get()); + } + + @Test + void withAuthentication_registersAuthenticationOnBuiltClient() { + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withAuthentication("myAuth", headers -> headers.put("Authorization", "Bearer token")) + .build(); + + assertTrue(client.getAuthentications().containsKey("myAuth")); + } + + @Test + void addFilter_throwsIllegalArgumentException_forNonFunctionFilter() { + ApiClientBuilder builder = new ApiClientBuilderJdk(); + + assertThrows(IllegalArgumentException.class, () -> builder.addFilter("not a function")); + } + + @Test + void proxyIsUsed_routesRequestThroughConfiguredProxy() throws IOException, ApiException { + try (FakeHttpProxy proxy = new FakeHttpProxy(false)) { + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://symphony-fake-target.invalid") + .withProxy("localhost", proxy.getPort()) + .build(); + + ApiResponse response = + client.invokeAPI("/test", "GET", Collections.emptyList(), null, Collections.emptyMap(), + Collections.emptyMap(), null, "application/json", "", null, null); + + assertEquals(200, response.getStatusCode()); + assertTrue(proxy.getLastRequestLine().contains("http://symphony-fake-target.invalid"), + "expected the request line to use the absolute-URI form sent to a forward proxy, got: " + + proxy.getLastRequestLine()); + } + } + + @Test + void proxyCredentials_answer407ChallengeSuccessfully() throws IOException, ApiException { + try (FakeHttpProxy proxy = new FakeHttpProxy(true)) { + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://symphony-fake-target.invalid") + .withProxy("localhost", proxy.getPort()) + .withProxyCredentials("proxyUser", "proxyPassword") + .build(); + + ApiResponse response = + client.invokeAPI("/test", "GET", Collections.emptyList(), null, Collections.emptyMap(), + Collections.emptyMap(), null, "application/json", "", null, null); + + assertEquals(200, response.getStatusCode()); + assertTrue(proxy.getRequestCount() >= 2, "expected a 407 challenge followed by an authenticated retry"); + String expectedAuthHeader = + "Basic " + Base64.getEncoder().encodeToString("proxyUser:proxyPassword".getBytes()); + assertEquals(expectedAuthHeader, proxy.getLastProxyAuthorizationHeader()); + } + } + + /** + * Directly exercises the {@code Authenticator} anonymous inner class {@link ApiClientBuilderJdk#configureProxy} + * registers, using {@link Authenticator#requestPasswordAuthentication} (the public dispatch entry point the + * JDK's own {@code HttpClient} internals use) to simulate both a proxy challenge and a non-proxy (server) + * challenge, without needing a real 401/407 round-trip. + */ + @Test + void proxyAuthenticator_answersProxyChallengeAndIgnoresServerChallenge() throws IOException { + ApiClientJdk client = (ApiClientJdk) new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withProxy("localhost", 12345) + .withProxyCredentials("proxyUser", null) + .build(); + + HttpClient httpClient = client.httpClient; + Authenticator authenticator = httpClient.authenticator().orElseThrow(); + + PasswordAuthentication proxyAuth = Authenticator.requestPasswordAuthentication( + authenticator, "localhost", InetAddress.getLoopbackAddress(), 12345, "http", "prompt", "basic", null, + Authenticator.RequestorType.PROXY); + assertEquals("proxyUser", proxyAuth.getUserName()); + assertEquals(0, proxyAuth.getPassword().length); + + PasswordAuthentication serverAuth = Authenticator.requestPasswordAuthentication( + authenticator, "localhost", InetAddress.getLoopbackAddress(), 80, "http", "prompt", "basic", null, + Authenticator.RequestorType.SERVER); + assertNull(serverAuth); + } + + private ByteArrayOutputStream getMockServerKeyStore() + throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { + KeyStore mockServerKeyStore = new KeyStoreFactory(new MockServerLogger()).loadOrCreateKeyStore(); + ByteArrayOutputStream keyStoreData = new ByteArrayOutputStream(); + mockServerKeyStore.store(keyStoreData, KEY_STORE_PWD.toCharArray()); + return keyStoreData; + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientBuilderProviderJdkTest.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientBuilderProviderJdkTest.java new file mode 100644 index 000000000..2233d46b0 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientBuilderProviderJdkTest.java @@ -0,0 +1,32 @@ +package com.symphony.bdk.http.jdk; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import com.symphony.bdk.core.util.ServiceLookup; +import com.symphony.bdk.http.api.ApiClientBuilder; +import com.symphony.bdk.http.api.ApiClientBuilderProvider; + +import org.junit.jupiter.api.Test; + +class ApiClientBuilderProviderJdkTest { + + @Test + void newInstance_returnsApiClientBuilderJdk() { + ApiClientBuilder builder = new ApiClientBuilderProviderJdk().newInstance(); + + assertInstanceOf(ApiClientBuilderJdk.class, builder); + } + + /** + * With only {@code symphony-bdk-http-jdk} on this module's own test classpath, {@link + * ServiceLookup#lookupSingleService} resolves {@link ApiClientBuilderProviderJdk} via {@link + * java.util.ServiceLoader} without any explicit configuration (spec: SPI Discoverability / Sole HTTP + * implementation on the runtime classpath). + */ + @Test + void lookupSingleService_resolvesApiClientBuilderProviderJdk_whenSoleImplementationOnClasspath() { + ApiClientBuilderProvider provider = ServiceLookup.lookupSingleService(ApiClientBuilderProvider.class); + + assertInstanceOf(ApiClientBuilderProviderJdk.class, provider); + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientJdkTest.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientJdkTest.java new file mode 100644 index 000000000..02ee3b266 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientJdkTest.java @@ -0,0 +1,589 @@ +package com.symphony.bdk.http.jdk; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; +import com.symphony.bdk.http.api.ApiClient; +import com.symphony.bdk.http.api.ApiClientBodyPart; +import com.symphony.bdk.http.api.ApiException; +import com.symphony.bdk.http.api.ApiResponse; +import com.symphony.bdk.http.api.tracing.DistributedTracingContext; +import com.symphony.bdk.http.api.util.TypeReference; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockserver.integration.ClientAndServer; +import org.mockserver.matchers.Times; +import org.mockserver.model.Delay; +import org.mockserver.model.HttpRequest; +import org.mockserver.model.HttpResponse; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.net.ConnectException; +import java.net.SocketTimeoutException; +import java.net.http.HttpClient; +import java.net.http.HttpConnectTimeoutException; +import java.net.http.HttpTimeoutException; +import java.nio.file.Path; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.TimeUnit; + +class ApiClientJdkTest { + + private ClientAndServer mockServer; + + @BeforeEach + void setUp() { + this.mockServer = ClientAndServer.startClientAndServer(); + } + + @AfterEach + void tearDown() { + this.mockServer.stop(); + } + + private ApiClient buildClient() { + return new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .build(); + } + + private ApiResponse invokeGet(ApiClient client, String path) throws ApiException { + return client.invokeAPI(path, "GET", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + new HashMap<>(), "application/json", "application/json", null, null); + } + + // -------------------------------------------------------------------------------------------- + // Distributed tracing header propagation + // -------------------------------------------------------------------------------------------- + + @Test + void traceIdIsGeneratedAndClearedWhenAbsentBeforeTheCall() throws ApiException { + DistributedTracingContext.clear(); + this.mockServer.when(HttpRequest.request().withPath("/test")).respond(HttpResponse.response().withStatusCode(200)); + + this.invokeGet(this.buildClient(), "/test"); + + assertTrue(DistributedTracingContext.getTraceId().isEmpty()); + } + + @Test + void traceIdIsPreservedAndNotClearedWhenAlreadySetBeforeTheCall() throws ApiException { + String traceId = UUID.randomUUID().toString(); + DistributedTracingContext.setTraceId(traceId); + this.mockServer.when(HttpRequest.request().withPath("/test")).respond(HttpResponse.response().withStatusCode(200)); + + this.invokeGet(this.buildClient(), "/test"); + + assertEquals(traceId, DistributedTracingContext.getTraceId()); + DistributedTracingContext.clear(); + } + + @Test + void traceIdHeaderIsSentOnOutgoingRequest() throws ApiException { + String traceId = UUID.randomUUID().toString(); + DistributedTracingContext.setTraceId(traceId); + this.mockServer.when(HttpRequest.request().withPath("/test")).respond(HttpResponse.response().withStatusCode(200)); + + this.invokeGet(this.buildClient(), "/test"); + + HttpRequest[] recorded = this.mockServer.retrieveRecordedRequests(HttpRequest.request().withPath("/test")); + assertEquals(1, recorded.length); + assertEquals(traceId, recorded[0].getFirstHeader(DistributedTracingContext.TRACE_ID)); + DistributedTracingContext.clear(); + } + + // -------------------------------------------------------------------------------------------- + // Content encoding: query params, cookies, JSON body, form-urlencoded, multipart + // -------------------------------------------------------------------------------------------- + + @Test + void queryParamsAreAppendedAndEscaped() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/query")).respond(HttpResponse.response().withStatusCode(200)); + + List queryParams = Arrays.asList(new com.symphony.bdk.http.api.Pair("q", "a b")); + + this.buildClient().invokeAPI("/query", "GET", queryParams, null, new HashMap<>(), new HashMap<>(), + new HashMap<>(), "application/json", "application/json", null, null); + + HttpRequest[] recorded = this.mockServer.retrieveRecordedRequests(HttpRequest.request().withPath("/query")); + assertEquals(1, recorded.length); + assertEquals("a b", recorded[0].getFirstQueryStringParameter("q")); + } + + @Test + void cookieParamsAreSentAsCookieHeader() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/cookie")).respond(HttpResponse.response().withStatusCode(200)); + + Map cookieParams = new HashMap<>(); + cookieParams.put("session", "abc123"); + + this.buildClient().invokeAPI("/cookie", "GET", Collections.emptyList(), null, new HashMap<>(), cookieParams, + new HashMap<>(), "application/json", "application/json", null, null); + + HttpRequest[] recorded = this.mockServer.retrieveRecordedRequests(HttpRequest.request().withPath("/cookie")); + assertEquals(1, recorded.length); + assertEquals("session=abc123", recorded[0].getFirstHeader("Cookie")); + } + + @Test + void jsonBodyIsSerializedUsingJacksonMapper() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/body")).respond(HttpResponse.response().withStatusCode(200)); + + Model model = new Model(); + model.name = "foo"; + + this.buildClient().invokeAPI("/body", "POST", Collections.emptyList(), model, new HashMap<>(), new HashMap<>(), + new HashMap<>(), "application/json", "application/json", null, null); + + String body = this.retrieveSingleRequestBody("/body"); + assertEquals("foo", new JSON().getMapper().readTree(body).get("name").asString()); + } + + @Test + void stringBodyIsSentAsRawUtf8Bytes() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/raw")).respond(HttpResponse.response().withStatusCode(200)); + + this.buildClient().invokeAPI("/raw", "POST", Collections.emptyList(), "raw-string-body", new HashMap<>(), + new HashMap<>(), new HashMap<>(), "application/json", "text/plain", null, null); + + assertEquals("raw-string-body", this.retrieveSingleRequestBody("/raw")); + } + + @Test + void unserializableBodyThrowsApiException() { + this.mockServer.when(HttpRequest.request().withPath("/bad-body")).respond(HttpResponse.response().withStatusCode(200)); + + Object unserializable = new Object() { + public String getName() { + throw new RuntimeException("boom"); + } + }; + + assertThrows(ApiException.class, () -> this.buildClient().invokeAPI("/bad-body", "POST", + Collections.emptyList(), unserializable, new HashMap<>(), new HashMap<>(), new HashMap<>(), + "application/json", "application/json", null, null)); + } + + @Test + void formUrlEncodedBodyIsSentAsPercentEncodedKeyValuePairs() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/form")).respond(HttpResponse.response().withStatusCode(200)); + + Map formParams = new HashMap<>(); + formParams.put("hello world", "a b&c"); + + this.buildClient().invokeAPI("/form", "POST", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + formParams, "application/json", "application/x-www-form-urlencoded", null, null); + + HttpRequest[] recorded = this.mockServer.retrieveRecordedRequests(HttpRequest.request().withPath("/form")); + assertEquals(1, recorded.length); + assertEquals("hello%20world=a%20b%26c", recorded[0].getBodyAsString()); + } + + @Test + void multipartFileFormParamProducesMatchingPart(@TempDir Path tempDir) throws ApiException, IOException { + this.mockServer.when(HttpRequest.request().withPath("/upload")).respond(HttpResponse.response().withStatusCode(200)); + + File file = tempDir.resolve("hello.txt").toFile(); + try (FileWriter writer = new FileWriter(file)) { + writer.write("hello content"); + } + + Map formParams = new HashMap<>(); + formParams.put("file", file); + + this.buildClient().invokeAPI("/upload", "POST", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + formParams, "application/json", "multipart/form-data", null, null); + + String body = this.retrieveSingleRequestBody("/upload"); + assertTrue(body.contains("Content-Disposition: form-data; name=\"file\"; filename=\"hello.txt\"")); + assertTrue(body.contains("hello content")); + } + + @Test + void multipartCollectionOfFilesProducesOnePartPerFile(@TempDir Path tempDir) throws ApiException, IOException { + this.mockServer.when(HttpRequest.request().withPath("/upload")).respond(HttpResponse.response().withStatusCode(200)); + + File file1 = tempDir.resolve("one.txt").toFile(); + File file2 = tempDir.resolve("two.txt").toFile(); + try (FileWriter w1 = new FileWriter(file1); FileWriter w2 = new FileWriter(file2)) { + w1.write("content-one"); + w2.write("content-two"); + } + + Map formParams = new HashMap<>(); + formParams.put("files", Arrays.asList(file1, file2)); + + this.buildClient().invokeAPI("/upload", "POST", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + formParams, "application/json", "multipart/form-data", null, null); + + String body = this.retrieveSingleRequestBody("/upload"); + assertTrue(body.contains("filename=\"one.txt\"")); + assertTrue(body.contains("content-one")); + assertTrue(body.contains("filename=\"two.txt\"")); + assertTrue(body.contains("content-two")); + } + + @Test + void multipartApiClientBodyPartProducesPartFromInputStream() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/upload")).respond(HttpResponse.response().withStatusCode(200)); + + Map formParams = new HashMap<>(); + formParams.put("attachment", + new ApiClientBodyPart(new java.io.ByteArrayInputStream("stream-content".getBytes()), "stream.bin")); + + this.buildClient().invokeAPI("/upload", "POST", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + formParams, "application/json", "multipart/form-data", null, null); + + String body = this.retrieveSingleRequestBody("/upload"); + assertTrue(body.contains("filename=\"stream.bin\"")); + assertTrue(body.contains("stream-content")); + } + + @Test + void multipartApiClientBodyPartArrayProducesOnePartPerElement() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/upload")).respond(HttpResponse.response().withStatusCode(200)); + + Map formParams = new HashMap<>(); + formParams.put("attachments", new ApiClientBodyPart[] { + new ApiClientBodyPart(new java.io.ByteArrayInputStream("first".getBytes()), "first.bin"), + new ApiClientBodyPart(new java.io.ByteArrayInputStream("second".getBytes()), "second.bin") + }); + + this.buildClient().invokeAPI("/upload", "POST", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + formParams, "application/json", "multipart/form-data", null, null); + + String body = this.retrieveSingleRequestBody("/upload"); + assertTrue(body.contains("filename=\"first.bin\"")); + assertTrue(body.contains("first")); + assertTrue(body.contains("filename=\"second.bin\"")); + assertTrue(body.contains("second")); + } + + @Test + void multipartPlainFieldIsSentAsFormDataField() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/upload")).respond(HttpResponse.response().withStatusCode(200)); + + Map formParams = new HashMap<>(); + formParams.put("description", "some text value"); + + this.buildClient().invokeAPI("/upload", "POST", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + formParams, "application/json", "multipart/form-data", null, null); + + String body = this.retrieveSingleRequestBody("/upload"); + assertTrue(body.contains("Content-Disposition: form-data; name=\"description\"")); + assertTrue(body.contains("some text value")); + } + + @Test + void multipartWithMissingFileThrowsApiException(@TempDir Path tempDir) { + this.mockServer.when(HttpRequest.request().withPath("/upload")).respond(HttpResponse.response().withStatusCode(200)); + + Map formParams = new HashMap<>(); + formParams.put("file", tempDir.resolve("does-not-exist.txt").toFile()); + + assertThrows(ApiException.class, () -> this.buildClient().invokeAPI("/upload", "POST", Collections.emptyList(), + null, new HashMap<>(), new HashMap<>(), formParams, "application/json", "multipart/form-data", null, + null)); + } + + private String retrieveSingleRequestBody(String path) { + HttpRequest[] recorded = this.mockServer.retrieveRecordedRequests(HttpRequest.request().withPath(path)); + assertEquals(1, recorded.length); + return recorded[0].getBodyAsString(); + } + + // -------------------------------------------------------------------------------------------- + // Response handling + // -------------------------------------------------------------------------------------------- + + @Test + void noContentResponseReturnsApiResponseWithNullData() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/empty")).respond(HttpResponse.response().withStatusCode(204)); + + ApiResponse response = this.invokeGet(this.buildClient(), "/empty"); + + assertEquals(204, response.getStatusCode()); + assertNull(response.getData()); + } + + @Test + void byteArrayReturnTypeReturnsRawResponseBytes() throws ApiException { + byte[] payload = {1, 2, 3, 4}; + this.mockServer.when(HttpRequest.request().withPath("/bytes")) + .respond(HttpResponse.response().withStatusCode(200).withBody(payload)); + + ApiResponse response = this.buildClient().invokeAPI("/bytes", "GET", Collections.emptyList(), null, + new HashMap<>(), new HashMap<>(), new HashMap<>(), "application/octet-stream", "application/json", null, + new TypeReference() {}); + + assertEquals(200, response.getStatusCode()); + assertTrue(Arrays.equals(payload, response.getData())); + } + + @Test + void jsonResponseIsDeserializedIntoReturnType() throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/model")) + .respond(HttpResponse.response().withStatusCode(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"name\":\"foo\",\"ignoredExtraField\":\"bar\"}")); + + ApiResponse response = this.buildClient().invokeAPI("/model", "GET", Collections.emptyList(), null, + new HashMap<>(), new HashMap<>(), new HashMap<>(), "application/json", "application/json", null, + new TypeReference() {}); + + assertEquals(200, response.getStatusCode()); + assertEquals("foo", response.getData().name); + } + + @Test + void nonSuccessResponseThrowsApiExceptionWithBody() { + this.mockServer.when(HttpRequest.request().withPath("/error")) + .respond(HttpResponse.response().withStatusCode(400).withBody("bad request details")); + + ApiException exception = + assertThrows(ApiException.class, () -> this.invokeGet(this.buildClient(), "/error")); + + assertEquals(400, exception.getCode()); + assertEquals("bad request details", exception.getResponseBody()); + } + + @Test + void fileDownloadWritesResponseBodyToTemporaryFolderUsingContentDispositionFilename(@TempDir Path tempDir) + throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/download")) + .respond(HttpResponse.response().withStatusCode(200) + .withHeader("Content-Disposition", "attachment; filename=\"report.pdf\"") + .withBody("file-content")); + + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withTemporaryFolderPath(tempDir.toString()) + .build(); + + ApiResponse response = client.invokeAPI("/download", "GET", Collections.emptyList(), null, + new HashMap<>(), new HashMap<>(), new HashMap<>(), "application/pdf", "application/json", null, + new TypeReference() {}); + + assertEquals(200, response.getStatusCode()); + File downloaded = response.getData(); + assertTrue(downloaded.getParentFile().toPath().equals(tempDir)); + assertTrue(downloaded.getName().startsWith("report")); + assertTrue(downloaded.getName().endsWith(".pdf")); + } + + @Test + void fileDownloadWithoutContentDispositionUsesGenericPrefix(@TempDir Path tempDir) throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/download")) + .respond(HttpResponse.response().withStatusCode(200).withBody("file-content")); + + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withTemporaryFolderPath(tempDir.toString()) + .build(); + + ApiResponse response = client.invokeAPI("/download", "GET", Collections.emptyList(), null, + new HashMap<>(), new HashMap<>(), new HashMap<>(), "application/pdf", "application/json", null, + new TypeReference() {}); + + assertEquals(200, response.getStatusCode()); + assertTrue(response.getData().getName().startsWith("download-")); + } + + @Test + void fileDownloadWithShortFilenameFallsBackToGenericPrefix(@TempDir Path tempDir) throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/download")) + .respond(HttpResponse.response().withStatusCode(200) + .withHeader("Content-Disposition", "attachment; filename=\"a\"") + .withBody("file-content")); + + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withTemporaryFolderPath(tempDir.toString()) + .build(); + + ApiResponse response = client.invokeAPI("/download", "GET", Collections.emptyList(), null, + new HashMap<>(), new HashMap<>(), new HashMap<>(), "application/pdf", "application/json", null, + new TypeReference() {}); + + assertEquals(200, response.getStatusCode()); + assertTrue(response.getData().getName().startsWith("download-")); + } + + @Test + void fileDownloadWithFilenameWithoutExtension(@TempDir Path tempDir) throws ApiException { + this.mockServer.when(HttpRequest.request().withPath("/download")) + .respond(HttpResponse.response().withStatusCode(200) + .withHeader("Content-Disposition", "attachment; filename=\"reportnoext\"") + .withBody("file-content")); + + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withTemporaryFolderPath(tempDir.toString()) + .build(); + + ApiResponse response = client.invokeAPI("/download", "GET", Collections.emptyList(), null, + new HashMap<>(), new HashMap<>(), new HashMap<>(), "application/pdf", "application/json", null, + new TypeReference() {}); + + assertEquals(200, response.getStatusCode()); + assertTrue(response.getData().getName().startsWith("reportnoext-")); + } + + @Test + void nonSuccessFileDownloadResponseThrowsApiExceptionWithBody(@TempDir Path tempDir) { + this.mockServer.when(HttpRequest.request().withPath("/download")) + .respond(HttpResponse.response().withStatusCode(404).withBody("not found")); + + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withTemporaryFolderPath(tempDir.toString()) + .build(); + + ApiException exception = assertThrows(ApiException.class, + () -> client.invokeAPI("/download", "GET", Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), + new HashMap<>(), "application/pdf", "application/json", null, new TypeReference() {})); + + assertEquals(404, exception.getCode()); + assertEquals("not found", exception.getResponseBody()); + } + + @Test + void malformedJsonResponseThrowsApiException() { + this.mockServer.when(HttpRequest.request().withPath("/malformed")) + .respond(HttpResponse.response().withStatusCode(200) + .withHeader("Content-Type", "application/json") + .withBody("not-json")); + + assertThrows(ApiException.class, () -> this.buildClient().invokeAPI("/malformed", "GET", + Collections.emptyList(), null, new HashMap<>(), new HashMap<>(), new HashMap<>(), "application/json", + "application/json", null, new TypeReference() {})); + } + + @Test + void interruptedThreadDuringSendThrowsIllegalStateException() throws IOException { + this.mockServer.when(HttpRequest.request().withPath("/slow")) + .respond(HttpResponse.response().withStatusCode(200).withDelay(new Delay(TimeUnit.SECONDS, 2))); + + ApiClient client = this.buildClient(); + Thread current = Thread.currentThread(); + java.util.concurrent.ScheduledExecutorService interrupter = + java.util.concurrent.Executors.newSingleThreadScheduledExecutor(); + interrupter.schedule(current::interrupt, 200, TimeUnit.MILLISECONDS); + + try { + assertThrows(IllegalStateException.class, () -> this.invokeGet(client, "/slow")); + } finally { + interrupter.shutdownNow(); + // clear the interrupted flag so later tests in the same JVM aren't affected + Thread.interrupted(); + } + } + + // -------------------------------------------------------------------------------------------- + // Outgoing request logging + // -------------------------------------------------------------------------------------------- + + @Test + void completedRequestProducesDebugLogEntry() throws ApiException { + Logger logger = (Logger) LoggerFactory.getLogger("com.symphony.bdk.requests.outgoing"); + Level originalLevel = logger.getLevel(); + logger.setLevel(Level.DEBUG); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + + try { + this.mockServer.when(HttpRequest.request().withPath("/logged")) + .respond(HttpResponse.response().withStatusCode(200)); + + this.invokeGet(this.buildClient(), "/logged"); + + boolean found = appender.list.stream() + .anyMatch(event -> event.getFormattedMessage().contains("status=200") + && event.getFormattedMessage().contains("/logged")); + assertTrue(found, "expected a DEBUG log entry with status and url, got: " + appender.list); + } finally { + logger.detachAppender(appender); + logger.setLevel(originalLevel); + } + } + + // -------------------------------------------------------------------------------------------- + // Exception translation for retry compatibility (D2) + // -------------------------------------------------------------------------------------------- + + @Test + void connectTimeoutTranslatesToConnectExceptionRootCause() { + ApiClientJdk client = + new ApiClientJdk(HttpClient.newHttpClient(), "http://localhost", new HashMap<>(), null, null, + new ArrayList<>()); + + RuntimeException translated = client.translateAndWrap(new HttpConnectTimeoutException("timed out")); + + assertTrue(hasExactRootCause(translated, ConnectException.class)); + assertTrue(RetryWithRecoveryBuilder.isNetworkIssueOrMinorError(translated)); + } + + @Test + void requestTimeoutTranslatesToSocketTimeoutExceptionRootCause() { + ApiClientJdk client = + new ApiClientJdk(HttpClient.newHttpClient(), "http://localhost", new HashMap<>(), null, null, + new ArrayList<>()); + + RuntimeException translated = client.translateAndWrap(new HttpTimeoutException("timed out")); + + assertTrue(hasExactRootCause(translated, SocketTimeoutException.class)); + assertTrue(RetryWithRecoveryBuilder.isNetworkIssueOrMinorError(translated)); + } + + @Test + void requestTimeoutDuringSendSurfacesAsRetryableSocketTimeoutException() { + this.mockServer.when(HttpRequest.request().withPath("/slow"), Times.once()) + .respond(HttpResponse.response().withStatusCode(200).withDelay(new Delay(TimeUnit.SECONDS, 3))); + + ApiClient client = new ApiClientBuilderJdk() + .withBasePath("http://localhost:" + this.mockServer.getPort()) + .withReadTimeout(200) + .build(); + + RuntimeException thrown = assertThrows(RuntimeException.class, () -> this.invokeGet(client, "/slow")); + + assertTrue(hasExactRootCause(thrown, SocketTimeoutException.class)); + assertTrue(RetryWithRecoveryBuilder.isNetworkIssueOrMinorError(thrown)); + } + + private static boolean hasExactRootCause(Throwable throwable, Class expected) { + Throwable current = throwable; + while (current != null) { + if (current.getClass().equals(expected)) { + return true; + } + current = current.getCause(); + } + return false; + } + + static class Model { + public String name; + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientJdkUnitTest.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientJdkUnitTest.java new file mode 100644 index 000000000..bbec4393b --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/ApiClientJdkUnitTest.java @@ -0,0 +1,160 @@ +package com.symphony.bdk.http.jdk; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.symphony.bdk.http.api.ApiException; +import com.symphony.bdk.http.api.Pair; + +import org.junit.jupiter.api.Test; + +import java.net.http.HttpClient; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Pure-logic unit tests for {@link ApiClientJdk} methods that don't require a network round-trip: parameter + * formatting, header selection, and authentication scheme handling. Network-facing behavior (request building, + * response handling, multipart, tracing, exception translation) is covered by {@link ApiClientJdkTest}. + */ +class ApiClientJdkUnitTest { + + private final ApiClientJdk client = + new ApiClientJdk(HttpClient.newHttpClient(), "http://base", new HashMap<>(), null, null, new ArrayList<>()); + + @Test + void getBasePath_returnsConfiguredBasePath() { + assertEquals("http://base", this.client.getBasePath()); + } + + @Test + void parameterToPairs_returnsEmptyList_whenNameOrValueMissing() { + assertTrue(this.client.parameterToPairs("csv", null, "value").isEmpty()); + assertTrue(this.client.parameterToPairs("csv", "", "value").isEmpty()); + assertTrue(this.client.parameterToPairs("csv", "name", null).isEmpty()); + } + + @Test + void parameterToPairs_returnsSinglePair_forNonCollectionValue() { + List pairs = this.client.parameterToPairs("csv", "name", "value"); + + assertEquals(1, pairs.size()); + assertEquals("value", pairs.get(0).getValue()); + } + + @Test + void parameterToPairs_returnsEmptyList_forEmptyCollection() { + assertTrue(this.client.parameterToPairs("csv", "name", Collections.emptyList()).isEmpty()); + } + + @Test + void parameterToPairs_multiFormat_returnsOnePairPerElement() { + List pairs = this.client.parameterToPairs("multi", "name", Arrays.asList("a", "b")); + + assertEquals(2, pairs.size()); + assertEquals("a", pairs.get(0).getValue()); + assertEquals("b", pairs.get(1).getValue()); + } + + @Test + void parameterToPairs_csvFormat_joinsWithComma() { + List pairs = this.client.parameterToPairs("csv", "name", Arrays.asList("a", "b")); + assertEquals("a,b", pairs.get(0).getValue()); + } + + @Test + void parameterToPairs_defaultFormat_joinsWithComma() { + List pairs = this.client.parameterToPairs(null, "name", Arrays.asList("a", "b")); + assertEquals("a,b", pairs.get(0).getValue()); + } + + @Test + void parameterToPairs_ssvFormat_joinsWithSpace() { + List pairs = this.client.parameterToPairs("ssv", "name", Arrays.asList("a", "b")); + assertEquals("a b", pairs.get(0).getValue()); + } + + @Test + void parameterToPairs_tsvFormat_joinsWithTab() { + List pairs = this.client.parameterToPairs("tsv", "name", Arrays.asList("a", "b")); + assertEquals("a\tb", pairs.get(0).getValue()); + } + + @Test + void parameterToPairs_pipesFormat_joinsWithPipe() { + List pairs = this.client.parameterToPairs("pipes", "name", Arrays.asList("a", "b")); + assertEquals("a|b", pairs.get(0).getValue()); + } + + @Test + void selectHeaderAccept_returnsNull_whenAcceptsEmpty() { + assertEquals(null, this.client.selectHeaderAccept(new String[0])); + } + + @Test + void selectHeaderAccept_prefersJsonMime() { + assertEquals("application/json", this.client.selectHeaderAccept(new String[] {"text/plain", "application/json"})); + } + + @Test + void selectHeaderAccept_joinsAllAccepts_whenNoJsonMime() { + assertEquals("text/plain,text/html", this.client.selectHeaderAccept(new String[] {"text/plain", "text/html"})); + } + + @Test + void selectHeaderContentType_returnsJson_whenContentTypesEmpty() { + assertEquals("application/json", this.client.selectHeaderContentType(new String[0])); + } + + @Test + void selectHeaderContentType_prefersJsonMime() { + assertEquals("application/json", + this.client.selectHeaderContentType(new String[] {"text/plain", "application/json"})); + } + + @Test + void selectHeaderContentType_returnsFirst_whenNoJsonMime() { + assertEquals("text/plain", this.client.selectHeaderContentType(new String[] {"text/plain", "text/html"})); + } + + @Test + void updateParamsForAuth_appliesNamedAuthentication() throws ApiException { + Map headers = new HashMap<>(); + this.client.getAuthentications().put("basic", h -> h.put("Authorization", "Basic xyz")); + + this.client.updateParamsForAuth(new String[] {"basic"}, headers); + + assertEquals("Basic xyz", headers.get("Authorization")); + } + + @Test + void updateParamsForAuth_appliesEnforcedAuthenticationScheme_evenWhenAuthNamesNull() throws ApiException { + Map headers = new HashMap<>(); + this.client.getAuthentications().put("enforced", h -> h.put("X-Enforced", "true")); + this.client.addEnforcedAuthenticationScheme("enforced"); + + this.client.updateParamsForAuth(null, headers); + + assertEquals("true", headers.get("X-Enforced")); + } + + @Test + void updateParamsForAuth_throwsRuntimeException_forUndefinedAuthentication() { + assertThrows(RuntimeException.class, + () -> this.client.updateParamsForAuth(new String[] {"unknown"}, new HashMap<>())); + } + + @Test + void updateParamsForAuth_doesNothing_whenNoAuthNamesAndNoEnforcedSchemes() throws ApiException { + Map headers = new HashMap<>(); + + this.client.updateParamsForAuth(null, headers); + + assertTrue(headers.isEmpty()); + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/FakeHttpProxy.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/FakeHttpProxy.java new file mode 100644 index 000000000..ab94511e4 --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/FakeHttpProxy.java @@ -0,0 +1,106 @@ +package com.symphony.bdk.http.jdk; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Minimal raw-socket forward HTTP proxy used to test {@link ApiClientBuilderJdk}'s proxy support (D9) without + * pulling in a third-party proxy test double. For plain HTTP forward-proxying, the client sends an + * absolute-URI request line directly to the proxy and the proxy is free to answer on behalf of the origin + * server; this fake never actually forwards anywhere, it just inspects the request line/headers it received + * and answers with a canned response, optionally gating on {@code Proxy-Authorization}. + */ +class FakeHttpProxy implements AutoCloseable { + + private final ServerSocket serverSocket; + private final ExecutorService executor = Executors.newSingleThreadExecutor(); + private final boolean requireAuth; + private final AtomicInteger requestCount = new AtomicInteger(); + + private volatile String lastRequestLine; + private volatile String lastProxyAuthorizationHeader; + + FakeHttpProxy(boolean requireAuth) throws IOException { + this.requireAuth = requireAuth; + this.serverSocket = new ServerSocket(0); + this.executor.submit(this::acceptLoop); + } + + int getPort() { + return this.serverSocket.getLocalPort(); + } + + int getRequestCount() { + return this.requestCount.get(); + } + + String getLastRequestLine() { + return this.lastRequestLine; + } + + String getLastProxyAuthorizationHeader() { + return this.lastProxyAuthorizationHeader; + } + + private void acceptLoop() { + while (!this.serverSocket.isClosed()) { + try (Socket socket = this.serverSocket.accept()) { + this.handle(socket); + } catch (IOException e) { + return; + } + } + } + + private void handle(Socket socket) throws IOException { + BufferedReader reader = + new BufferedReader(new InputStreamReader(socket.getInputStream(), StandardCharsets.US_ASCII)); + this.lastRequestLine = reader.readLine(); + + String line; + String authHeader = null; + while ((line = reader.readLine()) != null && !line.isEmpty()) { + if (line.regionMatches(true, 0, "Proxy-Authorization:", 0, "Proxy-Authorization:".length())) { + authHeader = line.substring(line.indexOf(':') + 1).trim(); + } + } + this.lastProxyAuthorizationHeader = authHeader; + this.requestCount.incrementAndGet(); + + OutputStream out = socket.getOutputStream(); + if (this.requireAuth && authHeader == null) { + String response = "HTTP/1.1 407 Proxy Authentication Required\r\n" + + "Proxy-Authenticate: Basic realm=\"fake-proxy\"\r\n" + + "Content-Length: 0\r\n" + + "Connection: close\r\n\r\n"; + out.write(response.getBytes(StandardCharsets.US_ASCII)); + } else { + String body = "{}"; + String response = "HTTP/1.1 200 OK\r\n" + + "Content-Type: application/json\r\n" + + "Content-Length: " + body.length() + "\r\n" + + "Connection: close\r\n\r\n" + + body; + out.write(response.getBytes(StandardCharsets.US_ASCII)); + } + out.flush(); + } + + @Override + public void close() { + this.executor.shutdownNow(); + try { + this.serverSocket.close(); + } catch (IOException ignored) { + // best-effort cleanup + } + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/JSONTest.java b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/JSONTest.java new file mode 100644 index 000000000..b2b4b2d2e --- /dev/null +++ b/symphony-bdk-http/symphony-bdk-http-jdk/src/test/java/com/symphony/bdk/http/jdk/JSONTest.java @@ -0,0 +1,92 @@ +package com.symphony.bdk.http.jdk; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.api.Test; +import tools.jackson.databind.ObjectMapper; + +import java.text.ParsePosition; +import java.time.Instant; +import java.util.Date; + +class JSONTest { + + static class DatedModel { + public Date date; + } + + static class SimpleModel { + public String name; + } + + @Test + void dateFieldSerializesAsRfc3339String() { + ObjectMapper mapper = new JSON().getMapper(); + DatedModel model = new DatedModel(); + model.date = Date.from(Instant.parse("2024-01-15T10:30:00.123Z")); + + String json = mapper.writeValueAsString(model); + + assertEquals("{\"date\":\"2024-01-15T10:30:00.123Z\"}", json); + } + + @Test + void dateFieldRoundTrips() { + ObjectMapper mapper = new JSON().getMapper(); + DatedModel model = new DatedModel(); + model.date = Date.from(Instant.parse("2024-01-15T10:30:00.123Z")); + + String json = mapper.writeValueAsString(model); + DatedModel parsed = mapper.readValue(json, DatedModel.class); + + assertEquals(model.date, parsed.date); + } + + @Test + void unknownPropertyIsIgnoredOnDeserialization() { + ObjectMapper mapper = new JSON().getMapper(); + String json = "{\"name\":\"foo\",\"unknownField\":\"bar\"}"; + + SimpleModel parsed = assertDoesNotThrow(() -> mapper.readValue(json, SimpleModel.class)); + + assertEquals("foo", parsed.name); + } + + @Test + void setDateFormatOverridesTheMapperConfiguration() { + JSON json = new JSON(); + json.setDateFormat(new RFC3339DateFormat()); + + DatedModel model = new DatedModel(); + model.date = Date.from(Instant.parse("2024-01-15T10:30:00.123Z")); + + String output = json.getMapper().writeValueAsString(model); + + assertEquals("{\"date\":\"2024-01-15T10:30:00.123Z\"}", output); + } + + @Test + void rfc3339DateFormatParseReturnsNull_forInvalidInput() { + RFC3339DateFormat format = new RFC3339DateFormat(); + ParsePosition pos = new ParsePosition(0); + + Date result = format.parse("not-a-date", pos); + + assertNull(result); + assertEquals(0, pos.getErrorIndex()); + } + + @Test + void nullFieldsAreExcludedFromSerializedOutput() { + ObjectMapper mapper = new JSON().getMapper(); + SimpleModel model = new SimpleModel(); + model.name = null; + + String json = mapper.writeValueAsString(model); + + assertEquals("{}", json); + assertNull(model.name); + } +} diff --git a/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderJersey2.java b/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderJersey2.java index 85fa865a9..84ec442ce 100644 --- a/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderJersey2.java +++ b/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderJersey2.java @@ -41,8 +41,13 @@ *

Please note that overriding this class is an {@link org.apiguardian.api.API.Status#EXPERIMENTAL} feature that we * offer to developers for {@link ApiClient} customization. The internal contract of this class (e.g. protected methods) * is subject to changes in the future. + * + * @deprecated in favor of {@code com.symphony.bdk.http.jdk.ApiClientBuilderJdk} (module {@code symphony-bdk-http-jdk}), + * the new default HTTP implementation for {@code symphony-bdk-core}, which has no third-party HTTP dependency. This + * module keeps shipping and working exactly as before; this is a soft, non-removing signal, not a functional + * change. See the migration guide's "New default HTTP client module" section for details. */ -@API(status = API.Status.STABLE) +@API(status = API.Status.DEPRECATED) public class ApiClientBuilderJersey2 implements ApiClientBuilder { private static final String TRUSTSTORE_FORMAT = "JKS"; diff --git a/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderProviderJersey2.java b/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderProviderJersey2.java index ac464a618..5deb858db 100644 --- a/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderProviderJersey2.java +++ b/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientBuilderProviderJersey2.java @@ -7,8 +7,13 @@ /** * Provides new {@link ApiClientBuilderJersey2} implementation of the {@link ApiClientBuilder} interface. + * + * @deprecated in favor of {@code com.symphony.bdk.http.jdk.ApiClientBuilderProviderJdk} (module + * {@code symphony-bdk-http-jdk}), the new default HTTP implementation for {@code symphony-bdk-core}, which has no + * third-party HTTP dependency. This module keeps shipping and working exactly as before; this is a soft, + * non-removing signal, not a functional change. */ -@API(status = API.Status.INTERNAL) +@API(status = API.Status.DEPRECATED) public class ApiClientBuilderProviderJersey2 implements ApiClientBuilderProvider { /** diff --git a/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientJersey2.java b/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientJersey2.java index 688334844..315a66a37 100644 --- a/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientJersey2.java +++ b/symphony-bdk-http/symphony-bdk-http-jersey/src/main/java/com/symphony/bdk/http/jersey2/ApiClientJersey2.java @@ -52,8 +52,13 @@ /** * Jersey2 implementation for the {@link ApiClient} interface called by generated code. + * + * @deprecated in favor of {@code com.symphony.bdk.http.jdk.ApiClientJdk} (module {@code symphony-bdk-http-jdk}), + * the new default HTTP implementation for {@code symphony-bdk-core}, which has no third-party HTTP dependency. This + * module keeps shipping and working exactly as before; this is a soft, non-removing signal, not a functional + * change. See the migration guide's "New default HTTP client module" section for details. */ -@API(status = API.Status.STABLE) +@API(status = API.Status.DEPRECATED) public class ApiClientJersey2 implements ApiClient { protected Client httpClient;