feat(fp): WP-05 — parse-don't-validate closure + MDX
Close the three remaining unvalidated `as <DomainType>` casts at the wire boundary (intake-policy, big-register aantekening type, brief passage scope), each replaced by a Result-returning parser with a rejection-case spec, plus the Foundations/Parse, don't validate curriculum page.
This commit is contained in:
@@ -44,7 +44,7 @@ for its existing violations, so every WP ends green.
|
|||||||
| [WP-02](WP-02-check-tokens.md) | Harden `check:tokens` + fix what it catches | 0 · gates | done |
|
| [WP-02](WP-02-check-tokens.md) | Harden `check:tokens` + fix what it catches | 0 · gates | done |
|
||||||
| [WP-03](WP-03-contracts-purity.md) | Boundaries I: contracts purity + ApiClient confinement | 0 · gates | done |
|
| [WP-03](WP-03-contracts-purity.md) | Boundaries I: contracts purity + ApiClient confinement | 0 · gates | done |
|
||||||
| [WP-04](WP-04-ui-not-infrastructure.md) | Boundaries II: `ui ↛ infrastructure` + showcase sanction | 0 · gates | done |
|
| [WP-04](WP-04-ui-not-infrastructure.md) | Boundaries II: `ui ↛ infrastructure` + showcase sanction | 0 · gates | done |
|
||||||
| [WP-05](WP-05-parse-boundaries.md) | Parse-don't-validate closure + MDX | 1 · FP/DDD | todo |
|
| [WP-05](WP-05-parse-boundaries.md) | Parse-don't-validate closure + MDX | 1 · FP/DDD | done |
|
||||||
| [WP-06](WP-06-typed-async.md) | Generic async template contexts — kill `$any()` | 1 · FP/DDD | todo |
|
| [WP-06](WP-06-typed-async.md) | Generic async template contexts — kill `$any()` | 1 · FP/DDD | todo |
|
||||||
| [WP-07](WP-07-brief-idioms.md) | Brief on the shared idioms + RemoteData MDX | 1 · FP/DDD | todo |
|
| [WP-07](WP-07-brief-idioms.md) | Brief on the shared idioms + RemoteData MDX | 1 · FP/DDD | todo |
|
||||||
| [WP-08](WP-08-store-idiom.md) | One store idiom + machine naming + TEA MDX | 1 · FP/DDD | todo |
|
| [WP-08](WP-08-store-idiom.md) | One store idiom + machine naming + TEA MDX | 1 · FP/DDD | todo |
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# WP-05 — Parse-don't-validate closure + MDX
|
# WP-05 — Parse-don't-validate closure + MDX
|
||||||
|
|
||||||
Status: todo
|
Status: done
|
||||||
Phase: 1 — FP/DDD core
|
Phase: 1 — FP/DDD core
|
||||||
|
|
||||||
## Why
|
## Why
|
||||||
@@ -45,10 +45,10 @@ PassageScope` → validated parse (the file is otherwise parse-heavy; this one f
|
|||||||
|
|
||||||
## Acceptance criteria
|
## Acceptance criteria
|
||||||
|
|
||||||
- [ ] No unvalidated `as <DomainType>` casts in `**/infrastructure/**` (the sanctioned
|
- [x] No unvalidated `as <DomainType>` casts in `**/infrastructure/**` (the sanctioned
|
||||||
"narrow unknown to `Partial<Dto>` then parse" entry-cast is fine).
|
"narrow unknown to `Partial<Dto>` then parse" entry-cast is fine).
|
||||||
- [ ] Each new parser has a spec including a rejection case.
|
- [x] Each new parser has a spec including a rejection case.
|
||||||
- [ ] MDX renders under Foundations in Storybook.
|
- [x] MDX renders under Foundations in Storybook.
|
||||||
|
|
||||||
## Verification
|
## Verification
|
||||||
|
|
||||||
|
|||||||
4095
documentation.json
4095
documentation.json
File diff suppressed because one or more lines are too long
@@ -141,6 +141,14 @@ describe('brief.adapter parse boundary', () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rejects a library passage with an unknown scope', () => {
|
||||||
|
const r = parseBriefView({
|
||||||
|
...view,
|
||||||
|
availablePassages: [{ ...view.availablePassages![0], scope: 'bogus' as never }],
|
||||||
|
});
|
||||||
|
expect(r.ok).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
it('rejects a passage block missing provenance', () => {
|
it('rejects a passage block missing provenance', () => {
|
||||||
const r = parseBrief({
|
const r = parseBrief({
|
||||||
...view.brief,
|
...view.brief,
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ import {
|
|||||||
LetterBlock,
|
LetterBlock,
|
||||||
LetterSection,
|
LetterSection,
|
||||||
LibraryPassage,
|
LibraryPassage,
|
||||||
PassageScope,
|
|
||||||
} from '@brief/domain/brief';
|
} from '@brief/domain/brief';
|
||||||
import { PlaceholderDef } from '@brief/domain/placeholders';
|
import { PlaceholderDef } from '@brief/domain/placeholders';
|
||||||
import { Mark, Paragraph, RichTextBlock, RichTextNode } from '@shared/kernel/rich-text';
|
import { Mark, Paragraph, RichTextBlock, RichTextNode } from '@shared/kernel/rich-text';
|
||||||
@@ -228,8 +227,9 @@ export function parseStatus(dto: BriefStatusDto | undefined): Result<string, Bri
|
|||||||
}
|
}
|
||||||
|
|
||||||
function parsePassage(dto: LibraryPassageDto): Result<string, LibraryPassage> {
|
function parsePassage(dto: LibraryPassageDto): Result<string, LibraryPassage> {
|
||||||
if (typeof dto.passageId !== 'string' || (dto.scope !== 'global' && dto.scope !== 'beroep'))
|
if (typeof dto.passageId !== 'string') return err('passage: bad shape');
|
||||||
return err('passage: bad shape');
|
if (dto.scope !== 'global' && dto.scope !== 'beroep')
|
||||||
|
return err(`passage: unknown scope ${dto.scope}`);
|
||||||
if (
|
if (
|
||||||
typeof dto.sectionKey !== 'string' ||
|
typeof dto.sectionKey !== 'string' ||
|
||||||
typeof dto.label !== 'string' ||
|
typeof dto.label !== 'string' ||
|
||||||
@@ -240,7 +240,7 @@ function parsePassage(dto: LibraryPassageDto): Result<string, LibraryPassage> {
|
|||||||
if (!content.ok) return content;
|
if (!content.ok) return content;
|
||||||
return ok({
|
return ok({
|
||||||
passageId: dto.passageId,
|
passageId: dto.passageId,
|
||||||
scope: dto.scope as PassageScope,
|
scope: dto.scope,
|
||||||
sectionKey: dto.sectionKey,
|
sectionKey: dto.sectionKey,
|
||||||
label: dto.label,
|
label: dto.label,
|
||||||
content: content.value,
|
content: content.value,
|
||||||
|
|||||||
@@ -42,6 +42,11 @@ export interface ValidIntake {
|
|||||||
fallback; the server value wins. */
|
fallback; the server value wins. */
|
||||||
export const SCHOLING_THRESHOLD_DEFAULT = 1000;
|
export const SCHOLING_THRESHOLD_DEFAULT = 1000;
|
||||||
|
|
||||||
|
/** The server-owned intake policy (domain-side, parsed from the wire at the boundary). */
|
||||||
|
export interface IntakePolicy {
|
||||||
|
readonly scholingThreshold: number;
|
||||||
|
}
|
||||||
|
|
||||||
/** True when NL-hours are low enough that the scholing question must be answered.
|
/** True when NL-hours are low enough that the scholing question must be answered.
|
||||||
The threshold is passed in (server-owned), not hardcoded. */
|
The threshold is passed in (server-owned), not hardcoded. */
|
||||||
export function lageUren(a: Answers, scholingThreshold = SCHOLING_THRESHOLD_DEFAULT): boolean {
|
export function lageUren(a: Answers, scholingThreshold = SCHOLING_THRESHOLD_DEFAULT): boolean {
|
||||||
|
|||||||
@@ -0,0 +1,15 @@
|
|||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { parseIntakePolicy } from './intake-policy.adapter';
|
||||||
|
|
||||||
|
describe('intake-policy.adapter parse boundary', () => {
|
||||||
|
it('parses a well-formed policy', () => {
|
||||||
|
const r = parseIntakePolicy({ scholingThreshold: 800 });
|
||||||
|
expect(r).toEqual({ ok: true, value: { scholingThreshold: 800 } });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a missing or non-numeric threshold', () => {
|
||||||
|
expect(parseIntakePolicy({}).ok).toBe(false);
|
||||||
|
expect(parseIntakePolicy({ scholingThreshold: '800' }).ok).toBe(false);
|
||||||
|
expect(parseIntakePolicy(null).ok).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,4 +1,6 @@
|
|||||||
import { Injectable, inject, resource } from '@angular/core';
|
import { Injectable, inject, resource } from '@angular/core';
|
||||||
|
import { Result, ok, err } from '@shared/kernel/fp';
|
||||||
|
import { IntakePolicy } from '@herregistratie/domain/intake.machine';
|
||||||
import { ApiClient } from '@shared/infrastructure/api-client';
|
import { ApiClient } from '@shared/infrastructure/api-client';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -11,6 +13,21 @@ export class IntakePolicyAdapter {
|
|||||||
private client = inject(ApiClient);
|
private client = inject(ApiClient);
|
||||||
|
|
||||||
policyResource() {
|
policyResource() {
|
||||||
return resource({ loader: () => this.client.policy() });
|
return resource({
|
||||||
|
loader: async () => {
|
||||||
|
const parsed = parseIntakePolicy(await this.client.policy());
|
||||||
|
if (!parsed.ok) throw new Error(parsed.error);
|
||||||
|
return parsed.value;
|
||||||
|
},
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Trust-boundary parse: an unrecognized/missing threshold is an explicit Failure. */
|
||||||
|
export function parseIntakePolicy(json: unknown): Result<string, IntakePolicy> {
|
||||||
|
if (typeof json !== 'object' || json === null) return err('intake-policy: not an object');
|
||||||
|
const dto = json as { scholingThreshold?: unknown };
|
||||||
|
if (typeof dto.scholingThreshold !== 'number')
|
||||||
|
return err('intake-policy: missing scholingThreshold');
|
||||||
|
return ok({ scholingThreshold: dto.scholingThreshold });
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,20 @@
|
|||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { parseAantekening } from './big-register.adapter';
|
||||||
|
|
||||||
|
describe('big-register.adapter parse boundary', () => {
|
||||||
|
it('parses known aantekening types', () => {
|
||||||
|
expect(parseAantekening({ type: 'Specialisme', omschrijving: 'x', datum: '2026-01-01' })).toEqual({
|
||||||
|
ok: true,
|
||||||
|
value: { type: 'Specialisme', omschrijving: 'x', datum: '2026-01-01' },
|
||||||
|
});
|
||||||
|
expect(parseAantekening({ type: 'Aantekening' })).toEqual({
|
||||||
|
ok: true,
|
||||||
|
value: { type: 'Aantekening', omschrijving: '', datum: '' },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects an unknown type', () => {
|
||||||
|
expect(parseAantekening({ type: 'Bogus' }).ok).toBe(false);
|
||||||
|
expect(parseAantekening({}).ok).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,4 +1,5 @@
|
|||||||
import { Injectable, inject, resource } from '@angular/core';
|
import { Injectable, inject, resource } from '@angular/core';
|
||||||
|
import { Result, ok, err } from '@shared/kernel/fp';
|
||||||
import { Aantekening, AantekeningType } from '../domain/registration';
|
import { Aantekening, AantekeningType } from '../domain/registration';
|
||||||
import { ApiClient, AantekeningDto } from '@shared/infrastructure/api-client';
|
import { ApiClient, AantekeningDto } from '@shared/infrastructure/api-client';
|
||||||
|
|
||||||
@@ -16,15 +17,30 @@ export class BigRegisterAdapter {
|
|||||||
private client = inject(ApiClient);
|
private client = inject(ApiClient);
|
||||||
|
|
||||||
aantekeningenResource() {
|
aantekeningenResource() {
|
||||||
return resource({ loader: () => this.client.notes().then((ns) => ns.map(toAantekening)) });
|
return resource({
|
||||||
|
loader: () =>
|
||||||
|
this.client.notes().then((ns) => {
|
||||||
|
const out: Aantekening[] = [];
|
||||||
|
for (const n of ns) {
|
||||||
|
const parsed = parseAantekening(n);
|
||||||
|
if (!parsed.ok) throw new Error(parsed.error);
|
||||||
|
out.push(parsed.value);
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}),
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Map the wire DTO (all fields optional) onto our domain type. */
|
const AANTEKENING_TYPES: readonly AantekeningType[] = ['Specialisme', 'Aantekening'];
|
||||||
function toAantekening(n: AantekeningDto): Aantekening {
|
|
||||||
return {
|
/** Trust-boundary parse: an unrecognized type is an explicit Failure, never a silent cast. */
|
||||||
|
export function parseAantekening(n: AantekeningDto): Result<string, Aantekening> {
|
||||||
|
if (!n.type || !AANTEKENING_TYPES.includes(n.type as AantekeningType))
|
||||||
|
return err(`aantekening: unknown type ${n.type}`);
|
||||||
|
return ok({
|
||||||
type: n.type as AantekeningType,
|
type: n.type as AantekeningType,
|
||||||
omschrijving: n.omschrijving ?? '',
|
omschrijving: n.omschrijving ?? '',
|
||||||
datum: n.datum ?? '',
|
datum: n.datum ?? '',
|
||||||
};
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
111
src/docs/parse-dont-validate.mdx
Normal file
111
src/docs/parse-dont-validate.mdx
Normal file
@@ -0,0 +1,111 @@
|
|||||||
|
import { Meta } from '@storybook/addon-docs/blocks';
|
||||||
|
|
||||||
|
<Meta title="Foundations/Parse, don't validate" />
|
||||||
|
|
||||||
|
# Parse, don't validate
|
||||||
|
|
||||||
|
The wire is untrusted. A `boolean`/`string` field coming back from `fetch` is typed `unknown`
|
||||||
|
until something checks it — casting it away with `as` doesn't check anything, it just tells the
|
||||||
|
compiler to stop complaining. This repo's rule: every response crosses the FE⇄BE seam through a
|
||||||
|
hand-written `parse*` function that returns a `Result<string, T>` (`src/app/shared/kernel/fp.ts`).
|
||||||
|
Once you hold the parsed value, you never re-check it — the type _is_ the proof.
|
||||||
|
|
||||||
|
## Two places this shows up
|
||||||
|
|
||||||
|
**Value objects** (`src/app/registratie/domain/value-objects/`) parse a single user-entered
|
||||||
|
field — `Postcode`, `Uren`, `BigNummer` — from a raw string into a branded type.
|
||||||
|
|
||||||
|
**Boundary parsers** (`*.adapter.ts` in every `infrastructure/`) parse a whole DTO — or one
|
||||||
|
enum-ish field inside it — from the generated `ApiClient`'s response into the domain shape the
|
||||||
|
rest of the app trusts.
|
||||||
|
|
||||||
|
```ts
|
||||||
|
parsePostcode(raw) // Result<string, Postcode>
|
||||||
|
|> mapErr(toLocalizedMessage) // swap raw msg → UI copy
|
||||||
|
|> map(toDomain) // only runs on success
|
||||||
|
```
|
||||||
|
|
||||||
|
## The failure mode this closes: the silent `as` cast
|
||||||
|
|
||||||
|
An `as SomeUnion` cast on a wire value compiles even when the value doesn't match — the tag
|
||||||
|
just gets forwarded as-is, and something far away breaks on an "impossible" case. A validated
|
||||||
|
parse turns that into an explicit `Failure` at the boundary, right where the untrusted data
|
||||||
|
enters.
|
||||||
|
|
||||||
|
### Before/after: `big-register.adapter.ts`
|
||||||
|
|
||||||
|
```ts
|
||||||
|
// before — the wire's `type` string is trusted outright
|
||||||
|
function toAantekening(n: AantekeningDto): Aantekening {
|
||||||
|
return { type: n.type as AantekeningType, omschrijving: n.omschrijving ?? '', datum: n.datum ?? '' };
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
```ts
|
||||||
|
// after — an unrecognized type is a Result you can spec, not a silently-wrong tag
|
||||||
|
const AANTEKENING_TYPES: readonly AantekeningType[] = ['Specialisme', 'Aantekening'];
|
||||||
|
|
||||||
|
export function parseAantekening(n: AantekeningDto): Result<string, Aantekening> {
|
||||||
|
if (!n.type || !AANTEKENING_TYPES.includes(n.type as AantekeningType))
|
||||||
|
return err(`aantekening: unknown type ${n.type}`);
|
||||||
|
return ok({ type: n.type as AantekeningType, omschrijving: n.omschrijving ?? '', datum: n.datum ?? '' });
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The resource loader throws on `Failure`, which Angular's `resource()` turns into its error
|
||||||
|
state — the same `Failure` a `RemoteData` consumer already renders, no new plumbing.
|
||||||
|
|
||||||
|
### Before/after: `brief.adapter.ts`
|
||||||
|
|
||||||
|
```ts
|
||||||
|
// before — `dto.scope` is checked, then re-cast anyway
|
||||||
|
if (typeof dto.passageId !== 'string' || (dto.scope !== 'global' && dto.scope !== 'beroep'))
|
||||||
|
return err('passage: bad shape');
|
||||||
|
// … scope: dto.scope as PassageScope
|
||||||
|
```
|
||||||
|
|
||||||
|
```ts
|
||||||
|
// after — split the guard so TS narrows `scope` on its own; no cast needed
|
||||||
|
if (dto.scope !== 'global' && dto.scope !== 'beroep')
|
||||||
|
return err(`passage: unknown scope ${dto.scope}`);
|
||||||
|
// … scope: dto.scope // already narrowed to PassageScope
|
||||||
|
```
|
||||||
|
|
||||||
|
Splitting a compound `if` into two single-condition guards is often enough to make the cast
|
||||||
|
disappear entirely — the compiler was already able to prove the narrowing, the `||` was just
|
||||||
|
hiding it.
|
||||||
|
|
||||||
|
### Before/after: `intake-policy.adapter.ts`
|
||||||
|
|
||||||
|
```ts
|
||||||
|
// before — the resource exposes the raw DTO; consumers reach into it with `?.`
|
||||||
|
policyResource() {
|
||||||
|
return resource({ loader: () => this.client.policy() });
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
```ts
|
||||||
|
// after — a domain-side type + a validated parse; the resource never surfaces raw wire shape
|
||||||
|
export interface IntakePolicy { readonly scholingThreshold: number }
|
||||||
|
|
||||||
|
export function parseIntakePolicy(json: unknown): Result<string, IntakePolicy> {
|
||||||
|
if (typeof json !== 'object' || json === null) return err('intake-policy: not an object');
|
||||||
|
const dto = json as { scholingThreshold?: unknown };
|
||||||
|
if (typeof dto.scholingThreshold !== 'number')
|
||||||
|
return err('intake-policy: missing scholingThreshold');
|
||||||
|
return ok({ scholingThreshold: dto.scholingThreshold });
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## The sanctioned exception
|
||||||
|
|
||||||
|
Narrowing `unknown` to `Partial<Dto>` so you can *start* checking fields is fine — that's not a
|
||||||
|
trust decision, it's just giving the compiler a shape to probe (`const dto = json as
|
||||||
|
Partial<DashboardViewDto>`, see `dashboard-view.adapter.ts`). What's never fine is casting a
|
||||||
|
field to its final domain type without having checked it first.
|
||||||
|
|
||||||
|
## Spec every parser like a decision table
|
||||||
|
|
||||||
|
Each parser gets a spec covering: a valid shape, a missing required field, and — for
|
||||||
|
tagged/enum-ish values — an unknown tag. See `big-register.adapter.spec.ts`,
|
||||||
|
`intake-policy.adapter.spec.ts`, and the scope-rejection case in `brief.adapter.spec.ts`.
|
||||||
Reference in New Issue
Block a user