Merge pull request #8 from MarketDataApp/clean-architecture-restart

Clean-architecture restart: SDK foundation + `utilities` resource

Author: MarketDataDev03

.gitignore

@@ -25,7 +25,16 @@ Thumbs.db
 .env
 .env.local
 
+# Python (examples/mock-server)
+.venv/
+__pycache__/
+*.pyc
+
 # Logs / coverage
 *.log
 hs_err_pid*
 replay_pid*
+
+
+.claude/reviews/
+.claude/prompts/

CHANGELOG.md

@@ -10,6 +10,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Default retry attempts corrected from 3 to 4 (one initial + three retries) to
   match SDK requirements §9.3 ("max 3 retries, yielding 4 total attempts").
+- `.env` parser now strips trailing inline `# comment` markers (quote-aware: a
+  `#` inside single/double quotes or adjacent to value chars stays part of the
+  value). Previously a line like `MARKETDATA_TOKEN=abc # prod` produced the
+  literal value `abc # prod`, which passes `validateApiKey` (printable ASCII)
+  and surfaces later as a confusing `AuthenticationError` far from the .env
+  source that caused it.
+- `RequestHeaders` canonical constructor now rejects a `null` `headers` map
+  with a clear `NullPointerException` naming the field, replacing the bare
+  `Map.copyOf(null)` NPE that left consumers hunting for the offending
+  argument. The wire-format deserializer additionally intercepts a top-level
+  JSON `null` body via `JsonDeserializer#getNullValue` and surfaces it as a
+  `ParseError` carrying the endpoint URL, status, and request id — preventing
+  a malformed `/headers/` response from manifesting as an opaque NPE further
+  down the call stack.
 
 ### Added
 - Project scaffold per ADRs 001–007: Gradle Kotlin DSL build, JDK 17 toolchain,

CLAUDE.md

@@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 ## Repository state
 
-This repo currently contains **documentation only** — no Java sources, no build scripts. Branch `00_base_setup` is the pre-implementation phase: all foundational technical decisions are being captured as ADRs *before* code lands. There is therefore nothing to build, lint, or test yet. When implementation starts, the build will be Gradle (Kotlin DSL) per ADR-003 — see "Locked-in tech stack" below.
+This repo contains a working Java SDK in active development on branch `clean-architecture-restart`. Gradle 9.0 (Kotlin DSL) build per ADR-003; ~48 main + ~28 test source files; full CI matrix per ADR-002. The transport, retry, rate-limit, status-cache, and exception layers are wired; only the `utilities` resource façade is implemented today — stocks/options/funds/markets resources are still to come (see "Deliberately deferred" below).
 
 Sibling repo: `../api/` is the backend (Python/Django). The Python SDK lives at `../../sdk-py/` (referenced from ADRs). The cross-language `sdk-requirements.md` is referenced as `../sdk-requirements.md` from inside `docs/`; it is canonical but not committed in this repo.
 
