Clean-architecture restart: SDK foundation + utilities resource#8
Conversation
Removes all Java sources from main/test/integrationTest source sets to restart implementation under the layered architecture described in the plan (HttpTransport agnostic → generic pipeline → MarketDataExecutor → façades). Skeleton, CI workflows, ADRs, and documentation are preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
The author of this PR, MarketDataDev03, is not an activated member of this organization on Codecov. |
| Function<String, @Nullable String> env, | ||
| Path dotEnvPath, | ||
| Consumer<DotEnvLoader.Warning> warnings) { | ||
| Map<String, String> dotEnv = DotEnvLoader.load(dotEnvPath, warnings, EnvVars.ALLOWED_KEYS); |
There was a problem hiding this comment.
check for inline comments in env file not being ignored, it would be nice to add that feature
| public record RequestHeaders(Map<String, String> headers) { | ||
|
|
||
| public RequestHeaders { | ||
| headers = Map.copyOf(headers); |
There was a problem hiding this comment.
null validation to avoid NullPointerException
There was a problem hiding this comment.
The only caller of new RequestHeaders(...) in main code is RequestHeadersDeserializer, which already guarantees a non-null map (via getNullValue override + raw == null guard). Defaulting null to Map.of() or remapping it to IllegalArgumentException would protect no real path — it would just obfuscate future @NullMarked contract violations.
Since the constructor can't actually fail today, I think it's better to fail loud and fast on any future violation than to silently swallow it.
public RequestHeaders { Objects.requireNonNull(headers, "headers"); headers = Map.copyOf(headers); }
Clean-architecture restart: SDK foundation +
utilitiesresourceFoundation for the Java SDK: configuration cascade, async-first HTTP transport, retry +
Retry-After+ status cache, 50-permit concurrency cap, sealed exception hierarchy,Response<T>surface, parallel-arrays JSON decoding, and theutilitiesresource façade.stocks/options/funds/marketsare deliberately deferred to follow-up PRs.Why this exists
The previous
maincarried an early, partial shape that did not satisfy the cross-language SDK Requirements: no retry/backoff, no rate-limit tracking, no sealed exceptions, noResponse<T>surface, no async/sync parity, no concurrency cap. This PR establishes the canonical foundation the way ADRs 001–007 dictate, then wires the smallest possible resource (utilities— three endpoints:/status/,/user/,/headers/, all unversioned) on top of it so the foundation can be exercised end-to-end. Future per-resource PRs (stocks,options,funds,markets) plug into the same transport / parser / response surface — they only declare their endpoints, wire-format columns, and response models.What's in this PR
Grouped by SDK layer. Each item references the spec section that demands it.
Configuration & lifecycle (§4, §5, §16)
MarketDataClient— public no-arg and 4-arg constructors; package-private 6-arg seam for hermetic tests;close(),utilities(),getRateLimits().Configuration.resolve(...)— explicit →MARKETDATA_*env var →.env→ default cascade, withbaseUrl/apiVersionnormalization + validation and printable-ASCIIapiKeyrejection (CRLF, NUL, high-bit bytes — issue Bump org.gradle.toolchains.foojay-resolver-convention from 0.10.0 to 1.0.0 #23).DotEnvLoader—.envfile loading with explicit allowed-keys whitelist; warnings buffered until the logger is configured (issue Bump com.fasterxml.jackson.core:jackson-databind from 2.18.2 to 2.22.0 #25 attaches them as suppressed ifresolveitself throws).EnvVars— the only place that knows theMARKETDATA_*variable names;systemLookup()restricts reads to the allowed set so SDK code cannot accidentally read unrelated env vars.DemoMode—apiKey == nullpredicate driving the §5 startup-validation skip.GET /user/(usesRetryPolicy.noRetry()so a slow/down API surfaces within seconds instead of burning the ~6.75 min retry budget); skipped in demo mode; partial-construction guard closes the transport and attaches the close failure as a suppressed exception if the probe throws.HTTP transport (§4, ADR-004, ADR-006)
HttpTransport— single point of contact between resources and the network. Owns the URL/request builders, applies status routing (2xx + 404 → success envelope; 4xx/5xx → typed exception). Async-first per ADR-006;joinSyncis a thin wrapper that unwrapsCompletionExceptionso consumers seeMarketDataExceptiondirectly.HttpDispatcher— single-shot send under the 50-permit semaphore. Captures sync-throws fromHttpClient.sendAsyncso the permit cannot leak. MapsIOExceptiontoNetworkErrorexactly once.HttpResponseEnvelope—byte[]body + status + request-id + headers + URL. Format-agnostic (resources decide JSON / CSV / HTML).HttpStatusMapper— status → sealed-exception table; default-bucket logic for 3xx / 1xx / out-of-range codes that still routes to a permit.RequestSpec— declarative GET specification: path, query params, format, versioned flag.Retry & backoff (§9)
RetryPolicy— 4 attempts default, exponential1s × 2^ncapped at 30s; retriesNetworkError(only withIOExceptionsomewhere in the cause chain — issue docs(examples): rewrite the consumer examples as teaching material #15) andServerError 501–599; never 500, never 4xx, neverParseError/RateLimitError/AuthenticationError/BadRequestError/NotFoundError.noRetry()factory for startup validation.RetryExecutor— pure orchestration ofSupplier<CompletableFuture<T>>retries viaCompletableFuture.delayedExecutor(no scheduled-thread pool); single cancellation handler installed once, in-flight attempt tracked viaAtomicReference.RetryAfterHeader— RFC 7231 parser: delta-seconds and RFC 1123 HTTP-date both supported; negative deltas and past dates clamp toDuration.ZERO.Retry-After10-minute cap (issue refactor: remove retired Options Strikes endpoint #21) — pathological server values (Retry-After: 9999999999) are logged and the SDK falls back to exponential. The raw value remains visible onServerError.getRetryAfter().StatusCache— stale-while-revalidate (270 s refresh threshold / 300 s expiry); longest-path-prefix match with trailing-slash normalization (issue feat: addof(...)convenience factory to StockQuotesRequest and OptionsQuotesRequest #18); re-check aftertriggerRefresh(issue docs: sync from documentation@55d35fe34473617fb1009ba844cffec94a57f0bc #19) so synchronous-fetcher test setups don't always answer ALLOW on first call; self-bypass for the/status/URI so the gate cannot trap its own refresh.Rate-limit tracking & preflight (§8, §10.3)
RateLimitHeaders.parse(...)— reads the fourx-api-ratelimit-*headers as an all-or-nothing set; partial deliveries returnnull(no phantom zeros that would freeze the preflight gate).RateLimitSnapshot— record:limit,remaining,reset(Instant),consumed.HttpTransport.checkRateLimitPreflight— fails fast withRateLimitErrorwhen the latest snapshot reportsremaining == 0andnow < reset. Returnsnullonceresetelapses so the next response refreshes the snapshot.ServerErrorcarrying a parsedRetry-After, the snapshot is ignored for that retry so the server-orchestrated backoff prevails.Concurrency (§12, ADR-007)
AsyncSemaphore— async-safe 50-permit limiter. Replacesj.u.c.SemaphoresoexecuteAsyncnever parks the caller's thread. Two invariants: every permit accounted in exactly one place (available / held / pending), andfuture.complete(...)always runs outside the lock.Response surface (§13.5)
Response<T>— typeddata(), defensive-copiedrawBody()(clone on both write and read),statusCode(),requestId()(Cloudflarecf-ray),requestUrl(),isJson()/isCsv()/isHtml(),isNoData()(true for 404, the API's no-data envelope status),saveToFile(Path), redactedtoString().Formatenum —JSON/CSV/HTML, package-private (queried via the boolean accessors so the enum can grow without breaking compiled consumers).JSON parsing (§11, ADR-005)
JsonResponseParser— oneObjectMapperper client; resources self-register wire-formatSimpleModules in their constructors. Pre-checks empty body (issue Bump actions/checkout from 4 to 7 #29) and emits a précisParseError("Empty response body…") instead of Jackson's generic "No content to map".ParallelArrays.zip+listDeserializer(...)— declarative factory that collapses the ~30-line custom-deserializer skeleton to three pieces: column names, row builder, container wrapper.Rowaccessors are strict by default (null/wrong-type cells raiseJsonMappingException) so a server-side dropped column cannot silently degrade to sentinel values.RequestHeadersDeserializer,UserDeserializer— hand-written deserializers for the two wire shapes that aren't parallel-arrays.Resources
UtilitiesResource— sync + async pair (ADR-006) for each of/status///user///headers/(all unversioned per the backend's URL conf); package-private constructor (ADR-007); registers its wire-formatSimpleModuleon the parser at construction time; package-privatevalidateAuth()intent-named for the startup probe.Exceptions (§6, ADR-002)
MarketDataException— sealed base, 7 permits:AuthenticationError,BadRequestError,NotFoundError,RateLimitError,ServerError,NetworkError,ParseError.ErrorContext— record:requestId?,requestUrl,statusCode,timestamp. FactoriesforResponse(...)/forNoResponse(...).getSupportInfo()— multi-line dump withAmerica/New_Yorktimestamp and exception-type tag, ready to paste into a support ticket.getRequestUrl()returns the query-redacted URL (§16);getContext().requestUrl()exposes the raw URL for discretionary diagnostic use.Logging (§7, §16)
MarketDataLogging.configure(...)— installsCanonicalLogFormatter({timestamp} - {logger} - {level} - {message}) on the SDK root logger (com.marketdata.sdk). First-install-wins; consumer-pre-config detection backs off entirely if the consumer already attached a handler or set a level (idempotency flag not latched on the skip, so the SDK can claim the slot later).CanonicalLogFormatter— JULFormatterenforcing the spec shape.MARKETDATA_LOGGING_LEVELenv var —DEBUG/INFO/WARNING/ERROR(case-insensitive); unrecognized values fall back toINFOwith a logged warning.HttpDispatcher.safeUri) — log lines never persist?token=,?account_id=,?symbol=; the full URI stays available onErrorContextfor consumer-side diagnostics.Tokens.redact) —≤8chars → fully masked;>8chars →***…***ABCD(enough to disambiguate, not enough to use).Packaging & CI
build.gradle.kts—java-library,maven-publish, Spotless, JaCoCo, Vanniktech Maven Publish.-PtestJdk=Ntoolchain hook applies uniformly totestandintegrationTest..github/workflows/(existing — not in this diff, but exercised by it):pull-request.yml(JDK 17 only),main.yml(matrix{17, 21, 25}+ integration tests on push tomain),pr-matrix-on-demand.yml(slash command),pr-integration-on-demand.yml(slash command, branch-protection check).What's deliberately not in this PR
(Copied from
CLAUDE.md"Deliberately deferred"; each item requires the per-resource layer to land first.)client.utilities()is wired today;client.stocks,client.options,client.funds,client.marketsare TODO.ParallelArrays.zip,JsonResponseParser,Response<T>) is in place — each new endpoint just declares its fields and row builder.Response<T>. Today the snapshot is client-level (viaclient.getRateLimits()).violationRules. Deferred until the resource layer lands so the ratchet meaningfully gates functional code, not scaffolding.Diff at a glance
src/mainsrc/testHttpTransportE2ETest).src/integrationTestMARKETDATA_TOKEN).build.gradle.kts-PtestJdkwiring.CLAUDE.md.gitignoreThe branch carries 54 commits — many small "after review" / "fix" commits. The final shape is what matters; the commit history is incremental, not a logical squash.
How to review this
Open
docs/REFACTOR_REVIEW_GUIDE.mdalongside the diff. The guide is structured by flow, not by file, so a reviewer can pick the slice that interests them:client.utilities().status()call traced through every class it touches. Mermaid sequence diagram. Start here if you've never read this codebase.new MarketDataClient(...): cascade, validation, logger, partial-construction guard, startup probe. Mermaid flowchart.Retry-After, andStatusCache— the §9 contract, with all the cap / bypass / synchronous-fetcher corners that look weird but exist for a reason. Mermaid flowchart.AsyncSemaphore) — invariants, why notj.u.c.Semaphore, FIFO drain on close.Response<T>+ JSON parsing — surface, defensive copies,ParallelArrays.zipand the strictRowaccessors.ErrorContext,getSupportInfo(), redacted vs raw URL.If you only have 30 minutes, read sections 2 → 4 → 10 in that order — that covers the load-bearing flows and the things most likely to look wrong on first read but aren't.
Test plan
The repo's
Makefilewraps both./gradlewand the demos underexamples/. Runmake helpfor the full list.Unit tests + build
make build— unit tests + Spotless + JaCoCo on JDK 17../gradlew -PtestJdk=21 test— unit tests on JDK 21. (No Make wrapper;-PtestJdk=Nis passed to gradlew directly.)./gradlew -PtestJdk=25 test— unit tests on JDK 25./integrationtest(JDK 17) or/integrationtestfull(matrix) — exercises the live API.Consumer demos (run
make publishonce first; the mock-server-based ones needmake mock-serverin a separate terminal)make demo-quickstart— idiomatic per-resource usage tour (today:utilities; grows with each new resource).make demo-live— happy path againstapi.marketdata.app.make demo-config— cascade + validation + redaction (needs mock server).make demo-exceptions— sealed-hierarchy round-trip (needs mock server).make demo-retry— backoff,Retry-After, preflight (needs mock server).make demo-response—Response<T>surface (needs mock server).make demo-concurrency— 50-permit cap end-to-end (needs mock server).make demos-all— all five mock-server-based demos back-to-back.Out of scope / known caveats
NotFoundErroris currently unreachable.HttpTransport.routeAndEnvelopeshort-circuits all 404 responses to the success-envelope branch (the API's no-data convention), so thecase 404 -> NotFoundError(...)inHttpStatusMappernever executes. The permit stays in the sealed hierarchy because ADR-002 fixes the 7 canonical permits — removing it would be an amendment, not a silent code change. Re-introducing reachability requires the SDK-side decision: only treat 404 as no-data when the body carriess:"no_data"; otherwise route throughNotFoundError. Tracked for a follow-up.isCsv()/isHtml()predicates work; no current utility endpoint negotiates a non-JSON format, so the path is in the surface but unproven end-to-end. Future resources that emit CSV will close the gap.RetryPolicycustomization is not yet a public API. Today a consumer can't pass a custom policy; the only public knobs are the policy defaults and the constructor'svalidateOnStartupflag. PromotingRetryPolicy(or a builder around it) to public requires deciding the shape — defer until a real consumer asks.mainin this PR. A priorMarketsResourcewas an early shape that didn't fit the new foundation; rebuilding it cleanly on top ofResponse<T>/ParallelArraysis the next per-resource PR.