From dcf46f6a0f3703d004041a3bd3f6fb30ad06732e Mon Sep 17 00:00:00 2001
From: Dino Chiesa <dchiesa@google.com>
Date: Mon, 4 Dec 2023 17:23:05 -0800
Subject: [PATCH] feat(PO033): add new checks for valid elements in PO033
 plugin

---
 .../plugins/PO033-extractVariables-hygiene.js | 29 +++++++++++++++++++
 ...URIPath-with-lowercase-pattern-element.xml |  6 ++++
 .../fail/EV-Variable-with-no-Pattern.xml      |  5 ++++
 .../fixtures/resources/PO033/fail/messages.js |  6 +++-
 .../pass/EV-URIPath-with-Pattern-element.xml  |  7 +++++
 ...033-extractVariables-JSON-Variable-type.js |  2 +-
 6 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 test/fixtures/resources/PO033/fail/EV-URIPath-with-lowercase-pattern-element.xml
 create mode 100644 test/fixtures/resources/PO033/fail/EV-Variable-with-no-Pattern.xml
 create mode 100644 test/fixtures/resources/PO033/pass/EV-URIPath-with-Pattern-element.xml

diff --git a/lib/package/plugins/PO033-extractVariables-hygiene.js b/lib/package/plugins/PO033-extractVariables-hygiene.js
index f19f2af7..b0545027 100644
--- 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
new file mode 100644
index 00000000..3b2722d7
--- /dev/null
+++ 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
new file mode 100644
index 00000000..7e3e275f
--- /dev/null
+++ 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
index 0b2b822d..4fab4397 100644
--- 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
new file mode 100644
index 00000000..6b3db02c
--- /dev/null
+++ 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
index fd901863..a3bc7b00 100644
--- 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) => {