diff --git a/PATTERN-DECISIONS.md b/PATTERN-DECISIONS.md index 3bdb62f09..c44042482 100644 --- a/PATTERN-DECISIONS.md +++ b/PATTERN-DECISIONS.md @@ -3,9 +3,6 @@ A CALM pattern can offer a choice. This document records what each tool guarantees about that choice. It describes behaviour only. It does not describe how a tool is built. -Each section names the tests that hold its guarantees. A guarantee below with no test is a -gap. - ## Terms | Term | Meaning | @@ -128,6 +125,20 @@ branches that declare the same property discard one of the two declarations. ## What generation guarantees -`calm generate` does not read `items`. It builds one node per `prefixItems` entry and -nothing else, so a node declared in `items` never reaches the architecture. `calm validate` -accepts the pattern, and the missing node is silent. +`calm generate` builds one element per `prefixItems` entry, in order. An answer that selects +an `items` member appends that member to the same list, so a selected member reaches the +architecture and an unselected one does not. + +An answer comes from a decision. A decision names an `items` member in its `nodes` or +`relationships` list, and `calm generate` then offers it. A catalogue that no decision names +is never offered, so `calm generate` builds none of its members. That is the correct result +for a catalogue written to constrain what an architecture may add. Declare a decision beside +the catalogue to offer its members instead. + +`calm validate` keeps reading the `items` block whichever way it is written. Selecting an +answer does not relax what a pattern permits. + +An `items` block that declares no `oneOf` or `anyOf` is left alone. Nothing selects from it, +so nothing is promoted out of it. + +An array that ends with no entries becomes `[]`. diff --git a/shared/src/commands/generate/components/instantiate.spec.ts b/shared/src/commands/generate/components/instantiate.spec.ts index dff2621e9..885988a0b 100644 --- a/shared/src/commands/generate/components/instantiate.spec.ts +++ b/shared/src/commands/generate/components/instantiate.spec.ts @@ -294,4 +294,22 @@ describe('instantiate', () => { 'nested-placeholder': '[[ NESTED_PLACEHOLDER ]]' }); }); + it('emits an empty array for an array with no prefixItems', async () => { + const patternWithUnselectedItems = { + $schema: 'schema#', + $id: 'test-pattern-items-only', + properties: { + nodes: { type: 'array', items: { oneOf: [{ properties: { 'unique-id': { const: 'cache' } } }] } }, + relationships: { type: 'array', prefixItems: [] } + } + }; + + (fs.readFileSync as Mock).mockImplementation(function () { return JSON.stringify(patternWithUnselectedItems); }); + + const pattern = JSON.parse(fs.readFileSync(patternPath, { encoding: 'utf-8' })); + const result = await instantiate(pattern, true, new SchemaDirectory(null as unknown as DocumentLoader)) as TestInstantiatedPattern; + + expect(result.nodes).toEqual([]); + expect(result.relationships).toEqual([]); + }); }); diff --git a/shared/src/commands/generate/components/instantiate.ts b/shared/src/commands/generate/components/instantiate.ts index 2d6170c65..45960d8d3 100644 --- a/shared/src/commands/generate/components/instantiate.ts +++ b/shared/src/commands/generate/components/instantiate.ts @@ -109,13 +109,12 @@ async function instantiateFromProperties( return await instantiateObject(resolvedItem, schemaDir, [key, `${idx}`]); }) ); + } else if (resolvedDef.const !== undefined) { + output[key] = resolvedDef.const; + } else if (resolvedDef.type === 'array') { + 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..228677278 100644 --- a/shared/src/commands/generate/components/options.spec.ts +++ b/shared/src/commands/generate/components/options.spec.ts @@ -394,4 +394,51 @@ describe('Pattern Options', () => { expect(extractOptions(allOfPattern)).toEqual(expectedOptions); }); }); -}); \ No newline at end of file + describe('items', () => { + const node = (id: string) => ({ type: 'object', properties: { 'unique-id': { const: id } } }); + const add = (id: string): CalmChoice => ({ description: `Add ${id}`, nodes: [id], relationships: [] }); + const pattern = (nodes: object) => ({ properties: { nodes, relationships: { type: 'array', prefixItems: [] } } }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const nodesOf = (result: object) => (result as any).properties.nodes; + + const withItems = () => pattern({ + type: 'array', + prefixItems: [node('webapp')], + items: { oneOf: [node('cache'), node('queue')] } + }); + + it('should promote a chosen items member into prefixItems', () => { + const result = nodesOf(selectChoices(withItems(), [add('cache')])); + expect(result.prefixItems).toEqual([node('webapp'), node('cache')]); + }); + + it('should keep items once its members have been promoted, because validation still needs it', () => { + expect(nodesOf(selectChoices(withItems(), [add('cache')])).items).toEqual({ oneOf: [node('cache'), node('queue')] }); + }); + + it('should promote every chosen items member', () => { + const result = nodesOf(selectChoices(withItems(), [add('cache'), add('queue')])); + expect(result.prefixItems).toEqual([node('webapp'), node('cache'), node('queue')]); + }); + + it('should promote nothing when no items member is chosen', () => { + expect(nodesOf(selectChoices(withItems(), [])).prefixItems).toEqual([node('webapp')]); + }); + + it('should leave a plain items schema untouched', () => { + const plain = pattern({ type: 'array', prefixItems: [node('webapp')], items: { $ref: 'core.json#/defs/node' } }); + expect(nodesOf(selectChoices(plain, [add('webapp')])).items).toEqual({ $ref: 'core.json#/defs/node' }); + }); + + it('should build prefixItems for a pattern that declares only items', () => { + const itemsOnly = pattern({ type: 'array', items: { oneOf: [node('cache')] } }); + expect(nodesOf(selectChoices(itemsOnly, [add('cache')])).prefixItems).toEqual([node('cache')]); + }); + + it('should accept a pattern with no relationships property', () => { + const noRelationships = { properties: { nodes: { type: 'array', prefixItems: [node('webapp')] } } }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((selectChoices(noRelationships, [add('webapp')]) as any).properties.nodes.prefixItems).toEqual([node('webapp')]); + }); + }); +}); diff --git a/shared/src/commands/generate/components/options.ts b/shared/src/commands/generate/components/options.ts index d740f2989..da7fd5ec7 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 { ALTERNATIVE_KEYWORDS, declaresOptions } from '../../../spectral/functions/pattern/declaration-paths'; /** * A node within a CALM pattern's JSON schema. The pattern is unvalidated JSON @@ -21,10 +22,6 @@ export interface CalmOption { choices: CalmChoice[], } -function isOptionsRelationship(relationship: SchemaNode): boolean { - return relationship['properties']?.['relationship-type']?.['properties']?.['options'] !== undefined; -} - function getItemsInOptionsRelationship(optionsRelationship: SchemaNode): SchemaNode[] { return optionsRelationship['properties']['relationship-type']['properties']['options']['prefixItems']; } @@ -84,7 +81,7 @@ export function extractOptions(pattern: object, debug: boolean = false): CalmOpt } const options: CalmOption[] = calmItems - .filter((rel: SchemaNode) => isOptionsRelationship(rel)) + .filter((rel: SchemaNode) => declaresOptions(rel)) .flatMap((optionsRel: SchemaNode) => [ ...extractOptionsFromBlock(optionsRel, 'oneOf'), ...extractOptionsFromBlock(optionsRel, 'anyOf') @@ -99,36 +96,40 @@ type Item = { anyOf?: object[], } -/** - * This function flattens oneOf and anyOf blocks into their constituent items if they match the selection predicate. - * If the passed item is not a oneOf or anyOf block, it returns the item as is in a list. - * @param item - The item to flatten - * @param selectionPredicate - A function that takes an item and returns true if it should be included in the flattened result - * @returns A list of items that match the selection predicate, or the item itself if it is not a oneOf or anyOf block - */ +function alternativesOf(item: Item | undefined): object[] | undefined { + const keyword = ALTERNATIVE_KEYWORDS.find(name => item?.[name]); + return keyword ? item?.[keyword] : undefined; +} + function flattenOneOfAndAnyOf(item: Item, selectionPredicate: (item: SchemaNode) => boolean): object[] { - if (!(item.oneOf || item.anyOf)) { - // If it isn't a oneOf or anyOf block, there isn't anything to flatten so return the item + const alternatives = alternativesOf(item); + if (!alternatives) { return [item]; } - const items: object[] = item.oneOf ?? item.anyOf ?? []; - - return items - .flatMap((x: object) => x) - .filter((x: SchemaNode) => selectionPredicate(x)); + return alternatives.filter((x: SchemaNode) => selectionPredicate(x)); } -function flattenCalmItems(pattern: SchemaNode, calmType: 'nodes' | 'relationships', ids: string[]): void { - const calmItems = pattern['properties'][calmType]['prefixItems']; +/** + * Selected `items` members are promoted into `prefixItems`, so the instantiator only ever + * reads positions. The `items` block itself stays, because validation compiles this same + * pattern and needs it to keep constraining whatever the architecture adds. + */ +function selectDeclarations(pattern: SchemaNode, calmType: 'nodes' | 'relationships', ids: string[]): void { + const declarations = pattern['properties'][calmType]; + if (!declarations) { + return; + } const selectionPredicate = (x: SchemaNode) => ids.includes(x['properties']['unique-id']['const']); - pattern['properties'][calmType]['prefixItems'] = calmItems + const catalogue: Item[] = alternativesOf(declarations['items']) ? [declarations['items']] : []; + + declarations['prefixItems'] = [...(declarations['prefixItems'] ?? []), ...catalogue] .flatMap((item: Item) => flattenOneOfAndAnyOf(item, selectionPredicate)); } function flattenOptionsRelationship(relationship: SchemaNode, choices: CalmChoice[]): SchemaNode { - if (!isOptionsRelationship(relationship)) { + if (!declaresOptions(relationship)) { return relationship; } @@ -141,7 +142,12 @@ function flattenOptionsRelationship(relationship: SchemaNode, choices: CalmChoic } function flattenOptionsRelationships(pattern: SchemaNode, choices: CalmChoice[]): void { - pattern['properties']['relationships']['prefixItems'] = pattern['properties']['relationships']['prefixItems'] + const relationships = pattern['properties']['relationships']; + if (!relationships?.['prefixItems']) { + return; + } + + relationships['prefixItems'] = relationships['prefixItems'] .map((rel: SchemaNode) => flattenOptionsRelationship(rel, choices)); } @@ -160,8 +166,8 @@ export function selectChoices(inputPattern: object, choices: CalmChoice[], debug const nodeIds: string[] = choices.flatMap(choice => choice.nodes); const relationshipIds: string[] = choices.flatMap(choice => choice.relationships); - flattenCalmItems(pattern, 'nodes', nodeIds); - flattenCalmItems(pattern, 'relationships', relationshipIds); + selectDeclarations(pattern, 'nodes', nodeIds); + selectDeclarations(pattern, 'relationships', relationshipIds); flattenOptionsRelationships(pattern, choices); diff --git a/shared/src/commands/validate/validate.e2e.spec.ts b/shared/src/commands/validate/validate.e2e.spec.ts index 5e5c4585a..80f0f2c9f 100644 --- a/shared/src/commands/validate/validate.e2e.spec.ts +++ b/shared/src/commands/validate/validate.e2e.spec.ts @@ -164,6 +164,47 @@ describe('validate E2E', () => { const newPattern = applyArchitectureOptionsToPattern(architecture, pattern, false); expect(newPattern).toStrictEqual(expectedResult); }); + + it('keeps the items catalogue, so validation still constrains what an architecture adds', () => { + const node = (id: string) => ({ properties: { 'unique-id': { const: id } } }); + const pattern = { + properties: { + nodes: { prefixItems: [node('gateway')], items: { oneOf: [node('cache'), node('queue')] } }, + relationships: { + prefixItems: [{ + properties: { + 'unique-id': { const: 'db-choice' }, + 'relationship-type': { + properties: { + options: { + prefixItems: [{ + oneOf: [{ + properties: { + description: { const: 'Use postgres' }, + nodes: { const: ['postgres'] }, + relationships: { const: [] } + } + }] + }] + } + } + } + } + }] + } + } + }; + const architecture = { + nodes: [], + relationships: [{ + 'unique-id': 'db-choice', + 'relationship-type': { options: [{ description: 'Use postgres', nodes: ['postgres'], relationships: [] }] } + }] + }; + + const resolved = applyArchitectureOptionsToPattern(architecture, pattern, false) as typeof pattern; + expect(resolved.properties.nodes.items).toBeDefined(); + }); }); describe('schema specific validations', () => { diff --git a/shared/src/spectral/functions/pattern/declaration-paths.ts b/shared/src/spectral/functions/pattern/declaration-paths.ts index 17a161d04..61392a0ba 100644 --- a/shared/src/spectral/functions/pattern/declaration-paths.ts +++ b/shared/src/spectral/functions/pattern/declaration-paths.ts @@ -2,7 +2,7 @@ import { get } from 'lodash'; export type CalmType = 'nodes' | 'relationships'; -const ALTERNATIVE_KEYWORDS = ['oneOf', 'anyOf']; +export const ALTERNATIVE_KEYWORDS = ['oneOf', 'anyOf'] as const; const ID = 'properties.unique-id.const'; const OPTIONS = 'properties.relationship-type.properties.options'; const INTERFACES = 'properties.interfaces.prefixItems[*]';