From 0ab67230169fcdf783e8dc4a133140c060ac99b5 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:44:48 +0800 Subject: [PATCH] perf(service-settings): getMany resolves same-namespace keys with one grouped row load (#10826) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveLocalizationContext (and getNamespace) called get() once per key, and every call ran loadRows over the whole namespace — three identical sys_setting reads inside one request (queries 16-18 of 24 on a live rig, PR #10824; already 1 leg since they run parallel, so per cloud#1539's calibration this is a query-count fix, not a latency fix, and the card is scheduled accordingly). getMany(namespace, keys, ctx): validate namespace + every key up front; resolve env overrides first (an override answers without touching the store, exactly as get() does); group the remaining keys by which loadRows argument their scope requires (user-scoped keys read (ns, userId), everything else (ns, null)); ONE load per group, both groups in parallel; walk each key's cascade over its group's rows. The cascade is EXTRACTED from get() into resolveKeyFromRows — shared, not copied — so per-key answers deep-equal get()'s by construction (pinned across env overrides, scope mixes, unknown-key refusal, and the read-count contract itself). getNamespace resolves through the same grouped path: N keys, <=2 loads. No caching; nothing survives the call. The resolve-authz-context caller switches to getMany in a follow-up on the #10825 branch (same-file serial ruling by the domain:engine seat). Co-Authored-By: Claude Opus 5 --- .changeset/settings-getmany-10826.md | 5 + .../src/settings-getmany.test.ts | 122 ++++++++++++++++++ .../service-settings/src/settings-service.ts | 93 ++++++++++++- 3 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 .changeset/settings-getmany-10826.md create mode 100644 packages/services/service-settings/src/settings-getmany.test.ts diff --git a/.changeset/settings-getmany-10826.md b/.changeset/settings-getmany-10826.md new file mode 100644 index 0000000000..87aca06ae8 --- /dev/null +++ b/.changeset/settings-getmany-10826.md @@ -0,0 +1,5 @@ +--- +'@objectstack/service-settings': patch +--- + +`SettingsService.getMany(namespace, keys, ctx)` resolves several same-namespace keys with at most two row loads instead of one per key (#10826) — env-overridden keys still answer without touching the store, and every key's value/source/lock/cascade deep-equals the per-key `get()` answer by construction (the cascade is extracted, not copied). `getNamespace` resolves through the same grouped path. diff --git a/packages/services/service-settings/src/settings-getmany.test.ts b/packages/services/service-settings/src/settings-getmany.test.ts new file mode 100644 index 0000000000..c0d1368f70 --- /dev/null +++ b/packages/services/service-settings/src/settings-getmany.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10826] `getMany` — one grouped row load instead of one per key. + * + * `resolveLocalizationContext` called `get()` three times for one namespace, + * and each call ran `loadRows` over the whole namespace: three identical + * `sys_setting` reads inside one request (queries 16–18 of 24 on a live rig, + * PR #10824). `getMany` resolves N same-namespace keys with AT MOST two row + * loads (one per required `loadRows` argument — `user`-scoped keys read + * `(ns, userId)`, everything else `(ns, null)`). + * + * The contract pinned here is EQUIVALENCE: for every key, `getMany`'s answer + * deep-equals what per-key `get()` returns — same value, same source, same + * lock, same cascadeChain — across env overrides, scope mixes, and the + * unknown-key refusal. Plus the read-count contract itself, measured at the + * engine: same-scope keys collapse to ONE find, mixed scopes to TWO, and a + * fully env-overridden set to ZERO. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { SettingsService } from './settings-service.js'; + +function makeEngine(rows: Array>) { + const find = vi.fn(async (_obj: string, opts: any) => { + const w = opts?.where ?? {}; + return rows.filter((r) => + Object.entries(w).every(([k, v]) => (r as any)[k] === v), + ); + }); + return { + find, + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), count: vi.fn(), + }; +} + +const MANIFEST = { + namespace: 'localization', + label: 'Localization', + specifiers: [ + { key: 'timezone', type: 'string', scope: 'user', default: 'UTC' }, + { key: 'locale', type: 'string', scope: 'user', default: 'en' }, + { key: 'currency', type: 'string', scope: 'tenant', default: null }, + ], +} as any; + +const ROWS = [ + { namespace: 'localization', key: 'timezone', scope: 'global', value: '"America/New_York"', user_id: null }, + { namespace: 'localization', key: 'locale', scope: 'user', value: '"zh-CN"', user_id: 'u1' }, + { namespace: 'localization', key: 'currency', scope: 'tenant', value: '"USD"', user_id: null }, +]; + +async function makeService(rows = ROWS) { + const engine = makeEngine(rows); + const svc = new SettingsService(); + svc.registerManifest(MANIFEST); + svc.bindEngine(engine as any); + return { svc, engine }; +} + +const prevEnv: Record = {}; +beforeEach(() => { prevEnv.OS_LOCALIZATION_TIMEZONE = process.env.OS_LOCALIZATION_TIMEZONE; }); +afterEach(() => { + if (prevEnv.OS_LOCALIZATION_TIMEZONE === undefined) delete process.env.OS_LOCALIZATION_TIMEZONE; + else process.env.OS_LOCALIZATION_TIMEZONE = prevEnv.OS_LOCALIZATION_TIMEZONE; +}); + +describe('[#10826] SettingsService.getMany', () => { + it('answers each key exactly as per-key get() does (value/source/lock/cascade)', async () => { + const { svc } = await makeService(); + const ctx = { userId: 'u1', tenantId: 't1' }; + const many = await svc.getMany('localization', ['timezone', 'locale', 'currency'], ctx); + for (const key of ['timezone', 'locale', 'currency']) { + expect(many[key]).toEqual(await svc.get('localization', key, ctx)); + } + }); + + it('collapses same-namespace reads: mixed scopes → TWO engine finds, not one per key', async () => { + const { svc, engine } = await makeService(); + engine.find.mockClear(); + await svc.getMany('localization', ['timezone', 'locale', 'currency'], { userId: 'u1' }); + // user-scoped keys share one load, the tenant-scoped key the other. + expect(engine.find).toHaveBeenCalledTimes(2); + }); + + it('same-scope keys → ONE engine find', async () => { + const { svc, engine } = await makeService(); + engine.find.mockClear(); + await svc.getMany('localization', ['timezone', 'locale'], { userId: 'u1' }); + expect(engine.find).toHaveBeenCalledTimes(1); + }); + + it('an env-overridden key answers without any row load, exactly like get()', async () => { + process.env.OS_LOCALIZATION_TIMEZONE = 'Asia/Tokyo'; + const { svc, engine } = await makeService(); + const ctx = { userId: 'u1' }; + engine.find.mockClear(); + const many = await svc.getMany('localization', ['timezone'], ctx); + expect(engine.find).toHaveBeenCalledTimes(0); + expect(many.timezone).toEqual(await svc.get('localization', 'timezone', ctx)); + expect(many.timezone.source).toBe('env'); + expect(many.timezone.locked).toBe(true); + }); + + it('refuses an unknown key up front, same error class as get()', async () => { + const { svc } = await makeService(); + await expect(svc.getMany('localization', ['timezone', 'nope'])).rejects.toThrow(/nope/); + await expect(svc.get('localization', 'nope')).rejects.toThrow(/nope/); + }); + + it('getNamespace resolves through the grouped path with unchanged answers', async () => { + const { svc, engine } = await makeService(); + const ctx = { userId: 'u1' }; + const ns = await svc.getNamespace('localization', ctx); + expect(ns.values.timezone).toEqual(await svc.get('localization', 'timezone', ctx)); + expect(ns.values.currency).toEqual(await svc.get('localization', 'currency', ctx)); + // and it no longer costs one load per key + engine.find.mockClear(); + await svc.getNamespace('localization', ctx); + expect(engine.find.mock.calls.length).toBeLessThanOrEqual(2); + }); +}); diff --git a/packages/services/service-settings/src/settings-service.ts b/packages/services/service-settings/src/settings-service.ts index c05abaa800..df5550e076 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -1196,8 +1196,92 @@ export class SettingsService { // For 'user' scope we pre-filter by user_id; for 'tenant' and 'global' // we load everything for the namespace and pick the right row below. const rows = await this.loadRows(namespace, scope === 'user' ? ctx.userId ?? null : null); + return this.resolveKeyFromRows(reg, key, scope, rows); + } + + /** + * [#10826] Resolve several keys of ONE namespace with at most TWO row loads + * instead of one per key. + * + * `resolveLocalizationContext` (and `getNamespace` below) called {@link get} + * once per key, and every call ran {@link loadRows} over the whole + * namespace — three identical `sys_setting` reads inside one request, + * measured as queries 16–18 of 24 on a live rig (PR #10824). The cascade + * itself is per-key and cheap; only the ROW LOAD repeats. So: resolve each + * key's env override first (an override answers without touching the store, + * exactly as {@link get} does), then group the remaining keys by which + * `loadRows` argument their scope requires — `user`-scoped keys read + * `(namespace, userId)`, everything else `(namespace, null)` — one load per + * group, and walk each key's cascade over its group's rows. + * + * Row-for-row equivalent to calling {@link get} per key BY CONSTRUCTION: + * the env-override branch, the scope→userId mapping, and the cascade are + * the same code ({@link resolveKeyFromRows} is extracted from `get`, not + * copied). Nothing is cached; nothing survives the call. + */ + async getMany( + namespace: string, + keys: readonly string[], + ctx: SettingsContext = {}, + ): Promise> { + const reg = this.registry.get(namespace); + if (!reg) throw new UnknownNamespaceError(namespace); + for (const key of keys) { + if (!reg.scopes.has(key)) throw new UnknownKeyError(namespace, key); + } + const out: Record = {}; + const pending: Array<{ key: string; scope: SpecifierScope }> = []; + for (const key of keys) { + const envOverride = this.effectiveEnvOverride(reg, namespace, key); + if (envOverride) { + const { envName, value } = envOverride; + out[key] = { + value, + source: 'env', + locked: true, + lockedReason: `Set via env: ${envName}`, + cascadeChain: [ + { scope: 'env', value, locked: true, lockedReason: `Set via env: ${envName}`, effective: true }, + ], + }; + continue; + } + pending.push({ key, scope: reg.scopes.get(key)! }); + } + if (pending.length > 0) { + const userKeys = pending.filter((p) => p.scope === 'user'); + const otherKeys = pending.filter((p) => p.scope !== 'user'); + const [userRows, otherRows] = await Promise.all([ + userKeys.length > 0 + ? this.loadRows(namespace, ctx.userId ?? null) + : Promise.resolve([] as SettingsRow[]), + otherKeys.length > 0 + ? this.loadRows(namespace, null) + : Promise.resolve([] as SettingsRow[]), + ]); + for (const { key, scope } of userKeys) { + out[key] = await this.resolveKeyFromRows(reg, key, scope, userRows); + } + for (const { key, scope } of otherKeys) { + out[key] = await this.resolveKeyFromRows(reg, key, scope, otherRows); + } + } + return out; + } - // 2. cascade walk — OS_* env (handled above) > global > tenant > user > default + /** + * The per-key cascade walk over already-loaded namespace rows — extracted + * verbatim from {@link get} for #10826 so `get`, `getMany` and + * `getNamespace` share ONE implementation of the resolution order + * (env is handled by the callers BEFORE the row load, exactly as before). + */ + private async resolveKeyFromRows( + reg: RegisteredManifest, + key: string, + scope: SpecifierScope, + rows: SettingsRow[], + ): Promise> { + // 2. cascade walk — OS_* env (handled by callers) > global > tenant > user > default // // Build the full chain in declared order so the UI can render // "Inherited from Global / Locked by Global / Overrides tenant" @@ -1266,10 +1350,9 @@ export class SettingsService { // capability for an enforced (HTTP) caller. this.assertPermitted(reg.manifest, 'read', ctx); - const values: Record = {}; - for (const [key] of reg.scopes) { - values[key] = await this.get(namespace, key, ctx); - } + // [#10826] One grouped row load instead of one per key — same resolution + // per key by construction (getMany shares get()'s extracted cascade). + const values = await this.getMany(namespace, Array.from(reg.scopes.keys()), ctx); return { manifest: reg.manifest, values }; }