diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ef788d2..132cb26e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,6 +114,14 @@ jobs: - name: bazel test (out-of-tree module) working-directory: examples/bazel-consumer run: bazelisk test //... --verbose_failures + # The day-2 model-evolution loop (docs/model-evolution.md): an additive + # member rebuilds handlers and tests untouched; a new operation fails + # compilation until handlers implement it. The properties are + # platform-independent, so one OS suffices. + - name: model evolution loop + if: matrix.os == 'ubuntu-24.04' + working-directory: examples/bazel-consumer + run: ./model-evolution-check.sh codegen: name: codegen (gradle) diff --git a/README.md b/README.md index d4c7dafc..7a33203c 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ See [`docs/PLAN.md`](docs/PLAN.md) for the full phased plan and | 3 | Client generation (simpleRestJson + rpcv2Cbor) | ✅ Done — serde + clients for both protocols with typed errors ([docs/generated-types.md](docs/generated-types.md)); official conformance suites green (documented exclusions) | | 4 | Server generation (simpleRestJson + rpcv2Cbor) | ✅ Done — handlers, routing, serde, all HTTP bindings incl. `@httpPayload`/`@httpPrefixHeaders`, constraint validation, parser strictness, content negotiation; official conformance suites green ([docs/server-guide.md](docs/server-guide.md)) | | 5 | Generated-client ↔ generated-server integration harness | ✅ Done — every fixture ships a generated integration suite: seeded random round-trips over loopback and real sockets, per-error mapping, unknown-member tolerance, mutation-checked ([docs/design/integration-testing.md](docs/design/integration-testing.md)) | -| 6 | Bazel rules, CLI, packaging (BCR + Maven Central), docs site | 🔨 In progress — `smithy_cpp_{types,client,server}_library` rules run the generator hermetically inside the build graph, out-of-tree consumer module tested in CI, CLI via `bazel run //codegen:generator` ([docs/quickstart.md](docs/quickstart.md)); BCR/Maven publishing deferred until production validation; docs site pending | +| 6 | Bazel rules, CLI, packaging (BCR + Maven Central), docs site | 🔨 In progress — `smithy_cpp_{types,client,server}_library` rules run the generator hermetically inside the build graph, out-of-tree consumer module tested in CI, CLI via `bazel run //codegen:generator` ([docs/quickstart.md](docs/quickstart.md)); model-evolution loop documented and exercised in CI ([docs/model-evolution.md](docs/model-evolution.md)); BCR/Maven publishing deferred until production validation; docs site pending | | 7 | Hardening, fuzzing, v0.1.0 | ✅ Done — retries with full-jitter exponential backoff, gzip `@requestCompression` (client + server), client interceptors + server middleware (auth/logging/metrics seams), `@httpBearerAuth`/`@httpApiKeyAuth` credential wiring, generated `@paginated` paginators, Beast graceful drain + header limits, consumer CI across linux/macos/windows ([docs/production-guide.md](docs/production-guide.md)); **BeastHttpClient** production client transport with TLS both directions (ADR-0007); Google Benchmark suite (informational CI job); release engineering — [CHANGELOG](CHANGELOG.md), [versioning/compatibility policy](docs/versioning.md), **v0.1.0**; **protocol realignment to vendor-neutral** — dropped AWS `restJson1` for `alloy#simpleRestJson` + its conformance suite, Smithy 1.58, no `aws.*` on the generator classpath; **`smithy.cpp.protocols#jsonRpc2`** — JSON-RPC 2.0 over a single POST endpoint with an authored conformance suite, a calculator example with hand-rolled-peer interop tests, and a consumer overlay | | 8 | Bidirectional streaming (event streams, WebSockets) | Not started | diff --git a/docs/PLAN.md b/docs/PLAN.md index f9c3725a..2daf434b 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -512,6 +512,10 @@ without reading generator internals or touching Gradle. regenerate vendored CLI output safely; keep hand-written handlers and tests working across regenerations. Exercise the flow in CI (scripted evolve-and-rebuild against the consumer example). + ✅ Done — docs/model-evolution.md (in-graph and vendored loops, diff review, CI drift + detection, worked example, compatibility cheat sheet); + `examples/bazel-consumer/model-evolution-check.sh` scripts the evolve-and-rebuild loop and + runs in the consumer CI job. **Docs:** the docs site is the deliverable. diff --git a/docs/development.md b/docs/development.md index 8dcf9ca2..baf31d9c 100644 --- a/docs/development.md +++ b/docs/development.md @@ -59,6 +59,10 @@ cd codegen && gradle spotlessApply dependencies whose BUILD files predate Bazel 9's removal of the native C++ rules. - Fixture Smithy models live under `examples//model/`; they double as test fixtures for codegen and the client↔server integration harness (PLAN Phases 2–5). +- Generated output under `examples/*/generated/` and `protocol-tests/*/generated/` is checked + in as goldens: regenerate with `cd codegen && gradle generateFixtures generateProtocolTests` + and commit model + output together — CI fails on drift. The full loop (including the + consumer-side story) is in [model-evolution.md](model-evolution.md). - Machine-specific Bazel flags go in `.bazelrc.user` (gitignored), e.g. a `--downloader_config` when working behind a proxy that blocks GitHub downloads. - The Boost-dependent targets (`//runtime:http_beast` and the tests that use it) fetch ~30 diff --git a/docs/model-evolution.md b/docs/model-evolution.md new file mode 100644 index 00000000..2cf550da --- /dev/null +++ b/docs/model-evolution.md @@ -0,0 +1,173 @@ +# Model evolution: the day-2 loop + +[quickstart.md](quickstart.md) gets a model generating a client and server; this guide covers +everything after that — the loop you run every time the model changes. The shape of the loop +depends on where generated code lives, and there are exactly two answers: + +| Mode | Generated code lives | Drift possible? | The loop | +|---|---|---|---| +| **In-graph** (the Bazel rules, recommended) | inside the build graph, never on disk | No — every build regenerates | edit model → `bazel test //...` → fix what the compiler names | +| **Vendored** (CLI output checked into git; also this repo's own `examples/*/generated` and `protocol-tests/`) | in the repository | Yes — model and output can diverge | edit model → regenerate → review the diff → commit both together | + +## The in-graph loop + +With `smithy_cpp_{types,client,server}_library` ([quickstart.md](quickstart.md) §3), the model +is the only source of truth: generation is a hermetic action, so an edited `.smithy` file +invalidates exactly the targets that depend on it. There is nothing to regenerate by hand and +nothing to drift. The compiler is the review: + +- **Adding an optional member** is source-compatible. Existing handlers, call sites, and tests + rebuild unchanged; the new member shows up as a `std::optional` (or `@default`-initialized) + field on the generated struct. +- **Adding an operation** grows the generated `Handler` interface by one pure-virtual + method, so every handler implementation stops compiling until it implements the new + operation. The unimplemented surface is found by the compiler, not by a 404 in production. +- **Renaming or removing** anything shows up the same way: every call site that mentioned the + old name fails to compile. Rename-refactors are grep-follow-the-errors sessions, not + archaeology. + +Both properties are pinned in CI: +[`examples/bazel-consumer/model-evolution-check.sh`](../examples/bazel-consumer/model-evolution-check.sh) +scripts the loop against the out-of-tree consumer module — stage 1 adds an optional member and +requires `bazel test //...` to stay green untouched; stage 2 adds a `ListTasks` operation and +requires the integration test to *fail compilation* naming the new handler method. The +consumer CI job runs it on every commit, so the promises in this section cannot silently rot. + +## The vendored loop (regenerate → diff → review → commit) + +Vendoring generated sources (the CLI escape hatch in [quickstart.md](quickstart.md) +"Generating outside Bazel") trades the in-graph guarantees for a plain source tree, and takes +on one obligation: **the model and its generated output must change in the same commit.** This +repo's own fixtures live this way — `examples/*/generated/` and `protocol-tests/*/generated/` +are checked in as goldens — so the workflow below is exercised on every commit here. + +1. **Edit the model** (`examples//model/*.smithy`, or your own). +2. **Regenerate.** In this repo, one command per fixture or all at once: + + ```sh + cd codegen + gradle generateWeatherFixture # one fixture… + gradle generateFixtures generateProtocolTests # …or everything checked in + ``` + + Out of tree, the same generator runs as a CLI: + + ```sh + bazel run @smithy_cpp//codegen:generator -- \ + --model $PWD/model/todo.smithy --service acme.todo#Todo \ + --namespace acme::todo --mode both --output $PWD/generated + ``` + + The Gradle tasks delete the output directory before regenerating, so *removals* show up as + deleted lines in the diff rather than orphaned files; do the same for vendored output + (`rm -rf generated/ && bazel run …`). +3. **Review the diff as a normal diff.** Generation is byte-deterministic (enforced by + `CppCodegenPluginTest.outputIsByteDeterministic`), so the diff is exactly the blast radius + of the model change — no noise, no timestamps. What to look for: + - `include/**/types.h` — the API surface consumers compile against. New members and + operations should look additive; anything *removed* here is a breaking change to your + consumers ([versioning.md](versioning.md) §"What counts as the compatibility surface"). + - `src/serde.cc`, `src/client.cc`, `src/server.cc` — the wire behavior. Internals may + reshuffle freely between generator versions (explicitly not a compatibility surface), but + for a pure model edit with a pinned generator the changes should mention only the shapes + you touched. + - `tests/` — regenerated integration/smoke suites pick up new members automatically. +4. **Build and test**, then **commit the model and generated output together.** + +## Drift detection in CI + +A vendored tree needs CI to fail fast when someone edits a model without regenerating (or +edits generated files by hand). The pattern is: regenerate, then require a clean tree. This +repo's `codegen` job does precisely that for the checked-in goldens: + +```yaml +- name: check generated fixtures are current + run: | + (cd codegen && gradle generateFixtures generateProtocolTests) + git diff --exit-code -- examples protocol-tests +``` + +A consumer vendoring CLI output wants the same job, substituting the CLI invocation: + +```yaml +- name: check generated code is current + run: | + rm -rf generated + bazel run @smithy_cpp//codegen:generator -- \ + --model $PWD/model/todo.smithy --service acme.todo#Todo \ + --namespace acme::todo --mode both --output $PWD/generated + git diff --exit-code -- generated +``` + +Two conventions that keep the check honest: + +- **Pin the generator.** Byte-identical regeneration is only guaranteed for a pinned generator + version (`git_override` commit / release tag); across generator upgrades the conformance + suites are the contract instead ([versioning.md](versioning.md) §"What a generator upgrade + may change"). Upgrading the pin regenerates everything in one reviewable commit. +- **Exempt generated code from format checks.** Its shape is locked by the drift check; this + repo's lint job skips `*/generated/*` for clang-format/clang-tidy rather than fighting the + generator. + +## Worked example: adding a field to the weather fixture + +The smallest real evolution, run against this repo. Add `chanceOfSnow` to `GetForecast`'s +output in `examples/weather/model/weather.smithy`: + +```smithy + output := { + chanceOfRain: Float + + chanceOfSnow: Float + } +``` + +Regenerate that one fixture and look at the blast radius: + +```sh +(cd codegen && gradle generateWeatherFixture) +git diff --stat -- examples/weather +# .../generated/include/example/weather/types.h | 1 + +# .../generated/src/serde.cc | 15 +++++++++++++++ +# .../generated/src/server.cc | 3 +++ +# .../generated/tests/integration_test.cc | 1 + +# .../model/weather.smithy | 2 ++ +``` + +22 added lines, nothing removed. The interesting hunk is the API surface: + +```diff + struct GetForecastOutput { + std::optional chanceOfRain{}; ++ std::optional chanceOfSnow{}; + + friend bool operator==(const GetForecastOutput&, const GetForecastOutput&) = default; + }; +``` + +plus the matching serialize/deserialize clauses in `serde.cc`/`server.cc` and one line in the +regenerated random-round-trip integration test. `bazel test //examples/weather/...` passes +with every hand-written test untouched — additive members are invisible to code that does not +use them. Commit `weather.smithy` and `examples/weather/generated/` together and the drift +check stays green. + +## Which model changes are safe? + +Wire compatibility (old peers keep working) and source compatibility (existing C++ keeps +compiling) are separate questions: + +| Model change | Wire | Source (consumers of generated headers) | +|---|---|---| +| Add optional member | ✅ old readers ignore it; absent → unset `std::optional` | ✅ additive | +| Add member with `@default` | ✅ absent → the default | ✅ additive (plain member, default-initialized) | +| Add operation | ✅ old clients never call it | ⚠️ handlers must implement the new pure-virtual method (compile error guides) | +| Add enum value | ✅ old readers preserve unknown values (`Value::kUnknown` + original text) | ✅ additive | +| Promote optional → `@required` + `@default` | ✅ absence on the wire keeps the default (the generator's evolution leniency) | ✅ member becomes plain (non-optional) — call sites reading `.has_value()` need updating | +| Promote optional → `@required` (no default) | ❌ old writers that omit it now fail deserialization | ❌ member type changes | +| Rename member | ❌ wire key changes — unless the old wire name is kept via `@jsonName` | ❌ compile errors at every use | +| Remove member / operation | ❌ breaking | ❌ breaking | +| Tighten a constraint (`@length`, `@pattern`, `@range`) | ⚠️ previously-valid requests now rejected with 400 `ValidationException` before the handler runs | ✅ no header change | + +For anything in the ❌ rows, [versioning.md](versioning.md) applies: it is a breaking change to +your service's consumers, and (for changes to this repo's generator itself) must be called out +in the CHANGELOG under a "Breaking" heading with a migration note. diff --git a/docs/quickstart.md b/docs/quickstart.md index 12f5bd66..81941702 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -132,3 +132,10 @@ bazel run @smithy_cpp//codegen:generator -- \ `--mode types|client|server|both` picks what to emit; `--emit-build-file false` suppresses the generated `BUILD.bazel` when you're writing your own. + +## Day 2: evolving the model + +Once the integration is running, the model keeps changing — new fields, new operations, +tightened constraints. [model-evolution.md](model-evolution.md) covers that loop: how edits +propagate through the build graph (or through regeneration for vendored output), how to review +generated diffs, and how CI catches drift and unimplemented handler methods. diff --git a/examples/bazel-consumer/model-evolution-check.sh b/examples/bazel-consumer/model-evolution-check.sh new file mode 100755 index 00000000..6912f8cd --- /dev/null +++ b/examples/bazel-consumer/model-evolution-check.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# Scripted model-evolution loop (docs/model-evolution.md): evolve the todo +# model the way a consumer would on day 2 and verify the two properties the +# doc promises. Run from anywhere; it operates on this directory and restores +# the model when it exits. CI runs it after the plain consumer test. +# +# 1. Additive change — a new optional member is source-compatible: the +# existing handler and integration test rebuild and pass untouched. +# 2. New operation — the generated handler interface grows a pure-virtual +# method, so every handler implementation fails to compile until it is +# updated: the compiler, not the wire, finds the unimplemented surface. +set -euo pipefail +cd "$(dirname "$0")" + +bazel="${BAZEL:-$(command -v bazelisk || command -v bazel)}" +model=model/todo.smithy + +restore() { git checkout -- "$model"; } +trap restore EXIT + +if ! git diff --quiet -- "$model"; then + echo "error: $model has local modifications; commit or stash them first" >&2 + exit 1 +fi + +echo "== stage 1: additive member (source-compatible) ==" +sed -i.bak 's/^ done: Boolean$/ done: Boolean\n\n notes: String/' "$model" +rm -f "$model.bak" +grep -q 'notes: String' "$model" || { echo "error: stage-1 edit did not apply" >&2; exit 1; } +"$bazel" test //... --verbose_failures + +echo "== stage 2: new operation (compile error until handlers implement it) ==" +sed -i.bak 's/ operations: \[AddTask, GetTask\]/ operations: [AddTask, GetTask, ListTasks]/' "$model" +rm -f "$model.bak" +grep -q 'ListTasks\]' "$model" || { echo "error: stage-2 edit did not apply" >&2; exit 1; } +cat >> "$model" <<'EOF' + +@readonly +@http(method: "GET", uri: "/tasks") +operation ListTasks { + output := { + @required + taskIds: TaskIdList + } +} + +list TaskIdList { + member: String +} +EOF + +if out=$("$bazel" build //:todo_integration_test 2>&1); then + echo "error: expected the handler implementations to fail compilation" >&2 + exit 1 +fi +if ! grep -q "ListTasks" <<< "$out"; then + echo "error: build failed, but not on the new ListTasks handler method:" >&2 + echo "$out" >&2 + exit 1 +fi + +echo "OK: additive member rebuilt cleanly; new operation caught at compile time"