@@ -62,14 +62,17 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements](
 
 **Already wired in:**
 - §1.1 client object — `MarketDataClient` with two public constructors: a no-arg one for production (everything from the cascade) and a 4-arg `(apiKey, baseUrl, apiVersion, validateOnStartup)` for tests and short-lived runtimes. All fields `final` (immutable). Default base URL `https://api.marketdata.app`, default API version `v1`, single shared `HttpClient`, `User-Agent: marketdata-sdk-java/{version}` (version auto-detected from JAR manifest), `close()` for resource release, `getRateLimits()` accessor.
-- §4 configuration cascade — `Configuration.resolve(...)` does explicit → `MARKETDATA_*` env var → `.env` in CWD → default. Env var names live in `EnvVars` (package-private, in the SDK root package). The 4-arg constructor's parameters feed step 1; the no-arg constructor skips it and starts at step 2.
-- §5 demo mode + `validateOnStartup` parameter on the 4-arg constructor (defaults to `true` via the no-arg constructor); token redaction via `Tokens.redact` (matches the spec example `***…***YKT0`).
+- §4 configuration cascade — `Configuration.resolve(...)` does explicit → `MARKETDATA_*` env var → `.env` in CWD → default. Env var names live in `EnvVars` (package-private, in the SDK root package). `baseUrl` and `apiVersion` are normalized (trailing/leading slashes stripped) and validated (scheme http/https, host present, no query/fragment/user-info on `baseUrl`; `[A-Za-z0-9._-]+` on `apiVersion`) at resolution time, so misconfigured cascade inputs fail at construction with a clear message instead of producing malformed URLs later.
+- §5 demo mode + `validateOnStartup` parameter on the 4-arg constructor (defaults to `true` via the no-arg constructor); token redaction via `Tokens.redact` (matches the spec example `***…***YKT0`). `runStartupValidation` fires a single `GET /user/` (unversioned, like `/status/` and `/headers/`) via `utilities.validateAuth()` with `RetryPolicy.noRetry()`, skipping in demo mode; the no-retry policy keeps construction snappy on a slow/down API.
 - §6 sealed `MarketDataException` hierarchy with the 7 canonical subtypes and full support context (`requestId`, `requestUrl`, `statusCode`, `timestamp`, `exceptionType`) + `getSupportInfo()`.
+- §7 logging — `MarketDataLogging.configure(...)` installs `CanonicalLogFormatter` (the spec's `{timestamp} - {logger_name} - {level} - {message}` shape) on `com.marketdata.sdk` and applies the level from `MARKETDATA_LOGGING_LEVEL`. Detects consumer-pre-configured loggers (handler attached or level set) and backs off entirely so embedding the SDK doesn't clobber existing logging setups.
+- §8 rate-limit tracking — `RateLimitHeaders.parse` reads the four `x-api-ratelimit-*` headers per response (all-or-nothing: a partial header set returns null rather than emitting a snapshot with phantom zeros); `HttpTransport.latestRateLimits` keeps the latest snapshot, exposed via `client.getRateLimits()`. §10.3 preflight (`HttpTransport.checkRateLimitPreflight`) fails fast on `remaining=0`, but only while `now < reset` — once the reset window elapses the request goes through so the next response refreshes the snapshot.
+- §9 retry/backoff — `RetryPolicy` (4 total attempts = 1 initial + 3 retries, exponential 1s→30s per §9.3) wired into `HttpTransport.executeAsync` via a per-attempt loop using `CompletableFuture.delayedExecutor` (no scheduled threads). Network errors and HTTP 501–599 retry; 500 and 4xx do not. §9.4 `Retry-After`: parsed from response headers via `RetryAfterHeader.parse` (supports both delta-seconds and HTTP-date), attached to `ServerError`, and honored by `RetryPolicy.backoffDelay` as an override of the calculated exponential delay. §9.5 `/status/` cache: `StatusCache` (stale-while-revalidate, 270s refresh / 300s expiry) gates retries on services reported offline; `HttpTransport.cacheAllowsRetry` has a self-referential bypass for `/status/` itself so the cache can never block its own refresh.
 - §10 timeouts: `REQUEST_TIMEOUT = 99s` and `CONNECT_TIMEOUT = 2s` exposed as constants on `MarketDataClient`. Connect timeout is wired into the `HttpClient`; the per-request 99 s timeout is applied via `HttpRequest.Builder#timeout` in `HttpTransport.buildRequest`.
 - §12 concurrency: 50-permit `AsyncSemaphore` on `HttpTransport` with acquire/release wired around every dispatch. The custom semaphore replaces `java.util.concurrent.Semaphore` so `executeAsync` never parks the caller's thread on a full pool (ADR-007).
-- §9 retry/backoff: `RetryPolicy` (4 total attempts = 1 initial + 3 retries, exponential 1s→30s per §9.3) wired into `HttpTransport.executeAsync` via a per-attempt loop using `CompletableFuture.delayedExecutor` (no scheduled threads). Network errors and HTTP 501–599 retry; 500 and 4xx do not.
+- §13.5 response object — `Response<T>` wrapper exposes typed `data()`, `rawBody()` (defensive copy), `statusCode()`, `requestUrl()`, `requestId()`, format predicates (`isJson()`/`isCsv()`/`isHtml()`), `isNoData()`, and `saveToFile(Path)`. Every resource endpoint returns `Response<T>` so consumers get a uniform surface regardless of format.
 - §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).
+- §16 security: tokens never logged verbatim (use `Tokens.redact`); TLS validated by default (`HttpClient` does not expose a skip-verify option); query strings stripped from log output (logged as `/path?…`) so request params never persist to logs — exception context retains the full URI for diagnostic use; `EnvVars.systemLookup()` restricts reads to the declared `MARKETDATA_*` keys.
 - 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).
