diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ed211da..33eaadb 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -34,16 +34,19 @@ jobs: - name: Checkout uses: actions/checkout@v4 - # Install both JDK 17 (for compilation) and the matrix JDK (for - # test execution). setup-java exports JAVA_HOME__; - # Gradle's toolchain auto-detection picks them up. - - name: Set up JDKs (compile=17, test=${{ matrix.java }}) + # Install both the matrix JDK (test execution) and JDK 17 (compile + + # Gradle daemon runtime). Order matters: setup-java sets JAVA_HOME + # to the LAST entry, and Gradle 8.12 only supports JDKs up to 23 as + # its own runtime — so JDK 17 must be last for matrix.java = 25. + # setup-java still exports JAVA_HOME__ for both, and + # setup-gradle registers them as toolchain candidates. + - name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17) uses: actions/setup-java@v4 with: distribution: temurin java-version: | - 17 ${{ matrix.java }} + 17 - name: Set up Gradle uses: gradle/actions/setup-gradle@v4 @@ -72,8 +75,50 @@ jobs: files: build/reports/jacoco/test/jacocoTestReport.xml fail_ci_if_error: true - # Integration-tests job is intentionally not wired up yet: - # SDK requirements §13 says they run on PRs and release pipelines, but - # they hit the live API and require a MARKETDATA_TOKEN secret. Add this - # job (gated on `if: ${{ secrets.MARKETDATA_TOKEN != '' }}`) once the - # token is configured in the repo's GitHub Actions secrets. + # SDK requirements §13: integration tests run mandatorily on every push + # to main (release-pipeline gate) on the full forward-compat matrix. + # On PRs they're triggered on demand instead — see + # pr-integration-on-demand.yml. + integration-tests: + name: Integration tests (live API, JDK ${{ matrix.java }}) + runs-on: ubuntu-latest + needs: verify + strategy: + fail-fast: false + matrix: + java: ['17', '21', '25'] + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17) + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: | + ${{ matrix.java }} + 17 + + - name: Set up Gradle + uses: gradle/actions/setup-gradle@v4 + + - name: Run integration tests against live API + env: + MARKETDATA_TOKEN: ${{ secrets.MARKETDATA_TOKEN }} + MARKETDATA_RUN_INTEGRATION_TESTS: 'true' + run: | + if [ -z "$MARKETDATA_TOKEN" ]; then + echo "::error::MARKETDATA_TOKEN secret is required on main; integration tests must run on every merge." + exit 1 + fi + ./gradlew integrationTest -PtestJdk=${{ matrix.java }} --stacktrace + + - name: Upload integration-test reports on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: integration-test-reports-jdk${{ matrix.java }} + path: | + build/reports/tests/integrationTest/ + build/test-results/integrationTest/ + retention-days: 14 diff --git a/.github/workflows/pr-integration-on-demand.yml b/.github/workflows/pr-integration-on-demand.yml new file mode 100644 index 0000000..9fc5a51 --- /dev/null +++ b/.github/workflows/pr-integration-on-demand.yml @@ -0,0 +1,226 @@ +name: Integration tests on demand + +# Manually triggered by commenting on an open PR with a slash-command on +# the FIRST line of the comment body (everything after the first line is +# ignored): +# /integrationtest → JDK 17 only +# /integrationtestfull → full matrix {17, 21, 25} +# +# The slash + first-line constraint prevents accidental triggers from +# review comments that mention the workflow by name, quoted replies +# (`> /integrationtest`), pasted documentation, or stack traces. The +# Guard job below filters on `startsWith(... '/integrationtest')` and +# then a strict bash `case` validates the exact command — anything that +# slips through is rejected before any live-API request is made. +# +# Integration tests hit the live Market Data API, so we don't run them +# automatically on every PR open/sync (saves API quota + CI minutes). +# They ARE required for merge — branch protection on `main` should list +# "Integration tests pass" as a required status check, which is the +# aggregator job below. PRs cannot merge until a reviewer comments one +# of the two slash-commands AND the resulting run is green. +# +# Important security note: workflows triggered by `issue_comment` always +# run from the *default branch's* version of the workflow file, not from +# the PR. Adding/changing this file on a feature branch has no effect +# until it lands on main. +on: + issue_comment: + types: [created] + +permissions: + contents: read + pull-requests: write # to react and post the result comment + +# Multiple trigger comments on the same PR cancel earlier runs. +concurrency: + group: pr-integration-on-demand-${{ github.event.issue.number }} + cancel-in-progress: true + +jobs: + guard: + name: Guard + runs-on: ubuntu-latest + # Only fire on PR comments whose body starts with `/integrationtest`. + # `startsWith` rejects comments that merely mention the command in + # passing (quoted replies start with `>`, prose with anything else, + # so they don't match). The strict `case` in the matrix step below + # rejects anything that slips through (e.g. `/integrationtest-foo`) + # before any live-API request fires. + if: | + github.event.issue.pull_request != null && + startsWith(github.event.comment.body, '/integrationtest') + outputs: + head_sha: ${{ steps.pr.outputs.head_sha }} + jdks: ${{ steps.matrix.outputs.jdks }} + mode: ${{ steps.matrix.outputs.mode }} + steps: + - name: Verify commenter has write permission + uses: actions/github-script@v7 + with: + script: | + const { data: perm } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: context.repo.owner, + repo: context.repo.repo, + username: context.payload.comment.user.login, + }); + const allowed = ['write', 'maintain', 'admin'].includes(perm.permission); + if (!allowed) { + core.setFailed( + `@${context.payload.comment.user.login} (${perm.permission}) ` + + `cannot trigger integration tests; write access required.` + ); + } + + - name: React 👀 to the trigger comment + uses: actions/github-script@v7 + with: + script: | + await github.rest.reactions.createForIssueComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: context.payload.comment.id, + content: 'eyes', + }); + + - name: Resolve PR head SHA + id: pr + uses: actions/github-script@v7 + with: + script: | + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.issue.number, + }); + if (pr.state !== 'open') { + core.setFailed(`PR #${pr.number} is ${pr.state}; refusing to run.`); + return; + } + core.setOutput('head_sha', pr.head.sha); + + - name: Decide JDK matrix from comment body + id: matrix + env: + BODY: ${{ github.event.comment.body }} + run: | + # The if: filter above only guarantees the body starts with + # '/integrationtest'. We still need to disambiguate single vs + # full and reject anything that just shares the prefix + # (e.g. '/integrationtest-foo' or '/integrationtestlong'). + # Match on the first line only — trailing context in the + # comment body is ignored. + first_line=$(printf '%s' "$BODY" | head -n 1 | tr -d '[:space:]') + case "$first_line" in + /integrationtest) + echo 'jdks=["17"]' >> "$GITHUB_OUTPUT" + echo 'mode=single' >> "$GITHUB_OUTPUT" + echo "Trigger: /integrationtest → JDK 17" + ;; + /integrationtestfull) + echo 'jdks=["17","21","25"]' >> "$GITHUB_OUTPUT" + echo 'mode=full' >> "$GITHUB_OUTPUT" + echo "Trigger: /integrationtestfull → matrix {17, 21, 25}" + ;; + *) + echo "::error::Unrecognized command on first line: '$first_line' (expected '/integrationtest' or '/integrationtestfull')" + exit 1 + ;; + esac + + integration-tests: + name: Integration tests (JDK ${{ matrix.java }}) + needs: guard + runs-on: ubuntu-latest + strategy: + # Don't cancel siblings: if JDK 21 fails, we still want 17 and 25 + # results to surface. + fail-fast: false + matrix: + java: ${{ fromJSON(needs.guard.outputs.jdks) }} + + steps: + # Check out exactly the PR's HEAD commit so we test the proposed + # change, not the merge ref. + - name: Checkout PR head + uses: actions/checkout@v4 + with: + ref: ${{ needs.guard.outputs.head_sha }} + + # Order matters: JDK 17 must be last so JAVA_HOME=17 (Gradle 8.12 + # only supports JDKs up to 23 as its daemon runtime). The matrix + # JDK is still installed and registered as a toolchain target. + - name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17) + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: | + ${{ matrix.java }} + 17 + + - name: Set up Gradle + uses: gradle/actions/setup-gradle@v4 + + - name: Run integration tests against live API + env: + MARKETDATA_TOKEN: ${{ secrets.MARKETDATA_TOKEN }} + MARKETDATA_RUN_INTEGRATION_TESTS: 'true' + run: | + if [ -z "$MARKETDATA_TOKEN" ]; then + echo "::error::MARKETDATA_TOKEN secret missing — cannot run integration tests." + exit 1 + fi + ./gradlew integrationTest -PtestJdk=${{ matrix.java }} --stacktrace + + - name: Upload integration-test reports on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: integration-test-reports-jdk${{ matrix.java }} + path: | + build/reports/tests/integrationTest/ + build/test-results/integrationTest/ + retention-days: 14 + + # Aggregator job. Branch protection on `main` should require this + # check name ("Integration tests pass") so a single required check + # covers both `integrationtest` (matrix=[17]) and `integrationtestfull` + # (matrix=[17,21,25]) modes uniformly. Without this, branch protection + # would have to list the per-matrix-entry check names which only exist + # in the `full` mode. + required-check: + name: Integration tests pass + needs: [guard, integration-tests] + if: always() && needs.guard.result == 'success' + runs-on: ubuntu-latest + steps: + - name: Aggregate matrix outcome + env: + MATRIX_RESULT: ${{ needs.integration-tests.result }} + MODE: ${{ needs.guard.outputs.mode }} + run: | + echo "Mode: $MODE" + echo "Matrix outcome: $MATRIX_RESULT" + if [[ "$MATRIX_RESULT" != "success" ]]; then + echo "::error::One or more integration-test JDK entries failed." + exit 1 + fi + echo "All integration tests passed." + + - name: Comment outcome on the PR + if: always() + uses: actions/github-script@v7 + with: + script: | + const ok = '${{ needs.integration-tests.result }}' === 'success'; + const mode = '${{ needs.guard.outputs.mode }}'; + const emoji = ok ? '✅' : '❌'; + const status = ok ? 'passed' : 'failed'; + const matrix = mode === 'full' ? '`{17, 21, 25}`' : '`17`'; + const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.payload.issue.number, + body: `${emoji} On-demand integration tests on JDK ${matrix} ${status}. [View run](${runUrl}).`, + }); diff --git a/.github/workflows/pr-matrix-on-demand.yml b/.github/workflows/pr-matrix-on-demand.yml index 6c309ad..5dda271 100644 --- a/.github/workflows/pr-matrix-on-demand.yml +++ b/.github/workflows/pr-matrix-on-demand.yml @@ -110,14 +110,16 @@ jobs: with: ref: ${{ needs.guard.outputs.head_sha }} - # Compile=17, test=matrix JDK; same shape as main.yml. - - name: Set up JDKs (compile=17, test=${{ matrix.java }}) + # Order matters: JDK 17 must be last so JAVA_HOME=17 (Gradle 8.12 + # doesn't support JDK 24+ as its daemon runtime). The matrix JDK + # is still installed and registered as a toolchain target. + - name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17) uses: actions/setup-java@v4 with: distribution: temurin java-version: | - 17 ${{ matrix.java }} + 17 - name: Set up Gradle uses: gradle/actions/setup-gradle@v4 diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 0ced2ec..4411957 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -63,3 +63,10 @@ jobs: token: ${{ secrets.CODECOV_TOKEN }} files: build/reports/jacoco/test/jacocoTestReport.xml fail_ci_if_error: true + + # NOTE: integration tests are NOT run automatically on PR open/sync. + # They are required for merge but only fire when a reviewer comments + # `integrationtest` (JDK 17) or `integrationtestfull` (matrix 17/21/25) + # — see .github/workflows/pr-integration-on-demand.yml. Branch-protection + # rules should require the "Integration tests pass" check name produced + # by that workflow before allowing merge to main. diff --git a/CLAUDE.md b/CLAUDE.md index 877be32..07cb5ca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,7 +27,7 @@ These decisions are not up for debate without amending the corresponding ADR: - **Java only.** Single artifact, no Kotlin sources. Published JAR must not bring `kotlin-stdlib` as a transitive dep. JSpecify is compile-time only and doesn't count. (ADR-001) - **Kotlin consumers are first-class via interop, not via a Kotlin artifact.** A separate `marketdata-sdk-java-kotlin` extensions JAR (Option E) is deferred. (ADR-001) - **JDK 17 minimum.** Build with `javac --release 17`; no multi-release JAR. CI test matrix is `{17, 21, 25}` for forward-compat. (ADR-002) -- **Gradle, Kotlin DSL.** `build.gradle.kts`, `settings.gradle.kts`, version catalog at `gradle/libs.versions.toml`. Standard plugins: `java-library`, `maven-publish`, Vanniktech Maven Publish (or Gradle Nexus Publish), Spotless, JaCoCo. Integration tests live in a separate `integrationTest` source set, env-var-gated. (ADR-003) +- **Gradle 9.0, Kotlin DSL.** `build.gradle.kts`, `settings.gradle.kts`, version catalog at `gradle/libs.versions.toml`. Wrapper pinned to **Gradle 9.0.0** — the first release that supports **JDK 25** for both daemon and toolchain (8.x maxed at JDK 24). The daemon still runs on JDK 17 via `JAVA_HOME` for stability (CI workflows order `setup-java`'s `java-version` so the matrix JDK is first and 17 is last); toolchain forks compile/test JDKs as needed via `-PtestJdk=N`. Standard plugins: `java-library`, `maven-publish`, Vanniktech Maven Publish (or Gradle Nexus Publish), Spotless, JaCoCo. Integration tests live in a separate `integrationTest` source set, env-var-gated. (ADR-003) - **`java.net.http.HttpClient` exclusively.** No third-party HTTP client (OkHttp, Apache) as a runtime dep — ever. HTTP/2 on (default). One shared `HttpClient` per `MarketDataClient`. Timeouts: 99s request, 2s connect. (ADR-004) - **Jackson (`jackson-databind`) for JSON.** Records-based response models (Jackson record support, 2.12+). The API's parallel-arrays wire format (e.g. `{"s":"ok","symbol":["AAPL","MSFT"],"price":[150.0,400.0]}`) is decoded via custom `JsonDeserializer` classes, *not* default reflection. Jackson is **not shaded** in v1; shading is held in reserve. (ADR-005) - **Sync + async parity per endpoint.** Every public endpoint exposes both `quote(...)` and `quoteAsync(...)`; async returns `CompletableFuture`. **Internal logic is async-first.** Sync methods are thin wrappers that call `.join()` and unwrap `CompletionException` to surface the underlying cause directly. Both surfaces share validation, retry, rate-limit, and concurrency-pool logic — no parallel implementations. Tests must cover both variants for every endpoint. (ADR-006) @@ -69,10 +69,13 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements]( - §12 concurrency: `Semaphore(50)` field on `MarketDataClient` (wiring of acquire/release lands with the request layer). - §15 packaging: SemVer, MIT `LICENSE`, `CHANGELOG.md` in Keep a Changelog format, version auto-detected via JAR manifest (`Implementation-Version`). - §16 security: tokens never logged verbatim (use `Tokens.redact`); TLS validated by default (`HttpClient` does not expose a skip-verify option). -- ADR-002 CI: split into three workflows. - - `.github/workflows/pull-request.yml` — runs on PR `opened`/`synchronize`/`reopened` (no pre-PR push trigger by design). JDK 17 only. Runs `./gradlew build` and uploads `build/reports/jacoco/test/jacocoTestReport.xml` to Codecov. - - `.github/workflows/main.yml` — runs only on `push` to `main`. Full forward-compat matrix `{17, 21, 25}` via `-PtestJdk=N` (wired into `tasks.test.javaLauncher` in `build.gradle.kts`). The JDK 17 matrix entry also uploads coverage to Codecov, establishing the base coverage that PRs compare against. - - `.github/workflows/pr-matrix-on-demand.yml` — manually triggered by commenting one of `/run-all-jdks`, `/jdk-matrix`, or `/test-all` on an open PR. Runs JDK 21 and 25 (17 already ran via `pull-request.yml`). Gated to commenters with write/maintain/admin permission. Reacts 👀 to the trigger comment and posts a result summary comment when the matrix finishes. Note: `issue_comment` workflows always execute from the default branch's copy of the file — feature-branch edits to this workflow have no effect until merged to main. +- ADR-002 CI: split into four workflows. + - `.github/workflows/pull-request.yml` — runs on PR `opened`/`synchronize`/`reopened` (no pre-PR push trigger by design). JDK 17 only. Runs `./gradlew build` (unit tests + Spotless + JaCoCo) and uploads coverage to Codecov. **Does not** run integration tests — those are handled by the on-demand workflow below. + - `.github/workflows/main.yml` — runs only on `push` to `main`. Two jobs: `verify` does the full forward-compat matrix `{17, 21, 25}` for unit tests via `-PtestJdk=N`; `integration-tests` does a parallel matrix `{17, 21, 25}` against the live API. Both are mandatory for the merge to be considered successful. The JDK 17 matrix entry of `verify` also uploads coverage to Codecov as the new baseline that PRs compare against. `integration-tests` fails the build if `MARKETDATA_TOKEN` secret is absent (it is required on main). + - `.github/workflows/pr-matrix-on-demand.yml` — manually triggered on a PR by commenting `/run-all-jdks`, `/jdk-matrix`, or `/test-all`. Runs the **unit-test** matrix on JDK 21 and 25 (17 already ran via `pull-request.yml`). Gated to write/maintain/admin commenters. Reacts 👀 to the trigger comment and posts a result summary. + - `.github/workflows/pr-integration-on-demand.yml` — manually triggered on a PR by commenting `integrationtest` (JDK 17 only) or `integrationtestfull` (matrix `{17, 21, 25}`). Runs the **integration-test** suite against the live API. Same write+ permission gate as the matrix-on-demand workflow. Aggregates the matrix outcome into a single required check named **"Integration tests pass"** so branch protection can require it uniformly regardless of which command was used. Branch-protection rules on `main` should list this check as required for merge. + - All four `issue_comment`-driven workflows execute from the default branch's copy of their YAML, not the PR's. Feature-branch edits to these workflows take effect only after merge to main. + - `-PtestJdk=N` is wired to **all** `Test` tasks (`test` and `integrationTest`) via `tasks.withType().configureEach { javaLauncher.set(...) }` in `build.gradle.kts`, so the matrix flag works uniformly across unit and integration tests. - Coverage ratchet lives in `codecov.yml`: project status with `target: auto, threshold: 5%` (cannot drop >5 pp vs base branch) plus a patch-coverage requirement of 70 % on new code. Requires a `CODECOV_TOKEN` repo secret — without it the upload step fails because workflows pass `fail_ci_if_error: true`. **Deliberately deferred (require the request/endpoint layer to land first):** @@ -84,10 +87,17 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements]( - §9 retry/backoff policy and `/status/` cache workflow. - §12 acquire/release of the concurrency semaphore around dispatched requests. - §13 100% coverage threshold via JaCoCo `violationRules`; deferred until there is functional code worth the threshold. -- §13 integration-test CI job: stubbed at the bottom of `ci.yml` with a comment. Gating on a `MARKETDATA_TOKEN` GitHub Actions secret; will be wired up once the secret exists. When picking up new work, check this list before reaching for the SDK requirements doc — most foundational rules are already encoded in code; missing pieces are deferred deliberately, not by accident. +**Known latent gaps to revisit when retry/timeout lands:** +- `HttpTransport.executeSync` only catches `CompletionException` from `.join()`, not `CancellationException`. Today the latter is unreachable — the user can't cancel a future they never see (the future is local to `executeSync`), no internal code cancels it, and `dispatch`'s `handle((response, error) -> ...)` translates every upstream error (including a hypothetical `CancellationException` from `sendAsync`) into `CompletionException(NetworkError)`. The gap becomes real once we add: + - `dispatched.orTimeout(99s)` / `completeOnTimeout` to enforce the §10 timeout strictly (these produce `CancellationException` on the downstream future). + - A retry coordinator (§9) that cancels in-flight futures when aborting a retry chain. + - A bump to JDK 21+ where `HttpClient.close()` cancels in-flight futures. + When any of those land, extend the catch in `executeSync` (or fold it into `asRuntime`) so cancellations don't escape as raw `RuntimeException` to sync callers. Tracked as Issue #2 of the 2026-05-11 review (`REVIEW-2026-05-11-markets-status.md`). +- `HttpTransport.buildUri` URL-encodes query-param values with `URLEncoder.encode(..., UTF_8)`, which is form-encoding semantics: spaces become `+`, not `%20`. Fine for today's typed params (dates, numerics) but a future endpoint that takes an arbitrary string (e.g. `symbol="BRK A"`) would round-trip differently against an RFC-3986-strict server. Switch to a path/query-segment-aware encoder when the first such param lands. Tracked as Issue #10 of the 2026-05-11 review. + ## Acceptance checklist `docs/java-sdk-requirements.md` ends with an "Acceptance Checklist" mapping each Java-specific requirements section to verifiable items. Treat it as the definition of done for v1: when implementing, work toward making each box checkable, and use it as a self-review pass before declaring a section complete. diff --git a/README.md b/README.md index 7d8bfd5..ee2f4c0 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,17 @@ install — the wrapper downloads the right Gradle version on first run. MARKETDATA_RUN_INTEGRATION_TESTS=true ./gradlew integrationTest ``` +On PRs, integration tests are not run automatically (live-API quota + +CI minutes). A reviewer with `write` access triggers them by posting a +slash-command on the **first line** of a PR comment: + +- `/integrationtest` — JDK 17 only. +- `/integrationtestfull` — full matrix `{17, 21, 25}`. + +The first-line rule means quoted replies (`> /integrationtest`) and +prose that merely mentions the command do not fire a run. Anything that +isn't an exact match is rejected before any request is made. + ## Package layout ``` diff --git a/build.gradle.kts b/build.gradle.kts index 995403f..24a5a70 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -41,7 +41,12 @@ tasks.jar { } // ADR-003: integration tests live in a separate, env-var-gated source set. -val integrationTest by sourceSets.creating +val integrationTest by sourceSets.creating { + // Wire the main and unit-test outputs into the integration test classpath + // so ITs can use both production code and test helpers (junit, assertj). + compileClasspath += sourceSets.main.get().output + sourceSets.test.get().output + runtimeClasspath += output + compileClasspath +} val integrationTestImplementation by configurations.getting { extendsFrom(configurations.testImplementation.get()) @@ -81,18 +86,19 @@ dependencies { tasks.test { useJUnitPlatform() finalizedBy(tasks.jacocoTestReport) +} - // ADR-002 CI matrix: optionally run tests on a specific JDK while - // compilation stays pinned to --release 17. The CI workflow passes - // -PtestJdk=17|21|25; locally you can do ./gradlew test -PtestJdk=21 - // (Gradle will provision the JDK via the foojay resolver if missing). - val testJdk = providers.gradleProperty("testJdk").orNull - if (testJdk != null) { - javaLauncher.set( - javaToolchains.launcherFor { - languageVersion = JavaLanguageVersion.of(testJdk.toInt()) - } - ) +// ADR-002 CI matrix: optionally run any Test task on a specific JDK +// while compilation stays pinned to --release 17. The flag is wired to +// every Test task (unit `test` + `integrationTest`) so on-demand and +// merge-to-main matrix runs cover the live API on JDK 17/21/25 too. +val testJdkProperty = providers.gradleProperty("testJdk").orNull +if (testJdkProperty != null) { + val launcher = javaToolchains.launcherFor { + languageVersion = JavaLanguageVersion.of(testJdkProperty.toInt()) + } + tasks.withType().configureEach { + javaLauncher.set(launcher) } } @@ -104,6 +110,30 @@ tasks.jacocoTestReport { } } +// Aggregate coverage across unit tests and integration tests. Opt-in: not +// wired into `check` so PR builds stay fast and don't require the IT secret. +// Invoke as `MARKETDATA_RUN_INTEGRATION_TESTS=true ./gradlew jacocoAggregateReport`. +tasks.register("jacocoAggregateReport") { + description = "Generates a JaCoCo report aggregating unit + integration test coverage." + group = "verification" + + dependsOn(tasks.test, integrationTestTask) + + sourceSets(sourceSets.main.get()) + executionData( + fileTree(layout.buildDirectory.dir("jacoco")) { + include("*.exec") + }, + ) + + reports { + xml.required = true + html.required = true + html.outputLocation = layout.buildDirectory.dir("reports/jacoco/aggregate/html") + xml.outputLocation = layout.buildDirectory.file("reports/jacoco/aggregate/jacoco.xml") + } +} + // Coverage ratchet (line coverage cannot drop more than 5 pp below // main's last value) is enforced in CI — see .github/workflows/pull-request.yml // and .github/scripts/check-coverage-delta.py. Not enforced locally so that diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index a4b76b9..8bdaf60 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index cea7a79..2a84e18 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.12-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-9.0.0-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/gradlew b/gradlew index f3b75f3..ef07e01 100755 --- a/gradlew +++ b/gradlew @@ -1,7 +1,7 @@ #!/bin/sh # -# Copyright © 2015-2021 the original authors. +# Copyright © 2015 the original authors. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -114,7 +114,7 @@ case "$( uname )" in #( NONSTOP* ) nonstop=true ;; esac -CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar +CLASSPATH="\\\"\\\"" # Determine the Java command to use to start the JVM. @@ -205,7 +205,7 @@ fi DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' # Collect all arguments for the java command: -# * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments, +# * DEFAULT_JVM_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments, # and any embedded shellness will be escaped. # * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be # treated as '${Hostname}' itself on the command line. @@ -213,7 +213,7 @@ DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' set -- \ "-Dorg.gradle.appname=$APP_BASE_NAME" \ -classpath "$CLASSPATH" \ - org.gradle.wrapper.GradleWrapperMain \ + -jar "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" \ "$@" # Stop when "xargs" is not available. diff --git a/gradlew.bat b/gradlew.bat index 9b42019..5eed7ee 100644 --- a/gradlew.bat +++ b/gradlew.bat @@ -70,11 +70,11 @@ goto fail :execute @rem Setup the command line -set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar +set CLASSPATH= @rem Execute Gradle -"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %* +"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* :end @rem End local scope for the variables with windows NT shell diff --git a/src/integrationTest/java/com/marketdata/sdk/AsyncSemaphoreIT.java b/src/integrationTest/java/com/marketdata/sdk/AsyncSemaphoreIT.java new file mode 100644 index 0000000..0676736 --- /dev/null +++ b/src/integrationTest/java/com/marketdata/sdk/AsyncSemaphoreIT.java @@ -0,0 +1,59 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.marketdata.sdk.markets.MarketStatus; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +/** + * Concurrency integration test against the live Market Data API. Verifies that the {@link + * AsyncSemaphore} + {@link HttpTransport} pipeline correctly handles fan-out beyond the pool size: + * the requests over the limit must traverse the semaphore's slow path (queue the waiter, complete + * it later via {@code release}) without deadlocking or losing a permit. + * + *

