Merge pull request #401 from DinoChiesa/more-ev-hygiene

feat(PO033): add new checks for valid elements in PO033 plugin
apigee · Dec 5, 2023 · 0bf2c26 · 0bf2c26
2 parents 696a085 + dcf46f6
commit 0bf2c26
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 2 deletions.
diff --git a/lib/package/plugins/PO033-extractVariables-hygiene.js b/lib/package/plugins/PO033-extractVariables-hygiene.js
@@ -83,6 +83,35 @@ const onPolicy = function (policy, cb) {
         }
       }
     });
+
+    ["Variable", "URIPath", "QueryParam", "Header", "FormParam"].forEach(
+      (elementName) => {
+        policy.select(`/ExtractVariables/${elementName}`).forEach((elt) => {
+          const children = xpath.select("*", elt);
+          if (!children || children.length == 0) {
+            foundIssue = true;
+            policy.addMessage({
+              plugin,
+              line: elt.lineNumber,
+              column: elt.columnNumber,
+              message: `There should be at least one Pattern element as child of ${elt.nodeName}.`
+            });
+          } else {
+            children.forEach((childElement) => {
+              if (childElement.nodeName != "Pattern") {
+                foundIssue = true;
+                policy.addMessage({
+                  plugin,
+                  line: childElement.lineNumber,
+                  column: childElement.columnNumber,
+                  message: `Unexpected element '${childElement.nodeName}' as child of ${elt.nodeName}.`
+                });
+              }
+            });
+          }
+        });
+      }
+    );
   }
   if (typeof cb == "function") {
     cb(null, foundIssue);

diff --git a/test/fixtures/resources/PO033/fail/EV-URIPath-with-lowercase-pattern-element.xml b/test/fixtures/resources/PO033/fail/EV-URIPath-with-lowercase-pattern-element.xml
@@ -0,0 +1,6 @@
+<ExtractVariables name="EV-URIPath-with-lowercase-pattern-element">
+  <URIPath>
+    <pattern ignoreCase="false">/accounts/{id}</pattern>
+  </URIPath>
+  <Source clearPayload="false">request</Source>
+</ExtractVariables>
diff --git a/test/fixtures/resources/PO033/fail/EV-Variable-with-no-Pattern.xml b/test/fixtures/resources/PO033/fail/EV-Variable-with-no-Pattern.xml
@@ -0,0 +1,5 @@
+<ExtractVariables name="EV-Variable-with-no-Pattern">
+  <Variable>
+  </Variable>
+  <Source clearPayload="false">request</Source>
+</ExtractVariables>
diff --git a/test/fixtures/resources/PO033/fail/messages.js b/test/fixtures/resources/PO033/fail/messages.js
@@ -9,5 +9,9 @@ module.exports = {
   "EV-Missing-Variable.xml":
     "JSONPayload element exists but there is no Variable element.",
   "EV-XML-bool.xml":
-    "XMLPayload/Variable/@type is (bool), must be one of boolean,double,float,integer,long,nodeset,string"
+    "XMLPayload/Variable/@type is (bool), must be one of boolean,double,float,integer,long,nodeset,string",
+  "EV-URIPath-with-lowercase-pattern-element.xml":
+    "Unexpected element 'pattern' as child of URIPath.",
+  "EV-Variable-with-no-Pattern.xml":
+    "There should be at least one Pattern element as child of Variable."
 };
diff --git a/test/fixtures/resources/PO033/pass/EV-URIPath-with-Pattern-element.xml b/test/fixtures/resources/PO033/pass/EV-URIPath-with-Pattern-element.xml
@@ -0,0 +1,7 @@
+<ExtractVariables name="EV-URIPath-with-Pattern-element">
+  <URIPath>
+    <Pattern ignoreCase="false">/accounts/{id}</Pattern>
+    <Pattern ignoreCase="false">/foob/{id}</Pattern>
+  </URIPath>
+  <Source clearPayload="false">request</Source>
+</ExtractVariables>
diff --git a/test/specs/PO033-extractVariables-JSON-Variable-type.js b/test/specs/PO033-extractVariables-JSON-Variable-type.js
@@ -62,7 +62,7 @@ describe(`${testID} - policy passes ExtractVariables hygiene check`, function ()
     .forEach(testOne);
 });
 
-describe(`${testID} - policy does not ExtractVariables hygiene check`, () => {
+describe(`${testID} - policy does not pass ExtractVariables hygiene check`, () => {
   const sourceDir = path.join(rootDir, "fail");
   const expectedErrorMessages = require(path.join(sourceDir, "messages.js"));
   const testOne = (shortFileName) => {