@@ -79,21 +82,14 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements](
   - `-PtestJdk=N` is wired to **all** `Test` tasks (`test` and `integrationTest`) via `tasks.withType<Test>().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):**
-- §1.2 resource groupings (`client.stocks`, `client.options`, `client.funds`, `client.markets`, `client.utilities`).
-- §2 endpoint method coverage; §3 universal parameters; §11 wire-format decoding.
-- §5 actual `/user/` startup validation call (the `validateOnStartup` flag is the seam; the call itself comes with the request layer).
-- §7 honoring `MARKETDATA_LOGGING_LEVEL` and the spec's exact `{timestamp} - {logger_name} - {level} - {message}` format. Currently the SDK uses `java.util.logging` with default formatting; consumers can attach their own handler.
-- §8 rate-limit header parsing, pre-flight check, request-scoped attachment.
-- §9 `/status/` cache workflow and `Retry-After` header override (retry/backoff itself lives in `RetryPolicy` and is wired; what is missing is the `/status/` pre-check before retrying 501–599 and respecting the server-specified `Retry-After` over the calculated exponential backoff).
-- §13 100% coverage threshold via JaCoCo `violationRules`; deferred until there is functional code worth the threshold.
+**Deliberately deferred (require the per-resource layer to land first):**
+- §1.2 resource groupings — only `client.utilities()` is wired today; `client.stocks`, `client.options`, `client.funds`, `client.markets` still to come.
+- §2 endpoint method coverage; §3 universal parameters; §11 wire-format decoding for resources beyond utilities. The plumbing (`ParallelArrays.zip` helper for the parallel-arrays shape, `JsonResponseParser`, `Response<T>` wrapper) is in place — each new endpoint just declares its fields and row builder.
+- §8 request-scoped rate-limit attachment — today the snapshot is client-level (via `client.getRateLimits()`); attaching a per-response snapshot to `Response<T>` is a small follow-up when a consumer needs it.
+- §13 100% coverage threshold via JaCoCo `violationRules`; deferred until the resource layer lands so the threshold meaningfully ratchets functional code, not just scaffolding.
 
 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:**
-- `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.
-- `Retry-After` server header is parsed and respected by neither `RetryPolicy` nor `HttpTransport`. Today every retry uses the calculated exponential backoff (`min(1s × 2^N, 30s)`). Implementing the override needs the response headers to reach `RetryPolicy.backoffDelay`, which today only sees the attempt index — most natural path is to surface a `Duration` on `ServerError` (or thread it through a separate channel) when 5xx responses carry the header. Follow-up of the §9 work.
-
 ## 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.

Makefile

@@ -0,0 +1,92 @@
+# Convenience wrapper around ./gradlew and the consumer-test / mock-server.
+# Run `make` (no args) or `make help` for the target list.
+
+CONSUMER_DIR := examples/consumer-test
+MOCK_DIR     := examples/mock-server
+
+.DEFAULT_GOAL := help
+
+# ---------------------------------------------------------------------------
+# Help
+# ---------------------------------------------------------------------------
+
+.PHONY: help
+help: ## Show this help
+	@awk 'BEGIN {FS = ":.*##"; printf "Usage:\n  make \033[36m<target>\033[0m\n\nTargets:\n"} \
+	  /^# ===/ { in_section = 1; next } \
+	  /^[a-zA-Z_-]+:.*?##/ { printf "  \033[36m%-22s\033[0m %s\n", $$1, $$2 } \
+	  /^## ---/ { printf "\n\033[33m%s\033[0m\n", substr($$0, 5) }' $(MAKEFILE_LIST)
+	@echo ""
+	@echo "Typical workflow:"
+	@echo "  make publish              # publish SDK to mavenLocal"
+	@echo "  make mock-server          # in another terminal"
+	@echo "  make demo-config          # in a third terminal"
+
+# ---------------------------------------------------------------------------
+## --- SDK build ---
+# ---------------------------------------------------------------------------
+
+.PHONY: build
+build: ## Full build: unit tests + Spotless + JaCoCo (JDK 17)
+	./gradlew build
+
+.PHONY: test
+test: ## Unit tests only
+	./gradlew test
+
+.PHONY: spotless
+spotless: ## Apply code formatting
+	./gradlew spotlessApply
+
+.PHONY: clean
+clean: ## Clean all Gradle outputs (SDK + consumer-test)
+	./gradlew clean
+	cd $(CONSUMER_DIR) && ./gradlew clean
+
+.PHONY: publish
+publish: ## Publish SDK to ~/.m2 (prereq for any demo)
+	./gradlew publishToMavenLocal
+
+# ---------------------------------------------------------------------------
+## --- Mock server ---
+# ---------------------------------------------------------------------------
+
+.PHONY: mock-server
+mock-server: ## Start the FastAPI mock server (blocks, Ctrl+C to stop)
+	cd $(MOCK_DIR) && ./run.sh
+
+# ---------------------------------------------------------------------------
+## --- Consumer demos (need `make publish` first) ---
+# ---------------------------------------------------------------------------
+
+.PHONY: demo-quickstart
+demo-quickstart: ## Idiomatic per-resource usage tour (live API; grows as resources land)
+	cd $(CONSUMER_DIR) && ./gradlew runQuickstart
+
+.PHONY: demo-live
+demo-live: ## Live API smoke (needs MARKETDATA_TOKEN in examples/consumer-test/.env)
+	cd $(CONSUMER_DIR) && ./gradlew runLive
+
+.PHONY: demo-config
+demo-config: ## Demo mode, cascade, validation (needs mock-server)
+	cd $(CONSUMER_DIR) && ./gradlew runDemoConfig
+
+.PHONY: demo-exceptions
+demo-exceptions: ## Every MarketDataException subtype (needs mock-server)
+	cd $(CONSUMER_DIR) && ./gradlew runExceptions
+
+.PHONY: demo-retry
+demo-retry: ## Retry, Retry-After, preflight (needs mock-server)
+	cd $(CONSUMER_DIR) && ./gradlew runRetry
+
+.PHONY: demo-response
+demo-response: ## Response<T> surface features (needs mock-server)
+	cd $(CONSUMER_DIR) && ./gradlew runResponse
+
+.PHONY: demo-concurrency
+demo-concurrency: ## 50-permit semaphore (needs mock-server)
+	cd $(CONSUMER_DIR) && ./gradlew runConcurrency
+
+.PHONY: demos-all
+demos-all: ## Run every mock-server-based demo back-to-back (needs mock-server)
+	cd $(CONSUMER_DIR) && ./gradlew runDemoConfig runExceptions runRetry runResponse runConcurrency

build.gradle.kts

@@ -138,6 +138,10 @@ tasks.register<JacocoReport>("jacocoAggregateReport") {
 // 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
 // dev iteration isn't blocked while coverage is in flux.
+//
+// SDK requirements §15.3 mandates 100% line coverage with explicit ignore
+// comments on untestable lines. Target deferred until business resources land
+// and the defensive-guards cleanup pass can run together.
 
 spotless {
     java {