Resolve Erroneous additionalProperty error on refWithReadOnly (#1004

) * This PR adds to the 'selective ignore' list that is maintained to bypass certain errors that we want to ignore. here's the thing. I really think that the true solution is to fix this in the schema schemaValidator, as it's error on the property we're adding to the schema. Is there such a thing as properties on the schema that are present, but totally ignored for the purposes of validation? Need to investigate. in the meantime, this is what it would look like to just ignore this specific error * move the test with the others * apply prettier fixes * add the necessary package and changelog updates to reflect the PR --------- Co-authored-by: Scott Beddall (from Dev Box) <[email protected]>
Azure · Sep 29, 2023 · 795eda5 · 795eda5
1 parent e09eb66
commit 795eda5
Show file tree

Hide file tree

Showing 52 changed files with 7,630 additions and 163 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -1,5 +1,9 @@
 # Change Log - oav
 
+## 09-29-2023 3.2.13
+
+- #1004 fixes an issue with the injected property refWithReadOnly causing additionalProperty error in schema validator.
+
 ## 09/25/2023 3.2.12
 
 - #996 Allows `required` properties to be ommitted from a response if they are also marked `readonly`.

diff --git a/lib/apiScenario/gen/ApiTestRuleBasedGenerator.ts b/lib/apiScenario/gen/ApiTestRuleBasedGenerator.ts
@@ -1,9 +1,9 @@
 import { existsSync, writeFileSync } from "fs";
+import { dirname, join, relative, resolve } from "path";
 import { mkdirpSync } from "fs-extra";
 import { injectable } from "inversify";
 import { dump } from "js-yaml";
 import _ from "lodash";
-import { dirname, join, relative, resolve } from "path";
 import { inversifyGetInstance } from "../../inversifyUtils";
 import { JsonLoader } from "../../swagger/jsonLoader";
 import { setDefaultOpts } from "../../swagger/loader";
@@ -316,7 +316,7 @@ class ArmResourceDependencyGenerator {
     return [this._basicScenario?.prepareSteps, this._basicScenario?.cleanUpSteps];
   }
   generateResourceCleanup(resource: ArmResourceManipulator, scenario: RawScenario) {
-    this._restlerGenerator?.addCleanupSteps(resource,scenario);
+    this._restlerGenerator?.addCleanupSteps(resource, scenario);
   }
 }
 
@@ -493,7 +493,7 @@ export class ApiTestRuleBasedGenerator {
             if (apiSenarios) {
               apiSenarios.description = "[This scenario is auto-generated]" + rule.description;
               dependency?.updateExampleFile(resource, apiSenarios);
-              dependency?.generateResourceCleanup(resource,apiSenarios);
+              dependency?.generateResourceCleanup(resource, apiSenarios);
               definition.scenarios.push({ scenario: rule.name, ...apiSenarios });
             }
           }

diff --git a/lib/apiScenario/gen/azureCliRecordingLoader.ts b/lib/apiScenario/gen/azureCliRecordingLoader.ts
@@ -3,9 +3,9 @@ import { URL } from "url";
 import { HttpMethods } from "@azure/core-http";
 import { injectable } from "inversify";
 import { Loader } from "../../swagger/loader";
+import { DEFAULT_ARM_ENDPOINT } from "../constants";
 import { RequestTracking, SingleRequestTracking } from "./testRecordingApiScenarioGenerator";
 import { parseRecordingBodyJson, transformRecordingHeaders } from "./dotnetRecordingLoader";
