Skip to content

Idempotent order placement, a redesigned README overview and a cheaper pull request CI - #83

Merged
NullButCute merged 3 commits into
mainfrom
claude/confident-knuth-abb7sr
Oct 15, 2026
Merged

NullButCute merged 3 commits into
mainfrom
claude/confident-knuth-abb7sr

Conversation

@NullButCute

@NullButCute NullButCute commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

This PR has three independent commits: idempotent order placement, a redesign of the README's overview and architecture sections, and a cheaper CI pipeline for pull requests.

1. Idempotent order placement

Problem

A client that times out or loses the connection on POST /api/orders cannot know whether its order was placed. Repeating the request placed it a second time, and charged it twice. Nothing tied a retry to the original request.

What changes

POST /api/orders accepts an optional Idempotency-Key header. The client sends the same key again when it retries.

Situation Result
First request with a key Processed as before. The response is stored with the key.
The same request again The stored response is returned as it was, with Idempotency-Replayed: true. No second order, outbox event or payment. The catalog and stock are not checked again, so the retry succeeds even if the stock ran out or a price changed.
The same key with a different request 422.
Two copies in flight at once One order is placed and every copy gets it back.
A request that got 400 or 503 Does not use up the key. Repeat it once the problem is fixed.
No key Unchanged. Idempotency__RequireKey=true rejects requests without one once all clients send it (default false).

Keys are 1-128 characters (A-Z a-z 0-9 . _ ~ : -), scoped to the user, and remembered for 24 hours (Idempotency__RetentionHours). DELETE /api/orders/{id} needs no key: cancelling already returns the same result when repeated.

How it stays correct

  • Atomic. The order, its outbox event, its read-model row and the key are saved in one transaction. The read-model row used to be a second save after the order, so a crash in between left an order without a summary; that gap is closed too.
  • No locks, no "in progress" state. A unique index on (UserId, Key) decides between copies running at the same moment. Both see a new key, one commits, and the other gets a unique-key violation. It rolls back completely and answers from the winner. A request that crashes midway cannot leave a key stuck, and nothing from a losing copy is left behind.
  • Fingerprint. A SHA-256 of the validated request (products, quantities, trimmed note). JSON formatting, line order and the ignored productName/unitPrice fields don't make a retry look different.
  • Retention. A background job deletes expired keys with one set-based DELETE every 15 minutes, safe on every replica.
  • Observability. shopeasy_orders_idempotent_replays_total{result} counts replays and rejected key reuse.
  • Gateway. YARP forwards the request header and the Idempotency-Replayed response header unchanged.

New table IdempotencyRecords (migration AddIdempotencyKeys); nothing existing is touched. Settings are in the Order appsettings.json, docker-compose.yml / .env.example, and the shopeasy-config ConfigMap. The controller now depends on a small IInventoryStock interface (implemented by the existing InventoryCacheService) so the real action can be tested with a fake stock.

Tests (324 in total: 271 without servers, 53 with PostgreSQL and RabbitMQ)

  • Header parsing and the request fingerprint.
  • The real PlaceOrder action on an in-memory database: a repeat returns the original order and body, the same key with a different request gets 422, keys are scoped per user, a 400 or 503 does not use up the key, a required key is enforced, and the order, outbox event, read-model row and key are all saved together.
  • Against PostgreSQL: eight copies of one request in flight at once place exactly one order (one outbox event, one summary, one key left over); concurrent requests with different keys, or with no key, each place their own order; concurrent reuse of one key for different orders lets one win and rejects the rest; a retry on another instance replays from the database; the clean-up removes only expired keys.
  • The concurrency tests fail when the duplicate-key handling is removed. I checked that.
  • Drift tests keep the appsettings.json, Compose, .env.example and ConfigMap copies of the settings equal to the code's defaults.

Also verified locally: the Order service starts, applies the migration and serves requests against PostgreSQL and RabbitMQ; docker compose config renders the settings (and an override); the Kubernetes manifests pass kubeconform -strict and the repo's deployment rules.

Not in this change

The Order saga's command publishes still happen after the database commit. A crash in that gap is recovered by the saga timeout service, as documented. Moving them onto a transactional outbox touches every saga consumer and is better reviewed on its own.

2. README overview and architecture redesign (documentation only)

