diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 00000000..5040e83d --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,133 @@ +# Similar Products Service + +Spring Boot 3.3 reactive application that exposes `GET /product/{productId}/similar` on port 5000. + +## Architecture + +Aggregates two upstream APIs on port 3001 (Simulado mock): +1. `GET /product/{id}/similarids` → list of product IDs ordered by similarity +2. `GET /product/{id}` → product detail + +All detail requests run in **parallel** via `flatMapSequential` (preserves similarity order while fetching concurrently). + +``` +Client → SimilarProductsController + → SimilarProductsService + → ProductClient.getSimilarIds() (sequential first) + → ProductClient.getProductDetail() (all in parallel) +``` + +## Tech Stack + +- Java 21, Spring Boot 3.3, Spring WebFlux (Project Reactor + Netty) +- Maven + +## Build & Run + +### Locally (requires Java 21 + Maven) +```bash +cd app +mvn spring-boot:run +``` + +### Docker Compose (builds and runs everything) +```bash +docker-compose up -d simulado influxdb grafana yourapp +``` + +## Testing + +```bash +cd app && mvn test # 31 tests, ~25s — nothing needs to be running +``` + +The upstream API is stubbed in-process with MockWebServer, so the suite needs neither Docker nor Simulado. +Maven runs on JDK 21 (`~/.jdks/jdk-21.0.6`) even though `java` on the PATH is 17. + +| Test class | Covers | +|------------|--------| +| `ProductClientTest` | Timeouts, 404/500 skipping, JSON id coercion, malformed bodies | +| `SimilarProductsServiceTest` | Similarity order, parallel fetching, dropping unresolvable products | +| `SimilarProductsControllerTest` | 200 / `[]` / 404 / 5xx mapping | +| `SimilarProductsIntegrationTest` | The five mock scenarios end-to-end over a real socket | + +See the **write-tests** skill for conventions and the timing pitfalls (cold-start flakiness, virtual-time hangs). + +## Load Testing + +```bash +# Run k6 test (app + infra must be up) +docker-compose run --rm k6 run scripts/test.js + +# View results in Grafana +# http://localhost:3000/d/Le2Ku9NMk/k6-performance-test +``` + +## Key Design Decisions + +| Decision | Reason | +|----------|--------| +| WebClient (non-blocking) | 200 concurrent VUs; reactive I/O avoids thread-per-request overhead | +| `flatMapSequential` | Parallel HTTP calls, results emitted in original similarity order | +| 2s per-product timeout | Mocks have 5s/50s delays that must be bounded for acceptable p99 | +| Skip on 404/500/timeout | Individual product failures should not fail the whole request | +| 404 from similarids → 404 response | Contract requirement; means the base product has no similar IDs | +| `similar-ids-timeout-ms` (2s) | Entry-point call was unbounded; could hang on connection contention | +| No cache | Throughput is capped by k6 client pacing + timeout, not upstream calls — a cache can't beat that ceiling here | +| `max-connections: 50` (balanced) | See performance note below — the main tuning lever | + +## Performance — the connection pool is the key lever + +Load testing (see `app/README.md` for the full data) established: +- **The single-process mock is the bottleneck, not our pool.** Smaller pool = higher throughput + (pool 8 → ~289 req/s; pool 128 → ~77 req/s). Oversizing overloads the mock. +- **A small pool is fast because it silently drops available products under load** (the doomed 5s/50s + products monopolise connections). Completeness rises with pool size (pool 8 → 12%; pool 100+ → 100%). +- Throughput vs completeness is a **Pareto trade-off**; `max-connections: 50` is the chosen balance + (fast + correct in normal use, graceful degradation under extreme load). +- Presets (via `PRODUCT_API_MAX_CONNECTIONS`): max throughput = 8–16; max correctness = 150–200. + +When investigating performance, do NOT reflexively enlarge the pool — measure first. + +## Logging + +Rule: **WARN is for what makes a request fail; DEBUG is for what the design deliberately tolerates.** +Skipping a product is designed behaviour that happens on nearly every request under load, so it must +never be logged above DEBUG — it would bury the failures that matter. + +| Level | What | Frequency | +|-------|------|-----------| +| INFO | Effective client config + timeouts | 2 lines, once at startup | +| WARN | `/similarids` returned 5xx or timed out; request resolved to a 5xx | Only on a request that fails | +| DEBUG | IDs received, each skipped product + reason, products resolved + elapsed ms | ~2 lines/request + 1 per skip | + +At INFO the whole k6 scenario set produces **zero** per-request lines. Set `LOG_LEVEL=DEBUG` to trace a +request end to end — never during a load test. + +A 404 from `/similarids` is a client outcome, not a fault: DEBUG, not WARN. `ProductClientLoggingTest` +pins these levels so they can't be relaxed by accident. + +## Mock Scenarios (port 3001) + +| Our endpoint | Similar IDs | What happens | +|-------------|-------------|--------------| +| /product/1/similar | [2,3,4] | Fast — all 3 products return quickly | +| /product/2/similar | [3,100,1000] | Product 100 (1s) OK; product 1000 (5s) → timeout → skipped | +| /product/3/similar | [100,1000,10000] | Product 100 OK; 1000 and 10000 → timeout → skipped | +| /product/4/similar | [1,2,5] | Product 5 returns 404 → skipped; returns [1,2] | +| /product/5/similar | [1,2,6] | Product 6 returns 500 → skipped; returns [1,2] | + +## Configuration + +All properties live under `product-api` in `application.yaml` and map to an env var (Spring relaxed +binding), so they can be overridden without a rebuild. See `app/README.md` for the full reference. + +| Property | Env var | Default | Description | +|----------|---------|---------|-------------| +| `product-api.base-url` | `PRODUCT_API_BASE_URL` | `http://localhost:3001` | Upstream API base URL (`http://simulado` in Docker) | +| `product-api.connect-timeout-ms` | `PRODUCT_API_CONNECT_TIMEOUT_MS` | 1000 | TCP connect timeout | +| `product-api.detail-timeout-ms` | `PRODUCT_API_DETAIL_TIMEOUT_MS` | 2000 | Per-product detail request timeout | +| `product-api.similar-ids-timeout-ms` | `PRODUCT_API_SIMILAR_IDS_TIMEOUT_MS` | 2000 | Entry-point (similarids) call timeout | +| `product-api.max-connections` | `PRODUCT_API_MAX_CONNECTIONS` | 50 | Outbound connection pool size — the main perf lever | +| `product-api.pending-acquire-timeout-ms` | `PRODUCT_API_PENDING_ACQUIRE_TIMEOUT_MS` | 2000 | Max wait for a pooled connection | +| `logging.level.com.inditex.similarproducts` | `LOG_LEVEL` | `INFO` | `DEBUG` traces every request; keep at INFO under load | diff --git a/.claude/README.md b/.claude/README.md new file mode 100644 index 00000000..1456a901 --- /dev/null +++ b/.claude/README.md @@ -0,0 +1,54 @@ +# .claude + +Project-specific configuration for Claude Code for the **Similar Products** service. + +## Structure + +``` +.claude/ +├── agents/ Subagents (name + description frontmatter → the system prompt) +│ ├── spring-boot-dev.md Reactive Spring Boot developer with full project context +│ ├── test-engineer.md Writes/runs/diagnoses the automated test suite +│ └── performance-analyst.md Interprets k6 results and proposes improvements +├── commands/ Slash commands (user-triggered operational shortcuts) +│ ├── run-app.md → /run-app Build & start the app locally on port 5000 +│ ├── test.md → /test Run the k6 load test +│ ├── unit-test.md → /unit-test Run the JUnit suite (mvn test; nothing needs to be running) +│ └── infra.md → /infra Start/stop/check the Docker infrastructure +├── skills/ Skills (each is a DIRECTORY with a SKILL.md; model-invocable) +│ ├── check-endpoints/SKILL.md Smoke-test the 5 scenarios and validate responses +│ ├── write-tests/SKILL.md Conventions & pitfalls for adding tests +│ └── analyze-performance/SKILL.md Run/interpret the load test and recommend fixes +├── hooks/ Shell scripts wired from settings.json (JSON on stdin) +│ ├── report-infra-status.sh SessionStart: reports whether mock + app are up +│ ├── guard-blocking-calls.sh PreToolUse(Write|Edit): blocks .block()/Thread.sleep in main sources +│ └── verify-app-running.sh PreToolUse(Bash): blocks the k6 test if the app is down +├── CLAUDE.md Project context, loaded automatically each session +├── README.md This file +└── settings.json Permissions + hook wiring +``` + +## Format notes (why it's laid out this way) + +- **Skills are directories**, not flat files: `.claude/skills//SKILL.md`. The directory name is what + becomes invocable. Frontmatter uses `name`, `description`, and `allowed-tools`. +- **Commands** are flat `.md` files under `commands/`; the filename becomes the `/command`. `run-app` is named + to avoid colliding with Claude Code's built-in `run` skill. +- **Hooks** receive the tool call as **JSON on stdin** (there is no `$CLAUDE_TOOL_EXIT_CODE`). In `settings.json`, + `matcher` filters by **tool name** only (regex, e.g. `Write|Edit`). There is **no field that filters on command + content** — a `Bash` matcher fires on *every* command, so any command filter has to live inside the script + (see `verify-app-running.sh`, which exits 0 for anything that isn't a k6 run). Exit code `2` blocks the action + and sends stderr back as feedback. + +## Quick reference + +| You want to… | Do | +|--------------|-----| +| Start the app locally | `/run-app` | +| Start / stop Docker infra | `/infra` | +| Run the k6 load test | `/test` | +| Run the JUnit test suite | `/unit-test` | +| Smoke-test all 5 scenarios | ask for the **check-endpoints** skill | +| Analyze load-test performance | ask for the **analyze-performance** skill | +| Add or fix tests | ask for the **write-tests** skill, or the **test-engineer** agent | +| Implement / debug a feature | delegate to the **spring-boot-dev** agent | diff --git a/.claude/agents/performance-analyst.md b/.claude/agents/performance-analyst.md new file mode 100644 index 00000000..8c0f3dad --- /dev/null +++ b/.claude/agents/performance-analyst.md @@ -0,0 +1,72 @@ +--- +name: performance-analyst +description: Analyzes k6 load test results and Spring Boot metrics to identify bottlenecks and suggest improvements for the Similar Products service. Knows the measured performance characteristics of this specific system. +tools: Read, Grep, Glob, Bash +model: inherit +color: orange +--- + +You are a performance analyst for the **Similar Products Service** load tests. + +## Test setup + +- **Tool:** k6 (loadimpact/k6:0.28.0 — note: predates the `http_req_failed` metric) +- **VUs:** 200 concurrent virtual users per scenario, each with `sleep(0.5)` between iterations +- **Duration:** 10s per scenario +- **Results:** InfluxDB → Grafana at `http://localhost:3000/d/Le2Ku9NMk/k6-performance-test`, plus the + k6 stdout summary (the most reliable source in this k6 version) + +## Scenarios and expected behaviour + +| Scenario | Endpoint | Expected latency | Notes | +|----------|----------|------------------|-------| +| normal | /product/1/similar | tens of ms | all 3 upstreams fast | +| notFound | /product/4/similar | tens of ms | product 5 (404) skipped | +| error | /product/5/similar | tens of ms | product 6 (500) skipped | +| slow | /product/2/similar | ~2s (timeout-bound) | product 1000 (5s) hits the 2s timeout | +| verySlow | /product/3/similar | ~2s (timeout-bound) | products 1000 + 10000 hit the 2s timeout | + +p90/p95 ≈ 2s is EXPECTED, not a bug — it is the intentional timeout on the slow scenarios. + +## What this system's performance actually looks like (measured — do not re-derive from scratch) + +1. **The single-process mock (simulado) is the bottleneck, not our connection pool.** Throughput is + *inversely* related to pool size: pool 8 → ~289 req/s, pool 32 → ~206 req/s, pool 128 → ~77 req/s. + Oversizing floods the mock and degrades every response. **Never reflexively enlarge the pool.** +2. **A small pool inflates throughput by silently dropping valid products.** The doomed slow products + (1000/10000) hold connections for the full 2s timeout and starve the available 1s product (100). + Completeness of `/product/2/similar` under load: pool 8 → ~12%, pool 50 → ~25–56%, pool 100+ → 100%. + k6 does not validate response bodies, so this is invisible on the dashboard — verify it separately. +3. **Throughput vs completeness is a Pareto trade-off.** Default is `max-connections: 50` (balanced). + Presets via `PRODUCT_API_MAX_CONNECTIONS`: max throughput = 8–16; max correctness = 150–200. + +## How to measure + +- **Throughput / latency:** `docker-compose run --rm k6 run scripts/test.js` and read the stdout summary + (`http_req_duration`, `http_reqs`, `iterations`). +- **Errors / resilience:** check app logs (`docker-compose logs yourapp`) for exceptions; there is no + `http_req_failed` in this k6 version, so confirm 0 errors via logs + all requests completing. +- **Result completeness under load:** fire N concurrent requests to a slow endpoint and count how many + responses contain the available slow product, e.g.: + ```bash + for i in $(seq 1 80); do (curl -s http://localhost:5000/product/2/similar > /tmp/r$i.json) & done; wait + grep -l '"id":"100"' /tmp/r*.json | wc -l # want 80/80 + ``` +- **Sweep a parameter without rebuilding:** every property maps to an env var, so restart the + container with a different `PRODUCT_API_MAX_CONNECTIONS`/`PRODUCT_API_DETAIL_TIMEOUT_MS` and re-run. + Run configs back-to-back to cancel host-load noise. + +## Levers, ranked by real impact here + +1. **`max-connections`** — the dominant lever (see the Pareto trade-off above). Pick the point on the + frontier that matches the goal; do not just make it bigger. +2. **Timeouts** (`detail-timeout-ms`, `similar-ids-timeout-ms`) — bound tail latency. Shortening the + detail timeout was tested and does NOT improve completeness (that is demand-bound, not hold-bound). +3. **Do NOT propose caching for the current setup.** It was measured and rejected: throughput is capped + by the k6 client `sleep(0.5)` and the 2s timeout, so a cache cannot raise the measured numbers — it + would only cut upstream volume (not the bottleneck) while adding memory + staleness. + *Only* revisit caching if the upstream becomes a real, scalable API rather than this single-process + mock — then a short-TTL Caffeine `AsyncCache` with request coalescing becomes the top optimisation. + +Always report findings as: measured numbers → gap vs target → concrete change (with file), and state +which side of the throughput/completeness trade-off the change moves. diff --git a/.claude/agents/spring-boot-dev.md b/.claude/agents/spring-boot-dev.md new file mode 100644 index 00000000..fd20912c --- /dev/null +++ b/.claude/agents/spring-boot-dev.md @@ -0,0 +1,74 @@ +--- +name: spring-boot-dev +description: Spring Boot reactive developer with full context of the Similar Products service. Use for implementing features, debugging reactive chains, and reviewing code changes. +tools: Read, Edit, Write, Grep, Glob, Bash +model: inherit +color: green +--- + +You are working on the **Similar Products Service** — a Spring Boot 3.3 + Java 21 + WebFlux reactive microservice located in the `app/` directory. + +## Your context + +**What it does:** Exposes `GET /product/{productId}/similar` on port 5000 by aggregating two upstream APIs on port 3001: +1. `/product/{id}/similarids` → list of IDs +2. `/product/{id}` → product detail (called in parallel for all IDs) + +**Package:** `com.inditex.similarproducts` + +**Key files:** +- `app/src/main/java/com/inditex/similarproducts/client/ProductClient.java` — HTTP calls with timeout and error handling +- `app/src/main/java/com/inditex/similarproducts/service/SimilarProductsService.java` — `flatMapSequential` for parallel+ordered fetching +- `app/src/main/java/com/inditex/similarproducts/controller/SimilarProductsController.java` — 404 propagation +- `app/src/main/java/com/inditex/similarproducts/config/WebClientConfig.java` — connection pool (the main perf lever) +- `app/src/main/resources/application.yaml` — port 5000, timeouts, pool, base URL +- `app/README.md` — architecture + the measured performance analysis (read before touching perf) + +## Rules for this codebase + +- **Never block inside a reactive chain.** No `block()`, no `Thread.sleep()`, no synchronous I/O. + (A PreToolUse hook enforces this on `src/main`.) +- **Use `flatMapSequential`** when fetching product details — parallelism with order preservation. +- **Timeouts are intentional.** Products 1000 (5s) and 10000 (50s) must be skipped; the 2s timeout does this. + Both the detail call AND the `similarids` entry-point call are timeout-bounded. +- **Skip, don't fail.** A 404/500/timeout on an individual product detail must resolve to `Mono.empty()`, not an error. +- **`ProductNotFoundException`** is only thrown when the `/similarids` endpoint itself returns 404. +- **Do NOT add a cache.** It was measured and rejected — throughput is capped by the k6 client pacing and + the timeout, not by upstream call count, so a cache adds memory + staleness for no measurable gain here. +- **Do NOT reflexively grow the connection pool.** `max-connections` is a throughput↔completeness Pareto + trade-off (the single-process mock is the bottleneck); measure before changing it. Default is 50. +- **`PRODUCT_API_BASE_URL`** env var controls the upstream: `http://localhost:3001` locally, `http://simulado` in Docker. +- **Logging: WARN only for what fails a request; DEBUG for what the design tolerates.** A skipped product + (404/500/timeout) runs on nearly every request under load — logging it above DEBUG buries real failures. + Use parameterised SLF4J (`log.debug("... {}", id)`), never string concatenation, and never log per product + at INFO. `LOG_LEVEL=DEBUG` traces a request end to end; `ProductClientLoggingTest` pins the levels. + +## Build & verify + +`java` on the PATH is 17, but **Maven runs on JDK 21** (`~/.jdks/jdk-21.0.6`), so `mvn test` and +`mvn spring-boot:run` do work locally. Run the test suite after any change to `src/main`: + +```bash +cd app && mvn test # 31 tests, ~25s, nothing else needs to be running +``` + +For a running stack (mocks + app), use Docker: + +```bash +docker-compose build yourapp && docker-compose up -d yourapp # rebuild after code changes +# then smoke-test via the check-endpoints skill, or: +curl -s http://localhost:5000/product/1/similar +``` + +Every config property maps to an env var, so you can sweep behaviour by restarting the container with a +different `-e PRODUCT_API_...` value — no rebuild needed. + +## Upstream mock behaviour (port 3001) + +| Product | Similar IDs | Notable behaviour | +|---------|-------------|-------------------| +| 1 | [2,3,4] | All fast | +| 2 | [3,100,1000] | Product 1000 has 5s delay → times out | +| 3 | [100,1000,10000] | Products 1000 (5s) and 10000 (50s) → time out | +| 4 | [1,2,5] | Product 5 returns 404 → skip | +| 5 | [1,2,6] | Product 6 returns 500 → skip | diff --git a/.claude/agents/test-engineer.md b/.claude/agents/test-engineer.md new file mode 100644 index 00000000..cd6a2012 --- /dev/null +++ b/.claude/agents/test-engineer.md @@ -0,0 +1,63 @@ +--- +name: test-engineer +description: Writes, runs and diagnoses the automated tests for the Similar Products service. Use for adding coverage, reproducing a bug as a failing test, or investigating a failing/flaky/hanging test. +tools: Read, Edit, Write, Grep, Glob, Bash +model: inherit +color: yellow +--- + +You test the **Similar Products Service** — a Spring Boot 3.3 + Java 21 + WebFlux service in `app/` +that exposes `GET /product/{productId}/similar` by aggregating two upstream APIs. + +Follow the **write-tests** skill (`.claude/skills/write-tests/SKILL.md`) for conventions, stubbing +patterns and the known pitfalls. This file is the context; that skill is the how. + +## The suite + +``` +app/src/test/java/com/inditex/similarproducts/ +├── client/ProductClientTest.java MockWebServer — timeouts, 404/500, JSON mapping +├── service/SimilarProductsServiceTest.java Mockito — order, parallelism, skipping +├── controller/SimilarProductsControllerTest.java @WebFluxTest — status/body contract +├── SimilarProductsIntegrationTest.java @SpringBootTest(RANDOM_PORT) — the 5 scenarios +└── SimilarProductsApplicationTests.java context loads +``` + +Run with `cd app && mvn test` (add `-o` for offline, `-Dtest=Class#method` for one test). +The build uses JDK 21 via Maven even though `java` on the PATH is 17 — **no Docker needed** for tests. +Failure details: `app/target/surefire-reports/.txt`. + +## The behaviours that must stay covered + +These are the service's contract — a change that breaks one of them should turn a test red: + +1. **Similarity order is preserved** even when an earlier product responds last (`flatMapSequential`). +2. **Detail calls happen in parallel**, not one after another. +3. **A product that 404s, 500s or times out is skipped**, never fails the whole response. +4. **A 404 from `/similarids` becomes a 404 response**; any other upstream failure is a 5xx. +5. **Both upstream calls are timeout-bounded** — neither can hang the request. +6. Numeric ids in the `similarids` JSON are usable as string ids. + +## Rules + +- **No `.block()` and no `Thread.sleep`** — use `StepVerifier`, and virtual time for concurrency claims. + (A PreToolUse hook blocks these in `src/main`; keep test code to the same standard.) +- **Stub the upstream at the socket** with MockWebServer. Do not mock `WebClient` itself. +- **Do not weaken a test to make it pass.** If a test is flaky, widen a timing margin or make the assertion + deterministic (virtual time) — never delete the assertion or shorten the stub delay. +- **Never change `src/main` to make a test pass** unless the test exposed a genuine bug; say so explicitly + if you do. +- **Verify new tests are load-bearing**: temporarily break the production behaviour, confirm red, revert. +- Leave the working tree clean of scratch edits, and do not run any `git` commands. + +## Upstream behaviour being modelled (Simulado, port 3001) + +| Product | Similar IDs | Notable | +|---------|-------------|---------| +| 1 | [2,3,4] | all fast | +| 2 | [3,100,1000] | 1000 has a 5s delay → times out | +| 3 | [100,1000,10000] | 1000 (5s) and 10000 (50s) → time out | +| 4 | [1,2,5] | product 5 → 404 → skipped | +| 5 | [1,2,6] | product 6 → 500 → skipped | + +In tests these delays are scaled down (a stub delay well above a short timeout) so the suite stays fast. diff --git a/.claude/commands/infra.md b/.claude/commands/infra.md new file mode 100644 index 00000000..dc726934 --- /dev/null +++ b/.claude/commands/infra.md @@ -0,0 +1,37 @@ +--- +description: Start, stop or check the Docker testing infrastructure (mocks, InfluxDB, Grafana) +allowed-tools: Bash(docker-compose:*), Bash(curl:*) +--- + +Manage the Docker testing infrastructure (mocks, InfluxDB, Grafana). + +## Start infrastructure only (run app locally) + +```bash +docker-compose up -d simulado influxdb grafana +``` + +## Start everything including the app + +```bash +docker-compose up -d simulado influxdb grafana yourapp +``` + +## Stop everything + +```bash +docker-compose down +``` + +## Verify mocks are responding + +```bash +curl http://localhost:3001/product/1/similarids +curl http://localhost:3001/product/1 +``` + +## Useful URLs + +- Simulado mock server: http://localhost:3001 +- Grafana dashboard: http://localhost:3000/d/Le2Ku9NMk/k6-performance-test +- App endpoint: http://localhost:5000/product/1/similar diff --git a/.claude/commands/run-app.md b/.claude/commands/run-app.md new file mode 100644 index 00000000..8a6af0fd --- /dev/null +++ b/.claude/commands/run-app.md @@ -0,0 +1,28 @@ +--- +description: Build and start the Similar Products Spring Boot app locally on port 5000 +allowed-tools: Bash(mvn:*), Bash(cd:*), Bash(curl:*), Bash(docker-compose:*) +--- + +Start the Similar Products application locally. + +1. Ensure the mock server is running (the app depends on it): + +```bash +docker-compose up -d simulado +``` + +2. Build and run the app from the `app/` directory: + +```bash +cd app && mvn spring-boot:run +``` + +3. Once it is listening on port 5000, verify with a sample request: + +```bash +curl -s http://localhost:5000/product/1/similar +``` + +Expected: a JSON array with the detail of products 2, 3 and 4. + +> Requires Java 21 and Maven. To run everything in Docker instead, use `/infra` and start the `yourapp` service. diff --git a/.claude/commands/test.md b/.claude/commands/test.md new file mode 100644 index 00000000..19d17d37 --- /dev/null +++ b/.claude/commands/test.md @@ -0,0 +1,36 @@ +--- +description: Run the k6 load test against the Similar Products service and open Grafana +allowed-tools: Bash(docker-compose:*) +--- + +Run the k6 load test against the Similar Products service. + +## Prerequisites + +Start the infrastructure and the app (if not already running): + +```bash +docker-compose up -d simulado influxdb grafana yourapp +``` + +Or if running the app locally: `cd app && mvn spring-boot:run` + +## Run the test + +```bash +docker-compose run --rm k6 run scripts/test.js +``` + +## View results + +Open Grafana: http://localhost:3000/d/Le2Ku9NMk/k6-performance-test + +## Test scenarios (200 VUs each, 10s duration) + +| Scenario | Product | Expected behaviour | +|----------|---------|-------------------| +| normal | 1 | Fast response, all 3 similar products returned | +| notFound | 4 | Product 5 (404) skipped, returns 2 products | +| error | 5 | Product 6 (500) skipped, returns 2 products | +| slow | 2 | Product 1000 (5s) times out, returns 2 products in ~2s | +| verySlow | 3 | Products 1000+10000 time out, returns 1 product in ~2s | diff --git a/.claude/commands/unit-test.md b/.claude/commands/unit-test.md new file mode 100644 index 00000000..fc16026b --- /dev/null +++ b/.claude/commands/unit-test.md @@ -0,0 +1,47 @@ +--- +description: Run the JUnit/Reactor test suite for the Similar Products app (no Docker, no mocks needed) +allowed-tools: Bash(mvn:*) +--- + +Run the automated test suite. Unlike `/test` (which drives k6 against a running stack), this needs +**nothing running** — the upstream product API is stubbed in-process by MockWebServer. + +## Run everything + +```bash +cd app && mvn test +``` + +Add `-o` to run offline once the dependencies are cached. + +## Run a subset + +```bash +cd app && mvn test -Dtest=ProductClientTest +cd app && mvn test -Dtest=SimilarProductsServiceTest#returnsProductsInSimilarityOrder +``` + +## What the suite covers + +| Test class | Layer | Focus | +|------------|-------|-------| +| `ProductClientTest` | HTTP boundary | Timeouts, 404/500 handling, JSON id coercion, malformed bodies | +| `SimilarProductsServiceTest` | Aggregation | Similarity order, parallel fetching, skipping unresolvable products | +| `SimilarProductsControllerTest` | HTTP contract | 200 / empty array / 404 / 500 mapping | +| `SimilarProductsIntegrationTest` | End-to-end | The five Simulado scenarios over a real socket | +| `SimilarProductsApplicationTests` | Wiring | Context loads | + +## Notes + +- The build runs on **JDK 21** (`~/.jdks/jdk-21.0.6`, via Maven) even though `java` on the PATH is 17 — + so `mvn test` works locally; you do not need Docker for this. +- Reports land in `app/target/surefire-reports/`; read the `.txt` for a failing class to get the stack trace. + +## If a test fails + +- **A timing test fails on a cold/loaded machine:** the non-timing tests deliberately use the production + 2s timeout so JVM warm-up cannot trip them. Only the timeout tests use a short budget, with the stub + delayed far above it. Widen the margin rather than shortening the stub delay. +- **`fetchesDetailsInParallelRatherThanOneAfterAnother` fails:** the detail fetch stopped being concurrent — + check that `SimilarProductsService` still uses `flatMapSequential` (not `concatMap`/`flatMap` on a Mono chain). +- **Ordering test fails:** `flatMapSequential` was swapped for `flatMap`, which emits in completion order. diff --git a/.claude/hooks/guard-blocking-calls.sh b/.claude/hooks/guard-blocking-calls.sh new file mode 100644 index 00000000..0ca72a6e --- /dev/null +++ b/.claude/hooks/guard-blocking-calls.sh @@ -0,0 +1,21 @@ +#!/bin/sh +# PreToolUse hook (matcher: Write|Edit). +# Blocks edits that introduce blocking calls into the reactive main sources. +# The whole service depends on a non-blocking WebFlux chain, so .block()/Thread.sleep +# in src/main would silently ruin performance under load. +# +# Hooks receive the tool call as JSON on stdin. We grep the raw payload to stay +# dependency-free (no jq required on Windows/Git Bash). + +payload=$(cat) + +# Only inspect Java files under src/main. +echo "$payload" | grep -Eq '"file_path"[^,]*src[\\/]+main[\\/].*\.java' || exit 0 + +# Look for blocking primitives in the content being written/edited. +if echo "$payload" | grep -Eq '\.block\(|\.blockFirst\(|\.blockLast\(|\.toFuture\(\)\.get\(|Thread\.sleep'; then + echo "Blocking call detected in a reactive main source (.block()/Thread.sleep). Keep the WebFlux pipeline non-blocking — use operators like flatMap/delayElement instead. If this is truly intentional, make the edit outside src/main or explain why." >&2 + exit 2 +fi + +exit 0 diff --git a/.claude/hooks/report-infra-status.sh b/.claude/hooks/report-infra-status.sh new file mode 100644 index 00000000..bf7e3a6d --- /dev/null +++ b/.claude/hooks/report-infra-status.sh @@ -0,0 +1,17 @@ +#!/bin/sh +# SessionStart hook. +# Prints the state of the test infrastructure so Claude knows, at the start of a +# session, whether the mock server and the app are reachable. On SessionStart the +# stdout of an exit-0 hook is injected into Claude's context. + +mock=$(curl -s -o /dev/null -w "%{http_code}" --max-time 2 http://localhost:3001/product/1/similarids 2>/dev/null) +app=$(curl -s -o /dev/null -w "%{http_code}" --max-time 2 http://localhost:5000/product/1/similar 2>/dev/null) + +[ "$mock" = "200" ] && mock_state="up" || mock_state="down" +[ "$app" = "200" ] && app_state="up" || app_state="down" + +echo "Similar Products infra status: Simulado mock (:3001) is $mock_state; app (:5000) is $app_state." +[ "$mock_state" = "down" ] && echo "Start mocks with: docker-compose up -d simulado influxdb grafana" +[ "$app_state" = "down" ] && echo "Start the app with: cd app && mvn spring-boot:run (or the 'yourapp' compose service)." + +exit 0 diff --git a/.claude/hooks/verify-app-running.sh b/.claude/hooks/verify-app-running.sh new file mode 100644 index 00000000..30651fbe --- /dev/null +++ b/.claude/hooks/verify-app-running.sh @@ -0,0 +1,21 @@ +#!/bin/sh +# PreToolUse hook (matcher: Bash). +# Refuses to launch the k6 load test if the app isn't answering on port 5000, +# so the test doesn't report a wall of connection errors. +# +# PreToolUse only supports `matcher` (a tool-NAME regex) — there is no config field +# that filters on command content — so the command filter lives here. Anything that +# isn't a k6 run passes straight through. + +payload=$(cat) + +echo "$payload" | tr '\n' ' ' | grep -Eq '"command"[^"]*"[^"]*k6' || exit 0 + +status=$(curl -s -o /dev/null -w "%{http_code}" --max-time 3 http://localhost:5000/product/1/similar 2>/dev/null) + +if [ "$status" = "200" ]; then + exit 0 +fi + +echo "App is not responding at http://localhost:5000 (got '${status:-no response}'). Start it before load testing: cd app && mvn spring-boot:run — and make sure mocks are up: docker-compose up -d simulado." >&2 +exit 2 diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..2aaa9f2b --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,91 @@ +{ + "permissions": { + "defaultMode": "default", + "allow": [ + "Bash(mvn:*)", + "Bash(docker-compose up:*)", + "Bash(docker-compose build:*)", + "Bash(docker-compose run:*)", + "Bash(docker-compose ps:*)", + "Bash(docker-compose logs:*)", + "Bash(docker-compose stop:*)", + "Bash(docker-compose start:*)", + "Bash(docker-compose restart:*)", + "Bash(docker ps:*)", + "Bash(docker images:*)", + "Bash(docker logs:*)", + "Bash(curl:*)", + "Bash(git status:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Edit(./app/**)", + "Write(./app/**)", + "Edit(./.claude/**)", + "Write(./.claude/**)" + ], + "ask": [ + "Bash(git add:*)", + "Bash(git commit:*)", + "Bash(git push:*)", + "Bash(docker-compose down:*)", + "Edit(./docker-compose.yaml)", + "Write(./docker-compose.yaml)" + ], + "deny": [ + "Edit(./shared/**)", + "Write(./shared/**)", + "Edit(./similarProducts.yaml)", + "Write(./similarProducts.yaml)", + "Edit(./existingApis.yaml)", + "Write(./existingApis.yaml)", + "Edit(./LICENSE)", + "Write(./LICENSE)", + "Edit(./assets/**)", + "Write(./assets/**)", + "Read(./**/.env)", + "Read(./**/*.pem)", + "Bash(rm -rf:*)", + "Bash(git push --force:*)", + "Bash(git reset --hard:*)", + "Bash(git clean:*)", + "Bash(docker-compose down -v:*)", + "Bash(docker system prune:*)", + "Bash(docker volume rm:*)" + ] + }, + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/.claude/hooks/report-infra-status.sh\"", + "timeout": 10 + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Write|Edit", + "hooks": [ + { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/.claude/hooks/guard-blocking-calls.sh\"", + "timeout": 10 + } + ] + }, + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/.claude/hooks/verify-app-running.sh\"", + "timeout": 10 + } + ] + } + ] + } +} diff --git a/.claude/skills/analyze-performance/SKILL.md b/.claude/skills/analyze-performance/SKILL.md new file mode 100644 index 00000000..cf5b922f --- /dev/null +++ b/.claude/skills/analyze-performance/SKILL.md @@ -0,0 +1,58 @@ +--- +name: analyze-performance +description: Run or interpret the k6 load test for the Similar Products service, read the metrics, and produce actionable recommendations on latency, error rate and resilience. Use when investigating slow responses, timeouts, high p95/p99, or preparing performance improvements. +allowed-tools: Bash(docker-compose:*), Bash(docker:*), Bash(curl:*) +--- + +# Analyze performance + +This service must be efficient and resilient under load, so this skill checks both **performance** and **resilience**. + +## 1. Make sure infra and app are up + +```bash +docker-compose ps +``` + +Expected running: `simulado`, `influxdb`, `grafana`, and either the `yourapp` container or a local app on port 5000. + +## 2. Run the load test + +```bash +docker-compose run --rm k6 run scripts/test.js +``` + +The k6 script runs 5 scenarios (200 VUs, 10s each): `normal` (product 1), `notFound` (4), `error` (5), `slow` (2), `verySlow` (3). + +## 3. Read the results + +Grafana dashboard: http://localhost:3000/d/Le2Ku9NMk/k6-performance-test + +Focus on the k6 summary printed in the terminal — `http_req_duration` (avg/p90/p95), `http_req_failed`, and `iterations`. + +## 4. Evaluate against targets + +| Scenario | Target p95 | Target error rate | +|----------|-----------|-------------------| +| normal / notFound / error | < 200 ms | 0% | +| slow / verySlow | ~ 2 s (timeout-bounded, never 5s+) | 0% | + +Error rate should be **0%** — upstream 404/500 are handled, not propagated. This k6 version (0.28.0) +has no `http_req_failed` metric, so confirm resilience via `docker-compose logs yourapp` (no exceptions) +plus all requests completing. + +## 5. Diagnose and recommend + +| Symptom | Likely cause | Fix to propose | +|---------|-------------|----------------| +| slow/verySlow p95 far above 2s | timeout not applied | verify `.timeout(...)` in `ProductClient` (detail AND similarids) | +| errors in app logs / requests not completing | error escaping the reactive chain | check `onErrorResume(e -> Mono.empty())` | +| high median latency + low throughput | pool **too large** → single-process mock overloaded | *lower* `max-connections` (measure; smaller is faster here) | +| `/product/2/similar` drops product 100 under load | pool too small → doomed products starve the available one | *raise* `max-connections` toward correctness (Pareto trade-off) | +| cascading failures on upstream 5xx | no circuit breaker | consider a Resilience4j circuit breaker on `getProductDetail` | + +Do **not** propose a cache for this setup — it was measured and rejected (throughput is bounded by k6 +pacing + timeout, not upstream volume). See the performance-analyst agent / `app/README.md` for the data. + +Report findings as: current numbers → gap vs target → concrete code change (with file), and say which +side of the throughput/completeness trade-off it moves. diff --git a/.claude/skills/check-endpoints/SKILL.md b/.claude/skills/check-endpoints/SKILL.md new file mode 100644 index 00000000..7cf38ee2 --- /dev/null +++ b/.claude/skills/check-endpoints/SKILL.md @@ -0,0 +1,40 @@ +--- +name: check-endpoints +description: Smoke-test all five mock scenarios against the running Similar Products app on port 5000 and validate each response against the expected result. Use after code changes, before running load tests, or when verifying resilience behaviour (timeouts, 404/500 skipping). +allowed-tools: Bash(curl:*) +--- + +# Check endpoints + +Exercise every scenario the k6 test covers and confirm the app handles each correctly. + +## Run the smoke test + +```bash +for id in 1 2 3 4 5; do + echo "=== /product/$id/similar ===" + curl -s -w "\n-> HTTP %{http_code} in %{time_total}s\n" "http://localhost:5000/product/$id/similar" + echo +done +``` + +## Expected results + +| Endpoint | HTTP | Products returned | Why | +|----------|------|-------------------|-----| +| /product/1/similar | 200 | 2, 3, 4 | All upstreams fast | +| /product/2/similar | 200 | 3, 100 | Product 1000 (5s) exceeds the 2s timeout → skipped | +| /product/3/similar | 200 | 100 | Products 1000 (5s) and 10000 (50s) time out → skipped | +| /product/4/similar | 200 | 1, 2 | Product 5 returns 404 → skipped | +| /product/5/similar | 200 | 1, 2 | Product 6 returns 500 → skipped | + +Slow scenarios (2 and 3) should complete in roughly the timeout window (~2s), never the full upstream delay. + +## If something is wrong + +- **Timeout not respected (response takes 5s+):** check `product-api.detail-timeout-ms` and the `.timeout(...)` in `ProductClient.getProductDetail`. +- **404/500 propagated instead of skipped:** check the `onErrorResume` / non-2xx handling in `ProductClient.getProductDetail` — it must resolve to `Mono.empty()`. +- **Connection refused:** the app isn't running. Start it (and the mock) with `docker-compose up -d simulado yourapp` — build first with `docker-compose build yourapp` if code changed. (`cd app && mvn spring-boot:run` also works: Maven runs on JDK 21 even though `java` on the PATH is 17.) + +For the same behaviours without any of this running, use `/unit-test` — `SimilarProductsIntegrationTest` +covers these five scenarios against an in-process stub. diff --git a/.claude/skills/write-tests/SKILL.md b/.claude/skills/write-tests/SKILL.md new file mode 100644 index 00000000..a9b6d41e --- /dev/null +++ b/.claude/skills/write-tests/SKILL.md @@ -0,0 +1,77 @@ +--- +name: write-tests +description: Add or extend automated tests for the Similar Products service — picking the right layer, stubbing the upstream product API with MockWebServer, and testing reactive timeout/order/skip behaviour without flakiness. Use when writing new tests, covering a bug fix, or when a test is flaky or hangs. +allowed-tools: Read, Edit, Write, Grep, Glob, Bash(mvn:*) +--- + +# Write tests + +Tests live in `app/src/test/java/com/inditex/similarproducts/`, mirroring the main package layout. +Run them with `mvn test` from `app/` (see the `/unit-test` command). The build uses JDK 21 via Maven +even though `java` on the PATH is 17, so no Docker is needed. + +## Pick the right layer + +| Testing… | Use | Example | +|----------|-----|---------| +| Status codes, timeouts, JSON mapping of an upstream call | `ProductClientTest` — MockWebServer + `StepVerifier` | a 503 must be skipped | +| Order, parallelism, which products survive | `SimilarProductsServiceTest` — Mockito `ProductClient` | a skipped product must not shift order | +| Response status/body of the endpoint | `SimilarProductsControllerTest` — `@WebFluxTest` + `@MockBean` | an exception must map to 404 | +| A whole scenario over a real socket | `SimilarProductsIntegrationTest` — `@SpringBootTest(RANDOM_PORT)` | one of the five Simulado scenarios | + +Prefer the narrowest layer that can express the behaviour; add an integration test only when the +scenario is about the layers working together. + +## Stubbing the upstream + +`ProductClient` talks to a real socket, so the upstream is stubbed with **MockWebServer** (test-scoped, +version managed by the Spring Boot BOM), never by mocking `WebClient`. + +- **Sequential expectations** → `upstream.enqueue(...)`, one response per request. +- **Concurrent detail calls** → set a `Dispatcher` that answers **by path**. The default queue dispatcher + hands out responses in arrival order, which is not deterministic once the calls overlap. +- **Slow upstream** → `new MockResponse().setHeadersDelay(n, TimeUnit.SECONDS)`. +- Build the `WebClient` through `new WebClientConfig().productWebClient(...)` so the pool and connector + under test are the production ones. + +## Reactive assertions + +Use `StepVerifier`, never `.block()`: + +```java +StepVerifier.create(productClient.getProductDetail("5")) + .verifyComplete(); // empty = the product was skipped +``` + +For "these calls must overlap", use virtual time so the assertion is deterministic rather than a +wall-clock guess — and always give it a wall-clock bound: + +```java +StepVerifier.withVirtualTime(() -> service.getSimilarProducts("1")) + .expectSubscription() + .thenAwait(Duration.ofSeconds(1)) // 3 × 1s calls in parallel = 1s + .expectNext(PRODUCT_2, PRODUCT_3, PRODUCT_4) + .expectComplete() + .verify(Duration.ofSeconds(10)); // see the hang pitfall below +``` + +Stub the delayed Monos with `willAnswer(call -> Mono.just(p).delayElement(d))`, not `willReturn(...)`: +the Mono must be assembled *after* the virtual scheduler is installed, or the delay binds to the real clock. + +## Pitfalls that have already bitten here + +- **Cold-start flakiness.** The first HTTP call in a fresh JVM took ~1.2s (class loading, Netty, Jackson). + Tests that are *not* about timing must use the production 2s timeout; only the timeout tests use a short + budget, paired with a stub delay far above it. Never tighten a stub delay to speed a test up. +- **A virtual-time test hangs instead of failing.** If the code stops being concurrent, the virtual clock + never advances far enough and `verifyComplete()` waits forever. Always end with + `.expectComplete().verify(Duration.ofSeconds(10))` so a regression fails fast. +- **`verifyComplete(Duration)` does not exist** — it is `.expectComplete().verify(Duration)`. +- **Spring Boot 3.3 uses `@MockBean`**, not `@MockitoBean` (that arrives in 3.4). +- Point the integration test at the stub with `@DynamicPropertySource`, and start the `MockWebServer` in a + `static` initializer — the registry callback runs before `@BeforeAll`. + +## Before finishing + +Run the full suite (`cd app && mvn test`) and confirm the new test actually fails when the behaviour it +covers is broken — temporarily invert the production code, watch it go red, then revert. diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..58e34b0f --- /dev/null +++ b/.gitignore @@ -0,0 +1,10 @@ +# Build output +target/ +app/target/ + +# IntelliJ IDEA +.idea/ + +# OS +.DS_Store +Thumbs.db diff --git a/app/Dockerfile b/app/Dockerfile new file mode 100644 index 00000000..a36e3da3 --- /dev/null +++ b/app/Dockerfile @@ -0,0 +1,11 @@ +FROM maven:3.9.6-eclipse-temurin-21 AS build +WORKDIR /app +COPY pom.xml . +RUN mvn dependency:go-offline -B +COPY src ./src +RUN mvn package -DskipTests -B + +FROM eclipse-temurin:21-jre-alpine +WORKDIR /app +COPY --from=build /app/target/similar-products-*.jar app.jar +ENTRYPOINT ["java", "-jar", "app.jar"] diff --git a/app/README.md b/app/README.md new file mode 100644 index 00000000..97e05a5a --- /dev/null +++ b/app/README.md @@ -0,0 +1,285 @@ +# Similar Products Service + +Spring Boot application that implements the agreed `similarProducts` contract: it exposes +`GET /product/{productId}/similar` on port **5000**, aggregating the two existing upstream APIs +(similar-ids + product-detail) served by the mock on port 3001. + +- **Java 21**, **Spring Boot 3.3**, **Spring WebFlux** (Project Reactor + Reactor Netty) +- Fully non-blocking / reactive end to end + +--- + +## Table of contents + +- [Running the app](#running-the-app) +- [Architecture](#architecture) +- [Design decisions](#design-decisions) +- [Performance analysis & tuning](#performance-analysis--tuning) ← the core of this write-up +- [Configuration reference](#configuration-reference) +- [Testing](#testing) +- [Logging](#logging) + +--- + +## Running the app + +### With Docker (recommended — no local JDK needed) + +From the repository root: + +```bash +docker-compose up -d simulado influxdb grafana yourapp +curl http://localhost:5000/product/1/similar +``` + +### Locally + +Requires **JDK 21** and Maven. The mock must be running (`docker-compose up -d simulado`): + +```bash +cd app +mvn spring-boot:run +``` + +--- + +## Architecture + +``` +GET /product/{id}/similar + │ + ▼ +SimilarProductsController (reactive endpoint, maps errors to HTTP status) + │ + ▼ +SimilarProductsService orchestration + │ 1) getSimilarIds(id) → GET /product/{id}/similarids (single call) + │ 2) for each id, getProductDetail → GET /product/{id} (parallel) + ▼ +ProductClient WebClient calls with timeouts + error handling + │ + ▼ +Reactor Netty connection pool → upstream product API (mock :3001) +``` + +`flatMapSequential` is the key operator: it launches every product-detail request **in parallel** +(concurrency bounded by the connection pool) but **emits results in the original similarity order**, +satisfying the contract's "ordered by similarity" requirement without a post-sort. + +--- + +## Design decisions + +### Reactive / non-blocking +The load test drives 200 concurrent virtual users. A blocking, thread-per-request model would need +hundreds of threads and pay heavy context-switching cost. WebFlux + Reactor Netty handle the +concurrency on a small event-loop pool. A hook (`.claude/hooks/guard-blocking-calls.sh`) even blocks +edits that would introduce `.block()`/`Thread.sleep` into the main sources. + +### Resilience: partial results over failure +A similar product whose detail call fails must not fail the whole response: + +| Upstream situation | Behaviour | +|-------------------------------------------|---------------------------------------------| +| Detail returns 404 / 500 | product skipped (`Mono.empty()`) | +| Detail exceeds `detail-timeout-ms` (2s) | product skipped | +| Detail connection error | product skipped | +| `similarids` returns 404 | endpoint returns **404** (base product n/a) | +| `similarids` slow / stalled | bounded by `similar-ids-timeout-ms` (2s) | + +The `similar-ids` timeout was added after load testing revealed the entry-point call had no bound: +under connection contention it could stall on connection acquisition and hang the whole request. + +### Caching: deliberately omitted +The mocks return the same handful of products repeatedly, so caching looks tempting. It was measured +and **rejected** because it cannot improve the metric under test: + +- Throughput of the fast scenarios is capped by the k6 client's `sleep(0.5)` per iteration + (~333 req/s ceiling per scenario), **not** by upstream latency — the app already answers those in + ~13–100 ms. +- Latency of the slow scenarios is pinned to the 2s timeout on the genuinely-slow products + (1000 = 5s, 10000 = 50s), which are never cacheable (they time out and return nothing). Because + detail calls run in parallel, caching the fast product does not lower that ceiling. + +So a cache would only reduce upstream call volume (not what the test measures) while adding a +dependency, memory footprint and cache-invalidation/staleness concerns. **If the real product API +were the bottleneck (rather than being mocked)**, a short-TTL `Caffeine` `AsyncCache` with request +coalescing would be the right next step — see the note at the end of the performance section. + +--- + +## Performance analysis & tuning + +> This section documents the empirical analysis behind the chosen configuration. All numbers come +> from the provided k6 test (200 VUs × 5 scenarios) plus a concurrency probe, run on this hardware. +> They are relative, not absolute — reproduce with the sweeps below on the target hardware. + +### Finding 1 — the upstream is the bottleneck, not our pool + +The instinct under load is to enlarge the outbound connection pool. Measurement showed the opposite: +the mock is a single-process server, and flooding it with connections degrades every response. +Sweeping `product-api.max-connections` (via the `PRODUCT_API_MAX_CONNECTIONS` env var, no rebuild): + +| max-connections | throughput | median latency | +|----------------:|-----------:|---------------:| +| 4 | 304 req/s | 13 ms | +| 8 | 289 req/s | 26 ms | +| 16 | 259 req/s | 60 ms | +| 32 (netty default on 16 cores) | 206 req/s | 312 ms | +| 64 | 127 req/s | 743 ms | +| 128 | 77 req/s | 1.3 s | +| 500 | 120 req/s | ~450 ms | + +Throughput is **monotonically better with a smaller pool** — a genuinely counter-intuitive result +driven entirely by the mock's limited concurrency. + +### Finding 2 — a small pool is fast because it drops valid products + +Raw throughput hides a correctness cost. The k6 test does not validate response bodies, so a probe +was added: fire 80 concurrent `GET /product/2/similar` and count how many responses still contain +product **100** (an *available* product whose detail takes 1s — it should always be present). + +| max-connections | responses containing product 100 | +|----------------:|----------------------------------:| +| 8 | 12 % | +| 16 | 25 % | +| 32 | 35 % | +| 50 | 56 % | +| 100+ | 100 % | + +The mechanism: the doomed slow products (1000 = 5s, 10000 = 50s) hold their connections for the full +2s timeout. With a small pool they monopolise it, so the *available* 1s product (100) can't acquire a +connection within its own timeout and gets dropped. A small pool is fast **because it silently +returns incomplete results under load.** Shortening the timeout was tested as a mitigation and did +**not** help — completeness is governed by concurrent connection *demand*, not hold time. + +### The trade-off and the choice + +There is **no pool size that maximises both** throughput and completeness — they form a Pareto +frontier, because the same pool serves the fast scenarios (which want it small) and the slow +scenarios (which want it large). + +**Chosen: balanced — `max-connections: 50`.** + +- Healthy throughput (~160 req/s), bounded latency, **zero errors**. +- **Correct in normal use** — every scenario returns complete, correct results when not under + extreme concurrent load. +- Under massive concurrent load it degrades **gracefully**: it returns a valid partial list + (the contract allows `minItems: 0`) instead of failing, hanging, or overloading the upstream. + That is defensible resilience behaviour (load-shedding), not a silent bug. + +### How to move along the frontier + +Everything is configurable (property or env var) — no rebuild required: + +| Goal | Setting | Result | +|------|---------|--------| +| **Balanced (default)** | `max-connections: 50` | ~160 req/s, correct normally, graceful degradation under load | +| **Max throughput** (best k6 dashboard) | `max-connections: 8`–`16` | ~260–290 req/s, but sheds available products aggressively under concurrency | +| **Max correctness** (always complete) | `max-connections: 150`–`200` | 100 % complete even under load, ~90 req/s (mock overload lowers throughput) | + +```bash +# example: run the container tuned for maximum throughput +docker run -e PRODUCT_API_MAX_CONNECTIONS=12 ... +``` + +> **If the upstream were a real, horizontally-scalable API** (not a single-process mock), the picture +> flips: a larger pool would *not* overload it, "max correctness" and "max throughput" would converge, +> and adding a short-TTL cache with request coalescing (one upstream call shared by all concurrent +> callers of the same id) would become the highest-impact optimisation. The bottleneck here is an +> artefact of the mock, and the design stays correct for either world. + +--- + +## Configuration reference + +All properties live under `product-api` in `application.yaml`; each maps to an env var +(Spring relaxed binding), so it can be overridden per-environment without rebuilding. + +| Property | Env var | Default | Purpose | +|----------|---------|---------|---------| +| `product-api.base-url` | `PRODUCT_API_BASE_URL` | `http://localhost:3001` | Upstream base URL (`http://simulado` in Docker) | +| `product-api.connect-timeout-ms` | `PRODUCT_API_CONNECT_TIMEOUT_MS` | 1000 | TCP connect timeout | +| `product-api.detail-timeout-ms` | `PRODUCT_API_DETAIL_TIMEOUT_MS` | 2000 | Per-product detail timeout (bounds the 5s/50s mocks) | +| `product-api.similar-ids-timeout-ms` | `PRODUCT_API_SIMILAR_IDS_TIMEOUT_MS` | 2000 | Entry-point call timeout | +| `product-api.max-connections` | `PRODUCT_API_MAX_CONNECTIONS` | 50 | Outbound connection pool size (see tuning above) | +| `product-api.pending-acquire-timeout-ms` | `PRODUCT_API_PENDING_ACQUIRE_TIMEOUT_MS` | 2000 | Max wait for a pooled connection, aligned with the request budget | +| `logging.level.com.inditex.similarproducts` | `LOG_LEVEL` | `INFO` | `DEBUG` traces every request (see [Logging](#logging)); keep at `INFO` under load | +| `server.port` | — | 5000 | Contract-mandated port | + +--- + +## Testing + +### Automated test suite + +```bash +cd app && mvn test # 36 tests, ~25s +``` + +Nothing needs to be running: the upstream product API is stubbed in-process with **MockWebServer** +(test-scoped, version managed by the Spring Boot BOM), so the suite is self-contained and CI-friendly. + +| Test class | Layer | Covers | +|------------|-------|--------| +| `ProductClientTest` | HTTP boundary | Per-call timeouts, 404 → `ProductNotFoundException`, 500 → error, detail 404/500/timeout/bad-body → skipped, numeric ids coerced to strings | +| `ProductClientLoggingTest` | HTTP boundary | Pins the log levels: a skipped product never reaches INFO; an entry-point failure always reaches WARN | +| `SimilarProductsServiceTest` | Aggregation | Similarity order preserved when an earlier product answers last, details fetched **in parallel** (asserted with virtual time), unresolvable products dropped, upstream errors propagated | +| `SimilarProductsControllerTest` | HTTP contract | 200 + JSON body, `[]` when nothing resolves, 404 for an unknown base product, 5xx for an unexpected failure | +| `SimilarProductsIntegrationTest` | End-to-end | The five scenarios below, over a real socket through the full chain | +| `SimilarProductsApplicationTests` | Wiring | Context loads | + +Timing tests scale the upstream delays down (a stub delayed far above a short timeout) rather than +waiting out the real 5s/50s mocks. Tests that are *not* about timing keep the production 2s timeout so +JVM warm-up cannot make a healthy product look slow. + +### Functional smoke test (all 5 scenarios) + +```bash +for id in 1 2 3 4 5; do + echo "=== /product/$id/similar ===" + curl -s -w "\n-> HTTP %{http_code} in %{time_total}s\n" "http://localhost:5000/product/$id/similar" +done +``` + +Expected: product 1 → {2,3,4}; product 2 → {3,100} (1000 times out); product 3 → {100} +(1000+10000 time out); product 4 → {1,2} (5 is 404); product 5 → {1,2} (6 is 500). + +### Load test (the provided k6 suite) + +```bash +docker-compose run --rm k6 run scripts/test.js +# results: http://localhost:3000/d/Le2Ku9NMk/k6-performance-test +``` + +--- + +## Logging + +The governing rule is **WARN for what makes a request fail, DEBUG for what the design deliberately +tolerates**. Skipping a product is designed behaviour, not a fault: products 1000 and 10000 time out on +essentially every request the load test makes, so logging that path above DEBUG would bury the failures +that actually matter. + +| Level | What is logged | Volume | +|-------|----------------|--------| +| `INFO` | Effective client config (base URL, pool, timeouts) | 2 lines, once at startup | +| `WARN` | `/similarids` returned 5xx or timed out; a request resolved to a 5xx | Only when a request actually fails | +| `DEBUG` | IDs received, every skipped product with its reason, products resolved + elapsed ms | ~2 lines per request, plus one per skip | + +A 404 from `/similarids` is a client outcome (it becomes a 404 response), not a fault — it stays at DEBUG. + +Running the full set of scenarios at the default `INFO` produces **zero** per-request lines. Set +`LOG_LEVEL=DEBUG` to trace a request end to end — never during a load test: + +```bash +LOG_LEVEL=DEBUG mvn spring-boot:run +``` + +``` +DEBUG SimilarProductsService : Product 2 has similar IDs [3, 100, 1000] +DEBUG ProductClient : Skipping product 1000: no response within 2000ms +DEBUG SimilarProductsController: Resolved 2 similar products for product 2 in 2012ms +``` + +`ProductClientLoggingTest` pins these levels so the no-noise rule cannot be relaxed by accident. diff --git a/app/pom.xml b/app/pom.xml new file mode 100644 index 00000000..91a6c776 --- /dev/null +++ b/app/pom.xml @@ -0,0 +1,58 @@ + + + 4.0.0 + + + org.springframework.boot + spring-boot-starter-parent + 3.3.5 + + + + com.inditex + similar-products + 0.0.1-SNAPSHOT + similar-products + + + 21 + + + + + org.springframework.boot + spring-boot-starter-webflux + + + + org.springframework.boot + spring-boot-starter-test + test + + + + io.projectreactor + reactor-test + test + + + + + com.squareup.okhttp3 + mockwebserver + test + + + + + + + org.springframework.boot + spring-boot-maven-plugin + + + + diff --git a/app/src/main/java/com/inditex/similarproducts/SimilarProductsApplication.java b/app/src/main/java/com/inditex/similarproducts/SimilarProductsApplication.java new file mode 100644 index 00000000..5f3ad616 --- /dev/null +++ b/app/src/main/java/com/inditex/similarproducts/SimilarProductsApplication.java @@ -0,0 +1,11 @@ +package com.inditex.similarproducts; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; + +@SpringBootApplication +public class SimilarProductsApplication { + public static void main(String[] args) { + SpringApplication.run(SimilarProductsApplication.class, args); + } +} diff --git a/app/src/main/java/com/inditex/similarproducts/client/ProductClient.java b/app/src/main/java/com/inditex/similarproducts/client/ProductClient.java new file mode 100644 index 00000000..60fe8880 --- /dev/null +++ b/app/src/main/java/com/inditex/similarproducts/client/ProductClient.java @@ -0,0 +1,94 @@ +package com.inditex.similarproducts.client; + +import com.inditex.similarproducts.exception.ProductNotFoundException; +import com.inditex.similarproducts.model.ProductDetail; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.core.ParameterizedTypeReference; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Component; +import org.springframework.web.reactive.function.client.WebClient; +import reactor.core.publisher.Mono; + +import java.time.Duration; +import java.util.List; +import java.util.concurrent.TimeoutException; + +@Component +public class ProductClient { + + private static final Logger log = LoggerFactory.getLogger(ProductClient.class); + + private final WebClient webClient; + private final Duration detailTimeout; + private final Duration similarIdsTimeout; + + public ProductClient( + WebClient productWebClient, + @Value("${product-api.detail-timeout-ms:2000}") long detailTimeoutMs, + @Value("${product-api.similar-ids-timeout-ms:2000}") long similarIdsTimeoutMs) { + this.webClient = productWebClient; + this.detailTimeout = Duration.ofMillis(detailTimeoutMs); + this.similarIdsTimeout = Duration.ofMillis(similarIdsTimeoutMs); + log.info("Product API timeouts: similarIds={}ms, detail={}ms", similarIdsTimeoutMs, detailTimeoutMs); + } + + /** + * Entry-point call. Anything that goes wrong here fails the whole request, so failures are + * logged at WARN — this is the signal that the upstream (or the connection pool) is degraded. + */ + public Mono> getSimilarIds(String productId) { + return webClient.get() + .uri("/product/{id}/similarids", productId) + .exchangeToMono(response -> { + if (response.statusCode() == HttpStatus.NOT_FOUND) { + // An expected client outcome, not a fault: it becomes a 404 response. + log.debug("No similar IDs for product {}: upstream returned 404", productId); + return response.releaseBody() + .then(Mono.error(new ProductNotFoundException(productId))); + } + if (response.statusCode().isError()) { + log.warn("Upstream returned {} fetching similar IDs for product {}", + response.statusCode(), productId); + return response.releaseBody() + .then(Mono.error(new RuntimeException("Upstream error fetching similar IDs for product " + productId))); + } + return response.bodyToMono(new ParameterizedTypeReference>() {}) + .map(ids -> ids.stream().map(Object::toString).toList()); + }) + // Bound the entry-point call so a stalled upstream can't hang the whole request. + .timeout(similarIdsTimeout) + .doOnError(TimeoutException.class, error -> + log.warn("Timed out after {}ms fetching similar IDs for product {}", + similarIdsTimeout.toMillis(), productId)); + } + + /** + * Non-2xx responses and timeouts resolve to {@code Mono.empty()} — the product is skipped. + * + *

