Skip to content

Commit fbc13d8

Browse files
dmealingclaude
andcommitted
fix(api-contract): settle the empty-PATCH contract at 200, and gate it
FR-037 PATCH-5 has always said a patch stripping to zero caller assignments reads the current row back. Nothing in the corpus asserted it, so the ports drifted to five answers for one input: TS 500 on the generated mount and 200 on the reference lane (fixed last commit), C# and Java 400 "validation" from their base AuthorApiServer while TphReferenceServer and JsonbReferenceServer in the same directory 200'd, Kotlin 200, Python 200. Both 400s cited "per FR-035". FR-035's own spec says the opposite. The rule was settled the whole time; what was missing was a fixture, and an unasserted corner is where divergence lives — this is the second contract this week found drifting behind a comment that called it deliberately unsettled. The C# and Java fix is the REMOVAL of an empty-SET guard. Both already appended the @autoset:onUpdate stamp unconditionally on the next line, so dropping the guard makes the SET non-empty on its own and the ordinary write path answers correctly. Nothing else moves. That also settles how PATCH-3 and PATCH-5 meet, which the fixture forced into the open: the server-side stamp IS an assignment, so on an entity carrying an onUpdate column the literal zero-assignment read-back path is never reached, and an empty patch bumps the stamp like any other write. The read-back path is reachable only for an entity with no such column. Both routes answer 200 with the current row, which is why the contract is stable either way. PATCH-5's wording is corrected: it read "no write is executed", true only in the second case. patch-empty-noop.yaml asserts the bump via fieldsNotEqual, the exact inverse of autoset-patch.yaml. r2 pins the arm a short-circuit misses: a no-op patch must still 404 on a row that does not exist, or the fast path becomes a hole reporting success for any id — on a TPH route, a hole through which one subtype answers for another's row. Verified in all five ports, both lanes each: TS 58, Python 89, C# 101, Java 58, Kotlin 58, all green. Also corrects a stale count in docs/CONFORMANCE.md: the m2m sub-corpus prose said 8 while the directory held 9 (route-spelling-multiword-collection.yaml landed in 5efdb9e and only the table was updated). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NrWUxA6euwvnFcwKxcK6A
1 parent 6cb176e commit fbc13d8

8 files changed

Lines changed: 123 additions & 28 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,29 @@ edit (two registered `description` strings) and was ruled a hold, as 1.0.4's was
126126
ObjectManager, already no-opped and returned the row, so the two api-contract lanes
127127
disagreed on this input.
128128

129-
**The cross-port contract for an empty PATCH remains deliberately unsettled** and this does
130-
not settle it — `fixtures/api-contract-conformance/scenarios/writeonce-patch-stripped.yaml`
131-
records that the C# reference 400s on an empty SET where TypeScript no-ops. That is a
132-
ruling to make, not a fix to sneak in under one port, so no conformance fixture pins it
133-
here. What is fixed is only this: 500 was defensible under no reading of it.
129+
This fix deliberately did not settle the cross-port contract; the entry below now does.
130+
131+
- **An empty PATCH answered five different ways across the ports; it is now one, and gated.**
132+
FR-037 PATCH-5 has always said a patch that strips to zero caller assignments reads the
133+
current row back — but nothing in the corpus asserted it, and an unasserted corner is where
134+
divergence lives. The ports had drifted to: TypeScript 500 on the generated mount and 200 on
135+
the reference lane (fixed above), C# and Java 400 `validation` from their base
136+
`AuthorApiServer` while `TphReferenceServer`/`JsonbReferenceServer` in the same directory
137+
200'd, Kotlin 200, Python 200. Both 400s cited "per FR-035" — a rule that says the opposite.
138+
The answer is now **200 with the current row** in all five ports and both lanes, `404` when
139+
the row does not exist, pinned by the new
140+
`fixtures/api-contract-conformance/scenarios/patch-empty-noop.yaml`.
141+
142+
The fix in C# and Java is the *removal* of an empty-`SET` guard: the `@autoSet:onUpdate`
143+
stamp was already being appended unconditionally right after it, so dropping the guard makes
144+
the `SET` non-empty on its own and the ordinary write path answers correctly. That also
145+
settles how PATCH-3 and PATCH-5 meet — the server-side stamp IS an assignment, so on an
146+
entity carrying one the literal zero-assignment read-back path is never reached, and
147+
`patch-empty-noop.yaml` asserts the stamp still bumps. `docs/features/generated-mutations.md`
148+
PATCH-5 is corrected accordingly; it previously read "no write is executed", which holds only
149+
for an entity with no onUpdate column.
150+
151+
No adopter action: a request that used to 400 or 500 now succeeds.
134152