The README's opening was a short description and an ASCII diagram that no longer matched the system. It is replaced by a header with badges and quick links, an Overview (what the platform is for, the guarantees it is built around, what it deliberately is not), an Architecture section with three diagrams GitHub renders (system overview, life of an order, saga state machine) plus tables for communication, data ownership, cross-cutting qualities and deployment, and The Services and Technology Stack as tables.

Checked: all three diagrams render from the README itself, all 62 internal links resolve, and the claims were verified against the code and configuration. Two inaccuracies were fixed on the way (Order has no fixed port in Compose; the read-model row is written in the order's transaction).

3. Cheaper CI for pull requests

  • Six per-service "Docker build" jobs removed from pull requests. The "Start the Compose stack" job already builds the same six images and then runs them, which is the stronger check, so the six jobs only repeated it: about 5 GB of downloads and six runners per run. The images are still built from their Dockerfiles on every pull request (in the Compose job) and built and published from main. They can be added back later if wanted.
  • Documentation-only pull requests skip the heavy jobs. A new "Detect changes" job (a plain git diff, no third-party action) decides whether anything other than *.md, docs/ or the licence changed. Skipped jobs count as passed in CI passed, which only fails for failed or cancelled jobs, so it keeps working as the required check. Runs on main, tags and manual dispatch always run everything. The logic was tested against typical change sets.
  • Hang protection. The test step now aborts a hung test after 5 minutes and names it (--blame-hang-timeout 5m). One earlier run on this branch sat in "Build and test" until the 25-minute limit and was cancelled without saying which test stalled.

Effect on a typical code PR: about 3-4 GB and 6 runner-minutes instead of 8-10 GB and 12. Wall-clock time stays about the same because the tests and the Compose job were already the longest jobs. A docs-only PR finishes in seconds. If branch protection requires any check other than CI passed, the removed Docker build (...) checks need to be taken out of it.

This push is also the first run of the new workflow: GitHub runs the workflow file from the PR itself.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vuhzoey6h5V9wPPVFuR7ME

A client that times out or loses the connection on POST /api/orders cannot
know whether its order was placed. Repeating the request placed it a second
time (and charged it twice): nothing tied a retry to the original request.

POST /api/orders now accepts an optional Idempotency-Key header (1-128
characters: letters, digits and . _ ~ : -; a UUID is the usual choice). The
client sends the same key again when it retries.

* First request with a key: processed as before; the response (201 and body)
  is stored with the key and a fingerprint of the request.
* The same request again: the stored response is returned as it was, with an
  Idempotency-Replayed: true header and the Location of the order. No second
  order, outbox event or payment, and the catalog and stock are not consulted
  again, so a retry still succeeds if the stock ran out or a price changed.
* The same key with a different request: 422. The fingerprint is a SHA-256 of
  the validated request (products, quantities, trimmed note), so JSON
  formatting, the order or splitting of lines and the ignored productName and
  unitPrice fields do not make a retry look different.
* Keys are scoped to the user. Only a request that actually placed an order
  uses up its key: a 400 or a 503 leaves nothing behind and can be repeated.
* No key: behaviour is unchanged. Idempotency:RequireKey=true rejects requests
  without one once every client sends it (default false).

Concurrency without locks or "in progress" states: the order, its outbox event,
its read-model row and the key are saved in ONE transaction, and a unique index
on (UserId, Key) decides between copies running at the same moment. Both see a
new key, one commits, the other gets a unique-key violation (recognised by
SQLSTATE 23505 on that index), rolls back completely and answers from the
winner. A request that crashes midway cannot leave a key stuck, and no order,
outbox event or summary of a losing copy is left behind. As part of this the
read-model row (OrderReadModelUpdater.Stage) is now written in the same
transaction as the order instead of in a second save after it, so an order can
no longer exist without its summary.

Keys are remembered for Idempotency:RetentionHours (default 24); a background
service deletes older ones with a single set-based DELETE every 15 minutes,
which is safe on every replica. New table IdempotencyRecords (migration
AddIdempotencyKeys, nothing existing is touched). The metric
shopeasy_orders_idempotent_replays_total{result} counts replays and rejected
key reuse. DELETE /api/orders/{id} needs no key: cancelling already returns the
same result when repeated.

Settings are written out like the others: Order appsettings.json, the Order
service's environment in docker-compose.yml (ORDER_IDEMPOTENCY_* in .env, shown
commented in .env.example) and the shopeasy-config ConfigMap. The controller
now depends on a small IInventoryStock interface (implemented by the existing
InventoryCacheService) so the real action can be tested with a fake stock.

