diff --git a/tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js b/tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js new file mode 100644 index 0000000000..339b067997 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js @@ -0,0 +1,185 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + schemas: { + Dog: { + type: 'object', + properties: { + breed: { type: 'string' }, + age: { type: 'integer' }, + }, + }, + Cat: { + type: 'object', + properties: { + color: { type: 'string' }, + livesLeft: { type: 'integer' }, + }, + }, + Bird: { + type: 'object', + properties: { + species: { type: 'string' }, + wingspan: { type: 'number' }, + }, + }, + Fish: { + type: 'object', + properties: { + species: { type: 'string' }, + waterType: { type: 'string' }, + }, + }, + }, +}; + +testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ + { + name: 'valid oneOf with discriminator and matching mapping', + document: { + components: componentSchemas, + schemas: { + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + propertyName: 'type', + mapping: { + dog: '#/components/schemas/Dog', + cat: '#/components/schemas/Cat', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid oneOf with discriminator but mismatched mapping', + document: { + components: componentSchemas, + schemas: { + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + propertyName: 'type', + mapping: { + dog: '#/components/schemas/Dog', + bird: '#/components/schemas/Bird', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-must-have-discriminator', + message: + 'The discriminator mapping must match the oneOf references. Unmatched Discriminator mappings with oneOf references: #/components/schemas/Bird', + path: ['schemas', 'Animal'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'invalid oneOf without discriminator', + document: { + components: componentSchemas, + schemas: { + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-must-have-discriminator', + message: 'The schema has oneOf but no discriminator property.', + path: ['schemas', 'Animal'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'invalid oneOf with non-object discriminator', + document: { + components: componentSchemas, + schemas: { + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: "I'm a string, not an object!", + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-must-have-discriminator', + message: 'Discriminator property is not an object.', + path: ['schemas', 'Animal'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'invalid oneOf with discriminator but no propertyName', + document: { + components: componentSchemas, + schemas: { + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + mapping: { + dog: '#/components/schemas/Dog', + cat: '#/components/schemas/Cat', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-must-have-discriminator', + message: 'Discriminator has no propertyName defined.', + path: ['schemas', 'Animal'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'invalid oneOf with discriminator but no mapping', + document: { + components: componentSchemas, + schemas: { + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + propertyName: 'type', + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-must-have-discriminator', + message: 'Discriminator must have a mapping object.', + path: ['schemas', 'Animal'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'oneOf with discriminator exemption', + document: { + components: componentSchemas, + schemas: { + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + 'x-xgen-IPA-exception': { + 'xgen-IPA-125-oneOf-must-have-discriminator': 'reason for exemption', + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/__tests__/__helpers__/testRule.js b/tools/spectral/ipa/__tests__/__helpers__/testRule.js index 824cb2687c..d3cc2e8240 100644 --- a/tools/spectral/ipa/__tests__/__helpers__/testRule.js +++ b/tools/spectral/ipa/__tests__/__helpers__/testRule.js @@ -4,6 +4,7 @@ import { describe, expect, it } from '@jest/globals'; import { Spectral, Document } from '@stoplight/spectral-core'; import { httpAndFileResolver } from '@stoplight/spectral-ref-resolver'; import { bundleAndLoadRuleset } from '@stoplight/spectral-ruleset-bundler/with-loader'; +import { fail } from 'node:assert'; export default (ruleName, tests) => { describe(`Rule ${ruleName}`, () => { @@ -13,6 +14,12 @@ export default (ruleName, tests) => { const doc = testCase.document instanceof Document ? testCase.document : JSON.stringify(testCase.document); const errors = await s.run(doc); + if (testCase.errors.length !== errors.length) { + fail(`Expected errors do not match actual errors + (${testCase.errors.length} !== ${errors.length}) + Expected errors: ${JSON.stringify(testCase.errors, undefined, 2)} + Actual errors: ${JSON.stringify(errors, undefined, 2)}`); + } expect(errors.length).toEqual(testCase.errors.length); errors.forEach((error, index) => { diff --git a/tools/spectral/ipa/rulesets/IPA-125.yaml b/tools/spectral/ipa/rulesets/IPA-125.yaml new file mode 100644 index 0000000000..5f0154f8fd --- /dev/null +++ b/tools/spectral/ipa/rulesets/IPA-125.yaml @@ -0,0 +1,41 @@ +# IPA-125: Single Type in Request and Response +# http://go/ipa/125 + +functions: + - IPA125OneOfMustHaveDiscriminator + +rules: + xgen-IPA-125-oneOf-must-have-discriminator: + description: | + Each oneOf property must include a discriminator property to define the exact type. + + ##### Implementation details + Rule checks for the following conditions: + - Applies only to schemas with `oneOf` containing references + - Ensures a `discriminator` property is present with a valid `propertyName` + - Validates that `discriminator.mapping` contains exactly the same number of entries as `oneOf` references + - Validates that each `discriminator.mapping` value matches a reference in the `oneOf` array + - Ignores `oneOf` definitions with inline schemas + + ##### Matching Logic + - The `discriminator.mapping` must have the same number of entries as there are references in the `oneOf` array + - Each value in the `discriminator.mapping` must match one of the `$ref` values in the `oneOf` array + - Each `$ref` in the `oneOf` array must have a corresponding entry in the `discriminator.mapping` + - Example: + ```yaml + oneOf: + - $ref: '#/components/schemas/Dog' + - $ref: '#/components/schemas/Cat' + discriminator: + propertyName: type + mapping: + dog: '#/components/schemas/Dog' + cat: '#/components/schemas/Cat' + ``` + This is valid because there are exactly 2 mappings for 2 oneOf references, and all values match. + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-125-oneOf-must-have-discriminator' + severity: error + given: '$..[?(@.oneOf)]' + then: + function: 'IPA125OneOfMustHaveDiscriminator' diff --git a/tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js b/tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js new file mode 100644 index 0000000000..e1a6bdf256 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js @@ -0,0 +1,61 @@ +import { collectAdoption, collectAndReturnViolation } from './utils/collectionUtils.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { hasException } from './utils/exceptions.js'; + +const RULE_NAME = 'xgen-IPA-125-oneOf-must-have-discriminator'; +const MISSING_DISCRIMINATOR_MESSAGE = 'The schema has oneOf but no discriminator property.'; +const INVALID_DISCRIMINATOR_MESSAGE = 'Discriminator property is not an object.'; +const MISSING_PROPERTY_NAME_MESSAGE = 'Discriminator has no propertyName defined.'; +const MISSING_MAPPING_MESSAGE = 'Discriminator must have a mapping object.'; +const MAPPING_ERROR_MESSAGE = + 'The discriminator mapping must match the oneOf references. Unmatched Discriminator mappings with oneOf references:'; + +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.unresolved; // Use unresolved document to access raw $ref + const schema = resolveObject(oas, path); + + if (!schema || !schema.oneOf || !Array.isArray(schema.oneOf)) { + return; + } + + // Check for exception first + if (hasException(schema, RULE_NAME)) { + return; + } + + // Check if all oneOf items are objects with a $ref property + const allReferences = schema.oneOf.every((item) => typeof item === 'object' && item.$ref); + if (!allReferences) { + return; + } + + // Validate the presence of a discriminator with more specific error messages + if (!schema.discriminator) { + return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_DISCRIMINATOR_MESSAGE }]); + } + + if (typeof schema.discriminator !== 'object') { + return collectAndReturnViolation(path, RULE_NAME, [{ message: INVALID_DISCRIMINATOR_MESSAGE }]); + } + + if (!schema.discriminator.propertyName) { + return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_PROPERTY_NAME_MESSAGE }]); + } + + if (!schema.discriminator.mapping) { + return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_MAPPING_MESSAGE }]); + } + + const oneOfRefs = schema.oneOf.map((item) => item.$ref); + const mappingValues = Object.values(schema.discriminator.mapping); + + // Check for discriminator mappings that don't match any oneOf reference + const unmatchedMappings = mappingValues.filter((mapping) => !oneOfRefs.includes(mapping)); + if (unmatchedMappings.length > 0) { + return collectAndReturnViolation(path, RULE_NAME, [ + { message: `${MAPPING_ERROR_MESSAGE} ${unmatchedMappings.join(', ')}` }, + ]); + } + + collectAdoption(path, RULE_NAME); +};