diff --git a/packages/rulesets/CHANGELOG.md b/packages/rulesets/CHANGELOG.md index e62e86884..6413c3722 100644 --- a/packages/rulesets/CHANGELOG.md +++ b/packages/rulesets/CHANGELOG.md @@ -1,5 +1,11 @@ # Change Log - @microsoft.azure/openapi-validator-rulesets +## 2.2.1 + +### Patches + +- [MutabilityWithReadOnly] Optimize rule to prevent excessive memory usage on large specs + ## 2.2.0 ### Minor changes diff --git a/packages/rulesets/generated/spectral/az-arm.js b/packages/rulesets/generated/spectral/az-arm.js index 2926c4f06..bb2146d44 100644 --- a/packages/rulesets/generated/spectral/az-arm.js +++ b/packages/rulesets/generated/spectral/az-arm.js @@ -144,9 +144,7 @@ const mutabilityWithReadOnly = (prop, _opts, ctx) => { if (prop === null || typeof prop !== "object") { return []; } - if (prop.readOnly === undefined || - prop["x-ms-mutability"] === undefined || - prop["x-ms-mutability"].length === 0) { + if (prop["x-ms-mutability"].length === 0) { return []; } const path = ctx.path || []; @@ -1067,7 +1065,7 @@ const ruleset$1 = { severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"], + given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"], then: { function: mutabilityWithReadOnly, }, diff --git a/packages/rulesets/generated/spectral/az-common.js b/packages/rulesets/generated/spectral/az-common.js index 1c428e839..8626815ea 100644 --- a/packages/rulesets/generated/spectral/az-common.js +++ b/packages/rulesets/generated/spectral/az-common.js @@ -141,9 +141,7 @@ const mutabilityWithReadOnly = (prop, _opts, ctx) => { if (prop === null || typeof prop !== "object") { return []; } - if (prop.readOnly === undefined || - prop["x-ms-mutability"] === undefined || - prop["x-ms-mutability"].length === 0) { + if (prop["x-ms-mutability"].length === 0) { return []; } const path = ctx.path || []; @@ -821,7 +819,7 @@ const ruleset = { severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"], + given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"], then: { function: mutabilityWithReadOnly, }, diff --git a/packages/rulesets/generated/spectral/az-dataplane.js b/packages/rulesets/generated/spectral/az-dataplane.js index 470b83de1..9052b283a 100644 --- a/packages/rulesets/generated/spectral/az-dataplane.js +++ b/packages/rulesets/generated/spectral/az-dataplane.js @@ -141,9 +141,7 @@ const mutabilityWithReadOnly = (prop, _opts, ctx) => { if (prop === null || typeof prop !== "object") { return []; } - if (prop.readOnly === undefined || - prop["x-ms-mutability"] === undefined || - prop["x-ms-mutability"].length === 0) { + if (prop["x-ms-mutability"].length === 0) { return []; } const path = ctx.path || []; @@ -851,7 +849,7 @@ const ruleset$1 = { severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"], + given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"], then: { function: mutabilityWithReadOnly, }, diff --git a/packages/rulesets/package.json b/packages/rulesets/package.json index 85c94750b..d176766ca 100644 --- a/packages/rulesets/package.json +++ b/packages/rulesets/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft.azure/openapi-validator-rulesets", - "version": "2.2.0", + "version": "2.2.1", "description": "Azure OpenAPI Validator", "main": "dist/index.js", "scripts": { diff --git a/packages/rulesets/src/spectral/az-common.ts b/packages/rulesets/src/spectral/az-common.ts index 560c38ac2..605e344cc 100644 --- a/packages/rulesets/src/spectral/az-common.ts +++ b/packages/rulesets/src/spectral/az-common.ts @@ -263,7 +263,7 @@ const ruleset: any = { severity: "error", resolved: true, formats: [oas2], - given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"], + given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"], then: { function: mutabilityWithReadOnly, }, diff --git a/packages/rulesets/src/spectral/functions/Extensions/mutability-with-read-only.ts b/packages/rulesets/src/spectral/functions/Extensions/mutability-with-read-only.ts index 45f425d41..c06242f9a 100644 --- a/packages/rulesets/src/spectral/functions/Extensions/mutability-with-read-only.ts +++ b/packages/rulesets/src/spectral/functions/Extensions/mutability-with-read-only.ts @@ -4,11 +4,8 @@ export const mutabilityWithReadOnly = (prop: any, _opts: any, ctx: any) => { if (prop === null || typeof prop !== "object") { return []; } - if ( - prop.readOnly === undefined || - prop["x-ms-mutability"] === undefined || - prop["x-ms-mutability"].length === 0 - ) { + // The "given" clause already filters for "readOnly !== undefined" and "x-ms-mutability !== undefined" + if (prop["x-ms-mutability"].length === 0) { return []; } const path = ctx.path || []; diff --git a/packages/rulesets/src/spectral/test/mutability-with-read-only.test.ts b/packages/rulesets/src/spectral/test/mutability-with-read-only.test.ts index 76273f7ea..9ccaced3f 100644 --- a/packages/rulesets/src/spectral/test/mutability-with-read-only.test.ts +++ b/packages/rulesets/src/spectral/test/mutability-with-read-only.test.ts @@ -8,88 +8,129 @@ beforeAll(async () => { return linter; }); -test("MutabilityWithReadOnly should find errors", () => { - const myOpenApiDocument = { - swagger: "2.0", - paths: { - "/api/Paths": { - put: { - operationId: "Path_Create", - responses: { - 200: { - description: "Success", - schema: { - $ref: "#/definitions/LroStatusCodeSchema", - }, +// Helper function to create OpenAPI document with properties +const createOpenApiDoc = (properties: unknown) => ({ + swagger: "2.0", + paths: { + "/api/Paths": { + put: { + operationId: "Path_Create", + responses: { + 200: { + description: "Success", + schema: { + $ref: "#/definitions/Schema", }, }, }, }, }, - definitions: { - LroStatusCodeSchema: { - type: "object", - properties: { - name: { - type: "string", - readOnly: true, - "x-ms-mutability": ["read", "update"], - }, - length: { - type: "string", - readOnly: false, - "x-ms-mutability": ["read"], - }, - }, - }, + }, + definitions: { + Schema: { + type: "object", + properties, + }, + }, +}); + +test("MutabilityWithReadOnly: valid combinations", () => { + const myOpenApiDocument = createOpenApiDoc({ + readOnlyTrueValid: { + type: "string", + readOnly: true, + "x-ms-mutability": ["read"], + }, + readOnlyFalseValid1: { + type: "string", + readOnly: false, + "x-ms-mutability": ["read", "create"], + }, + readOnlyFalseValid2: { + type: "string", + readOnly: false, + "x-ms-mutability": ["update"], }, - }; + readOnlyFalseValid3: { + type: "string", + readOnly: false, + "x-ms-mutability": ["create"], + }, + readOnlyFalseValid4: { + type: "string", + readOnly: false, + "x-ms-mutability": ["read", "create", "update"], + }, + }); return linter.run(myOpenApiDocument).then((results) => { - expect(results.length).toBe(2); - expect(results[0].message).toBe(`When property is modeled as "readOnly": true then x-ms-mutability extension can only have "read" value. When property is modeled as "readOnly": false then applying x-ms-mutability extension with only "read" value is not allowed. Extension contains invalid values: 'read'.`); - expect(results[0].path.join(".")).toBe("paths./api/Paths.put.responses.200.schema.properties.length"); - expect(results[1].message).toBe(`When property is modeled as "readOnly": true then x-ms-mutability extension can only have "read" value. When property is modeled as "readOnly": false then applying x-ms-mutability extension with only "read" value is not allowed. Extension contains invalid values: 'read, update'.`); - expect(results[1].path.join(".")).toBe("paths./api/Paths.put.responses.200.schema.properties.name"); + expect(results.length).toBe(0); }); }); -test("MutabilityWithReadOnly should find no errors", () => { - const myOpenApiDocument = { - swagger: "2.0", - paths: { - "/api/Paths": { - put: { - operationId: "Path_Create", - responses: { - 200: { - description: "Success", - schema: { - $ref: "#/definitions/LroStatusCodeSchema", - }, - }, - }, - }, - }, +test("MutabilityWithReadOnly: invalid combinations", () => { + const myOpenApiDocument = createOpenApiDoc({ + readOnlyTrueInvalid1: { + type: "string", + readOnly: true, + "x-ms-mutability": ["read", "update"], }, - definitions: { - LroStatusCodeSchema: { - type: "object", - properties: { - name: { - type: "string", - readOnly: true, - "x-ms-mutability": ["read"], - }, - length: { - type: "string", - readOnly: false, - "x-ms-mutability": ["read", "update"], - }, - }, - }, + readOnlyTrueInvalid2: { + type: "string", + readOnly: true, + "x-ms-mutability": ["update"], + }, + readOnlyTrueInvalid3: { + type: "string", + readOnly: true, + "x-ms-mutability": ["create"], + }, + readOnlyTrueInvalid4: { + type: "string", + readOnly: true, + "x-ms-mutability": ["read", "create"], }, - }; + readOnlyTrueInvalid5: { + type: "string", + readOnly: true, + "x-ms-mutability": ["read", "create", "update"], + }, + readOnlyFalseInvalid: { + type: "string", + readOnly: false, + "x-ms-mutability": ["read"], + }, + }); + return linter.run(myOpenApiDocument).then((results) => { + // 5 invalid readOnly: true + 1 invalid readOnly: false = 6 total errors + expect(results.length).toBe(6); + // Verify all are error messages (not just checking counts since all errors have same message format) + results.forEach((result) => { + expect(result.message).toContain("Extension contains invalid values:"); + }); + }); +}); + +test("MutabilityWithReadOnly: properties ignored by given clause", () => { + const myOpenApiDocument = createOpenApiDoc({ + emptyMutability: { + type: "string", + readOnly: true, + "x-ms-mutability": [], + }, + onlyReadOnly: { + type: "string", + readOnly: true, + }, + onlyMutability: { + type: "string", + "x-ms-mutability": ["read"], + }, + neitherField: { + type: "string", + }, + }); return linter.run(myOpenApiDocument).then((results) => { + // All properties should be ignored: empty array, missing field, or both missing expect(results.length).toBe(0); }); });