Costs {@code CONCURRENCY_LIMIT + 5 = 55} requests against the live {@code /markets/status/} + * endpoint per run. With a typical RTT of ~100 ms and pool size 50, the test wall time is well + * under a second. + * + *

Gated by {@code MARKETDATA_RUN_INTEGRATION_TESTS=true} like the rest of this source set. + */ +class AsyncSemaphoreIT { + + /** + * If a permit ever leaked or the slow-path queue stopped being drained, {@code allOf.join()} + * would block forever. The 30 s timeout fails the test fast instead of leaving CI hung. + */ + @Test + @Timeout(value = 30, unit = TimeUnit.SECONDS) + void concurrentFanOutBeyondPoolLimitCompletesWithoutDeadlock() { + try (var client = new MarketDataClient(null, null, null, false)) { + int n = HttpTransport.CONCURRENCY_LIMIT + 5; + List> futures = new ArrayList<>(n); + + // Fire all N requests as fast as the loop runs. With pool=50, the first 50 take the + // fast path (already-completed acquire future) and dispatch immediately; requests + // 51..55 take the slow path and enqueue waiters that complete only when one of the + // first 50 releases. + for (int i = 0; i < n; i++) { + futures.add(client.markets().statusAsync()); + } + + // allOf.join() throws on any underlying failure; we let it propagate so a 429 / network + // hiccup surfaces as a real test failure rather than silently masking the issue. + CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join(); + + // Every response must be a valid MarketStatus. Empty results would suggest a hidden + // failure (auth issue, rate limit) that wasn't observable from allOf alone. + for (CompletableFuture f : futures) { + MarketStatus status = f.join(); + assertThat(status.days()).isNotEmpty(); + assertThat(status.days().get(0).date()).isNotNull(); + } + } + } +} diff --git a/src/integrationTest/java/com/marketdata/sdk/MarketsStatusIT.java b/src/integrationTest/java/com/marketdata/sdk/MarketsStatusIT.java new file mode 100644 index 0000000..3fe05ee --- /dev/null +++ b/src/integrationTest/java/com/marketdata/sdk/MarketsStatusIT.java @@ -0,0 +1,52 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.marketdata.sdk.markets.MarketStatus; +import java.time.LocalDate; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +/** + * Integration test against the live Market Data API. Gated by the {@code integrationTest} source + * set, which itself only runs when {@code MARKETDATA_RUN_INTEGRATION_TESTS=true} is exported (see + * {@code build.gradle.kts}). + * + *

Requires a valid {@code MARKETDATA_TOKEN} env var (or {@code .env} entry). Without one the + * client enters demo mode and the {@code /markets/status/} endpoint is not on the demo allow-list, + * so the test would receive an {@code AuthenticationError}. + * + *

Each scenario runs once for {@link CallMode#SYNC} and once for {@link CallMode#ASYNC} so we + * satisfy SDK requirements §13's "tests must cover both sync and async variants for every endpoint" + * against the real wire. + */ +class MarketsStatusIT { + + @ParameterizedTest + @EnumSource(CallMode.class) + void todayStatusReturnsAtLeastOneEntry(CallMode mode) { + try (var client = new MarketDataClient(null, null, null, false)) { + MarketStatus status = mode.statusNoArgs(client.markets()); + + // The endpoint always returns at least one entry for "today" — even on weekends/holidays + // there's a row with status="closed". + assertThat(status.days()).isNotEmpty(); + assertThat(status.days().get(0).date()).isNotNull(); + } + } + + @ParameterizedTest + @EnumSource(CallMode.class) + void historicalRangeReturnsExpectedDays(CallMode mode) { + LocalDate from = LocalDate.now().minusDays(7); + LocalDate to = LocalDate.now().minusDays(1); + + try (var client = new MarketDataClient(null, null, null, false)) { + MarketStatus status = mode.statusForRange(client.markets(), from, to); + + assertThat(status.days()).hasSizeBetween(1, 7); + assertThat(status.days()) + .allSatisfy(d -> assertThat(d.date()).isBetween(from.minusDays(1), to.plusDays(1))); + } + } +} diff --git a/src/integrationTest/java/com/marketdata/sdk/PlaceholderIT.java b/src/integrationTest/java/com/marketdata/sdk/PlaceholderIT.java deleted file mode 100644 index b336e45..0000000 --- a/src/integrationTest/java/com/marketdata/sdk/PlaceholderIT.java +++ /dev/null @@ -1,16 +0,0 @@ -package com.marketdata.sdk; - -import org.junit.jupiter.api.Test; - -/** - * Placeholder so the {@code integrationTest} source set compiles before any real integration tests - * exist. Replace with live-API tests in subsequent iterations. Gated by {@code - * MARKETDATA_RUN_INTEGRATION_TESTS=true}. - */ -class PlaceholderIT { - - @Test - void sourceSetCompiles() { - // Intentionally empty. - } -} diff --git a/src/main/java/com/marketdata/sdk/AsyncSemaphore.java b/src/main/java/com/marketdata/sdk/AsyncSemaphore.java new file mode 100644 index 0000000..7945108 --- /dev/null +++ b/src/main/java/com/marketdata/sdk/AsyncSemaphore.java @@ -0,0 +1,102 @@ +package com.marketdata.sdk; + +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.concurrent.CompletableFuture; + +/** + * Async-safe concurrency limiter. Replaces {@link java.util.concurrent.Semaphore} in the HTTP path + * so that {@code executeAsync} never parks the caller's thread when the pool is at capacity — it + * returns a {@link CompletableFuture} that completes when a permit is released by an in-flight + * request. See ADR-007 for the rationale. + * + *

Two invariants: + * + *

    + *
  1. Every permit is accounted for exactly once — it is either in {@link #availablePermits()} + * (free), held by an in-flight caller (and will be released via {@link #release()}), or + * pending in the waiter queue (and will be released by completing the waiter's future). + *
  2. {@link CompletableFuture#complete} of a transferred permit always runs outside the + * lock. Completing a future runs the caller's attached callbacks synchronously on the + * releasing thread, and we never want those running while our lock is held. + *
+ * + *

Cancelled or otherwise-completed waiters are skipped on {@link #release()} so a cancelled + * {@code acquire} doesn't burn a permit. + */ +final class AsyncSemaphore { + + private final Object lock = new Object(); + private final Deque> waiters = new ArrayDeque<>(); + private int available; + + AsyncSemaphore(int permits) { + if (permits < 0) { + throw new IllegalArgumentException("permits must be >= 0, was " + permits); + } + this.available = permits; + } + + /** + * Asynchronously claim a permit. + * + *

Fast path: a permit is available, returns an already-completed future. Slow path: pool is + * exhausted, returns a pending future enqueued FIFO; it completes when some in-flight caller + * calls {@link #release()}. Either way, the caller's thread is never parked. + */ + CompletableFuture acquire() { + synchronized (lock) { + if (available > 0) { + available--; + return CompletableFuture.completedFuture(null); + } + CompletableFuture waiter = new CompletableFuture<>(); + waiters.addLast(waiter); + return waiter; + } + } + + /** + * Release a permit. If a live waiter is enqueued, the permit is transferred to it (its future is + * completed) without going through the counter. Otherwise the counter is incremented. + */ + void release() { + // Outer loop handles the TOCTOU window between pollFirst (inside the lock) and + // complete (outside): if the waiter is cancelled in that gap, complete(null) returns + // false and the permit hasn't actually been transferred. Retry with the next waiter, + // or fall through to the counter when the queue runs out of live waiters. + while (true) { + CompletableFuture next = null; + synchronized (lock) { + while (!waiters.isEmpty()) { + CompletableFuture w = waiters.pollFirst(); + if (!w.isDone()) { + next = w; + break; + } + } + if (next == null) { + available++; + return; + } + } + if (next.complete(null)) { + return; + } + } + } + + /** Permits not currently held nor pending in the queue. */ + int availablePermits() { + synchronized (lock) { + return available; + } + } + + /** Number of pending waiters on the slow path. Useful for diagnostics and tests. */ + int queueLength() { + synchronized (lock) { + return waiters.size(); + } + } +} diff --git a/src/main/java/com/marketdata/sdk/HttpStatusMapper.java b/src/main/java/com/marketdata/sdk/HttpStatusMapper.java new file mode 100644 index 0000000..dff9f0f --- /dev/null +++ b/src/main/java/com/marketdata/sdk/HttpStatusMapper.java @@ -0,0 +1,37 @@ +package com.marketdata.sdk; + +import com.marketdata.sdk.exception.AuthenticationError; +import com.marketdata.sdk.exception.BadRequestError; +import com.marketdata.sdk.exception.ErrorContext; +import com.marketdata.sdk.exception.MarketDataException; +import com.marketdata.sdk.exception.RateLimitError; +import com.marketdata.sdk.exception.ServerError; +import org.jspecify.annotations.Nullable; + +/** + * Maps an HTTP status code to the {@link MarketDataException} subtype the SDK requirements doc §9.1 + * mandates. + * + *

Note that 200 / 203 (success) and 404 (no-data sentinel returned by the API as {@code + * {"s":"no_data"}}) are not handled here — those status codes mean "got a body, + * decode it" and the resource layer interprets them. This mapper only fires on hard failures. + */ +final class HttpStatusMapper { + + private HttpStatusMapper() {} + + static MarketDataException toException( + int status, String requestUrl, @Nullable String requestId) { + ErrorContext ctx = new ErrorContext(emptyToNull(requestId), requestUrl, status); + return switch (status) { + case 400, 422 -> new BadRequestError("HTTP " + status + ": invalid request", ctx); + case 401 -> new AuthenticationError("HTTP 401: invalid or missing API token", ctx); + case 429 -> new RateLimitError("HTTP 429: rate limit exceeded", ctx); + default -> new ServerError("HTTP " + status + ": server error", ctx); + }; + } + + private static @Nullable String emptyToNull(@Nullable String s) { + return (s == null || s.isBlank()) ? null : s; + } +} diff --git a/src/main/java/com/marketdata/sdk/HttpTransport.java b/src/main/java/com/marketdata/sdk/HttpTransport.java new file mode 100644 index 0000000..fc4692f --- /dev/null +++ b/src/main/java/com/marketdata/sdk/HttpTransport.java @@ -0,0 +1,297 @@ +package com.marketdata.sdk; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.marketdata.sdk.exception.ErrorContext; +import com.marketdata.sdk.exception.MarketDataException; +import com.marketdata.sdk.exception.NetworkError; +import com.marketdata.sdk.exception.ParseError; +import com.marketdata.sdk.markets.MarketStatus; +import java.io.IOException; +import java.net.URI; +import java.net.URLEncoder; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.Map; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import org.jspecify.annotations.Nullable; + +/** + * The single point of contact between resource façades and the network. + * + *

Owned by {@link MarketDataClient}, instantiated once per client. All HTTP-shaped concerns live + * here so resources never see a {@link HttpClient}, an {@link ObjectMapper}, the concurrency + * semaphore, or the rate-limit headers — they get a {@link RequestSpec} in and a typed domain + * object out. + * + *

Per ADR-006 the design is async-first: {@link #executeAsync} is the canonical path; {@link + * #executeSync} is a thin wrapper that calls {@link CompletableFuture#join()} and unwraps any + * {@link CompletionException} so the caller sees the underlying cause directly. + * + *

Per ADR-007 wire-format deserializers are registered programmatically on the {@link + * ObjectMapper} via a {@link SimpleModule}, so response records do not carry + * {@code @JsonDeserialize} annotations. + */ +final class HttpTransport implements AutoCloseable { + + /** SDK requirements §10: fixed 99-second per-request timeout. */ + static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(99); + + /** SDK requirements §10: fixed 2-second connect timeout. */ + static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(2); + + /** SDK requirements §12: 50-permit global concurrency pool. */ + static final int CONCURRENCY_LIMIT = 50; + + private static final String CF_RAY = "cf-ray"; + + private final HttpClient httpClient; + private final ObjectMapper jsonMapper; + private final AsyncSemaphore concurrencyPermits; + private final AtomicReference<@Nullable RateLimits> latestRateLimits = new AtomicReference<>(); + + private final String baseUrl; + private final String apiVersion; + private final String userAgent; + private final @Nullable String token; + + HttpTransport(String baseUrl, String apiVersion, String userAgent, @Nullable String token) { + this(baseUrl, apiVersion, userAgent, token, defaultHttpClient()); + } + + // Package-private constructor used by tests to inject a stubbed HttpClient + // (e.g. one whose sendAsync throws synchronously, to verify permit release). + HttpTransport( + String baseUrl, + String apiVersion, + String userAgent, + @Nullable String token, + HttpClient httpClient) { + this.baseUrl = baseUrl; + this.apiVersion = apiVersion; + this.userAgent = userAgent; + this.token = token; + this.concurrencyPermits = new AsyncSemaphore(CONCURRENCY_LIMIT); + this.jsonMapper = buildJsonMapper(); + this.httpClient = httpClient; + } + + private static HttpClient defaultHttpClient() { + return HttpClient.newBuilder() + .connectTimeout(CONNECT_TIMEOUT) + .version(HttpClient.Version.HTTP_2) + .followRedirects(HttpClient.Redirect.NORMAL) + .build(); + } + + /** + * Latest client-level rate-limit snapshot, or {@code null} if the client has not yet received a + * response that carried parseable {@code x-api-ratelimit-*} headers. Once populated, the snapshot + * reflects the most recent rate-limit-bearing response — successful responses that arrive without + * headers do not reset it. + */ + @Nullable RateLimits getLatestRateLimits() { + return latestRateLimits.get(); + } + + /** + * Async-first request execution. + * + *

Acquires a concurrency permit, fires the request, parses rate-limit headers, decodes the + * body when the status is 200/203/404 (the API returns 404 with {@code {"s":"no_data"}} as a + * sentinel — see SDK requirements §9.1), and translates other status codes to the appropriate + * {@link MarketDataException} subtype. + */ + CompletableFuture executeAsync(RequestSpec spec, Class responseType) { + URI uri = buildUri(spec); + HttpRequest request = buildRequest(uri); + + // ADR-007: acquire returns a CompletableFuture instead of parking the caller's thread. + // When permits are available the future is already completed (fast path) and thenCompose + // runs synchronously; when the pool is exhausted the future completes later, on the + // thread that calls release() — the caller's thread is never blocked here. + CompletableFuture permit = concurrencyPermits.acquire(); + CompletableFuture dispatched = + permit.thenCompose(unused -> dispatch(uri, request, responseType)); + + // Cancellation of `dispatched` doesn't propagate to `permit` by default, so a slow-path + // waiter would stay live in the semaphore queue; release() would later "transfer" the + // permit by completing the waiter, but thenCompose's function wouldn't run (its + // dependent is already cancelled), and dispatch — which registers whenComplete(release) + // — would never fire. Cancelling `permit` here makes AsyncSemaphore.release skip the + // waiter. The narrow race where the waiter is cancelled between release()'s pollFirst + // and complete() is handled inside release() itself by retrying. + dispatched.whenComplete( + (r, t) -> { + if (t instanceof CancellationException) { + permit.cancel(false); + } + }); + + return dispatched; + } + + private CompletableFuture dispatch(URI uri, HttpRequest request, Class responseType) { + CompletableFuture> sendFuture; + try { + sendFuture = httpClient.sendAsync(request, BodyHandlers.ofByteArray()); + } catch (Throwable t) { + // sendAsync threw synchronously (e.g. malformed request, internal NPE, OOM). + // The future never formed, so whenComplete will not fire — release the permit + // here to prevent a permanent leak that would degrade the pool to deadlock. + concurrencyPermits.release(); + if (t instanceof Error err) { + throw err; + } + return CompletableFuture.failedFuture( + new NetworkError( + "Request to " + uri + " failed before dispatch: " + t.getMessage(), + new ErrorContext(null, uri.toString(), null), + t)); + } + + return sendFuture + .whenComplete((r, t) -> concurrencyPermits.release()) + .handle( + (response, error) -> { + if (error != null) { + Throwable root = unwrap(error); + throw new CompletionException( + new NetworkError( + "Request to " + uri + " failed: " + root.getMessage(), + new ErrorContext(null, uri.toString(), null), + root)); + } + // Only overwrite the snapshot when the response carried parseable rate-limit + // headers. The API's rate-limit middleware can silently swallow its own errors + // and respond without headers; clobbering with null on every such response would + // make `client.getRateLimits()` flicker between populated and null across + // consecutive calls. Spec §8 says "update client-level snapshot" — implicitly only + // when there is something to update. + RateLimits parsed = RateLimitHeaders.parse(response.headers()); + if (parsed != null) { + latestRateLimits.set(parsed); + } + return processResponse(response, responseType, uri.toString()); + }); + } + + /** + * Sync wrapper around {@link #executeAsync}. Per ADR-006, calls {@code .join()} and unwraps + * {@link CompletionException} so callers see the underlying {@link MarketDataException} directly. + */ + T executeSync(RequestSpec spec, Class responseType) { + try { + return executeAsync(spec, responseType).join(); + } catch (CompletionException e) { + throw asRuntime(e.getCause()); + } + } + + // Visible for tests: under our current SDK design, executeAsync always wraps failures as + // MarketDataException so the `MDE` branch is the only one reached from the public surface. + // The other two branches are defensive guardrails — extracted so they can be exercised + // directly by tests rather than relying on a synthetic public-API path. + static RuntimeException asRuntime(@Nullable Throwable cause) { + if (cause instanceof MarketDataException mde) { + return mde; + } + if (cause instanceof RuntimeException re) { + return re; + } + return new NetworkError("Unexpected failure invoking SDK", ErrorContext.empty(), cause); + } + + @Override + public void close() { + // java.net.http.HttpClient gained explicit close() in JDK 21; until + // the SDK's minimum bumps to 21+ this is a no-op (ADR-002). + } + + private T processResponse(HttpResponse response, Class responseType, String url) { + int status = response.statusCode(); + String requestId = response.headers().firstValue(CF_RAY).orElse(null); + + // 200 OK + 203 Non-Authoritative + 404 (with {"s":"no_data"} body) all + // carry a JSON payload the resource wants to decode. Other statuses + // mean we never got a usable body — translate to a typed exception. + if (status == 200 || status == 203 || status == 404) { + try { + return jsonMapper.readValue(response.body(), responseType); + } catch (IOException e) { + throw new ParseError( + "Failed to decode response from " + url + ": " + e.getMessage(), + new ErrorContext(requestId, url, status), + e); + } + } + throw HttpStatusMapper.toException(status, url, requestId); + } + + private URI buildUri(RequestSpec spec) { + // RequestSpec's Javadoc says path has no leading slash, but a caller mistake would produce + // baseUrl/v1//markets/status (double slash). Strip defensively so the URL stays well-formed + // regardless of which side of the contract the bug is on. + String path = spec.path(); + if (path.startsWith("/")) { + path = path.substring(1); + } + StringBuilder sb = new StringBuilder(); + sb.append(baseUrl).append('/').append(apiVersion).append('/').append(path); + if (!path.endsWith("/")) { + sb.append('/'); + } + Map params = spec.queryParams(); + if (!params.isEmpty()) { + sb.append('?'); + boolean first = true; + for (Map.Entry e : params.entrySet()) { + if (!first) { + sb.append('&'); + } + sb.append(URLEncoder.encode(e.getKey(), StandardCharsets.UTF_8)) + .append('=') + .append(URLEncoder.encode(e.getValue(), StandardCharsets.UTF_8)); + first = false; + } + } + return URI.create(sb.toString()); + } + + private HttpRequest buildRequest(URI uri) { + HttpRequest.Builder b = + HttpRequest.newBuilder(uri) + .GET() + .timeout(REQUEST_TIMEOUT) + .header("User-Agent", userAgent) + .header("Accept", "application/json"); + if (token != null) { + b.header("Authorization", "Bearer " + token); + } + return b.build(); + } + + /** + * Builds the {@link ObjectMapper} used to decode every wire body. Per ADR-007 the wire-format + * deserializers register here, not via annotations on the response records. + */ + private static ObjectMapper buildJsonMapper() { + ObjectMapper mapper = new ObjectMapper(); + SimpleModule wireModule = new SimpleModule("marketdata-wire"); + wireModule.addDeserializer(MarketStatus.class, new MarketStatusDeserializer()); + mapper.registerModule(wireModule); + return mapper; + } + + // Package-private so the unwrap-when-nested-and-when-not branches are reachable from tests. + static Throwable unwrap(Throwable t) { + return (t instanceof CompletionException && t.getCause() != null) ? t.getCause() : t; + } +} diff --git a/src/main/java/com/marketdata/sdk/MarketDataClient.java b/src/main/java/com/marketdata/sdk/MarketDataClient.java index 4447806..85cc821 100644 --- a/src/main/java/com/marketdata/sdk/MarketDataClient.java +++ b/src/main/java/com/marketdata/sdk/MarketDataClient.java @@ -1,9 +1,6 @@ package com.marketdata.sdk; -import java.net.http.HttpClient; import java.time.Duration; -import java.util.concurrent.Semaphore; -import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import org.jspecify.annotations.Nullable; @@ -11,9 +8,10 @@ /** * Entry point to the Market Data Java SDK. * - *

One {@code MarketDataClient} per application. Holds a single shared {@link HttpClient} - * (HTTP/2, 2 s connect timeout) for connection pooling and a 50-permit semaphore that gates the - * global concurrency pool required by SDK requirements §12. + *

One {@code MarketDataClient} per application. Resource façades (e.g. {@link #markets()}) are + * accessed through the client; all HTTP-shaped concerns (connection pooling, HTTP/2, the global + * concurrency semaphore, rate-limit header parsing) live in the internal {@link HttpTransport} the + * client owns. * *

Two constructors: * @@ -32,19 +30,17 @@ public final class MarketDataClient implements AutoCloseable { /** SDK requirements §10: fixed 99-second per-request timeout. */ - public static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(99); + public static final Duration REQUEST_TIMEOUT = HttpTransport.REQUEST_TIMEOUT; /** SDK requirements §10: fixed 2-second connect timeout. */ - public static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(2); + public static final Duration CONNECT_TIMEOUT = HttpTransport.CONNECT_TIMEOUT; /** SDK requirements §12: maximum concurrent in-flight requests per client. */ - public static final int CONCURRENCY_LIMIT = 50; + public static final int CONCURRENCY_LIMIT = HttpTransport.CONCURRENCY_LIMIT; private static final Logger LOG = Logger.getLogger(MarketDataClient.class.getName()); - private final HttpClient httpClient; - private final Semaphore concurrencyPermits; - private final AtomicReference<@Nullable RateLimits> latestRateLimits = new AtomicReference<>(); + private final HttpTransport transport; private final @Nullable String token; private final String baseUrl; @@ -53,6 +49,9 @@ public final class MarketDataClient implements AutoCloseable { private final boolean demoMode; private final boolean validateOnStartup; + // Resources — eagerly constructed; one record-shaped object per resource group. + private final MarketsResource markets; + /** * Production constructor. Resolves all settings from the configuration cascade in SDK * requirements §4 (env var → {@code .env} → built-in default) and enables startup validation. @@ -95,13 +94,8 @@ public MarketDataClient( this.validateOnStartup = validateOnStartup; this.userAgent = "marketdata-sdk-java/" + Version.current(); - this.httpClient = - HttpClient.newBuilder() - .connectTimeout(CONNECT_TIMEOUT) - .version(HttpClient.Version.HTTP_2) - .followRedirects(HttpClient.Redirect.NORMAL) - .build(); - this.concurrencyPermits = new Semaphore(CONCURRENCY_LIMIT); + this.transport = new HttpTransport(this.baseUrl, this.apiVersion, this.userAgent, this.token); + this.markets = new MarketsResource(this.transport); LOG.log( Level.INFO, @@ -109,16 +103,29 @@ public MarketDataClient( new Object[] {Version.current(), this.baseUrl, this.apiVersion, this.demoMode}); if (this.demoMode) { LOG.warning( - "No API token provided — running in demo mode. Authenticated endpoints will" - + " fail; rate-limit initialization is skipped."); + "No API token provided — running in demo mode. Authenticated endpoints will fail with" + + " AuthenticationError on first call."); } else if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Token: {0}", Tokens.redact(this.token)); } // SDK requirements §5: validate on startup by default. The actual - // /user/ call lands with the request layer; this flag is the seam. + // /user/ call lands with the user resource; this flag is the seam. + } + + // --------------------------------------------------------------------- + // Resource accessors + // --------------------------------------------------------------------- + + /** Façade for the {@code /v1/markets/*} endpoint group. */ + public MarketsResource markets() { + return markets; } + // --------------------------------------------------------------------- + // Configuration accessors + // --------------------------------------------------------------------- + public String getBaseUrl() { return baseUrl; } @@ -139,17 +146,19 @@ public boolean isValidateOnStartup() { return validateOnStartup; } - /** Latest client-level rate-limit snapshot, or {@code null} if none has been received yet. */ + /** + * Latest client-level rate-limit snapshot, or {@code null} if no rate-limit-bearing response has + * been received yet. Once populated, the snapshot persists across subsequent calls — a successful + * response that arrives without {@code x-api-ratelimit-*} headers (e.g. during a server-side + * middleware outage) does not clear it. + */ public @Nullable RateLimits getRateLimits() { - return latestRateLimits.get(); + return transport.getLatestRateLimits(); } @Override public void close() { - // java.net.http.HttpClient gained explicit close() in JDK 21. - // While the minimum target is JDK 17, this method is a no-op: - // the JVM releases the executor and connection pool on process - // exit. Revisit if/when the minimum bumps to 21+. + transport.close(); } private static String trimTrailingSlash(String url) { diff --git a/src/main/java/com/marketdata/sdk/MarketStatusDeserializer.java b/src/main/java/com/marketdata/sdk/MarketStatusDeserializer.java new file mode 100644 index 0000000..5dd4815 --- /dev/null +++ b/src/main/java/com/marketdata/sdk/MarketStatusDeserializer.java @@ -0,0 +1,81 @@ +package com.marketdata.sdk; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; +import com.marketdata.sdk.markets.DailyStatus; +import com.marketdata.sdk.markets.MarketStatus; +import java.io.IOException; +import java.time.Instant; +import java.time.LocalDate; +import java.time.ZoneId; +import java.util.ArrayList; +import java.util.List; + +/** + * Jackson deserializer for the {@code /v1/markets/status/} parallel-arrays wire format. + * + *

Wire shape (success): + * + * 

{@code
+ * { "s": "ok",
+ *   "date":   [1706745600, 1706832000, 1706918400],
+ *   "status": ["open", "open", "closed"] }
+ * }
+ * + *

Wire shape (no data, also returned for non-US countries by design): + * + * 

{@code
+ * { "s": "no_data" }
+ * }
+ * + *

The deserializer expands the parallel arrays into a list of {@link DailyStatus} (one per + * index), normalizes the unix timestamps to {@link LocalDate} in US/Eastern (SDK requirements + * §11.4), and represents {@code "no_data"} as an empty list. + */ +final class MarketStatusDeserializer extends JsonDeserializer { + + private static final ZoneId EASTERN = ZoneId.of("America/New_York"); + private static final String STATUS_OK = "ok"; + private static final String STATUS_NO_DATA = "no_data"; + + @Override + public MarketStatus deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { + JsonNode root = p.readValueAsTree(); + String s = root.path("s").asText(""); + + if (STATUS_NO_DATA.equals(s)) { + return new MarketStatus(List.of()); + } + if (!STATUS_OK.equals(s)) { + throw new IOException( + "Unexpected status field in /markets/status response: '" + + s + + "' (expected 'ok' or 'no_data')"); + } + + JsonNode dates = root.path("date"); + JsonNode statuses = root.path("status"); + if (!dates.isArray() || !statuses.isArray()) { + throw new IOException( + "Malformed /markets/status response: expected 'date' and 'status' arrays"); + } + if (dates.size() != statuses.size()) { + throw new IOException( + "Malformed /markets/status response: 'date' and 'status' arrays have different sizes (" + + dates.size() + + " vs " + + statuses.size() + + ")"); + } + + List days = new ArrayList<>(dates.size()); + for (int i = 0; i < dates.size(); i++) { + LocalDate date = Instant.ofEpochSecond(dates.get(i).asLong()).atZone(EASTERN).toLocalDate(); + boolean open = "open".equalsIgnoreCase(statuses.get(i).asText()); + days.add(new DailyStatus(date, open)); + } + return new MarketStatus(List.copyOf(days)); + } +} diff --git a/src/main/java/com/marketdata/sdk/MarketsResource.java b/src/main/java/com/marketdata/sdk/MarketsResource.java new file mode 100644 index 0000000..13af185 --- /dev/null +++ b/src/main/java/com/marketdata/sdk/MarketsResource.java @@ -0,0 +1,91 @@ +package com.marketdata.sdk; + +import com.marketdata.sdk.markets.MarketStatus; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; +import java.util.concurrent.CompletableFuture; + +/** + * Façade for the {@code /v1/markets/*} endpoint group. + * + *

Per ADR-006 every endpoint exposes a sync and an {@code …Async} variant. Both share the same + * request-building code; the sync forms are thin wrappers around the async path. + * + *

Per ADR-007 this resource lives in the SDK root package alongside the infra it depends on + * ({@link HttpTransport}, {@link RequestSpec}). Its constructor is package-private so only {@link + * MarketDataClient} can build one — consumers reach it via {@link MarketDataClient#markets()}. + * + *

Currently only {@code /v1/markets/status/} is implemented. Future markets-related endpoints + * (none planned today) would land here. + */ +public final class MarketsResource { + + private static final String STATUS_PATH = "markets/status"; + private static final DateTimeFormatter ISO_DATE = DateTimeFormatter.ISO_LOCAL_DATE; + + private final HttpTransport transport; + + MarketsResource(HttpTransport transport) { + this.transport = transport; + } + + /** + * Today's market status for US exchanges. Equivalent to {@code GET /v1/markets/status/}. + * + *

Sync. The async sibling is {@link #statusAsync()}. + */ + public MarketStatus status() { + return transport.executeSync(RequestSpec.get(STATUS_PATH).build(), MarketStatus.class); + } + + /** Async variant of {@link #status()}. */ + public CompletableFuture statusAsync() { + return transport.executeAsync(RequestSpec.get(STATUS_PATH).build(), MarketStatus.class); + } + + /** + * Market status for a single trading day. Equivalent to {@code GET + * /v1/markets/status/?date=YYYY-MM-DD}. + * + * @param date the trading day to look up; sent in ISO-8601 format + */ + public MarketStatus status(LocalDate date) { + return transport.executeSync(forDate(date), MarketStatus.class); + } + + /** Async variant of {@link #status(LocalDate)}. */ + public CompletableFuture statusAsync(LocalDate date) { + return transport.executeAsync(forDate(date), MarketStatus.class); + } + + /** + * Market status for a closed date range. Equivalent to {@code GET + * /v1/markets/status/?from=YYYY-MM-DD&to=YYYY-MM-DD}. Both endpoints are inclusive. + * + * @param from start of the range (inclusive) + * @param to end of the range (inclusive) + * @throws IllegalArgumentException if {@code from} is after {@code to} + */ + public MarketStatus status(LocalDate from, LocalDate to) { + return transport.executeSync(forRange(from, to), MarketStatus.class); + } + + /** Async variant of {@link #status(LocalDate, LocalDate)}. */ + public CompletableFuture statusAsync(LocalDate from, LocalDate to) { + return transport.executeAsync(forRange(from, to), MarketStatus.class); + } + + private static RequestSpec forDate(LocalDate date) { + return RequestSpec.get(STATUS_PATH).query("date", ISO_DATE.format(date)).build(); + } + + private static RequestSpec forRange(LocalDate from, LocalDate to) { + if (from.isAfter(to)) { + throw new IllegalArgumentException("from (" + from + ") must not be after to (" + to + ")"); + } + return RequestSpec.get(STATUS_PATH) + .query("from", ISO_DATE.format(from)) + .query("to", ISO_DATE.format(to)) + .build(); + } +} diff --git a/src/main/java/com/marketdata/sdk/RateLimitHeaders.java b/src/main/java/com/marketdata/sdk/RateLimitHeaders.java new file mode 100644 index 0000000..909cffa --- /dev/null +++ b/src/main/java/com/marketdata/sdk/RateLimitHeaders.java @@ -0,0 +1,52 @@ +package com.marketdata.sdk; + +import java.net.http.HttpHeaders; +import java.time.Instant; +import org.jspecify.annotations.Nullable; + +/** + * Parses the {@code x-api-ratelimit-*} response headers that the API sets on every successful + * request (SDK requirements §8.2) into a {@link RateLimits} record. + * + *

Returns {@code null} when none of the relevant headers are present, which happens during a + * rate-limit-tracking outage on the server side (the API silently swallows the error and keeps + * serving the request, see {@code request_rate_middleware.py:30–40}). + */ +final class RateLimitHeaders { + + private static final String LIMIT = "x-api-ratelimit-limit"; + private static final String REMAINING = "x-api-ratelimit-remaining"; + private static final String RESET = "x-api-ratelimit-reset"; + private static final String CONSUMED = "x-api-ratelimit-consumed"; + + private RateLimitHeaders() {} + + static @Nullable RateLimits parse(HttpHeaders headers) { + Long limit = readLong(headers, LIMIT); + Long remaining = readLong(headers, REMAINING); + Long reset = readLong(headers, RESET); + Long consumed = readLong(headers, CONSUMED); + if (limit == null && remaining == null && reset == null && consumed == null) { + return null; + } + return new RateLimits( + limit != null ? limit : 0L, + remaining != null ? remaining : 0L, + Instant.ofEpochSecond(reset != null ? reset : 0L), + consumed != null ? consumed : 0L); + } + + private static @Nullable Long readLong(HttpHeaders headers, String name) { + return headers + .firstValue(name) + .map( + v -> { + try { + return Long.parseLong(v.trim()); + } catch (NumberFormatException e) { + return null; + } + }) + .orElse(null); + } +} diff --git a/src/main/java/com/marketdata/sdk/RequestSpec.java b/src/main/java/com/marketdata/sdk/RequestSpec.java new file mode 100644 index 0000000..428c30c --- /dev/null +++ b/src/main/java/com/marketdata/sdk/RequestSpec.java @@ -0,0 +1,54 @@ +package com.marketdata.sdk; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Declarative description of an HTTP GET request the SDK wants to make. + * + *

Resources build instances of this and hand them to {@link HttpTransport}; the transport is the + * only code that knows about base URLs, auth headers, timeouts, and the like. + * + * @param path API-relative path with no leading {@code /v1/} prefix and no trailing slash, e.g. + * {@code "markets/status"}. The transport adds the base URL, version prefix, and trailing + * slash. + * @param queryParams ordered query parameters (insertion order preserved for predictable URLs in + * tests). Values are URL-encoded by the transport. + */ +record RequestSpec(String path, Map queryParams) { + + RequestSpec { + // Preserve insertion order — Map.copyOf would defensively copy but + // strip the iteration order, which breaks predictable URLs in tests + // and in any caller that cares about query-param order on the wire. + queryParams = Collections.unmodifiableMap(new LinkedHashMap<>(queryParams)); + } + + static Builder get(String path) { + return new Builder(path); + } + + static final class Builder { + private final String path; + private final Map queryParams = new LinkedHashMap<>(); + + private Builder(String path) { + this.path = path; + } + + /** Adds a query parameter only if {@code value} is non-null. */ + Builder query(String key, Object value) { + if (value != null) { + queryParams.put(key, value.toString()); + } + return this; + } + + RequestSpec build() { + // Pass the raw LinkedHashMap — the record's compact constructor defensively copies and + // wraps it as unmodifiable, so wrapping here too would just rebuild a redundant view. + return new RequestSpec(path, queryParams); + } + } +} diff --git a/src/main/java/com/marketdata/sdk/Version.java b/src/main/java/com/marketdata/sdk/Version.java index db0241a..909c6c9 100644 --- a/src/main/java/com/marketdata/sdk/Version.java +++ b/src/main/java/com/marketdata/sdk/Version.java @@ -1,5 +1,7 @@ package com.marketdata.sdk; +import org.jspecify.annotations.Nullable; + /** * Reads the SDK's version from the JAR manifest's {@code Implementation-Version} attribute (SDK * requirements §15: "version must be automatically detected from package metadata"). @@ -9,12 +11,17 @@ */ final class Version { - private static final String FALLBACK = "0.0.0-dev"; + static final String FALLBACK = "0.0.0-dev"; private Version() {} - public static String current() { - String version = Version.class.getPackage().getImplementationVersion(); - return version != null && !version.isBlank() ? version : FALLBACK; + static String current() { + return resolve(Version.class.getPackage().getImplementationVersion()); + } + + // Extracted so tests can exercise both the present-version and fallback branches without + // requiring the SDK to be loaded from an actual JAR with an Implementation-Version manifest. + static String resolve(@Nullable String detected) { + return detected != null && !detected.isBlank() ? detected : FALLBACK; } } diff --git a/src/main/java/com/marketdata/sdk/markets/DailyStatus.java b/src/main/java/com/marketdata/sdk/markets/DailyStatus.java new file mode 100644 index 0000000..2f20caa --- /dev/null +++ b/src/main/java/com/marketdata/sdk/markets/DailyStatus.java @@ -0,0 +1,13 @@ +package com.marketdata.sdk.markets; + +import java.time.LocalDate; + +/** + * Whether the market was open on a single trading day. + * + * @param date the calendar date in the exchange's local time zone (US/Eastern for the default + * country US, per SDK requirements §11.4) + * @param open {@code true} if the market session was open on that date, {@code false} if closed + * (weekend, holiday, etc.) + */ +public record DailyStatus(LocalDate date, boolean open) {} diff --git a/src/main/java/com/marketdata/sdk/markets/MarketStatus.java b/src/main/java/com/marketdata/sdk/markets/MarketStatus.java new file mode 100644 index 0000000..18ff857 --- /dev/null +++ b/src/main/java/com/marketdata/sdk/markets/MarketStatus.java @@ -0,0 +1,23 @@ +package com.marketdata.sdk.markets; + +import java.util.List; + +/** + * Result of a {@code /v1/markets/status/} call: one {@link DailyStatus} per requested date, in + * chronological order. + * + *

The wire format the API returns is a compressed parallel-arrays JSON payload (per SDK + * requirements §11.1); the SDK expands it into this idiomatic typed shape via a custom Jackson + * deserializer registered programmatically by the transport (ADR-005, ADR-007). + * + *

An empty {@code days} list means the API responded with no data — either an HTTP 404 with + * {@code {"s":"no_data"}} or an unsupported country (currently only {@code US} returns data). + * + * @param days the per-day market status, never {@code null}; empty when the API has no data + */ +public record MarketStatus(List days) { + + public boolean isEmpty() { + return days.isEmpty(); + } +} diff --git a/src/main/java/com/marketdata/sdk/markets/package-info.java b/src/main/java/com/marketdata/sdk/markets/package-info.java new file mode 100644 index 0000000..f8f4dee --- /dev/null +++ b/src/main/java/com/marketdata/sdk/markets/package-info.java @@ -0,0 +1,8 @@ +/** + * Public response records for the {@code /v1/markets/*} endpoint group. The façade itself ({@link + * com.marketdata.sdk.MarketsResource}) lives in the SDK root package per ADR-007. + */ +@NullMarked +package com.marketdata.sdk.markets; + +import org.jspecify.annotations.NullMarked; diff --git a/src/test/java/com/marketdata/sdk/AsyncSemaphoreTest.java b/src/test/java/com/marketdata/sdk/AsyncSemaphoreTest.java new file mode 100644 index 0000000..ed03e6f --- /dev/null +++ b/src/test/java/com/marketdata/sdk/AsyncSemaphoreTest.java @@ -0,0 +1,230 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CyclicBarrier; +import org.junit.jupiter.api.RepeatedTest; +import org.junit.jupiter.api.Test; + +class AsyncSemaphoreTest { + + // ---------- fast path ---------- + + @Test + void acquireReturnsCompletedFutureWhenPermitsAvailable() { + AsyncSemaphore sem = new AsyncSemaphore(3); + + CompletableFuture a = sem.acquire(); + CompletableFuture b = sem.acquire(); + CompletableFuture c = sem.acquire(); + + assertThat(a).isCompleted(); + assertThat(b).isCompleted(); + assertThat(c).isCompleted(); + assertThat(sem.availablePermits()).isZero(); + assertThat(sem.queueLength()).isZero(); + } + + // ---------- slow path ---------- + + @Test + void acquireReturnsPendingFutureWhenPoolExhausted() { + AsyncSemaphore sem = new AsyncSemaphore(2); + sem.acquire(); + sem.acquire(); + + CompletableFuture waiter = sem.acquire(); + + assertThat(waiter).isNotCompleted(); + assertThat(sem.availablePermits()).isZero(); + assertThat(sem.queueLength()).isOne(); + } + + @Test + void releaseTransfersPermitDirectlyToFirstWaiter() { + AsyncSemaphore sem = new AsyncSemaphore(1); + sem.acquire(); // pool empty + + CompletableFuture w1 = sem.acquire(); + CompletableFuture w2 = sem.acquire(); + + sem.release(); + + // The permit goes from the in-flight caller straight to w1 — never re-counted. + assertThat(w1).isCompleted(); + assertThat(w2).isNotCompleted(); + assertThat(sem.availablePermits()).isZero(); + assertThat(sem.queueLength()).isOne(); + + sem.release(); + + assertThat(w2).isCompleted(); + assertThat(sem.availablePermits()).isZero(); + assertThat(sem.queueLength()).isZero(); + } + + @Test + void releaseWithNoWaitersIncrementsCounter() { + AsyncSemaphore sem = new AsyncSemaphore(2); + sem.acquire(); + sem.acquire(); + + sem.release(); + assertThat(sem.availablePermits()).isOne(); + + sem.release(); + assertThat(sem.availablePermits()).isEqualTo(2); + } + + // ---------- cancellation ---------- + + @Test + void cancelledWaiterIsSkippedOnRelease() { + AsyncSemaphore sem = new AsyncSemaphore(1); + sem.acquire(); // pool empty + + CompletableFuture cancelled = sem.acquire(); + CompletableFuture alive = sem.acquire(); + cancelled.cancel(false); + + sem.release(); + + // The cancelled waiter is skipped; the next live one gets the permit. + assertThat(alive).isCompleted(); + assertThat(sem.queueLength()).isZero(); + assertThat(sem.availablePermits()).isZero(); + } + + @Test + void releaseWhenAllWaitersCancelledFallsBackToCounter() { + AsyncSemaphore sem = new AsyncSemaphore(1); + sem.acquire(); + + sem.acquire().cancel(false); + sem.acquire().cancel(false); + + sem.release(); + + // No live waiter — the permit goes back to the pool. + assertThat(sem.availablePermits()).isOne(); + assertThat(sem.queueLength()).isZero(); + } + + // ---------- ordering ---------- + + @Test + void waitersAreServedFifo() { + AsyncSemaphore sem = new AsyncSemaphore(0); + List completionOrder = new ArrayList<>(); + + for (int i = 0; i < 10; i++) { + int id = i; + sem.acquire().thenRun(() -> completionOrder.add(id)); + } + + for (int i = 0; i < 10; i++) { + sem.release(); + } + + assertThat(completionOrder).containsExactly(0, 1, 2, 3, 4, 5, 6, 7, 8, 9); + } + + // ---------- race between release() and waiter cancellation (Issue #1, Component B) ---------- + + /** + * Regression for the TOCTOU race in {@link AsyncSemaphore#release()} between {@code pollFirst()} + * (inside the lock) and {@code complete(null)} (outside the lock). If the polled waiter is + * cancelled in that window, {@code complete(null)} returns false and — under the current + * implementation — the permit is silently lost: it was already removed from the counter when + * release() "transferred" it, and the cancelled waiter never delivers it anywhere. + * + *

The race is timing-sensitive; we coordinate two threads through a {@link CyclicBarrier} and + * repeat the scenario many times so at least some iterations hit the bad window. The invariant we + * assert is permit-conservation: + * + *

    + *
  • If the canceller won the race, the waiter is cancelled and {@code release()} must have + * found an alternative home for the permit — either the next live waiter, or the + * available-permits counter. + *
  • If the releaser won the race, the waiter completes normally and the counter stays at 0. + *
+ * + * Either way, the permit is never lost. + */ + @RepeatedTest(200) + void releaseDoesNotLosePermitWhenWaiterIsCancelledMidRelease() throws Exception { + AsyncSemaphore sem = new AsyncSemaphore(1); + sem.acquire(); // pool now empty + + CompletableFuture waiter = sem.acquire(); // queued + + CyclicBarrier barrier = new CyclicBarrier(2); + + Thread releaser = + new Thread( + () -> { + awaitBarrier(barrier); + sem.release(); + }); + Thread canceller = + new Thread( + () -> { + awaitBarrier(barrier); + waiter.cancel(false); + }); + + releaser.start(); + canceller.start(); + releaser.join(); + canceller.join(); + + assertThat(sem.queueLength()).as("queue must be drained").isZero(); + + if (waiter.isCancelled()) { + // Canceller observed (or won) the race. Whatever release() did, the permit must have + // landed somewhere — and with no other waiter present, that "somewhere" is the counter. + assertThat(sem.availablePermits()) + .as("permit must return to the pool when the only waiter is cancelled") + .isEqualTo(1); + } else { + // Releaser completed the waiter before cancel arrived. waiter must be done-normally, + // and the permit is considered "held" by the (notional) downstream consumer of the waiter. + assertThat(waiter) + .as("if not cancelled, waiter must be completed normally") + .isCompletedWithValue(null); + assertThat(sem.availablePermits()).isZero(); + } + } + + private static void awaitBarrier(CyclicBarrier barrier) { + try { + barrier.await(); + } catch (Exception e) { + throw new AssertionError("barrier interrupted", e); + } + } + + // ---------- argument validation ---------- + + @Test + void rejectsNegativeInitialPermits() { + assertThatThrownBy(() -> new AsyncSemaphore(-1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("permits"); + } + + @Test + void zeroInitialPermitsIsValidAndForcesSlowPath() { + AsyncSemaphore sem = new AsyncSemaphore(0); + + CompletableFuture w = sem.acquire(); + assertThat(w).isNotCompleted(); + + sem.release(); + assertThat(w).isCompleted(); + } +} diff --git a/src/test/java/com/marketdata/sdk/CallMode.java b/src/test/java/com/marketdata/sdk/CallMode.java new file mode 100644 index 0000000..d9144bc --- /dev/null +++ b/src/test/java/com/marketdata/sdk/CallMode.java @@ -0,0 +1,68 @@ +package com.marketdata.sdk; + +import com.marketdata.sdk.markets.MarketStatus; +import java.time.LocalDate; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +/** + * Drives a {@code /v1/markets/*} call through either the sync or async surface. ASYNC mode unwraps + * {@link CompletionException} so caller-visible behavior matches sync (per ADR-006: sync wraps + * {@code .join()} and surfaces the underlying cause directly). + * + *

Lives in the unit-test source set so it is reusable from the integration-test source set — + * {@code integrationTest}'s compileClasspath includes the unit-test output (see {@code + * build.gradle.kts}). Package-private intentionally: only test classes in {@code + * com.marketdata.sdk} need it. + */ +enum CallMode { + SYNC { + @Override + MarketStatus statusNoArgs(MarketsResource r) { + return r.status(); + } + + @Override + MarketStatus statusForDate(MarketsResource r, LocalDate date) { + return r.status(date); + } + + @Override + MarketStatus statusForRange(MarketsResource r, LocalDate from, LocalDate to) { + return r.status(from, to); + } + }, + ASYNC { + @Override + MarketStatus statusNoArgs(MarketsResource r) { + return joinUnwrapping(r.statusAsync()); + } + + @Override + MarketStatus statusForDate(MarketsResource r, LocalDate date) { + return joinUnwrapping(r.statusAsync(date)); + } + + @Override + MarketStatus statusForRange(MarketsResource r, LocalDate from, LocalDate to) { + return joinUnwrapping(r.statusAsync(from, to)); + } + }; + + abstract MarketStatus statusNoArgs(MarketsResource r); + + abstract MarketStatus statusForDate(MarketsResource r, LocalDate date); + + abstract MarketStatus statusForRange(MarketsResource r, LocalDate from, LocalDate to); + + private static T joinUnwrapping(CompletableFuture future) { + try { + return future.join(); + } catch (CompletionException e) { + if (e.getCause() instanceof RuntimeException re) { + throw re; + } + throw e; + } + } +} diff --git a/src/test/java/com/marketdata/sdk/ConfigurationTest.java b/src/test/java/com/marketdata/sdk/ConfigurationTest.java index 7151ec1..8cda17f 100644 --- a/src/test/java/com/marketdata/sdk/ConfigurationTest.java +++ b/src/test/java/com/marketdata/sdk/ConfigurationTest.java @@ -130,6 +130,29 @@ void missingDotEnvReturnsEmpty(@TempDir Path tmp) { assertThat(Configuration.readDotEnvFile(tmp.resolve(".env"))).isEmpty(); } + @Test + void mismatchedQuotesArePreservedVerbatim(@TempDir Path tmp) throws IOException { + // stripQuotes only strips when the first AND last characters match (both " or both '). + // Lines with mixed or unbalanced quotes must keep the value as-is. Covers the right-hand + // false branches of the `||` in (first == '"' && last == '"') || (first == '\'' && last == + // '\''). + Path dotenv = tmp.resolve(".env"); + Files.writeString( + dotenv, + """ + UNCLOSED_DOUBLE="abc + UNCLOSED_SINGLE='abc + MIXED_QUOTES="abc' + """); + + Map parsed = Configuration.readDotEnvFile(dotenv); + + assertThat(parsed) + .containsEntry("UNCLOSED_DOUBLE", "\"abc") + .containsEntry("UNCLOSED_SINGLE", "'abc") + .containsEntry("MIXED_QUOTES", "\"abc'"); + } + @Test void dotEnvParsingIntegratesWithCascade(@TempDir Path tmp) throws IOException { Path dotenv = tmp.resolve(".env"); diff --git a/src/test/java/com/marketdata/sdk/HttpStatusMapperTest.java b/src/test/java/com/marketdata/sdk/HttpStatusMapperTest.java new file mode 100644 index 0000000..a7e5cef --- /dev/null +++ b/src/test/java/com/marketdata/sdk/HttpStatusMapperTest.java @@ -0,0 +1,83 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.marketdata.sdk.exception.AuthenticationError; +import com.marketdata.sdk.exception.BadRequestError; +import com.marketdata.sdk.exception.MarketDataException; +import com.marketdata.sdk.exception.RateLimitError; +import com.marketdata.sdk.exception.ServerError; +import org.junit.jupiter.api.Test; + +class HttpStatusMapperTest { + + private static final String URL = "https://api.marketdata.app/v1/test/"; + private static final String RAY = "ray-1"; + + // ---------- switch coverage: each case + default ---------- + + @Test + void status400MapsToBadRequest() { + MarketDataException e = HttpStatusMapper.toException(400, URL, RAY); + assertThat(e).isInstanceOf(BadRequestError.class); + assertThat(e.getStatusCode()).isEqualTo(400); + assertThat(e.getMessage()).contains("400"); + } + + @Test + void status422AlsoMapsToBadRequest() { + // Same case-arm as 400; without exercising 422 explicitly, half the multi-label arm is + // unrecorded by JaCoCo. + MarketDataException e = HttpStatusMapper.toException(422, URL, RAY); + assertThat(e).isInstanceOf(BadRequestError.class); + assertThat(e.getStatusCode()).isEqualTo(422); + assertThat(e.getMessage()).contains("422"); + } + + @Test + void status401MapsToAuthenticationError() { + MarketDataException e = HttpStatusMapper.toException(401, URL, RAY); + assertThat(e).isInstanceOf(AuthenticationError.class); + assertThat(e.getStatusCode()).isEqualTo(401); + } + + @Test + void status429MapsToRateLimitError() { + MarketDataException e = HttpStatusMapper.toException(429, URL, RAY); + assertThat(e).isInstanceOf(RateLimitError.class); + assertThat(e.getStatusCode()).isEqualTo(429); + } + + @Test + void everyOtherStatusFallsThroughToServerError() { + // Any status not explicitly handled (402, 500, 502, 503, 504, weird ones) maps to + // ServerError. Covers the `default ->` arm. + for (int code : new int[] {402, 500, 502, 503, 504, 599}) { + MarketDataException e = HttpStatusMapper.toException(code, URL, RAY); + assertThat(e).as("status %d", code).isInstanceOf(ServerError.class); + assertThat(e.getStatusCode()).isEqualTo(code); + } + } + + // ---------- emptyToNull: null vs blank vs valid ---------- + + @Test + void nullRequestIdIsPropagatedAsNull() { + MarketDataException e = HttpStatusMapper.toException(500, URL, null); + assertThat(e.getRequestId()).isNull(); + } + + @Test + void blankRequestIdIsTreatedAsNull() { + // emptyToNull's `s == null || s.isBlank()` short-circuits — without an explicit blank + // input, the right-hand isBlank() branch is never evaluated. + MarketDataException e = HttpStatusMapper.toException(500, URL, " "); + assertThat(e.getRequestId()).isNull(); + } + + @Test + void validRequestIdIsPreserved() { + MarketDataException e = HttpStatusMapper.toException(500, URL, "ray-abc"); + assertThat(e.getRequestId()).isEqualTo("ray-abc"); + } +} diff --git a/src/test/java/com/marketdata/sdk/HttpTransportE2ETest.java b/src/test/java/com/marketdata/sdk/HttpTransportE2ETest.java new file mode 100644 index 0000000..f8a8a39 --- /dev/null +++ b/src/test/java/com/marketdata/sdk/HttpTransportE2ETest.java @@ -0,0 +1,119 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * End-to-end tests for {@link HttpTransport} that exercise URI shapes and status codes the public + * resource façades don't naturally hit (status 203, trailing-slash paths). Uses the JDK's built-in + * {@link HttpServer} to avoid any extra mocking dependencies. + */ +class HttpTransportE2ETest { + + private HttpServer server; + private final AtomicReference capturedUri = new AtomicReference<>(); + private RouteHandler handler; + + /** Minimal record matching {@code {"value": "..."}} so we can verify a successful decode. */ + record Echo(@JsonProperty("value") String value) {} + + @BeforeEach + void startServer() throws IOException { + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + handler = new RouteHandler(); + server.createContext("/", handler); + server.start(); + } + + @AfterEach + void stopServer() { + server.stop(0); + } + + private HttpTransport newTransport() { + int port = server.getAddress().getPort(); + return new HttpTransport("http://127.0.0.1:" + port, "v1", "test/0.0", null); + } + + /** + * Status 203 (Non-Authoritative Information) is treated identically to 200 by the transport — + * decoding the body and returning the result. The check {@code status == 200 || status == 203 || + * status == 404} in {@code processResponse} is the only place 203 appears, and without an + * explicit test the 203 leg is dead from JaCoCo's perspective. + */ + @Test + void status203IsTreatedAsSuccess() { + handler.setResponse(203, "{\"value\":\"ok\"}"); + + Echo result = newTransport().executeSync(RequestSpec.get("ping").build(), Echo.class); + + assertThat(result.value()).isEqualTo("ok"); + } + + /** + * When the {@link RequestSpec#path()} already ends with a slash, the transport must not append + * another one. Covers the {@code endsWith("/")} → true branch in {@code buildUri}. + */ + @Test + void pathEndingInSlashIsNotDoubled() { + handler.setResponse(200, "{\"value\":\"ok\"}"); + + Echo result = newTransport().executeSync(RequestSpec.get("ping/").build(), Echo.class); + + assertThat(result.value()).isEqualTo("ok"); + assertThat(capturedUri.get().getPath()).isEqualTo("/v1/ping/"); + assertThat(capturedUri.get().getPath()).doesNotContain("//"); + } + + /** + * RequestSpec's Javadoc says paths should not start with {@code /}, but a caller mistake would + * otherwise produce {@code /v1//ping/} (double slash, which some HTTP routers reject). The + * transport strips the leading slash defensively so a path of {@code "/ping"} produces the same + * URL as {@code "ping"}. + */ + @Test + void pathStartingWithSlashIsStripped() { + handler.setResponse(200, "{\"value\":\"ok\"}"); + + Echo result = newTransport().executeSync(RequestSpec.get("/ping").build(), Echo.class); + + assertThat(result.value()).isEqualTo("ok"); + assertThat(capturedUri.get().getPath()).isEqualTo("/v1/ping/"); + assertThat(capturedUri.get().getPath()).doesNotContain("//"); + } + + // ---------- in-process server plumbing ---------- + + private final class RouteHandler implements HttpHandler { + private int statusCode = 200; + private String body = "{}"; + + void setResponse(int code, String body) { + this.statusCode = code; + this.body = body; + } + + @Override + public void handle(HttpExchange exchange) throws IOException { + capturedUri.set(exchange.getRequestURI()); + + byte[] bodyBytes = body.getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + exchange.sendResponseHeaders(statusCode, bodyBytes.length); + exchange.getResponseBody().write(bodyBytes); + exchange.getResponseBody().close(); + } + } +} diff --git a/src/test/java/com/marketdata/sdk/HttpTransportTest.java b/src/test/java/com/marketdata/sdk/HttpTransportTest.java new file mode 100644 index 0000000..fe94e71 --- /dev/null +++ b/src/test/java/com/marketdata/sdk/HttpTransportTest.java @@ -0,0 +1,490 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.marketdata.sdk.exception.NetworkError; +import java.io.IOException; +import java.lang.reflect.Field; +import java.net.Authenticator; +import java.net.CookieHandler; +import java.net.ProxySelector; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.WebSocket; +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.Executor; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLParameters; +import org.junit.jupiter.api.Test; + +class HttpTransportTest { + + /** + * Regression for the synchronous-throw permit leak: if {@code httpClient.sendAsync(...)} throws + * before returning a future (rare but possible — malformed request, internal NPE, OOM), the + * {@code whenComplete(release)} chain never forms. Without explicit release in the catch, every + * such failure burns a permit forever; a long-lived process eventually deadlocks once 50 such + * failures accumulate. + * + *

This test runs more requests than {@link HttpTransport#CONCURRENCY_LIMIT} against a stub + * client whose {@code sendAsync} always throws — if a permit ever leaked, the {@code + * (limit+1)}-th call would block indefinitely on {@code acquire()} and the test would time out. + */ + @Test + void permitReleasedWhenSendAsyncThrowsSynchronously() throws Exception { + HttpTransport transport = + new HttpTransport("http://localhost", "v1", "test/0.0", null, new SyncThrowingHttpClient()); + + AsyncSemaphore permits = readSemaphore(transport); + int initial = permits.availablePermits(); + assertThat(initial).isEqualTo(HttpTransport.CONCURRENCY_LIMIT); + + int n = HttpTransport.CONCURRENCY_LIMIT + 5; + for (int i = 0; i < n; i++) { + CompletableFuture f = + transport.executeAsync(RequestSpec.get("ping").build(), Object.class); + + assertThat(f).isCompletedExceptionally(); + assertThatThrownBy(f::join) + .isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(NetworkError.class) + .hasMessageContaining("before dispatch"); + } + + // If even one permit had leaked, this would be < initial; the (limit+1)-th call would + // also have blocked instead of failing fast. + assertThat(permits.availablePermits()).isEqualTo(initial); + } + + /** + * Errors thrown synchronously by {@link HttpClient#sendAsync} (e.g. {@code OutOfMemoryError}) + * must surface with their original type preserved — wrapping a JVM-level {@link Error} in a + * {@link com.marketdata.sdk.exception.NetworkError} would mask the real cause and produce a + * misleading "network failure" for what is actually a runtime crash. Covers the {@code if (t + * instanceof Error err) throw err;} branch in {@code dispatch}; the {@link + * java.util.concurrent.CompletableFuture#thenCompose} machinery catches the rethrown Error and + * exposes it as the future's root cause rather than letting it propagate synchronously. + */ + @Test + void errorThrownSynchronouslyIsPreservedAsRootCause() throws Exception { + HttpTransport transport = + new HttpTransport( + "http://localhost", "v1", "test/0.0", null, new ErrorThrowingHttpClient()); + + AsyncSemaphore permits = readSemaphore(transport); + int initial = permits.availablePermits(); + + CompletableFuture f = + transport.executeAsync(RequestSpec.get("ping").build(), Object.class); + + assertThat(f).isCompletedExceptionally(); + assertThatThrownBy(f::join) + .isInstanceOf(CompletionException.class) + .hasRootCauseInstanceOf(OutOfMemoryError.class) + .hasRootCauseMessage("simulated synchronous Error from sendAsync"); + + // Permit released even though the catch took the Error branch — a leak here would + // accumulate over a long-lived process and eventually deadlock the pool. + assertThat(permits.availablePermits()).isEqualTo(initial); + } + + /** + * Regression for the slow-path cancellation leak (Issue #1, Component A). When the pool is + * saturated, {@code acquire()} returns a pending waiter that is enqueued. The future the caller + * actually sees is the downstream {@code thenCompose} result, NOT the waiter. Cancelling the + * downstream does not propagate to the waiter (standard CompletableFuture semantics), so + * the waiter is still alive when {@code release()} runs — release() "transfers" the permit by + * completing the waiter, but the {@code thenCompose} function never executes because its + * dependent future is already cancelled. Result: the permit is lost forever. + * + *

This test saturates the pool with {@link HttpTransport#CONCURRENCY_LIMIT} fast-path + * dispatches whose HTTP futures we control, queues {@code extras} slow-path callers, cancels all + * the slow-path futures, and then completes the fast-path HTTP futures so {@code release()} + * fires. Once every dispatch has settled, every permit must be back in the pool. + */ + @Test + void permitsAreReleasedWhenSlowPathFuturesAreCancelled() throws Exception { + ControllableHttpClient client = new ControllableHttpClient(); + HttpTransport transport = new HttpTransport("http://localhost", "v1", "test/0.0", null, client); + + AsyncSemaphore permits = readSemaphore(transport); + int initial = permits.availablePermits(); + assertThat(initial).isEqualTo(HttpTransport.CONCURRENCY_LIMIT); + + // Saturate the pool — these go through the fast path (acquire returns an already-completed + // future), dispatch is invoked, sendAsync is called → ControllableHttpClient returns a + // pending future we hold the handle to. + List> fastPath = new ArrayList<>(initial); + for (int i = 0; i < initial; i++) { + fastPath.add(transport.executeAsync(RequestSpec.get("ping").build(), Object.class)); + } + assertThat(permits.availablePermits()).isZero(); + assertThat(permits.queueLength()).isZero(); + assertThat(client.pendingCount()).isEqualTo(initial); + + // Slow path — these enqueue waiters in the semaphore. dispatch is NOT yet called for them. + int extras = 5; + List> slowPath = new ArrayList<>(extras); + for (int i = 0; i < extras; i++) { + slowPath.add(transport.executeAsync(RequestSpec.get("ping").build(), Object.class)); + } + assertThat(permits.queueLength()).isEqualTo(extras); + + // Caller cancels every slow-path future. Without the fix, the waiters stay live in the + // queue — release() will later transfer permits into the cancelled-downstream waiters + // and the permits disappear. + for (CompletableFuture f : slowPath) { + f.cancel(false); + } + + // Complete every fast-path HTTP future. Each completion fires whenComplete(release). + // Failing the future bypasses body decoding (which would NPE on a null response) while + // still exercising the release path. + client.failAll(new IOException("simulated end of test")); + + // After every dispatch has settled, the pool must be fully restored. + assertThat(permits.queueLength()).isZero(); + assertThat(permits.availablePermits()) + .as("every permit should be back in the pool — no leaks from cancelled slow-path futures") + .isEqualTo(initial); + } + + // ---------- asRuntime: covers the three branches in the executeSync catch ---------- + + @Test + void asRuntimeReturnsMarketDataExceptionUnchanged() { + // The `instanceof MarketDataException` branch — the only one reached from the public + // surface today (every failure from executeAsync is wrapped as an MDE subtype). + com.marketdata.sdk.exception.BadRequestError mde = + new com.marketdata.sdk.exception.BadRequestError( + "bad", com.marketdata.sdk.exception.ErrorContext.empty()); + + RuntimeException result = HttpTransport.asRuntime(mde); + + assertThat(result).isSameAs(mde); + } + + @Test + void asRuntimeRethrowsNonMdeRuntimeExceptionUnchanged() { + // Defensive guardrail: if some future code path lets a non-MDE RuntimeException reach + // .join()'s cause, surface it as-is rather than wrapping it. + IllegalStateException re = new IllegalStateException("unexpected"); + + RuntimeException result = HttpTransport.asRuntime(re); + + assertThat(result).isSameAs(re); + } + + @Test + void asRuntimeWrapsNonRuntimeCauseInNetworkError() { + // Last-resort branch: cause is an Error (or null). Wrap in NetworkError so the public + // surface still observes the sealed MarketDataException hierarchy. + OutOfMemoryError error = new OutOfMemoryError("simulated"); + + RuntimeException result = HttpTransport.asRuntime(error); + + assertThat(result).isInstanceOf(com.marketdata.sdk.exception.NetworkError.class); + assertThat(result.getCause()).isSameAs(error); + assertThat(result.getMessage()).contains("Unexpected failure invoking SDK"); + } + + @Test + void asRuntimeWrapsNullCauseInNetworkError() { + // CompletableFuture.join() can in principle deliver a CompletionException whose cause + // is null (defensive: should never happen in practice but ergonomically harmless). + RuntimeException result = HttpTransport.asRuntime(null); + + assertThat(result).isInstanceOf(com.marketdata.sdk.exception.NetworkError.class); + assertThat(result.getCause()).isNull(); + } + + // ---------- unwrap: covers all 4 branches of `t instanceof CE && t.getCause() != null` + // ---------- + + @Test + void unwrapReturnsNonCompletionExceptionUnchanged() { + // First branch of `&&` is false → short-circuit, return t as-is. The most common path + // in production: handle() in CompletableFuture already unwraps CompletionException. + java.io.IOException io = new java.io.IOException("boom"); + assertThat(HttpTransport.unwrap(io)).isSameAs(io); + } + + @Test + void unwrapReturnsCauseOfNestedCompletionException() { + // Both branches true: CompletionException with a cause. Returns the cause. + java.io.IOException root = new java.io.IOException("root"); + CompletionException wrapped = new CompletionException(root); + + assertThat(HttpTransport.unwrap(wrapped)).isSameAs(root); + } + + @Test + void unwrapReturnsCompletionExceptionWithoutCauseUnchanged() { + // First branch true, second branch false: CompletionException with `null` cause. The + // method returns t itself rather than dereferencing the missing cause. + CompletionException causeless = new CompletionException(null); + + assertThat(HttpTransport.unwrap(causeless)).isSameAs(causeless); + } + + // ---------- helpers ---------- + + private static AsyncSemaphore readSemaphore(HttpTransport t) throws Exception { + Field f = HttpTransport.class.getDeclaredField("concurrencyPermits"); + f.setAccessible(true); + return (AsyncSemaphore) f.get(t); + } + + /** + * Bare-bones {@link HttpClient} subclass whose {@code sendAsync} throws synchronously. Every + * other abstract method is stubbed with {@code UnsupportedOperationException} since the test + * never exercises them. + */ + private static final class SyncThrowingHttpClient extends HttpClient { + @Override + public CompletableFuture> sendAsync( + HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) { + throw new IllegalArgumentException("simulated synchronous throw from sendAsync"); + } + + @Override + public Optional cookieHandler() { + return Optional.empty(); + } + + @Override + public Optional connectTimeout() { + return Optional.empty(); + } + + @Override + public Redirect followRedirects() { + return Redirect.NEVER; + } + + @Override + public Optional proxy() { + return Optional.empty(); + } + + @Override + public SSLContext sslContext() { + throw new UnsupportedOperationException(); + } + + @Override + public SSLParameters sslParameters() { + throw new UnsupportedOperationException(); + } + + @Override + public Optional authenticator() { + return Optional.empty(); + } + + @Override + public Version version() { + return Version.HTTP_1_1; + } + + @Override + public Optional executor() { + return Optional.empty(); + } + + @Override + public HttpResponse send( + HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) + throws IOException, InterruptedException { + throw new UnsupportedOperationException(); + } + + @Override + public CompletableFuture> sendAsync( + HttpRequest request, + HttpResponse.BodyHandler responseBodyHandler, + HttpResponse.PushPromiseHandler pushPromiseHandler) { + throw new UnsupportedOperationException(); + } + + @Override + public WebSocket.Builder newWebSocketBuilder() { + throw new UnsupportedOperationException(); + } + } + + /** + * Stub {@link HttpClient} whose {@code sendAsync} returns a fresh, never-auto-completing future + * for each call. The test holds the references and chooses when to complete them — that's the + * lever the slow-path cancellation regression test pulls to deterministically drive the {@code + * whenComplete(release)} path. + */ + private static final class ControllableHttpClient extends HttpClient { + private final List>> pending = new ArrayList<>(); + + @SuppressWarnings("unchecked") + @Override + public CompletableFuture> sendAsync( + HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) { + CompletableFuture> f = new CompletableFuture<>(); + pending.add((CompletableFuture>) (CompletableFuture) f); + return f; + } + + int pendingCount() { + return pending.size(); + } + + void failAll(Throwable t) { + for (CompletableFuture> f : pending) { + f.completeExceptionally(t); + } + } + + @Override + public Optional cookieHandler() { + return Optional.empty(); + } + + @Override + public Optional connectTimeout() { + return Optional.empty(); + } + + @Override + public Redirect followRedirects() { + return Redirect.NEVER; + } + + @Override + public Optional proxy() { + return Optional.empty(); + } + + @Override + public SSLContext sslContext() { + throw new UnsupportedOperationException(); + } + + @Override + public SSLParameters sslParameters() { + throw new UnsupportedOperationException(); + } + + @Override + public Optional authenticator() { + return Optional.empty(); + } + + @Override + public Version version() { + return Version.HTTP_1_1; + } + + @Override + public Optional executor() { + return Optional.empty(); + } + + @Override + public HttpResponse send( + HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) + throws IOException, InterruptedException { + throw new UnsupportedOperationException(); + } + + @Override + public CompletableFuture> sendAsync( + HttpRequest request, + HttpResponse.BodyHandler responseBodyHandler, + HttpResponse.PushPromiseHandler pushPromiseHandler) { + throw new UnsupportedOperationException(); + } + + @Override + public WebSocket.Builder newWebSocketBuilder() { + throw new UnsupportedOperationException(); + } + } + + /** Same skeleton as {@link SyncThrowingHttpClient} but throws an {@link Error} (OOM-shaped). */ + private static final class ErrorThrowingHttpClient extends HttpClient { + @Override + public CompletableFuture> sendAsync( + HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) { + throw new OutOfMemoryError("simulated synchronous Error from sendAsync"); + } + + @Override + public Optional cookieHandler() { + return Optional.empty(); + } + + @Override + public Optional connectTimeout() { + return Optional.empty(); + } + + @Override + public Redirect followRedirects() { + return Redirect.NEVER; + } + + @Override + public Optional proxy() { + return Optional.empty(); + } + + @Override + public SSLContext sslContext() { + throw new UnsupportedOperationException(); + } + + @Override + public SSLParameters sslParameters() { + throw new UnsupportedOperationException(); + } + + @Override + public Optional authenticator() { + return Optional.empty(); + } + + @Override + public Version version() { + return Version.HTTP_1_1; + } + + @Override + public Optional executor() { + return Optional.empty(); + } + + @Override + public HttpResponse send( + HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) + throws IOException, InterruptedException { + throw new UnsupportedOperationException(); + } + + @Override + public CompletableFuture> sendAsync( + HttpRequest request, + HttpResponse.BodyHandler responseBodyHandler, + HttpResponse.PushPromiseHandler pushPromiseHandler) { + throw new UnsupportedOperationException(); + } + + @Override + public WebSocket.Builder newWebSocketBuilder() { + throw new UnsupportedOperationException(); + } + } +} diff --git a/src/test/java/com/marketdata/sdk/MarketDataClientTest.java b/src/test/java/com/marketdata/sdk/MarketDataClientTest.java index 40b6fd5..ca51fbd 100644 --- a/src/test/java/com/marketdata/sdk/MarketDataClientTest.java +++ b/src/test/java/com/marketdata/sdk/MarketDataClientTest.java @@ -2,6 +2,12 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; import org.junit.jupiter.api.Test; class MarketDataClientTest { @@ -17,17 +23,68 @@ void buildsWithExplicitToken() { @Test void demoModeWhenNoTokenAvailable() { - // No apiKey passed to the constructor. Demo mode iff the env/dotenv - // cascade also yields nothing — true on any CI environment that - // doesn't export MARKETDATA_TOKEN. This assertion is conditional - // so the test stays valid in both cases. + // Demo mode iff the full cascade (env var → .env → null) yields nothing. Deriving the + // expectation from the same Configuration helper the constructor uses keeps the test + // valid both on CI (no token anywhere → demoMode) and locally (.env-supplied token → + // not demoMode); a plain `System.getenv` check would miss the .env source and break + // locally. try (var client = new MarketDataClient()) { - String envToken = System.getenv("MARKETDATA_TOKEN"); - boolean expectDemo = envToken == null || envToken.isBlank(); + boolean expectDemo = Configuration.loadFromProcess().resolve(null, EnvVars.TOKEN) == null; assertThat(client.isDemoMode()).isEqualTo(expectDemo); } } + @Test + void fineLevelLoggingEmitsRedactedToken() { + // The constructor logs the redacted token at FINE only. With the default logger + // configuration (INFO), `LOG.isLoggable(FINE)` returns false and the line is dead from + // JaCoCo's perspective. This test installs a capturing handler at FINE and asserts the + // redacted token shows up — the unredacted token must not. + Logger logger = Logger.getLogger(MarketDataClient.class.getName()); + Level previousLevel = logger.getLevel(); + boolean previousUseParent = logger.getUseParentHandlers(); + CapturingHandler capture = new CapturingHandler(); + logger.addHandler(capture); + logger.setLevel(Level.FINE); + logger.setUseParentHandlers(false); + + try (var client = new MarketDataClient("supersecret-token-VALUE-YKT0", null, null, false)) { + assertThat(client.isDemoMode()).isFalse(); + } finally { + logger.removeHandler(capture); + logger.setLevel(previousLevel); + logger.setUseParentHandlers(previousUseParent); + } + + assertThat(capture.records) + .anySatisfy( + r -> { + assertThat(r.getLevel()).isEqualTo(Level.FINE); + assertThat(r.getMessage()).contains("Token"); + }); + // Whatever was logged at FINE, the raw token must never appear in any record. + for (LogRecord r : capture.records) { + assertThat(r.getMessage() == null ? "" : r.getMessage()) + .doesNotContain("supersecret-token-VALUE-YKT0"); + } + } + + /** Minimal {@link Handler} that buffers everything in memory for assertions. */ + private static final class CapturingHandler extends Handler { + final List records = new ArrayList<>(); + + @Override + public void publish(LogRecord record) { + records.add(record); + } + + @Override + public void flush() {} + + @Override + public void close() {} + } + @Test void noArgConstructorAppliesProductionDefaults() { // The no-arg constructor must be equivalent to `new MarketDataClient(null, null, null, diff --git a/src/test/java/com/marketdata/sdk/MarketStatusDeserializerTest.java b/src/test/java/com/marketdata/sdk/MarketStatusDeserializerTest.java new file mode 100644 index 0000000..cdb6c79 --- /dev/null +++ b/src/test/java/com/marketdata/sdk/MarketStatusDeserializerTest.java @@ -0,0 +1,104 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.marketdata.sdk.markets.MarketStatus; +import java.io.IOException; +import java.time.LocalDate; +import org.junit.jupiter.api.Test; + +class MarketStatusDeserializerTest { + + private final ObjectMapper mapper = newMapper(); + + private static ObjectMapper newMapper() { + // Per ADR-007 response records carry no @JsonDeserialize annotation — the deserializer + // is registered programmatically (HttpTransport does this in production; the test mirrors + // the same wiring so it exercises the real deserializer). + ObjectMapper m = new ObjectMapper(); + SimpleModule module = new SimpleModule("marketdata-wire-test"); + module.addDeserializer(MarketStatus.class, new MarketStatusDeserializer()); + m.registerModule(module); + return m; + } + + @Test + void parsesOkResponseIntoChronologicalDays() throws IOException { + // 1706745600 = 2024-02-01 00:00:00 UTC = 2024-01-31 19:00 US/Eastern → date 2024-01-31 + // The API normalizes "trading day midnight Eastern" to a unix timestamp; we expect the + // deserializer to recover the local Eastern date. + String json = + """ + { "s": "ok", + "date": [1706673600, 1706760000, 1706846400], + "status": ["open", "open", "closed"] } + """; + + MarketStatus status = mapper.readValue(json, MarketStatus.class); + + assertThat(status.days()).hasSize(3); + assertThat(status.days().get(0).open()).isTrue(); + assertThat(status.days().get(1).open()).isTrue(); + assertThat(status.days().get(2).open()).isFalse(); + assertThat(status.days().get(0).date()).isInstanceOf(LocalDate.class); + assertThat(status.isEmpty()).isFalse(); + } + + @Test + void noDataResponseProducesEmptyResult() throws IOException { + MarketStatus status = mapper.readValue("{\"s\":\"no_data\"}", MarketStatus.class); + + assertThat(status.days()).isEmpty(); + assertThat(status.isEmpty()).isTrue(); + } + + @Test + void rejectsUnknownStatusField() { + assertThatThrownBy(() -> mapper.readValue("{\"s\":\"weird\"}", MarketStatus.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("'weird'"); + } + + @Test + void rejectsMismatchedArraySizes() { + String json = + """ + { "s": "ok", + "date": [1706673600, 1706760000], + "status": ["open"] } + """; + + assertThatThrownBy(() -> mapper.readValue(json, MarketStatus.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("different sizes"); + } + + @Test + void rejectsResponseMissingArrays() { + String json = "{\"s\":\"ok\"}"; + + assertThatThrownBy(() -> mapper.readValue(json, MarketStatus.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("expected 'date' and 'status' arrays"); + } + + @Test + void rejectsResponseWhereDateIsArrayButStatusIsMissing() { + // Covers the right-hand branch of the `||` in `!dates.isArray() || !statuses.isArray()`: + // dates is a valid array, but statuses is absent. Without this test, the short-circuit + // means the second condition is only ever evaluated when the first is false and matches. + String json = + """ + { "s": "ok", + "date": [1706673600] } + """; + + assertThatThrownBy(() -> mapper.readValue(json, MarketStatus.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("expected 'date' and 'status' arrays"); + } +} diff --git a/src/test/java/com/marketdata/sdk/MarketsResourceTest.java b/src/test/java/com/marketdata/sdk/MarketsResourceTest.java new file mode 100644 index 0000000..9db68cf --- /dev/null +++ b/src/test/java/com/marketdata/sdk/MarketsResourceTest.java @@ -0,0 +1,461 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.marketdata.sdk.exception.AuthenticationError; +import com.marketdata.sdk.exception.NetworkError; +import com.marketdata.sdk.exception.ParseError; +import com.marketdata.sdk.exception.RateLimitError; +import com.marketdata.sdk.exception.ServerError; +import com.marketdata.sdk.markets.MarketStatus; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +/** + * Exercises the full resource → transport → HTTP path against an in-process {@link HttpServer} (JDK + * built-in — no extra mock dep). Verifies URL construction, query-param encoding, response + * decoding, error mapping, and rate-limit header parsing. + */ +class MarketsResourceTest { + + private HttpServer server; + private final AtomicReference lastRequest = new AtomicReference<>(); + private RouteHandler handler; + + @BeforeEach + void startServer() throws IOException { + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + handler = new RouteHandler(); + server.createContext("/", handler); + server.start(); + } + + @AfterEach + void stopServer() { + server.stop(0); + } + + private MarketDataClient newClient() { + int port = server.getAddress().getPort(); + return new MarketDataClient("test-key", "http://127.0.0.1:" + port, null, false); + } + + // ---------- success paths ---------- + + /** + * The 5 paths exercised below are the load-bearing scenarios — each runs once for {@link + * CallMode#SYNC} and once for {@link CallMode#ASYNC} so we satisfy SDK requirements §13's "tests + * must cover both sync and async variants for every endpoint" without duplicating every single + * mechanical case. + */ + @ParameterizedTest + @EnumSource(CallMode.class) + void statusNoArgsHitsCanonicalUrlAndDecodesPayload(CallMode mode) { + handler.setResponse( + 200, + """ + { "s":"ok", "date":[1706673600,1706760000], "status":["open","closed"] } + """, + List.of( + rateLimitHeader("limit", "50000"), + rateLimitHeader("remaining", "49500"), + rateLimitHeader("reset", "1735689600"), + rateLimitHeader("consumed", "1"))); + + try (var client = newClient()) { + MarketStatus result = mode.statusNoArgs(client.markets()); + + assertThat(result.days()).hasSize(2); + assertThat(result.days().get(0).open()).isTrue(); + assertThat(result.days().get(1).open()).isFalse(); + + RecordedRequest req = lastRequest.get(); + assertThat(req.path).isEqualTo("/v1/markets/status/"); + assertThat(req.query).isNull(); + assertThat(req.headers.firstValue("Authorization")).hasValue("Bearer test-key"); + assertThat(req.headers.firstValue("User-Agent")) + .get() + .asString() + .startsWith("marketdata-sdk-java/"); + assertThat(req.headers.firstValue("Accept")).hasValue("application/json"); + + RateLimits rl = client.getRateLimits(); + assertThat(rl).isNotNull(); + assertThat(rl.limit()).isEqualTo(50000L); + assertThat(rl.remaining()).isEqualTo(49500L); + assertThat(rl.consumed()).isEqualTo(1L); + } + } + + @ParameterizedTest + @EnumSource(CallMode.class) + void statusForDateBuildsDateQueryParam(CallMode mode) { + handler.setResponse( + 200, "{\"s\":\"ok\",\"date\":[1706760000],\"status\":[\"open\"]}", List.of()); + + try (var client = newClient()) { + MarketStatus result = mode.statusForDate(client.markets(), LocalDate.of(2024, 2, 1)); + + assertThat(result.days()).hasSize(1); + assertThat(lastRequest.get().path).isEqualTo("/v1/markets/status/"); + assertThat(lastRequest.get().query).isEqualTo("date=2024-02-01"); + } + } + + @ParameterizedTest + @EnumSource(CallMode.class) + void statusForRangeBuildsFromAndToQueryParams(CallMode mode) { + handler.setResponse( + 200, "{\"s\":\"ok\",\"date\":[1706673600],\"status\":[\"open\"]}", List.of()); + + try (var client = newClient()) { + mode.statusForRange(client.markets(), LocalDate.of(2024, 1, 31), LocalDate.of(2024, 2, 5)); + + assertThat(lastRequest.get().query).isEqualTo("from=2024-01-31&to=2024-02-05"); + } + } + + @Test + void rangeWithSwappedBoundsThrowsIllegalArgument() { + try (var client = newClient()) { + assertThatThrownBy( + () -> client.markets().status(LocalDate.of(2024, 2, 5), LocalDate.of(2024, 1, 31))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must not be after"); + } + } + + // ---------- async-specific smoke ---------- + + /** + * Verifies that {@code statusAsync()} returns a real {@link + * java.util.concurrent.CompletableFuture} usable with the standard {@code .get()} contract + * (checked exception path). The {@code @ParameterizedTest}s above cover .join() semantics; this + * one covers .get(). + */ + @Test + void statusAsyncReturnsRealCompletableFuture() throws Exception { + handler.setResponse( + 200, "{\"s\":\"ok\",\"date\":[1706760000],\"status\":[\"closed\"]}", List.of()); + + try (var client = newClient()) { + MarketStatus async = client.markets().statusAsync().get(); + assertThat(async.days()).hasSize(1); + assertThat(async.days().get(0).open()).isFalse(); + } + } + + // ---------- no-data and error paths ---------- + + @ParameterizedTest + @EnumSource(CallMode.class) + void notFoundWithNoDataBodyDecodesAsEmpty(CallMode mode) { + handler.setResponse(404, "{\"s\":\"no_data\"}", List.of()); + + try (var client = newClient()) { + MarketStatus result = mode.statusNoArgs(client.markets()); + assertThat(result.isEmpty()).isTrue(); + } + } + + @ParameterizedTest + @EnumSource(CallMode.class) + void http401ThrowsAuthenticationError(CallMode mode) { + handler.setResponse(401, "{}", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> mode.statusNoArgs(client.markets())) + .isInstanceOf(AuthenticationError.class) + .satisfies( + t -> { + AuthenticationError ae = (AuthenticationError) t; + assertThat(ae.getStatusCode()).isEqualTo(401); + assertThat(ae.getRequestUrl()).contains("/v1/markets/status/"); + }); + } + } + + @Test + void http429ThrowsRateLimitError() { + handler.setResponse(429, "{}", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()).isInstanceOf(RateLimitError.class); + } + } + + @Test + void http500ThrowsServerError() { + handler.setResponse(500, "{}", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()).isInstanceOf(ServerError.class); + } + } + + // ---------- malformed responses ---------- + + @ParameterizedTest + @EnumSource(CallMode.class) + void garbageBodyOnSuccessProducesParseError(CallMode mode) { + handler.setResponse(200, "this is plainly not json", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> mode.statusNoArgs(client.markets())) + .isInstanceOf(ParseError.class) + .hasMessageContaining("Failed to decode"); + } + } + + @Test + void emptyBodyOnSuccessProducesParseError() { + handler.setResponse(200, "", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()).isInstanceOf(ParseError.class); + } + } + + @Test + void unknownStatusFieldProducesParseError() { + handler.setResponse(200, "{\"s\":\"weird\"}", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()) + .isInstanceOf(ParseError.class) + .hasMessageContaining("weird"); + } + } + + @Test + void responseMissingArraysProducesParseError() { + handler.setResponse(200, "{\"s\":\"ok\"}", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()) + .isInstanceOf(ParseError.class) + .hasMessageContaining("date"); + } + } + + @Test + void mismatchedArraySizesProduceParseError() { + handler.setResponse( + 200, "{\"s\":\"ok\",\"date\":[1706673600,1706760000],\"status\":[\"open\"]}", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()) + .isInstanceOf(ParseError.class) + .hasMessageContaining("different sizes"); + } + } + + // ---------- weird headers ---------- + + @Test + void successWithoutAnyRateLimitHeadersLeavesSnapshotNull() { + handler.setResponse( + 200, "{\"s\":\"ok\",\"date\":[1706673600],\"status\":[\"open\"]}", List.of()); + + try (var client = newClient()) { + client.markets().status(); + assertThat(client.getRateLimits()).isNull(); + } + } + + @Test + void partialRateLimitHeadersStillProduceSnapshot() { + handler.setResponse( + 200, + "{\"s\":\"ok\",\"date\":[1706673600],\"status\":[\"open\"]}", + List.of( + new String[] {"x-api-ratelimit-limit", "100000"}, + new String[] {"x-api-ratelimit-remaining", "99999"})); + + try (var client = newClient()) { + client.markets().status(); + + RateLimits rl = client.getRateLimits(); + assertThat(rl).isNotNull(); + assertThat(rl.limit()).isEqualTo(100_000L); + assertThat(rl.remaining()).isEqualTo(99_999L); + assertThat(rl.consumed()).isEqualTo(0L); // missing → defaulted + } + } + + @Test + void allUnparseableRateLimitHeadersAreIgnoredAsAbsent() { + handler.setResponse( + 200, + "{\"s\":\"ok\",\"date\":[1706673600],\"status\":[\"open\"]}", + List.of( + new String[] {"x-api-ratelimit-limit", "not-a-number"}, + new String[] {"x-api-ratelimit-remaining", "still-not"}, + new String[] {"x-api-ratelimit-reset", "??"}, + new String[] {"x-api-ratelimit-consumed", "wat"})); + + try (var client = newClient()) { + client.markets().status(); + assertThat(client.getRateLimits()).isNull(); + } + } + + /** + * Regression for Issue #4: a successful request that arrives without rate-limit headers must not + * clobber the previously-cached snapshot. The API's rate-limit middleware can silently swallow + * its own errors and serve the response without headers; if we overwrote the snapshot with {@code + * null} on each such response, the user-visible {@code getRateLimits()} would flicker between + * populated and {@code null} across consecutive successful calls. Spec §8 mandates " update + * client-level snapshot" — implicitly only when there's something to update. + */ + @Test + void successWithoutHeadersDoesNotClobberPreviousSnapshot() { + handler.setResponse( + 200, + "{\"s\":\"ok\",\"date\":[1706673600],\"status\":[\"open\"]}", + List.of( + new String[] {"x-api-ratelimit-limit", "50000"}, + new String[] {"x-api-ratelimit-remaining", "49000"}, + new String[] {"x-api-ratelimit-reset", "1735689600"}, + new String[] {"x-api-ratelimit-consumed", "1000"})); + + try (var client = newClient()) { + client.markets().status(); + RateLimits before = client.getRateLimits(); + assertThat(before).isNotNull(); + assertThat(before.remaining()).isEqualTo(49000L); + + // Same client, second successful call — but the server didn't include rate-limit headers + // this time (e.g. middleware glitch on the API side). + handler.setResponse( + 200, "{\"s\":\"ok\",\"date\":[1706760000],\"status\":[\"closed\"]}", List.of()); + client.markets().status(); + + RateLimits after = client.getRateLimits(); + assertThat(after) + .as("snapshot must retain the last known rate-limit data, not reset to null") + .isNotNull(); + assertThat(after.remaining()).isEqualTo(49000L); + } + } + + @Test + void errorResponseWithoutCfRayProducesNullRequestId() { + handler.setResponse(401, "{}", List.of()); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()) + .isInstanceOf(AuthenticationError.class) + .satisfies(t -> assertThat(((AuthenticationError) t).getRequestId()).isNull()); + } + } + + @Test + void errorResponseWithCfRayPropagatesRequestId() { + handler.setResponse(401, "{}", List.of(new String[] {"cf-ray", "abc123-XYZ"})); + + try (var client = newClient()) { + assertThatThrownBy(() -> client.markets().status()) + .isInstanceOf(AuthenticationError.class) + .satisfies( + t -> assertThat(((AuthenticationError) t).getRequestId()).isEqualTo("abc123-XYZ")); + } + } + + // ---------- network failure (connect refused — fast-failing proxy for timeout class) ---------- + + /** + * The 99-second per-request timeout is fixed by SDK requirements §10. Forcing a real timeout in a + * test would block for ~99 s, which we don't want. Instead we exercise the {@link NetworkError} + * path by pointing the client at a port nothing is listening on (TCP RST → fast failure). This + * proves the transport surfaces transport-level failures as a typed exception rather than letting + * raw {@code IOException}s leak. + */ + @ParameterizedTest + @EnumSource(CallMode.class) + void connectionRefusedProducesNetworkError(CallMode mode) throws IOException { + // Bind to an ephemeral port and immediately close — the OS guarantees that connecting to a + // recently-closed local port produces a fast RST (Linux/macOS) or ConnectException (Windows) + // rather than the long timeouts some hardened sandboxes serve on the historically-privileged + // port 1. The narrow window where another process could grab the port before our connect + // attempt is theoretical on CI. + int closedPort; + try (java.net.ServerSocket probe = + new java.net.ServerSocket(0, 0, java.net.InetAddress.getByName("127.0.0.1"))) { + closedPort = probe.getLocalPort(); + } + + try (var client = + new MarketDataClient("test-key", "http://127.0.0.1:" + closedPort, null, false)) { + + assertThatThrownBy(() -> mode.statusNoArgs(client.markets())) + .isInstanceOf(NetworkError.class) + .satisfies( + t -> { + NetworkError ne = (NetworkError) t; + assertThat(ne.getCause()).isNotNull(); + assertThat(ne.getRequestUrl()).contains("127.0.0.1:" + closedPort); + }); + } + } + + // ---------- helpers ---------- + + // CallMode (sync vs async dispatcher) lives in its own file so the integration-test source set + // can reuse it. See CallMode.java in this same package. + + private static String[] rateLimitHeader(String suffix, String value) { + return new String[] {"x-api-ratelimit-" + suffix, value}; + } + + private record RecordedRequest(String path, String query, java.net.http.HttpHeaders headers) {} + + private final class RouteHandler implements HttpHandler { + private int statusCode = 200; + private String body = "{}"; + private List extraHeaders = List.of(); + + void setResponse(int code, String body, List extraHeaders) { + this.statusCode = code; + this.body = body; + this.extraHeaders = extraHeaders; + } + + @Override + public void handle(HttpExchange exchange) throws IOException { + // Snapshot request shape for assertions. + URI uri = exchange.getRequestURI(); + var headerMap = new java.util.HashMap>(); + exchange.getRequestHeaders().forEach((k, v) -> headerMap.put(k, new ArrayList<>(v))); + lastRequest.set( + new RecordedRequest( + uri.getPath(), + uri.getRawQuery(), + java.net.http.HttpHeaders.of(headerMap, (a, b) -> true))); + + for (String[] h : extraHeaders) { + exchange.getResponseHeaders().add(h[0], h[1]); + } + byte[] bodyBytes = body.getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + exchange.sendResponseHeaders(statusCode, bodyBytes.length); + exchange.getResponseBody().write(bodyBytes); + exchange.getResponseBody().close(); + } + } +} diff --git a/src/test/java/com/marketdata/sdk/RateLimitHeadersTest.java b/src/test/java/com/marketdata/sdk/RateLimitHeadersTest.java new file mode 100644 index 0000000..b3074f5 --- /dev/null +++ b/src/test/java/com/marketdata/sdk/RateLimitHeadersTest.java @@ -0,0 +1,164 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import java.net.http.HttpHeaders; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import org.junit.jupiter.api.Test; + +class RateLimitHeadersTest { + + // ---------- helpers ---------- + + /** + * Builds an immutable {@link HttpHeaders} from a flat key→value map. The JDK only exposes + * builders via {@link java.net.http.HttpClient}; this is the canonical workaround using {@link + * HttpHeaders#of}. + */ + private static HttpHeaders headersOf(Map entries) { + Map> multi = new TreeMap<>(); + entries.forEach((k, v) -> multi.put(k, List.of(v))); + return HttpHeaders.of(multi, (a, b) -> true); + } + + // ---------- happy path ---------- + + @Test + void parsesAllFourHeaders() { + HttpHeaders headers = + headersOf( + Map.of( + "x-api-ratelimit-limit", "1000", + "x-api-ratelimit-remaining", "987", + "x-api-ratelimit-reset", "1714867200", + "x-api-ratelimit-consumed", "13")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNotNull(); + assertThat(rl.limit()).isEqualTo(1000L); + assertThat(rl.remaining()).isEqualTo(987L); + assertThat(rl.reset()).isEqualTo(Instant.ofEpochSecond(1714867200L)); + assertThat(rl.consumed()).isEqualTo(13L); + } + + // ---------- the all-null short-circuit ---------- + + @Test + void returnsNullWhenNoRateLimitHeadersPresent() { + // With every header absent the long `&&` chain in `parse()` evaluates each side fully — + // covers the "all four are null" branches. + HttpHeaders headers = headersOf(Map.of("content-type", "application/json")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNull(); + } + + // ---------- partial headers (one present, others missing) ---------- + + @Test + void onlyLimitPresentZerosTheOthers() { + // Covers the `null` branch of three of the four `x != null ? x : 0L` ternaries while + // keeping `limit` non-null (the all-null short-circuit doesn't apply). + HttpHeaders headers = headersOf(Map.of("x-api-ratelimit-limit", "500")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNotNull(); + assertThat(rl.limit()).isEqualTo(500L); + assertThat(rl.remaining()).isZero(); + assertThat(rl.reset()).isEqualTo(Instant.ofEpochSecond(0L)); + assertThat(rl.consumed()).isZero(); + } + + @Test + void onlyConsumedPresentZerosTheOthers() { + // Covers the case where the head of the && chain is null but the tail is not — exercises + // a different short-circuit path than onlyLimitPresent. + HttpHeaders headers = headersOf(Map.of("x-api-ratelimit-consumed", "42")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNotNull(); + assertThat(rl.consumed()).isEqualTo(42L); + assertThat(rl.limit()).isZero(); + } + + @Test + void onlyRemainingPresentExitsAtSecondCondition() { + // Forces the && chain past `limit == null` and stops at `remaining == null`. Without this + // test, the false-branch of the second condition is never evaluated. + HttpHeaders headers = headersOf(Map.of("x-api-ratelimit-remaining", "1234")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNotNull(); + assertThat(rl.remaining()).isEqualTo(1234L); + assertThat(rl.limit()).isZero(); + } + + @Test + void onlyResetPresentExitsAtThirdCondition() { + // Forces the && chain past `limit` and `remaining` to evaluate `reset == null` as false. + HttpHeaders headers = headersOf(Map.of("x-api-ratelimit-reset", "1735689600")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNotNull(); + assertThat(rl.reset()).isEqualTo(Instant.ofEpochSecond(1735689600L)); + assertThat(rl.limit()).isZero(); + assertThat(rl.remaining()).isZero(); + assertThat(rl.consumed()).isZero(); + } + + // ---------- malformed values ---------- + + @Test + void malformedNumberIsTreatedAsAbsent() { + // readLong's catch(NumberFormatException) returns null; the header is then treated as + // missing. With every header malformed the result must be null, same as none-present. + HttpHeaders headers = + headersOf( + Map.of( + "x-api-ratelimit-limit", "not-a-number", + "x-api-ratelimit-remaining", "also-broken")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNull(); + } + + @Test + void valuesAreTrimmedBeforeParsing() { + HttpHeaders headers = headersOf(Map.of("x-api-ratelimit-limit", " 1000 ")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNotNull(); + assertThat(rl.limit()).isEqualTo(1000L); + } + + // ---------- sanity: parse() doesn't depend on URI/method ---------- + + @Test + void parseIgnoresNonRateLimitHeaders() { + URI dummy = URI.create("https://example/"); + HttpHeaders headers = + headersOf( + Map.of( + "cf-ray", "abc", + "content-type", "application/json", + "x-api-ratelimit-limit", "100")); + + RateLimits rl = RateLimitHeaders.parse(headers); + + assertThat(rl).isNotNull(); + assertThat(rl.limit()).isEqualTo(100L); + assertThat(dummy).isNotNull(); // silence unused + } +} diff --git a/src/test/java/com/marketdata/sdk/RequestSpecTest.java b/src/test/java/com/marketdata/sdk/RequestSpecTest.java new file mode 100644 index 0000000..c192322 --- /dev/null +++ b/src/test/java/com/marketdata/sdk/RequestSpecTest.java @@ -0,0 +1,56 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class RequestSpecTest { + + @Test + void buildPreservesPathAndOmitsNullQueryParams() { + // Covers both branches of `if (value != null)` in Builder.query: the null branch is + // exercised by .query("ignored", null), the non-null branch by .query("date", "2024-05-01"). + RequestSpec spec = + RequestSpec.get("markets/status") + .query("date", "2024-05-01") + .query("ignored", null) + .query("from", "2024-01-01") + .build(); + + assertThat(spec.path()).isEqualTo("markets/status"); + assertThat(spec.queryParams()) + .containsExactly( + java.util.Map.entry("date", "2024-05-01"), java.util.Map.entry("from", "2024-01-01")); + assertThat(spec.queryParams()).doesNotContainKey("ignored"); + } + + @Test + void buildWithNoQueryParamsProducesEmptyMap() { + RequestSpec spec = RequestSpec.get("markets/status").build(); + + assertThat(spec.path()).isEqualTo("markets/status"); + assertThat(spec.queryParams()).isEmpty(); + } + + @Test + void queryParamsAreImmutable() { + RequestSpec spec = RequestSpec.get("markets/status").query("date", "2024-05-01").build(); + + org.assertj.core.api.Assertions.assertThatThrownBy( + () -> spec.queryParams().put("hacked", "value")) + .isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void queryConvertsNonStringValuesViaToString() { + // value.toString() is called when value is non-null. Numbers, enums, etc. should serialise + // through their toString(). + RequestSpec spec = + RequestSpec.get("markets/candles") + .query("countback", 5) + .query("limit", Long.valueOf(100L)) + .build(); + + assertThat(spec.queryParams()).containsEntry("countback", "5").containsEntry("limit", "100"); + } +} diff --git a/src/test/java/com/marketdata/sdk/VersionTest.java b/src/test/java/com/marketdata/sdk/VersionTest.java new file mode 100644 index 0000000..8ab9742 --- /dev/null +++ b/src/test/java/com/marketdata/sdk/VersionTest.java @@ -0,0 +1,43 @@ +package com.marketdata.sdk; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class VersionTest { + + // ---------- resolve: covers all 4 branch outcomes of the `!= null && !isBlank()` chain + // ---------- + + @Test + void resolveReturnsDetectedVersionWhenPresent() { + assertThat(Version.resolve("1.2.3")).isEqualTo("1.2.3"); + } + + @Test + void resolveFallsBackWhenDetectedIsNull() { + assertThat(Version.resolve(null)).isEqualTo(Version.FALLBACK); + } + + @Test + void resolveFallsBackWhenDetectedIsEmpty() { + assertThat(Version.resolve("")).isEqualTo(Version.FALLBACK); + } + + @Test + void resolveFallsBackWhenDetectedIsBlank() { + // Exercises the second condition independently (`!isBlank()` evaluated `false` on whitespace). + assertThat(Version.resolve(" ")).isEqualTo(Version.FALLBACK); + } + + // ---------- current: lives at the package boundary; only asserts the contract ---------- + + @Test + void currentNeverReturnsNullOrBlank() { + // From class files in tests, the manifest has no Implementation-Version so current() + // exercises the fallback path. From a published JAR it would return the manifest value. + // Either way the contract holds. + String v = Version.current(); + assertThat(v).isNotNull().isNotBlank(); + } +} diff --git a/src/test/java/com/marketdata/sdk/exception/MarketDataExceptionTest.java b/src/test/java/com/marketdata/sdk/exception/MarketDataExceptionTest.java index 9ce9f7b..16c7d39 100644 --- a/src/test/java/com/marketdata/sdk/exception/MarketDataExceptionTest.java +++ b/src/test/java/com/marketdata/sdk/exception/MarketDataExceptionTest.java @@ -98,6 +98,25 @@ void everySubtypeExposesBothConstructors() { } } + @Test + void supportInfoFormatsNullContextAsNotApplicable() { + // When the exception is built from ErrorContext.empty() (e.g. client-side validation + // errors that fire before any HTTP request), getSupportInfo() must render each null + // field as "(n/a)" instead of literal "null". Covers the null-branches of the three + // ternaries in MarketDataException.getSupportInfo. + var error = new BadRequestError("symbol must not be blank", ErrorContext.empty()); + + String supportInfo = error.getSupportInfo(); + + assertThat(supportInfo) + .contains("BadRequestError") + .contains("symbol must not be blank") + .contains("Status code: (n/a)") + .contains("Request ID: (n/a)") + .contains("Request URL: (n/a)") + .doesNotContain("null"); + } + @Test void supportInfoNeverContainsSensitiveData() { // The exception itself never receives the token; we just