Skip to content

Commit fc8627e

Browse files
os-litantclaude
andauthored
docs(service-settings): state getMany's all-or-nothing key validation on its own declaration (#12169)
`SettingsService.getMany` validates every requested key against the namespace manifest before reading any env override and before loading any row, so one undeclared key rejects the whole call with `UnknownKeyError` (`code: 'SETTINGS_UNKNOWN_KEY'`) and the caller gets nothing — not the subset it was entitled to. N per-key `get()` calls part ways on exactly that input. The doc comment was otherwise detailed and claimed row-for-row equivalence with per-key `get` "BY CONSTRUCTION" — true for every key that resolves, and not for the refusal. It now draws that line, states the blast radius, and says what a caller against a partial manifest should expect. Adds a sibling pin that asserts the property rather than the throw: the error envelope, that zero rows were loaded (undeclared key last in the request), and that per-key `get()` still answers each declared key. The pre-existing pin asserted only `rejects.toThrow(/nope/)`, which stays green whatever the blast radius is — measured: it survived an ablation that deferred validation until after the row load, while the new pin turned red. No behaviour change: the implementation file's diff is 34 added lines and 0 removed. Claude-Session: https://claude.ai/code/session_01NDGG54XF5gbTLdQzCtnaVV Co-authored-by: Claude <noreply@anthropic.com>
1 parent 494279c commit fc8627e

3 files changed

Lines changed: 107 additions & 0 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/service-settings": patch
3+
---
4+
5+
docs(service-settings): state `getMany`'s all-or-nothing key validation on the declaration that owns it (#11680)
6+
7+
Documentation and a pin. **No behaviour change** — the accept set and every
8+
resolved value are byte-identical.
9+
10+
`SettingsService.getMany` validates **every** requested key against the
11+
namespace manifest before it reads a single env override and before it loads a
12+
single row, so one undeclared key rejects the whole call with
13+
`UnknownKeyError` (`code: 'SETTINGS_UNKNOWN_KEY'`) and the caller receives
14+
nothing — not the subset it was entitled to. N per-key `get()` calls behave
15+
differently on exactly that input: each declared key still answers, and only
16+
the undeclared one throws.
17+
18+
The doc comment was otherwise detailed — it explained the grouped row load and
19+
the env-override ordering, and claimed row-for-row equivalence with per-key
20+
`get` "BY CONSTRUCTION" — but never drew this line. That equivalence claim
21+
holds for every key that *resolves* and not for the refusal, so a batched
22+
consumer had to rediscover the rule from a test. `resolveLocalizationContext`
23+
was the first to inherit it and had to record the consequence locally: a host
24+
registering a **partial** `localization` manifest loses all its keys at once
25+
and drops to a shorter cascade, where the per-key path would still have
26+
resolved the declared ones.
27+
28+
`getMany`'s doc comment now states the rule, its blast radius, why validating
29+
ahead of the grouped walk makes the refusal independent of key order and scope
30+
grouping, and what a caller on a partial manifest should expect.
31+
32+
The pre-existing pin asserted only `rejects.toThrow(/nope/)` — green whatever
33+
the blast radius is. A sibling pin now asserts the property instead: the error
34+
envelope (`code: 'SETTINGS_UNKNOWN_KEY'`), that **zero** rows were loaded
35+
(the refusal is up-front, with the undeclared key last in the request), and
36+
the contrast that per-key `get()` still answers each declared key.

packages/services/service-settings/src/settings-getmany.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2222
import { SettingsService } from './settings-service.js';
23+
import { UnknownKeyError } from './settings-service.types.js';
2324

2425
// WHERE-matcher gate: implement exactly the combinators the service emits and
2526
// THROW on the rest — a bare field-equality read of `$or` would silently match
@@ -118,6 +119,42 @@ describe('[#10826] SettingsService.getMany', () => {
118119
await expect(svc.get('localization', 'nope')).rejects.toThrow(/nope/);
119120
});
120121

