|
| 1 | +# Markets Review Guide — `13_market_resource` |
| 2 | + |
| 3 | +This guide walks a reviewer through the `markets` resource added on the `13_market_resource` branch. It is organized by **flow**, not by file. |
| 4 | + |
| 5 | +This PR closes out the resource series: it **adopts the conventions the [`options`](OPTIONS_REVIEW_GUIDE.md) and [`stocks`](STOCKS_REVIEW_GUIDE.md) PRs established** — `MarketDataResponse<T>` + named responses, the Builder-based per-endpoint request, nullable fields + `columns` + Option A, the CSV/HTML facets — and applies them to the single markets endpoint, `GET /v1/markets/status/`. **No shared-layer changes at all**: transport, retry, rate-limit parsing, `ParallelArrays`, `JsonResponseParser`, `MarketDataDates`, and `RequestConfig` are reused untouched. |
| 6 | + |
| 7 | +If you reviewed the stocks PR, the only genuinely new content is the markets-specific semantics (§3) and the all-optional parameter surface (§4). ~10 minutes. |
| 8 | + |
| 9 | +Suggested reading order: §1 (what's here) → §3 (markets-specific semantics) → §4 (request + query translation) → §5 (deserializer). `file:line` citations target `HEAD` on this branch. |
| 10 | + |
| 11 | +## Table of contents |
| 12 | + |
| 13 | +- [Running it locally](#running-it-locally) |
| 14 | +1. [What this PR adds](#1-what-this-pr-adds) |
| 15 | +2. [The response model (reused)](#2-the-response-model-reused) |
| 16 | +3. [Markets-specific semantics](#3-markets-specific-semantics) |
| 17 | +4. [Request → query translation](#4-request--query-translation) |
| 18 | +5. [The row deserializer: nullable + columns + Option A](#5-the-row-deserializer-nullable--columns--option-a) |
| 19 | +6. [Universal parameters + the CSV/HTML facets](#6-universal-parameters--the-csvhtml-facets) |
| 20 | +- [Reviewer checklist](#reviewer-checklist) |
| 21 | + |
| 22 | +--- |
| 23 | + |
| 24 | +## Running it locally |
| 25 | + |
| 26 | +```bash |
| 27 | +make build # unit tests + Spotless + JaCoCo (JDK 17) |
| 28 | + |
| 29 | +# Integration tests hit the live API (gated). A token in .env or the env is required: |
| 30 | +MARKETDATA_RUN_INTEGRATION_TESTS=true ./gradlew integrationTest |
| 31 | + |
| 32 | +# Full markets demo against the mock server (all params, null status cells, CSV facet, |
| 33 | +# columns projection, Option A). Needs the mock server up: |
| 34 | +make publish && make mock-server # (in one terminal) |
| 35 | +make demo-markets # (in another) — or: ./gradlew -p examples/consumer-test runMarkets |
| 36 | +``` |
| 37 | + |
| 38 | +`MarketsIntegrationTest` (shape assertions over a one-week window) runs against `api.marketdata.app`. `MarketsApp` (`examples/consumer-test`) scripts the mock server's responses to demonstrate every scenario — it was run green end-to-end (`make demo-markets`). |
| 39 | + |
| 40 | +--- |
| 41 | + |
| 42 | +## 1. What this PR adds |
| 43 | + |
| 44 | +### 1.1 Public API surface (new) |
| 45 | + |
| 46 | +``` |
| 47 | +com.marketdata.sdk.MarketsResource (returned from client.markets()) |
| 48 | +com.marketdata.sdk.MarketsCsvResource (returned from .asCsv()) |
| 49 | +com.marketdata.sdk.MarketStatusResponse (named response) |
| 50 | +
|
| 51 | +com.marketdata.sdk.markets.MarketStatusRequest (Builder-based request — every param optional) |
| 52 | +com.marketdata.sdk.markets.MarketStatus (row record: date/status + isOpen()/isClosed()) |
| 53 | +``` |
| 54 | + |
| 55 | +Same packaging rules as options/stocks (ADR-007): façades `public final` with package-private constructors in the **root** package; request/row records in the public `com.marketdata.sdk.markets` subpackage (`@NullMarked` via `package-info.java`); `MarketsHtmlResource` built but package-private (`asHtml()` stays hidden until the backend serves HTML). ADR-007 named `MarketsResource` as its canonical example — this PR makes that example real. |
| 56 | + |
| 57 | +### 1.2 Files to review, by role |
| 58 | + |
| 59 | +| Area | Files | What to check | |
| 60 | +|---|---|---| |
| 61 | +| Resource façade | `MarketsResource.java` | universal-param config, `statusSpec`, the row deserializer + Option A, `asCsv()`/`asHtml()` | |
| 62 | +| CSV/HTML facets | `MarketsCsvResource.java`, `MarketsHtmlResource.java` | reuse of the static `statusSpec`, `format=csv/html` | |
| 63 | +| Response | `MarketStatusResponse.java` | thin subclass of `AbstractMarketDataResponse<List<MarketStatus>>` | |
| 64 | +| Requests | `markets/MarketStatusRequest.java`, `MarketRequests.java` | Builder validation, window rules, the no-required-args `of()` | |
| 65 | +| Row record | `markets/MarketStatus.java` | `@Nullable` fields; `isOpen()`/`isClosed()` predicates | |
| 66 | +| Wiring | `MarketDataClient.java` | `client.markets()` | |
| 67 | +| Demos | `examples/.../MarketsApp.java`, `QuickstartApp.java` | mock-server walk-through; quickstart section enabled | |
| 68 | + |
| 69 | +--- |
| 70 | + |
| 71 | +## 2. The response model (reused) |
| 72 | + |
| 73 | +No new model concepts. `MarketStatusResponse` is a thin subclass of `AbstractMarketDataResponse<T>`; `values()` is `List<MarketStatus>` — one row per calendar day. Per-response `rateLimit()` (§8.2) and the full `MarketDataResponse` surface come from the base for free. |
| 74 | + |
| 75 | +--- |
| 76 | + |
| 77 | +## 3. Markets-specific semantics |
| 78 | + |
| 79 | +The load-bearing review points — each is a *contract* fact, verified against the backend (`api/marketDataApi/markets/` + `common/util/markets_helper.py`) and the Python SDK: |
| 80 | + |
| 81 | +1. **This is the exchange calendar, not API health.** `markets.status` answers "was/is the market open on these days?" from the `MarketHoliday` table; `utilities().status()` reports the API's *own* per-service health from the unversioned `/status/` route. The class javadocs call the distinction out on both `MarketsResource` and `MarketDataClient.markets()`. (No interaction with the §9.5 `StatusCache` either — that cache keys on the unversioned `/status/` path, which `/v1/markets/status/` is not.) |
| 82 | +2. **Every parameter is optional.** A bare `MarketStatusRequest.of()` returns today's status for the US calendar — this is the only request type in the SDK with a no-args `of()`. Window shapes: `date` (single day) XOR `from`/`to` (inclusive range) XOR `to`+`countback`. |
| 83 | +3. **`status` cells can be null.** The backend's holiday data is bounded; days outside its coverage come back as a **null cell in a present column** — so Option A is satisfied and the row decodes with `status() == null` (`isOpen()`/`isClosed()` both false). A null status means "the calendar has no answer", never a decode failure. Test: `statusNullCellsOutsideCalendarCoverageDecodeToNull`. |
| 84 | +4. **`country` is pass-through.** Two-digit ISO 3166; the backend currently serves US only and answers `no_data` (404 + `{"s":"no_data"}`, which the SDK surfaces as a successful empty response) for anything else — we don't second-guess that server-side rule client-side. |
| 85 | +5. **`isOpen()`/`isClosed()` are derived predicates** on the row record (`"open".equals(status)` / `"closed".equals(status)`), not stored fields — the wire value stays exposed verbatim via `status()`. |
| 86 | + |
| 87 | +**Python-SDK parity:** `sdk-py` exposes `country/date/from_date/to_date/countback` — this PR exposes exactly the same five, same window validation (`from > to` rejected; here additionally `date` XOR range and `countback`-pairs-with-`to`, the same client-side rules stocks/funds apply). |
| 88 | + |
| 89 | +--- |
| 90 | + |
| 91 | +## 4. Request → query translation |
| 92 | + |
| 93 | +One spec builder, `MarketsResource.statusSpec` (package-private static, reused by both facets): |
| 94 | + |
| 95 | +| Endpoint | Path | Params | |
| 96 | +|---|---|---| |
| 97 | +| `statusSpec` | `markets/status` | `country`, `date`/`from`/`to`/`countback` | |
| 98 | + |
| 99 | +What to verify: |
| 100 | +- No path parameters — everything is a query param; dates ISO-formatted (`2025-01-17`). |
| 101 | +- Window rules in `MarketRequests.validateWindow` (same as stocks/funds): `date` mutually exclusive with `from`/`to`/`countback`; `countback` positive, pairs with `to` not `from` (the backend silently *ignores* countback when `from` is present — we reject the combination instead of silently dropping one side). |
| 102 | +- The bare request produces `/v1/markets/status/` with no query string. |
| 103 | + |
| 104 | +--- |
| 105 | + |
| 106 | +## 5. The row deserializer: nullable + columns + Option A |
| 107 | + |
| 108 | +Identical mechanics to stocks/funds (see [Stocks guide §5](STOCKS_REVIEW_GUIDE.md#5-the-row-deserializers-nullable--columns--option-a)): the `rowsDeserializer`/`validateRequestedColumns` pair is **copied per resource** (the agreed pre-v1 dedup refactor folds these together with the universal-param setters). |
| 109 | + |
| 110 | +- `STATUS_FIELDS = [date, status]` — both **required** (either may be projected away via `columns`). Note the asymmetry with §3.3: a missing `status` *column* is an Option A anomaly; a null `status` *cell* is data. |
| 111 | +- `date` decodes through the tolerant `MarketDataDates.parseDateOrTimestampField` — unix seconds by default, date-only strings (`"2025-01-17"`) under `dateformat=timestamp`, lifted to market-zone midnight (`America/New_York`). |
| 112 | +- Envelope handling via `ParallelArrays.zip`: `"s":"error"` → `ParseError` carrying `errmsg`; `"s":"no_data"` → empty `values()`. |
| 113 | + |
| 114 | +The wire module registers under the name `marketdata-markets` in `MarketsResource`'s client-facing constructor — same once-per-client pattern as the other resources. |
| 115 | + |
| 116 | +--- |
| 117 | + |
| 118 | +## 6. Universal parameters + the CSV/HTML facets |
| 119 | + |
| 120 | +Same shape as stocks/options/funds: `dateFormat`/`mode`/`limit`/`offset`/`columns` return configured copies of `MarketsResource` ("configure once, call many"; the config carries into `asCsv()`); the CSV facet adds the output-shaping `human`/`headers`. The known copy-paste of these setters across resources is tracked tech debt for the pre-v1 self-typed-base refactor — do not review it as accidental duplication. |
| 121 | + |
| 122 | +--- |
| 123 | + |
| 124 | +## Reviewer checklist |
| 125 | + |
| 126 | +- [ ] `client.markets().status(...)` hits `GET /v1/markets/status/` with every param translated (unit: `statusAttachesAllParams`, `statusAttachesDateAndCountbackWindows`) and a bare `of()` produces no query params (`statusHitsVersionedEndpointWithNoRequiredParams`) |
| 127 | +- [ ] `date` + `status` are both required columns under Option A; a null `status` **cell** decodes to null (`statusNullCellsOutsideCalendarCoverageDecodeToNull`) |
| 128 | +- [ ] `isOpen()`/`isClosed()` are derived from the verbatim wire value, both false on a null cell |
| 129 | +- [ ] Sync + async parity (`status` / `statusAsync`) via `transport.joinSync` (ADR-006) |
| 130 | +- [ ] CSV facet sends `format=csv` + shaping params; HTML facet stays package-private |
| 131 | +- [ ] `no_data` → empty list; error envelope → `ParseError` with `errmsg` |
| 132 | +- [ ] Per-response `rateLimit()` populated from the four `x-api-ratelimit-*` headers |
| 133 | +- [ ] `MarketDataClient` wires `markets()` like the other resources (constructed before `StatusCache`, inside the partial-construction guard); javadoc disambiguates vs `utilities().status()` |
| 134 | +- [ ] Demos: `make demo-markets` green; `QuickstartApp` markets section enabled |
| 135 | +- [ ] Integration: `MarketsIntegrationTest` (one-week window: both statuses present; countback window) passes with a token |
0 commit comments