From c2985279d3ae127c131670b02761b8f9aa07e583 Mon Sep 17 00:00:00 2001 From: wtrocki Date: Thu, 20 Mar 2025 19:10:38 +0100 Subject: [PATCH 1/2] CLOUDP-306580: avoid mixed refs --- .../__tests__/IPA125OneOfNoBaseTypes.test.js | 25 +++++++++++-- tools/spectral/ipa/rulesets/IPA-125.yaml | 37 +++---------------- .../functions/IPA125OneOfNoBaseTypes.js | 35 +++++++++++++----- 3 files changed, 53 insertions(+), 44 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js index 401a2a7996..ef5828fa0b 100644 --- a/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js +++ b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js @@ -70,9 +70,9 @@ testRule('xgen-IPA-125-oneOf-no-base-types', [ errors: [ { code: 'xgen-IPA-125-oneOf-no-base-types', - message: 'oneOf should not contain base types like integer, number, string, or boolean.', + message: 'oneOf should not mix base types with references.', path: ['components', 'schemas', 'MixedType'], - severity: DiagnosticSeverity.Warning, + severity: DiagnosticSeverity.Error, }, ], }, @@ -91,12 +91,29 @@ testRule('xgen-IPA-125-oneOf-no-base-types', [ errors: [ { code: 'xgen-IPA-125-oneOf-no-base-types', - message: 'oneOf should not contain base types like integer, number, string, or boolean.', + message: 'oneOf should not contain multiple different base types.', path: ['components', 'schemas', 'BaseTypes'], - severity: DiagnosticSeverity.Warning, + severity: DiagnosticSeverity.Error, }, ], }, + { + name: 'valid oneOf with same base type multiple times', + document: { + components: { + schemas: { + ...componentSchemas, + SameBaseType: { + oneOf: [ + { type: 'string', enum: ['one'] }, + { type: 'string', enum: ['two'] }, + ], + }, + }, + }, + }, + errors: [], + }, { name: 'oneOf with exception', document: { diff --git a/tools/spectral/ipa/rulesets/IPA-125.yaml b/tools/spectral/ipa/rulesets/IPA-125.yaml index 6c87d7fecf..9684bc810a 100644 --- a/tools/spectral/ipa/rulesets/IPA-125.yaml +++ b/tools/spectral/ipa/rulesets/IPA-125.yaml @@ -43,48 +43,23 @@ rules: xgen-IPA-125-oneOf-no-base-types: description: | - API producers should not use oneOf with base types like integer, string, boolean, or number. + API producers should not use oneOf with different base types like integer, string, boolean, or number or references at the same time. ##### Implementation details Rule checks for the following conditions: - Applies to schemas with `oneOf` arrays - - Ensures no element within oneOf has a type property that is a primitive/base type + - Ensures no mixing of base types with references + - Ensures no multiple different base types in the same oneOf - Base types considered are: integer, string, boolean, number + - Using the same base type multiple times is allowed (e.g., multiple string enums) ##### Rationale - Using oneOf with primitive types can lead to ambiguity and validation problems. Clients may not + Using oneOf with multiple primitive types can lead to ambiguity and validation problems. Clients may not be able to properly determine which type to use in which context. Instead, use more specific object types with clear discriminators. - ##### Example Violation - ```yaml - # Incorrect - Using oneOf with base types - type: object - properties: - value: - oneOf: - - type: string - - type: integer - ``` - - ##### Example Compliance - ```yaml - # Correct - Using oneOf with object types only - type: object - properties: - value: - oneOf: - - $ref: '#/components/schemas/StringValue' - - $ref: '#/components/schemas/IntegerValue' - discriminator: - propertyName: valueType - mapping: - string: '#/components/schemas/StringValue' - integer: '#/components/schemas/IntegerValue' - ``` - message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-125-oneOf-no-base-types' - severity: warn + severity: error given: '$.components.schemas[*]' then: function: 'IPA125OneOfNoBaseTypes' diff --git a/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js b/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js index c9aa54394f..9bb3b2f89c 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js +++ b/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js @@ -3,7 +3,8 @@ import { resolveObject } from './utils/componentUtils.js'; import { hasException } from './utils/exceptions.js'; const RULE_NAME = 'xgen-IPA-125-oneOf-no-base-types'; -const ERROR_MESSAGE = 'oneOf should not contain base types like integer, number, string, or boolean.'; +const ERROR_MESSAGE_MIXED = 'oneOf should not mix base types with references.'; +const ERROR_MESSAGE_MULTIPLE = 'oneOf should not contain multiple different base types.'; export default (input, _, { path, documentInventory }) => { if (!input.oneOf || !Array.isArray(input.oneOf)) { @@ -22,14 +23,30 @@ export default (input, _, { path, documentInventory }) => { }; function checkViolationsAndReturnErrors(schema, path) { - // Check if any oneOf item is a base type - const baseTypes = ['integer', 'number', 'string', 'boolean']; - const hasBaseType = schema.oneOf.some( - (item) => typeof item === 'object' && item.type && baseTypes.includes(item.type) - ); - - if (hasBaseType) { - return [{ path, message: ERROR_MESSAGE }]; + const baseTypes = ['string', 'number', 'integer', 'boolean']; + const foundBaseTypes = new Set(); + let hasRef = false; + let hasBaseType = false; + + // Check each oneOf item + for (const item of schema.oneOf) { + if (item.$ref) { + hasRef = true; + continue; + } + + if (item.type && baseTypes.includes(item.type)) { + hasBaseType = true; + foundBaseTypes.add(item.type); + } + } + + if (hasRef && hasBaseType) { + return [{ path, message: ERROR_MESSAGE_MIXED }]; + } + + if (foundBaseTypes.size > 1) { + return [{ path, message: ERROR_MESSAGE_MULTIPLE }]; } return []; From 294235f0023fc256c6ed3b83980d3d87bd9460cf Mon Sep 17 00:00:00 2001 From: wtrocki Date: Thu, 20 Mar 2025 19:19:08 +0100 Subject: [PATCH 2/2] docs --- .../__tests__/IPA125OneOfNoBaseTypes.test.js | 4 +-- tools/spectral/ipa/rulesets/IPA-125.yaml | 2 +- tools/spectral/ipa/rulesets/README.md | 35 +++---------------- 3 files changed, 8 insertions(+), 33 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js index ef5828fa0b..24867dfee1 100644 --- a/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js +++ b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js @@ -72,7 +72,7 @@ testRule('xgen-IPA-125-oneOf-no-base-types', [ code: 'xgen-IPA-125-oneOf-no-base-types', message: 'oneOf should not mix base types with references.', path: ['components', 'schemas', 'MixedType'], - severity: DiagnosticSeverity.Error, + severity: DiagnosticSeverity.Warning, }, ], }, @@ -93,7 +93,7 @@ testRule('xgen-IPA-125-oneOf-no-base-types', [ code: 'xgen-IPA-125-oneOf-no-base-types', message: 'oneOf should not contain multiple different base types.', path: ['components', 'schemas', 'BaseTypes'], - severity: DiagnosticSeverity.Error, + severity: DiagnosticSeverity.Warning, }, ], }, diff --git a/tools/spectral/ipa/rulesets/IPA-125.yaml b/tools/spectral/ipa/rulesets/IPA-125.yaml index 9684bc810a..0de89e318d 100644 --- a/tools/spectral/ipa/rulesets/IPA-125.yaml +++ b/tools/spectral/ipa/rulesets/IPA-125.yaml @@ -59,7 +59,7 @@ rules: object types with clear discriminators. message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-125-oneOf-no-base-types' - severity: error + severity: warn given: '$.components.schemas[*]' then: function: 'IPA125OneOfNoBaseTypes' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 184732fd1f..962f8d4d32 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -560,46 +560,21 @@ Rule checks for the following conditions: #### xgen-IPA-125-oneOf-no-base-types ![warn](https://img.shields.io/badge/warning-yellow) -API producers should not use oneOf with base types like integer, string, boolean, or number. +API producers should not use oneOf with different base types like integer, string, boolean, or number or references at the same time. ##### Implementation details Rule checks for the following conditions: - Applies to schemas with `oneOf` arrays - - Ensures no element within oneOf has a type property that is a primitive/base type + - Ensures no mixing of base types with references + - Ensures no multiple different base types in the same oneOf - Base types considered are: integer, string, boolean, number + - Using the same base type multiple times is allowed (e.g., multiple string enums) ##### Rationale -Using oneOf with primitive types can lead to ambiguity and validation problems. Clients may not +Using oneOf with multiple primitive types can lead to ambiguity and validation problems. Clients may not be able to properly determine which type to use in which context. Instead, use more specific object types with clear discriminators. -##### Example Violation -```yaml -# Incorrect - Using oneOf with base types -type: object -properties: - value: - oneOf: - - type: string - - type: integer -``` - -##### Example Compliance -```yaml -# Correct - Using oneOf with object types only -type: object -properties: - value: - oneOf: - - $ref: '#/components/schemas/StringValue' - - $ref: '#/components/schemas/IntegerValue' - discriminator: - propertyName: valueType - mapping: - string: '#/components/schemas/StringValue' - integer: '#/components/schemas/IntegerValue' -``` -