Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
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: 'Each oneOf property must include a discriminator property to define the exact type.',
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: [],
},
]);
7 changes: 7 additions & 0 deletions tools/spectral/ipa/__tests__/__helpers__/testRule.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`, () => {
Expand All @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

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) => {
Expand Down
41 changes: 41 additions & 0 deletions tools/spectral/ipa/rulesets/IPA-125.yaml
Original file line number Diff line number Diff line change
@@ -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'
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
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 ERROR_MESSAGE = 'Each oneOf property must include a discriminator property to define the exact type.';
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
if (!schema.discriminator || typeof schema.discriminator !== 'object' || !schema.discriminator.propertyName) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Nit] line 29-47 can be included in checkViolationsAndReturnErrors function

return collectAndReturnViolation(path, RULE_NAME, [{ message: ERROR_MESSAGE }]);
}

if (!schema.discriminator.mapping) {
return collectAndReturnViolation(path, RULE_NAME, [{ message: ERROR_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);
};
Loading