diff --git a/packages/core/src/security/resolve-authz-context.test.ts b/packages/core/src/security/resolve-authz-context.test.ts index 3d75075369..23d041f355 100644 --- a/packages/core/src/security/resolve-authz-context.test.ts +++ b/packages/core/src/security/resolve-authz-context.test.ts @@ -255,6 +255,123 @@ describe('resolveLocalizationContext — batched fallback read (#2409)', () => { expect(loc.timezone).toBe('Europe/Paris'); expect(ql.counts.sys_setting).toBe(1); // the batched $in fallback ran once }); + + // ── [#11222 items 2 + 3] LEGS, and the resolution context ──────────────── + // + // The pins above count CALLS (`getMany.calls`, `gets === 3`). #10826's whole + // calibration is that the three reads it collapsed already ran in ONE leg — + // "a query-count fix, not a latency fix" (cloud#1539). Nothing pinned that. + // A future edit turning the per-key fallback's `Promise.all` into a + // sequential `for` loop takes legs 1 -> 3 while every call-count assertion + // above stays green; legs are the latency multiplier, so that regression is + // exactly the one the existing pins cannot see. + // + // And the `getMany` double above is declared with TWO parameters, so it is + // structurally unable to observe the third argument: drop `sctx` from the + // production call and all three pins stay green. The rig's double takes it. + + /** + * A settings-service double that MEASURES what the resolver asks for. + * + * - `queries` — every row load. `loadRows` stands in for the real service's + * namespace row load, so one call is one `sys_setting` query. + * - `legs` — every row load that STARTS while nothing else is in flight, + * i.e. the number of sequential round-trip WAVES. Three loads issued + * inside one `Promise.all` all increment `inFlight` synchronously before + * any of them resumes past its first `await`, so they count as ONE leg; + * three awaited in sequence count as three. That is the leg definition + * cloud#1539 used, and it is what makes "3 queries, 1 leg" measurable + * rather than asserted. + * + * `batched: false` reproduces the pre-#10826 occupant (only `get`), which is + * also the shape the resolver still falls back to for a host-provided + * settings service that predates `getMany`. + */ + function makeSettingsRig( + values: Record, + { batched }: { batched: boolean }, + ) { + const stats = { queries: 0, legs: 0 }; + const calls: Array<{ method: string; args: any[] }> = []; + let inFlight = 0; + const loadRows = async () => { + stats.queries += 1; + if (inFlight === 0) stats.legs += 1; // nothing else in flight -> a new wave + inFlight += 1; + try { + await Promise.resolve(); + return values; + } finally { + inFlight -= 1; + } + }; + const pick = (loaded: Record, key: string) => + key in loaded + ? { value: loaded[key], source: 'tenant' } + : { value: undefined, source: 'default' }; + const rig: any = { + stats, + calls, + async get(namespace: string, key: string, ctx: any) { + calls.push({ method: 'get', args: [namespace, key, ctx] }); + return pick(await loadRows(), key); + }, + }; + if (batched) { + // NOTE the THIRD parameter — this is item 3's fix, not a detail: the + // double must accept `ctx` to be able to assert it was forwarded. + rig.getMany = async (namespace: string, keys: readonly string[], ctx: any) => { + calls.push({ method: 'getMany', args: [namespace, [...keys], ctx] }); + const loaded = await loadRows(); + const out: Record = {}; + for (const key of keys) out[key] = pick(loaded, key); + return out; + }; + } + return rig; + } + + const VALUES = { timezone: 'Asia/Tokyo', locale: 'ja-JP', currency: 'JPY' }; + const EXPECTED = { timezone: 'Asia/Tokyo', locale: 'ja-JP', currency: 'JPY' }; + + it('a per-key occupant issues three namespace reads in ONE leg (the pre-#10826 cost)', async () => { + const settings = makeSettingsRig(VALUES, { batched: false }); + const ql = makeCountingQl({ sys_setting: [] }); + const loc = await resolveLocalizationContext({ ql, settings, tenantId: 'o1', userId: 'u1' }); + expect(loc).toEqual(EXPECTED); + expect(settings.stats).toEqual({ queries: 3, legs: 1 }); + // The settings path answered, so the direct `sys_setting` fallback is not + // reached — the three reads above are the whole cost. + expect(ql.counts.sys_setting ?? 0).toBe(0); + }); + + it('the batched occupant issues ONE namespace read, in the same ONE leg', async () => { + const settings = makeSettingsRig(VALUES, { batched: true }); + const ql = makeCountingQl({ sys_setting: [] }); + const loc = await resolveLocalizationContext({ ql, settings, tenantId: 'o1', userId: 'u1' }); + expect(loc).toEqual(EXPECTED); + // queries 3 -> 1, legs 1 -> 1. #10826 was correctly scheduled as a + // query-count fix; the `legs` half is what a sequential-loop regression + // would move, and it is now pinned in both directions. + expect(settings.stats).toEqual({ queries: 1, legs: 1 }); + expect(ql.counts.sys_setting ?? 0).toBe(0); + }); + + it('asks for all three keys of the one namespace in a single call, with the resolution context', async () => { + const settings = makeSettingsRig(VALUES, { batched: true }); + await resolveLocalizationContext({ + ql: makeCountingQl({ sys_setting: [] }), + settings, + tenantId: 'o1', + userId: 'u1', + }); + expect(settings.calls).toEqual([ + { + method: 'getMany', + args: ['localization', ['timezone', 'locale', 'currency'], { tenantId: 'o1', userId: 'u1' }], + }, + ]); + }); }); // #10221: a fresh environment's `sys_setting` table doesn't exist yet, so diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 876f9598ac..bd69be7586 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -647,7 +647,23 @@ function coerceCurrency(value: unknown): string | undefined { export interface ResolveLocalizationInput { ql: any; - /** Settings service exposing `get(namespace, key, { tenantId, userId })`. */ + /** + * Settings service occupant. Two methods are consumed, in this order: + * + * - `getMany(namespace, keys, { tenantId, userId })` — PREFERRED since + * #10826, and what this resolver calls for all three localization keys in + * ONE grouped read. + * - `get(namespace, key, { tenantId, userId })` — the per-key fallback, + * taken only when the occupant does not expose `getMany` (three parallel + * reads; see the feature-detect below). + * + * `getMany` is OPTIONAL for an occupant: the branch is feature-detected, so + * a service that predates it still resolves — at three reads instead of one. + * Typed `any` deliberately (the occupant's shape varies by host); the + * declaration above is the contract this resolver actually relies on, and it + * is prose precisely because nothing type-checks it — `getService` is a cast + * and `rest-server.ts` widens the provider's return to a bare promise. + */ settings?: any; tenantId?: string; userId?: string; @@ -757,6 +773,19 @@ async function resolveLocalizationContextUncached( // A thrown `getMany` lands in the same place a thrown `get` did — // `failed = true` and the direct `$in` fallback below, which reads the // exact same three keys. + // + // [#11222 item 4] ONE non-equivalence, inherent to batching and recorded + // here because it is this CALLER's degradation, not the service's: + // `getMany` validates every requested key up front and throws for the + // WHOLE call, so a host that registered a PARTIAL `localization` + // manifest (missing any of the three keys) loses all three at once, + // where the per-key path would still have resolved the declared ones. + // The `$in` fallback below then answers from tenant-scoped rows only — + // it has no `global` scope layer and no `OS_LOCALIZATION_*` env + // override. Degradation, never a wrong answer, and unreachable against + // the in-repo `localizationSettingsManifest`, which declares all three. + // The all-or-nothing rule itself is `SettingsService.getMany`'s own + // contract and is documented there, not here. let tzRes: any; let localeRes: any; let currencyRes: any; if (typeof settings.getMany === 'function') { try { diff --git a/packages/rest/src/rest-api-plugin.ts b/packages/rest/src/rest-api-plugin.ts index b99b06c92b..7fde12ec74 100644 --- a/packages/rest/src/rest-api-plugin.ts +++ b/packages/rest/src/rest-api-plugin.ts @@ -51,11 +51,29 @@ interface DefaultEnvironmentSurface { * [#4251 B4] Named surface rather than a ledger entry, following the B2 * decision for this slot: `service-settings` is OPTIONAL, so the REST layer * must not acquire a runtime dependency on it, and its `SettingsService` - * declares no `implements` — there is no contract to name. The one method the - * platform consumes is `get`, through `resolveLocalizationContext`'s 4-tier - * timezone/locale/currency cascade; its return type is the PUBLIC + * declares no `implements` — there is no contract to name. Both methods below + * are consumed through `resolveLocalizationContext`'s 4-tier + * timezone/locale/currency cascade; their return types are the PUBLIC * `ResolvedSettingValue` from `@objectstack/spec/system`, so only the context * argument is described structurally (`SettingsContext` is service-local). + * + * ## What an occupant must implement + * + * `get` is REQUIRED; `getMany` is OPTIONAL and worth implementing. Since + * #10826 the resolver feature-detects `getMany` and, when present, reads all + * three localization keys in ONE grouped call; an occupant without it takes + * the three-parallel-`get` path and still answers correctly — the cost is + * three namespace reads instead of one, not a wrong result. That is why + * `getMany` is declared optional rather than required: making it required + * would over-state the contract, and omitting it (as this interface did until + * #11222) under-states what the platform actually calls. + * + * ⚠️ Neither member is type-checked at the call site. `ctx.getService` is a + * cast, `rest-server.ts` widens this provider's return to a bare promise, and + * `resolveLocalizationContext` receives `settings?: any` — so this interface + * is documentation for host authors, not enforcement. `check:slot-lookup` + * (#4251) only bans erasing the lookup to `any`; it never checks the named + * type is COMPLETE, which is how the missing `getMany` stayed invisible. */ interface SettingsReadSurface { get( @@ -63,6 +81,15 @@ interface SettingsReadSurface { key: string, ctx?: { tenantId?: string; userId?: string }, ): Promise>; + /** + * Resolve several keys of ONE namespace in a single grouped read. + * Optional — feature-detected by `resolveLocalizationContext`. + */ + getMany?( + namespace: string, + keys: readonly string[], + ctx?: { tenantId?: string; userId?: string }, + ): Promise>>; } export interface RestApiPluginConfig {