Merge pull request #17100 from artfulrobot/artfulrobot-lab-1917

Replace CaseType's own XML encoding function
civicrm · Apr 18, 2020 · 4327f20 · 4327f20
2 parents 816fa1d + a45cd5a
commit 4327f20
Show file tree

Hide file tree

Showing 11 changed files with 729 additions and 665 deletions.
diff --git a/CRM/Case/BAO/CaseType.php b/CRM/Case/BAO/CaseType.php
@@ -96,119 +96,156 @@ protected function assignTestValue($fieldName, &$fieldDef, $counter) {
    *   XML
    */
   public static function convertDefinitionToXML($name, $definition) {
-    $xmlFile = '<?xml version="1.0" encoding="utf-8" ?>' . "\n\n<CaseType>\n";
-    $xmlFile .= "<name>" . self::encodeXmlString($name) . "</name>\n";
+
+    $xw = new XMLWriter();
+    $xw->openMemory();
+    $xw->setIndent(TRUE);
+    $xw->setIndentString(' ');
+    $xw->startDocument("1.0", 'UTF-8');
+
+    $xw->startElement('CaseType');
+
+    $xw->startElement('name');
+    $xw->text($name);
+    $xw->fullEndElement();
 
     if (array_key_exists('forkable', $definition)) {
-      $xmlFile .= "<forkable>" . ((int) $definition['forkable']) . "</forkable>\n";
+      $xw->startElement('forkable');
+      $xw->text((int) $definition['forkable']);
+      $xw->fullEndElement();
     }
 
     if (isset($definition['activityTypes'])) {
-      $xmlFile .= "<ActivityTypes>\n";
+      $xw->startElement('ActivityTypes');
+
       foreach ($definition['activityTypes'] as $values) {
-        $xmlFile .= "<ActivityType>\n";
+        $xw->startElement('ActivityType');
         foreach ($values as $key => $value) {
-          $xmlFile .= "<{$key}>" . self::encodeXmlString($value) . "</{$key}>\n";
+          $xw->startElement($key);
+          $xw->text($value);
+          $xw->fullEndElement();
         }
-        $xmlFile .= "</ActivityType>\n";
+        // ActivityType
+        $xw->fullEndElement();
       }
-      $xmlFile .= "</ActivityTypes>\n";
+      // ActivityTypes
+      $xw->fullEndElement();
     }
 
     if (!empty($definition['statuses'])) {
-      $xmlFile .= "<Statuses>\n";
+      $xw->startElement('Statuses');
+
       foreach ($definition['statuses'] as $value) {
-        $xmlFile .= "<Status>$value</Status>\n";
+        $xw->startElement('Status');
+        $xw->text($value);
+        $xw->fullEndElement();
       }
-      $xmlFile .= "</Statuses>\n";
+      // Statuses
+      $xw->fullEndElement();
     }
 
     if (isset($definition['activitySets'])) {
-      $xmlFile .= "<ActivitySets>\n";
+
+      $xw->startElement('ActivitySets');
       foreach ($definition['activitySets'] as $k => $val) {
-        $xmlFile .= "<ActivitySet>\n";
+
+        $xw->startElement('ActivitySet');
         foreach ($val as $index => $setVal) {
           switch ($index) {
             case 'activityTypes':
               if (!empty($setVal)) {
-                $xmlFile .= "<ActivityTypes>\n";
+                $xw->startElement('ActivityTypes');
                 foreach ($setVal as $values) {
-                  $xmlFile .= "<ActivityType>\n";
+                  $xw->startElement('ActivityType');
                   foreach ($values as $key => $value) {
-                    $xmlFile .= "<{$key}>" . self::encodeXmlString($value) . "</{$key}>\n";
+                    // Some parameters here may be arrays of values.
+                    // Also, the tests expect an empty array to be represented as an empty value.
+                    $value = (array) $value;
+                    if (count($value) === 0) {
+                      // Create an empty value.
+                      $value[] = '';
+                    }
+
+                    foreach ($value as $val) {
+                      $xw->startElement($key);
+                      $xw->text($val);
+                      $xw->fullEndElement();
+                    }
                   }
-                  $xmlFile .= "</ActivityType>\n";
+                  // ActivityType
+                  $xw->fullEndElement();
                 }
-                $xmlFile .= "</ActivityTypes>\n";
+                // ActivityTypes
+                $xw->fullEndElement();
               }
               break;
 
             // passthrough
             case 'sequence':
             case 'timeline':
               if ($setVal) {
-                $xmlFile .= "<{$index}>true</{$index}>\n";
+                $xw->startElement($index);
+                $xw->text('true');
+                $xw->fullEndElement();
               }
               break;
 
             default:
-              $xmlFile .= "<{$index}>" . self::encodeXmlString($setVal) . "</{$index}>\n";
+              $xw->startElement($index);
+              $xw->text($setVal);
+              $xw->fullEndElement();
           }
         }
-
-        $xmlFile .= "</ActivitySet>\n";
+        // ActivitySet
+        $xw->fullEndElement();
       }
-
-      $xmlFile .= "</ActivitySets>\n";
+      // ActivitySets
+      $xw->fullEndElement();
     }
 
     if (isset($definition['caseRoles'])) {
-      $xmlFile .= "<CaseRoles>\n";
+      $xw->startElement('CaseRoles');
       foreach ($definition['caseRoles'] as $values) {
-        $xmlFile .= "<RelationshipType>\n";
+        $xw->startElement('RelationshipType');
         foreach ($values as $key => $value) {
-          $xmlFile .= "<{$key}>" . ($key == 'groups' ? implode(',', array_map(['\CRM_Case_BAO_CaseType', 'encodeXmlString'], (array) $value)) : self::encodeXmlString($value)) . "</{$key}>\n";
+          $xw->startElement($key);
+          if ($key == 'groups') {
+            $xw->text(implode(',', (array) $value));
+          }
+          else {
+            $xw->text($value);
+          }
+          // $key
+          $xw->fullEndElement();
         }
-        $xmlFile .= "</RelationshipType>\n";
+        // RelationshipType
+        $xw->fullEndElement();
       }
-      $xmlFile .= "</CaseRoles>\n";
+      // CaseRoles
+      $xw->fullEndElement();
     }
 
     if (array_key_exists('restrictActivityAsgmtToCmsUser', $definition)) {
-      $xmlFile .= "<RestrictActivityAsgmtToCmsUser>" . $definition['restrictActivityAsgmtToCmsUser'] . "</RestrictActivityAsgmtToCmsUser>\n";
+      $xw->startElement('RestrictActivityAsgmtToCmsUser');
+      $xw->text($definition['restrictActivityAsgmtToCmsUser']);
+      $xw->fullEndElement();
     }
-
     if (!empty($definition['activityAsgmtGrps'])) {
-      $xmlFile .= "<ActivityAsgmtGrps>\n";
+      $xw->startElement('ActivityAsgmtGrps');
       foreach ((array) $definition['activityAsgmtGrps'] as $value) {
-        $xmlFile .= "<Group>$value</Group>\n";
+        $xw->startElement('Group');
+        $xw->text($value);
+        $xw->fullEndElement();
       }
-      $xmlFile .= "</ActivityAsgmtGrps>\n";
+      // ActivityAsgmtGrps
+      $xw->fullEndElement();
     }
 
-    $xmlFile .= '</CaseType>';
-
-    return $xmlFile;
-  }
+    // CaseType
+    $xw->fullEndElement();
+    $xw->endDocument();
 
-  /**
-   * Ugh. This shouldn't exist. Use a real XML-encoder.
-   *
-   * Escape a string for use in XML.
-   *
-   * @param string $str
-   *   A string which should outputted to XML.
-   * @return string
-   * @deprecated
-   */
-  protected static function encodeXmlString($str) {
-    // PHP 5.4: return htmlspecialchars($str, ENT_XML1, 'UTF-8')
-    if (is_scalar($str)) {
-      return htmlspecialchars($str);
-    }
-    else {
-      return NULL;
-    }
+    return $xw->outputMemory();
   }
 
   /**

diff --git a/tests/phpunit/CRM/Case/BAO/CaseTypeTest.php b/tests/phpunit/CRM/Case/BAO/CaseTypeTest.php
@@ -28,6 +28,18 @@ public function definitionProvider() {
       'xml' => file_get_contents(__DIR__ . '/xml/empty-lists.xml'),
     ];
 
+    $fixtures['statuses'] = [
+      'json' => json_encode([
+        'activitySets' => [],
+        'activityTypes' => [],
+        'caseRoles' => [],
+        'statuses' => ['Ongoing', 'Completed', 'This & That'],
+        //'statuses' => ['Ongoing', 'Completed'],
+        'timelineActivityTypes' => [],
+      ]),
+      'xml' => file_get_contents(__DIR__ . '/xml/statuses.xml'),
+    ];
+
     $fixtures['one-item-in-each'] = [
       'json' => json_encode([
         'activityTypes' => [
@@ -177,6 +189,7 @@ public function definitionProvider() {
       'two-items-in-each',
       'forkable-0',
       'forkable-1',
+      'statuses',
     ] as $key) {
       $cases[] = [$key, $fixtures[$key]['json'], $fixtures[$key]['xml']];
     }

diff --git a/tests/phpunit/CRM/Case/BAO/xml/empty-defn.xml b/tests/phpunit/CRM/Case/BAO/xml/empty-defn.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="UTF-8"?>
 <CaseType>
   <name>Housing Support</name>
 </CaseType>
diff --git a/tests/phpunit/CRM/Case/BAO/xml/empty-lists.xml b/tests/phpunit/CRM/Case/BAO/xml/empty-lists.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="UTF-8"?>
 <CaseType>
   <name>Housing Support</name>
   <ActivityTypes></ActivityTypes>

diff --git a/tests/phpunit/CRM/Case/BAO/xml/empty-node-text.xml b/tests/phpunit/CRM/Case/BAO/xml/empty-node-text.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="UTF-8"?>
 <CaseType>
   <name>Housing Support</name>
   <ActivityTypes>

diff --git a/tests/phpunit/CRM/Case/BAO/xml/forkable-0.xml b/tests/phpunit/CRM/Case/BAO/xml/forkable-0.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="UTF-8"?>
 <CaseType>
   <name>Housing Support</name>
   <forkable>0</forkable>

diff --git a/tests/phpunit/CRM/Case/BAO/xml/forkable-1.xml b/tests/phpunit/CRM/Case/BAO/xml/forkable-1.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="UTF-8"?>
 <CaseType>
   <name>Housing Support</name>
   <forkable>1</forkable>

diff --git a/tests/phpunit/CRM/Case/BAO/xml/one-item-in-each.xml b/tests/phpunit/CRM/Case/BAO/xml/one-item-in-each.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="UTF-8"?>
 <CaseType>
   <name>Housing Support</name>
   <ActivityTypes>

diff --git a/tests/phpunit/CRM/Case/BAO/xml/statuses.xml b/tests/phpunit/CRM/Case/BAO/xml/statuses.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<CaseType>
+  <name>Housing Support</name>
+  <ActivityTypes></ActivityTypes>
+  <Statuses>
+    <Status>Ongoing</Status>
+    <Status>Completed</Status>
+    <Status>This &amp; That</Status>
+  </Statuses>
+  <ActivitySets></ActivitySets>
+  <CaseRoles></CaseRoles>
+</CaseType>
+
diff --git a/tests/phpunit/CRM/Case/BAO/xml/two-items-in-each.xml b/tests/phpunit/CRM/Case/BAO/xml/two-items-in-each.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="UTF-8"?>
 <CaseType>
   <name>Housing Support</name>
   <ActivityTypes>