diff --git a/tools/spectral/ipa/__tests__/IPA125OneOfSchemaPropertySameType.test.js b/tools/spectral/ipa/__tests__/IPA125OneOfSchemaPropertySameType.test.js new file mode 100644 index 0000000000..84fb6306b0 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA125OneOfSchemaPropertySameType.test.js @@ -0,0 +1,689 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-125-oneOf-schema-property-same-type', [ + { + name: 'valid oneOf with same property base types', + document: { + components: { + schemas: { + ExampleSchema: { + oneOf: [ + { + type: 'object', + properties: { + id: { type: 'string' }, + name: { type: 'string' }, + }, + }, + { + type: 'object', + properties: { + id: { type: 'string' }, + nickname: { type: 'string' }, + }, + }, + ], + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid oneOf with different property base types', + document: { + components: { + schemas: { + ExampleSchemaInvalid: { + oneOf: [ + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'number' }, + }, + }, + { + type: 'object', + properties: { + id: { type: 'number' }, // not string + age: { type: 'number' }, + }, + }, + ], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchemaInvalid'], + message: "Property 'id' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid oneOf with multiple different base property types', + document: { + components: { + schemas: { + ExampleSchemaInvalid: { + oneOf: [ + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'string' }, + }, + }, + { + type: 'object', + properties: { + id: { type: 'number' }, // not string + age: { type: 'number' }, // not string + }, + }, + ], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchemaInvalid'], + message: "Property 'id' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchemaInvalid'], + message: "Property 'age' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'exception silences invalid oneOf with multiple different base property types', + document: { + components: { + schemas: { + ExampleSchemaInvalid: { + oneOf: [ + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'string' }, + }, + }, + { + type: 'object', + properties: { + id: { type: 'number' }, + age: { type: 'number' }, + }, + }, + ], + 'x-xgen-IPA-exception': { + 'xgen-IPA-125-oneOf-schema-property-same-type': 'reason for exemption', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid oneOf with same schema for object type property', + document: { + components: { + schemas: { + ExampleSchemaValid: { + oneOf: [ + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'number' }, + name: { + // same schema for object type property + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'string' }, + }, + }, + }, + }, + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'number' }, + name: { + // same schema for object type property + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'string' }, + }, + }, + }, + }, + ], + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid oneOf with different schema for object type property', + document: { + components: { + schemas: { + ExampleSchemaInvalid: { + oneOf: [ + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'number' }, + name: { + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'string' }, + }, + }, + address: { + type: 'object', + properties: { + addressLine: { type: 'string' }, + zip: { type: 'string' }, + }, + }, + }, + }, + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'number' }, + name: { + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'number' }, // not string + }, + }, + address: { + type: 'object', + properties: { + addressLine: { type: 'string' }, // no zip property + }, + }, + }, + }, + ], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchemaInvalid'], + message: "Property 'name' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchemaInvalid'], + message: "Property 'address' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'exception silences invalid oneOf with different schema for object type property', + document: { + components: { + schemas: { + ExampleSchemaInvalid: { + oneOf: [ + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'number' }, + name: { + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'string' }, + }, + }, + address: { + type: 'object', + properties: { + addressLine: { type: 'string' }, + zip: { type: 'string' }, + }, + }, + }, + }, + { + type: 'object', + properties: { + id: { type: 'string' }, + age: { type: 'number' }, + name: { + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'number' }, // not string + }, + }, + address: { + type: 'object', + properties: { + addressLine: { type: 'string' }, // no zip property + }, + }, + }, + }, + ], + 'x-xgen-IPA-exception': { + 'xgen-IPA-125-oneOf-schema-property-same-type': 'reason for exemption', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid deep oneOf', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + properties: { + id: { type: 'string' }, + name: { + oneOf: [ + { + $ref: '#/components/schemas/NameSchema1', + }, + { + $ref: '#/components/schemas/NameSchema2', + }, + ], + }, + }, + }, + NameSchema1: { + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'string' }, + }, + }, + NameSchema2: { + type: 'object', + properties: { + first: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid deep oneOf', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + properties: { + id: { type: 'string' }, + name: { + oneOf: [ + { + $ref: '#/components/schemas/NameSchema1', + }, + { + $ref: '#/components/schemas/NameSchema2', + }, + ], + }, + }, + }, + NameSchema1: { + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'string' }, + }, + }, + NameSchema2: { + type: 'object', + properties: { + first: { type: 'number' }, // not string + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchema', 'properties', 'name'], + message: "Property 'first' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'exception silences invalid deep oneOf', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + properties: { + id: { type: 'string' }, + name: { + oneOf: [ + { + $ref: '#/components/schemas/NameSchema1', + }, + { + $ref: '#/components/schemas/NameSchema2', + }, + ], + 'x-xgen-IPA-exception': { + 'xgen-IPA-125-oneOf-schema-property-same-type': 'reason for exemption', + }, + }, + }, + }, + NameSchema1: { + type: 'object', + properties: { + first: { type: 'string' }, + last: { type: 'string' }, + }, + }, + NameSchema2: { + type: 'object', + properties: { + first: { type: 'number' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid nested oneOf', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + oneOf: [ + { + type: 'object', + properties: { + counter: { + type: 'string', + enum: ['ONE', 'TWO'], + }, + }, + }, + { + type: 'object', + properties: { + counter: { + type: 'string', + enum: ['THREE', 'FOUR'], + }, + }, + }, + { + type: 'object', + properties: { + counter: { + // Nested oneOf is valid since the entries are enums + oneOf: [ + { + type: 'string', + enum: ['FIVE', 'SIX'], + }, + { + type: 'string', + enum: ['5', '6'], + }, + ], + type: 'object', + }, + }, + }, + ], + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid nested oneOf', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + oneOf: [ + { + type: 'object', + properties: { + counter: { + type: 'string', + enum: ['ONE', 'TWO'], + }, + }, + }, + { + type: 'object', + properties: { + counter: { + type: 'string', + enum: ['THREE', 'FOUR'], + }, + }, + }, + { + type: 'object', + properties: { + counter: { + // nested onOf counter is not enum + oneOf: [ + { + type: 'number', + }, + { + type: 'number', + }, + ], + type: 'object', + }, + }, + }, + ], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchema'], + message: "Property 'counter' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'valid nested oneOf with object type', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + oneOf: [ + { + type: 'object', + properties: { + threshold: { + $ref: '#/components/schemas/ThresholdSchema', + }, + }, + }, + { + type: 'object', + properties: { + threshold: { + $ref: '#/components/schemas/ThresholdSchema', + }, + }, + }, + { + type: 'object', + properties: { + threshold: { + type: 'object', + oneOf: [ + { + $ref: '#/components/schemas/ThresholdSchema', + }, + { + $ref: '#/components/schemas/ThresholdSchema', + }, + ], + }, + }, + }, + ], + }, + ThresholdSchema: { + type: 'object', + properties: { + threshold: { + type: 'object', + properties: { + value: { + type: 'number', + }, + unit: { + type: 'string', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid nested oneOf with object type', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + oneOf: [ + { + type: 'object', + properties: { + threshold: { + $ref: '#/components/schemas/ThresholdSchema', + }, + }, + }, + { + type: 'object', + properties: { + threshold: { + $ref: '#/components/schemas/ThresholdSchema', + }, + }, + }, + { + type: 'object', + properties: { + threshold: { + type: 'object', + oneOf: [ + { + $ref: '#/components/schemas/DifferentThresholdSchema', + }, + { + $ref: '#/components/schemas/ThresholdSchema', + }, + ], + }, + }, + }, + ], + }, + ThresholdSchema: { + type: 'object', + properties: { + value: { + type: 'number', + }, + unit: { + type: 'string', + }, + }, + }, + DifferentThresholdSchema: { + type: 'object', + properties: { + value: { + type: 'number', + }, + unit: { + type: 'string', + }, + metricType: { + // Extra parameter + type: 'string', + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-schema-property-same-type', + path: ['components', 'schemas', 'ExampleSchema'], + message: "Property 'threshold' has different types or schemas in oneOf items.", + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/package-lock.json b/tools/spectral/ipa/package-lock.json index f3cf715877..0c8997a2e7 100644 --- a/tools/spectral/ipa/package-lock.json +++ b/tools/spectral/ipa/package-lock.json @@ -12,7 +12,8 @@ "@stoplight/spectral-cli": "6.15.0", "@stoplight/spectral-core": "1.20.0", "@stoplight/spectral-functions": "1.10.1", - "inflection": "3.0.2" + "inflection": "3.0.2", + "lodash": "^4.17.21" }, "devDependencies": { "auto-changelog": "2.5.0" diff --git a/tools/spectral/ipa/package.json b/tools/spectral/ipa/package.json index 9967158307..d0e93f3e6d 100644 --- a/tools/spectral/ipa/package.json +++ b/tools/spectral/ipa/package.json @@ -26,7 +26,8 @@ "@stoplight/spectral-cli": "6.15.0", "@stoplight/spectral-core": "1.20.0", "@stoplight/spectral-functions": "1.10.1", - "inflection": "3.0.2" + "inflection": "3.0.2", + "lodash": "^4.17.21" }, "engineStrict": false, "engines": { diff --git a/tools/spectral/ipa/rulesets/IPA-125.yaml b/tools/spectral/ipa/rulesets/IPA-125.yaml index 21ef3be6e5..4bcd53a84c 100644 --- a/tools/spectral/ipa/rulesets/IPA-125.yaml +++ b/tools/spectral/ipa/rulesets/IPA-125.yaml @@ -4,6 +4,7 @@ functions: - IPA125OneOfMustHaveDiscriminator - IPA125OneOfNoBaseTypes + - IPA125OneOfSchemaPropertySameType rules: xgen-IPA-125-oneOf-must-have-discriminator: @@ -63,3 +64,18 @@ rules: given: '$.components.schemas[*]' then: function: 'IPA125OneOfNoBaseTypes' + + xgen-IPA-125-oneOf-schema-property-same-type: + description: | + If multiple `oneOf` models define a property with the same name, that property **must** have the same base type or schema in each model + + ##### Implementation details + Rule checks for the following conditions: + - Applies only to object type schemas with `oneOf` + - Ensures that if a property is defined in multiple `oneOf` schemas, it must have the same type in each schema (base type or object schema) + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-125-oneOf-schema-property-same-type' + severity: warn + given: '$.components.schemas..oneOf' + then: + function: 'IPA125OneOfSchemaPropertySameType' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index af80e9a52a..0f58a44dd1 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -989,6 +989,16 @@ Using oneOf with multiple primitive types can lead to ambiguity and validation p be able to properly determine which type to use in which context. Instead, use more specific object types with clear discriminators. +#### xgen-IPA-125-oneOf-schema-property-same-type + + ![warn](https://img.shields.io/badge/warning-yellow) +If multiple `oneOf` models define a property with the same name, that property **must** have the same base type or schema in each model + +##### Implementation details +Rule checks for the following conditions: + - Applies only to object type schemas with `oneOf` + - Ensures that if a property is defined in multiple `oneOf` schemas, it must have the same type in each schema (base type or object schema) + ### IPA-126 diff --git a/tools/spectral/ipa/rulesets/functions/IPA125OneOfSchemaPropertySameType.js b/tools/spectral/ipa/rulesets/functions/IPA125OneOfSchemaPropertySameType.js new file mode 100644 index 0000000000..4412b36bd5 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA125OneOfSchemaPropertySameType.js @@ -0,0 +1,100 @@ +import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js'; +import { isEqual, uniq } from 'lodash'; +import { resolveObject } from './utils/componentUtils.js'; + +const RULE_NAME = 'xgen-IPA-125-oneOf-schema-property-same-type'; + +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.resolved; + const schemaPath = path.slice(0, path.length - 1); + const parentSchema = resolveObject(oas, schemaPath); + + // Ignore base types, see IPA125OneOfNoBaseTypes.js + if (input.some((oneOfOption) => oneOfOption.type !== 'object')) { + return; + } + + const errors = checkViolationsAndReturnErrors(input, schemaPath); + return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, parentSchema, schemaPath); +}; + +function checkViolationsAndReturnErrors(schemas, path) { + if (schemas.length > 1) { + const uniquePropertyKeys = getUniquePropertyKeys(schemas); + + const propertiesAcrossMultipleOneOf = []; + uniquePropertyKeys.forEach((property) => { + const propertySchemas = []; + schemas.forEach( + (schema) => schema['properties'][property] && propertySchemas.push(schema['properties'][property]) + ); + + if (propertySchemas.length > 1) { + propertiesAcrossMultipleOneOf.push({ + propertyKey: property, + propertySchemas: propertySchemas, + }); + } + }); + + const errors = []; + propertiesAcrossMultipleOneOf.forEach(({ propertyKey, propertySchemas }) => { + // Check schemas for properties of the same name have the same type/schema + if (!hasSameType(propertySchemas)) { + errors.push({ + path, + message: `Property '${propertyKey}' has different types or schemas in oneOf items.`, + }); + } + }); + return errors; + } + return []; +} + +/** + * Takes a list of schemas and returns all property keys present in the schemas. The list has no duplicates. + * @param {[{}]} schemas all schemas to evaluate + * @returns {string[]} a list of property keys present in the schemas without duplicates + */ +function getUniquePropertyKeys(schemas) { + const properties = []; + schemas.forEach((schema) => properties.push(...Object.keys(schema.properties))); + return uniq(properties); +} + +/** + * Validates that all schemas in the list have the same base type, or the same schema in case of 'object' type + * @param {[{}]} propertySchemas all schemas to evaluate + * @returns {boolean} true if the schemas have the same type or schema + */ +function hasSameType(propertySchemas) { + // Validate properties of type 'object' + if (propertySchemas[0]['type'] === 'object' && !propertySchemas[0]['oneOf']) { + if (allPropsAreObjects(propertySchemas)) { + // Check all property schemas are equal + return propertySchemas.every( + (prop) => isEqual(prop, propertySchemas[0]) || oneOfTypeIsEqual(prop, propertySchemas[0]) + ); + } + return false; + } + + // Validate properties of base type + const expectedType = propertySchemas[0]['oneOf'] + ? propertySchemas[0]['oneOf'][0]['type'] + : propertySchemas[0]['type']; + return propertySchemas.every((prop) => prop['type'] === expectedType || oneOfBaseTypeIsEqual(prop, expectedType)); +} + +function allPropsAreObjects(propertySchemas) { + return propertySchemas.every((prop) => prop['type'] === 'object'); +} + +function oneOfBaseTypeIsEqual(prop, expectedType) { + return prop['oneOf'] && prop['oneOf'][0].type === expectedType; +} + +function oneOfTypeIsEqual(prop, expectedSchema) { + return prop['oneOf'] && isEqual(prop['oneOf'][0], expectedSchema); +}