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
36 changes: 36 additions & 0 deletions .changeset/settings-getmany-all-or-nothing-doc.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
"@objectstack/service-settings": patch
---

docs(service-settings): state `getMany`'s all-or-nothing key validation on the declaration that owns it (#11680)

Documentation and a pin. **No behaviour change** — the accept set and every
resolved value are byte-identical.

`SettingsService.getMany` validates **every** requested key against the
namespace manifest before it reads a single env override and before it loads a
single row, so one undeclared key rejects the whole call with
`UnknownKeyError` (`code: 'SETTINGS_UNKNOWN_KEY'`) and the caller receives
nothing — not the subset it was entitled to. N per-key `get()` calls behave
differently on exactly that input: each declared key still answers, and only
the undeclared one throws.

The doc comment was otherwise detailed — it explained the grouped row load and
the env-override ordering, and claimed row-for-row equivalence with per-key
`get` "BY CONSTRUCTION" — but never drew this line. That equivalence claim
holds for every key that *resolves* and not for the refusal, so a batched
consumer had to rediscover the rule from a test. `resolveLocalizationContext`
was the first to inherit it and had to record the consequence locally: a host
registering a **partial** `localization` manifest loses all its keys at once
and drops to a shorter cascade, where the per-key path would still have
resolved the declared ones.

`getMany`'s doc comment now states the rule, its blast radius, why validating
ahead of the grouped walk makes the refusal independent of key order and scope
grouping, and what a caller on a partial manifest should expect.

The pre-existing pin asserted only `rejects.toThrow(/nope/)` — green whatever
the blast radius is. A sibling pin now asserts the property instead: the error
envelope (`code: 'SETTINGS_UNKNOWN_KEY'`), that **zero** rows were loaded
(the refusal is up-front, with the undeclared key last in the request), and
the contrast that per-key `get()` still answers each declared key.
37 changes: 37 additions & 0 deletions packages/services/service-settings/src/settings-getmany.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@

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

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

// [#11680] The rule the doc comment now states, pinned as a PROPERTY rather
// than as "it throws": the refusal is TOTAL (no partial Record), it lands
// BEFORE any row load, and it is the one input on which `getMany` and N
// per-key `get()` calls part ways. The sibling above asserts only that the
// message names the bad key — which stays green whatever the blast radius is.
it('one undeclared key rejects the WHOLE call — before any row load, no partial result', async () => {
const { svc, engine } = await makeService();
engine.find.mockClear();

const err: unknown = await svc
.getMany('localization', ['timezone', 'currency', 'nope'])
.then(() => null, (e: unknown) => e);

// The envelope, not just the throw. This is a service-layer error class:
// it carries `code` and no `status` (no HTTP boundary here), so `code` is
// the whole machine-readable envelope there is to assert.
expect(err).toBeInstanceOf(UnknownKeyError);
expect((err as UnknownKeyError).code).toBe('SETTINGS_UNKNOWN_KEY');
expect((err as Error).message).toMatch(
/Key 'nope' is not declared in manifest 'localization'/,
);

// TOTAL and UP-FRONT: the two DECLARED keys were neither answered nor even
// loaded — validation runs ahead of the grouped `loadRows`.
expect(engine.find).toHaveBeenCalledTimes(0);

// ...and here is the non-equivalence: per-key `get()` still answers every
// declared key on the same input; only the undeclared one throws. Asserted
// on the resolved cascade LAYER, not on the literal — this fixture stores
// JSON text in `value` while the service persists values verbatim, so a
// literal here would pin the fixture's encoding rather than the rule.
expect((await svc.get('localization', 'timezone')).source).toBe('global');
expect((await svc.get('localization', 'currency')).source).toBe('tenant');
await expect(svc.get('localization', 'nope')).rejects.toBeInstanceOf(UnknownKeyError);
});

it('getNamespace resolves through the grouped path with unchanged answers', async () => {
const { svc, engine } = await makeService();
const ctx = { userId: 'u1' };
Expand Down
34 changes: 34 additions & 0 deletions packages/services/service-settings/src/settings-service.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1218,6 +1218,40 @@ export class SettingsService {
* 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.
*
* ## Key validation is UP-FRONT and TOTAL
*
* That equivalence covers every key that RESOLVES. It does not cover the
* refusal, and this is the one respect in which `getMany` is not N `get`
* calls — so it is stated here rather than left to be rediscovered from a
* test.
*
* EVERY requested key is checked against the namespace's manifest before a
* single env override is read and before any row is loaded. One undeclared
* key therefore rejects the WHOLE call — {@link UnknownKeyError}, `code:
* 'SETTINGS_UNKNOWN_KEY'` — and the caller receives NOTHING: no partial
* `Record`, not even the subset it was entitled to. N per-key {@link get}
* calls part ways on exactly this input: each declared key still answers,
* and only the undeclared one throws. Same error class, same code; the
* blast radius is what differs. (An unregistered NAMESPACE is refused first
* and identically to `get`: {@link UnknownNamespaceError}.)
*
* Validating before the grouped walk rather than inside it is what makes
* the refusal independent of key order, of scope grouping, and of which
* keys happened to carry an env override — the call either refuses or
* answers all of them, never something in between. `setMany` pre-flights
* its whole patch the same way.
*
* What it costs a caller: against a host that registers a PARTIAL manifest,
* a batched consumer loses ALL of its keys at once and must degrade for the
* whole set — it cannot fall back key by key. `resolveLocalizationContext`
* is the first consumer to inherit this and records its own degradation
* locally: a partial `localization` manifest drops it to a shorter cascade
* (no `global` scope layer, no `OS_LOCALIZATION_*` override) where the
* per-key path would still have resolved the declared keys. A caller that
* cannot afford the rule should intersect `keys` with the manifest itself,
* or read per key. {@link getNamespace} can never trip it — it passes
* exactly the registered keys.
*/
async getMany(
namespace: string,
Expand Down
Loading