From 3c40b7afb542b0e127ddd4c4921bb563a0236661 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Fri, 14 Aug 2026 16:00:05 +0100 Subject: [PATCH 01/12] feat(ipa): add IPA-132 OperationResponse schema rules Add the three rules validating how long-running operation status is communicated through the OperationResponse schema: - xgen-IPA-132-operation-endpoints-must-return-operation-response: the single Operation endpoint must reference the OperationResponse schema, and the Operations collection endpoint must return a paginated response whose results reference it. - xgen-IPA-132-operation-status-must-use-the-standard-status-enum: the schema must report progress through a status field using exactly the enum PENDING, IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED. - xgen-IPA-132-operation-response-must-include-core-metadata: operationId, operationType, createdAt and updatedAt must be defined and required, operationType must use exactly the enum CREATE, UPDATE, DELETE, CUSTOM, and customMethod must be defined but not required. The schema name, both enums and the exact-enum-match helper live in utils/longRunningOperations.js for reuse by the remaining IPA-132 rules. --- ...dpointsMustReturnOperationResponse.test.js | 362 ++++++++++++++++++ ...ionResponseMustIncludeCoreMetadata.test.js | 194 ++++++++++ ...StatusMustUseTheStandardStatusEnum.test.js | 225 +++++++++++ .../utils/longRunningOperations.test.js | 24 ++ tools/spectral/ipa/rulesets/IPA-132.yaml | 72 ++++ tools/spectral/ipa/rulesets/README.md | 53 +++ ...ionEndpointsMustReturnOperationResponse.js | 76 ++++ ...perationResponseMustIncludeCoreMetadata.js | 59 +++ ...ationStatusMustUseTheStandardStatusEnum.js | 34 ++ .../functions/utils/longRunningOperations.js | 22 ++ 10 files changed, 1121 insertions(+) create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js diff --git a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js new file mode 100644 index 0000000000..e76f0c28ca --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js @@ -0,0 +1,362 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const SINGLE_OPERATION_ERROR_MESSAGE = 'The Operation endpoint must return the OperationResponse schema.'; +const OPERATIONS_COLLECTION_ERROR_MESSAGE = + 'The Operations collection endpoint must return a paginated response whose results reference the OperationResponse schema.'; + +const components = { + schemas: { + OperationResponse: { + type: 'object', + properties: { + operationId: { type: 'string' }, + }, + }, + PaginatedOperationResponse: { + type: 'object', + properties: { + results: { + type: 'array', + items: { $ref: '#/components/schemas/OperationResponse' }, + }, + totalCount: { type: 'integer' }, + }, + }, + Order: { + type: 'object', + properties: { + id: { type: 'string' }, + }, + }, + PaginatedOrderResponse: { + type: 'object', + properties: { + results: { + type: 'array', + items: { $ref: '#/components/schemas/Order' }, + }, + }, + }, + ApiError: { + type: 'object', + properties: { + error: { type: 'string' }, + }, + }, + }, +}; + +testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ + { + name: 'valid Operations endpoints returning OperationResponse', + document: { + paths: { + '/api/atlas/v2/resourceName/operations': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/PaginatedOperationResponse' }, + }, + }, + }, + }, + }, + }, + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + 404: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/ApiError' }, + }, + }, + }, + }, + }, + }, + '/api/atlas/v2/resourceName/{pathParam}/operations': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + // The paginated wrapper may also be defined inline + schema: { + type: 'object', + properties: { + results: { + type: 'array', + items: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + '/api/atlas/v2/resourceName/{pathParam}/operations/{operationId}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + }, + }, + }, + // Not an Operations endpoint, the returned schema is not restricted + '/api/atlas/v2/resourceName/{pathParam}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/Order' }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [], + }, + { + name: 'invalid single Operation endpoint returning the parent resource', + document: { + paths: { + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/Order' }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: SINGLE_OPERATION_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations/{operationId}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid single Operation endpoint with an inline schema', + document: { + paths: { + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + type: 'object', + properties: { + operationId: { type: 'string' }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: SINGLE_OPERATION_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations/{operationId}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid Operations collection returning a non-Operation paginated response', + document: { + paths: { + '/api/atlas/v2/resourceName/operations': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/PaginatedOrderResponse' }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: OPERATIONS_COLLECTION_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid Operations collection returning an unpaginated OperationResponse', + document: { + paths: { + '/api/atlas/v2/resourceName/operations': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: OPERATIONS_COLLECTION_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid responses with exceptions', + document: { + paths: { + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/Order' }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-endpoints-must-return-operation-response': 'reason', + }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [], + }, + { + name: 'compliant responses do not need an exception', + document: { + paths: { + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/OperationResponse' }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-endpoints-must-return-operation-response': 'reason', + }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations/{operationId}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js new file mode 100644 index 0000000000..5840b0b463 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js @@ -0,0 +1,194 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const OPERATION_TYPE_ENUM_ERROR_MESSAGE = + 'The operationType field must use exactly the enum [CREATE, UPDATE, DELETE, CUSTOM].'; +const CUSTOM_METHOD_MISSING_ERROR_MESSAGE = + 'OperationResponse must define a customMethod property for custom method operations.'; +const CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE = + 'The customMethod property must not be listed as required. It is required only when operationType is CUSTOM.'; + +const validOperationResponse = { + type: 'object', + required: ['operationId', 'operationType', 'createdAt', 'updatedAt'], + properties: { + operationId: { type: 'string' }, + operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, + customMethod: { type: 'string' }, + createdAt: { type: 'string', format: 'date-time' }, + updatedAt: { type: 'string', format: 'date-time' }, + }, +}; + +testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ + { + name: 'valid OperationResponse with all core metadata', + document: { + components: { + schemas: { + OperationResponse: validOperationResponse, + }, + }, + }, + errors: [], + }, + { + name: 'documents without an OperationResponse schema are ignored', + document: { + components: { + schemas: { + Order: { + type: 'object', + properties: { + id: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid OperationResponse missing core metadata', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, + createdAt: { type: 'string', format: 'date-time' }, + }, + }, + }, + }, + }, + errors: [ + // Results are ordered by document position: the schema-level errors anchor at the schema + // object, before the property-level errors + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: 'OperationResponse must define the operationId property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: 'OperationResponse must define the updatedAt property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: CUSTOM_METHOD_MISSING_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: 'The operationType property must be listed as required.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'operationType'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: 'The createdAt property must be listed as required.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'createdAt'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid operationType enum missing a value', + document: { + components: { + schemas: { + OperationResponse: { + ...validOperationResponse, + properties: { + ...validOperationResponse.properties, + operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE'] }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: OPERATION_TYPE_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'operationType'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid customMethod listed as required', + document: { + components: { + schemas: { + OperationResponse: { + ...validOperationResponse, + required: ['operationId', 'operationType', 'createdAt', 'updatedAt', 'customMethod'], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'customMethod'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse with an exception', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-include-core-metadata': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant OperationResponse does not need an exception', + document: { + components: { + schemas: { + OperationResponse: { + ...validOperationResponse, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-include-core-metadata': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'OperationResponse', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operation-response-must-include-core-metadata', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js new file mode 100644 index 0000000000..836e01fd31 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js @@ -0,0 +1,225 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const MISSING_STATUS_ERROR_MESSAGE = 'OperationResponse must report progress through a status field.'; +const STATUS_ENUM_ERROR_MESSAGE = + 'The status field must use exactly the enum [PENDING, IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED].'; + +testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ + { + name: 'valid OperationResponse with the standard status enum', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { + type: 'string', + enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid OperationResponse with the standard status enum in a different order', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { + type: 'string', + enum: ['SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED', 'PENDING', 'IN_PROGRESS'], + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'documents without an OperationResponse schema are ignored', + document: { + components: { + schemas: { + Order: { + type: 'object', + properties: { + status: { + type: 'string', + enum: ['OPEN', 'CLOSED'], + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid OperationResponse without a status field', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + state: { + type: 'string', + enum: ['QUEUED', 'RUNNING', 'COMPLETE', 'ERROR', 'CANCELLED'], + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', + message: MISSING_STATUS_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid status enum with non-standard values', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { + type: 'string', + enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELLED', 'SUPERSEDED'], + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', + message: STATUS_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid status enum with a missing value', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { + type: 'string', + enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED'], + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', + message: STATUS_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid status field without an enum', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { + type: 'string', + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', + message: STATUS_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse with an exception', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + state: { + type: 'string', + }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant OperationResponse does not need an exception', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { + type: 'string', + enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], + }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'OperationResponse', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js b/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js index 461e303c0b..ae50e41b99 100644 --- a/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js +++ b/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js @@ -1,10 +1,13 @@ import { describe, expect, it } from '@jest/globals'; import { containsOperationsSegment, + isExactEnumMatch, isOperationsCollectionPath, isOperationsPath, isSingleOperationPath, operationsSegmentIsLeaf, + OPERATION_STATUS_ENUM, + OPERATION_TYPE_ENUM, } from '../../rulesets/functions/utils/longRunningOperations'; describe('tools/spectral/ipa/utils/longRunningOperations.js', () => { @@ -88,4 +91,25 @@ describe('tools/spectral/ipa/utils/longRunningOperations.js', () => { expect(operationsSegmentIsLeaf('/api/atlas/v2/operations/subresource')).toBe(false); }); }); + + describe('isExactEnumMatch', () => { + it('accepts exact matches in any order', () => { + expect( + isExactEnumMatch( + ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], + OPERATION_STATUS_ENUM + ) + ).toBe(true); + expect(isExactEnumMatch(['CUSTOM', 'DELETE', 'UPDATE', 'CREATE'], OPERATION_TYPE_ENUM)).toBe(true); + }); + + it('rejects missing, extra, duplicate or undefined values', () => { + expect( + isExactEnumMatch(['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED'], OPERATION_STATUS_ENUM) + ).toBe(false); + expect(isExactEnumMatch([...OPERATION_STATUS_ENUM, 'CANCELLING'], OPERATION_STATUS_ENUM)).toBe(false); + expect(isExactEnumMatch(['CREATE', 'CREATE', 'UPDATE', 'DELETE'], OPERATION_TYPE_ENUM)).toBe(false); + expect(isExactEnumMatch(undefined, OPERATION_TYPE_ENUM)).toBe(false); + }); + }); }); diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index c8a3a422bf..6c1f1c476c 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -5,6 +5,9 @@ functions: - IPA132OperationsEndpointMustNotBeGlobal - IPA132OperationMustBeAReadOnlyResource - IPA132OperationsEndpointMustBeALeafResource + - IPA132OperationEndpointsMustReturnOperationResponse + - IPA132OperationStatusMustUseTheStandardStatusEnum + - IPA132OperationResponseMustIncludeCoreMetadata rules: xgen-IPA-132-operations-endpoint-must-not-be-global: @@ -72,3 +75,72 @@ rules: then: field: '@key' function: IPA132OperationsEndpointMustBeALeafResource + + xgen-IPA-132-operation-endpoints-must-return-operation-response: + description: | + Operation endpoints must return the OperationResponse schema to report long-running + operation status. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to 2xx JSON responses of GET methods on Operations endpoints + - The single Operation endpoint (`.../operations/{operationId}`) must reference the + `OperationResponse` schema + - The Operations collection endpoint (`.../operations`) must return a paginated response + whose `results` items reference the `OperationResponse` schema; the paginated wrapper + may be a referenced or an inline schema + - Inline Operation schemas are violations, the response must reference the predefined + `OperationResponse` schema + - Responses with `x-xgen-IPA-exception` for this rule at the content media type level are + excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-endpoints-must-return-operation-response' + severity: warn + given: $.paths[*].get.responses[*].content + then: + field: '@key' + function: IPA132OperationEndpointsMustReturnOperationResponse + + xgen-IPA-132-operation-status-must-use-the-standard-status-enum: + description: | + An Operation must report progress through a status field, using exactly the enum PENDING, + IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The schema must define a `status` property + - The `status` enum must contain exactly the values `PENDING`, `IN_PROGRESS`, `SUCCEEDED`, + `FAILED`, `CANCELED` and `SUPERSEDED`, in any order + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-status-must-use-the-standard-status-enum' + severity: warn + given: $.components.schemas.OperationResponse + then: + function: IPA132OperationStatusMustUseTheStandardStatusEnum + + xgen-IPA-132-operation-response-must-include-core-metadata: + description: | + An OperationResponse must include stable core metadata for the operation record: a required + operationId, a required operationType, a required createdAt and a required updatedAt. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The properties `operationId`, `operationType`, `createdAt` and `updatedAt` must be + defined and listed as required + - The `operationType` enum must contain exactly the values `CREATE`, `UPDATE`, `DELETE` + and `CUSTOM`, in any order + - A `customMethod` property must be defined and must not be listed as required: it is + required only when `operationType` is `CUSTOM`, which cannot be validated statically + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-include-core-metadata' + severity: warn + given: $.components.schemas.OperationResponse + then: + function: IPA132OperationResponseMustIncludeCoreMetadata diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 0b4dacbcb0..20f4bca9d6 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1458,6 +1458,59 @@ Rule checks for the following conditions: `.../operations/{operationId}/subresource`, is a violation - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation +#### xgen-IPA-132-operation-endpoints-must-return-operation-response + + ![warn](https://img.shields.io/badge/warning-yellow) +Operation endpoints must return the OperationResponse schema to report long-running +operation status. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to 2xx JSON responses of GET methods on Operations endpoints + - The single Operation endpoint (`.../operations/{operationId}`) must reference the + `OperationResponse` schema + - The Operations collection endpoint (`.../operations`) must return a paginated response + whose `results` items reference the `OperationResponse` schema; the paginated wrapper + may be a referenced or an inline schema + - Inline Operation schemas are violations, the response must reference the predefined + `OperationResponse` schema + - Responses with `x-xgen-IPA-exception` for this rule at the content media type level are + excluded from validation + +#### xgen-IPA-132-operation-status-must-use-the-standard-status-enum + + ![warn](https://img.shields.io/badge/warning-yellow) +An Operation must report progress through a status field, using exactly the enum PENDING, +IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The schema must define a `status` property + - The `status` enum must contain exactly the values `PENDING`, `IN_PROGRESS`, `SUCCEEDED`, + `FAILED`, `CANCELED` and `SUPERSEDED`, in any order + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + +#### xgen-IPA-132-operation-response-must-include-core-metadata + + ![warn](https://img.shields.io/badge/warning-yellow) +An OperationResponse must include stable core metadata for the operation record: a required +operationId, a required operationType, a required createdAt and a required updatedAt. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The properties `operationId`, `operationType`, `createdAt` and `updatedAt` must be + defined and listed as required + - The `operationType` enum must contain exactly the values `CREATE`, `UPDATE`, `DELETE` + and `CUSTOM`, in any order + - A `customMethod` property must be defined and must not be listed as required: it is + required only when `operationType` is `CUSTOM`, which cannot be validated statically + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js new file mode 100644 index 0000000000..b1d0339e72 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js @@ -0,0 +1,76 @@ +import { resolveObject } from './utils/componentUtils.js'; +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { getSchemaNameFromRef, getSchemaRef } from './utils/methodUtils.js'; +import { + isOperationsCollectionPath, + isSingleOperationPath, + OPERATION_RESPONSE_SCHEMA_NAME, +} from './utils/longRunningOperations.js'; + +const SINGLE_OPERATION_ERROR_MESSAGE = `The Operation endpoint must return the ${OPERATION_RESPONSE_SCHEMA_NAME} schema.`; +const OPERATIONS_COLLECTION_ERROR_MESSAGE = `The Operations collection endpoint must return a paginated response whose results reference the ${OPERATION_RESPONSE_SCHEMA_NAME} schema.`; + +/** + * Checks that Operations endpoints defined by IPA-132 return the OperationResponse schema: the + * single Operation endpoint must reference it directly, and the Operations collection endpoint + * must return a paginated response whose results items reference it. + * + * @param {string} input - The media type from the response content of the GET method + * @param {object} _ - Unused + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, _, { path, documentInventory, rule }) => { + const ruleName = rule.name; + const resourcePath = path[1]; + const responseCode = path[4]; + const oas = documentInventory.unresolved; + const contentPerMediaType = resolveObject(oas, path); + + if ( + !responseCode.startsWith('2') || + !contentPerMediaType || + !contentPerMediaType.schema || + !input.endsWith('json') || + (!isSingleOperationPath(resourcePath) && !isOperationsCollectionPath(resourcePath)) + ) { + return; + } + + const errors = checkViolationsAndReturnErrors(resourcePath, contentPerMediaType.schema, oas, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, contentPerMediaType, path); +}; + +function checkViolationsAndReturnErrors(resourcePath, schema, oas, path, ruleName) { + try { + if (isSingleOperationPath(resourcePath)) { + if (schemaReferencesOperationResponse(schema)) { + return []; + } + return [{ path, message: SINGLE_OPERATION_ERROR_MESSAGE }]; + } + + // The Operations collection response may reference a paginated wrapper schema, or define the + // paginated wrapper inline; the results items must reference the Operation schema either way + const wrapperSchema = getReferencedSchema(schema, oas) ?? schema; + const resultsItems = wrapperSchema?.properties?.results?.items; + if (resultsItems && schemaReferencesOperationResponse(resultsItems)) { + return []; + } + return [{ path, message: OPERATIONS_COLLECTION_ERROR_MESSAGE }]; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} + +function schemaReferencesOperationResponse(schema) { + const schemaRef = getSchemaRef(schema); + return schemaRef !== undefined && getSchemaNameFromRef(schemaRef) === OPERATION_RESPONSE_SCHEMA_NAME; +} + +function getReferencedSchema(schema, oas) { + const schemaRef = schema.$ref; + if (!schemaRef) { + return undefined; + } + return oas.components?.schemas?.[getSchemaNameFromRef(schemaRef)]; +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js new file mode 100644 index 0000000000..5b88fa857c --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js @@ -0,0 +1,59 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { isExactEnumMatch, OPERATION_TYPE_ENUM } from './utils/longRunningOperations.js'; + +const CORE_METADATA_FIELDS = ['operationId', 'operationType', 'createdAt', 'updatedAt']; +const OPERATION_TYPE_ENUM_ERROR_MESSAGE = `The operationType field must use exactly the enum [${OPERATION_TYPE_ENUM.join(', ')}].`; +const CUSTOM_METHOD_MISSING_ERROR_MESSAGE = + 'OperationResponse must define a customMethod property for custom method operations.'; +const CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE = + 'The customMethod property must not be listed as required. It is required only when operationType is CUSTOM.'; + +/** + * Checks that the OperationResponse schema defined by IPA-132 includes the stable core metadata + * for the operation record: required operationId, operationType, createdAt and updatedAt + * properties, the standard operationType enum, and an optional customMethod property. + * + * @param {object} input - The OperationResponse schema object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, _, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, path, ruleName) { + try { + const properties = schema.properties ?? {}; + const required = Array.isArray(schema.required) ? schema.required : []; + const errors = []; + + for (const field of CORE_METADATA_FIELDS) { + if (!properties[field]) { + errors.push({ path, message: `OperationResponse must define the ${field} property.` }); + } else if (!required.includes(field)) { + errors.push({ + path: [...path, 'properties', field], + message: `The ${field} property must be listed as required.`, + }); + } + } + + if (properties.operationType && !isExactEnumMatch(properties.operationType.enum, OPERATION_TYPE_ENUM)) { + errors.push({ path: [...path, 'properties', 'operationType'], message: OPERATION_TYPE_ENUM_ERROR_MESSAGE }); + } + + // customMethod is required only when operationType is CUSTOM, which is a runtime condition + // that cannot be validated statically: the property must be defined and must not be required + if (!properties.customMethod) { + errors.push({ path, message: CUSTOM_METHOD_MISSING_ERROR_MESSAGE }); + } else if (required.includes('customMethod')) { + errors.push({ path: [...path, 'properties', 'customMethod'], message: CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE }); + } + + return errors; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js new file mode 100644 index 0000000000..8bcac91c43 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js @@ -0,0 +1,34 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { isExactEnumMatch, OPERATION_STATUS_ENUM } from './utils/longRunningOperations.js'; + +const MISSING_STATUS_ERROR_MESSAGE = 'OperationResponse must report progress through a status field.'; +const STATUS_ENUM_ERROR_MESSAGE = `The status field must use exactly the enum [${OPERATION_STATUS_ENUM.join(', ')}].`; + +/** + * Checks that the OperationResponse schema defined by IPA-132 reports progress through a status + * field using exactly the standard status enum. + * + * @param {object} input - The OperationResponse schema object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, _, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, path, ruleName) { + try { + const status = schema.properties?.status; + if (!status) { + return [{ path, message: MISSING_STATUS_ERROR_MESSAGE }]; + } + if (!isExactEnumMatch(status.enum, OPERATION_STATUS_ENUM)) { + return [{ path: [...path, 'properties', 'status'], message: STATUS_ENUM_ERROR_MESSAGE }]; + } + return []; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js index 7e82cc02b6..00c561eeb6 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js +++ b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js @@ -100,3 +100,25 @@ export function operationsSegmentIsLeaf(path) { const trailingSegments = segments.slice(firstIndex + 1); return trailingSegments.length === 0 || (trailingSegments.length === 1 && isPathParam(trailingSegments[0])); } + +export const OPERATION_RESPONSE_SCHEMA_NAME = 'OperationResponse'; + +export const OPERATION_STATUS_ENUM = ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED']; + +export const OPERATION_TYPE_ENUM = ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM']; + +/** + * Checks if the enum values defined in a schema match an expected set of values exactly, in any + * order. + * + * @param {string[]|undefined} enumValues the enum values defined in the schema + * @param {string[]} expectedValues the expected enum values + * @returns {boolean} true if the enum values match the expected values exactly + */ +export function isExactEnumMatch(enumValues, expectedValues) { + return ( + Array.isArray(enumValues) && + enumValues.length === expectedValues.length && + expectedValues.every((value) => enumValues.includes(value)) + ); +} From ed530f0daf72f964159b871d4b06961405f6c4f3 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Mon, 17 Aug 2026 10:46:04 +0100 Subject: [PATCH 02/12] fix(ipa): require the status property on OperationResponse Per review of the IPA-132 guideline examples, the status field must be listed as required: an OperationResponse that can legally omit status defeats polling. The status enum rule now checks required membership in addition to the exact enum, matching the updated guideline example. --- ...StatusMustUseTheStandardStatusEnum.test.js | 33 +++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-132.yaml | 2 +- tools/spectral/ipa/rulesets/README.md | 2 +- ...ationStatusMustUseTheStandardStatusEnum.js | 11 +++++-- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js index 836e01fd31..76b2cb453d 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js @@ -2,6 +2,7 @@ import testRule from './__helpers__/testRule'; import { DiagnosticSeverity } from '@stoplight/types'; const MISSING_STATUS_ERROR_MESSAGE = 'OperationResponse must report progress through a status field.'; +const STATUS_REQUIRED_ERROR_MESSAGE = 'The status property must be listed as required.'; const STATUS_ENUM_ERROR_MESSAGE = 'The status field must use exactly the enum [PENDING, IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED].'; @@ -13,6 +14,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ schemas: { OperationResponse: { type: 'object', + required: ['status'], properties: { status: { type: 'string', @@ -32,6 +34,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ schemas: { OperationResponse: { type: 'object', + required: ['status'], properties: { status: { type: 'string', @@ -89,6 +92,32 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ }, ], }, + { + name: 'invalid status property not listed as required', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { + type: 'string', + enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', + message: STATUS_REQUIRED_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'invalid status enum with non-standard values', document: { @@ -96,6 +125,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ schemas: { OperationResponse: { type: 'object', + required: ['status'], properties: { status: { type: 'string', @@ -122,6 +152,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ schemas: { OperationResponse: { type: 'object', + required: ['status'], properties: { status: { type: 'string', @@ -148,6 +179,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ schemas: { OperationResponse: { type: 'object', + required: ['status'], properties: { status: { type: 'string', @@ -194,6 +226,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ schemas: { OperationResponse: { type: 'object', + required: ['status'], properties: { status: { type: 'string', diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index 6c1f1c476c..786cde1bf7 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -111,7 +111,7 @@ rules: Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The schema must define a `status` property + - The schema must define a `status` property, listed as required - The `status` enum must contain exactly the values `PENDING`, `IN_PROGRESS`, `SUCCEEDED`, `FAILED`, `CANCELED` and `SUPERSEDED`, in any order - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 20f4bca9d6..1411f4bfbd 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1488,7 +1488,7 @@ IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED. Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The schema must define a `status` property + - The schema must define a `status` property, listed as required - The `status` enum must contain exactly the values `PENDING`, `IN_PROGRESS`, `SUCCEEDED`, `FAILED`, `CANCELED` and `SUPERSEDED`, in any order - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js index 8bcac91c43..9079d6f4cb 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js @@ -2,6 +2,7 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/c import { isExactEnumMatch, OPERATION_STATUS_ENUM } from './utils/longRunningOperations.js'; const MISSING_STATUS_ERROR_MESSAGE = 'OperationResponse must report progress through a status field.'; +const STATUS_REQUIRED_ERROR_MESSAGE = 'The status property must be listed as required.'; const STATUS_ENUM_ERROR_MESSAGE = `The status field must use exactly the enum [${OPERATION_STATUS_ENUM.join(', ')}].`; /** @@ -24,10 +25,16 @@ function checkViolationsAndReturnErrors(schema, path, ruleName) { if (!status) { return [{ path, message: MISSING_STATUS_ERROR_MESSAGE }]; } + + const errors = []; + const required = Array.isArray(schema.required) ? schema.required : []; + if (!required.includes('status')) { + errors.push({ path: [...path, 'properties', 'status'], message: STATUS_REQUIRED_ERROR_MESSAGE }); + } if (!isExactEnumMatch(status.enum, OPERATION_STATUS_ENUM)) { - return [{ path: [...path, 'properties', 'status'], message: STATUS_ENUM_ERROR_MESSAGE }]; + errors.push({ path: [...path, 'properties', 'status'], message: STATUS_ENUM_ERROR_MESSAGE }); } - return []; + return errors; } catch (e) { return handleInternalError(ruleName, path, e); } From 5abfb8adc5395e9d01e38c75617e4ce79b68f1a2 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Mon, 17 Aug 2026 12:03:50 +0100 Subject: [PATCH 03/12] test(ipa): define shared operation examples once in the IPA-132 tests Per review, the test fixtures follow the same pattern across the IPA-132 test files: the operations and schemas under test are defined once at the top of the file and reused across the test cases. --- ...dpointsMustReturnOperationResponse.test.js | 228 +++++++++--------- ...StatusMustUseTheStandardStatusEnum.test.js | 52 ++-- 2 files changed, 128 insertions(+), 152 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js index e76f0c28ca..415c5e8e5a 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js @@ -47,92 +47,125 @@ const components = { }, }; +const operationResponseGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + }, +}; + +const operationResponseGetWithNotFound = { + responses: { + ...operationResponseGet.responses, + 404: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/ApiError' }, + }, + }, + }, + }, +}; + +const paginatedOperationResponseGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/PaginatedOperationResponse' }, + }, + }, + }, + }, +}; + +// The paginated wrapper may also be defined inline +const inlinePaginatedOperationResponseGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + type: 'object', + properties: { + results: { + type: 'array', + items: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + }, + }, + }, + }, +}; + +const orderGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/Order' }, + }, + }, + }, + }, +}; + +const paginatedOrderResponseGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/PaginatedOrderResponse' }, + }, + }, + }, + }, +}; + +const inlineSchemaGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + type: 'object', + properties: { + operationId: { type: 'string' }, + }, + }, + }, + }, + }, + }, +}; + testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ { name: 'valid Operations endpoints returning OperationResponse', document: { paths: { '/api/atlas/v2/resourceName/operations': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/PaginatedOperationResponse' }, - }, - }, - }, - }, - }, + get: paginatedOperationResponseGet, }, '/api/atlas/v2/resourceName/operations/{operationId}': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/OperationResponse' }, - }, - }, - }, - 404: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/ApiError' }, - }, - }, - }, - }, - }, + get: operationResponseGetWithNotFound, }, '/api/atlas/v2/resourceName/{pathParam}/operations': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - // The paginated wrapper may also be defined inline - schema: { - type: 'object', - properties: { - results: { - type: 'array', - items: { $ref: '#/components/schemas/OperationResponse' }, - }, - }, - }, - }, - }, - }, - }, - }, + get: inlinePaginatedOperationResponseGet, }, '/api/atlas/v2/resourceName/{pathParam}/operations/{operationId}': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/OperationResponse' }, - }, - }, - }, - }, - }, + get: operationResponseGet, }, // Not an Operations endpoint, the returned schema is not restricted '/api/atlas/v2/resourceName/{pathParam}': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/Order' }, - }, - }, - }, - }, - }, + get: orderGet, }, }, components, @@ -144,17 +177,7 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ document: { paths: { '/api/atlas/v2/resourceName/operations/{operationId}': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/Order' }, - }, - }, - }, - }, - }, + get: orderGet, }, }, components, @@ -181,22 +204,7 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ document: { paths: { '/api/atlas/v2/resourceName/operations/{operationId}': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { - type: 'object', - properties: { - operationId: { type: 'string' }, - }, - }, - }, - }, - }, - }, - }, + get: inlineSchemaGet, }, }, components, @@ -223,17 +231,7 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ document: { paths: { '/api/atlas/v2/resourceName/operations': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/PaginatedOrderResponse' }, - }, - }, - }, - }, - }, + get: paginatedOrderResponseGet, }, }, components, @@ -260,17 +258,7 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ document: { paths: { '/api/atlas/v2/resourceName/operations': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/OperationResponse' }, - }, - }, - }, - }, - }, + get: operationResponseGet, }, }, components, diff --git a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js index 76b2cb453d..87818c641c 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js @@ -6,22 +6,26 @@ const STATUS_REQUIRED_ERROR_MESSAGE = 'The status property must be listed as req const STATUS_ENUM_ERROR_MESSAGE = 'The status field must use exactly the enum [PENDING, IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED].'; +const STANDARD_STATUS_ENUM = ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED']; + +const operationResponseWithStandardStatus = { + type: 'object', + required: ['status'], + properties: { + status: { + type: 'string', + enum: STANDARD_STATUS_ENUM, + }, + }, +}; + testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ { name: 'valid OperationResponse with the standard status enum', document: { components: { schemas: { - OperationResponse: { - type: 'object', - required: ['status'], - properties: { - status: { - type: 'string', - enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], - }, - }, - }, + OperationResponse: operationResponseWithStandardStatus, }, }, }, @@ -33,8 +37,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ components: { schemas: { OperationResponse: { - type: 'object', - required: ['status'], + ...operationResponseWithStandardStatus, properties: { status: { type: 'string', @@ -99,12 +102,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ schemas: { OperationResponse: { type: 'object', - properties: { - status: { - type: 'string', - enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], - }, - }, + properties: operationResponseWithStandardStatus.properties, }, }, }, @@ -124,8 +122,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ components: { schemas: { OperationResponse: { - type: 'object', - required: ['status'], + ...operationResponseWithStandardStatus, properties: { status: { type: 'string', @@ -151,8 +148,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ components: { schemas: { OperationResponse: { - type: 'object', - required: ['status'], + ...operationResponseWithStandardStatus, properties: { status: { type: 'string', @@ -178,8 +174,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ components: { schemas: { OperationResponse: { - type: 'object', - required: ['status'], + ...operationResponseWithStandardStatus, properties: { status: { type: 'string', @@ -225,14 +220,7 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ components: { schemas: { OperationResponse: { - type: 'object', - required: ['status'], - properties: { - status: { - type: 'string', - enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], - }, - }, + ...operationResponseWithStandardStatus, 'x-xgen-IPA-exception': { 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum': 'reason', }, From cf929ced56d1df3a2635f21edc451de7d8770cd2 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Mon, 17 Aug 2026 12:11:26 +0100 Subject: [PATCH 04/12] fix(ipa): handle allOf composition in the OperationResponse rules Address findings from a deeper review of the OperationResponse rules: - The status enum and core metadata rules read properties and required lists through allOf sub-schemas, which Spectral does not flatten, so an allOf-composed OperationResponse no longer produces false violations. - The Operations collection endpoint must reference a paginated wrapper schema: inline wrappers are rejected, consistent with xgen-IPA-110-collections-use-paginated-prefix, which rejects them at error severity. The results property of the wrapper may live in one of its allOf sub-schemas. - The single Operation endpoint no longer accepts an array schema whose items reference OperationResponse. - The endpoint rule checks its cheap path and media type gates before walking the document. --- ...dpointsMustReturnOperationResponse.test.js | 106 +++++++++++++++++- ...ionResponseMustIncludeCoreMetadata.test.js | 19 ++++ ...StatusMustUseTheStandardStatusEnum.test.js | 19 ++++ tools/spectral/ipa/rulesets/IPA-132.yaml | 6 +- tools/spectral/ipa/rulesets/README.md | 6 +- ...ionEndpointsMustReturnOperationResponse.js | 51 ++++++--- ...perationResponseMustIncludeCoreMetadata.js | 12 +- ...ationStatusMustUseTheStandardStatusEnum.js | 13 ++- .../functions/utils/longRunningOperations.js | 30 +++++ 9 files changed, 232 insertions(+), 30 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js index 415c5e8e5a..4b62f8c09a 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js @@ -38,6 +38,27 @@ const components = { }, }, }, + PaginatedMetadata: { + type: 'object', + properties: { + totalCount: { type: 'integer' }, + }, + }, + // A paginated wrapper composed with allOf, which Spectral does not flatten + PaginatedAllOfOperationResponse: { + allOf: [ + { $ref: '#/components/schemas/PaginatedMetadata' }, + { + type: 'object', + properties: { + results: { + type: 'array', + items: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + ], + }, ApiError: { type: 'object', properties: { @@ -84,7 +105,19 @@ const paginatedOperationResponseGet = { }, }; -// The paginated wrapper may also be defined inline +const paginatedAllOfOperationResponseGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/PaginatedAllOfOperationResponse' }, + }, + }, + }, + }, +}; + +// Inline paginated wrappers are rejected, consistent with xgen-IPA-110-collections-use-paginated-prefix const inlinePaginatedOperationResponseGet = { responses: { 200: { @@ -105,6 +138,21 @@ const inlinePaginatedOperationResponseGet = { }, }; +const operationResponseArrayGet = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + type: 'array', + items: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + }, + }, +}; + const orderGet = { responses: { 200: { @@ -158,7 +206,7 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ get: operationResponseGetWithNotFound, }, '/api/atlas/v2/resourceName/{pathParam}/operations': { - get: inlinePaginatedOperationResponseGet, + get: paginatedAllOfOperationResponseGet, }, '/api/atlas/v2/resourceName/{pathParam}/operations/{operationId}': { get: operationResponseGet, @@ -253,6 +301,60 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ }, ], }, + { + name: 'invalid single Operation endpoint returning an array of OperationResponse', + document: { + paths: { + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: operationResponseArrayGet, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: SINGLE_OPERATION_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations/{operationId}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid Operations collection with an inline paginated wrapper', + document: { + paths: { + '/api/atlas/v2/resourceName/operations': { + get: inlinePaginatedOperationResponseGet, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: OPERATIONS_COLLECTION_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'invalid Operations collection returning an unpaginated OperationResponse', document: { diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js index 5840b0b463..7c34a5dbe9 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js @@ -32,6 +32,25 @@ testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ }, errors: [], }, + { + name: 'valid OperationResponse composed with allOf', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, validOperationResponse], + }, + OperationMetadata: { + type: 'object', + properties: { + statusMessage: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, { name: 'documents without an OperationResponse schema are ignored', document: { diff --git a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js index 87818c641c..906450a7f0 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js @@ -50,6 +50,25 @@ testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ }, errors: [], }, + { + name: 'valid OperationResponse composed with allOf', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, operationResponseWithStandardStatus], + }, + OperationMetadata: { + type: 'object', + properties: { + operationId: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, { name: 'documents without an OperationResponse schema are ignored', document: { diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index 786cde1bf7..6f1c08cca7 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -87,9 +87,9 @@ rules: - Applies to 2xx JSON responses of GET methods on Operations endpoints - The single Operation endpoint (`.../operations/{operationId}`) must reference the `OperationResponse` schema - - The Operations collection endpoint (`.../operations`) must return a paginated response - whose `results` items reference the `OperationResponse` schema; the paginated wrapper - may be a referenced or an inline schema + - The Operations collection endpoint (`.../operations`) must reference a paginated wrapper + schema whose `results` items reference the `OperationResponse` schema; inline paginated + wrappers are rejected, consistent with `xgen-IPA-110-collections-use-paginated-prefix` - Inline Operation schemas are violations, the response must reference the predefined `OperationResponse` schema - Responses with `x-xgen-IPA-exception` for this rule at the content media type level are diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 1411f4bfbd..239a776b0d 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1470,9 +1470,9 @@ Rule checks for the following conditions: - Applies to 2xx JSON responses of GET methods on Operations endpoints - The single Operation endpoint (`.../operations/{operationId}`) must reference the `OperationResponse` schema - - The Operations collection endpoint (`.../operations`) must return a paginated response - whose `results` items reference the `OperationResponse` schema; the paginated wrapper - may be a referenced or an inline schema + - The Operations collection endpoint (`.../operations`) must reference a paginated wrapper + schema whose `results` items reference the `OperationResponse` schema; inline paginated + wrappers are rejected, consistent with `xgen-IPA-110-collections-use-paginated-prefix` - Inline Operation schemas are violations, the response must reference the predefined `OperationResponse` schema - Responses with `x-xgen-IPA-exception` for this rule at the content media type level are diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js index b1d0339e72..6b0bc7a450 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js @@ -1,6 +1,6 @@ import { resolveObject } from './utils/componentUtils.js'; import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getSchemaNameFromRef, getSchemaRef } from './utils/methodUtils.js'; +import { getSchemaNameFromRef } from './utils/methodUtils.js'; import { isOperationsCollectionPath, isSingleOperationPath, @@ -13,7 +13,7 @@ const OPERATIONS_COLLECTION_ERROR_MESSAGE = `The Operations collection endpoint /** * Checks that Operations endpoints defined by IPA-132 return the OperationResponse schema: the * single Operation endpoint must reference it directly, and the Operations collection endpoint - * must return a paginated response whose results items reference it. + * must reference a paginated wrapper schema whose results items reference it. * * @param {string} input - The media type from the response content of the GET method * @param {object} _ - Unused @@ -23,19 +23,21 @@ export default (input, _, { path, documentInventory, rule }) => { const ruleName = rule.name; const resourcePath = path[1]; const responseCode = path[4]; - const oas = documentInventory.unresolved; - const contentPerMediaType = resolveObject(oas, path); if ( !responseCode.startsWith('2') || - !contentPerMediaType || - !contentPerMediaType.schema || !input.endsWith('json') || (!isSingleOperationPath(resourcePath) && !isOperationsCollectionPath(resourcePath)) ) { return; } + const oas = documentInventory.unresolved; + const contentPerMediaType = resolveObject(oas, path); + if (!contentPerMediaType || !contentPerMediaType.schema) { + return; + } + const errors = checkViolationsAndReturnErrors(resourcePath, contentPerMediaType.schema, oas, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, contentPerMediaType, path); }; @@ -49,10 +51,10 @@ function checkViolationsAndReturnErrors(resourcePath, schema, oas, path, ruleNam return [{ path, message: SINGLE_OPERATION_ERROR_MESSAGE }]; } - // The Operations collection response may reference a paginated wrapper schema, or define the - // paginated wrapper inline; the results items must reference the Operation schema either way - const wrapperSchema = getReferencedSchema(schema, oas) ?? schema; - const resultsItems = wrapperSchema?.properties?.results?.items; + // The Operations collection response must reference a paginated wrapper schema, since inline + // paginated schemas are rejected by xgen-IPA-110-collections-use-paginated-prefix + const wrapperSchema = getReferencedSchema(schema, oas); + const resultsItems = getResultsItems(wrapperSchema); if (resultsItems && schemaReferencesOperationResponse(resultsItems)) { return []; } @@ -63,14 +65,33 @@ function checkViolationsAndReturnErrors(resourcePath, schema, oas, path, ruleNam } function schemaReferencesOperationResponse(schema) { - const schemaRef = getSchemaRef(schema); - return schemaRef !== undefined && getSchemaNameFromRef(schemaRef) === OPERATION_RESPONSE_SCHEMA_NAME; + return schema.$ref !== undefined && getSchemaNameFromRef(schema.$ref) === OPERATION_RESPONSE_SCHEMA_NAME; } function getReferencedSchema(schema, oas) { - const schemaRef = schema.$ref; - if (!schemaRef) { + if (!schema.$ref) { + return undefined; + } + return oas.components?.schemas?.[getSchemaNameFromRef(schema.$ref)]; +} + +/** + * Finds the results items schema of a paginated wrapper. The results property may be defined + * directly on the wrapper, or within one of its allOf sub-schemas, which Spectral does not flatten. + */ +function getResultsItems(wrapperSchema) { + if (!wrapperSchema) { return undefined; } - return oas.components?.schemas?.[getSchemaNameFromRef(schemaRef)]; + const directItems = wrapperSchema.properties?.results?.items; + if (directItems) { + return directItems; + } + for (const subSchema of wrapperSchema.allOf ?? []) { + const items = subSchema.properties?.results?.items; + if (items) { + return items; + } + } + return undefined; } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js index 5b88fa857c..b558c41788 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js @@ -1,5 +1,10 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { isExactEnumMatch, OPERATION_TYPE_ENUM } from './utils/longRunningOperations.js'; +import { + getMergedProperties, + getMergedRequired, + isExactEnumMatch, + OPERATION_TYPE_ENUM, +} from './utils/longRunningOperations.js'; const CORE_METADATA_FIELDS = ['operationId', 'operationType', 'createdAt', 'updatedAt']; const OPERATION_TYPE_ENUM_ERROR_MESSAGE = `The operationType field must use exactly the enum [${OPERATION_TYPE_ENUM.join(', ')}].`; @@ -25,8 +30,9 @@ export default (input, _, { path, rule }) => { function checkViolationsAndReturnErrors(schema, path, ruleName) { try { - const properties = schema.properties ?? {}; - const required = Array.isArray(schema.required) ? schema.required : []; + // Properties and required lists may be spread across allOf sub-schemas + const properties = getMergedProperties(schema); + const required = getMergedRequired(schema); const errors = []; for (const field of CORE_METADATA_FIELDS) { diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js index 9079d6f4cb..bf1ce9bd88 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js @@ -1,5 +1,10 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { isExactEnumMatch, OPERATION_STATUS_ENUM } from './utils/longRunningOperations.js'; +import { + getMergedProperties, + getMergedRequired, + isExactEnumMatch, + OPERATION_STATUS_ENUM, +} from './utils/longRunningOperations.js'; const MISSING_STATUS_ERROR_MESSAGE = 'OperationResponse must report progress through a status field.'; const STATUS_REQUIRED_ERROR_MESSAGE = 'The status property must be listed as required.'; @@ -21,14 +26,14 @@ export default (input, _, { path, rule }) => { function checkViolationsAndReturnErrors(schema, path, ruleName) { try { - const status = schema.properties?.status; + // Properties and required lists may be spread across allOf sub-schemas + const status = getMergedProperties(schema).status; if (!status) { return [{ path, message: MISSING_STATUS_ERROR_MESSAGE }]; } const errors = []; - const required = Array.isArray(schema.required) ? schema.required : []; - if (!required.includes('status')) { + if (!getMergedRequired(schema).includes('status')) { errors.push({ path: [...path, 'properties', 'status'], message: STATUS_REQUIRED_ERROR_MESSAGE }); } if (!isExactEnumMatch(status.enum, OPERATION_STATUS_ENUM)) { diff --git a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js index 00c561eeb6..775d26178c 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js +++ b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js @@ -122,3 +122,33 @@ export function isExactEnumMatch(enumValues, expectedValues) { expectedValues.every((value) => enumValues.includes(value)) ); } + +/** + * Collects the properties of a schema, merging the properties of its allOf sub-schemas, which + * Spectral does not flatten during resolution. + * + * @param {object} schema the schema to collect properties for + * @returns {object} the properties of the schema and all of its allOf sub-schemas + */ +export function getMergedProperties(schema) { + const properties = { ...(schema.properties ?? {}) }; + for (const subSchema of schema.allOf ?? []) { + Object.assign(properties, getMergedProperties(subSchema)); + } + return properties; +} + +/** + * Collects the required property names of a schema, merging the required lists of its allOf + * sub-schemas, which Spectral does not flatten during resolution. + * + * @param {object} schema the schema to collect required property names for + * @returns {string[]} the required property names of the schema and all of its allOf sub-schemas + */ +export function getMergedRequired(schema) { + const required = Array.isArray(schema.required) ? [...schema.required] : []; + for (const subSchema of schema.allOf ?? []) { + required.push(...getMergedRequired(subSchema)); + } + return required; +} From d70c98d6c809ccbe7176424055a678ec159d7c74 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Tue, 18 Aug 2026 11:15:01 +0100 Subject: [PATCH 05/12] fix(ipa): reuse an OperationResponseSchema alias in the IPA-132 ruleset --- tools/spectral/ipa/rulesets/IPA-132.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index 6f1c08cca7..86d899dfe9 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -9,6 +9,10 @@ functions: - IPA132OperationStatusMustUseTheStandardStatusEnum - IPA132OperationResponseMustIncludeCoreMetadata +aliases: + OperationResponseSchema: + - '$.components.schemas.OperationResponse' + rules: xgen-IPA-132-operations-endpoint-must-not-be-global: description: | @@ -118,7 +122,7 @@ rules: message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-status-must-use-the-standard-status-enum' severity: warn - given: $.components.schemas.OperationResponse + given: '#OperationResponseSchema' then: function: IPA132OperationStatusMustUseTheStandardStatusEnum @@ -141,6 +145,6 @@ rules: message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-include-core-metadata' severity: warn - given: $.components.schemas.OperationResponse + given: '#OperationResponseSchema' then: function: IPA132OperationResponseMustIncludeCoreMetadata From 2a00266e1217482f912671544cf60495eee9aae0 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Wed, 19 Aug 2026 10:58:32 +0100 Subject: [PATCH 06/12] refactor(ipa): restructure OperationResponse validation into check-type rules Replace the per-guideline schema rules with four declarative rules driven by functionOptions field lists, validating the OperationResponse reference schema by check type: required fields, conditionally-present optional fields, standard enum values and nested object structure. The should-level status fields rule stays separate because severity is per-rule. --- ...nseMustDefineNestedObjectStructure.test.js | 190 +++++++++++++ ...onResponseMustDefineOptionalFields.test.js | 178 ++++++++++++ ...nResponseMustDefineRequiredFields.test.js} | 89 ++---- ...nResponseMustUseStandardEnumValues.test.js | 182 ++++++++++++ ...StatusMustUseTheStandardStatusEnum.test.js | 265 ------------------ ...OperationsShouldExposeStatusFields.test.js | 182 ++++++++++++ tools/spectral/ipa/rulesets/IPA-132.yaml | 152 ++++++++-- tools/spectral/ipa/rulesets/README.md | 92 +++++- ...ResponseMustDefineNestedObjectStructure.js | 48 ++++ ...erationResponseMustDefineOptionalFields.js | 42 +++ ...erationResponseMustDefineRequiredFields.js | 41 +++ ...perationResponseMustIncludeCoreMetadata.js | 65 ----- ...rationResponseMustUseStandardEnumValues.js | 54 ++++ ...ationStatusMustUseTheStandardStatusEnum.js | 46 --- ...PA132OperationsShouldExposeStatusFields.js | 48 ++++ 15 files changed, 1189 insertions(+), 485 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js rename tools/spectral/ipa/__tests__/{IPA132OperationResponseMustIncludeCoreMetadata.test.js => IPA132OperationResponseMustDefineRequiredFields.test.js} (54%) create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js delete mode 100644 tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js delete mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js delete mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js new file mode 100644 index 0000000000..ba7d261074 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js @@ -0,0 +1,190 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const validOperationResponse = { + type: 'object', + properties: { + error: { + type: 'object', + properties: { + code: { type: 'string' }, + message: { type: 'string' }, + retryable: { type: 'boolean' }, + retryStrategy: { type: 'string', enum: ['IMMEDIATE', 'BACKOFF', 'NONE'] }, + details: { type: 'object' }, + }, + }, + }, +}; + +testRule('xgen-IPA-132-operation-response-must-define-nested-object-structure', [ + { + name: 'valid OperationResponse with a structured error object', + document: { + components: { + schemas: { + OperationResponse: validOperationResponse, + }, + }, + }, + errors: [], + }, + { + name: 'valid OperationResponse with a referenced error object', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + error: { $ref: '#/components/schemas/OperationError' }, + }, + }, + OperationError: { + type: 'object', + properties: { + code: { type: 'string' }, + message: { type: 'string' }, + retryable: { type: 'boolean' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'nested objects that are not defined are skipped', + document: { + components: { + schemas: { + // A missing error field is the optional-fields rule's concern + OperationResponse: { + type: 'object', + properties: { + operationId: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid error object missing structured properties', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + error: { + type: 'object', + properties: { + code: { type: 'string' }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-define-nested-object-structure', + message: 'The error object must define a message property.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'error'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-define-nested-object-structure', + message: 'The error object must define a retryable property.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'error'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid error modeled as a plain string', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + error: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-define-nested-object-structure', + message: 'The error object must define a code property.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'error'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-define-nested-object-structure', + message: 'The error object must define a message property.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'error'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-define-nested-object-structure', + message: 'The error object must define a retryable property.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'error'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse with an exception', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + error: { type: 'string' }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-define-nested-object-structure': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant OperationResponse does not need an exception', + document: { + components: { + schemas: { + OperationResponse: { + ...validOperationResponse, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-define-nested-object-structure': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-define-nested-object-structure', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'OperationResponse', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operation-response-must-define-nested-object-structure', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js new file mode 100644 index 0000000000..47ff7ceae5 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js @@ -0,0 +1,178 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE = + 'The customMethod property must not be listed as required. It is present only when operationType is CUSTOM, which cannot be validated statically.'; +const ERROR_REQUIRED_ERROR_MESSAGE = + 'The error property must not be listed as required. It is present only when the operation has FAILED, which cannot be validated statically.'; + +const validOperationResponse = { + type: 'object', + properties: { + customMethod: { type: 'string' }, + error: { + type: 'object', + properties: { + code: { type: 'string' }, + message: { type: 'string' }, + retryable: { type: 'boolean' }, + }, + }, + resultHref: { type: 'string', format: 'uri' }, + retryAfterSeconds: { type: 'integer' }, + }, +}; + +testRule('xgen-IPA-132-operation-response-must-define-optional-fields', [ + { + name: 'valid OperationResponse with all conditional fields', + document: { + components: { + schemas: { + OperationResponse: validOperationResponse, + }, + }, + }, + errors: [], + }, + { + name: 'valid OperationResponse composed with allOf', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [{ $ref: '#/components/schemas/OperationOutcome' }, validOperationResponse], + }, + OperationOutcome: { + type: 'object', + properties: { + statusMessage: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'documents without an OperationResponse schema are ignored', + document: { + components: { + schemas: { + Order: { + type: 'object', + properties: { + id: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid OperationResponse missing conditional fields', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + customMethod: { type: 'string' }, + resultHref: { type: 'string', format: 'uri' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-define-optional-fields', + message: 'OperationResponse must define the error property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-define-optional-fields', + message: 'OperationResponse must define the retryAfterSeconds property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid conditional fields listed as required', + document: { + components: { + schemas: { + OperationResponse: { + ...validOperationResponse, + required: ['customMethod', 'error'], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-define-optional-fields', + message: CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'customMethod'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-define-optional-fields', + message: ERROR_REQUIRED_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'error'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse with an exception', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + customMethod: { type: 'string' }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-define-optional-fields': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant OperationResponse does not need an exception', + document: { + components: { + schemas: { + OperationResponse: { + ...validOperationResponse, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-define-optional-fields': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-define-optional-fields', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'OperationResponse', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operation-response-must-define-optional-fields', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js similarity index 54% rename from tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js rename to tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js index 7c34a5dbe9..17ffc3b4ba 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustIncludeCoreMetadata.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js @@ -1,28 +1,21 @@ import testRule from './__helpers__/testRule'; import { DiagnosticSeverity } from '@stoplight/types'; -const OPERATION_TYPE_ENUM_ERROR_MESSAGE = - 'The operationType field must use exactly the enum [CREATE, UPDATE, DELETE, CUSTOM].'; -const CUSTOM_METHOD_MISSING_ERROR_MESSAGE = - 'OperationResponse must define a customMethod property for custom method operations.'; -const CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE = - 'The customMethod property must not be listed as required. It is required only when operationType is CUSTOM.'; - const validOperationResponse = { type: 'object', - required: ['operationId', 'operationType', 'createdAt', 'updatedAt'], + required: ['operationId', 'status', 'operationType', 'createdAt', 'updatedAt'], properties: { operationId: { type: 'string' }, + status: { type: 'string', enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'] }, operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, - customMethod: { type: 'string' }, createdAt: { type: 'string', format: 'date-time' }, updatedAt: { type: 'string', format: 'date-time' }, }, }; -testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ +testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ { - name: 'valid OperationResponse with all core metadata', + name: 'valid OperationResponse with all required fields', document: { components: { schemas: { @@ -68,12 +61,13 @@ testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ errors: [], }, { - name: 'invalid OperationResponse missing core metadata', + name: 'invalid OperationResponse missing required fields', document: { components: { schemas: { OperationResponse: { type: 'object', + required: ['operationType'], properties: { operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, createdAt: { type: 'string', format: 'date-time' }, @@ -86,82 +80,31 @@ testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ // Results are ordered by document position: the schema-level errors anchor at the schema // object, before the property-level errors { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + code: 'xgen-IPA-132-operation-response-must-define-required-fields', message: 'OperationResponse must define the operationId property.', path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', - message: 'OperationResponse must define the updatedAt property.', + code: 'xgen-IPA-132-operation-response-must-define-required-fields', + message: 'OperationResponse must define the status property.', path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', - message: CUSTOM_METHOD_MISSING_ERROR_MESSAGE, + code: 'xgen-IPA-132-operation-response-must-define-required-fields', + message: 'OperationResponse must define the updatedAt property.', path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', - message: 'The operationType property must be listed as required.', - path: ['components', 'schemas', 'OperationResponse', 'properties', 'operationType'], - severity: DiagnosticSeverity.Warning, - }, - { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + code: 'xgen-IPA-132-operation-response-must-define-required-fields', message: 'The createdAt property must be listed as required.', path: ['components', 'schemas', 'OperationResponse', 'properties', 'createdAt'], severity: DiagnosticSeverity.Warning, }, ], }, - { - name: 'invalid operationType enum missing a value', - document: { - components: { - schemas: { - OperationResponse: { - ...validOperationResponse, - properties: { - ...validOperationResponse.properties, - operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE'] }, - }, - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', - message: OPERATION_TYPE_ENUM_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse', 'properties', 'operationType'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'invalid customMethod listed as required', - document: { - components: { - schemas: { - OperationResponse: { - ...validOperationResponse, - required: ['operationId', 'operationType', 'createdAt', 'updatedAt', 'customMethod'], - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', - message: CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse', 'properties', 'customMethod'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, { name: 'invalid OperationResponse with an exception', document: { @@ -173,7 +116,7 @@ testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, }, 'x-xgen-IPA-exception': { - 'xgen-IPA-132-operation-response-must-include-core-metadata': 'reason', + 'xgen-IPA-132-operation-response-must-define-required-fields': 'reason', }, }, }, @@ -189,7 +132,7 @@ testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ OperationResponse: { ...validOperationResponse, 'x-xgen-IPA-exception': { - 'xgen-IPA-132-operation-response-must-include-core-metadata': 'reason', + 'xgen-IPA-132-operation-response-must-define-required-fields': 'reason', }, }, }, @@ -197,14 +140,14 @@ testRule('xgen-IPA-132-operation-response-must-include-core-metadata', [ }, errors: [ { - code: 'xgen-IPA-132-operation-response-must-include-core-metadata', + code: 'xgen-IPA-132-operation-response-must-define-required-fields', message: 'This component adopts the rule and does not need an exception. Please remove the exception.', path: [ 'components', 'schemas', 'OperationResponse', 'x-xgen-IPA-exception', - 'xgen-IPA-132-operation-response-must-include-core-metadata', + 'xgen-IPA-132-operation-response-must-define-required-fields', ], severity: DiagnosticSeverity.Warning, }, diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js new file mode 100644 index 0000000000..47f41d3e51 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js @@ -0,0 +1,182 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const STATUS_ENUM_ERROR_MESSAGE = + 'The status field must use exactly the enum [PENDING, IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED].'; +const OPERATION_TYPE_ENUM_ERROR_MESSAGE = + 'The operationType field must use exactly the enum [CREATE, UPDATE, DELETE, CUSTOM].'; +const RETRY_STRATEGY_ENUM_ERROR_MESSAGE = + 'The retryStrategy field must use exactly the enum [IMMEDIATE, BACKOFF, NONE].'; + +const validOperationResponse = { + type: 'object', + properties: { + status: { type: 'string', enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'] }, + operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, + error: { + type: 'object', + properties: { + code: { type: 'string' }, + message: { type: 'string' }, + retryable: { type: 'boolean' }, + retryStrategy: { type: 'string', enum: ['IMMEDIATE', 'BACKOFF', 'NONE'] }, + }, + }, + }, +}; + +testRule('xgen-IPA-132-operation-response-must-use-standard-enum-values', [ + { + name: 'valid OperationResponse with all standard enums', + document: { + components: { + schemas: { + OperationResponse: validOperationResponse, + }, + }, + }, + errors: [], + }, + { + name: 'valid OperationResponse composed with allOf', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, validOperationResponse], + }, + OperationMetadata: { + type: 'object', + properties: { + statusMessage: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'enum fields that are not defined are skipped', + document: { + components: { + schemas: { + // Missing fields are the required-fields and optional-fields rules' concern + OperationResponse: { + type: 'object', + properties: { + operationId: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid enums with missing and renamed values', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { type: 'string', enum: ['QUEUED', 'RUNNING', 'COMPLETE', 'ERROR', 'CANCELLED'] }, + operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE'] }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-use-standard-enum-values', + message: STATUS_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-response-must-use-standard-enum-values', + message: OPERATION_TYPE_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'operationType'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid retryStrategy enum nested in the error object', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + error: { + type: 'object', + properties: { + retryStrategy: { type: 'string', enum: ['IMMEDIATE', 'EXPONENTIAL'] }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-use-standard-enum-values', + message: RETRY_STRATEGY_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'error'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse with an exception', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { type: 'string', enum: ['QUEUED'] }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-use-standard-enum-values': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant OperationResponse does not need an exception', + document: { + components: { + schemas: { + OperationResponse: { + ...validOperationResponse, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-use-standard-enum-values': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-use-standard-enum-values', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'OperationResponse', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operation-response-must-use-standard-enum-values', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js b/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js deleted file mode 100644 index 906450a7f0..0000000000 --- a/tools/spectral/ipa/__tests__/IPA132OperationStatusMustUseTheStandardStatusEnum.test.js +++ /dev/null @@ -1,265 +0,0 @@ -import testRule from './__helpers__/testRule'; -import { DiagnosticSeverity } from '@stoplight/types'; - -const MISSING_STATUS_ERROR_MESSAGE = 'OperationResponse must report progress through a status field.'; -const STATUS_REQUIRED_ERROR_MESSAGE = 'The status property must be listed as required.'; -const STATUS_ENUM_ERROR_MESSAGE = - 'The status field must use exactly the enum [PENDING, IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED].'; - -const STANDARD_STATUS_ENUM = ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED']; - -const operationResponseWithStandardStatus = { - type: 'object', - required: ['status'], - properties: { - status: { - type: 'string', - enum: STANDARD_STATUS_ENUM, - }, - }, -}; - -testRule('xgen-IPA-132-operation-status-must-use-the-standard-status-enum', [ - { - name: 'valid OperationResponse with the standard status enum', - document: { - components: { - schemas: { - OperationResponse: operationResponseWithStandardStatus, - }, - }, - }, - errors: [], - }, - { - name: 'valid OperationResponse with the standard status enum in a different order', - document: { - components: { - schemas: { - OperationResponse: { - ...operationResponseWithStandardStatus, - properties: { - status: { - type: 'string', - enum: ['SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED', 'PENDING', 'IN_PROGRESS'], - }, - }, - }, - }, - }, - }, - errors: [], - }, - { - name: 'valid OperationResponse composed with allOf', - document: { - components: { - schemas: { - OperationResponse: { - allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, operationResponseWithStandardStatus], - }, - OperationMetadata: { - type: 'object', - properties: { - operationId: { type: 'string' }, - }, - }, - }, - }, - }, - errors: [], - }, - { - name: 'documents without an OperationResponse schema are ignored', - document: { - components: { - schemas: { - Order: { - type: 'object', - properties: { - status: { - type: 'string', - enum: ['OPEN', 'CLOSED'], - }, - }, - }, - }, - }, - }, - errors: [], - }, - { - name: 'invalid OperationResponse without a status field', - document: { - components: { - schemas: { - OperationResponse: { - type: 'object', - properties: { - state: { - type: 'string', - enum: ['QUEUED', 'RUNNING', 'COMPLETE', 'ERROR', 'CANCELLED'], - }, - }, - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', - message: MISSING_STATUS_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'invalid status property not listed as required', - document: { - components: { - schemas: { - OperationResponse: { - type: 'object', - properties: operationResponseWithStandardStatus.properties, - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', - message: STATUS_REQUIRED_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'invalid status enum with non-standard values', - document: { - components: { - schemas: { - OperationResponse: { - ...operationResponseWithStandardStatus, - properties: { - status: { - type: 'string', - enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELLED', 'SUPERSEDED'], - }, - }, - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', - message: STATUS_ENUM_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'invalid status enum with a missing value', - document: { - components: { - schemas: { - OperationResponse: { - ...operationResponseWithStandardStatus, - properties: { - status: { - type: 'string', - enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED'], - }, - }, - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', - message: STATUS_ENUM_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'invalid status field without an enum', - document: { - components: { - schemas: { - OperationResponse: { - ...operationResponseWithStandardStatus, - properties: { - status: { - type: 'string', - }, - }, - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', - message: STATUS_ENUM_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'invalid OperationResponse with an exception', - document: { - components: { - schemas: { - OperationResponse: { - type: 'object', - properties: { - state: { - type: 'string', - }, - }, - 'x-xgen-IPA-exception': { - 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum': 'reason', - }, - }, - }, - }, - }, - errors: [], - }, - { - name: 'compliant OperationResponse does not need an exception', - document: { - components: { - schemas: { - OperationResponse: { - ...operationResponseWithStandardStatus, - 'x-xgen-IPA-exception': { - 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum': 'reason', - }, - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', - message: 'This component adopts the rule and does not need an exception. Please remove the exception.', - path: [ - 'components', - 'schemas', - 'OperationResponse', - 'x-xgen-IPA-exception', - 'xgen-IPA-132-operation-status-must-use-the-standard-status-enum', - ], - severity: DiagnosticSeverity.Warning, - }, - ], - }, -]); diff --git a/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js new file mode 100644 index 0000000000..fe97b5b8a4 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js @@ -0,0 +1,182 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const progress = { + type: 'object', + properties: { + completed: { type: 'number' }, + total: { type: 'number' }, + unit: { type: 'string' }, + }, +}; + +const operationResponseWithStatusFields = { + type: 'object', + properties: { + statusMessage: { type: 'string' }, + progress, + estimatedCompletionTime: { type: 'string', format: 'date-time' }, + }, +}; + +testRule('xgen-IPA-132-operations-should-expose-status-fields', [ + { + name: 'valid OperationResponse with all status fields', + document: { + components: { + schemas: { + OperationResponse: operationResponseWithStatusFields, + }, + }, + }, + errors: [], + }, + { + name: 'valid OperationResponse composed with allOf', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, operationResponseWithStatusFields], + }, + OperationMetadata: { + type: 'object', + properties: { + operationId: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'documents without an OperationResponse schema are ignored', + document: { + components: { + schemas: { + Order: { + type: 'object', + properties: { + id: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid OperationResponse without any status fields', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operations-should-expose-status-fields', + message: 'OperationResponse should define a statusMessage property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operations-should-expose-status-fields', + message: 'OperationResponse should define a progress property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operations-should-expose-status-fields', + message: 'OperationResponse should define a estimatedCompletionTime property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid progress object missing a field', + document: { + components: { + schemas: { + OperationResponse: { + ...operationResponseWithStatusFields, + properties: { + ...operationResponseWithStatusFields.properties, + progress: { + type: 'object', + properties: { + completed: { type: 'number' }, + total: { type: 'number' }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operations-should-expose-status-fields', + message: 'The progress object should define a unit property.', + path: ['components', 'schemas', 'OperationResponse', 'properties', 'progress'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse with an exception', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { type: 'string' }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operations-should-expose-status-fields': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant OperationResponse does not need an exception', + document: { + components: { + schemas: { + OperationResponse: { + ...operationResponseWithStatusFields, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operations-should-expose-status-fields': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operations-should-expose-status-fields', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'OperationResponse', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operations-should-expose-status-fields', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index 86d899dfe9..ded0af63ef 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -6,8 +6,11 @@ functions: - IPA132OperationMustBeAReadOnlyResource - IPA132OperationsEndpointMustBeALeafResource - IPA132OperationEndpointsMustReturnOperationResponse - - IPA132OperationStatusMustUseTheStandardStatusEnum - - IPA132OperationResponseMustIncludeCoreMetadata + - IPA132OperationResponseMustDefineRequiredFields + - IPA132OperationResponseMustDefineOptionalFields + - IPA132OperationResponseMustUseStandardEnumValues + - IPA132OperationResponseMustDefineNestedObjectStructure + - IPA132OperationsShouldExposeStatusFields aliases: OperationResponseSchema: @@ -106,45 +109,152 @@ rules: field: '@key' function: IPA132OperationEndpointsMustReturnOperationResponse - xgen-IPA-132-operation-status-must-use-the-standard-status-enum: + xgen-IPA-132-operation-response-must-define-required-fields: description: | - An Operation must report progress through a status field, using exactly the enum PENDING, - IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED. + The OperationResponse schema must define every field the IPA-132 standard marks as always + required: operationId, status, operationType, createdAt and updatedAt. ##### Implementation details Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The schema must define a `status` property, listed as required - - The `status` enum must contain exactly the values `PENDING`, `IN_PROGRESS`, `SUCCEEDED`, - `FAILED`, `CANCELED` and `SUPERSEDED`, in any order + - Each field listed in the rule's functionOptions must be defined and listed as required + - Properties and required lists spread across allOf sub-schemas are merged before + validation + - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum + and IPA-132-operation-response-must-include-core-metadata guidelines - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation - message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-status-must-use-the-standard-status-enum' + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-required-fields' severity: warn given: '#OperationResponseSchema' then: - function: IPA132OperationStatusMustUseTheStandardStatusEnum + function: IPA132OperationResponseMustDefineRequiredFields + functionOptions: + fields: + - operationId + - status + - operationType + - createdAt + - updatedAt - xgen-IPA-132-operation-response-must-include-core-metadata: + xgen-IPA-132-operation-response-must-define-optional-fields: description: | - An OperationResponse must include stable core metadata for the operation record: a required - operationId, a required operationType, a required createdAt and a required updatedAt. + The OperationResponse schema must define every field the IPA-132 standard marks as + conditionally present, and none of them may be listed as required: customMethod, error, + resultHref and retryAfterSeconds are present only under runtime conditions. ##### Implementation details Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The properties `operationId`, `operationType`, `createdAt` and `updatedAt` must be - defined and listed as required - - The `operationType` enum must contain exactly the values `CREATE`, `UPDATE`, `DELETE` - and `CUSTOM`, in any order - - A `customMethod` property must be defined and must not be listed as required: it is - required only when `operationType` is `CUSTOM`, which cannot be validated statically + - Each field listed in the rule's functionOptions must be defined and must not be listed + as required: the condition under which it is present is a runtime state that cannot be + validated statically + - Properties and required lists spread across allOf sub-schemas are merged before + validation + - Covers the conditional-field checks of the IPA-132-operation-response-must-include-core-metadata, + IPA-132-failed-lros-must-report-a-structured-error, + IPA-132-successful-lros-must-reference-the-result-resource and + IPA-132-in-progress-operations-must-expose-retry-after-seconds guidelines - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation - message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-include-core-metadata' + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-optional-fields' severity: warn given: '#OperationResponseSchema' then: - function: IPA132OperationResponseMustIncludeCoreMetadata + function: IPA132OperationResponseMustDefineOptionalFields + functionOptions: + fields: + - name: customMethod + condition: when operationType is CUSTOM + - name: error + condition: when the operation has FAILED + - name: resultHref + condition: when the operation has SUCCEEDED + - name: retryAfterSeconds + condition: while the operation is in a non-terminal state + + xgen-IPA-132-operation-response-must-use-standard-enum-values: + description: | + Every enum field of the OperationResponse schema must use exactly the standard enum values + defined by IPA-132: the status lifecycle enum, the operationType enum and the retryStrategy + enum of the error object. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - Each enum field listed in the rule's functionOptions must use exactly the listed values, + in any order + - Fields that are not defined are skipped: their presence is validated by the + required-fields and optional-fields rules + - Properties spread across allOf sub-schemas are merged before validation + - Covers the enum checks of the IPA-132-operation-status-must-use-the-standard-status-enum, + IPA-132-operation-response-must-include-core-metadata and + IPA-132-failed-lros-must-report-a-structured-error guidelines + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-use-standard-enum-values' + severity: warn + given: '#OperationResponseSchema' + then: + function: IPA132OperationResponseMustUseStandardEnumValues + functionOptions: + enums: + - field: status + values: [PENDING, IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED] + - field: operationType + values: [CREATE, UPDATE, DELETE, CUSTOM] + - field: retryStrategy + parent: error + values: [IMMEDIATE, BACKOFF, NONE] + + xgen-IPA-132-operation-response-must-define-nested-object-structure: + description: | + Every nested object field of the OperationResponse schema must define the properties the + IPA-132 standard requires of it: the error object must define code, message and retryable. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - Each nested object field listed in the rule's functionOptions must define the listed + properties + - Fields that are not defined are skipped: their presence is validated by the + optional-fields rule + - Properties spread across allOf sub-schemas are merged before validation + - Covers the error object structure check of the + IPA-132-failed-lros-must-report-a-structured-error guideline + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-nested-object-structure' + severity: warn + given: '#OperationResponseSchema' + then: + function: IPA132OperationResponseMustDefineNestedObjectStructure + functionOptions: + objects: + - field: error + properties: [code, message, retryable] + + xgen-IPA-132-operations-should-expose-status-fields: + description: | + An Operation should expose statusMessage, progress, and estimatedCompletionTime so clients + can reason about in-flight work. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The schema should define `statusMessage`, `progress` and `estimatedCompletionTime` + properties + - The progress object should define `completed`, `total` and `unit` properties + - Kept separate from the must-level rules because it enforces should-level guidance + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operations-should-expose-status-fields' + severity: warn + given: '#OperationResponseSchema' + then: + function: IPA132OperationsShouldExposeStatusFields diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 239a776b0d..5f2b7c3001 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1478,37 +1478,99 @@ Rule checks for the following conditions: - Responses with `x-xgen-IPA-exception` for this rule at the content media type level are excluded from validation -#### xgen-IPA-132-operation-status-must-use-the-standard-status-enum +#### xgen-IPA-132-operation-response-must-define-required-fields ![warn](https://img.shields.io/badge/warning-yellow) -An Operation must report progress through a status field, using exactly the enum PENDING, -IN_PROGRESS, SUCCEEDED, FAILED, CANCELED, SUPERSEDED. +The OperationResponse schema must define every field the IPA-132 standard marks as always +required: operationId, status, operationType, createdAt and updatedAt. ##### Implementation details Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The schema must define a `status` property, listed as required - - The `status` enum must contain exactly the values `PENDING`, `IN_PROGRESS`, `SUCCEEDED`, - `FAILED`, `CANCELED` and `SUPERSEDED`, in any order + - Each field listed in the rule's functionOptions must be defined and listed as required + - Properties and required lists spread across allOf sub-schemas are merged before + validation + - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum + and IPA-132-operation-response-must-include-core-metadata guidelines - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation -#### xgen-IPA-132-operation-response-must-include-core-metadata +#### xgen-IPA-132-operation-response-must-define-optional-fields ![warn](https://img.shields.io/badge/warning-yellow) -An OperationResponse must include stable core metadata for the operation record: a required -operationId, a required operationType, a required createdAt and a required updatedAt. +The OperationResponse schema must define every field the IPA-132 standard marks as +conditionally present, and none of them may be listed as required: customMethod, error, +resultHref and retryAfterSeconds are present only under runtime conditions. ##### Implementation details Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The properties `operationId`, `operationType`, `createdAt` and `updatedAt` must be - defined and listed as required - - The `operationType` enum must contain exactly the values `CREATE`, `UPDATE`, `DELETE` - and `CUSTOM`, in any order - - A `customMethod` property must be defined and must not be listed as required: it is - required only when `operationType` is `CUSTOM`, which cannot be validated statically + - Each field listed in the rule's functionOptions must be defined and must not be listed + as required: the condition under which it is present is a runtime state that cannot be + validated statically + - Properties and required lists spread across allOf sub-schemas are merged before + validation + - Covers the conditional-field checks of the IPA-132-operation-response-must-include-core-metadata, + IPA-132-failed-lros-must-report-a-structured-error, + IPA-132-successful-lros-must-reference-the-result-resource and + IPA-132-in-progress-operations-must-expose-retry-after-seconds guidelines + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + +#### xgen-IPA-132-operation-response-must-use-standard-enum-values + + ![warn](https://img.shields.io/badge/warning-yellow) +Every enum field of the OperationResponse schema must use exactly the standard enum values +defined by IPA-132: the status lifecycle enum, the operationType enum and the retryStrategy +enum of the error object. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - Each enum field listed in the rule's functionOptions must use exactly the listed values, + in any order + - Fields that are not defined are skipped: their presence is validated by the + required-fields and optional-fields rules + - Properties spread across allOf sub-schemas are merged before validation + - Covers the enum checks of the IPA-132-operation-status-must-use-the-standard-status-enum, + IPA-132-operation-response-must-include-core-metadata and + IPA-132-failed-lros-must-report-a-structured-error guidelines + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + +#### xgen-IPA-132-operation-response-must-define-nested-object-structure + + ![warn](https://img.shields.io/badge/warning-yellow) +Every nested object field of the OperationResponse schema must define the properties the +IPA-132 standard requires of it: the error object must define code, message and retryable. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - Each nested object field listed in the rule's functionOptions must define the listed + properties + - Fields that are not defined are skipped: their presence is validated by the + optional-fields rule + - Properties spread across allOf sub-schemas are merged before validation + - Covers the error object structure check of the + IPA-132-failed-lros-must-report-a-structured-error guideline + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + +#### xgen-IPA-132-operations-should-expose-status-fields + + ![warn](https://img.shields.io/badge/warning-yellow) +An Operation should expose statusMessage, progress, and estimatedCompletionTime so clients +can reason about in-flight work. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The schema should define `statusMessage`, `progress` and `estimatedCompletionTime` + properties + - The progress object should define `completed`, `total` and `unit` properties + - Kept separate from the must-level rules because it enforces should-level guidance - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js new file mode 100644 index 0000000000..f5f73ae60e --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js @@ -0,0 +1,48 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { getMergedProperties } from './utils/longRunningOperations.js'; + +/** + * Checks that every nested object field of the OperationResponse schema defined by IPA-132 + * defines the properties the standard requires of it. The object fields and their properties are + * provided declaratively through the rule's functionOptions. Fields that are not defined are + * skipped: their presence is validated by the optional-fields rule. + * + * @param {object} input - The OperationResponse schema object + * @param {object} opts - The rule's functionOptions, carrying the object fields and their properties + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, opts, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, opts.objects, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, objects, path, ruleName) { + try { + // Properties may be spread across allOf sub-schemas + const schemaProperties = getMergedProperties(schema); + const errors = []; + + for (const { field, properties } of objects) { + const objectSchema = schemaProperties[field]; + // Absent fields are the optional-fields rule's concern + if (!objectSchema) { + continue; + } + + const objectProperties = getMergedProperties(objectSchema); + for (const property of properties) { + if (!objectProperties[property]) { + errors.push({ + path: [...path, 'properties', field], + message: `The ${field} object must define a ${property} property.`, + }); + } + } + } + + return errors; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js new file mode 100644 index 0000000000..83aef26fa3 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js @@ -0,0 +1,42 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { getMergedProperties, getMergedRequired } from './utils/longRunningOperations.js'; + +/** + * Checks that the OperationResponse schema defined by IPA-132 defines every field the standard + * marks as conditionally present, and that none of them is listed as required: each field is + * present only under a runtime condition that cannot be validated statically. The field names + * and their conditions are provided declaratively through the rule's functionOptions. + * + * @param {object} input - The OperationResponse schema object + * @param {object} opts - The rule's functionOptions, carrying the field names and conditions + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, opts, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, opts.fields, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, fields, path, ruleName) { + try { + // Properties and required lists may be spread across allOf sub-schemas + const properties = getMergedProperties(schema); + const required = getMergedRequired(schema); + const errors = []; + + for (const { name, condition } of fields) { + if (!properties[name]) { + errors.push({ path, message: `OperationResponse must define the ${name} property.` }); + } else if (required.includes(name)) { + errors.push({ + path: [...path, 'properties', name], + message: `The ${name} property must not be listed as required. It is present only ${condition}, which cannot be validated statically.`, + }); + } + } + + return errors; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js new file mode 100644 index 0000000000..8755f8935d --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js @@ -0,0 +1,41 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { getMergedProperties, getMergedRequired } from './utils/longRunningOperations.js'; + +/** + * Checks that the OperationResponse schema defined by IPA-132 defines every field the standard + * marks as always required, and lists each of them in the schema's required array. The field + * names are provided declaratively through the rule's functionOptions. + * + * @param {object} input - The OperationResponse schema object + * @param {object} opts - The rule's functionOptions, carrying the required field names + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, opts, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, opts.fields, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, fields, path, ruleName) { + try { + // Properties and required lists may be spread across allOf sub-schemas + const properties = getMergedProperties(schema); + const required = getMergedRequired(schema); + const errors = []; + + for (const field of fields) { + if (!properties[field]) { + errors.push({ path, message: `OperationResponse must define the ${field} property.` }); + } else if (!required.includes(field)) { + errors.push({ + path: [...path, 'properties', field], + message: `The ${field} property must be listed as required.`, + }); + } + } + + return errors; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js deleted file mode 100644 index b558c41788..0000000000 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustIncludeCoreMetadata.js +++ /dev/null @@ -1,65 +0,0 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { - getMergedProperties, - getMergedRequired, - isExactEnumMatch, - OPERATION_TYPE_ENUM, -} from './utils/longRunningOperations.js'; - -const CORE_METADATA_FIELDS = ['operationId', 'operationType', 'createdAt', 'updatedAt']; -const OPERATION_TYPE_ENUM_ERROR_MESSAGE = `The operationType field must use exactly the enum [${OPERATION_TYPE_ENUM.join(', ')}].`; -const CUSTOM_METHOD_MISSING_ERROR_MESSAGE = - 'OperationResponse must define a customMethod property for custom method operations.'; -const CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE = - 'The customMethod property must not be listed as required. It is required only when operationType is CUSTOM.'; - -/** - * Checks that the OperationResponse schema defined by IPA-132 includes the stable core metadata - * for the operation record: required operationId, operationType, createdAt and updatedAt - * properties, the standard operationType enum, and an optional customMethod property. - * - * @param {object} input - The OperationResponse schema object - * @param {object} _ - Unused - * @param {object} context - The context object containing the path, documentInventory and rule - */ -export default (input, _, { path, rule }) => { - const ruleName = rule.name; - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(schema, path, ruleName) { - try { - // Properties and required lists may be spread across allOf sub-schemas - const properties = getMergedProperties(schema); - const required = getMergedRequired(schema); - const errors = []; - - for (const field of CORE_METADATA_FIELDS) { - if (!properties[field]) { - errors.push({ path, message: `OperationResponse must define the ${field} property.` }); - } else if (!required.includes(field)) { - errors.push({ - path: [...path, 'properties', field], - message: `The ${field} property must be listed as required.`, - }); - } - } - - if (properties.operationType && !isExactEnumMatch(properties.operationType.enum, OPERATION_TYPE_ENUM)) { - errors.push({ path: [...path, 'properties', 'operationType'], message: OPERATION_TYPE_ENUM_ERROR_MESSAGE }); - } - - // customMethod is required only when operationType is CUSTOM, which is a runtime condition - // that cannot be validated statically: the property must be defined and must not be required - if (!properties.customMethod) { - errors.push({ path, message: CUSTOM_METHOD_MISSING_ERROR_MESSAGE }); - } else if (required.includes('customMethod')) { - errors.push({ path: [...path, 'properties', 'customMethod'], message: CUSTOM_METHOD_REQUIRED_ERROR_MESSAGE }); - } - - return errors; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js new file mode 100644 index 0000000000..de39f24f7c --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js @@ -0,0 +1,54 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { getMergedProperties, isExactEnumMatch } from './utils/longRunningOperations.js'; + +/** + * Checks that every enum field of the OperationResponse schema defined by IPA-132 uses exactly + * the standard enum values, in any order. The field names and expected values are provided + * declaratively through the rule's functionOptions. Fields that are not defined are skipped: + * their presence is validated by the required-fields and optional-fields rules. + * + * @param {object} input - The OperationResponse schema object + * @param {object} opts - The rule's functionOptions, carrying the enum fields and their values + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, opts, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, opts.enums, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, enums, path, ruleName) { + try { + // Properties may be spread across allOf sub-schemas + const properties = getMergedProperties(schema); + const errors = []; + + for (const { field, parent, values } of enums) { + let property; + let propertyPath; + if (parent) { + const parentProperty = properties[parent]; + property = parentProperty ? getMergedProperties(parentProperty)[field] : undefined; + propertyPath = [...path, 'properties', parent]; + } else { + property = properties[field]; + propertyPath = [...path, 'properties', field]; + } + + // Absent fields are the required-fields and optional-fields rules' concern + if (!property) { + continue; + } + if (!isExactEnumMatch(property.enum, values)) { + errors.push({ + path: propertyPath, + message: `The ${field} field must use exactly the enum [${values.join(', ')}].`, + }); + } + } + + return errors; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js deleted file mode 100644 index bf1ce9bd88..0000000000 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationStatusMustUseTheStandardStatusEnum.js +++ /dev/null @@ -1,46 +0,0 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { - getMergedProperties, - getMergedRequired, - isExactEnumMatch, - OPERATION_STATUS_ENUM, -} from './utils/longRunningOperations.js'; - -const MISSING_STATUS_ERROR_MESSAGE = 'OperationResponse must report progress through a status field.'; -const STATUS_REQUIRED_ERROR_MESSAGE = 'The status property must be listed as required.'; -const STATUS_ENUM_ERROR_MESSAGE = `The status field must use exactly the enum [${OPERATION_STATUS_ENUM.join(', ')}].`; - -/** - * Checks that the OperationResponse schema defined by IPA-132 reports progress through a status - * field using exactly the standard status enum. - * - * @param {object} input - The OperationResponse schema object - * @param {object} _ - Unused - * @param {object} context - The context object containing the path, documentInventory and rule - */ -export default (input, _, { path, rule }) => { - const ruleName = rule.name; - const errors = checkViolationsAndReturnErrors(input, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); -}; - -function checkViolationsAndReturnErrors(schema, path, ruleName) { - try { - // Properties and required lists may be spread across allOf sub-schemas - const status = getMergedProperties(schema).status; - if (!status) { - return [{ path, message: MISSING_STATUS_ERROR_MESSAGE }]; - } - - const errors = []; - if (!getMergedRequired(schema).includes('status')) { - errors.push({ path: [...path, 'properties', 'status'], message: STATUS_REQUIRED_ERROR_MESSAGE }); - } - if (!isExactEnumMatch(status.enum, OPERATION_STATUS_ENUM)) { - errors.push({ path: [...path, 'properties', 'status'], message: STATUS_ENUM_ERROR_MESSAGE }); - } - return errors; - } catch (e) { - return handleInternalError(ruleName, path, e); - } -} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js new file mode 100644 index 0000000000..66301e244c --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js @@ -0,0 +1,48 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { getMergedProperties } from './utils/longRunningOperations.js'; + +const STATUS_FIELDS = ['statusMessage', 'progress', 'estimatedCompletionTime']; +const PROGRESS_FIELDS = ['completed', 'total', 'unit']; + +/** + * Checks that the OperationResponse schema defined by IPA-132 exposes the optional progress + * fields statusMessage, progress and estimatedCompletionTime, so clients can reason about + * in-flight work. + * + * @param {object} input - The OperationResponse schema object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, _, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, path, ruleName) { + try { + // Properties may be spread across allOf sub-schemas + const properties = getMergedProperties(schema); + const errors = []; + for (const field of STATUS_FIELDS) { + if (!properties[field]) { + errors.push({ path, message: `OperationResponse should define a ${field} property.` }); + } + } + + if (properties.progress) { + const progressProperties = getMergedProperties(properties.progress); + for (const field of PROGRESS_FIELDS) { + if (!progressProperties[field]) { + errors.push({ + path: [...path, 'properties', 'progress'], + message: `The progress object should define a ${field} property.`, + }); + } + } + } + return errors; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} From 907d1c6b576c1379036e08465e95fb127f1b5537 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Wed, 19 Aug 2026 12:08:05 +0100 Subject: [PATCH 07/12] fix(ipa): address deep-review findings in the IPA-132 schema rules - Follow responses defined as refs to components.responses instead of silently skipping them, and flag 2xx responses without a JSON schema - Reject allOf-composed paginated wrappers, consistent with the IPA-110 pagination rules, and require the results array directly on the wrapper - Merge allOf properties conjunctively: keyword maps combine instead of overwriting and enums apply intersected - Anchor property violations at their actual document location inside allOf members instead of a location that only exists after merging - Iterate response codes from the responses object so missing content is visible, moving rule exceptions to the response level - Drive the should-level status fields rule from functionOptions like its siblings, and fix the a/an grammar in its messages - Drop the unused enum constants: the ruleset functionOptions are the single source of truth for enum values --- ...dpointsMustReturnOperationResponse.test.js | 167 ++++++++++++++++-- ...onResponseMustDefineRequiredFields.test.js | 36 ++++ ...nResponseMustUseStandardEnumValues.test.js | 48 +++++ ...OperationsShouldExposeStatusFields.test.js | 8 +- .../utils/longRunningOperations.test.js | 74 ++++++-- tools/spectral/ipa/rulesets/IPA-132.yaml | 36 ++-- tools/spectral/ipa/rulesets/README.md | 29 +-- ...ionEndpointsMustReturnOperationResponse.js | 111 +++++++----- ...ResponseMustDefineNestedObjectStructure.js | 6 +- ...erationResponseMustDefineOptionalFields.js | 5 +- ...erationResponseMustDefineRequiredFields.js | 5 +- ...rationResponseMustUseStandardEnumValues.js | 7 +- ...PA132OperationsShouldExposeStatusFields.js | 38 ++-- .../functions/utils/longRunningOperations.js | 61 ++++++- 14 files changed, 499 insertions(+), 132 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js index 4b62f8c09a..8772617595 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js @@ -4,6 +4,7 @@ import { DiagnosticSeverity } from '@stoplight/types'; const SINGLE_OPERATION_ERROR_MESSAGE = 'The Operation endpoint must return the OperationResponse schema.'; const OPERATIONS_COLLECTION_ERROR_MESSAGE = 'The Operations collection endpoint must return a paginated response whose results reference the OperationResponse schema.'; +const MISSING_SCHEMA_ERROR_MESSAGE = 'The Operations endpoint must define a JSON response schema.'; const components = { schemas: { @@ -44,7 +45,8 @@ const components = { totalCount: { type: 'integer' }, }, }, - // A paginated wrapper composed with allOf, which Spectral does not flatten + // A wrapper composed with allOf is rejected: the IPA-110 pagination rules require the + // results array to be defined directly on the wrapper schema PaginatedAllOfOperationResponse: { allOf: [ { $ref: '#/components/schemas/PaginatedMetadata' }, @@ -66,6 +68,24 @@ const components = { }, }, }, + responses: { + OperationResponseOk: { + description: 'Operation status.', + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/OperationResponse' }, + }, + }, + }, + OrderOk: { + description: 'An order.', + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { $ref: '#/components/schemas/Order' }, + }, + }, + }, + }, }; const operationResponseGet = { @@ -206,10 +226,15 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ get: operationResponseGetWithNotFound, }, '/api/atlas/v2/resourceName/{pathParam}/operations': { - get: paginatedAllOfOperationResponseGet, + get: paginatedOperationResponseGet, }, + // The response may be defined as a reference to a shared component response '/api/atlas/v2/resourceName/{pathParam}/operations/{operationId}': { - get: operationResponseGet, + get: { + responses: { + 200: { $ref: '#/components/responses/OperationResponseOk' }, + }, + }, }, // Not an Operations endpoint, the returned schema is not restricted '/api/atlas/v2/resourceName/{pathParam}': { @@ -274,6 +299,99 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ }, ], }, + { + name: 'invalid single Operation endpoint referencing a shared non-Operation response', + document: { + paths: { + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: { + responses: { + 200: { $ref: '#/components/responses/OrderOk' }, + }, + }, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: SINGLE_OPERATION_ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName/operations/{operationId}', 'get', 'responses', '200'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid Operations endpoint without a JSON response schema', + document: { + paths: { + // No content at all + '/api/atlas/v2/resourceName/operations/{operationId}': { + get: { + responses: { + 200: { description: 'Operation status.' }, + }, + }, + }, + // A JSON media type without a schema + '/api/atlas/v2/resourceName/operations': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': {}, + }, + }, + }, + }, + }, + // Only non-JSON media types + '/api/atlas/v2/resourceName/{pathParam}/operations': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+gzip': { + schema: { type: 'string', format: 'binary' }, + }, + }, + }, + }, + }, + }, + }, + components, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: MISSING_SCHEMA_ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName/operations/{operationId}', 'get', 'responses', '200'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: MISSING_SCHEMA_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: MISSING_SCHEMA_ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName/{pathParam}/operations', 'get', 'responses', '200'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'invalid Operations collection returning a non-Operation paginated response', document: { @@ -355,6 +473,35 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ }, ], }, + { + name: 'invalid Operations collection with an allOf-composed paginated wrapper', + document: { + paths: { + '/api/atlas/v2/resourceName/operations': { + get: paginatedAllOfOperationResponseGet, + }, + }, + components, + }, + errors: [ + // The IPA-110 pagination rules require the results array directly on the wrapper schema, + // so an allOf-composed wrapper would fail xgen-IPA-110-collections-response-define-results-array + { + code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', + message: OPERATIONS_COLLECTION_ERROR_MESSAGE, + path: [ + 'paths', + '/api/atlas/v2/resourceName/operations', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'invalid Operations collection returning an unpaginated OperationResponse', document: { @@ -393,11 +540,11 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ content: { 'application/vnd.atlas.2024-08-05+json': { schema: { $ref: '#/components/schemas/Order' }, - 'x-xgen-IPA-exception': { - 'xgen-IPA-132-operation-endpoints-must-return-operation-response': 'reason', - }, }, }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-endpoints-must-return-operation-response': 'reason', + }, }, }, }, @@ -418,11 +565,11 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ content: { 'application/vnd.atlas.2024-08-05+json': { schema: { $ref: '#/components/schemas/OperationResponse' }, - 'x-xgen-IPA-exception': { - 'xgen-IPA-132-operation-endpoints-must-return-operation-response': 'reason', - }, }, }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-endpoints-must-return-operation-response': 'reason', + }, }, }, }, @@ -440,8 +587,6 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ 'get', 'responses', '200', - 'content', - 'application/vnd.atlas.2024-08-05+json', 'x-xgen-IPA-exception', 'xgen-IPA-132-operation-endpoints-must-return-operation-response', ], diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js index 17ffc3b4ba..3343657dd1 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js @@ -105,6 +105,42 @@ testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ }, ], }, + { + name: 'invalid required field defined only within an allOf member', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [ + { + type: 'object', + required: ['operationId', 'status', 'operationType', 'updatedAt'], + properties: { + operationId: { type: 'string' }, + status: { + type: 'string', + enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], + }, + operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, + createdAt: { type: 'string', format: 'date-time' }, + updatedAt: { type: 'string', format: 'date-time' }, + }, + }, + ], + }, + }, + }, + }, + errors: [ + // The violation anchors at the property's actual location inside the allOf member + { + code: 'xgen-IPA-132-operation-response-must-define-required-fields', + message: 'The createdAt property must be listed as required.', + path: ['components', 'schemas', 'OperationResponse', 'allOf', '0', 'properties', 'createdAt'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'invalid OperationResponse with an exception', document: { diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js index 47f41d3e51..001e97916c 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js @@ -56,6 +56,54 @@ testRule('xgen-IPA-132-operation-response-must-use-standard-enum-values', [ }, errors: [], }, + { + name: 'valid enum re-declared by an allOf member carrying only a description', + document: { + components: { + schemas: { + // allOf is conjunctive: the description-only re-declaration must not clobber the enum + OperationResponse: { + ...validOperationResponse, + allOf: [{ properties: { status: { description: 'Lifecycle state.' } } }], + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid enum masked by a correct allOf member', + document: { + components: { + schemas: { + OperationResponse: { + type: 'object', + properties: { + status: { type: 'string', enum: ['PENDING', 'DONE'] }, + }, + allOf: [ + { + properties: { + status: { + enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], + }, + }, + }, + ], + }, + }, + }, + }, + errors: [ + // The effective enum is the intersection of both declarations, [PENDING] + { + code: 'xgen-IPA-132-operation-response-must-use-standard-enum-values', + message: STATUS_ENUM_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, { name: 'enum fields that are not defined are skipped', document: { diff --git a/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js index fe97b5b8a4..9b719f76be 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js @@ -83,19 +83,19 @@ testRule('xgen-IPA-132-operations-should-expose-status-fields', [ errors: [ { code: 'xgen-IPA-132-operations-should-expose-status-fields', - message: 'OperationResponse should define a statusMessage property.', + message: 'OperationResponse should define the statusMessage property.', path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-132-operations-should-expose-status-fields', - message: 'OperationResponse should define a progress property.', + message: 'OperationResponse should define the progress property.', path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-132-operations-should-expose-status-fields', - message: 'OperationResponse should define a estimatedCompletionTime property.', + message: 'OperationResponse should define the estimatedCompletionTime property.', path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, @@ -125,7 +125,7 @@ testRule('xgen-IPA-132-operations-should-expose-status-fields', [ errors: [ { code: 'xgen-IPA-132-operations-should-expose-status-fields', - message: 'The progress object should define a unit property.', + message: 'The progress object should define the unit property.', path: ['components', 'schemas', 'OperationResponse', 'properties', 'progress'], severity: DiagnosticSeverity.Warning, }, diff --git a/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js b/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js index ae50e41b99..9f8101d4c2 100644 --- a/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js +++ b/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js @@ -6,8 +6,8 @@ import { isOperationsPath, isSingleOperationPath, operationsSegmentIsLeaf, - OPERATION_STATUS_ENUM, - OPERATION_TYPE_ENUM, + getMergedProperties, + getPropertyPath, } from '../../rulesets/functions/utils/longRunningOperations'; describe('tools/spectral/ipa/utils/longRunningOperations.js', () => { @@ -94,22 +94,66 @@ describe('tools/spectral/ipa/utils/longRunningOperations.js', () => { describe('isExactEnumMatch', () => { it('accepts exact matches in any order', () => { - expect( - isExactEnumMatch( - ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], - OPERATION_STATUS_ENUM - ) - ).toBe(true); - expect(isExactEnumMatch(['CUSTOM', 'DELETE', 'UPDATE', 'CREATE'], OPERATION_TYPE_ENUM)).toBe(true); + expect(isExactEnumMatch(['SUCCEEDED', 'PENDING', 'FAILED'], ['PENDING', 'SUCCEEDED', 'FAILED'])).toBe(true); + expect(isExactEnumMatch(['CUSTOM', 'DELETE', 'UPDATE', 'CREATE'], ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'])).toBe( + true + ); }); it('rejects missing, extra, duplicate or undefined values', () => { - expect( - isExactEnumMatch(['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED'], OPERATION_STATUS_ENUM) - ).toBe(false); - expect(isExactEnumMatch([...OPERATION_STATUS_ENUM, 'CANCELLING'], OPERATION_STATUS_ENUM)).toBe(false); - expect(isExactEnumMatch(['CREATE', 'CREATE', 'UPDATE', 'DELETE'], OPERATION_TYPE_ENUM)).toBe(false); - expect(isExactEnumMatch(undefined, OPERATION_TYPE_ENUM)).toBe(false); + expect(isExactEnumMatch(['PENDING', 'SUCCEEDED'], ['PENDING', 'SUCCEEDED', 'FAILED'])).toBe(false); + expect(isExactEnumMatch(['PENDING', 'SUCCEEDED', 'FAILED', 'CANCELED'], ['PENDING', 'SUCCEEDED', 'FAILED'])).toBe( + false + ); + expect(isExactEnumMatch(['CREATE', 'CREATE', 'UPDATE'], ['CREATE', 'UPDATE', 'DELETE'])).toBe(false); + expect(isExactEnumMatch(undefined, ['CREATE', 'UPDATE'])).toBe(false); + }); + }); + + describe('getMergedProperties', () => { + it('combines keyword maps conjunctively instead of overwriting them', () => { + const schema = { + properties: { + status: { type: 'string', enum: ['PENDING', 'SUCCEEDED', 'FAILED'] }, + }, + allOf: [{ properties: { status: { description: 'Lifecycle state.' } } }], + }; + const merged = getMergedProperties(schema); + expect(merged.status.enum).toEqual(['PENDING', 'SUCCEEDED', 'FAILED']); + expect(merged.status.description).toBe('Lifecycle state.'); + }); + + it('intersects enums declared by several allOf members', () => { + const schema = { + properties: { + status: { enum: ['PENDING', 'DONE'] }, + }, + allOf: [{ properties: { status: { enum: ['PENDING', 'SUCCEEDED', 'FAILED'] } } }], + }; + expect(getMergedProperties(schema).status.enum).toEqual(['PENDING']); + }); + + it('merges nested property maps recursively', () => { + const schema = { + allOf: [ + { properties: { error: { properties: { code: { type: 'string' } } } } }, + { properties: { error: { properties: { message: { type: 'string' } } } } }, + ], + }; + const merged = getMergedProperties(schema); + expect(Object.keys(merged.error.properties)).toEqual(['code', 'message']); + }); + }); + + describe('getPropertyPath', () => { + it('locates properties defined directly and within allOf members', () => { + const schema = { + properties: { operationId: { type: 'string' } }, + allOf: [{ allOf: [{ properties: { createdAt: { type: 'string' } } }] }], + }; + expect(getPropertyPath(schema, 'operationId')).toEqual(['properties', 'operationId']); + expect(getPropertyPath(schema, 'createdAt')).toEqual(['allOf', 0, 'allOf', 0, 'properties', 'createdAt']); + expect(getPropertyPath(schema, 'unknown')).toBeUndefined(); }); }); }); diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index ded0af63ef..a15dcfceb8 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -91,20 +91,23 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies to 2xx JSON responses of GET methods on Operations endpoints + - Applies to 2xx responses of GET methods on Operations endpoints; responses defined as + references to `components.responses` are followed + - Every 2xx response must define at least one JSON media type carrying a schema - The single Operation endpoint (`.../operations/{operationId}`) must reference the `OperationResponse` schema - The Operations collection endpoint (`.../operations`) must reference a paginated wrapper - schema whose `results` items reference the `OperationResponse` schema; inline paginated - wrappers are rejected, consistent with `xgen-IPA-110-collections-use-paginated-prefix` + schema that defines the `results` array directly; inline and `allOf`-composed wrappers + are rejected, consistent with `xgen-IPA-110-collections-use-paginated-prefix` and + `xgen-IPA-110-collections-response-define-results-array` - Inline Operation schemas are violations, the response must reference the predefined `OperationResponse` schema - - Responses with `x-xgen-IPA-exception` for this rule at the content media type level are - excluded from validation + - Responses with `x-xgen-IPA-exception` for this rule at the response level are excluded + from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-endpoints-must-return-operation-response' severity: warn - given: $.paths[*].get.responses[*].content + given: $.paths[*].get.responses then: field: '@key' function: IPA132OperationEndpointsMustReturnOperationResponse @@ -120,7 +123,8 @@ rules: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - Properties and required lists spread across allOf sub-schemas are merged before - validation + validation; composition through oneOf/anyOf is not merged and is reported as missing + fields - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum and IPA-132-operation-response-must-include-core-metadata guidelines - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -187,9 +191,11 @@ rules: - Applies to the OperationResponse component schema - Each enum field listed in the rule's functionOptions must use exactly the listed values, in any order - - Fields that are not defined are skipped: their presence is validated by the - required-fields and optional-fields rules - - Properties spread across allOf sub-schemas are merged before validation + - Fields that are not defined are skipped: top-level field presence is validated by the + required-fields and optional-fields rules, and the nested `retryStrategy` field is + optional by the IPA-132-failed-lros-must-report-a-structured-error guideline + - Properties spread across allOf sub-schemas are merged before validation, with enums + declared by several sub-schemas applied intersected - Covers the enum checks of the IPA-132-operation-status-must-use-the-standard-status-enum, IPA-132-operation-response-must-include-core-metadata and IPA-132-failed-lros-must-report-a-structured-error guidelines @@ -247,10 +253,11 @@ rules: Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The schema should define `statusMessage`, `progress` and `estimatedCompletionTime` - properties + - The schema should define the fields listed in the rule's functionOptions: + `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance + - Properties spread across allOf sub-schemas are merged before validation - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operations-should-expose-status-fields' @@ -258,3 +265,8 @@ rules: given: '#OperationResponseSchema' then: function: IPA132OperationsShouldExposeStatusFields + functionOptions: + fields: [statusMessage, progress, estimatedCompletionTime] + objects: + - field: progress + properties: [completed, total, unit] diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 5f2b7c3001..575062fc1d 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1467,16 +1467,19 @@ operation status. ##### Implementation details Rule checks for the following conditions: - - Applies to 2xx JSON responses of GET methods on Operations endpoints + - Applies to 2xx responses of GET methods on Operations endpoints; responses defined as + references to `components.responses` are followed + - Every 2xx response must define at least one JSON media type carrying a schema - The single Operation endpoint (`.../operations/{operationId}`) must reference the `OperationResponse` schema - The Operations collection endpoint (`.../operations`) must reference a paginated wrapper - schema whose `results` items reference the `OperationResponse` schema; inline paginated - wrappers are rejected, consistent with `xgen-IPA-110-collections-use-paginated-prefix` + schema that defines the `results` array directly; inline and `allOf`-composed wrappers + are rejected, consistent with `xgen-IPA-110-collections-use-paginated-prefix` and + `xgen-IPA-110-collections-response-define-results-array` - Inline Operation schemas are violations, the response must reference the predefined `OperationResponse` schema - - Responses with `x-xgen-IPA-exception` for this rule at the content media type level are - excluded from validation + - Responses with `x-xgen-IPA-exception` for this rule at the response level are excluded + from validation #### xgen-IPA-132-operation-response-must-define-required-fields @@ -1490,7 +1493,8 @@ Rule checks for the following conditions: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - Properties and required lists spread across allOf sub-schemas are merged before - validation + validation; composition through oneOf/anyOf is not merged and is reported as missing + fields - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum and IPA-132-operation-response-must-include-core-metadata guidelines - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -1530,9 +1534,11 @@ Rule checks for the following conditions: - Applies to the OperationResponse component schema - Each enum field listed in the rule's functionOptions must use exactly the listed values, in any order - - Fields that are not defined are skipped: their presence is validated by the - required-fields and optional-fields rules - - Properties spread across allOf sub-schemas are merged before validation + - Fields that are not defined are skipped: top-level field presence is validated by the + required-fields and optional-fields rules, and the nested `retryStrategy` field is + optional by the IPA-132-failed-lros-must-report-a-structured-error guideline + - Properties spread across allOf sub-schemas are merged before validation, with enums + declared by several sub-schemas applied intersected - Covers the enum checks of the IPA-132-operation-status-must-use-the-standard-status-enum, IPA-132-operation-response-must-include-core-metadata and IPA-132-failed-lros-must-report-a-structured-error guidelines @@ -1567,10 +1573,11 @@ can reason about in-flight work. Rule checks for the following conditions: - Applies to the OperationResponse component schema - - The schema should define `statusMessage`, `progress` and `estimatedCompletionTime` - properties + - The schema should define the fields listed in the rule's functionOptions: + `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance + - Properties spread across allOf sub-schemas are merged before validation - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js index 6b0bc7a450..466fa701c3 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js @@ -9,89 +9,108 @@ import { const SINGLE_OPERATION_ERROR_MESSAGE = `The Operation endpoint must return the ${OPERATION_RESPONSE_SCHEMA_NAME} schema.`; const OPERATIONS_COLLECTION_ERROR_MESSAGE = `The Operations collection endpoint must return a paginated response whose results reference the ${OPERATION_RESPONSE_SCHEMA_NAME} schema.`; +const MISSING_SCHEMA_ERROR_MESSAGE = 'The Operations endpoint must define a JSON response schema.'; + +const COMPONENT_RESPONSES_REF_PREFIX = '#/components/responses/'; +const COMPONENT_SCHEMAS_REF_PREFIX = '#/components/schemas/'; /** * Checks that Operations endpoints defined by IPA-132 return the OperationResponse schema: the * single Operation endpoint must reference it directly, and the Operations collection endpoint * must reference a paginated wrapper schema whose results items reference it. * - * @param {string} input - The media type from the response content of the GET method + * @param {string} input - The response status code of the GET method * @param {object} _ - Unused * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, _, { path, documentInventory, rule }) => { const ruleName = rule.name; const resourcePath = path[1]; - const responseCode = path[4]; + const responseCode = input; - if ( - !responseCode.startsWith('2') || - !input.endsWith('json') || - (!isSingleOperationPath(resourcePath) && !isOperationsCollectionPath(resourcePath)) - ) { + if (!responseCode.startsWith('2')) { + return; + } + const isSingleOperation = isSingleOperationPath(resourcePath); + if (!isSingleOperation && !isOperationsCollectionPath(resourcePath)) { return; } + // Work on the unresolved document, where schema references are still visible by name const oas = documentInventory.unresolved; - const contentPerMediaType = resolveObject(oas, path); - if (!contentPerMediaType || !contentPerMediaType.schema) { + const responseOrRef = resolveObject(oas, path); + const response = resolveResponseRef(responseOrRef, oas); + if (!response) { return; } + // Errors on a shared component response anchor at the referencing response, since the shared + // component's content is not present at this document location + const contentBasePath = responseOrRef === response ? path : undefined; - const errors = checkViolationsAndReturnErrors(resourcePath, contentPerMediaType.schema, oas, path, ruleName); - return evaluateAndCollectAdoptionStatus(errors, ruleName, contentPerMediaType, path); + const errors = checkViolationsAndReturnErrors(isSingleOperation, response, contentBasePath, oas, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, response, path); }; -function checkViolationsAndReturnErrors(resourcePath, schema, oas, path, ruleName) { +function checkViolationsAndReturnErrors(isSingleOperation, response, contentBasePath, oas, path, ruleName) { try { - if (isSingleOperationPath(resourcePath)) { - if (schemaReferencesOperationResponse(schema)) { - return []; - } - return [{ path, message: SINGLE_OPERATION_ERROR_MESSAGE }]; + const jsonContentEntries = Object.entries(response.content ?? {}).filter(([mediaType]) => + mediaType.endsWith('json') + ); + // A 2xx response without a JSON media type cannot return the OperationResponse schema + if (jsonContentEntries.length === 0) { + return [{ path, message: MISSING_SCHEMA_ERROR_MESSAGE }]; } - // The Operations collection response must reference a paginated wrapper schema, since inline - // paginated schemas are rejected by xgen-IPA-110-collections-use-paginated-prefix - const wrapperSchema = getReferencedSchema(schema, oas); - const resultsItems = getResultsItems(wrapperSchema); - if (resultsItems && schemaReferencesOperationResponse(resultsItems)) { - return []; + const errors = []; + for (const [mediaType, content] of jsonContentEntries) { + const mediaTypePath = contentBasePath ? [...contentBasePath, 'content', mediaType] : path; + if (!content.schema) { + errors.push({ path: mediaTypePath, message: MISSING_SCHEMA_ERROR_MESSAGE }); + continue; + } + + if (isSingleOperation) { + if (!schemaReferencesOperationResponse(content.schema)) { + errors.push({ path: mediaTypePath, message: SINGLE_OPERATION_ERROR_MESSAGE }); + } + continue; + } + + // The Operations collection response must reference a paginated wrapper schema that defines + // the results array directly, the shape accepted by the IPA-110 pagination rules: inline + // and allOf-composed wrappers are rejected, consistent with + // xgen-IPA-110-collections-use-paginated-prefix and + // xgen-IPA-110-collections-response-define-results-array + const wrapperSchema = getReferencedSchema(content.schema, oas); + const resultsItems = wrapperSchema?.properties?.results?.items; + if (!(resultsItems && schemaReferencesOperationResponse(resultsItems))) { + errors.push({ path: mediaTypePath, message: OPERATIONS_COLLECTION_ERROR_MESSAGE }); + } } - return [{ path, message: OPERATIONS_COLLECTION_ERROR_MESSAGE }]; + return errors; } catch (e) { return handleInternalError(ruleName, path, e); } } +// The response itself may be defined as a reference to a shared component response +function resolveResponseRef(responseOrRef, oas) { + if (responseOrRef?.$ref === undefined) { + return responseOrRef; + } + if (!responseOrRef.$ref.startsWith(COMPONENT_RESPONSES_REF_PREFIX)) { + return undefined; + } + return oas.components?.responses?.[getSchemaNameFromRef(responseOrRef.$ref)]; +} + function schemaReferencesOperationResponse(schema) { return schema.$ref !== undefined && getSchemaNameFromRef(schema.$ref) === OPERATION_RESPONSE_SCHEMA_NAME; } function getReferencedSchema(schema, oas) { - if (!schema.$ref) { + if (schema.$ref === undefined || !schema.$ref.startsWith(COMPONENT_SCHEMAS_REF_PREFIX)) { return undefined; } return oas.components?.schemas?.[getSchemaNameFromRef(schema.$ref)]; } - -/** - * Finds the results items schema of a paginated wrapper. The results property may be defined - * directly on the wrapper, or within one of its allOf sub-schemas, which Spectral does not flatten. - */ -function getResultsItems(wrapperSchema) { - if (!wrapperSchema) { - return undefined; - } - const directItems = wrapperSchema.properties?.results?.items; - if (directItems) { - return directItems; - } - for (const subSchema of wrapperSchema.allOf ?? []) { - const items = subSchema.properties?.results?.items; - if (items) { - return items; - } - } - return undefined; -} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js index f5f73ae60e..1357af9fdf 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties } from './utils/longRunningOperations.js'; +import { getMergedProperties, getPropertyPath } from './utils/longRunningOperations.js'; /** * Checks that every nested object field of the OperationResponse schema defined by IPA-132 @@ -31,10 +31,12 @@ function checkViolationsAndReturnErrors(schema, objects, path, ruleName) { } const objectProperties = getMergedProperties(objectSchema); + // Anchor at the object property's actual location, which may sit inside an allOf sub-schema + const fieldPath = [...path, ...(getPropertyPath(schema, field) ?? [])]; for (const property of properties) { if (!objectProperties[property]) { errors.push({ - path: [...path, 'properties', field], + path: fieldPath, message: `The ${field} object must define a ${property} property.`, }); } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js index 83aef26fa3..3aa2c2b144 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, getMergedRequired } from './utils/longRunningOperations.js'; +import { getMergedProperties, getMergedRequired, getPropertyPath } from './utils/longRunningOperations.js'; /** * Checks that the OperationResponse schema defined by IPA-132 defines every field the standard @@ -28,8 +28,9 @@ function checkViolationsAndReturnErrors(schema, fields, path, ruleName) { if (!properties[name]) { errors.push({ path, message: `OperationResponse must define the ${name} property.` }); } else if (required.includes(name)) { + // Anchor at the property's actual location, which may sit inside an allOf sub-schema errors.push({ - path: [...path, 'properties', name], + path: [...path, ...getPropertyPath(schema, name)], message: `The ${name} property must not be listed as required. It is present only ${condition}, which cannot be validated statically.`, }); } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js index 8755f8935d..022436d179 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, getMergedRequired } from './utils/longRunningOperations.js'; +import { getMergedProperties, getMergedRequired, getPropertyPath } from './utils/longRunningOperations.js'; /** * Checks that the OperationResponse schema defined by IPA-132 defines every field the standard @@ -27,8 +27,9 @@ function checkViolationsAndReturnErrors(schema, fields, path, ruleName) { if (!properties[field]) { errors.push({ path, message: `OperationResponse must define the ${field} property.` }); } else if (!required.includes(field)) { + // Anchor at the property's actual location, which may sit inside an allOf sub-schema errors.push({ - path: [...path, 'properties', field], + path: [...path, ...getPropertyPath(schema, field)], message: `The ${field} property must be listed as required.`, }); } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js index de39f24f7c..5b9090da18 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, isExactEnumMatch } from './utils/longRunningOperations.js'; +import { getMergedProperties, getPropertyPath, isExactEnumMatch } from './utils/longRunningOperations.js'; /** * Checks that every enum field of the OperationResponse schema defined by IPA-132 uses exactly @@ -29,10 +29,11 @@ function checkViolationsAndReturnErrors(schema, enums, path, ruleName) { if (parent) { const parentProperty = properties[parent]; property = parentProperty ? getMergedProperties(parentProperty)[field] : undefined; - propertyPath = [...path, 'properties', parent]; + // Anchor at the parent property's actual location, which may sit inside an allOf sub-schema + propertyPath = [...path, ...(getPropertyPath(schema, parent) ?? [])]; } else { property = properties[field]; - propertyPath = [...path, 'properties', field]; + propertyPath = [...path, ...(getPropertyPath(schema, field) ?? [])]; } // Absent fields are the required-fields and optional-fields rules' concern diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js index 66301e244c..bff4c23bd3 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js @@ -1,42 +1,46 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties } from './utils/longRunningOperations.js'; - -const STATUS_FIELDS = ['statusMessage', 'progress', 'estimatedCompletionTime']; -const PROGRESS_FIELDS = ['completed', 'total', 'unit']; +import { getMergedProperties, getPropertyPath } from './utils/longRunningOperations.js'; /** * Checks that the OperationResponse schema defined by IPA-132 exposes the optional progress * fields statusMessage, progress and estimatedCompletionTime, so clients can reason about - * in-flight work. + * in-flight work. The field names and nested object structures are provided declaratively + * through the rule's functionOptions. * * @param {object} input - The OperationResponse schema object - * @param {object} _ - Unused + * @param {object} opts - The rule's functionOptions, carrying the field names and nested objects * @param {object} context - The context object containing the path, documentInventory and rule */ -export default (input, _, { path, rule }) => { +export default (input, opts, { path, rule }) => { const ruleName = rule.name; - const errors = checkViolationsAndReturnErrors(input, path, ruleName); + const errors = checkViolationsAndReturnErrors(input, opts, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); }; -function checkViolationsAndReturnErrors(schema, path, ruleName) { +function checkViolationsAndReturnErrors(schema, { fields, objects }, path, ruleName) { try { // Properties may be spread across allOf sub-schemas const properties = getMergedProperties(schema); const errors = []; - for (const field of STATUS_FIELDS) { + for (const field of fields) { if (!properties[field]) { - errors.push({ path, message: `OperationResponse should define a ${field} property.` }); + errors.push({ path, message: `OperationResponse should define the ${field} property.` }); } } - if (properties.progress) { - const progressProperties = getMergedProperties(properties.progress); - for (const field of PROGRESS_FIELDS) { - if (!progressProperties[field]) { + for (const { field, properties: objectFields } of objects) { + const objectSchema = properties[field]; + if (!objectSchema) { + continue; + } + const objectProperties = getMergedProperties(objectSchema); + // Anchor at the object property's actual location, which may sit inside an allOf sub-schema + const fieldPath = [...path, ...(getPropertyPath(schema, field) ?? [])]; + for (const objectField of objectFields) { + if (!objectProperties[objectField]) { errors.push({ - path: [...path, 'properties', 'progress'], - message: `The progress object should define a ${field} property.`, + path: fieldPath, + message: `The ${field} object should define the ${objectField} property.`, }); } } diff --git a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js index 775d26178c..621c17d42b 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js +++ b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js @@ -103,10 +103,6 @@ export function operationsSegmentIsLeaf(path) { export const OPERATION_RESPONSE_SCHEMA_NAME = 'OperationResponse'; -export const OPERATION_STATUS_ENUM = ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED']; - -export const OPERATION_TYPE_ENUM = ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM']; - /** * Checks if the enum values defined in a schema match an expected set of values exactly, in any * order. @@ -125,19 +121,70 @@ export function isExactEnumMatch(enumValues, expectedValues) { /** * Collects the properties of a schema, merging the properties of its allOf sub-schemas, which - * Spectral does not flatten during resolution. + * Spectral does not flatten during resolution. allOf is conjunctive, so a property re-declared + * by several sub-schemas combines the keywords of every declaration instead of overwriting them: + * enums apply intersected, and nested property maps are merged recursively. Composition through + * oneOf/anyOf is not merged, and circular references, which Spectral leaves as unresolved $ref + * stubs, contribute nothing. * * @param {object} schema the schema to collect properties for * @returns {object} the properties of the schema and all of its allOf sub-schemas */ export function getMergedProperties(schema) { - const properties = { ...(schema.properties ?? {}) }; + const properties = Object.create(null); + mergePropertyMaps(properties, schema.properties); for (const subSchema of schema.allOf ?? []) { - Object.assign(properties, getMergedProperties(subSchema)); + mergePropertyMaps(properties, getMergedProperties(subSchema)); } return properties; } +function mergePropertyMaps(target, source) { + for (const [name, definition] of Object.entries(source ?? {})) { + const existing = target[name]; + if (!existing) { + target[name] = definition; + continue; + } + const merged = { ...existing, ...definition }; + if (Array.isArray(existing.enum) && Array.isArray(definition.enum)) { + merged.enum = definition.enum.filter((value) => existing.enum.includes(value)); + } else if (existing.enum !== undefined || definition.enum !== undefined) { + merged.enum = definition.enum ?? existing.enum; + } + if (existing.properties !== undefined && definition.properties !== undefined) { + const mergedNested = Object.create(null); + mergePropertyMaps(mergedNested, existing.properties); + mergePropertyMaps(mergedNested, definition.properties); + merged.properties = mergedNested; + } + target[name] = merged; + } +} + +/** + * Finds the JSONPath segments locating a property within a schema, descending into allOf + * sub-schemas, so that violations anchor at the property's actual document location instead of a + * location that only exists after merging. + * + * @param {object} schema the schema defining the property + * @param {string} property the property name to locate + * @returns {(string|number)[]|undefined} the path segments relative to the schema, or undefined + */ +export function getPropertyPath(schema, property) { + if (schema.properties?.[property]) { + return ['properties', property]; + } + const subSchemas = schema.allOf ?? []; + for (let i = 0; i < subSchemas.length; i++) { + const subPath = getPropertyPath(subSchemas[i], property); + if (subPath) { + return ['allOf', i, ...subPath]; + } + } + return undefined; +} + /** * Collects the required property names of a schema, merging the required lists of its allOf * sub-schemas, which Spectral does not flatten during resolution. From ebe64aaef3872c6129d073b32d485f802b3c7d97 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Wed, 19 Aug 2026 14:52:02 +0100 Subject: [PATCH 08/12] feat(ipa): require the expiresAt field on OperationResponse Operation records must expire, so the record must always carry its expiration date. The retention behavior itself and the 404 after expiry remain review-level, only the field declaration is statically checkable. --- ...OperationResponseMustDefineRequiredFields.test.js | 12 ++++++++++-- tools/spectral/ipa/rulesets/IPA-132.yaml | 10 +++++++--- tools/spectral/ipa/rulesets/README.md | 9 ++++++--- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js index 3343657dd1..e1cc8e545d 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js @@ -3,13 +3,14 @@ import { DiagnosticSeverity } from '@stoplight/types'; const validOperationResponse = { type: 'object', - required: ['operationId', 'status', 'operationType', 'createdAt', 'updatedAt'], + required: ['operationId', 'status', 'operationType', 'createdAt', 'updatedAt', 'expiresAt'], properties: { operationId: { type: 'string' }, status: { type: 'string', enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'] }, operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, createdAt: { type: 'string', format: 'date-time' }, updatedAt: { type: 'string', format: 'date-time' }, + expiresAt: { type: 'string', format: 'date-time' }, }, }; @@ -97,6 +98,12 @@ testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-132-operation-response-must-define-required-fields', + message: 'OperationResponse must define the expiresAt property.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, { code: 'xgen-IPA-132-operation-response-must-define-required-fields', message: 'The createdAt property must be listed as required.', @@ -114,7 +121,7 @@ testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ allOf: [ { type: 'object', - required: ['operationId', 'status', 'operationType', 'updatedAt'], + required: ['operationId', 'status', 'operationType', 'updatedAt', 'expiresAt'], properties: { operationId: { type: 'string' }, status: { @@ -124,6 +131,7 @@ testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, createdAt: { type: 'string', format: 'date-time' }, updatedAt: { type: 'string', format: 'date-time' }, + expiresAt: { type: 'string', format: 'date-time' }, }, }, ], diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index a15dcfceb8..d2dd99431f 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -115,7 +115,7 @@ rules: xgen-IPA-132-operation-response-must-define-required-fields: description: | The OperationResponse schema must define every field the IPA-132 standard marks as always - required: operationId, status, operationType, createdAt and updatedAt. + required: operationId, status, operationType, createdAt, updatedAt and expiresAt. ##### Implementation details Rule checks for the following conditions: @@ -125,8 +125,11 @@ rules: - Properties and required lists spread across allOf sub-schemas are merged before validation; composition through oneOf/anyOf is not merged and is reported as missing fields - - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum - and IPA-132-operation-response-must-include-core-metadata guidelines + - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum, + IPA-132-operation-response-must-include-core-metadata and + IPA-132-operations-must-expire-after-a-finite-retention-period guidelines; for the + latter, only the expiresAt declaration is statically checkable, the retention behavior + itself is not - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-required-fields' @@ -141,6 +144,7 @@ rules: - operationType - createdAt - updatedAt + - expiresAt xgen-IPA-132-operation-response-must-define-optional-fields: description: | diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 575062fc1d..f113bfdde3 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1485,7 +1485,7 @@ Rule checks for the following conditions: ![warn](https://img.shields.io/badge/warning-yellow) The OperationResponse schema must define every field the IPA-132 standard marks as always -required: operationId, status, operationType, createdAt and updatedAt. +required: operationId, status, operationType, createdAt, updatedAt and expiresAt. ##### Implementation details Rule checks for the following conditions: @@ -1495,8 +1495,11 @@ Rule checks for the following conditions: - Properties and required lists spread across allOf sub-schemas are merged before validation; composition through oneOf/anyOf is not merged and is reported as missing fields - - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum - and IPA-132-operation-response-must-include-core-metadata guidelines + - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum, + IPA-132-operation-response-must-include-core-metadata and + IPA-132-operations-must-expire-after-a-finite-retention-period guidelines; for the + latter, only the expiresAt declaration is statically checkable, the retention behavior + itself is not - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-define-optional-fields From d49810608132f832733382cf92bd7907ebf07086 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Wed, 19 Aug 2026 15:06:35 +0100 Subject: [PATCH 09/12] docs(ipa): apply review feedback to the IPA-132 rule descriptions Drop the internal guideline-ID mappings from the rendered descriptions, clarify how allOf/oneOf/anyOf composition relates to the OperationResponse schema, and trim the enum rule's field-presence justification. --- tools/spectral/ipa/rulesets/IPA-132.yaml | 42 ++++++++++-------------- tools/spectral/ipa/rulesets/README.md | 42 ++++++++++-------------- 2 files changed, 34 insertions(+), 50 deletions(-) diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index d2dd99431f..bd3fe4c05b 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -122,14 +122,14 @@ rules: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - - Properties and required lists spread across allOf sub-schemas are merged before - validation; composition through oneOf/anyOf is not merged and is reported as missing - fields - - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum, - IPA-132-operation-response-must-include-core-metadata and - IPA-132-operations-must-expire-after-a-finite-retention-period guidelines; for the - latter, only the expiresAt declaration is statically checkable, the retention behavior - itself is not + - The schema may be composed with allOf, e.g. when the OpenAPI generator emits Java class + inheritance as an allOf composition; the properties and required lists of all allOf + sub-schemas are combined before validation, since Spectral resolves references but does + not flatten allOf + - oneOf and anyOf describe alternative schemas rather than one combined schema, so they + are not merged: fields declared only inside oneOf or anyOf are reported as missing + - For expiresAt, only the field declaration is validated; the retention behavior and the + 404 after expiry are backend-defined - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-required-fields' @@ -159,12 +159,8 @@ rules: - Each field listed in the rule's functionOptions must be defined and must not be listed as required: the condition under which it is present is a runtime state that cannot be validated statically - - Properties and required lists spread across allOf sub-schemas are merged before - validation - - Covers the conditional-field checks of the IPA-132-operation-response-must-include-core-metadata, - IPA-132-failed-lros-must-report-a-structured-error, - IPA-132-successful-lros-must-reference-the-result-resource and - IPA-132-in-progress-operations-must-expose-retry-after-seconds guidelines + - The properties and required lists of all allOf sub-schemas are combined before + validation, since Spectral does not flatten allOf - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-optional-fields' @@ -196,13 +192,9 @@ rules: - Each enum field listed in the rule's functionOptions must use exactly the listed values, in any order - Fields that are not defined are skipped: top-level field presence is validated by the - required-fields and optional-fields rules, and the nested `retryStrategy` field is - optional by the IPA-132-failed-lros-must-report-a-structured-error guideline - - Properties spread across allOf sub-schemas are merged before validation, with enums - declared by several sub-schemas applied intersected - - Covers the enum checks of the IPA-132-operation-status-must-use-the-standard-status-enum, - IPA-132-operation-response-must-include-core-metadata and - IPA-132-failed-lros-must-report-a-structured-error guidelines + required-fields and optional-fields rules + - The properties of all allOf sub-schemas are combined before validation, since Spectral + does not flatten allOf; enums declared by several sub-schemas apply intersected - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-use-standard-enum-values' @@ -233,9 +225,8 @@ rules: properties - Fields that are not defined are skipped: their presence is validated by the optional-fields rule - - Properties spread across allOf sub-schemas are merged before validation - - Covers the error object structure check of the - IPA-132-failed-lros-must-report-a-structured-error guideline + - The properties of all allOf sub-schemas are combined before validation, since Spectral + does not flatten allOf - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-nested-object-structure' @@ -261,7 +252,8 @@ rules: `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance - - Properties spread across allOf sub-schemas are merged before validation + - The properties of all allOf sub-schemas are combined before validation, since Spectral + does not flatten allOf - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operations-should-expose-status-fields' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index f113bfdde3..2bd60079da 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1492,14 +1492,14 @@ Rule checks for the following conditions: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - - Properties and required lists spread across allOf sub-schemas are merged before - validation; composition through oneOf/anyOf is not merged and is reported as missing - fields - - Covers the required-field checks of the IPA-132-operation-status-must-use-the-standard-status-enum, - IPA-132-operation-response-must-include-core-metadata and - IPA-132-operations-must-expire-after-a-finite-retention-period guidelines; for the - latter, only the expiresAt declaration is statically checkable, the retention behavior - itself is not + - The schema may be composed with allOf, e.g. when the OpenAPI generator emits Java class + inheritance as an allOf composition; the properties and required lists of all allOf + sub-schemas are combined before validation, since Spectral resolves references but does + not flatten allOf + - oneOf and anyOf describe alternative schemas rather than one combined schema, so they + are not merged: fields declared only inside oneOf or anyOf are reported as missing + - For expiresAt, only the field declaration is validated; the retention behavior and the + 404 after expiry are backend-defined - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-define-optional-fields @@ -1516,12 +1516,8 @@ Rule checks for the following conditions: - Each field listed in the rule's functionOptions must be defined and must not be listed as required: the condition under which it is present is a runtime state that cannot be validated statically - - Properties and required lists spread across allOf sub-schemas are merged before - validation - - Covers the conditional-field checks of the IPA-132-operation-response-must-include-core-metadata, - IPA-132-failed-lros-must-report-a-structured-error, - IPA-132-successful-lros-must-reference-the-result-resource and - IPA-132-in-progress-operations-must-expose-retry-after-seconds guidelines + - The properties and required lists of all allOf sub-schemas are combined before + validation, since Spectral does not flatten allOf - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-use-standard-enum-values @@ -1538,13 +1534,9 @@ Rule checks for the following conditions: - Each enum field listed in the rule's functionOptions must use exactly the listed values, in any order - Fields that are not defined are skipped: top-level field presence is validated by the - required-fields and optional-fields rules, and the nested `retryStrategy` field is - optional by the IPA-132-failed-lros-must-report-a-structured-error guideline - - Properties spread across allOf sub-schemas are merged before validation, with enums - declared by several sub-schemas applied intersected - - Covers the enum checks of the IPA-132-operation-status-must-use-the-standard-status-enum, - IPA-132-operation-response-must-include-core-metadata and - IPA-132-failed-lros-must-report-a-structured-error guidelines + required-fields and optional-fields rules + - The properties of all allOf sub-schemas are combined before validation, since Spectral + does not flatten allOf; enums declared by several sub-schemas apply intersected - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-define-nested-object-structure @@ -1561,9 +1553,8 @@ Rule checks for the following conditions: properties - Fields that are not defined are skipped: their presence is validated by the optional-fields rule - - Properties spread across allOf sub-schemas are merged before validation - - Covers the error object structure check of the - IPA-132-failed-lros-must-report-a-structured-error guideline + - The properties of all allOf sub-schemas are combined before validation, since Spectral + does not flatten allOf - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operations-should-expose-status-fields @@ -1580,7 +1571,8 @@ Rule checks for the following conditions: `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance - - Properties spread across allOf sub-schemas are merged before validation + - The properties of all allOf sub-schemas are combined before validation, since Spectral + does not flatten allOf - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation From 7cda85d6f4124ea91738f0dd2a73dce902013078 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Wed, 19 Aug 2026 17:26:19 +0100 Subject: [PATCH 10/12] fix(ipa): require a flat OperationResponse schema without composition There is one and only one OperationResponse, provided as scaffolding, so the schema must define its properties directly: allOf, oneOf and anyOf composition is a violation reported by the required-fields rule, and the remaining schema rules skip composed schemas. Removes the allOf merge machinery this design decision makes unnecessary. --- ...nseMustDefineNestedObjectStructure.test.js | 14 ++++ ...onResponseMustDefineOptionalFields.test.js | 19 ++--- ...onResponseMustDefineRequiredFields.test.js | 71 +++++++--------- ...nResponseMustUseStandardEnumValues.test.js | 65 ++------------- ...OperationsShouldExposeStatusFields.test.js | 19 ++--- .../utils/longRunningOperations.test.js | 53 ++---------- tools/spectral/ipa/rulesets/IPA-132.yaml | 20 ++--- tools/spectral/ipa/rulesets/README.md | 20 ++--- ...ResponseMustDefineNestedObjectStructure.js | 15 ++-- ...erationResponseMustDefineOptionalFields.js | 14 ++-- ...erationResponseMustDefineRequiredFields.js | 24 ++++-- ...rationResponseMustUseStandardEnumValues.js | 16 ++-- ...PA132OperationsShouldExposeStatusFields.js | 15 ++-- .../functions/utils/longRunningOperations.js | 83 ++----------------- 14 files changed, 139 insertions(+), 309 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js index ba7d261074..140f0b1907 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js @@ -70,6 +70,20 @@ testRule('xgen-IPA-132-operation-response-must-define-nested-object-structure', }, errors: [], }, + { + name: 'composed schemas are skipped', + document: { + components: { + schemas: { + // Composition is rejected by the required-fields rule, so this rule stays silent + OperationResponse: { + allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], + }, + }, + }, + }, + errors: [], + }, { name: 'invalid error object missing structured properties', document: { diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js index 47ff7ceae5..740ddeb0bb 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js @@ -36,17 +36,14 @@ testRule('xgen-IPA-132-operation-response-must-define-optional-fields', [ errors: [], }, { - name: 'valid OperationResponse composed with allOf', + name: 'documents without an OperationResponse schema are ignored', document: { components: { schemas: { - OperationResponse: { - allOf: [{ $ref: '#/components/schemas/OperationOutcome' }, validOperationResponse], - }, - OperationOutcome: { + Order: { type: 'object', properties: { - statusMessage: { type: 'string' }, + id: { type: 'string' }, }, }, }, @@ -55,15 +52,13 @@ testRule('xgen-IPA-132-operation-response-must-define-optional-fields', [ errors: [], }, { - name: 'documents without an OperationResponse schema are ignored', + name: 'composed schemas are skipped', document: { components: { schemas: { - Order: { - type: 'object', - properties: { - id: { type: 'string' }, - }, + // Composition is rejected by the required-fields rule, so this rule stays silent + OperationResponse: { + allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, }, }, diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js index e1cc8e545d..7e08bcdeda 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js @@ -26,25 +26,6 @@ testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ }, errors: [], }, - { - name: 'valid OperationResponse composed with allOf', - document: { - components: { - schemas: { - OperationResponse: { - allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, validOperationResponse], - }, - OperationMetadata: { - type: 'object', - properties: { - statusMessage: { type: 'string' }, - }, - }, - }, - }, - }, - errors: [], - }, { name: 'documents without an OperationResponse schema are ignored', document: { @@ -113,38 +94,48 @@ testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ ], }, { - name: 'invalid required field defined only within an allOf member', + name: 'invalid OperationResponse composed with allOf', + document: { + components: { + schemas: { + // There is one and only one OperationResponse: composition is not allowed + OperationResponse: { + allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, validOperationResponse], + }, + OperationMetadata: { + type: 'object', + properties: { + statusMessage: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-define-required-fields', + message: 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.', + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse composed with oneOf', document: { components: { schemas: { OperationResponse: { - allOf: [ - { - type: 'object', - required: ['operationId', 'status', 'operationType', 'updatedAt', 'expiresAt'], - properties: { - operationId: { type: 'string' }, - status: { - type: 'string', - enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], - }, - operationType: { type: 'string', enum: ['CREATE', 'UPDATE', 'DELETE', 'CUSTOM'] }, - createdAt: { type: 'string', format: 'date-time' }, - updatedAt: { type: 'string', format: 'date-time' }, - expiresAt: { type: 'string', format: 'date-time' }, - }, - }, - ], + oneOf: [validOperationResponse], }, }, }, }, errors: [ - // The violation anchors at the property's actual location inside the allOf member { code: 'xgen-IPA-132-operation-response-must-define-required-fields', - message: 'The createdAt property must be listed as required.', - path: ['components', 'schemas', 'OperationResponse', 'allOf', '0', 'properties', 'createdAt'], + message: 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.', + path: ['components', 'schemas', 'OperationResponse'], severity: DiagnosticSeverity.Warning, }, ], diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js index 001e97916c..fb1f88821e 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js @@ -38,17 +38,15 @@ testRule('xgen-IPA-132-operation-response-must-use-standard-enum-values', [ errors: [], }, { - name: 'valid OperationResponse composed with allOf', + name: 'enum fields that are not defined are skipped', document: { components: { schemas: { + // Missing fields are the required-fields and optional-fields rules' concern OperationResponse: { - allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, validOperationResponse], - }, - OperationMetadata: { type: 'object', properties: { - statusMessage: { type: 'string' }, + operationId: { type: 'string' }, }, }, }, @@ -57,64 +55,13 @@ testRule('xgen-IPA-132-operation-response-must-use-standard-enum-values', [ errors: [], }, { - name: 'valid enum re-declared by an allOf member carrying only a description', - document: { - components: { - schemas: { - // allOf is conjunctive: the description-only re-declaration must not clobber the enum - OperationResponse: { - ...validOperationResponse, - allOf: [{ properties: { status: { description: 'Lifecycle state.' } } }], - }, - }, - }, - }, - errors: [], - }, - { - name: 'invalid enum masked by a correct allOf member', - document: { - components: { - schemas: { - OperationResponse: { - type: 'object', - properties: { - status: { type: 'string', enum: ['PENDING', 'DONE'] }, - }, - allOf: [ - { - properties: { - status: { - enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'], - }, - }, - }, - ], - }, - }, - }, - }, - errors: [ - // The effective enum is the intersection of both declarations, [PENDING] - { - code: 'xgen-IPA-132-operation-response-must-use-standard-enum-values', - message: STATUS_ENUM_ERROR_MESSAGE, - path: ['components', 'schemas', 'OperationResponse', 'properties', 'status'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'enum fields that are not defined are skipped', + name: 'composed schemas are skipped', document: { components: { schemas: { - // Missing fields are the required-fields and optional-fields rules' concern + // Composition is rejected by the required-fields rule, so this rule stays silent OperationResponse: { - type: 'object', - properties: { - operationId: { type: 'string' }, - }, + allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, }, }, diff --git a/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js index 9b719f76be..3df4c61fec 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js @@ -32,17 +32,14 @@ testRule('xgen-IPA-132-operations-should-expose-status-fields', [ errors: [], }, { - name: 'valid OperationResponse composed with allOf', + name: 'documents without an OperationResponse schema are ignored', document: { components: { schemas: { - OperationResponse: { - allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, operationResponseWithStatusFields], - }, - OperationMetadata: { + Order: { type: 'object', properties: { - operationId: { type: 'string' }, + id: { type: 'string' }, }, }, }, @@ -51,15 +48,13 @@ testRule('xgen-IPA-132-operations-should-expose-status-fields', [ errors: [], }, { - name: 'documents without an OperationResponse schema are ignored', + name: 'composed schemas are skipped', document: { components: { schemas: { - Order: { - type: 'object', - properties: { - id: { type: 'string' }, - }, + // Composition is rejected by the required-fields rule, so this rule stays silent + OperationResponse: { + allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, }, }, diff --git a/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js b/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js index 9f8101d4c2..d64ab03e1a 100644 --- a/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js +++ b/tools/spectral/ipa/__tests__/utils/longRunningOperations.test.js @@ -6,8 +6,7 @@ import { isOperationsPath, isSingleOperationPath, operationsSegmentIsLeaf, - getMergedProperties, - getPropertyPath, + usesComposition, } from '../../rulesets/functions/utils/longRunningOperations'; describe('tools/spectral/ipa/utils/longRunningOperations.js', () => { @@ -110,50 +109,12 @@ describe('tools/spectral/ipa/utils/longRunningOperations.js', () => { }); }); - describe('getMergedProperties', () => { - it('combines keyword maps conjunctively instead of overwriting them', () => { - const schema = { - properties: { - status: { type: 'string', enum: ['PENDING', 'SUCCEEDED', 'FAILED'] }, - }, - allOf: [{ properties: { status: { description: 'Lifecycle state.' } } }], - }; - const merged = getMergedProperties(schema); - expect(merged.status.enum).toEqual(['PENDING', 'SUCCEEDED', 'FAILED']); - expect(merged.status.description).toBe('Lifecycle state.'); - }); - - it('intersects enums declared by several allOf members', () => { - const schema = { - properties: { - status: { enum: ['PENDING', 'DONE'] }, - }, - allOf: [{ properties: { status: { enum: ['PENDING', 'SUCCEEDED', 'FAILED'] } } }], - }; - expect(getMergedProperties(schema).status.enum).toEqual(['PENDING']); - }); - - it('merges nested property maps recursively', () => { - const schema = { - allOf: [ - { properties: { error: { properties: { code: { type: 'string' } } } } }, - { properties: { error: { properties: { message: { type: 'string' } } } } }, - ], - }; - const merged = getMergedProperties(schema); - expect(Object.keys(merged.error.properties)).toEqual(['code', 'message']); - }); - }); - - describe('getPropertyPath', () => { - it('locates properties defined directly and within allOf members', () => { - const schema = { - properties: { operationId: { type: 'string' } }, - allOf: [{ allOf: [{ properties: { createdAt: { type: 'string' } } }] }], - }; - expect(getPropertyPath(schema, 'operationId')).toEqual(['properties', 'operationId']); - expect(getPropertyPath(schema, 'createdAt')).toEqual(['allOf', 0, 'allOf', 0, 'properties', 'createdAt']); - expect(getPropertyPath(schema, 'unknown')).toBeUndefined(); + describe('usesComposition', () => { + it('detects allOf, oneOf and anyOf composition', () => { + expect(usesComposition({ allOf: [{ type: 'object' }] })).toBe(true); + expect(usesComposition({ oneOf: [{ type: 'object' }] })).toBe(true); + expect(usesComposition({ anyOf: [{ type: 'object' }] })).toBe(true); + expect(usesComposition({ type: 'object', properties: { id: { type: 'string' } } })).toBe(false); }); }); }); diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index bd3fe4c05b..42e1477c7a 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -122,12 +122,8 @@ rules: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - - The schema may be composed with allOf, e.g. when the OpenAPI generator emits Java class - inheritance as an allOf composition; the properties and required lists of all allOf - sub-schemas are combined before validation, since Spectral resolves references but does - not flatten allOf - - oneOf and anyOf describe alternative schemas rather than one combined schema, so they - are not merged: fields declared only inside oneOf or anyOf are reported as missing + - There is one and only one OperationResponse: the schema must define its properties + directly, so allOf, oneOf and anyOf composition is a violation - For expiresAt, only the field declaration is validated; the retention behavior and the 404 after expiry are backend-defined - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -159,8 +155,7 @@ rules: - Each field listed in the rule's functionOptions must be defined and must not be listed as required: the condition under which it is present is a runtime state that cannot be validated statically - - The properties and required lists of all allOf sub-schemas are combined before - validation, since Spectral does not flatten allOf + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-optional-fields' @@ -193,8 +188,7 @@ rules: in any order - Fields that are not defined are skipped: top-level field presence is validated by the required-fields and optional-fields rules - - The properties of all allOf sub-schemas are combined before validation, since Spectral - does not flatten allOf; enums declared by several sub-schemas apply intersected + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-use-standard-enum-values' @@ -225,8 +219,7 @@ rules: properties - Fields that are not defined are skipped: their presence is validated by the optional-fields rule - - The properties of all allOf sub-schemas are combined before validation, since Spectral - does not flatten allOf + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-nested-object-structure' @@ -252,8 +245,7 @@ rules: `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance - - The properties of all allOf sub-schemas are combined before validation, since Spectral - does not flatten allOf + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operations-should-expose-status-fields' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 2bd60079da..2c5d56a41c 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1492,12 +1492,8 @@ Rule checks for the following conditions: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - - The schema may be composed with allOf, e.g. when the OpenAPI generator emits Java class - inheritance as an allOf composition; the properties and required lists of all allOf - sub-schemas are combined before validation, since Spectral resolves references but does - not flatten allOf - - oneOf and anyOf describe alternative schemas rather than one combined schema, so they - are not merged: fields declared only inside oneOf or anyOf are reported as missing + - There is one and only one OperationResponse: the schema must define its properties + directly, so allOf, oneOf and anyOf composition is a violation - For expiresAt, only the field declaration is validated; the retention behavior and the 404 after expiry are backend-defined - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -1516,8 +1512,7 @@ Rule checks for the following conditions: - Each field listed in the rule's functionOptions must be defined and must not be listed as required: the condition under which it is present is a runtime state that cannot be validated statically - - The properties and required lists of all allOf sub-schemas are combined before - validation, since Spectral does not flatten allOf + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-use-standard-enum-values @@ -1535,8 +1530,7 @@ Rule checks for the following conditions: in any order - Fields that are not defined are skipped: top-level field presence is validated by the required-fields and optional-fields rules - - The properties of all allOf sub-schemas are combined before validation, since Spectral - does not flatten allOf; enums declared by several sub-schemas apply intersected + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-define-nested-object-structure @@ -1553,8 +1547,7 @@ Rule checks for the following conditions: properties - Fields that are not defined are skipped: their presence is validated by the optional-fields rule - - The properties of all allOf sub-schemas are combined before validation, since Spectral - does not flatten allOf + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operations-should-expose-status-fields @@ -1571,8 +1564,7 @@ Rule checks for the following conditions: `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance - - The properties of all allOf sub-schemas are combined before validation, since Spectral - does not flatten allOf + - Composed schemas are skipped: composition is rejected by the required-fields rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js index 1357af9fdf..2483a43e67 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, getPropertyPath } from './utils/longRunningOperations.js'; +import { usesComposition } from './utils/longRunningOperations.js'; /** * Checks that every nested object field of the OperationResponse schema defined by IPA-132 @@ -12,6 +12,10 @@ import { getMergedProperties, getPropertyPath } from './utils/longRunningOperati * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { + // Composed schemas are rejected by the required-fields rule + if (usesComposition(input)) { + return; + } const ruleName = rule.name; const errors = checkViolationsAndReturnErrors(input, opts.objects, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); @@ -19,8 +23,7 @@ export default (input, opts, { path, rule }) => { function checkViolationsAndReturnErrors(schema, objects, path, ruleName) { try { - // Properties may be spread across allOf sub-schemas - const schemaProperties = getMergedProperties(schema); + const schemaProperties = schema.properties ?? {}; const errors = []; for (const { field, properties } of objects) { @@ -30,13 +33,11 @@ function checkViolationsAndReturnErrors(schema, objects, path, ruleName) { continue; } - const objectProperties = getMergedProperties(objectSchema); - // Anchor at the object property's actual location, which may sit inside an allOf sub-schema - const fieldPath = [...path, ...(getPropertyPath(schema, field) ?? [])]; + const objectProperties = objectSchema.properties ?? {}; for (const property of properties) { if (!objectProperties[property]) { errors.push({ - path: fieldPath, + path: [...path, 'properties', field], message: `The ${field} object must define a ${property} property.`, }); } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js index 3aa2c2b144..541d9b4050 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, getMergedRequired, getPropertyPath } from './utils/longRunningOperations.js'; +import { usesComposition } from './utils/longRunningOperations.js'; /** * Checks that the OperationResponse schema defined by IPA-132 defines every field the standard @@ -12,6 +12,10 @@ import { getMergedProperties, getMergedRequired, getPropertyPath } from './utils * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { + // Composed schemas are rejected by the required-fields rule + if (usesComposition(input)) { + return; + } const ruleName = rule.name; const errors = checkViolationsAndReturnErrors(input, opts.fields, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); @@ -19,18 +23,16 @@ export default (input, opts, { path, rule }) => { function checkViolationsAndReturnErrors(schema, fields, path, ruleName) { try { - // Properties and required lists may be spread across allOf sub-schemas - const properties = getMergedProperties(schema); - const required = getMergedRequired(schema); + const properties = schema.properties ?? {}; + const required = schema.required ?? []; const errors = []; for (const { name, condition } of fields) { if (!properties[name]) { errors.push({ path, message: `OperationResponse must define the ${name} property.` }); } else if (required.includes(name)) { - // Anchor at the property's actual location, which may sit inside an allOf sub-schema errors.push({ - path: [...path, ...getPropertyPath(schema, name)], + path: [...path, 'properties', name], message: `The ${name} property must not be listed as required. It is present only ${condition}, which cannot be validated statically.`, }); } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js index 022436d179..51fb9c96c7 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js @@ -1,10 +1,13 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, getMergedRequired, getPropertyPath } from './utils/longRunningOperations.js'; +import { usesComposition } from './utils/longRunningOperations.js'; + +const COMPOSITION_ERROR_MESSAGE = + 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.'; /** - * Checks that the OperationResponse schema defined by IPA-132 defines every field the standard - * marks as always required, and lists each of them in the schema's required array. The field - * names are provided declaratively through the rule's functionOptions. + * Checks that the OperationResponse schema defined by IPA-132 is a single flat schema defining + * every field the standard marks as always required, each listed in the schema's required array. + * The field names are provided declaratively through the rule's functionOptions. * * @param {object} input - The OperationResponse schema object * @param {object} opts - The rule's functionOptions, carrying the required field names @@ -18,18 +21,21 @@ export default (input, opts, { path, rule }) => { function checkViolationsAndReturnErrors(schema, fields, path, ruleName) { try { - // Properties and required lists may be spread across allOf sub-schemas - const properties = getMergedProperties(schema); - const required = getMergedRequired(schema); + // There is one and only one OperationResponse: a flat schema, so composition is a violation + if (usesComposition(schema)) { + return [{ path, message: COMPOSITION_ERROR_MESSAGE }]; + } + + const properties = schema.properties ?? {}; + const required = schema.required ?? []; const errors = []; for (const field of fields) { if (!properties[field]) { errors.push({ path, message: `OperationResponse must define the ${field} property.` }); } else if (!required.includes(field)) { - // Anchor at the property's actual location, which may sit inside an allOf sub-schema errors.push({ - path: [...path, ...getPropertyPath(schema, field)], + path: [...path, 'properties', field], message: `The ${field} property must be listed as required.`, }); } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js index 5b9090da18..a8d16c61e9 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, getPropertyPath, isExactEnumMatch } from './utils/longRunningOperations.js'; +import { isExactEnumMatch, usesComposition } from './utils/longRunningOperations.js'; /** * Checks that every enum field of the OperationResponse schema defined by IPA-132 uses exactly @@ -12,6 +12,10 @@ import { getMergedProperties, getPropertyPath, isExactEnumMatch } from './utils/ * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { + // Composed schemas are rejected by the required-fields rule + if (usesComposition(input)) { + return; + } const ruleName = rule.name; const errors = checkViolationsAndReturnErrors(input, opts.enums, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); @@ -19,8 +23,7 @@ export default (input, opts, { path, rule }) => { function checkViolationsAndReturnErrors(schema, enums, path, ruleName) { try { - // Properties may be spread across allOf sub-schemas - const properties = getMergedProperties(schema); + const properties = schema.properties ?? {}; const errors = []; for (const { field, parent, values } of enums) { @@ -28,12 +31,11 @@ function checkViolationsAndReturnErrors(schema, enums, path, ruleName) { let propertyPath; if (parent) { const parentProperty = properties[parent]; - property = parentProperty ? getMergedProperties(parentProperty)[field] : undefined; - // Anchor at the parent property's actual location, which may sit inside an allOf sub-schema - propertyPath = [...path, ...(getPropertyPath(schema, parent) ?? [])]; + property = parentProperty?.properties?.[field]; + propertyPath = [...path, 'properties', parent]; } else { property = properties[field]; - propertyPath = [...path, ...(getPropertyPath(schema, field) ?? [])]; + propertyPath = [...path, 'properties', field]; } // Absent fields are the required-fields and optional-fields rules' concern diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js index bff4c23bd3..b7e733a802 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js @@ -1,5 +1,5 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; -import { getMergedProperties, getPropertyPath } from './utils/longRunningOperations.js'; +import { usesComposition } from './utils/longRunningOperations.js'; /** * Checks that the OperationResponse schema defined by IPA-132 exposes the optional progress @@ -12,6 +12,10 @@ import { getMergedProperties, getPropertyPath } from './utils/longRunningOperati * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { + // Composed schemas are rejected by the required-fields rule + if (usesComposition(input)) { + return; + } const ruleName = rule.name; const errors = checkViolationsAndReturnErrors(input, opts, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); @@ -19,8 +23,7 @@ export default (input, opts, { path, rule }) => { function checkViolationsAndReturnErrors(schema, { fields, objects }, path, ruleName) { try { - // Properties may be spread across allOf sub-schemas - const properties = getMergedProperties(schema); + const properties = schema.properties ?? {}; const errors = []; for (const field of fields) { if (!properties[field]) { @@ -33,13 +36,11 @@ function checkViolationsAndReturnErrors(schema, { fields, objects }, path, ruleN if (!objectSchema) { continue; } - const objectProperties = getMergedProperties(objectSchema); - // Anchor at the object property's actual location, which may sit inside an allOf sub-schema - const fieldPath = [...path, ...(getPropertyPath(schema, field) ?? [])]; + const objectProperties = objectSchema.properties ?? {}; for (const objectField of objectFields) { if (!objectProperties[objectField]) { errors.push({ - path: fieldPath, + path: [...path, 'properties', field], message: `The ${field} object should define the ${objectField} property.`, }); } diff --git a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js index 621c17d42b..9dfd2e4e3e 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js +++ b/tools/spectral/ipa/rulesets/functions/utils/longRunningOperations.js @@ -120,82 +120,13 @@ export function isExactEnumMatch(enumValues, expectedValues) { } /** - * Collects the properties of a schema, merging the properties of its allOf sub-schemas, which - * Spectral does not flatten during resolution. allOf is conjunctive, so a property re-declared - * by several sub-schemas combines the keywords of every declaration instead of overwriting them: - * enums apply intersected, and nested property maps are merged recursively. Composition through - * oneOf/anyOf is not merged, and circular references, which Spectral leaves as unresolved $ref - * stubs, contribute nothing. + * Checks if a schema is composed through allOf, oneOf or anyOf. The OperationResponse schema + * defined by IPA-132 is a single flat schema, so composition is not allowed and the schema rules + * reject or skip composed schemas. * - * @param {object} schema the schema to collect properties for - * @returns {object} the properties of the schema and all of its allOf sub-schemas + * @param {object} schema the schema to evaluate + * @returns {boolean} true if the schema uses allOf, oneOf or anyOf composition */ -export function getMergedProperties(schema) { - const properties = Object.create(null); - mergePropertyMaps(properties, schema.properties); - for (const subSchema of schema.allOf ?? []) { - mergePropertyMaps(properties, getMergedProperties(subSchema)); - } - return properties; -} - -function mergePropertyMaps(target, source) { - for (const [name, definition] of Object.entries(source ?? {})) { - const existing = target[name]; - if (!existing) { - target[name] = definition; - continue; - } - const merged = { ...existing, ...definition }; - if (Array.isArray(existing.enum) && Array.isArray(definition.enum)) { - merged.enum = definition.enum.filter((value) => existing.enum.includes(value)); - } else if (existing.enum !== undefined || definition.enum !== undefined) { - merged.enum = definition.enum ?? existing.enum; - } - if (existing.properties !== undefined && definition.properties !== undefined) { - const mergedNested = Object.create(null); - mergePropertyMaps(mergedNested, existing.properties); - mergePropertyMaps(mergedNested, definition.properties); - merged.properties = mergedNested; - } - target[name] = merged; - } -} - -/** - * Finds the JSONPath segments locating a property within a schema, descending into allOf - * sub-schemas, so that violations anchor at the property's actual document location instead of a - * location that only exists after merging. - * - * @param {object} schema the schema defining the property - * @param {string} property the property name to locate - * @returns {(string|number)[]|undefined} the path segments relative to the schema, or undefined - */ -export function getPropertyPath(schema, property) { - if (schema.properties?.[property]) { - return ['properties', property]; - } - const subSchemas = schema.allOf ?? []; - for (let i = 0; i < subSchemas.length; i++) { - const subPath = getPropertyPath(subSchemas[i], property); - if (subPath) { - return ['allOf', i, ...subPath]; - } - } - return undefined; -} - -/** - * Collects the required property names of a schema, merging the required lists of its allOf - * sub-schemas, which Spectral does not flatten during resolution. - * - * @param {object} schema the schema to collect required property names for - * @returns {string[]} the required property names of the schema and all of its allOf sub-schemas - */ -export function getMergedRequired(schema) { - const required = Array.isArray(schema.required) ? [...schema.required] : []; - for (const subSchema of schema.allOf ?? []) { - required.push(...getMergedRequired(subSchema)); - } - return required; +export function usesComposition(schema) { + return schema.allOf !== undefined || schema.oneOf !== undefined || schema.anyOf !== undefined; } From 726dabce14f41e56a71afdf06404d6d54b6077e3 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Thu, 20 Aug 2026 12:09:32 +0100 Subject: [PATCH 11/12] fix(ipa): remove unreachable shared-response handling from the endpoint rule In this spec components.responses is reserved for error responses: every 2xx response defines its content inline. The endpoint rule only evaluates 2xx codes, so resolveResponseRef and the contentBasePath branch could never run. Read response.content directly instead. Co-Authored-By: Claude Fable 5 --- ...dpointsMustReturnOperationResponse.test.js | 49 ------------------- tools/spectral/ipa/rulesets/IPA-132.yaml | 4 +- tools/spectral/ipa/rulesets/README.md | 4 +- ...ionEndpointsMustReturnOperationResponse.js | 24 ++------- 4 files changed, 8 insertions(+), 73 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js index 8772617595..378a54b3f8 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationEndpointsMustReturnOperationResponse.test.js @@ -68,24 +68,6 @@ const components = { }, }, }, - responses: { - OperationResponseOk: { - description: 'Operation status.', - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/OperationResponse' }, - }, - }, - }, - OrderOk: { - description: 'An order.', - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { $ref: '#/components/schemas/Order' }, - }, - }, - }, - }, }; const operationResponseGet = { @@ -228,14 +210,6 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ '/api/atlas/v2/resourceName/{pathParam}/operations': { get: paginatedOperationResponseGet, }, - // The response may be defined as a reference to a shared component response - '/api/atlas/v2/resourceName/{pathParam}/operations/{operationId}': { - get: { - responses: { - 200: { $ref: '#/components/responses/OperationResponseOk' }, - }, - }, - }, // Not an Operations endpoint, the returned schema is not restricted '/api/atlas/v2/resourceName/{pathParam}': { get: orderGet, @@ -299,29 +273,6 @@ testRule('xgen-IPA-132-operation-endpoints-must-return-operation-response', [ }, ], }, - { - name: 'invalid single Operation endpoint referencing a shared non-Operation response', - document: { - paths: { - '/api/atlas/v2/resourceName/operations/{operationId}': { - get: { - responses: { - 200: { $ref: '#/components/responses/OrderOk' }, - }, - }, - }, - }, - components, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-endpoints-must-return-operation-response', - message: SINGLE_OPERATION_ERROR_MESSAGE, - path: ['paths', '/api/atlas/v2/resourceName/operations/{operationId}', 'get', 'responses', '200'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, { name: 'invalid Operations endpoint without a JSON response schema', document: { diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index 42e1477c7a..c8d5a2f357 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -91,8 +91,8 @@ rules: ##### Implementation details Rule checks for the following conditions: - - Applies to 2xx responses of GET methods on Operations endpoints; responses defined as - references to `components.responses` are followed + - Applies to 2xx responses of GET methods on Operations endpoints; the response content + is defined inline, `components.responses` is reserved for error responses - Every 2xx response must define at least one JSON media type carrying a schema - The single Operation endpoint (`.../operations/{operationId}`) must reference the `OperationResponse` schema diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 2c5d56a41c..4cf183cebf 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1467,8 +1467,8 @@ operation status. ##### Implementation details Rule checks for the following conditions: - - Applies to 2xx responses of GET methods on Operations endpoints; responses defined as - references to `components.responses` are followed + - Applies to 2xx responses of GET methods on Operations endpoints; the response content + is defined inline, `components.responses` is reserved for error responses - Every 2xx response must define at least one JSON media type carrying a schema - The single Operation endpoint (`.../operations/{operationId}`) must reference the `OperationResponse` schema diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js index 466fa701c3..1883f814de 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationEndpointsMustReturnOperationResponse.js @@ -11,7 +11,6 @@ const SINGLE_OPERATION_ERROR_MESSAGE = `The Operation endpoint must return the $ const OPERATIONS_COLLECTION_ERROR_MESSAGE = `The Operations collection endpoint must return a paginated response whose results reference the ${OPERATION_RESPONSE_SCHEMA_NAME} schema.`; const MISSING_SCHEMA_ERROR_MESSAGE = 'The Operations endpoint must define a JSON response schema.'; -const COMPONENT_RESPONSES_REF_PREFIX = '#/components/responses/'; const COMPONENT_SCHEMAS_REF_PREFIX = '#/components/schemas/'; /** @@ -38,20 +37,16 @@ export default (input, _, { path, documentInventory, rule }) => { // Work on the unresolved document, where schema references are still visible by name const oas = documentInventory.unresolved; - const responseOrRef = resolveObject(oas, path); - const response = resolveResponseRef(responseOrRef, oas); + const response = resolveObject(oas, path); if (!response) { return; } - // Errors on a shared component response anchor at the referencing response, since the shared - // component's content is not present at this document location - const contentBasePath = responseOrRef === response ? path : undefined; - const errors = checkViolationsAndReturnErrors(isSingleOperation, response, contentBasePath, oas, path, ruleName); + const errors = checkViolationsAndReturnErrors(isSingleOperation, response, oas, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, response, path); }; -function checkViolationsAndReturnErrors(isSingleOperation, response, contentBasePath, oas, path, ruleName) { +function checkViolationsAndReturnErrors(isSingleOperation, response, oas, path, ruleName) { try { const jsonContentEntries = Object.entries(response.content ?? {}).filter(([mediaType]) => mediaType.endsWith('json') @@ -63,7 +58,7 @@ function checkViolationsAndReturnErrors(isSingleOperation, response, contentBase const errors = []; for (const [mediaType, content] of jsonContentEntries) { - const mediaTypePath = contentBasePath ? [...contentBasePath, 'content', mediaType] : path; + const mediaTypePath = [...path, 'content', mediaType]; if (!content.schema) { errors.push({ path: mediaTypePath, message: MISSING_SCHEMA_ERROR_MESSAGE }); continue; @@ -93,17 +88,6 @@ function checkViolationsAndReturnErrors(isSingleOperation, response, contentBase } } -// The response itself may be defined as a reference to a shared component response -function resolveResponseRef(responseOrRef, oas) { - if (responseOrRef?.$ref === undefined) { - return responseOrRef; - } - if (!responseOrRef.$ref.startsWith(COMPONENT_RESPONSES_REF_PREFIX)) { - return undefined; - } - return oas.components?.responses?.[getSchemaNameFromRef(responseOrRef.$ref)]; -} - function schemaReferencesOperationResponse(schema) { return schema.$ref !== undefined && getSchemaNameFromRef(schema.$ref) === OPERATION_RESPONSE_SCHEMA_NAME; } From 478430648b201a270324a2511d933e970ca595e1 Mon Sep 17 00:00:00 2001 From: Julius Jogela Date: Thu, 20 Aug 2026 12:17:03 +0100 Subject: [PATCH 12/12] fix(ipa): extract the composition ban into its own IPA-132 rule The required-fields rule owned two unrelated checks sharing one exception key, so an exception granted for composition also silenced the six required-field checks. Composition rejection now lives in xgen-IPA-132-operation-response-must-not-use-composition with its own exception key and adoption metric, and every other OperationResponse schema rule skips composed schemas relying on it. Co-Authored-By: Claude Fable 5 --- ...nseMustDefineNestedObjectStructure.test.js | 2 +- ...onResponseMustDefineOptionalFields.test.js | 2 +- ...onResponseMustDefineRequiredFields.test.js | 41 +--- ...ationResponseMustNotUseComposition.test.js | 178 ++++++++++++++++++ ...nResponseMustUseStandardEnumValues.test.js | 2 +- ...OperationsShouldExposeStatusFields.test.js | 2 +- tools/spectral/ipa/rulesets/IPA-132.yaml | 37 +++- tools/spectral/ipa/rulesets/README.md | 31 ++- ...ResponseMustDefineNestedObjectStructure.js | 2 +- ...erationResponseMustDefineOptionalFields.js | 2 +- ...erationResponseMustDefineRequiredFields.js | 18 +- ...2OperationResponseMustNotUseComposition.js | 31 +++ ...rationResponseMustUseStandardEnumValues.js | 2 +- ...PA132OperationsShouldExposeStatusFields.js | 2 +- 14 files changed, 284 insertions(+), 68 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/IPA132OperationResponseMustNotUseComposition.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustNotUseComposition.js diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js index 140f0b1907..ff07fbbc75 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineNestedObjectStructure.test.js @@ -75,7 +75,7 @@ testRule('xgen-IPA-132-operation-response-must-define-nested-object-structure', document: { components: { schemas: { - // Composition is rejected by the required-fields rule, so this rule stays silent + // Composition is rejected by the must-not-use-composition rule, so this rule stays silent OperationResponse: { allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js index 740ddeb0bb..ae313a5ac0 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineOptionalFields.test.js @@ -56,7 +56,7 @@ testRule('xgen-IPA-132-operation-response-must-define-optional-fields', [ document: { components: { schemas: { - // Composition is rejected by the required-fields rule, so this rule stays silent + // Composition is rejected by the must-not-use-composition rule, so this rule stays silent OperationResponse: { allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js index 7e08bcdeda..b24dddf489 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustDefineRequiredFields.test.js @@ -94,51 +94,18 @@ testRule('xgen-IPA-132-operation-response-must-define-required-fields', [ ], }, { - name: 'invalid OperationResponse composed with allOf', + name: 'composed schemas are skipped', document: { components: { schemas: { - // There is one and only one OperationResponse: composition is not allowed + // Composition is rejected by the must-not-use-composition rule, so this rule stays silent OperationResponse: { - allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, validOperationResponse], - }, - OperationMetadata: { - type: 'object', - properties: { - statusMessage: { type: 'string' }, - }, + allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, }, }, }, - errors: [ - { - code: 'xgen-IPA-132-operation-response-must-define-required-fields', - message: 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.', - path: ['components', 'schemas', 'OperationResponse'], - severity: DiagnosticSeverity.Warning, - }, - ], - }, - { - name: 'invalid OperationResponse composed with oneOf', - document: { - components: { - schemas: { - OperationResponse: { - oneOf: [validOperationResponse], - }, - }, - }, - }, - errors: [ - { - code: 'xgen-IPA-132-operation-response-must-define-required-fields', - message: 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.', - path: ['components', 'schemas', 'OperationResponse'], - severity: DiagnosticSeverity.Warning, - }, - ], + errors: [], }, { name: 'invalid OperationResponse with an exception', diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustNotUseComposition.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustNotUseComposition.test.js new file mode 100644 index 0000000000..364b22682a --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustNotUseComposition.test.js @@ -0,0 +1,178 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const COMPOSITION_ERROR_MESSAGE = + 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.'; + +const flatOperationResponse = { + type: 'object', + properties: { + operationId: { type: 'string' }, + status: { type: 'string', enum: ['PENDING', 'IN_PROGRESS', 'SUCCEEDED', 'FAILED', 'CANCELED', 'SUPERSEDED'] }, + }, +}; + +testRule('xgen-IPA-132-operation-response-must-not-use-composition', [ + { + name: 'valid flat OperationResponse', + document: { + components: { + schemas: { + OperationResponse: flatOperationResponse, + }, + }, + }, + errors: [], + }, + { + name: 'documents without an OperationResponse schema are ignored', + document: { + components: { + schemas: { + Order: { + type: 'object', + properties: { + id: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid OperationResponse composed with allOf', + document: { + components: { + schemas: { + // There is one and only one OperationResponse: composition is not allowed + OperationResponse: { + allOf: [{ $ref: '#/components/schemas/OperationMetadata' }, flatOperationResponse], + }, + OperationMetadata: { + type: 'object', + properties: { + statusMessage: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-not-use-composition', + message: COMPOSITION_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse composed with oneOf', + document: { + components: { + schemas: { + OperationResponse: { + oneOf: [flatOperationResponse], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-not-use-composition', + message: COMPOSITION_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse composed with anyOf', + document: { + components: { + schemas: { + OperationResponse: { + anyOf: [flatOperationResponse], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-not-use-composition', + message: COMPOSITION_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'an exception for another rule does not suppress the composition violation', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [flatOperationResponse], + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-define-required-fields': 'we compose from a base schema', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-not-use-composition', + message: COMPOSITION_ERROR_MESSAGE, + path: ['components', 'schemas', 'OperationResponse'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid OperationResponse with an exception', + document: { + components: { + schemas: { + OperationResponse: { + allOf: [flatOperationResponse], + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-not-use-composition': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant OperationResponse does not need an exception', + document: { + components: { + schemas: { + OperationResponse: { + ...flatOperationResponse, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-operation-response-must-not-use-composition': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-operation-response-must-not-use-composition', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'OperationResponse', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-operation-response-must-not-use-composition', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js index fb1f88821e..335b91528d 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationResponseMustUseStandardEnumValues.test.js @@ -59,7 +59,7 @@ testRule('xgen-IPA-132-operation-response-must-use-standard-enum-values', [ document: { components: { schemas: { - // Composition is rejected by the required-fields rule, so this rule stays silent + // Composition is rejected by the must-not-use-composition rule, so this rule stays silent OperationResponse: { allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, diff --git a/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js index 3df4c61fec..d3139cd79c 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationsShouldExposeStatusFields.test.js @@ -52,7 +52,7 @@ testRule('xgen-IPA-132-operations-should-expose-status-fields', [ document: { components: { schemas: { - // Composition is rejected by the required-fields rule, so this rule stays silent + // Composition is rejected by the must-not-use-composition rule, so this rule stays silent OperationResponse: { allOf: [{ type: 'object', properties: { statusMessage: { type: 'string' } } }], }, diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index c8d5a2f357..a2b4c13503 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -6,6 +6,7 @@ functions: - IPA132OperationMustBeAReadOnlyResource - IPA132OperationsEndpointMustBeALeafResource - IPA132OperationEndpointsMustReturnOperationResponse + - IPA132OperationResponseMustNotUseComposition - IPA132OperationResponseMustDefineRequiredFields - IPA132OperationResponseMustDefineOptionalFields - IPA132OperationResponseMustUseStandardEnumValues @@ -112,6 +113,26 @@ rules: field: '@key' function: IPA132OperationEndpointsMustReturnOperationResponse + xgen-IPA-132-operation-response-must-not-use-composition: + description: | + The OperationResponse schema must define its properties directly. There is one and only one + OperationResponse, so allOf, oneOf and anyOf composition is a violation. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The schema must not use allOf, oneOf or anyOf composition + - The other OperationResponse schema rules skip composed schemas, relying on this rule to + reject them + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-not-use-composition' + severity: warn + given: '#OperationResponseSchema' + then: + function: IPA132OperationResponseMustNotUseComposition + xgen-IPA-132-operation-response-must-define-required-fields: description: | The OperationResponse schema must define every field the IPA-132 standard marks as always @@ -122,8 +143,8 @@ rules: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - - There is one and only one OperationResponse: the schema must define its properties - directly, so allOf, oneOf and anyOf composition is a violation + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - For expiresAt, only the field declaration is validated; the retention behavior and the 404 after expiry are backend-defined - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -155,7 +176,8 @@ rules: - Each field listed in the rule's functionOptions must be defined and must not be listed as required: the condition under which it is present is a runtime state that cannot be validated statically - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-optional-fields' @@ -188,7 +210,8 @@ rules: in any order - Fields that are not defined are skipped: top-level field presence is validated by the required-fields and optional-fields rules - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-use-standard-enum-values' @@ -219,7 +242,8 @@ rules: properties - Fields that are not defined are skipped: their presence is validated by the optional-fields rule - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-response-must-define-nested-object-structure' @@ -245,7 +269,8 @@ rules: `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operations-should-expose-status-fields' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 4cf183cebf..7657503932 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1481,6 +1481,21 @@ Rule checks for the following conditions: - Responses with `x-xgen-IPA-exception` for this rule at the response level are excluded from validation +#### xgen-IPA-132-operation-response-must-not-use-composition + + ![warn](https://img.shields.io/badge/warning-yellow) +The OperationResponse schema must define its properties directly. There is one and only one +OperationResponse, so allOf, oneOf and anyOf composition is a violation. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to the OperationResponse component schema + - The schema must not use allOf, oneOf or anyOf composition + - The other OperationResponse schema rules skip composed schemas, relying on this rule to + reject them + - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation + #### xgen-IPA-132-operation-response-must-define-required-fields ![warn](https://img.shields.io/badge/warning-yellow) @@ -1492,8 +1507,8 @@ Rule checks for the following conditions: - Applies to the OperationResponse component schema - Each field listed in the rule's functionOptions must be defined and listed as required - - There is one and only one OperationResponse: the schema must define its properties - directly, so allOf, oneOf and anyOf composition is a violation + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - For expiresAt, only the field declaration is validated; the retention behavior and the 404 after expiry are backend-defined - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -1512,7 +1527,8 @@ Rule checks for the following conditions: - Each field listed in the rule's functionOptions must be defined and must not be listed as required: the condition under which it is present is a runtime state that cannot be validated statically - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-use-standard-enum-values @@ -1530,7 +1546,8 @@ Rule checks for the following conditions: in any order - Fields that are not defined are skipped: top-level field presence is validated by the required-fields and optional-fields rules - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operation-response-must-define-nested-object-structure @@ -1547,7 +1564,8 @@ Rule checks for the following conditions: properties - Fields that are not defined are skipped: their presence is validated by the optional-fields rule - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operations-should-expose-status-fields @@ -1564,7 +1582,8 @@ Rule checks for the following conditions: `statusMessage`, `progress` and `estimatedCompletionTime` - The progress object should define `completed`, `total` and `unit` properties - Kept separate from the must-level rules because it enforces should-level guidance - - Composed schemas are skipped: composition is rejected by the required-fields rule + - Composed schemas are skipped: composition is rejected by the must-not-use-composition + rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js index 2483a43e67..59c8d04a9b 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineNestedObjectStructure.js @@ -12,7 +12,7 @@ import { usesComposition } from './utils/longRunningOperations.js'; * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { - // Composed schemas are rejected by the required-fields rule + // Composed schemas are rejected by the must-not-use-composition rule if (usesComposition(input)) { return; } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js index 541d9b4050..ca1679fdc9 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineOptionalFields.js @@ -12,7 +12,7 @@ import { usesComposition } from './utils/longRunningOperations.js'; * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { - // Composed schemas are rejected by the required-fields rule + // Composed schemas are rejected by the must-not-use-composition rule if (usesComposition(input)) { return; } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js index 51fb9c96c7..53a09ba2a2 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustDefineRequiredFields.js @@ -1,19 +1,20 @@ import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; import { usesComposition } from './utils/longRunningOperations.js'; -const COMPOSITION_ERROR_MESSAGE = - 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.'; - /** - * Checks that the OperationResponse schema defined by IPA-132 is a single flat schema defining - * every field the standard marks as always required, each listed in the schema's required array. - * The field names are provided declaratively through the rule's functionOptions. + * Checks that the OperationResponse schema defined by IPA-132 defines every field the standard + * marks as always required, each listed in the schema's required array. The field names are + * provided declaratively through the rule's functionOptions. * * @param {object} input - The OperationResponse schema object * @param {object} opts - The rule's functionOptions, carrying the required field names * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { + // Composed schemas are rejected by the must-not-use-composition rule + if (usesComposition(input)) { + return; + } const ruleName = rule.name; const errors = checkViolationsAndReturnErrors(input, opts.fields, path, ruleName); return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); @@ -21,11 +22,6 @@ export default (input, opts, { path, rule }) => { function checkViolationsAndReturnErrors(schema, fields, path, ruleName) { try { - // There is one and only one OperationResponse: a flat schema, so composition is a violation - if (usesComposition(schema)) { - return [{ path, message: COMPOSITION_ERROR_MESSAGE }]; - } - const properties = schema.properties ?? {}; const required = schema.required ?? []; const errors = []; diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustNotUseComposition.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustNotUseComposition.js new file mode 100644 index 0000000000..3c045cfd85 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustNotUseComposition.js @@ -0,0 +1,31 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { usesComposition } from './utils/longRunningOperations.js'; + +const COMPOSITION_ERROR_MESSAGE = + 'OperationResponse must define its properties directly, without allOf, oneOf or anyOf composition.'; + +/** + * Checks that the OperationResponse schema defined by IPA-132 is a single flat schema: there is + * one and only one OperationResponse, so allOf, oneOf and anyOf composition is a violation. The + * other OperationResponse schema rules skip composed schemas, relying on this rule to reject them. + * + * @param {object} input - The OperationResponse schema object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, _, { path, rule }) => { + const ruleName = rule.name; + const errors = checkViolationsAndReturnErrors(input, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(schema, path, ruleName) { + try { + if (usesComposition(schema)) { + return [{ path, message: COMPOSITION_ERROR_MESSAGE }]; + } + return []; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js index a8d16c61e9..ea851ab181 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationResponseMustUseStandardEnumValues.js @@ -12,7 +12,7 @@ import { isExactEnumMatch, usesComposition } from './utils/longRunningOperations * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { - // Composed schemas are rejected by the required-fields rule + // Composed schemas are rejected by the must-not-use-composition rule if (usesComposition(input)) { return; } diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js index b7e733a802..fb58a55489 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationsShouldExposeStatusFields.js @@ -12,7 +12,7 @@ import { usesComposition } from './utils/longRunningOperations.js'; * @param {object} context - The context object containing the path, documentInventory and rule */ export default (input, opts, { path, rule }) => { - // Composed schemas are rejected by the required-fields rule + // Composed schemas are rejected by the must-not-use-composition rule if (usesComposition(input)) { return; }