From 7779268fd843bab731c86f930ff5c4df8943a347 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 28 Feb 2025 15:05:33 +0000 Subject: [PATCH 1/7] CLOUDP-302982: IPA-104 - Get object should have suffix "Response" --- ...ethodReturnsResponseSuffixedObject.test.js | 181 ++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-104.yaml | 8 + .../getMethodReturnsResponseSuffixedObject.js | 40 ++++ .../rulesets/functions/utils/methodUtils.js | 51 ++++- 4 files changed, 274 insertions(+), 6 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js diff --git a/tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js b/tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js new file mode 100644 index 0000000000..aab8bd686e --- /dev/null +++ b/tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js @@ -0,0 +1,181 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + schemas: { + ExampleResponse: { + properties: { + exampleProperty: { + type: 'string', + }, + }, + }, + ExampleRequest: { + properties: { + exampleProperty: { + type: 'string', + }, + }, + }, + }, +}; + +testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ + { + name: 'valid schema names names', + document: { + paths: { + '/resource1/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ExampleResponse', + }, + }, + }, + }, + }, + }, + }, + '/resource2/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + type: 'string', + }, + }, + }, + }, + }, + }, + '/singleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ExampleResponse', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [], + }, + { + name: 'invalid resources', + document: { + paths: { + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ExampleRequest', + }, + }, + }, + }, + }, + }, + }, + '/singleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ExampleRequest', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [ + { + code: 'xgen-IPA-104-get-method-returns-response-suffixed-object', + message: 'ExampleRequest schema name should end in "Response". http://go/ipa/104', + path: [ + 'paths', + '/resource/{id}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-104-get-method-returns-response-suffixed-object', + message: 'ExampleRequest schema name should end in "Response". http://go/ipa/104', + path: ['paths', '/singleton', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-08-05+json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid resources with exceptions', + document: { + paths: { + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-returns-response-suffixed-object': 'reason', + }, + schema: { + $ref: '#/components/schemas/ExampleRequest', + }, + }, + }, + }, + }, + }, + }, + '/singleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-returns-response-suffixed-object': 'reason', + }, + schema: { + $ref: '#/components/schemas/ExampleRequest', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-104.yaml b/tools/spectral/ipa/rulesets/IPA-104.yaml index 5dc44a952a..fdaa18e6d6 100644 --- a/tools/spectral/ipa/rulesets/IPA-104.yaml +++ b/tools/spectral/ipa/rulesets/IPA-104.yaml @@ -4,6 +4,7 @@ functions: - eachResourceHasGetMethod - getMethodReturnsSingleResource + - getMethodReturnsResponseSuffixedObject rules: xgen-IPA-104-resource-has-GET: @@ -21,3 +22,10 @@ rules: given: '$.paths[*].get' then: function: 'getMethodReturnsSingleResource' + xgen-IPA-104-get-method-returns-response-suffixed-object: + description: 'The get method of a resource should return a "Response" suffixed object. http://go/ipa/104' + message: '{{error}} http://go/ipa/104' + severity: warn + given: '$.paths[*].get' + then: + function: 'getMethodReturnsResponseSuffixedObject' diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js new file mode 100644 index 0000000000..947cf1f526 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js @@ -0,0 +1,40 @@ +import { isChild, isCustomMethod, isSingletonResource, getResourcePaths } from './utils/resourceEvaluation.js'; +import { getAllSuccessfulResponseSchemaNames } from './utils/methodUtils.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { hasException } from './utils/exceptions.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; + +const RULE_NAME = 'xgen-IPA-104-get-method-returns-response-suffixed-object'; +const ERROR_MESSAGE = 'schema name should end in "Response".'; + +export default (input, _, { path, document }) => { + const resourcePath = path[1]; + const oas = document.data; + const resourcePaths = getResourcePaths(resourcePath, Object.keys(oas.paths)); + + if (isCustomMethod(resourcePath) || (!isChild(resourcePath) && !isSingletonResource(resourcePaths))) { + return; + } + + const errors = []; + + const responseSchemaNames = getAllSuccessfulResponseSchemaNames(oas.paths[resourcePath].get); + responseSchemaNames.forEach(({ schemaName, schemaPath }) => { + const fullPath = path.concat(schemaPath); + const responseObject = resolveObject(oas, fullPath); + + if (hasException(responseObject, RULE_NAME)) { + collectException(responseObject, RULE_NAME, fullPath); + } else if (!schemaName.endsWith('Response')) { + collectAndReturnViolation(fullPath, RULE_NAME, `${schemaName} ${ERROR_MESSAGE}`); + errors.push({ + path: fullPath, + message: `${schemaName} ${ERROR_MESSAGE}`, + }); + } else { + collectAdoption(fullPath, RULE_NAME); + } + }); + + return errors; +}; diff --git a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js index 8781360966..6e8447878e 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js @@ -1,10 +1,48 @@ /** * Returns a list of all successful response schemas for the passed operation, i.e. for any 2xx response. + * If a response doesn't have any schema property, an empty array is returned. * * @param {object} operationObject the object for the operation - * @returns {Object[{schemaPath: Array, schema: Object}]} all 2xx response schemas and the path to each schema + * @returns {Array<{schemaPath: Array, schema: Object}>} all 2xx response schemas and the path to each schema */ export function getAllSuccessfulResponseSchemas(operationObject) { + const getSchema = (schema) => { + return { schema }; + }; + + return forEachSuccessfulResponseSchema(operationObject, getSchema); +} + +/** + * Returns a list of all Schema names for successful responses for the passed operation, i.e. for any 2xx response. + * If a response doesn't have any schema property or the schemas are unnamed, an empty array is returned. + * + * @param {object} operationObject the unresolved object for the operation, containing "$ref" references to any schemas + * @returns {Array<{schemaPath: Array, schemaName: Object}>} all 2xx response schema names and the path to each schema + */ +export function getAllSuccessfulResponseSchemaNames(operationObject) { + const getSchemaNameFromSchema = (schema) => { + if (Object.keys(schema).includes('$ref')) { + const schemaRefSections = schema['$ref'].split('/'); + return { + schemaName: schemaRefSections[schemaRefSections.length - 1], + }; + } + }; + + return forEachSuccessfulResponseSchema(operationObject, getSchemaNameFromSchema); +} + +/** + * For each successful response schema, applies the passed function forEachGet to each schema, + * and returns the result from the forEachGet as well as the path to the response schema. + * + * @param operationObject the operationObject + * @param {(schema: string) => Object} forEachGet a function applied to each schema in the operationObject + * @returns {Object[{schemaPath: Array, any}]} an array of all 2xx response schema paths and the result of forEachGet for each schema + * @returns {Array} an array of all 2xx response schema paths and the result of forEachGet for each schema, for example [{schemaPath: string[], someProperty: Object}] + */ +function forEachSuccessfulResponseSchema(operationObject, forEachGet) { const path = []; const responses = operationObject['responses']; @@ -19,12 +57,13 @@ export function getAllSuccessfulResponseSchemas(operationObject) { const result = []; Object.keys(responseContent).forEach((k) => { const schema = responseContent[k]['schema']; - const schemaPath = path.concat([k]); if (schema) { - result.push({ - schemaPath, - schema, - }); + const schemaPath = path.concat([k]); + const schemaProperty = forEachGet(schema); + if (schemaProperty) { + schemaProperty.schemaPath = schemaPath; + result.push(schemaProperty); + } } }); return result; From a2e127cb3c26bf1444126d4f531e0396011c207b Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 6 Mar 2025 14:51:54 +0000 Subject: [PATCH 2/7] CLOUDP-302982: Fix --- ...ethodReturnsResponseSuffixedObject.test.js | 142 ++++++++++++++---- tools/spectral/ipa/rulesets/IPA-104.yaml | 3 +- ...ethodRequestBodyIsRequestSuffixedObject.js | 25 +-- .../getMethodReturnsResponseSuffixedObject.js | 54 ++++--- .../rulesets/functions/utils/methodUtils.js | 64 +++----- 5 files changed, 167 insertions(+), 121 deletions(-) diff --git a/tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js b/tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js index aab8bd686e..17a91427f1 100644 --- a/tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js +++ b/tools/spectral/ipa/__tests__/getMethodReturnsResponseSuffixedObject.test.js @@ -3,19 +3,11 @@ import { DiagnosticSeverity } from '@stoplight/types'; const componentSchemas = { schemas: { - ExampleResponse: { - properties: { - exampleProperty: { - type: 'string', - }, - }, + SchemaResponse: { + type: 'object', }, - ExampleRequest: { - properties: { - exampleProperty: { - type: 'string', - }, - }, + Schema: { + type: 'object', }, }, }; @@ -25,14 +17,36 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ name: 'valid schema names names', document: { paths: { - '/resource1/{id}': { + '/resource/{id}': { get: { responses: { 200: { content: { - 'application/vnd.atlas.2024-08-05+json': { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaResponse', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaResponse', + }, + }, + 'application/vnd.atlas.2025-01-01+json': { + schema: { + type: 'array', + items: { + $ref: '#/components/schemas/SchemaResponse', + }, + }, + }, + }, + }, + 400: { + content: { + 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/ExampleResponse', + $ref: '#/components/schemas/Schema', }, }, }, @@ -40,7 +54,7 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ }, }, }, - '/resource2/{id}': { + '/resourceTwo/{id}': { get: { responses: { 200: { @@ -53,14 +67,14 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ }, }, }, - '/singleton': { + '/resource/{id}/singleton': { get: { responses: { 200: { content: { 'application/vnd.atlas.2024-08-05+json': { schema: { - $ref: '#/components/schemas/ExampleResponse', + $ref: '#/components/schemas/SchemaResponse', }, }, }, @@ -82,9 +96,22 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ responses: { 200: { content: { - 'application/vnd.atlas.2024-08-05+json': { + 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/ExampleRequest', + $ref: '#/components/schemas/Schema', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + 'application/vnd.atlas.2025-01-01+json': { + schema: { + type: 'array', + items: { + $ref: '#/components/schemas/Schema', + }, }, }, }, @@ -92,14 +119,14 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ }, }, }, - '/singleton': { + '/resource/{id}/singleton': { get: { responses: { 200: { content: { 'application/vnd.atlas.2024-08-05+json': { schema: { - $ref: '#/components/schemas/ExampleRequest', + $ref: '#/components/schemas/Schema', }, }, }, @@ -113,7 +140,7 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ errors: [ { code: 'xgen-IPA-104-get-method-returns-response-suffixed-object', - message: 'ExampleRequest schema name should end in "Response". http://go/ipa/104', + message: 'The request schema must reference a schema with a Response suffix. http://go/ipa/104', path: [ 'paths', '/resource/{id}', @@ -121,14 +148,50 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ 'responses', '200', 'content', - 'application/vnd.atlas.2024-08-05+json', + 'application/vnd.atlas.2023-01-01+json', ], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-104-get-method-returns-response-suffixed-object', - message: 'ExampleRequest schema name should end in "Response". http://go/ipa/104', - path: ['paths', '/singleton', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-08-05+json'], + message: 'The request schema must reference a schema with a Response suffix. http://go/ipa/104', + path: [ + 'paths', + '/resource/{id}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-01-01+json', + ], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-104-get-method-returns-response-suffixed-object', + message: 'The request schema must reference a schema with a Response suffix. http://go/ipa/104', + path: [ + 'paths', + '/resource/{id}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2025-01-01+json', + ], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-104-get-method-returns-response-suffixed-object', + message: 'The request schema must reference a schema with a Response suffix. http://go/ipa/104', + path: [ + 'paths', + '/resource/{id}/singleton', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], severity: DiagnosticSeverity.Warning, }, ], @@ -142,12 +205,31 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ responses: { 200: { content: { - 'application/vnd.atlas.2024-08-05+json': { + 'application/vnd.atlas.2023-01-01+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-returns-response-suffixed-object': 'reason', + }, + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-returns-response-suffixed-object': 'reason', + }, + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + 'application/vnd.atlas.2025-01-01+json': { 'x-xgen-IPA-exception': { 'xgen-IPA-104-get-method-returns-response-suffixed-object': 'reason', }, schema: { - $ref: '#/components/schemas/ExampleRequest', + type: 'array', + items: { + $ref: '#/components/schemas/Schema', + }, }, }, }, @@ -155,7 +237,7 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ }, }, }, - '/singleton': { + '/resource/{id}/singleton': { get: { responses: { 200: { @@ -165,7 +247,7 @@ testRule('xgen-IPA-104-get-method-returns-response-suffixed-object', [ 'xgen-IPA-104-get-method-returns-response-suffixed-object': 'reason', }, schema: { - $ref: '#/components/schemas/ExampleRequest', + $ref: '#/components/schemas/Schema', }, }, }, diff --git a/tools/spectral/ipa/rulesets/IPA-104.yaml b/tools/spectral/ipa/rulesets/IPA-104.yaml index 4c1ca78a56..afa98a8b70 100644 --- a/tools/spectral/ipa/rulesets/IPA-104.yaml +++ b/tools/spectral/ipa/rulesets/IPA-104.yaml @@ -34,6 +34,7 @@ rules: description: 'The get method of a resource should return a "Response" suffixed object. http://go/ipa/104' message: '{{error}} http://go/ipa/104' severity: warn - given: '$.paths[*].get' + given: '$.paths[*].get.responses[*].content' then: + field: '@key' function: 'getMethodReturnsResponseSuffixedObject' diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js index edb96b029c..50a74a8edd 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js @@ -2,6 +2,7 @@ import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; import { resolveObject } from './utils/componentUtils.js'; +import { getSchemaRef } from './utils/methodUtils.js'; const RULE_NAME = 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object'; const ERROR_MESSAGE_SCHEMA_NAME = 'The response body schema must reference a schema with a Request suffix.'; @@ -23,25 +24,15 @@ export default (input, _, { path, documentInventory }) => { } if (contentPerMediaType.schema) { + console.log(contentPerMediaType); const schema = contentPerMediaType.schema; - if (schema.type === 'array' && schema.items) { - let schemaItems = schema.items; - if (!schemaItems.$ref) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF); - } - if (!schemaItems.$ref.endsWith('Request')) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME); - } - } else { - if (!schema.$ref) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF); - } - - if (!schema.$ref.endsWith('Request')) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME); - } + const schemaName = getSchemaRef(schema); + if (!schemaName) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF); + } + if (!schemaName.endsWith('Request')) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME); } - collectAdoption(path, RULE_NAME); } }; diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js index ea44eca34a..805f712ec4 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js @@ -1,48 +1,46 @@ import { isSingleResourceIdentifier, - isCustomMethodIdentifier, isSingletonResource, getResourcePathItems, + isResourceCollectionIdentifier, } from './utils/resourceEvaluation.js'; -import { getAllSuccessfulResponseSchemaNames } from './utils/methodUtils.js'; import { resolveObject } from './utils/componentUtils.js'; import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { getSchemaRef } from './utils/methodUtils.js'; const RULE_NAME = 'xgen-IPA-104-get-method-returns-response-suffixed-object'; -const ERROR_MESSAGE = 'schema name should end in "Response".'; +const ERROR_MESSAGE_SCHEMA_NAME = 'The request schema must reference a schema with a Response suffix.'; +const ERROR_MESSAGE_SCHEMA_REF = 'The response body schema is defined inline and must reference a predefined schema.'; -export default (input, _, { path, document }) => { +export default (input, _, { path, documentInventory }) => { const resourcePath = path[1]; - const oas = document.data; + const responseCode = path[4]; + const oas = documentInventory.unresolved; const resourcePaths = getResourcePathItems(resourcePath, oas.paths); if ( - isCustomMethodIdentifier(resourcePath) || - (!isSingleResourceIdentifier(resourcePath) && !isSingletonResource(resourcePaths)) + responseCode.startsWith('2') || + isResourceCollectionIdentifier(resourcePath) || + (isSingleResourceIdentifier(resourcePath) && isSingletonResource(resourcePaths)) ) { - return; - } - - const errors = []; - - const responseSchemaNames = getAllSuccessfulResponseSchemaNames(oas.paths[resourcePath].get); - responseSchemaNames.forEach(({ schemaName, schemaPath }) => { - const fullPath = path.concat(schemaPath); - const responseObject = resolveObject(oas, fullPath); + const contentPerMediaType = resolveObject(oas, path); - if (hasException(responseObject, RULE_NAME)) { - collectException(responseObject, RULE_NAME, fullPath); - } else if (!schemaName.endsWith('Response')) { - collectAndReturnViolation(fullPath, RULE_NAME, `${schemaName} ${ERROR_MESSAGE}`); - errors.push({ - path: fullPath, - message: `${schemaName} ${ERROR_MESSAGE}`, - }); - } else { - collectAdoption(fullPath, RULE_NAME); + if (hasException(contentPerMediaType, RULE_NAME)) { + collectException(contentPerMediaType, RULE_NAME, path); + return; } - }); - return errors; + if (contentPerMediaType.schema) { + const schema = contentPerMediaType.schema; + const schemaName = getSchemaRef(schema); + if (!schemaName) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF); + } + if (!schemaName.endsWith('Response')) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME); + } + collectAdoption(path, RULE_NAME); + } + } }; diff --git a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js index 6e8447878e..46467dab57 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js @@ -1,48 +1,10 @@ /** * Returns a list of all successful response schemas for the passed operation, i.e. for any 2xx response. - * If a response doesn't have any schema property, an empty array is returned. * * @param {object} operationObject the object for the operation - * @returns {Array<{schemaPath: Array, schema: Object}>} all 2xx response schemas and the path to each schema + * @returns {Object[{schemaPath: Array, schema: Object}]} all 2xx response schemas and the path to each schema */ export function getAllSuccessfulResponseSchemas(operationObject) { - const getSchema = (schema) => { - return { schema }; - }; - - return forEachSuccessfulResponseSchema(operationObject, getSchema); -} - -/** - * Returns a list of all Schema names for successful responses for the passed operation, i.e. for any 2xx response. - * If a response doesn't have any schema property or the schemas are unnamed, an empty array is returned. - * - * @param {object} operationObject the unresolved object for the operation, containing "$ref" references to any schemas - * @returns {Array<{schemaPath: Array, schemaName: Object}>} all 2xx response schema names and the path to each schema - */ -export function getAllSuccessfulResponseSchemaNames(operationObject) { - const getSchemaNameFromSchema = (schema) => { - if (Object.keys(schema).includes('$ref')) { - const schemaRefSections = schema['$ref'].split('/'); - return { - schemaName: schemaRefSections[schemaRefSections.length - 1], - }; - } - }; - - return forEachSuccessfulResponseSchema(operationObject, getSchemaNameFromSchema); -} - -/** - * For each successful response schema, applies the passed function forEachGet to each schema, - * and returns the result from the forEachGet as well as the path to the response schema. - * - * @param operationObject the operationObject - * @param {(schema: string) => Object} forEachGet a function applied to each schema in the operationObject - * @returns {Object[{schemaPath: Array, any}]} an array of all 2xx response schema paths and the result of forEachGet for each schema - * @returns {Array} an array of all 2xx response schema paths and the result of forEachGet for each schema, for example [{schemaPath: string[], someProperty: Object}] - */ -function forEachSuccessfulResponseSchema(operationObject, forEachGet) { const path = []; const responses = operationObject['responses']; @@ -57,14 +19,26 @@ function forEachSuccessfulResponseSchema(operationObject, forEachGet) { const result = []; Object.keys(responseContent).forEach((k) => { const schema = responseContent[k]['schema']; + const schemaPath = path.concat([k]); if (schema) { - const schemaPath = path.concat([k]); - const schemaProperty = forEachGet(schema); - if (schemaProperty) { - schemaProperty.schemaPath = schemaPath; - result.push(schemaProperty); - } + result.push({ + schemaPath, + schema, + }); } }); return result; } + +/** + * Gets the schema reference for a schema object. If the schema does not have a reference, undefined is returned. + * + * @param {object} schema the schema object + * @returns {string} the schema ref + */ +export function getSchemaRef(schema) { + if (schema.type === 'array' && schema.items) { + return schema.items.$ref; + } + return schema.$ref; +} From 95ec6221ced7f65df7974321c90c39ba74a8d0a9 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 6 Mar 2025 14:55:40 +0000 Subject: [PATCH 3/7] CLOUDP-302982: Fix --- .../createMethodRequestBodyIsRequestSuffixedObject.js | 6 +++--- .../functions/getMethodReturnsResponseSuffixedObject.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js index 50a74a8edd..5e6b7e0aa7 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js @@ -26,11 +26,11 @@ export default (input, _, { path, documentInventory }) => { if (contentPerMediaType.schema) { console.log(contentPerMediaType); const schema = contentPerMediaType.schema; - const schemaName = getSchemaRef(schema); - if (!schemaName) { + const schemaRef = getSchemaRef(schema); + if (!schemaRef) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF); } - if (!schemaName.endsWith('Request')) { + if (!schemaRef.endsWith('Request')) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME); } collectAdoption(path, RULE_NAME); diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js index 805f712ec4..6b964acf2d 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js @@ -33,11 +33,11 @@ export default (input, _, { path, documentInventory }) => { if (contentPerMediaType.schema) { const schema = contentPerMediaType.schema; - const schemaName = getSchemaRef(schema); - if (!schemaName) { + const schemaRef = getSchemaRef(schema); + if (!schemaRef) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF); } - if (!schemaName.endsWith('Response')) { + if (!schemaRef.endsWith('Response')) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME); } collectAdoption(path, RULE_NAME); From c0f679d62f6a7dbe3fd33893d72931045bbfe68e Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 6 Mar 2025 15:10:07 +0000 Subject: [PATCH 4/7] CLOUDP-302982: Fix --- tools/spectral/ipa/rulesets/README.md | 11 ++++++----- .../getMethodReturnsResponseSuffixedObject.js | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 5d12512aee..469d5c3752 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -28,11 +28,12 @@ For rule definitions, see [IPA-102.yaml](https://github.com/mongodb/openapi/blob For rule definitions, see [IPA-104.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-104.yaml). -| Rule Name | Description | Severity | -| ----------------------------------------------- | ----------------------------------------------------------------------------------------- | -------- | -| xgen-IPA-104-resource-has-GET | APIs must provide a get method for resources. http://go/ipa/104 | warn | -| xgen-IPA-104-get-method-returns-single-resource | The purpose of the get method is to return data from a single resource. http://go/ipa/104 | warn | -| xgen-IPA-104-get-method-response-code-is-200 | The Get method must return a 200 OK response. http://go/ipa/104 | warn | +| Rule Name | Description | Severity | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------ | -------- | +| xgen-IPA-104-resource-has-GET | APIs must provide a get method for resources. http://go/ipa/104 | warn | +| xgen-IPA-104-get-method-returns-single-resource | The purpose of the get method is to return data from a single resource. http://go/ipa/104 | warn | +| xgen-IPA-104-get-method-response-code-is-200 | The Get method must return a 200 OK response. http://go/ipa/104 | warn | +| xgen-IPA-104-get-method-returns-response-suffixed-object | The get method of a resource should return a "Response" suffixed object. http://go/ipa/104 | warn | ### IPA-106 diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js index 6b964acf2d..2249b8a14e 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js @@ -20,9 +20,9 @@ export default (input, _, { path, documentInventory }) => { const resourcePaths = getResourcePathItems(resourcePath, oas.paths); if ( - responseCode.startsWith('2') || - isResourceCollectionIdentifier(resourcePath) || - (isSingleResourceIdentifier(resourcePath) && isSingletonResource(resourcePaths)) + responseCode.startsWith('2') && + (isResourceCollectionIdentifier(resourcePath) || + (isSingleResourceIdentifier(resourcePath) && isSingletonResource(resourcePaths))) ) { const contentPerMediaType = resolveObject(oas, path); From 53b04b8fbcebb5b25b2087841e0e2a4cca1573fc Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 6 Mar 2025 15:30:28 +0000 Subject: [PATCH 5/7] CLOUDP-302982: Fix --- .../createMethodRequestBodyIsRequestSuffixedObject.js | 4 ++-- .../functions/getMethodReturnsResponseSuffixedObject.js | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js index 5e6b7e0aa7..3ec368dd59 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js @@ -11,8 +11,9 @@ const ERROR_MESSAGE_SCHEMA_REF = 'The response body schema is defined inline and export default (input, _, { path, documentInventory }) => { const oas = documentInventory.unresolved; const resourcePath = path[1]; + const contentMediaType = path[path.length - 1]; - if (isCustomMethodIdentifier(resourcePath)) { + if (isCustomMethodIdentifier(resourcePath) || !contentMediaType.endsWith('json')) { return; } @@ -24,7 +25,6 @@ export default (input, _, { path, documentInventory }) => { } if (contentPerMediaType.schema) { - console.log(contentPerMediaType); const schema = contentPerMediaType.schema; const schemaRef = getSchemaRef(schema); if (!schemaRef) { diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js index 2249b8a14e..a05fd490ef 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsResponseSuffixedObject.js @@ -16,13 +16,15 @@ const ERROR_MESSAGE_SCHEMA_REF = 'The response body schema is defined inline and export default (input, _, { path, documentInventory }) => { const resourcePath = path[1]; const responseCode = path[4]; + const contentMediaType = path[path.length - 1]; const oas = documentInventory.unresolved; const resourcePaths = getResourcePathItems(resourcePath, oas.paths); if ( responseCode.startsWith('2') && - (isResourceCollectionIdentifier(resourcePath) || - (isSingleResourceIdentifier(resourcePath) && isSingletonResource(resourcePaths))) + contentMediaType.endsWith('json') && + (isSingleResourceIdentifier(resourcePath) || + (isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePaths))) ) { const contentPerMediaType = resolveObject(oas, path); From b8bb4eac939e6d6cd8ab61040fdf6c26183d88a5 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 6 Mar 2025 15:44:19 +0000 Subject: [PATCH 6/7] CLOUDP-302982: Fix --- tools/spectral/ipa/rulesets/IPA-104.yaml | 6 +++--- tools/spectral/ipa/rulesets/README.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/spectral/ipa/rulesets/IPA-104.yaml b/tools/spectral/ipa/rulesets/IPA-104.yaml index afa98a8b70..dcf9e6461f 100644 --- a/tools/spectral/ipa/rulesets/IPA-104.yaml +++ b/tools/spectral/ipa/rulesets/IPA-104.yaml @@ -9,7 +9,7 @@ functions: rules: xgen-IPA-104-resource-has-GET: - description: 'APIs must provide a get method for resources. http://go/ipa/104' + description: 'APIs must provide a Get method for resources. http://go/ipa/104' message: '{{error}} http://go/ipa/104' severity: warn given: '$.paths' @@ -17,7 +17,7 @@ rules: field: '@key' function: 'eachResourceHasGetMethod' xgen-IPA-104-get-method-returns-single-resource: - description: 'The purpose of the get method is to return data from a single resource. http://go/ipa/104' + description: 'The purpose of the Get method is to return data from a single resource. http://go/ipa/104' message: '{{error}} http://go/ipa/104' severity: warn given: '$.paths[*].get' @@ -31,7 +31,7 @@ rules: then: function: 'getResponseCodeShouldBe200OK' xgen-IPA-104-get-method-returns-response-suffixed-object: - description: 'The get method of a resource should return a "Response" suffixed object. http://go/ipa/104' + description: 'The Get method of a resource should return a "Response" suffixed object. http://go/ipa/104' message: '{{error}} http://go/ipa/104' severity: warn given: '$.paths[*].get.responses[*].content' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 469d5c3752..82a7e342a2 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -30,10 +30,10 @@ For rule definitions, see [IPA-104.yaml](https://github.com/mongodb/openapi/blob | Rule Name | Description | Severity | | -------------------------------------------------------- | ------------------------------------------------------------------------------------------ | -------- | -| xgen-IPA-104-resource-has-GET | APIs must provide a get method for resources. http://go/ipa/104 | warn | -| xgen-IPA-104-get-method-returns-single-resource | The purpose of the get method is to return data from a single resource. http://go/ipa/104 | warn | +| xgen-IPA-104-resource-has-GET | APIs must provide a Get method for resources. http://go/ipa/104 | warn | +| xgen-IPA-104-get-method-returns-single-resource | The purpose of the Get method is to return data from a single resource. http://go/ipa/104 | warn | | xgen-IPA-104-get-method-response-code-is-200 | The Get method must return a 200 OK response. http://go/ipa/104 | warn | -| xgen-IPA-104-get-method-returns-response-suffixed-object | The get method of a resource should return a "Response" suffixed object. http://go/ipa/104 | warn | +| xgen-IPA-104-get-method-returns-response-suffixed-object | The Get method of a resource should return a "Response" suffixed object. http://go/ipa/104 | warn | ### IPA-106 From f814fa667fed4ca6bf299662874eb78f67f02b67 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 6 Mar 2025 15:56:12 +0000 Subject: [PATCH 7/7] CLOUDP-302982: Fix comment --- tools/spectral/ipa/rulesets/functions/utils/methodUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js index 46467dab57..0f97df3f52 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js @@ -33,7 +33,7 @@ export function getAllSuccessfulResponseSchemas(operationObject) { /** * Gets the schema reference for a schema object. If the schema does not have a reference, undefined is returned. * - * @param {object} schema the schema object + * @param {object} schema the unresolved schema object * @returns {string} the schema ref */ export function getSchemaRef(schema) {