135153
- **Java: a field name that is legal metadata could not be a record component.** The JLS
136154
forbids a record component named after a no-argument `Object` method

‎docs/CONFORMANCE.md‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ regenerate with `ls -d fixtures/<corpus>/*/ | wc -l` for directory-shaped corpor
3434
| [`fixtures/extract-conformance/`](../fixtures/extract-conformance/) | 33 | ✓ | ✓ | inherits the shared JVM engine | ✓ | ✓ |
3535
| [`fixtures/output-prompt-conformance/`](../fixtures/output-prompt-conformance/) | 14 | ✓ | ✓ | ✓ | ✓ | ✓ |
3636
| [`fixtures/persistence-conformance/`](../fixtures/persistence-conformance/) | 33 (27 query + 6 migration) | all 33 | 27 query (migrations TS-only, ADR-0015) | 27 query (via Exposed) | 27 query | 27 query |
37-
| [`fixtures/api-contract-conformance/`](../fixtures/api-contract-conformance/) | 50 (28 core + 9 tph + 9 m2m + 2 jsonb + 2 write-through) | ✓ (Fastify reference + generated lane) | ✓ (embedded HTTP + JDBC) | ✓ (embedded HTTP + Exposed) | ✓ (HttpListener + Npgsql) | ✓ (FastAPI + pg8000) |
37+
| [`fixtures/api-contract-conformance/`](../fixtures/api-contract-conformance/) | 51 (29 core + 9 tph + 9 m2m + 2 jsonb + 2 write-through) | ✓ (Fastify reference + generated lane) | ✓ (embedded HTTP + JDBC) | ✓ (embedded HTTP + Exposed) | ✓ (HttpListener + Npgsql) | ✓ (FastAPI + pg8000) |
3838
| [`fixtures/validation-conformance/`](../fixtures/validation-conformance/) | 16 cases | ✓ | ✓ | ✓ | ✓ | ✓ |
3939
| [`fixtures/registry-conformance/`](../fixtures/registry-conformance/) | 1 canonical manifest | ✓ (reference emitter) | ✓ | ✓ | ✓ | ✓ |
4040
| [`fixtures/object-model-conformance/`](../fixtures/object-model-conformance/) | 1 shared metadata fixture (per-port scenarios) | ✓ | ✓ | ✓ | ✓ | ✓ |
@@ -226,9 +226,9 @@ All 31 fixtures → [features/migrations-and-drift.md](features/migrations-and-d
226226
- `migrations/*` (6) → [features/migrations-and-drift.md](features/migrations-and-drift.md) (schema migration section)
227227
- `queries/*` (27) → [features/source-kinds.md](features/source-kinds.md) (query semantics against `source.rdb`)
228228

229-
### `fixtures/api-contract-conformance/` (50)
229+
### `fixtures/api-contract-conformance/` (51)
230230

231-
All 50 scenarios → [features/api-contract.md](features/api-contract.md) (cross-port
231+
All 51 scenarios → [features/api-contract.md](features/api-contract.md) (cross-port
232232
REST API URL grammar + JSON wire format). Verifies every backend's emitted CRUD
233233
routes answer identically over HTTP — list / get / create / patch+put / delete,
234234
plus pagination (`limit`/`offset`), sort (`sort=field:dir`), the `withCount=1`
@@ -238,10 +238,11 @@ status codes.
238238
The corpus also covers the 9 cross-port filter operators (`eq`, `ne`, `gt`,
239239
`gte`, `lt`, `lte`, `in`, `like`, `isNull`) plus the implicit-AND combinator
240240
and 2 error shapes (`invalid_filter_field` / `invalid_filter_op`) under the
241-
URL grammar `?filter[<field>][<op>]=<value>` (FR-009). On top of the 28 core
241+
URL grammar `?filter[<field>][<op>]=<value>` (FR-009). On top of the 29 core
242242
scenarios the corpus carries four sub-corpora — `tph/` (9, single-table
243-
inheritance), `m2m/` (8 — 3 plain plus 5 gating TPH x M:N together, the
244-
combination each corpus alone could not reach), `jsonb/` (2, typed value-object columns) and
243+
inheritance), `m2m/` (9 — 3 plain, 5 gating TPH x M:N together, the combination
244+
each corpus alone could not reach, and 1 pinning the collection-URL spelling),
245+
`jsonb/` (2, typed value-object columns) and
245246
`write-through/` (2, table-write + view-read entities). All 5 ports — TS, Java,
246247
Kotlin, C#, Python — run it in BOTH lanes: a hand-rolled reference server and
247248
the port's own GENERATED API artifact booted over HTTP.

‎docs/features/generated-mutations.md‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,15 @@ but the semantics are one contract — call them **PATCH-1..7** (FR-035):
4949
entity carrying one.
5050
- **PATCH-4 (validation).** Assigned values run the same per-field validation and
5151
the same write codec as `create`.
52-
- **PATCH-5 (empty patch).** Zero assignments is a no-op — the current row is read
53-
back and returned, never an empty-`SET` SQL error.
52+
- **PATCH-5 (empty patch).** A body that strips to zero *caller* assignments answers
53+
`200` with the current row — never a `400`, and never an empty-`SET` SQL error.
54+
Note how this meets PATCH-3: the server-side `@autoSet:onUpdate` stamp is itself an
55+
assignment, so on an entity carrying one the `SET` is never actually empty and an
56+
ordinary write runs (bumping that column, as it does for any other patch). The
57+
literal zero-assignment read-back path is reachable only for an entity with **no**
58+
onUpdate column. Both routes answer identically, which is what makes the contract
59+
stable; what a port must not do is let the caller's empty set decide the status.
60+
Gated cross-port by `fixtures/api-contract-conformance/scenarios/patch-empty-noop.yaml`.
5461
- **PATCH-6 (result).** The patch returns the full updated row (`RETURNING`, or a
5562
same-transaction re-read); a missing row is the port's not-found idiom
5663
(`null` / `Optional.empty` / `404`).

‎fixtures/api-contract-conformance/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ fixtures/api-contract-conformance/
3030
├── create-constraint-violation-400.yaml # FR-036 POST field-constraint → 400 {error:"validation"}
3131
├── update-patch-and-put.yaml
3232
├── update-constraint-violation-400.yaml # FR-036 PATCH present-value constraint → 400
33+
├── patch-empty-noop.yaml # FR-037 PATCH-5 a patch stripping to zero assignments is a no-op read-back
3334
├── delete-204-and-404.yaml
3435
├── invalid-sort-400.yaml
3536
├── filter-eq.yaml # FR-009 filter operators
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
name: patch-empty-noop
2+
description: >
3+
FR-037 PATCH-5 — a PATCH whose body strips to ZERO CALLER assignments answers
4+
200 with the current row. Never a 400, and never an empty-`SET` SQL error.
5+
6+
This is a documented contract (`docs/features/generated-mutations.md` PATCH-5:
7+
"Zero assignments → no write is executed; the current row is read back and
8+
returned"), but until 2026-09-20 nothing in this corpus asserted it, and the
9+
ports had drifted to FIVE different answers for one input: TS 500'd on the
10+
generated mount and 200'd on the reference lane, C# and Java 400'd from
11+
`AuthorApiServer` while their own Tph/Jsonb reference servers 200'd, Kotlin
12+
200'd but wrote, and Python 200'd. An unasserted corner is where divergence
13+
lives; this scenario closes it.
14+
15+
The probe names ONLY `issuedCurrency`, which is `@mutability: "writeOnce"` and
16+
therefore STRIPPED on this path (PATCH-3). Stripping the sole key is what
17+
produces the empty assignment set, so this exercises the real mechanism rather
18+
than a synthetic empty body — which also keeps the request non-empty as JSON,
19+
so no runner has to decide whether to send `{}` at all. It is exactly the arm
20+
`writeonce-patch-stripped.yaml` avoids by pairing the frozen key with a
21+
settable sibling; that file's r3 comment recorded this case as deliberately
22+
unsettled, and now points here instead.
23+
requests:
24+
- id: r1
25+
method: PATCH
26+
path: /api/authors/1
27+
body:
28+
issuedCurrency: "EUR"
29+
expect:
30+
status: 200
31+
body:
32+
row:
33+
# The row comes back whole and UNCHANGED — a read, answered at a write verb.
34+
id: 1
35+
name: "Ada Lovelace"
36+
bio: "First computer programmer"
37+
# The writeOnce column keeps its created value; the presented "EUR" was stripped.
38+
issuedCurrency: "USD"
39+
# PATCH-3 and PATCH-5 meet here, and the order matters. The onUpdate
40+
# @autoSet column is stamped server-side "regardless of the caller", and
41+
# that stamp IS an assignment — so on an entity carrying one, the SET is
42+
# never actually empty and a real write runs. PATCH-5's literal
43+
# zero-assignment read-back path is reachable only for an entity with NO
44+
# onUpdate column. Both readings answer 200 with the current row, which is
45+
# why the contract is stable either way; what a port must not do is let the
46+
# CALLER's empty set decide, and 400 or 500 before the stamp is applied.
47+
# Asserted field-vs-field (format- and timing-agnostic: the seed's 2000
48+
# sentinel vs a 2026 now()), same idiom as autoset-patch.yaml.
49+
fieldsNotEqual: [autoCreatedAt, autoUpdatedAt]
50+
- id: r2
51+
method: PATCH
52+
path: /api/authors/999
53+
body:
54+
issuedCurrency: "EUR"
55+
expect:
56+
# The arm a short-circuit misses. A no-op patch still has to find the row, or
57+
# the fast path becomes a hole that reports success for any id — and on a TPH
58+
# route, a hole through which one subtype answers for another's row.
59+
status: 404
60+
body:
61+
error: not_found

‎fixtures/api-contract-conformance/scenarios/writeonce-patch-stripped.yaml‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,12 @@ requests:
5353
path: /api/authors/1
5454
body:
5555
# A readWrite field rides along DELIBERATELY. Sending the null ALONE would
56-
# also probe the empty-PATCH contract, which this corpus has never settled
57-
# cross-port (TS no-ops and returns the row; the C# reference 400s on an
58-
# empty SET) — and settling that is not this scenario's business. With a
59-
# settable sibling present, the request is unambiguously non-empty and the
60-
# only thing under test is the writeOnce column's response to a null.
56+
# also probe the empty-PATCH contract, and settling that is not this
57+
# scenario's business. With a settable sibling present, the request is
58+
# unambiguously non-empty and the only thing under test is the writeOnce
59+
# column's response to a null. (The empty-PATCH contract was unsettled
60+
# cross-port when this was written; it is FR-037 PATCH-5 — a no-op
61+
# read-back — and is now pinned by `patch-empty-noop.yaml`.)
6162
bio: "cleared-currency probe"
6263
issuedCurrency: null
6364
expect:

‎server/csharp/MetaObjects.IntegrationTests/Api/AuthorApiServer.cs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -617,15 +617,17 @@ private async Task UpdateAuthorAsync(HttpListenerContext ctx, string idStr)
617617
// 400'd, matching every other excluded key on this path. Do NOT add a branch for
618618
// it: that would leave the mode unenforced. The omission also covers the FR-035
619619
// present-null arm for free, since clearing a column is itself a write.
620-
if (sets.Count == 0)
621-
{
622-
await SendJsonAsync(ctx, 400, new Dictionary<string, object?> { ["error"] = "validation" });
623-
return;
624-
}
620+
// FR-037 PATCH-5 (patch-empty-noop): NO empty-SET guard here. A patch that strips to
621+
// zero CALLER assignments is a no-op answering 200 with the current row — never a 400.
622+
// This used to 400 "per FR-035", citing a rule that says the opposite, and it disagreed
623+
// with TphReferenceServer/JsonbReferenceServer in this very directory.
624+
625625
// Issue #203 / ADR-0045 — bump the onUpdate @autoSet column (autoUpdatedAt) to now();
626626
// autoCreatedAt (onCreate) is NEVER written on update (the write-once created-at contract),
627-
// so after a PATCH the two diverge. Appended AFTER the empty-body guard so a no-op PATCH
628-
// still 400s per FR-035. Unspecified kind to match the plain-TIMESTAMP column.
627+
// so after a PATCH the two diverge. Appended UNCONDITIONALLY, which is also what keeps the
628+
// SET non-empty when the caller assigned nothing — so the empty-SET SQL error PATCH-5
629+
// warns about cannot arise on an entity carrying an onUpdate column.
630+
// Unspecified kind to match the plain-TIMESTAMP column.
629631
sets.Add("\"autoUpdatedAt\" = @p" + vals.Count);
630632
vals.Add(DateTime.SpecifyKind(DateTimeOffset.UtcNow.UtcDateTime, DateTimeKind.Unspecified));
631633
int updated;

‎server/java/integration-tests/src/test/java/com/metaobjects/integration/api/AuthorApiServer.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -517,12 +517,16 @@ private void updateAuthor(HttpExchange exchange, String idStr) throws IOExceptio
517517
// rejected, matching every other excluded key on this path. Do NOT add a branch:
518518
// that would leave the mode unenforced. The omission covers the FR-035
519519
// present-null arm too, since clearing a column is itself a write.
520-
if (setClauses.isEmpty()) { sendJson(exchange, 400, Map.of("error", "validation")); return; }
520+
// FR-037 PATCH-5 (patch-empty-noop): NO empty-SET guard here. A patch that strips to
521+
// zero CALLER assignments is a no-op answering 200 with the current row — never a 400.
522+
// This used to 400 "per FR-035", citing a rule that says the opposite, and it disagreed
523+
// with TphReferenceServer/JsonbReferenceServer in this very package.
521524

522525
// Issue #203 / ADR-0045: honor @autoSet onUpdate — bump autoUpdatedAt to now() on EVERY
523526
// update, and NEVER write autoCreatedAt (created-at is immutable; rewriting it is the
524-
// lost-update bug the gate catches). Appended after the caller-field empty-check so the
525-
// empty-body → 400 semantics stay keyed on the caller's fields only.
527+
// lost-update bug the gate catches). Appended UNCONDITIONALLY, which is also what keeps
528+
// the SET non-empty when the caller assigned nothing — so the empty-SET SQL error
529+
// PATCH-5 warns about cannot arise on an entity carrying an onUpdate column.
526530
setClauses.add("\"autoUpdatedAt\" = ?");
527531
values.add(OffsetDateTime.now(ZoneOffset.UTC));
528532

0 commit comments

Comments
 (0)