diff --git a/tools/spectral/ipa/CONTRIBUTING.md b/tools/spectral/ipa/CONTRIBUTING.md index 3c9c2df596..97af074452 100644 --- a/tools/spectral/ipa/CONTRIBUTING.md +++ b/tools/spectral/ipa/CONTRIBUTING.md @@ -44,6 +44,18 @@ npm run test -- singletonHasNoId ``` --- +### Checking that a change does not alter results + +For a refactor of the ruleset, `ipa-snapshot-compare.js` lints a spec with the ruleset at a git ref and with this checkout, and reports per rule which findings and collector entries (violations, adoptions, exceptions) were gained or lost. It exits 1 on any difference. + +``` +node tools/spectral/ipa/scripts/ipa-snapshot-compare.js --spec openapi/v2.json --base origin/main +``` + +- The ref is checked out into a temporary worktree that is removed afterwards, so your working tree is not touched. Nothing needs building. +- Without `--head`, the head is this checkout as it is now, including uncommitted changes. Pass `--head ` to compare two refs. +- Add `--verbose` to list the entries. A full run on `openapi/v2.json` takes about 15 seconds. + ## Code Style - Use [Prettier](https://prettier.io/) for code formatting @@ -106,11 +118,35 @@ To learn more about Spectral custom functions, see the [Spectral Documentation]( --- +### Where a Rule Applies: Conditions and `defineRule` + +Concepts such as "singleton resource", "read-only resource" or "single resource path" are defined once, as named **conditions** in [`rulesets/functions/utils/conditions`](./rulesets/functions/utils/conditions). Each condition has a plain-English definition and examples (add a `note` to an example when what makes it match isn't visible in its path), and the glossary [`rulesets/CONDITIONS.md`](./rulesets/CONDITIONS.md) is generated from them. + +Build new rules with [`defineRule`](./rulesets/functions/utils/defineRule.js), declaring the conditions under which the rule applies instead of returning early: + +```js +import { defineRule } from './utils/defineRule.js'; +import { not } from './utils/conditions/registry.js'; +import { isReadOnly, isSingleton } from './utils/conditions/resource.js'; + +export default defineRule({ + appliesTo: [isSingleton, not(isReadOnly)], + check: ({ input, path }) => (hasPutMethod(input) || hasPatchMethod(input) ? [] : [{ path, message: ERROR_MESSAGE }]), +}); +``` + +- `appliesTo` conditions are evaluated in order. If one does not hold, the rule does not apply and nothing is collected. +- `check` returns the validation errors; adoptions, violations and exceptions are collected for you (see below). +- The rule's "Applies to" section in [`rulesets/README.md`](./rulesets/README.md) is generated from `appliesTo`, so don't describe these conditions in the YAML description. +- Reuse an existing condition where possible. If a rule needs a different definition, add a condition with `variantOf` and a `divergence` note rather than changing the shared one. + +To check that a refactor changes no result, run `tools/spectral/ipa/scripts/ipa-snapshot.js` on the same spec before and after, then compare the two outputs with `ipa-snapshot-compare.js` (see the header of each script for options). + ### Resource & Singleton Evaluation In IPA Spectral validation, a **resource** is typically identified using a *resource collection path*, such as `/resource`. -To develop rules that evaluate resource and singleton patterns, you can use the following utility functions: +The conditions above wrap the following utility functions, which rules not yet built with `defineRule` use directly: #### Retrieve Resource Path Items diff --git a/tools/spectral/ipa/__tests__/scripts/ipaSnapshot.test.js b/tools/spectral/ipa/__tests__/scripts/ipaSnapshot.test.js new file mode 100644 index 0000000000..3c966f347b --- /dev/null +++ b/tools/spectral/ipa/__tests__/scripts/ipaSnapshot.test.js @@ -0,0 +1,89 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from '@jest/globals'; +import { normalizeCollectorEntries, normalizeFindings } from '../../scripts/ipa-snapshot'; +import { compareRefs, diffEntries } from '../../scripts/ipa-snapshot-compare'; + +describe('tools/spectral/ipa/scripts/ipa-snapshot.js', () => { + it('sorts findings and drops the source file', () => { + const finding = (code, jsonPath) => ({ code, path: jsonPath, severity: 0, message: 'm', source: '/tmp/a.yaml' }); + expect(normalizeFindings([finding('b', ['paths', '/b']), finding('a', ['paths', '/a'])])).toEqual([ + { code: 'a', path: 'paths./a', severity: 0, message: 'm' }, + { code: 'b', path: 'paths./b', severity: 0, message: 'm' }, + ]); + }); + + it('counts identical collector entries regardless of key order', () => { + const normalized = normalizeCollectorEntries({ + violations: [ + { ruleName: 'r', componentId: 'c' }, + { componentId: 'c', ruleName: 'r' }, + { ruleName: 'r', componentId: 'd' }, + ], + }); + expect(normalized.violations).toEqual([ + { componentId: 'c', ruleName: 'r', count: 2 }, + { componentId: 'd', ruleName: 'r', count: 1 }, + ]); + }); +}); + +describe('tools/spectral/ipa/scripts/ipa-snapshot-compare.js', () => { + const ruleOf = (entry) => entry.ruleName; + + it('reports nothing for identical entries', () => { + const entries = [{ ruleName: 'r', componentId: 'c', count: 2 }]; + expect(diffEntries(entries, entries, ruleOf).size).toBe(0); + }); + + it('reports added and removed entries per rule, using the count difference', () => { + const base = [ + { ruleName: 'r', componentId: 'c', count: 2 }, + { ruleName: 'r', componentId: 'gone' }, + ]; + const head = [ + { ruleName: 'r', componentId: 'c', count: 3 }, + { ruleName: 's', componentId: 'new' }, + ]; + const diff = diffEntries(base, head, ruleOf); + expect(diff.get('r')).toEqual({ + added: [{ ruleName: 'r', componentId: 'c', count: 1 }], + removed: [{ ruleName: 'r', componentId: 'gone', count: 1 }], + }); + expect(diff.get('s')).toEqual({ added: [{ ruleName: 's', componentId: 'new', count: 1 }], removed: [] }); + }); +}); + +describe('tools/spectral/ipa/scripts/ipa-snapshot-compare.js compareRefs', () => { + const spec = path.join(os.tmpdir(), `ipa-snapshot-test-spec-${process.pid}.yaml`); + + beforeAll(() => { + fs.writeFileSync( + spec, + [ + 'openapi: 3.0.1', + 'info: { title: t, version: "1" }', + 'paths:', + ' /api/atlas/v2/groups/{groupId}:', + ' get:', + ' operationId: getGroup', + ' responses:', + " '200': { description: OK }", + 'components: {}', + ].join('\n') + ); + }); + + afterAll(() => { + fs.rmSync(spec, { force: true }); + }); + + it('finds no difference between a ref and the same checkout, and leaves no worktree behind', () => { + const { findings, collector } = compareRefs({ spec, base: 'HEAD' }); + expect(findings.size).toBe(0); + expect(Object.values(collector).every((diff) => diff.size === 0)).toBe(true); + const leftovers = fs.readdirSync(os.tmpdir()).filter((name) => /^ipa-(worktree|snapshot-compare)-/.test(name)); + expect(leftovers).toEqual([]); + }, 60000); +}); diff --git a/tools/spectral/ipa/__tests__/utils/conditions.test.js b/tools/spectral/ipa/__tests__/utils/conditions.test.js new file mode 100644 index 0000000000..023c62d23b --- /dev/null +++ b/tools/spectral/ipa/__tests__/utils/conditions.test.js @@ -0,0 +1,94 @@ +import { describe, expect, it } from '@jest/globals'; +import { + allOf, + anyOf, + defineCondition, + evaluate, + listConditions, + not, +} from '../../rulesets/functions/utils/conditions/registry'; +import { RuleContext } from '../../rulesets/functions/utils/ruleContext'; +import '../../rulesets/functions/utils/conditions/path'; +import '../../rulesets/functions/utils/conditions/resource'; +import '../../rulesets/functions/utils/conditions/operation'; +import '../../rulesets/functions/utils/conditions/response'; + +function contextFrom({ input, options, path, paths = {} }) { + return new RuleContext({ input, options, path, ruleName: 'test-rule', oas: { paths } }); +} + +describe('tools/spectral/ipa/rulesets/functions/utils/conditions', () => { + describe.each(listConditions().map((condition) => [condition.id, condition]))('%s', (_id, condition) => { + it('has a definition and examples', () => { + expect(condition.definition).toBeTruthy(); + expect(condition.examples.match.length).toBeGreaterThan(0); + expect(condition.examples.noMatch.length).toBeGreaterThan(0); + }); + + it('matches its match examples', () => { + condition.examples.match.forEach((example) => { + expect({ example, pass: evaluate(condition, contextFrom(example)).pass }).toEqual({ example, pass: true }); + }); + }); + + it('does not match its noMatch examples', () => { + condition.examples.noMatch.forEach((example) => { + expect({ example, pass: evaluate(condition, contextFrom(example)).pass }).toEqual({ example, pass: false }); + }); + }); + + it('has examples that can be told apart in the glossary', () => { + const shown = [...condition.examples.match, ...condition.examples.noMatch].map( + (example) => `${example.path.slice(1).join(' ')} ${example.note ?? ''}` + ); + expect(new Set(shown).size).toBe(shown.length); + }); + + it('has an id that starts with its subject', () => { + expect(condition.id.split('.')[0]).toBe(condition.subject); + }); + + it('names a known condition as variantOf', () => { + if (condition.variantOf) { + expect(listConditions().map((c) => c.id)).toContain(condition.variantOf); + expect(condition.divergence).toBeTruthy(); + } + }); + }); + + describe('combinators', () => { + const yes = defineCondition({ id: 'test.yes', fn: () => true }); + const no = defineCondition({ id: 'test.no', fn: () => false }); + const named = defineCondition({ id: 'test.named', of: allOf(yes, no) }); + + it('reports the failing leaf of an anonymous allOf', () => { + expect(evaluate(allOf(yes, no), {})).toEqual({ pass: false, failedAt: ['test.no'] }); + }); + + it('reports a named composite and the operand that failed', () => { + expect(evaluate(named, {})).toEqual({ pass: false, failedAt: ['test.named', 'test.no'] }); + }); + + it('evaluates not and anyOf', () => { + expect(evaluate(not(no), {}).pass).toBe(true); + expect(evaluate(not(yes), {})).toEqual({ pass: false, failedAt: ['not(test.yes)'] }); + expect(evaluate(anyOf(no, yes), {}).pass).toBe(true); + expect(evaluate(anyOf(no, no), {}).pass).toBe(false); + }); + + it('short-circuits like && and ||', () => { + const throws = defineCondition({ + id: 'test.throws', + fn: () => { + throw new Error('evaluated'); + }, + }); + expect(evaluate(allOf(no, throws), {}).pass).toBe(false); + expect(evaluate(anyOf(yes, throws), {}).pass).toBe(true); + }); + + it('rejects duplicate ids', () => { + expect(() => defineCondition({ id: 'test.yes', fn: () => true })).toThrow('already defined'); + }); + }); +}); diff --git a/tools/spectral/ipa/__tests__/utils/defineRule.test.js b/tools/spectral/ipa/__tests__/utils/defineRule.test.js new file mode 100644 index 0000000000..848a4d4e18 --- /dev/null +++ b/tools/spectral/ipa/__tests__/utils/defineRule.test.js @@ -0,0 +1,110 @@ +import { beforeEach, describe, expect, it } from '@jest/globals'; +import collector, { EntryType } from '../../metrics/collector'; +import { defineRule } from '../../rulesets/functions/utils/defineRule'; +import { defineCondition } from '../../rulesets/functions/utils/conditions/registry'; + +const holds = defineCondition({ id: 'defineRuleTest.holds', fn: () => true }); +const fails = defineCondition({ id: 'defineRuleTest.fails', fn: () => false }); + +const spectralContext = { + path: ['paths', '/resource', 'get'], + documentInventory: { resolved: { paths: { '/resource': { get: {} } } }, unresolved: { paths: {} } }, + rule: { name: 'test-rule' }, +}; + +function collected() { + return Object.fromEntries(Object.entries(collector.entries).map(([type, list]) => [type, list.length])); +} + +describe('tools/spectral/ipa/rulesets/functions/utils/defineRule.js', () => { + beforeEach(() => { + collector.entries = { [EntryType.VIOLATION]: [], [EntryType.ADOPTION]: [], [EntryType.EXCEPTION]: [] }; + }); + + it('attaches the conditions as meta', () => { + const rule = defineRule({ appliesTo: [holds], check: () => [] }); + expect(rule.meta).toEqual({ + appliesTo: [holds], + checkUses: [], + document: 'resolved', + onInternalError: 'violation', + }); + }); + + it('does not run the check or collect when a condition fails', () => { + let checked = false; + const rule = defineRule({ + appliesTo: [holds, fails], + check: () => { + checked = true; + return []; + }, + }); + expect(rule({}, null, spectralContext)).toBeUndefined(); + expect(checked).toBe(false); + expect(collected()).toEqual({ violations: 0, adoptions: 0, exceptions: 0 }); + }); + + it('collects an adoption when the check returns no errors', () => { + const rule = defineRule({ appliesTo: [holds], check: () => [] }); + expect(rule({}, null, spectralContext)).toBeUndefined(); + expect(collected()).toEqual({ violations: 0, adoptions: 1, exceptions: 0 }); + }); + + it('collects an exception from the exception target', () => { + const rule = defineRule({ + check: ({ path }) => [{ path, message: 'bad' }], + exceptionTarget: () => ({ 'x-xgen-IPA-exception': { 'test-rule': 'reason' } }), + }); + expect(rule({}, null, spectralContext)).toBeUndefined(); + expect(collected()).toEqual({ violations: 0, adoptions: 0, exceptions: 1 }); + }); + + it('gives the check the context and options', () => { + const rule = defineRule({ + check: ({ options, resourcePath, path }) => [{ path, message: `${options.a} ${resourcePath}` }], + }); + expect(rule({}, { a: 'x' }, spectralContext)).toEqual([{ path: spectralContext.path, message: 'x /resource' }]); + }); + + it('runs beforeCheck only when the rule applies, and lets its exceptions propagate', () => { + const beforeCheck = () => { + throw new Error('before'); + }; + expect(() => defineRule({ appliesTo: [holds], beforeCheck, check: () => [] })({}, null, spectralContext)).toThrow( + 'before' + ); + expect(() => + defineRule({ appliesTo: [fails], beforeCheck, check: () => [] })({}, null, spectralContext) + ).not.toThrow(); + }); + + it('names the function after the rule, so a crash outside the check says which rule', () => { + const rule = defineRule({ check: () => [] }); + rule({}, null, spectralContext); + expect(rule.name).toBe('test-rule'); + }); + + describe('when the check throws', () => { + const throwing = () => { + throw new Error('boom'); + }; + + it("reports and collects a violation in 'violation' mode", () => { + const rule = defineRule({ check: throwing }); + expect(rule({}, null, spectralContext)[0].message).toContain('test-rule Internal Rule Error: Error: boom'); + expect(collected()).toEqual({ violations: 1, adoptions: 0, exceptions: 0 }); + }); + + it("reports without collecting in 'return' mode", () => { + const rule = defineRule({ check: throwing, onInternalError: 'return' }); + expect(rule({}, null, spectralContext)[0].message).toContain('Internal Rule Error'); + expect(collected()).toEqual({ violations: 0, adoptions: 0, exceptions: 0 }); + }); + + it("propagates in 'throw' mode", () => { + const rule = defineRule({ check: throwing, onInternalError: 'throw' }); + expect(() => rule({}, null, spectralContext)).toThrow('boom'); + }); + }); +}); diff --git a/tools/spectral/ipa/metrics/collector.js b/tools/spectral/ipa/metrics/collector.js index 15ec5131d2..e41cb77c71 100644 --- a/tools/spectral/ipa/metrics/collector.js +++ b/tools/spectral/ipa/metrics/collector.js @@ -27,7 +27,8 @@ class Collector { [EntryType.EXCEPTION]: [], }; - this.fileName = 'ipa-collector-results-combined.log'; + // IPA_COLLECTOR_FILE lets tooling (e.g. scripts/ipa-snapshot.js) direct the log to a private path + this.fileName = process.env.IPA_COLLECTOR_FILE || 'ipa-collector-results-combined.log'; process.on('exit', () => this.flushToFile()); process.on('SIGINT', () => { diff --git a/tools/spectral/ipa/rulesets/CONDITIONS.md b/tools/spectral/ipa/rulesets/CONDITIONS.md new file mode 100644 index 0000000000..2f04d57a73 --- /dev/null +++ b/tools/spectral/ipa/rulesets/CONDITIONS.md @@ -0,0 +1,280 @@ + + +# IPA Validation Conditions + +Named conditions that decide where IPA rules apply. They are defined in [rulesets/functions/utils/conditions](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/rulesets/functions/utils/conditions) and used by rules built with `defineRule`. Only conditions used by at least one rule are listed. + +### mediaType.baseGetHasMediaType + +Evaluated on: mediaType + +The resource the custom method belongs to (its path without `:`) has a Get method with a 200 response for the same media type as the current response. + +Matches: +- `/groups/{groupId}/settings:reset post responses 200 content application/json` (the Get on /groups/{groupId}/settings has an application/json 200 response) + +Does not match: +- `/groups/{groupId}/settings:reset post responses 200 content application/json` (the base resource has no Get) + +Used by: [xgen-IPA-113-reset-method-response-is-get-method-response](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-response-is-get-method-response) + +### mediaType.hasSchema + +Evaluated on: mediaType + +The media type object exists and defines a `schema`. + +Matches: +- `/groups/{groupId} get responses 200 content application/json` (has a schema) + +Does not match: +- `/groups/{groupId} get responses 200 content application/json` (an example but no schema) + +Used by: [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource), [xgen-IPA-113-reset-method-response-is-get-method-response](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-response-is-get-method-response) + +### mediaType.isJson + +Evaluated on: mediaType + +The media type ends in `json`, e.g. `application/vnd.atlas.2023-01-01+json`. + +Matches: +- `/groups/{groupId} get responses 200 content application/vnd.atlas.2023-01-01+json` + +Does not match: +- `/groups/{groupId} get responses 200 content application/vnd.atlas.2023-01-01+csv` + +Used by: [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource), [xgen-IPA-113-reset-method-response-is-get-method-response](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-response-is-get-method-response) + +### operation.hasCustomMethodOverride + +Evaluated on: operation + +The operation has an `x-xgen-method-verb-override` extension with `customMethod: true`, declaring a legacy custom method that has no `:` in its path. + +Matches: +- `/groups/{groupId} get` (verb override `search` with customMethod: true) + +Does not match: +- `/groups/{groupId} get` (no verb override) +- `/groups/{groupId} get` (verb override `get` with customMethod: false) + +Used by: [xgen-IPA-104-operation-id-length](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-operation-id-length), [xgen-IPA-104-valid-operation-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-valid-operation-id) + +### operation.hasListVerbOverride + +Evaluated on: operation + +The operation has an `x-xgen-method-verb-override` extension whose verb starts with `list`, declaring it a List method. + +Matches: +- `/groups/{groupId} get` (verb override `listByGroup`) + +Does not match: +- `/groups/{groupId} get` (verb override `get`) + +Used by: [xgen-IPA-104-operation-id-length](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-operation-id-length), [xgen-IPA-104-valid-operation-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-valid-operation-id) + +### operation.hasRuleMethodVerbOverride + +Evaluated on: operation + +The operation has an `x-xgen-method-verb-override` extension whose verb starts with the rule's `methodName` option (for IPA-104, `get`), declaring it that standard method. + +Matches: +- `/groups/{groupId} get` (verb override `get`, rule methodName `get`) + +Does not match: +- `/groups/{groupId} get` (no verb override, rule methodName `get`) + +Used by: [xgen-IPA-104-operation-id-length](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-operation-id-length), [xgen-IPA-104-valid-operation-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-valid-operation-id) + +### path.isCollection + +Evaluated on: path + +The path ends in a resource name: either a single top-level segment (`/groups`) or a segment following a path parameter (`/groups/{groupId}/clusters`). This is the path of a resource collection or of a singleton resource. + +Matches: +- `/api/atlas/v2/groups` +- `/api/atlas/v2/groups/{groupId}/clusters` + +Does not match: +- `/api/atlas/v2/groups/{groupId}` +- `/api/atlas/v2/groups/clusters` +- `/api/atlas/v2/groups:search` + +Used by: [xgen-IPA-104-get-method-no-request-body](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-no-request-body), [xgen-IPA-104-get-method-response-code-is-200](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-code-is-200), [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource), [xgen-IPA-104-resource-has-GET](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-resource-has-GET), [xgen-IPA-113-reset-method-only-on-singleton-resources](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-only-on-singleton-resources), [xgen-IPA-113-singleton-must-not-have-delete-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-delete-method), [xgen-IPA-113-singleton-must-not-have-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-id), [xgen-IPA-113-singleton-should-have-update-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method) + +### path.isGetTarget + +Evaluated on: path + +The path is where a standard Get method belongs: an instance path, or the path of a singleton resource. + +Defined as: any of: the path is a single resource (instance) path ([`path.isInstance`](#pathisinstance)); the path is a singleton resource ([`resource.isSingleton`](#resourceissingleton)) + +Matches: +- `/groups/{groupId}` (instance path) +- `/groups/{groupId}/settings` (singleton) + +Does not match: +- `/groups` (collection of a standard resource) +- `/groups/settings/{name}` (not an instance path) + +Used by: [xgen-IPA-104-get-method-no-request-body](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-no-request-body), [xgen-IPA-104-get-method-response-code-is-200](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-code-is-200), [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource) + +### path.isInstance + +Evaluated on: path + +The path is a collection path (see `path.isCollection`) followed by a single path parameter, e.g. `/groups/{groupId}`. This is the path of one resource in a collection. + +Matches: +- `/api/atlas/v2/groups/{groupId}` +- `/api/atlas/v2/groups/{groupId}/clusters/{clusterName}` + +Does not match: +- `/api/atlas/v2/groups` +- `/api/atlas/v2/groups/{groupId}/clusters` +- `/api/atlas/v2/groups/settings/{name}` + +Used by: [xgen-IPA-104-get-method-no-request-body](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-no-request-body), [xgen-IPA-104-get-method-response-code-is-200](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-code-is-200), [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource) + +### path.isLegacyGetTarget + +Evaluated on: path + +The path ends in a path parameter (its last character is `}`), or is the path of a singleton resource. Implemented by `methodLogic.isInvalidGetMethod`. + +> **Variant of [`path.isGetTarget`](#pathisgettarget).** Accepts any path ending in `}`, including ones that are not instance paths such as `/groups/settings/{name}`. Used by the IPA-104 operation ID rules; the other IPA-104 rules use `path.isGetTarget`. + +Matches: +- `/groups/{groupId}` (instance path) +- `/groups/settings/{name}` (ends in a path parameter, though not an instance path) + +Does not match: +- `/groups` (collection of a standard resource) + +Used by: [xgen-IPA-104-operation-id-length](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-operation-id-length), [xgen-IPA-104-valid-operation-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-valid-operation-id) + +### path.isResetMethod + +Evaluated on: path + +The path is a custom method named `reset`, e.g. `/groups/{groupId}/settings:reset`. + +Matches: +- `/api/atlas/v2/groups/{groupId}/settings:reset` + +Does not match: +- `/api/atlas/v2/groups/{groupId}/settings` +- `/api/atlas/v2/groups/{groupId}/settings:restart` + +Used by: [xgen-IPA-113-reset-method-must-not-have-request-body](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-must-not-have-request-body), [xgen-IPA-113-reset-method-must-return-200-OK](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-must-return-200-OK), [xgen-IPA-113-reset-method-must-use-POST](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-must-use-POST), [xgen-IPA-113-reset-method-not-on-readonly-singleton](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-not-on-readonly-singleton), [xgen-IPA-113-reset-method-only-on-singleton-resources](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-only-on-singleton-resources), [xgen-IPA-113-reset-method-response-is-get-method-response](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-response-is-get-method-response), [xgen-IPA-113-reset-method-valid-operation-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-valid-operation-id) + +### pathItem.hasGet + +Evaluated on: pathItem + +The path item defines a `get` operation. + +Matches: +- `/groups/{groupId}/settings` (defines get) + +Does not match: +- `/groups/{groupId}/settings` (patch only) + +Used by: [xgen-IPA-113-singleton-must-not-have-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-id) + +### resource.getResponseIsAllReadOnly + +Evaluated on: resource + +Every property of the 2xx response schema of the resource's Get method is `readOnly: true`. The Get on the instance path is used if there is one; otherwise the first path with a Get, which may be a List, in which case `results.items` is inspected. + +Matches: +- `/groups/{groupId}/settings` (every Get response property is readOnly) + +Does not match: +- `/groups/{groupId}/settings` (the Get response has a writable property) + +Used by: [xgen-IPA-113-reset-method-not-on-readonly-singleton](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-not-on-readonly-singleton), [xgen-IPA-113-singleton-should-have-update-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method) + +### resource.hasResourceHasGetException + +Evaluated on: resource + +Some path item of the resource carries an `x-xgen-IPA-exception` for `xgen-IPA-104-resource-has-GET`. + +Matches: +- `/groups/{groupId}/settings` (the path has the exception) + +Does not match: +- `/groups/{groupId}/settings` (no exception) + +Used by: [xgen-IPA-113-reset-method-not-on-readonly-singleton](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-not-on-readonly-singleton), [xgen-IPA-113-singleton-should-have-update-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method) + +### resource.hasSingletonShape + +Evaluated on: resource + +The resource's path items contain exactly one collection path, that path belongs to a parent resource (`/parent/{id}/child`), it has no Create (POST) method, and every other path is a custom method. The current path is not required to be the collection path; see `resource.isSingleton`. + +Matches: +- `/groups/{groupId}/settings` (Get and Patch only) + +Does not match: +- `/groups/{groupId}/clusters` (has Create and an instance path) +- `/settings` (no parent resource) + +Used by: [xgen-IPA-104-get-method-no-request-body](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-no-request-body), [xgen-IPA-104-get-method-response-code-is-200](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-code-is-200), [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource), [xgen-IPA-104-resource-has-GET](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-resource-has-GET), [xgen-IPA-113-reset-method-only-on-singleton-resources](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-only-on-singleton-resources), [xgen-IPA-113-singleton-must-not-have-delete-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-delete-method), [xgen-IPA-113-singleton-must-not-have-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-id), [xgen-IPA-113-singleton-should-have-update-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method) + +### resource.isReadOnly + +Evaluated on: resource + +The resource has a Get method whose response is entirely read-only, and no path of the resource has an exception for `xgen-IPA-104-resource-has-GET`. + +Defined as: all of: not: some path of the resource has an exception for `xgen-IPA-104-resource-has-GET` ([`resource.hasResourceHasGetException`](#resourcehasresourcehasgetexception)); all properties of the resource's Get response are read-only ([`resource.getResponseIsAllReadOnly`](#resourcegetresponseisallreadonly)) + +> **Divergence.** The exception clause is not part of the IPA definition: a resource with an IPA-104 resource-has-GET exception is never read-only, whatever its Get response looks like. + +Matches: +- `/groups/{groupId}/settings` (every Get response property is readOnly) + +Does not match: +- `/groups/{groupId}/settings` (the same read-only Get response, but the path has an xgen-IPA-104-resource-has-GET exception) + +Used by: [xgen-IPA-113-reset-method-not-on-readonly-singleton](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-not-on-readonly-singleton), [xgen-IPA-113-singleton-should-have-update-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method) + +### resource.isSingleton + +Evaluated on: resource + +The path is a collection path and the resource it names has the singleton shape. + +Defined as: all of: the path is a resource collection path ([`path.isCollection`](#pathiscollection)); the resource has the singleton shape ([`resource.hasSingletonShape`](#resourcehassingletonshape)) + +Matches: +- `/groups/{groupId}/settings` (singleton with a custom method) + +Does not match: +- `/groups/{groupId}/settings:reset` (the custom method path of a singleton, not its collection path) + +Used by: [xgen-IPA-104-get-method-no-request-body](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-no-request-body), [xgen-IPA-104-get-method-response-code-is-200](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-code-is-200), [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource), [xgen-IPA-113-reset-method-only-on-singleton-resources](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-only-on-singleton-resources), [xgen-IPA-113-singleton-must-not-have-delete-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-delete-method), [xgen-IPA-113-singleton-must-not-have-id](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-id), [xgen-IPA-113-singleton-should-have-update-method](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method) + +### response.is2xx + +Evaluated on: response + +The response status code starts with `2`. + +Matches: +- `/groups/{groupId} get responses 200 content application/json` +- `/groups/{groupId} get responses 202 content application/json` + +Does not match: +- `/groups/{groupId} get responses 404 content application/json` + +Used by: [xgen-IPA-104-get-method-response-has-no-input-fields](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields), [xgen-IPA-104-get-method-returns-response-suffixed-object](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object), [xgen-IPA-104-get-method-returns-single-resource](https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource) diff --git a/tools/spectral/ipa/rulesets/IPA-104.yaml b/tools/spectral/ipa/rulesets/IPA-104.yaml index f13f83baff..47fd4cf125 100644 --- a/tools/spectral/ipa/rulesets/IPA-104.yaml +++ b/tools/spectral/ipa/rulesets/IPA-104.yaml @@ -22,7 +22,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Only applies to resource collection identifiers - For singleton resources, verifies the resource has a GET method - For regular resources, verifies there is a single resource path with a GET method message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-resource-has-GET' @@ -37,7 +36,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to 2xx responses of GET methods on single resources or singleton resources - Verifies the response is not an array or paginated result - Different error messages are provided for standard vs singleton resources message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-single-resource' @@ -51,7 +49,6 @@ rules: The Get method must return a 200 OK response. ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Verifies the 200 OK response code is present - Fails if the method lacks a 200 OK response or defines a different 2xx status code message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-code-is-200' @@ -64,7 +61,6 @@ rules: The Get method of a resource should return a "Response" suffixed object. ##### Implementation details Rule checks for the following conditions: - - Applies only to 2xx responses of GET methods on single resources or singleton resources - Verifies the schema references a predefined schema (not inline) - Confirms the referenced schema name ends with "Response" suffix message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-returns-response-suffixed-object' @@ -78,7 +74,6 @@ rules: The Get method response object must not include writeOnly properties (fields that should be used only on creation or update, ie output fields). ##### Implementation details Rule checks for the following conditions: - - Applies only to 2xx responses of GET methods on single resources or singleton resources - Searches through the schema to find any properties marked with writeOnly attribute - Fails if any writeOnly properties are found in the response schema message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-response-has-no-input-fields' @@ -92,7 +87,6 @@ rules: The Get method request must not include a body. ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Verifies that the operation object does not contain a requestBody property message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-104-get-method-no-request-body' severity: error @@ -108,7 +102,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Confirms that the existing OperationId is compliant with generated IPA Compliant OperationId ##### Configuration @@ -133,7 +126,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Verifies that the operation ID is not longer than 4 words - If the operation ID is longer than 4 words, requires an 'x-xgen-operation-id-override' extension - Can be exempted using 'x-xgen-IPA-exception' for APIs where long operation IDs are intentional diff --git a/tools/spectral/ipa/rulesets/IPA-113.yaml b/tools/spectral/ipa/rulesets/IPA-113.yaml index eaa966aaa7..e27bd066e3 100644 --- a/tools/spectral/ipa/rulesets/IPA-113.yaml +++ b/tools/spectral/ipa/rulesets/IPA-113.yaml @@ -20,8 +20,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to singleton resources that are identified as resource collection identifiers - - Checks that the resource has a GET method defined - Examines all 2xx response schemas from the GET method - Verifies that no schema contains 'id' or '_id' properties in their object definitions - Fails if any response schema contains these identifier properties @@ -36,7 +34,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to singleton resources - Checks that the resource does not have a DELETE method defined message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-must-not-have-delete-method' severity: error @@ -49,8 +46,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to singleton resources - - Excludes read-only singleton resources (where all properties in the GET response schema are marked as readOnly; for List responses, all properties in the items schema must be readOnly) - Checks that the resource has the PUT and/or PATCH methods defined message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method' severity: error @@ -63,7 +58,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to paths ending with :reset - Verifies that only POST method is defined - Fails if GET or any other HTTP method is used - Fails if multiple HTTP methods are defined for the same :reset endpoint @@ -78,7 +72,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - Verifies that the operation object does not contain a requestBody property - Fails if any request body is defined message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-must-not-have-request-body' @@ -92,7 +85,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - Verifies that a 200 OK response code is present - Fails if the method lacks a 200 OK response or defines a different 2xx status code - Verifies that the 200 response has a response body with schema @@ -107,8 +99,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - - Applies only to JSON response content types - Verifies that both :reset and GET methods have schema references - Confirms that the :reset method 200 response schema reference matches the GET method response schema reference - Ensures the reset resource returned is the same type as the singleton resource @@ -124,9 +114,7 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to paths ending with :reset - - Verifies that the parent path (without :reset) is a singleton resource - - Uses existing isSingletonResource() helper function + - Verifies that the base resource (the path without :reset) is a singleton resource (see [`resource.isSingleton`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceissingleton)) - Fails if :reset is defined on a non-singleton resource message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-only-on-singleton-resources' severity: error @@ -139,10 +127,8 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to paths ending with :reset - - Verifies that the parent singleton resource is not read-only - - Uses existing isReadOnlyResource() helper function - - Fails if the singleton resource has all properties marked as readOnly: true + - Verifies that the base resource (the path without :reset) is not read-only (see [`resource.isReadOnly`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceisreadonly)) + - Does not check that the base resource is a singleton; that is covered by xgen-IPA-113-reset-method-only-on-singleton-resources message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-not-on-readonly-singleton' severity: error given: '$.paths[*]' @@ -154,7 +140,6 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - Confirms that the operation ID follows the pattern: reset{ResourceName} - Uses existing operation ID validation infrastructure diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 0816e17fb6..4f8f94a42c 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -92,10 +92,13 @@ APIs must provide a Get method for resources. ##### Implementation details Rule checks for the following conditions: - - Only applies to resource collection identifiers - For singleton resources, verifies the resource has a GET method - For regular resources, verifies there is a single resource path with a GET method +##### Applies to +Each path, when: + - the path is a resource collection path ([`path.isCollection`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathiscollection)) + #### xgen-IPA-104-get-method-returns-single-resource ![error](https://img.shields.io/badge/error-red) @@ -103,49 +106,73 @@ The purpose of the Get method is to return data from a single resource. ##### Implementation details Rule checks for the following conditions: - - Applies only to 2xx responses of GET methods on single resources or singleton resources - Verifies the response is not an array or paginated result - Different error messages are provided for standard vs singleton resources +##### Applies to +Each media type of each Get response, when: + - the response is a 2xx response ([`response.is2xx`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#responseis2xx)) + - the media type defines a schema ([`mediaType.hasSchema`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypehasschema)) + - the media type is JSON ([`mediaType.isJson`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypeisjson)) + - the path is a single resource path or a singleton resource ([`path.isGetTarget`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisgettarget)) + #### xgen-IPA-104-get-method-response-code-is-200 ![error](https://img.shields.io/badge/error-red) The Get method must return a 200 OK response. ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Verifies the 200 OK response code is present - Fails if the method lacks a 200 OK response or defines a different 2xx status code +##### Applies to +Each Get operation, when: + - the path is a single resource path or a singleton resource ([`path.isGetTarget`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisgettarget)) + #### xgen-IPA-104-get-method-returns-response-suffixed-object ![error](https://img.shields.io/badge/error-red) The Get method of a resource should return a "Response" suffixed object. ##### Implementation details Rule checks for the following conditions: - - Applies only to 2xx responses of GET methods on single resources or singleton resources - Verifies the schema references a predefined schema (not inline) - Confirms the referenced schema name ends with "Response" suffix +##### Applies to +Each media type of each Get response, when: + - the response is a 2xx response ([`response.is2xx`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#responseis2xx)) + - the media type defines a schema ([`mediaType.hasSchema`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypehasschema)) + - the media type is JSON ([`mediaType.isJson`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypeisjson)) + - the path is a single resource path or a singleton resource ([`path.isGetTarget`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisgettarget)) + #### xgen-IPA-104-get-method-response-has-no-input-fields ![error](https://img.shields.io/badge/error-red) The Get method response object must not include writeOnly properties (fields that should be used only on creation or update, ie output fields). ##### Implementation details Rule checks for the following conditions: - - Applies only to 2xx responses of GET methods on single resources or singleton resources - Searches through the schema to find any properties marked with writeOnly attribute - Fails if any writeOnly properties are found in the response schema +##### Applies to +Each media type of each Get response, when: + - the response is a 2xx response ([`response.is2xx`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#responseis2xx)) + - the media type defines a schema ([`mediaType.hasSchema`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypehasschema)) + - the media type is JSON ([`mediaType.isJson`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypeisjson)) + - the path is a single resource path or a singleton resource ([`path.isGetTarget`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisgettarget)) + #### xgen-IPA-104-get-method-no-request-body ![error](https://img.shields.io/badge/error-red) The Get method request must not include a body. ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Verifies that the operation object does not contain a requestBody property +##### Applies to +Each Get operation, when: + - the path is a single resource path or a singleton resource ([`path.isGetTarget`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisgettarget)) + #### xgen-IPA-104-valid-operation-id ![error](https://img.shields.io/badge/error-red) @@ -156,7 +183,6 @@ For a collection-scoped Operations resource, such as '/resources/operations/{ope ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Confirms that the existing OperationId is compliant with generated IPA Compliant OperationId ##### Configuration @@ -165,6 +191,14 @@ This rule includes three configuration options: - `ignoreSingularizationList`: Words that are allowed to maintain their assumed plurality (e.g., "Fts") - `maxLength`: The maximum number of words allowed in the operation ID override (default: 4) +##### Applies to +Each Get operation, when: + - any of: the path ends in a path parameter or is a singleton resource ([`path.isLegacyGetTarget`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathislegacygettarget)); the operation is declared the rule's method by `x-xgen-method-verb-override` ([`operation.hasRuleMethodVerbOverride`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#operationhasrulemethodverboverride)) + +Except when: + - the operation is declared a custom method by `x-xgen-method-verb-override` ([`operation.hasCustomMethodOverride`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#operationhascustommethodoverride)) + - the operation is declared a List method by `x-xgen-method-verb-override` ([`operation.hasListVerbOverride`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#operationhaslistverboverride)) + #### xgen-IPA-104-operation-id-length ![error](https://img.shields.io/badge/error-red) @@ -173,7 +207,6 @@ add an 'x-xgen-operation-id-override' extension with a shorter operation ID. ##### Implementation details Rule checks for the following conditions: - - Applies only to GET methods on single resources or singleton resources - Verifies that the operation ID is not longer than 4 words - If the operation ID is longer than 4 words, requires an 'x-xgen-operation-id-override' extension - Can be exempted using 'x-xgen-IPA-exception' for APIs where long operation IDs are intentional @@ -184,6 +217,14 @@ This rule includes three configuration options: - `ignoreSingularizationList`: Words that are allowed to maintain their assumed plurality (e.g., "Fts") - `maxLength`: The maximum number of words allowed in the operation ID (default: 4) +##### Applies to +Each Get operation, when: + - any of: the path ends in a path parameter or is a singleton resource ([`path.isLegacyGetTarget`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathislegacygettarget)); the operation is declared the rule's method by `x-xgen-method-verb-override` ([`operation.hasRuleMethodVerbOverride`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#operationhasrulemethodverboverride)) + +Except when: + - the operation is declared a custom method by `x-xgen-method-verb-override` ([`operation.hasCustomMethodOverride`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#operationhascustommethodoverride)) + - the operation is declared a List method by `x-xgen-method-verb-override` ([`operation.hasListVerbOverride`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#operationhaslistverboverride)) + ### IPA-105 @@ -877,12 +918,15 @@ Singleton resources must not have a user-provided or system-generated ID. ##### Implementation details Rule checks for the following conditions: - - Applies only to singleton resources that are identified as resource collection identifiers - - Checks that the resource has a GET method defined - Examines all 2xx response schemas from the GET method - Verifies that no schema contains 'id' or '_id' properties in their object definitions - Fails if any response schema contains these identifier properties +##### Applies to +Each path item, when: + - the path is a singleton resource ([`resource.isSingleton`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceissingleton)) + - the path defines a Get method ([`pathItem.hasGet`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathitemhasget)) + #### xgen-IPA-113-singleton-must-not-have-delete-method ![error](https://img.shields.io/badge/error-red) @@ -890,9 +934,12 @@ Singleton resources must not define the Delete standard method. ##### Implementation details Rule checks for the following conditions: - - Applies only to singleton resources - Checks that the resource does not have a DELETE method defined +##### Applies to +Each path item, when: + - the path is a singleton resource ([`resource.isSingleton`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceissingleton)) + #### xgen-IPA-113-singleton-should-have-update-method ![error](https://img.shields.io/badge/error-red) @@ -900,10 +947,15 @@ Singleton resources should define the Update method. Validation for the presence ##### Implementation details Rule checks for the following conditions: - - Applies only to singleton resources - - Excludes read-only singleton resources (where all properties in the GET response schema are marked as readOnly; for List responses, all properties in the items schema must be readOnly) - Checks that the resource has the PUT and/or PATCH methods defined +##### Applies to +Each path item, when: + - the path is a singleton resource ([`resource.isSingleton`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceissingleton)) + +Except when: + - the resource is read-only ([`resource.isReadOnly`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceisreadonly)) + #### xgen-IPA-113-reset-method-must-use-POST ![error](https://img.shields.io/badge/error-red) @@ -911,11 +963,14 @@ The :reset custom method must use the POST HTTP method. ##### Implementation details Rule checks for the following conditions: - - Applies only to paths ending with :reset - Verifies that only POST method is defined - Fails if GET or any other HTTP method is used - Fails if multiple HTTP methods are defined for the same :reset endpoint +##### Applies to +Each path item, when: + - the path is a `:reset` custom method ([`path.isResetMethod`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisresetmethod)) + #### xgen-IPA-113-reset-method-must-not-have-request-body ![error](https://img.shields.io/badge/error-red) @@ -923,10 +978,13 @@ The :reset custom method must not have a request body. ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - Verifies that the operation object does not contain a requestBody property - Fails if any request body is defined +##### Applies to +Each POST operation, when: + - the path is a `:reset` custom method ([`path.isResetMethod`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisresetmethod)) + #### xgen-IPA-113-reset-method-must-return-200-OK ![error](https://img.shields.io/badge/error-red) @@ -934,11 +992,14 @@ The :reset custom method must return a 200 OK response with the reset resource i ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - Verifies that a 200 OK response code is present - Fails if the method lacks a 200 OK response or defines a different 2xx status code - Verifies that the 200 response has a response body with schema +##### Applies to +Each POST operation, when: + - the path is a `:reset` custom method ([`path.isResetMethod`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisresetmethod)) + #### xgen-IPA-113-reset-method-response-is-get-method-response ![error](https://img.shields.io/badge/error-red) @@ -946,12 +1007,17 @@ The :reset custom method response must match the GET method response schema. ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - - Applies only to JSON response content types - Verifies that both :reset and GET methods have schema references - Confirms that the :reset method 200 response schema reference matches the GET method response schema reference - Ensures the reset resource returned is the same type as the singleton resource +##### Applies to +Each media type of each POST 200 response, when: + - the media type is JSON ([`mediaType.isJson`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypeisjson)) + - the path is a `:reset` custom method ([`path.isResetMethod`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisresetmethod)) + - the media type defines a schema ([`mediaType.hasSchema`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypehasschema)) + - the custom method's resource has a Get response for the same media type ([`mediaType.baseGetHasMediaType`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#mediatypebasegethasmediatype)) + #### xgen-IPA-113-reset-method-only-on-singleton-resources ![error](https://img.shields.io/badge/error-red) @@ -959,11 +1025,13 @@ The :reset custom method must only be defined on singleton resources. ##### Implementation details Rule checks for the following conditions: - - Applies only to paths ending with :reset - - Verifies that the parent path (without :reset) is a singleton resource - - Uses existing isSingletonResource() helper function + - Verifies that the base resource (the path without :reset) is a singleton resource (see [`resource.isSingleton`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceissingleton)) - Fails if :reset is defined on a non-singleton resource +##### Applies to +Each path item, when: + - the path is a `:reset` custom method ([`path.isResetMethod`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisresetmethod)) + #### xgen-IPA-113-reset-method-not-on-readonly-singleton ![error](https://img.shields.io/badge/error-red) @@ -971,10 +1039,12 @@ Read-only singleton resources must not define a :reset custom method. ##### Implementation details Rule checks for the following conditions: - - Applies only to paths ending with :reset - - Verifies that the parent singleton resource is not read-only - - Uses existing isReadOnlyResource() helper function - - Fails if the singleton resource has all properties marked as readOnly: true + - Verifies that the base resource (the path without :reset) is not read-only (see [`resource.isReadOnly`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#resourceisreadonly)) + - Does not check that the base resource is a singleton; that is covered by xgen-IPA-113-reset-method-only-on-singleton-resources + +##### Applies to +Each path item, when: + - the path is a `:reset` custom method ([`path.isResetMethod`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisresetmethod)) #### xgen-IPA-113-reset-method-valid-operation-id @@ -983,7 +1053,6 @@ The :reset custom method must have a valid operation ID. ##### Implementation details Rule checks for the following conditions: - - Applies only to POST methods on paths ending with :reset - Confirms that the operation ID follows the pattern: reset{ResourceName} - Uses existing operation ID validation infrastructure @@ -991,6 +1060,10 @@ Rule checks for the following conditions: This rule includes a configuration option: - `ignoreSingularizationList`: Words that are allowed to maintain their assumed plurality (e.g., "Fts") +##### Applies to +Each POST operation, when: + - the path is a `:reset` custom method ([`path.isResetMethod`](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md#pathisresetmethod)) + ### IPA-114 diff --git a/tools/spectral/ipa/rulesets/functions/IPA104EachResourceHasGetMethod.js b/tools/spectral/ipa/rulesets/functions/IPA104EachResourceHasGetMethod.js index 51f5673488..4a0fa67bb9 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104EachResourceHasGetMethod.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104EachResourceHasGetMethod.js @@ -1,47 +1,23 @@ -import { - getResourcePathItems, - hasGetMethod, - isResourceCollectionIdentifier, - isSingleResourceIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isCollection } from './utils/conditions/path.js'; +import { hasSingletonShape, instancePath } from './utils/conditions/resource.js'; +import { hasGetMethod } from './utils/resourceEvaluation.js'; const ERROR_MESSAGE = 'APIs must provide a get method for resources.'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - if (!isResourceCollectionIdentifier(input)) { - return; - } - - const oas = documentInventory.resolved; - - const errors = checkViolationsAndReturnErrors(oas.paths, input, path, ruleName); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, oas.paths[input], path); -}; - -function checkViolationsAndReturnErrors(oasPaths, input, path, ruleName) { - try { - const resourcePathItems = getResourcePathItems(input, oasPaths); - const resourcePaths = Object.keys(resourcePathItems); - - if (isSingletonResource(resourcePathItems)) { - if (!hasGetMethod(resourcePathItems[resourcePaths[0]])) { - return [{ path, message: ERROR_MESSAGE }]; - } - } else { - if (resourcePaths.length === 1) { - return [{ path, message: ERROR_MESSAGE }]; - } - const singleResourcePath = resourcePaths.find((p) => isSingleResourceIdentifier(p)); - if (!singleResourcePath || !hasGetMethod(resourcePathItems[singleResourcePath])) { - return [{ path, message: ERROR_MESSAGE }]; - } +export default defineRule({ + appliesTo: [isCollection], + checkUses: [hasSingletonShape], + exceptionTarget: (ctx) => ctx.pathItem, + check: (ctx) => { + const error = [{ path: ctx.path, message: ERROR_MESSAGE }]; + // A singleton has its Get on the first of its paths (in spec order), other resources on the instance path + if (ctx.is(hasSingletonShape)) { + const [firstPath] = Object.keys(ctx.resourcePathItems); + return hasGetMethod(ctx.resourcePathItems[firstPath]) ? [] : error; } - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} + const instance = instancePath(ctx); + return instance && hasGetMethod(ctx.oas.paths[instance]) ? [] : error; + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodHasNoRequestBody.js b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodHasNoRequestBody.js index b35635e0df..8f0a62032a 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodHasNoRequestBody.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodHasNoRequestBody.js @@ -1,35 +1,11 @@ -import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js'; -import { - getResourcePathItems, - isResourceCollectionIdentifier, - isSingleResourceIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isGetTarget } from './utils/conditions/resource.js'; const ERROR_MESSAGE = 'The Get method must not include a request body.'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const resourcePath = path[1]; - const oas = documentInventory.resolved; - - if ( - !isSingleResourceIdentifier(resourcePath) && - !( - isResourceCollectionIdentifier(resourcePath) && isSingletonResource(getResourcePathItems(resourcePath, oas.paths)) - ) - ) { - return; - } - - const errors = checkViolationsAndReturnErrors(input, path); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(getOperationObject, path) { - if (getOperationObject.requestBody) { - return [{ path, message: ERROR_MESSAGE }]; - } - return []; -} +export default defineRule({ + appliesTo: [isGetTarget], + onInternalError: 'throw', + check: ({ input, path }) => (input.requestBody ? [{ path, message: ERROR_MESSAGE }] : []), +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodResponseHasNoInputFields.js b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodResponseHasNoInputFields.js index 02f85faf3b..d55149f341 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodResponseHasNoInputFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodResponseHasNoInputFields.js @@ -1,41 +1,15 @@ -import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js'; -import { - getResourcePathItems, - isResourceCollectionIdentifier, - isSingleResourceIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; -import { resolveObject } from './utils/componentUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isGetTarget } from './utils/conditions/resource.js'; +import { hasSchema, is2xx, isJson } from './utils/conditions/response.js'; import { checkForbiddenPropertyAttributesAndReturnErrors } from './utils/validations/checkForbiddenPropertyAttributesAndReturnErrors.js'; const ERROR_MESSAGE = 'The get method response object must not include output fields (writeOnly properties).'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const resourcePath = path[1]; - const responseCode = path[4]; - const oas = documentInventory.resolved; - const resourcePaths = getResourcePathItems(resourcePath, oas.paths); - const contentPerMediaType = resolveObject(oas, path); - - if ( - !responseCode.startsWith('2') || - !contentPerMediaType || - !contentPerMediaType.schema || - !input.endsWith('json') || - (!isSingleResourceIdentifier(resourcePath) && - !(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePaths))) - ) { - return; - } - - const errors = checkForbiddenPropertyAttributesAndReturnErrors( - contentPerMediaType.schema, - 'writeOnly', - path, - [], - ERROR_MESSAGE - ); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, contentPerMediaType, path); -}; +export default defineRule({ + appliesTo: [is2xx, hasSchema, isJson, isGetTarget], + onInternalError: 'throw', + exceptionTarget: (ctx) => ctx.node, + check: ({ node, path }) => + checkForbiddenPropertyAttributesAndReturnErrors(node.schema, 'writeOnly', path, [], ERROR_MESSAGE), +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsResponseSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsResponseSuffixedObject.js index 8303011dfd..d48f2a71a8 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsResponseSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsResponseSuffixedObject.js @@ -1,33 +1,14 @@ -import { - getResourcePathItems, - isResourceCollectionIdentifier, - isSingleResourceIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; -import { resolveObject } from './utils/componentUtils.js'; -import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isGetTarget } from './utils/conditions/resource.js'; +import { hasSchema, is2xx, isJson } from './utils/conditions/response.js'; import { checkSchemaRefSuffixAndReturnErrors } from './utils/validations/checkSchemaRefSuffixAndReturnErrors.js'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const resourcePath = path[1]; - const responseCode = path[4]; - const oas = documentInventory.unresolved; - const resourcePaths = getResourcePathItems(resourcePath, oas.paths); - const contentPerMediaType = resolveObject(oas, path); - - if ( - !responseCode.startsWith('2') || - !contentPerMediaType || - !contentPerMediaType.schema || - !input.endsWith('json') || - (!isSingleResourceIdentifier(resourcePath) && - !(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePaths))) - ) { - return; - } - - const errors = checkSchemaRefSuffixAndReturnErrors(path, contentPerMediaType, 'Response', ruleName); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, contentPerMediaType, path); -}; +export default defineRule({ + // Reads the unresolved document so the schema $ref name can be checked + document: 'unresolved', + appliesTo: [is2xx, hasSchema, isJson, isGetTarget], + onInternalError: 'throw', + exceptionTarget: (ctx) => ctx.node, + check: ({ node, path, ruleName }) => checkSchemaRefSuffixAndReturnErrors(path, node, 'Response', ruleName), +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsSingleResource.js b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsSingleResource.js index ef092decb3..43a2f30eb8 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsSingleResource.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104GetMethodReturnsSingleResource.js @@ -1,57 +1,28 @@ -import { - getResourcePathItems, - isResourceCollectionIdentifier, - isSingleResourceIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isGetTarget, isSingleton } from './utils/conditions/resource.js'; +import { hasSchema, is2xx, isJson } from './utils/conditions/response.js'; import { schemaIsArray, schemaIsPaginated } from './utils/schemaUtils.js'; -import { resolveObject } from './utils/componentUtils.js'; const ERROR_MESSAGE_STANDARD_RESOURCE = 'Get methods should return data for a single resource. This method returns an array or a paginated response.'; const ERROR_MESSAGE_SINGLETON_RESOURCE = 'Get methods for singleton resource should return data for a single resource. This method returns an array or a paginated response. If this is not a singleton resource, please implement all standard methods.'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const oas = documentInventory.resolved; - const resourcePath = path[1]; - const responseCode = path[4]; - const resourcePaths = getResourcePathItems(resourcePath, oas.paths); - const contentPerMediaType = resolveObject(oas, path); - - const isSingleResource = isSingleResourceIdentifier(resourcePath); - const isSingleton = isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePaths); - - if ( - !responseCode.startsWith('2') || - !contentPerMediaType || - !contentPerMediaType.schema || - !input.endsWith('json') || - (!isSingleResource && !isSingleton) - ) { - return; - } - - const errors = checkViolationsAndReturnErrors(contentPerMediaType, path, isSingleton, ruleName); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, contentPerMediaType, path); -}; - -function checkViolationsAndReturnErrors(contentPerMediaType, path, isSingleton, ruleName) { - try { - const schema = contentPerMediaType.schema; +export default defineRule({ + appliesTo: [is2xx, hasSchema, isJson, isGetTarget], + checkUses: [isSingleton], + exceptionTarget: (ctx) => ctx.node, + check: (ctx) => { + const schema = ctx.node.schema; if (schemaIsPaginated(schema) || schemaIsArray(schema)) { return [ { - path, - message: isSingleton ? ERROR_MESSAGE_SINGLETON_RESOURCE : ERROR_MESSAGE_STANDARD_RESOURCE, + path: ctx.path, + message: ctx.is(isSingleton) ? ERROR_MESSAGE_SINGLETON_RESOURCE : ERROR_MESSAGE_STANDARD_RESOURCE, }, ]; } return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA104GetResponseCodeShouldBe200OK.js b/tools/spectral/ipa/rulesets/functions/IPA104GetResponseCodeShouldBe200OK.js index a4a94231bd..4d526218a9 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104GetResponseCodeShouldBe200OK.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104GetResponseCodeShouldBe200OK.js @@ -1,29 +1,13 @@ -import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js'; -import { - getResourcePathItems, - isResourceCollectionIdentifier, - isSingleResourceIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isGetTarget } from './utils/conditions/resource.js'; import { checkResponseCodeAndReturnErrors } from './utils/validations/checkResponseCodeAndReturnErrors.js'; const ERROR_MESSAGE = 'The Get method must return a 200 OK response. This method either lacks a 200 OK response or defines a different 2xx status code.'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const resourcePath = path[1]; - const oas = documentInventory.resolved; - const resourcePaths = getResourcePathItems(resourcePath, oas.paths); - - if ( - !isSingleResourceIdentifier(resourcePath) && - !(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePaths)) - ) { - return; - } - - const errors = checkResponseCodeAndReturnErrors(input, '200', path, ruleName, ERROR_MESSAGE); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; +export default defineRule({ + appliesTo: [isGetTarget], + onInternalError: 'throw', + check: ({ input, path, ruleName }) => checkResponseCodeAndReturnErrors(input, '200', path, ruleName, ERROR_MESSAGE), +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA104OperationIdLength.js b/tools/spectral/ipa/rulesets/functions/IPA104OperationIdLength.js index 3c380691df..bf36b04052 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104OperationIdLength.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104OperationIdLength.js @@ -1,33 +1,5 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getResourcePathItems } from './utils/resourceEvaluation.js'; -import { hasCustomMethodOverride, hasMethodVerbOverride, VERB_OVERRIDE_EXTENSION } from './utils/extensions.js'; -import { isInvalidGetMethod } from './utils/methodLogic.js'; -import { validateOperationIdLengthAndReturnErrors } from './utils/validations/validateOperationIdAndReturnErrors.js'; -import { generateOperationID } from './utils/operationIdGeneration.js'; +// @ts-check +import { getOperationIdRuleConditions } from './utils/conditions/operationIdRules.js'; +import { operationIdLengthRule } from './utils/operationIdRules.js'; -export default (input, { methodName, ignoreSingularizationList, maxLength }, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const resourcePath = path[1]; - const oas = documentInventory.resolved; - const resourcePaths = getResourcePathItems(resourcePath, oas.paths); - - if ( - hasCustomMethodOverride(input) || - hasMethodVerbOverride(input, 'list') || - (isInvalidGetMethod(resourcePath, resourcePaths) && !hasMethodVerbOverride(input, methodName)) - ) { - return; - } - - try { - if (hasMethodVerbOverride(input, methodName)) { - methodName = input[VERB_OVERRIDE_EXTENSION].verb; - } - const expectedOperationId = generateOperationID(methodName, resourcePath, ignoreSingularizationList); - const errors = validateOperationIdLengthAndReturnErrors(input, path, expectedOperationId, maxLength); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); - } catch (e) { - return handleInternalError(ruleName, path, e); - } -}; +export default operationIdLengthRule(getOperationIdRuleConditions()); diff --git a/tools/spectral/ipa/rulesets/functions/IPA104ValidOperationID.js b/tools/spectral/ipa/rulesets/functions/IPA104ValidOperationID.js index 08dda68753..c23d067afe 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA104ValidOperationID.js +++ b/tools/spectral/ipa/rulesets/functions/IPA104ValidOperationID.js @@ -1,31 +1,5 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getResourcePathItems } from './utils/resourceEvaluation.js'; -import { hasCustomMethodOverride, hasMethodVerbOverride, VERB_OVERRIDE_EXTENSION } from './utils/extensions.js'; -import { isInvalidGetMethod } from './utils/methodLogic.js'; -import { validateOperationIdAndReturnErrors } from './utils/validations/validateOperationIdAndReturnErrors.js'; +// @ts-check +import { getOperationIdRuleConditions } from './utils/conditions/operationIdRules.js'; +import { validOperationIdRule } from './utils/operationIdRules.js'; -export default (input, { methodName, ignoreSingularizationList }, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const resourcePath = path[1]; - const oas = documentInventory.resolved; - const resourcePaths = getResourcePathItems(resourcePath, oas.paths); - - if ( - hasCustomMethodOverride(input) || - hasMethodVerbOverride(input, 'list') || - (isInvalidGetMethod(resourcePath, resourcePaths) && !hasMethodVerbOverride(input, methodName)) - ) { - return; - } - - try { - if (hasMethodVerbOverride(input, methodName)) { - methodName = input[VERB_OVERRIDE_EXTENSION].verb; - } - const errors = validateOperationIdAndReturnErrors(methodName, resourcePath, input, path, ignoreSingularizationList); - - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); - } catch (e) { - return handleInternalError(ruleName, path, e); - } -}; +export default validOperationIdRule(getOperationIdRuleConditions()); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustNotHaveRequestBody.js b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustNotHaveRequestBody.js index 57e03c66a2..ad3632d321 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustNotHaveRequestBody.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustNotHaveRequestBody.js @@ -1,27 +1,10 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { isResetMethod } from './utils/resourceEvaluation.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isReset } from './utils/conditions/path.js'; const ERROR_MESSAGE = 'The :reset custom method must not have a request body.'; -export default (input, _, { path, rule }) => { - const ruleName = rule.name; - const pathString = path[1]; // e.g., ['paths', '/resource/{id}/singleton:reset', 'post'] - - if (!isResetMethod(pathString)) { - return; - } - - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(input, path, ruleName) { - try { - if (input.requestBody) { - return [{ path, message: ERROR_MESSAGE }]; - } - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} +export default defineRule({ + appliesTo: [isReset], + check: ({ input, path }) => (input.requestBody ? [{ path, message: ERROR_MESSAGE }] : []), +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustReturn200OK.js b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustReturn200OK.js index 4cac9b7c03..49b91ea1cd 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustReturn200OK.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustReturn200OK.js @@ -1,61 +1,30 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { isResetMethod } from './utils/resourceEvaluation.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isReset } from './utils/conditions/path.js'; import { checkResponseCodeAndReturnErrors } from './utils/validations/checkResponseCodeAndReturnErrors.js'; const ERROR_MESSAGE = 'The :reset custom method must return a 200 OK response with the reset resource in the response body. This method either lacks a 200 OK response or defines a different 2xx status code.'; +const MISSING_BODY_ERROR_MESSAGE = + 'The :reset custom method must return a 200 OK response with the reset resource in the response body.'; -export default (input, _, { path, rule }) => { - const ruleName = rule.name; - const pathString = path[1]; - - if (!isResetMethod(pathString)) { - return; - } - - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(input, path, ruleName) { - try { +export default defineRule({ + appliesTo: [isReset], + check: ({ input, path, ruleName }) => { // First check if 200 OK response exists const responseCodeErrors = checkResponseCodeAndReturnErrors(input, '200', path, ruleName, ERROR_MESSAGE); if (responseCodeErrors.length > 0) { return responseCodeErrors; } - // Check if the 200 response has a response body with schema + // Check that the 200 response has a body, and at least one content type has a schema const response200 = input.responses['200']; - if (!response200 || !response200.content) { - return [ - { - path, - message: - 'The :reset custom method must return a 200 OK response with the reset resource in the response body.', - }, - ]; - } - - // Verify that at least one content type has a schema - const contentTypes = Object.keys(response200.content); - const hasSchema = contentTypes.some((contentType) => { - const mediaTypeObj = response200.content[contentType]; - return mediaTypeObj && mediaTypeObj.schema; - }); - + const hasSchema = + response200?.content && + Object.values(response200.content).some((mediaTypeObject) => mediaTypeObject && mediaTypeObject.schema); if (!hasSchema) { - return [ - { - path, - message: - 'The :reset custom method must return a 200 OK response with the reset resource in the response body.', - }, - ]; + return [{ path, message: MISSING_BODY_ERROR_MESSAGE }]; } - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustUsePost.js b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustUsePost.js index 1169a9425d..26f54dd88c 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustUsePost.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodMustUsePost.js @@ -1,41 +1,19 @@ -import { isResetMethod } from './utils/resourceEvaluation.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isReset } from './utils/conditions/path.js'; const ERROR_MESSAGE = 'The :reset custom method must use the POST HTTP method.'; const VALID_METHOD = 'post'; const HTTP_METHODS = ['get', 'put', 'post', 'delete', 'options', 'head', 'patch', 'trace']; -export default (input, _, { path, rule }) => { - const ruleName = rule.name; - // Extract the path key (e.g., '/resource/{id}/singleton:reset') from the JSONPath. - let pathKey = path[1]; - - if (!isResetMethod(pathKey)) { - return; - } - - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(input, path, ruleName) { - try { - // Extract the keys which are equivalent of the http methods - let keys = Object.keys(input); - const httpMethods = keys.filter((key) => HTTP_METHODS.includes(key)); - - // Check if POST is not used or if other methods are used - if (!httpMethods.includes(VALID_METHOD)) { +export default defineRule({ + appliesTo: [isReset], + check: ({ input, path }) => { + // POST must be the only HTTP method defined on the path + const httpMethods = Object.keys(input).filter((key) => HTTP_METHODS.includes(key)); + if (!httpMethods.includes(VALID_METHOD) || httpMethods.length > 1) { return [{ path, message: ERROR_MESSAGE }]; } - - // Check for multiple methods (only POST should be defined) - if (httpMethods.length > 1) { - return [{ path, message: ERROR_MESSAGE }]; - } - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodNotOnReadonlySingleton.js b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodNotOnReadonlySingleton.js index 7b1a9b00af..652dcf14a7 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodNotOnReadonlySingleton.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodNotOnReadonlySingleton.js @@ -1,38 +1,18 @@ -import { - getResourcePathItems, - isReadOnlyResource, - isResetMethod, - stripCustomMethodName, -} from './utils/resourceEvaluation.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isReset } from './utils/conditions/path.js'; +import { isReadOnly } from './utils/conditions/resource.js'; +import { stripCustomMethodName } from './utils/resourceEvaluation.js'; const ERROR_MESSAGE = 'Read-only singleton resources must not define a :reset custom method. Read-only resources cannot be modified, so reset is not applicable.'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const oas = documentInventory.resolved; - const resourcePath = path[1]; - - if (!isResetMethod(resourcePath)) { - return; - } - - const errors = checkViolationsAndReturnErrors(resourcePath, oas, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(resourcePath, oas, path, ruleName) { - try { - const baseResourcePath = stripCustomMethodName(resourcePath); - const resourcePathItems = getResourcePathItems(baseResourcePath, oas.paths); - - if (isReadOnlyResource(resourcePathItems)) { - return [{ path, message: ERROR_MESSAGE }]; - } - - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} +// Despite the rule name, the base resource is not required to be a singleton: any read-only resource with :reset violates +export default defineRule({ + appliesTo: [isReset], + checkUses: [isReadOnly], + check: (ctx) => { + const baseResource = ctx.atPath(stripCustomMethodName(ctx.resourcePath)); + return baseResource.is(isReadOnly) ? [{ path: ctx.path, message: ERROR_MESSAGE }] : []; + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodOnlyOnSingletonResources.js b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodOnlyOnSingletonResources.js index dce0a08dd7..814fb5aa04 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodOnlyOnSingletonResources.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodOnlyOnSingletonResources.js @@ -1,40 +1,16 @@ -import { - getResourcePathItems, - isResetMethod, - isResourceCollectionIdentifier, - isSingletonResource, - stripCustomMethodName, -} from './utils/resourceEvaluation.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isReset } from './utils/conditions/path.js'; +import { isSingleton } from './utils/conditions/resource.js'; +import { stripCustomMethodName } from './utils/resourceEvaluation.js'; const ERROR_MESSAGE = 'The :reset custom method must only be defined on singleton resources.'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const oas = documentInventory.resolved; - const resourcePath = path[1]; - - if (!isResetMethod(resourcePath)) { - return; - } - - const errors = checkViolationsAndReturnErrors(resourcePath, oas, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(resourcePath, oas, path, ruleName) { - try { - // Get the base resource path (without :reset) - const baseResourcePath = stripCustomMethodName(resourcePath); - const resourcePathItems = getResourcePathItems(baseResourcePath, oas.paths); - - // Check if the base resource is a singleton - if (!(isResourceCollectionIdentifier(baseResourcePath) && isSingletonResource(resourcePathItems))) { - return [{ path, message: ERROR_MESSAGE }]; - } - - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} +export default defineRule({ + appliesTo: [isReset], + checkUses: [isSingleton], + check: (ctx) => { + const baseResource = ctx.atPath(stripCustomMethodName(ctx.resourcePath)); + return baseResource.is(isSingleton) ? [] : [{ path: ctx.path, message: ERROR_MESSAGE }]; + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodResponseIsGetMethodResponse.js b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodResponseIsGetMethodResponse.js index f2243a5526..c0f85b2dbf 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodResponseIsGetMethodResponse.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodResponseIsGetMethodResponse.js @@ -1,54 +1,21 @@ -import { getResourcePathItems, isResetMethod, stripCustomMethodName } from './utils/resourceEvaluation.js'; -import { resolveObject } from './utils/componentUtils.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getGETMethodResponseSchemaFromPathItem, getSchemaRef } from './utils/methodUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isReset } from './utils/conditions/path.js'; +import { baseGetHasMediaType, baseGetResponse } from './utils/conditions/resource.js'; +import { hasSchema, isJson } from './utils/conditions/response.js'; +import { getSchemaRef } from './utils/methodUtils.js'; const ERROR_MESSAGE = 'The schema in the :reset method response must be the same schema as the response of the Get method.'; -export default (input, _, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const oas = documentInventory.unresolved; - const resourcePath = path[1]; - const mediaType = input; +export default defineRule({ + document: 'unresolved', + appliesTo: [isJson, isReset, hasSchema, baseGetHasMediaType], + exceptionTarget: (ctx) => ctx.node, + check: (ctx) => { + const path = ctx.path; + const getMethodResponseContent = baseGetResponse(ctx); - if (!mediaType.endsWith('json') || !isResetMethod(resourcePath)) { - return; - } - - // Ignore if the Reset method does not have a response schema - const resetMethodResponse = resolveObject(oas, path); - - if (!resetMethodResponse || !resetMethodResponse.schema) { - return; - } - - // Get the base resource path (without :reset) - const baseResourcePath = stripCustomMethodName(resourcePath); - const resourcePathItems = getResourcePathItems(baseResourcePath, oas.paths); - - // Ignore if there is no matching Get method on the base resource - const basePathItem = resourcePathItems[baseResourcePath]; - if (!basePathItem) { - return; - } - - const getMethodResponseContentPerMediaType = getGETMethodResponseSchemaFromPathItem(basePathItem, mediaType); - if (!getMethodResponseContentPerMediaType) { - return; - } - - const errors = checkViolationsAndReturnErrors( - path, - resetMethodResponse, - getMethodResponseContentPerMediaType, - ruleName - ); - return evaluateAndCollectAdoptionStatus(errors, ruleName, resetMethodResponse, path); -}; - -function checkViolationsAndReturnErrors(path, resetMethodResponseContent, getMethodResponseContent, ruleName) { - try { // Error if the Get method does not have a schema if (!getMethodResponseContent.schema) { return [ @@ -59,7 +26,7 @@ function checkViolationsAndReturnErrors(path, resetMethodResponseContent, getMet ]; } - const resetMethodSchemaRef = getSchemaRef(resetMethodResponseContent.schema); + const resetMethodSchemaRef = getSchemaRef(ctx.node.schema); const getMethodSchemaRef = getSchemaRef(getMethodResponseContent.schema); // Error if the Get method does not have a schema ref @@ -77,7 +44,5 @@ function checkViolationsAndReturnErrors(path, resetMethodResponseContent, getMet return [{ path, message: ERROR_MESSAGE }]; } return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodValidOperationID.js b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodValidOperationID.js index 26c3e2e39e..567635eacd 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodValidOperationID.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113ResetMethodValidOperationID.js @@ -1,21 +1,18 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { isResetMethod, stripCustomMethodName } from './utils/resourceEvaluation.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isReset } from './utils/conditions/path.js'; +import { stripCustomMethodName } from './utils/resourceEvaluation.js'; import { validateOperationIdAndReturnErrors } from './utils/validations/validateOperationIdAndReturnErrors.js'; -export default (input, { ignoreSingularizationList }, { path, rule }) => { - const ruleName = rule.name; - const resourcePath = path[1]; - - if (!isResetMethod(resourcePath)) { - return; - } - - try { - const methodName = 'reset'; - const endpointUrl = stripCustomMethodName(resourcePath); - const errors = validateOperationIdAndReturnErrors(methodName, endpointUrl, input, path, ignoreSingularizationList); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); - } catch (e) { - return handleInternalError(ruleName, path, e); - } -}; +export default defineRule({ + appliesTo: [isReset], + onInternalError: 'return', + check: ({ input, options, path, resourcePath }) => + validateOperationIdAndReturnErrors( + 'reset', + stripCustomMethodName(resourcePath), + input, + path, + options.ignoreSingularizationList + ), +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoDeleteMethod.js b/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoDeleteMethod.js index e0e30dc7ef..460db969a6 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoDeleteMethod.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoDeleteMethod.js @@ -1,35 +1,12 @@ -import { - getResourcePathItems, - hasDeleteMethod, - isResourceCollectionIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isSingleton } from './utils/conditions/resource.js'; +import { hasDeleteMethod } from './utils/resourceEvaluation.js'; const ERROR_MESSAGE = 'Singleton resources must not define the Delete standard method. If this is not a singleton resource, please implement all CRUDL methods.'; -export default (input, opts, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const oas = documentInventory.resolved; - const resourcePath = path[1]; - const resourcePathItems = getResourcePathItems(resourcePath, oas.paths); - - if (!(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePathItems))) { - return; - } - - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(input, path, ruleName) { - try { - if (hasDeleteMethod(input)) { - return [{ path, message: ERROR_MESSAGE }]; - } - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} +export default defineRule({ + appliesTo: [isSingleton], + check: ({ input, path }) => (hasDeleteMethod(input) ? [{ path, message: ERROR_MESSAGE }] : []), +}); diff --git a/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoId.js b/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoId.js index 91b902f35b..7577a946ee 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoId.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasNoId.js @@ -1,31 +1,18 @@ -import { - getResourcePathItems, - hasGetMethod, - isResourceCollectionIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { isSingleton } from './utils/conditions/resource.js'; +import { hasGet } from './utils/conditions/operation.js'; import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; const ERROR_MESSAGE = 'Singleton resources must not have a user-provided or system-generated ID.'; -export default (input, opts, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const oas = documentInventory.resolved; - const resourcePath = path[1]; - const resourcePathItems = getResourcePathItems(resourcePath, oas.paths); - - if (!(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePathItems))) { - return; - } - - if (!hasGetMethod(input)) { - return; - } - - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; +export default defineRule({ + appliesTo: [isSingleton, hasGet], + check: ({ input, path }) => { + const resourceSchemas = getAllSuccessfulResponseSchemas(input['get']); + return resourceSchemas.some(({ schema }) => schemaHasIdProperty(schema)) ? [{ path, message: ERROR_MESSAGE }] : []; + }, +}); function schemaHasIdProperty(schema) { if (Object.keys(schema).includes('properties')) { @@ -34,15 +21,3 @@ function schemaHasIdProperty(schema) { } return false; } - -function checkViolationsAndReturnErrors(input, path, ruleName) { - try { - const resourceSchemas = getAllSuccessfulResponseSchemas(input['get']); - if (resourceSchemas.some(({ schema }) => schemaHasIdProperty(schema))) { - return [{ path, message: ERROR_MESSAGE }]; - } - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} diff --git a/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasUpdateMethod.js b/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasUpdateMethod.js index 889a29cb42..ef023a71c4 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasUpdateMethod.js +++ b/tools/spectral/ipa/rulesets/functions/IPA113SingletonHasUpdateMethod.js @@ -1,41 +1,13 @@ -import { - getResourcePathItems, - hasPatchMethod, - hasPutMethod, - isReadOnlyResource, - isResourceCollectionIdentifier, - isSingletonResource, -} from './utils/resourceEvaluation.js'; -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +// @ts-check +import { defineRule } from './utils/defineRule.js'; +import { not } from './utils/conditions/registry.js'; +import { isReadOnly, isSingleton } from './utils/conditions/resource.js'; +import { hasPatchMethod, hasPutMethod } from './utils/resourceEvaluation.js'; const ERROR_MESSAGE = 'Singleton resources should define the Update method. If this is not a singleton resource, please implement all CRUDL methods.'; -export default (input, opts, { path, documentInventory, rule }) => { - const ruleName = rule.name; - const oas = documentInventory.resolved; - const resourcePath = path[1]; - const resourcePathItems = getResourcePathItems(resourcePath, oas.paths); - - if (!isResourceCollectionIdentifier(resourcePath) || !isSingletonResource(resourcePathItems)) { - return; - } - - if (isReadOnlyResource(resourcePathItems)) { - return; - } - - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(input, path, ruleName) { - try { - if (!(hasPutMethod(input) || hasPatchMethod(input))) { - return [{ path, message: ERROR_MESSAGE }]; - } - return []; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} +export default defineRule({ + appliesTo: [isSingleton, not(isReadOnly)], + check: ({ input, path }) => (hasPutMethod(input) || hasPatchMethod(input) ? [] : [{ path, message: ERROR_MESSAGE }]), +}); diff --git a/tools/spectral/ipa/rulesets/functions/utils/conditions/operation.js b/tools/spectral/ipa/rulesets/functions/utils/conditions/operation.js new file mode 100644 index 0000000000..df75af50f6 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/conditions/operation.js @@ -0,0 +1,101 @@ +// @ts-check +// Conditions on a path item (ctx.pathItem) or an operation (ctx.operation) and its extensions. + +import { defineCondition } from './registry.js'; +import { hasGetMethod } from '../resourceEvaluation.js'; +import { hasCustomMethodOverride as hasCustomMethodOverrideExtension, hasMethodVerbOverride } from '../extensions.js'; + +const withOperation = (operation, note, options = {}) => ({ + path: ['paths', '/groups/{groupId}', 'get'], + paths: { '/groups/{groupId}': { get: operation } }, + options, + note, +}); + +export const hasGet = defineCondition({ + id: 'pathItem.hasGet', + label: 'the path defines a Get method', + subject: 'pathItem', + definition: 'The path item defines a `get` operation.', + fn: (ctx) => hasGetMethod(ctx.pathItem), + examples: { + match: [ + { + path: ['paths', '/groups/{groupId}/settings'], + paths: { '/groups/{groupId}/settings': { get: {} } }, + note: 'defines get', + }, + ], + noMatch: [ + { + path: ['paths', '/groups/{groupId}/settings'], + paths: { '/groups/{groupId}/settings': { patch: {} } }, + note: 'patch only', + }, + ], + }, +}); + +export const hasCustomMethodOverride = defineCondition({ + id: 'operation.hasCustomMethodOverride', + label: 'the operation is declared a custom method by `x-xgen-method-verb-override`', + subject: 'operation', + definition: + 'The operation has an `x-xgen-method-verb-override` extension with `customMethod: true`, declaring a legacy custom method that has no `:` in its path.', + fn: (ctx) => hasCustomMethodOverrideExtension(ctx.operation), + examples: { + match: [ + withOperation( + { 'x-xgen-method-verb-override': { verb: 'search', customMethod: true } }, + 'verb override `search` with customMethod: true' + ), + ], + noMatch: [ + withOperation({}, 'no verb override'), + withOperation( + { 'x-xgen-method-verb-override': { verb: 'get', customMethod: false } }, + 'verb override `get` with customMethod: false' + ), + ], + }, +}); + +export const hasListVerbOverride = defineCondition({ + id: 'operation.hasListVerbOverride', + label: 'the operation is declared a List method by `x-xgen-method-verb-override`', + subject: 'operation', + definition: + 'The operation has an `x-xgen-method-verb-override` extension whose verb starts with `list`, declaring it a List method.', + fn: (ctx) => hasMethodVerbOverride(ctx.operation, 'list'), + examples: { + match: [ + withOperation( + { 'x-xgen-method-verb-override': { verb: 'listByGroup', customMethod: false } }, + 'verb override `listByGroup`' + ), + ], + noMatch: [ + withOperation({ 'x-xgen-method-verb-override': { verb: 'get', customMethod: false } }, 'verb override `get`'), + ], + }, +}); + +export const hasRuleMethodVerbOverride = defineCondition({ + id: 'operation.hasRuleMethodVerbOverride', + label: "the operation is declared the rule's method by `x-xgen-method-verb-override`", + subject: 'operation', + definition: + "The operation has an `x-xgen-method-verb-override` extension whose verb starts with the rule's `methodName` option (for IPA-104, `get`), declaring it that standard method.", + readsRuleOptions: true, + fn: (ctx) => hasMethodVerbOverride(ctx.operation, ctx.options.methodName), + examples: { + match: [ + withOperation( + { 'x-xgen-method-verb-override': { verb: 'get', customMethod: false } }, + 'verb override `get`, rule methodName `get`', + { methodName: 'get' } + ), + ], + noMatch: [withOperation({}, 'no verb override, rule methodName `get`', { methodName: 'get' })], + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/utils/conditions/operationIdRules.js b/tools/spectral/ipa/rulesets/functions/utils/conditions/operationIdRules.js new file mode 100644 index 0000000000..fea13793ec --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/conditions/operationIdRules.js @@ -0,0 +1,25 @@ +// @ts-check +// Conditions that decide which operations the operation ID rules of the standard methods apply to (see +// utils/operationIdRules.js for the rules). + +import { anyOf, not } from './registry.js'; +import { hasCustomMethodOverride, hasListVerbOverride, hasRuleMethodVerbOverride } from './operation.js'; +import { isLegacyGetTarget } from './resource.js'; +import { hasMethodVerbOverride, VERB_OVERRIDE_EXTENSION } from '../extensions.js'; + +/** + * @returns {Array} the operation is a Get: not a custom method or List by verb + * override, and either on a Get path or declared a Get by verb override + */ +export function getOperationIdRuleConditions() { + return [not(hasCustomMethodOverride), not(hasListVerbOverride), anyOf(isLegacyGetTarget, hasRuleMethodVerbOverride)]; +} + +/** + * @param {Object} operation + * @param {string} methodName the rule's methodName option + * @returns {string} the verb from the operation's verb override if it overrides methodName, otherwise methodName + */ +export function getMethodName(operation, methodName) { + return hasMethodVerbOverride(operation, methodName) ? operation[VERB_OVERRIDE_EXTENSION].verb : methodName; +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/conditions/path.js b/tools/spectral/ipa/rulesets/functions/utils/conditions/path.js new file mode 100644 index 0000000000..eb4ee25ca5 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/conditions/path.js @@ -0,0 +1,72 @@ +// @ts-check +// Conditions on the path string alone (ctx.resourcePath, e.g. '/api/atlas/v2/groups/{groupId}/clusters'). +// The /api/atlas/v2 and /api/atlas/v2/unauth prefixes are ignored. + +import { defineCondition } from './registry.js'; +import { + isCustomMethodIdentifier, + isResetMethod, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, +} from '../resourceEvaluation.js'; + +const at = (resourcePath) => ({ path: ['paths', resourcePath] }); + +export const isCollection = defineCondition({ + id: 'path.isCollection', + label: 'the path is a resource collection path', + subject: 'path', + definition: + 'The path ends in a resource name: either a single top-level segment (`/groups`) or a segment following a path parameter (`/groups/{groupId}/clusters`). This is the path of a resource collection or of a singleton resource.', + fn: (ctx) => isResourceCollectionIdentifier(ctx.resourcePath), + examples: { + match: [at('/api/atlas/v2/groups'), at('/api/atlas/v2/groups/{groupId}/clusters')], + noMatch: [ + at('/api/atlas/v2/groups/{groupId}'), + at('/api/atlas/v2/groups/clusters'), + at('/api/atlas/v2/groups:search'), + ], + }, +}); + +export const isInstance = defineCondition({ + id: 'path.isInstance', + label: 'the path is a single resource (instance) path', + subject: 'path', + definition: + 'The path is a collection path (see `path.isCollection`) followed by a single path parameter, e.g. `/groups/{groupId}`. This is the path of one resource in a collection.', + fn: (ctx) => isSingleResourceIdentifier(ctx.resourcePath), + examples: { + match: [at('/api/atlas/v2/groups/{groupId}'), at('/api/atlas/v2/groups/{groupId}/clusters/{clusterName}')], + noMatch: [ + at('/api/atlas/v2/groups'), + at('/api/atlas/v2/groups/{groupId}/clusters'), + at('/api/atlas/v2/groups/settings/{name}'), + ], + }, +}); + +export const isCustomMethod = defineCondition({ + id: 'path.isCustomMethod', + label: 'the path is a custom method', + subject: 'path', + definition: + 'The path contains `:`, e.g. `/groups/{groupId}:migrate`. Custom methods declared by the `x-xgen-method-verb-override` extension are not detected (see `operation.hasCustomMethodOverride`).', + fn: (ctx) => isCustomMethodIdentifier(ctx.resourcePath), + examples: { + match: [at('/api/atlas/v2/groups/{groupId}:migrate'), at('/api/atlas/v2/groups:search')], + noMatch: [at('/api/atlas/v2/groups/{groupId}')], + }, +}); + +export const isReset = defineCondition({ + id: 'path.isResetMethod', + label: 'the path is a `:reset` custom method', + subject: 'path', + definition: 'The path is a custom method named `reset`, e.g. `/groups/{groupId}/settings:reset`.', + fn: (ctx) => isResetMethod(ctx.resourcePath), + examples: { + match: [at('/api/atlas/v2/groups/{groupId}/settings:reset')], + noMatch: [at('/api/atlas/v2/groups/{groupId}/settings'), at('/api/atlas/v2/groups/{groupId}/settings:restart')], + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/utils/conditions/registry.js b/tools/spectral/ipa/rulesets/functions/utils/conditions/registry.js new file mode 100644 index 0000000000..71976aa5e5 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/conditions/registry.js @@ -0,0 +1,126 @@ +// @ts-check +// Named conditions that decide whether a rule applies to a component. +// +// A rule declares its conditions (see defineRule.js) instead of hiding them in early returns, so the same +// definition is shared across rules, documented once (rulesets/CONDITIONS.md), and can be counted and traced. + +/** + * @typedef {import('../ruleContext.js').RuleContext} RuleContext + */ + +/** + * @typedef {Object} Condition + * @property {string} id unique; the part before the dot is the subject, e.g. 'resource.isSingleton' (checked by the tests) + * @property {'leaf'|'allOf'|'anyOf'|'not'} op + * @property {(ctx: RuleContext) => unknown} [fn] predicate for leaf conditions; reads the rule context + * @property {Array} [children] operands for allOf/anyOf/not + * @property {string} [label] short phrase completing "Applies when ...", rendered in the rule docs + * @property {string} [subject] what the condition is evaluated on, e.g. 'path', 'resource', 'operation' + * @property {string} [definition] plain-English definition, rendered in the glossary + * @property {{match: Array, noMatch: Array}} [examples] rule context fixtures, run by the tests; each may + * have a `note` saying what makes it (not) match, shown in the glossary when that is not visible in its path + * @property {string} [variantOf] id of the condition this one is a divergent variant of + * @property {string} [divergence] how this variant differs from variantOf, and where it is used + * @property {boolean} [readsRuleOptions] the result depends on the rule's functionOptions, so it has no rule-independent count + */ + +/** + * @typedef {Object} EvaluationResult + * @property {boolean} pass + * @property {Array} failedAt condition ids from the evaluated condition down to the one that failed; empty on pass + */ + +/** @type {Map} */ +const registry = new Map(); + +/** + * Defines and registers a named condition. Provide either fn (a leaf) or of (a named combination of conditions). + * + * @param {Omit & {of?: Condition}} spec + * @returns {Condition} + */ +export function defineCondition({ of, ...spec }) { + if (registry.has(spec.id)) { + throw new Error(`Condition ${spec.id} is already defined`); + } + /** @type {Condition} */ + const condition = of ? { ...spec, op: of.op, children: of.children } : { ...spec, op: 'leaf' }; + registry.set(condition.id, condition); + return condition; +} + +/** + * @param {Condition} condition + * @returns {Condition} holds when condition does not + */ +export function not(condition) { + return { id: `not(${condition.id})`, op: 'not', children: [condition] }; +} + +/** + * @param {...Condition} conditions evaluated in order, stopping at the first that fails + * @returns {Condition} holds when all conditions hold + */ +export function allOf(...conditions) { + return { id: `allOf(${conditions.map((c) => c.id).join(', ')})`, op: 'allOf', children: conditions }; +} + +/** + * @param {...Condition} conditions evaluated in order, stopping at the first that holds + * @returns {Condition} holds when at least one condition holds + */ +export function anyOf(...conditions) { + return { id: `anyOf(${conditions.map((c) => c.id).join(', ')})`, op: 'anyOf', children: conditions }; +} + +/** + * Evaluates a condition, short-circuiting like && and ||. + * + * @param {Condition} condition + * @param {RuleContext} ctx + * @param {(condition: Condition) => EvaluationResult} [evaluateChild] evaluates operands, e.g. with a cache; defaults + * to evaluating them directly + * @returns {EvaluationResult} + */ +export function evaluate(condition, ctx, evaluateChild = (child) => evaluate(child, ctx, evaluateChild)) { + const children = condition.children ?? []; + switch (condition.op) { + case 'leaf': + return condition.fn?.(ctx) ? passed() : failed(condition); + case 'not': + return evaluateChild(children[0]).pass ? failed(condition) : passed(); + case 'allOf': + for (const child of children) { + const result = evaluateChild(child); + if (!result.pass) { + // Report the named composite as well as the operand that failed, e.g. ['resource.isSingleton', 'path.isCollection'] + return { pass: false, failedAt: isNamed(condition) ? [condition.id, ...result.failedAt] : result.failedAt }; + } + } + return passed(); + case 'anyOf': + return children.some((child) => evaluateChild(child).pass) ? passed() : failed(condition); + default: + throw new Error(`Unknown condition op: ${condition.op}`); + } +} + +/** + * @returns {Array} all named conditions, in definition order + */ +export function listConditions() { + return [...registry.values()]; +} + +// By id rather than identity, so conditions from another copy of this module (e.g. Spectral's bundle) are recognized +function isNamed(condition) { + return registry.has(condition.id); +} + +function passed() { + return { pass: true, failedAt: [] }; +} + +function failed(condition) { + return { pass: false, failedAt: [condition.id] }; +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/conditions/resource.js b/tools/spectral/ipa/rulesets/functions/utils/conditions/resource.js new file mode 100644 index 0000000000..1898663867 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/conditions/resource.js @@ -0,0 +1,261 @@ +// @ts-check +// Conditions on a resource: the path items that belong to it (ctx.resourcePathItems, see getResourcePathItems), +// e.g. '/groups/{groupId}/clusters', '/groups/{groupId}/clusters/{clusterName}' and their custom methods. + +import { allOf, anyOf, defineCondition, not } from './registry.js'; +import { isCollection, isInstance } from './path.js'; +import { + anyPathHasException, + getResponseIsAllReadOnly, + isSingleResourceIdentifier, + isSingletonResource, + stripCustomMethodName, +} from '../resourceEvaluation.js'; +import { isInvalidGetMethod } from '../methodLogic.js'; +import { getGETMethodResponseSchemaFromPathItem } from '../methodUtils.js'; + +const readOnlyGet = { get: { responses: { 200: { content: { 'application/json': { schema: readOnlySchema() } } } } } }; +const writableGet = { + get: { + responses: { 200: { content: { 'application/json': { schema: { properties: { name: { type: 'string' } } } } } } }, + }, +}; + +function readOnlySchema() { + return { properties: { id: { type: 'string', readOnly: true } } }; +} + +const example = (resourcePath, paths, note) => ({ path: ['paths', resourcePath], paths, ...(note && { note }) }); + +export const hasSingletonShape = defineCondition({ + id: 'resource.hasSingletonShape', + label: 'the resource has the singleton shape', + subject: 'resource', + definition: + "The resource's path items contain exactly one collection path, that path belongs to a parent resource (`/parent/{id}/child`), it has no Create (POST) method, and every other path is a custom method. The current path is not required to be the collection path; see `resource.isSingleton`.", + fn: (ctx) => isSingletonResource(ctx.resourcePathItems), + examples: { + match: [ + example( + '/groups/{groupId}/settings', + { '/groups/{groupId}/settings': { get: {}, patch: {} } }, + 'Get and Patch only' + ), + ], + noMatch: [ + example( + '/groups/{groupId}/clusters', + { + '/groups/{groupId}/clusters': { get: {}, post: {} }, + '/groups/{groupId}/clusters/{clusterName}': { get: {} }, + }, + 'has Create and an instance path' + ), + example('/settings', { '/settings': { get: {} } }, 'no parent resource'), + ], + }, +}); + +export const isSingleton = defineCondition({ + id: 'resource.isSingleton', + label: 'the path is a singleton resource', + subject: 'resource', + definition: 'The path is a collection path and the resource it names has the singleton shape.', + of: allOf(isCollection, hasSingletonShape), + examples: { + match: [ + example( + '/groups/{groupId}/settings', + { '/groups/{groupId}/settings': { get: {} }, '/groups/{groupId}/settings:reset': { post: {} } }, + 'singleton with a custom method' + ), + ], + noMatch: [ + example( + '/groups/{groupId}/settings:reset', + { '/groups/{groupId}/settings': { get: {} }, '/groups/{groupId}/settings:reset': { post: {} } }, + 'the custom method path of a singleton, not its collection path' + ), + ], + }, +}); + +export const hasResourceHasGetException = defineCondition({ + id: 'resource.hasResourceHasGetException', + label: 'some path of the resource has an exception for `xgen-IPA-104-resource-has-GET`', + subject: 'resource', + definition: 'Some path item of the resource carries an `x-xgen-IPA-exception` for `xgen-IPA-104-resource-has-GET`.', + fn: (ctx) => anyPathHasException(ctx.resourcePathItems, 'xgen-IPA-104-resource-has-GET'), + examples: { + match: [ + example( + '/groups/{groupId}/settings', + { '/groups/{groupId}/settings': { 'x-xgen-IPA-exception': { 'xgen-IPA-104-resource-has-GET': 'reason' } } }, + 'the path has the exception' + ), + ], + noMatch: [ + example('/groups/{groupId}/settings', { '/groups/{groupId}/settings': { ...readOnlyGet } }, 'no exception'), + ], + }, +}); + +export const getResponseAllReadOnly = defineCondition({ + id: 'resource.getResponseIsAllReadOnly', + label: "all properties of the resource's Get response are read-only", + subject: 'resource', + definition: + "Every property of the 2xx response schema of the resource's Get method is `readOnly: true`. The Get on the instance path is used if there is one; otherwise the first path with a Get, which may be a List, in which case `results.items` is inspected.", + fn: (ctx) => getResponseIsAllReadOnly(ctx.resourcePathItems), + examples: { + match: [ + example( + '/groups/{groupId}/settings', + { '/groups/{groupId}/settings': { ...readOnlyGet } }, + 'every Get response property is readOnly' + ), + ], + noMatch: [ + example( + '/groups/{groupId}/settings', + { '/groups/{groupId}/settings': { ...writableGet } }, + 'the Get response has a writable property' + ), + ], + }, +}); + +export const isReadOnly = defineCondition({ + id: 'resource.isReadOnly', + label: 'the resource is read-only', + subject: 'resource', + definition: + 'The resource has a Get method whose response is entirely read-only, and no path of the resource has an exception for `xgen-IPA-104-resource-has-GET`.', + divergence: + 'The exception clause is not part of the IPA definition: a resource with an IPA-104 resource-has-GET exception is never read-only, whatever its Get response looks like.', + of: allOf(not(hasResourceHasGetException), getResponseAllReadOnly), + examples: { + match: [ + example( + '/groups/{groupId}/settings', + { '/groups/{groupId}/settings': { ...readOnlyGet } }, + 'every Get response property is readOnly' + ), + ], + noMatch: [ + example( + '/groups/{groupId}/settings', + { + '/groups/{groupId}/settings': { + ...readOnlyGet, + 'x-xgen-IPA-exception': { 'xgen-IPA-104-resource-has-GET': 'reason' }, + }, + }, + 'the same read-only Get response, but the path has an xgen-IPA-104-resource-has-GET exception' + ), + ], + }, +}); + +export const isGetTarget = defineCondition({ + id: 'path.isGetTarget', + label: 'the path is a single resource path or a singleton resource', + subject: 'path', + definition: 'The path is where a standard Get method belongs: an instance path, or the path of a singleton resource.', + of: anyOf(isInstance, isSingleton), + examples: { + match: [ + example('/groups/{groupId}', { '/groups/{groupId}': { get: {} } }, 'instance path'), + example('/groups/{groupId}/settings', { '/groups/{groupId}/settings': { get: {} } }, 'singleton'), + ], + noMatch: [ + example( + '/groups', + { '/groups': { get: {}, post: {} }, '/groups/{groupId}': { get: {} } }, + 'collection of a standard resource' + ), + example('/groups/settings/{name}', { '/groups/settings/{name}': { get: {} } }, 'not an instance path'), + ], + }, +}); + +export const isLegacyGetTarget = defineCondition({ + id: 'path.isLegacyGetTarget', + label: 'the path ends in a path parameter or is a singleton resource', + subject: 'path', + variantOf: 'path.isGetTarget', + definition: + 'The path ends in a path parameter (its last character is `}`), or is the path of a singleton resource. Implemented by `methodLogic.isInvalidGetMethod`.', + divergence: + 'Accepts any path ending in `}`, including ones that are not instance paths such as `/groups/settings/{name}`. Used by the IPA-104 operation ID rules; the other IPA-104 rules use `path.isGetTarget`.', + fn: (ctx) => !isInvalidGetMethod(ctx.resourcePath, ctx.resourcePathItems), + examples: { + match: [ + example('/groups/{groupId}', { '/groups/{groupId}': { get: {} } }, 'instance path'), + example( + '/groups/settings/{name}', + { '/groups/settings/{name}': { get: {} } }, + 'ends in a path parameter, though not an instance path' + ), + ], + noMatch: [ + example( + '/groups', + { '/groups': { get: {}, post: {} }, '/groups/{groupId}': { get: {} } }, + 'collection of a standard resource' + ), + ], + }, +}); + +/** + * The 200 response of the Get method of the resource a custom method belongs to, for the current media type, in the + * document the rule reads. Shared by the condition below and the rule's check. + * + * @param {import('../ruleContext.js').RuleContext} ctx a context whose path ends in a media type + * @returns {{schema?: Object}|null|undefined} + */ +export function baseGetResponse(ctx) { + return ctx.memo('baseGetResponse', () => { + const basePathItem = ctx.oas.paths[stripCustomMethodName(ctx.resourcePath)]; + return basePathItem && getGETMethodResponseSchemaFromPathItem(basePathItem, ctx.path.at(-1)); + }); +} + +export const baseGetHasMediaType = defineCondition({ + id: 'mediaType.baseGetHasMediaType', + label: "the custom method's resource has a Get response for the same media type", + subject: 'mediaType', + definition: + 'The resource the custom method belongs to (its path without `:`) has a Get method with a 200 response for the same media type as the current response.', + fn: (ctx) => Boolean(baseGetResponse(ctx)), + examples: { + match: [ + { + path: ['paths', '/groups/{groupId}/settings:reset', 'post', 'responses', '200', 'content', 'application/json'], + paths: { + '/groups/{groupId}/settings': { get: { responses: { 200: { content: { 'application/json': {} } } } } }, + }, + note: 'the Get on /groups/{groupId}/settings has an application/json 200 response', + }, + ], + noMatch: [ + { + path: ['paths', '/groups/{groupId}/settings:reset', 'post', 'responses', '200', 'content', 'application/json'], + paths: { '/groups/{groupId}/settings': { patch: {} } }, + note: 'the base resource has no Get', + }, + ], + }, +}); + +/** + * The instance path of the resource at the current path, e.g. '/groups/{groupId}' for '/groups'. Undefined when the + * resource has no instance path. + * + * @param {import('../ruleContext.js').RuleContext} ctx + * @returns {string|undefined} + */ +export function instancePath(ctx) { + return ctx.memo('instancePath', () => Object.keys(ctx.resourcePathItems).find((p) => isSingleResourceIdentifier(p))); +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/conditions/response.js b/tools/spectral/ipa/rulesets/functions/utils/conditions/response.js new file mode 100644 index 0000000000..f608f4d269 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/conditions/response.js @@ -0,0 +1,47 @@ +// @ts-check +// Conditions on a response or media type object, for rules whose path is +// ['paths', , , 'responses', , 'content', ]. + +import { defineCondition } from './registry.js'; + +const content = (code, mediaType, mediaTypeObject, note) => ({ + path: ['paths', '/groups/{groupId}', 'get', 'responses', code, 'content', mediaType], + paths: { '/groups/{groupId}': { get: { responses: { [code]: { content: { [mediaType]: mediaTypeObject } } } } } }, + ...(note && { note }), +}); + +export const is2xx = defineCondition({ + id: 'response.is2xx', + label: 'the response is a 2xx response', + subject: 'response', + definition: 'The response status code starts with `2`.', + fn: (ctx) => ctx.path[4].startsWith('2'), + examples: { + match: [content('200', 'application/json', {}), content('202', 'application/json', {})], + noMatch: [content('404', 'application/json', {})], + }, +}); + +export const hasSchema = defineCondition({ + id: 'mediaType.hasSchema', + label: 'the media type defines a schema', + subject: 'mediaType', + definition: 'The media type object exists and defines a `schema`.', + fn: (ctx) => Boolean(ctx.node?.schema), + examples: { + match: [content('200', 'application/json', { schema: { type: 'object' } }, 'has a schema')], + noMatch: [content('200', 'application/json', { example: {} }, 'an example but no schema')], + }, +}); + +export const isJson = defineCondition({ + id: 'mediaType.isJson', + label: 'the media type is JSON', + subject: 'mediaType', + definition: 'The media type ends in `json`, e.g. `application/vnd.atlas.2023-01-01+json`.', + fn: (ctx) => ctx.path.at(-1).endsWith('json'), + examples: { + match: [content('200', 'application/vnd.atlas.2023-01-01+json', {})], + noMatch: [content('200', 'application/vnd.atlas.2023-01-01+csv', {})], + }, +}); diff --git a/tools/spectral/ipa/rulesets/functions/utils/defineRule.js b/tools/spectral/ipa/rulesets/functions/utils/defineRule.js new file mode 100644 index 0000000000..3f8f569c73 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/defineRule.js @@ -0,0 +1,95 @@ +// @ts-check +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './collectionUtils.js'; +import { createRuleContext } from './ruleContext.js'; + +/** + * @typedef {import('./conditions/registry.js').Condition} Condition + * @typedef {import('./ruleContext.js').RuleContext} RuleContext + * @typedef {Array<{path?: Array, message: string}>} RuleErrors + */ + +/** + * How a rule reports an exception thrown by its check. The modes preserve how rules behaved before defineRule; they + * are not a recommendation. Conditions are never covered: an exception there always fails the lint run. + * - 'violation': the internal error is reported, and collected as a violation (or exception, if the component has one) + * - 'return': the internal error is reported, and nothing is collected + * - 'throw': the exception propagates and Spectral fails the lint run + * @typedef {'violation'|'return'|'throw'} InternalErrorMode + */ + +/** + * Builds a Spectral rule function from the conditions under which the rule applies and the check it performs. + * + * A call goes through these steps: + * 1. The rule's `given` in the YAML selects the input. + * 2. `appliesTo` conditions are evaluated in order. If one fails, the rule does not apply and nothing is collected. + * 3. `check` returns the validation errors, an empty array meaning the component adopts the rule. What happens when + * it throws depends on `onInternalError`. + * 4. The result is collected as an adoption, violation or exception, reading `x-xgen-IPA-exception` from + * `exceptionTarget`. + * + * The conditions are attached to the function as `meta`, for generated docs and reports: `appliesTo`, and `checkUses` for + * conditions the check evaluates itself (e.g. to pick an error message), which have no effect on whether the rule applies. + * + * @param {Object} spec + * @param {Array} [spec.appliesTo] conditions that must all hold for the rule to apply + * @param {(ctx: RuleContext) => RuleErrors} spec.check + * @param {(ctx: RuleContext) => void} [spec.beforeCheck] runs once the rule applies, outside any try like the conditions, so + * an exception fails the lint run instead of being reported as an internal error + * @param {Array} [spec.checkUses] conditions evaluated inside check, listed for docs and reports only + * @param {(ctx: RuleContext) => Object} [spec.exceptionTarget] the object that carries the exception, defaults to the input + * @param {import('./ruleContext.js').DocumentKind} [spec.document] the document the context reads, defaults to 'resolved' + * @param {InternalErrorMode} [spec.onInternalError] what happens when check throws, defaults to 'violation' + * @returns {Function & {meta: {appliesTo: Array, checkUses: Array, document: string, onInternalError: InternalErrorMode}}} + */ +export function defineRule({ + appliesTo = [], + check, + beforeCheck, + checkUses = [], + exceptionTarget = (ctx) => ctx.input, + document = 'resolved', + onInternalError = 'violation', +}) { + const collect = (ctx, errors) => + evaluateAndCollectAdoptionStatus(errors, ctx.ruleName, exceptionTarget(ctx), ctx.path); + + const ruleFunction = (input, options, spectralContext) => { + const ctx = createRuleContext(input, options, spectralContext, document); + // Spectral names the function in the error when it throws, and every rule here would otherwise be 'ruleFunction' + Object.defineProperty(ruleFunction, 'name', { value: ctx.ruleName }); + + // Conditions are evaluated outside any try, so a condition that throws fails the lint run + for (const condition of appliesTo) { + if (!ctx.is(condition)) { + return; + } + } + + beforeCheck?.(ctx); + + switch (onInternalError) { + case 'violation': { + let errors; + try { + errors = check(ctx); + } catch (e) { + errors = handleInternalError(ctx.ruleName, ctx.path, e); + } + return collect(ctx, errors); + } + case 'return': + try { + return collect(ctx, check(ctx)); + } catch (e) { + return handleInternalError(ctx.ruleName, ctx.path, e); + } + case 'throw': + return collect(ctx, check(ctx)); + default: + throw new Error(`Unknown onInternalError mode: ${onInternalError}`); + } + }; + + return Object.assign(ruleFunction, { meta: { appliesTo, checkUses, document, onInternalError } }); +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/operationIdRules.js b/tools/spectral/ipa/rulesets/functions/utils/operationIdRules.js new file mode 100644 index 0000000000..96f392bdc9 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/operationIdRules.js @@ -0,0 +1,58 @@ +// @ts-check +// Builds the valid-operation-id and operation-id-length rules of the standard methods. Kept out of utils/conditions, +// which tooling loads outside Spectral: operationIdGeneration.js only loads through Spectral's bundler. + +import { defineRule } from './defineRule.js'; +import { getMethodName } from './conditions/operationIdRules.js'; +import { generateOperationID } from './operationIdGeneration.js'; +import { + validateOperationIdAndReturnErrors, + validateOperationIdLengthAndReturnErrors, +} from './validations/validateOperationIdAndReturnErrors.js'; + +// A verb override without a `verb` fails the lint run, as it did before these rules were built with defineRule +const verbOverrideIsReadable = ({ input, options }) => getMethodName(input, options.methodName); + +/** + * Builds a rule checking that the operation ID is the one generated from the method and the path. The verb is the rule's + * `methodName` option, or the operation's verb override. + * + * @param {Array} appliesTo which operations are the rule's method + */ +export function validOperationIdRule(appliesTo) { + return defineRule({ + appliesTo, + onInternalError: 'return', + beforeCheck: verbOverrideIsReadable, + check: ({ input, options, path, resourcePath }) => + validateOperationIdAndReturnErrors( + getMethodName(input, options.methodName), + resourcePath, + input, + path, + options.ignoreSingularizationList + ), + }); +} + +/** + * Builds a rule checking that the operation ID, or its `x-xgen-operation-id-override`, is at most `maxLength` words. + * + * @param {Array} appliesTo which operations are the rule's method + */ +export function operationIdLengthRule(appliesTo) { + return defineRule({ + appliesTo, + onInternalError: 'return', + beforeCheck: verbOverrideIsReadable, + check: ({ input, options, path, resourcePath }) => { + // The generated ID is only used to suggest a shorter override in the message + const expectedOperationId = generateOperationID( + getMethodName(input, options.methodName), + resourcePath, + options.ignoreSingularizationList + ); + return validateOperationIdLengthAndReturnErrors(input, path, expectedOperationId, options.maxLength); + }, + }); +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 607c76bf91..1b42cfae5a 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -292,15 +292,33 @@ export function allPropertiesAreReadOnly(schema) { * @returns {boolean} true if the resource is read-only, false otherwise */ export function isReadOnlyResource(resourcePathItems) { - const resourcePaths = Object.keys(resourcePathItems); + // If any path has an exception for xgen-IPA-104-resource-has-GET, we cannot determine if the resource is read-only + return ( + !anyPathHasException(resourcePathItems, 'xgen-IPA-104-resource-has-GET') && + getResponseIsAllReadOnly(resourcePathItems) + ); +} - // Check if any path has an exception for xgen-IPA-104-resource-has-GET - // If so, we cannot determine if the resource is read-only - for (const path of resourcePaths) { - if (hasException(resourcePathItems[path], 'xgen-IPA-104-resource-has-GET')) { - return false; - } - } +/** + * Checks if any path item of a resource has an exception for the given rule. + * + * @param {Object} resourcePathItems - All path items for the resource to be evaluated + * @param {string} ruleName - The name of the rule to look for + * @returns {boolean} + */ +export function anyPathHasException(resourcePathItems, ruleName) { + return Object.values(resourcePathItems).some((pathItem) => hasException(pathItem, ruleName)); +} + +/** + * Checks if all properties in the response schema of a resource's Get method are readOnly: true. The standard Get method + * (on the single resource path) is preferred; otherwise the first path with a Get method is used, which may be a List. + * + * @param {Object} resourcePathItems - All path items for the resource to be evaluated + * @returns {boolean} + */ +export function getResponseIsAllReadOnly(resourcePathItems) { + const resourcePaths = Object.keys(resourcePathItems); // First, look for a standard Get method let getPathItem = null; diff --git a/tools/spectral/ipa/rulesets/functions/utils/ruleContext.js b/tools/spectral/ipa/rulesets/functions/utils/ruleContext.js new file mode 100644 index 0000000000..d30e7f7f03 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/ruleContext.js @@ -0,0 +1,128 @@ +// @ts-check +import { getResourcePathItems } from './resourceEvaluation.js'; +import { resolveObject } from './componentUtils.js'; +import { evaluate } from './conditions/registry.js'; + +/** + * @typedef {'resolved'|'unresolved'} DocumentKind + * @typedef {import('./conditions/registry.js').Condition} Condition + * @typedef {import('./conditions/registry.js').EvaluationResult} EvaluationResult + */ + +/** + * Per-call context for a rule function. Derived values are computed on first use and cached, so conditions and + * the rule's check share them. + */ +export class RuleContext { + /** + * @param {Object} params + * @param {*} params.input the value Spectral passes to the rule function + * @param {Object} params.options the rule's functionOptions + * @param {Array} params.path the JSON path of the input, e.g. ['paths', '/groups/{groupId}', 'get'] + * @param {string} params.ruleName + * @param {{paths: Object}} params.oas the document the rule reads, see {@link DocumentKind} + */ + constructor({ input, options, path, ruleName, oas }) { + this.input = input; + this.options = options; + this.path = path; + this.ruleName = ruleName; + this.oas = oas; + /** @type {Map} */ + this.evaluations = new Map(); + /** @type {Map} */ + this.cache = new Map(); + } + + /** @returns {string} the path key, e.g. '/api/atlas/v2/groups/{groupId}' */ + get resourcePath() { + return this.path[1]; + } + + /** @returns {Object} the path items of the resource at resourcePath, see getResourcePathItems */ + get resourcePathItems() { + return this.memo('resourcePathItems', () => getResourcePathItems(this.resourcePath, this.oas.paths)); + } + + /** @returns {Object|undefined} the path item at resourcePath */ + get pathItem() { + return this.oas.paths[this.resourcePath]; + } + + /** @returns {Object|undefined} the operation at path[2], if the path points into one */ + get operation() { + return this.pathItem?.[this.path[2]]; + } + + /** @returns {*} the value at path in the document the rule reads, or undefined if missing or falsy */ + get node() { + return this.memo('node', () => resolveObject(this.oas, this.path)); + } + + /** + * @param {Condition} condition + * @returns {EvaluationResult} + */ + evaluate(condition) { + if (!this.evaluations.has(condition.id)) { + // Operands go through this cache too, so a condition shared by appliesTo and the check is evaluated once + this.evaluations.set( + condition.id, + evaluate(condition, this, (child) => this.evaluate(child)) + ); + } + return /** @type {EvaluationResult} */ (this.evaluations.get(condition.id)); + } + + /** + * @param {Condition} condition + * @returns {boolean} + */ + is(condition) { + return this.evaluate(condition).pass; + } + + /** + * @param {string} resourcePath another path key in the same document, e.g. the base resource of a custom method + * @returns {RuleContext} a context for that path, without an input + */ + atPath(resourcePath) { + return new RuleContext({ + input: undefined, + options: this.options, + path: ['paths', resourcePath], + ruleName: this.ruleName, + oas: this.oas, + }); + } + + /** + * @param {string} key + * @param {() => *} compute + */ + memo(key, compute) { + if (!this.cache.has(key)) { + this.cache.set(key, compute()); + } + return this.cache.get(key); + } +} + +/** + * Creates the context for one call of a rule function. + * + * @param {*} input + * @param {Object} options + * @param {{path: Array, documentInventory: {resolved: Object, unresolved: Object}, rule: {name: string}}} spectralContext + * @param {DocumentKind} document which document the rule reads; independent of the rule's YAML `resolved` flag + * @returns {RuleContext} + */ +export function createRuleContext(input, options, { path, documentInventory, rule }, document) { + return new RuleContext({ + input, + options, + path, + ruleName: rule.name, + oas: documentInventory[document], + }); +} diff --git a/tools/spectral/ipa/scripts/generateRulesetReadme.js b/tools/spectral/ipa/scripts/generateRulesetReadme.js index 560f5a8917..c8abb3bb5f 100644 --- a/tools/spectral/ipa/scripts/generateRulesetReadme.js +++ b/tools/spectral/ipa/scripts/generateRulesetReadme.js @@ -6,6 +6,18 @@ import { loadRuleset } from '../utils.js'; const dirname = path.dirname(fileURLToPath(import.meta.url)); const readmeFilePath = path.join(dirname, '../rulesets', 'README.md'); +const conditionsFilePath = path.join(dirname, '../rulesets', 'CONDITIONS.md'); +const CONDITIONS_URL = 'https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/CONDITIONS.md'; + +// Plain-English names for the `given` of rules built with defineRule, for their Applies to section +const GIVEN_DESCRIPTIONS = { + '$.paths': 'each path', + '$.paths[*]': 'each path item', + '$.paths[*].post': 'each POST operation', + '#GetOperationObject': 'each Get operation', + '#GetOperationObject.responses[*].content': 'each media type of each Get response', + '$.paths[*].post.responses.200.content': 'each media type of each POST 200 response', +}; const rulesetsSection = await getRulesetsSection(); @@ -23,6 +35,13 @@ fs.writeFile(readmeFilePath, fileContent, (error) => { } }); +fs.writeFile(conditionsFilePath, await getConditionsFileContent(), (error) => { + if (error) { + console.error('Error while generating the IPA rulesets CONDITIONS.md:', error); + process.exit(1); + } +}); + async function getRulesetsSection() { let content = '## Rulesets\n\n' + 'Below is a list of all available rules, their descriptions and severity levels.\n\n'; @@ -48,6 +67,8 @@ function generateRulesetSections(rules) { name: ruleName, description: rules[ruleName].description, severity: rules[ruleName].definition.severity, + given: rules[ruleName].definition.given, + meta: rules[ruleName].then[0]?.function?.meta, })) .sort((a, b) => { if (a.severity < b.severity) { @@ -61,6 +82,9 @@ function generateRulesetSections(rules) { sortedRuleEntries.forEach((rule) => { const severityFormatted = formatSeverity(rule.severity); sections += `#### ${rule.name}\n\n ${severityFormatted} \n${rule.description}\n`; + if (rule.meta) { + sections += getAppliesToSection(rule); + } }); return sections; @@ -115,3 +139,100 @@ function filterRulesByIpaNumber(ipaNumber, rules) { }; }, {}); } + +// ---------- Rules built with defineRule: generated from the rule's conditions ---------- + +function getAppliesToSection(rule) { + const given = [rule.given].flat().map((g) => GIVEN_DESCRIPTIONS[g] ?? `\`${g}\``); + const subject = given.join(', ').replace(/^./, (c) => c.toUpperCase()); + // not(...) conditions read better as a separate list than as "when: unless ..." + const required = rule.meta.appliesTo.filter((c) => c.op !== 'not' || c.label); + const excluded = rule.meta.appliesTo.filter((c) => c.op === 'not' && !c.label).map((c) => c.children[0]); + let section = `##### Applies to\n${subject}${required.length ? ', when:' : ''}\n`; + required.forEach((condition) => { + section += ` - ${describeCondition(condition)}\n`; + }); + if (excluded.length) { + section += `${required.length ? '\nExcept' : 'Except'} when:\n`; + excluded.forEach((condition) => { + section += ` - ${describeCondition(condition)}\n`; + }); + } + return section + '\n'; +} + +// Named conditions link to the glossary (in-page when rendering the glossary itself); combinations are spelled out +function describeCondition(condition, glossaryUrl = CONDITIONS_URL) { + const children = condition.children ?? []; + if (condition.label) { + return `${condition.label} ([\`${condition.id}\`](${glossaryUrl}#${anchor(condition.id)}))`; + } + switch (condition.op) { + case 'not': + return `not: ${describeCondition(children[0], glossaryUrl)}`; + case 'anyOf': + return `any of: ${children.map((c) => describeCondition(c, glossaryUrl)).join('; ')}`; + case 'allOf': + return `all of: ${children.map((c) => describeCondition(c, glossaryUrl)).join('; ')}`; + default: + return `\`${condition.id}\``; + } +} + +function anchor(conditionId) { + return conditionId.toLowerCase().replace(/[^a-z0-9-]/g, ''); +} + +async function getConditionsFileContent() { + const rules = await getAllRules(); + const named = new Map(); + const usedBy = new Map(); + const collect = (condition, ruleName) => { + if (condition.label) { + named.set(condition.id, condition); + if (!usedBy.has(condition.id)) { + usedBy.set(condition.id, new Set()); + } + usedBy.get(condition.id).add(ruleName); + } + (condition.children ?? []).forEach((child) => collect(child, ruleName)); + }; + Object.entries(rules).forEach(([ruleName, rule]) => { + const meta = rule.then[0]?.function?.meta; + [...(meta?.appliesTo ?? []), ...(meta?.checkUses ?? [])].forEach((condition) => collect(condition, ruleName)); + }); + + let content = + '\n\n' + + '# IPA Validation Conditions\n\n' + + 'Named conditions that decide where IPA rules apply. They are defined in ' + + '[rulesets/functions/utils/conditions](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/rulesets/functions/utils/conditions) ' + + 'and used by rules built with `defineRule`. Only conditions used by at least one rule are listed.\n\n'; + + const sorted = [...named.values()].sort((a, b) => a.id.localeCompare(b.id)); + sorted.forEach((condition) => { + content += `### ${condition.id}\n\n`; + content += `Evaluated on: ${condition.subject ?? 'n/a'} + +${condition.definition}\n\n`; + if (condition.children?.length) { + content += `Defined as: ${describeCondition({ ...condition, label: undefined }, '')}\n\n`; + } + if (condition.variantOf) { + content += `> **Variant of [\`${condition.variantOf}\`](#${anchor(condition.variantOf)}).** ${condition.divergence}\n\n`; + } else if (condition.divergence) { + content += `> **Divergence.** ${condition.divergence}\n\n`; + } + // An example's note says what makes it (not) match when that is not visible in the path + const examples = condition.examples; + if (examples) { + const describeExample = (example) => + `\`${example.path.slice(1).join(' ')}\`${example.note ? ` (${example.note})` : ''}`; + content += `Matches:\n${examples.match.map((e) => `- ${describeExample(e)}`).join('\n')}\n\n`; + content += `Does not match:\n${examples.noMatch.map((e) => `- ${describeExample(e)}`).join('\n')}\n\n`; + } + const ruleNames = [...usedBy.get(condition.id)].sort(); + content += `Used by: ${ruleNames.map((name) => `[${name}](https://mdb.link/mongodb-atlas-openapi-validation#${name})`).join(', ')}\n\n`; + }); + return content.trimEnd() + '\n'; +} diff --git a/tools/spectral/ipa/scripts/ipa-snapshot-compare.js b/tools/spectral/ipa/scripts/ipa-snapshot-compare.js new file mode 100644 index 0000000000..1785747dd0 --- /dev/null +++ b/tools/spectral/ipa/scripts/ipa-snapshot-compare.js @@ -0,0 +1,163 @@ +// Compares the results of the IPA ruleset at two points and reports per-rule differences. +// +// Usage: node tools/spectral/ipa/scripts/ipa-snapshot-compare.js --spec --base [--head ] [--verbose] +// node tools/spectral/ipa/scripts/ipa-snapshot-compare.js [--verbose] +// +// With --base, each side is linted with the ruleset of that git ref, checked out into a temporary worktree and removed +// afterwards. Without --head, the head is this checkout as it is now, including uncommitted changes. This script and +// the spec always come from this checkout, so the base ref does not need to contain them. +// +// With two directories, compares snapshots already written by scripts/ipa-snapshot.js. +// +// Prints, for the findings and for each collector entry type, which rules gained (+) or lost (-) entries, and with +// --verbose the entries themselves. Exits 0 when the results are identical and 1 otherwise, so it can be used to +// prove that a refactor is behavior-neutral. + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { parseArgs } from 'node:util'; +import { DEFAULT_RULESET, checkoutWorktree, takeSnapshot } from './ipa-snapshot.js'; + +/** + * Computes the multiset difference between two lists of entries, grouped by rule. An entry that appears three times in + * base and twice in head counts as one removed. + * + * @param {Array} base entries, optionally carrying a 'count' multiplicity + * @param {Array} head entries, optionally carrying a 'count' multiplicity + * @param {(entry: Object) => string} ruleOf returns the rule name of an entry + * @returns {Map, removed: Array}>} differences per rule, only for rules that differ + */ +export function diffEntries(base, head, ruleOf) { + const counts = new Map(); + const tally = (list, sign) => { + for (const entry of list) { + const { count = 1, ...rest } = entry; + const key = JSON.stringify(rest); + const delta = sign * count; + counts.set(key, (counts.get(key) ?? 0) + delta); + } + }; + tally(base, -1); + tally(head, 1); + + const byRule = new Map(); + for (const [key, delta] of counts) { + if (delta === 0) { + continue; + } + const entry = JSON.parse(key); + const rule = ruleOf(entry); + if (!byRule.has(rule)) { + byRule.set(rule, { added: [], removed: [] }); + } + const bucket = delta > 0 ? byRule.get(rule).added : byRule.get(rule).removed; + bucket.push({ ...entry, count: Math.abs(delta) }); + } + return byRule; +} + +/** + * Compares two snapshot directories. + * + * @param {string} baseDir + * @param {string} headDir + * @returns {{findings: Map, collector: Object}} differences for findings and for each collector entry type + */ +export function compareSnapshots(baseDir, headDir) { + const read = (dir, file) => JSON.parse(fs.readFileSync(path.join(dir, file), 'utf-8')); + const findings = diffEntries(read(baseDir, 'findings.json'), read(headDir, 'findings.json'), (f) => f.code); + + const baseCollector = read(baseDir, 'collector.json'); + const headCollector = read(headDir, 'collector.json'); + const collector = {}; + for (const type of new Set([...Object.keys(baseCollector), ...Object.keys(headCollector)])) { + collector[type] = diffEntries(baseCollector[type] ?? [], headCollector[type] ?? [], (e) => e.ruleName); + } + return { findings, collector }; +} + +/** + * Lints a spec with the ruleset at two git refs and compares the results. The temporary worktrees and snapshots are + * removed afterwards. + * + * @param {Object} params + * @param {string} params.spec the OpenAPI file, linted unchanged on both sides + * @param {string} params.base git ref for the base ruleset + * @param {string} [params.head] git ref for the head ruleset; this checkout, as it is now, when omitted + * @returns {{findings: Map, collector: Object}} see compareSnapshots + */ +export function compareRefs({ spec, base, head }) { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'ipa-snapshot-compare-')); + const worktrees = []; + const snapshotOf = (name, ref) => { + let ruleset = DEFAULT_RULESET; + if (ref) { + const worktree = checkoutWorktree(ref); + worktrees.push(worktree); + ruleset = worktree.ruleset; + } + const out = path.join(tmp, name); + takeSnapshot({ spec, out, ruleset }); + return out; + }; + try { + const baseDir = snapshotOf('base', base); + const headDir = snapshotOf('head', head); + return compareSnapshots(baseDir, headDir); + } finally { + worktrees.forEach((worktree) => worktree.remove()); + fs.rmSync(tmp, { recursive: true, force: true }); + } +} + +function sum(list) { + return list.reduce((total, entry) => total + entry.count, 0); +} + +function printSection(title, diff, verbose, describe) { + if (diff.size === 0) { + console.log(`${title}: identical`); + return; + } + console.log(`${title}:`); + for (const rule of [...diff.keys()].sort()) { + const { added, removed } = diff.get(rule); + console.log(` ${rule}: +${sum(added)} -${sum(removed)}`); + if (verbose) { + removed.forEach((e) => console.log(` - ${describe(e)}${e.count > 1 ? ` (x${e.count})` : ''}`)); + added.forEach((e) => console.log(` + ${describe(e)}${e.count > 1 ? ` (x${e.count})` : ''}`)); + } + } +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const { values, positionals } = parseArgs({ + allowPositionals: true, + options: { + verbose: { type: 'boolean', short: 'v', default: false }, + spec: { type: 'string' }, + base: { type: 'string' }, + head: { type: 'string' }, + }, + }); + const byRef = Boolean(values.spec && values.base) && positionals.length === 0; + const byDir = positionals.length === 2 && !values.spec && !values.base && !values.head; + if (!byRef && !byDir) { + console.error( + 'Usage: node ipa-snapshot-compare.js --spec --base [--head ] [--verbose]' + ); + console.error(' node ipa-snapshot-compare.js [--verbose]'); + process.exit(2); + } + + const { findings, collector } = byRef ? compareRefs(values) : compareSnapshots(positionals[0], positionals[1]); + printSection('Findings', findings, values.verbose, (f) => `${f.path}: ${f.message}`); + for (const [type, diff] of Object.entries(collector)) { + printSection(`Collector ${type}`, diff, values.verbose, (e) => e.componentId); + } + + const identical = findings.size === 0 && Object.values(collector).every((diff) => diff.size === 0); + process.exit(identical ? 0 : 1); +} diff --git a/tools/spectral/ipa/scripts/ipa-snapshot.js b/tools/spectral/ipa/scripts/ipa-snapshot.js new file mode 100644 index 0000000000..35670b38bf --- /dev/null +++ b/tools/spectral/ipa/scripts/ipa-snapshot.js @@ -0,0 +1,148 @@ +// Runs the IPA ruleset against a spec and writes a normalized, diffable snapshot of the results. +// +// Use it to show that a change to the ruleset does not change its results: snapshot the same spec before and after, +// then compare the two with scripts/ipa-snapshot-compare.js. +// +// Usage: node tools/spectral/ipa/scripts/ipa-snapshot.js --spec --out [--ruleset ] +// +// Writes to : +// findings.json - what Spectral reports: [{ code, path, severity, message }], sorted. +// code is the rule name and path is the JSON path of the finding, e.g. "paths./groups.get". +// Source file paths are left out so snapshots from different checkouts compare equal. +// collector.json - what the metrics collector records, per entry type (violations, adoptions, exceptions): +// { violations: [{ componentId, ruleName, count }] }. An adoption is a component that passed a +// rule. An exception is a component that failed a rule but carries x-xgen-IPA-exception for it, so +// no finding is reported (the entry also has an exceptionReason). These feed the metrics and do +// not line up one to one with the findings: adoptions and exceptions never appear in them. + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { parseArgs } from 'node:util'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const IPA_DIR_IN_REPO = path.join('tools', 'spectral', 'ipa'); +const REPO_ROOT = path.join(__dirname, '..', '..', '..', '..'); +export const DEFAULT_RULESET = path.join(__dirname, '..', 'ipa-spectral.yaml'); +const SPECTRAL_BIN = path.join(REPO_ROOT, 'node_modules', '.bin', 'spectral'); +// Rulesets that predate IPA_COLLECTOR_FILE (e.g. an older ref checked out for comparison) write this file to the cwd +const DEFAULT_COLLECTOR_FILE_NAME = 'ipa-collector-results-combined.log'; + +// Spectral's raw output has no stable order and carries absolute source file paths, so keep only the fields that +// identify a finding and sort them. Two runs that find the same things then produce identical JSON. +export function normalizeFindings(results) { + return results + .map(({ code, path: jsonPath, severity, message }) => ({ code, path: jsonPath.join('.'), severity, message })) + .sort(compareByJson); +} + +// The collector appends entries in the order rules ran, and a component can be recorded more than once (for example a +// schema reached through several $refs). Group identical entries and count them, in a stable order, so the snapshot +// does not depend on run order but still notices when the number of entries changes. +export function normalizeCollectorEntries(entries) { + const normalized = {}; + for (const [type, list] of Object.entries(entries)) { + const counts = new Map(); + for (const entry of list) { + const key = JSON.stringify(sortKeys(entry)); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + normalized[type] = [...counts.entries()] + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) + .map(([key, count]) => ({ ...JSON.parse(key), count })); + } + return normalized; +} + +// So that entries with the same fields in a different order give the same JSON key above +function sortKeys(object) { + return Object.fromEntries(Object.entries(object).sort(([a], [b]) => (a < b ? -1 : 1))); +} + +function compareByJson(a, b) { + const x = JSON.stringify(a); + const y = JSON.stringify(b); + return x < y ? -1 : x > y ? 1 : 0; +} + +/** + * Checks out a git ref into a temporary worktree that shares this checkout's node_modules, so the ruleset of that ref + * can be run without touching the working tree. Nothing needs building. + * + * @param {string} ref any git ref or commit + * @returns {{dir: string, ruleset: string, remove: () => void}} the ruleset file inside the worktree + */ +export function checkoutWorktree(ref) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ipa-worktree-')); + execFileSync('git', ['worktree', 'add', '--detach', dir, ref], { cwd: REPO_ROOT, stdio: 'ignore' }); + fs.symlinkSync(path.join(REPO_ROOT, 'node_modules'), path.join(dir, 'node_modules')); + return { + dir, + ruleset: path.join(dir, IPA_DIR_IN_REPO, 'ipa-spectral.yaml'), + remove: () => execFileSync('git', ['worktree', 'remove', '--force', dir], { cwd: REPO_ROOT, stdio: 'ignore' }), + }; +} + +export function takeSnapshot({ spec, out, ruleset = DEFAULT_RULESET }) { + fs.mkdirSync(out, { recursive: true }); + const rawFindingsFile = path.resolve(out, 'spectral-output.json'); + const rawCollectorFile = path.resolve(out, DEFAULT_COLLECTOR_FILE_NAME); + fs.rmSync(rawCollectorFile, { force: true }); + + // Running in the out dir keeps older rulesets from overwriting the collector log in the caller's cwd + const result = spawnSync( + SPECTRAL_BIN, + ['lint', path.resolve(spec), '--ruleset', path.resolve(ruleset), '-f', 'json', '-o', rawFindingsFile], + { + cwd: out, + encoding: 'utf-8', + maxBuffer: 100 * 1024 * 1024, + env: { ...process.env, IPA_COLLECTOR_FILE: rawCollectorFile }, + } + ); + + if (result.error) { + throw result.error; + } + // Spectral exits 1 when it reports error-severity findings; anything else means the lint itself failed + if (result.status !== 0 && result.status !== 1) { + throw new Error(`Spectral exited with status ${result.status}:\n${result.stderr}`); + } + if (!fs.existsSync(rawFindingsFile)) { + throw new Error(`Spectral did not produce ${rawFindingsFile}:\n${result.stderr}`); + } + + const findings = normalizeFindings(JSON.parse(fs.readFileSync(rawFindingsFile, 'utf-8'))); + const collectorEntries = fs.existsSync(rawCollectorFile) + ? normalizeCollectorEntries(JSON.parse(fs.readFileSync(rawCollectorFile, 'utf-8'))) + : {}; + + fs.writeFileSync(path.join(out, 'findings.json'), JSON.stringify(findings, null, 2)); + fs.writeFileSync(path.join(out, 'collector.json'), JSON.stringify(collectorEntries, null, 2)); + fs.rmSync(rawFindingsFile, { force: true }); + fs.rmSync(rawCollectorFile, { force: true }); + + return { findings, collectorEntries }; +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const { values } = parseArgs({ + options: { + spec: { type: 'string' }, + out: { type: 'string' }, + ruleset: { type: 'string', default: DEFAULT_RULESET }, + }, + }); + if (!values.spec || !values.out) { + console.error('Usage: node ipa-snapshot.js --spec --out [--ruleset ]'); + process.exit(2); + } + + const { findings, collectorEntries } = takeSnapshot(values); + const counts = Object.entries(collectorEntries) + .map(([type, list]) => `${type}: ${list.reduce((sum, e) => sum + e.count, 0)}`) + .join(', '); + console.log(`Wrote snapshot to ${values.out} (findings: ${findings.length}; ${counts})`); +}