Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions packages/core/src/security/resolve-authz-context.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<string, unknown>,
{ 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<string, unknown>, 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<string, unknown> = {};
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
Expand Down
31 changes: 30 additions & 1 deletion packages/core/src/security/resolve-authz-context.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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 {
Expand Down
33 changes: 30 additions & 3 deletions packages/rest/src/rest-api-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,18 +51,45 @@ 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<T = unknown>(
namespace: string,
key: string,
ctx?: { tenantId?: string; userId?: string },
): Promise<ResolvedSettingValue<T>>;
/**
* Resolve several keys of ONE namespace in a single grouped read.
* Optional — feature-detected by `resolveLocalizationContext`.
*/
getMany?<T = unknown>(
namespace: string,
keys: readonly string[],
ctx?: { tenantId?: string; userId?: string },
): Promise<Record<string, ResolvedSettingValue<T>>>;
}

export interface RestApiPluginConfig {
Expand Down
Loading