diff --git a/.changeset/client-saveitem-ifmatch-header.md b/.changeset/client-saveitem-ifmatch-header.md new file mode 100644 index 0000000000..f79835ad06 --- /dev/null +++ b/.changeset/client-saveitem-ifmatch-header.md @@ -0,0 +1,42 @@ +--- +"@objectstack/client": minor +--- + +feat(client): `meta.saveItem` can send the `If-Match` OCC header it already told callers to send (#11713) + +`saveItem`'s own docstring has always named the ADR-0008 optimistic-concurrency +protocol: the resolved `version` is the token, echo it back as the `If-Match` +request header on the next write to the same item, and a concurrent edit is +reported as `409 METADATA_CONFLICT` instead of silently overwriting. Both REST +`PUT` doors read `if-match` and thread it as `parentVersion`, so that sentence +was true of a raw-HTTP caller. It was **false** of a first-party SDK caller: +neither `saveItem` declaration accepted a header, an `ifMatch`, or anything +that became one — so an SDK caller who did exactly what the docstring said had +nowhere to put the token and their concurrent edit overwrote anyway, answered +`200`. Declared, not enforced, with no signal at the call site. + +**What is new:** `ifMatch?: string` joins the `SaveMetaItemOptions` bag that +`#11391` landed, on **both** `saveItem` declarations — the unscoped +`ObjectStackClient.meta` and the environment-scoped +`ScopedProjectClient.meta` — wired to the `If-Match` request header through a +single shared builder, the same way the three query parameters go through one +shared query builder. The twins cannot drift. + +```ts +const saved = await client.meta.saveItem('object', 'customer', doc); +// …later, guarded against a concurrent edit: +await client.meta.saveItem('object', 'customer', next, { ifMatch: saved.version }); +// a stale token now answers 409 METADATA_CONFLICT instead of overwriting +``` + +Purely additive and opt-in. Only a non-empty token reaches the wire: +`undefined` and `''` both omit the header entirely — the `init` handed to +`fetch` carries no `headers` key at all — so every existing call is +byte-identical and last-write-wins remains the default, on the wire and on the +door. Unlike the bag's `mode`, `ifMatch` reaches **both** save doors: the +compound-name twin `PUT /meta/:type/:section/:name` reads `if-match` and strips +ETag-style quotes exactly as the single-segment door does. + +Aligned deliberately with the other first-party client: the same member name, +the same header, and the same truthy guard as `MetadataClient.save` in +`@object-ui/data-objectstack` — two first-party clients, one behaviour. diff --git a/packages/client/src/client.test.ts b/packages/client/src/client.test.ts index 8d8f3ed70a..55d6af2c06 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -2764,3 +2764,214 @@ describe('[#11391] the destructive-409 remedy loop is now closed for an SDK call expect(saved).toEqual({ success: true, version: 3 }); }); }); + +// ---------------------------------------------------------------------- +// [#11713] `meta.saveItem`'s `If-Match` HEADER — the ADR-0008 OCC token the +// method's own docstring told callers to echo back, on BOTH clients. +// +// The docstring above `saveItem` has always said: the resolved `version` is +// the optimistic-concurrency token, echo it back as `If-Match` and a +// concurrent edit answers 409 instead of silently overwriting. Both REST PUT +// doors read `if-match` and thread it as `parentVersion`, so that sentence was +// true of a raw-HTTP caller. It was FALSE of a first-party SDK caller: neither +// declaration accepted a header, an `ifMatch`, or anything that became one, so +// following the instruction was impossible and the concurrent edit overwrote. +// +// ⭐ These pins are on the HEADERS the client BUILDS. The pre-existing #11391 +// pins measure the URL only — and a URL pin cannot see this defect, because +// the whole defect is that the value goes NOWHERE: an `ifMatch` swallowed +// silently leaves the URL byte-identical, which is exactly what those pins +// assert. So each case here asserts both directions — the header PRESENT with +// the caller's token when supplied, and ABSENT when it is not. +// ---------------------------------------------------------------------- + +/** Pull the headers object the client handed `fetch` on its Nth call. */ +function headersOfCall(fetchMock: ReturnType, i = 0): Record { + return (fetchMock.mock.calls[i]?.[1]?.headers ?? {}) as Record; +} + +/** + * The header NAMES that call put on the wire, sorted. + * + * The byte-identity claim has to be spelled this way rather than as "the + * `init` has no `headers` key": the mock here is `fetchImpl`, and the client's + * private `fetch` always hands it a merged header object (`Content-Type`, plus + * auth / environment / locale when configured). `metaSaveHeaders` returning + * `undefined` is what keeps that merge byte-identical to an un-pinned save — + * this is where that shows up. + */ +function headerNamesOf(fetchMock: ReturnType, i = 0): string[] { + return Object.keys(headersOfCall(fetchMock, i)).sort(); +} + +/** What an un-pinned save sends on this bare mock client: nothing but the body type. */ +const BASELINE_HEADERS = ['Content-Type']; + +const OCC_TOKEN = 'sha256:' + 'b'.repeat(64); + +describe('[#11713] meta.saveItem sends the If-Match header (unscoped client)', () => { + it('sends `ifMatch` as the If-Match request header, verbatim', async () => { + const { client, fetchMock } = createMockClient({ success: true, version: 'sha256:next' }); + await client.meta.saveItem('object', 'customer', { name: 'customer' }, { ifMatch: OCC_TOKEN }); + // THE assertion this card exists for: the token reaches the wire. + expect(headersOfCall(fetchMock)['If-Match']).toBe(OCC_TOKEN); + // Verbatim and unquoted — the sibling first-party client + // (`@object-ui/data-objectstack` MetadataClient.save) sends exactly + // these bytes, and the door strips ETag quotes rather than requiring + // them. + expect(headersOfCall(fetchMock)['If-Match']).not.toContain('"'); + }); + + it('ABSENT when the caller does not pin: no If-Match, and no `headers` key at all', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.meta.saveItem('object', 'customer', { name: 'customer' }); + expect(headersOfCall(fetchMock)['If-Match']).toBeUndefined(); + // Byte-identity, not merely "no If-Match": the builder returns + // `undefined`, so the header set this save puts on the wire is exactly + // the one an un-pinned save always sent. Last-write-wins stays default. + expect(headerNamesOf(fetchMock)).toEqual(BASELINE_HEADERS); + }); + + it('ABSENT when the bag is present but carries no token', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.meta.saveItem('object', 'customer', {}, { force: true }); + expect(headersOfCall(fetchMock)['If-Match']).toBeUndefined(); + expect(headerNamesOf(fetchMock)).toEqual(BASELINE_HEADERS); + }); + + it("ABSENT for an empty token — `''` never reaches the wire", async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.meta.saveItem('object', 'customer', {}, { ifMatch: '' }); + // An empty `If-Match` is not a no-op on the door: presence means "pin + // this write", so an emitted empty header would pin against the empty + // string and refuse a save the caller never asked to pin. + expect(headersOfCall(fetchMock)['If-Match']).toBeUndefined(); + expect(headerNamesOf(fetchMock)).toEqual(BASELINE_HEADERS); + }); + + it('is a HEADER, not a query parameter: the URL is byte-identical to an unpinned save', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.meta.saveItem('object', 'customer', {}, { ifMatch: OCC_TOKEN }); + // `?ifMatch=` is read by neither PUT door. If it ever appeared here it + // would look set at the call site and protect nothing. + expect(String(fetchMock.mock.calls[0][0])).toBe( + 'http://localhost:3000/api/v1/meta/object/customer', + ); + expect(headersOfCall(fetchMock)['If-Match']).toBe(OCC_TOKEN); + }); + + it('rides alongside the #11391 query parameters without disturbing them', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.meta.saveItem('object', 'customer', { name: 'customer' }, { + ifMatch: OCC_TOKEN, + force: true, + packageId: 'app.crm', + mode: 'draft', + }); + expect(String(fetchMock.mock.calls[0][0])).toBe( + 'http://localhost:3000/api/v1/meta/object/customer?force=true&package=app.crm&mode=draft', + ); + expect(headersOfCall(fetchMock)['If-Match']).toBe(OCC_TOKEN); + const init = fetchMock.mock.calls[0][1]; + expect(init.method).toBe('PUT'); + // The token is not a field on the document being saved. + expect(JSON.parse(init.body)).toEqual({ name: 'customer' }); + }); + + it('OCC-guards a COMPOUND name too — unlike `mode`, this reaches both doors', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.meta.saveItem('object', 'views/all_leads', { label: 'All leads' }, { ifMatch: OCC_TOKEN }); + // The compound-name door `PUT /meta/:type/:section/:name` reads + // `if-match` and strips ETag quotes exactly as the single-segment door + // does — measured in rest-server.ts. `mode` is the member that does NOT + // reach it; this one does, so the slash must survive AND the pin must + // ride along. + expect(String(fetchMock.mock.calls[0][0])).toBe( + 'http://localhost:3000/api/v1/meta/object/views/all_leads', + ); + expect(headersOfCall(fetchMock)['If-Match']).toBe(OCC_TOKEN); + }); +}); + +describe('[#11713] meta.saveItem sends the If-Match header (environment-scoped twin)', () => { + it('sends the header on the scoped client too', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.project('proj-123').meta.saveItem( + 'object', 'customer', { name: 'customer' }, { ifMatch: OCC_TOKEN }, + ); + expect(String(fetchMock.mock.calls[0][0])).toBe( + 'http://localhost:3000/api/v1/environments/proj-123/meta/object/customer', + ); + expect(headersOfCall(fetchMock)['If-Match']).toBe(OCC_TOKEN); + }); + + it('ABSENT on the scoped client when the caller does not pin', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.project('proj-123').meta.saveItem('object', 'customer', { name: 'customer' }); + expect(headersOfCall(fetchMock)['If-Match']).toBeUndefined(); + expect(headerNamesOf(fetchMock)).toEqual(BASELINE_HEADERS); + }); + + it('IN STEP with the unscoped twin: identical If-Match for identical options', async () => { + // The divergence this card is about is a fix landing on one twin only. + // Comparing the two headers keeps holding if either path changes, + // rather than restating a literal on both sides. + const { client, fetchMock } = createMockClient({ success: true }); + const opts = { ifMatch: OCC_TOKEN, force: true } as const; + await client.meta.saveItem('object', 'customer', {}, opts); + await client.project('proj-123').meta.saveItem('object', 'customer', {}, opts); + expect(headersOfCall(fetchMock, 1)['If-Match']).toBe(headersOfCall(fetchMock, 0)['If-Match']); + expect(headersOfCall(fetchMock, 0)['If-Match']).toBe(OCC_TOKEN); + }); + + it('IN STEP when unpinned too: neither twin adds a header', async () => { + const { client, fetchMock } = createMockClient({ success: true }); + await client.meta.saveItem('object', 'customer', {}, { force: true }); + await client.project('proj-123').meta.saveItem('object', 'customer', {}, { force: true }); + expect(headerNamesOf(fetchMock, 0)).toEqual(BASELINE_HEADERS); + expect(headerNamesOf(fetchMock, 1)).toEqual(BASELINE_HEADERS); + }); +}); + +describe('[#11713] the docstring\'s OCC instruction is now executable end to end', () => { + it('save → pin the resolved `version` → the stale write is REFUSED, not silently applied', async () => { + // Round 1 answers a real save body; round 2 answers the real conflict + // envelope the door emits when `parentVersion` does not match + // (`{ code: 'METADATA_CONFLICT' }` at HTTP 409 — pinned in + // packages/rest/src/rest.test.ts). + const conflict = { + error: 'parentVersion mismatch', + code: 'METADATA_CONFLICT', + }; + const responses: any[] = [ + { + ok: true, status: 200, statusText: 'OK', + json: async () => ({ success: true, version: OCC_TOKEN, seq: 4, state: 'active' }), + headers: new Headers(), + }, + { ok: false, status: 409, statusText: 'Conflict', json: async () => conflict, headers: new Headers() }, + ]; + const fetchMock = vi.fn().mockImplementation(() => Promise.resolve(responses.shift())); + const client = new ObjectStackClient({ baseUrl: 'http://localhost:3000', fetch: fetchMock }); + + // 1. A save resolves the OCC carrier the docstring names. + const saved = await client.meta.saveItem('object', 'customer', { name: 'customer' }); + expect(saved.version).toBe(OCC_TOKEN); + // Un-pinned, so no token was sent — this is the before-state. + expect(headersOfCall(fetchMock, 0)['If-Match']).toBeUndefined(); + + // 2. Do literally what the docstring prescribes. THIS is the + // acceptance criterion: before #11713 there was no argument to + // pass here, so the instruction could not be followed at all. + const err: any = await client.meta + .saveItem('object', 'customer', { name: 'customer v2' }, { ifMatch: saved.version }) + .then(() => { throw new Error('expected the stale save to be refused'); }, (e) => e); + // Assert the ENVELOPE the caller branches on, not merely that + // something threw: a bare `.toThrow()` stays green against any error, + // including one from a client that never sent the header. + expect(err.code).toBe('METADATA_CONFLICT'); + expect(err.httpStatus).toBe(409); + // And the pin really rode the second request. + expect(headersOfCall(fetchMock, 1)['If-Match']).toBe(OCC_TOKEN); + }); +}); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 65548bfe4b..104de7f6f9 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -519,9 +519,15 @@ function normalizeActionResult(payload: any): { success: boolean; data?: T; e } /** - * Query-string options for `meta.saveItem` on BOTH clients — the unscoped + * Write options for `meta.saveItem` on BOTH clients — the unscoped * `ObjectStackClient.meta` and {@link ScopedProjectClient.meta}. * + * Named for the WRITE, not for the query string: three members ride the query + * string and `ifMatch` rides a request HEADER (#11713). One bag per write + * whatever carrier each member takes — the same shape the other first-party + * client's `MetadataClientSaveOptions` + * (`@object-ui/data-objectstack`) already carries, and for the same reason. + * * ONE exported type deliberately shared by the two declarations, rather than * an inline literal copied into each. The two `saveItem`s are the same method * on two clients reaching one pair of routes; every divergence measured @@ -529,7 +535,7 @@ function normalizeActionResult(payload: any): { success: boolean; data?: T; e * it), and a bag spelled twice is a divergence waiting to be introduced by * whoever next extends only the copy they happened to open. * - * ## Why these three, and why they are the ones that exist + * ## Why these three QUERY parameters, and why they are the ones that exist * * `PUT /api/v1/meta/:type/:name` reads exactly these three query parameters, * and until this type existed the SDK sent NONE of them — `saveItem` built a @@ -558,6 +564,39 @@ function normalizeActionResult(payload: any): { success: boolean; data?: T; e * repair this from the message side — the parameter had to become reachable. */ export interface SaveMetaItemOptions { + /** + * `If-Match: ` — the ADR-0008 optimistic-concurrency token, and + * the one member here that is NOT a query parameter. + * + * Echo back the `version` a previous `saveItem` (or `publishItem`) + * resolved, and a concurrent edit is refused with `409 + * metadata_conflict` instead of silently overwriting the other author's + * write. Omit for the last-write-wins behaviour every call had before + * this member existed — the pin is opt-in on the door too. + * + * Opaque: echo it verbatim, never parse it. `SaveMetaItemResponse.version` + * says so in the contract itself — currently `sha256:<64 hex chars>`, with + * the format explicitly not promised. + * + * Only a non-empty token reaches the wire. `undefined` and `''` both OMIT + * the header rather than sending an empty `If-Match`, which is not merely + * tidier: the door reads the header's PRESENCE as "pin this write", so an + * empty spelling would pin the write against the empty string and refuse + * every save with a 409 the caller never asked for. + * + * ✅ REACHES BOTH DOORS — unlike `mode` below. The compound-name twin + * `PUT /meta/:type/:section/:name` reads `if-match` and strips ETag-style + * quotes exactly as the single-segment door does, so + * `saveItem('object', 'views/all_leads', item, { ifMatch })` is + * OCC-guarded like any other save. + * + * Same member name, same header, same truthy guard as the sibling + * first-party `@object-ui/data-objectstack` `MetadataClient.save`, whose + * read surface names this token `checksum` — one content hash, one + * behaviour, two clients. `data.update` / `data.delete` on this same + * client carry it under the same name too. + */ + ifMatch?: string; /** * `?force=true` — acknowledge and proceed past the Phase 3a * destructive-change refusal (`409 DESTRUCTIVE_CHANGE`), whose message @@ -635,6 +674,29 @@ function metaSaveQuery(options?: SaveMetaItemOptions): string { return qs ? `?${qs}` : ''; } +/** + * Compose the `meta.saveItem` request headers — the ONE builder both + * `saveItem` declarations call, for the same reason {@link metaSaveQuery} is + * one function: the twins must not be able to drift in what they put on the + * wire. + * + * Returns `undefined` (never `{}`) when nothing is set, so the call site can + * omit the `headers` key entirely and an options-less save stays + * BYTE-IDENTICAL to what it sent before this existed. + * + * ⛔ `ifMatch` must never be added to {@link metaSaveQuery}. It is a header; + * neither PUT door reads an `?ifMatch=` parameter, so a query spelling would + * look set at the call site and protect nothing — the exact failure #11713 + * exists to close, one carrier over. + */ +function metaSaveHeaders(options?: SaveMetaItemOptions): Record | undefined { + if (!options?.ifMatch) return undefined; + // Verbatim and UNQUOTED. The door tolerates ETag-style quotes by stripping + // them, but the token a save resolves carries none, and wrapping it here + // would put bytes on the wire the sibling first-party client does not send. + return { 'If-Match': String(options.ifMatch) }; +} + export class ObjectStackClient { private baseUrl: string; private token?: string; @@ -844,9 +906,11 @@ export class ObjectStackClient { * @param item - The metadata content to save * * The resolved `version` is the ADR-0008 optimistic-concurrency token: - * echo it back as the `If-Match` request header on the next write to the - * same item and a concurrent edit is reported as 409 `metadata_conflict` - * instead of silently overwriting. It is nameable here only because + * pass it back as `options.ifMatch` on the next write to the same item — + * this method sends it as the `If-Match` request header — and a concurrent + * edit is reported as 409 `metadata_conflict` instead of silently + * overwriting. Until #11713 that instruction named a header the SDK had no + * argument for, so a first-party caller who followed it overwrote anyway. It is nameable here only because * `SaveMetaItemResponseSchema` declares the full body since #5745 — the * declaration used to stop at `{ success, message }`, and annotating * against that subset would have hidden the OCC carrier (#5545). @@ -867,12 +931,18 @@ export class ObjectStackClient { // `…nameforce=true`. This value carries its `?`; the distinct name // says so without the reader having to go and look. const query = metaSaveQuery(options); + // The OCC token rides a HEADER, not the query string — see + // {@link metaSaveHeaders}. `undefined` when unset, and the spread then + // omits the `headers` key altogether, so a save without `ifMatch` + // hands `fetch` the same `init` it always did. + const headers = metaSaveHeaders(options); // `type`/`name` stay UNENCODED — a compound name's slash must survive // so the request reaches `PUT /meta/:type/:section/:name` instead of // collapsing onto the 3-segment route (pinned in client.test.ts). const res = await this.fetch(`${this.baseUrl}${route}/${type}/${name}${query}`, { method: 'PUT', - body: JSON.stringify(item) + body: JSON.stringify(item), + ...(headers ? { headers } : {}), }); return this.unwrapResponse(res); }, @@ -1124,8 +1194,9 @@ export class ObjectStackClient { * through unencoded, like `getItem`. * * The resolved `version` is the ADR-0008 optimistic-concurrency token, the - * same carrier `saveItem` returns and with the same job: echo it back as - * `If-Match` on the next write to the item. It is nameable here only since + * same carrier `saveItem` returns and with the same job: pass it back as + * `saveItem`'s `options.ifMatch`, which sends it as `If-Match` on the next + * write to the item. It is nameable here only since * #7294, which declared `PublishMetaItemResponseSchema` — this method * resolved to `any` before that, because the publish door had no * declaration at all for a return type to point at. @@ -1540,7 +1611,8 @@ export class ObjectStackClient { * single-item declaration; this method resolved to `any` before that. * The batch is all-or-nothing (ADR-0067 D2): `success: false` on a 200 * means NOTHING landed — read `failed[]`. Each `published[]` element's - * `version` is the ADR-0008 OCC token (echo as `If-Match`), and the + * `version` is the ADR-0008 OCC token (pass to `saveItem` as + * `options.ifMatch`, which sends it as `If-Match`), and the * conditional receipts (`seedApplied` / `materializeApplied` / * `unhiddenApps` / `unhideError` / `rebindError`) each report their own * outcome: a 200 does not mean the data plane or the visibility flip @@ -5495,13 +5567,15 @@ export class ScopedProjectClient { return this.parent._unwrap(res); }, /** - * Carries the ADR-0008 OCC token in `version` — see the unscoped twin. + * Carries the ADR-0008 OCC token in `version` — see the unscoped twin, + * and send it back here as `options.ifMatch`. * * `options` is the SAME {@link SaveMetaItemOptions} bag the unscoped twin * takes, and reaches the SAME handler: the scoped mount is not a second * implementation, it is one `registerForBase` call replayed against * `/environments/:environmentId` (see `RestServer`), so this door reads - * `?force` / `?package` / `?mode` byte-identically. A bag on only one of + * `?force` / `?package` / `?mode` — and the `If-Match` header — + * byte-identically. A bag on only one of * the two clients would be a fresh divergence of the kind #7019 rules * against, not half a fix. */ @@ -5513,9 +5587,13 @@ export class ScopedProjectClient { ): Promise => { // `query`, not `qs` — it carries its own `?`; see the unscoped twin. const query = metaSaveQuery(options); + // Header half of the same bag, through the same one builder the twin + // calls — see {@link metaSaveHeaders}. + const headers = metaSaveHeaders(options); const res = await this.parent._fetch(this.url(`/meta/${type}/${name}${query}`), { method: 'PUT', body: JSON.stringify(item), + ...(headers ? { headers } : {}), }); return this.parent._unwrap(res); },