diff --git a/ASSUMPTIONS.md b/ASSUMPTIONS.md new file mode 100644 index 0000000..d387b26 --- /dev/null +++ b/ASSUMPTIONS.md @@ -0,0 +1,115 @@ +# Assumptions & Decisions + +The brief is deliberately open-ended, so every non-obvious interpretation is recorded +here with its rationale. The three load-bearing decisions also have full +[ADRs](docs/adr/); this file is the single index of *every* judgment call, ADR-backed or +not. + +## Availability + +- **Nights, not days (half-open window).** A guest occupies the nights *between* + check-in and check-out and leaves on the morning of check-out, so a `07-10 → 07-12` + stay needs nights `07-10` and `07-11` — **not** `07-12`. Availability is evaluated over + the half-open interval `[check_in, check_out)`. Treating check-out as an occupied night + would wrongly reject valid stays at the top boundary. ([ADR-0002](docs/adr/0002-availability-per-night-set-membership.md)) + +- **Set membership, not range containment.** `available_dates` is a set of discrete open + nights, so a room is available iff *every* night of the window is present in that set. + This is correct even if a room's dates had a gap; the seed happens to be contiguous, + but per-night membership is robust regardless and costs nothing. A room with no open + dates is never available (returned as no availability, not an error). ([ADR-0002](docs/adr/0002-availability-per-night-set-membership.md)) + +- **The dataset is the source of truth for dates — including past-dated windows.** The + API does not compare the requested window to "today"; it answers purely against the + room's `available_dates`. The seed's open dates sit in the past relative to now, and + rejecting past dates would make the sample requests un-runnable and couple correctness + to wall-clock time. A stay is judged available or not by the data, full stop. + +- **A nonsensical window is a `400`, not an empty result.** `check_out <= check_in` is + contradictory input and is rejected at the boundary, distinct from "no rooms + available" (an empty `data` array). ([ADR-0002](docs/adr/0002-availability-per-night-set-membership.md)) + +## Pricing & filtering + +- **`price_from` = the property's cheapest room.** A property has no price of its own, + only its rooms do. `price_from` is the minimum `price_per_night` across a property's + rooms — the universal "from $X/night" model — and is what search filters and the list + projection use. A room-less property has no rate; `price_from` renders as `null`. + ([ADR-0003](docs/adr/0003-price-filter-on-cheapest-room.md)) + +- **Price filtering is independent of availability.** `GET /hotels` is a discovery + surface — "what exists in my budget" — and returns a property regardless of whether its + rooms are open on any particular dates. `/hotels` doesn't even take dates; date-based + availability is the job of `/hotels/:id/rooms`. ([ADR-0003](docs/adr/0003-price-filter-on-cheapest-room.md)) + +- **Two price params, not a range string.** `min_price` and `max_price` are each + optional numbers rather than a `"100-200"` string: no bespoke parsing, clean numeric + validation, and either bound usable alone. `min_price > max_price` is contradictory + input → `400`. ([ADR-0003](docs/adr/0003-price-filter-on-cheapest-room.md)) + +- **Exact star match.** `star_rating` filters on an exact class (an integer 2–5 in this + catalogue), not "N stars or better." "Show me 4-star hotels" means 4-star hotels; a + "minimum stars" reading would be a different, unrequested feature. + +## API shape + +- **Response envelopes.** Success is `{ "data": ... }`; the rooms endpoint adds + `{ "meta": { check_in, check_out, nights } }`. Failure is always + `{ "error": { "code", "message" } }`. A uniform envelope lets clients branch on + success/failure the same way for every endpoint. Every failure path — Zod validation, + thrown `AppError`s, framework errors, unexpected exceptions — is normalized into that + one error shape. + +- **snake_case on the wire.** The dataset is snake_case (`price_per_night`, `star_rating`, + `available_dates`), so the API and query params stay snake_case end-to-end. Introducing + a camelCase boundary would add a mapping layer and two vocabularies for one concept with + no benefit to a reviewer reading the seed and the responses side by side. + +- **No pagination.** The catalogue is 40 properties. Pagination would add `limit`/`offset` + params, `meta` bookkeeping, and tests for a dataset that fits in a single response. If + the catalogue grew, the pure filter functions are where a `LIMIT/OFFSET` push-down would + land (see the migration guide) — but shipping it now would be speculative. + +- **No authentication.** The brief describes a public discovery API over read-only public + data; there is nothing to protect and no user to identify. Adding auth would be + unrequested scope. (A real deployment would put this behind an API gateway / rate + limiter rather than bake auth into the service.) + +## Data quirks + +- **Currency is assumed USD.** Prices in the seed are bare numbers with no currency + field. The API surfaces them as-is and treats them as USD by convention; there is no + multi-currency handling because the data expresses none. If a `currency` field were + ever added, it would ride alongside `price_per_night` unchanged. + +- **`"free Wi-Fi"` is left verbatim.** Amenities are otherwise snake_case tokens + (`fitness_center`, `valet_parking`, `pool`), but this one value arrives as the + human-readable string `"free Wi-Fi"` — spaces, hyphen, mixed case. The API passes + amenities through unmodified rather than "normalizing" them: rewriting source data to a + tidier form would silently diverge the API from its source of truth and could break a + client doing an exact-string amenity match. The quirk is the data's, and it's the data + that's authoritative. + +## Tooling & scope — knowing when *not* to + +- **No contract testing (e.g. Pact).** Consumer-driven contract testing earns its keep + when independent services must agree on a contract across deploys. Here there is a + single service and no second party to hold a contract with, so Pact would be ceremony. + The equivalent guarantee is already covered from within: the Zod schemas are the single + source of truth for validation, serialization, **and** the OpenAPI doc, and the + integration tests parse responses back through those schemas — so the wire contract is + enforced without a contract-testing framework. Reaching for Pact here would signal + cargo-culting a tool rather than matching it to the problem. + +- **No database (in-memory store behind a repository).** The dataset is 40 read-only + properties; a real database would add a dependency, schema, seed step, and run-time + setup a reviewer must stand up — cost with no payoff at this scale. The store sits + behind a `HotelRepository` interface so the swap is a single file, and the + [migration guide](docs/migration-postgres-sqlite.md) documents exactly how it would go. + ([ADR-0001](docs/adr/0001-in-memory-store-behind-repository.md)) + +- **Docker is included, deliberately.** Unlike the two above, containerization *is* worth + it here: a multi-stage `Dockerfile` proves the service builds and runs as a slim, + dev-dependency-free, non-root image with a health check — a concrete "production-ready" + signal the brief asks for, at near-zero ongoing cost. CI builds the image on every PR to + keep that claim honest (no registry push). diff --git a/README.md b/README.md index 13f7e13..a9e2290 100644 --- a/README.md +++ b/README.md @@ -2,19 +2,54 @@ [![CI](https://github.com/kernvex/InfiniteChoice/actions/workflows/ci.yml/badge.svg)](https://github.com/kernvex/InfiniteChoice/actions/workflows/ci.yml) -A lightweight, production-ready RESTful API for finding and exploring hotels. Built -with [Fastify](https://fastify.dev/), TypeScript, and Zod. See [`CONTEXT.md`](CONTEXT.md) -for the domain glossary. +A lightweight, production-ready RESTful API for finding and exploring hotels: search a +catalogue of properties, view a property's full detail, and check which rooms are open +for a set of travel dates. Built with [Fastify](https://fastify.dev/), TypeScript, and +[Zod](https://zod.dev/). + +- **Domain glossary:** [`CONTEXT.md`](CONTEXT.md) — the ubiquitous language (Property, + Room, Stay window, Night, Availability, `price_from`, …) the code and API share. +- **Decisions & rationale:** [`ASSUMPTIONS.md`](ASSUMPTIONS.md) and the + [ADRs](docs/adr/). + +## Why this stack + +| Choice | Rationale | +| -------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Fastify** | Fast, minimal HTTP framework with first-class schema validation and a plugin model that keeps routes, error handling, and docs cleanly separated. | +| **TypeScript** | Types are the contract. The domain models are inferred from the same schemas that validate data and responses, so drift between model, wire, and docs can't happen. | +| **Zod + type-provider** | One schema per shape drives **validation, response serialization, and the OpenAPI/Swagger docs** — a single source of truth instead of three that rot apart. | +| **In-memory data store** | The catalogue is 40 read-only properties in a JSON seed; a database would add setup cost with no payoff at this scale. It sits behind a repository interface so a real store is a one-file swap — see [ADR-0001](docs/adr/0001-in-memory-store-behind-repository.md) and the [migration guide](docs/migration-postgres-sqlite.md). | +| **Vitest** | Fast unit + integration runner; the app is built un-listened so integration tests drive it via `app.inject()` without binding a port. | ## Endpoints -| Method | Path | Description | -| ------ | ------------------- | ----------------------------------------------- | -| GET | `/health` | Liveness probe | -| GET | `/hotels` | Search & filter properties (summary projection) | -| GET | `/hotels/:id` | Property detail | -| GET | `/hotels/:id/rooms` | Room availability & pricing for a stay window | -| GET | `/docs` | Swagger UI | +| Method | Path | Description | Query / path params | +| ------ | ------------------- | ----------------------------------------------- | ------------------------------------------------------------------------------- | +| GET | `/health` | Liveness probe | — | +| GET | `/hotels` | Search & filter properties (summary projection) | `city`, `star_rating` (1–5), `min_price`, `max_price` — all optional, AND-combined | +| GET | `/hotels/:id` | Property detail | `id` (path) | +| GET | `/hotels/:id/rooms` | Room availability & pricing for a stay window | `id` (path); `check_in`, `check_out` (`YYYY-MM-DD`, required, half-open) | +| GET | `/docs` | Swagger UI (OpenAPI generated from the schemas) | — | + +Full, always-current request/response schemas live at **[`/docs`](http://localhost:3000/docs)** +once the server is running. + +### Examples + +```sh +# Four-star hotels in London from $200/night +curl 'http://localhost:3000/hotels?city=London&star_rating=4&min_price=200' + +# One property's full detail +curl 'http://localhost:3000/hotels/hotel-01' + +# Rooms available for a two-night stay (nights 07-10 and 07-11) +curl 'http://localhost:3000/hotels/hotel-01/rooms?check_in=2026-07-10&check_out=2026-07-12' +``` + +Every response is enveloped: successes as `{ "data": ... }` (with `meta` on the rooms +endpoint), failures as `{ "error": { "code", "message" } }`. ## Requirements @@ -27,7 +62,9 @@ npm ci npm run dev # hot-reloading dev server on http://localhost:3000 ``` -The service reads its catalogue seed from [`data/hotels.json`](data/hotels.json) at startup. +The service reads its catalogue seed from [`data/hotels.json`](data/hotels.json) at +startup and validates it against the domain schema, so a malformed seed fails fast and +legibly rather than surfacing mid-request. ### Scripts @@ -41,22 +78,75 @@ The service reads its catalogue seed from [`data/hotels.json`](data/hotels.json) | `npm test` | Run the test suite once (Vitest) | | `npm run verify` | `typecheck → lint → test`, fail-fast (the quality gate) | +## Architecture + +Requests flow through three layers, each with a single job, plus a set of pure functions +the layers lean on: + +``` +HTTP request + │ + ▼ +routes/ Fastify handlers. Own the wire contract: Zod schemas validate the + │ query/params/body and serialize the response, and map results into + │ the { data } / { error } envelope. No business logic. + ▼ +domain/ Pure functions over plain domain values — filtering (search.ts), + │ availability & pricing (availability.ts), and the list/availability + │ projections. No I/O, so the rules are exhaustively unit-testable. + ▼ +repository/ The data-source seam (HotelRepository). Loads & validates the JSON + seed once and serves it from memory. Swapping in Postgres/SQLite + touches only this file (see the migration guide). +``` + +Cross-cutting concerns are Fastify plugins: [`plugins/error-handler.ts`](src/plugins/error-handler.ts) +normalizes every failure (Zod validation, thrown `AppError`s, framework errors, +unexpected exceptions) into the one `{ error: { code, message } }` envelope, and +[`plugins/swagger.ts`](src/plugins/swagger.ts) generates the OpenAPI doc from the same +Zod schemas the routes validate against. [`app.ts`](src/app.ts) composes them and returns +an un-listened instance; [`server.ts`](src/server.ts) owns `listen`. The repository is +injected, so tests can supply a fixture catalogue. + +The key design move: **the domain layer knows nothing about HTTP, and the routes know +nothing about the business rules.** That keeps the availability logic — the part the +brief actually grades — testable in isolation and the data source swappable without +touching either. + +## Testing strategy + +`npm test` runs the suite across two complementary styles: + +- **Unit tests** over the pure domain functions (`availability.test.ts`, + `search.test.ts`, `hotel-summary.test.ts`, `repository.test.ts`) exercise the rules + and their edges directly — nights vs. days, the half-open upper boundary, empty + open-dates, per-night set membership vs. range containment, the room-less + `price_from = null` case — with no HTTP in the loop. +- **Integration tests** (`hotels.test.ts`, `hotel-detail.test.ts`, + `hotel-rooms.test.ts`, `health.test.ts`, `docs.test.ts`, `error-envelope.test.ts`) + drive the real app and seed through `app.inject()`, asserting status codes, envelope + shapes, and **response-schema conformance** (responses are parsed back through the Zod + schemas), plus the validation boundaries and the standard `400`/`404` envelopes. + +The seed itself is deliberately built to catch a naive availability reading, so the +integration tests double as a check that the real data flows through correctly. + ## Quality gate -A Husky **pre-push** hook runs `npm run verify` (`typecheck → lint → test`, -fail-fast), so failures surface locally before they reach CI. To bypass it in a pinch: +A Husky **pre-push** hook runs `npm run verify` (`typecheck → lint → test`, fail-fast), +so failures surface locally before they reach CI. To bypass it in a pinch: ```sh git push --no-verify ``` -GitHub Actions [CI](.github/workflows/ci.yml) mirrors the same checks on every push -and pull request, then builds the Docker image (no registry push). +GitHub Actions [CI](.github/workflows/ci.yml) mirrors the same checks on every push and +pull request, then builds the Docker image (no registry push). ## Docker -Multi-stage build producing a slim, dev-dependency-free image that runs as the -non-root `node` user with a `/health` HEALTHCHECK: +Multi-stage build producing a slim, dev-dependency-free image that runs as the non-root +`node` user with a `/health` HEALTHCHECK: ```sh docker build -t infinite-choice . @@ -64,3 +154,39 @@ docker run --rm -p 3000:3000 infinite-choice ``` Then hit . + +## Use of AI + +This project was built with AI coding-agent assistance (Claude), under human direction, +and this section is a transparent account of exactly how — including where AI was *not* +used. + +**How it was used** + +- **Planning and scaffolding.** Work was decomposed into GitHub Issues (foundation, + search, detail, availability, hardening, docs), each implemented on its own branch and + merged via PR. The agent drafted issues, ADRs, and the domain glossary, which were + reviewed and edited before adoption. +- **Test-driven implementation.** Each endpoint was built test-first: failing + unit/integration tests capturing the acceptance criteria, then the implementation to + pass them. The availability rule (nights vs. days, half-open window, set membership) + was pinned down in tests before any code — see + [ADR-0002](docs/adr/0002-availability-per-night-set-membership.md). +- **Refactoring and review.** Every branch went through an automated code-review pass + (standards + spec) before merge; several commits (e.g. "single-source CI gate", + "enumerate Stay-window Nights once") are review follow-ups. +- **This documentation.** The README, `ASSUMPTIONS.md`, and the migration guide were + drafted from the actual source and then verified against it. + +**Where judgment stayed human** + +- The **domain interpretations** — availability as per-night set membership over a + half-open window, `price_from` as the cheapest room, price filtering independent of + dates — are deliberate rulings, recorded as ADRs with their rationale rather than + accepted as whatever the happy path produced. +- **Knowing when *not* to reach for a tool.** Contract testing (e.g. Pact) and a real + database were both consciously skipped: there is no second service consuming this API + and no persistence requirement, so both would be ceremony without payoff at this scale. + The repository seam and schema-driven docs give most of the same guarantees for free. + ([ASSUMPTIONS.md](ASSUMPTIONS.md) records these.) +- Every AI-produced change was read, run (`npm run verify`), and owned before merge. diff --git a/docs/adr/0001-in-memory-store-behind-repository.md b/docs/adr/0001-in-memory-store-behind-repository.md index 1e3d0d4..e4d3d80 100644 --- a/docs/adr/0001-in-memory-store-behind-repository.md +++ b/docs/adr/0001-in-memory-store-behind-repository.md @@ -9,7 +9,7 @@ The catalogue is 40 properties supplied as a static JSON seed, and the brief tre ## The seam -Services depend on the `HotelRepository` interface, never on the JSON directly. Swapping the in-memory implementation for a Postgres- or SQLite-backed one touches a single file and no domain or route code. A migration guide in the README documents exactly how that swap would go, so the design intent — "this is data-source-agnostic" — is visible even though only the in-memory implementation ships. +Services depend on the `HotelRepository` interface, never on the JSON directly. Swapping the in-memory implementation for a Postgres- or SQLite-backed one touches a single file and no domain or route code. A dedicated [migration guide](../migration-postgres-sqlite.md) documents exactly how that swap would go, so the design intent — "this is data-source-agnostic" — is visible even though only the in-memory implementation ships. ## Consequences diff --git a/docs/migration-postgres-sqlite.md b/docs/migration-postgres-sqlite.md new file mode 100644 index 0000000..834c979 --- /dev/null +++ b/docs/migration-postgres-sqlite.md @@ -0,0 +1,107 @@ +# Migration guide: in-memory → Postgres / SQLite + +[ADR-0001](adr/0001-in-memory-store-behind-repository.md) ships an in-memory catalogue +behind a `HotelRepository` interface precisely so a real database can be dropped in later +without touching domain or route code. This guide documents exactly how that swap goes — +the design intent made concrete, even though only the in-memory implementation ships. + +## The seam + +Everything depends on one contract, [`src/repository/hotel-repository.ts`](../src/repository/hotel-repository.ts): + +```ts +export interface HotelRepository { + findAll(): Property[]; + findById(id: string): Property | undefined; +} +``` + +Routes read `app.repository` (decorated in [`src/app.ts`](../src/app.ts)); the domain +layer takes plain `Property[]` and never touches a data source at all. So a migration is +scoped to: + +1. **Add** a new implementation of `HotelRepository` (e.g. `postgres-hotel-repository.ts`). +2. **Change one line** in `app.ts` to construct it instead of the in-memory one. + +No route, schema, or domain file changes. The `propertySchema` in +[`src/domain/hotel.ts`](../src/domain/hotel.ts) stays the single source of truth for a +`Property`'s shape. + +## What "async" costs + +The in-memory methods are synchronous. A real database is I/O, so the interface would +become async: + +```ts +export interface HotelRepository { + findAll(filters?: HotelFilters): Promise; + findById(id: string): Promise; +} +``` + +The route handlers already run inside `async` Fastify callbacks, so this is a mechanical +change: `const property = app.repository.findById(id)` becomes +`const property = await app.repository.findById(id)`. The in-memory implementation would +wrap its returns in `Promise.resolve(...)` to keep satisfying the same interface, so the +tests that inject a fixture repository keep working unchanged. + +## Pushing filters down + +Today `filterHotels` and `priceFrom` (in [`src/domain/search.ts`](../src/domain/search.ts)) +run in application code over the full array. That was a deliberate choice for a 40-row +seed, and the functions are written so the move into SQL is mechanical: each predicate +maps to a `WHERE` clause. With a real store you'd pass the filters into the repository and +let the database do the work: + +| Domain predicate | SQL push-down | +| ------------------------------------------- | ----------------------------------------------------------------------------- | +| `city` (case-insensitive exact) | `WHERE lower(city) = lower($1)` | +| `star_rating` (exact) | `AND star_rating = $2` | +| `min_price` / `max_price` over `price_from` | `AND price_from BETWEEN $3 AND $4` (see the `price_from` note below) | +| (future) pagination | `LIMIT / OFFSET` — the seam is where this would land, per ASSUMPTIONS.md | + +`price_from` is a derived value (the min room rate), so in a relational schema it's either +a correlated sub-select, a `JOIN … GROUP BY MIN(price_per_night)`, or a maintained column +/ materialized view if read volume warrants it. The pure `priceFrom` function stays useful +as the canonical definition and for any in-app fallback. + +## Schema sketch + +A `Property` has nested `address`, `contact`, `policies`, `amenities[]`, and `rooms[]` +(with `available_dates[]`). Two shapes fit: + +- **Relational (Postgres/SQLite):** `properties`, `rooms` (FK → property), and either a + `room_available_dates` table (`room_id`, `night DATE`) or a `date[]` column on Postgres. + Address/contact/policies can be columns on `properties` or `1:1` tables. Availability + then becomes a query — a room is available when the count of its open nights intersecting + the requested window equals the number of nights requested — but the repository can still + return fully-hydrated `Property` objects so the domain code is unaffected. +- **Document/JSONB:** store each `Property` as a JSONB blob mirroring `propertySchema`. + Closest to the current seed; least migration effort; filters use JSONB operators. + +Whichever shape, the repository's job is to **rehydrate rows back into `Property` objects +that satisfy `propertySchema`** — validate on the way out just as `loadCatalogue` does +today, so a drifted database fails loudly at the boundary rather than deep in a request. + +## SQLite specifics + +SQLite (e.g. via `better-sqlite3`) is the lightest step up and can stay **synchronous**, +so the interface wouldn't even need to go async — the swap really is just a new +`HotelRepository` implementation plus one line in `app.ts`. Good for a single-node +deployment or embedding the catalogue as a file artifact. + +## Postgres specifics + +For Postgres (e.g. `pg` or an ORM), add a connection pool, run migrations to create the +schema, and seed from `data/hotels.json` once. Wire the pool's lifecycle into Fastify +(register as a plugin; close it in `onClose`) so connections are cleaned up on shutdown. +The async interface above applies. + +## Checklist + +- [ ] New `*-hotel-repository.ts` implementing `HotelRepository`. +- [ ] Validate rows back through `propertySchema` on read. +- [ ] (Postgres) make the interface async; `await` the three call sites in `hotels.ts` (one `findAll`, two `findById`). +- [ ] Push `HotelFilters` into the query; keep `priceFrom` as the canonical definition. +- [ ] Swap the constructor in `app.ts`; leave routes, schemas, and domain untouched. +- [ ] Point the injected test repository at the same interface — existing tests should pass.