-import { DEFAULT_ARM_ENDPOINT } from "../constants";
 
 interface RecordingFile {
   interactions: RecordingEntry[];

diff --git a/lib/apiScenario/gen/restlerApiScenarioGenerator.ts b/lib/apiScenario/gen/restlerApiScenarioGenerator.ts
@@ -28,10 +28,10 @@ import {
 import * as util from "../../generator/util";
 import { setDefaultOpts } from "../../swagger/loader";
 import Mocker from "../../generator/mocker";
-import { ArmResourceManipulator } from "./ApiTestRuleBasedGenerator";
 import { logger } from ".././logger";
 import { xmsExamples, xmsSkipUrlEncoding } from "../../util/constants";
 import { ApiScenarioYamlLoader } from "../apiScenarioYamlLoader";
+import { ArmResourceManipulator } from "./ApiTestRuleBasedGenerator";
 
 export interface ApiScenarioGeneratorOption extends ApiScenarioLoaderOption {
   swaggerFilePaths: string[];

diff --git a/lib/apiScenario/gen/rules/noChildResourceCreated.ts b/lib/apiScenario/gen/rules/noChildResourceCreated.ts
@@ -3,7 +3,7 @@ import { ApiTestGeneratorRule, ResourceManipulatorInterface } from "../ApiTestRu
 
 export const NoChildResourceCreated: ApiTestGeneratorRule = {
   name: "NoChildResourceCreated",
-  armRpcCodes: ["RPC-Put-V1-16","RPC-Put-V1-17"],
+  armRpcCodes: ["RPC-Put-V1-16", "RPC-Put-V1-17"],
   description: "Check if put operation will create nested resource implicitly.",
   resourceKinds: ["Tracked", "Extension"],
   appliesTo: ["ARM"],
@@ -13,18 +13,18 @@ export const NoChildResourceCreated: ApiTestGeneratorRule = {
     if (childResources.length === 0) {
       return null;
     }
-    let hit = false
+    let hit = false;
     for (resource of childResources) {
-      const listOperation = resource.getListOperations()[0]
+      const listOperation = resource.getListOperations()[0];
       if (listOperation && listOperation.examples[0]) {
-        const step:RawStep = {
+        const step: RawStep = {
           operationId: listOperation.operationId,
-          responses: {200: {body:{ value:[]}}}
+          responses: { 200: { body: { value: [] } } },
         };
         base.steps.push(step);
         hit = true;
       }
     }
-    return hit ? base : null
+    return hit ? base : null;
   },
 };
diff --git a/lib/apiScenario/gen/rules/resourceNameCaseInsensitive.ts b/lib/apiScenario/gen/rules/resourceNameCaseInsensitive.ts
@@ -14,7 +14,8 @@ function getResourceNameParameter(path: string) {
 export const ResourceNameCaseInsensitive: ApiTestGeneratorRule = {
   name: "ResourceNameCaseInSensitive",
   armRpcCodes: ["RPC-V2-PUT-3"],
-  description: "Check if the resource name in the response has same case with resource name in the request.",
+  description:
+    "Check if the resource name in the response has same case with resource name in the request.",
   resourceKinds: ["Tracked"],
   appliesTo: ["ARM"],
   useExample: true,
@@ -27,14 +28,15 @@ export const ResourceNameCaseInsensitive: ApiTestGeneratorRule = {
       // generate a mocked value
       base.variables._mockedRandom = { type: "string", prefix: "r" };
       // set the operation variable
-      const oldPrefix = (resourceNameVar as any).prefix ||  `${resourceName.toLocaleLowerCase().substring(0, 10)}`;
+      const oldPrefix =
+        (resourceNameVar as any).prefix || `${resourceName.toLocaleLowerCase().substring(0, 10)}`;
       (resourceNameVar as any).value = `${toUpper(oldPrefix)}$(_mockedRandom)`;
       delete (resourceNameVar as any).prefix;
       // modify the global variable
       base.variables[resourceName] = { value: `${oldPrefix}$(_mockedRandom)`, type: "string" };
       variables[resourceName] = resourceNameVar;
     }
-    const step:RawStep = { operationId: getOp.operationId ,variables};
+    const step: RawStep = { operationId: getOp.operationId, variables };
     base.steps.push(step);
     return base;
   },

diff --git a/lib/apiScenario/gen/rules/systemDataExistsInResponse.ts b/lib/apiScenario/gen/rules/systemDataExistsInResponse.ts
@@ -5,20 +5,19 @@ export const SystemDataExistsInResponse: ApiTestGeneratorRule = {
   name: "SystemDataExistsInResponse",
   armRpcCodes: ["RPC-SystemData-V1-01"],
   description: "Check if the systemData exists in the response.",
-  resourceKinds: ["Tracked","Proxy","Extension"],
+  resourceKinds: ["Tracked", "Proxy", "Extension"],
   appliesTo: ["ARM"],
   useExample: true,
   generator: (resource: ResourceManipulatorInterface, base: RawScenario) => {
     const getOp = resource.getResourceOperation("Get");
-    const step:RawStep = { operationId: getOp.operationId };
+    const step: RawStep = { operationId: getOp.operationId };
     const responses = {} as any;
 
-    if (resource.getProperty( "systemData")) {
+    if (resource.getProperty("systemData")) {
       responses["200"] = [{ test: "/body/systemData", expression: "to.not.be.undefined" }];
-      step.responses = responses
-    }
-    else {
-      return null
+      step.responses = responses;
+    } else {
+      return null;
     }
     base.steps.push(step);
     return base;

diff --git a/lib/swaggerValidator/ajvSchemaValidator.ts b/lib/swaggerValidator/ajvSchemaValidator.ts
@@ -285,6 +285,14 @@ const shouldSkipError = (error: ErrorObject, cxt: SchemaValidateContext) => {
     return true;
   }
 
+  // If we're erroring on the added property refWithReadOnly simply ignore the error
+  if (
+    error.keyword === "additionalProperties" &&
+    (params as any).additionalProperty === "refWithReadOnly"
+  ) {
+    return true;
+  }
+
   // If a response has x-ms-mutability property and its missing the read we can skip this error
   if (
     cxt.isResponse &&

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "oav",
-  "version": "3.2.12",
+  "version": "3.2.13",
   "author": {
     "name": "Microsoft Corporation",
     "email": "[email protected]",

diff --git a/test/ApiTestRuleBasedGeneratorTest.ts b/test/ApiTestRuleBasedGeneratorTest.ts
@@ -1,10 +1,13 @@
 import * as assert from "assert";
 import { exec } from "child_process";
-import { existsSync,readFileSync, writeFileSync } from "fs";
+import { existsSync, readFileSync, writeFileSync } from "fs";
+import { dirname, join, resolve } from "path";
 import { mkdirpSync } from "fs-extra";
 import glob from "glob";
-import {dirname, join, resolve } from "path";
-import { ApiTestGeneratorRule, ApiTestRuleBasedGenerator } from "../lib/apiScenario/gen/ApiTestRuleBasedGenerator";
+import {
+  ApiTestGeneratorRule,
+  ApiTestRuleBasedGenerator,
+} from "../lib/apiScenario/gen/ApiTestRuleBasedGenerator";
 import { NoChildResourceCreated } from "../lib/apiScenario/gen/rules/noChildResourceCreated";
 import { ResourceNameCaseInsensitive } from "../lib/apiScenario/gen/rules/resourceNameCaseInsensitive";
 import { SystemDataExistsInResponse } from "../lib/apiScenario/gen/rules/systemDataExistsInResponse";
@@ -18,7 +21,7 @@ jest.setTimeout(9999999);
 
 export const testApiTestRuleBase = async (
   swaggers: string[],
-  specFolder:string,
+  specFolder: string,
   rules: ApiTestGeneratorRule[],
   isRPaaS?: string
 ) => {
@@ -30,46 +33,46 @@ export const testApiTestRuleBase = async (
   const swaggerLoader = inversifyGetInstance(SwaggerLoader, opts);
   const jsonLoader = inversifyGetInstance(JsonLoader, opts);
 
-  const generateDependencyFile = async (swaggers:string[],specFolder:string) => {
-      const outputDir = `.restler_output_${swaggers[0].split("/")[1]}`
-      const restlerConfig = {
-        SwaggerSpecFilePath: swaggers.map(s => join("/swagger",s)),
-      };
-      const restlerConfigFile =  join("restler_config", "config.json")
-      if (!existsSync(dirname(join(specFolder, restlerConfigFile)))) {
-        mkdirpSync(dirname(join(specFolder, restlerConfigFile)));
-      }
-      writeFileSync(join(specFolder, restlerConfigFile), JSON.stringify(restlerConfig));
-      const { err, stderr } = (await new Promise((res) =>
-        exec(
-          `docker run --rm -v $(pwd):/swagger -w /swagger/${outputDir} mcr.microsoft.com/restlerfuzzer/restler dotnet /RESTler/restler/Restler.dll compile /swagger/${restlerConfigFile}`,
-          { encoding: "utf8", maxBuffer: 1024 * 1024 * 64, cwd: specFolder },
-          (err: unknown, stdout: unknown, stderr: unknown) =>
-            res({ err: err, stderr: stderr, stdout: stdout })
-        )
-      )) as any;
-     if (err || stderr) {
-      console.log(err || stderr)
-     }
-     const dependencyFile = resolve(specFolder,outputDir,"Compile/dependencies.json")
-     if (existsSync(dependencyFile)) {
-       return dependencyFile
-     }
-     console.log(`Could not find dependency file:${dependencyFile}.`)
-     return null
-  }
+  const generateDependencyFile = async (swaggers: string[], specFolder: string) => {
+    const outputDir = `.restler_output_${swaggers[0].split("/")[1]}`;
+    const restlerConfig = {
+      SwaggerSpecFilePath: swaggers.map((s) => join("/swagger", s)),
+    };
+    const restlerConfigFile = join("restler_config", "config.json");
+    if (!existsSync(dirname(join(specFolder, restlerConfigFile)))) {
+      mkdirpSync(dirname(join(specFolder, restlerConfigFile)));
+    }
+    writeFileSync(join(specFolder, restlerConfigFile), JSON.stringify(restlerConfig));
+    const { err, stderr } = (await new Promise((res) =>
+      exec(
+        `docker run --rm -v $(pwd):/swagger -w /swagger/${outputDir} mcr.microsoft.com/restlerfuzzer/restler dotnet /RESTler/restler/Restler.dll compile /swagger/${restlerConfigFile}`,
+        { encoding: "utf8", maxBuffer: 1024 * 1024 * 64, cwd: specFolder },
+        (err: unknown, stdout: unknown, stderr: unknown) =>
+          res({ err: err, stderr: stderr, stdout: stdout })
+      )
+    )) as any;
+    if (err || stderr) {
+      console.log(err || stderr);
+    }
+    const dependencyFile = resolve(specFolder, outputDir, "Compile/dependencies.json");
+    if (existsSync(dependencyFile)) {
+      return dependencyFile;
+    }
+    console.log(`Could not find dependency file:${dependencyFile}.`);
+    return null;
+  };
   const dependencyFile = await generateDependencyFile(swaggers, specFolder);
-  const outputDir = `${join(specFolder,dirname(swaggers[0]))}/generatedScenarios`;
-  assert.ok(dependencyFile)
+  const outputDir = `${join(specFolder, dirname(swaggers[0]))}/generatedScenarios`;
+  assert.ok(dependencyFile);
   const generator = new ApiTestRuleBasedGenerator(
     swaggerLoader,
     jsonLoader,
     rules,
-    swaggers.map(s =>resolve(specFolder,s)),
+    swaggers.map((s) => resolve(specFolder, s)),
     dependencyFile!
   );
   await generator.run(outputDir, isRPaaS ? "RPaaS" : "ARM");
-  const pathPattern = resolve(outputDir,"*.yaml")
+  const pathPattern = resolve(outputDir, "*.yaml");
   return glob.sync(pathPattern, {
     ignore: ["**/examples/**/*.json", "**/quickstart-templates/*.json", "**/schema/*.json"],
   });
@@ -84,14 +87,15 @@ async function testApiTestRuleBaseForReadme(
   const inputs = (await getInputFiles(join(specFolder, readmeMd))).map((it: string) =>
     join(dirname(readmeMd), it)
   );
-  return  await testApiTestRuleBase(
+  return await testApiTestRuleBase(
     inputs.filter((spec) => !isCommonSpec(join(specFolder, spec))),
     specFolder,
-    rules,isRPaaS
+    rules,
+    isRPaaS
   );
 }
 
-function isCommonSpec(swagger:string) {
+function isCommonSpec(swagger: string) {
   const swaggerDefinition = JSON.parse(readFileSync(swagger).toString());
   return !swaggerDefinition.paths || Object.keys(swaggerDefinition.paths).length === 0;
 }
@@ -108,21 +112,19 @@ describe("Api Test rule based generator test", () => {
       ignore: ["**/examples/**/*.json", "**/quickstart-templates/*.json", "**/schema/*.json"],
     }
   );
-  const selectedRps = ["appconfiguration","monitor","sql","hdinsight", "resource","storage"];
+  const selectedRps = ["appconfiguration", "monitor", "sql", "hdinsight", "resource", "storage"];
   const allSpecs = specPaths
-    .filter(
-      (p: string) => selectedRps.some((rp: string) => p.includes(`specification/${rp}/`))
-    )
+    .filter((p: string) => selectedRps.some((rp: string) => p.includes(`specification/${rp}/`)))
     .map((f) => f.substring(specFolder.length + 1));
-  
+
   it("test rules", async () => {
     for (const readmeMd of allSpecs) {
-      const scenarioFiles = await testApiTestRuleBaseForReadme(
-        readmeMd,
-        specFolder,
-        [NoChildResourceCreated,ResourceNameCaseInsensitive,SystemDataExistsInResponse]
-      );
+      const scenarioFiles = await testApiTestRuleBaseForReadme(readmeMd, specFolder, [
+        NoChildResourceCreated,
+        ResourceNameCaseInsensitive,
+        SystemDataExistsInResponse,
+      ]);
       assert.ok(scenarioFiles);
     }
-  })
-})
+  });
+});
diff --git a/test/liveValidatorTests.ts b/test/liveValidatorTests.ts
@@ -29,7 +29,7 @@ describe("Live Validator", () => {
         isArmCall: false,
         isPathCaseSensitive: false,
         loadValidatorInBackground: true,
-        loadValidatorInInitialize: false
+        loadValidatorInInitialize: false,
       };
       const validator = new LiveValidator();
       assert.equal(0, validator.operationSearcher.cache.size);
@@ -78,7 +78,7 @@ describe("Live Validator", () => {
         },
         directory: path.resolve(os.homedir(), "repo"),
         loadValidatorInBackground: true,
-        loadValidatorInInitialize: false
+        loadValidatorInInitialize: false,
       };
       const validator = new LiveValidator({ swaggerPaths });
       assert.equal(0, validator.operationSearcher.cache.size);
@@ -98,7 +98,7 @@ describe("Live Validator", () => {
         },
         directory,
         loadValidatorInBackground: true,
-        loadValidatorInInitialize: false
+        loadValidatorInInitialize: false,
       };
       const validator = new LiveValidator({ swaggerPaths, directory });
       assert.equal(0, validator.operationSearcher.cache.size);
@@ -122,7 +122,7 @@ describe("Live Validator", () => {
         },
         directory,
         loadValidatorInBackground: true,
-        loadValidatorInInitialize: false
+        loadValidatorInInitialize: false,
       };
       const validator = new LiveValidator({
         swaggerPaths,
@@ -148,7 +148,7 @@ describe("Live Validator", () => {
         isArmCall: false,
         isPathCaseSensitive: false,
         loadValidatorInBackground: true,
-        loadValidatorInInitialize: false
+        loadValidatorInInitialize: false,
       };
       const validator = new LiveValidator({
         swaggerPaths,
@@ -1063,7 +1063,7 @@ describe("Live Validator", () => {
         git: {
           shouldClone: false,
         },
-        isArmCall: true
+        isArmCall: true,
       };
       const liveValidator = new LiveValidator(options);
       await liveValidator.initialize();