[MutabilityWithReadOnly] Optimize to prevent excessive memory usage on large specs

packages/rulesets/CHANGELOG.md

@@ -1,5 +1,11 @@
 # Change Log - @microsoft.azure/openapi-validator-rulesets
 
+## 2.2.1
+
+### Patches
+
+- [MutabilityWithReadOnly] Optimize rule to prevent out-of-memory issues by filtering in JSONPath given clause (fixes #812)
+
 ## 2.2.0
 
 ### Minor changes

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,
             },

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,
             },

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,
             },

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": {

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,
       },

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 || [];

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);
   });
 });