Tests (324 in total, 271 without servers, 53 with PostgreSQL and RabbitMQ):
header parsing and the request fingerprint; the real PlaceOrder action on an
in-memory database (repeat returns the original, same key different request is
422, per user, 400/503 do not use the key, required key, everything saved
together); against PostgreSQL, eight copies of one request in flight at once
place exactly one order with nothing left over (verified to fail when the
duplicate-key handling is removed), concurrent requests with different keys
or without a key each place their own, concurrent reuse of one key for
different orders lets one win and rejects the rest, and the retention clean-up
removes only expired keys. Drift tests keep the appsettings, Compose,
.env.example and ConfigMap copies of the settings equal to the code's defaults.
The README documents the header (Order API section), the mechanism, settings
and tests.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vuhzoey6h5V9wPPVFuR7ME
The opening of the README was a one-paragraph description and an ASCII box
diagram that no longer matched the system (it showed the Order service on a
fixed port, no messaging flow, and nothing about how an order actually moves
through the services). It is replaced by a structure a reader can scan:

* Header with the project name, a one-sentence description, CI and technology
  badges and quick links.
* Overview: what the platform is for, the guarantees it is built around
  (correct order processing, safe under retries and duplicates, no overselling,
  resilience, security, observability, deployability) and what it deliberately
  is not (no storefront, simulated payments).
* Architecture, with three diagrams that GitHub renders (all checked to render
  from the README itself): the system overview (edge, services, shared
  infrastructure, observability), the life of an order from the request to the
  confirmed order, and the saga state machine including its compensation
  branches. Alongside them: how the services talk to each other and why, where
  each service's data lives, how the cross-cutting qualities are achieved (with
  links to the pattern that implements each), and Compose versus Kubernetes.
* The Services and Technology Stack sections become tables; the service table
  now states what each service owns and its interfaces, and gives the right
  ports (Order is scaled behind the gateway and has no fixed port; Inventory's
  gRPC port is internal).

Also corrected while checking the text against the code: the Swagger URLs row
no longer claims Order is on port 5102, the read-model description says the row
is written in the same transaction as the order, and a sentence about the
Kubernetes data stores is worded as a plain limitation. All 62 internal links
in the README resolve. No code or configuration changes.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vuhzoey6h5V9wPPVFuR7ME
@NullButCute NullButCute changed the title Make order placement idempotent with an Idempotency-Key header Idempotent order placement (Idempotency-Key) and a redesigned README overview Oct 15, 2026
…only changes

Every pull request ran six separate "Docker build" jobs and a "Start the
Compose stack" job. The Compose job already builds the same six images (and then
starts them, which is the stronger check), so the six jobs only repeated that
work: about 5 GB of base-image and NuGet downloads and six extra runners per run
for no additional safety. They are removed; the images are still built from
their Dockerfiles in the Compose job on every pull request, and built and
published from main.

A change that only touches documentation (*.md, docs/, the licence) used to run
the whole pipeline as well. A new "Detect changes" job (a plain git diff, no
third-party action) now decides whether anything that is built, tested or
deployed changed. Documentation-only pull requests skip the build, the tests and
the container jobs; everything else, and every run on main, a tag or a manual
dispatch, runs the full pipeline. "CI passed" counts skipped jobs as passed (it
only fails for failed or cancelled ones), so it keeps working as the single
required status check. The detection logic was checked against typical change
sets (README only, README plus code, docs plus licence, workflow or Compose
changes, empty diff).

The test step also gets --blame-hang-timeout 5m: a test that hangs is aborted
and named after five minutes instead of holding the job until its 25-minute
limit (which is what happened on one run, where the job was cancelled without
saying which test stalled).

Effect on a typical code pull request: roughly 3-4 GB of downloads and 6
runner-minutes instead of 8-10 GB and 12; the wall-clock time stays about the
same because the tests and the Compose job were already the longest jobs. A
documentation-only pull request finishes in seconds. The README's CI/CD section
describes the new layout. If branch protection requires any check other than
"CI passed", the removed "Docker build (...)" checks must be taken out of it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vuhzoey6h5V9wPPVFuR7ME
@NullButCute NullButCute changed the title Idempotent order placement (Idempotency-Key) and a redesigned README overview Idempotent order placement, a redesigned README overview and a cheaper pull request CI Oct 15, 2026
@NullButCute
NullButCute merged commit 6cd9f37 into main Oct 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants