Skip to content

Commit 9dd553e

Browse files
wtrockiwtrocki
authored
CLOUDP-306580: IPA 125 - Each oneOf property must be accompanied with discriminator (#572)
1 parent 8e4dbf6 commit 9dd553e

File tree

4 files changed

+294
-0
lines changed

4 files changed

+294
-0
lines changed

‎tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
import testRule from './__helpers__/testRule';
2+
import { DiagnosticSeverity } from '@stoplight/types';
3+
4+
const componentSchemas = {
5+
schemas: {
6+
Dog: {
7+
type: 'object',
8+
properties: {
9+
breed: { type: 'string' },
10+
age: { type: 'integer' },
11+
},
12+
},
13+
Cat: {
14+
type: 'object',
15+
properties: {
16+
color: { type: 'string' },
17+
livesLeft: { type: 'integer' },
18+
},
19+
},
20+
Bird: {
21+
type: 'object',
22+
properties: {
23+
species: { type: 'string' },
24+
wingspan: { type: 'number' },
25+
},
26+
},
27+
Fish: {
28+
type: 'object',
29+
properties: {
30+
species: { type: 'string' },
31+
waterType: { type: 'string' },
32+
},
33+
},
34+
},
35+
};
36+
37+
testRule('xgen-IPA-125-oneOf-must-have-discriminator', [
38+
{
39+
name: 'valid oneOf with discriminator and matching mapping',
40+
document: {
41+
components: componentSchemas,
42+
schemas: {
43+
Animal: {
44+
oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }],
45+
discriminator: {
46+
propertyName: 'type',
47+
mapping: {
48+
dog: '#/components/schemas/Dog',
49+
cat: '#/components/schemas/Cat',
50+
},
51+
},
52+
},
53+
},
54+
},
55+
errors: [],
56+
},
57+
{
58+
name: 'invalid oneOf with discriminator but mismatched mapping',
59+
document: {
60+
components: componentSchemas,
61+
schemas: {
62+
Animal: {
63+
oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }],
64+
discriminator: {
65+
propertyName: 'type',
66+
mapping: {
67+
dog: '#/components/schemas/Dog',
68+
bird: '#/components/schemas/Bird',
69+
},
70+
},
71+
},
72+
},
73+
},
74+
errors: [
75+
{
76+
code: 'xgen-IPA-125-oneOf-must-have-discriminator',
77+
message:
78+
'The discriminator mapping must match the oneOf references. Unmatched Discriminator mappings with oneOf references: #/components/schemas/Bird',
79+
path: ['schemas', 'Animal'],
80+
severity: DiagnosticSeverity.Error,
81+
},
82+
],
83+
},
84+
{
85+
name: 'invalid oneOf without discriminator',
86+
document: {
87+
components: componentSchemas,
88+
schemas: {
89+
Animal: {
90+
oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }],
91+
},
92+
},
93+
},
94+
errors: [
95+
{
96+
code: 'xgen-IPA-125-oneOf-must-have-discriminator',
97+
message: 'The schema has oneOf but no discriminator property.',
98+
path: ['schemas', 'Animal'],
99+
severity: DiagnosticSeverity.Error,
100+
},
101+
],
102+
},
103+
{
104+
name: 'invalid oneOf with non-object discriminator',
105+
document: {
106+
components: componentSchemas,
107+
schemas: {
108+
Animal: {
109+
oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }],
110+
discriminator: "I'm a string, not an object!",
111+
},
112+
},
113+
},
114+
errors: [
115+
{
116+
code: 'xgen-IPA-125-oneOf-must-have-discriminator',
117+
message: 'Discriminator property is not an object.',
118+
path: ['schemas', 'Animal'],
119+
severity: DiagnosticSeverity.Error,
120+
},
121+
],
122+
},
123+
{
124+
name: 'invalid oneOf with discriminator but no propertyName',
125+
document: {
126+
components: componentSchemas,
127+
schemas: {
128+
Animal: {
129+
oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }],
130+
discriminator: {
131+
mapping: {
132+
dog: '#/components/schemas/Dog',
133+
cat: '#/components/schemas/Cat',
134+
},
135+
},
136+
},
137+
},
138+
},
139+
errors: [
140+
{
141+
code: 'xgen-IPA-125-oneOf-must-have-discriminator',
142+
message: 'Discriminator has no propertyName defined.',
143+
path: ['schemas', 'Animal'],
144+
severity: DiagnosticSeverity.Error,
145+
},
146+
],
147+
},
148+
{
149+
name: 'invalid oneOf with discriminator but no mapping',
150+
document: {
151+
components: componentSchemas,
152+
schemas: {
153+
Animal: {
154+
oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }],
155+
discriminator: {
156+
propertyName: 'type',
157+
},
158+
},
159+
},
160+
},
161+
errors: [
162+
{
163+
code: 'xgen-IPA-125-oneOf-must-have-discriminator',
164+
message: 'Discriminator must have a mapping object.',
165+
path: ['schemas', 'Animal'],
166+
severity: DiagnosticSeverity.Error,
167+
},
168+
],
169+
},
170+
{
171+
name: 'oneOf with discriminator exemption',
172+
document: {
173+
components: componentSchemas,
174+
schemas: {
175+
Animal: {
176+
oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }],
177+
'x-xgen-IPA-exception': {
178+
'xgen-IPA-125-oneOf-must-have-discriminator': 'reason for exemption',
179+
},
180+
},
181+
},
182+
},
183+
errors: [],
184+
},
185+
]);

‎tools/spectral/ipa/__tests__/__helpers__/testRule.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { describe, expect, it } from '@jest/globals';
44
import { Spectral, Document } from '@stoplight/spectral-core';
55
import { httpAndFileResolver } from '@stoplight/spectral-ref-resolver';
66
import { bundleAndLoadRuleset } from '@stoplight/spectral-ruleset-bundler/with-loader';
7+
import { fail } from 'node:assert';
78

89
export default (ruleName, tests) => {
910
describe(`Rule ${ruleName}`, () => {
@@ -13,6 +14,12 @@ export default (ruleName, tests) => {
1314
const doc = testCase.document instanceof Document ? testCase.document : JSON.stringify(testCase.document);
1415
const errors = await s.run(doc);
1516

17+
if (testCase.errors.length !== errors.length) {
18+
fail(`Expected errors do not match actual errors
19+
(${testCase.errors.length} !== ${errors.length})
20+
Expected errors: ${JSON.stringify(testCase.errors, undefined, 2)}
21+
Actual errors: ${JSON.stringify(errors, undefined, 2)}`);
22+
}
1623
expect(errors.length).toEqual(testCase.errors.length);
1724

1825
errors.forEach((error, index) => {

‎tools/spectral/ipa/rulesets/IPA-125.yaml‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# IPA-125: Single Type in Request and Response
2+
# http://go/ipa/125
3+
4+
functions:
5+
- IPA125OneOfMustHaveDiscriminator
6+
7+
rules:
8+
xgen-IPA-125-oneOf-must-have-discriminator:
9+
description: |
10+
Each oneOf property must include a discriminator property to define the exact type.
11+
12+
##### Implementation details
13+
Rule checks for the following conditions:
14+
- Applies only to schemas with `oneOf` containing references
15+
- Ensures a `discriminator` property is present with a valid `propertyName`
16+
- Validates that `discriminator.mapping` contains exactly the same number of entries as `oneOf` references
17+
- Validates that each `discriminator.mapping` value matches a reference in the `oneOf` array
18+
- Ignores `oneOf` definitions with inline schemas
19+
20+
##### Matching Logic
21+
- The `discriminator.mapping` must have the same number of entries as there are references in the `oneOf` array
22+
- Each value in the `discriminator.mapping` must match one of the `$ref` values in the `oneOf` array
23+
- Each `$ref` in the `oneOf` array must have a corresponding entry in the `discriminator.mapping`
24+
- Example:
25+
```yaml
26+
oneOf:
27+
- $ref: '#/components/schemas/Dog'
28+
- $ref: '#/components/schemas/Cat'
29+
discriminator:
30+
propertyName: type
31+
mapping:
32+
dog: '#/components/schemas/Dog'
33+
cat: '#/components/schemas/Cat'
34+
```
35+
This is valid because there are exactly 2 mappings for 2 oneOf references, and all values match.
36+
37+
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-125-oneOf-must-have-discriminator'
38+
severity: error
39+
given: '$..[?(@.oneOf)]'
40+
then:
41+
function: 'IPA125OneOfMustHaveDiscriminator'

‎tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { collectAdoption, collectAndReturnViolation } from './utils/collectionUtils.js';
2+
import { resolveObject } from './utils/componentUtils.js';
3+
import { hasException } from './utils/exceptions.js';
4+
5+
const RULE_NAME = 'xgen-IPA-125-oneOf-must-have-discriminator';
6+
const MISSING_DISCRIMINATOR_MESSAGE = 'The schema has oneOf but no discriminator property.';
7+
const INVALID_DISCRIMINATOR_MESSAGE = 'Discriminator property is not an object.';
8+
const MISSING_PROPERTY_NAME_MESSAGE = 'Discriminator has no propertyName defined.';
9+
const MISSING_MAPPING_MESSAGE = 'Discriminator must have a mapping object.';
10+
const MAPPING_ERROR_MESSAGE =
11+
'The discriminator mapping must match the oneOf references. Unmatched Discriminator mappings with oneOf references:';
12+
13+
export default (input, _, { path, documentInventory }) => {
14+
const oas = documentInventory.unresolved; // Use unresolved document to access raw $ref
15+
const schema = resolveObject(oas, path);
16+
17+
if (!schema || !schema.oneOf || !Array.isArray(schema.oneOf)) {
18+
return;
19+
}
20+
21+
// Check for exception first
22+
if (hasException(schema, RULE_NAME)) {
23+
return;
24+
}
25+
26+
// Check if all oneOf items are objects with a $ref property
27+
const allReferences = schema.oneOf.every((item) => typeof item === 'object' && item.$ref);
28+
if (!allReferences) {
29+
return;
30+
}
31+
32+
// Validate the presence of a discriminator with more specific error messages
33+
if (!schema.discriminator) {
34+
return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_DISCRIMINATOR_MESSAGE }]);
35+
}
36+
37+
if (typeof schema.discriminator !== 'object') {
38+
return collectAndReturnViolation(path, RULE_NAME, [{ message: INVALID_DISCRIMINATOR_MESSAGE }]);
39+
}
40+
41+
if (!schema.discriminator.propertyName) {
42+
return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_PROPERTY_NAME_MESSAGE }]);
43+
}
44+
45+
if (!schema.discriminator.mapping) {
46+
return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_MAPPING_MESSAGE }]);
47+
}
48+
49+
const oneOfRefs = schema.oneOf.map((item) => item.$ref);
50+
const mappingValues = Object.values(schema.discriminator.mapping);
51+
52+
// Check for discriminator mappings that don't match any oneOf reference
53+
const unmatchedMappings = mappingValues.filter((mapping) => !oneOfRefs.includes(mapping));
54+
if (unmatchedMappings.length > 0) {
55+
return collectAndReturnViolation(path, RULE_NAME, [
56+
{ message: `${MAPPING_ERROR_MESSAGE} ${unmatchedMappings.join(', ')}` },
57+
]);
58+
}
59+
60+
collectAdoption(path, RULE_NAME);
61+
};

0 commit comments

Comments
 (0)