diff --git a/.changeset/swift-otters-wander.md b/.changeset/swift-otters-wander.md new file mode 100644 index 0000000000..96e58d1f27 --- /dev/null +++ b/.changeset/swift-otters-wander.md @@ -0,0 +1,6 @@ +--- +'@redocly/openapi-core': minor +'@redocly/cli': minor +--- + +Added the `spec-parameters-in-by-context` Arazzo rule, which validates that a parameter's `in` field is specified when the parent workflow, step, success action, or failure action does not reference a `workflowId`. diff --git a/.changeset/thin-aliens-accept.md b/.changeset/thin-aliens-accept.md new file mode 100644 index 0000000000..2bf346aee3 --- /dev/null +++ b/.changeset/thin-aliens-accept.md @@ -0,0 +1,7 @@ +--- +'@redocly/openapi-core': minor +'@redocly/respect-core': minor +'@redocly/cli': minor +--- + +Extended success and failure action objects to accept a `parameters` property that maps to workflow inputs. diff --git a/docs/@v2/rules/arazzo/spec-parameters-in-by-context.md b/docs/@v2/rules/arazzo/spec-parameters-in-by-context.md new file mode 100644 index 0000000000..136fee1622 --- /dev/null +++ b/docs/@v2/rules/arazzo/spec-parameters-in-by-context.md @@ -0,0 +1,122 @@ +# spec-parameters-in-by-context + +Validates how the `in` field is used on parameters based on the parent context. + +| Arazzo | Compatibility | +| ------ | ------------- | +| 1.x | ✅ | + +## Design principles + +The `in` field on an Arazzo parameter is not a required property — omitting it carries semantics. +When a step references a `workflowId`, a parameter with no `in` field is mapped to the referenced workflow's inputs. +When `in` is specified, the parameter is sent at that request location (`header`, `query`, `path`, or `cookie`) against the targeted operation. + +This rule enforces the following: + +- For a step that does not reference a `workflowId` (for example, one using `operationId`, `operationPath`, or `x-operation`), and for parameters defined at the workflow level, `in` must be specified on each inline parameter. +- Parameters on success and failure actions are only valid when the action references a `workflowId` — these parameters map to the referenced workflow's inputs and the spec states that `in` MUST NOT be used on them (see the [Success Action Object](https://spec.openapis.org/arazzo/latest.html#success-action-object) and [Failure Action Object](https://spec.openapis.org/arazzo/latest.html#failure-action-object)). + +## Configuration + +| Option | Type | Description | +| -------- | ------ | ------------------------------------------------------- | +| severity | string | Possible values: `off`, `warn`, `error`. Default `off`. | + +An example configuration: + +```yaml +rules: + spec-parameters-in-by-context: error +``` + +## Examples + +Given the following configuration: + +```yaml +rules: + spec-parameters-in-by-context: error +``` + +Example of a **correct** step referencing an `operationId` (each parameter declares `in`): + +```yaml +# Correct example - operationId +workflows: + - workflowId: get-museum-hours + steps: + - stepId: list-hours + operationId: listMuseumHours + parameters: + - in: query + name: startDate + value: '2024-01-01' +``` + +Example of a **correct** step referencing a `workflowId` (parameters omit `in` and are mapped to the referenced workflow's inputs): + +```yaml +# Correct example - workflowId +workflows: + - workflowId: buy-tickets + steps: + - stepId: reuse-hours-workflow + workflowId: get-museum-hours + parameters: + - name: startDate + value: '2024-01-01' +``` + +Example of a **correct** success action transferring to another workflow with mapped parameters: + +```yaml +# Correct example - success action +workflows: + - workflowId: buy-tickets + steps: + - stepId: purchase + operationId: createTicket + onSuccess: + - name: continue-to-hours + type: goto + workflowId: get-museum-hours + parameters: + - name: startDate + value: '2024-01-01' +``` + +Example of an **incorrect** step referencing an `operationId` while omitting `in`: + +```yaml +# Incorrect example - operationId without `in` +workflows: + - workflowId: get-museum-hours + steps: + - stepId: list-hours + operationId: listMuseumHours + parameters: + - name: startDate + value: '2024-01-01' +``` + +Example of an **incorrect** success action defining `parameters` without referencing a `workflowId`: + +```yaml +# Incorrect example - action without workflowId +workflows: + - workflowId: buy-tickets + steps: + - stepId: purchase + operationId: createTicket + onSuccess: + - name: end-with-params + type: end + parameters: + - name: startDate + value: '2024-01-01' +``` + +## Resources + +- [Rule source](https://github.com/Redocly/redocly-cli/blob/main/packages/core/src/rules/arazzo/spec-parameters-in-by-context.ts) diff --git a/docs/@v2/rules/built-in-rules.md b/docs/@v2/rules/built-in-rules.md index c9891808a7..5ef74a75c4 100644 --- a/docs/@v2/rules/built-in-rules.md +++ b/docs/@v2/rules/built-in-rules.md @@ -135,6 +135,7 @@ Within the Arazzo family of rules, there are rules for the main Arazzo specifica - [requestBody-replacements-unique](./arazzo/requestBody-replacements-unique.md): the `replacements` of the `requestBody` object must be unique - [sourceDescription-name-unique](./arazzo/sourceDescription-name-unique.md): the `name` property of the `sourceDescription` object must be unique across all source descriptions - [sourceDescription-type](./arazzo/sourceDescription-type.md): the `type` property of the `sourceDescription` object must be either `openapi` or `arazzo` +- [spec-parameters-in-by-context](./arazzo/spec-parameters-in-by-context.md): the parameter `in` field must be specified or omitted based on whether the parent step or action references a `workflowId` - [stepId-unique](./arazzo/stepId-unique.md): the `stepId` must be unique amongst all steps described in the workflow - [step-onFailure-unique](./arazzo/step-onFailure-unique.md): the `onFailure` actions of the `step` object must be unique - [step-onSuccess-unique](./arazzo/step-onSuccess-unique.md): the `onSuccess` actions of the `step` object must be unique diff --git a/docs/@v2/v2.sidebars.yaml b/docs/@v2/v2.sidebars.yaml index 2e11a05018..66b14f142b 100644 --- a/docs/@v2/v2.sidebars.yaml +++ b/docs/@v2/v2.sidebars.yaml @@ -168,6 +168,7 @@ - page: rules/arazzo/requestBody-replacements-unique.md - page: rules/arazzo/sourceDescription-name-unique.md - page: rules/arazzo/sourceDescription-type.md + - page: rules/arazzo/spec-parameters-in-by-context.md - page: rules/arazzo/stepId-unique.md - page: rules/arazzo/step-onFailure-unique.md - page: rules/arazzo/step-onSuccess-unique.md diff --git a/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap b/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap index 7e026eed64..555c0e9787 100644 --- a/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap +++ b/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap @@ -1324,6 +1324,8 @@ exports[`createConfigTypes > matches snapshot for the default config schema 1`] "OpenAPISourceDescription", "ArazzoSourceDescription", "Parameters", + "ActionParameters", + "ActionParameter", "ReusableObject", "Workflows", "Workflow", diff --git a/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap b/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap index ed14440bc6..5d8968b516 100644 --- a/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap +++ b/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap @@ -20,6 +20,7 @@ exports[`resolveConfig > should ignore minimal from the root and read local file "sourceDescription-name-unique": "error", "sourceDescription-type": "error", "sourceDescriptions-not-empty": "error", + "spec-parameters-in-by-context": "warn", "step-onFailure-unique": "warn", "step-onSuccess-unique": "warn", "stepId-unique": "error", @@ -407,6 +408,7 @@ exports[`resolveConfig > should resolve extends with local file config which con "sourceDescription-name-unique": "error", "sourceDescription-type": "error", "sourceDescriptions-not-empty": "error", + "spec-parameters-in-by-context": "warn", "step-onFailure-unique": "warn", "step-onSuccess-unique": "warn", "stepId-unique": "error", diff --git a/packages/core/src/config/__tests__/load.test.ts b/packages/core/src/config/__tests__/load.test.ts index e88230d17e..ca61567529 100644 --- a/packages/core/src/config/__tests__/load.test.ts +++ b/packages/core/src/config/__tests__/load.test.ts @@ -143,6 +143,7 @@ describe('loadConfig', () => { "sourceDescription-name-unique": "off", "sourceDescription-type": "off", "sourceDescriptions-not-empty": "off", + "spec-parameters-in-by-context": "off", "step-onFailure-unique": "off", "step-onSuccess-unique": "off", "stepId-unique": "error", @@ -467,6 +468,7 @@ describe('loadConfig', () => { "sourceDescription-name-unique": "error", "sourceDescription-type": "error", "sourceDescriptions-not-empty": "error", + "spec-parameters-in-by-context": "warn", "step-onFailure-unique": "warn", "step-onSuccess-unique": "warn", "stepId-unique": "error", @@ -796,6 +798,7 @@ describe('loadConfig', () => { "sourceDescription-name-unique": "off", "sourceDescription-type": "off", "sourceDescriptions-not-empty": "off", + "spec-parameters-in-by-context": "off", "step-onFailure-unique": "off", "step-onSuccess-unique": "off", "stepId-unique": "error", @@ -1209,6 +1212,7 @@ describe('loadConfig', () => { "sourceDescription-name-unique": "off", "sourceDescription-type": "off", "sourceDescriptions-not-empty": "off", + "spec-parameters-in-by-context": "off", "step-onFailure-unique": "off", "step-onSuccess-unique": "off", "stepId-unique": "error", @@ -1533,6 +1537,7 @@ describe('loadConfig', () => { "sourceDescription-name-unique": "error", "sourceDescription-type": "error", "sourceDescriptions-not-empty": "error", + "spec-parameters-in-by-context": "warn", "step-onFailure-unique": "warn", "step-onSuccess-unique": "warn", "stepId-unique": "error", @@ -1862,6 +1867,7 @@ describe('loadConfig', () => { "sourceDescription-name-unique": "off", "sourceDescription-type": "off", "sourceDescriptions-not-empty": "off", + "spec-parameters-in-by-context": "off", "step-onFailure-unique": "off", "step-onSuccess-unique": "off", "stepId-unique": "error", diff --git a/packages/core/src/config/all.ts b/packages/core/src/config/all.ts index fbdd42c095..0b7e3405e1 100644 --- a/packages/core/src/config/all.ts +++ b/packages/core/src/config/all.ts @@ -294,6 +294,7 @@ const all: RawGovernanceConfig<'built-in'> = { 'sourceDescription-name-unique': 'error', 'sourceDescription-type': 'error', 'sourceDescriptions-not-empty': 'error', + 'spec-parameters-in-by-context': 'error', 'step-onFailure-unique': 'error', 'step-onSuccess-unique': 'error', 'stepId-unique': 'error', diff --git a/packages/core/src/config/minimal.ts b/packages/core/src/config/minimal.ts index 4a00e4964e..f5ef806256 100644 --- a/packages/core/src/config/minimal.ts +++ b/packages/core/src/config/minimal.ts @@ -273,6 +273,7 @@ const minimal: RawGovernanceConfig<'built-in'> = { 'sourceDescription-name-unique': 'off', 'sourceDescription-type': 'off', 'sourceDescriptions-not-empty': 'off', + 'spec-parameters-in-by-context': 'off', 'step-onFailure-unique': 'off', 'step-onSuccess-unique': 'off', 'stepId-unique': 'error', diff --git a/packages/core/src/config/recommended-strict.ts b/packages/core/src/config/recommended-strict.ts index 250a6d5636..08e46d1691 100644 --- a/packages/core/src/config/recommended-strict.ts +++ b/packages/core/src/config/recommended-strict.ts @@ -273,6 +273,7 @@ const recommendedStrict: RawGovernanceConfig<'built-in'> = { 'sourceDescription-name-unique': 'error', 'sourceDescription-type': 'error', 'sourceDescriptions-not-empty': 'error', + 'spec-parameters-in-by-context': 'error', 'step-onFailure-unique': 'error', 'step-onSuccess-unique': 'error', 'stepId-unique': 'error', diff --git a/packages/core/src/config/recommended.ts b/packages/core/src/config/recommended.ts index f3333ef029..286eeb93be 100644 --- a/packages/core/src/config/recommended.ts +++ b/packages/core/src/config/recommended.ts @@ -273,6 +273,7 @@ const recommended: RawGovernanceConfig<'built-in'> = { 'sourceDescription-name-unique': 'error', 'sourceDescription-type': 'error', 'sourceDescriptions-not-empty': 'error', + 'spec-parameters-in-by-context': 'warn', 'step-onFailure-unique': 'warn', 'step-onSuccess-unique': 'warn', 'stepId-unique': 'error', diff --git a/packages/core/src/config/spec.ts b/packages/core/src/config/spec.ts index bea4c5e16e..7688933ef3 100644 --- a/packages/core/src/config/spec.ts +++ b/packages/core/src/config/spec.ts @@ -273,6 +273,7 @@ const spec: RawGovernanceConfig<'built-in'> = { 'sourceDescription-name-unique': 'error', 'sourceDescription-type': 'error', 'sourceDescriptions-not-empty': 'error', + 'spec-parameters-in-by-context': 'error', 'step-onFailure-unique': 'error', 'step-onSuccess-unique': 'error', 'stepId-unique': 'error', diff --git a/packages/core/src/rules/arazzo/__tests__/parameters-unique.test.ts b/packages/core/src/rules/arazzo/__tests__/parameters-unique.test.ts index 9722cc776c..c5f342d66f 100644 --- a/packages/core/src/rules/arazzo/__tests__/parameters-unique.test.ts +++ b/packages/core/src/rules/arazzo/__tests__/parameters-unique.test.ts @@ -112,4 +112,87 @@ describe('Arazzo parameters-unique', () => { ] `); }); + + it('should report on duplicated `parameters` defined on success/failure actions', async () => { + const actionDocument = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + description: A cool API + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: outer + steps: + - stepId: step-1 + operationId: museum-api.getMuseumHours + onSuccess: + - name: go-next + type: goto + workflowId: inner + parameters: + - name: token + value: a + - name: token + value: b + onFailure: + - name: recover + type: goto + workflowId: inner + parameters: + - name: retryToken + value: a + - name: retryToken + value: b + - workflowId: inner + steps: + - stepId: noop + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document: actionDocument, + config: await createConfig({ + rules: { 'parameters-unique': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/workflows/0/steps/0/onSuccess/0/parameters/1", + "reportOnKey": false, + "source": "arazzo.yaml", + }, + ], + "message": "The parameter \`name\` must be unique amongst listed parameters.", + "ruleId": "parameters-unique", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/workflows/0/steps/0/onFailure/0/parameters/1", + "reportOnKey": false, + "source": "arazzo.yaml", + }, + ], + "message": "The parameter \`name\` must be unique amongst listed parameters.", + "ruleId": "parameters-unique", + "severity": "error", + "suggest": [], + }, + ] + `); + }); }); diff --git a/packages/core/src/rules/arazzo/__tests__/spec-parameters-in-by-context.test.ts b/packages/core/src/rules/arazzo/__tests__/spec-parameters-in-by-context.test.ts new file mode 100644 index 0000000000..9a360d9839 --- /dev/null +++ b/packages/core/src/rules/arazzo/__tests__/spec-parameters-in-by-context.test.ts @@ -0,0 +1,453 @@ +import { outdent } from 'outdent'; + +import { parseYamlToDocument, replaceSourceWithRef } from '../../../../__tests__/utils.js'; +import { createConfig } from '../../../config/index.js'; +import { lintDocument } from '../../../lint.js'; +import { BaseResolver } from '../../../resolve.js'; + +describe('Arazzo spec-parameters-in-by-context', () => { + it('should not report when step references operationId and parameters specify `in`', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: get-museum-hours + steps: + - stepId: get-museum-hours + operationId: museum-api.getMuseumHours + parameters: + - in: header + name: Secret + value: Basic Og== + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should report when step references operationId but a parameter is missing `in`', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: get-museum-hours + steps: + - stepId: get-museum-hours + operationId: museum-api.getMuseumHours + parameters: + - name: Secret + value: Basic Og== + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/workflows/0/steps/0/parameters/0", + "reportOnKey": false, + "source": "arazzo.yaml", + }, + ], + "message": "Parameter \`in\` field MUST be specified when the parent does not reference a \`workflowId\`.", + "ruleId": "spec-parameters-in-by-context", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when step references workflowId and a parameter declares `in` (spec is silent on steps)', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: outer + steps: + - stepId: call-inner + workflowId: inner + parameters: + - in: header + name: token + value: abc + - workflowId: inner + steps: + - stepId: noop + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should not report when step references workflowId and parameter has no `in`', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: outer + steps: + - stepId: call-inner + workflowId: inner + parameters: + - name: token + value: abc + - workflowId: inner + steps: + - stepId: noop + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should report a struct error when an action parameter specifies `in`', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: outer + steps: + - stepId: step-1 + operationId: museum-api.getMuseumHours + onSuccess: + - name: go-next + type: goto + workflowId: inner + parameters: + - in: header + name: token + value: abc + - workflowId: inner + steps: + - stepId: noop + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { struct: 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "from": undefined, + "location": [ + { + "pointer": "#/workflows/0/steps/0/onSuccess/0/parameters/0/in", + "reportOnKey": true, + "source": "arazzo.yaml", + }, + ], + "message": "Property \`in\` is not expected here.", + "ruleId": "struct", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when onFailure action with workflowId has parameter without `in`', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: outer + steps: + - stepId: step-1 + operationId: museum-api.getMuseumHours + onFailure: + - name: recover + type: goto + workflowId: inner + parameters: + - name: token + value: abc + - workflowId: inner + steps: + - stepId: noop + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should report when onSuccess action without workflowId defines parameters', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: outer + steps: + - stepId: step-1 + operationId: museum-api.getMuseumHours + onSuccess: + - name: go-step + type: goto + stepId: step-2 + parameters: + - in: header + name: token + value: abc + - stepId: step-2 + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/workflows/0/steps/0/onSuccess/0/parameters", + "reportOnKey": true, + "source": "arazzo.yaml", + }, + ], + "message": "Parameters on success actions are only valid when the action references a \`workflowId\`.", + "ruleId": "spec-parameters-in-by-context", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should ignore reusable parameters (only `reference`)', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + components: + parameters: + shared: + in: header + name: token + value: abc + workflows: + - workflowId: outer + steps: + - stepId: step-1 + operationId: museum-api.getMuseumHours + parameters: + - reference: $components.parameters.shared + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should not report when workflow-level parameters specify `in`', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: get-museum-hours + parameters: + - in: header + name: Secret + value: Basic Og== + steps: + - stepId: get-museum-hours + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should report when workflow-level parameter is missing `in`', async () => { + const document = parseYamlToDocument( + outdent` + arazzo: '1.0.1' + info: + title: Cool API + version: 1.0.0 + sourceDescriptions: + - name: museum-api + type: openapi + url: openapi.yaml + workflows: + - workflowId: get-museum-hours + parameters: + - name: Secret + value: Basic Og== + steps: + - stepId: get-museum-hours + operationId: museum-api.getMuseumHours + `, + 'arazzo.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'spec-parameters-in-by-context': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/workflows/0/parameters/0", + "reportOnKey": false, + "source": "arazzo.yaml", + }, + ], + "message": "Parameter \`in\` field MUST be specified when the parent does not reference a \`workflowId\`.", + "ruleId": "spec-parameters-in-by-context", + "severity": "error", + "suggest": [], + }, + ] + `); + }); +}); diff --git a/packages/core/src/rules/arazzo/index.ts b/packages/core/src/rules/arazzo/index.ts index 2242a76ee1..2a5073399d 100644 --- a/packages/core/src/rules/arazzo/index.ts +++ b/packages/core/src/rules/arazzo/index.ts @@ -20,6 +20,7 @@ import { ParametersUnique } from './parameters-unique.js'; import { RequestBodyReplacementsUnique } from './requestBody-replacements-unique.js'; import { SourceDescriptionsNameUnique } from './sourceDescriptions-name-unique.js'; import { SourceDescriptionsNotEmpty } from './sourceDescriptions-not-empty.js'; +import { SpecParametersInByContext } from './spec-parameters-in-by-context.js'; import { StepOnFailureUnique } from './step-onFailure-unique.js'; import { StepOnSuccessUnique } from './step-onSuccess-unique.js'; import { StepIdUnique } from './stepId-unique.js'; @@ -44,6 +45,7 @@ export const rules: Arazzo1RuleSet<'built-in'> = { 'sourceDescription-name-unique': SourceDescriptionsNameUnique, 'sourceDescription-type': SourceDescriptionType, 'sourceDescriptions-not-empty': SourceDescriptionsNotEmpty, + 'spec-parameters-in-by-context': SpecParametersInByContext, 'step-onFailure-unique': StepOnFailureUnique, 'step-onSuccess-unique': StepOnSuccessUnique, 'stepId-unique': StepIdUnique, diff --git a/packages/core/src/rules/arazzo/parameters-unique.ts b/packages/core/src/rules/arazzo/parameters-unique.ts index ca69a82cfb..283e1fae80 100644 --- a/packages/core/src/rules/arazzo/parameters-unique.ts +++ b/packages/core/src/rules/arazzo/parameters-unique.ts @@ -1,30 +1,40 @@ +import type { Parameter } from '../../typings/arazzo.js'; import type { Arazzo1Rule } from '../../visitors.js'; import type { UserContext } from '../../walk.js'; -export const ParametersUnique: Arazzo1Rule = () => { - return { - Parameters: { - enter(parameters, { report, location }: UserContext) { - if (!parameters) return; - const seenParameters = new Set(); +function checkParametersUnique(parameters: Parameter[], { report, location }: UserContext) { + if (!parameters) return; + const seenParameters = new Set(); - for (const parameter of parameters) { - if (seenParameters.has(parameter?.name)) { - report({ - message: 'The parameter `name` must be unique amongst listed parameters.', - location: location.child([parameters.indexOf(parameter)]), - }); - } + for (const parameter of parameters) { + if (seenParameters.has(parameter?.name)) { + report({ + message: 'The parameter `name` must be unique amongst listed parameters.', + location: location.child([parameters.indexOf(parameter)]), + }); + } - if (seenParameters.has(parameter?.reference)) { - report({ - message: 'The parameter `reference` must be unique amongst listed parameters.', - location: location.child([parameters.indexOf(parameter)]), - }); - } + if (seenParameters.has(parameter?.reference)) { + report({ + message: 'The parameter `reference` must be unique amongst listed parameters.', + location: location.child([parameters.indexOf(parameter)]), + }); + } - seenParameters.add(parameter?.name ?? parameter?.reference); - } + seenParameters.add(parameter?.name ?? parameter?.reference); + } +} + +export const ParametersUnique: Arazzo1Rule = () => { + return { + Parameters: { + enter(parameters, ctx: UserContext) { + checkParametersUnique(parameters, ctx); + }, + }, + ActionParameters: { + enter(parameters, ctx: UserContext) { + checkParametersUnique(parameters, ctx); }, }, }; diff --git a/packages/core/src/rules/arazzo/spec-parameters-in-by-context.ts b/packages/core/src/rules/arazzo/spec-parameters-in-by-context.ts new file mode 100644 index 0000000000..b119c2d11c --- /dev/null +++ b/packages/core/src/rules/arazzo/spec-parameters-in-by-context.ts @@ -0,0 +1,70 @@ +import type { Parameter } from '../../typings/arazzo.js'; +import { isPlainObject } from '../../utils/is-plain-object.js'; +import type { Arazzo1Rule } from '../../visitors.js'; +import type { UserContext } from '../../walk.js'; + +function isInlineParameter(parameter: Parameter): boolean { + return isPlainObject(parameter) && !('reference' in parameter); +} + +function checkInRequired(parameters: Parameter[], { report, location }: UserContext) { + if (!Array.isArray(parameters)) return; + + for (let i = 0; i < parameters.length; i++) { + const parameter = parameters[i]; + if (!isInlineParameter(parameter)) continue; + + if (!('in' in parameter)) { + report({ + message: + 'Parameter `in` field MUST be specified when the parent does not reference a `workflowId`.', + location: location.child(['parameters', i]), + }); + } + } +} + +export const SpecParametersInByContext: Arazzo1Rule = () => { + return { + Workflow: { + enter(workflow, ctx: UserContext) { + if (!workflow.parameters) return; + // A workflow never references another workflow, so `in` is always required. + checkInRequired(workflow.parameters, ctx); + }, + }, + Step: { + enter(step, ctx: UserContext) { + if (!step.parameters) return; + if (step.workflowId) return; + checkInRequired(step.parameters, ctx); + }, + }, + SuccessActionObject: { + enter(action, ctx: UserContext) { + if (!action.parameters) return; + + if (!action.workflowId) { + ctx.report({ + message: + 'Parameters on success actions are only valid when the action references a `workflowId`.', + location: ctx.location.child(['parameters']).key(), + }); + } + }, + }, + FailureActionObject: { + enter(action, ctx: UserContext) { + if (!action.parameters) return; + + if (!action.workflowId) { + ctx.report({ + message: + 'Parameters on failure actions are only valid when the action references a `workflowId`.', + location: ctx.location.child(['parameters']).key(), + }); + } + }, + }, + }; +}; diff --git a/packages/core/src/types/arazzo.ts b/packages/core/src/types/arazzo.ts index db00fad02e..7afbe4b333 100755 --- a/packages/core/src/types/arazzo.ts +++ b/packages/core/src/types/arazzo.ts @@ -161,6 +161,29 @@ const Parameters: NodeType = { } }, }; +const ActionParameter: NodeType = { + properties: { + name: { + type: 'string', + description: 'REQUIRED. The name of the parameter. Parameter names are case sensitive.', + }, + value: {}, + }, + required: ['name', 'value'], + extensionsPrefix: 'x-', + description: + 'Describes a single parameter passed to a workflow referenced by a success or failure action. Action parameters map to the referenced workflow inputs, so the `in` field MUST NOT be used.', +}; +const ActionParameters: NodeType = { + properties: {}, + items: (value: any) => { + if (value?.reference) { + return 'ReusableObject'; + } else { + return 'ActionParameter'; + } + }, +}; const Workflow: NodeType = { properties: { workflowId: { @@ -397,6 +420,7 @@ const SuccessActionObject: NodeType = { description: 'The workflowId referencing an existing workflow within the Arazzo Description to transfer to upon success of the step. This field is only relevant when the type field value is "goto". If the referenced workflow is contained within an arazzo type sourceDescription, then the workflowId MUST be specified using a Runtime Expression (e.g., $sourceDescriptions..) to avoid ambiguity or potential clashes. This field is mutually exclusive to stepId.', }, + parameters: 'ActionParameters', criteria: listOf('CriterionObject', { description: 'A list of assertions to determine if this action SHALL be executed. Each assertion is described using a Criterion Object. All criteria assertions MUST be satisfied for the action to be executed.', @@ -450,6 +474,7 @@ const FailureActionObject: NodeType = { description: 'A non-negative integer indicating how many attempts to retry the step MAY be attempted before failing the overall step. If not specified then a single retry SHALL be attempted. This field only applies when the type field value is "retry". The retryLimit MUST be exhausted prior to executing subsequent failure actions.', }, + parameters: 'ActionParameters', criteria: listOf('CriterionObject', { description: 'A list of assertions to determine if this action SHALL be executed. Each assertion is described using a Criterion Object.', @@ -478,6 +503,8 @@ export const Arazzo1Types: Record = { ArazzoSourceDescription, Parameters, Parameter, + ActionParameters, + ActionParameter, ReusableObject, Workflows, Workflow, diff --git a/packages/core/src/types/redocly-yaml.ts b/packages/core/src/types/redocly-yaml.ts index fefcfcf823..1db048a5b3 100644 --- a/packages/core/src/types/redocly-yaml.ts +++ b/packages/core/src/types/redocly-yaml.ts @@ -163,6 +163,7 @@ const builtInArazzo1Rules = [ 'workflow-dependsOn', 'outputs-defined', 'parameters-unique', + 'spec-parameters-in-by-context', 'step-onSuccess-unique', 'step-onFailure-unique', 'respect-supported-versions', diff --git a/packages/core/src/typings/arazzo.ts b/packages/core/src/typings/arazzo.ts index bbc485461f..5a11a15484 100644 --- a/packages/core/src/typings/arazzo.ts +++ b/packages/core/src/typings/arazzo.ts @@ -37,6 +37,12 @@ export interface Parameter { reference?: string; } +export interface ActionParameter { + name: string; + value: string | number | boolean; + reference?: string; +} + export type ExtendedSecurity = | { schemeName: string; @@ -144,6 +150,7 @@ export interface OnSuccessObject { type: 'goto' | 'end'; stepId?: string; workflowId?: string; + parameters?: ActionParameter[]; criteria?: CriterionObject[]; } @@ -154,6 +161,7 @@ export interface OnFailureObject { stepId?: string; retryAfter?: number; retryLimit?: number; + parameters?: ActionParameter[]; criteria?: CriterionObject[]; } diff --git a/packages/respect-core/src/arazzo-schema.ts b/packages/respect-core/src/arazzo-schema.ts index 331cc1cb26..e2ec1eb0c6 100644 --- a/packages/respect-core/src/arazzo-schema.ts +++ b/packages/respect-core/src/arazzo-schema.ts @@ -150,6 +150,7 @@ export const reusableObject = { required: ['reference'], additionalProperties: false, } as const; + export const parameter = { type: 'object', oneOf: [ @@ -169,10 +170,35 @@ export const parameter = { reusableObject, ], } as const; + const parameters = { type: 'array', items: parameter, } as const; + +export const actionParameter = { + type: 'object', + oneOf: [ + { + type: 'object', + properties: { + name: { type: 'string' }, + value: { + oneOf: [{ type: 'string' }, { type: 'number' }, { type: 'boolean' }], + }, + }, + required: ['name', 'value'], + additionalProperties: false, + }, + reusableObject, + ], +} as const; + +const actionParameters = { + type: 'array', + items: actionParameter, +} as const; + export const infoObject = { type: 'object', properties: { @@ -260,6 +286,7 @@ export const onSuccessObject = { type: { type: 'string', enum: ['goto', 'end'] }, stepId: { type: 'string' }, workflowId: { type: 'string' }, + parameters: actionParameters, criteria: criteriaObjects, }, additionalProperties: false, @@ -280,6 +307,7 @@ export const onFailureObject = { stepId: { type: 'string' }, retryAfter: { type: 'number', minimum: 0 }, retryLimit: { type: 'number', minimum: 0 }, + parameters: actionParameters, criteria: criteriaObjects, }, additionalProperties: false, diff --git a/packages/respect-core/src/modules/__tests__/flow-runner/run-step.test.ts b/packages/respect-core/src/modules/__tests__/flow-runner/run-step.test.ts index 89189658f4..8076beed92 100644 --- a/packages/respect-core/src/modules/__tests__/flow-runner/run-step.test.ts +++ b/packages/respect-core/src/modules/__tests__/flow-runner/run-step.test.ts @@ -1113,6 +1113,108 @@ describe('runStep', () => { expect(runWorkflow).toHaveBeenCalled(); }); + it('should map onSuccess action parameters to the target workflow inputs', async () => { + const stepOne: Step = { + stepId: 'get-bird', + 'x-operation': { + url: 'http://localhost:3000/bird', + method: 'get', + }, + successCriteria: [{ condition: '$statusCode == 200' }], + onSuccess: [ + { + name: 'success-action', + workflowId: 'success-action-workflow', + type: 'goto', + parameters: [{ name: 'birdId', value: 'abc-123' }], + criteria: [{ condition: '$statusCode == 200' }], + }, + ], + checks: [], + response: {} as any, + }; + const workflowId = 'get-bird-workflow'; + + vi.mocked(callAPIAndAnalyzeResults).mockImplementationOnce(async ({ step }: { step: Step }) => { + step.checks = [ + { + name: CHECKS.STATUS_CODE_CHECK, + passed: true, + message: '', + severity: 'error', + }, + ]; + + return { + successCriteriaCheck: true, + schemaCheck: true, + networkCheck: true, + unexpectedErrorCheck: true, + statusCodeCheck: true, + }; + }); + + vi.mocked(checkCriteria).mockImplementation(() => [ + { + name: CHECKS.SUCCESS_CRITERIA_CHECK, + passed: true, + message: 'Checking simple criteria: {"condition":"$statusCode == 200"}', + severity: 'error', + }, + ]); + + const context = { + ...basicCTX, + $workflows: { + 'get-bird-workflow': { steps: {}, inputs: {} }, + 'success-action-workflow': { steps: {}, inputs: {} }, + }, + workflows: [ + { + workflowId: 'get-bird-workflow', + steps: [stepOne], + }, + { + workflowId: 'success-action-workflow', + steps: [ + { + stepId: 'use-bird', + 'x-operation': { + url: 'http://localhost:3000/bird', + method: 'get', + }, + checks: [], + }, + ], + }, + ], + } as unknown as TestContext; + + let capturedInputs: Record | undefined; + vi.mocked(runWorkflow).mockImplementationOnce(async ({ ctx }) => { + capturedInputs = ctx.$workflows['success-action-workflow']?.inputs; + return { + type: 'workflow', + invocationContext: {}, + workflowId, + } as WorkflowExecutionResult; + }); + + vi.mocked(resolveWorkflowContext).mockImplementationOnce(async () => { + return { ...context, executedSteps: [] }; + }); + + await runStep({ + step: stepOne, + ctx: context, + workflowId, + executedStepsCount: { value: 0 }, + }); + + expect(runWorkflow).toHaveBeenCalled(); + expect(capturedInputs).toEqual({ birdId: 'abc-123' }); + }); + it('should log error when onSuccess step criteria with goto StepId and WorkflowId provided', async () => { const stepOne: Step = { stepId: 'get-bird', diff --git a/packages/respect-core/src/modules/flow-runner/run-step.ts b/packages/respect-core/src/modules/flow-runner/run-step.ts index 7493a89acc..405856f45a 100644 --- a/packages/respect-core/src/modules/flow-runner/run-step.ts +++ b/packages/respect-core/src/modules/flow-runner/run-step.ts @@ -33,6 +33,35 @@ import { prepareRequest, type RequestData } from './prepare-request.js'; import { runWorkflow, resolveWorkflowContext } from './runner.js'; import { checkCriteria } from './success-criteria/index.js'; +function mapParametersToWorkflowInputs({ + parameters, + ctx, + workflowId, +}: { + parameters: ResolvedParameter[]; + ctx: TestContext; + workflowId: string | undefined; +}): Record { + return parameters.filter(isParameterWithoutIn).reduce( + (acc, parameter: ParameterWithoutIn) => { + const ctxWithInputs = { + ...ctx, + $inputs: { + ...ctx.$inputs, + ...(workflowId ? ctx.$workflows[workflowId]?.inputs : {}), + }, + }; + acc[parameter.name] = getValueFromContext({ + value: parameter.value, + ctx: ctxWithInputs, + logger: ctx.options.logger, + }); + return acc; + }, + {} as Record + ); +} + export async function runStep({ step, ctx, @@ -89,25 +118,11 @@ export async function runStep({ if (resolvedParameters && resolvedParameters.length > 0) { // When the step in context specifies a workflowId, then all parameters without `in` maps to workflow inputs. - const workflowInputParameters = resolvedParameters.filter(isParameterWithoutIn).reduce( - (acc, parameter: ParameterWithoutIn) => { - const ctxWithInputs = { - ...ctx, - $inputs: { - ...ctx.$inputs, - ...(workflowId ? ctx.$workflows[workflowId]?.inputs : {}), - }, - }; - // Ensure parameter is of type ParameterWithoutIn - acc[parameter.name] = getValueFromContext({ - value: parameter.value, - ctx: ctxWithInputs, - logger: ctx.options.logger, - }); - return acc; - }, - {} as Record - ); + const workflowInputParameters = mapParametersToWorkflowInputs({ + parameters: resolvedParameters, + ctx, + workflowId, + }); // Merge the runtime inputs with the inputs passed in the step as parameters for the workflow workflowCtx.$workflows[targetWorkflow.workflowId].inputs = { @@ -304,11 +319,12 @@ export async function runStep({ if (matchesCriteria) { const targetWorkflow = action.workflowId - ? (getValueFromContext({ + ? ctx.workflows.find((w) => w.workflowId === action.workflowId) || + (getValueFromContext({ value: action.workflowId, ctx, logger: ctx.options.logger, - }) as Workflow) + }) as Workflow | undefined) : undefined; const targetCtx = action.workflowId && targetWorkflow @@ -320,6 +336,25 @@ export async function runStep({ ) : { ...ctx, executedSteps: [] }; + const targetWorkflowInputs = + targetWorkflow?.workflowId && targetCtx.$workflows[targetWorkflow.workflowId]; + if (targetWorkflowInputs && action.parameters?.length) { + // The action parameters map to the inputs of the workflow referenced by the action's workflowId. + const resolvedActionParameters = action.parameters.map( + (parameter) => resolveReusableComponentItem(parameter, ctx) as ResolvedParameter + ); + const workflowInputParameters = mapParametersToWorkflowInputs({ + parameters: resolvedActionParameters, + ctx, + workflowId, + }); + + targetWorkflowInputs.inputs = { + ...targetWorkflowInputs.inputs, + ...workflowInputParameters, + }; + } + const targetStep = action.stepId ? action.stepId : undefined; if (type === 'retry') { diff --git a/tests/e2e/build-docs/build-docs-with-disabled-search/snapshot.txt b/tests/e2e/build-docs/build-docs-with-disabled-search/snapshot.txt index 0a74414c31..e3f77a2337 100644 --- a/tests/e2e/build-docs/build-docs-with-disabled-search/snapshot.txt +++ b/tests/e2e/build-docs/build-docs-with-disabled-search/snapshot.txt @@ -12,273 +12,273 @@ margin: 0; } - -
API docs by Redocly
API docs by Redocly

Sample API (1.0.0)

Download OpenAPI specification:

Test.

-

Test

Responses

Response samples

Content type
application/json
{ }
+ " fill="currentColor">

Sample API (1.0.0)

Download OpenAPI specification:

Test.

+

Test

Responses

Response samples

Content type
application/json
{ }