122+
// [#11680] The rule the doc comment now states, pinned as a PROPERTY rather
123+
// than as "it throws": the refusal is TOTAL (no partial Record), it lands
124+
// BEFORE any row load, and it is the one input on which `getMany` and N
125+
// per-key `get()` calls part ways. The sibling above asserts only that the
126+
// message names the bad key — which stays green whatever the blast radius is.
127+
it('one undeclared key rejects the WHOLE call — before any row load, no partial result', async () => {
128+
const { svc, engine } = await makeService();
129+
engine.find.mockClear();
130+
131+
const err: unknown = await svc
132+
.getMany('localization', ['timezone', 'currency', 'nope'])
133+
.then(() => null, (e: unknown) => e);
134+
135+
// The envelope, not just the throw. This is a service-layer error class:
136+
// it carries `code` and no `status` (no HTTP boundary here), so `code` is
137+
// the whole machine-readable envelope there is to assert.
138+
expect(err).toBeInstanceOf(UnknownKeyError);
139+
expect((err as UnknownKeyError).code).toBe('SETTINGS_UNKNOWN_KEY');
140+
expect((err as Error).message).toMatch(
141+
/Key 'nope' is not declared in manifest 'localization'/,
142+
);
143+
144+
// TOTAL and UP-FRONT: the two DECLARED keys were neither answered nor even
145+
// loaded — validation runs ahead of the grouped `loadRows`.
146+
expect(engine.find).toHaveBeenCalledTimes(0);
147+
148+
// ...and here is the non-equivalence: per-key `get()` still answers every
149+
// declared key on the same input; only the undeclared one throws. Asserted
150+
// on the resolved cascade LAYER, not on the literal — this fixture stores
151+
// JSON text in `value` while the service persists values verbatim, so a
152+
// literal here would pin the fixture's encoding rather than the rule.
153+
expect((await svc.get('localization', 'timezone')).source).toBe('global');
154+
expect((await svc.get('localization', 'currency')).source).toBe('tenant');
155+
await expect(svc.get('localization', 'nope')).rejects.toBeInstanceOf(UnknownKeyError);
156+
});
157+
121158
it('getNamespace resolves through the grouped path with unchanged answers', async () => {
122159
const { svc, engine } = await makeService();
123160
const ctx = { userId: 'u1' };

packages/services/service-settings/src/settings-service.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,40 @@ export class SettingsService {
12181218
* the env-override branch, the scope→userId mapping, and the cascade are
12191219
* the same code ({@link resolveKeyFromRows} is extracted from `get`, not
12201220
* copied). Nothing is cached; nothing survives the call.
1221+
*
1222+
* ## Key validation is UP-FRONT and TOTAL
1223+
*
1224+
* That equivalence covers every key that RESOLVES. It does not cover the
1225+
* refusal, and this is the one respect in which `getMany` is not N `get`
1226+
* calls — so it is stated here rather than left to be rediscovered from a
1227+
* test.
1228+
*
1229+
* EVERY requested key is checked against the namespace's manifest before a
1230+
* single env override is read and before any row is loaded. One undeclared
1231+
* key therefore rejects the WHOLE call — {@link UnknownKeyError}, `code:
1232+
* 'SETTINGS_UNKNOWN_KEY'` — and the caller receives NOTHING: no partial
1233+
* `Record`, not even the subset it was entitled to. N per-key {@link get}
1234+
* calls part ways on exactly this input: each declared key still answers,
1235+
* and only the undeclared one throws. Same error class, same code; the
1236+
* blast radius is what differs. (An unregistered NAMESPACE is refused first
1237+
* and identically to `get`: {@link UnknownNamespaceError}.)
1238+
*
1239+
* Validating before the grouped walk rather than inside it is what makes
1240+
* the refusal independent of key order, of scope grouping, and of which
1241+
* keys happened to carry an env override — the call either refuses or
1242+
* answers all of them, never something in between. `setMany` pre-flights
1243+
* its whole patch the same way.
1244+
*
1245+
* What it costs a caller: against a host that registers a PARTIAL manifest,
1246+
* a batched consumer loses ALL of its keys at once and must degrade for the
1247+
* whole set — it cannot fall back key by key. `resolveLocalizationContext`
1248+
* is the first consumer to inherit this and records its own degradation
1249+
* locally: a partial `localization` manifest drops it to a shorter cascade
1250+
* (no `global` scope layer, no `OS_LOCALIZATION_*` override) where the
1251+
* per-key path would still have resolved the declared keys. A caller that
1252+
* cannot afford the rule should intersect `keys` with the manifest itself,
1253+
* or read per key. {@link getNamespace} can never trip it — it passes
1254+
* exactly the registered keys.
12211255
*/
12221256
async getMany(
12231257
namespace: string,

0 commit comments

Comments
 (0)