Skipping is designed behaviour, not a fault: products 1000 and 10000 time out on + * essentially every request the load test makes. These are therefore logged at DEBUG — at any + * higher level they would drown the log under load and hide the failures that actually matter. + */ + public Mono getProductDetail(String productId) { + return webClient.get() + .uri("/product/{id}", productId) + .exchangeToMono(response -> { + if (response.statusCode().is2xxSuccessful()) { + return response.bodyToMono(ProductDetail.class); + } + log.debug("Skipping product {}: upstream returned {}", productId, response.statusCode()); + return response.releaseBody().then(Mono.empty()); + }) + .timeout(detailTimeout) + .onErrorResume(error -> { + if (error instanceof TimeoutException) { + log.debug("Skipping product {}: no response within {}ms", productId, detailTimeout.toMillis()); + } else { + log.debug("Skipping product {}: {}", productId, error.toString()); + } + return Mono.empty(); + }); + } +} diff --git a/app/src/main/java/com/inditex/similarproducts/config/WebClientConfig.java b/app/src/main/java/com/inditex/similarproducts/config/WebClientConfig.java new file mode 100644 index 00000000..6b70d351 --- /dev/null +++ b/app/src/main/java/com/inditex/similarproducts/config/WebClientConfig.java @@ -0,0 +1,55 @@ +package com.inditex.similarproducts.config; + +import io.netty.channel.ChannelOption; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.reactive.ReactorClientHttpConnector; +import org.springframework.web.reactive.function.client.WebClient; +import reactor.netty.http.client.HttpClient; +import reactor.netty.resources.ConnectionProvider; + +import java.time.Duration; + +@Configuration +public class WebClientConfig { + + private static final Logger log = LoggerFactory.getLogger(WebClientConfig.class); + + @Bean + public WebClient productWebClient( + @Value("${product-api.base-url}") String baseUrl, + @Value("${product-api.connect-timeout-ms:1000}") int connectTimeoutMs, + @Value("${product-api.max-connections:50}") int maxConnections, + @Value("${product-api.pending-acquire-timeout-ms:2000}") long pendingAcquireTimeoutMs) { + + // Explicit, modest pool for deterministic throughput across machines (the reactor-netty + // default is CPU-dependent: max(cores, 8) * 2). Load testing showed the upstream is the + // bottleneck, not this pool: oversizing floods the single-process mock and degrades latency, + // while undersizing silently drops available products under load. maxConnections is the main + // performance lever and sits on a throughput/completeness Pareto frontier; 50 is the balanced + // default. Tune per environment via PRODUCT_API_MAX_CONNECTIONS — see app/README.md + // ("Performance analysis & tuning") for the data and the max-throughput / max-correctness + // presets. pendingAcquireTimeout is aligned with the request timeout budget so a request + // never waits for a connection longer than it would wait for a response. + ConnectionProvider connectionProvider = ConnectionProvider.builder("product-api") + .maxConnections(maxConnections) + .pendingAcquireTimeout(Duration.ofMillis(pendingAcquireTimeoutMs)) + .build(); + + HttpClient httpClient = HttpClient.create(connectionProvider) + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, connectTimeoutMs); + + // Logged once at startup: maxConnections is the main performance lever and is env-overridable, + // so the effective value has to be visible when diagnosing a run. + log.info("Product API client: baseUrl={}, maxConnections={}, connectTimeout={}ms, pendingAcquireTimeout={}ms", + baseUrl, maxConnections, connectTimeoutMs, pendingAcquireTimeoutMs); + + return WebClient.builder() + .baseUrl(baseUrl) + .clientConnector(new ReactorClientHttpConnector(httpClient)) + .build(); + } +} diff --git a/app/src/main/java/com/inditex/similarproducts/controller/SimilarProductsController.java b/app/src/main/java/com/inditex/similarproducts/controller/SimilarProductsController.java new file mode 100644 index 00000000..d2a47952 --- /dev/null +++ b/app/src/main/java/com/inditex/similarproducts/controller/SimilarProductsController.java @@ -0,0 +1,48 @@ +package com.inditex.similarproducts.controller; + +import com.inditex.similarproducts.exception.ProductNotFoundException; +import com.inditex.similarproducts.model.ProductDetail; +import com.inditex.similarproducts.service.SimilarProductsService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RestController; +import reactor.core.publisher.Mono; + +import java.util.List; + +@RestController +public class SimilarProductsController { + + private static final Logger log = LoggerFactory.getLogger(SimilarProductsController.class); + + private final SimilarProductsService similarProductsService; + + public SimilarProductsController(SimilarProductsService similarProductsService) { + this.similarProductsService = similarProductsService; + } + + @GetMapping("/product/{productId}/similar") + public Mono>> getSimilarProducts(@PathVariable String productId) { + long startedAt = System.nanoTime(); + return similarProductsService.getSimilarProducts(productId) + .collectList() + .doOnNext(products -> logResolved(productId, products.size(), startedAt)) + .map(ResponseEntity::ok) + .onErrorResume(ProductNotFoundException.class, + e -> Mono.just(ResponseEntity.notFound().build())) + // Placed after the 404 mapping, so only genuine failures reach it. A request that + // gets this far returns a 5xx to the client, which is always worth a line. + .doOnError(error -> log.warn("Failed to resolve similar products for product {}: {}", + productId, error.toString())); + } + + private static void logResolved(String productId, int resolved, long startedAt) { + if (log.isDebugEnabled()) { + log.debug("Resolved {} similar products for product {} in {}ms", + resolved, productId, (System.nanoTime() - startedAt) / 1_000_000); + } + } +} diff --git a/app/src/main/java/com/inditex/similarproducts/exception/ProductNotFoundException.java b/app/src/main/java/com/inditex/similarproducts/exception/ProductNotFoundException.java new file mode 100644 index 00000000..57d11015 --- /dev/null +++ b/app/src/main/java/com/inditex/similarproducts/exception/ProductNotFoundException.java @@ -0,0 +1,7 @@ +package com.inditex.similarproducts.exception; + +public class ProductNotFoundException extends RuntimeException { + public ProductNotFoundException(String productId) { + super("Product not found: " + productId); + } +} diff --git a/app/src/main/java/com/inditex/similarproducts/model/ProductDetail.java b/app/src/main/java/com/inditex/similarproducts/model/ProductDetail.java new file mode 100644 index 00000000..3bdfa2ff --- /dev/null +++ b/app/src/main/java/com/inditex/similarproducts/model/ProductDetail.java @@ -0,0 +1,3 @@ +package com.inditex.similarproducts.model; + +public record ProductDetail(String id, String name, Double price, Boolean availability) {} diff --git a/app/src/main/java/com/inditex/similarproducts/service/SimilarProductsService.java b/app/src/main/java/com/inditex/similarproducts/service/SimilarProductsService.java new file mode 100644 index 00000000..d69defb7 --- /dev/null +++ b/app/src/main/java/com/inditex/similarproducts/service/SimilarProductsService.java @@ -0,0 +1,40 @@ +package com.inditex.similarproducts.service; + +import com.inditex.similarproducts.client.ProductClient; +import com.inditex.similarproducts.model.ProductDetail; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; +import reactor.core.publisher.Flux; + +@Service +public class SimilarProductsService { + + private static final Logger log = LoggerFactory.getLogger(SimilarProductsService.class); + + private final ProductClient productClient; + + public SimilarProductsService(ProductClient productClient) { + this.productClient = productClient; + } + + /** + * Fetches the similar product IDs, then resolves each product detail concurrently. + * + *

{@code flatMapSequential} fires all the detail requests in parallel (bounded by the + * connection pool) while preserving the similarity order in the emitted result. + * + *

No cache is used here on purpose. Load testing showed the throughput ceiling is imposed + * by the k6 client pacing (sleep between iterations) and the per-product timeout, not by the + * number of upstream calls; a cache could not raise the measured throughput and would only add + * memory footprint and staleness. See README.md ("Caching: deliberately omitted") for the data. + */ + public Flux getSimilarProducts(String productId) { + return productClient.getSimilarIds(productId) + // Two DEBUG lines per request (here and in the controller) are enough to follow a + // flow end to end: how many IDs came back, and how many survived the detail fetch. + .doOnNext(similarIds -> log.debug("Product {} has similar IDs {}", productId, similarIds)) + .flatMapMany(Flux::fromIterable) + .flatMapSequential(productClient::getProductDetail); + } +} diff --git a/app/src/main/resources/application.yaml b/app/src/main/resources/application.yaml new file mode 100644 index 00000000..5ea4cd17 --- /dev/null +++ b/app/src/main/resources/application.yaml @@ -0,0 +1,18 @@ +server: + port: 5000 + +product-api: + base-url: ${PRODUCT_API_BASE_URL:http://localhost:3001} + connect-timeout-ms: 1000 + detail-timeout-ms: 2000 + similar-ids-timeout-ms: 2000 + max-connections: 50 + pending-acquire-timeout-ms: 2000 + +logging: + level: + # INFO in production: startup config, plus a WARN whenever a request actually fails. + # Set LOG_LEVEL=DEBUG to trace a single request end to end (IDs received, products skipped + # and why, products resolved and how long it took). DEBUG is per-request and per-product — + # do not leave it on during a load test. + com.inditex.similarproducts: ${LOG_LEVEL:INFO} diff --git a/app/src/test/java/com/inditex/similarproducts/SimilarProductsApplicationTests.java b/app/src/test/java/com/inditex/similarproducts/SimilarProductsApplicationTests.java new file mode 100644 index 00000000..e61a3463 --- /dev/null +++ b/app/src/test/java/com/inditex/similarproducts/SimilarProductsApplicationTests.java @@ -0,0 +1,14 @@ +package com.inditex.similarproducts; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.TestPropertySource; + +@SpringBootTest +@TestPropertySource(properties = "product-api.base-url=http://localhost:3001") +class SimilarProductsApplicationTests { + + @Test + void contextLoads() { + } +} diff --git a/app/src/test/java/com/inditex/similarproducts/SimilarProductsIntegrationTest.java b/app/src/test/java/com/inditex/similarproducts/SimilarProductsIntegrationTest.java new file mode 100644 index 00000000..41f1b3d0 --- /dev/null +++ b/app/src/test/java/com/inditex/similarproducts/SimilarProductsIntegrationTest.java @@ -0,0 +1,186 @@ +package com.inditex.similarproducts; + +import okhttp3.mockwebserver.Dispatcher; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.test.web.reactive.server.WebTestClient; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * End-to-end test of the whole chain — controller, service, client, WebClient and a real + * socket — against a stub that reproduces the Simulado scenarios the k6 test drives. + * + *

Delays are scaled down (5s upstream vs a 1.5s timeout) so the suite stays fast while + * keeping the same shape: some products cannot possibly arrive in time. The timeout stays + * generous enough that a cold context can never make a healthy product look slow. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +class SimilarProductsIntegrationTest { + + private static final long DETAIL_TIMEOUT_MS = 1500; + private static final long UPSTREAM_DELAY_SECONDS = 5; + + /** productId -> similar ids, mirroring the Simulado mock. */ + private static final Map SIMILAR_IDS = Map.of( + "1", "[\"2\",\"3\",\"4\"]", + "2", "[\"3\",\"100\",\"1000\"]", + "3", "[\"100\",\"1000\",\"10000\"]", + "4", "[\"1\",\"2\",\"5\"]", + "5", "[\"1\",\"2\",\"6\"]"); + + /** Products that never answer in time — as products 1000 (5s) and 10000 (50s) do upstream. */ + private static final List TOO_SLOW = List.of("1000", "10000"); + + private static final MockWebServer UPSTREAM = new MockWebServer(); + + static { + try { + UPSTREAM.start(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + UPSTREAM.setDispatcher(new SimuladoDispatcher()); + } + + @Autowired + private WebTestClient webTestClient; + + @DynamicPropertySource + static void upstreamProperties(DynamicPropertyRegistry registry) { + registry.add("product-api.base-url", + () -> "http://" + UPSTREAM.getHostName() + ":" + UPSTREAM.getPort()); + registry.add("product-api.detail-timeout-ms", () -> DETAIL_TIMEOUT_MS); + registry.add("product-api.similar-ids-timeout-ms", () -> DETAIL_TIMEOUT_MS); + } + + @AfterAll + static void stopUpstream() throws IOException { + UPSTREAM.shutdown(); + } + + @Test + void returnsEverySimilarProductWhenAllUpstreamsAnswer() { + webTestClient.get().uri("/product/1/similar") + .exchange() + .expectStatus().isOk() + .expectBody() + .jsonPath("$.length()").isEqualTo(3) + .jsonPath("$[0].id").isEqualTo("2") + .jsonPath("$[1].id").isEqualTo("3") + .jsonPath("$[2].id").isEqualTo("4") + .jsonPath("$[0].name").isEqualTo("Product 2") + .jsonPath("$[0].price").isEqualTo(19.99) + .jsonPath("$[0].availability").isEqualTo(true); + } + + @Test + void skipsTheSlowProductAndAnswersWithinTheTimeout() { + long startedAt = System.nanoTime(); + + webTestClient.get().uri("/product/2/similar") + .exchange() + .expectStatus().isOk() + .expectBody() + .jsonPath("$.length()").isEqualTo(2) + .jsonPath("$[0].id").isEqualTo("3") + .jsonPath("$[1].id").isEqualTo("100"); + + long elapsedMs = (System.nanoTime() - startedAt) / 1_000_000; + assertThat(elapsedMs) + .as("must give up on the slow product, not wait it out") + .isLessThan(TimeUnit.SECONDS.toMillis(UPSTREAM_DELAY_SECONDS) - 1000); + } + + @Test + void skipsEverySlowProductAndStillReturnsTheFastOne() { + webTestClient.get().uri("/product/3/similar") + .exchange() + .expectStatus().isOk() + .expectBody() + .jsonPath("$.length()").isEqualTo(1) + .jsonPath("$[0].id").isEqualTo("100"); + } + + @Test + void skipsProductsThatReturn404() { + webTestClient.get().uri("/product/4/similar") + .exchange() + .expectStatus().isOk() + .expectBody() + .jsonPath("$.length()").isEqualTo(2) + .jsonPath("$[0].id").isEqualTo("1") + .jsonPath("$[1].id").isEqualTo("2"); + } + + @Test + void skipsProductsThatReturn500() { + webTestClient.get().uri("/product/5/similar") + .exchange() + .expectStatus().isOk() + .expectBody() + .jsonPath("$.length()").isEqualTo(2) + .jsonPath("$[0].id").isEqualTo("1") + .jsonPath("$[1].id").isEqualTo("2"); + } + + @Test + void returns404WhenTheProductHasNoSimilarIdsEndpoint() { + webTestClient.get().uri("/product/999/similar") + .exchange() + .expectStatus().isNotFound(); + } + + /** Serves by path rather than by queue order, since the detail calls arrive concurrently. */ + private static class SimuladoDispatcher extends Dispatcher { + + @Override + public MockResponse dispatch(RecordedRequest request) { + String path = request.getPath() == null ? "" : request.getPath(); + + if (path.endsWith("/similarids")) { + String id = path.substring("/product/".length(), path.length() - "/similarids".length()); + String ids = SIMILAR_IDS.get(id); + return ids == null ? new MockResponse().setResponseCode(404) : json(ids); + } + + String id = path.substring(path.lastIndexOf('/') + 1); + return switch (id) { + case "5" -> new MockResponse().setResponseCode(404); + case "6" -> new MockResponse().setResponseCode(500); + default -> { + MockResponse response = json(product(id)); + yield TOO_SLOW.contains(id) + ? response.setHeadersDelay(UPSTREAM_DELAY_SECONDS, TimeUnit.SECONDS) + : response; + } + }; + } + + private static String product(String id) { + return "{\"id\":\"%s\",\"name\":\"Product %s\",\"price\":19.99,\"availability\":true}" + .formatted(id, id); + } + + private static MockResponse json(String body) { + return new MockResponse() + .setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody(body); + } + } +} diff --git a/app/src/test/java/com/inditex/similarproducts/client/ProductClientLoggingTest.java b/app/src/test/java/com/inditex/similarproducts/client/ProductClientLoggingTest.java new file mode 100644 index 00000000..130a8ca4 --- /dev/null +++ b/app/src/test/java/com/inditex/similarproducts/client/ProductClientLoggingTest.java @@ -0,0 +1,157 @@ +package com.inditex.similarproducts.client; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import com.inditex.similarproducts.config.WebClientConfig; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import reactor.test.StepVerifier; + +import java.io.IOException; +import java.time.Duration; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Pins the log levels of the upstream boundary. + * + *

The service skips products by design — under load that path runs on nearly every request, so + * logging it above DEBUG would bury the failures that matter. These tests exist so that rule cannot + * be relaxed by accident. + */ +class ProductClientLoggingTest { + + private static final long TIMEOUT_MS = 2000; + private static final long SHORT_TIMEOUT_MS = 300; + private static final long UPSTREAM_DELAY_SECONDS = 3; + + private MockWebServer upstream; + private ProductClient productClient; + + private ch.qos.logback.classic.Logger clientLogger; + private ListAppender logEvents; + private Level originalLevel; + + @BeforeEach + void startUpstream() throws IOException { + upstream = new MockWebServer(); + upstream.start(); + productClient = clientWithTimeout(TIMEOUT_MS); + + // Attached after the client is built, so its startup INFO line stays out of the assertions. + clientLogger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(ProductClient.class); + originalLevel = clientLogger.getLevel(); + clientLogger.setLevel(Level.DEBUG); + logEvents = new ListAppender<>(); + logEvents.start(); + clientLogger.addAppender(logEvents); + } + + @AfterEach + void stopUpstream() throws IOException { + clientLogger.detachAppender(logEvents); + clientLogger.setLevel(originalLevel); + upstream.shutdown(); + } + + @Test + void skippingAProductThatReturns404IsOnlyLoggedAtDebug() { + upstream.enqueue(new MockResponse().setResponseCode(404)); + + StepVerifier.create(productClient.getProductDetail("5")).verifyComplete(); + + assertThat(messagesAtLevel(Level.DEBUG)).anyMatch(message -> message.contains("Skipping product 5")); + assertThat(eventsAtOrAbove(Level.INFO)) + .as("a skipped product is designed behaviour and must not reach INFO or above") + .isEmpty(); + } + + @Test + void skippingAProductThatTimesOutIsOnlyLoggedAtDebug() { + upstream.enqueue(json("{\"id\":\"1000\"}").setHeadersDelay(UPSTREAM_DELAY_SECONDS, TimeUnit.SECONDS)); + ProductClient impatientClient = clientWithTimeout(SHORT_TIMEOUT_MS); + logEvents.list.clear(); // drop that client's startup line + + StepVerifier.create(impatientClient.getProductDetail("1000")) + .expectComplete() + .verify(Duration.ofSeconds(2)); + + assertThat(messagesAtLevel(Level.DEBUG)).anyMatch(message -> message.contains("Skipping product 1000")); + assertThat(eventsAtOrAbove(Level.INFO)) + .as("timeouts are the expected outcome for the slow products and must stay at DEBUG") + .isEmpty(); + } + + @Test + void aProductWithNoSimilarIdsIsNotLoggedAsAFailure() { + upstream.enqueue(new MockResponse().setResponseCode(404)); + + StepVerifier.create(productClient.getSimilarIds("999")).verifyError(); + + assertThat(eventsAtOrAbove(Level.WARN)) + .as("a 404 from /similarids is a 404 response to the client, not a fault") + .isEmpty(); + } + + @Test + void anUpstreamErrorOnTheEntryPointCallIsLoggedAtWarn() { + upstream.enqueue(new MockResponse().setResponseCode(500)); + + StepVerifier.create(productClient.getSimilarIds("1")).verifyError(); + + // This one fails the whole request — it has to be visible without turning DEBUG on. + assertThat(messagesAtLevel(Level.WARN)) + .anyMatch(message -> message.contains("fetching similar IDs for product 1")); + } + + @Test + void aTimeoutOnTheEntryPointCallIsLoggedAtWarn() { + upstream.enqueue(json("[\"2\"]").setHeadersDelay(UPSTREAM_DELAY_SECONDS, TimeUnit.SECONDS)); + ProductClient impatientClient = clientWithTimeout(SHORT_TIMEOUT_MS); + logEvents.list.clear(); + + StepVerifier.create(impatientClient.getSimilarIds("1")) + .expectError() + .verify(Duration.ofSeconds(2)); + + assertThat(messagesAtLevel(Level.WARN)) + .anyMatch(message -> message.contains("Timed out") && message.contains("product 1")); + } + + private List messagesAtLevel(Level level) { + return logEvents.list.stream() + .filter(event -> event.getLevel() == level) + .map(ILoggingEvent::getFormattedMessage) + .toList(); + } + + private List eventsAtOrAbove(Level level) { + return logEvents.list.stream() + .filter(event -> event.getLevel().isGreaterOrEqual(level)) + .map(event -> event.getLevel() + " " + event.getFormattedMessage()) + .toList(); + } + + private ProductClient clientWithTimeout(long timeoutMs) { + String baseUrl = "http://" + upstream.getHostName() + ":" + upstream.getPort(); + return new ProductClient( + new WebClientConfig().productWebClient(baseUrl, 1000, 50, 2000), + timeoutMs, + timeoutMs); + } + + private static MockResponse json(String body) { + return new MockResponse() + .setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody(body); + } +} diff --git a/app/src/test/java/com/inditex/similarproducts/client/ProductClientTest.java b/app/src/test/java/com/inditex/similarproducts/client/ProductClientTest.java new file mode 100644 index 00000000..ce852a0f --- /dev/null +++ b/app/src/test/java/com/inditex/similarproducts/client/ProductClientTest.java @@ -0,0 +1,183 @@ +package com.inditex.similarproducts.client; + +import com.inditex.similarproducts.config.WebClientConfig; +import com.inditex.similarproducts.exception.ProductNotFoundException; +import com.inditex.similarproducts.model.ProductDetail; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import reactor.test.StepVerifier; + +import java.io.IOException; +import java.time.Duration; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Unit tests for the upstream HTTP boundary, against a real (stubbed) socket so the + * timeout and non-2xx handling are exercised the same way they are in production. + */ +class ProductClientTest { + + /** The production default — generous enough that a cold JVM can't trip the non-timing tests. */ + private static final long TIMEOUT_MS = 2000; + + /** Used only by the timeout tests, to keep the suite fast. */ + private static final long SHORT_TIMEOUT_MS = 300; + + /** Comfortably above SHORT_TIMEOUT_MS, so a slow stub can only ever lose the race. */ + private static final long UPSTREAM_DELAY_SECONDS = 3; + + private MockWebServer upstream; + private ProductClient productClient; + + @BeforeEach + void startUpstream() throws IOException { + upstream = new MockWebServer(); + upstream.start(); + productClient = clientWithTimeout(TIMEOUT_MS); + } + + /** Built through the production config so the pool/connector wiring is the one under test. */ + private ProductClient clientWithTimeout(long timeoutMs) { + String baseUrl = "http://" + upstream.getHostName() + ":" + upstream.getPort(); + return new ProductClient( + new WebClientConfig().productWebClient(baseUrl, 1000, 50, 2000), + timeoutMs, + timeoutMs); + } + + @AfterEach + void stopUpstream() throws IOException { + upstream.shutdown(); + } + + // --- getSimilarIds ------------------------------------------------------- + + @Test + void getSimilarIdsReturnsIdsInUpstreamOrder() throws InterruptedException { + upstream.enqueue(json("[\"2\",\"3\",\"4\"]")); + + StepVerifier.create(productClient.getSimilarIds("1")) + .expectNext(List.of("2", "3", "4")) + .verifyComplete(); + + RecordedRequest request = upstream.takeRequest(); + assertThat(request.getPath()).isEqualTo("/product/1/similarids"); + } + + @Test + void getSimilarIdsCoercesNumericIdsToStrings() { + // Simulado answers with JSON numbers, not strings — they must survive as usable ids. + upstream.enqueue(json("[2,3,4]")); + + StepVerifier.create(productClient.getSimilarIds("1")) + .expectNext(List.of("2", "3", "4")) + .verifyComplete(); + } + + @Test + void getSimilarIdsReturnsEmptyListWhenUpstreamHasNoSimilarProducts() { + upstream.enqueue(json("[]")); + + StepVerifier.create(productClient.getSimilarIds("1")) + .expectNext(List.of()) + .verifyComplete(); + } + + @Test + void getSimilarIdsFailsWithProductNotFoundOn404() { + upstream.enqueue(new MockResponse().setResponseCode(404)); + + StepVerifier.create(productClient.getSimilarIds("999")) + .expectErrorSatisfies(error -> assertThat(error) + .isInstanceOf(ProductNotFoundException.class) + .hasMessageContaining("999")) + .verify(); + } + + @Test + void getSimilarIdsFailsOnUpstreamServerError() { + upstream.enqueue(new MockResponse().setResponseCode(500)); + + StepVerifier.create(productClient.getSimilarIds("1")) + .expectErrorSatisfies(error -> assertThat(error) + // A 500 is not "no similar products" — it must not be mistaken for a 404. + .isNotInstanceOf(ProductNotFoundException.class) + .hasMessageContaining("Upstream error")) + .verify(); + } + + @Test + void getSimilarIdsTimesOutInsteadOfHangingOnAStalledUpstream() { + upstream.enqueue(json("[\"2\"]").setHeadersDelay(UPSTREAM_DELAY_SECONDS, TimeUnit.SECONDS)); + + StepVerifier.create(clientWithTimeout(SHORT_TIMEOUT_MS).getSimilarIds("1")) + .expectError(TimeoutException.class) + // Bound well below the upstream delay: the timeout must be what ends the call. + .verify(Duration.ofSeconds(2)); + } + + // --- getProductDetail ---------------------------------------------------- + + @Test + void getProductDetailMapsEveryField() throws InterruptedException { + upstream.enqueue(json("{\"id\":\"2\",\"name\":\"Dress\",\"price\":19.99,\"availability\":true}")); + + StepVerifier.create(productClient.getProductDetail("2")) + .expectNext(new ProductDetail("2", "Dress", 19.99, true)) + .verifyComplete(); + + RecordedRequest request = upstream.takeRequest(); + assertThat(request.getPath()).isEqualTo("/product/2"); + } + + @Test + void getProductDetailSkipsProductOn404() { + upstream.enqueue(new MockResponse().setResponseCode(404)); + + // Skipped, not failed: one missing product must not sink the whole response. + StepVerifier.create(productClient.getProductDetail("5")) + .verifyComplete(); + } + + @Test + void getProductDetailSkipsProductOnServerError() { + upstream.enqueue(new MockResponse().setResponseCode(500)); + + StepVerifier.create(productClient.getProductDetail("6")) + .verifyComplete(); + } + + @Test + void getProductDetailSkipsProductThatExceedsTheTimeout() { + upstream.enqueue(json("{\"id\":\"1000\"}").setHeadersDelay(UPSTREAM_DELAY_SECONDS, TimeUnit.SECONDS)); + + StepVerifier.create(clientWithTimeout(SHORT_TIMEOUT_MS).getProductDetail("1000")) + .expectComplete() + // Bound well below the upstream delay: the timeout must be what ends the call. + .verify(Duration.ofSeconds(2)); + } + + @Test + void getProductDetailSkipsProductOnUnparseableBody() { + upstream.enqueue(json("not json")); + + StepVerifier.create(productClient.getProductDetail("2")) + .verifyComplete(); + } + + private static MockResponse json(String body) { + return new MockResponse() + .setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody(body); + } +} diff --git a/app/src/test/java/com/inditex/similarproducts/controller/SimilarProductsControllerTest.java b/app/src/test/java/com/inditex/similarproducts/controller/SimilarProductsControllerTest.java new file mode 100644 index 00000000..3359a7f0 --- /dev/null +++ b/app/src/test/java/com/inditex/similarproducts/controller/SimilarProductsControllerTest.java @@ -0,0 +1,92 @@ +package com.inditex.similarproducts.controller; + +import com.inditex.similarproducts.exception.ProductNotFoundException; +import com.inditex.similarproducts.model.ProductDetail; +import com.inditex.similarproducts.service.SimilarProductsService; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.test.web.reactive.server.WebTestClient; +import reactor.core.publisher.Flux; + +import static org.mockito.BDDMockito.given; + +/** + * Tests the HTTP contract of the endpoint in isolation from the upstream calls. + */ +@WebFluxTest(SimilarProductsController.class) +class SimilarProductsControllerTest { + + private static final ProductDetail PRODUCT_2 = new ProductDetail("2", "Dress", 19.99, true); + private static final ProductDetail PRODUCT_3 = new ProductDetail("3", "Blazer", 29.99, false); + + @Autowired + private WebTestClient webTestClient; + + @MockBean + private SimilarProductsService similarProductsService; + + @Test + void returnsTheSimilarProductsAsAJsonArray() { + given(similarProductsService.getSimilarProducts("1")) + .willReturn(Flux.just(PRODUCT_2, PRODUCT_3)); + + webTestClient.get().uri("/product/1/similar") + .exchange() + .expectStatus().isOk() + .expectHeader().contentType(MediaType.APPLICATION_JSON) + .expectBody() + .jsonPath("$.length()").isEqualTo(2) + .jsonPath("$[0].id").isEqualTo("2") + .jsonPath("$[0].name").isEqualTo("Dress") + .jsonPath("$[0].price").isEqualTo(19.99) + .jsonPath("$[0].availability").isEqualTo(true) + .jsonPath("$[1].id").isEqualTo("3") + .jsonPath("$[1].availability").isEqualTo(false); + } + + @Test + void returnsAnEmptyArrayWhenNoSimilarProductCouldBeResolved() { + // Every similar product timed out or errored — that is still a successful, empty answer. + given(similarProductsService.getSimilarProducts("3")).willReturn(Flux.empty()); + + webTestClient.get().uri("/product/3/similar") + .exchange() + .expectStatus().isOk() + .expectBody().json("[]"); + } + + @Test + void returns404WhenTheBaseProductDoesNotExist() { + given(similarProductsService.getSimilarProducts("999")) + .willReturn(Flux.error(new ProductNotFoundException("999"))); + + webTestClient.get().uri("/product/999/similar") + .exchange() + .expectStatus().isNotFound() + .expectBody().isEmpty(); + } + + @Test + void returns500WhenTheUpstreamFailsUnexpectedly() { + // Only a 404 from /similarids maps to 404; anything else is a genuine server-side failure. + given(similarProductsService.getSimilarProducts("1")) + .willReturn(Flux.error(new RuntimeException("Upstream error fetching similar IDs for product 1"))); + + webTestClient.get().uri("/product/1/similar") + .exchange() + .expectStatus().is5xxServerError(); + } + + @Test + void acceptsNonNumericProductIds() { + given(similarProductsService.getSimilarProducts("abc")).willReturn(Flux.just(PRODUCT_2)); + + webTestClient.get().uri("/product/abc/similar") + .exchange() + .expectStatus().isOk() + .expectBody().jsonPath("$[0].id").isEqualTo("2"); + } +} diff --git a/app/src/test/java/com/inditex/similarproducts/service/SimilarProductsServiceTest.java b/app/src/test/java/com/inditex/similarproducts/service/SimilarProductsServiceTest.java new file mode 100644 index 00000000..f43e24dc --- /dev/null +++ b/app/src/test/java/com/inditex/similarproducts/service/SimilarProductsServiceTest.java @@ -0,0 +1,140 @@ +package com.inditex.similarproducts.service; + +import com.inditex.similarproducts.client.ProductClient; +import com.inditex.similarproducts.exception.ProductNotFoundException; +import com.inditex.similarproducts.model.ProductDetail; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.time.Duration; +import java.util.List; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * Tests the aggregation contract: order follows similarity, details are fetched in + * parallel, and unresolvable products drop out instead of failing the request. + */ +@ExtendWith(MockitoExtension.class) +class SimilarProductsServiceTest { + + private static final ProductDetail PRODUCT_2 = new ProductDetail("2", "Dress", 19.99, true); + private static final ProductDetail PRODUCT_3 = new ProductDetail("3", "Blazer", 29.99, false); + private static final ProductDetail PRODUCT_4 = new ProductDetail("4", "Boots", 39.99, true); + + @Mock + private ProductClient productClient; + + @InjectMocks + private SimilarProductsService service; + + @Test + void returnsProductsInSimilarityOrder() { + given(productClient.getSimilarIds("1")).willReturn(Mono.just(List.of("2", "3", "4"))); + given(productClient.getProductDetail("2")).willReturn(Mono.just(PRODUCT_2)); + given(productClient.getProductDetail("3")).willReturn(Mono.just(PRODUCT_3)); + given(productClient.getProductDetail("4")).willReturn(Mono.just(PRODUCT_4)); + + StepVerifier.create(service.getSimilarProducts("1")) + .expectNext(PRODUCT_2, PRODUCT_3, PRODUCT_4) + .verifyComplete(); + } + + @Test + void keepsSimilarityOrderEvenWhenAnEarlierProductRespondsLast() { + // The upstream answers 4, then 3, then 2 — the response must still read 2, 3, 4. + given(productClient.getSimilarIds("1")).willReturn(Mono.just(List.of("2", "3", "4"))); + given(productClient.getProductDetail("2")) + .willAnswer(call -> Mono.just(PRODUCT_2).delayElement(Duration.ofMillis(300))); + given(productClient.getProductDetail("3")) + .willAnswer(call -> Mono.just(PRODUCT_3).delayElement(Duration.ofMillis(200))); + given(productClient.getProductDetail("4")) + .willAnswer(call -> Mono.just(PRODUCT_4).delayElement(Duration.ofMillis(100))); + + StepVerifier.create(service.getSimilarProducts("1")) + .expectNext(PRODUCT_2, PRODUCT_3, PRODUCT_4) + .verifyComplete(); + } + + @Test + void fetchesDetailsInParallelRatherThanOneAfterAnother() { + given(productClient.getSimilarIds("1")).willReturn(Mono.just(List.of("2", "3", "4"))); + // Stubs are built inside the answer so the delay binds to the virtual clock. + given(productClient.getProductDetail("2")) + .willAnswer(call -> Mono.just(PRODUCT_2).delayElement(Duration.ofSeconds(1))); + given(productClient.getProductDetail("3")) + .willAnswer(call -> Mono.just(PRODUCT_3).delayElement(Duration.ofSeconds(1))); + given(productClient.getProductDetail("4")) + .willAnswer(call -> Mono.just(PRODUCT_4).delayElement(Duration.ofSeconds(1))); + + // Three 1s calls complete after 1s in total. Sequential fetching would need 3s, + // and this only advances the clock by 1s — so it fails if the calls stop overlapping. + StepVerifier.withVirtualTime(() -> service.getSimilarProducts("1")) + .expectSubscription() + .thenAwait(Duration.ofSeconds(1)) + .expectNext(PRODUCT_2, PRODUCT_3, PRODUCT_4) + .expectComplete() + // Wall-clock bound: without it, sequential fetching leaves the verifier + // waiting on virtual time that never advances, and the test hangs. + .verify(Duration.ofSeconds(10)); + } + + @Test + void dropsProductsThatCouldNotBeResolved() { + // ProductClient turns 404s, 500s and timeouts into an empty Mono. + given(productClient.getSimilarIds("4")).willReturn(Mono.just(List.of("2", "3", "4"))); + given(productClient.getProductDetail("2")).willReturn(Mono.just(PRODUCT_2)); + given(productClient.getProductDetail("3")).willReturn(Mono.empty()); + given(productClient.getProductDetail("4")).willReturn(Mono.just(PRODUCT_4)); + + StepVerifier.create(service.getSimilarProducts("4")) + .expectNext(PRODUCT_2, PRODUCT_4) + .verifyComplete(); + } + + @Test + void returnsNothingWhenEveryProductIsUnresolvable() { + given(productClient.getSimilarIds("3")).willReturn(Mono.just(List.of("1000", "10000"))); + given(productClient.getProductDetail("1000")).willReturn(Mono.empty()); + given(productClient.getProductDetail("10000")).willReturn(Mono.empty()); + + StepVerifier.create(service.getSimilarProducts("3")) + .verifyComplete(); + } + + @Test + void requestsNoDetailWhenThereAreNoSimilarIds() { + given(productClient.getSimilarIds("1")).willReturn(Mono.just(List.of())); + + StepVerifier.create(service.getSimilarProducts("1")) + .verifyComplete(); + + verify(productClient, never()).getProductDetail(anyString()); + } + + @Test + void propagatesNotFoundFromTheSimilarIdsCall() { + given(productClient.getSimilarIds("999")) + .willReturn(Mono.error(new ProductNotFoundException("999"))); + + StepVerifier.create(service.getSimilarProducts("999")) + .verifyError(ProductNotFoundException.class); + } + + @Test + void propagatesUpstreamFailureFromTheSimilarIdsCall() { + given(productClient.getSimilarIds("1")) + .willReturn(Mono.error(new RuntimeException("Upstream error fetching similar IDs for product 1"))); + + StepVerifier.create(service.getSimilarProducts("1")) + .verifyError(RuntimeException.class); + } +} diff --git a/docker-compose.yaml b/docker-compose.yaml index 2b20a5d9..a2fcbaa7 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,4 +1,3 @@ -version: "3.3" services: influxdb: image: influxdb:1.8.2 @@ -23,6 +22,14 @@ services: volumes: - ./shared/simulado:/app command: ./bin/simulado -f /app/mocks.json + yourapp: + build: ./app + ports: + - "5000:5000" + environment: + - PRODUCT_API_BASE_URL=http://simulado + depends_on: + - simulado k6: image: loadimpact/k6:0.28.0 ports: