diff --git a/.changeset/share-links-dispatcher-dual-key.md b/.changeset/share-links-dispatcher-dual-key.md new file mode 100644 index 0000000000..3df18d04f7 --- /dev/null +++ b/.changeset/share-links-dispatcher-dual-key.md @@ -0,0 +1,54 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime)!: the /share-links dispatcher domain stops emitting a duplicate `link`/`links` beside `data` (#4038) + +The producer-side other half of #3983. That PR moved the sharing plugin's routes +onto the declared envelope; this removes the compatibility shim the dispatcher +twin had been carrying *because* that surface answered bare. + +Create and list answered with the payload under **two** keys: + +```ts +{ success: true, data: link, link } // POST /share-links +{ success: true, data: links, links } // GET /share-links +``` + +The duplicate existed so readers predating the envelope kept working — which is +why objectui's `ShareDialog` reads `body.links ?? body.data`. Once #3983 made both +surfaces answer `data`, that first branch had no producer left, and the duplicate +had no reader in **any** repo: + +- **framework** — no consumer of these routes at all +- **objectui** — `ShareDialog` already falls through to `body.data` +- **cloud** — swept: it only *registers* `SharingServicePlugin` into per-environment + kernels with `registerShareLinkRoutes: false` so this dispatcher serves the paths. + It never calls them and never reads a body. That sweep is what #4038 was waiting + on, and it came back clean. + +## Shape + +| route | was | now | +|---|---|---| +| `POST /share-links` | `{ success, data: link, link }` | `{ success, data: link }` | +| `GET /share-links` | `{ success, data: links, links }` | `{ success, data: links }` | + +`data` is unchanged in both — only the duplicate key is gone. Anything reading +`body.data`, or going through `ObjectStackClient.unwrapResponse`, sees no +difference. A raw reader of the top-level `body.link` / `body.links` must move to +`body.data`. + +The list route now routes through `deps.success(...)` like the domain's other +three. Create stays hand-built, because `deps.success` hardcodes status 200 and +this route is a **201** — the same reason `/keys` hand-builds its own 201, and the +same shape it uses. + +## Guard + +`scripts/check-route-envelope.mjs` does not and cannot cover this file: it scans +route modules that write via `res.json(...)`, while dispatcher domains return +`{ status, body }` for a central sender. So the drift was invisible to it by +construction. Three tests in `domain-handler-registry.test.ts` cover it instead — +two per-route, plus a general one asserting no success body carries a top-level +key outside `success` / `data` / `meta`. Restoring the duplicates fails all three. diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index 628aceb6ce..5d46434709 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -357,6 +357,62 @@ describe('HttpDispatcher extracted domains (PR-4: share-links)', () => { expect(record?.secret).toBeUndefined(); }); + // ── #4038: one shape, no duplicate keys ────────────────────────────── + // + // Create and list used to answer `{ success: true, data: link, link }` / + // `{ …, data: links, links }` — the payload under BOTH the envelope's `data` + // and a legacy top-level key. That shim existed because the sharing plugin's + // routes (the other surface for these same paths) answered bare, so every + // consumer had to read `body.links ?? body.data`. #3983 moved that surface + // onto this shape and left the duplicate with no reader anywhere — framework, + // objectui, or cloud — so it is gone. These pin that: a body that grows a + // second spelling of its payload fails here. + const authed: any = { executionContext: { userId: 'u1' } }; + const LINK = { id: 'l1', token: 't1', object_name: 'account', record_id: 'r1' }; + + it('POST /share-links answers 201 { success, data } — data IS the link, no top-level `link`', async () => { + const shareLinks = { createLink: vi.fn().mockResolvedValue(LINK) }; + const result = await makeDispatcher({ shareLinks }) + .handleShareLinks('', 'POST', { object: 'account', recordId: 'r1' }, {}, authed); + expect(result.response?.status).toBe(201); + expect(result.response?.body?.success).toBe(true); + expect(result.response?.body?.data).toMatchObject({ id: 'l1', token: 't1' }); + expect(result.response?.body?.link).toBeUndefined(); + }); + + it('GET /share-links answers { success, data } — data IS the array, no top-level `links`', async () => { + const shareLinks = { listLinks: vi.fn().mockResolvedValue([LINK]) }; + const result = await makeDispatcher({ shareLinks }).handleShareLinks('', 'GET', undefined, {}, authed); + expect(result.response?.status).toBe(200); + expect(result.response?.body?.success).toBe(true); + expect(Array.isArray(result.response?.body?.data)).toBe(true); + expect(result.response?.body?.data?.[0]).toMatchObject({ id: 'l1' }); + expect(result.response?.body?.links).toBeUndefined(); + }); + + it('every success body carries its payload under `data` and nowhere else', async () => { + // The general form of the two assertions above, over all four success + // routes: no key beside the envelope's own may hold the payload. + const ENVELOPE_KEYS = new Set(['success', 'data', 'meta']); + const shareLinks = { + createLink: vi.fn().mockResolvedValue(LINK), + listLinks: vi.fn().mockResolvedValue([LINK]), + revokeLink: vi.fn().mockResolvedValue(undefined), + }; + const d = makeDispatcher({ shareLinks }); + const bodies = [ + (await d.handleShareLinks('', 'POST', { object: 'account', recordId: 'r1' }, {}, authed)).response?.body, + (await d.handleShareLinks('', 'GET', undefined, {}, authed)).response?.body, + (await d.handleShareLinks('/l1', 'DELETE', undefined, {}, authed)).response?.body, + ]; + for (const body of bodies) { + expect(body?.success).toBe(true); + for (const key of Object.keys(body ?? {})) { + expect(ENVELOPE_KEYS.has(key), `body carries a non-envelope top-level key: ${key}`).toBe(true); + } + } + }); + it('unmatched sub-path returns the standard ROUTE_NOT_FOUND envelope', async () => { const shareLinks = { resolveToken: vi.fn(), createLink: vi.fn(), listLinks: vi.fn(), revokeLink: vi.fn() }; const context: any = { executionContext: { userId: 'u1' } }; diff --git a/packages/runtime/src/domains/share-links.ts b/packages/runtime/src/domains/share-links.ts index f805d555bf..3803c185b9 100644 --- a/packages/runtime/src/domains/share-links.ts +++ b/packages/runtime/src/domains/share-links.ts @@ -23,6 +23,16 @@ * the authorisation. The underlying record is fetched with a SYSTEM * context (per-env RLS is bypassed because the token gates access), and * `redactFields` are stripped before the record leaves the server. + * + * Every route answers the declared `{ success: true, data }` envelope with `data` + * carrying the payload directly. Create and list used to emit a duplicate + * top-level `link` / `links` beside `data` — a producer-side shim for readers + * predating the envelope, kept alive because the sharing plugin's routes (the + * OTHER surface for these same paths) still answered bare. #3983 moved that + * surface onto this shape, which left the duplicate with no reader in any repo — + * framework, objectui, or cloud — so #4038 removed it. Both surfaces now emit one + * shape, which is what lets `ObjectStackClient.unwrapResponse` return the same + * value whichever one served the request. */ import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; @@ -191,7 +201,11 @@ export async function handleShareLinksRequest( }, callerCtx, ); - return { handled: true, response: { status: 201, body: { success: true, data: link, link } } }; + // Hand-built rather than `deps.success(...)` for the 201 alone — that + // helper hardcodes 200. Same shape the `/keys` domain builds for its + // own 201, and nothing more: the duplicate top-level `link` this used + // to carry beside `data` is gone (#4038). + return { handled: true, response: { status: 201, body: { success: true, data: link } } }; } // GET /share-links?object&recordId → list the caller's own links @@ -207,7 +221,7 @@ export async function handleShareLinksRequest( }, callerCtx, ); - return { handled: true, response: { status: 200, body: { success: true, data: links, links } } }; + return { handled: true, response: deps.success(links) }; } // DELETE /share-links/:idOrToken → revoke