Remove additionalProperties validation suppression (#1027)

* reflect the updates from #1027, but without paying the merge conflict price
* ensure that the response validation is failed as expected

ChangeLog.md

@@ -1,5 +1,9 @@
 # Change Log - oav
 
+## 06/17/2024 3.3.8
+
+- Remove suppression of `additionalProperties` errors when `isArmCall === true`. (ARM liveValidation scenarios)
+
 ## 06/11/2024 3.3.7
 
 - Released during an npmjs.org outage. Version not showing up, but shows as successful release. Patch bump to re-attempt releasing the same version.

lib/liveValidation/operationValidator.ts

@@ -283,9 +283,9 @@ export const schemaValidateIssueToLiveValidationIssue = (
   operation: Operation,
   ctx: SchemaValidateContext,
   output: LiveValidationIssue[],
-  operationContext: OperationContext,
-  isArmCall?: boolean,
-  logging?: LoggingFn
+  _operationContext: OperationContext,
+  _isArmCall?: boolean,
+  _logging?: LoggingFn
 ) => {
   for (const i of input) {
     const issue = i as Writable<LiveValidationIssue>;
@@ -305,30 +305,6 @@ export const schemaValidateIssueToLiveValidationIssue = (
           skipIssue = true;
           return "";
         }
-      } else if (issue.code === "INVALID_TYPE" && isArmCall === false) {
-        // See Azure/oav#983 for additional information as to why this special case is present.
-        // RPs working with the RPaaS team were having dificulty with additionalProperties validation due to the fact
-        // that when we turned it on, a LOT of real, live requests were being rejected due to invalid additionalProperties settings.
-        //
-        // We need oav to have the capability to skip this if we are invoking an arm call, but when we roll any new versions of OAV
-        // out to azure/azure-rest-api-specs, we need the errors actually pop there! When enough of the RPs have resolved this problem,
-        // we can re-enable loud failures in the validation image.
-        //
-        // Model and Semantic validation both run with isArmCall set to TRUE, so by setting FALSE a user will activate this special skip logic.
-        if (issue.schemaPath.includes("additionalProperties")) {
-          skipIssue = true;
-          if (logging) {
-            logging(
-              `AdditionalProperties validation failed:${JSON.stringify(issue, undefined, 2)}`,
-              LiveValidatorLoggingLevels.error,
-              LiveValidatorLoggingTypes.trace,
-              "Oav.OperationValidator.schemaValidateIssueToLiveValidationIssue",
-              undefined,
-              operationContext.validationRequest
-            );
-          }
-          return "";
-        }
       }
 
       const isMissingRequiredProperty = issue.code === "OBJECT_MISSING_REQUIRED_PROPERTY";

package.json

@@ -1,6 +1,6 @@
 {
   "name": "oav",
-  "version": "3.3.7",
+  "version": "3.3.8",
   "author": {
     "name": "Microsoft Corporation",
     "email": "[email protected]",

test/liveValidatorTests.ts

@@ -1069,7 +1069,10 @@ describe("Live Validator", () => {
       await liveValidator.initialize();
       const payload = require(`${__dirname}/liveValidation/payloads/additionalProperties_invalid_mapType.json`);
       const result = await liveValidator.validateLiveRequestResponse(payload);
-      assert.strictEqual(result.responseValidationResult.isSuccessful, true);
+
+      assert.strictEqual(result.responseValidationResult.errors.length, 1);
+      assert.strictEqual(result.responseValidationResult.errors[0].code, "INVALID_TYPE")
+      assert.strictEqual(result.responseValidationResult.isSuccessful, false);
       assert.strictEqual(result.requestValidationResult.isSuccessful, true);
     });