diff --git a/calm-ai/tools/pattern-creation.md b/calm-ai/tools/pattern-creation.md index b7bb5184c..f151579b5 100644 --- a/calm-ai/tools/pattern-creation.md +++ b/calm-ai/tools/pattern-creation.md @@ -98,6 +98,148 @@ Patterns use JSON schema constructs to provide choices and options: } ``` +### Optional Nodes with an `items` Catalog (zero or more) + +`prefixItems` describes fixed array positions: a slot is always present, and a `oneOf`/`anyOf` inside it chooses *which kind* of node fills that position. Use it when a node must exist and you are only choosing its type. + +When you instead want an **open catalog** of optional nodes — "include any combination of these, including none" — declare the candidates under `items` (which applies to every array entry) rather than `prefixItems` (which pins entries to positions). Mandatory nodes go in `prefixItems`; the optional catalog goes in `items`: + +```json +{ + "properties": { + "nodes": { + "type": "array", + "minItems": 2, + "prefixItems": [ + { + "$ref": "https://calm.finos.org/release/1.2/meta/core.json#/defs/node", + "type": "object", + "properties": { + "unique-id": { "const": "webapp" }, + "name": { "const": "Web Application" }, + "node-type": { "const": "webclient" } + } + }, + { + "$ref": "https://calm.finos.org/release/1.2/meta/core.json#/defs/node", + "type": "object", + "properties": { + "unique-id": { "const": "database" }, + "name": { "const": "Database" }, + "node-type": { "const": "database" } + } + } + ], + "items": { + "oneOf": [ + { + "$ref": "https://calm.finos.org/release/1.2/meta/core.json#/defs/node", + "type": "object", + "properties": { + "unique-id": { "const": "cache" }, + "name": { "const": "Cache" }, + "node-type": { "const": "service" } + } + }, + { + "$ref": "https://calm.finos.org/release/1.2/meta/core.json#/defs/node", + "type": "object", + "properties": { + "unique-id": { "const": "queue" }, + "name": { "const": "Message Queue" }, + "node-type": { "const": "service" } + } + } + ] + } + } + } +} +``` + +Here `webapp` and `database` are always present, while `cache` and `queue` form an optional catalog: an instantiated architecture may include neither, either, or both. + +A catalog on its own does nothing. Two different kinds of object are involved, and it is worth being precise about which is which: + +- A **candidate** is a concrete node or relationship that may or may not end up in the generated architecture. Candidates are what an `items` catalog holds. Relationship candidates can use an `items` catalog exactly as node candidates do. +- A **decision holder** is a relationship carrying `relationship-type.options`. It is not part of the architecture being described — it asks the user a question and lists the choice bundles that answer it. Each bundle names candidates by `unique-id`. + +A candidate is included in the output only when a chosen bundle names its `unique-id`. So every catalog needs a decision holder pointing at it, and **a decision holder must be declared in `properties.relationships.prefixItems`** — never inside an `items` catalog itself. A holder is the mechanism that drives generation, so it must always be present; putting it in a catalog makes the question itself optional, and `calm generate` will not offer it. + +### `oneOf` and `anyOf` mean different things in different places + +The same two keywords appear in three positions and do three different jobs. Getting this wrong is the commonest authoring mistake with catalogs. + +**Inside the holder's `options` — this is the cardinality.** Use `anyOf` for a zero-or-more catalog (the user may pick any combination, including none) and `oneOf` where exactly one candidate must be chosen. This is the only place that controls how many candidates the user may select. + +**Inside a `prefixItems` slot — this picks which node fills one fixed position.** The slot always exists; the alternatives compete for it. + +**Inside `items` — neither.** The keyword there constrains what each individual array *entry* may look like, not how many entries there are. `items: { "oneOf": [cache, queue] }` reads as "each entry must be exactly one of cache or queue" — an architecture containing *both* is perfectly valid, because each entry independently matches exactly one candidate. "Zero or more" comes from `items` itself (plus `minItems`/`maxItems`), and how many are actually selected comes from the holder. + +Because every candidate pins its `unique-id` with a `const`, an entry can match at most one candidate schema, so `oneOf` and `anyOf` accept exactly the same architectures here. **Use `oneOf`** — it is the accurate assertion and matches the example above. + +**Never declare both `oneOf` and `anyOf` on one `items` block.** Only the `oneOf` list is read. Candidates under `anyOf` are silently dropped: `calm generate` will still *offer* them if a decision names them, then discard your answer without an error, and they will not appear in the diagram either. + +The holder that drives the catalog above looks like this: + +```json +{ + "properties": { + "relationships": { + "type": "array", + "prefixItems": [ + { + "$ref": "https://calm.finos.org/release/1.2/meta/core.json#/defs/relationship", + "type": "object", + "properties": { + "unique-id": { "const": "optional-components" }, + "description": { "const": "Which optional components do you want?" }, + "relationship-type": { + "type": "object", + "properties": { + "options": { + "type": "array", + "prefixItems": [ + { + "anyOf": [ + { + "$ref": "https://calm.finos.org/release/1.2/meta/core.json#/defs/decision", + "type": "object", + "properties": { + "description": { "const": "Add a cache" }, + "nodes": { "const": ["cache"] }, + "relationships": { "const": [] } + } + }, + { + "$ref": "https://calm.finos.org/release/1.2/meta/core.json#/defs/decision", + "type": "object", + "properties": { + "description": { "const": "Add a message queue" }, + "nodes": { "const": ["queue"] }, + "relationships": { "const": [] } + } + } + ] + } + ] + } + } + } + } + } + ] + } + } +} +``` + +Guidance: + +- Keep every candidate that a single decision references within one declaration site. A decision whose candidates are split between a `prefixItems` slot and an `items` catalog, or spread across nodes with inconsistent container membership, is a pattern smell — model the choice at one consistent level. +- Declare every decision holder (a relationship with `relationship-type.options`) in `properties.relationships.prefixItems`. A catalog with no holder pointing at it can never be selected from, and `calm validate` warns that its candidates are unreferenced. +- Duplicate `unique-id`s inside an `items` catalog are rejected by `calm validate`, and a catalog node that no relationship or decision references produces a warning — the same check a plain `prefixItems` entry gets. A candidate declared as a `prefixItems[i].oneOf`/`anyOf` alternative does not get this warning — the rule reaches a plain `prefixItems` entry and an `items` catalog member, but not inside a slot's alternatives. + ### Relationship Options with Decision Points ```json @@ -618,7 +760,9 @@ Always use specific interface schema references: ### Array Handling -- Use `prefixItems` to define specific array positions +- Use `prefixItems` to define specific array positions (fixed slots) +- Use `items` to define an open catalog of optional entries (zero or more, any combination); combine with `prefixItems` for mandatory-plus-optional arrays +- Inside `items`, use `oneOf` (not both `oneOf` and `anyOf` — see above). The keyword constrains each entry's shape; it does **not** limit how many entries the array may hold - Use `minItems`/`maxItems` to constrain array sizes - Each array item should reference base schema + add constraints @@ -643,13 +787,16 @@ The CLI will prompt for choices when encountering `anyOf`/`oneOf` options, or yo - `const` - Fixed values that cannot be changed - `enum` - List of allowed values - `minItems`/`maxItems` - Array size constraints -- `prefixItems` - Define specific array items +- `prefixItems` - Define specific array items by position (fixed slots) +- `items` - Define the rule every array entry must satisfy; with a `oneOf`/`anyOf` inside it, an open catalog of optional entries (zero or more) ### Option Constructs - `anyOf` - One or more options can be true - `oneOf` - Exactly one option must be true - `allOf` - All conditions must be true +- Placed inside a `prefixItems` slot, `oneOf`/`anyOf` chooses which node fills that fixed position; placed inside `items`, they define the optional catalog an entry may be drawn from +- Do not split a single property's definition (e.g. `properties.nodes`) across separate `allOf` branches — the merge is shallow, so the later branch's definition replaces the earlier one wholesale rather than combining them. `allOf` is unsupported for `relationships`: decision holders are only discovered in `properties.relationships.prefixItems` on the raw pattern, before `allOf` is flattened ### Schema References @@ -666,7 +813,8 @@ The CLI will prompt for choices when encountering `anyOf`/`oneOf` options, or yo 4. Relationship definitions must use `$ref` to core relationship schema 5. Use `const` for fixed values, `anyOf`/`oneOf` for options 6. All constraint properties must be valid JSON schema constructs -7. Pattern should be testable with `calm validate -p ` +7. `unique-id`s must be unique across the whole pattern, including inside `items.oneOf`/`anyOf` catalogs +8. Pattern should be testable with `calm validate -p ` ## Best Practices diff --git a/calm-hub-ui/src/diff/components/utils/patternDiffTransformer.test.ts b/calm-hub-ui/src/diff/components/utils/patternDiffTransformer.test.ts index c511e4bb1..e8f9a846f 100644 --- a/calm-hub-ui/src/diff/components/utils/patternDiffTransformer.test.ts +++ b/calm-hub-ui/src/diff/components/utils/patternDiffTransformer.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { DiffResult } from '@finos/calm-models/diff'; +import { DiffResult, diffPatterns } from '@finos/calm-models/diff'; import { parsePatternDataWithDiff } from './patternDiffTransformer.js'; const pattern = { @@ -58,4 +58,43 @@ describe('patternDiffTransformer', () => { expect(added?.style).toMatchObject({ boxShadow: '0 0 0 3px #16a34a' }); expect(result.nodes.find((n) => n.id === 'api-gateway')?.data.diffStatus).toBe('unchanged'); }); + + it('marks a newly added items-catalog candidate as added, using the real diffPatterns output', () => { + const patternBeforeCatalog = { + properties: { + nodes: { + type: 'array', + prefixItems: [ + { properties: { 'unique-id': { const: 'api-gateway' }, name: { const: 'API Gateway' }, 'node-type': { const: 'service' } } }, + ], + }, + relationships: { type: 'array', prefixItems: [] }, + }, + }; + const patternWithCatalog = { + properties: { + nodes: { + type: 'array', + prefixItems: [ + { properties: { 'unique-id': { const: 'api-gateway' }, name: { const: 'API Gateway' }, 'node-type': { const: 'service' } } }, + ], + items: { + anyOf: [ + { properties: { 'unique-id': { const: 'redis' }, name: { const: 'Redis' }, 'node-type': { const: 'database' } } }, + ], + }, + }, + relationships: { type: 'array', prefixItems: [] }, + }, + }; + + // Not a hand-built DiffResult - this is the real diffPatterns output, so the + // test proves the pattern-diff reader and the graph transformer agree on catalog + // candidates, not just that applyDiffStatus honours whatever it's given. + const diffResult = diffPatterns(patternBeforeCatalog, patternWithCatalog); + const result = parsePatternDataWithDiff(patternWithCatalog, diffResult, false); + const redis = result.nodes.find((n) => n.id === 'redis'); + expect(redis).toBeDefined(); + expect(redis?.data.diffStatus).toBe('added'); + }); }); diff --git a/calm-hub-ui/src/visualizer/components/drawer/Drawer.test.tsx b/calm-hub-ui/src/visualizer/components/drawer/Drawer.test.tsx index 9bb1f74f0..79bf6c227 100644 --- a/calm-hub-ui/src/visualizer/components/drawer/Drawer.test.tsx +++ b/calm-hub-ui/src/visualizer/components/drawer/Drawer.test.tsx @@ -260,6 +260,28 @@ describe('Drawer', () => { expect(screen.getByTestId('reactflow-visualizer')).toBeInTheDocument(); }); + it('classifies a catalog-only pattern (nodes via items, no prefixItems) dropped as a file as a pattern', async () => { + render(); + + // A pattern whose nodes are declared solely through an `items` catalog has no + // `prefixItems`. It must still be routed to the PatternVisualizer, not the + // architecture ReactFlowVisualizer, on the file-upload path. + await act(async () => { + await mockDropzone.onDrop?.([ + fakeFile( + JSON.stringify({ + properties: { + nodes: { items: { oneOf: [{ properties: { 'unique-id': { const: 'cache' } } }] } }, + }, + }) + ), + ]); + }); + + expect(screen.getByTestId('pattern-visualizer')).toBeInTheDocument(); + expect(screen.queryByTestId('reactflow-visualizer')).not.toBeInTheDocument(); + }); + // Regression coverage for the dropped-file stale-layout bug: `defaultLayout`/ // `layoutEpoch` describe the currently-*loaded* resource's saved server layout, // so they must collapse alongside `viewportKey` once a file is dropped — diff --git a/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx b/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx index 75ad314f7..98af2b954 100644 --- a/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx +++ b/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx @@ -13,13 +13,19 @@ import type { DrawerProps, Flow, Control, Decorator } from '../../contracts/cont /** * Detect whether JSON data is a CALM pattern (JSON Schema) or an architecture instance. - * Patterns have properties.nodes.prefixItems; architectures have nodes directly. + * A pattern declares its nodes as a JSON Schema array — either as positional + * `prefixItems` slots or as an open `items` catalog (or both); an architecture has + * `nodes` directly as a plain array of instances. A catalog-only pattern has no + * `prefixItems`, so `items` must be accepted too, otherwise it would be misclassified + * as an architecture on the file-upload path (the Hub path is saved separately by the + * `calmType === 'Patterns'` fallback below). */ function isPatternData(data: unknown): boolean { if (!data || typeof data !== 'object') return false; const obj = data as Record; const props = obj['properties'] as Record | undefined; - return !!(props?.['nodes'] && typeof props['nodes'] === 'object' && (props['nodes'] as Record)['prefixItems']); + const nodes = props?.['nodes'] as Record | undefined; + return !!(nodes && typeof nodes === 'object' && (nodes['prefixItems'] || nodes['items'])); } /** diff --git a/calm-hub-ui/src/visualizer/components/reactflow/utils/decisionAgreement.test.ts b/calm-hub-ui/src/visualizer/components/reactflow/utils/decisionAgreement.test.ts new file mode 100644 index 000000000..0812185f7 --- /dev/null +++ b/calm-hub-ui/src/visualizer/components/reactflow/utils/decisionAgreement.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect } from 'vitest'; +import fs from 'fs'; +import path from 'path'; +import { parsePatternData } from './patternTransformer'; +import { extractDecisionPoints, getVisibleNodeIds, DecisionSelections } from './decisionUtils'; + +/** + * The visualiser half of the decision-agreement contract. See + * `test_fixtures/decision-agreement/README.md` - `shared` asserts the same two things + * against the same files, so a drift on either side fails a test. + */ + +const FIXTURES = path.resolve(__dirname, '../../../../../../test_fixtures/decision-agreement'); + +interface Expected { + decisions: { + optionId: string; + prompt: string; + optionType: 'oneOf' | 'anyOf'; + choices: { description: string; nodes: string[]; relationships: string[] }[]; + }[]; + answered: { choose: Record; nodes: string[] }[]; +} + +const read = (name: string, suffix: string) => + JSON.parse(fs.readFileSync(path.join(FIXTURES, `${name}.${suffix}.json`), 'utf8')); + +const cases = ['one-decision-one-catalog', 'two-decisions-one-catalog']; + +describe.each(cases)('decision agreement: %s (visualiser side)', (name) => { + const pattern = read(name, 'pattern'); + const expected: Expected = read(name, 'expected'); + + /** + * A decision box is identified by its prompt, not its group id. The id is an + * implementation detail; the prompt is what the fixture and the user both see. + */ + const pointsByPrompt = () => { + const { nodes } = parsePatternData(pattern); + const points = extractDecisionPoints(nodes); + return { nodes, points }; + }; + + it('draws exactly the expected decisions', () => { + const { points } = pointsByPrompt(); + const actual = points + .map((p) => ({ + prompt: p.prompt, + optionType: p.decisionType, + choices: p.choices.map((c) => ({ + description: c.description, + nodes: c.nodes, + relationships: c.relationships, + })), + })) + .sort((a, b) => a.prompt.localeCompare(b.prompt)); + + const wanted = expected.decisions + .map(({ prompt, optionType, choices }) => ({ prompt, optionType, choices })) + .sort((a, b) => a.prompt.localeCompare(b.prompt)); + + expect(actual).toEqual(wanted); + }); + + it.each(expected.answered)('shows the expected nodes for $choose', ({ choose, nodes: wanted }) => { + const { nodes, points } = pointsByPrompt(); + + const selections: DecisionSelections = new Map(); + Object.entries(choose).forEach(([optionId, description]) => { + // The fixture names decisions by the holder's unique-id. The visualiser + // keys them by group id, so match on the prompt the holder declares. + const expectedDecision = expected.decisions.find((d) => d.optionId === optionId)!; + const point = points.find((p) => p.prompt === expectedDecision.prompt); + if (!point) throw new Error(`no decision box for "${expectedDecision.prompt}"`); + const index = point.choices.findIndex((c) => c.description === description); + if (index < 0) throw new Error(`no choice "${description}" on "${point.prompt}"`); + selections.set(point.groupId, [index]); + }); + + const visible = getVisibleNodeIds(nodes, points, selections); + expect(visible, 'no filter was applied').not.toBeNull(); + + // Compare only real nodes. Box nodes are always visible and have no counterpart + // in a generated architecture. + const boxIds = new Set(nodes.filter((n) => n.type === 'group' || n.type === 'decisionGroup').map((n) => n.id)); + const actual = [...visible!].filter((id) => !boxIds.has(id)).sort(); + + expect(actual).toEqual([...wanted].sort()); + }); +}); diff --git a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts index 7e1aca850..5ca0c06a4 100644 --- a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts +++ b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts @@ -55,6 +55,61 @@ function makePattern( }; } +// Helper to build a pattern with both prefixItems (mandatory) and an +// items.oneOf/anyOf open catalog for nodes and/or relationships. +function makePatternWithItems( + prefixNodes: unknown[], + itemsCatalogNodes: unknown[] = [], + relationships: unknown[] = [], + itemsCatalogRelationships: unknown[] = [], + catalogType: 'oneOf' | 'anyOf' = 'oneOf' +) { + return { + properties: { + nodes: { + prefixItems: prefixNodes, + ...(itemsCatalogNodes.length > 0 && { items: { [catalogType]: itemsCatalogNodes } }), + }, + relationships: { + prefixItems: relationships, + ...(itemsCatalogRelationships.length > 0 && { items: { [catalogType]: itemsCatalogRelationships } }), + }, + }, + }; +} + +// Helper to build an options (decision) relationship schema item +function optionsRelationship( + uniqueId: string, + description: string, + choices: { description: string; nodes: string[]; relationships?: string[] }[], + optionType: 'oneOf' | 'anyOf' = 'oneOf' +) { + return { + properties: { + 'unique-id': { const: uniqueId }, + description: { const: description }, + 'relationship-type': { + properties: { + options: { + prefixItems: [ + { + [optionType]: choices.map((c) => ({ + properties: { + description: { const: c.description }, + nodes: { const: c.nodes }, + relationships: { const: c.relationships || [] }, + }, + })), + }, + ], + }, + }, + }, + }, + }; +} + describe('parsePatternData', () => { it('returns empty arrays for null data', () => { const result = parsePatternData(null as unknown as Record); @@ -127,6 +182,81 @@ describe('parsePatternData', () => { expect(groupNodes[0].data.decisionType).toBe('anyOf'); }); + it('creates a decision group box for an items catalog with no options relationship', () => { + // An items.oneOf catalog with no decision referencing it still renders as a + // oneOf-labelled group box (no prompt), exercising the extract-but-never-folded path. + const pattern = { + properties: { + nodes: { + items: { + oneOf: [ + schemaNode('cache', 'Cache', 'service'), + schemaNode('queue', 'Queue', 'service'), + ], + }, + }, + relationships: { prefixItems: [] }, + }, + }; + const result = parsePatternData(pattern); + + const groupNodes = result.nodes.filter((n) => n.type === 'decisionGroup'); + expect(groupNodes).toHaveLength(1); + expect(groupNodes[0].data.decisionType).toBe('oneOf'); + expect(groupNodes[0].data.prompt).toBeUndefined(); + + const regularNodes = result.nodes.filter((n) => n.type === 'custom'); + expect(regularNodes).toHaveLength(2); + expect(regularNodes.every((n) => n.parentId === groupNodes[0].id)).toBe(true); + }); + + it('creates an anyOf decision group for an items.anyOf node catalog', () => { + // The UI-side anyOf catalog path: nodes declared through items.anyOf must + // produce an anyOf-typed decision group whose candidates parent into it, + // mirroring the oneOf case above. + const pattern = makePatternWithItems( + [], + [ + schemaNode('redis', 'Redis', 'service'), + schemaNode('kafka', 'Kafka', 'service'), + ], + [], + [], + 'anyOf' + ); + const result = parsePatternData(pattern); + + const groupNodes = result.nodes.filter((n) => n.type === 'decisionGroup'); + expect(groupNodes).toHaveLength(1); + expect(groupNodes[0].data.decisionType).toBe('anyOf'); + + const regularNodes = result.nodes.filter((n) => n.type === 'custom'); + expect(regularNodes).toHaveLength(2); + expect(regularNodes.every((n) => n.parentId === groupNodes[0].id)).toBe(true); + }); + + it('renders edges from a relationships items catalog as dashed decision edges', () => { + // Relationships declared solely through an items.oneOf catalog flow through + // the `rel-decision-items` branch and carry a decisionGroupId, so their + // edges must render dashed (strokeDasharray '5,5') — unlike the solid edge a + // plain prefixItems connects relationship produces. + const pattern = makePatternWithItems( + [ + schemaNode('node-1', 'Node 1', 'service'), + schemaNode('node-2', 'Node 2', 'service'), + ], + [], + [], + [connectsRelationship('rel-cat', 'node-1', 'node-2')] + ); + const result = parsePatternData(pattern); + + expect(result.edges).toHaveLength(1); + expect(result.edges[0].source).toBe('node-1'); + expect(result.edges[0].target).toBe('node-2'); + expect(result.edges[0].style?.strokeDasharray).toBe('5,5'); + }); + it('creates edges from connects relationships', () => { const pattern = makePattern( [ @@ -400,6 +530,70 @@ describe('parsePatternData', () => { expect(groupNode?.data.choices[1].description).toBe('Use Option B'); }); + it('renders a decision candidate that is also a container child inside the container, not the choice box', () => { + // opt-a is both a oneOf decision candidate AND deployed inside the k8s + // container. The container must win: opt-a's parent is k8s. opt-b, which is + // not in any container, stays in the decision group. + const pattern = makePattern( + [ + schemaNode('k8s', 'Kubernetes', 'system'), + { + oneOf: [ + schemaNode('opt-a', 'Option A', 'service'), + schemaNode('opt-b', 'Option B', 'service'), + ], + }, + ], + [ + { + properties: { + 'unique-id': { const: 'deploy-a' }, + 'relationship-type': { + const: { 'deployed-in': { container: 'k8s', nodes: ['opt-a'] } }, + }, + }, + }, + ] + ); + const result = parsePatternData(pattern); + + expect(result.nodes.find((n) => n.id === 'opt-a')?.parentId).toBe('k8s'); + const groupNodes = result.nodes.filter((n) => n.type === 'decisionGroup'); + expect(groupNodes).toHaveLength(1); + expect(result.nodes.find((n) => n.id === 'opt-b')?.parentId).toBe(groupNodes[0].id); + }); + + it('does not render an empty decision box when every candidate is pulled into a container', () => { + // Both oneOf candidates are deployed inside k8s, so the decision group is + // emptied by container precedence and must not be drawn as an empty box. + const pattern = makePattern( + [ + schemaNode('k8s', 'Kubernetes', 'system'), + { + oneOf: [ + schemaNode('opt-a', 'Option A', 'service'), + schemaNode('opt-b', 'Option B', 'service'), + ], + }, + ], + [ + { + properties: { + 'unique-id': { const: 'deploy-both' }, + 'relationship-type': { + const: { 'deployed-in': { container: 'k8s', nodes: ['opt-a', 'opt-b'] } }, + }, + }, + }, + ] + ); + const result = parsePatternData(pattern); + + expect(result.nodes.filter((n) => n.type === 'decisionGroup')).toHaveLength(0); + expect(result.nodes.find((n) => n.id === 'opt-a')?.parentId).toBe('k8s'); + expect(result.nodes.find((n) => n.id === 'opt-b')?.parentId).toBe('k8s'); + }); + it('sets protocol on edges', () => { const pattern = makePattern( [ @@ -471,6 +665,77 @@ describe('parsePatternData', () => { const result = parsePatternData(pattern); expect(result.edges).toHaveLength(0); }); + + it('creates a decision group for a decision referencing only items-declared catalog candidates', () => { + const pattern = makePatternWithItems( + [schemaNode('webapp', 'Web App', 'service')], + [schemaNode('cache', 'Cache', 'service'), schemaNode('queue', 'Queue', 'service')], + [ + optionsRelationship('options-rel', 'Choose extras', [ + { description: 'Use Cache', nodes: ['cache'] }, + { description: 'Use Queue', nodes: ['queue'] }, + ]), + ] + ); + const result = parsePatternData(pattern); + + const groupNodes = result.nodes.filter((n) => n.type === 'decisionGroup'); + expect(groupNodes).toHaveLength(1); + expect(groupNodes[0].data.prompt).toBe('Choose extras'); + expect(groupNodes[0].data.choices).toHaveLength(2); + + const cacheNode = result.nodes.find((n) => n.id === 'cache'); + const queueNode = result.nodes.find((n) => n.id === 'queue'); + expect(cacheNode?.parentId).toBe(groupNodes[0].id); + expect(queueNode?.parentId).toBe(groupNodes[0].id); + }); + + it('folds a decision referencing a mix of prefixItems- and items-declared candidates into one group', () => { + const pattern = makePatternWithItems( + [ + { + oneOf: [ + schemaNode('option-a', 'Option A', 'service'), + schemaNode('option-b', 'Option B', 'service'), + ], + }, + ], + [schemaNode('cache', 'Cache', 'service')], + [ + optionsRelationship('options-rel', 'Choose a setup', [ + { description: 'Use A with cache', nodes: ['option-a', 'cache'] }, + { description: 'Use B', nodes: ['option-b'] }, + ]), + ] + ); + const result = parsePatternData(pattern); + + const groupNodes = result.nodes.filter((n) => n.type === 'decisionGroup'); + expect(groupNodes).toHaveLength(1); + + const optionA = result.nodes.find((n) => n.id === 'option-a'); + const optionB = result.nodes.find((n) => n.id === 'option-b'); + const cache = result.nodes.find((n) => n.id === 'cache'); + expect(optionA?.parentId).toBe(groupNodes[0].id); + expect(optionB?.parentId).toBe(groupNodes[0].id); + expect(cache?.parentId).toBe(groupNodes[0].id); + }); + + it('renders nothing for a decision referencing only a dangling/typo\'d id', () => { + const pattern = makePattern( + [schemaNode('webapp', 'Web App', 'service')], + [ + optionsRelationship('options-rel', 'Choose extras', [ + { description: 'Use nonexistent', nodes: ['nonexistent-node'] }, + ]), + ] + ); + const result = parsePatternData(pattern); + + const groupNodes = result.nodes.filter((n) => n.type === 'decisionGroup'); + expect(groupNodes).toHaveLength(0); + expect(result.nodes.find((n) => n.id === 'nonexistent-node')).toBeUndefined(); + }); }); describe('nested container ordering', () => { @@ -519,3 +784,227 @@ describe('nested container ordering', () => { expect(ids).toEqual(['A', 'B', 'C', 'system']); }); }); + +describe('decision groups are keyed per decision, not per declaration site', () => { + const cacheQueueCatalog = [ + schemaNode('redis', 'Redis', 'database'), + schemaNode('memcached', 'Memcached', 'database'), + schemaNode('kafka', 'Kafka', 'queue'), + schemaNode('rabbitmq', 'RabbitMQ', 'queue'), + ]; + + type RfNode = { id: string; type?: string; parentId?: string; data: Record }; + + /** The prompt on each rendered decision box, sorted. */ + const prompts = (nodes: RfNode[]) => + nodes.filter((n) => n.type === 'decisionGroup').map((n) => n.data.prompt as unknown as string).sort(); + + /** The box a candidate was placed in. Fails the test if it is in none. */ + const boxOf = (nodes: RfNode[], id: string): string => { + const parentId = nodes.find((n) => n.id === id)?.parentId; + expect(parentId, `${id} is in no decision box`).toBeDefined(); + return parentId as string; + }; + + /** The choice descriptions on the box carrying a given prompt. */ + const choicesFor = (nodes: RfNode[], prompt: string): string[] => { + const box = nodes.find((n) => n.type === 'decisionGroup' && n.data.prompt === prompt); + expect(box, `no decision box with prompt "${prompt}"`).toBeDefined(); + return (box!.data.choices as unknown as { description: string }[]).map((c) => c.description); + }; + + const twoDecisionPattern = (relationships: unknown[]) => + makePatternWithItems( + [schemaNode('webapp', 'Web App', 'service')], + cacheQueueCatalog, + relationships, + [], + 'anyOf' + ); + + const cacheDecision = optionsRelationship('cache-choice', 'Pick a cache', [ + { description: 'Use Redis', nodes: ['redis'] }, + { description: 'Use Memcached', nodes: ['memcached'] }, + ]); + const queueDecision = optionsRelationship('queue-choice', 'Pick a queue', [ + { description: 'Use Kafka', nodes: ['kafka'] }, + { description: 'Use RabbitMQ', nodes: ['rabbitmq'] }, + ]); + + it('renders one box per decision when two decisions draw from one catalog', () => { + const result = parsePatternData(twoDecisionPattern([cacheDecision, queueDecision])) as { nodes: RfNode[] }; + + expect(prompts(result.nodes)).toEqual(['Pick a cache', 'Pick a queue']); + + // Each box carries its own choices, not the other decision's. + expect(choicesFor(result.nodes, 'Pick a cache')).toEqual(['Use Redis', 'Use Memcached']); + expect(choicesFor(result.nodes, 'Pick a queue')).toEqual(['Use Kafka', 'Use RabbitMQ']); + + // Every candidate is drawn, and each decision's candidates share one box. + ['redis', 'memcached', 'kafka', 'rabbitmq'].forEach((id) => + expect(result.nodes.find((n) => n.id === id), id).toBeDefined() + ); + expect(boxOf(result.nodes, 'redis')).toBe(boxOf(result.nodes, 'memcached')); + expect(boxOf(result.nodes, 'kafka')).toBe(boxOf(result.nodes, 'rabbitmq')); + expect(boxOf(result.nodes, 'redis')).not.toBe(boxOf(result.nodes, 'kafka')); + }); + + it('still renders one box when one decision draws from one catalog', () => { + const pattern = makePatternWithItems( + [schemaNode('webapp', 'Web App', 'service')], + [schemaNode('redis', 'Redis', 'database'), schemaNode('memcached', 'Memcached', 'database')], + [cacheDecision], + [], + 'anyOf' + ); + const result = parsePatternData(pattern) as { nodes: RfNode[] }; + + expect(prompts(result.nodes)).toEqual(['Pick a cache']); + expect(choicesFor(result.nodes, 'Pick a cache')).toEqual(['Use Redis', 'Use Memcached']); + expect(boxOf(result.nodes, 'redis')).toBe(boxOf(result.nodes, 'memcached')); + }); + + it('gives a shared candidate to the first decision and still renders the second', () => { + // memcached is named by both. A node has one parent, so the first decision + // keeps it and the second renders with what is left. + const storeDecision = optionsRelationship('store-choice', 'Pick a store', [ + { description: 'Use Memcached', nodes: ['memcached'] }, + { description: 'Use Kafka', nodes: ['kafka'] }, + ]); + // Exactly the three candidates the two decisions name. A fourth, unreferenced + // candidate would keep its own declaration-site box and is not what this pins. + const pattern = makePatternWithItems( + [schemaNode('webapp', 'Web App', 'service')], + [ + schemaNode('redis', 'Redis', 'database'), + schemaNode('memcached', 'Memcached', 'database'), + schemaNode('kafka', 'Kafka', 'queue'), + ], + [cacheDecision, storeDecision], + [], + 'anyOf' + ); + const result = parsePatternData(pattern) as { nodes: RfNode[] }; + + expect(prompts(result.nodes)).toEqual(['Pick a cache', 'Pick a store']); + expect(boxOf(result.nodes, 'memcached')).toBe(boxOf(result.nodes, 'redis')); + expect(boxOf(result.nodes, 'kafka')).not.toBe(boxOf(result.nodes, 'redis')); + + // The second box still offers the shared candidate as a choice. Only the + // drawing is exclusive, not the decision. + expect(choicesFor(result.nodes, 'Pick a store')).toEqual(['Use Memcached', 'Use Kafka']); + }); + + it('is declaration order that decides which decision keeps a shared candidate', () => { + const storeDecision = optionsRelationship('store-choice', 'Pick a store', [ + { description: 'Use Memcached', nodes: ['memcached'] }, + { description: 'Use Kafka', nodes: ['kafka'] }, + ]); + const pattern = makePatternWithItems( + [schemaNode('webapp', 'Web App', 'service')], + [ + schemaNode('redis', 'Redis', 'database'), + schemaNode('memcached', 'Memcached', 'database'), + schemaNode('kafka', 'Kafka', 'queue'), + ], + [storeDecision, cacheDecision], + [], + 'anyOf' + ); + const reversed = parsePatternData(pattern) as { nodes: RfNode[] }; + + // Same two decisions, declared the other way round: now the store box keeps it. + expect(boxOf(reversed.nodes, 'memcached')).toBe(boxOf(reversed.nodes, 'kafka')); + expect(boxOf(reversed.nodes, 'memcached')).not.toBe(boxOf(reversed.nodes, 'redis')); + }); + + it('renders no box for a decision whose candidates are all claimed by an earlier one', () => { + // A documented limit, not a fix. Boxing one node twice needs #2933. + const pattern = makePatternWithItems( + [schemaNode('webapp', 'Web App', 'service')], + [schemaNode('redis', 'Redis', 'database')], + [ + optionsRelationship('cache-choice', 'Pick a cache', [ + { description: 'Use Redis', nodes: ['redis'] }, + ]), + optionsRelationship('store-choice', 'Pick a store', [ + { description: 'Use Redis', nodes: ['redis'] }, + ]), + ], + [], + 'anyOf' + ); + const result = parsePatternData(pattern) as { nodes: RfNode[] }; + + expect(prompts(result.nodes)).toEqual(['Pick a cache']); + }); +}); + +describe('decisions and containers', () => { + type RfNode = { id: string; type?: string; parentId?: string; data: Record }; + + const deployedIn = (uniqueId: string, container: string, nodes: string[]) => ({ + properties: { + 'unique-id': { const: uniqueId }, + 'relationship-type': { const: { 'deployed-in': { container, nodes } } }, + }, + }); + + it('keeps the decision box when every candidate is itself a container', () => { + // opt-a and opt-b each contain a leaf, so both are containers. The box must + // still carry the decision text. Nesting them inside it is #2933. + const pattern = makePattern( + [ + schemaNode('opt-a', 'Option A', 'system'), + schemaNode('opt-b', 'Option B', 'system'), + schemaNode('leaf-a', 'Leaf A', 'service'), + schemaNode('leaf-b', 'Leaf B', 'service'), + ], + [ + deployedIn('deploy-a', 'opt-a', ['leaf-a']), + deployedIn('deploy-b', 'opt-b', ['leaf-b']), + optionsRelationship('subsystem-choice', 'Pick a subsystem', [ + { description: 'Use A', nodes: ['opt-a'] }, + { description: 'Use B', nodes: ['opt-b'] }, + ]), + ] + ); + const result = parsePatternData(pattern) as { nodes: RfNode[] }; + + const groups = result.nodes.filter((n) => n.type === 'decisionGroup'); + expect(groups).toHaveLength(1); + expect(groups[0].data.prompt).toBe('Pick a subsystem'); + + // The containers must NOT be nested inside the box - that is #2933's change. + expect(result.nodes.find((n) => n.id === 'opt-a')?.parentId).toBeUndefined(); + expect(result.nodes.find((n) => n.id === 'opt-b')?.parentId).toBeUndefined(); + + // Containment is unchanged. + expect(result.nodes.find((n) => n.id === 'leaf-a')?.parentId).toBe('opt-a'); + expect(result.nodes.find((n) => n.id === 'leaf-b')?.parentId).toBe('opt-b'); + }); + + it('still suppresses the box when every candidate is pulled into a container', () => { + // The other branch: the candidates are container children, not containers. + // Suppression stays, even though a prompt exists to lose. + const pattern = makePattern( + [ + schemaNode('k8s', 'Kubernetes', 'system'), + schemaNode('opt-a', 'Option A', 'service'), + schemaNode('opt-b', 'Option B', 'service'), + ], + [ + deployedIn('deploy-both', 'k8s', ['opt-a', 'opt-b']), + optionsRelationship('svc-choice', 'Pick a service', [ + { description: 'Use A', nodes: ['opt-a'] }, + { description: 'Use B', nodes: ['opt-b'] }, + ]), + ] + ); + const result = parsePatternData(pattern) as { nodes: RfNode[] }; + + expect(result.nodes.filter((n) => n.type === 'decisionGroup')).toHaveLength(0); + expect(result.nodes.find((n) => n.id === 'opt-a')?.parentId).toBe('k8s'); + expect(result.nodes.find((n) => n.id === 'opt-b')?.parentId).toBe('k8s'); + }); +}); diff --git a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts index 08160fa02..bc984d8ca 100644 --- a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts +++ b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts @@ -9,6 +9,7 @@ import { import { createEdge } from './edgeFactory'; import { GRAPH_LAYOUT } from './constants'; import { THEME } from '../theme'; +import { getPatternArray, resolveOperativeChoiceBlock } from '@finos/calm-models/pattern'; /** * Result of parsing pattern data into ReactFlow elements @@ -26,20 +27,20 @@ type SchemaObject = Record; /** * Gets the prefixItems for a given top-level key (e.g. 'nodes' or 'relationships') + * from a pattern, handling allOf structures. Absent prefixItems yields an empty + * array so callers can iterate unconditionally. + */ +function getPrefixItems(pattern: SchemaObject, key: 'nodes' | 'relationships'): SchemaObject[] { + return getPatternArray(pattern, key).prefixItems as SchemaObject[]; +} + +/** + * Gets the `items` catalog schema (the `items.oneOf`/`items.anyOf` open-catalog + * declaration) for a given top-level key (e.g. 'nodes' or 'relationships') * from a pattern, handling allOf structures. */ -function getPrefixItems(pattern: SchemaObject, key: string): SchemaObject[] { - if (pattern['properties']?.[key]?.['prefixItems']) { - return pattern['properties'][key]['prefixItems']; - } - if (pattern['allOf'] && Array.isArray(pattern['allOf'])) { - for (const schema of pattern['allOf']) { - if (schema['properties']?.[key]?.['prefixItems']) { - return schema['properties'][key]['prefixItems']; - } - } - } - return []; +function getItems(pattern: SchemaObject, key: 'nodes' | 'relationships'): SchemaObject | undefined { + return getPatternArray(pattern, key).catalog as SchemaObject | undefined; } /** @@ -166,33 +167,43 @@ interface DecisionGroup { nodeIds: string[]; } +/** + * Extracts a set of oneOf/anyOf node alternatives into ExtractedNodes, stamping + * them all with the same decisionGroupId and recording a DecisionGroup for them. + */ +function extractNodeDecisionGroup( + alternatives: SchemaObject[], + groupId: string, + groupType: 'oneOf' | 'anyOf', + nodes: ExtractedNode[], + decisionGroups: DecisionGroup[], +): void { + const groupNodeIds: string[] = []; + + alternatives.forEach((alt: SchemaObject) => { + const node = extractNodeFromSchemaItem(alt); + if (node) { + node.decisionGroupId = groupId; + nodes.push(node); + groupNodeIds.push(node.uniqueId); + } + }); + + if (groupNodeIds.length > 0) { + decisionGroups.push({ groupId, groupType, nodeIds: groupNodeIds }); + } +} + function extractNodesFromPattern(pattern: SchemaObject): { nodes: ExtractedNode[]; decisionGroups: DecisionGroup[] } { const prefixItems = getPrefixItems(pattern, 'nodes'); + const items = getItems(pattern, 'nodes'); const nodes: ExtractedNode[] = []; const decisionGroups: DecisionGroup[] = []; prefixItems.forEach((item: SchemaObject, index: number) => { - const hasOneOf = Array.isArray(item['oneOf']); - const hasAnyOf = Array.isArray(item['anyOf']); - - if (hasOneOf || hasAnyOf) { - const groupType: 'oneOf' | 'anyOf' = hasOneOf ? 'oneOf' : 'anyOf'; - const alternatives: SchemaObject[] = hasOneOf ? item['oneOf'] : item['anyOf']; - const groupId = `node-decision-${index}`; - const groupNodeIds: string[] = []; - - alternatives.forEach((alt: SchemaObject) => { - const node = extractNodeFromSchemaItem(alt); - if (node) { - node.decisionGroupId = groupId; - nodes.push(node); - groupNodeIds.push(node.uniqueId); - } - }); - - if (groupNodeIds.length > 0) { - decisionGroups.push({ groupId, groupType, nodeIds: groupNodeIds }); - } + const block = resolveOperativeChoiceBlock(item); + if (block) { + extractNodeDecisionGroup(block.alternatives as SchemaObject[], `node-decision-${index}`, block.groupType, nodes, decisionGroups); } else { const node = extractNodeFromSchemaItem(item); if (node) { @@ -201,6 +212,16 @@ function extractNodesFromPattern(pattern: SchemaObject): { nodes: ExtractedNode[ } }); + // Open catalog: `items.oneOf`/`items.anyOf` declares zero-or-more candidates + // that aren't tied to a specific positional slot. Treat the whole catalog as + // a single decision-group slot. + if (items) { + const catalog = resolveOperativeChoiceBlock(items); + if (catalog) { + extractNodeDecisionGroup(catalog.alternatives as SchemaObject[], 'node-decision-items', catalog.groupType, nodes, decisionGroups); + } + } + return { nodes, decisionGroups }; } @@ -226,6 +247,7 @@ interface OptionsMetadata { prompt: string; optionType: 'oneOf' | 'anyOf'; choices: { description: string; nodes: string[]; relationships: string[] }[]; + relationshipId: string; } function extractRelTypeFromConst(relTypeConst: SchemaObject): Omit | null { @@ -286,17 +308,17 @@ function extractSingleRelationship(item: SchemaObject): ExtractedRelationship | function extractOptionsMetadata(item: SchemaObject): OptionsMetadata | null { const prompt = readSchemaValue(item, 'description') || 'Decision'; + const relationshipId = readSchemaValue(item, 'unique-id') || ''; const optionsPrefixItems: SchemaObject[] = item['properties']?.['relationship-type']?.['properties']?.['options']?.['prefixItems'] || []; for (const prefixItem of optionsPrefixItems) { - const hasOneOf = Array.isArray(prefixItem['oneOf']); - const hasAnyOf = Array.isArray(prefixItem['anyOf']); + const block = resolveOperativeChoiceBlock(prefixItem); - if (hasOneOf || hasAnyOf) { - const optionType: 'oneOf' | 'anyOf' = hasOneOf ? 'oneOf' : 'anyOf'; - const alternatives: SchemaObject[] = hasOneOf ? prefixItem['oneOf'] : prefixItem['anyOf']; + if (block) { + const optionType = block.groupType; + const alternatives = block.alternatives as SchemaObject[]; const choices = alternatives .map((alt: SchemaObject) => { @@ -312,18 +334,38 @@ function extractOptionsMetadata(item: SchemaObject): OptionsMetadata | null { .filter((c) => c.description); if (choices.length > 0) { - return { prompt, optionType, choices }; + return { prompt, optionType, choices, relationshipId }; } } } return null; } +/** + * Extracts a set of oneOf/anyOf relationship alternatives, stamping them all + * with the same decisionGroupId (relationship-side grouping only affects edge + * color/dash — there is no relationship decision-group box). + */ +function extractRelationshipDecisionGroup( + alternatives: SchemaObject[], + groupId: string, + relationships: ExtractedRelationship[], +): void { + alternatives.forEach((alt: SchemaObject) => { + const rel = extractSingleRelationship(alt); + if (rel) { + rel.decisionGroupId = groupId; + relationships.push(rel); + } + }); +} + function extractRelationshipsFromPattern(pattern: SchemaObject): { relationships: ExtractedRelationship[]; optionsMetadata: OptionsMetadata[]; } { const prefixItems = getPrefixItems(pattern, 'relationships'); + const items = getItems(pattern, 'relationships'); const relationships: ExtractedRelationship[] = []; const optionsMetadata: OptionsMetadata[] = []; @@ -338,20 +380,9 @@ function extractRelationshipsFromPattern(pattern: SchemaObject): { } // Check for oneOf/anyOf wrapped relationships - const hasOneOf = Array.isArray(item['oneOf']); - const hasAnyOf = Array.isArray(item['anyOf']); - - if (hasOneOf || hasAnyOf) { - const alternatives: SchemaObject[] = hasOneOf ? item['oneOf'] : item['anyOf']; - const groupId = `rel-decision-${index}`; - - alternatives.forEach((alt: SchemaObject) => { - const rel = extractSingleRelationship(alt); - if (rel) { - rel.decisionGroupId = groupId; - relationships.push(rel); - } - }); + const block = resolveOperativeChoiceBlock(item); + if (block) { + extractRelationshipDecisionGroup(block.alternatives as SchemaObject[], `rel-decision-${index}`, relationships); return; } @@ -362,6 +393,15 @@ function extractRelationshipsFromPattern(pattern: SchemaObject): { } }); + // Open catalog: `items.oneOf`/`items.anyOf` declares zero-or-more relationship + // candidates not tied to a specific positional slot. + if (items) { + const catalog = resolveOperativeChoiceBlock(items); + if (catalog) { + extractRelationshipDecisionGroup(catalog.alternatives as SchemaObject[], 'rel-decision-items', relationships); + } + } + return { relationships, optionsMetadata }; } @@ -398,8 +438,34 @@ function createReactFlowNodes( const groupNodes: Node[] = []; const { containerNodeIds, parentMap } = containerInfo; - // Create decision group parent nodes + // Determine each non-container node's parent. A container (deployed-in / + // composed-of) takes precedence over a decision group: a node that is both an + // optional decision candidate AND a container child renders inside its + // container, not in the choice box. Track which decision groups actually keep + // at least one child after that precedence is applied. + const effectiveParent = new Map(); + const usedDecisionGroupIds = new Set(); + extractedNodes.forEach((node) => { + if (containerNodeIds.has(node.uniqueId)) { + // A container candidate is not drawn inside the box, but the decision is + // still real, so the box must survive to carry its prompt. Nesting the + // container inside the box is #2933. + if (node.decisionGroupId) usedDecisionGroupIds.add(node.decisionGroupId); + return; + } + const parentId = parentMap.get(node.uniqueId) || node.decisionGroupId; + if (!parentId) return; + effectiveParent.set(node.uniqueId, parentId); + if (parentId === node.decisionGroupId) { + usedDecisionGroupIds.add(node.decisionGroupId); + } + }); + + // Create decision group parent nodes — but only for groups that still have at + // least one child after container precedence is applied, so a group whose every + // member was pulled into a container does not render as an empty box. decisionGroups.forEach((group) => { + if (!usedDecisionGroupIds.has(group.groupId)) return; const optionsMeta = groupOptionsMap?.get(group.groupId); groupNodes.push({ id: group.groupId, @@ -453,8 +519,7 @@ function createReactFlowNodes( extractedNodes.forEach((node) => { if (containerNodeIds.has(node.uniqueId)) return; // already a group node - // Determine parent: decision group takes precedence, then container - const parentId = node.decisionGroupId || parentMap.get(node.uniqueId); + const parentId = effectiveParent.get(node.uniqueId); regularNodes.push({ id: node.uniqueId, @@ -621,6 +686,95 @@ function applyPatternLayout(regularNodes: Node[], groupNodes: Node[], edges: Edg return { nodes: allNodes, edges }; } +// ---- Decision group / options-metadata folding ---- + +/** + * Folds each options-relationship decision's referenced node ids into a single + * decision group, mutating `decisionGroups`/`extractedNodes` in place: + * + * - Every decision gets its own new group (id derived from the options + * relationship's own unique-id). Each referenced id is moved into that new + * group, out of whatever group it previously belonged to (built during node + * extraction, e.g. a prefixItems oneOf slot or the items catalog, or an + * earlier decision processed in this same pass). + * - A candidate can be drawn in one box only: once an id has been claimed by a + * decision, a later decision naming the same id does not draw it again, so + * that later decision's group contains only the ids that are still free. + * - Ids that don't resolve to a real extracted node (dangling/typo'd + * references) are dropped; if a decision ends up with no valid ids at all, + * it is skipped entirely so no empty box is rendered. + * + * Returns the map from decision-group id to the OptionsMetadata that targets it. + */ +function foldOptionsMetadataIntoDecisionGroups( + extractedNodes: ExtractedNode[], + decisionGroups: DecisionGroup[], + optionsMetadata: OptionsMetadata[], +): Map { + const nodeToGroupMap = new Map(); + decisionGroups.forEach((g) => g.nodeIds.forEach((nid) => nodeToGroupMap.set(nid, g.groupId))); + + const extractedNodeIds = new Set(extractedNodes.map((n) => n.uniqueId)); + const nodesById = new Map(extractedNodes.map((n) => [n.uniqueId, n])); + const groupsById = new Map(decisionGroups.map((g) => [g.groupId, g])); + + const groupOptionsMap = new Map(); + + // A candidate can be drawn in one box only, so the first decision to name it keeps it. + const claimedByDecision = new Set(); + + optionsMetadata.forEach((meta) => { + const referencedIds = Array.from(new Set(meta.choices.flatMap((c) => c.nodes))).filter( + (id) => extractedNodeIds.has(id) && !claimedByDecision.has(id) + ); + + // Nothing left to draw: either the ids are dangling, or an earlier decision has + // them all. Boxing one node twice needs the nesting rework (#2933). + if (referencedIds.length === 0) return; + + const targetGroup: DecisionGroup = { + groupId: `node-decision-${meta.relationshipId || referencedIds.join('-')}`, + groupType: meta.optionType, + nodeIds: [], + }; + decisionGroups.push(targetGroup); + groupsById.set(targetGroup.groupId, targetGroup); + + referencedIds.forEach((id) => { + const currentGroupId = nodeToGroupMap.get(id); + if (currentGroupId) { + const oldGroup = groupsById.get(currentGroupId); + if (oldGroup) { + oldGroup.nodeIds = oldGroup.nodeIds.filter((nid) => nid !== id); + } + } + + targetGroup.nodeIds.push(id); + nodeToGroupMap.set(id, targetGroup.groupId); + claimedByDecision.add(id); + + const node = nodesById.get(id); + if (node) node.decisionGroupId = targetGroup.groupId; + }); + + groupOptionsMap.set(targetGroup.groupId, meta); + }); + + // Drop any groups emptied out by the folding above so no empty box renders. + const emptiedGroupIds = new Set( + decisionGroups.filter((g) => g.nodeIds.length === 0).map((g) => g.groupId) + ); + if (emptiedGroupIds.size > 0) { + for (let i = decisionGroups.length - 1; i >= 0; i--) { + if (emptiedGroupIds.has(decisionGroups[i].groupId)) { + decisionGroups.splice(i, 1); + } + } + } + + return groupOptionsMap; +} + // ---- Public API ---- /** @@ -633,22 +787,10 @@ export function parsePatternData(pattern: SchemaObject): ParsedPatternData { const { nodes: extractedNodes, decisionGroups } = extractNodesFromPattern(pattern); const { relationships, optionsMetadata } = extractRelationshipsFromPattern(pattern); - // Build map from node ID to its decision group for options metadata mapping - const nodeToGroupMap = new Map(); - decisionGroups.forEach((g) => g.nodeIds.forEach((nid) => nodeToGroupMap.set(nid, g.groupId))); - - // Map options metadata to the decision groups they target - const groupOptionsMap = new Map(); - optionsMetadata.forEach((meta) => { - const allNodes = meta.choices.flatMap((c) => c.nodes); - for (const nodeId of allNodes) { - const groupId = nodeToGroupMap.get(nodeId); - if (groupId && !groupOptionsMap.has(groupId)) { - groupOptionsMap.set(groupId, meta); - break; - } - } - }); + // Fold each decision's referenced node ids into a single decision group + // (creating one if none of its ids already belong to one) and build the + // map from decision-group id to the OptionsMetadata that targets it. + const groupOptionsMap = foldOptionsMetadataIntoDecisionGroups(extractedNodes, decisionGroups, optionsMetadata); const containerInfo = buildContainerInfo(relationships); const { regularNodes, groupNodes } = createReactFlowNodes( diff --git a/calm-models/package.json b/calm-models/package.json index ff29f317f..ebfb15691 100644 --- a/calm-models/package.json +++ b/calm-models/package.json @@ -22,6 +22,11 @@ "types": "./dist/diff/index.d.ts", "import": "./dist/diff/index.js", "default": "./dist/diff/index.js" + }, + "./pattern": { + "types": "./dist/pattern/index.d.ts", + "import": "./dist/pattern/index.js", + "default": "./dist/pattern/index.js" } }, "files": [ diff --git a/calm-models/src/diff/fixtures/diff-test-patterns.json b/calm-models/src/diff/fixtures/diff-test-patterns.json index 8a25df1f4..652573020 100644 --- a/calm-models/src/diff/fixtures/diff-test-patterns.json +++ b/calm-models/src/diff/fixtures/diff-test-patterns.json @@ -490,5 +490,74 @@ "prefixItems": [] } } + }, + "catalogBasePattern": { + "$schema": "https://calm.finos.org/release/1.0-rc2/meta/calm.json", + "type": "object", + "title": "Catalog Base Pattern", + "properties": { + "nodes": { + "type": "array", + "prefixItems": [ + { + "properties": { + "unique-id": { "const": "webapp" }, + "name": { "const": "Web App" }, + "node-type": { "const": "service" } + } + } + ], + "items": { + "anyOf": [ + { + "properties": { + "unique-id": { "const": "redis" }, + "name": { "const": "Redis" }, + "node-type": { "const": "database" } + } + } + ] + } + }, + "relationships": { "type": "array", "prefixItems": [] } + } + }, + "catalogAdditionPattern": { + "$schema": "https://calm.finos.org/release/1.0-rc2/meta/calm.json", + "type": "object", + "title": "Catalog Addition Pattern", + "properties": { + "nodes": { + "type": "array", + "prefixItems": [ + { + "properties": { + "unique-id": { "const": "webapp" }, + "name": { "const": "Web App" }, + "node-type": { "const": "service" } + } + } + ], + "items": { + "anyOf": [ + { + "properties": { + "unique-id": { "const": "redis" }, + "name": { "const": "Redis" }, + "node-type": { "const": "database" } + } + }, + { + "properties": { + "unique-id": { "const": "valkey" }, + "name": { "const": "Valkey" }, + "node-type": { "const": "database" } + } + } + ] + } + }, + "relationships": { "type": "array", "prefixItems": [] } + } } } diff --git a/calm-models/src/diff/pattern-diff.spec.ts b/calm-models/src/diff/pattern-diff.spec.ts index 8f6f50936..293a12379 100644 --- a/calm-models/src/diff/pattern-diff.spec.ts +++ b/calm-models/src/diff/pattern-diff.spec.ts @@ -55,6 +55,11 @@ describe('normalisePatternToInstance', () => { const { nodes } = normalisePatternToInstance(testPatterns.undiffablePattern); expect(nodes.map((n) => n['unique-id'])).toEqual(['api-gateway']); }); + + it('includes items-catalog candidates alongside prefixItems', () => { + const { nodes } = normalisePatternToInstance(testPatterns.catalogBasePattern); + expect(nodes.map((n) => n['unique-id'])).toEqual(['webapp', 'redis']); + }); }); describe('diffPatterns', () => { @@ -148,4 +153,25 @@ describe('diffPatterns', () => { const result = diffPatterns(testPatterns.basePattern, testPatterns.additionPattern); expect(result.undiffableItems).toBeUndefined(); }); + + it('detects a candidate added to an items catalog', () => { + const result = diffPatterns(testPatterns.catalogBasePattern, testPatterns.catalogAdditionPattern); + const addedIds = result.nodesAdded.map((n) => n['unique-id']); + expect(addedIds).toContain('valkey'); + expect(addedIds).not.toContain('redis'); + }); + + it('detects a candidate removed from an items catalog', () => { + const result = diffPatterns(testPatterns.catalogAdditionPattern, testPatterns.catalogBasePattern); + const removedIds = result.nodesRemoved.map((n) => n['unique-id']); + expect(removedIds).toContain('valkey'); + expect(removedIds).not.toContain('redis'); + }); + + it('treats an unchanged catalog candidate as same, not added', () => { + const result = diffPatterns(testPatterns.catalogBasePattern, testPatterns.catalogBasePattern); + const sameIds = result.nodesSame.map((n) => n['unique-id']); + expect(sameIds).toContain('redis'); + expect(result.nodesAdded).toHaveLength(0); + }); }); diff --git a/calm-models/src/diff/pattern-diff.ts b/calm-models/src/diff/pattern-diff.ts index 520701606..c5d5b26c1 100644 --- a/calm-models/src/diff/pattern-diff.ts +++ b/calm-models/src/diff/pattern-diff.ts @@ -1,6 +1,7 @@ import type { CalmNodeSchema, CalmRelationshipSchema } from '../types/index.js'; import type { NodesAndRelationshipsDiffResult } from './diff-types.js'; import { canonicalKey, diffNodesAndRelationships } from './diff.js'; +import { getPatternArray } from '../pattern/pattern-reader.js'; type SchemaObject = Record; @@ -40,28 +41,15 @@ function collapseSchema(schema: unknown): unknown { } /** - * Reads the `prefixItems` for a top-level pattern field (e.g. `nodes`, - * `relationships`), handling both direct `properties` and `allOf` wrapping. + * Reads the declared candidates for a top-level pattern field (e.g. `nodes`, + * `relationships`): its `prefixItems` slots plus, if present, its `items` open + * catalog, appended as one more block for {@link expandAlternatives} to unpack - + * a catalog is itself a `oneOf`/`anyOf` block, the same shape a decision slot is. + * Delegates to `getPatternArray` for the `properties`/`allOf` resolution. */ -function getPrefixItems(pattern: SchemaObject, key: string): SchemaObject[] { - const direct = isObject(pattern['properties']) - ? pattern['properties'][key] - : undefined; - if (isObject(direct) && Array.isArray(direct['prefixItems'])) { - return direct['prefixItems'] as SchemaObject[]; - } - - if (Array.isArray(pattern['allOf'])) { - for (const sub of pattern['allOf']) { - if (!isObject(sub) || !isObject(sub['properties'])) continue; - const field = sub['properties'][key]; - if (isObject(field) && Array.isArray(field['prefixItems'])) { - return field['prefixItems'] as SchemaObject[]; - } - } - } - - return []; +function getCandidateItems(pattern: SchemaObject, key: 'nodes' | 'relationships'): SchemaObject[] { + const { prefixItems, catalog } = getPatternArray(pattern, key); + return catalog ? [...prefixItems, catalog] as SchemaObject[] : prefixItems as SchemaObject[]; } /** @@ -146,8 +134,8 @@ function partitionPattern(pattern: unknown): { nodes: PatternPartition; relation }; } return { - nodes: partitionPrefixItems(getPrefixItems(pattern, 'nodes')), - relationships: partitionPrefixItems(getPrefixItems(pattern, 'relationships')), + nodes: partitionPrefixItems(getCandidateItems(pattern, 'nodes')), + relationships: partitionPrefixItems(getCandidateItems(pattern, 'relationships')), }; } diff --git a/calm-models/src/pattern/index.ts b/calm-models/src/pattern/index.ts new file mode 100644 index 000000000..658c7057e --- /dev/null +++ b/calm-models/src/pattern/index.ts @@ -0,0 +1,12 @@ +export { + getPatternArray, + resolveOperativeChoiceBlock, + listDeclaredCandidates, + listSelectableCandidates, + listNodeInterfaces, + type SchemaNode, + type PatternArray, + type ChoiceBlock, + type Candidate, + type DeclaredInterface, +} from './pattern-reader.js'; diff --git a/calm-models/src/pattern/pattern-reader.spec.ts b/calm-models/src/pattern/pattern-reader.spec.ts new file mode 100644 index 000000000..022b596b8 --- /dev/null +++ b/calm-models/src/pattern/pattern-reader.spec.ts @@ -0,0 +1,327 @@ +import { describe, it, expect } from 'vitest'; +import { + getPatternArray, + resolveOperativeChoiceBlock, + listDeclaredCandidates, + listSelectableCandidates, + type SchemaNode, +} from './pattern-reader.js'; + +function nodeWithId(uniqueId: string): SchemaNode { + return { properties: { 'unique-id': { const: uniqueId } } }; +} + +describe('getPatternArray', () => { + it('reads prefixItems and catalog declared directly under properties', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [nodeWithId('a')], + items: { oneOf: [nodeWithId('b')] }, + }, + }, + }; + + const result = getPatternArray(pattern, 'nodes'); + expect(result.prefixItems).toEqual([nodeWithId('a')]); + expect(result.catalog).toEqual({ oneOf: [nodeWithId('b')] }); + }); + + it('yields an empty prefixItems array and undefined catalog when neither is declared', () => { + const result = getPatternArray({ properties: {} }, 'nodes'); + expect(result.prefixItems).toEqual([]); + expect(result.catalog).toBeUndefined(); + }); + + it('falls back to the first allOf branch that declares the array', () => { + const pattern = { + allOf: [ + { properties: { nodes: { prefixItems: [nodeWithId('first-branch')] } } }, + { properties: { nodes: { prefixItems: [nodeWithId('second-branch')] } } }, + ], + }; + + const result = getPatternArray(pattern, 'nodes'); + // TEMPORARY (first-branch-wins): a later branch declaring the same path is + // invisible today. Do not "fix" this here — see the reader's allOf note. + expect(result.prefixItems).toEqual([nodeWithId('first-branch')]); + }); + + it('prefers a direct declaration over an allOf branch', () => { + const pattern = { + properties: { nodes: { prefixItems: [nodeWithId('direct')] } }, + allOf: [{ properties: { nodes: { prefixItems: [nodeWithId('branch')] } } }], + }; + + expect(getPatternArray(pattern, 'nodes').prefixItems).toEqual([nodeWithId('direct')]); + }); + + it('treats a falsy keyword value (e.g. items: false closing a tuple) as absent', () => { + const pattern = { properties: { nodes: { prefixItems: [nodeWithId('a')], items: false } } }; + expect(getPatternArray(pattern, 'nodes').catalog).toBeUndefined(); + }); + + it('does not compose prefixItems from one source with a catalog from another', () => { + const pattern = { + properties: { nodes: { prefixItems: [nodeWithId('root-a')] } }, + allOf: [{ + properties: { + nodes: { + prefixItems: [nodeWithId('branch-x')], + items: { anyOf: [nodeWithId('branch-cat')] }, + }, + }, + }], + }; + + const result = getPatternArray(pattern, 'nodes'); + // The root's own prefixItems wins wholesale (same first-source-wins precedent as + // the test above) - it must NOT borrow the allOf branch's catalog, which would + // describe an array no single declaration site in the document actually contains. + expect(result.prefixItems).toEqual([nodeWithId('root-a')]); + expect(result.catalog).toBeUndefined(); + }); + + it('does not compose the other way either: a catalog from one source with prefixItems from another', () => { + const pattern = { + properties: { nodes: { items: { anyOf: [nodeWithId('root-cat')] } } }, + allOf: [{ properties: { nodes: { prefixItems: [nodeWithId('branch-a')] } } }], + }; + + const result = getPatternArray(pattern, 'nodes'); + // The root's own declaration (a catalog, no prefixItems of its own) wins wholesale. + // Its own gap - no prefixItems - must stay a gap, not get silently patched by + // borrowing the allOf branch's prefixItems. + expect(result.prefixItems).toEqual([]); + expect(result.catalog).toEqual({ anyOf: [nodeWithId('root-cat')] }); + }); + + it('does not compose across two different allOf branches when the root declares neither', () => { + const pattern = { + allOf: [ + { properties: { nodes: { prefixItems: [nodeWithId('branch1-a')] } } }, + { properties: { nodes: { items: { anyOf: [nodeWithId('branch2-cat')] } } } }, + ], + }; + + const result = getPatternArray(pattern, 'nodes'); + // The first branch that declares anything wins wholesale; the second branch's + // catalog must not be borrowed to fill the first branch's gap. + expect(result.prefixItems).toEqual([nodeWithId('branch1-a')]); + expect(result.catalog).toBeUndefined(); + }); + + it('skips a source that declares the property but neither keyword, and resolves from the next one', () => { + const pattern = { + properties: { nodes: { minItems: 1 } }, // declares `nodes` but with nothing selectable + allOf: [{ + properties: { + nodes: { + prefixItems: [nodeWithId('branch-a')], + items: { anyOf: [nodeWithId('branch-cat')] }, + }, + }, + }], + }; + + const result = getPatternArray(pattern, 'nodes'); + expect(result.prefixItems).toEqual([nodeWithId('branch-a')]); + expect(result.catalog).toEqual({ anyOf: [nodeWithId('branch-cat')] }); + }); +}); + +describe('resolveOperativeChoiceBlock', () => { + it('returns null for an undefined catalog', () => { + expect(resolveOperativeChoiceBlock(undefined)).toBeNull(); + }); + + it('returns null when neither oneOf nor anyOf is an array', () => { + expect(resolveOperativeChoiceBlock({})).toBeNull(); + expect(resolveOperativeChoiceBlock({ oneOf: 'not-an-array' })).toBeNull(); + }); + + it('reads a oneOf-only catalog', () => { + const alternatives = [nodeWithId('a'), nodeWithId('b')]; + expect(resolveOperativeChoiceBlock({ oneOf: alternatives })).toEqual({ groupType: 'oneOf', alternatives }); + }); + + it('reads an anyOf-only catalog', () => { + const alternatives = [nodeWithId('a')]; + expect(resolveOperativeChoiceBlock({ anyOf: alternatives })).toEqual({ groupType: 'anyOf', alternatives }); + }); + + it('prefers oneOf over anyOf when both are present', () => { + const oneOfAlts = [nodeWithId('one')]; + const anyOfAlts = [nodeWithId('any')]; + expect(resolveOperativeChoiceBlock({ oneOf: oneOfAlts, anyOf: anyOfAlts })).toEqual({ + groupType: 'oneOf', + alternatives: oneOfAlts, + }); + }); +}); + +describe('listDeclaredCandidates', () => { + it('lists a plain prefixItems entry', () => { + const pattern = { properties: { nodes: { prefixItems: [nodeWithId('solo')] } } }; + expect(listDeclaredCandidates(pattern, 'nodes')).toEqual([ + { uniqueId: 'solo', site: 'prefixItem', node: nodeWithId('solo'), path: ['properties', 'nodes', 'prefixItems', 0] }, + ]); + }); + + it('unions oneOf and anyOf on the same slot, unlike resolveOperativeChoiceBlock', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [{ oneOf: [nodeWithId('a')], anyOf: [nodeWithId('b')] }], + }, + }, + }; + + const candidates = listDeclaredCandidates(pattern, 'nodes'); + expect(candidates.map((c) => c.uniqueId)).toEqual(['a', 'b']); + expect(candidates.map((c) => c.blockType)).toEqual(['oneOf', 'anyOf']); + expect(candidates.every((c) => c.site === 'prefixItemAlternative' && c.slotIndex === 0)).toBe(true); + }); + + it('yields both the slot and its alternatives for a hybrid slot', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [{ ...nodeWithId('hybrid'), oneOf: [nodeWithId('alt')] }], + }, + }, + }; + + const candidates = listDeclaredCandidates(pattern, 'nodes'); + expect(candidates.map((c) => ({ uniqueId: c.uniqueId, site: c.site }))).toEqual([ + { uniqueId: 'hybrid', site: 'prefixItem' }, + { uniqueId: 'alt', site: 'prefixItemAlternative' }, + ]); + }); + + it('lists items.oneOf and items.anyOf catalog members together', () => { + const pattern = { + properties: { + nodes: { + items: { oneOf: [nodeWithId('cat-one')], anyOf: [nodeWithId('cat-any')] }, + }, + }, + }; + + const candidates = listDeclaredCandidates(pattern, 'nodes'); + expect(candidates).toEqual([ + { uniqueId: 'cat-one', site: 'catalogMember', node: nodeWithId('cat-one'), path: ['properties', 'nodes', 'items', 'oneOf', 0], blockType: 'oneOf' }, + { uniqueId: 'cat-any', site: 'catalogMember', node: nodeWithId('cat-any'), path: ['properties', 'nodes', 'items', 'anyOf', 0], blockType: 'anyOf' }, + ]); + }); + + it('skips a pure choice-block slot with no unique-id of its own', () => { + const pattern = { + properties: { + nodes: { prefixItems: [{ oneOf: [nodeWithId('a'), nodeWithId('b')] }] }, + }, + }; + + const candidates = listDeclaredCandidates(pattern, 'nodes'); + expect(candidates.map((c) => c.uniqueId)).toEqual(['a', 'b']); + expect(candidates.some((c) => c.uniqueId === undefined)).toBe(false); + }); + + it('skips a catalog member with no const-pinned unique-id', () => { + const pattern = { + properties: { nodes: { items: { oneOf: [{ properties: {} }] } } }, + }; + expect(listDeclaredCandidates(pattern, 'nodes')).toEqual([]); + }); + + it('returns an empty array when the calmType is absent', () => { + expect(listDeclaredCandidates({ properties: {} }, 'nodes')).toEqual([]); + }); + + it('does not fall back into an allOf branch, unlike getPatternArray', () => { + const pattern = { + allOf: [{ properties: { nodes: { prefixItems: [nodeWithId('in-a-branch')] } } }], + }; + expect(listDeclaredCandidates(pattern, 'nodes')).toEqual([]); + }); +}); + +describe('listSelectableCandidates', () => { + it('lists a plain prefixItems entry, same as listDeclaredCandidates', () => { + const pattern = { properties: { nodes: { prefixItems: [nodeWithId('solo')] } } }; + expect(listSelectableCandidates(pattern, 'nodes')).toEqual([ + { uniqueId: 'solo', site: 'prefixItem', node: nodeWithId('solo'), path: ['properties', 'nodes', 'prefixItems', 0] }, + ]); + }); + + it('resolves only the winning keyword of a dual-keyword block, unlike listDeclaredCandidates', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [{ oneOf: [nodeWithId('a')], anyOf: [nodeWithId('b')] }], + }, + }, + }; + + const declared = listDeclaredCandidates(pattern, 'nodes').map((c) => c.uniqueId); + const selectable = listSelectableCandidates(pattern, 'nodes').map((c) => c.uniqueId); + expect(declared).toEqual(['a', 'b']); + expect(selectable).toEqual(['a']); + }); + + it('yields both the slot and its winning alternatives for a hybrid slot', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [{ ...nodeWithId('hybrid'), oneOf: [nodeWithId('alt-a')], anyOf: [nodeWithId('alt-b')] }], + }, + }, + }; + + const candidates = listSelectableCandidates(pattern, 'nodes'); + expect(candidates.map((c) => ({ uniqueId: c.uniqueId, site: c.site }))).toEqual([ + { uniqueId: 'hybrid', site: 'prefixItem' }, + { uniqueId: 'alt-a', site: 'prefixItemAlternative' }, + ]); + }); + + it('resolves only the winning keyword of a dual-keyword items catalog', () => { + const pattern = { + properties: { + nodes: { + items: { oneOf: [nodeWithId('cat-one')], anyOf: [nodeWithId('cat-any')] }, + }, + }, + }; + + expect(listSelectableCandidates(pattern, 'nodes').map((c) => c.uniqueId)).toEqual(['cat-one']); + }); + + it('lists every alternative when only one keyword is declared, same as listDeclaredCandidates', () => { + const pattern = { + properties: { + nodes: { prefixItems: [{ anyOf: [nodeWithId('a'), nodeWithId('b')] }] }, + }, + }; + expect(listSelectableCandidates(pattern, 'nodes').map((c) => c.uniqueId)).toEqual(['a', 'b']); + }); + + it('skips a catalog member with no const-pinned unique-id', () => { + const pattern = { + properties: { nodes: { items: { oneOf: [{ properties: {} }] } } }, + }; + expect(listSelectableCandidates(pattern, 'nodes')).toEqual([]); + }); + + it('returns an empty array when the calmType is absent', () => { + expect(listSelectableCandidates({ properties: {} }, 'nodes')).toEqual([]); + }); + + it('does not fall back into an allOf branch, unlike getPatternArray', () => { + const pattern = { + allOf: [{ properties: { nodes: { prefixItems: [nodeWithId('in-a-branch')] } } }], + }; + expect(listSelectableCandidates(pattern, 'nodes')).toEqual([]); + }); +}); + diff --git a/calm-models/src/pattern/pattern-reader.ts b/calm-models/src/pattern/pattern-reader.ts new file mode 100644 index 000000000..ba8816419 --- /dev/null +++ b/calm-models/src/pattern/pattern-reader.ts @@ -0,0 +1,274 @@ +/** + * Read-only reader for where candidate nodes and relationships live in a CALM pattern's + * JSON Schema, and how a `oneOf`/`anyOf` choice block should be read. + * + * A CALM pattern declares candidates in four places: a plain `prefixItems` entry, a + * `prefixItems[i].oneOf`/`anyOf` alternative, or an `items.oneOf`/`items.anyOf` open + * catalog member. This module is the single place that knows how to find them, so + * generation, validation, the visualiser and the pattern differ stop hand-rolling the + * same traversal. + * + * Three different questions get three different functions, deliberately kept apart: + * `resolveOperativeChoiceBlock` picks the single form a decision offers (`oneOf` wins over `anyOf`); + * `listDeclaredCandidates` unions both, because validation needs every id a pattern declares; + * `listSelectableCandidates` defers to `resolveOperativeChoiceBlock`'s single form, because + * generation needs only what an answer can actually reach. + * + * No selection, no mutation, no rendering — only reading. The surface is deliberately + * limited to what has a caller today; add functions when a consumer needs them, so their + * shape is validated by real use rather than guessed. + */ + +export type SchemaNode = Record; + +function isObject(value: unknown): value is SchemaNode { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + + +/** + * TEMPORARY. Replicates today's first-`allOf`-branch-wins reading of a top-level pattern + * property. A later branch declaring the same path is ignored, and `prefixItems`/`items` + * are always read from the *same* resolved source - never composed from two different + * branches, which would describe an array no single declaration site actually contains. + * + * It exists because candidate discovery runs on the raw pattern, before `flattenAllOf`. + * Do not correct the precedence here - a reader that disagrees with `flattenAllOf` is worse + * than one that is wrong the same way. This function becomes removable once `allOf` branches + * are combined by real schema intersection, not before. + */ +function resolveArrayContainer(pattern: SchemaNode, key: string): SchemaNode | undefined { + const direct = pattern['properties']; + if (isObject(direct)) { + const field = direct[key]; + if (isObject(field) && (field['prefixItems'] || field['items'])) { + return field as SchemaNode; + } + } + + if (Array.isArray(pattern['allOf'])) { + for (const branch of pattern['allOf']) { + if (!isObject(branch)) continue; + const branchProperties = branch['properties']; + if (!isObject(branchProperties)) continue; + const field = branchProperties[key]; + if (isObject(field) && (field['prefixItems'] || field['items'])) { + return field as SchemaNode; + } + } + } + + return undefined; +} + +export interface PatternArray { + prefixItems: SchemaNode[]; + catalog: SchemaNode | undefined; +} + +/** + * Reads the `prefixItems` array and `items` open-catalog declared for a top-level + * pattern property (`nodes` or `relationships`), resolving `allOf` per `resolveArrayContainer` + * above. Absent `prefixItems` yields an empty array so callers can iterate + * unconditionally; an absent catalog yields `undefined`. + */ +export function getPatternArray(pattern: SchemaNode, calmType: 'nodes' | 'relationships'): PatternArray { + const container = resolveArrayContainer(pattern, calmType); + const prefixItems = container?.['prefixItems']; + const catalog = container?.['items']; + return { + prefixItems: Array.isArray(prefixItems) ? (prefixItems as SchemaNode[]) : [], + catalog: isObject(catalog) ? (catalog as SchemaNode) : undefined, + }; +} + +export interface ChoiceBlock { + groupType: 'oneOf' | 'anyOf'; + alternatives: SchemaNode[]; +} + +/** + * The alternatives that one choice block offers, for a `prefixItems` slot or an `items` + * catalog. `oneOf` wins when both are present. Returns `null` when neither keyword is + * an array. + * + * This picks one answer. A caller that needs every declared id must use `listDeclaredCandidates`. + */ +export function resolveOperativeChoiceBlock(items: SchemaNode | undefined): ChoiceBlock | null { + if (!items) return null; + + const hasOneOf = Array.isArray(items['oneOf']); + const hasAnyOf = Array.isArray(items['anyOf']); + if (!hasOneOf && !hasAnyOf) return null; + + return { + groupType: hasOneOf ? 'oneOf' : 'anyOf', + alternatives: (hasOneOf ? items['oneOf'] : items['anyOf']) as SchemaNode[], + }; +} + +function readUniqueId(node: SchemaNode): string | undefined { + const properties = node['properties']; + if (!isObject(properties)) return undefined; + const uniqueIdSchema = properties['unique-id']; + if (!isObject(uniqueIdSchema)) return undefined; + const constValue = uniqueIdSchema['const']; + return typeof constValue === 'string' ? constValue : undefined; +} + +export type Candidate = { + uniqueId: string; + site: 'prefixItem' | 'prefixItemAlternative' | 'catalogMember'; + node: SchemaNode; + path: (string | number)[]; + slotIndex?: number; + blockType?: 'oneOf' | 'anyOf'; +}; + +/** How a `oneOf`/`anyOf` choice block contributes candidates. */ +type BlockResolution = + /** Every alternative of every keyword present - what the pattern *declares*. */ + | 'all' + /** Only the operative keyword's alternatives - what selection can *reach*. */ + | 'operative'; + +/** + * The keywords a choice block contributes candidates from, for a given resolution. + * `'all'` unions both; `'operative'` defers to `resolveOperativeChoiceBlock`'s oneOf-wins rule so + * only the resolvable keyword is walked. + */ +function blockKeywords(container: SchemaNode, resolution: BlockResolution): ReadonlyArray<'oneOf' | 'anyOf'> { + if (resolution === 'all') return ['oneOf', 'anyOf']; + const block = resolveOperativeChoiceBlock(container); + return block ? [block.groupType] : []; +} + +/** + * Every node/relationship candidate declared under `properties.`, across all + * four declaration sites, resolved per `resolution`. + * + * Skips a candidate with no `const`-pinned `unique-id`. A pure choice-block slot has no + * id of its own, and counting it would create a false diagnostic. + * + * Reads the direct path only. It does not fall back into `allOf`, because + * `getPatternArray` discards which branch it read, so `path` could not be trusted. + */ +function walkCandidates( + pattern: SchemaNode, + calmType: 'nodes' | 'relationships', + resolution: BlockResolution +): Candidate[] { + const candidates: Candidate[] = []; + const properties = pattern['properties']; + const field = isObject(properties) ? properties[calmType] : undefined; + if (!isObject(field)) return candidates; + + const prefixItems = Array.isArray(field['prefixItems']) ? (field['prefixItems'] as SchemaNode[]) : []; + + prefixItems.forEach((item, i) => { + if (!isObject(item)) return; + + // A hybrid slot carries its own id and alternatives. Both are emitted regardless + // of resolution - `resolveOperativeChoiceBlock` only decides which *alternatives* keyword wins. + const uniqueId = readUniqueId(item); + if (uniqueId) { + candidates.push({ + uniqueId, + site: 'prefixItem', + node: item, + path: ['properties', calmType, 'prefixItems', i], + }); + } + + blockKeywords(item, resolution).forEach((blockType) => { + const alternatives = item[blockType]; + if (!Array.isArray(alternatives)) return; + alternatives.forEach((alt, j) => { + if (!isObject(alt)) return; + const altUniqueId = readUniqueId(alt); + if (!altUniqueId) return; + candidates.push({ + uniqueId: altUniqueId, + site: 'prefixItemAlternative', + node: alt, + path: ['properties', calmType, 'prefixItems', i, blockType, j], + slotIndex: i, + blockType, + }); + }); + }); + }); + + const itemsCatalog = field['items']; + if (isObject(itemsCatalog)) { + blockKeywords(itemsCatalog, resolution).forEach((blockType) => { + const alternatives = itemsCatalog[blockType]; + if (!Array.isArray(alternatives)) return; + alternatives.forEach((alt, j) => { + if (!isObject(alt)) return; + const altUniqueId = readUniqueId(alt); + if (!altUniqueId) return; + candidates.push({ + uniqueId: altUniqueId, + site: 'catalogMember', + node: alt, + path: ['properties', calmType, 'items', blockType, j], + blockType, + }); + }); + }); + } + + return candidates; +} + +/** + * Every node/relationship candidate a pattern declares. Unions `oneOf` and `anyOf`, + * which is the opposite of `resolveOperativeChoiceBlock`. Validation needs every declared id. Do + * not route this through `resolveOperativeChoiceBlock` - that drops every `anyOf` candidate when + * `oneOf` is also present. + */ +export function listDeclaredCandidates(pattern: SchemaNode, calmType: 'nodes' | 'relationships'): Candidate[] { + return walkCandidates(pattern, calmType, 'all'); +} + +/** + * Only the candidates selection can reach, resolved as `selectChoices` resolves them: + * a dual-keyword block's `anyOf` alternatives are declared but not selectable. + * `listDeclaredCandidates` is a silent bug here, because it reports the losing keyword's + * alternatives as available. Use it for "can this answer be honoured". + */ +export function listSelectableCandidates(pattern: SchemaNode, calmType: 'nodes' | 'relationships'): Candidate[] { + return walkCandidates(pattern, calmType, 'operative'); +} + +/** An interface a node candidate declares, and its position in `interfaces.prefixItems`. */ +export type DeclaredInterface = { + uniqueId: string; + index: number; +}; + +/** + * Reads a node candidate's own `interfaces.prefixItems`, in declaration order, skipping any + * entry with no `const`-pinned `unique-id` - the same rule the candidate readers above apply + * to the candidates themselves. + * + * Reports `index`, not a `path`: it receives the candidate's schema without knowing where that + * schema sits, so only the caller can turn a position into a document location. + */ +export function listNodeInterfaces(node: SchemaNode): DeclaredInterface[] { + const properties = node['properties']; + const interfacesSchema = isObject(properties) ? properties['interfaces'] : undefined; + const prefixItems = isObject(interfacesSchema) && Array.isArray(interfacesSchema['prefixItems']) + ? (interfacesSchema['prefixItems'] as SchemaNode[]) + : []; + + const result: DeclaredInterface[] = []; + prefixItems.forEach((iface, index) => { + if (!isObject(iface)) return; + const uniqueId = readUniqueId(iface); + if (!uniqueId) return; + result.push({ uniqueId, index }); + }); + return result; +} diff --git a/calm-plugins/vscode/src/webview/panels/PatternPicker.test.ts b/calm-plugins/vscode/src/webview/panels/PatternPicker.test.ts new file mode 100644 index 000000000..1cbc0f120 --- /dev/null +++ b/calm-plugins/vscode/src/webview/panels/PatternPicker.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect } from 'vitest'; +import { instantiateFromPattern } from './PatternPicker.js'; + +function node(uniqueId: string, name: string) { + return { + properties: { + 'unique-id': { const: uniqueId }, + name: { const: name }, + 'node-type': { const: 'service' }, + }, + }; +} + +describe('instantiateFromPattern', () => { + it('instantiates prefixItems nodes and relationships', () => { + const pattern = { + properties: { + nodes: { prefixItems: [node('api-gateway', 'API Gateway')] }, + relationships: { prefixItems: [] }, + }, + }; + const result = instantiateFromPattern(pattern); + expect(result.nodes).toHaveLength(1); + expect(result.nodes[0]['unique-id']).toBe('api-gateway'); + }); + + it('instantiates a candidate from an items catalog alongside prefixItems', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [node('webapp', 'Web App')], + items: { anyOf: [node('redis', 'Redis')] }, + }, + relationships: { prefixItems: [] }, + }, + }; + const result = instantiateFromPattern(pattern); + const ids = result.nodes.map((n: Record) => n['unique-id']); + expect(ids).toContain('webapp'); + expect(ids).toContain('redis'); + }); + + it('previews a catalog-only pattern rather than leaving it empty', () => { + const pattern = { + properties: { + nodes: { items: { oneOf: [node('cache', 'Cache')] } }, + relationships: { prefixItems: [] }, + }, + }; + const result = instantiateFromPattern(pattern); + expect(result.nodes.map((n: Record) => n['unique-id'])).toEqual(['cache']); + }); +}); diff --git a/calm-plugins/vscode/src/webview/panels/PatternPicker.tsx b/calm-plugins/vscode/src/webview/panels/PatternPicker.tsx index 58f3f4bc9..ac3d46994 100644 --- a/calm-plugins/vscode/src/webview/panels/PatternPicker.tsx +++ b/calm-plugins/vscode/src/webview/panels/PatternPicker.tsx @@ -61,10 +61,20 @@ export function PatternPicker({ visible, mode, patterns, onApply, onClose }: Pat ); } -function instantiateFromPattern(schema: unknown): any { +/** + * An `items` open catalog is structurally the same as a `prefixItems` slot's own + * `oneOf`/`anyOf` block - `instantiateNode`/`instantiateRel` already know how to unwrap + * one of those by taking its first alternative, so appending the catalog schema here + * reuses that path instead of needing a second one. + */ +function catalogEntry(items: unknown): any[] { + return items && typeof items === 'object' ? [items] : []; +} + +export function instantiateFromPattern(schema: unknown): any { const p = schema as any; - const nodeSchemas = p?.properties?.nodes?.prefixItems ?? []; - const relSchemas = p?.properties?.relationships?.prefixItems ?? []; + const nodeSchemas = [...(p?.properties?.nodes?.prefixItems ?? []), ...catalogEntry(p?.properties?.nodes?.items)]; + const relSchemas = [...(p?.properties?.relationships?.prefixItems ?? []), ...catalogEntry(p?.properties?.relationships?.items)]; const nodes = nodeSchemas.map((s: any) => instantiateNode(s)).filter(Boolean).map((n: any) => { if (!n['unique-id']) n['unique-id'] = '[[PLACEHOLDER]]'; diff --git a/cli/src/cli.e2e.spec.ts b/cli/src/cli.e2e.spec.ts index 8f064779b..7ea2c1d30 100644 --- a/cli/src/cli.e2e.spec.ts +++ b/cli/src/cli.e2e.spec.ts @@ -681,6 +681,29 @@ describe('CLI Integration Tests', () => { ); await expectFilesMatch(expectedOutputArchitecture, outputArchitecture); + // BASELINE, not a target to make pass silently. The generated architecture does not + // round-trip: validating it against the same pattern fails on a control requirement's + // control-id. permitted-connection-jdbc.config.json declares "security-003", but the + // requirement schema it is checked against pins "security-002" - a one-value copy-paste + // slip; the sibling http config uses "security-002" correctly. Reproduces identically on + // `main`; unrelated to this PR. Tracked separately - if this test starts failing because + // validation now passes, the bug is fixed and this block should be deleted. + let validateError: { stdout?: string } | undefined; + try { + await cli.run(['validate', '-p', inputPattern, '-a', outputArchitecture, '-u', STATIC_GETTING_STARTED_MAPPING_PATH]); + } catch (err) { + validateError = err as { stdout?: string }; + } + expect(validateError, 'expected calm validate to fail - if it now passes, delete this baseline block').toBeDefined(); + const validateOutput = JSON.parse(validateError!.stdout!); + expect(validateOutput.hasErrors).toBe(true); + expect(validateOutput.jsonSchemaValidationOutputs).toContainEqual( + expect.objectContaining({ + code: 'control-requirement-validation', + path: expect.stringContaining('control-id'), + }) + ); + //STEP 2: Generate Docify Website From Architecture const outputWebsite = path.resolve(actualOutputDir, 'website'); await cli.run([ diff --git a/cli/test_fixtures/validate_output_junit.xml b/cli/test_fixtures/validate_output_junit.xml index da3078e73..c138044f5 100644 --- a/cli/test_fixtures/validate_output_junit.xml +++ b/cli/test_fixtures/validate_output_junit.xml @@ -1,10 +1,10 @@ - + - @@ -42,5 +42,9 @@ + + + + \ No newline at end of file diff --git a/shared/AGENTS.md b/shared/AGENTS.md index 00ebf149e..d8373621a 100644 --- a/shared/AGENTS.md +++ b/shared/AGENTS.md @@ -45,6 +45,7 @@ npx vitest run ${TEST FILE} - `validate()` - Main validation function (`commands/validate/validate.ts`) used by CLI and VSCode - `enrichWithDocumentPositions()` - Adds precise line/character positions to validation output using `@stoplight/json` - `parseDocumentWithPositions()` - Parses JSON/YAML with position tracking for error location +- **Generate** (`commands/generate/`): Pattern → architecture instantiation. `flatten-allof.ts` composes `allOf` branches, `options.ts` resolves user decisions (`extractOptions`, `selectChoices`), and `instantiate.ts` materializes the result. See [Pattern Decisions](#pattern-decisions) before changing any of these. - **Schema Directory** (`schema-directory.ts`): Registry of bundled CALM schemas, used for lookup by schema URL (`getSchema`). - **Docify** (`docify/`): Documentation generator (`docifier`) with C4/relationship graphing (`docify/graphing`) and template bundles (`docify/template-bundles`, e.g. `ants`, `docusaurus`). - **Resolver** (`resolver/`): CALM reference resolver plus the network-addressable extractor and validator. @@ -52,6 +53,18 @@ npx vitest run ${TEST FILE} - **View Model** (`view-model/`): ADR (Architecture Decision Record) view-model logic. - **Auth** (`auth/`): Auth plugin abstraction (`auth-plugin`, `no-auth-plugin`). +## Pattern Decisions + +See [PATTERN-DECISIONS.md](./PATTERN-DECISIONS.md) before you change how a pattern's decisions +are read, merged, or rendered. It covers where a candidate can be declared, what `oneOf`/`anyOf` +mean in each location, which of the three pattern-reader functions to call for which question, +why `allOf` has three readers that disagree, the enforcement rules, how decisions fold into the +visualiser's boxes, why `calm generate` offers decision choices it then discards, and what a +shared decision-holder reader would have to settle first. + +Drive new tests from `extractOptions`, not from hand-built choices, or you do not test whether +the decision is discoverable. See `catalog-decisions.spec.ts`. + ## Common Workflows **IMPORTANT**: Always run npm commands from the **repository root** using workspaces, not from within this package directory. diff --git a/shared/PATTERN-DECISIONS.md b/shared/PATTERN-DECISIONS.md new file mode 100644 index 000000000..cf38df9b6 --- /dev/null +++ b/shared/PATTERN-DECISIONS.md @@ -0,0 +1,226 @@ +# Pattern Decisions + +This document explains how CALM patterns express decisions. It ties together rules +that are spread across several packages. Read it before you change how a pattern is +read, merged, or rendered. + +It describes the behaviour the code has. Where two parts of the system disagree, this +document names the disagreement and gives the reason. It does not pick a winner, except +where only one answer is coherent - and then it says why the other is not available. + +For what a **decision holder** and a **candidate** are, see +`calm-ai/tools/pattern-creation.md`. That guide is for pattern authors. This +document is for people who change the code that reads a pattern. + +## Where a candidate can be declared + +A pattern can declare a candidate in four places: + +- a plain `prefixItems` entry +- a `prefixItems[i].oneOf` alternative +- a `prefixItems[i].anyOf` alternative +- an `items.oneOf`/`items.anyOf` catalog member + +One function, `getPatternArray` (`@finos/calm-models/pattern`), finds the `prefixItems` +array and the `items` catalog for `nodes` or `relationships`. Other functions build on +top of it. + +## `oneOf` and `anyOf` mean different things by location + +The same two keywords carry different meaning depending on where they sit. Get this +wrong and the pattern still validates, but generation or the visualiser reads it +incorrectly. + +**Inside a decision holder's `options`.** This sets the cardinality. `oneOf` means the +user must pick exactly one choice. `anyOf` means the user may pick any number of +choices, including none. + +**Inside a `prefixItems[i]` slot.** This picks which single candidate fills that fixed +array position. `oneOf` and `anyOf` behave the same way here: only one alternative can +occupy the slot. + +**Inside an `items` catalog.** This constrains the shape of one array entry, not how +many entries exist. Each candidate pins its own `unique-id` with `const`, so an entry +can match at most one candidate schema either way. Use `oneOf` here. It is the accurate +keyword, and it is the one the readers below resolve to when both are present. + +**Never declare both `oneOf` and `anyOf` on one block.** Nothing in JSON Schema +forbids it, so a pattern can still do it by mistake. When it happens, every operation +that resolves a block to what will actually exist reads it the same way: `oneOf` wins, +and the `anyOf` alternatives are dropped. Operations that only enumerate what a pattern +declares union both keywords instead, which is why `listDeclaredCandidates` exists +alongside `listSelectableCandidates`. See "A known disagreement" below for the one +operation that enumerates on the union and then resolves against it. + +## Three functions answer three different questions + +Each function lives in `@finos/calm-models/pattern`. Use the wrong one and the failure +is silent, not an error. + +| Question | Function | +|---|---| +| What does this one block resolve to? (`oneOf` wins) | `resolveOperativeChoiceBlock` | +| What does the pattern declare, in total? (both keywords) | `listDeclaredCandidates` | +| What can a selection actually reach? (one keyword) | `listSelectableCandidates` | + +Use `listDeclaredCandidates` for questions about the document itself: is an id unique, +does a reference dangle. Use `listSelectableCandidates` for questions about an answer: +can this choice be honoured. The two functions differ only where a block declares both +keywords — which a well-formed pattern never does. + +`getPatternArray` is a fourth relevant function. It resolves the array a decision +lives in before either of the two `list*` functions walks it. + +## `calm generate` never validates + +`calm generate` does not run `calm validate`. It has its own guard instead. +`assertChoicesAreSelectable` throws from `runGenerate` when a chosen answer names a +candidate that selection cannot reach. It is not called from `selectChoices`, because +`calm validate` calls `selectChoices` too, and a malformed pattern must show its own +schema error there instead. + +This means a decision holder placed somewhere illegal, such as inside an `items` +catalog, produces a `calm validate` error but not a `calm generate` error. A user who +only runs `calm generate` sees no error. The decision is simply never offered. The +behaviour is deliberate, and it is easy to miss. + +## `allOf` has three unreconciled readers + +Three parts of the system read `allOf` for `nodes` and `relationships`, and they do not +agree. Nothing may assume consistent behaviour across them. Pattern authors are told not +to split a property's definition across branches - see `calm-ai/tools/pattern-creation.md`. + +| Reader | Used by | `allOf` behaviour | Why it cannot do what the others do | +|---|---|---|---| +| `deepMergeSchemas` (`flatten-allof.ts`) | generation (`runGenerate`, before anything else) | Shallow merge of all branches. A repeated property loses its `type`, so `instantiate` emits `{}` for it. | It must hand `instantiate` one schema to materialise from, so it has to combine branches rather than choose one. | +| `getPatternArray` | generation (`extractOptions`), visualisation (`patternTransformer.ts`), pattern diff (`diff/pattern-diff.ts`) | Resolves one branch per property: the root schema, or else the first `allOf` branch declaring it. `prefixItems` and `items` always come from that same branch. | It returns a *location*, not a merged schema, and runs on the raw pattern. Merging two branches would describe an array no declaration site contains. Its private `resolveArrayContainer` carries the TEMPORARY marker. | +| `listDeclaredCandidates` / `listSelectableCandidates` | validation (4 Spectral rules), generation (`assertChoicesAreSelectable`, which runs after `flattenAllOf` and so never meets an `allOf`) | Ignore `allOf` entirely. A candidate declared only inside a branch is invisible to validation. | Each candidate reports a `path` used in diagnostics. `getPatternArray` discards which branch it read, so following `allOf` would produce a path the document does not contain. | + +The split is observable on one document. Give all three the same pattern whose `nodes` and +`relationships` live only inside an `allOf` branch: `extractOptions` finds the decision, and +`listDeclaredCandidates` reports no candidates at all. Generation offers a choice over a node +validation believes does not exist. + +**Why this is not one answer.** `allOf` means intersection, not union, because `calm +validate` never flattens a pattern before checking it. A correct merge would combine +the branches the way a real JSON Schema validator combines them. No part of this +codebase does that. Each reader above makes its own narrow choice instead, scoped to +what its one caller needs. + +Keep one implementation of each `list*` function, and keep it in `calm-models`. Do not +add a copy that follows `allOf` through `getPatternArray`: that combination reports a +`path` the document does not contain, which is wrong for a diagnostic. Reconciling the +three readers needs the real intersection merge described above, which is larger work +than any single caller justifies. + +## Enforcement + +These rules run only on `calm validate`. `calm generate` never runs them. + +| Rule | Severity | Catches | +|---|---|---| +| `pattern-option-relationship-must-be-in-prefix-items` | error | A decision holder placed inside an `items` catalog. | +| `pattern-decision-must-reference-selectable-nodes` / `-relationships` | error | A choice bundle naming a candidate that is declared but not reachable. | +| `group-relationship-with-const-nodes-references-existing-nodes-in-pattern` | error | A choice bundle naming an id that does not exist at all. | +| `pattern-items-catalog-must-declare-one-choice-keyword` | warn | An `items` catalog block, or a `prefixItems[*]` slot, that declares both keywords. | + +The last rule's eight `given` paths reach `properties..items` and +`properties..prefixItems[*]`, each also under `allOf[*]`. So the rule +does reach a decision holder, as one of the `relationships.prefixItems[*]` entries, but it +only checks the keywords that entry declares itself. It never descends into the holder's own +`relationship-type.options.prefixItems[*]`, so no rule catches a decision holder that declares +both keywords in its options block. This is the root cause of the disagreement below. + +`pattern-nodes-must-be-referenced` does not help with decision-holder placement. Its +recursive query matches a holder regardless of which array it sits in, so it cannot +tell a legal holder from an illegal one. + +## How decisions fold into the visualiser's boxes + +The pattern visualiser (`calm-hub-ui`) draws each decision as a box. Each rule below +is deliberate behaviour. + +**Every decision gets its own box.** Two decisions drawing from the same catalog +produce two boxes, each with its own prompt. + +**A candidate can be drawn in one box only.** If two decisions name the same +candidate, the first decision (in document order) keeps it. The second decision's box +still offers that candidate as a choice, but does not draw it. If every one of a +decision's candidates is claimed by an earlier decision, that decision renders no box +at all. + +**A container beats a decision box.** If a candidate is both a decision candidate and +a child of a container (`deployed-in` or `composed-of`), it is drawn inside the +container, not inside the choice box. + +**A decision whose candidates are themselves containers keeps its box.** The box is +drawn next to the containers, not around them. Issue #2933 covers nesting. + +**A decision whose every candidate is pulled into one shared container loses its box +entirely, prompt included.** This is the one case where container precedence removes +the question from the diagram, not just its candidates. The alternative was an empty +box with nothing inside it. Issue #2933 covers nesting the box inside the container, +so that the question survives. + +## A known disagreement: generation offers more than it resolves + +Four operations read a decision holder's **question block** - the `oneOf`/`anyOf` array +under `relationship-type.options.prefixItems[i]`, whose elements are choice bundles. Inside +that block the keyword sets cardinality. It is not a candidate declaration site, so the +`nodes`/`relationships` rules above do not apply to it. + +| Operation | Surface | Reads the question block as | +|---|---|---| +| `extractOptionsFromBlock` (`options.ts`) | generation, offering | union of both keywords | +| `flattenOneOfAndAnyOf` (`options.ts`) | generation, resolving | `oneOf` wins | +| `extractOptionsMetadata` (`patternTransformer.ts`) | visualisation | `oneOf` wins | +| `pattern-decision-must-reference-selectable-nodes` | validation | union, by JSONPath recursive descent | + +**The disagreement is inside `options.ts`.** `extractOptionsFromBlock` builds the prompt from +both keywords. `flattenOneOfAndAnyOf` then applies the answer through +`resolveOperativeChoiceBlock`, which resolves `oneOf` only. So a choice from the `anyOf` half +is offered, accepted, and then discarded. `calm generate` reports success, and the holder is +dropped from the output. + +`assertChoicesAreSelectable` does not catch this. It checks the answer's ids against +`listSelectableCandidates`, which reads the candidate declaration site, not the question +block. Ids declared normally there pass, so the guard stays silent. + +Neither other surface is a party to this. `extractOptionsMetadata` already matches +generation's resolution step. The validation rule enumerates and never resolves, so union is +correct for it, and it reads the block through a JSONPath `given` rather than a reader +function. + +**Which side changes.** Resolution cannot read the union. Within a question block `oneOf` +means pick exactly one and `anyOf` means pick any number, so a block declaring both would +have to be single-select and multi-select at once. Resolution must drop one keyword, and +`oneOf`-wins is already that rule everywhere else. Enumeration is the side that moves: +`extractOptionsFromBlock` should call `resolveOperativeChoiceBlock` too. A prompt that offers +what resolution discards is a defect, not a competing policy. + +**What this blocks.** `options.ts` and `patternTransformer.ts` each re-implement finding a +decision holder and reading its question blocks. `isOptionsRelationship` is identical in both. +The helpers beside it are not, and they diverge three ways. + +| Divergence | `options.ts` | `patternTransformer.ts` | +|---|---|---| +| A block declaring both keywords | union when offering, `oneOf` wins when resolving | `oneOf` wins | +| Question blocks read per holder | every entry in `options.prefixItems` | the first entry yielding a described choice, then returns | +| A malformed holder | unguarded property access, throws | optional chaining, renders nothing | + +The second divergence also breaks generation on its own. Two question blocks under one holder +emit two options, and both take `optionId` from the holder's `unique-id`, so they collide. The +effect differs by path. Interactively, both questions are asked and both answers are applied; +only the replayable `--option-choices` string that gets logged is lossy, because the second +answer overwrites the first under the shared key. Non-interactively, `loadChoicesFromInput` +resolves with `find`, so the second block's choices are unreachable and naming one is rejected +as invalid. + +No pattern in this repository declares more than one question block per holder. The format +allows it: `option-type` in the meta-schema is an unbounded array of `decision` objects, so a +holder is designed to be able to ask several questions. + +One shared reader would close all three. `calm-hub-ui` depends on `@finos/calm-models` and not +on `shared`, so it would live in `calm-models`, beside the readers above. It would have three +callers, and two of them already want the same thing, so the keyword policy above is what +stands in the way. diff --git a/shared/src/commands/generate/components/catalog-decisions.spec.ts b/shared/src/commands/generate/components/catalog-decisions.spec.ts new file mode 100644 index 000000000..a0aecefa5 --- /dev/null +++ b/shared/src/commands/generate/components/catalog-decisions.spec.ts @@ -0,0 +1,355 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { instantiate } from './instantiate'; +import { assertChoicesAreSelectable, extractOptions, selectChoices, CalmChoice, CalmOption } from './options'; +import { flattenAllOf } from './flatten-allof'; +import { SchemaDirectory } from '../../../schema-directory'; + +/** + * End-to-end coverage for items-catalog decisions: a holder in `relationships.prefixItems` + * selecting candidates from a `nodes.items` catalog. + * + * Driven the way `calm generate` drives it - `extractOptions`, then `selectChoices`, then + * `instantiate`. The catalog tests in `instantiate.spec.ts` hand-build their choices, so + * they prove a catalog can be consumed but not that a decision is discoverable. + */ + +vi.mock('fs'); + +vi.mock('../../../logger', () => ({ + initLogger: vi.fn(function () { + return { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + }) +})); + +const schemaDirMocks = vi.hoisted(() => ({ + loadSchemas: vi.fn(), + loadCurrentPatternAsSchema: vi.fn(), + getDefinition: vi.fn() +})); + +vi.mock('../../../schema-directory', () => ({ + SchemaDirectory: vi.fn(function () { + return { + loadSchemas: schemaDirMocks.loadSchemas, + loadCurrentPatternAsSchema: schemaDirMocks.loadCurrentPatternAsSchema, + getDefinition: schemaDirMocks.getDefinition + }; + }), +})); + +interface InstantiatedArchitecture { + nodes: Array>; + relationships: Array>; +} + +function node(uniqueId: string, name: string) { + return { + properties: { + 'unique-id': { const: uniqueId }, + 'name': { const: name }, + 'node-type': { const: 'service' } + } + }; +} + +function choice(description: string, nodeId: string) { + return { + properties: { + description: { const: description }, + nodes: { const: [nodeId] }, + relationships: { const: [] } + } + }; +} + +/** A decision holder: a relationship carrying `relationship-type.options`. */ +function decision(uniqueId: string, prompt: string, blockType: 'oneOf' | 'anyOf', choices: object[]) { + return { + properties: { + 'unique-id': { const: uniqueId }, + 'description': { const: prompt }, + 'relationship-type': { + type: 'object', + properties: { + options: { + type: 'array', + prefixItems: [{ [blockType]: choices }] + } + } + } + } + }; +} + +/** + * One mandatory node, four optional candidates in a single catalog, and two independent + * decisions drawing from it. A pattern has exactly one nodes catalog and only a catalog + * can express an optional node, so this is the shape any pattern offering two optional + * components must take. + */ +const twoDecisionPattern = { + $schema: 'schema#', + $id: 'two-decision-catalog-pattern', + properties: { + nodes: { + type: 'array', + prefixItems: [node('webapp', 'Web App')], + items: { + anyOf: [ + node('redis', 'Redis'), + node('memcached', 'Memcached'), + node('kafka', 'Kafka'), + node('rabbitmq', 'RabbitMQ'), + ] + } + }, + relationships: { + type: 'array', + prefixItems: [ + decision('cache-choice', 'Pick a cache', 'anyOf', [ + choice('Use Redis', 'redis'), + choice('Use Memcached', 'memcached'), + ]), + decision('queue-choice', 'Pick a queue', 'anyOf', [ + choice('Use Kafka', 'kafka'), + choice('Use RabbitMQ', 'rabbitmq'), + ]), + ] + } + } +}; + +async function generate(pattern: object, chosen: CalmChoice[]): Promise { + const selected = selectChoices(pattern, chosen); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return await instantiate(selected, false, new SchemaDirectory({} as any)) as unknown as InstantiatedArchitecture; +} + +const nodeIds = (arch: InstantiatedArchitecture) => arch.nodes.map((n) => n['unique-id']); + +/** Finds a named choice on a named decision, the way the CLI resolves `--option-choices`. */ +function pick(options: CalmOption[], optionId: string, description: string): CalmChoice { + const option = options.find((o) => o.optionId === optionId); + if (!option) throw new Error(`no such decision: ${optionId}`); + const found = option.choices.find((c) => c.description === description); + if (!found) throw new Error(`no such choice on ${optionId}: ${description}`); + return found; +} + +describe('items catalog decisions, end to end', () => { + beforeEach(() => { + vi.clearAllMocks(); + schemaDirMocks.getDefinition.mockResolvedValue({ type: 'object', properties: {} }); + }); + + describe('discovery', () => { + it('offers both decisions, each with its own prompt and choices', () => { + const options = extractOptions(twoDecisionPattern); + + expect(options.map((o) => o.optionId)).toEqual(['cache-choice', 'queue-choice']); + expect(options.map((o) => o.prompt)).toEqual(['Pick a cache', 'Pick a queue']); + expect(options.every((o) => o.optionType === 'anyOf')).toBe(true); + expect(options[0].choices.map((c) => c.description)).toEqual(['Use Redis', 'Use Memcached']); + expect(options[1].choices.map((c) => c.description)).toEqual(['Use Kafka', 'Use RabbitMQ']); + }); + + it('names only catalog candidates in its choices', () => { + const options = extractOptions(twoDecisionPattern); + expect(options.flatMap((o) => o.choices.flatMap((c) => c.nodes))) + .toEqual(['redis', 'memcached', 'kafka', 'rabbitmq']); + }); + }); + + describe('zero or more selection', () => { + it('materializes nothing from the catalog when no choice is made', async () => { + const arch = await generate(twoDecisionPattern, []); + expect(nodeIds(arch)).toEqual(['webapp']); + }); + + it('materializes one candidate per answered decision', async () => { + const options = extractOptions(twoDecisionPattern); + const arch = await generate(twoDecisionPattern, [ + pick(options, 'cache-choice', 'Use Redis'), + pick(options, 'queue-choice', 'Use Kafka'), + ]); + expect(nodeIds(arch)).toEqual(['webapp', 'redis', 'kafka']); + }); + + it('answers each decision independently — one answered, one left alone', async () => { + const options = extractOptions(twoDecisionPattern); + const arch = await generate(twoDecisionPattern, [ + pick(options, 'queue-choice', 'Use RabbitMQ'), + ]); + expect(nodeIds(arch)).toEqual(['webapp', 'rabbitmq']); + }); + + it('accepts several answers to one anyOf decision', async () => { + const options = extractOptions(twoDecisionPattern); + const arch = await generate(twoDecisionPattern, [ + pick(options, 'cache-choice', 'Use Redis'), + pick(options, 'cache-choice', 'Use Memcached'), + ]); + expect(nodeIds(arch)).toEqual(['webapp', 'redis', 'memcached']); + }); + + it('records the answer on the decision holder, which survives into the architecture', async () => { + // The holder is not scaffolding that gets stripped - it stays, carrying the + // chosen bundle, so the generated architecture records which option was taken. + const options = extractOptions(twoDecisionPattern); + const arch = await generate(twoDecisionPattern, [pick(options, 'cache-choice', 'Use Redis')]); + + const holder = arch.relationships.find((r) => r['unique-id'] === 'cache-choice'); + expect(holder).toBeDefined(); + expect(holder!['description']).toBe('Pick a cache'); + + const relationshipType = holder!['relationship-type'] as Record; + const recorded = relationshipType['options'] as Array>; + expect(recorded.map((o) => o['description'])).toEqual(['Use Redis']); + expect(recorded.flatMap((o) => o['nodes'] as string[])).toEqual(['redis']); + }); + + it('drops the unchosen alternatives from the recorded answer', async () => { + const options = extractOptions(twoDecisionPattern); + const arch = await generate(twoDecisionPattern, [pick(options, 'cache-choice', 'Use Memcached')]); + + const holder = arch.relationships.find((r) => r['unique-id'] === 'cache-choice')!; + const recorded = (holder['relationship-type'] as Record)['options'] as Array>; + expect(recorded.map((o) => o['description'])).toEqual(['Use Memcached']); + }); + + it('clears the catalog so the array is fully expressed by prefixItems', async () => { + const selected = selectChoices(twoDecisionPattern, []) as Record; + const nodes = selected['properties']['nodes']; + expect(nodes['items']).toBeUndefined(); + expect(nodes['prefixItems']).toHaveLength(1); + }); + }); + + describe('mandatory nodes are unaffected by decisions', () => { + it('always emits the prefixItems node regardless of answers', async () => { + const options = extractOptions(twoDecisionPattern); + const none = await generate(twoDecisionPattern, []); + const all = await generate(twoDecisionPattern, [ + pick(options, 'cache-choice', 'Use Redis'), + pick(options, 'cache-choice', 'Use Memcached'), + pick(options, 'queue-choice', 'Use Kafka'), + pick(options, 'queue-choice', 'Use RabbitMQ'), + ]); + expect(nodeIds(none)).toContain('webapp'); + expect(nodeIds(all)).toEqual(['webapp', 'redis', 'memcached', 'kafka', 'rabbitmq']); + }); + }); +}); + +describe('an answer that cannot be honoured is refused, not discarded', () => { + // The check lives on the generate path (runGenerate calls it), deliberately not inside + // selectChoices - validation calls selectChoices too, to replay an architecture's + // options onto its pattern, and a malformed pattern must surface its own schema errors + // there rather than be pre-empted by this one. + // extractOptions builds the prompt straight from a choice bundle without checking its + // ids resolve, so the user is offered the choice either way. Before this guard the + // answer was silently dropped and the architecture generated without it. + const catalogPattern = (items: object) => ({ + $schema: 'schema#', $id: 'unreachable', + properties: { + nodes: { type: 'array', prefixItems: [node('webapp', 'Web App')], items }, + relationships: { type: 'array', prefixItems: [] }, + } + }); + + it('throws when a choice bundle names a node id that does not exist', () => { + const pattern = catalogPattern({ oneOf: [node('redis', 'Redis')] }); + expect(() => assertChoicesAreSelectable(pattern, [ + { description: 'Use Redis', nodes: ['rediss'], relationships: [] }, + ])).toThrow(/rediss/); + }); + + it('throws when a catalog declares both keywords, making the anyOf side unreachable', () => { + // Legal JSON Schema, but only the oneOf list is resolved, so kafka can never be + // selected even though validation sees it as a properly declared node. + const pattern = catalogPattern({ oneOf: [node('redis', 'Redis')], anyOf: [node('kafka', 'Kafka')] }); + expect(() => assertChoicesAreSelectable(pattern, [ + { description: 'Use Kafka', nodes: ['kafka'], relationships: [] }, + ])).toThrow(/kafka/); + }); + + it('throws when a bundle names an unreachable relationship id', () => { + const pattern = { + $schema: 'schema#', $id: 'unreachable-rel', + properties: { + nodes: { type: 'array', prefixItems: [node('webapp', 'Web App')] }, + relationships: { type: 'array', prefixItems: [] }, + } + }; + expect(() => assertChoicesAreSelectable(pattern, [ + { description: 'Link them', nodes: [], relationships: ['ghost-link'] }, + ])).toThrow(/ghost-link/); + }); + + it('accepts ids reachable through a catalog, a slot, or a plain entry', () => { + expect(() => assertChoicesAreSelectable(catalogPattern({ oneOf: [node('redis', 'Redis')] }), [ + { description: 'Use Redis', nodes: ['redis'], relationships: [] }, + ])).not.toThrow(); + + const slotPattern = { + $schema: 'schema#', $id: 'slot', + properties: { + nodes: { type: 'array', prefixItems: [{ oneOf: [node('a', 'A'), node('b', 'B')] }] }, + relationships: { type: 'array', prefixItems: [] }, + } + }; + expect(() => assertChoicesAreSelectable(slotPattern, [ + { description: 'Use A', nodes: ['a'], relationships: [] }, + ])).not.toThrow(); + }); +}); + +/** + * Pins the real division of labour for a decision declared inside an `allOf` branch: + * `extractOptions` discovers it on the raw pattern (`getPatternArray` falls back into + * `allOf`, matching what `main` already did by hand before this reader existed), and + * `runGenerate` flattens the same raw pattern with `flattenAllOf` - pre-existing, unrelated + * to this PR - before the guard or selection ever run. Neither the guard nor selection has + * ever tolerated a raw `allOf` pattern; `main`'s own `flattenCalmItems` threw on one with no + * fallback at all. So the guard is exercised here the way `runGenerate` actually calls it - + * post-flatten - not directly against the raw pattern `extractOptions` reads. + */ +describe('decisions declared under allOf', () => { + const allOfDecisionPattern = { + $schema: 'schema#', + $id: 'allof-decision-pattern', + allOf: [ + { + properties: { + nodes: { + type: 'array', + prefixItems: [node('webapp', 'Web App')], + items: { anyOf: [node('redis', 'Redis'), node('memcached', 'Memcached')] } + }, + relationships: { + type: 'array', + prefixItems: [ + decision('cache-choice', 'Pick a cache', 'anyOf', [ + choice('Use Redis', 'redis'), + choice('Use Memcached', 'memcached'), + ]) + ] + } + } + } + ] + }; + + it('are discovered by extractOptions', () => { + const options = extractOptions(allOfDecisionPattern); + expect(options.map((o) => o.optionId)).toEqual(['cache-choice']); + }); + + it('accept an answer the guard can resolve to a real candidate, once flattened as runGenerate flattens it', async () => { + const options = extractOptions(allOfDecisionPattern); + const chosen = pick(options, 'cache-choice', 'Use Redis'); + + const flattened = await flattenAllOf(allOfDecisionPattern, new SchemaDirectory({} as never), false); + expect(() => assertChoicesAreSelectable(flattened as never, [chosen])).not.toThrow(); + }); +}); diff --git a/shared/src/commands/generate/components/decision-agreement.spec.ts b/shared/src/commands/generate/components/decision-agreement.spec.ts new file mode 100644 index 000000000..89636484e --- /dev/null +++ b/shared/src/commands/generate/components/decision-agreement.spec.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi } from 'vitest'; +import fs from 'fs'; +import path from 'path'; +import { instantiate } from './instantiate'; +import { extractOptions, selectChoices, CalmChoice } from './options'; +import { SchemaDirectory } from '../../../schema-directory'; + +/** + * The generation half of the decision-agreement contract. See + * `test_fixtures/decision-agreement/README.md` - the visualiser asserts the same two + * things against the same files, so a drift on either side fails a test. + */ + +vi.mock('fs', async () => { + const actual = await vi.importActual('fs'); + return { ...actual, default: actual }; +}); + +vi.mock('../../../logger', () => ({ + initLogger: vi.fn(() => ({ info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() })) +})); + +const schemaDirMocks = vi.hoisted(() => ({ + loadSchemas: vi.fn(), + loadCurrentPatternAsSchema: vi.fn(), + getDefinition: vi.fn() +})); + +vi.mock('../../../schema-directory', () => ({ + SchemaDirectory: vi.fn(function () { + return { + loadSchemas: schemaDirMocks.loadSchemas, + loadCurrentPatternAsSchema: schemaDirMocks.loadCurrentPatternAsSchema, + getDefinition: schemaDirMocks.getDefinition + }; + }), +})); + +const FIXTURES = path.resolve(__dirname, '../../../../../test_fixtures/decision-agreement'); + +interface ExpectedDecision { + optionId: string; + prompt: string; + optionType: 'oneOf' | 'anyOf'; + choices: { description: string; nodes: string[]; relationships: string[] }[]; +} +interface Expected { + decisions: ExpectedDecision[]; + answered: { choose: Record; nodes: string[] }[]; +} + +const read = (name: string, suffix: string) => + JSON.parse(fs.readFileSync(path.join(FIXTURES, `${name}.${suffix}.json`), 'utf8')); + +const cases = ['one-decision-one-catalog', 'two-decisions-one-catalog']; + +describe.each(cases)('decision agreement: %s (generation side)', (name) => { + const pattern = read(name, 'pattern'); + const expected: Expected = read(name, 'expected'); + + it('offers exactly the expected decisions', () => { + const actual = extractOptions(pattern).map((o) => ({ + optionId: o.optionId, + prompt: o.prompt, + optionType: o.optionType, + choices: o.choices.map((c) => ({ + description: c.description, + nodes: c.nodes, + relationships: c.relationships, + })), + })); + expect(actual).toEqual(expected.decisions); + }); + + it.each(expected.answered)('produces the expected nodes for $choose', async ({ choose, nodes }) => { + const options = extractOptions(pattern); + const chosen: CalmChoice[] = Object.entries(choose).map(([optionId, description]) => { + const option = options.find((o) => o.optionId === optionId); + const choice = option?.choices.find((c) => c.description === description); + if (!choice) throw new Error(`fixture names a choice that does not exist: ${optionId} / ${description}`); + return choice; + }); + + const selected = selectChoices(pattern, chosen); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const arch = await instantiate(selected, false, new SchemaDirectory({} as any)) as unknown as { + nodes: Record[]; + }; + expect(arch.nodes.map((n) => n['unique-id']).sort()).toEqual([...nodes].sort()); + }); +}); diff --git a/shared/src/commands/generate/components/flatten-allof.spec.ts b/shared/src/commands/generate/components/flatten-allof.spec.ts index c75fc8f07..75521ea64 100644 --- a/shared/src/commands/generate/components/flatten-allof.spec.ts +++ b/shared/src/commands/generate/components/flatten-allof.spec.ts @@ -2,6 +2,19 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { flattenAllOf } from './flatten-allof'; import { SchemaDirectory } from '../../../schema-directory'; +// Spy on the logger's debug and warn channels so discarded-key warnings can be asserted. +// Hoisted so the (hoisted) vi.mock factory below can reference them. +const { mockDebug, mockWarn } = vi.hoisted(() => ({ mockDebug: vi.fn(), mockWarn: vi.fn() })); +vi.mock('../../../logger', () => ({ + initLogger: () => ({ + log: vi.fn(), + debug: mockDebug, + info: vi.fn(), + warn: mockWarn, + error: vi.fn(), + }), +})); + // Mock SchemaDirectory const mockSchemaDir = { getDefinition: vi.fn(), @@ -235,4 +248,120 @@ describe('flattenAllOf', () => { level3: { type: 'string' } }); }); + + describe('discarded-key debug logging across allOf', () => { + it('logs at debug when two allOf branches each declare a nodes items catalog', async () => { + // Both branches declare `properties.nodes.items`; the shallow properties + // merge makes the later branch's catalog win and silently drops the first. + const schema = { + allOf: [ + { properties: { nodes: { items: { oneOf: [{ const: 'a' }] } } } }, + { properties: { nodes: { items: { oneOf: [{ const: 'b' }] } } } }, + ], + }; + + await flattenAllOf(schema, mockSchemaDir, true); + + expect(mockDebug).toHaveBeenCalledWith( + expect.stringContaining('allOf merge on property \'nodes\' discarded keys [items] declared in an earlier branch') + ); + }); + + it('does not log when only one allOf branch declares a catalog', async () => { + const schema = { + allOf: [ + { properties: { nodes: { items: { oneOf: [{ const: 'a' }] } } } }, + { properties: { relationships: { prefixItems: [] } } }, + ], + }; + + await flattenAllOf(schema, mockSchemaDir, true); + + expect(mockDebug).not.toHaveBeenCalledWith(expect.stringContaining('discarded keys')); + }); + + it('names prefixItems as discarded, not the catalog, when the catalog is in the later branch', async () => { + // The later branch is the one that survives the merge, so it is the earlier + // branch's prefixItems that is lost here — not the catalog that replaces it. + const schema = { + allOf: [ + { + properties: { + nodes: { + type: 'array', + prefixItems: [{ properties: { 'unique-id': { const: 'a' } } }], + }, + }, + }, + { + properties: { + nodes: { + type: 'array', + items: { oneOf: [{ properties: { 'unique-id': { const: 'b' } } }] }, + }, + }, + }, + ], + }; + + await flattenAllOf(schema, mockSchemaDir, true); + + expect(mockDebug).toHaveBeenCalledWith( + expect.stringContaining('allOf merge on property \'nodes\' discarded keys [prefixItems] declared in an earlier branch') + ); + }); + + it('logs at debug and names [type, prefixItems] on a prefixItems + minItems collision with no catalog anywhere', async () => { + const schema = { + allOf: [ + { + properties: { + nodes: { + type: 'array', + prefixItems: [{ properties: { 'unique-id': { const: 'a' } } }], + }, + }, + }, + { properties: { nodes: { minItems: 1 } } }, + ], + }; + + await flattenAllOf(schema, mockSchemaDir, true); + + expect(mockDebug).toHaveBeenCalledWith( + expect.stringContaining('allOf merge on property \'nodes\' discarded keys [type, prefixItems] declared in an earlier branch') + ); + }); + + it('does not log on a $ref refinement where the resolved def and siblings both declare the same property', async () => { + // allOf: [{ $ref: ..., properties: {...} }] is the idiomatic composition form: + // refining a $ref'd definition with local sibling keys is ordinary JSON Schema, + // not a lossy allOf-branch collision, even though it discards the same keys. + const referencedSchema = { + type: 'object', + properties: { + nodes: { + type: 'array', + prefixItems: [{ properties: { 'unique-id': { const: 'ref-node' } } }], + }, + }, + }; + (mockSchemaDir.getDefinition as ReturnType).mockResolvedValueOnce(referencedSchema); + + const schema = { + allOf: [ + { + $ref: 'https://example.com/base-schema.json', + properties: { + nodes: { minItems: 1 }, + }, + }, + ], + }; + + await flattenAllOf(schema, mockSchemaDir, true); + + expect(mockDebug).not.toHaveBeenCalledWith(expect.stringContaining('discarded keys')); + }); + }); }); diff --git a/shared/src/commands/generate/components/flatten-allof.ts b/shared/src/commands/generate/components/flatten-allof.ts index 2fd2e7933..ef2f3e4d3 100644 --- a/shared/src/commands/generate/components/flatten-allof.ts +++ b/shared/src/commands/generate/components/flatten-allof.ts @@ -1,5 +1,5 @@ import { SchemaDirectory } from '../../../schema-directory'; -import { initLogger } from '../../../logger'; +import { initLogger, Logger } from '../../../logger'; interface SchemaWithAllOf { allOf?: object[]; @@ -11,12 +11,144 @@ interface SchemaWithAllOf { [key: string]: unknown; } +// Which branch of the merge is discarding keys, so the warning can name the right direction. +type DiscardWarning = false | 'allOf-branch' | 'root-override'; + +const VALUE_SET_KEYWORDS = new Set(['enum', 'type', 'required']); + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isStructural(value: unknown): boolean { + return Array.isArray(value) || isPlainObject(value); +} + +function isEmptyStructural(value: unknown): boolean { + if (Array.isArray(value)) return value.length === 0; + if (isPlainObject(value)) return Object.keys(value).length === 0; + return false; +} + +function deepEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && a.every((v, i) => deepEqual(v, b[i])); + } + if (isPlainObject(a) && isPlainObject(b)) { + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + return aKeys.length === bKeys.length && aKeys.every((k) => k in b && deepEqual(a[k], b[k])); + } + return false; +} + +/** + * True when `later` is deep-equal to `earlier`, or structurally contains/extends it: + * every entry of an earlier array is present in the later array (set containment, + * order-insensitive), or every key of an earlier object is present in the later object + * with a value that itself contains/extends the earlier one. + */ +function isContainedOrExtended(earlier: unknown, later: unknown): boolean { + if (deepEqual(earlier, later)) return true; + if (Array.isArray(earlier)) { + return Array.isArray(later) && earlier.every((e) => later.some((l) => deepEqual(e, l))); + } + if (isPlainObject(earlier)) { + return ( + isPlainObject(later) && + Object.entries(earlier).every( + ([k, v]) => k in later && isContainedOrExtended(v, later[k]) + ) + ); + } + return false; +} + +/** + * Names the keys of an earlier property definition that are lost when it is replaced + * wholesale by a later one (see `deepMergeSchemas`' `properties` branch). A key is + * discarded when: (1) the later definition omits it; (2) the earlier value is a + * non-empty object/array that the later value neither equals nor contains/extends; or + * (3) the earlier value is a scalar and the later value is an object or array. Narrowing + * a value-set keyword (`enum`, `type`, `required`) and redefining a scalar as a different + * scalar are legitimate `allOf` refinement, not loss. + */ +function computeDiscardedKeys( + earlier: Record, + later: Record +): string[] { + const discarded: string[] = []; + + for (const key of Object.keys(earlier)) { + const earlierVal = earlier[key]; + + if (!(key in later)) { + discarded.push(key); // rule 1 + continue; + } + + const laterVal = later[key]; + + if (isContainedOrExtended(earlierVal, laterVal)) continue; + + if (VALUE_SET_KEYWORDS.has(key) && Array.isArray(earlierVal) && Array.isArray(laterVal)) { + const isNarrowing = laterVal.every((l) => earlierVal.some((e) => deepEqual(e, l))); + if (isNarrowing) continue; + } + + if (isStructural(earlierVal) && !isEmptyStructural(earlierVal)) { + discarded.push(key); // rule 2 + continue; + } + + if (!isStructural(earlierVal) && isStructural(laterVal)) { + discarded.push(key); // rule 3 + } + // Remaining case: scalar redefined as a different scalar — legitimate refinement. + } + + return discarded; +} + +function warnOnDiscardedKeys( + propKey: string, + earlierVal: unknown, + laterVal: unknown, + direction: 'allOf-branch' | 'root-override', + logger: Logger +): void { + if (!isPlainObject(earlierVal) || !isPlainObject(laterVal)) return; + + const discardedKeys = computeDiscardedKeys(earlierVal, laterVal); + if (discardedKeys.length === 0) return; + + const message = + direction === 'allOf-branch' + ? `allOf merge on property '${propKey}' discarded keys [${discardedKeys.join(', ')}] declared in an earlier branch. This is a limitation of allOf merging, not an error in the pattern.` + : `allOf merge on property '${propKey}' discarded keys [${discardedKeys.join(', ')}] declared in an allOf branch and overridden by the schema's own properties.`; + + // Debug, not warn: this is new machinery over a construct the pattern's own + // documentation already declares unsupported (`allOf` for nodes/relationships), and a + // false "discarded" report on legitimate refinement is a support question with no + // action attached. Worth surfacing for someone diagnosing the merge, not worth + // interrupting everyone else's normal output. + logger.debug(message); +} + /** * Deep merges two schema objects, combining properties, required arrays, and prefixItems. + * + * @param warnOnDiscard - When set, logs (via `logger.debug`) about property keys that the + * `properties` branch's shallow spread discards. `'allOf-branch'` and `'root-override'` + * select the message's direction; `false` (the default) stays silent, which positional + * `prefixItems` merges and `$ref`-refinement merges rely on. */ function deepMergeSchemas( target: Record, - source: Record + source: Record, + logger: Logger, + warnOnDiscard: DiscardWarning = false ): Record { const result = { ...target }; @@ -24,10 +156,26 @@ function deepMergeSchemas( if (value === undefined) continue; if (key === 'properties' && result.properties) { + const targetProps = result.properties as Record; + const sourceProps = value as Record; + + if (warnOnDiscard) { + for (const propKey of Object.keys(sourceProps)) { + if (!(propKey in targetProps)) continue; + warnOnDiscardedKeys( + propKey, + targetProps[propKey], + sourceProps[propKey], + warnOnDiscard, + logger + ); + } + } + // Merge properties objects result.properties = { - ...(result.properties as Record), - ...(value as Record), + ...targetProps, + ...sourceProps, }; } else if (key === 'required' && result.required) { // Combine required arrays, removing duplicates @@ -38,9 +186,11 @@ function deepMergeSchemas( // Merge prefixItems by position result.prefixItems = mergePrefixItems( result.prefixItems as unknown[], - value as unknown[] + value as unknown[], + logger ); } else { + // Any other key is replaced rather than deep-merged. result[key] = value; } } @@ -51,17 +201,22 @@ function deepMergeSchemas( /** * Merges two prefixItems arrays by position, combining schemas at each index. */ -function mergePrefixItems(target: unknown[], source: unknown[]): unknown[] { +function mergePrefixItems(target: unknown[], source: unknown[], logger: Logger): unknown[] { const maxLen = Math.max(target.length, source.length); const result: unknown[] = []; for (let i = 0; i < maxLen; i++) { if (i < target.length && i < source.length) { - // Both have items at this position - merge them + // Both have items at this position - merge them. Positional merges stay silent: + // the result fuses index i of two branches into a schema neither branch declares + // on its own, so there is no authored declaration whose lost keys are worth + // reporting. result.push( deepMergeSchemas( target[i] as Record, - source[i] as Record + source[i] as Record, + logger, + false ) ); } else if (i < target.length) { @@ -94,10 +249,12 @@ export async function flattenAllOf( if (schema.$ref && !schema.allOf) { logger.debug(`Resolving root $ref: ${schema.$ref}`); const resolved = await schemaDir.getDefinition(schema.$ref); - // Merge any additional properties from the original schema + // Merge any additional properties from the original schema. Refining a $ref'd + // definition with local sibling keys is ordinary JSON Schema composition, so this + // merge stays silent even when it discards a key. const { $ref: _$ref, ...rest } = schema; const flattened = await flattenAllOf(resolved as SchemaWithAllOf, schemaDir, debug); - return deepMergeSchemas(flattened as Record, rest); + return deepMergeSchemas(flattened as Record, rest, logger); } // If no allOf, return schema as-is @@ -118,24 +275,30 @@ export async function flattenAllOf( if (resolved.$ref) { logger.debug(`Resolving $ref in allOf: ${resolved.$ref}`); const refResolved = await schemaDir.getDefinition(resolved.$ref); - // Merge any additional properties from the $ref schema + // Merge any additional properties from the $ref schema. Same $ref-refinement + // case as above — stays silent. const { $ref: _$ref, ...rest } = resolved; resolved = deepMergeSchemas( refResolved as Record, - rest + rest, + logger ) as SchemaWithAllOf; } // Recursively flatten nested allOf resolved = (await flattenAllOf(resolved, schemaDir, debug)) as SchemaWithAllOf; - // Deep merge into accumulated result - merged = deepMergeSchemas(merged, resolved); + // Deep merge into accumulated result. Two allOf branches are being merged here, so + // a key an earlier branch declared and this one discards is worth a warning. + merged = deepMergeSchemas(merged, resolved, logger, 'allOf-branch'); } - // Preserve top-level fields that aren't part of allOf (like $id, $schema, title, etc.) + // Preserve top-level fields that aren't part of allOf (like $id, $schema, title, etc.). + // This merges the root schema's own keys OVER the allOf result, so a discarded key here + // was declared in an allOf branch and lost to the schema's own properties — the + // opposite direction from the loop above. const { allOf: _allOf, ...rest } = schema; - merged = deepMergeSchemas(merged, rest); + merged = deepMergeSchemas(merged, rest, logger, 'root-override'); logger.debug(`Flattened schema has properties: ${Object.keys(merged.properties || {}).join(', ')}`); diff --git a/shared/src/commands/generate/components/instantiate.spec.ts b/shared/src/commands/generate/components/instantiate.spec.ts index dff2621e9..a7f73bf58 100644 --- a/shared/src/commands/generate/components/instantiate.spec.ts +++ b/shared/src/commands/generate/components/instantiate.spec.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, Mock } from 'vitest'; import * as fs from 'fs'; import { instantiate } from './instantiate'; // replace with actual relative path +import { CalmChoice, selectChoices } from './options'; import { SchemaDirectory } from '../../../schema-directory'; import { DocumentLoader } from '../../../document-loader/document-loader'; @@ -294,4 +295,139 @@ describe('instantiate', () => { 'nested-placeholder': '[[ NESTED_PLACEHOLDER ]]' }); }); + + describe('items catalog (open oneOf/anyOf) support', () => { + function catalogNode(id: string, description: string) { + return { + $ref: 'schema#/defs/node', + properties: { + 'unique-id': { const: id }, + 'description': { const: description }, + 'details': { + type: 'object', + properties: { arch: { type: 'string' } } + } + }, + required: ['unique-id', 'details'] + }; + } + + const patternWithItemsCatalog = { + $schema: 'schema#', + $id: 'test-pattern-items-catalog', + properties: { + nodes: { + type: 'array', + prefixItems: [ + { + $ref: 'schema#/defs/node', + properties: { + 'unique-id': { const: 'webapp' }, + 'description': { const: 'the web app' }, + 'details': { type: 'object', properties: { arch: { type: 'string' } } } + }, + required: ['unique-id', 'details'] + } + ], + items: { + oneOf: [ + catalogNode('cache', 'an optional cache'), + catalogNode('queue', 'an optional queue'), + ] + } + }, + relationships: { + type: 'array', + prefixItems: [] + } + } + }; + + it('instantiates an architecture containing exactly the chosen items-catalog nodes', async () => { + const cacheChoice: CalmChoice = { description: 'Use cache', nodes: ['cache'], relationships: [] }; + const selected = selectChoices(patternWithItemsCatalog, [cacheChoice]); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await instantiate(selected, true, new SchemaDirectory({} as any)) as TestInstantiatedPattern; + + const nodeIds = result.nodes.map((n) => n['unique-id']); + expect(nodeIds).toEqual(['webapp', 'cache']); + }); + + it('yields only the mandatory nodes when no items-catalog candidates are selected', async () => { + const selected = selectChoices(patternWithItemsCatalog, []); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await instantiate(selected, true, new SchemaDirectory({} as any)) as TestInstantiatedPattern; + + const nodeIds = result.nodes.map((n) => n['unique-id']); + expect(nodeIds).toEqual(['webapp']); + }); + + it('does not throw for a nodes array with only an items catalog and no prefixItems', async () => { + const catalogOnlyPattern = { + $schema: 'schema#', + $id: 'catalog-only-pattern', + properties: { + nodes: { + type: 'array', + items: { oneOf: [catalogNode('cache', 'an optional cache')] } + }, + relationships: { type: 'array', prefixItems: [] } + } + }; + const cacheChoice: CalmChoice = { description: 'Use cache', nodes: ['cache'], relationships: [] }; + const selected = selectChoices(catalogOnlyPattern, [cacheChoice]); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await instantiate(selected, true, new SchemaDirectory({} as any)) as TestInstantiatedPattern; + + expect(result.nodes.map((n) => n['unique-id'])).toEqual(['cache']); + }); + + it('materializes an array-typed property that carries a const rather than emptying it to []', async () => { + // Guards branch ordering in instantiateFromProperties: the const check must + // run before the bare-array fallback, or an array carrying a const would be + // wrongly emitted as []. + const patternWithConstArray = { + $schema: 'schema#', + $id: 'const-array-pattern', + properties: { + nodes: { type: 'array', prefixItems: [] }, + relationships: { type: 'array', prefixItems: [] }, + 'some-array': { type: 'array', const: ['a', 'b'] } + } + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await instantiate(patternWithConstArray, true, new SchemaDirectory({} as any)) as TestInstantiatedPattern; + + expect(result['some-array']).toEqual(['a', 'b']); + }); + + it('emits an empty array (not an object) for an all-optional catalog generated with no selections', async () => { + // Regression guard: a nodes array declared entirely through an items + // catalog (no prefixItems) that is instantiated WITHOUT selectChoices + // ever running - i.e. `calm generate` with no choices made. There are + // no chosen candidates to move into prefixItems, so the array is empty. + // It must materialize as `[]`, not `{}`, or the architecture is invalid. + const catalogOnlyPattern = { + $schema: 'schema#', + $id: 'catalog-only-pattern-no-selection', + properties: { + nodes: { + type: 'array', + items: { oneOf: [catalogNode('cache', 'an optional cache')] } + }, + relationships: { type: 'array', prefixItems: [] } + } + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await instantiate(catalogOnlyPattern, true, new SchemaDirectory({} as any)) as TestInstantiatedPattern; + + expect(Array.isArray(result.nodes)).toBe(true); + expect(result.nodes).toEqual([]); + }); + }); }); diff --git a/shared/src/commands/generate/components/instantiate.ts b/shared/src/commands/generate/components/instantiate.ts index 2d6170c65..dac34391e 100644 --- a/shared/src/commands/generate/components/instantiate.ts +++ b/shared/src/commands/generate/components/instantiate.ts @@ -99,6 +99,11 @@ async function instantiateFromProperties( for (const [key, def] of Object.entries(properties)) { const resolvedDef = await resolveSchema(def as JsonSchema, schemaDir); + // Arrays declaring an `items.oneOf`/`items.anyOf` open catalog (in addition + // to `prefixItems`) are normalized down to a single `prefixItems` array by + // `selectChoices()` before `instantiate()` runs, so no separate `items` + // handling is needed here - the selected catalog entries are materialized + // via the same `prefixItems` path below. if (resolvedDef.type === 'array' && resolvedDef.prefixItems) { output[key] = await Promise.all( resolvedDef.prefixItems.map(async (itemDef, idx) => { @@ -109,13 +114,23 @@ async function instantiateFromProperties( return await instantiateObject(resolvedItem, schemaDir, [key, `${idx}`]); }) ); + } else if (resolvedDef.const !== undefined) { + // const value at the top level. Checked before the bare-array fallback + // below so an array schema that carries a `const` still materializes its + // const value rather than being emptied to []. + output[key] = resolvedDef.const; + } else if (resolvedDef.type === 'array') { + // An array with no `prefixItems` to materialize - e.g. a nodes array + // whose entries are declared entirely through an `items.oneOf`/`anyOf` + // open catalog, generated with no choices selected (so `selectChoices()` + // never ran to move any chosen candidates into `prefixItems`). The + // correct instance is an empty array. Without this branch the value + // would fall through to `instantiateObject` and be emitted as `{}`, + // producing a structurally invalid architecture (an object where an + // array is required). + output[key] = []; } else { - // Check for const values at the top level - if (resolvedDef.const !== undefined) { - output[key] = resolvedDef.const; - } else { - output[key] = await instantiateObject(resolvedDef, schemaDir, [key]); - } + output[key] = await instantiateObject(resolvedDef, schemaDir, [key]); } } diff --git a/shared/src/commands/generate/components/options.spec.ts b/shared/src/commands/generate/components/options.spec.ts index 3c6f6a352..db9340f42 100644 --- a/shared/src/commands/generate/components/options.spec.ts +++ b/shared/src/commands/generate/components/options.spec.ts @@ -1,6 +1,6 @@ -import { CalmChoice, CalmOption, extractOptions, selectChoices } from './options'; +import { assertChoicesAreSelectable, CalmChoice, CalmOption, extractOptions, selectChoices } from './options'; const applicationAtoC: CalmChoice = { description: 'Application A connects to Application C', @@ -126,6 +126,27 @@ function buildPattern(nodes: object[], relationships: object[]) { }; } +// Builds a pattern with mandatory prefixItems plus an open items.oneOf/anyOf catalog +function buildPatternWithItemsCatalog( + mandatoryNodes: object[], + catalogNodes: object[], + relationships: object[] = [], + catalogRelationships: object[] = [] +) { + return { + 'properties': { + 'nodes': { + 'prefixItems': mandatoryNodes, + ...(catalogNodes.length > 0 && { 'items': { 'oneOf': catalogNodes } }), + }, + 'relationships': { + 'prefixItems': relationships, + ...(catalogRelationships.length > 0 && { 'items': { 'oneOf': catalogRelationships } }), + }, + }, + }; +} + describe('Pattern Options', () => { describe('optionsFor', () => { it('should return a oneOf option from a spec', () => { @@ -300,6 +321,128 @@ describe('Pattern Options', () => { expect(pattern).toEqual(patternBeforeSelection); }); + it('should move selected items-catalog candidates into prefixItems', () => { + const webapp = buildNode('webapp'); + const cache = buildNode('cache'); + const queue = buildNode('queue'); + + const pattern = buildPatternWithItemsCatalog([webapp], [cache, queue]); + + const cacheChoice: CalmChoice = { description: 'Use cache', nodes: ['cache'], relationships: [] }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = selectChoices(pattern, [cacheChoice]) as any; + + expect(result.properties.nodes.prefixItems).toEqual([webapp, cache]); + expect(result.properties.nodes.items).toBeUndefined(); + }); + + it('should keep only the mandatory nodes when no items-catalog candidates are selected', () => { + const webapp = buildNode('webapp'); + const cache = buildNode('cache'); + const queue = buildNode('queue'); + + const pattern = buildPatternWithItemsCatalog([webapp], [cache, queue]); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = selectChoices(pattern, []) as any; + + expect(result.properties.nodes.prefixItems).toEqual([webapp]); + }); + + it('should not throw when a calmType has only an items catalog and no prefixItems', () => { + const cache = buildNode('cache'); + const pattern = { + 'properties': { + 'nodes': { 'items': { 'oneOf': [cache] } }, + 'relationships': { 'prefixItems': [] }, + }, + }; + + const cacheChoice: CalmChoice = { description: 'Use cache', nodes: ['cache'], relationships: [] }; + + expect(() => selectChoices(pattern, [cacheChoice])).not.toThrow(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = selectChoices(pattern, [cacheChoice]) as any; + expect(result.properties.nodes.prefixItems).toEqual([cache]); + }); + + it('should move a selected items-catalog relationship into prefixItems and delete items', () => { + const webapp = buildNode('webapp'); + const edge = buildConnectsRelationship('webapp-to-db', 'webapp to db', 'webapp', 'db'); + + const pattern = buildPatternWithItemsCatalog([webapp], [], [], [edge]); + + const wireChoice: CalmChoice = { description: 'Wire db', nodes: [], relationships: ['webapp-to-db'] }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = selectChoices(pattern, [wireChoice]) as any; + + expect(result.properties.relationships.prefixItems).toEqual([edge]); + expect(result.properties.relationships.items).toBeUndefined(); + }); + + it('should not throw when a pattern has a nodes items catalog and no relationships property', () => { + const cache = buildNode('cache'); + // No `relationships` property at all - the shape a nodes-only catalog pattern takes. + const pattern = { + 'properties': { + 'nodes': { 'items': { 'oneOf': [cache] } }, + }, + }; + + const cacheChoice: CalmChoice = { description: 'Use cache', nodes: ['cache'], relationships: [] }; + + expect(() => selectChoices(pattern, [cacheChoice])).not.toThrow(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = selectChoices(pattern, [cacheChoice]) as any; + expect(result.properties.nodes.prefixItems).toEqual([cache]); + }); + + it('recovers from a malformed items-catalog oneOf ({}) via the sibling anyOf, instead of throwing', () => { + // `oneOf: {}` is not an array, so the old `??`-based selection treated it as + // present and threw ("catalogAlternatives.filter is not a function") instead of + // falling through to the valid `anyOf`. + const webapp = buildNode('webapp'); + const cache = buildNode('cache'); + const pattern = { + 'properties': { + 'nodes': { + 'prefixItems': [webapp], + // eslint-disable-next-line @typescript-eslint/no-explicit-any + 'items': { 'oneOf': {} as any, 'anyOf': [cache] }, + }, + 'relationships': { 'prefixItems': [] }, + }, + }; + const cacheChoice: CalmChoice = { description: 'Use cache', nodes: ['cache'], relationships: [] }; + + expect(() => selectChoices(pattern, [cacheChoice])).not.toThrow(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = selectChoices(pattern, [cacheChoice]) as any; + expect(result.properties.nodes.prefixItems).toEqual([webapp, cache]); + }); + + it('throws on a malformed prefixItems slot (oneOf: {}) instead of emitting it as a candidate', () => { + // A slot with a truthy but non-array `oneOf` and no `anyOf` is not a valid + // choice block. Passing it through unflattened would emit the malformed + // `{ oneOf: {} }` object itself as a node candidate in generated output. + const pattern = { + 'properties': { + 'nodes': { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + 'prefixItems': [{ 'oneOf': {} as any }], + }, + 'relationships': { 'prefixItems': [] }, + }, + }; + + expect(() => selectChoices(pattern, [])).toThrow(/Malformed oneOf\/anyOf block/); + }); + it('should not affect a normal pattern', () => { const applicationA = buildNode('application-a'); const applicationB = buildNode('application-b'); @@ -318,6 +461,50 @@ describe('Pattern Options', () => { ); expect(selectChoices(pattern, [])).toEqual(expectedPattern); }); + + it('drops a decision holder entirely when nothing is selected for it, instead of an illegal empty options.prefixItems', () => { + const applicationA = buildNode('application-a'); + const applicationC = buildNode('application-c'); + const connectsRelationshipA = buildConnectsRelationship('application-a-to-c', 'app a to app c', 'application-a', 'application-c'); + + const pattern = buildPattern( + [applicationA, applicationC], + [ + buildPatternOptionRelationship( + 'cache-choice', + 'Pick a cache', + buildPatternOption('anyOf', buildPatternChoice(applicationAtoC)) + ), + buildPatternOptionRelationship( + 'queue-choice', + 'Pick a queue', + buildPatternOption('anyOf', buildPatternChoice(applicationBtoC)) + ), + connectsRelationshipA, + ] + ); + + // Answer cache-choice; leave queue-choice with zero selections - the checkbox + // (anyOf) case the feature advertises as a legitimate answer. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = selectChoices(pattern, [applicationAtoC]) as any; + + const relationshipIds = result.properties.relationships.prefixItems + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .map((r: any) => r.properties['unique-id'].const); + expect(relationshipIds).not.toContain('queue-choice'); + expect(relationshipIds).toContain('cache-choice'); + + // An options.prefixItems of length 0 is not a legal JSON Schema (prefixItems + // must hold at least one entry) - compiling exactly that is what broke + // `calm validate` on this input before the fix. + for (const rel of result.properties.relationships.prefixItems) { + const options = rel.properties?.['relationship-type']?.properties?.options; + if (options) { + expect(options.prefixItems.length).toBeGreaterThan(0); + } + } + }); }); describe('allOf pattern support', () => { @@ -394,4 +581,59 @@ describe('Pattern Options', () => { expect(extractOptions(allOfPattern)).toEqual(expectedOptions); }); }); + + describe('assertChoicesAreSelectable', () => { + function choice(description: string, nodes: string[] = [], relationships: string[] = []): CalmChoice { + return { description, nodes, relationships }; + } + + it('does not throw when every choice names a plain prefixItems candidate', () => { + const pattern = buildPattern([buildNode('webapp')], []); + expect(() => assertChoicesAreSelectable(pattern, [choice('pick webapp', ['webapp'])])).not.toThrow(); + }); + + it('does not throw when every choice names a reachable items-catalog candidate', () => { + const pattern = buildPatternWithItemsCatalog([], [buildNode('redis')]); + expect(() => assertChoicesAreSelectable(pattern, [choice('pick redis', ['redis'])])).not.toThrow(); + }); + + it('does not throw when choices is empty', () => { + const pattern = buildPattern([buildNode('webapp')], []); + expect(() => assertChoicesAreSelectable(pattern, [])).not.toThrow(); + }); + + it('does not throw for a choice naming a reachable prefixItems slot alternative', () => { + const pattern = buildPattern( + [{ oneOf: [buildNode('sql-store'), buildNode('nosql-store')] }], + [] + ); + expect(() => assertChoicesAreSelectable(pattern, [choice('pick sql', ['sql-store'])])).not.toThrow(); + }); + + it('throws when a choice names a candidate in the losing keyword of a dual-keyword catalog', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [buildNode('webapp')], + items: { oneOf: [buildNode('redis')], anyOf: [buildNode('kafka')] } + }, + relationships: { prefixItems: [] } + } + }; + expect(() => assertChoicesAreSelectable(pattern, [choice('pick kafka', ['kafka'])])) + .toThrow(/node "kafka" \(choice "pick kafka"\)/); + }); + + it('throws when a choice names a node id the pattern does not declare at all', () => { + const pattern = buildPattern([buildNode('webapp')], []); + expect(() => assertChoicesAreSelectable(pattern, [choice('typo', ['webbapp'])])) + .toThrow(/node "webbapp" \(choice "typo"\)/); + }); + + it('throws when a choice names an unreachable relationship candidate', () => { + const pattern = buildPattern([], [buildConnectsRelationship('r1', 'prompt', 'a', 'b')]); + expect(() => assertChoicesAreSelectable(pattern, [choice('typo', [], ['ghost'])])) + .toThrow(/relationship "ghost" \(choice "typo"\)/); + }); + }); }); \ No newline at end of file diff --git a/shared/src/commands/generate/components/options.ts b/shared/src/commands/generate/components/options.ts index d740f2989..3911e0e1b 100644 --- a/shared/src/commands/generate/components/options.ts +++ b/shared/src/commands/generate/components/options.ts @@ -1,4 +1,5 @@ import { initLogger } from '../../../logger'; +import { getPatternArray, resolveOperativeChoiceBlock, listSelectableCandidates } from '@finos/calm-models/pattern'; /** * A node within a CALM pattern's JSON schema. The pattern is unvalidated JSON @@ -51,21 +52,7 @@ function extractOptionsFromBlock(optionsRelationship: SchemaNode, blockType: 'on * @returns The prefixItems array from relationships, or empty array if not found */ function getRelationshipsPrefixItems(pattern: SchemaNode): SchemaNode[] { - // Direct access for standard patterns - if (pattern['properties']?.['relationships']?.['prefixItems']) { - return pattern['properties']['relationships']['prefixItems']; - } - - // Handle allOf patterns - look for relationships in each allOf schema - if (pattern['allOf'] && Array.isArray(pattern['allOf'])) { - for (const schema of pattern['allOf']) { - if (schema['properties']?.['relationships']?.['prefixItems']) { - return schema['properties']['relationships']['prefixItems']; - } - } - } - - return []; + return getPatternArray(pattern, 'relationships').prefixItems as SchemaNode[]; } /** @@ -107,27 +94,71 @@ type Item = { * @returns A list of items that match the selection predicate, or the item itself if it is not a oneOf or anyOf block */ function flattenOneOfAndAnyOf(item: Item, selectionPredicate: (item: SchemaNode) => boolean): object[] { - if (!(item.oneOf || item.anyOf)) { + const block = resolveOperativeChoiceBlock(item); + + if (!block) { + if (item.oneOf || item.anyOf) { + // A oneOf/anyOf key is present but isn't a usable choice block (neither value + // is an array). Passing the item through as-is would emit this malformed + // block itself as a node/relationship candidate in the generated output - + // fail loudly instead. + throw new Error(`Malformed oneOf/anyOf block: neither "oneOf" nor "anyOf" is an array in ${JSON.stringify(item)}`); + } // If it isn't a oneOf or anyOf block, there isn't anything to flatten so return the item return [item]; } - const items: object[] = item.oneOf ?? item.anyOf ?? []; - - return items + return (block.alternatives as object[]) .flatMap((x: object) => x) .filter((x: SchemaNode) => selectionPredicate(x)); } +/** + * Flattens the prefixItems slots (positional "pick exactly one" decisions) and, + * if present, the `items.oneOf`/`items.anyOf` open catalog (zero-or-more + * selections) for a given calmType down to the concrete set of chosen entries. + * + * The selected catalog candidates are appended onto `prefixItems` so that + * `instantiate()` - which only understands `prefixItems` - materializes them + * the same way it already does for the rest of the array. `items` is then + * cleared, since the pattern's array is now fully expressed via `prefixItems`. + */ function flattenCalmItems(pattern: SchemaNode, calmType: 'nodes' | 'relationships', ids: string[]): void { - const calmItems = pattern['properties'][calmType]['prefixItems']; + const calmProps: SchemaNode = pattern['properties']?.[calmType]; + if (!calmProps) return; const selectionPredicate = (x: SchemaNode) => ids.includes(x['properties']['unique-id']['const']); - pattern['properties'][calmType]['prefixItems'] = calmItems + + const prefixItems: SchemaNode[] = calmProps['prefixItems'] ?? []; + const flattenedPrefixItems = prefixItems .flatMap((item: Item) => flattenOneOfAndAnyOf(item, selectionPredicate)); + + const itemsCatalog: Item | undefined = calmProps['items']; + // Only treat `items` as a decision catalog when it is a oneOf/anyOf of candidates. A plain + // `items` schema (or `items: false` closing a tuple) is not part of the decision mechanism and + // must be left untouched rather than stripped. + const catalogBlock = resolveOperativeChoiceBlock(itemsCatalog); + const isCatalog = catalogBlock !== null; + const selectedCatalogItems: SchemaNode[] = isCatalog + ? (catalogBlock!.alternatives as SchemaNode[]).filter(selectionPredicate) + : []; + + calmProps['prefixItems'] = [...flattenedPrefixItems, ...selectedCatalogItems]; + + if (isCatalog) { + delete calmProps['items']; + } } -function flattenOptionsRelationship(relationship: SchemaNode, choices: CalmChoice[]): SchemaNode { +/** + * Returns `undefined` when nothing was selected for this decision, rather than the + * relationship with an empty `options.prefixItems`. An empty `prefixItems` is not a + * legal JSON Schema, so writing one there breaks the next schema compilation - both + * `calm generate` and `calm validate` compile the narrowed pattern via `selectChoices`. + * A decision resolved to "nothing chosen" has nothing to materialize, so the holder + * itself is dropped instead. + */ +function flattenOptionsRelationship(relationship: SchemaNode, choices: CalmChoice[]): SchemaNode | undefined { if (!isOptionsRelationship(relationship)) { return relationship; } @@ -136,13 +167,63 @@ function flattenOptionsRelationship(relationship: SchemaNode, choices: CalmChoic const newItems = getItemsInOptionsRelationship(relationship) .flatMap((item: Item) => flattenOneOfAndAnyOf(item, selectionPredicate)); + if (newItems.length === 0) { + return undefined; + } + relationship['properties']['relationship-type']['properties']['options']['prefixItems'] = newItems; return relationship; } function flattenOptionsRelationships(pattern: SchemaNode, choices: CalmChoice[]): void { - pattern['properties']['relationships']['prefixItems'] = pattern['properties']['relationships']['prefixItems'] - .map((rel: SchemaNode) => flattenOptionsRelationship(rel, choices)); + // Guard the relationships access the same way `flattenCalmItems` guards its + // own: a pattern whose nodes are declared entirely through an `items` catalog + // may carry no `relationships` property at all, and reaching straight through + // to `prefixItems` would throw on that shape. + const relationships: SchemaNode | undefined = pattern['properties']?.['relationships']; + if (!relationships?.['prefixItems']) return; + + relationships['prefixItems'] = relationships['prefixItems'] + .map((rel: SchemaNode) => flattenOptionsRelationship(rel, choices)) + .filter((rel: SchemaNode | undefined): rel is SchemaNode => rel !== undefined); +} + +/** + * Fails when a chosen bundle names a candidate that selection cannot reach. + * + * Nothing upstream checks that a bundle's ids resolve. `extractOptions` builds the prompt + * from the bundle, so the user is offered the choice either way. Selection then finds no + * match and adds nothing, which discards the answer in silence. + * + * Two causes: a typo in the bundle, or a block that declares both `oneOf` and `anyOf`. + * In the second case the `anyOf` candidates look declared to validation, but only the + * `oneOf` list is resolved. + * + * Called from `runGenerate`, and deliberately not from `selectChoices`. Validation also + * calls `selectChoices`, and a malformed pattern must show its own schema errors there. + */ +export function assertChoicesAreSelectable(pattern: SchemaNode, choices: CalmChoice[]): void { + const declaredNodes = new Set(listSelectableCandidates(pattern, 'nodes').map((c) => c.uniqueId)); + const declaredRelationships = new Set(listSelectableCandidates(pattern, 'relationships').map((c) => c.uniqueId)); + + const unresolved: string[] = []; + for (const choice of choices) { + for (const id of choice.nodes) { + if (!declaredNodes.has(id)) unresolved.push(`node "${id}" (choice "${choice.description}")`); + } + for (const id of choice.relationships) { + if (!declaredRelationships.has(id)) unresolved.push(`relationship "${id}" (choice "${choice.description}")`); + } + } + + if (unresolved.length > 0) { + throw new Error( + 'The pattern does not declare every candidate its decisions reference, so the ' + + 'selection cannot be applied: ' + unresolved.join('; ') + '. Check the ' + + 'unique-ids in the choice bundles, and that a catalog does not declare both ' + + '"oneOf" and "anyOf" (only the "oneOf" candidates are selectable).' + ); + } } /** @@ -152,6 +233,7 @@ function flattenOptionsRelationships(pattern: SchemaNode, choices: CalmChoice[]) * @param debug - Whether to enable debug logging * @returns A new pattern object with the selected choices and all oneOf and anyOf blocks flattened */ + export function selectChoices(inputPattern: object, choices: CalmChoice[], debug: boolean = false): object { const logger = initLogger(debug, 'calm-generate-options'); logger.debug(`Selecting these choices from the pattern [${JSON.stringify(choices)}]`); diff --git a/shared/src/commands/generate/generate.ts b/shared/src/commands/generate/generate.ts index 15ce20e9d..ea952d10a 100644 --- a/shared/src/commands/generate/generate.ts +++ b/shared/src/commands/generate/generate.ts @@ -2,12 +2,14 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { mkdirp } from 'mkdirp'; -import { CalmChoice, selectChoices } from './components/options.js'; +import { assertChoicesAreSelectable, CalmChoice, selectChoices } from './components/options.js'; import { instantiate } from './components/instantiate'; import { flattenAllOf } from './components/flatten-allof'; import { initLogger } from '../../logger.js'; import { SchemaDirectory } from '../../schema-directory.js'; +type SchemaNodeLike = { [key: string]: unknown }; + export async function runGenerate(pattern: object, outputPath: string, debug: boolean, schemaDirectory: SchemaDirectory, chosenChoices?: CalmChoice[]): Promise { const logger = initLogger(debug, 'calm-generate'); logger.info('Generating a CALM architecture...'); @@ -21,6 +23,9 @@ export async function runGenerate(pattern: object, outputPath: string, debug: bo ); if (chosenChoices) { + // Refuse rather than silently drop an answer the pattern cannot honour. The + // catch below turns this into a logged error and no output file. + assertChoicesAreSelectable(flattenedPattern as SchemaNodeLike, chosenChoices); flattenedPattern = selectChoices(flattenedPattern, chosenChoices, debug); } diff --git a/shared/src/commands/generate/items-catalog-round-trip.e2e.spec.ts b/shared/src/commands/generate/items-catalog-round-trip.e2e.spec.ts new file mode 100644 index 000000000..f389b9796 --- /dev/null +++ b/shared/src/commands/generate/items-catalog-round-trip.e2e.spec.ts @@ -0,0 +1,141 @@ +import { describe, it, expect, vi } from 'vitest'; +import { selectChoices, extractOptions, CalmChoice } from './components/options'; +import { instantiate } from './components/instantiate'; +import { validate } from '../validate/validate'; +import { SchemaDirectory } from '../../schema-directory'; + +/** + * Round-trip baseline for `items` catalog decisions: generate, then validate the result + * against the same pattern, through the real functions each side actually uses - not just + * unit-level assertions on one function in isolation. No control requirements, so this is + * unaffected by the separate, pre-existing conference-signup round-trip gap (see #2932 + * follow-up tracking). + */ + +vi.mock('../../schema-directory', () => ({ + SchemaDirectory: vi.fn(function () { + return { loadSchemas: vi.fn(), loadCurrentPatternAsSchema: vi.fn(), getDefinition: vi.fn() }; + }), +})); + +function node(uniqueId: string) { + return { + type: 'object', + properties: { + 'unique-id': { const: uniqueId }, + name: { const: uniqueId }, + 'node-type': { const: 'service' }, + }, + required: ['unique-id', 'name', 'node-type'], + }; +} + +function connects(id: string, source: string, destination: string) { + return { + type: 'object', + properties: { + 'unique-id': { const: id }, + description: { const: `${source} to ${destination}` }, + 'relationship-type': { + const: { connects: { source: { node: source }, destination: { node: destination } } }, + }, + }, + required: ['unique-id', 'description', 'relationship-type'], + }; +} + +function choice(description: string, nodeId: string, relationshipId: string) { + return { + type: 'object', + properties: { + description: { const: description }, + nodes: { const: [nodeId] }, + relationships: { const: [relationshipId] }, + }, + }; +} + +const pattern = { + $schema: 'https://json-schema.org/draft/2020-12/schema', + $id: 'items-catalog-round-trip-pattern', + properties: { + nodes: { + type: 'array', + prefixItems: [node('webapp')], + items: { anyOf: [node('candidate-a'), node('candidate-b')] }, + }, + relationships: { + type: 'array', + prefixItems: [ + { + type: 'object', + properties: { + 'unique-id': { const: 'catalog-choice' }, + description: { const: 'Which optional components?' }, + 'relationship-type': { + type: 'object', + properties: { + options: { + type: 'array', + prefixItems: [ + { + anyOf: [ + choice('Use Candidate A', 'candidate-a', 'webapp-to-a'), + choice('Use Candidate B', 'candidate-b', 'webapp-to-b'), + ], + }, + ], + }, + }, + }, + }, + }, + ], + items: { anyOf: [connects('webapp-to-a', 'webapp', 'candidate-a'), connects('webapp-to-b', 'webapp', 'candidate-b')] }, + }, + }, +}; + +async function generateAndValidate(choices: CalmChoice[]) { + const selected = selectChoices(pattern, choices, false); + const architecture = await instantiate(selected, false, new SchemaDirectory({} as never)) as { + nodes: Array>; + relationships: Array>; + }; + const response = await validate(architecture, pattern, undefined, new SchemaDirectory({} as never), false); + return { architecture, response }; +} + +describe('items-catalog round trip (baseline)', () => { + it('succeeds when two candidates are selected and both validate cleanly', async () => { + const options = extractOptions(pattern); + const chosen = options[0].choices; // both choices for the one anyOf decision + const { response } = await generateAndValidate(chosen); + + expect(response.hasErrors).toBe(false); + }); + + it('does not crash and produces a valid empty selection when zero candidates are chosen', async () => { + const { response } = await generateAndValidate([]); + + expect(response.hasErrors).toBe(false); + }); + + it('catches a corruption on the second selection, not just the first', async () => { + const options = extractOptions(pattern); + const chosen = options[0].choices; + const { architecture } = await generateAndValidate(chosen); + + // Corrupt the second selected node's const-constrained field. + const secondNode = architecture.nodes.find((n) => n['unique-id'] === 'candidate-b') as Record; + expect(secondNode).toBeDefined(); + secondNode['node-type'] = 'not-a-valid-value'; + + const response = await validate(architecture, pattern, undefined, new SchemaDirectory({} as never), false); + + expect(response.hasErrors).toBe(true); + expect(response.jsonSchemaValidationOutputs).toContainEqual( + expect.objectContaining({ path: '/nodes/2/node-type' }) + ); + }); +}); diff --git a/shared/src/commands/validate/validate.e2e.spec.ts b/shared/src/commands/validate/validate.e2e.spec.ts index 5e5c4585a..7f7837c47 100644 --- a/shared/src/commands/validate/validate.e2e.spec.ts +++ b/shared/src/commands/validate/validate.e2e.spec.ts @@ -164,6 +164,70 @@ describe('validate E2E', () => { const newPattern = applyArchitectureOptionsToPattern(architecture, pattern, false); expect(newPattern).toStrictEqual(expectedResult); }); + + it('narrows the pattern to every chosen candidate of a multi-select anyOf decision, not just the first', async () => { + const pattern = { + properties: { + nodes: { + type: 'array', + prefixItems: [{ properties: { 'unique-id': { const: 'webapp' } } }], + items: { + anyOf: [ + { properties: { 'unique-id': { const: 'postgres-db' } } }, + { properties: { 'unique-id': { const: 'mysql-db' } } }, + ], + }, + }, + relationships: { + type: 'array', + prefixItems: [ + { + properties: { + 'unique-id': { const: 'database-choice' }, + description: { const: 'Which database(s)?' }, + 'relationship-type': { + type: 'object', + properties: { + options: { + type: 'array', + prefixItems: [ + { + anyOf: [ + { properties: { description: { const: 'Use PostgreSQL' }, nodes: { const: ['postgres-db'] }, relationships: { const: [] } } }, + { properties: { description: { const: 'Use MySQL' }, nodes: { const: ['mysql-db'] }, relationships: { const: [] } } }, + ], + }, + ], + }, + }, + }, + }, + }, + ], + }, + }, + }; + + const architecture = { + relationships: [ + { + 'unique-id': 'database-choice', + 'relationship-type': { + options: [ + { description: 'Use PostgreSQL', nodes: ['postgres-db'], relationships: [] }, + { description: 'Use MySQL', nodes: ['mysql-db'], relationships: [] }, + ], + }, + }, + ], + }; + + const narrowed = applyArchitectureOptionsToPattern(architecture, pattern, false) as { + properties: { nodes: { prefixItems: { properties: { 'unique-id': { const: string } } }[] } }; + }; + const narrowedNodeIds = narrowed.properties.nodes.prefixItems.map((n) => n.properties['unique-id'].const); + expect(narrowedNodeIds).toEqual(['webapp', 'postgres-db', 'mysql-db']); + }); }); describe('schema specific validations', () => { diff --git a/shared/src/commands/validate/validate.spec.ts b/shared/src/commands/validate/validate.spec.ts index 7063d67c3..d049e932e 100644 --- a/shared/src/commands/validate/validate.spec.ts +++ b/shared/src/commands/validate/validate.spec.ts @@ -409,6 +409,66 @@ describe('validation support functions', () => { const choices = extractChoicesFromArchitecture(architecture); expect(choices).toHaveLength(2); }); + + it('captures every answer of a multi-select anyOf decision, not just the first', async () => { + // Real shape confirmed via the actual generate pipeline: selecting two anyOf + // candidates produces one options[] entry per selection, not one combined entry. + const architecture = { + relationships: [ + { + 'unique-id': 'database-choice', + 'relationship-type': { + options: [ + { 'description': 'Use PostgreSQL', 'nodes': ['postgres-db'], 'relationships': [] }, + { 'description': 'Use MySQL', 'nodes': ['mysql-db'], 'relationships': [] } + ] + } + } + ] + }; + const choices = extractChoicesFromArchitecture(architecture); + expect(choices).toHaveLength(2); + expect(choices.map((c) => c.description)).toEqual(['Use PostgreSQL', 'Use MySQL']); + }); + + it('does not throw on a zero-answer anyOf decision (options: [])', async () => { + const architecture = { + relationships: [ + { + 'unique-id': 'database-choice', + 'relationship-type': { options: [] } + } + ] + }; + expect(() => extractChoicesFromArchitecture(architecture)).not.toThrow(); + expect(extractChoicesFromArchitecture(architecture)).toHaveLength(0); + }); + + it('combines a multi-answer decision with a separate single-answer decision', async () => { + const architecture = { + relationships: [ + { + 'unique-id': 'database-choice', + 'relationship-type': { + options: [ + { 'description': 'Use PostgreSQL', 'nodes': ['postgres-db'], 'relationships': [] }, + { 'description': 'Use MySQL', 'nodes': ['mysql-db'], 'relationships': [] } + ] + } + }, + { + 'unique-id': 'cache-choice', + 'relationship-type': { + options: [ + { 'description': 'Use Redis', 'nodes': ['redis'], 'relationships': [] } + ] + } + } + ] + }; + const choices = extractChoicesFromArchitecture(architecture); + expect(choices).toHaveLength(3); + }); }); }); diff --git a/shared/src/commands/validate/validation-helpers.ts b/shared/src/commands/validate/validation-helpers.ts index e4d61fd8e..73f66d2d7 100644 --- a/shared/src/commands/validate/validation-helpers.ts +++ b/shared/src/commands/validate/validation-helpers.ts @@ -84,7 +84,7 @@ export function extractChoicesFromArchitecture(architecture: object): CalmChoice return relationships .filter((rel) => rel['relationship-type'] && Object.prototype.hasOwnProperty.call(rel['relationship-type'], 'options')) - .map((rel) => rel['relationship-type']['options'][0]) + .flatMap((rel) => rel['relationship-type']['options']) .map((rel) => ({ description: rel['description'], nodes: rel['nodes'] || [], diff --git a/shared/src/spectral/functions/pattern/catalog-single-choice-keyword.ts b/shared/src/spectral/functions/pattern/catalog-single-choice-keyword.ts new file mode 100644 index 000000000..93c12ad58 --- /dev/null +++ b/shared/src/spectral/functions/pattern/catalog-single-choice-keyword.ts @@ -0,0 +1,27 @@ +import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; + +/** + * Reports a choice block that declares both `oneOf` and `anyOf`. Only `oneOf` is + * resolved, so the `anyOf` candidates cannot be reached. + * + * The built-in `xor` cannot do this. A plain `items` schema declares neither keyword, + * and it must be left alone. + */ +export function catalogSingleChoiceKeyword(input: unknown, _: unknown, context: RulesetFunctionContext): IFunctionResult[] { + if (typeof input !== 'object' || input === null || Array.isArray(input)) { + return []; + } + + const items = input as Record; + if (!Array.isArray(items['oneOf']) || !Array.isArray(items['anyOf'])) { + return []; + } + + return [{ + message: + 'This choice block declares both "oneOf" and "anyOf". Only the "oneOf" candidates are ' + + 'selectable - candidates under "anyOf" are silently dropped from generation and from ' + + 'the diagram. Declare exactly one of the two.', + path: [...context.path], + }]; +} diff --git a/shared/src/spectral/functions/pattern/decision-references-selectable-candidate.ts b/shared/src/spectral/functions/pattern/decision-references-selectable-candidate.ts new file mode 100644 index 000000000..5acf91040 --- /dev/null +++ b/shared/src/spectral/functions/pattern/decision-references-selectable-candidate.ts @@ -0,0 +1,40 @@ +import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; +import { listDeclaredCandidates, listSelectableCandidates, type SchemaNode } from '@finos/calm-models/pattern'; + +/** + * Reports a candidate that a decision names, but that selection cannot reach. + * + * An id that does not exist at all is already an error from + * `group-relationship-with-const-nodes-references-existing-nodes-in-pattern`. This rule + * covers only the declared-but-unreachable case, so a typo is not reported twice. + */ +export function decisionReferencesSelectableCandidate( + input: unknown, + { calmType }: { calmType: 'nodes' | 'relationships' }, + context: RulesetFunctionContext +): IFunctionResult[] { + if (!input || typeof input !== 'string') { + return []; + } + + const pattern = context.document.data as SchemaNode; + + const selectable = listSelectableCandidates(pattern, calmType).some((c) => c.uniqueId === input); + if (selectable) { + return []; + } + + // Undeclared ids belong to the other rule. + const declared = listDeclaredCandidates(pattern, calmType).some((c) => c.uniqueId === input); + if (!declared) { + return []; + } + + return [{ + message: + `'${input}' is declared but cannot be selected: its block declares both 'oneOf' and ` + + '\'anyOf\', and only the \'oneOf\' candidates are resolved. Declare one keyword per ' + + 'block so every candidate a decision references can be chosen.', + path: [...context.path], + }]; +} diff --git a/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts b/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts index 91f27e3b6..b23a9efac 100644 --- a/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts +++ b/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts @@ -1,6 +1,13 @@ +import { readFileSync } from 'fs'; +import path from 'path'; import { asContext } from '../spectral-test-helpers'; import idsAreUnique from './ids-are-unique'; +const optionsPrototypePatternPath = path.join( + __dirname, + '../../../../../calm/release/1.0-rc2/prototype/multiple-choices/options-prototype.pattern.json' +); + describe('idsAreUnique', () => { it('should return an empty array when there is no input', () => { const input = null; @@ -120,6 +127,114 @@ describe('idsAreUnique', () => { + it('should return messages for duplicate IDs within nodes.items.oneOf', () => { + const input = {}; + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { 'properties': { 'unique-id': { 'const': 'webapp' } } } + ], + items: { + oneOf: [ + { 'properties': { 'unique-id': { 'const': 'cache' } } }, + { 'properties': { 'unique-id': { 'const': 'cache' } } } + ] + } + } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result.length).toBeGreaterThan(0); + expect(result[0].message).toContain('Duplicate unique-id detected. ID: cache, path: /properties/nodes/items/oneOf/1/properties/unique-id/const'); + }); + + it('should return messages for duplicate IDs within relationships.items.oneOf', () => { + const input = {}; + const context = { + document: { + data: { + properties: { + relationships: { + items: { + oneOf: [ + { 'properties': { 'unique-id': { 'const': 'edge' } } }, + { 'properties': { 'unique-id': { 'const': 'edge' } } } + ] + } + } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result.length).toBeGreaterThan(0); + expect(result[0].message).toContain('Duplicate unique-id detected. ID: edge, path: /properties/relationships/items/oneOf/1/properties/unique-id/const'); + }); + + it('should return messages for duplicate IDs across a prefixItems node and an items.oneOf node', () => { + const input = {}; + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { 'properties': { 'unique-id': { 'const': 'webapp' } } } + ], + items: { + oneOf: [ + { 'properties': { 'unique-id': { 'const': 'webapp' } } } + ] + } + } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result.length).toBeGreaterThan(0); + expect(result[0].message).toContain('Duplicate unique-id detected. ID: webapp, path: /properties/nodes/items/oneOf/0/properties/unique-id/const'); + }); + + it('should return messages for duplicate interface IDs inside catalog nodes', () => { + const input = {}; + const context = { + document: { + data: { + properties: { + nodes: { + items: { + anyOf: [ + { 'properties': { + 'unique-id': { 'const': 'cache' }, + 'interfaces': { prefixItems: [{ 'properties': { 'unique-id': { 'const': 'intf1' } } }] } } + }, + { 'properties': { + 'unique-id': { 'const': 'queue' }, + 'interfaces': { prefixItems: [{ 'properties': { 'unique-id': { 'const': 'intf1' } } }] } } + } + ] + } + } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result.length).toBeGreaterThan(0); + expect(result[0].message).toContain('Duplicate unique-id detected. ID: intf1, path: /properties/nodes/items/anyOf/1/properties/interfaces/prefixItems/0/properties/unique-id/const'); + }); + + it('should return messages for duplicate IDs across unique-ids', () => { const input = {}; const context = { @@ -147,4 +262,45 @@ describe('idsAreUnique', () => { expect(result.length).toBeGreaterThan(0); expect(result[0].message).toContain('Duplicate unique-id detected. ID: node1, path: /properties/relationships/prefixItems/0/properties/unique-id/const'); }); + + it('detects a duplicate id declared across prefixItems oneOf alternatives (previously undetected)', () => { + // Positional slot alternatives were invisible to this rule until the migration to + // listDeclaredCandidates: the identical clash inside an items catalog already errored, but + // two prefixItems[*].oneOf[*] alternatives sharing an id did not. + const input = {}; + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { + oneOf: [ + { properties: { 'unique-id': { const: 'sql-store' } } }, + { properties: { 'unique-id': { const: 'sql-store' } } } + ] + } + ] + } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result.length).toBeGreaterThan(0); + expect(result[0].message).toContain('Duplicate unique-id detected. ID: sql-store, path: /properties/nodes/prefixItems/0/oneOf/1/properties/unique-id/const'); + }); + + it('reports no diagnostics for the shipped multiple-choices options prototype pattern', () => { + // Regression test: nodes.prefixItems[0] and relationships.prefixItems[0] are pure + // oneOf choice blocks with no unique-id of their own. A candidate enumeration that + // doesn't skip id-less slots fabricates an error-severity diagnostic for each. + const input = {}; + const pattern = JSON.parse(readFileSync(optionsPrototypePatternPath, 'utf-8')); + const context = { document: { data: pattern } }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result).toEqual([]); + }); }); \ No newline at end of file diff --git a/shared/src/spectral/functions/pattern/ids-are-unique.ts b/shared/src/spectral/functions/pattern/ids-are-unique.ts index 7a04b2400..b77765711 100644 --- a/shared/src/spectral/functions/pattern/ids-are-unique.ts +++ b/shared/src/spectral/functions/pattern/ids-are-unique.ts @@ -1,25 +1,62 @@ -import { JSONPath } from 'jsonpath-plus'; import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; -import { detectDuplicates } from '../helper-functions'; +import { listDeclaredCandidates, listNodeInterfaces, type Candidate, type SchemaNode } from '@finos/calm-models/pattern'; + +// Spectral's IFunctionResult.path is an array of path segments, not a pointer string. +// candidate.path stops at the candidate's own schema object, so the message and the +// reported path both need this suffix to point at the value that actually collided. +const UNIQUE_ID_PATH_SUFFIX = ['properties', 'unique-id', 'const']; + +interface DuplicateCheckEntry { + uniqueId: string; + path: (string | number)[]; +} + +function toEntry(candidate: Candidate): DuplicateCheckEntry { + return { uniqueId: candidate.uniqueId, path: candidate.path }; +} + +function detectDuplicateEntries(entries: DuplicateCheckEntry[], seenIds: Set, messages: IFunctionResult[]): void { + for (const entry of entries) { + if (seenIds.has(entry.uniqueId)) { + const path = [...entry.path, ...UNIQUE_ID_PATH_SUFFIX]; + messages.push({ + message: `Duplicate unique-id detected. ID: ${entry.uniqueId}, path: /${path.join('/')}`, + path, + }); + } else { + seenIds.add(entry.uniqueId); + } + } +} + /** - * Checks that the input value exists as a node with a matching unique ID. + * Checks that every node, relationship and interface unique-id in the pattern is unique. + * Node/relationship ids and their nested interface ids all share one `seenIds` set, so an + * id reused across nodes, relationships and interfaces is flagged too, not just within + * one of those. */ export default (input: unknown, _: unknown, context: RulesetFunctionContext): IFunctionResult[] => { if (!input) { return []; } - // get uniqueIds of all nodes - const nodeIdMatches = JSONPath({ path: '$.properties.nodes.prefixItems[*].properties.unique-id.const', json: context.document.data as object, resultType: 'all' }); - const relationshipIdMatches = JSONPath({ path: '$.properties.relationships.prefixItems[*].properties.unique-id.const', json: context.document.data as object, resultType: 'all' }); - const interfaceIdMatches = JSONPath({ path: '$.properties.nodes.prefixItems[*].properties.interfaces.prefixItems[*].properties.unique-id.const', json: context.document.data as object, resultType: 'all' }); - const seenIds = new Set(); + const pattern = context.document.data as SchemaNode; + const nodeCandidates = listDeclaredCandidates(pattern, 'nodes'); + const relationshipCandidates = listDeclaredCandidates(pattern, 'relationships'); + + const interfaceEntries: DuplicateCheckEntry[] = nodeCandidates.flatMap((nodeCandidate) => + listNodeInterfaces(nodeCandidate.node).map((iface) => ({ + uniqueId: iface.uniqueId, + path: [...nodeCandidate.path, 'properties', 'interfaces', 'prefixItems', iface.index], + })) + ); + const seenIds = new Set(); const messages: IFunctionResult[] = []; - detectDuplicates(nodeIdMatches, seenIds, messages); - detectDuplicates(relationshipIdMatches, seenIds, messages); - detectDuplicates(interfaceIdMatches, seenIds, messages); + detectDuplicateEntries(nodeCandidates.map(toEntry), seenIds, messages); + detectDuplicateEntries(relationshipCandidates.map(toEntry), seenIds, messages); + detectDuplicateEntries(interfaceEntries, seenIds, messages); return messages; -}; \ No newline at end of file +}; diff --git a/shared/src/spectral/functions/pattern/interface-id-exists-on-node.spec.ts b/shared/src/spectral/functions/pattern/interface-id-exists-on-node.spec.ts index 44218b4f3..4d88928ca 100644 --- a/shared/src/spectral/functions/pattern/interface-id-exists-on-node.spec.ts +++ b/shared/src/spectral/functions/pattern/interface-id-exists-on-node.spec.ts @@ -128,6 +128,70 @@ describe('interfaceIdExistsOnNode', () => { expect(result[0].path).toEqual(['/relationships/0/connects/destination']); }); + it('should find a node declared in an items.oneOf catalog and validate its interfaces', () => { + const input = { node: 'cache', interfaces: ['cache-intf'] }; + const context = { + document: { + data: { + properties: { + nodes: { + items: { + oneOf: [ + { + properties: { + 'unique-id': { const: 'cache' }, + 'interfaces': { + prefixItems: [ + { properties: { 'unique-id': { const: 'cache-intf' } } } + ] + } + } + } + ] + } + } + } + } + } + }; + + const result = interfaceIdExistsOnNode(input, null, asContext(context)); + expect(result).toEqual([]); + }); + + it('should report a missing interface on an items-catalog node instead of silently skipping it', () => { + const input = { node: 'cache', interfaces: ['does-not-exist'] }; + const context = { + document: { + data: { + properties: { + nodes: { + items: { + oneOf: [ + { + properties: { + 'unique-id': { const: 'cache' }, + 'interfaces': { + prefixItems: [ + { properties: { 'unique-id': { const: 'cache-intf' } } } + ] + } + } + } + ] + } + } + } + } + }, + path: ['/relationships/0/connects/destination'] + }; + + const result = interfaceIdExistsOnNode(input, null, asContext(context)); + expect(result.length).toBe(1); + expect(result[0].message).toBe(`Referenced interface with ID '${input.interfaces[0]}' was not defined on the node with ID '${input.node}'.`); + }); + it('should return a message when one interface does not exist', () => { const input = { node: 'node1', interfaces: ['intf1', 'intf2'] }; const context = { @@ -159,4 +223,67 @@ describe('interfaceIdExistsOnNode', () => { expect(result[0].message).toBe(`Referenced interface with ID '${input.interfaces[1]}' was not defined on the node with ID '${input.node}'.`); expect(result[0].path).toEqual(['/relationships/0/connects/destination']); }); + + describe('a prefixItems oneOf slot with two node alternatives', () => { + // { oneOf: [A(interfaces:[iA]), B(interfaces:[iB])] }. Before the migration to + // listDeclaredCandidates, the inner unwrapping always resolved to alternative 0 and unioned + // interfaces across every alternative in the slot - so only the first two rows here + // passed; B was never findable, and interfaces borrowed across alternatives passed + // silently. All four rows must now behave correctly. + function contextWithTwoAlternatives() { + return { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { + oneOf: [ + { + properties: { + 'unique-id': { const: 'A' }, + interfaces: { prefixItems: [{ properties: { 'unique-id': { const: 'iA' } } }] } + } + }, + { + properties: { + 'unique-id': { const: 'B' }, + interfaces: { prefixItems: [{ properties: { 'unique-id': { const: 'iB' } } }] } + } + } + ] + } + ] + } + } + } + }, + path: ['/relationships/0/connects/destination'] + }; + } + + it('accepts A referencing its own interface iA', () => { + const input = { node: 'A', interfaces: ['iA'] }; + expect(interfaceIdExistsOnNode(input, null, asContext(contextWithTwoAlternatives()))).toEqual([]); + }); + + it('rejects A referencing iB, which belongs to B (was a false negative)', () => { + const input = { node: 'A', interfaces: ['iB'] }; + const result = interfaceIdExistsOnNode(input, null, asContext(contextWithTwoAlternatives())); + expect(result.length).toBe(1); + expect(result[0].message).toBe('Referenced interface with ID \'iB\' was not defined on the node with ID \'A\'.'); + }); + + it('accepts B referencing its own interface iB (B was previously never findable)', () => { + const input = { node: 'B', interfaces: ['iB'] }; + expect(interfaceIdExistsOnNode(input, null, asContext(contextWithTwoAlternatives()))).toEqual([]); + }); + + it('rejects B referencing a nonexistent interface (was silently unchecked)', () => { + const input = { node: 'B', interfaces: ['nope'] }; + const result = interfaceIdExistsOnNode(input, null, asContext(contextWithTwoAlternatives())); + expect(result.length).toBe(1); + expect(result[0].message).toBe('Referenced interface with ID \'nope\' was not defined on the node with ID \'B\'.'); + }); + }); }); \ No newline at end of file diff --git a/shared/src/spectral/functions/pattern/interface-id-exists-on-node.ts b/shared/src/spectral/functions/pattern/interface-id-exists-on-node.ts index 19c765bdf..bca105d74 100644 --- a/shared/src/spectral/functions/pattern/interface-id-exists-on-node.ts +++ b/shared/src/spectral/functions/pattern/interface-id-exists-on-node.ts @@ -1,6 +1,6 @@ -import { JSONPath } from 'jsonpath-plus'; import { difference } from 'lodash'; import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; +import { listDeclaredCandidates, listNodeInterfaces, type SchemaNode } from '@finos/calm-models/pattern'; interface ConnectsRelationship { node?: string; @@ -23,14 +23,12 @@ export function interfaceIdExistsOnNode(input: ConnectsRelationship | null | und } const nodeId = input.node; - const nodes: object[] = JSONPath({ path: '$.properties.nodes.prefixItems[*]', json: context.document.data as object }); - const node = nodes.find((node) => { - const uniqueId: string[] = JSONPath({ path: '$.properties.unique-id.const', json: node }); - uniqueId.push(...JSONPath({ path: '$.oneOf[*].properties.unique-id.const', json: node })); - uniqueId.push(...JSONPath({ path: '$.anyOf[*].properties.unique-id.const', json: node })); - return uniqueId && uniqueId[0] === nodeId; - }); - if (!node) { + const pattern = context.document.data as SchemaNode; + // Each candidate carries its own unique-id, so this finds the exact alternative that + // matches - not, as the old JSONPath-based lookup did, just the first alternative in + // a oneOf/anyOf slot regardless of which one actually has this id. + const nodeCandidate = listDeclaredCandidates(pattern, 'nodes').find((candidate) => candidate.uniqueId === nodeId); + if (!nodeCandidate) { // other rule will report undefined node return []; } @@ -38,10 +36,10 @@ export function interfaceIdExistsOnNode(input: ConnectsRelationship | null | und // all of these must be present on the referenced node const desiredInterfaces = input.interfaces; - const nodeInterfaces = JSONPath({ path: '$.properties.interfaces.prefixItems[*].properties.unique-id.const', json: node }); - nodeInterfaces.push(...JSONPath({ path: '$.oneOf[*].properties.interfaces.prefixItems[*].properties.unique-id.const', json: node })); - nodeInterfaces.push(...JSONPath({ path: '$.anyOf[*].properties.interfaces.prefixItems[*].properties.unique-id.const', json: node })); - if (!nodeInterfaces || nodeInterfaces.length === 0) { + // Only this node's own interfaces - not, as before, the union of interfaces declared + // across every alternative in its oneOf/anyOf slot. + const nodeInterfaces = listNodeInterfaces(nodeCandidate.node).map((iface) => iface.uniqueId); + if (nodeInterfaces.length === 0) { return [ { message: `Node with unique-id ${nodeId} has no interfaces defined, expected interfaces [${desiredInterfaces}]` } ]; @@ -62,4 +60,4 @@ export function interfaceIdExistsOnNode(input: ConnectsRelationship | null | und }); } return results; -} \ No newline at end of file +} diff --git a/shared/src/spectral/functions/pattern/node-id-exists.spec.ts b/shared/src/spectral/functions/pattern/node-id-exists.spec.ts new file mode 100644 index 000000000..9d8e595f2 --- /dev/null +++ b/shared/src/spectral/functions/pattern/node-id-exists.spec.ts @@ -0,0 +1,135 @@ +import { readFileSync } from 'fs'; +import path from 'path'; +import { asContext } from '../spectral-test-helpers'; +import nodeIdExists from './node-id-exists'; + +const optionsPrototypePatternPath = path.join( + __dirname, + '../../../../../calm/release/1.0-rc2/prototype/multiple-choices/options-prototype.pattern.json' +); + +describe('nodeIdExists (pattern)', () => { + it('should return an empty array for non-string input', () => { + const context = { document: { data: {} } }; + expect(nodeIdExists(null, null, asContext(context))).toEqual([]); + expect(nodeIdExists(42, null, asContext(context))).toEqual([]); + }); + + it('should accept a node declared in prefixItems', () => { + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { properties: { 'unique-id': { const: 'webapp' } } } + ] + } + } + } + } + }; + expect(nodeIdExists('webapp', null, asContext(context))).toEqual([]); + }); + + it('should accept a node declared only in an items.oneOf catalog', () => { + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { properties: { 'unique-id': { const: 'webapp' } } } + ], + items: { + oneOf: [ + { properties: { 'unique-id': { const: 'cache' } } }, + { properties: { 'unique-id': { const: 'queue' } } } + ] + } + } + } + } + } + }; + // Referenced by a deployed-in relationship / decision choice — must not false-positive. + expect(nodeIdExists('cache', null, asContext(context))).toEqual([]); + expect(nodeIdExists('queue', null, asContext(context))).toEqual([]); + }); + + it('should accept a node declared only in an items.anyOf catalog', () => { + const context = { + document: { + data: { + properties: { + nodes: { + items: { + anyOf: [ + { properties: { 'unique-id': { const: 'cache' } } } + ] + } + } + } + } + } + }; + expect(nodeIdExists('cache', null, asContext(context))).toEqual([]); + }); + + it('should report a node id that exists in neither prefixItems nor the items catalog', () => { + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { properties: { 'unique-id': { const: 'webapp' } } } + ], + items: { + oneOf: [ + { properties: { 'unique-id': { const: 'cache' } } } + ] + } + } + } + } + }, + path: ['/relationships/0/relationship-type/deployed-in/nodes/0'] + }; + const missingId = 'ghost'; + const result = nodeIdExists(missingId, null, asContext(context)); + expect(result.length).toBe(1); + expect(result[0].message).toBe(`'${missingId}' does not refer to the unique-id of an existing node.`); + }); + + it('accepts a node declared as a prefixItems oneOf/anyOf slot alternative', () => { + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [ + { oneOf: [{ properties: { 'unique-id': { const: 'sql-store' } } }] }, + { anyOf: [{ properties: { 'unique-id': { const: 'nosql-store' } } }] } + ] + } + } + } + } + }; + expect(nodeIdExists('sql-store', null, asContext(context))).toEqual([]); + expect(nodeIdExists('nosql-store', null, asContext(context))).toEqual([]); + }); + + it('accepts every node in the shipped multiple-choices options prototype pattern', () => { + // Regression test for the same fixture ids-are-unique is pinned against: nodes + // declared as prefixItems oneOf/anyOf slot alternatives must resolve correctly + // through the migrated candidate enumeration. + const pattern = JSON.parse(readFileSync(optionsPrototypePatternPath, 'utf-8')); + const context = { document: { data: pattern } }; + + for (const nodeId of ['application-a', 'application-b', 'node-1', 'node-2', 'application-c', 'database']) { + expect(nodeIdExists(nodeId, null, asContext(context))).toEqual([]); + } + }); +}); diff --git a/shared/src/spectral/functions/pattern/node-id-exists.ts b/shared/src/spectral/functions/pattern/node-id-exists.ts index b34820b48..cf62ee80f 100644 --- a/shared/src/spectral/functions/pattern/node-id-exists.ts +++ b/shared/src/spectral/functions/pattern/node-id-exists.ts @@ -1,5 +1,6 @@ -import { JSONPath } from 'jsonpath-plus'; import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; +import { listDeclaredCandidates, type SchemaNode } from '@finos/calm-models/pattern'; + /** * Checks that the input value exists as a node with a matching unique ID. */ @@ -8,14 +9,12 @@ export default (input: unknown, _: unknown, context: RulesetFunctionContext): IF return []; } - const names = JSONPath({ path: '$.properties.nodes.prefixItems[*].properties.unique-id.const', json: context.document.data as object }); - const oneofs = JSONPath({ path: '$.properties.nodes.prefixItems[*].oneOf[*].properties.unique-id.const', json: context.document.data as object }); - const anyofs = JSONPath({ path: '$.properties.nodes.prefixItems[*].anyOf[*].properties.unique-id.const', json: context.document.data as object }); + const pattern = context.document.data as SchemaNode; + const nodeIds = listDeclaredCandidates(pattern, 'nodes').map((candidate) => candidate.uniqueId); - // get uniqueIds of all nodes const results: IFunctionResult[] = []; - if (!names.includes(input) && !oneofs.includes(input) && !anyofs.includes(input)) { + if (!nodeIds.includes(input)) { results.push({ message: `'${input}' does not refer to the unique-id of an existing node.`, path: [...context.path], diff --git a/shared/src/spectral/rules-pattern.spec.ts b/shared/src/spectral/rules-pattern.spec.ts new file mode 100644 index 000000000..b6e1a5a93 --- /dev/null +++ b/shared/src/spectral/rules-pattern.spec.ts @@ -0,0 +1,330 @@ +import { describe, it, expect } from 'vitest'; +import patternRules from './rules-pattern'; +import { runSpectralValidations } from '../commands/validate/validation-helpers'; + +const RULE = 'pattern-option-relationship-must-be-in-prefix-items'; + +/** A decision holder: a relationship carrying `relationship-type.options`. */ +function decisionHolder() { + return { + properties: { + 'unique-id': { const: 'connection-options' }, + 'description': { const: 'Which optional components do you want?' }, + 'relationship-type': { + properties: { + options: { + prefixItems: [{ + anyOf: [{ + properties: { + description: { const: 'Add a cache' }, + nodes: { const: ['cache'] }, + relationships: { const: ['app-to-cache'] } + } + }] + }] + } + } + } + } + }; +} + +/** A plain relationship candidate - legal inside an items catalog. */ +function candidateRelationship(id: string) { + return { properties: { 'unique-id': { const: id }, 'description': { const: `the ${id} link` } } }; +} + +async function ruleSeverityFor(pattern: object, code: string): Promise { + const result = await runSpectralValidations(JSON.stringify(pattern), patternRules, 'test'); + return result.spectralIssues.find((issue) => issue.code === code)?.severity; +} + +async function ruleCodesFor(pattern: object): Promise { + const result = await runSpectralValidations(JSON.stringify(pattern), patternRules, 'test'); + return result.spectralIssues.map(issue => issue.code); +} + +async function ruleMessageFor(pattern: object, code: string): Promise { + const result = await runSpectralValidations(JSON.stringify(pattern), patternRules, 'test'); + return result.spectralIssues.find((issue) => issue.code === code)?.message; +} + +describe('pattern-option-relationship-must-be-in-prefix-items', () => { + it('passes when the decision holder is in prefixItems and only candidates are in the items catalog', async () => { + const pattern = { + properties: { + nodes: { prefixItems: [] }, + relationships: { + prefixItems: [decisionHolder()], + items: { oneOf: [candidateRelationship('app-to-cache'), candidateRelationship('app-to-queue')] } + } + } + }; + + expect(await ruleCodesFor(pattern)).not.toContain(RULE); + }); + + it('fails when the decision holder is declared inside an items.oneOf catalog', async () => { + const pattern = { + properties: { + nodes: { prefixItems: [] }, + relationships: { + prefixItems: [], + items: { oneOf: [decisionHolder(), candidateRelationship('app-to-cache')] } + } + } + }; + + expect(await ruleCodesFor(pattern)).toContain(RULE); + }); + + it('fails when the decision holder is declared inside an items.anyOf catalog', async () => { + const pattern = { + properties: { + nodes: { prefixItems: [] }, + relationships: { + prefixItems: [], + items: { anyOf: [decisionHolder()] } + } + } + }; + + expect(await ruleCodesFor(pattern)).toContain(RULE); + }); + + it('fails when the misplaced decision holder sits inside an allOf branch', async () => { + const pattern = { + allOf: [{ + properties: { + nodes: { prefixItems: [] }, + relationships: { + prefixItems: [], + items: { oneOf: [decisionHolder()] } + } + } + }] + }; + + expect(await ruleCodesFor(pattern)).toContain(RULE); + }); + + it('does not fire for a nodes items catalog, which never holds decisions', async () => { + const pattern = { + properties: { + nodes: { + prefixItems: [], + items: { oneOf: [{ properties: { 'unique-id': { const: 'cache' }, 'description': { const: 'a cache' } } }] } + }, + relationships: { prefixItems: [decisionHolder()] } + } + }; + + expect(await ruleCodesFor(pattern)).not.toContain(RULE); + }); +}); + +describe('pattern-items-catalog-must-declare-one-choice-keyword', () => { + const RULE_BOTH = 'pattern-items-catalog-must-declare-one-choice-keyword'; + + function candidate(id: string) { + return { properties: { 'unique-id': { const: id } } }; + } + + it('reports when a nodes catalog declares both oneOf and anyOf', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [], items: { oneOf: [candidate('a')], anyOf: [candidate('b')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).toContain(RULE_BOTH); + }); + + it('reports it as a warning, not an error', async () => { + // Declaring both is legal JSON Schema - both keywords apply, which for distinct + // `const` ids is unsatisfiable - so it is a smell, not an invalid document. The + // harm it can cause is caught in `selectChoices`, which throws when a chosen + // bundle names a candidate selection cannot reach. + const severity = await ruleSeverityFor({ + properties: { + nodes: { prefixItems: [], items: { oneOf: [candidate('a')], anyOf: [candidate('b')] } }, + relationships: { prefixItems: [] } + } + }, RULE_BOTH); + expect(severity).toBe('warning'); + }); + + it('reports when a relationships catalog declares both', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [] }, + relationships: { prefixItems: [], items: { oneOf: [candidate('r1')], anyOf: [candidate('r2')] } } + } + }); + expect(codes).toContain(RULE_BOTH); + }); + + it('reports a catalog declared inside an allOf branch', async () => { + const codes = await ruleCodesFor({ + allOf: [{ + properties: { + nodes: { prefixItems: [], items: { oneOf: [candidate('a')], anyOf: [candidate('b')] } }, + relationships: { prefixItems: [] } + } + }] + }); + expect(codes).toContain(RULE_BOTH); + }); + + it('passes a catalog declaring only oneOf', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [], items: { oneOf: [candidate('a'), candidate('b')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).not.toContain(RULE_BOTH); + }); + + it('passes a catalog declaring only anyOf', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [], items: { anyOf: [candidate('a'), candidate('b')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).not.toContain(RULE_BOTH); + }); + + it('fires on a prefixItems slot declaring both oneOf and anyOf, not just an items catalog', async () => { + // The bug this rule catches - oneOf silently winning over anyOf - resolves + // identically for a prefixItems slot and an items catalog (both go through + // resolveOperativeChoiceBlock), and reproduces on main for prefixItems slots. The `given` + // already covers this site; this pins that it is actually exercised. + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [{ oneOf: [candidate('a')], anyOf: [candidate('b')] }] }, + relationships: { prefixItems: [] } + } + }); + expect(codes).toContain(RULE_BOTH); + }); + + it('names the block generically, not as "an items catalog", since a prefixItems slot triggers it too', async () => { + const pattern = { + properties: { + nodes: { prefixItems: [{ oneOf: [candidate('a')], anyOf: [candidate('b')] }] }, + relationships: { prefixItems: [] } + } + }; + const message = await ruleMessageFor(pattern, RULE_BOTH); + expect(message).toBeDefined(); + expect(message?.toLowerCase()).not.toContain('items catalog'); + }); + + it('leaves a plain items schema alone — it is not a catalog', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [candidate('a')], items: false }, + relationships: { prefixItems: [] } + } + }); + expect(codes).not.toContain(RULE_BOTH); + }); +}); + +describe('pattern-decision-must-reference-selectable-nodes', () => { + const RULE_SELECTABLE = 'pattern-decision-must-reference-selectable-nodes'; + const TYPO_RULE = 'group-relationship-with-const-nodes-references-existing-nodes-in-pattern'; + + function candidateNode(id: string) { + return { properties: { 'unique-id': { const: id }, name: { const: id }, 'node-type': { const: 'service' } } }; + } + + function decisionNaming(id: string) { + return { + properties: { + 'unique-id': { const: 'choice' }, + description: { const: 'Pick one' }, + 'relationship-type': { + type: 'object', + properties: { + options: { + type: 'array', + prefixItems: [{ + oneOf: [{ + properties: { + description: { const: 'Use it' }, + nodes: { const: [id] }, + relationships: { const: [] } + } + }] + }] + } + } + } + } + }; + } + + it('errors when a decision names a candidate in the losing keyword of a dual-keyword catalog', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [candidateNode('webapp')], items: { oneOf: [candidateNode('redis')], anyOf: [candidateNode('kafka')] } }, + relationships: { prefixItems: [decisionNaming('kafka')] } + } + }); + expect(codes).toContain(RULE_SELECTABLE); + }); + + it('errors for the same shape declared as a prefixItems slot rather than a catalog', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [candidateNode('webapp'), { oneOf: [candidateNode('a')], anyOf: [candidateNode('b')] }] }, + relationships: { prefixItems: [decisionNaming('b')] } + } + }); + expect(codes).toContain(RULE_SELECTABLE); + }); + + it('reports it as an error - the same harm as naming a candidate that does not exist', async () => { + const severity = await ruleSeverityFor({ + properties: { + nodes: { prefixItems: [candidateNode('webapp')], items: { oneOf: [candidateNode('redis')], anyOf: [candidateNode('kafka')] } }, + relationships: { prefixItems: [decisionNaming('kafka')] } + } + }, RULE_SELECTABLE); + expect(severity).toBe('error'); + }); + + it('leaves a plain typo to the existing rule rather than double-reporting it', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [candidateNode('webapp')], items: { oneOf: [candidateNode('redis')] } }, + relationships: { prefixItems: [decisionNaming('rediss')] } + } + }); + expect(codes).toContain(TYPO_RULE); + expect(codes).not.toContain(RULE_SELECTABLE); + }); + + it('passes a decision naming a reachable catalog candidate', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [candidateNode('webapp')], items: { oneOf: [candidateNode('redis')] } }, + relationships: { prefixItems: [decisionNaming('redis')] } + } + }); + expect(codes).not.toContain(RULE_SELECTABLE); + }); + + it('passes a decision naming a reachable slot alternative', async () => { + const codes = await ruleCodesFor({ + properties: { + nodes: { prefixItems: [candidateNode('webapp'), { oneOf: [candidateNode('a'), candidateNode('b')] }] }, + relationships: { prefixItems: [decisionNaming('a')] } + } + }); + expect(codes).not.toContain(RULE_SELECTABLE); + }); +}); diff --git a/shared/src/spectral/rules-pattern.ts b/shared/src/spectral/rules-pattern.ts index 8bec5c21d..5c095dd85 100644 --- a/shared/src/spectral/rules-pattern.ts +++ b/shared/src/spectral/rules-pattern.ts @@ -1,5 +1,6 @@ import { RulesetDefinition } from '@stoplight/spectral-core'; -import { pattern, truthy, length, xor } from '@stoplight/spectral-functions'; +import { falsy, pattern, truthy, length, xor } from '@stoplight/spectral-functions'; +import { catalogSingleChoiceKeyword } from './functions/pattern/catalog-single-choice-keyword'; import { numericalPlaceHolder } from './functions/helper-functions'; import nodeIdExists from './functions/pattern/node-id-exists'; import idsAreUnique from './functions/pattern/ids-are-unique'; @@ -7,6 +8,7 @@ import nodeHasRelationship from './functions/pattern/node-has-relationship'; import { interfaceIdExists } from './functions/pattern/interface-id-exists'; import { interfaceIdExistsOnNode } from './functions/pattern/interface-id-exists-on-node'; import { isDefinedInOneOfOrAnyOf } from './functions/pattern/is-defined-in-oneof-or-anyof'; +import { decisionReferencesSelectableCandidate } from './functions/pattern/decision-references-selectable-candidate'; const patternRules: RulesetDefinition = { @@ -134,7 +136,11 @@ const patternRules: RulesetDefinition = { description: 'Nodes must be referenced by at least one relationship', severity: 'warn', message: '{{error}}', - given: '$.properties.nodes.prefixItems[*].properties.unique-id.const', + given: [ + '$.properties.nodes.prefixItems[*].properties.unique-id.const', + '$.properties.nodes.items.oneOf[*].properties.unique-id.const', + '$.properties.nodes.items.anyOf[*].properties.unique-id.const', + ], then: { function: nodeHasRelationship, }, @@ -198,6 +204,67 @@ const patternRules: RulesetDefinition = { max: 1 }, }, + }, + 'pattern-items-catalog-must-declare-one-choice-keyword': { + // Legal JSON Schema, so this is a smell and not an invalid document. Raising it + // to `error` would not help generation, because `calm generate` never validates. + description: 'A choice block must declare only one of oneOf/anyOf', + severity: 'warn', + message: '{{error}}', + given: [ + '$.properties.nodes.items', + '$.properties.relationships.items', + '$.properties.nodes.prefixItems[*]', + '$.properties.relationships.prefixItems[*]', + '$.allOf[*].properties.nodes.items', + '$.allOf[*].properties.relationships.items', + '$.allOf[*].properties.nodes.prefixItems[*]', + '$.allOf[*].properties.relationships.prefixItems[*]', + ], + then: { + function: catalogSingleChoiceKeyword, + }, + }, + 'pattern-decision-must-reference-selectable-nodes': { + // Covers only the declared-but-unreachable case. A plain typo is already an + // error from group-relationship-with-const-nodes-references-existing-nodes. + description: 'Nodes referenced by a pattern decision must be selectable', + severity: 'error', + message: '{{error}}', + given: '$..relationship-type.properties.options.prefixItems[*]..nodes.const[*]', + then: { + function: decisionReferencesSelectableCandidate, + functionOptions: { + calmType: 'nodes' + } + }, + }, + 'pattern-decision-must-reference-selectable-relationships': { + description: 'Relationships referenced by a pattern decision must be selectable', + severity: 'error', + message: '{{error}}', + given: '$..relationship-type.properties.options.prefixItems[*]..relationships.const[*]', + then: { + function: decisionReferencesSelectableCandidate, + functionOptions: { + calmType: 'relationships' + } + }, + }, + 'pattern-option-relationship-must-be-in-prefix-items': { + // A holder in an `items` catalog makes the decision itself optional, so + // `calm generate` never offers it. Candidates go in the catalog; the decision + // that selects them goes in prefixItems. + description: 'Options relationships must be declared in relationships.prefixItems, not in an items catalog', + severity: 'error', + message: 'A relationship declaring "relationship-type.options" must be in "properties.relationships.prefixItems". Declaring a decision inside an "items" catalog makes the decision itself optional - move the options relationship into prefixItems and leave only its candidates in the catalog.', + given: [ + '$..relationships.items.oneOf[*].properties.relationship-type.properties.options', + '$..relationships.items.anyOf[*].properties.relationship-type.properties.options', + ], + then: { + function: falsy, + }, } } }; diff --git a/test_fixtures/decision-agreement/README.md b/test_fixtures/decision-agreement/README.md new file mode 100644 index 000000000..7628e6806 --- /dev/null +++ b/test_fixtures/decision-agreement/README.md @@ -0,0 +1,18 @@ +# Decision agreement fixtures + +`calm generate` and the pattern visualiser read a pattern's decisions independently. +Nothing made them answer the same way, which is how the defects in PR #2932 arose. + +Each case here is one pattern plus the decisions and results both surfaces must produce. +`calm-hub-ui` does not depend on `shared`, so no test can import both sides. The fixture +is the contract instead. + +| Side | Reads | Asserts | +|---|---|---| +| `shared` | `extractOptions`, `selectChoices` + `instantiate` | `decisions`, then `answered[].nodes` | +| `calm-hub-ui` | `parsePatternData` + `extractDecisionPoints`, `getVisibleNodeIds` | the same two | + +`answered` cases name a choice for **every** decision. An unanswered decision is left out +on purpose: the visualiser shows all its candidates ("what is still possible") and +generation contributes none ("what you asked for"). Which one is correct is an open +product question, so no test pins it. diff --git a/test_fixtures/decision-agreement/one-decision-one-catalog.expected.json b/test_fixtures/decision-agreement/one-decision-one-catalog.expected.json new file mode 100644 index 000000000..a21842751 --- /dev/null +++ b/test_fixtures/decision-agreement/one-decision-one-catalog.expected.json @@ -0,0 +1,17 @@ +{ + "decisions": [ + { + "optionId": "cache-choice", + "prompt": "Pick a cache", + "optionType": "anyOf", + "choices": [ + { "description": "Use Redis", "nodes": ["redis"], "relationships": [] }, + { "description": "Use Memcached", "nodes": ["memcached"], "relationships": [] } + ] + } + ], + "answered": [ + { "choose": { "cache-choice": "Use Redis" }, "nodes": ["webapp", "redis"] }, + { "choose": { "cache-choice": "Use Memcached" }, "nodes": ["webapp", "memcached"] } + ] +} diff --git a/test_fixtures/decision-agreement/one-decision-one-catalog.pattern.json b/test_fixtures/decision-agreement/one-decision-one-catalog.pattern.json new file mode 100644 index 000000000..8cc722ffe --- /dev/null +++ b/test_fixtures/decision-agreement/one-decision-one-catalog.pattern.json @@ -0,0 +1,76 @@ +{ + "$schema": "https://calm.finos.org/release/1.0/meta/calm.json", + "$id": "one-decision-one-catalog", + "title": "One decision drawing from one catalog", + "properties": { + "nodes": { + "type": "array", + "prefixItems": [ + { + "properties": { + "unique-id": { "const": "webapp" }, + "name": { "const": "Web App" }, + "node-type": { "const": "service" } + } + } + ], + "items": { + "anyOf": [ + { + "properties": { + "unique-id": { "const": "redis" }, + "name": { "const": "Redis" }, + "node-type": { "const": "database" } + } + }, + { + "properties": { + "unique-id": { "const": "memcached" }, + "name": { "const": "Memcached" }, + "node-type": { "const": "database" } + } + } + ] + } + }, + "relationships": { + "type": "array", + "prefixItems": [ + { + "properties": { + "unique-id": { "const": "cache-choice" }, + "description": { "const": "Pick a cache" }, + "relationship-type": { + "type": "object", + "properties": { + "options": { + "type": "array", + "prefixItems": [ + { + "anyOf": [ + { + "properties": { + "description": { "const": "Use Redis" }, + "nodes": { "const": ["redis"] }, + "relationships": { "const": [] } + } + }, + { + "properties": { + "description": { "const": "Use Memcached" }, + "nodes": { "const": ["memcached"] }, + "relationships": { "const": [] } + } + } + ] + } + ] + } + } + } + } + } + ] + } + } +} diff --git a/test_fixtures/decision-agreement/two-decisions-one-catalog.expected.json b/test_fixtures/decision-agreement/two-decisions-one-catalog.expected.json new file mode 100644 index 000000000..53c7626cc --- /dev/null +++ b/test_fixtures/decision-agreement/two-decisions-one-catalog.expected.json @@ -0,0 +1,70 @@ +{ + "decisions": [ + { + "optionId": "cache-choice", + "prompt": "Pick a cache", + "optionType": "anyOf", + "choices": [ + { + "description": "Use Redis", + "nodes": [ + "redis" + ], + "relationships": [] + }, + { + "description": "Use Memcached", + "nodes": [ + "memcached" + ], + "relationships": [] + } + ] + }, + { + "optionId": "queue-choice", + "prompt": "Pick a queue", + "optionType": "anyOf", + "choices": [ + { + "description": "Use Kafka", + "nodes": [ + "kafka" + ], + "relationships": [] + }, + { + "description": "Use RabbitMQ", + "nodes": [ + "rabbitmq" + ], + "relationships": [] + } + ] + } + ], + "answered": [ + { + "choose": { + "cache-choice": "Use Redis", + "queue-choice": "Use Kafka" + }, + "nodes": [ + "webapp", + "redis", + "kafka" + ] + }, + { + "choose": { + "cache-choice": "Use Memcached", + "queue-choice": "Use RabbitMQ" + }, + "nodes": [ + "webapp", + "memcached", + "rabbitmq" + ] + } + ] +} diff --git a/test_fixtures/decision-agreement/two-decisions-one-catalog.pattern.json b/test_fixtures/decision-agreement/two-decisions-one-catalog.pattern.json new file mode 100644 index 000000000..4ab122d4b --- /dev/null +++ b/test_fixtures/decision-agreement/two-decisions-one-catalog.pattern.json @@ -0,0 +1,194 @@ +{ + "$schema": "https://calm.finos.org/release/1.0/meta/calm.json", + "$id": "two-decisions-one-catalog", + "title": "Two decisions drawing from one catalog", + "properties": { + "nodes": { + "type": "array", + "prefixItems": [ + { + "properties": { + "unique-id": { + "const": "webapp" + }, + "name": { + "const": "Web App" + }, + "node-type": { + "const": "service" + } + } + } + ], + "items": { + "anyOf": [ + { + "properties": { + "unique-id": { + "const": "redis" + }, + "name": { + "const": "Redis" + }, + "node-type": { + "const": "database" + } + } + }, + { + "properties": { + "unique-id": { + "const": "memcached" + }, + "name": { + "const": "Memcached" + }, + "node-type": { + "const": "database" + } + } + }, + { + "properties": { + "unique-id": { + "const": "kafka" + }, + "name": { + "const": "Kafka" + }, + "node-type": { + "const": "queue" + } + } + }, + { + "properties": { + "unique-id": { + "const": "rabbitmq" + }, + "name": { + "const": "RabbitMQ" + }, + "node-type": { + "const": "queue" + } + } + } + ] + } + }, + "relationships": { + "type": "array", + "prefixItems": [ + { + "properties": { + "unique-id": { + "const": "cache-choice" + }, + "description": { + "const": "Pick a cache" + }, + "relationship-type": { + "type": "object", + "properties": { + "options": { + "type": "array", + "prefixItems": [ + { + "anyOf": [ + { + "properties": { + "description": { + "const": "Use Redis" + }, + "nodes": { + "const": [ + "redis" + ] + }, + "relationships": { + "const": [] + } + } + }, + { + "properties": { + "description": { + "const": "Use Memcached" + }, + "nodes": { + "const": [ + "memcached" + ] + }, + "relationships": { + "const": [] + } + } + } + ] + } + ] + } + } + } + } + }, + { + "properties": { + "unique-id": { + "const": "queue-choice" + }, + "description": { + "const": "Pick a queue" + }, + "relationship-type": { + "type": "object", + "properties": { + "options": { + "type": "array", + "prefixItems": [ + { + "anyOf": [ + { + "properties": { + "description": { + "const": "Use Kafka" + }, + "nodes": { + "const": [ + "kafka" + ] + }, + "relationships": { + "const": [] + } + } + }, + { + "properties": { + "description": { + "const": "Use RabbitMQ" + }, + "nodes": { + "const": [ + "rabbitmq" + ] + }, + "relationships": { + "const": [] + } + } + } + ] + } + ] + } + } + } + } + } + ] + } + } +}